From 57a68c70e12cde1bce7e1ae1252d19c7bea9b5ca Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Wed, 13 Nov 2024 15:53:03 -0600 Subject: [PATCH 1/2] Scope to session --- .changeset/funny-beans-fly.md | 5 + .changeset/khaki-cheetahs-sit.md | 7 + .../src/core/buffer/__tests__/buffer.test.ts | 117 +++++++++++- .../signals/signals/src/core/buffer/index.ts | 176 +++++++++++++----- .../signals/src/core/signals/settings.ts | 2 + .../plugin/__tests__/signals-plugin.test.ts | 32 +++- .../signals/src/plugin/signals-plugin.ts | 1 + .../signals/signals/src/test-helpers/range.ts | 1 + .../signals/signals/src/types/settings.ts | 9 + 9 files changed, 292 insertions(+), 58 deletions(-) create mode 100644 .changeset/funny-beans-fly.md create mode 100644 .changeset/khaki-cheetahs-sit.md create mode 100644 packages/signals/signals/src/test-helpers/range.ts diff --git a/.changeset/funny-beans-fly.md b/.changeset/funny-beans-fly.md new file mode 100644 index 000000000..55d2f8319 --- /dev/null +++ b/.changeset/funny-beans-fly.md @@ -0,0 +1,5 @@ +--- +'@segment/analytics-signals': patch +--- + +Scope signal buffer to session diff --git a/.changeset/khaki-cheetahs-sit.md b/.changeset/khaki-cheetahs-sit.md new file mode 100644 index 000000000..17b8354ad --- /dev/null +++ b/.changeset/khaki-cheetahs-sit.md @@ -0,0 +1,7 @@ +--- +'@segment/analytics-signals': minor +--- + +* Clear signal buffer at start of new session +* Prune signalBuffer to maxBufferSize on new session (if different) +* Add sessionStorage storage type diff --git a/packages/signals/signals/src/core/buffer/__tests__/buffer.test.ts b/packages/signals/signals/src/core/buffer/__tests__/buffer.test.ts index 21e4741bd..8b639d79e 100644 --- a/packages/signals/signals/src/core/buffer/__tests__/buffer.test.ts +++ b/packages/signals/signals/src/core/buffer/__tests__/buffer.test.ts @@ -1,26 +1,123 @@ +import { sleep } from '@segment/analytics-core' +import { range } from '../../../test-helpers/range' import { createInteractionSignal } from '../../../types/factories' import { getSignalBuffer, SignalBuffer } from '../index' describe(getSignalBuffer, () => { let buffer: SignalBuffer beforeEach(async () => { + sessionStorage.clear() buffer = getSignalBuffer({ maxBufferSize: 10, }) await buffer.clear() }) + describe('indexDB', () => { + it('should instantiate without throwing an error', () => { + expect(buffer).toBeTruthy() + }) + it('should add and clear', async () => { + const mockSignal = createInteractionSignal({ + eventType: 'submit', + target: {}, + }) + await buffer.add(mockSignal) + await expect(buffer.getAll()).resolves.toEqual([mockSignal]) + await buffer.clear() + await expect(buffer.getAll()).resolves.toHaveLength(0) + }) + + it('should delete older signals when maxBufferSize is exceeded', async () => { + const signals = range(15).map((_, idx) => + createInteractionSignal({ + idx: idx, + eventType: 'change', + target: {}, + }) + ) + + for (const signal of signals) { + await buffer.add(signal) + } + + const storedSignals = await buffer.getAll() + expect(storedSignals).toHaveLength(10) + expect(storedSignals).toEqual(signals.slice(-10).reverse()) + }) + + it('should delete older signals on initialize if current number exceeds maxBufferSize', async () => { + const signals = range(15).map((_, idx) => + createInteractionSignal({ + idx: idx, + eventType: 'change', + target: {}, + }) + ) + + for (const signal of signals) { + await buffer.add(signal) + } + + // Re-initialize buffer + buffer = getSignalBuffer({ + maxBufferSize: 10, + }) + + const storedSignals = await buffer.getAll() + expect(storedSignals).toHaveLength(10) + expect(storedSignals).toEqual(signals.slice(-10).reverse()) + }) - it('should instantiate without throwing an error', () => { - expect(buffer).toBeTruthy() + it('should clear signal buffer if there is a new session according to session storage', async () => { + const mockSignal = createInteractionSignal({ + eventType: 'submit', + target: {}, + }) + await buffer.add(mockSignal) + await expect(buffer.getAll()).resolves.toEqual([mockSignal]) + + // Simulate a new session by clearing session storage and re-initializing the buffer + sessionStorage.clear() + await sleep(100) + buffer = getSignalBuffer({ + maxBufferSize: 10, + }) + + await expect(buffer.getAll()).resolves.toHaveLength(0) + }) }) - it('should add and clear', async () => { - const mockSignal = createInteractionSignal({ - eventType: 'submit', - target: {}, + describe('sessionStorage', () => { + it('should instantiate without throwing an error', () => { + expect(buffer).toBeTruthy() + }) + + it('should add and clear', async () => { + const mockSignal = createInteractionSignal({ + eventType: 'submit', + target: {}, + }) + await buffer.add(mockSignal) + await expect(buffer.getAll()).resolves.toEqual([mockSignal]) + await buffer.clear() + await expect(buffer.getAll()).resolves.toHaveLength(0) + }) + + it('should delete older signals when maxBufferSize is exceeded', async () => { + const signals = range(15).map((_, idx) => + createInteractionSignal({ + idx: idx, + eventType: 'change', + target: {}, + }) + ) + + for (const signal of signals) { + await buffer.add(signal) + } + + const storedSignals = await buffer.getAll() + expect(storedSignals).toHaveLength(10) + expect(storedSignals).toEqual(signals.slice(-10).reverse()) }) - await buffer.add(mockSignal) - await expect(buffer.getAll()).resolves.toEqual([mockSignal]) - await buffer.clear() - await expect(buffer.getAll()).resolves.toHaveLength(0) }) }) diff --git a/packages/signals/signals/src/core/buffer/index.ts b/packages/signals/signals/src/core/buffer/index.ts index 565d8ac5d..9ee40a548 100644 --- a/packages/signals/signals/src/core/buffer/index.ts +++ b/packages/signals/signals/src/core/buffer/index.ts @@ -1,6 +1,7 @@ import { Signal } from '@segment/analytics-signals-runtime' -import { openDB, DBSchema, IDBPDatabase } from 'idb' +import { openDB, DBSchema, IDBPDatabase, IDBPObjectStore } from 'idb' import { logger } from '../../lib/logger' +import { WebStorage } from '../../lib/storage/web-storage' interface SignalDatabase extends DBSchema { signals: { @@ -15,77 +16,147 @@ export interface SignalPersistentStorage { clear(): void } -export class SignalStore implements SignalPersistentStorage { +interface IDBPDatabaseSignals extends IDBPDatabase {} +interface IDBPObjectStoreSignals + extends IDBPObjectStore< + SignalDatabase, + ['signals'], + 'signals', + 'readonly' | 'readwrite' | 'versionchange' + > {} + +interface StoreSettings { + maxBufferSize: number +} +export class SignalStoreIndexDB implements SignalPersistentStorage { static readonly DB_NAME = 'Segment Signals Buffer' static readonly STORE_NAME = 'signals' - private signalStore: Promise> - private signalCount = 0 + private db: Promise private maxBufferSize: number - - public length() { - return this.signalCount - } - + private sessionKeyStorage = new WebStorage(window.sessionStorage) static deleteDatabase() { - return indexedDB.deleteDatabase(SignalStore.DB_NAME) + return indexedDB.deleteDatabase(SignalStoreIndexDB.DB_NAME) } - constructor(settings: { maxBufferSize?: number } = {}) { - this.maxBufferSize = settings.maxBufferSize ?? 50 - this.signalStore = this.createSignalStore() - void this.initializeSignalCount() + async getStore( + permission: IDBTransactionMode, + database?: IDBPDatabaseSignals + ): Promise { + const db = database ?? (await this.db) + const store = db + .transaction(SignalStoreIndexDB.STORE_NAME, permission) + .objectStore(SignalStoreIndexDB.STORE_NAME) + return store } - private getStore() { - return this.signalStore + constructor(settings: StoreSettings) { + this.maxBufferSize = settings.maxBufferSize + this.db = this.initSignalDB() } - private async createSignalStore() { - const db = await openDB(SignalStore.DB_NAME, 1, { + private async initSignalDB(): Promise { + const db = await openDB(SignalStoreIndexDB.DB_NAME, 1, { upgrade(db) { - db.createObjectStore(SignalStore.STORE_NAME, { autoIncrement: true }) + db.createObjectStore(SignalStoreIndexDB.STORE_NAME, { + autoIncrement: true, + }) }, }) logger.debug('Signals Buffer (indexDB) initialized') + // if the signal buffer is too large, delete the oldest signals (e.g, the settings have changed) + const store = await this.getStore('readwrite', db) + await this.clearStoreIfNeeded(store) + await this.countAndDeleteOldestIfNeeded(store, true) + await store.transaction.done return db } - private async initializeSignalCount() { - const store = await this.signalStore - this.signalCount = await store.count(SignalStore.STORE_NAME) - logger.debug( - `Signal count initialized with ${this.signalCount} signals (max: ${this.maxBufferSize})` - ) + private async clearStoreIfNeeded(store: IDBPObjectStoreSignals) { + // prevent the signals buffer from persisting across sessions (e.g, user closes tab and reopens) + const sessionKey = 'segment_signals_db_session_key' + if (!sessionStorage.getItem(sessionKey)) { + this.sessionKeyStorage.setItem(sessionKey, true) + await store.clear!() + logger.debug('New Session, so signals buffer cleared') + } } async add(signal: Signal): Promise { - const store = await this.signalStore - if (this.signalCount >= this.maxBufferSize) { - // Get the key of the oldest signal and delete it - const oldestKey = await store - .transaction(SignalStore.STORE_NAME) - .store.getKey(IDBKeyRange.lowerBound(0)) - if (oldestKey !== undefined) { - await store.delete(SignalStore.STORE_NAME, oldestKey) - } else { - this.signalCount-- + const store = await this.getStore('readwrite') + await store.add!(signal) + await this.countAndDeleteOldestIfNeeded(store) + return store.transaction.done + } + + private async countAndDeleteOldestIfNeeded( + store: IDBPObjectStoreSignals, + deleteMultiple = false + ): Promise { + let count = await store.count() + if (count > this.maxBufferSize) { + const cursor = await store.openCursor() + if (cursor) { + // delete up to maxItems + if (deleteMultiple) { + while (count > this.maxBufferSize) { + await cursor.delete!() + await cursor.continue() + count-- + } + logger.debug( + `Signals Buffer: Purged signals to max buffer size of ${this.maxBufferSize}` + ) + } else { + // just delete the oldest item + await cursor.delete!() + count-- + } } } - await store.add(SignalStore.STORE_NAME, signal) - this.signalCount++ } /** * Get list of signals from the store, with the newest signals first. */ async getAll(): Promise { - const store = await this.getStore() - return (await store.getAll(SignalStore.STORE_NAME)).reverse() + const store = await this.getStore('readonly') + const signals = await store.getAll() + await store.transaction.done + return signals.reverse() } - async clear() { - const store = await this.getStore() - return store.clear(SignalStore.STORE_NAME) + async clear(): Promise { + const store = await this.getStore('readwrite') + await store.clear!() + await store.transaction.done + } +} + +export class SignalStoreSessionStorage implements SignalPersistentStorage { + private readonly storageKey = 'segment_signals_buffer' + private maxBufferSize: number + + constructor(settings: StoreSettings) { + this.maxBufferSize = settings.maxBufferSize + } + + add(signal: Signal): void { + const signals = this.getAll() + signals.unshift(signal) + if (signals.length > this.maxBufferSize) { + // delete the last one + signals.splice(-1) + } + sessionStorage.setItem(this.storageKey, JSON.stringify(signals)) + } + + clear(): void { + sessionStorage.removeItem(this.storageKey) + } + + getAll(): Signal[] { + const signals = sessionStorage.getItem(this.storageKey) + return signals ? JSON.parse(signals) : [] } } @@ -125,7 +196,19 @@ export class SignalBuffer< export interface SignalBufferSettingsConfig< T extends SignalPersistentStorage = SignalPersistentStorage > { + /** + * Maximum number of signals to store. Only applies if no custom storage implementation is provided. + */ maxBufferSize?: number + /** + * Choose between sessionStorage and indexDB. Only applies if no custom storage implementation is provided. + * @default 'indexDB' + */ + storageType?: 'session' | 'indexDB' + /** + * Custom storage implementation + * @default SignalStoreIndexDB + */ signalStorage?: T } export const getSignalBuffer = < @@ -133,6 +216,13 @@ export const getSignalBuffer = < >( settings: SignalBufferSettingsConfig ) => { - const store = settings.signalStorage ?? new SignalStore(settings) + const settingsWithDefaults: StoreSettings = { + maxBufferSize: 50, + ...settings, + } + const store = + settings.signalStorage ?? settings.storageType === 'session' + ? new SignalStoreSessionStorage(settingsWithDefaults) + : new SignalStoreIndexDB(settingsWithDefaults) return new SignalBuffer(store) } diff --git a/packages/signals/signals/src/core/signals/settings.ts b/packages/signals/signals/src/core/signals/settings.ts index 042515933..da9f31833 100644 --- a/packages/signals/signals/src/core/signals/settings.ts +++ b/packages/signals/signals/src/core/signals/settings.ts @@ -19,6 +19,7 @@ export type SignalsSettingsConfig = Pick< | 'networkSignalsAllowList' | 'networkSignalsDisallowList' | 'networkSignalsAllowSameDomain' + | 'signalStorageType' > & { signalStorage?: SignalPersistentStorage processSignal?: string @@ -52,6 +53,7 @@ export class SignalGlobalSettings { this.signalBuffer = { signalStorage: settings.signalStorage, + storageType: settings.signalStorageType, maxBufferSize: settings.maxBufferSize, } this.ingestClient = { diff --git a/packages/signals/signals/src/plugin/__tests__/signals-plugin.test.ts b/packages/signals/signals/src/plugin/__tests__/signals-plugin.test.ts index 7afb4b728..5c700843b 100644 --- a/packages/signals/signals/src/plugin/__tests__/signals-plugin.test.ts +++ b/packages/signals/signals/src/plugin/__tests__/signals-plugin.test.ts @@ -1,16 +1,37 @@ import { SignalsPlugin } from '../signals-plugin' +// this specific test was throwing a bunch of warnings: +// 'Cannot read properties of null (reading '_origin') at Window.get sessionStorage [as sessionStorage]' +// no idea why, as sessionStorage works as usual in other tests. +const sessionStorageMock = (() => { + let store: Record = {} + return { + // @ts-ignore + getItem: (key: string) => store[key] || null, + setItem: (key: string, value: unknown) => { + // @ts-ignore + store[key] = value.toString() + }, + removeItem: (key: string) => { + // @ts-ignore + delete store[key] + }, + clear: () => { + store = {} + }, + } +})() + +Object.defineProperty(window, 'sessionStorage', { + value: sessionStorageMock, +}) /** * This should be tested at the integration level * A few tests, just for example purposes. */ describe(SignalsPlugin, () => { - let plugin: SignalsPlugin - beforeEach(() => { - plugin = new SignalsPlugin() - }) - test('onSignal method registers callback', () => { + const plugin = new SignalsPlugin() const callback = jest.fn() const emitterSpy = jest.spyOn(plugin.signals.signalEmitter, 'subscribe') plugin.onSignal(callback) @@ -19,6 +40,7 @@ describe(SignalsPlugin, () => { }) test('addSignal method emits signal', () => { + const plugin = new SignalsPlugin() const signal = { data: 'test' } as any const emitterSpy = jest.spyOn(plugin.signals.signalEmitter, 'emit') plugin.addSignal(signal) diff --git a/packages/signals/signals/src/plugin/signals-plugin.ts b/packages/signals/signals/src/plugin/signals-plugin.ts index cd9b2e133..927d18f83 100644 --- a/packages/signals/signals/src/plugin/signals-plugin.ts +++ b/packages/signals/signals/src/plugin/signals-plugin.ts @@ -55,6 +55,7 @@ export class SignalsPlugin implements Plugin, SignalsAugmentedFunctionality { networkSignalsAllowList: settings.networkSignalsAllowList, networkSignalsDisallowList: settings.networkSignalsDisallowList, signalStorage: settings.signalStorage, + signalStorageType: settings.signalStorageType, }) } diff --git a/packages/signals/signals/src/test-helpers/range.ts b/packages/signals/signals/src/test-helpers/range.ts new file mode 100644 index 000000000..bfd1290ab --- /dev/null +++ b/packages/signals/signals/src/test-helpers/range.ts @@ -0,0 +1 @@ +export const range = (n: number) => Array.from({ length: n }, (_, i) => i) diff --git a/packages/signals/signals/src/types/settings.ts b/packages/signals/signals/src/types/settings.ts index 3e20d02da..2ecc95508 100644 --- a/packages/signals/signals/src/types/settings.ts +++ b/packages/signals/signals/src/types/settings.ts @@ -72,6 +72,15 @@ export interface SignalsPluginSettingsConfig { * Custom signal storage implementation */ signalStorage?: SignalStorage | undefined + + /** + * Choose between sessionStorage and indexDB. + * IndexDB is more performant and has no size requirements, but has the security caveat that signals can be retained across sessions (cleared on new session, but still technically accessible), and only cleared at the beginning of a new session. + * SessionStorage is cleared on tab close. + * + * @default 'indexDB' + */ + signalStorageType?: 'session' | 'indexDB' | undefined } export type RegexLike = RegExp | string From c70e737629212da4df592d07a23a6a764c697f4f Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Thu, 14 Nov 2024 15:38:38 -0600 Subject: [PATCH 2/2] Delete .changeset/funny-beans-fly.md --- .changeset/funny-beans-fly.md | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 .changeset/funny-beans-fly.md diff --git a/.changeset/funny-beans-fly.md b/.changeset/funny-beans-fly.md deleted file mode 100644 index 55d2f8319..000000000 --- a/.changeset/funny-beans-fly.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@segment/analytics-signals': patch ---- - -Scope signal buffer to session