diff --git a/.changeset/smooth-seahorses-unite.md b/.changeset/smooth-seahorses-unite.md index ae8fa2dc9..ef526d79c 100644 --- a/.changeset/smooth-seahorses-unite.md +++ b/.changeset/smooth-seahorses-unite.md @@ -2,4 +2,4 @@ '@segment/analytics-next': minor --- -Adds storage option in analytics client to specify priority of storage (e.g use cookies over localstorage) or use a custom implementation +Adds storage option in analytics client to specify priority of storage (e.g use cookies over localstorage) diff --git a/packages/browser/src/core/analytics/__tests__/integration.test.ts b/packages/browser/src/core/analytics/__tests__/integration.test.ts index eb1c6beda..73337107d 100644 --- a/packages/browser/src/core/analytics/__tests__/integration.test.ts +++ b/packages/browser/src/core/analytics/__tests__/integration.test.ts @@ -288,7 +288,13 @@ describe('Analytics', () => { const analytics = new Analytics( { writeKey: '' }, { - storage: [StoreType.Cookie, StoreType.LocalStorage, StoreType.Memory], + storage: { + stores: [ + StoreType.Cookie, + StoreType.LocalStorage, + StoreType.Memory, + ], + }, } ) @@ -308,7 +314,9 @@ describe('Analytics', () => { const analytics = new Analytics( { writeKey: '' }, { - storage: [StoreType.Cookie, StoreType.Memory], + storage: { + stores: [StoreType.Cookie, StoreType.Memory], + }, } ) diff --git a/packages/browser/src/core/analytics/index.ts b/packages/browser/src/core/analytics/index.ts index 22ea0d97a..ef88ed131 100644 --- a/packages/browser/src/core/analytics/index.ts +++ b/packages/browser/src/core/analytics/index.ts @@ -46,13 +46,11 @@ import { CookieOptions, MemoryStorage, UniversalStorage, - Storage, StorageSettings, StoreType, applyCookieOptions, initializeStorages, isArrayOfStoreType, - isStorageObject, } from '../storage' const deprecationWarning = @@ -138,7 +136,7 @@ export class Analytics private _group: Group private eventFactory: EventFactory private _debug = false - private _universalStorage: Storage + private _universalStorage: UniversalStorage initialized = false integrations: Integrations @@ -213,22 +211,19 @@ export class Analytics disablePersistance: boolean, storageSetting: InitOptions['storage'], cookieOptions?: CookieOptions | undefined - ): Storage { + ): UniversalStorage { // DisablePersistance option overrides all, no storage will be used outside of memory even if specified if (disablePersistance) { - return new MemoryStorage() + return new UniversalStorage([new MemoryStorage()]) } else { if (storageSetting) { if (isArrayOfStoreType(storageSetting)) { // We will create the store with the priority for customer settings return new UniversalStorage( initializeStorages( - applyCookieOptions(storageSetting, cookieOptions) + applyCookieOptions(storageSetting.stores, cookieOptions) ) ) - } else if (isStorageObject(storageSetting)) { - // If it is an object we will use the customer provided storage - return storageSetting } } } @@ -245,7 +240,7 @@ export class Analytics ) } - get storage(): Storage { + get storage(): UniversalStorage { return this._universalStorage } diff --git a/packages/browser/src/core/storage/__tests__/cookieStorage.test.ts b/packages/browser/src/core/storage/__tests__/cookieStorage.test.ts index da02beccc..3ac9dd34f 100644 --- a/packages/browser/src/core/storage/__tests__/cookieStorage.test.ts +++ b/packages/browser/src/core/storage/__tests__/cookieStorage.test.ts @@ -1,6 +1,5 @@ import { CookieStorage } from '../cookieStorage' import jar from 'js-cookie' -import { disableCookies } from './test-helpers' describe('cookieStorage', () => { function clearCookies() { @@ -15,23 +14,6 @@ describe('cookieStorage', () => { clearCookies() }) - describe('#available', () => { - afterEach(() => { - jest.restoreAllMocks() - }) - - it('is available', () => { - const cookie = new CookieStorage() - expect(cookie.available).toBe(true) - }) - - it("is unavailable if can't write cookies", () => { - disableCookies() - const cookie = new CookieStorage() - expect(cookie.available).toBe(false) - }) - }) - describe('cookie options', () => { it('should have default cookie options', () => { const cookie = new CookieStorage() diff --git a/packages/browser/src/core/storage/__tests__/localStorage.test.ts b/packages/browser/src/core/storage/__tests__/localStorage.test.ts index 30c73a121..09c3b9f2d 100644 --- a/packages/browser/src/core/storage/__tests__/localStorage.test.ts +++ b/packages/browser/src/core/storage/__tests__/localStorage.test.ts @@ -63,7 +63,7 @@ describe('LocalStorage', function () { it('should be able to remove a record', function () { store.set('x', { a: 'b' }) expect(store.get('x')).toStrictEqual({ a: 'b' }) - store.clear('x') + store.remove('x') expect(store.get('x')).toBe(null) }) }) diff --git a/packages/browser/src/core/storage/__tests__/test-helpers.ts b/packages/browser/src/core/storage/__tests__/test-helpers.ts index 7bd5af18b..20faf069a 100644 --- a/packages/browser/src/core/storage/__tests__/test-helpers.ts +++ b/packages/browser/src/core/storage/__tests__/test-helpers.ts @@ -1,19 +1,27 @@ +import jar from 'js-cookie' /** * Disables Cookies * @returns jest spy */ -export function disableCookies(): jest.SpyInstance { - return jest - .spyOn(window.navigator, 'cookieEnabled', 'get') - .mockReturnValue(false) +export function disableCookies(): void { + jest.spyOn(window.navigator, 'cookieEnabled', 'get').mockReturnValue(false) + jest.spyOn(jar, 'set').mockImplementation(() => { + throw new Error() + }) + jest.spyOn(jar, 'get').mockImplementation(() => { + throw new Error() + }) } /** * Disables LocalStorage * @returns jest spy */ -export function disableLocalStorage(): jest.SpyInstance { - return jest.spyOn(Storage.prototype, 'setItem').mockImplementation(() => { +export function disableLocalStorage(): void { + jest.spyOn(Storage.prototype, 'setItem').mockImplementation(() => { + throw new Error() + }) + jest.spyOn(Storage.prototype, 'getItem').mockImplementation(() => { throw new Error() }) } diff --git a/packages/browser/src/core/storage/__tests__/universalStorage.test.ts b/packages/browser/src/core/storage/__tests__/universalStorage.test.ts index e147cb5a7..93bdaffc9 100644 --- a/packages/browser/src/core/storage/__tests__/universalStorage.test.ts +++ b/packages/browser/src/core/storage/__tests__/universalStorage.test.ts @@ -103,21 +103,20 @@ describe('UniversalStorage', function () { expect(us.get('ajs_test_key')).toEqual('💰') }) - it('does not write to cookies when cookies are not available', function () { - disableCookies() + it('handles cookie errors gracefully', function () { + disableCookies() // Cookies is going to throw exceptions now const us = new UniversalStorage([ new LocalStorage(), new CookieStorage(), new MemoryStorage(), ]) us.set('ajs_test_key', '💰') - expect(jar.get('ajs_test_key')).toEqual(undefined) expect(getFromLS('ajs_test_key')).toEqual('💰') expect(us.get('ajs_test_key')).toEqual('💰') }) it('does not write to LS when LS is not available', function () { - disableLocalStorage() + disableLocalStorage() // Localstorage will throw exceptions const us = new UniversalStorage([ new LocalStorage(), new CookieStorage(), @@ -125,7 +124,6 @@ describe('UniversalStorage', function () { ]) us.set('ajs_test_key', '💰') expect(jar.get('ajs_test_key')).toEqual('💰') - expect(localStorage.getItem('ajs_test_key')).toEqual(null) expect(us.get('ajs_test_key')).toEqual('💰') }) }) diff --git a/packages/browser/src/core/storage/cookieStorage.ts b/packages/browser/src/core/storage/cookieStorage.ts index 02f5eb50a..9450d55d9 100644 --- a/packages/browser/src/core/storage/cookieStorage.ts +++ b/packages/browser/src/core/storage/cookieStorage.ts @@ -1,4 +1,4 @@ -import { BaseStorage, StorageObject, StoreType } from './types' +import { Store, StorageObject } from './types' import jar from 'js-cookie' import { tld } from '../user/tld' @@ -15,25 +15,9 @@ export interface CookieOptions { /** * Data storage using browser cookies */ -export class CookieStorage< - Data extends StorageObject = StorageObject -> extends BaseStorage { - get available(): boolean { - let cookieEnabled = window.navigator.cookieEnabled - - if (!cookieEnabled) { - jar.set('ajs:cookies', 'test') - cookieEnabled = document.cookie.includes('ajs:cookies') - jar.remove('ajs:cookies') - } - - return cookieEnabled - } - - get type() { - return StoreType.Cookie - } - +export class CookieStorage + implements Store +{ static get defaults(): CookieOptions { return { maxage: ONE_YEAR, @@ -46,7 +30,6 @@ export class CookieStorage< private options: Required constructor(options: CookieOptions = CookieStorage.defaults) { - super() this.options = { ...CookieStorage.defaults, ...options, @@ -91,7 +74,7 @@ export class CookieStorage< } } - clear(key: K): void { + remove(key: K): void { return jar.remove(key, this.opts()) } } diff --git a/packages/browser/src/core/storage/index.ts b/packages/browser/src/core/storage/index.ts index 5793795b0..aeb9a3b84 100644 --- a/packages/browser/src/core/storage/index.ts +++ b/packages/browser/src/core/storage/index.ts @@ -2,7 +2,7 @@ import { CookieOptions, CookieStorage } from './cookieStorage' import { LocalStorage } from './localStorage' import { MemoryStorage } from './memoryStorage' import { isStoreTypeWithSettings } from './settings' -import { StoreType, Storage, InitializeStorageArgs } from './types' +import { StoreType, Store, InitializeStorageArgs } from './types' export * from './types' export * from './localStorage' @@ -16,7 +16,7 @@ export * from './settings' * @param args StoreType and options * @returns Storage array */ -export function initializeStorages(args: InitializeStorageArgs): Storage[] { +export function initializeStorages(args: InitializeStorageArgs): Store[] { const storages = args.map((s) => { let type: StoreType let settings diff --git a/packages/browser/src/core/storage/localStorage.ts b/packages/browser/src/core/storage/localStorage.ts index f5385f965..9eb369d77 100644 --- a/packages/browser/src/core/storage/localStorage.ts +++ b/packages/browser/src/core/storage/localStorage.ts @@ -1,30 +1,15 @@ -import { BaseStorage, StorageObject, StoreType } from './types' +import { StorageObject, Store } from './types' /** * Data storage using browser's localStorage */ -export class LocalStorage< - Data extends StorageObject = StorageObject -> extends BaseStorage { +export class LocalStorage + implements Store +{ private localStorageWarning(key: keyof Data, state: 'full' | 'unavailable') { console.warn(`Unable to access ${key}, localStorage may be ${state}`) } - get type() { - return StoreType.LocalStorage - } - - get available(): boolean { - const test = 'test' - try { - localStorage.setItem(test, test) - localStorage.removeItem(test) - return true - } catch (e) { - return false - } - } - get(key: K): Data[K] | null { try { const val = localStorage.getItem(key) @@ -50,7 +35,7 @@ export class LocalStorage< } } - clear(key: K): void { + remove(key: K): void { try { return localStorage.removeItem(key) } catch (err) { diff --git a/packages/browser/src/core/storage/memoryStorage.ts b/packages/browser/src/core/storage/memoryStorage.ts index fa9a3d580..7840e7ba7 100644 --- a/packages/browser/src/core/storage/memoryStorage.ts +++ b/packages/browser/src/core/storage/memoryStorage.ts @@ -1,21 +1,13 @@ -import { BaseStorage, StorageObject, StoreType } from './types' +import { Store, StorageObject } from './types' /** * Data Storage using in memory object */ -export class MemoryStorage< - Data extends StorageObject = StorageObject -> extends BaseStorage { +export class MemoryStorage + implements Store +{ private cache: Record = {} - get type() { - return StoreType.Memory - } - - get available(): boolean { - return true - } - get(key: K): Data[K] | null { return (this.cache[key] ?? null) as Data[K] | null } @@ -24,7 +16,7 @@ export class MemoryStorage< this.cache[key] = value } - clear(key: K): void { + remove(key: K): void { delete this.cache[key] } } diff --git a/packages/browser/src/core/storage/settings.ts b/packages/browser/src/core/storage/settings.ts index 1503bcb1c..cc11a58fb 100644 --- a/packages/browser/src/core/storage/settings.ts +++ b/packages/browser/src/core/storage/settings.ts @@ -1,19 +1,21 @@ -import { Storage, StoreType, StoreTypeWithSettings } from './types' +import { StoreType, StoreTypeWithSettings } from './types' -export type StorageSettings = Storage | StoreType[] +export type UniversalStorageSettings = { stores: StoreType[] } -export function isArrayOfStoreType(s: StorageSettings): s is StoreType[] { +// This is setup this way to permit eventually a different set of settings for custom storage +export type StorageSettings = UniversalStorageSettings + +export function isArrayOfStoreType( + s: StorageSettings +): s is UniversalStorageSettings { return ( s && - Array.isArray(s) && - s.every((e) => Object.values(StoreType).includes(e)) + s.stores && + Array.isArray(s.stores) && + s.stores.every((e) => Object.values(StoreType).includes(e)) ) } -export function isStorageObject(s: StorageSettings): s is Storage { - return s && !Array.isArray(s) && typeof s === 'object' && s.get !== undefined -} - export function isStoreTypeWithSettings( s: StoreTypeWithSettings | StoreType ): s is StoreTypeWithSettings { diff --git a/packages/browser/src/core/storage/types.ts b/packages/browser/src/core/storage/types.ts index eaa0c192f..c2943ebe8 100644 --- a/packages/browser/src/core/storage/types.ts +++ b/packages/browser/src/core/storage/types.ts @@ -16,36 +16,14 @@ export type StorageObject = Record /** * Defines a Storage object for use in AJS Client. */ -export interface Storage { - /** - * Returns the kind of storage. - * @example cookie, localStorage, custom - */ - get type(): StoreType | string - - /** - * Tests if the storage is available for use in the current environment - */ - get available(): boolean +export interface Store { /** * get value for the key from the stores. it will return the first value found in the stores * @param key key for the value to be retrieved * @returns value for the key or null if not found */ get(key: K): Data[K] | null - /* - This is to support few scenarios where: - - value exist in one of the stores ( as a result of other stores being cleared from browser ) and we want to resync them - - read values in AJS 1.0 format ( for customers after 1.0 --> 2.0 migration ) and then re-write them in AJS 2.0 format - */ - /** - * get value for the key from the stores. it will pick the first value found in the stores, and then sync the value to all the stores - * if the found value is a number, it will be converted to a string. this is to support legacy behavior that existed in AJS 1.0 - * @param key key for the value to be retrieved - * @returns value for the key or null if not found - */ - getAndSync(key: K): Data[K] | null /** * it will set the value for the key in all the stores * @param key key for the value to be stored @@ -58,32 +36,7 @@ export interface Storage { * @param key key for the value to be removed * @param storeTypes optional array of store types to be used for removing the value */ - clear(key: K): void -} - -/** - * Abstract class for creating basic storage systems - */ -export abstract class BaseStorage - implements Storage -{ - abstract get type(): StoreType | string - abstract get available(): boolean - abstract get(key: K): Data[K] | null - abstract set(key: K, value: Data[K] | null): void - abstract clear(key: K): void - /** - * By default a storage getAndSync will handle calls exactly as a normal get. - * getAndSync needs to be implemented for more complex storage types that might wrap several base storage systems (see UniversalStorage) - */ - getAndSync(key: K): Data[K] | null { - const val = this.get(key) - // legacy behavior, getAndSync can change the type of a value from number to string (AJS 1.0 stores numerical values as a number) - const coercedValue = (typeof val === 'number' ? val.toString() : val) as - | Data[K] - | null - return coercedValue - } + remove(key: K): void } export interface StoreTypeWithSettings { diff --git a/packages/browser/src/core/storage/universalStorage.ts b/packages/browser/src/core/storage/universalStorage.ts index 9e16bac38..41cebf926 100644 --- a/packages/browser/src/core/storage/universalStorage.ts +++ b/packages/browser/src/core/storage/universalStorage.ts @@ -1,45 +1,56 @@ -import { Storage, StorageObject } from './types' +import { Store, StorageObject } from './types' /** * Uses multiple storages in a priority list to get/set values in the order they are specified. */ -export class UniversalStorage - implements Storage -{ - private stores: Storage[] +export class UniversalStorage { + private stores: Store[] - constructor(stores: Storage[]) { - this.stores = stores.filter((s) => s.available) - } - - get available(): boolean { - return this.stores.some((s) => s.available) - } - - get type(): string { - return 'PriorityListStorage' + constructor(stores: Store[]) { + this.stores = stores } get(key: K): Data[K] | null { let val: Data[K] | null = null for (const store of this.stores) { - val = store.get(key) as Data[K] | null - if (val !== undefined && val !== null) { - return val + try { + val = store.get(key) as Data[K] | null + if (val !== undefined && val !== null) { + return val + } + } catch (e) { + console.warn(`Can't access ${key}: ${e}`) } } return null } set(key: K, value: Data[K] | null): void { - this.stores.forEach((s) => s.set(key, value)) + this.stores.forEach((s) => { + try { + s.set(key, value) + } catch (e) { + console.warn(`Can't set ${key}: ${e}`) + } + }) } clear(key: K): void { - this.stores.forEach((s) => s.clear(key)) + this.stores.forEach((s) => { + try { + s.remove(key) + } catch (e) { + console.warn(`Can't remove ${key}: ${e}`) + } + }) } + /* + This is to support few scenarios where: + - value exist in one of the stores ( as a result of other stores being cleared from browser ) and we want to resync them + - read values in AJS 1.0 format ( for customers after 1.0 --> 2.0 migration ) and then re-write them in AJS 2.0 format + */ getAndSync(key: K): Data[K] | null { const val = this.get(key) diff --git a/packages/browser/src/core/user/__tests__/index.test.ts b/packages/browser/src/core/user/__tests__/index.test.ts index 045c3cedd..f047ef4d6 100644 --- a/packages/browser/src/core/user/__tests__/index.test.ts +++ b/packages/browser/src/core/user/__tests__/index.test.ts @@ -1,12 +1,11 @@ import assert from 'assert' import jar from 'js-cookie' import { Group, User } from '..' -import { LocalStorage, StoreType, Storage } from '../../storage' +import { LocalStorage, StoreType, Store } from '../../storage' import { disableCookies, disableLocalStorage, } from '../../storage/__tests__/test-helpers' -import { MemoryStorage } from '../../storage/memoryStorage' function clear(): void { document.cookie.split(';').forEach(function (c) { @@ -149,9 +148,6 @@ describe('user', () => { it('should get an id from memory', () => { user.id('id') assert(user.id() === 'id') - - expect(jar.get(cookieKey)).toBeFalsy() - expect(store.get(cookieKey)).toBeFalsy() }) it('should get an id when not persisting', () => { @@ -197,9 +193,6 @@ describe('user', () => { it('should get an id from memory', () => { user.id('id') assert(user.id() === 'id') - - expect(jar.get(cookieKey)).toBeFalsy() - expect(store.get(cookieKey)).toBeFalsy() }) it('should be null by default', () => { @@ -340,7 +333,6 @@ describe('user', () => { it('should get an id from memory', () => { user.anonymousId('anon-id') assert(user.anonymousId() === 'anon-id') - expect(jar.get('ajs_anonymous_id')).toBeFalsy() }) }) @@ -731,7 +723,9 @@ describe('user', () => { jar.set('ajs_anonymous_id', expected) store.set('ajs_anonymous_id', 'localStorageValue') const user = new User({ - storage: [StoreType.Cookie, StoreType.LocalStorage, StoreType.Memory], + storage: { + stores: [StoreType.Cookie, StoreType.LocalStorage, StoreType.Memory], + }, }) expect(user.anonymousId()).toEqual(expected) }) @@ -743,7 +737,9 @@ describe('user', () => { disableCookies() store.set('ajs_anonymous_id', expected) const user = new User({ - storage: [StoreType.Cookie, StoreType.LocalStorage, StoreType.Memory], + storage: { + stores: [StoreType.Cookie, StoreType.LocalStorage, StoreType.Memory], + }, }) expect(user.anonymousId()).toEqual(expected) }) @@ -751,7 +747,9 @@ describe('user', () => { it('persist option overrides any custom storage', () => { const setCookieSpy = jest.spyOn(jar, 'set') const user = new User({ - storage: [StoreType.Cookie, StoreType.LocalStorage, StoreType.Memory], + storage: { + stores: [StoreType.Cookie, StoreType.LocalStorage, StoreType.Memory], + }, persist: false, }) user.id('id') @@ -765,7 +763,9 @@ describe('user', () => { it('disable option overrides any custom storage', () => { const setCookieSpy = jest.spyOn(jar, 'set') const user = new User({ - storage: [StoreType.Cookie, StoreType.LocalStorage, StoreType.Memory], + storage: { + stores: [StoreType.Cookie, StoreType.LocalStorage, StoreType.Memory], + }, disable: true, }) user.id('id') @@ -775,26 +775,6 @@ describe('user', () => { expect(store.get('ajs_user_id')).toBeFalsy() expect(setCookieSpy.mock.calls.length).toBe(0) }) - - it('can use a fully custom storage object', () => { - const customStore: Storage = { - get type() { - return 'something' - }, - get available() { - return true - }, - get: jest.fn().mockReturnValue('custom'), - set: jest.fn(), - clear: jest.fn(), - getAndSync: jest.fn().mockReturnValue('custom'), - } - - const user = new User({ storage: customStore }) - user.id('id') - expect(customStore.set).toHaveBeenCalled() - expect(user.id()).toBe('custom') - }) }) }) diff --git a/packages/browser/src/core/user/index.ts b/packages/browser/src/core/user/index.ts index b958ff350..cd9d86dc6 100644 --- a/packages/browser/src/core/user/index.ts +++ b/packages/browser/src/core/user/index.ts @@ -5,14 +5,12 @@ import { CookieOptions, UniversalStorage, MemoryStorage, - Storage, StorageObject, StorageSettings, StoreType, applyCookieOptions, initializeStorages, isArrayOfStoreType, - isStorageObject, } from '../storage' export type ID = string | null | undefined @@ -35,8 +33,8 @@ export interface UserOptions { } /** - * Storage system to use - * @example new MemoryStorage, [StoreType.Cookie, StoreType.Memory] + * Store priority + * @example stores: [StoreType.Cookie, StoreType.Memory] */ storage?: StorageSettings } @@ -60,7 +58,7 @@ export class User { private anonKey: string private cookieOptions?: CookieOptions - private legacyUserStore: Storage<{ + private legacyUserStore: UniversalStorage<{ [k: string]: | { id?: string @@ -68,11 +66,11 @@ export class User { } | string }> - private traitsStore: Storage<{ + private traitsStore: UniversalStorage<{ [k: string]: Traits }> - private identityStore: Storage<{ + private identityStore: UniversalStorage<{ [k: string]: string }> @@ -235,7 +233,7 @@ export class User { options: UserOptions, cookieOpts?: CookieOptions, filterStores?: (value: StoreType) => boolean - ): Storage { + ): UniversalStorage { let stores: StoreType[] = [ StoreType.LocalStorage, StoreType.Cookie, @@ -249,16 +247,13 @@ export class User { // If persistance is disabled we will always fallback to Memory Storage if (!options.persist) { - return new MemoryStorage() + return new UniversalStorage([new MemoryStorage()]) } if (options.storage !== undefined && options.storage !== null) { - // If the user is sending its own storage implementation we will use that without any modifications - if (isStorageObject(options.storage)) { - return options.storage as Storage - } else if (isArrayOfStoreType(options.storage)) { + if (isArrayOfStoreType(options.storage)) { // If the user only specified order of stores we will still apply filters and transformations e.g. not using localStorage if localStorageFallbackDisabled - stores = options.storage + stores = options.storage.stores } }