Skip to content

Commit

Permalink
Refactor event factory, remove spark-md5 lib, fix hash collisions on …
Browse files Browse the repository at this point in the history
…messageId (#1045)

Co-authored-by: Andrew Loyola <andrewloyola@users.noreply.github.com>
  • Loading branch information
silesky and andrewloyola authored Apr 24, 2024
1 parent 3658811 commit 3c37def
Show file tree
Hide file tree
Showing 22 changed files with 326 additions and 599 deletions.
8 changes: 8 additions & 0 deletions .changeset/clean-trains-perform.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@segment/analytics-next': minor
---
- Remove validation plugin
- Remove `spark-md5` dependency
- Update messageId algorithm to be consistent with node (analytics-next-[epoch time]-[uuid])
- Browser Validation:
- Throws errors in the EventFactory (not just in a plugin) if the event is invalid
4 changes: 4 additions & 0 deletions .changeset/nasty-bulldogs-unite.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
'@segment/analytics-core': patch
---
Share `EventFactory` between node and browser.
2 changes: 0 additions & 2 deletions packages/browser/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@
"dset": "^3.1.2",
"js-cookie": "3.0.1",
"node-fetch": "^2.6.7",
"spark-md5": "^3.0.1",
"tslib": "^2.4.1",
"unfetch": "^4.1.0"
},
Expand All @@ -76,7 +75,6 @@
"@types/mime": "^2.0.3",
"@types/node": "^16",
"@types/serve-handler": "^6.1.0",
"@types/spark-md5": "^3.0.2",
"aws-sdk": "^2.814.0",
"circular-dependency-plugin": "^5.2.2",
"compression-webpack-plugin": "^8.0.1",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ describe('Inspector', () => {

await deliveryPromise

expect(enrichedFn).toHaveBeenCalledTimes(2) // will be called once for every before or enrichment plugin.
expect(enrichedFn).toHaveBeenCalledTimes(1) // will be called once for every before or enrichment plugin.
expect(deliveredFn).toHaveBeenCalledTimes(1)
})

Expand Down
1 change: 0 additions & 1 deletion packages/browser/src/browser/__tests__/integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,6 @@ describe('Dispatch', () => {
"plugin_time",
"plugin_time",
"plugin_time",
"plugin_time",
"message_delivered",
"delivered",
]
Expand Down
2 changes: 0 additions & 2 deletions packages/browser/src/browser/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import {
} from '../plugins/remote-loader'
import type { RoutingRule } from '../plugins/routing-middleware'
import { segmentio, SegmentioSettings } from '../plugins/segmentio'
import { validation } from '../plugins/validation'
import {
AnalyticsBuffered,
PreInitMethodCallBuffer,
Expand Down Expand Up @@ -255,7 +254,6 @@ async function registerPlugins(
).catch(() => [])

const toRegister = [
validation,
envEnrichment,
...plugins,
...legacyDestinations,
Expand Down
2 changes: 1 addition & 1 deletion packages/browser/src/core/analytics/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,7 @@ export class Analytics

normalize(msg: SegmentEvent): SegmentEvent {
console.warn(deprecationWarning)
return this.eventFactory.normalize(msg)
return this.eventFactory['normalize'](msg)
}

get failedInitializations(): string[] {
Expand Down
46 changes: 19 additions & 27 deletions packages/browser/src/core/events/__tests__/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { range, uniq } from 'lodash'
import { EventFactory } from '..'
import { getDefaultPageContext } from '../../page'
import { User } from '../../user'
import { SegmentEvent, Options } from '../interfaces'

describe('Event Factory', () => {
let user: User
Expand Down Expand Up @@ -59,6 +58,25 @@ describe('Event Factory', () => {
const group = factory.group('coolKidsId', { coolkids: true })
expect(group.groupId).toEqual('coolKidsId')
})

it('allows userId / anonymousId to be specified as an option', () => {
const group = factory.group('my_group_id', undefined, {
userId: 'bar',
anonymousId: 'foo',
})
expect(group.userId).toBe('bar')
expect(group.anonymousId).toBe('foo')
})

it('uses userId / anonymousId from the user class (if specified)', function () {
const user = new User()
user.anonymousId = () => '123'
user.id = () => 'abc'
factory = new EventFactory(user)
const group = factory.group('my_group_id')
expect(group.userId).toBe('abc')
expect(group.anonymousId).toBe('123')
})
})

describe('page', () => {
Expand Down Expand Up @@ -394,32 +412,6 @@ describe('Event Factory', () => {
})
})

describe('normalize', function () {
const msg: SegmentEvent = { type: 'track' }
const opts: Options = (msg.options = {})

describe('message', function () {
it('should merge original with normalized', function () {
msg.userId = 'user-id'
opts.integrations = { Segment: true }
const normalized = factory['normalize'](msg)

expect(normalized.messageId?.length).toBeGreaterThanOrEqual(41) // 'ajs-next-md5(content + [UUID])'
delete normalized.messageId

expect(normalized.timestamp).toBeInstanceOf(Date)
delete normalized.timestamp

expect(normalized).toStrictEqual({
integrations: { Segment: true },
type: 'track',
userId: 'user-id',
context: defaultContext,
})
})
})
})

describe('Page context augmentation', () => {
// minimal tests -- more tests should be specifically around addPageContext
factory = new EventFactory(new User())
Expand Down
Loading

0 comments on commit 3c37def

Please sign in to comment.