Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: custom event integrations override support, refactor integration merge logic #379

Merged
merged 3 commits into from
Nov 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion example/e2e/main.e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ describe('#mainTest', () => {
expect(mockServerListener).toHaveBeenCalledTimes(1);

const request = mockServerListener.mock.calls[0][0];
const context = request.context;
const context = request.batch[0].context;

expect(request.batch).toHaveLength(1);
expect(context.app.name).toBe('Analytics');
Expand Down
12 changes: 3 additions & 9 deletions packages/core/src/__tests__/api.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {
Context,
EventType,
Integrations,
SegmentAPIIntegrations,
TrackEventType,
UserTraits,
} from '../types';
Expand Down Expand Up @@ -43,7 +43,7 @@ describe('#sendEvents', () => {
// Context and Integration exist on SegmentEvents but are transmitted separately to avoid duplication
const additionalEventProperties: {
context: Context;
integrations: Integrations;
integrations: SegmentAPIIntegrations;
} = {
context: await context.getContext({ name: 'Hello' }),
integrations: {
Expand All @@ -66,13 +66,7 @@ describe('#sendEvents', () => {
expect(fetch).toHaveBeenCalledWith('https://api.segment.io/v1/batch', {
method: 'POST',
body: JSON.stringify({
batch: [
{ ...serializedEventProperties, sentAt: '2001-01-01T00:00:00.000Z' },
],
context: { appName: 'Segment Example', traits: { name: 'Hello' } },
integrations: {
Firebase: false,
},
batch: [{ ...event, sentAt: '2001-01-01T00:00:00.000Z' }],
}),
headers: {
'Authorization': 'Basic U0VHTUVOVF9LRVk6',
Expand Down
110 changes: 50 additions & 60 deletions packages/core/src/analytics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,11 @@ import {
Store,
} from './store';
import { Timeline } from './timeline';
import type {
import {
Config,
GroupTraits,
JsonMap,
PluginType,
SegmentAPISettings,
SegmentEvent,
UserTraits,
Expand Down Expand Up @@ -106,13 +107,34 @@ export class SegmentClient {
return plugins;
}

settings() {
let settings: SegmentAPISettings | undefined;
/**
* Retrieves a copy of the settings
* @returns Configuration object for all plugins
*/
getSettings() {
const { system } = this.store.getState();
if (system.settings) {
settings = system.settings;
return { integrations: { ...system.settings?.integrations } };
}

/**
* Returns the plugins currently loaded in the timeline
* @param ofType Type of plugins, defaults to all
* @returns List of Plugin objects
*/
getPlugins(ofType?: PluginType): readonly Plugin[] {
const plugins = { ...this.timeline.plugins };
if (ofType !== undefined) {
return [...(plugins[ofType] ?? [])];
}
return settings;
return (
[
...this.getPlugins(PluginType.before),
...this.getPlugins(PluginType.enrichment),
...this.getPlugins(PluginType.utility),
...this.getPlugins(PluginType.destination),
...this.getPlugins(PluginType.after),
] ?? []
);
}

constructor({
Expand Down Expand Up @@ -159,7 +181,7 @@ export class SegmentClient {
);
}

async getSettings() {
async fetchSettings() {
await getSettings.bind(this)();
}

Expand Down Expand Up @@ -270,29 +292,34 @@ export class SegmentClient {
}

/**
Adds a new plugin to the currently loaded set.

- Parameter plugin: The plugin to be added.
- Returns: Returns the name of the supplied plugin.

*/
add({ plugin }: { plugin: Plugin }) {
* Adds a new plugin to the currently loaded set.
* @param {{ plugin: Plugin, settings?: SegmentAPISettings }} Plugin to be added. Settings are optional if you want to force a configuration instead of the Segment Cloud received one
*/
add({
plugin,
settings,
}: {
plugin: Plugin;
settings?: Plugin extends DestinationPlugin ? SegmentAPISettings : never;
}) {
// plugins can either be added immediately or
// can be cached and added later during the next state update
// this is to avoid adding plugins before network requests made as part of setup have resolved
if (settings !== undefined && plugin.type === PluginType.destination) {
this.store.dispatch(
this.actions.system.addDestination({
destination: {
key: (plugin as DestinationPlugin).key,
settings,
},
})
);
}

if (!this.isReady) {
this.pluginsToAdd.push(plugin);
} else {
this.addPlugin(plugin);
const isIntegration = this.isNonSegmentDestinationPlugin(plugin);
if (isIntegration) {
// need to maintain the list of integrations to inject into payload
this.store.dispatch(
this.actions.system.addIntegrations([
{ key: (plugin as DestinationPlugin).key },
])
);
}
}
}

Expand All @@ -301,33 +328,13 @@ export class SegmentClient {
this.timeline.add(plugin);
}

private isNonSegmentDestinationPlugin(plugin: Plugin) {
const isSegmentDestination =
Object.getPrototypeOf(plugin).constructor.name === 'SegmentDestination';
if (!isSegmentDestination) {
const destPlugin = plugin as DestinationPlugin;
if (destPlugin.key) {
return true;
}
}
return false;
}

/**
Removes and unloads plugins with a matching name from the system.

- Parameter pluginName: An plugin name.
*/
remove({ plugin }: { plugin: Plugin }) {
this.timeline.remove(plugin);
const isIntegration = this.isNonSegmentDestinationPlugin(plugin);
if (isIntegration) {
this.store.dispatch(
this.actions.system.removeIntegration({
key: (plugin as DestinationPlugin).key,
})
);
}
}

process(incomingEvent: SegmentEvent) {
Expand All @@ -348,27 +355,10 @@ export class SegmentClient {
this.addPlugin(plugin);
});

// filter to see if we need to register any
const destPlugins = this.pluginsToAdd.filter(
this.isNonSegmentDestinationPlugin
);

// now that they're all added, clear the cache
// this prevents this block running for every update
this.pluginsToAdd = [];

// if we do have destPlugins, bulk-register them with the system
// this isn't done as part of addPlugin to avoid dispatching an update as part of an update
// which can lead to an infinite loop
// this is safe to fire & forget here as we've cleared pluginsToAdd
if (destPlugins.length > 0) {
this.store.dispatch(
this.actions.system.addIntegrations(
(destPlugins as DestinationPlugin[]).map(({ key }) => ({ key }))
)
);
}

// finally set the flag which means plugins will be added + registered immediately in future
this.isReady = true;
} finally {
Expand Down
18 changes: 4 additions & 14 deletions packages/core/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,25 +9,15 @@ export const sendEvents = async ({
config: Config;
events: SegmentEvent[];
}) => {
const updatedEvents = events.map((event) => {
const updatedEvent = {
...event,
sentAt: new Date().toISOString(),
};

// Context and Integration exist on SegmentEvents but are transmitted separately to avoid duplication
delete updatedEvent.context;
delete updatedEvent.integrations;

return updatedEvent;
});
const updatedEvents = events.map((event) => ({
...event,
sentAt: new Date().toISOString(),
}));

await fetch(batchApi, {
method: 'POST',
body: JSON.stringify({
batch: updatedEvents,
context: events[0].context,
integrations: events[0].integrations,
}),
headers: {
'Authorization': `Basic ${Base64.encode(`${config.writeKey}:`)}`,
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/client.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const doClientSetup = async (client: SegmentClient) => {
await client.bootstrapStore();

// get destination settings
await client.getSettings();
await client.fetchSettings();

// flush any stored events
client.flush();
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,14 @@ const isAliasEvent = (event: SegmentEvent): event is AliasEventType =>
event.type === EventType.AliasEvent;

export const applyRawEventData = (event: SegmentEvent, store: Store) => {
const { system, userInfo } = store.getState();
const { userInfo } = store.getState();

return {
...event,
anonymousId: userInfo.anonymousId,
messageId: getUUID(),
timestamp: new Date().toISOString(),
integrations: system.integrations,
integrations: event.integrations ?? {},
userId: isAliasEvent(event) ? event.userId : userInfo.userId,
};
};
11 changes: 0 additions & 11 deletions packages/core/src/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,17 +138,6 @@ export class DestinationPlugin extends EventPlugin {
*/
remove(plugin: Plugin) {
this.timeline.remove(plugin);
const isSegmentDestination =
Object.getPrototypeOf(plugin).constructor.name === 'SegmentDestination';
if (!isSegmentDestination) {
const destPlugin = plugin as DestinationPlugin;
if (destPlugin.key) {
const { store, actions } = this.analytics!;
store.dispatch(
actions.system.addIntegrations([{ key: destPlugin.key }])
);
}
}
}

// find(pluginType: PluginType) {
Expand Down
53 changes: 25 additions & 28 deletions packages/core/src/plugins/SegmentDestination.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,11 @@
import { DestinationPlugin } from '../plugin';
import {
PluginType,
SegmentAPIIntegrations,
SegmentAPISettings,
SegmentEvent,
UpdateType,
} from '../types';
import type {
AliasEventType,
GroupEventType,
IdentifyEventType,
ScreenEventType,
TrackEventType,
} from '../types';
import { chunk } from '../util';
import { sendEvents } from '../api';

Expand All @@ -26,29 +20,32 @@ export class SegmentDestination extends DestinationPlugin {
// see flush() below
}

identify(event: IdentifyEventType) {
this.queueEvent(event);
return event;
}

track(event: TrackEventType) {
this.queueEvent(event);
return event;
}

screen(event: ScreenEventType) {
this.queueEvent(event);
return event;
}
execute(event: SegmentEvent): SegmentEvent {
const pluginSettings = this.analytics?.getSettings();
const plugins = this.analytics?.getPlugins(PluginType.destination);

alias(event: AliasEventType) {
this.queueEvent(event);
return event;
}
// Disable all destinations that have a device mode plugin
const deviceModePlugins =
plugins?.map((plugin) => (plugin as DestinationPlugin).key) ?? [];
const cloudSettings: SegmentAPIIntegrations = {
...pluginSettings?.integrations,
};
for (const key of deviceModePlugins) {
if (key in cloudSettings) {
cloudSettings[key] = false;
}
}

group(event: GroupEventType) {
this.queueEvent(event);
return event;
// User/event defined integrations override the cloud/device mode merge
const mergedEvent = {
...event,
integrations: {
...cloudSettings,
...event?.integrations,
},
};
this.queueEvent(mergedEvent);
return mergedEvent;
}

queueEvent(event: SegmentEvent) {
Expand Down
Loading