From 119cc26dcdd38ded9899c48104c3695f223e3518 Mon Sep 17 00:00:00 2001 From: Yannick Adam Date: Tue, 9 May 2023 17:32:18 +0200 Subject: [PATCH 01/13] =?UTF-8?q?=F0=9F=8E=A8=20Renamed=20cookieSession=20?= =?UTF-8?q?to=20sessionState?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../core/src/domain/session/sessionStore.ts | 52 +++++++++---------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/packages/core/src/domain/session/sessionStore.ts b/packages/core/src/domain/session/sessionStore.ts index 98c3fe390b..2f481d50ec 100644 --- a/packages/core/src/domain/session/sessionStore.ts +++ b/packages/core/src/domain/session/sessionStore.ts @@ -48,16 +48,16 @@ export function startSessionStore( let isTracked: boolean withCookieLockAccess({ options, - process: (cookieSession) => { - const synchronizedSession = synchronizeSession(cookieSession) + process: (sessionState) => { + const synchronizedSession = synchronizeSession(sessionState) isTracked = expandOrRenewCookie(synchronizedSession) return synchronizedSession }, - after: (cookieSession) => { + after: (sessionState) => { if (isTracked && !hasSessionInCache()) { - renewSessionInCache(cookieSession) + renewSessionInCache(sessionState) } - sessionCache = cookieSession + sessionCache = sessionState }, }) } @@ -65,7 +65,7 @@ export function startSessionStore( function expandSession() { withCookieLockAccess({ options, - process: (cookieSession) => (hasSessionInCache() ? synchronizeSession(cookieSession) : undefined), + process: (sessionState) => (hasSessionInCache() ? synchronizeSession(sessionState) : undefined), }) } @@ -77,31 +77,31 @@ export function startSessionStore( function watchSession() { withCookieLockAccess({ options, - process: (cookieSession) => (!isActiveSession(cookieSession) ? {} : undefined), + process: (sessionState) => (!isActiveSession(sessionState) ? {} : undefined), after: synchronizeSession, }) } - function synchronizeSession(cookieSession: SessionState) { - if (!isActiveSession(cookieSession)) { - cookieSession = {} + function synchronizeSession(sessionState: SessionState) { + if (!isActiveSession(sessionState)) { + sessionState = {} } if (hasSessionInCache()) { - if (isSessionInCacheOutdated(cookieSession)) { + if (isSessionInCacheOutdated(sessionState)) { expireSessionInCache() } else { - sessionCache = cookieSession + sessionCache = sessionState } } - return cookieSession + return sessionState } - function expandOrRenewCookie(cookieSession: SessionState) { - const { trackingType, isTracked } = computeSessionState(cookieSession[productKey]) - cookieSession[productKey] = trackingType - if (isTracked && !cookieSession.id) { - cookieSession.id = generateUUID() - cookieSession.created = String(dateNow()) + function expandOrRenewCookie(sessionState: SessionState) { + const { trackingType, isTracked } = computeSessionState(sessionState[productKey]) + sessionState[productKey] = trackingType + if (isTracked && !sessionState.id) { + sessionState.id = generateUUID() + sessionState.created = String(dateNow()) } return isTracked } @@ -110,8 +110,8 @@ export function startSessionStore( return sessionCache[productKey] !== undefined } - function isSessionInCacheOutdated(cookieSession: SessionState) { - return sessionCache.id !== cookieSession.id || sessionCache[productKey] !== cookieSession[productKey] + function isSessionInCacheOutdated(sessionState: SessionState) { + return sessionCache.id !== sessionState.id || sessionCache[productKey] !== sessionState[productKey] } function expireSessionInCache() { @@ -119,8 +119,8 @@ export function startSessionStore( expireObservable.notify() } - function renewSessionInCache(cookieSession: SessionState) { - sessionCache = cookieSession + function renewSessionInCache(sessionState: SessionState) { + sessionCache = sessionState renewObservable.notify() } @@ -132,12 +132,12 @@ export function startSessionStore( return {} } - function isActiveSession(session: SessionState) { + function isActiveSession(sessionDate: SessionState) { // created and expire can be undefined for versions which was not storing them // these checks could be removed when older versions will not be available/live anymore return ( - (session.created === undefined || dateNow() - Number(session.created) < SESSION_TIME_OUT_DELAY) && - (session.expire === undefined || dateNow() < Number(session.expire)) + (sessionDate.created === undefined || dateNow() - Number(sessionDate.created) < SESSION_TIME_OUT_DELAY) && + (sessionDate.expire === undefined || dateNow() < Number(sessionDate.expire)) ) } From 3aaa1fcf96e1b6a7d2bdbb6a5d883835523473cd Mon Sep 17 00:00:00 2001 From: Yannick Adam Date: Tue, 9 May 2023 17:50:51 +0200 Subject: [PATCH 02/13] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Create=20a=20Session?= =?UTF-8?q?Storage=20interface?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../core/src/domain/session/oldCookiesMigration.ts | 2 +- .../src/domain/session/sessionCookieStore.spec.ts | 2 +- .../core/src/domain/session/sessionCookieStore.ts | 2 +- packages/core/src/domain/session/sessionStorage.ts | 14 ++++++++++++++ packages/core/src/domain/session/sessionStore.ts | 10 +--------- 5 files changed, 18 insertions(+), 12 deletions(-) create mode 100644 packages/core/src/domain/session/sessionStorage.ts diff --git a/packages/core/src/domain/session/oldCookiesMigration.ts b/packages/core/src/domain/session/oldCookiesMigration.ts index c78e5eaf9e..486f7f82c5 100644 --- a/packages/core/src/domain/session/oldCookiesMigration.ts +++ b/packages/core/src/domain/session/oldCookiesMigration.ts @@ -1,6 +1,6 @@ import type { CookieOptions } from '../../browser/cookie' import { getCookie } from '../../browser/cookie' -import type { SessionState } from './sessionStore' +import type { SessionState } from './sessionStorage' import { SESSION_COOKIE_NAME, persistSessionCookie } from './sessionCookieStore' export const OLD_SESSION_COOKIE_NAME = '_dd' diff --git a/packages/core/src/domain/session/sessionCookieStore.spec.ts b/packages/core/src/domain/session/sessionCookieStore.spec.ts index 343b94934c..2d098175b6 100644 --- a/packages/core/src/domain/session/sessionCookieStore.spec.ts +++ b/packages/core/src/domain/session/sessionCookieStore.spec.ts @@ -9,7 +9,7 @@ import { LOCK_RETRY_DELAY, withCookieLockAccess, } from './sessionCookieStore' -import type { SessionState } from './sessionStore' +import type { SessionState } from './sessionStorage' describe('session cookie store', () => { const COOKIE_OPTIONS = {} diff --git a/packages/core/src/domain/session/sessionCookieStore.ts b/packages/core/src/domain/session/sessionCookieStore.ts index 4efeb76083..f1bb7e183e 100644 --- a/packages/core/src/domain/session/sessionCookieStore.ts +++ b/packages/core/src/domain/session/sessionCookieStore.ts @@ -7,7 +7,7 @@ import { objectEntries } from '../../tools/utils/polyfills' import { isEmptyObject } from '../../tools/utils/objectUtils' import { generateUUID } from '../../tools/utils/stringUtils' import { SESSION_EXPIRATION_DELAY } from './sessionConstants' -import type { SessionState } from './sessionStore' +import type { SessionState } from './sessionStorage' const SESSION_ENTRY_REGEXP = /^([a-z]+)=([a-z0-9-]+)$/ const SESSION_ENTRY_SEPARATOR = '&' diff --git a/packages/core/src/domain/session/sessionStorage.ts b/packages/core/src/domain/session/sessionStorage.ts new file mode 100644 index 0000000000..bdb4f2e8b8 --- /dev/null +++ b/packages/core/src/domain/session/sessionStorage.ts @@ -0,0 +1,14 @@ +export interface SessionState { + id?: string + created?: string + expire?: string + lock?: string + + [key: string]: string | undefined +} + +export interface SessionStorage { + persistSession: (session: SessionState) => void + retrieveSession: () => SessionState + clearSession: () => void +} diff --git a/packages/core/src/domain/session/sessionStore.ts b/packages/core/src/domain/session/sessionStore.ts index 2f481d50ec..a16efe7b81 100644 --- a/packages/core/src/domain/session/sessionStore.ts +++ b/packages/core/src/domain/session/sessionStore.ts @@ -7,6 +7,7 @@ import { throttle } from '../../tools/utils/functionUtils' import { generateUUID } from '../../tools/utils/stringUtils' import { SESSION_TIME_OUT_DELAY } from './sessionConstants' import { deleteSessionCookie, retrieveSessionCookie, withCookieLockAccess } from './sessionCookieStore' +import type { SessionState } from './sessionStorage' export interface SessionStore { expandOrRenewSession: () => void @@ -18,15 +19,6 @@ export interface SessionStore { stop: () => void } -export interface SessionState { - id?: string - created?: string - expire?: string - lock?: string - - [key: string]: string | undefined -} - /** * Different session concepts: * - tracked, the session has an id and is updated along the user navigation From c021c6712794b89980e1b2532832d73103d7e136 Mon Sep 17 00:00:00 2001 From: Yannick Adam Date: Tue, 9 May 2023 20:19:04 +0200 Subject: [PATCH 03/13] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20=20Move=20withCookie?= =?UTF-8?q?LockAccess=20to=20SessionStore=20and=20rename=20processStorageO?= =?UTF-8?q?perations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../domain/session/sessionCookieStore.spec.ts | 248 +------ .../src/domain/session/sessionCookieStore.ts | 113 +-- .../src/domain/session/sessionStore.spec.ts | 683 ++++++++++++------ .../core/src/domain/session/sessionStore.ts | 108 ++- 4 files changed, 599 insertions(+), 553 deletions(-) diff --git a/packages/core/src/domain/session/sessionCookieStore.spec.ts b/packages/core/src/domain/session/sessionCookieStore.spec.ts index 2d098175b6..8fe1034724 100644 --- a/packages/core/src/domain/session/sessionCookieStore.spec.ts +++ b/packages/core/src/domain/session/sessionCookieStore.spec.ts @@ -1,237 +1,41 @@ -import { mockClock, stubCookie } from '../../../test' -import { isChromium } from '../../tools/utils/browserDetection' +import type { CookieOptions } from '../../browser/cookie' +import { setCookie, deleteCookie } from '../../browser/cookie' +import { SESSION_EXPIRATION_DELAY } from './sessionConstants' import { SESSION_COOKIE_NAME, - toSessionString, - retrieveSessionCookie, + deleteSessionCookie, persistSessionCookie, - MAX_NUMBER_OF_LOCK_RETRIES, - LOCK_RETRY_DELAY, - withCookieLockAccess, + retrieveSessionCookie, } from './sessionCookieStore' + import type { SessionState } from './sessionStorage' describe('session cookie store', () => { - const COOKIE_OPTIONS = {} - let initialSession: SessionState - let otherSession: SessionState - let processSpy: jasmine.Spy - let afterSpy: jasmine.Spy - let cookie: ReturnType + const sessionState: SessionState = { id: '123', created: '0' } + const noOptions: CookieOptions = {} - beforeEach(() => { - initialSession = { id: '123', created: '0' } - otherSession = { id: '456', created: '100' } - processSpy = jasmine.createSpy('process') - afterSpy = jasmine.createSpy('after') - cookie = stubCookie() + afterEach(() => { + deleteCookie(SESSION_COOKIE_NAME) }) - describe('with cookie-lock disabled', () => { - beforeEach(() => { - isChromium() && pending('cookie-lock only disabled on non chromium browsers') - }) - - it('should persist session when process return a value', () => { - persistSessionCookie(initialSession, COOKIE_OPTIONS) - processSpy.and.returnValue({ ...otherSession }) - - withCookieLockAccess({ options: COOKIE_OPTIONS, process: processSpy, after: afterSpy }) - - expect(processSpy).toHaveBeenCalledWith(initialSession) - const expectedSession = { ...otherSession, expire: jasmine.any(String) } - expect(retrieveSessionCookie()).toEqual(expectedSession) - expect(afterSpy).toHaveBeenCalledWith(expectedSession) - }) - - it('should clear session when process return an empty value', () => { - persistSessionCookie(initialSession, COOKIE_OPTIONS) - processSpy.and.returnValue({}) - - withCookieLockAccess({ options: COOKIE_OPTIONS, process: processSpy, after: afterSpy }) - - expect(processSpy).toHaveBeenCalledWith(initialSession) - const expectedSession = {} - expect(retrieveSessionCookie()).toEqual(expectedSession) - expect(afterSpy).toHaveBeenCalledWith(expectedSession) - }) - - it('should not persist session when process return undefined', () => { - persistSessionCookie(initialSession, COOKIE_OPTIONS) - processSpy.and.returnValue(undefined) - - withCookieLockAccess({ options: COOKIE_OPTIONS, process: processSpy, after: afterSpy }) - - expect(processSpy).toHaveBeenCalledWith(initialSession) - expect(retrieveSessionCookie()).toEqual(initialSession) - expect(afterSpy).toHaveBeenCalledWith(initialSession) - }) + it('should persist a session in a cookie', () => { + const now = Date.now() + persistSessionCookie(sessionState, noOptions) + const session = retrieveSessionCookie() + expect(session).toEqual({ ...sessionState }) + expect(+session.expire!).toBeGreaterThanOrEqual(now + SESSION_EXPIRATION_DELAY) }) - describe('with cookie-lock enabled', () => { - beforeEach(() => { - !isChromium() && pending('cookie-lock only enabled on chromium browsers') - }) - - it('should persist session when process return a value', () => { - persistSessionCookie(initialSession, COOKIE_OPTIONS) - processSpy.and.callFake((session) => ({ ...otherSession, lock: session.lock })) - - withCookieLockAccess({ options: COOKIE_OPTIONS, process: processSpy, after: afterSpy }) - - expect(processSpy).toHaveBeenCalledWith({ ...initialSession, lock: jasmine.any(String) }) - const expectedSession = { ...otherSession, expire: jasmine.any(String) } - expect(retrieveSessionCookie()).toEqual(expectedSession) - expect(afterSpy).toHaveBeenCalledWith(expectedSession) - }) - - it('should clear session when process return an empty value', () => { - persistSessionCookie(initialSession, COOKIE_OPTIONS) - processSpy.and.returnValue({}) - - withCookieLockAccess({ options: COOKIE_OPTIONS, process: processSpy, after: afterSpy }) - - expect(processSpy).toHaveBeenCalledWith({ ...initialSession, lock: jasmine.any(String) }) - const expectedSession = {} - expect(retrieveSessionCookie()).toEqual(expectedSession) - expect(afterSpy).toHaveBeenCalledWith(expectedSession) - }) - - it('should not persist session when process return undefined', () => { - persistSessionCookie(initialSession, COOKIE_OPTIONS) - processSpy.and.returnValue(undefined) - - withCookieLockAccess({ options: COOKIE_OPTIONS, process: processSpy, after: afterSpy }) - - expect(processSpy).toHaveBeenCalledWith({ ...initialSession, lock: jasmine.any(String) }) - expect(retrieveSessionCookie()).toEqual(initialSession) - expect(afterSpy).toHaveBeenCalledWith(initialSession) - }) - - type OnLockCheck = () => { currentState: SessionState; retryState: SessionState } - - function lockScenario({ - onInitialLockCheck, - onAcquiredLockCheck, - onPostProcessLockCheck, - onPostPersistLockCheck, - }: { - onInitialLockCheck?: OnLockCheck - onAcquiredLockCheck?: OnLockCheck - onPostProcessLockCheck?: OnLockCheck - onPostPersistLockCheck?: OnLockCheck - }) { - const onLockChecks = [onInitialLockCheck, onAcquiredLockCheck, onPostProcessLockCheck, onPostPersistLockCheck] - cookie.getSpy.and.callFake(() => { - const currentOnLockCheck = onLockChecks.shift() - if (!currentOnLockCheck) { - return cookie.currentValue() - } - const { currentState, retryState } = currentOnLockCheck() - cookie.setCurrentValue(buildSessionString(retryState)) - return buildSessionString(currentState) - }) - } - - function buildSessionString(currentState: SessionState) { - return `${SESSION_COOKIE_NAME}=${toSessionString(currentState)}` - } - - ;[ - { - description: 'should wait for lock to be free', - lockConflict: 'onInitialLockCheck', - }, - { - description: 'should retry if lock was acquired before process', - lockConflict: 'onAcquiredLockCheck', - }, - { - description: 'should retry if lock was acquired after process', - lockConflict: 'onPostProcessLockCheck', - }, - { - description: 'should retry if lock was acquired after persist', - lockConflict: 'onPostPersistLockCheck', - }, - ].forEach(({ description, lockConflict }) => { - it(description, (done) => { - lockScenario({ - [lockConflict]: () => ({ - currentState: { ...initialSession, lock: 'locked' }, - retryState: { ...initialSession, other: 'other' }, - }), - }) - persistSessionCookie(initialSession, COOKIE_OPTIONS) - processSpy.and.callFake((session) => ({ ...session, processed: 'processed' } as SessionState)) - - withCookieLockAccess({ - options: COOKIE_OPTIONS, - process: processSpy, - after: (afterSession) => { - // session with 'other' value on process - expect(processSpy).toHaveBeenCalledWith({ - ...initialSession, - other: 'other', - lock: jasmine.any(String), - expire: jasmine.any(String), - }) - - // end state with session 'other' and 'processed' value - const expectedSession = { - ...initialSession, - other: 'other', - processed: 'processed', - expire: jasmine.any(String), - } - expect(retrieveSessionCookie()).toEqual(expectedSession) - expect(afterSession).toEqual(expectedSession) - done() - }, - }) - }) - }) - - it('should abort after a max number of retry', () => { - const clock = mockClock() - - persistSessionCookie(initialSession, COOKIE_OPTIONS) - cookie.setSpy.calls.reset() - - cookie.getSpy.and.returnValue(buildSessionString({ ...initialSession, lock: 'locked' })) - withCookieLockAccess({ options: COOKIE_OPTIONS, process: processSpy, after: afterSpy }) - - clock.tick(MAX_NUMBER_OF_LOCK_RETRIES * LOCK_RETRY_DELAY) - expect(processSpy).not.toHaveBeenCalled() - expect(afterSpy).not.toHaveBeenCalled() - expect(cookie.setSpy).not.toHaveBeenCalled() - - clock.cleanup() - }) - - it('should execute cookie accesses in order', (done) => { - lockScenario({ - onInitialLockCheck: () => ({ - currentState: { ...initialSession, lock: 'locked' }, // force to retry the first access later - retryState: initialSession, - }), - }) - persistSessionCookie(initialSession, COOKIE_OPTIONS) + it('should delete the cookie holding the session', () => { + persistSessionCookie(sessionState, noOptions) + deleteSessionCookie(noOptions) + const session = retrieveSessionCookie() + expect(session).toEqual({}) + }) - withCookieLockAccess({ - options: COOKIE_OPTIONS, - process: (session) => ({ ...session, value: 'foo' }), - after: afterSpy, - }) - withCookieLockAccess({ - options: COOKIE_OPTIONS, - process: (session) => ({ ...session, value: `${session.value || ''}bar` }), - after: (session) => { - expect(session.value).toBe('foobar') - expect(afterSpy).toHaveBeenCalled() - done() - }, - }) - }) + it('should return an empty object if session string is invalid', () => { + setCookie(SESSION_COOKIE_NAME, '{test:42}', 1000) + const session = retrieveSessionCookie() + expect(session).toEqual({}) }) }) diff --git a/packages/core/src/domain/session/sessionCookieStore.ts b/packages/core/src/domain/session/sessionCookieStore.ts index f1bb7e183e..ab455fb731 100644 --- a/packages/core/src/domain/session/sessionCookieStore.ts +++ b/packages/core/src/domain/session/sessionCookieStore.ts @@ -1,11 +1,9 @@ import type { CookieOptions } from '../../browser/cookie' import { deleteCookie, getCookie, setCookie } from '../../browser/cookie' -import { setTimeout } from '../../tools/timer' import { isChromium } from '../../tools/utils/browserDetection' import { dateNow } from '../../tools/utils/timeUtils' import { objectEntries } from '../../tools/utils/polyfills' import { isEmptyObject } from '../../tools/utils/objectUtils' -import { generateUUID } from '../../tools/utils/stringUtils' import { SESSION_EXPIRATION_DELAY } from './sessionConstants' import type { SessionState } from './sessionStorage' @@ -14,105 +12,6 @@ const SESSION_ENTRY_SEPARATOR = '&' export const SESSION_COOKIE_NAME = '_dd_s' -// arbitrary values -export const LOCK_RETRY_DELAY = 10 -export const MAX_NUMBER_OF_LOCK_RETRIES = 100 - -type Operations = { - options: CookieOptions - process: (cookieSession: SessionState) => SessionState | undefined - after?: (cookieSession: SessionState) => void -} - -const bufferedOperations: Operations[] = [] -let ongoingOperations: Operations | undefined - -export function withCookieLockAccess(operations: Operations, numberOfRetries = 0) { - if (!ongoingOperations) { - ongoingOperations = operations - } - if (operations !== ongoingOperations) { - bufferedOperations.push(operations) - return - } - if (numberOfRetries >= MAX_NUMBER_OF_LOCK_RETRIES) { - next() - return - } - let currentLock: string - let currentSession = retrieveSessionCookie() - if (isCookieLockEnabled()) { - // if someone has lock, retry later - if (currentSession.lock) { - retryLater(operations, numberOfRetries) - return - } - // acquire lock - currentLock = generateUUID() - currentSession.lock = currentLock - setSessionCookie(currentSession, operations.options) - // if lock is not acquired, retry later - currentSession = retrieveSessionCookie() - if (currentSession.lock !== currentLock) { - retryLater(operations, numberOfRetries) - return - } - } - let processedSession = operations.process(currentSession) - if (isCookieLockEnabled()) { - // if lock corrupted after process, retry later - currentSession = retrieveSessionCookie() - if (currentSession.lock !== currentLock!) { - retryLater(operations, numberOfRetries) - return - } - } - if (processedSession) { - persistSessionCookie(processedSession, operations.options) - } - if (isCookieLockEnabled()) { - // correctly handle lock around expiration would require to handle this case properly at several levels - // since we don't have evidence of lock issues around expiration, let's just not do the corruption check for it - if (!(processedSession && isExpiredState(processedSession))) { - // if lock corrupted after persist, retry later - currentSession = retrieveSessionCookie() - if (currentSession.lock !== currentLock!) { - retryLater(operations, numberOfRetries) - return - } - delete currentSession.lock - setSessionCookie(currentSession, operations.options) - processedSession = currentSession - } - } - // call after even if session is not persisted in order to perform operations on - // up-to-date cookie value, the value could have been modified by another tab - operations.after?.(processedSession || currentSession) - next() -} - -/** - * Cookie lock strategy allows mitigating issues due to concurrent access to cookie. - * This issue concerns only chromium browsers and enabling this on firefox increase cookie write failures. - */ -function isCookieLockEnabled() { - return isChromium() -} - -function retryLater(operations: Operations, currentNumberOfRetries: number) { - setTimeout(() => { - withCookieLockAccess(operations, currentNumberOfRetries + 1) - }, LOCK_RETRY_DELAY) -} - -function next() { - ongoingOperations = undefined - const nextOperations = bufferedOperations.shift() - if (nextOperations) { - withCookieLockAccess(nextOperations) - } -} - export function persistSessionCookie(session: SessionState, options: CookieOptions) { if (isExpiredState(session)) { deleteSessionCookie(options) @@ -122,7 +21,7 @@ export function persistSessionCookie(session: SessionState, options: CookieOptio setSessionCookie(session, options) } -function setSessionCookie(session: SessionState, options: CookieOptions) { +export function setSessionCookie(session: SessionState, options: CookieOptions) { setCookie(SESSION_COOKIE_NAME, toSessionString(session), SESSION_EXPIRATION_DELAY, options) } @@ -158,6 +57,14 @@ function isValidSessionString(sessionString: string | undefined): sessionString ) } -function isExpiredState(session: SessionState) { +export function isExpiredState(session: SessionState) { return isEmptyObject(session) } + +/** + * Cookie lock strategy allows mitigating issues due to concurrent access to cookie. + * This issue concerns only chromium browsers and enabling this on firefox increase cookie write failures. + */ +export function isCookieLockEnabled() { + return isChromium() +} diff --git a/packages/core/src/domain/session/sessionStore.spec.ts b/packages/core/src/domain/session/sessionStore.spec.ts index a4123e9b06..eab672cf00 100644 --- a/packages/core/src/domain/session/sessionStore.spec.ts +++ b/packages/core/src/domain/session/sessionStore.spec.ts @@ -1,11 +1,18 @@ import type { Clock } from '../../../test' -import { mockClock } from '../../../test' +import { stubCookie, mockClock } from '../../../test' import type { CookieOptions } from '../../browser/cookie' import { getCookie, setCookie, COOKIE_ACCESS_DELAY } from '../../browser/cookie' +import { isChromium } from '../../tools/utils/browserDetection' import type { SessionStore } from './sessionStore' -import { startSessionStore } from './sessionStore' -import { SESSION_COOKIE_NAME } from './sessionCookieStore' +import { + LOCK_RETRY_DELAY, + MAX_NUMBER_OF_LOCK_RETRIES, + processStorageOperations, + startSessionStore, +} from './sessionStore' +import { SESSION_COOKIE_NAME, persistSessionCookie, retrieveSessionCookie, toSessionString } from './sessionCookieStore' import { SESSION_EXPIRATION_DELAY, SESSION_TIME_OUT_DELAY } from './sessionConstants' +import type { SessionState } from './sessionStorage' const enum FakeTrackingType { TRACKED = 'tracked', @@ -47,303 +54,533 @@ function resetSessionInStore() { } describe('session store', () => { - let expireSpy: () => void - let renewSpy: () => void - let sessionStore: SessionStore - let clock: Clock - - function setupSessionStore( - computeSessionState: (rawTrackingType?: string) => { trackingType: FakeTrackingType; isTracked: boolean } = () => ({ - isTracked: true, - trackingType: FakeTrackingType.TRACKED, + describe('session lifecyle mechanism', () => { + let expireSpy: () => void + let renewSpy: () => void + let sessionStore: SessionStore + let clock: Clock + + function setupSessionStore( + computeSessionState: (rawTrackingType?: string) => { + trackingType: FakeTrackingType + isTracked: boolean + } = () => ({ + isTracked: true, + trackingType: FakeTrackingType.TRACKED, + }) + ) { + sessionStore = startSessionStore(COOKIE_OPTIONS, PRODUCT_KEY, computeSessionState) + sessionStore.expireObservable.subscribe(expireSpy) + sessionStore.renewObservable.subscribe(renewSpy) + } + + beforeEach(() => { + expireSpy = jasmine.createSpy('expire session') + renewSpy = jasmine.createSpy('renew session') + clock = mockClock() }) - ) { - sessionStore = startSessionStore(COOKIE_OPTIONS, PRODUCT_KEY, computeSessionState) - sessionStore.expireObservable.subscribe(expireSpy) - sessionStore.renewObservable.subscribe(renewSpy) - } - - beforeEach(() => { - expireSpy = jasmine.createSpy('expire session') - renewSpy = jasmine.createSpy('renew session') - clock = mockClock() - }) - afterEach(() => { - resetSessionInStore() - clock.cleanup() - sessionStore.stop() - }) + afterEach(() => { + resetSessionInStore() + clock.cleanup() + sessionStore.stop() + }) - describe('expand or renew session', () => { - it( - 'when session not in cache, session not in store and new session tracked, ' + - 'should create new session and trigger renew session ', - () => { + describe('expand or renew session', () => { + it( + 'when session not in cache, session not in store and new session tracked, ' + + 'should create new session and trigger renew session ', + () => { + setupSessionStore() + + sessionStore.expandOrRenewSession() + + expect(sessionStore.getSession().id).toBeDefined() + expectTrackedSessionToBeInStore() + expect(expireSpy).not.toHaveBeenCalled() + expect(renewSpy).toHaveBeenCalled() + } + ) + + it( + 'when session not in cache, session not in store and new session not tracked, ' + + 'should store not tracked session', + () => { + setupSessionStore(() => ({ isTracked: false, trackingType: FakeTrackingType.NOT_TRACKED })) + + sessionStore.expandOrRenewSession() + + expect(sessionStore.getSession().id).toBeUndefined() + expectNotTrackedSessionToBeInStore() + expect(expireSpy).not.toHaveBeenCalled() + expect(renewSpy).not.toHaveBeenCalled() + } + ) + + it('when session not in cache and session in store, should expand session and trigger renew session', () => { setupSessionStore() + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) sessionStore.expandOrRenewSession() - expect(sessionStore.getSession().id).toBeDefined() - expectTrackedSessionToBeInStore() + expect(sessionStore.getSession().id).toBe(FIRST_ID) + expectTrackedSessionToBeInStore(FIRST_ID) expect(expireSpy).not.toHaveBeenCalled() expect(renewSpy).toHaveBeenCalled() - } - ) - - it( - 'when session not in cache, session not in store and new session not tracked, ' + - 'should store not tracked session', - () => { - setupSessionStore(() => ({ isTracked: false, trackingType: FakeTrackingType.NOT_TRACKED })) + }) + + it( + 'when session in cache, session not in store and new session tracked, ' + + 'should expire session, create a new one and trigger renew session', + () => { + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + setupSessionStore() + resetSessionInStore() + + sessionStore.expandOrRenewSession() + + const sessionId = sessionStore.getSession().id + expect(sessionId).toBeDefined() + expect(sessionId).not.toBe(FIRST_ID) + expectTrackedSessionToBeInStore(sessionId) + expect(expireSpy).toHaveBeenCalled() + expect(renewSpy).toHaveBeenCalled() + } + ) + + it( + 'when session in cache, session not in store and new session not tracked, ' + + 'should expire session and store not tracked session', + () => { + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + setupSessionStore(() => ({ isTracked: false, trackingType: FakeTrackingType.NOT_TRACKED })) + resetSessionInStore() + + sessionStore.expandOrRenewSession() + + expect(sessionStore.getSession().id).toBeUndefined() + expect(sessionStore.getSession()[PRODUCT_KEY]).toBeDefined() + expectNotTrackedSessionToBeInStore() + expect(expireSpy).toHaveBeenCalled() + expect(renewSpy).not.toHaveBeenCalled() + } + ) + + it( + 'when session not tracked in cache, session not in store and new session not tracked, ' + + 'should expire session and store not tracked session', + () => { + setSessionInStore(FakeTrackingType.NOT_TRACKED) + setupSessionStore(() => ({ isTracked: false, trackingType: FakeTrackingType.NOT_TRACKED })) + resetSessionInStore() + + sessionStore.expandOrRenewSession() + + expect(sessionStore.getSession().id).toBeUndefined() + expect(sessionStore.getSession()[PRODUCT_KEY]).toBeDefined() + expectNotTrackedSessionToBeInStore() + expect(expireSpy).toHaveBeenCalled() + expect(renewSpy).not.toHaveBeenCalled() + } + ) + + it('when session in cache is same session than in store, should expand session', () => { + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + setupSessionStore() + clock.tick(10) sessionStore.expandOrRenewSession() - expect(sessionStore.getSession().id).toBeUndefined() - expectNotTrackedSessionToBeInStore() + expect(sessionStore.getSession().id).toBe(FIRST_ID) + expect(sessionStore.getSession().expire).toBe(getStoreExpiration()) + expectTrackedSessionToBeInStore(FIRST_ID) expect(expireSpy).not.toHaveBeenCalled() expect(renewSpy).not.toHaveBeenCalled() - } - ) - - it('when session not in cache and session in store, should expand session and trigger renew session', () => { - setupSessionStore() - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) - - sessionStore.expandOrRenewSession() - - expect(sessionStore.getSession().id).toBe(FIRST_ID) - expectTrackedSessionToBeInStore(FIRST_ID) - expect(expireSpy).not.toHaveBeenCalled() - expect(renewSpy).toHaveBeenCalled() + }) + + it( + 'when session in cache is different session than in store and store session is tracked, ' + + 'should expire session, expand store session and trigger renew', + () => { + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + setupSessionStore() + setSessionInStore(FakeTrackingType.TRACKED, SECOND_ID) + + sessionStore.expandOrRenewSession() + + expect(sessionStore.getSession().id).toBe(SECOND_ID) + expectTrackedSessionToBeInStore(SECOND_ID) + expect(expireSpy).toHaveBeenCalled() + expect(renewSpy).toHaveBeenCalled() + } + ) + + it( + 'when session in cache is different session than in store and store session is not tracked, ' + + 'should expire session and store not tracked session', + () => { + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + setupSessionStore((rawTrackingType) => ({ + isTracked: rawTrackingType === FakeTrackingType.TRACKED, + trackingType: rawTrackingType as FakeTrackingType, + })) + setSessionInStore(FakeTrackingType.NOT_TRACKED, '') + + sessionStore.expandOrRenewSession() + + expect(sessionStore.getSession().id).toBeUndefined() + expectNotTrackedSessionToBeInStore() + expect(expireSpy).toHaveBeenCalled() + expect(renewSpy).not.toHaveBeenCalled() + } + ) }) - it( - 'when session in cache, session not in store and new session tracked, ' + - 'should expire session, create a new one and trigger renew session', - () => { - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + describe('expand session', () => { + it('when session not in cache and session not in store, should do nothing', () => { setupSessionStore() - resetSessionInStore() - sessionStore.expandOrRenewSession() + sessionStore.expandSession() - const sessionId = sessionStore.getSession().id - expect(sessionId).toBeDefined() - expect(sessionId).not.toBe(FIRST_ID) - expectTrackedSessionToBeInStore(sessionId) - expect(expireSpy).toHaveBeenCalled() - expect(renewSpy).toHaveBeenCalled() - } - ) + expect(sessionStore.getSession().id).toBeUndefined() + expect(expireSpy).not.toHaveBeenCalled() + }) - it( - 'when session in cache, session not in store and new session not tracked, ' + - 'should expire session and store not tracked session', - () => { + it('when session not in cache and session in store, should do nothing', () => { + setupSessionStore() setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) - setupSessionStore(() => ({ isTracked: false, trackingType: FakeTrackingType.NOT_TRACKED })) - resetSessionInStore() - sessionStore.expandOrRenewSession() + sessionStore.expandSession() expect(sessionStore.getSession().id).toBeUndefined() - expect(sessionStore.getSession()[PRODUCT_KEY]).toBeDefined() - expectNotTrackedSessionToBeInStore() - expect(expireSpy).toHaveBeenCalled() - expect(renewSpy).not.toHaveBeenCalled() - } - ) - - it( - 'when session not tracked in cache, session not in store and new session not tracked, ' + - 'should expire session and store not tracked session', - () => { - setSessionInStore(FakeTrackingType.NOT_TRACKED) - setupSessionStore(() => ({ isTracked: false, trackingType: FakeTrackingType.NOT_TRACKED })) + expect(expireSpy).not.toHaveBeenCalled() + }) + + it('when session in cache and session not in store, should expire session', () => { + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + setupSessionStore() resetSessionInStore() - sessionStore.expandOrRenewSession() + sessionStore.expandSession() expect(sessionStore.getSession().id).toBeUndefined() - expect(sessionStore.getSession()[PRODUCT_KEY]).toBeDefined() - expectNotTrackedSessionToBeInStore() expect(expireSpy).toHaveBeenCalled() - expect(renewSpy).not.toHaveBeenCalled() - } - ) + }) - it('when session in cache is same session than in store, should expand session', () => { - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) - setupSessionStore() + it('when session in cache is same session than in store, should expand session', () => { + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + setupSessionStore() - clock.tick(10) - sessionStore.expandOrRenewSession() + clock.tick(10) + sessionStore.expandSession() - expect(sessionStore.getSession().id).toBe(FIRST_ID) - expect(sessionStore.getSession().expire).toBe(getStoreExpiration()) - expectTrackedSessionToBeInStore(FIRST_ID) - expect(expireSpy).not.toHaveBeenCalled() - expect(renewSpy).not.toHaveBeenCalled() - }) + expect(sessionStore.getSession().id).toBe(FIRST_ID) + expect(sessionStore.getSession().expire).toBe(getStoreExpiration()) + expect(expireSpy).not.toHaveBeenCalled() + }) - it( - 'when session in cache is different session than in store and store session is tracked, ' + - 'should expire session, expand store session and trigger renew', - () => { + it('when session in cache is different session than in store, should expire session', () => { setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) setupSessionStore() setSessionInStore(FakeTrackingType.TRACKED, SECOND_ID) - sessionStore.expandOrRenewSession() + sessionStore.expandSession() - expect(sessionStore.getSession().id).toBe(SECOND_ID) + expect(sessionStore.getSession().id).toBeUndefined() expectTrackedSessionToBeInStore(SECOND_ID) expect(expireSpy).toHaveBeenCalled() - expect(renewSpy).toHaveBeenCalled() - } - ) + }) + }) - it( - 'when session in cache is different session than in store and store session is not tracked, ' + - 'should expire session and store not tracked session', - () => { - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) - setupSessionStore((rawTrackingType) => ({ - isTracked: rawTrackingType === FakeTrackingType.TRACKED, - trackingType: rawTrackingType as FakeTrackingType, - })) - setSessionInStore(FakeTrackingType.NOT_TRACKED, '') + describe('regular watch', () => { + it('when session not in cache and session not in store, should do nothing', () => { + setupSessionStore() - sessionStore.expandOrRenewSession() + clock.tick(COOKIE_ACCESS_DELAY) expect(sessionStore.getSession().id).toBeUndefined() - expectNotTrackedSessionToBeInStore() - expect(expireSpy).toHaveBeenCalled() - expect(renewSpy).not.toHaveBeenCalled() - } - ) - }) + expect(expireSpy).not.toHaveBeenCalled() + }) - describe('expand session', () => { - it('when session not in cache and session not in store, should do nothing', () => { - setupSessionStore() + it('when session not in cache and session in store, should do nothing', () => { + setupSessionStore() + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) - sessionStore.expandSession() + clock.tick(COOKIE_ACCESS_DELAY) - expect(sessionStore.getSession().id).toBeUndefined() - expect(expireSpy).not.toHaveBeenCalled() - }) + expect(sessionStore.getSession().id).toBeUndefined() + expect(expireSpy).not.toHaveBeenCalled() + }) - it('when session not in cache and session in store, should do nothing', () => { - setupSessionStore() - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + it('when session in cache and session not in store, should expire session', () => { + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + setupSessionStore() + resetSessionInStore() - sessionStore.expandSession() + clock.tick(COOKIE_ACCESS_DELAY) - expect(sessionStore.getSession().id).toBeUndefined() - expect(expireSpy).not.toHaveBeenCalled() - }) + expect(sessionStore.getSession().id).toBeUndefined() + expect(expireSpy).toHaveBeenCalled() + }) - it('when session in cache and session not in store, should expire session', () => { - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) - setupSessionStore() - resetSessionInStore() + it('when session in cache is same session than in store, should synchronize session', () => { + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + setupSessionStore() + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID, Date.now() + SESSION_TIME_OUT_DELAY + 10) - sessionStore.expandSession() + clock.tick(COOKIE_ACCESS_DELAY) - expect(sessionStore.getSession().id).toBeUndefined() - expect(expireSpy).toHaveBeenCalled() - }) + expect(sessionStore.getSession().id).toBe(FIRST_ID) + expect(sessionStore.getSession().expire).toBe(getStoreExpiration()) + expect(expireSpy).not.toHaveBeenCalled() + }) - it('when session in cache is same session than in store, should expand session', () => { - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) - setupSessionStore() + it('when session id in cache is different than session id in store, should expire session', () => { + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + setupSessionStore() + setSessionInStore(FakeTrackingType.TRACKED, SECOND_ID) - clock.tick(10) - sessionStore.expandSession() + clock.tick(COOKIE_ACCESS_DELAY) - expect(sessionStore.getSession().id).toBe(FIRST_ID) - expect(sessionStore.getSession().expire).toBe(getStoreExpiration()) - expect(expireSpy).not.toHaveBeenCalled() - }) + expect(sessionStore.getSession().id).toBeUndefined() + expect(expireSpy).toHaveBeenCalled() + }) - it('when session in cache is different session than in store, should expire session', () => { - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) - setupSessionStore() - setSessionInStore(FakeTrackingType.TRACKED, SECOND_ID) + it('when session type in cache is different than session type in store, should expire session', () => { + setSessionInStore(FakeTrackingType.NOT_TRACKED, FIRST_ID) + setupSessionStore() + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) - sessionStore.expandSession() + clock.tick(COOKIE_ACCESS_DELAY) - expect(sessionStore.getSession().id).toBeUndefined() - expectTrackedSessionToBeInStore(SECOND_ID) - expect(expireSpy).toHaveBeenCalled() + expect(sessionStore.getSession().id).toBeUndefined() + expect(expireSpy).toHaveBeenCalled() + }) }) }) - describe('regular watch', () => { - it('when session not in cache and session not in store, should do nothing', () => { - setupSessionStore() - - clock.tick(COOKIE_ACCESS_DELAY) - - expect(sessionStore.getSession().id).toBeUndefined() - expect(expireSpy).not.toHaveBeenCalled() + describe('process operations mechanism', () => { + const COOKIE_OPTIONS = {} + let initialSession: SessionState + let otherSession: SessionState + let processSpy: jasmine.Spy + let afterSpy: jasmine.Spy + let cookie: ReturnType + + beforeEach(() => { + initialSession = { id: '123', created: '0' } + otherSession = { id: '456', created: '100' } + processSpy = jasmine.createSpy('process') + afterSpy = jasmine.createSpy('after') + cookie = stubCookie() }) - it('when session not in cache and session in store, should do nothing', () => { - setupSessionStore() - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + describe('with cookie-lock disabled', () => { + beforeEach(() => { + isChromium() && pending('cookie-lock only disabled on non chromium browsers') + }) - clock.tick(COOKIE_ACCESS_DELAY) + it('should persist session when process returns a value', () => { + persistSessionCookie(initialSession, COOKIE_OPTIONS) + processSpy.and.returnValue({ ...otherSession }) - expect(sessionStore.getSession().id).toBeUndefined() - expect(expireSpy).not.toHaveBeenCalled() - }) + processStorageOperations({ options: COOKIE_OPTIONS, process: processSpy, after: afterSpy }) - it('when session in cache and session not in store, should expire session', () => { - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) - setupSessionStore() - resetSessionInStore() + expect(processSpy).toHaveBeenCalledWith(initialSession) + const expectedSession = { ...otherSession, expire: jasmine.any(String) } + expect(retrieveSessionCookie()).toEqual(expectedSession) + expect(afterSpy).toHaveBeenCalledWith(expectedSession) + }) - clock.tick(COOKIE_ACCESS_DELAY) + it('should clear session when process return an empty value', () => { + persistSessionCookie(initialSession, COOKIE_OPTIONS) + processSpy.and.returnValue({}) - expect(sessionStore.getSession().id).toBeUndefined() - expect(expireSpy).toHaveBeenCalled() - }) + processStorageOperations({ options: COOKIE_OPTIONS, process: processSpy, after: afterSpy }) - it('when session in cache is same session than in store, should synchronize session', () => { - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) - setupSessionStore() - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID, Date.now() + SESSION_TIME_OUT_DELAY + 10) + expect(processSpy).toHaveBeenCalledWith(initialSession) + const expectedSession = {} + expect(retrieveSessionCookie()).toEqual(expectedSession) + expect(afterSpy).toHaveBeenCalledWith(expectedSession) + }) - clock.tick(COOKIE_ACCESS_DELAY) + it('should not persist session when process return undefined', () => { + persistSessionCookie(initialSession, COOKIE_OPTIONS) + processSpy.and.returnValue(undefined) - expect(sessionStore.getSession().id).toBe(FIRST_ID) - expect(sessionStore.getSession().expire).toBe(getStoreExpiration()) - expect(expireSpy).not.toHaveBeenCalled() - }) - - it('when session id in cache is different than session id in store, should expire session', () => { - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) - setupSessionStore() - setSessionInStore(FakeTrackingType.TRACKED, SECOND_ID) - - clock.tick(COOKIE_ACCESS_DELAY) + processStorageOperations({ options: COOKIE_OPTIONS, process: processSpy, after: afterSpy }) - expect(sessionStore.getSession().id).toBeUndefined() - expect(expireSpy).toHaveBeenCalled() + expect(processSpy).toHaveBeenCalledWith(initialSession) + expect(retrieveSessionCookie()).toEqual(initialSession) + expect(afterSpy).toHaveBeenCalledWith(initialSession) + }) }) - it('when session type in cache is different than session type in store, should expire session', () => { - setSessionInStore(FakeTrackingType.NOT_TRACKED, FIRST_ID) - setupSessionStore() - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + describe('with cookie-lock enabled', () => { + beforeEach(() => { + !isChromium() && pending('cookie-lock only enabled on chromium browsers') + }) + + it('should persist session when process return a value', () => { + persistSessionCookie(initialSession, COOKIE_OPTIONS) + processSpy.and.callFake((session) => ({ ...otherSession, lock: session.lock })) + + processStorageOperations({ options: COOKIE_OPTIONS, process: processSpy, after: afterSpy }) + + expect(processSpy).toHaveBeenCalledWith({ ...initialSession, lock: jasmine.any(String) }) + const expectedSession = { ...otherSession, expire: jasmine.any(String) } + expect(retrieveSessionCookie()).toEqual(expectedSession) + expect(afterSpy).toHaveBeenCalledWith(expectedSession) + }) + + it('should clear session when process return an empty value', () => { + persistSessionCookie(initialSession, COOKIE_OPTIONS) + processSpy.and.returnValue({}) + + processStorageOperations({ options: COOKIE_OPTIONS, process: processSpy, after: afterSpy }) + + expect(processSpy).toHaveBeenCalledWith({ ...initialSession, lock: jasmine.any(String) }) + const expectedSession = {} + expect(retrieveSessionCookie()).toEqual(expectedSession) + expect(afterSpy).toHaveBeenCalledWith(expectedSession) + }) + + it('should not persist session when process return undefined', () => { + persistSessionCookie(initialSession, COOKIE_OPTIONS) + processSpy.and.returnValue(undefined) + + processStorageOperations({ options: COOKIE_OPTIONS, process: processSpy, after: afterSpy }) + + expect(processSpy).toHaveBeenCalledWith({ ...initialSession, lock: jasmine.any(String) }) + expect(retrieveSessionCookie()).toEqual(initialSession) + expect(afterSpy).toHaveBeenCalledWith(initialSession) + }) + + type OnLockCheck = () => { currentState: SessionState; retryState: SessionState } + + function lockScenario({ + onInitialLockCheck, + onAcquiredLockCheck, + onPostProcessLockCheck, + onPostPersistLockCheck, + }: { + onInitialLockCheck?: OnLockCheck + onAcquiredLockCheck?: OnLockCheck + onPostProcessLockCheck?: OnLockCheck + onPostPersistLockCheck?: OnLockCheck + }) { + const onLockChecks = [onInitialLockCheck, onAcquiredLockCheck, onPostProcessLockCheck, onPostPersistLockCheck] + cookie.getSpy.and.callFake(() => { + const currentOnLockCheck = onLockChecks.shift() + if (!currentOnLockCheck) { + return cookie.currentValue() + } + const { currentState, retryState } = currentOnLockCheck() + cookie.setCurrentValue(buildSessionString(retryState)) + return buildSessionString(currentState) + }) + } - clock.tick(COOKIE_ACCESS_DELAY) + function buildSessionString(currentState: SessionState) { + return `${SESSION_COOKIE_NAME}=${toSessionString(currentState)}` + } - expect(sessionStore.getSession().id).toBeUndefined() - expect(expireSpy).toHaveBeenCalled() + ;[ + { + description: 'should wait for lock to be free', + lockConflict: 'onInitialLockCheck', + }, + { + description: 'should retry if lock was acquired before process', + lockConflict: 'onAcquiredLockCheck', + }, + { + description: 'should retry if lock was acquired after process', + lockConflict: 'onPostProcessLockCheck', + }, + { + description: 'should retry if lock was acquired after persist', + lockConflict: 'onPostPersistLockCheck', + }, + ].forEach(({ description, lockConflict }) => { + it(description, (done) => { + lockScenario({ + [lockConflict]: () => ({ + currentState: { ...initialSession, lock: 'locked' }, + retryState: { ...initialSession, other: 'other' }, + }), + }) + persistSessionCookie(initialSession, COOKIE_OPTIONS) + processSpy.and.callFake((session) => ({ ...session, processed: 'processed' } as SessionState)) + + processStorageOperations({ + options: COOKIE_OPTIONS, + process: processSpy, + after: (afterSession) => { + // session with 'other' value on process + expect(processSpy).toHaveBeenCalledWith({ + ...initialSession, + other: 'other', + lock: jasmine.any(String), + expire: jasmine.any(String), + }) + + // end state with session 'other' and 'processed' value + const expectedSession = { + ...initialSession, + other: 'other', + processed: 'processed', + expire: jasmine.any(String), + } + expect(retrieveSessionCookie()).toEqual(expectedSession) + expect(afterSession).toEqual(expectedSession) + done() + }, + }) + }) + }) + + it('should abort after a max number of retry', () => { + const clock = mockClock() + + persistSessionCookie(initialSession, COOKIE_OPTIONS) + cookie.setSpy.calls.reset() + + cookie.getSpy.and.returnValue(buildSessionString({ ...initialSession, lock: 'locked' })) + processStorageOperations({ options: COOKIE_OPTIONS, process: processSpy, after: afterSpy }) + + clock.tick(MAX_NUMBER_OF_LOCK_RETRIES * LOCK_RETRY_DELAY) + expect(processSpy).not.toHaveBeenCalled() + expect(afterSpy).not.toHaveBeenCalled() + expect(cookie.setSpy).not.toHaveBeenCalled() + + clock.cleanup() + }) + + it('should execute cookie accesses in order', (done) => { + lockScenario({ + onInitialLockCheck: () => ({ + currentState: { ...initialSession, lock: 'locked' }, // force to retry the first access later + retryState: initialSession, + }), + }) + persistSessionCookie(initialSession, COOKIE_OPTIONS) + + processStorageOperations({ + options: COOKIE_OPTIONS, + process: (session) => ({ ...session, value: 'foo' }), + after: afterSpy, + }) + processStorageOperations({ + options: COOKIE_OPTIONS, + process: (session) => ({ ...session, value: `${session.value || ''}bar` }), + after: (session) => { + expect(session.value).toBe('foobar') + expect(afterSpy).toHaveBeenCalled() + done() + }, + }) + }) }) }) }) diff --git a/packages/core/src/domain/session/sessionStore.ts b/packages/core/src/domain/session/sessionStore.ts index a16efe7b81..278628cff0 100644 --- a/packages/core/src/domain/session/sessionStore.ts +++ b/packages/core/src/domain/session/sessionStore.ts @@ -1,12 +1,19 @@ import type { CookieOptions } from '../../browser/cookie' import { COOKIE_ACCESS_DELAY } from '../../browser/cookie' -import { clearInterval, setInterval } from '../../tools/timer' +import { clearInterval, setInterval, setTimeout } from '../../tools/timer' import { Observable } from '../../tools/observable' import { dateNow } from '../../tools/utils/timeUtils' import { throttle } from '../../tools/utils/functionUtils' import { generateUUID } from '../../tools/utils/stringUtils' import { SESSION_TIME_OUT_DELAY } from './sessionConstants' -import { deleteSessionCookie, retrieveSessionCookie, withCookieLockAccess } from './sessionCookieStore' +import { + deleteSessionCookie, + isCookieLockEnabled, + isExpiredState, + persistSessionCookie, + retrieveSessionCookie, + setSessionCookie, +} from './sessionCookieStore' import type { SessionState } from './sessionStorage' export interface SessionStore { @@ -38,7 +45,7 @@ export function startSessionStore( function expandOrRenewSession() { let isTracked: boolean - withCookieLockAccess({ + processStorageOperations({ options, process: (sessionState) => { const synchronizedSession = synchronizeSession(sessionState) @@ -55,7 +62,7 @@ export function startSessionStore( } function expandSession() { - withCookieLockAccess({ + processStorageOperations({ options, process: (sessionState) => (hasSessionInCache() ? synchronizeSession(sessionState) : undefined), }) @@ -67,7 +74,7 @@ export function startSessionStore( * - if the session is not active, clear the session cookie and expire the session cache */ function watchSession() { - withCookieLockAccess({ + processStorageOperations({ options, process: (sessionState) => (!isActiveSession(sessionState) ? {} : undefined), after: synchronizeSession, @@ -148,3 +155,94 @@ export function startSessionStore( }, } } + +// arbitrary values +export const LOCK_RETRY_DELAY = 10 +export const MAX_NUMBER_OF_LOCK_RETRIES = 100 + +type Operations = { + options: CookieOptions + process: (cookieSession: SessionState) => SessionState | undefined + after?: (cookieSession: SessionState) => void +} + +const bufferedOperations: Operations[] = [] +let ongoingOperations: Operations | undefined + +export function processStorageOperations(operations: Operations, numberOfRetries = 0) { + if (!ongoingOperations) { + ongoingOperations = operations + } + if (operations !== ongoingOperations) { + bufferedOperations.push(operations) + return + } + if (numberOfRetries >= MAX_NUMBER_OF_LOCK_RETRIES) { + next() + return + } + let currentLock: string + let currentSession = retrieveSessionCookie() + if (isCookieLockEnabled()) { + // if someone has lock, retry later + if (currentSession.lock) { + retryLater(operations, numberOfRetries) + return + } + // acquire lock + currentLock = generateUUID() + currentSession.lock = currentLock + setSessionCookie(currentSession, operations.options) + // if lock is not acquired, retry later + currentSession = retrieveSessionCookie() + if (currentSession.lock !== currentLock) { + retryLater(operations, numberOfRetries) + return + } + } + let processedSession = operations.process(currentSession) + if (isCookieLockEnabled()) { + // if lock corrupted after process, retry later + currentSession = retrieveSessionCookie() + if (currentSession.lock !== currentLock!) { + retryLater(operations, numberOfRetries) + return + } + } + if (processedSession) { + persistSessionCookie(processedSession, operations.options) + } + if (isCookieLockEnabled()) { + // correctly handle lock around expiration would require to handle this case properly at several levels + // since we don't have evidence of lock issues around expiration, let's just not do the corruption check for it + if (!(processedSession && isExpiredState(processedSession))) { + // if lock corrupted after persist, retry later + currentSession = retrieveSessionCookie() + if (currentSession.lock !== currentLock!) { + retryLater(operations, numberOfRetries) + return + } + delete currentSession.lock + setSessionCookie(currentSession, operations.options) + processedSession = currentSession + } + } + // call after even if session is not persisted in order to perform operations on + // up-to-date cookie value, the value could have been modified by another tab + operations.after?.(processedSession || currentSession) + next() +} + +function retryLater(operations: Operations, currentNumberOfRetries: number) { + setTimeout(() => { + processStorageOperations(operations, currentNumberOfRetries + 1) + }, LOCK_RETRY_DELAY) +} + +function next() { + ongoingOperations = undefined + const nextOperations = bufferedOperations.shift() + if (nextOperations) { + processStorageOperations(nextOperations) + } +} From ce2f8157ef3dc22497e046fe8ab6c8411e47df7b Mon Sep 17 00:00:00 2001 From: Yannick Adam Date: Thu, 11 May 2023 11:49:50 +0200 Subject: [PATCH 04/13] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20=20Refactor=20Sessio?= =?UTF-8?q?nCookieStore=20to=20use=20SessionStorage=20interface?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/domain/session/oldCookiesMigration.ts | 13 +- .../domain/session/sessionCookieStore.spec.ts | 29 ++-- .../src/domain/session/sessionCookieStore.ts | 31 ++--- .../src/domain/session/sessionManager.spec.ts | 4 +- .../src/domain/session/sessionStorage.spec.ts | 15 ++ .../core/src/domain/session/sessionStorage.ts | 6 + .../src/domain/session/sessionStore.spec.ts | 129 ++++++++++-------- .../core/src/domain/session/sessionStore.ts | 117 ++++++++-------- 8 files changed, 193 insertions(+), 151 deletions(-) create mode 100644 packages/core/src/domain/session/sessionStorage.spec.ts diff --git a/packages/core/src/domain/session/oldCookiesMigration.ts b/packages/core/src/domain/session/oldCookiesMigration.ts index 486f7f82c5..02937b4129 100644 --- a/packages/core/src/domain/session/oldCookiesMigration.ts +++ b/packages/core/src/domain/session/oldCookiesMigration.ts @@ -1,7 +1,10 @@ import type { CookieOptions } from '../../browser/cookie' import { getCookie } from '../../browser/cookie' +import { dateNow } from '../../tools/utils/timeUtils' import type { SessionState } from './sessionStorage' -import { SESSION_COOKIE_NAME, persistSessionCookie } from './sessionCookieStore' +import { isSessionInExpiredState } from './sessionStorage' +import { SESSION_COOKIE_NAME, deleteSessionCookie, persistSessionCookie } from './sessionCookieStore' +import { SESSION_EXPIRATION_DELAY } from './sessionConstants' export const OLD_SESSION_COOKIE_NAME = '_dd' export const OLD_RUM_COOKIE_NAME = '_dd_r' @@ -31,6 +34,12 @@ export function tryOldCookiesMigration(options: CookieOptions) { if (oldRumType && /^[012]$/.test(oldRumType)) { session[RUM_SESSION_KEY] = oldRumType } - persistSessionCookie(session, options) + + if (!isSessionInExpiredState(session)) { + session.expire = String(dateNow() + SESSION_EXPIRATION_DELAY) + persistSessionCookie(options)(session) + } else { + deleteSessionCookie(options)() + } } } diff --git a/packages/core/src/domain/session/sessionCookieStore.spec.ts b/packages/core/src/domain/session/sessionCookieStore.spec.ts index 8fe1034724..5ca34e57d4 100644 --- a/packages/core/src/domain/session/sessionCookieStore.spec.ts +++ b/packages/core/src/domain/session/sessionCookieStore.spec.ts @@ -1,41 +1,38 @@ import type { CookieOptions } from '../../browser/cookie' import { setCookie, deleteCookie } from '../../browser/cookie' -import { SESSION_EXPIRATION_DELAY } from './sessionConstants' -import { - SESSION_COOKIE_NAME, - deleteSessionCookie, - persistSessionCookie, - retrieveSessionCookie, -} from './sessionCookieStore' +import { SESSION_COOKIE_NAME, initCookieStorage } from './sessionCookieStore' -import type { SessionState } from './sessionStorage' +import type { SessionState, SessionStorage } from './sessionStorage' describe('session cookie store', () => { const sessionState: SessionState = { id: '123', created: '0' } const noOptions: CookieOptions = {} + let cookieStorage: SessionStorage + + beforeEach(() => { + cookieStorage = initCookieStorage(noOptions) + }) afterEach(() => { deleteCookie(SESSION_COOKIE_NAME) }) it('should persist a session in a cookie', () => { - const now = Date.now() - persistSessionCookie(sessionState, noOptions) - const session = retrieveSessionCookie() + cookieStorage.persistSession(sessionState) + const session = cookieStorage.retrieveSession() expect(session).toEqual({ ...sessionState }) - expect(+session.expire!).toBeGreaterThanOrEqual(now + SESSION_EXPIRATION_DELAY) }) it('should delete the cookie holding the session', () => { - persistSessionCookie(sessionState, noOptions) - deleteSessionCookie(noOptions) - const session = retrieveSessionCookie() + cookieStorage.persistSession(sessionState) + cookieStorage.clearSession() + const session = cookieStorage.retrieveSession() expect(session).toEqual({}) }) it('should return an empty object if session string is invalid', () => { setCookie(SESSION_COOKIE_NAME, '{test:42}', 1000) - const session = retrieveSessionCookie() + const session = cookieStorage.retrieveSession() expect(session).toEqual({}) }) }) diff --git a/packages/core/src/domain/session/sessionCookieStore.ts b/packages/core/src/domain/session/sessionCookieStore.ts index ab455fb731..e90354f09c 100644 --- a/packages/core/src/domain/session/sessionCookieStore.ts +++ b/packages/core/src/domain/session/sessionCookieStore.ts @@ -1,28 +1,27 @@ import type { CookieOptions } from '../../browser/cookie' import { deleteCookie, getCookie, setCookie } from '../../browser/cookie' import { isChromium } from '../../tools/utils/browserDetection' -import { dateNow } from '../../tools/utils/timeUtils' import { objectEntries } from '../../tools/utils/polyfills' -import { isEmptyObject } from '../../tools/utils/objectUtils' import { SESSION_EXPIRATION_DELAY } from './sessionConstants' -import type { SessionState } from './sessionStorage' +import type { SessionState, SessionStorage } from './sessionStorage' const SESSION_ENTRY_REGEXP = /^([a-z]+)=([a-z0-9-]+)$/ const SESSION_ENTRY_SEPARATOR = '&' export const SESSION_COOKIE_NAME = '_dd_s' -export function persistSessionCookie(session: SessionState, options: CookieOptions) { - if (isExpiredState(session)) { - deleteSessionCookie(options) - return +export function initCookieStorage(options: CookieOptions): SessionStorage { + return { + persistSession: persistSessionCookie(options), + retrieveSession: retrieveSessionCookie, + clearSession: deleteSessionCookie(options), } - session.expire = String(dateNow() + SESSION_EXPIRATION_DELAY) - setSessionCookie(session, options) } -export function setSessionCookie(session: SessionState, options: CookieOptions) { - setCookie(SESSION_COOKIE_NAME, toSessionString(session), SESSION_EXPIRATION_DELAY, options) +export function persistSessionCookie(options: CookieOptions) { + return (session: SessionState) => { + setCookie(SESSION_COOKIE_NAME, toSessionString(session), SESSION_EXPIRATION_DELAY, options) + } } export function toSessionString(session: SessionState) { @@ -31,7 +30,7 @@ export function toSessionString(session: SessionState) { .join(SESSION_ENTRY_SEPARATOR) } -export function retrieveSessionCookie(): SessionState { +function retrieveSessionCookie(): SessionState { const sessionString = getCookie(SESSION_COOKIE_NAME) const session: SessionState = {} if (isValidSessionString(sessionString)) { @@ -47,7 +46,9 @@ export function retrieveSessionCookie(): SessionState { } export function deleteSessionCookie(options: CookieOptions) { - deleteCookie(SESSION_COOKIE_NAME, options) + return () => { + deleteCookie(SESSION_COOKIE_NAME, options) + } } function isValidSessionString(sessionString: string | undefined): sessionString is string { @@ -57,10 +58,6 @@ function isValidSessionString(sessionString: string | undefined): sessionString ) } -export function isExpiredState(session: SessionState) { - return isEmptyObject(session) -} - /** * Cookie lock strategy allows mitigating issues due to concurrent access to cookie. * This issue concerns only chromium browsers and enabling this on firefox increase cookie write failures. diff --git a/packages/core/src/domain/session/sessionManager.spec.ts b/packages/core/src/domain/session/sessionManager.spec.ts index a2cbc72d52..90c27f9f79 100644 --- a/packages/core/src/domain/session/sessionManager.spec.ts +++ b/packages/core/src/domain/session/sessionManager.spec.ts @@ -19,12 +19,12 @@ const enum FakeTrackingType { const TRACKED_SESSION_STATE = { isTracked: true, trackingType: FakeTrackingType.TRACKED, -} +} as const const NOT_TRACKED_SESSION_STATE = { isTracked: false, trackingType: FakeTrackingType.NOT_TRACKED, -} +} as const describe('startSessionManager', () => { const DURATION = 123456 diff --git a/packages/core/src/domain/session/sessionStorage.spec.ts b/packages/core/src/domain/session/sessionStorage.spec.ts new file mode 100644 index 0000000000..e3b8de5dba --- /dev/null +++ b/packages/core/src/domain/session/sessionStorage.spec.ts @@ -0,0 +1,15 @@ +import type { SessionState } from './sessionStorage' +import { isSessionInExpiredState } from './sessionStorage' + +describe('session storage utilities', () => { + const EXPIRED_SESSION: SessionState = {} + const LIVE_SESSION: SessionState = { created: '0', id: '123' } + + it('should correctly identify a session in expired state', () => { + expect(isSessionInExpiredState(EXPIRED_SESSION)).toBe(true) + }) + + it('should correctly identify a session in live state', () => { + expect(isSessionInExpiredState(LIVE_SESSION)).toBe(false) + }) +}) diff --git a/packages/core/src/domain/session/sessionStorage.ts b/packages/core/src/domain/session/sessionStorage.ts index bdb4f2e8b8..8b21dc5be0 100644 --- a/packages/core/src/domain/session/sessionStorage.ts +++ b/packages/core/src/domain/session/sessionStorage.ts @@ -1,3 +1,5 @@ +import { isEmptyObject } from '../../tools/utils/objectUtils' + export interface SessionState { id?: string created?: string @@ -12,3 +14,7 @@ export interface SessionStorage { retrieveSession: () => SessionState clearSession: () => void } + +export function isSessionInExpiredState(session: SessionState) { + return isEmptyObject(session) +} diff --git a/packages/core/src/domain/session/sessionStore.spec.ts b/packages/core/src/domain/session/sessionStore.spec.ts index eab672cf00..6a2f8ae3bb 100644 --- a/packages/core/src/domain/session/sessionStore.spec.ts +++ b/packages/core/src/domain/session/sessionStore.spec.ts @@ -10,9 +10,9 @@ import { processStorageOperations, startSessionStore, } from './sessionStore' -import { SESSION_COOKIE_NAME, persistSessionCookie, retrieveSessionCookie, toSessionString } from './sessionCookieStore' +import { SESSION_COOKIE_NAME, initCookieStorage, toSessionString } from './sessionCookieStore' import { SESSION_EXPIRATION_DELAY, SESSION_TIME_OUT_DELAY } from './sessionConstants' -import type { SessionState } from './sessionStorage' +import type { SessionState, SessionStorage } from './sessionStorage' const enum FakeTrackingType { TRACKED = 'tracked', @@ -366,8 +366,10 @@ describe('session store', () => { let processSpy: jasmine.Spy let afterSpy: jasmine.Spy let cookie: ReturnType + let cookieStorage: SessionStorage beforeEach(() => { + cookieStorage = initCookieStorage(COOKIE_OPTIONS) initialSession = { id: '123', created: '0' } otherSession = { id: '456', created: '100' } processSpy = jasmine.createSpy('process') @@ -381,37 +383,37 @@ describe('session store', () => { }) it('should persist session when process returns a value', () => { - persistSessionCookie(initialSession, COOKIE_OPTIONS) + cookieStorage.persistSession(initialSession) processSpy.and.returnValue({ ...otherSession }) - processStorageOperations({ options: COOKIE_OPTIONS, process: processSpy, after: afterSpy }) + processStorageOperations({ process: processSpy, after: afterSpy }, cookieStorage) expect(processSpy).toHaveBeenCalledWith(initialSession) const expectedSession = { ...otherSession, expire: jasmine.any(String) } - expect(retrieveSessionCookie()).toEqual(expectedSession) + expect(cookieStorage.retrieveSession()).toEqual(expectedSession) expect(afterSpy).toHaveBeenCalledWith(expectedSession) }) it('should clear session when process return an empty value', () => { - persistSessionCookie(initialSession, COOKIE_OPTIONS) + cookieStorage.persistSession(initialSession) processSpy.and.returnValue({}) - processStorageOperations({ options: COOKIE_OPTIONS, process: processSpy, after: afterSpy }) + processStorageOperations({ process: processSpy, after: afterSpy }, cookieStorage) expect(processSpy).toHaveBeenCalledWith(initialSession) const expectedSession = {} - expect(retrieveSessionCookie()).toEqual(expectedSession) + expect(cookieStorage.retrieveSession()).toEqual(expectedSession) expect(afterSpy).toHaveBeenCalledWith(expectedSession) }) it('should not persist session when process return undefined', () => { - persistSessionCookie(initialSession, COOKIE_OPTIONS) + cookieStorage.persistSession(initialSession) processSpy.and.returnValue(undefined) - processStorageOperations({ options: COOKIE_OPTIONS, process: processSpy, after: afterSpy }) + processStorageOperations({ process: processSpy, after: afterSpy }, cookieStorage) expect(processSpy).toHaveBeenCalledWith(initialSession) - expect(retrieveSessionCookie()).toEqual(initialSession) + expect(cookieStorage.retrieveSession()).toEqual(initialSession) expect(afterSpy).toHaveBeenCalledWith(initialSession) }) }) @@ -422,37 +424,37 @@ describe('session store', () => { }) it('should persist session when process return a value', () => { - persistSessionCookie(initialSession, COOKIE_OPTIONS) + cookieStorage.persistSession(initialSession) processSpy.and.callFake((session) => ({ ...otherSession, lock: session.lock })) - processStorageOperations({ options: COOKIE_OPTIONS, process: processSpy, after: afterSpy }) + processStorageOperations({ process: processSpy, after: afterSpy }, cookieStorage) expect(processSpy).toHaveBeenCalledWith({ ...initialSession, lock: jasmine.any(String) }) const expectedSession = { ...otherSession, expire: jasmine.any(String) } - expect(retrieveSessionCookie()).toEqual(expectedSession) + expect(cookieStorage.retrieveSession()).toEqual(expectedSession) expect(afterSpy).toHaveBeenCalledWith(expectedSession) }) it('should clear session when process return an empty value', () => { - persistSessionCookie(initialSession, COOKIE_OPTIONS) + cookieStorage.persistSession(initialSession) processSpy.and.returnValue({}) - processStorageOperations({ options: COOKIE_OPTIONS, process: processSpy, after: afterSpy }) + processStorageOperations({ process: processSpy, after: afterSpy }, cookieStorage) expect(processSpy).toHaveBeenCalledWith({ ...initialSession, lock: jasmine.any(String) }) const expectedSession = {} - expect(retrieveSessionCookie()).toEqual(expectedSession) + expect(cookieStorage.retrieveSession()).toEqual(expectedSession) expect(afterSpy).toHaveBeenCalledWith(expectedSession) }) it('should not persist session when process return undefined', () => { - persistSessionCookie(initialSession, COOKIE_OPTIONS) + cookieStorage.persistSession(initialSession) processSpy.and.returnValue(undefined) - processStorageOperations({ options: COOKIE_OPTIONS, process: processSpy, after: afterSpy }) + processStorageOperations({ process: processSpy, after: afterSpy }, cookieStorage) expect(processSpy).toHaveBeenCalledWith({ ...initialSession, lock: jasmine.any(String) }) - expect(retrieveSessionCookie()).toEqual(initialSession) + expect(cookieStorage.retrieveSession()).toEqual(initialSession) expect(afterSpy).toHaveBeenCalledWith(initialSession) }) @@ -510,44 +512,47 @@ describe('session store', () => { retryState: { ...initialSession, other: 'other' }, }), }) - persistSessionCookie(initialSession, COOKIE_OPTIONS) + initialSession.expire = String(Date.now() + SESSION_EXPIRATION_DELAY) + cookieStorage.persistSession(initialSession) processSpy.and.callFake((session) => ({ ...session, processed: 'processed' } as SessionState)) - processStorageOperations({ - options: COOKIE_OPTIONS, - process: processSpy, - after: (afterSession) => { - // session with 'other' value on process - expect(processSpy).toHaveBeenCalledWith({ - ...initialSession, - other: 'other', - lock: jasmine.any(String), - expire: jasmine.any(String), - }) - - // end state with session 'other' and 'processed' value - const expectedSession = { - ...initialSession, - other: 'other', - processed: 'processed', - expire: jasmine.any(String), - } - expect(retrieveSessionCookie()).toEqual(expectedSession) - expect(afterSession).toEqual(expectedSession) - done() + processStorageOperations( + { + process: processSpy, + after: (afterSession) => { + // session with 'other' value on process + expect(processSpy).toHaveBeenCalledWith({ + ...initialSession, + other: 'other', + lock: jasmine.any(String), + expire: jasmine.any(String), + }) + + // end state with session 'other' and 'processed' value + const expectedSession = { + ...initialSession, + other: 'other', + processed: 'processed', + expire: jasmine.any(String), + } + expect(cookieStorage.retrieveSession()).toEqual(expectedSession) + expect(afterSession).toEqual(expectedSession) + done() + }, }, - }) + cookieStorage + ) }) }) it('should abort after a max number of retry', () => { const clock = mockClock() - persistSessionCookie(initialSession, COOKIE_OPTIONS) + cookieStorage.persistSession(initialSession) cookie.setSpy.calls.reset() cookie.getSpy.and.returnValue(buildSessionString({ ...initialSession, lock: 'locked' })) - processStorageOperations({ options: COOKIE_OPTIONS, process: processSpy, after: afterSpy }) + processStorageOperations({ process: processSpy, after: afterSpy }, cookieStorage) clock.tick(MAX_NUMBER_OF_LOCK_RETRIES * LOCK_RETRY_DELAY) expect(processSpy).not.toHaveBeenCalled() @@ -564,22 +569,26 @@ describe('session store', () => { retryState: initialSession, }), }) - persistSessionCookie(initialSession, COOKIE_OPTIONS) + cookieStorage.persistSession(initialSession) - processStorageOperations({ - options: COOKIE_OPTIONS, - process: (session) => ({ ...session, value: 'foo' }), - after: afterSpy, - }) - processStorageOperations({ - options: COOKIE_OPTIONS, - process: (session) => ({ ...session, value: `${session.value || ''}bar` }), - after: (session) => { - expect(session.value).toBe('foobar') - expect(afterSpy).toHaveBeenCalled() - done() + processStorageOperations( + { + process: (session) => ({ ...session, value: 'foo' }), + after: afterSpy, }, - }) + cookieStorage + ) + processStorageOperations( + { + process: (session) => ({ ...session, value: `${session.value || ''}bar` }), + after: (session) => { + expect(session.value).toBe('foobar') + expect(afterSpy).toHaveBeenCalled() + done() + }, + }, + cookieStorage + ) }) }) }) diff --git a/packages/core/src/domain/session/sessionStore.ts b/packages/core/src/domain/session/sessionStore.ts index 278628cff0..46982ad573 100644 --- a/packages/core/src/domain/session/sessionStore.ts +++ b/packages/core/src/domain/session/sessionStore.ts @@ -5,16 +5,10 @@ import { Observable } from '../../tools/observable' import { dateNow } from '../../tools/utils/timeUtils' import { throttle } from '../../tools/utils/functionUtils' import { generateUUID } from '../../tools/utils/stringUtils' -import { SESSION_TIME_OUT_DELAY } from './sessionConstants' -import { - deleteSessionCookie, - isCookieLockEnabled, - isExpiredState, - persistSessionCookie, - retrieveSessionCookie, - setSessionCookie, -} from './sessionCookieStore' -import type { SessionState } from './sessionStorage' +import { SESSION_EXPIRATION_DELAY, SESSION_TIME_OUT_DELAY } from './sessionConstants' +import { isCookieLockEnabled, initCookieStorage } from './sessionCookieStore' +import type { SessionState, SessionStorage } from './sessionStorage' +import { isSessionInExpiredState } from './sessionStorage' export interface SessionStore { expandOrRenewSession: () => void @@ -40,32 +34,39 @@ export function startSessionStore( const renewObservable = new Observable() const expireObservable = new Observable() + const sessionStorage = initCookieStorage(options) + const { clearSession, retrieveSession } = sessionStorage + const watchSessionTimeoutId = setInterval(watchSession, COOKIE_ACCESS_DELAY) let sessionCache: SessionState = retrieveActiveSession() function expandOrRenewSession() { let isTracked: boolean - processStorageOperations({ - options, - process: (sessionState) => { - const synchronizedSession = synchronizeSession(sessionState) - isTracked = expandOrRenewCookie(synchronizedSession) - return synchronizedSession - }, - after: (sessionState) => { - if (isTracked && !hasSessionInCache()) { - renewSessionInCache(sessionState) - } - sessionCache = sessionState + processStorageOperations( + { + process: (sessionState) => { + const synchronizedSession = synchronizeSession(sessionState) + isTracked = expandOrRenewCookie(synchronizedSession) + return synchronizedSession + }, + after: (sessionState) => { + if (isTracked && !hasSessionInCache()) { + renewSessionInCache(sessionState) + } + sessionCache = sessionState + }, }, - }) + sessionStorage + ) } function expandSession() { - processStorageOperations({ - options, - process: (sessionState) => (hasSessionInCache() ? synchronizeSession(sessionState) : undefined), - }) + processStorageOperations( + { + process: (sessionState) => (hasSessionInCache() ? synchronizeSession(sessionState) : undefined), + }, + sessionStorage + ) } /** @@ -74,11 +75,13 @@ export function startSessionStore( * - if the session is not active, clear the session cookie and expire the session cache */ function watchSession() { - processStorageOperations({ - options, - process: (sessionState) => (!isActiveSession(sessionState) ? {} : undefined), - after: synchronizeSession, - }) + processStorageOperations( + { + process: (sessionState) => (!isActiveSession(sessionState) ? {} : undefined), + after: synchronizeSession, + }, + sessionStorage + ) } function synchronizeSession(sessionState: SessionState) { @@ -124,7 +127,7 @@ export function startSessionStore( } function retrieveActiveSession(): SessionState { - const session = retrieveSessionCookie() + const session = retrieveSession() if (isActiveSession(session)) { return session } @@ -147,7 +150,7 @@ export function startSessionStore( renewObservable, expireObservable, expire: () => { - deleteSessionCookie(options) + clearSession() synchronizeSession({}) }, stop: () => { @@ -161,7 +164,6 @@ export const LOCK_RETRY_DELAY = 10 export const MAX_NUMBER_OF_LOCK_RETRIES = 100 type Operations = { - options: CookieOptions process: (cookieSession: SessionState) => SessionState | undefined after?: (cookieSession: SessionState) => void } @@ -169,7 +171,9 @@ type Operations = { const bufferedOperations: Operations[] = [] let ongoingOperations: Operations | undefined -export function processStorageOperations(operations: Operations, numberOfRetries = 0) { +export function processStorageOperations(operations: Operations, sessionStorage: SessionStorage, numberOfRetries = 0) { + const { retrieveSession, persistSession, clearSession } = sessionStorage + if (!ongoingOperations) { ongoingOperations = operations } @@ -178,71 +182,76 @@ export function processStorageOperations(operations: Operations, numberOfRetries return } if (numberOfRetries >= MAX_NUMBER_OF_LOCK_RETRIES) { - next() + next(sessionStorage) return } let currentLock: string - let currentSession = retrieveSessionCookie() + let currentSession = retrieveSession() if (isCookieLockEnabled()) { // if someone has lock, retry later if (currentSession.lock) { - retryLater(operations, numberOfRetries) + retryLater(operations, sessionStorage, numberOfRetries) return } // acquire lock currentLock = generateUUID() currentSession.lock = currentLock - setSessionCookie(currentSession, operations.options) + persistSession(currentSession) // if lock is not acquired, retry later - currentSession = retrieveSessionCookie() + currentSession = retrieveSession() if (currentSession.lock !== currentLock) { - retryLater(operations, numberOfRetries) + retryLater(operations, sessionStorage, numberOfRetries) return } } let processedSession = operations.process(currentSession) if (isCookieLockEnabled()) { // if lock corrupted after process, retry later - currentSession = retrieveSessionCookie() + currentSession = retrieveSession() if (currentSession.lock !== currentLock!) { - retryLater(operations, numberOfRetries) + retryLater(operations, sessionStorage, numberOfRetries) return } } if (processedSession) { - persistSessionCookie(processedSession, operations.options) + if (isSessionInExpiredState(processedSession)) { + clearSession() + } else { + processedSession.expire = String(dateNow() + SESSION_EXPIRATION_DELAY) + persistSession(processedSession) + } } if (isCookieLockEnabled()) { // correctly handle lock around expiration would require to handle this case properly at several levels // since we don't have evidence of lock issues around expiration, let's just not do the corruption check for it - if (!(processedSession && isExpiredState(processedSession))) { + if (!(processedSession && isSessionInExpiredState(processedSession))) { // if lock corrupted after persist, retry later - currentSession = retrieveSessionCookie() + currentSession = retrieveSession() if (currentSession.lock !== currentLock!) { - retryLater(operations, numberOfRetries) + retryLater(operations, sessionStorage, numberOfRetries) return } delete currentSession.lock - setSessionCookie(currentSession, operations.options) + persistSession(currentSession) processedSession = currentSession } } // call after even if session is not persisted in order to perform operations on // up-to-date cookie value, the value could have been modified by another tab operations.after?.(processedSession || currentSession) - next() + next(sessionStorage) } -function retryLater(operations: Operations, currentNumberOfRetries: number) { +function retryLater(operations: Operations, sessionStorage: SessionStorage, currentNumberOfRetries: number) { setTimeout(() => { - processStorageOperations(operations, currentNumberOfRetries + 1) + processStorageOperations(operations, sessionStorage, currentNumberOfRetries + 1) }, LOCK_RETRY_DELAY) } -function next() { +function next(sessionStorage: SessionStorage) { ongoingOperations = undefined const nextOperations = bufferedOperations.shift() if (nextOperations) { - processStorageOperations(nextOperations) + processStorageOperations(nextOperations, sessionStorage) } } From f26717e8a6e1703f0daf8910302470872e39e2f2 Mon Sep 17 00:00:00 2001 From: Yannick Adam Date: Mon, 15 May 2023 10:56:28 +0200 Subject: [PATCH 05/13] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20=20Update=20session?= =?UTF-8?q?=20storage=20interface=20and=20move=20back=20lock=20configurati?= =?UTF-8?q?on=20to=20storage?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/domain/session/sessionCookieStore.ts | 24 +++++---- .../core/src/domain/session/sessionStorage.ts | 18 +++++++ .../src/domain/session/sessionStore.spec.ts | 16 +++--- .../core/src/domain/session/sessionStore.ts | 49 +++++++++---------- 4 files changed, 66 insertions(+), 41 deletions(-) diff --git a/packages/core/src/domain/session/sessionCookieStore.ts b/packages/core/src/domain/session/sessionCookieStore.ts index e90354f09c..d887916aeb 100644 --- a/packages/core/src/domain/session/sessionCookieStore.ts +++ b/packages/core/src/domain/session/sessionCookieStore.ts @@ -1,5 +1,5 @@ import type { CookieOptions } from '../../browser/cookie' -import { deleteCookie, getCookie, setCookie } from '../../browser/cookie' +import { COOKIE_ACCESS_DELAY, deleteCookie, getCookie, setCookie } from '../../browser/cookie' import { isChromium } from '../../tools/utils/browserDetection' import { objectEntries } from '../../tools/utils/polyfills' import { SESSION_EXPIRATION_DELAY } from './sessionConstants' @@ -10,8 +10,22 @@ const SESSION_ENTRY_SEPARATOR = '&' export const SESSION_COOKIE_NAME = '_dd_s' +// Arbitrary values +export const LOCK_RETRY_DELAY = 10 +export const MAX_NUMBER_OF_LOCK_RETRIES = 100 + export function initCookieStorage(options: CookieOptions): SessionStorage { return { + storageAccessOptions: { + pollDelay: COOKIE_ACCESS_DELAY, + /** + * Cookie lock strategy allows mitigating issues due to concurrent access to cookie. + * This issue concerns only chromium browsers and enabling this on firefox increase cookie write failures. + */ + lockEnabled: isChromium(), + lockRetryDelay: LOCK_RETRY_DELAY, + lockMaxTries: MAX_NUMBER_OF_LOCK_RETRIES, + }, persistSession: persistSessionCookie(options), retrieveSession: retrieveSessionCookie, clearSession: deleteSessionCookie(options), @@ -57,11 +71,3 @@ function isValidSessionString(sessionString: string | undefined): sessionString (sessionString.indexOf(SESSION_ENTRY_SEPARATOR) !== -1 || SESSION_ENTRY_REGEXP.test(sessionString)) ) } - -/** - * Cookie lock strategy allows mitigating issues due to concurrent access to cookie. - * This issue concerns only chromium browsers and enabling this on firefox increase cookie write failures. - */ -export function isCookieLockEnabled() { - return isChromium() -} diff --git a/packages/core/src/domain/session/sessionStorage.ts b/packages/core/src/domain/session/sessionStorage.ts index 8b21dc5be0..45e8ec04b6 100644 --- a/packages/core/src/domain/session/sessionStorage.ts +++ b/packages/core/src/domain/session/sessionStorage.ts @@ -1,3 +1,4 @@ +import type { CookieOptions } from '../../browser/cookie' import { isEmptyObject } from '../../tools/utils/objectUtils' export interface SessionState { @@ -9,7 +10,24 @@ export interface SessionState { [key: string]: string | undefined } +interface StorageAccessOptionsWithLock { + pollDelay: number + lockEnabled: true + lockRetryDelay: number + lockMaxTries: number +} + +interface StorageAccessOptionsWithoutLock { + pollDelay: number + lockEnabled: false +} + +type StorageAccessOptions = StorageAccessOptionsWithLock | StorageAccessOptionsWithoutLock + +export type StorageInitOptions = CookieOptions + export interface SessionStorage { + storageAccessOptions: StorageAccessOptions persistSession: (session: SessionState) => void retrieveSession: () => SessionState clearSession: () => void diff --git a/packages/core/src/domain/session/sessionStore.spec.ts b/packages/core/src/domain/session/sessionStore.spec.ts index 6a2f8ae3bb..70d122b642 100644 --- a/packages/core/src/domain/session/sessionStore.spec.ts +++ b/packages/core/src/domain/session/sessionStore.spec.ts @@ -4,12 +4,7 @@ import type { CookieOptions } from '../../browser/cookie' import { getCookie, setCookie, COOKIE_ACCESS_DELAY } from '../../browser/cookie' import { isChromium } from '../../tools/utils/browserDetection' import type { SessionStore } from './sessionStore' -import { - LOCK_RETRY_DELAY, - MAX_NUMBER_OF_LOCK_RETRIES, - processStorageOperations, - startSessionStore, -} from './sessionStore' +import { processStorageOperations, startSessionStore } from './sessionStore' import { SESSION_COOKIE_NAME, initCookieStorage, toSessionString } from './sessionCookieStore' import { SESSION_EXPIRATION_DELAY, SESSION_TIME_OUT_DELAY } from './sessionConstants' import type { SessionState, SessionStorage } from './sessionStorage' @@ -554,7 +549,14 @@ describe('session store', () => { cookie.getSpy.and.returnValue(buildSessionString({ ...initialSession, lock: 'locked' })) processStorageOperations({ process: processSpy, after: afterSpy }, cookieStorage) - clock.tick(MAX_NUMBER_OF_LOCK_RETRIES * LOCK_RETRY_DELAY) + const lockMaxTries = cookieStorage.storageAccessOptions.lockEnabled + ? cookieStorage.storageAccessOptions.lockMaxTries + : 0 + const lockRetryDelay = cookieStorage.storageAccessOptions.lockEnabled + ? cookieStorage.storageAccessOptions.lockRetryDelay + : 0 + + clock.tick(lockMaxTries * lockRetryDelay) expect(processSpy).not.toHaveBeenCalled() expect(afterSpy).not.toHaveBeenCalled() expect(cookie.setSpy).not.toHaveBeenCalled() diff --git a/packages/core/src/domain/session/sessionStore.ts b/packages/core/src/domain/session/sessionStore.ts index 46982ad573..b2001c5e84 100644 --- a/packages/core/src/domain/session/sessionStore.ts +++ b/packages/core/src/domain/session/sessionStore.ts @@ -1,13 +1,11 @@ -import type { CookieOptions } from '../../browser/cookie' -import { COOKIE_ACCESS_DELAY } from '../../browser/cookie' import { clearInterval, setInterval, setTimeout } from '../../tools/timer' import { Observable } from '../../tools/observable' import { dateNow } from '../../tools/utils/timeUtils' import { throttle } from '../../tools/utils/functionUtils' import { generateUUID } from '../../tools/utils/stringUtils' import { SESSION_EXPIRATION_DELAY, SESSION_TIME_OUT_DELAY } from './sessionConstants' -import { isCookieLockEnabled, initCookieStorage } from './sessionCookieStore' -import type { SessionState, SessionStorage } from './sessionStorage' +import { initCookieStorage } from './sessionCookieStore' +import type { SessionState, SessionStorage, StorageInitOptions } from './sessionStorage' import { isSessionInExpiredState } from './sessionStorage' export interface SessionStore { @@ -27,7 +25,7 @@ export interface SessionStore { * - inactive, no session in store or session expired, waiting for a renew session */ export function startSessionStore( - options: CookieOptions, + options: StorageInitOptions, productKey: string, computeSessionState: (rawTrackingType?: string) => { trackingType: TrackingType; isTracked: boolean } ): SessionStore { @@ -35,9 +33,9 @@ export function startSessionStore( const expireObservable = new Observable() const sessionStorage = initCookieStorage(options) - const { clearSession, retrieveSession } = sessionStorage + const { clearSession, retrieveSession, storageAccessOptions } = sessionStorage - const watchSessionTimeoutId = setInterval(watchSession, COOKIE_ACCESS_DELAY) + const watchSessionTimeoutId = setInterval(watchSession, storageAccessOptions.pollDelay) let sessionCache: SessionState = retrieveActiveSession() function expandOrRenewSession() { @@ -144,7 +142,7 @@ export function startSessionStore( } return { - expandOrRenewSession: throttle(expandOrRenewSession, COOKIE_ACCESS_DELAY).throttled, + expandOrRenewSession: throttle(expandOrRenewSession, storageAccessOptions.pollDelay).throttled, expandSession, getSession: () => sessionCache, renewObservable, @@ -159,20 +157,16 @@ export function startSessionStore( } } -// arbitrary values -export const LOCK_RETRY_DELAY = 10 -export const MAX_NUMBER_OF_LOCK_RETRIES = 100 - type Operations = { - process: (cookieSession: SessionState) => SessionState | undefined - after?: (cookieSession: SessionState) => void + process: (sessionState: SessionState) => SessionState | undefined + after?: (sessionState: SessionState) => void } const bufferedOperations: Operations[] = [] let ongoingOperations: Operations | undefined export function processStorageOperations(operations: Operations, sessionStorage: SessionStorage, numberOfRetries = 0) { - const { retrieveSession, persistSession, clearSession } = sessionStorage + const { retrieveSession, persistSession, clearSession, storageAccessOptions } = sessionStorage if (!ongoingOperations) { ongoingOperations = operations @@ -181,16 +175,16 @@ export function processStorageOperations(operations: Operations, sessionStorage: bufferedOperations.push(operations) return } - if (numberOfRetries >= MAX_NUMBER_OF_LOCK_RETRIES) { + if (storageAccessOptions.lockEnabled && numberOfRetries >= storageAccessOptions.lockMaxTries) { next(sessionStorage) return } let currentLock: string let currentSession = retrieveSession() - if (isCookieLockEnabled()) { + if (storageAccessOptions.lockEnabled) { // if someone has lock, retry later if (currentSession.lock) { - retryLater(operations, sessionStorage, numberOfRetries) + retryLater(operations, sessionStorage, numberOfRetries, storageAccessOptions.lockRetryDelay) return } // acquire lock @@ -200,16 +194,16 @@ export function processStorageOperations(operations: Operations, sessionStorage: // if lock is not acquired, retry later currentSession = retrieveSession() if (currentSession.lock !== currentLock) { - retryLater(operations, sessionStorage, numberOfRetries) + retryLater(operations, sessionStorage, numberOfRetries, storageAccessOptions.lockRetryDelay) return } } let processedSession = operations.process(currentSession) - if (isCookieLockEnabled()) { + if (storageAccessOptions.lockEnabled) { // if lock corrupted after process, retry later currentSession = retrieveSession() if (currentSession.lock !== currentLock!) { - retryLater(operations, sessionStorage, numberOfRetries) + retryLater(operations, sessionStorage, numberOfRetries, storageAccessOptions.lockRetryDelay) return } } @@ -221,14 +215,14 @@ export function processStorageOperations(operations: Operations, sessionStorage: persistSession(processedSession) } } - if (isCookieLockEnabled()) { + if (storageAccessOptions.lockEnabled) { // correctly handle lock around expiration would require to handle this case properly at several levels // since we don't have evidence of lock issues around expiration, let's just not do the corruption check for it if (!(processedSession && isSessionInExpiredState(processedSession))) { // if lock corrupted after persist, retry later currentSession = retrieveSession() if (currentSession.lock !== currentLock!) { - retryLater(operations, sessionStorage, numberOfRetries) + retryLater(operations, sessionStorage, numberOfRetries, storageAccessOptions.lockRetryDelay) return } delete currentSession.lock @@ -242,10 +236,15 @@ export function processStorageOperations(operations: Operations, sessionStorage: next(sessionStorage) } -function retryLater(operations: Operations, sessionStorage: SessionStorage, currentNumberOfRetries: number) { +function retryLater( + operations: Operations, + sessionStorage: SessionStorage, + currentNumberOfRetries: number, + retryDelay: number +) { setTimeout(() => { processStorageOperations(operations, sessionStorage, currentNumberOfRetries + 1) - }, LOCK_RETRY_DELAY) + }, retryDelay) } function next(sessionStorage: SessionStorage) { From 44d3ed7a8125b6486e8f4376e79daae3caecd458 Mon Sep 17 00:00:00 2001 From: Yannick Adam Date: Mon, 15 May 2023 11:15:35 +0200 Subject: [PATCH 06/13] =?UTF-8?q?=F0=9F=8E=A8=20Remove=20remaining=20refer?= =?UTF-8?q?ences=20to=20cookies=20from=20sessionStore?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/src/domain/session/sessionStore.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/core/src/domain/session/sessionStore.ts b/packages/core/src/domain/session/sessionStore.ts index b2001c5e84..34bf3a9625 100644 --- a/packages/core/src/domain/session/sessionStore.ts +++ b/packages/core/src/domain/session/sessionStore.ts @@ -69,8 +69,8 @@ export function startSessionStore( /** * allows two behaviors: - * - if the session is active, synchronize the session cache without updating the session cookie - * - if the session is not active, clear the session cookie and expire the session cache + * - if the session is active, synchronize the session cache without updating the session storage + * - if the session is not active, clear the session storage and expire the session cache */ function watchSession() { processStorageOperations( @@ -231,7 +231,7 @@ export function processStorageOperations(operations: Operations, sessionStorage: } } // call after even if session is not persisted in order to perform operations on - // up-to-date cookie value, the value could have been modified by another tab + // up-to-date session state value => the value could have been modified by another tab operations.after?.(processedSession || currentSession) next(sessionStorage) } From 20b0d29a9fa9c4f187c135d25318a04201dbd52d Mon Sep 17 00:00:00 2001 From: Yannick Adam Date: Thu, 25 May 2023 17:51:35 +0200 Subject: [PATCH 07/13] =?UTF-8?q?=F0=9F=8E=A8=20Session=20components=20nam?= =?UTF-8?q?ing=20update?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/domain/session/oldCookiesMigration.ts | 4 +- .../domain/session/sessionCookieStore.spec.ts | 8 +- .../src/domain/session/sessionCookieStore.ts | 6 +- .../core/src/domain/session/sessionManager.ts | 4 +- .../src/domain/session/sessionStorage.spec.ts | 15 - .../core/src/domain/session/sessionStorage.ts | 38 -- .../src/domain/session/sessionStore.spec.ts | 600 +----------------- .../core/src/domain/session/sessionStore.ts | 270 +------- .../session/sessionStoreManager.spec.ts | 597 +++++++++++++++++ .../src/domain/session/sessionStoreManager.ts | 256 ++++++++ 10 files changed, 898 insertions(+), 900 deletions(-) delete mode 100644 packages/core/src/domain/session/sessionStorage.spec.ts delete mode 100644 packages/core/src/domain/session/sessionStorage.ts create mode 100644 packages/core/src/domain/session/sessionStoreManager.spec.ts create mode 100644 packages/core/src/domain/session/sessionStoreManager.ts diff --git a/packages/core/src/domain/session/oldCookiesMigration.ts b/packages/core/src/domain/session/oldCookiesMigration.ts index 02937b4129..fd1e0a256f 100644 --- a/packages/core/src/domain/session/oldCookiesMigration.ts +++ b/packages/core/src/domain/session/oldCookiesMigration.ts @@ -1,8 +1,8 @@ import type { CookieOptions } from '../../browser/cookie' import { getCookie } from '../../browser/cookie' import { dateNow } from '../../tools/utils/timeUtils' -import type { SessionState } from './sessionStorage' -import { isSessionInExpiredState } from './sessionStorage' +import type { SessionState } from './sessionStore' +import { isSessionInExpiredState } from './sessionStore' import { SESSION_COOKIE_NAME, deleteSessionCookie, persistSessionCookie } from './sessionCookieStore' import { SESSION_EXPIRATION_DELAY } from './sessionConstants' diff --git a/packages/core/src/domain/session/sessionCookieStore.spec.ts b/packages/core/src/domain/session/sessionCookieStore.spec.ts index 5ca34e57d4..214c46693b 100644 --- a/packages/core/src/domain/session/sessionCookieStore.spec.ts +++ b/packages/core/src/domain/session/sessionCookieStore.spec.ts @@ -1,16 +1,16 @@ import type { CookieOptions } from '../../browser/cookie' import { setCookie, deleteCookie } from '../../browser/cookie' -import { SESSION_COOKIE_NAME, initCookieStorage } from './sessionCookieStore' +import { SESSION_COOKIE_NAME, initCookieStore } from './sessionCookieStore' -import type { SessionState, SessionStorage } from './sessionStorage' +import type { SessionState, SessionStore } from './sessionStore' describe('session cookie store', () => { const sessionState: SessionState = { id: '123', created: '0' } const noOptions: CookieOptions = {} - let cookieStorage: SessionStorage + let cookieStorage: SessionStore beforeEach(() => { - cookieStorage = initCookieStorage(noOptions) + cookieStorage = initCookieStore(noOptions) }) afterEach(() => { diff --git a/packages/core/src/domain/session/sessionCookieStore.ts b/packages/core/src/domain/session/sessionCookieStore.ts index d887916aeb..57b6b549c7 100644 --- a/packages/core/src/domain/session/sessionCookieStore.ts +++ b/packages/core/src/domain/session/sessionCookieStore.ts @@ -3,7 +3,7 @@ import { COOKIE_ACCESS_DELAY, deleteCookie, getCookie, setCookie } from '../../b import { isChromium } from '../../tools/utils/browserDetection' import { objectEntries } from '../../tools/utils/polyfills' import { SESSION_EXPIRATION_DELAY } from './sessionConstants' -import type { SessionState, SessionStorage } from './sessionStorage' +import type { SessionState, SessionStore } from './sessionStore' const SESSION_ENTRY_REGEXP = /^([a-z]+)=([a-z0-9-]+)$/ const SESSION_ENTRY_SEPARATOR = '&' @@ -14,9 +14,9 @@ export const SESSION_COOKIE_NAME = '_dd_s' export const LOCK_RETRY_DELAY = 10 export const MAX_NUMBER_OF_LOCK_RETRIES = 100 -export function initCookieStorage(options: CookieOptions): SessionStorage { +export function initCookieStore(options: CookieOptions): SessionStore { return { - storageAccessOptions: { + storeAccessOptions: { pollDelay: COOKIE_ACCESS_DELAY, /** * Cookie lock strategy allows mitigating issues due to concurrent access to cookie. diff --git a/packages/core/src/domain/session/sessionManager.ts b/packages/core/src/domain/session/sessionManager.ts index f492f5ad39..724fe58c2c 100644 --- a/packages/core/src/domain/session/sessionManager.ts +++ b/packages/core/src/domain/session/sessionManager.ts @@ -7,7 +7,7 @@ import { relativeNow, clocksOrigin, ONE_MINUTE } from '../../tools/utils/timeUti import { DOM_EVENT, addEventListener, addEventListeners } from '../../browser/addEventListener' import { clearInterval, setInterval } from '../../tools/timer' import { tryOldCookiesMigration } from './oldCookiesMigration' -import { startSessionStore } from './sessionStore' +import { startSessionStoreManager } from './sessionStoreManager' import { SESSION_TIME_OUT_DELAY } from './sessionConstants' export interface SessionManager { @@ -32,7 +32,7 @@ export function startSessionManager( computeSessionState: (rawTrackingType?: string) => { trackingType: TrackingType; isTracked: boolean } ): SessionManager { tryOldCookiesMigration(options) - const sessionStore = startSessionStore(options, productKey, computeSessionState) + const sessionStore = startSessionStoreManager(options, productKey, computeSessionState) stopCallbacks.push(() => sessionStore.stop()) const sessionContextHistory = new ValueHistory>(SESSION_CONTEXT_TIMEOUT_DELAY) diff --git a/packages/core/src/domain/session/sessionStorage.spec.ts b/packages/core/src/domain/session/sessionStorage.spec.ts deleted file mode 100644 index e3b8de5dba..0000000000 --- a/packages/core/src/domain/session/sessionStorage.spec.ts +++ /dev/null @@ -1,15 +0,0 @@ -import type { SessionState } from './sessionStorage' -import { isSessionInExpiredState } from './sessionStorage' - -describe('session storage utilities', () => { - const EXPIRED_SESSION: SessionState = {} - const LIVE_SESSION: SessionState = { created: '0', id: '123' } - - it('should correctly identify a session in expired state', () => { - expect(isSessionInExpiredState(EXPIRED_SESSION)).toBe(true) - }) - - it('should correctly identify a session in live state', () => { - expect(isSessionInExpiredState(LIVE_SESSION)).toBe(false) - }) -}) diff --git a/packages/core/src/domain/session/sessionStorage.ts b/packages/core/src/domain/session/sessionStorage.ts deleted file mode 100644 index 45e8ec04b6..0000000000 --- a/packages/core/src/domain/session/sessionStorage.ts +++ /dev/null @@ -1,38 +0,0 @@ -import type { CookieOptions } from '../../browser/cookie' -import { isEmptyObject } from '../../tools/utils/objectUtils' - -export interface SessionState { - id?: string - created?: string - expire?: string - lock?: string - - [key: string]: string | undefined -} - -interface StorageAccessOptionsWithLock { - pollDelay: number - lockEnabled: true - lockRetryDelay: number - lockMaxTries: number -} - -interface StorageAccessOptionsWithoutLock { - pollDelay: number - lockEnabled: false -} - -type StorageAccessOptions = StorageAccessOptionsWithLock | StorageAccessOptionsWithoutLock - -export type StorageInitOptions = CookieOptions - -export interface SessionStorage { - storageAccessOptions: StorageAccessOptions - persistSession: (session: SessionState) => void - retrieveSession: () => SessionState - clearSession: () => void -} - -export function isSessionInExpiredState(session: SessionState) { - return isEmptyObject(session) -} diff --git a/packages/core/src/domain/session/sessionStore.spec.ts b/packages/core/src/domain/session/sessionStore.spec.ts index 70d122b642..ed67c69cf0 100644 --- a/packages/core/src/domain/session/sessionStore.spec.ts +++ b/packages/core/src/domain/session/sessionStore.spec.ts @@ -1,597 +1,15 @@ -import type { Clock } from '../../../test' -import { stubCookie, mockClock } from '../../../test' -import type { CookieOptions } from '../../browser/cookie' -import { getCookie, setCookie, COOKIE_ACCESS_DELAY } from '../../browser/cookie' -import { isChromium } from '../../tools/utils/browserDetection' -import type { SessionStore } from './sessionStore' -import { processStorageOperations, startSessionStore } from './sessionStore' -import { SESSION_COOKIE_NAME, initCookieStorage, toSessionString } from './sessionCookieStore' -import { SESSION_EXPIRATION_DELAY, SESSION_TIME_OUT_DELAY } from './sessionConstants' -import type { SessionState, SessionStorage } from './sessionStorage' +import type { SessionState } from './sessionStore' +import { isSessionInExpiredState } from './sessionStore' -const enum FakeTrackingType { - TRACKED = 'tracked', - NOT_TRACKED = 'not-tracked', -} +describe('session storage utilities', () => { + const EXPIRED_SESSION: SessionState = {} + const LIVE_SESSION: SessionState = { created: '0', id: '123' } -const DURATION = 123456 -const PRODUCT_KEY = 'product' -const FIRST_ID = 'first' -const SECOND_ID = 'second' -const COOKIE_OPTIONS: CookieOptions = {} - -function setSessionInStore(trackingType: FakeTrackingType = FakeTrackingType.TRACKED, id?: string, expire?: number) { - setCookie( - SESSION_COOKIE_NAME, - `${id ? `id=${id}&` : ''}${PRODUCT_KEY}=${trackingType}&created=${Date.now()}&expire=${ - expire || Date.now() + SESSION_EXPIRATION_DELAY - }`, - DURATION - ) -} - -function expectTrackedSessionToBeInStore(id?: string) { - expect(getCookie(SESSION_COOKIE_NAME)).toMatch(new RegExp(`id=${id ? id : '[a-f0-9-]+'}`)) - expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${PRODUCT_KEY}=${FakeTrackingType.TRACKED}`) -} - -function expectNotTrackedSessionToBeInStore() { - expect(getCookie(SESSION_COOKIE_NAME)).not.toContain('id=') - expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${PRODUCT_KEY}=${FakeTrackingType.NOT_TRACKED}`) -} - -function getStoreExpiration() { - return /expire=(\d+)/.exec(getCookie(SESSION_COOKIE_NAME)!)?.[1] -} - -function resetSessionInStore() { - setCookie(SESSION_COOKIE_NAME, '', DURATION) -} - -describe('session store', () => { - describe('session lifecyle mechanism', () => { - let expireSpy: () => void - let renewSpy: () => void - let sessionStore: SessionStore - let clock: Clock - - function setupSessionStore( - computeSessionState: (rawTrackingType?: string) => { - trackingType: FakeTrackingType - isTracked: boolean - } = () => ({ - isTracked: true, - trackingType: FakeTrackingType.TRACKED, - }) - ) { - sessionStore = startSessionStore(COOKIE_OPTIONS, PRODUCT_KEY, computeSessionState) - sessionStore.expireObservable.subscribe(expireSpy) - sessionStore.renewObservable.subscribe(renewSpy) - } - - beforeEach(() => { - expireSpy = jasmine.createSpy('expire session') - renewSpy = jasmine.createSpy('renew session') - clock = mockClock() - }) - - afterEach(() => { - resetSessionInStore() - clock.cleanup() - sessionStore.stop() - }) - - describe('expand or renew session', () => { - it( - 'when session not in cache, session not in store and new session tracked, ' + - 'should create new session and trigger renew session ', - () => { - setupSessionStore() - - sessionStore.expandOrRenewSession() - - expect(sessionStore.getSession().id).toBeDefined() - expectTrackedSessionToBeInStore() - expect(expireSpy).not.toHaveBeenCalled() - expect(renewSpy).toHaveBeenCalled() - } - ) - - it( - 'when session not in cache, session not in store and new session not tracked, ' + - 'should store not tracked session', - () => { - setupSessionStore(() => ({ isTracked: false, trackingType: FakeTrackingType.NOT_TRACKED })) - - sessionStore.expandOrRenewSession() - - expect(sessionStore.getSession().id).toBeUndefined() - expectNotTrackedSessionToBeInStore() - expect(expireSpy).not.toHaveBeenCalled() - expect(renewSpy).not.toHaveBeenCalled() - } - ) - - it('when session not in cache and session in store, should expand session and trigger renew session', () => { - setupSessionStore() - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) - - sessionStore.expandOrRenewSession() - - expect(sessionStore.getSession().id).toBe(FIRST_ID) - expectTrackedSessionToBeInStore(FIRST_ID) - expect(expireSpy).not.toHaveBeenCalled() - expect(renewSpy).toHaveBeenCalled() - }) - - it( - 'when session in cache, session not in store and new session tracked, ' + - 'should expire session, create a new one and trigger renew session', - () => { - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) - setupSessionStore() - resetSessionInStore() - - sessionStore.expandOrRenewSession() - - const sessionId = sessionStore.getSession().id - expect(sessionId).toBeDefined() - expect(sessionId).not.toBe(FIRST_ID) - expectTrackedSessionToBeInStore(sessionId) - expect(expireSpy).toHaveBeenCalled() - expect(renewSpy).toHaveBeenCalled() - } - ) - - it( - 'when session in cache, session not in store and new session not tracked, ' + - 'should expire session and store not tracked session', - () => { - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) - setupSessionStore(() => ({ isTracked: false, trackingType: FakeTrackingType.NOT_TRACKED })) - resetSessionInStore() - - sessionStore.expandOrRenewSession() - - expect(sessionStore.getSession().id).toBeUndefined() - expect(sessionStore.getSession()[PRODUCT_KEY]).toBeDefined() - expectNotTrackedSessionToBeInStore() - expect(expireSpy).toHaveBeenCalled() - expect(renewSpy).not.toHaveBeenCalled() - } - ) - - it( - 'when session not tracked in cache, session not in store and new session not tracked, ' + - 'should expire session and store not tracked session', - () => { - setSessionInStore(FakeTrackingType.NOT_TRACKED) - setupSessionStore(() => ({ isTracked: false, trackingType: FakeTrackingType.NOT_TRACKED })) - resetSessionInStore() - - sessionStore.expandOrRenewSession() - - expect(sessionStore.getSession().id).toBeUndefined() - expect(sessionStore.getSession()[PRODUCT_KEY]).toBeDefined() - expectNotTrackedSessionToBeInStore() - expect(expireSpy).toHaveBeenCalled() - expect(renewSpy).not.toHaveBeenCalled() - } - ) - - it('when session in cache is same session than in store, should expand session', () => { - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) - setupSessionStore() - - clock.tick(10) - sessionStore.expandOrRenewSession() - - expect(sessionStore.getSession().id).toBe(FIRST_ID) - expect(sessionStore.getSession().expire).toBe(getStoreExpiration()) - expectTrackedSessionToBeInStore(FIRST_ID) - expect(expireSpy).not.toHaveBeenCalled() - expect(renewSpy).not.toHaveBeenCalled() - }) - - it( - 'when session in cache is different session than in store and store session is tracked, ' + - 'should expire session, expand store session and trigger renew', - () => { - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) - setupSessionStore() - setSessionInStore(FakeTrackingType.TRACKED, SECOND_ID) - - sessionStore.expandOrRenewSession() - - expect(sessionStore.getSession().id).toBe(SECOND_ID) - expectTrackedSessionToBeInStore(SECOND_ID) - expect(expireSpy).toHaveBeenCalled() - expect(renewSpy).toHaveBeenCalled() - } - ) - - it( - 'when session in cache is different session than in store and store session is not tracked, ' + - 'should expire session and store not tracked session', - () => { - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) - setupSessionStore((rawTrackingType) => ({ - isTracked: rawTrackingType === FakeTrackingType.TRACKED, - trackingType: rawTrackingType as FakeTrackingType, - })) - setSessionInStore(FakeTrackingType.NOT_TRACKED, '') - - sessionStore.expandOrRenewSession() - - expect(sessionStore.getSession().id).toBeUndefined() - expectNotTrackedSessionToBeInStore() - expect(expireSpy).toHaveBeenCalled() - expect(renewSpy).not.toHaveBeenCalled() - } - ) - }) - - describe('expand session', () => { - it('when session not in cache and session not in store, should do nothing', () => { - setupSessionStore() - - sessionStore.expandSession() - - expect(sessionStore.getSession().id).toBeUndefined() - expect(expireSpy).not.toHaveBeenCalled() - }) - - it('when session not in cache and session in store, should do nothing', () => { - setupSessionStore() - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) - - sessionStore.expandSession() - - expect(sessionStore.getSession().id).toBeUndefined() - expect(expireSpy).not.toHaveBeenCalled() - }) - - it('when session in cache and session not in store, should expire session', () => { - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) - setupSessionStore() - resetSessionInStore() - - sessionStore.expandSession() - - expect(sessionStore.getSession().id).toBeUndefined() - expect(expireSpy).toHaveBeenCalled() - }) - - it('when session in cache is same session than in store, should expand session', () => { - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) - setupSessionStore() - - clock.tick(10) - sessionStore.expandSession() - - expect(sessionStore.getSession().id).toBe(FIRST_ID) - expect(sessionStore.getSession().expire).toBe(getStoreExpiration()) - expect(expireSpy).not.toHaveBeenCalled() - }) - - it('when session in cache is different session than in store, should expire session', () => { - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) - setupSessionStore() - setSessionInStore(FakeTrackingType.TRACKED, SECOND_ID) - - sessionStore.expandSession() - - expect(sessionStore.getSession().id).toBeUndefined() - expectTrackedSessionToBeInStore(SECOND_ID) - expect(expireSpy).toHaveBeenCalled() - }) - }) - - describe('regular watch', () => { - it('when session not in cache and session not in store, should do nothing', () => { - setupSessionStore() - - clock.tick(COOKIE_ACCESS_DELAY) - - expect(sessionStore.getSession().id).toBeUndefined() - expect(expireSpy).not.toHaveBeenCalled() - }) - - it('when session not in cache and session in store, should do nothing', () => { - setupSessionStore() - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) - - clock.tick(COOKIE_ACCESS_DELAY) - - expect(sessionStore.getSession().id).toBeUndefined() - expect(expireSpy).not.toHaveBeenCalled() - }) - - it('when session in cache and session not in store, should expire session', () => { - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) - setupSessionStore() - resetSessionInStore() - - clock.tick(COOKIE_ACCESS_DELAY) - - expect(sessionStore.getSession().id).toBeUndefined() - expect(expireSpy).toHaveBeenCalled() - }) - - it('when session in cache is same session than in store, should synchronize session', () => { - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) - setupSessionStore() - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID, Date.now() + SESSION_TIME_OUT_DELAY + 10) - - clock.tick(COOKIE_ACCESS_DELAY) - - expect(sessionStore.getSession().id).toBe(FIRST_ID) - expect(sessionStore.getSession().expire).toBe(getStoreExpiration()) - expect(expireSpy).not.toHaveBeenCalled() - }) - - it('when session id in cache is different than session id in store, should expire session', () => { - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) - setupSessionStore() - setSessionInStore(FakeTrackingType.TRACKED, SECOND_ID) - - clock.tick(COOKIE_ACCESS_DELAY) - - expect(sessionStore.getSession().id).toBeUndefined() - expect(expireSpy).toHaveBeenCalled() - }) - - it('when session type in cache is different than session type in store, should expire session', () => { - setSessionInStore(FakeTrackingType.NOT_TRACKED, FIRST_ID) - setupSessionStore() - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) - - clock.tick(COOKIE_ACCESS_DELAY) - - expect(sessionStore.getSession().id).toBeUndefined() - expect(expireSpy).toHaveBeenCalled() - }) - }) + it('should correctly identify a session in expired state', () => { + expect(isSessionInExpiredState(EXPIRED_SESSION)).toBe(true) }) - describe('process operations mechanism', () => { - const COOKIE_OPTIONS = {} - let initialSession: SessionState - let otherSession: SessionState - let processSpy: jasmine.Spy - let afterSpy: jasmine.Spy - let cookie: ReturnType - let cookieStorage: SessionStorage - - beforeEach(() => { - cookieStorage = initCookieStorage(COOKIE_OPTIONS) - initialSession = { id: '123', created: '0' } - otherSession = { id: '456', created: '100' } - processSpy = jasmine.createSpy('process') - afterSpy = jasmine.createSpy('after') - cookie = stubCookie() - }) - - describe('with cookie-lock disabled', () => { - beforeEach(() => { - isChromium() && pending('cookie-lock only disabled on non chromium browsers') - }) - - it('should persist session when process returns a value', () => { - cookieStorage.persistSession(initialSession) - processSpy.and.returnValue({ ...otherSession }) - - processStorageOperations({ process: processSpy, after: afterSpy }, cookieStorage) - - expect(processSpy).toHaveBeenCalledWith(initialSession) - const expectedSession = { ...otherSession, expire: jasmine.any(String) } - expect(cookieStorage.retrieveSession()).toEqual(expectedSession) - expect(afterSpy).toHaveBeenCalledWith(expectedSession) - }) - - it('should clear session when process return an empty value', () => { - cookieStorage.persistSession(initialSession) - processSpy.and.returnValue({}) - - processStorageOperations({ process: processSpy, after: afterSpy }, cookieStorage) - - expect(processSpy).toHaveBeenCalledWith(initialSession) - const expectedSession = {} - expect(cookieStorage.retrieveSession()).toEqual(expectedSession) - expect(afterSpy).toHaveBeenCalledWith(expectedSession) - }) - - it('should not persist session when process return undefined', () => { - cookieStorage.persistSession(initialSession) - processSpy.and.returnValue(undefined) - - processStorageOperations({ process: processSpy, after: afterSpy }, cookieStorage) - - expect(processSpy).toHaveBeenCalledWith(initialSession) - expect(cookieStorage.retrieveSession()).toEqual(initialSession) - expect(afterSpy).toHaveBeenCalledWith(initialSession) - }) - }) - - describe('with cookie-lock enabled', () => { - beforeEach(() => { - !isChromium() && pending('cookie-lock only enabled on chromium browsers') - }) - - it('should persist session when process return a value', () => { - cookieStorage.persistSession(initialSession) - processSpy.and.callFake((session) => ({ ...otherSession, lock: session.lock })) - - processStorageOperations({ process: processSpy, after: afterSpy }, cookieStorage) - - expect(processSpy).toHaveBeenCalledWith({ ...initialSession, lock: jasmine.any(String) }) - const expectedSession = { ...otherSession, expire: jasmine.any(String) } - expect(cookieStorage.retrieveSession()).toEqual(expectedSession) - expect(afterSpy).toHaveBeenCalledWith(expectedSession) - }) - - it('should clear session when process return an empty value', () => { - cookieStorage.persistSession(initialSession) - processSpy.and.returnValue({}) - - processStorageOperations({ process: processSpy, after: afterSpy }, cookieStorage) - - expect(processSpy).toHaveBeenCalledWith({ ...initialSession, lock: jasmine.any(String) }) - const expectedSession = {} - expect(cookieStorage.retrieveSession()).toEqual(expectedSession) - expect(afterSpy).toHaveBeenCalledWith(expectedSession) - }) - - it('should not persist session when process return undefined', () => { - cookieStorage.persistSession(initialSession) - processSpy.and.returnValue(undefined) - - processStorageOperations({ process: processSpy, after: afterSpy }, cookieStorage) - - expect(processSpy).toHaveBeenCalledWith({ ...initialSession, lock: jasmine.any(String) }) - expect(cookieStorage.retrieveSession()).toEqual(initialSession) - expect(afterSpy).toHaveBeenCalledWith(initialSession) - }) - - type OnLockCheck = () => { currentState: SessionState; retryState: SessionState } - - function lockScenario({ - onInitialLockCheck, - onAcquiredLockCheck, - onPostProcessLockCheck, - onPostPersistLockCheck, - }: { - onInitialLockCheck?: OnLockCheck - onAcquiredLockCheck?: OnLockCheck - onPostProcessLockCheck?: OnLockCheck - onPostPersistLockCheck?: OnLockCheck - }) { - const onLockChecks = [onInitialLockCheck, onAcquiredLockCheck, onPostProcessLockCheck, onPostPersistLockCheck] - cookie.getSpy.and.callFake(() => { - const currentOnLockCheck = onLockChecks.shift() - if (!currentOnLockCheck) { - return cookie.currentValue() - } - const { currentState, retryState } = currentOnLockCheck() - cookie.setCurrentValue(buildSessionString(retryState)) - return buildSessionString(currentState) - }) - } - - function buildSessionString(currentState: SessionState) { - return `${SESSION_COOKIE_NAME}=${toSessionString(currentState)}` - } - - ;[ - { - description: 'should wait for lock to be free', - lockConflict: 'onInitialLockCheck', - }, - { - description: 'should retry if lock was acquired before process', - lockConflict: 'onAcquiredLockCheck', - }, - { - description: 'should retry if lock was acquired after process', - lockConflict: 'onPostProcessLockCheck', - }, - { - description: 'should retry if lock was acquired after persist', - lockConflict: 'onPostPersistLockCheck', - }, - ].forEach(({ description, lockConflict }) => { - it(description, (done) => { - lockScenario({ - [lockConflict]: () => ({ - currentState: { ...initialSession, lock: 'locked' }, - retryState: { ...initialSession, other: 'other' }, - }), - }) - initialSession.expire = String(Date.now() + SESSION_EXPIRATION_DELAY) - cookieStorage.persistSession(initialSession) - processSpy.and.callFake((session) => ({ ...session, processed: 'processed' } as SessionState)) - - processStorageOperations( - { - process: processSpy, - after: (afterSession) => { - // session with 'other' value on process - expect(processSpy).toHaveBeenCalledWith({ - ...initialSession, - other: 'other', - lock: jasmine.any(String), - expire: jasmine.any(String), - }) - - // end state with session 'other' and 'processed' value - const expectedSession = { - ...initialSession, - other: 'other', - processed: 'processed', - expire: jasmine.any(String), - } - expect(cookieStorage.retrieveSession()).toEqual(expectedSession) - expect(afterSession).toEqual(expectedSession) - done() - }, - }, - cookieStorage - ) - }) - }) - - it('should abort after a max number of retry', () => { - const clock = mockClock() - - cookieStorage.persistSession(initialSession) - cookie.setSpy.calls.reset() - - cookie.getSpy.and.returnValue(buildSessionString({ ...initialSession, lock: 'locked' })) - processStorageOperations({ process: processSpy, after: afterSpy }, cookieStorage) - - const lockMaxTries = cookieStorage.storageAccessOptions.lockEnabled - ? cookieStorage.storageAccessOptions.lockMaxTries - : 0 - const lockRetryDelay = cookieStorage.storageAccessOptions.lockEnabled - ? cookieStorage.storageAccessOptions.lockRetryDelay - : 0 - - clock.tick(lockMaxTries * lockRetryDelay) - expect(processSpy).not.toHaveBeenCalled() - expect(afterSpy).not.toHaveBeenCalled() - expect(cookie.setSpy).not.toHaveBeenCalled() - - clock.cleanup() - }) - - it('should execute cookie accesses in order', (done) => { - lockScenario({ - onInitialLockCheck: () => ({ - currentState: { ...initialSession, lock: 'locked' }, // force to retry the first access later - retryState: initialSession, - }), - }) - cookieStorage.persistSession(initialSession) - - processStorageOperations( - { - process: (session) => ({ ...session, value: 'foo' }), - after: afterSpy, - }, - cookieStorage - ) - processStorageOperations( - { - process: (session) => ({ ...session, value: `${session.value || ''}bar` }), - after: (session) => { - expect(session.value).toBe('foobar') - expect(afterSpy).toHaveBeenCalled() - done() - }, - }, - cookieStorage - ) - }) - }) + it('should correctly identify a session in live state', () => { + expect(isSessionInExpiredState(LIVE_SESSION)).toBe(false) }) }) diff --git a/packages/core/src/domain/session/sessionStore.ts b/packages/core/src/domain/session/sessionStore.ts index 34bf3a9625..18049f846b 100644 --- a/packages/core/src/domain/session/sessionStore.ts +++ b/packages/core/src/domain/session/sessionStore.ts @@ -1,256 +1,36 @@ -import { clearInterval, setInterval, setTimeout } from '../../tools/timer' -import { Observable } from '../../tools/observable' -import { dateNow } from '../../tools/utils/timeUtils' -import { throttle } from '../../tools/utils/functionUtils' -import { generateUUID } from '../../tools/utils/stringUtils' -import { SESSION_EXPIRATION_DELAY, SESSION_TIME_OUT_DELAY } from './sessionConstants' -import { initCookieStorage } from './sessionCookieStore' -import type { SessionState, SessionStorage, StorageInitOptions } from './sessionStorage' -import { isSessionInExpiredState } from './sessionStorage' +import type { CookieOptions } from '../../browser/cookie' +import { isEmptyObject } from '../../tools/utils/objectUtils' -export interface SessionStore { - expandOrRenewSession: () => void - expandSession: () => void - getSession: () => SessionState - renewObservable: Observable - expireObservable: Observable - expire: () => void - stop: () => void -} - -/** - * Different session concepts: - * - tracked, the session has an id and is updated along the user navigation - * - not tracked, the session does not have an id but it is updated along the user navigation - * - inactive, no session in store or session expired, waiting for a renew session - */ -export function startSessionStore( - options: StorageInitOptions, - productKey: string, - computeSessionState: (rawTrackingType?: string) => { trackingType: TrackingType; isTracked: boolean } -): SessionStore { - const renewObservable = new Observable() - const expireObservable = new Observable() - - const sessionStorage = initCookieStorage(options) - const { clearSession, retrieveSession, storageAccessOptions } = sessionStorage - - const watchSessionTimeoutId = setInterval(watchSession, storageAccessOptions.pollDelay) - let sessionCache: SessionState = retrieveActiveSession() - - function expandOrRenewSession() { - let isTracked: boolean - processStorageOperations( - { - process: (sessionState) => { - const synchronizedSession = synchronizeSession(sessionState) - isTracked = expandOrRenewCookie(synchronizedSession) - return synchronizedSession - }, - after: (sessionState) => { - if (isTracked && !hasSessionInCache()) { - renewSessionInCache(sessionState) - } - sessionCache = sessionState - }, - }, - sessionStorage - ) - } - - function expandSession() { - processStorageOperations( - { - process: (sessionState) => (hasSessionInCache() ? synchronizeSession(sessionState) : undefined), - }, - sessionStorage - ) - } - - /** - * allows two behaviors: - * - if the session is active, synchronize the session cache without updating the session storage - * - if the session is not active, clear the session storage and expire the session cache - */ - function watchSession() { - processStorageOperations( - { - process: (sessionState) => (!isActiveSession(sessionState) ? {} : undefined), - after: synchronizeSession, - }, - sessionStorage - ) - } - - function synchronizeSession(sessionState: SessionState) { - if (!isActiveSession(sessionState)) { - sessionState = {} - } - if (hasSessionInCache()) { - if (isSessionInCacheOutdated(sessionState)) { - expireSessionInCache() - } else { - sessionCache = sessionState - } - } - return sessionState - } - - function expandOrRenewCookie(sessionState: SessionState) { - const { trackingType, isTracked } = computeSessionState(sessionState[productKey]) - sessionState[productKey] = trackingType - if (isTracked && !sessionState.id) { - sessionState.id = generateUUID() - sessionState.created = String(dateNow()) - } - return isTracked - } +export interface SessionState { + id?: string + created?: string + expire?: string + lock?: string - function hasSessionInCache() { - return sessionCache[productKey] !== undefined - } - - function isSessionInCacheOutdated(sessionState: SessionState) { - return sessionCache.id !== sessionState.id || sessionCache[productKey] !== sessionState[productKey] - } - - function expireSessionInCache() { - sessionCache = {} - expireObservable.notify() - } - - function renewSessionInCache(sessionState: SessionState) { - sessionCache = sessionState - renewObservable.notify() - } - - function retrieveActiveSession(): SessionState { - const session = retrieveSession() - if (isActiveSession(session)) { - return session - } - return {} - } - - function isActiveSession(sessionDate: SessionState) { - // created and expire can be undefined for versions which was not storing them - // these checks could be removed when older versions will not be available/live anymore - return ( - (sessionDate.created === undefined || dateNow() - Number(sessionDate.created) < SESSION_TIME_OUT_DELAY) && - (sessionDate.expire === undefined || dateNow() < Number(sessionDate.expire)) - ) - } - - return { - expandOrRenewSession: throttle(expandOrRenewSession, storageAccessOptions.pollDelay).throttled, - expandSession, - getSession: () => sessionCache, - renewObservable, - expireObservable, - expire: () => { - clearSession() - synchronizeSession({}) - }, - stop: () => { - clearInterval(watchSessionTimeoutId) - }, - } + [key: string]: string | undefined } -type Operations = { - process: (sessionState: SessionState) => SessionState | undefined - after?: (sessionState: SessionState) => void +type StoreAccessOptionsWithLock = { + pollDelay: number + lockEnabled: true + lockRetryDelay: number + lockMaxTries: number } -const bufferedOperations: Operations[] = [] -let ongoingOperations: Operations | undefined - -export function processStorageOperations(operations: Operations, sessionStorage: SessionStorage, numberOfRetries = 0) { - const { retrieveSession, persistSession, clearSession, storageAccessOptions } = sessionStorage - - if (!ongoingOperations) { - ongoingOperations = operations - } - if (operations !== ongoingOperations) { - bufferedOperations.push(operations) - return - } - if (storageAccessOptions.lockEnabled && numberOfRetries >= storageAccessOptions.lockMaxTries) { - next(sessionStorage) - return - } - let currentLock: string - let currentSession = retrieveSession() - if (storageAccessOptions.lockEnabled) { - // if someone has lock, retry later - if (currentSession.lock) { - retryLater(operations, sessionStorage, numberOfRetries, storageAccessOptions.lockRetryDelay) - return - } - // acquire lock - currentLock = generateUUID() - currentSession.lock = currentLock - persistSession(currentSession) - // if lock is not acquired, retry later - currentSession = retrieveSession() - if (currentSession.lock !== currentLock) { - retryLater(operations, sessionStorage, numberOfRetries, storageAccessOptions.lockRetryDelay) - return - } - } - let processedSession = operations.process(currentSession) - if (storageAccessOptions.lockEnabled) { - // if lock corrupted after process, retry later - currentSession = retrieveSession() - if (currentSession.lock !== currentLock!) { - retryLater(operations, sessionStorage, numberOfRetries, storageAccessOptions.lockRetryDelay) - return - } - } - if (processedSession) { - if (isSessionInExpiredState(processedSession)) { - clearSession() - } else { - processedSession.expire = String(dateNow() + SESSION_EXPIRATION_DELAY) - persistSession(processedSession) - } - } - if (storageAccessOptions.lockEnabled) { - // correctly handle lock around expiration would require to handle this case properly at several levels - // since we don't have evidence of lock issues around expiration, let's just not do the corruption check for it - if (!(processedSession && isSessionInExpiredState(processedSession))) { - // if lock corrupted after persist, retry later - currentSession = retrieveSession() - if (currentSession.lock !== currentLock!) { - retryLater(operations, sessionStorage, numberOfRetries, storageAccessOptions.lockRetryDelay) - return - } - delete currentSession.lock - persistSession(currentSession) - processedSession = currentSession - } - } - // call after even if session is not persisted in order to perform operations on - // up-to-date session state value => the value could have been modified by another tab - operations.after?.(processedSession || currentSession) - next(sessionStorage) +type StoreAccessOptionsWithoutLock = { + pollDelay: number + lockEnabled: false } -function retryLater( - operations: Operations, - sessionStorage: SessionStorage, - currentNumberOfRetries: number, - retryDelay: number -) { - setTimeout(() => { - processStorageOperations(operations, sessionStorage, currentNumberOfRetries + 1) - }, retryDelay) +export type StoreInitOptions = CookieOptions + +export interface SessionStore { + storeAccessOptions: StoreAccessOptionsWithLock | StoreAccessOptionsWithoutLock + persistSession: (session: SessionState) => void + retrieveSession: () => SessionState + clearSession: () => void } -function next(sessionStorage: SessionStorage) { - ongoingOperations = undefined - const nextOperations = bufferedOperations.shift() - if (nextOperations) { - processStorageOperations(nextOperations, sessionStorage) - } +export function isSessionInExpiredState(session: SessionState) { + return isEmptyObject(session) } diff --git a/packages/core/src/domain/session/sessionStoreManager.spec.ts b/packages/core/src/domain/session/sessionStoreManager.spec.ts new file mode 100644 index 0000000000..3751ea21fb --- /dev/null +++ b/packages/core/src/domain/session/sessionStoreManager.spec.ts @@ -0,0 +1,597 @@ +import type { Clock } from '../../../test' +import { stubCookie, mockClock } from '../../../test' +import type { CookieOptions } from '../../browser/cookie' +import { getCookie, setCookie, COOKIE_ACCESS_DELAY } from '../../browser/cookie' +import { isChromium } from '../../tools/utils/browserDetection' +import type { SessionStoreManager } from './sessionStoreManager' +import { processSessionStoreOperations, startSessionStoreManager } from './sessionStoreManager' +import { SESSION_COOKIE_NAME, initCookieStore, toSessionString } from './sessionCookieStore' +import { SESSION_EXPIRATION_DELAY, SESSION_TIME_OUT_DELAY } from './sessionConstants' +import type { SessionState, SessionStore } from './sessionStore' + +const enum FakeTrackingType { + TRACKED = 'tracked', + NOT_TRACKED = 'not-tracked', +} + +const DURATION = 123456 +const PRODUCT_KEY = 'product' +const FIRST_ID = 'first' +const SECOND_ID = 'second' +const COOKIE_OPTIONS: CookieOptions = {} + +function setSessionInStore(trackingType: FakeTrackingType = FakeTrackingType.TRACKED, id?: string, expire?: number) { + setCookie( + SESSION_COOKIE_NAME, + `${id ? `id=${id}&` : ''}${PRODUCT_KEY}=${trackingType}&created=${Date.now()}&expire=${ + expire || Date.now() + SESSION_EXPIRATION_DELAY + }`, + DURATION + ) +} + +function expectTrackedSessionToBeInStore(id?: string) { + expect(getCookie(SESSION_COOKIE_NAME)).toMatch(new RegExp(`id=${id ? id : '[a-f0-9-]+'}`)) + expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${PRODUCT_KEY}=${FakeTrackingType.TRACKED}`) +} + +function expectNotTrackedSessionToBeInStore() { + expect(getCookie(SESSION_COOKIE_NAME)).not.toContain('id=') + expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${PRODUCT_KEY}=${FakeTrackingType.NOT_TRACKED}`) +} + +function getStoreExpiration() { + return /expire=(\d+)/.exec(getCookie(SESSION_COOKIE_NAME)!)?.[1] +} + +function resetSessionInStore() { + setCookie(SESSION_COOKIE_NAME, '', DURATION) +} + +describe('session store', () => { + describe('session lifecyle mechanism', () => { + let expireSpy: () => void + let renewSpy: () => void + let sessionStore: SessionStoreManager + let clock: Clock + + function setupSessionStore( + computeSessionState: (rawTrackingType?: string) => { + trackingType: FakeTrackingType + isTracked: boolean + } = () => ({ + isTracked: true, + trackingType: FakeTrackingType.TRACKED, + }) + ) { + sessionStore = startSessionStoreManager(COOKIE_OPTIONS, PRODUCT_KEY, computeSessionState) + sessionStore.expireObservable.subscribe(expireSpy) + sessionStore.renewObservable.subscribe(renewSpy) + } + + beforeEach(() => { + expireSpy = jasmine.createSpy('expire session') + renewSpy = jasmine.createSpy('renew session') + clock = mockClock() + }) + + afterEach(() => { + resetSessionInStore() + clock.cleanup() + sessionStore.stop() + }) + + describe('expand or renew session', () => { + it( + 'when session not in cache, session not in store and new session tracked, ' + + 'should create new session and trigger renew session ', + () => { + setupSessionStore() + + sessionStore.expandOrRenewSession() + + expect(sessionStore.getSession().id).toBeDefined() + expectTrackedSessionToBeInStore() + expect(expireSpy).not.toHaveBeenCalled() + expect(renewSpy).toHaveBeenCalled() + } + ) + + it( + 'when session not in cache, session not in store and new session not tracked, ' + + 'should store not tracked session', + () => { + setupSessionStore(() => ({ isTracked: false, trackingType: FakeTrackingType.NOT_TRACKED })) + + sessionStore.expandOrRenewSession() + + expect(sessionStore.getSession().id).toBeUndefined() + expectNotTrackedSessionToBeInStore() + expect(expireSpy).not.toHaveBeenCalled() + expect(renewSpy).not.toHaveBeenCalled() + } + ) + + it('when session not in cache and session in store, should expand session and trigger renew session', () => { + setupSessionStore() + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + + sessionStore.expandOrRenewSession() + + expect(sessionStore.getSession().id).toBe(FIRST_ID) + expectTrackedSessionToBeInStore(FIRST_ID) + expect(expireSpy).not.toHaveBeenCalled() + expect(renewSpy).toHaveBeenCalled() + }) + + it( + 'when session in cache, session not in store and new session tracked, ' + + 'should expire session, create a new one and trigger renew session', + () => { + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + setupSessionStore() + resetSessionInStore() + + sessionStore.expandOrRenewSession() + + const sessionId = sessionStore.getSession().id + expect(sessionId).toBeDefined() + expect(sessionId).not.toBe(FIRST_ID) + expectTrackedSessionToBeInStore(sessionId) + expect(expireSpy).toHaveBeenCalled() + expect(renewSpy).toHaveBeenCalled() + } + ) + + it( + 'when session in cache, session not in store and new session not tracked, ' + + 'should expire session and store not tracked session', + () => { + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + setupSessionStore(() => ({ isTracked: false, trackingType: FakeTrackingType.NOT_TRACKED })) + resetSessionInStore() + + sessionStore.expandOrRenewSession() + + expect(sessionStore.getSession().id).toBeUndefined() + expect(sessionStore.getSession()[PRODUCT_KEY]).toBeDefined() + expectNotTrackedSessionToBeInStore() + expect(expireSpy).toHaveBeenCalled() + expect(renewSpy).not.toHaveBeenCalled() + } + ) + + it( + 'when session not tracked in cache, session not in store and new session not tracked, ' + + 'should expire session and store not tracked session', + () => { + setSessionInStore(FakeTrackingType.NOT_TRACKED) + setupSessionStore(() => ({ isTracked: false, trackingType: FakeTrackingType.NOT_TRACKED })) + resetSessionInStore() + + sessionStore.expandOrRenewSession() + + expect(sessionStore.getSession().id).toBeUndefined() + expect(sessionStore.getSession()[PRODUCT_KEY]).toBeDefined() + expectNotTrackedSessionToBeInStore() + expect(expireSpy).toHaveBeenCalled() + expect(renewSpy).not.toHaveBeenCalled() + } + ) + + it('when session in cache is same session than in store, should expand session', () => { + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + setupSessionStore() + + clock.tick(10) + sessionStore.expandOrRenewSession() + + expect(sessionStore.getSession().id).toBe(FIRST_ID) + expect(sessionStore.getSession().expire).toBe(getStoreExpiration()) + expectTrackedSessionToBeInStore(FIRST_ID) + expect(expireSpy).not.toHaveBeenCalled() + expect(renewSpy).not.toHaveBeenCalled() + }) + + it( + 'when session in cache is different session than in store and store session is tracked, ' + + 'should expire session, expand store session and trigger renew', + () => { + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + setupSessionStore() + setSessionInStore(FakeTrackingType.TRACKED, SECOND_ID) + + sessionStore.expandOrRenewSession() + + expect(sessionStore.getSession().id).toBe(SECOND_ID) + expectTrackedSessionToBeInStore(SECOND_ID) + expect(expireSpy).toHaveBeenCalled() + expect(renewSpy).toHaveBeenCalled() + } + ) + + it( + 'when session in cache is different session than in store and store session is not tracked, ' + + 'should expire session and store not tracked session', + () => { + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + setupSessionStore((rawTrackingType) => ({ + isTracked: rawTrackingType === FakeTrackingType.TRACKED, + trackingType: rawTrackingType as FakeTrackingType, + })) + setSessionInStore(FakeTrackingType.NOT_TRACKED, '') + + sessionStore.expandOrRenewSession() + + expect(sessionStore.getSession().id).toBeUndefined() + expectNotTrackedSessionToBeInStore() + expect(expireSpy).toHaveBeenCalled() + expect(renewSpy).not.toHaveBeenCalled() + } + ) + }) + + describe('expand session', () => { + it('when session not in cache and session not in store, should do nothing', () => { + setupSessionStore() + + sessionStore.expandSession() + + expect(sessionStore.getSession().id).toBeUndefined() + expect(expireSpy).not.toHaveBeenCalled() + }) + + it('when session not in cache and session in store, should do nothing', () => { + setupSessionStore() + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + + sessionStore.expandSession() + + expect(sessionStore.getSession().id).toBeUndefined() + expect(expireSpy).not.toHaveBeenCalled() + }) + + it('when session in cache and session not in store, should expire session', () => { + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + setupSessionStore() + resetSessionInStore() + + sessionStore.expandSession() + + expect(sessionStore.getSession().id).toBeUndefined() + expect(expireSpy).toHaveBeenCalled() + }) + + it('when session in cache is same session than in store, should expand session', () => { + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + setupSessionStore() + + clock.tick(10) + sessionStore.expandSession() + + expect(sessionStore.getSession().id).toBe(FIRST_ID) + expect(sessionStore.getSession().expire).toBe(getStoreExpiration()) + expect(expireSpy).not.toHaveBeenCalled() + }) + + it('when session in cache is different session than in store, should expire session', () => { + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + setupSessionStore() + setSessionInStore(FakeTrackingType.TRACKED, SECOND_ID) + + sessionStore.expandSession() + + expect(sessionStore.getSession().id).toBeUndefined() + expectTrackedSessionToBeInStore(SECOND_ID) + expect(expireSpy).toHaveBeenCalled() + }) + }) + + describe('regular watch', () => { + it('when session not in cache and session not in store, should do nothing', () => { + setupSessionStore() + + clock.tick(COOKIE_ACCESS_DELAY) + + expect(sessionStore.getSession().id).toBeUndefined() + expect(expireSpy).not.toHaveBeenCalled() + }) + + it('when session not in cache and session in store, should do nothing', () => { + setupSessionStore() + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + + clock.tick(COOKIE_ACCESS_DELAY) + + expect(sessionStore.getSession().id).toBeUndefined() + expect(expireSpy).not.toHaveBeenCalled() + }) + + it('when session in cache and session not in store, should expire session', () => { + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + setupSessionStore() + resetSessionInStore() + + clock.tick(COOKIE_ACCESS_DELAY) + + expect(sessionStore.getSession().id).toBeUndefined() + expect(expireSpy).toHaveBeenCalled() + }) + + it('when session in cache is same session than in store, should synchronize session', () => { + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + setupSessionStore() + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID, Date.now() + SESSION_TIME_OUT_DELAY + 10) + + clock.tick(COOKIE_ACCESS_DELAY) + + expect(sessionStore.getSession().id).toBe(FIRST_ID) + expect(sessionStore.getSession().expire).toBe(getStoreExpiration()) + expect(expireSpy).not.toHaveBeenCalled() + }) + + it('when session id in cache is different than session id in store, should expire session', () => { + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + setupSessionStore() + setSessionInStore(FakeTrackingType.TRACKED, SECOND_ID) + + clock.tick(COOKIE_ACCESS_DELAY) + + expect(sessionStore.getSession().id).toBeUndefined() + expect(expireSpy).toHaveBeenCalled() + }) + + it('when session type in cache is different than session type in store, should expire session', () => { + setSessionInStore(FakeTrackingType.NOT_TRACKED, FIRST_ID) + setupSessionStore() + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + + clock.tick(COOKIE_ACCESS_DELAY) + + expect(sessionStore.getSession().id).toBeUndefined() + expect(expireSpy).toHaveBeenCalled() + }) + }) + }) + + describe('process operations mechanism', () => { + const COOKIE_OPTIONS = {} + let initialSession: SessionState + let otherSession: SessionState + let processSpy: jasmine.Spy + let afterSpy: jasmine.Spy + let cookie: ReturnType + let cookieStorage: SessionStore + + beforeEach(() => { + cookieStorage = initCookieStore(COOKIE_OPTIONS) + initialSession = { id: '123', created: '0' } + otherSession = { id: '456', created: '100' } + processSpy = jasmine.createSpy('process') + afterSpy = jasmine.createSpy('after') + cookie = stubCookie() + }) + + describe('with cookie-lock disabled', () => { + beforeEach(() => { + isChromium() && pending('cookie-lock only disabled on non chromium browsers') + }) + + it('should persist session when process returns a value', () => { + cookieStorage.persistSession(initialSession) + processSpy.and.returnValue({ ...otherSession }) + + processSessionStoreOperations({ process: processSpy, after: afterSpy }, cookieStorage) + + expect(processSpy).toHaveBeenCalledWith(initialSession) + const expectedSession = { ...otherSession, expire: jasmine.any(String) } + expect(cookieStorage.retrieveSession()).toEqual(expectedSession) + expect(afterSpy).toHaveBeenCalledWith(expectedSession) + }) + + it('should clear session when process return an empty value', () => { + cookieStorage.persistSession(initialSession) + processSpy.and.returnValue({}) + + processSessionStoreOperations({ process: processSpy, after: afterSpy }, cookieStorage) + + expect(processSpy).toHaveBeenCalledWith(initialSession) + const expectedSession = {} + expect(cookieStorage.retrieveSession()).toEqual(expectedSession) + expect(afterSpy).toHaveBeenCalledWith(expectedSession) + }) + + it('should not persist session when process return undefined', () => { + cookieStorage.persistSession(initialSession) + processSpy.and.returnValue(undefined) + + processSessionStoreOperations({ process: processSpy, after: afterSpy }, cookieStorage) + + expect(processSpy).toHaveBeenCalledWith(initialSession) + expect(cookieStorage.retrieveSession()).toEqual(initialSession) + expect(afterSpy).toHaveBeenCalledWith(initialSession) + }) + }) + + describe('with cookie-lock enabled', () => { + beforeEach(() => { + !isChromium() && pending('cookie-lock only enabled on chromium browsers') + }) + + it('should persist session when process return a value', () => { + cookieStorage.persistSession(initialSession) + processSpy.and.callFake((session) => ({ ...otherSession, lock: session.lock })) + + processSessionStoreOperations({ process: processSpy, after: afterSpy }, cookieStorage) + + expect(processSpy).toHaveBeenCalledWith({ ...initialSession, lock: jasmine.any(String) }) + const expectedSession = { ...otherSession, expire: jasmine.any(String) } + expect(cookieStorage.retrieveSession()).toEqual(expectedSession) + expect(afterSpy).toHaveBeenCalledWith(expectedSession) + }) + + it('should clear session when process return an empty value', () => { + cookieStorage.persistSession(initialSession) + processSpy.and.returnValue({}) + + processSessionStoreOperations({ process: processSpy, after: afterSpy }, cookieStorage) + + expect(processSpy).toHaveBeenCalledWith({ ...initialSession, lock: jasmine.any(String) }) + const expectedSession = {} + expect(cookieStorage.retrieveSession()).toEqual(expectedSession) + expect(afterSpy).toHaveBeenCalledWith(expectedSession) + }) + + it('should not persist session when process return undefined', () => { + cookieStorage.persistSession(initialSession) + processSpy.and.returnValue(undefined) + + processSessionStoreOperations({ process: processSpy, after: afterSpy }, cookieStorage) + + expect(processSpy).toHaveBeenCalledWith({ ...initialSession, lock: jasmine.any(String) }) + expect(cookieStorage.retrieveSession()).toEqual(initialSession) + expect(afterSpy).toHaveBeenCalledWith(initialSession) + }) + + type OnLockCheck = () => { currentState: SessionState; retryState: SessionState } + + function lockScenario({ + onInitialLockCheck, + onAcquiredLockCheck, + onPostProcessLockCheck, + onPostPersistLockCheck, + }: { + onInitialLockCheck?: OnLockCheck + onAcquiredLockCheck?: OnLockCheck + onPostProcessLockCheck?: OnLockCheck + onPostPersistLockCheck?: OnLockCheck + }) { + const onLockChecks = [onInitialLockCheck, onAcquiredLockCheck, onPostProcessLockCheck, onPostPersistLockCheck] + cookie.getSpy.and.callFake(() => { + const currentOnLockCheck = onLockChecks.shift() + if (!currentOnLockCheck) { + return cookie.currentValue() + } + const { currentState, retryState } = currentOnLockCheck() + cookie.setCurrentValue(buildSessionString(retryState)) + return buildSessionString(currentState) + }) + } + + function buildSessionString(currentState: SessionState) { + return `${SESSION_COOKIE_NAME}=${toSessionString(currentState)}` + } + + ;[ + { + description: 'should wait for lock to be free', + lockConflict: 'onInitialLockCheck', + }, + { + description: 'should retry if lock was acquired before process', + lockConflict: 'onAcquiredLockCheck', + }, + { + description: 'should retry if lock was acquired after process', + lockConflict: 'onPostProcessLockCheck', + }, + { + description: 'should retry if lock was acquired after persist', + lockConflict: 'onPostPersistLockCheck', + }, + ].forEach(({ description, lockConflict }) => { + it(description, (done) => { + lockScenario({ + [lockConflict]: () => ({ + currentState: { ...initialSession, lock: 'locked' }, + retryState: { ...initialSession, other: 'other' }, + }), + }) + initialSession.expire = String(Date.now() + SESSION_EXPIRATION_DELAY) + cookieStorage.persistSession(initialSession) + processSpy.and.callFake((session) => ({ ...session, processed: 'processed' } as SessionState)) + + processSessionStoreOperations( + { + process: processSpy, + after: (afterSession) => { + // session with 'other' value on process + expect(processSpy).toHaveBeenCalledWith({ + ...initialSession, + other: 'other', + lock: jasmine.any(String), + expire: jasmine.any(String), + }) + + // end state with session 'other' and 'processed' value + const expectedSession = { + ...initialSession, + other: 'other', + processed: 'processed', + expire: jasmine.any(String), + } + expect(cookieStorage.retrieveSession()).toEqual(expectedSession) + expect(afterSession).toEqual(expectedSession) + done() + }, + }, + cookieStorage + ) + }) + }) + + it('should abort after a max number of retry', () => { + const clock = mockClock() + + cookieStorage.persistSession(initialSession) + cookie.setSpy.calls.reset() + + cookie.getSpy.and.returnValue(buildSessionString({ ...initialSession, lock: 'locked' })) + processSessionStoreOperations({ process: processSpy, after: afterSpy }, cookieStorage) + + const lockMaxTries = cookieStorage.storeAccessOptions.lockEnabled + ? cookieStorage.storeAccessOptions.lockMaxTries + : 0 + const lockRetryDelay = cookieStorage.storeAccessOptions.lockEnabled + ? cookieStorage.storeAccessOptions.lockRetryDelay + : 0 + + clock.tick(lockMaxTries * lockRetryDelay) + expect(processSpy).not.toHaveBeenCalled() + expect(afterSpy).not.toHaveBeenCalled() + expect(cookie.setSpy).not.toHaveBeenCalled() + + clock.cleanup() + }) + + it('should execute cookie accesses in order', (done) => { + lockScenario({ + onInitialLockCheck: () => ({ + currentState: { ...initialSession, lock: 'locked' }, // force to retry the first access later + retryState: initialSession, + }), + }) + cookieStorage.persistSession(initialSession) + + processSessionStoreOperations( + { + process: (session) => ({ ...session, value: 'foo' }), + after: afterSpy, + }, + cookieStorage + ) + processSessionStoreOperations( + { + process: (session) => ({ ...session, value: `${session.value || ''}bar` }), + after: (session) => { + expect(session.value).toBe('foobar') + expect(afterSpy).toHaveBeenCalled() + done() + }, + }, + cookieStorage + ) + }) + }) + }) +}) diff --git a/packages/core/src/domain/session/sessionStoreManager.ts b/packages/core/src/domain/session/sessionStoreManager.ts new file mode 100644 index 0000000000..d410317e3e --- /dev/null +++ b/packages/core/src/domain/session/sessionStoreManager.ts @@ -0,0 +1,256 @@ +import { clearInterval, setInterval, setTimeout } from '../../tools/timer' +import { Observable } from '../../tools/observable' +import { dateNow } from '../../tools/utils/timeUtils' +import { throttle } from '../../tools/utils/functionUtils' +import { generateUUID } from '../../tools/utils/stringUtils' +import { SESSION_EXPIRATION_DELAY, SESSION_TIME_OUT_DELAY } from './sessionConstants' +import { initCookieStore } from './sessionCookieStore' +import type { SessionState, SessionStore, StoreInitOptions } from './sessionStore' +import { isSessionInExpiredState } from './sessionStore' + +export interface SessionStoreManager { + expandOrRenewSession: () => void + expandSession: () => void + getSession: () => SessionState + renewObservable: Observable + expireObservable: Observable + expire: () => void + stop: () => void +} + +/** + * Different session concepts: + * - tracked, the session has an id and is updated along the user navigation + * - not tracked, the session does not have an id but it is updated along the user navigation + * - inactive, no session in store or session expired, waiting for a renew session + */ +export function startSessionStoreManager( + options: StoreInitOptions, + productKey: string, + computeSessionState: (rawTrackingType?: string) => { trackingType: TrackingType; isTracked: boolean } +): SessionStoreManager { + const renewObservable = new Observable() + const expireObservable = new Observable() + + const sessionStore = initCookieStore(options) + const { clearSession, retrieveSession, storeAccessOptions } = sessionStore + + const watchSessionTimeoutId = setInterval(watchSession, storeAccessOptions.pollDelay) + let sessionCache: SessionState = retrieveActiveSession() + + function expandOrRenewSession() { + let isTracked: boolean + processSessionStoreOperations( + { + process: (sessionState) => { + const synchronizedSession = synchronizeSession(sessionState) + isTracked = expandOrRenewCookie(synchronizedSession) + return synchronizedSession + }, + after: (sessionState) => { + if (isTracked && !hasSessionInCache()) { + renewSessionInCache(sessionState) + } + sessionCache = sessionState + }, + }, + sessionStore + ) + } + + function expandSession() { + processSessionStoreOperations( + { + process: (sessionState) => (hasSessionInCache() ? synchronizeSession(sessionState) : undefined), + }, + sessionStore + ) + } + + /** + * allows two behaviors: + * - if the session is active, synchronize the session cache without updating the session store + * - if the session is not active, clear the session store and expire the session cache + */ + function watchSession() { + processSessionStoreOperations( + { + process: (sessionState) => (!isActiveSession(sessionState) ? {} : undefined), + after: synchronizeSession, + }, + sessionStore + ) + } + + function synchronizeSession(sessionState: SessionState) { + if (!isActiveSession(sessionState)) { + sessionState = {} + } + if (hasSessionInCache()) { + if (isSessionInCacheOutdated(sessionState)) { + expireSessionInCache() + } else { + sessionCache = sessionState + } + } + return sessionState + } + + function expandOrRenewCookie(sessionState: SessionState) { + const { trackingType, isTracked } = computeSessionState(sessionState[productKey]) + sessionState[productKey] = trackingType + if (isTracked && !sessionState.id) { + sessionState.id = generateUUID() + sessionState.created = String(dateNow()) + } + return isTracked + } + + function hasSessionInCache() { + return sessionCache[productKey] !== undefined + } + + function isSessionInCacheOutdated(sessionState: SessionState) { + return sessionCache.id !== sessionState.id || sessionCache[productKey] !== sessionState[productKey] + } + + function expireSessionInCache() { + sessionCache = {} + expireObservable.notify() + } + + function renewSessionInCache(sessionState: SessionState) { + sessionCache = sessionState + renewObservable.notify() + } + + function retrieveActiveSession(): SessionState { + const session = retrieveSession() + if (isActiveSession(session)) { + return session + } + return {} + } + + function isActiveSession(sessionDate: SessionState) { + // created and expire can be undefined for versions which was not storing them + // these checks could be removed when older versions will not be available/live anymore + return ( + (sessionDate.created === undefined || dateNow() - Number(sessionDate.created) < SESSION_TIME_OUT_DELAY) && + (sessionDate.expire === undefined || dateNow() < Number(sessionDate.expire)) + ) + } + + return { + expandOrRenewSession: throttle(expandOrRenewSession, storeAccessOptions.pollDelay).throttled, + expandSession, + getSession: () => sessionCache, + renewObservable, + expireObservable, + expire: () => { + clearSession() + synchronizeSession({}) + }, + stop: () => { + clearInterval(watchSessionTimeoutId) + }, + } +} + +type Operations = { + process: (sessionState: SessionState) => SessionState | undefined + after?: (sessionState: SessionState) => void +} + +const bufferedOperations: Operations[] = [] +let ongoingOperations: Operations | undefined + +export function processSessionStoreOperations(operations: Operations, sessionStore: SessionStore, numberOfRetries = 0) { + const { retrieveSession, persistSession, clearSession, storeAccessOptions } = sessionStore + + if (!ongoingOperations) { + ongoingOperations = operations + } + if (operations !== ongoingOperations) { + bufferedOperations.push(operations) + return + } + if (storeAccessOptions.lockEnabled && numberOfRetries >= storeAccessOptions.lockMaxTries) { + next(sessionStore) + return + } + let currentLock: string + let currentSession = retrieveSession() + if (storeAccessOptions.lockEnabled) { + // if someone has lock, retry later + if (currentSession.lock) { + retryLater(operations, sessionStore, numberOfRetries, storeAccessOptions.lockRetryDelay) + return + } + // acquire lock + currentLock = generateUUID() + currentSession.lock = currentLock + persistSession(currentSession) + // if lock is not acquired, retry later + currentSession = retrieveSession() + if (currentSession.lock !== currentLock) { + retryLater(operations, sessionStore, numberOfRetries, storeAccessOptions.lockRetryDelay) + return + } + } + let processedSession = operations.process(currentSession) + if (storeAccessOptions.lockEnabled) { + // if lock corrupted after process, retry later + currentSession = retrieveSession() + if (currentSession.lock !== currentLock!) { + retryLater(operations, sessionStore, numberOfRetries, storeAccessOptions.lockRetryDelay) + return + } + } + if (processedSession) { + if (isSessionInExpiredState(processedSession)) { + clearSession() + } else { + processedSession.expire = String(dateNow() + SESSION_EXPIRATION_DELAY) + persistSession(processedSession) + } + } + if (storeAccessOptions.lockEnabled) { + // correctly handle lock around expiration would require to handle this case properly at several levels + // since we don't have evidence of lock issues around expiration, let's just not do the corruption check for it + if (!(processedSession && isSessionInExpiredState(processedSession))) { + // if lock corrupted after persist, retry later + currentSession = retrieveSession() + if (currentSession.lock !== currentLock!) { + retryLater(operations, sessionStore, numberOfRetries, storeAccessOptions.lockRetryDelay) + return + } + delete currentSession.lock + persistSession(currentSession) + processedSession = currentSession + } + } + // call after even if session is not persisted in order to perform operations on + // up-to-date session state value => the value could have been modified by another tab + operations.after?.(processedSession || currentSession) + next(sessionStore) +} + +function retryLater( + operations: Operations, + sessionStore: SessionStore, + currentNumberOfRetries: number, + retryDelay: number +) { + setTimeout(() => { + processSessionStoreOperations(operations, sessionStore, currentNumberOfRetries + 1) + }, retryDelay) +} + +function next(sessionStore: SessionStore) { + ongoingOperations = undefined + const nextOperations = bufferedOperations.shift() + if (nextOperations) { + processSessionStoreOperations(nextOperations, sessionStore) + } +} From 17920d8b005f902c15f967e2f556c3827456c0e9 Mon Sep 17 00:00:00 2001 From: Yannick Adam Date: Thu, 1 Jun 2023 09:11:21 +0200 Subject: [PATCH 08/13] =?UTF-8?q?=F0=9F=91=8C=20Removed=20StoreAccessOptio?= =?UTF-8?q?ns?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/domain/session/sessionCookieStore.ts | 17 +------ .../core/src/domain/session/sessionStore.ts | 13 ----- .../session/sessionStoreManager.spec.ts | 16 ++++--- .../src/domain/session/sessionStoreManager.ts | 48 +++++++++++-------- 4 files changed, 38 insertions(+), 56 deletions(-) diff --git a/packages/core/src/domain/session/sessionCookieStore.ts b/packages/core/src/domain/session/sessionCookieStore.ts index 57b6b549c7..a22b1fc2f2 100644 --- a/packages/core/src/domain/session/sessionCookieStore.ts +++ b/packages/core/src/domain/session/sessionCookieStore.ts @@ -1,6 +1,5 @@ import type { CookieOptions } from '../../browser/cookie' -import { COOKIE_ACCESS_DELAY, deleteCookie, getCookie, setCookie } from '../../browser/cookie' -import { isChromium } from '../../tools/utils/browserDetection' +import { deleteCookie, getCookie, setCookie } from '../../browser/cookie' import { objectEntries } from '../../tools/utils/polyfills' import { SESSION_EXPIRATION_DELAY } from './sessionConstants' import type { SessionState, SessionStore } from './sessionStore' @@ -10,22 +9,8 @@ const SESSION_ENTRY_SEPARATOR = '&' export const SESSION_COOKIE_NAME = '_dd_s' -// Arbitrary values -export const LOCK_RETRY_DELAY = 10 -export const MAX_NUMBER_OF_LOCK_RETRIES = 100 - export function initCookieStore(options: CookieOptions): SessionStore { return { - storeAccessOptions: { - pollDelay: COOKIE_ACCESS_DELAY, - /** - * Cookie lock strategy allows mitigating issues due to concurrent access to cookie. - * This issue concerns only chromium browsers and enabling this on firefox increase cookie write failures. - */ - lockEnabled: isChromium(), - lockRetryDelay: LOCK_RETRY_DELAY, - lockMaxTries: MAX_NUMBER_OF_LOCK_RETRIES, - }, persistSession: persistSessionCookie(options), retrieveSession: retrieveSessionCookie, clearSession: deleteSessionCookie(options), diff --git a/packages/core/src/domain/session/sessionStore.ts b/packages/core/src/domain/session/sessionStore.ts index 18049f846b..7399e927a9 100644 --- a/packages/core/src/domain/session/sessionStore.ts +++ b/packages/core/src/domain/session/sessionStore.ts @@ -10,22 +10,9 @@ export interface SessionState { [key: string]: string | undefined } -type StoreAccessOptionsWithLock = { - pollDelay: number - lockEnabled: true - lockRetryDelay: number - lockMaxTries: number -} - -type StoreAccessOptionsWithoutLock = { - pollDelay: number - lockEnabled: false -} - export type StoreInitOptions = CookieOptions export interface SessionStore { - storeAccessOptions: StoreAccessOptionsWithLock | StoreAccessOptionsWithoutLock persistSession: (session: SessionState) => void retrieveSession: () => SessionState clearSession: () => void diff --git a/packages/core/src/domain/session/sessionStoreManager.spec.ts b/packages/core/src/domain/session/sessionStoreManager.spec.ts index 3751ea21fb..532bf51509 100644 --- a/packages/core/src/domain/session/sessionStoreManager.spec.ts +++ b/packages/core/src/domain/session/sessionStoreManager.spec.ts @@ -4,7 +4,13 @@ import type { CookieOptions } from '../../browser/cookie' import { getCookie, setCookie, COOKIE_ACCESS_DELAY } from '../../browser/cookie' import { isChromium } from '../../tools/utils/browserDetection' import type { SessionStoreManager } from './sessionStoreManager' -import { processSessionStoreOperations, startSessionStoreManager } from './sessionStoreManager' +import { + LOCK_MAX_TRIES, + LOCK_RETRY_DELAY, + isLockEnabled, + processSessionStoreOperations, + startSessionStoreManager, +} from './sessionStoreManager' import { SESSION_COOKIE_NAME, initCookieStore, toSessionString } from './sessionCookieStore' import { SESSION_EXPIRATION_DELAY, SESSION_TIME_OUT_DELAY } from './sessionConstants' import type { SessionState, SessionStore } from './sessionStore' @@ -549,12 +555,8 @@ describe('session store', () => { cookie.getSpy.and.returnValue(buildSessionString({ ...initialSession, lock: 'locked' })) processSessionStoreOperations({ process: processSpy, after: afterSpy }, cookieStorage) - const lockMaxTries = cookieStorage.storeAccessOptions.lockEnabled - ? cookieStorage.storeAccessOptions.lockMaxTries - : 0 - const lockRetryDelay = cookieStorage.storeAccessOptions.lockEnabled - ? cookieStorage.storeAccessOptions.lockRetryDelay - : 0 + const lockMaxTries = isLockEnabled() ? LOCK_MAX_TRIES : 0 + const lockRetryDelay = isLockEnabled() ? LOCK_RETRY_DELAY : 0 clock.tick(lockMaxTries * lockRetryDelay) expect(processSpy).not.toHaveBeenCalled() diff --git a/packages/core/src/domain/session/sessionStoreManager.ts b/packages/core/src/domain/session/sessionStoreManager.ts index d410317e3e..01bc61228d 100644 --- a/packages/core/src/domain/session/sessionStoreManager.ts +++ b/packages/core/src/domain/session/sessionStoreManager.ts @@ -1,8 +1,9 @@ import { clearInterval, setInterval, setTimeout } from '../../tools/timer' import { Observable } from '../../tools/observable' -import { dateNow } from '../../tools/utils/timeUtils' +import { ONE_SECOND, dateNow } from '../../tools/utils/timeUtils' import { throttle } from '../../tools/utils/functionUtils' import { generateUUID } from '../../tools/utils/stringUtils' +import { isChromium } from '../../tools/utils/browserDetection' import { SESSION_EXPIRATION_DELAY, SESSION_TIME_OUT_DELAY } from './sessionConstants' import { initCookieStore } from './sessionCookieStore' import type { SessionState, SessionStore, StoreInitOptions } from './sessionStore' @@ -18,6 +19,8 @@ export interface SessionStoreManager { stop: () => void } +const POLL_DELAY = ONE_SECOND + /** * Different session concepts: * - tracked, the session has an id and is updated along the user navigation @@ -33,9 +36,9 @@ export function startSessionStoreManager( const expireObservable = new Observable() const sessionStore = initCookieStore(options) - const { clearSession, retrieveSession, storeAccessOptions } = sessionStore + const { clearSession, retrieveSession } = sessionStore - const watchSessionTimeoutId = setInterval(watchSession, storeAccessOptions.pollDelay) + const watchSessionTimeoutId = setInterval(watchSession, POLL_DELAY) let sessionCache: SessionState = retrieveActiveSession() function expandOrRenewSession() { @@ -142,7 +145,7 @@ export function startSessionStoreManager( } return { - expandOrRenewSession: throttle(expandOrRenewSession, storeAccessOptions.pollDelay).throttled, + expandOrRenewSession: throttle(expandOrRenewSession, POLL_DELAY).throttled, expandSession, getSession: () => sessionCache, renewObservable, @@ -162,11 +165,15 @@ type Operations = { after?: (sessionState: SessionState) => void } +export const LOCK_RETRY_DELAY = 10 +export const LOCK_MAX_TRIES = 100 + const bufferedOperations: Operations[] = [] let ongoingOperations: Operations | undefined export function processSessionStoreOperations(operations: Operations, sessionStore: SessionStore, numberOfRetries = 0) { - const { retrieveSession, persistSession, clearSession, storeAccessOptions } = sessionStore + const { retrieveSession, persistSession, clearSession } = sessionStore + const lockEnabled = isLockEnabled() if (!ongoingOperations) { ongoingOperations = operations @@ -175,16 +182,16 @@ export function processSessionStoreOperations(operations: Operations, sessionSto bufferedOperations.push(operations) return } - if (storeAccessOptions.lockEnabled && numberOfRetries >= storeAccessOptions.lockMaxTries) { + if (lockEnabled && numberOfRetries >= LOCK_MAX_TRIES) { next(sessionStore) return } let currentLock: string let currentSession = retrieveSession() - if (storeAccessOptions.lockEnabled) { + if (lockEnabled) { // if someone has lock, retry later if (currentSession.lock) { - retryLater(operations, sessionStore, numberOfRetries, storeAccessOptions.lockRetryDelay) + retryLater(operations, sessionStore, numberOfRetries) return } // acquire lock @@ -194,16 +201,16 @@ export function processSessionStoreOperations(operations: Operations, sessionSto // if lock is not acquired, retry later currentSession = retrieveSession() if (currentSession.lock !== currentLock) { - retryLater(operations, sessionStore, numberOfRetries, storeAccessOptions.lockRetryDelay) + retryLater(operations, sessionStore, numberOfRetries) return } } let processedSession = operations.process(currentSession) - if (storeAccessOptions.lockEnabled) { + if (lockEnabled) { // if lock corrupted after process, retry later currentSession = retrieveSession() if (currentSession.lock !== currentLock!) { - retryLater(operations, sessionStore, numberOfRetries, storeAccessOptions.lockRetryDelay) + retryLater(operations, sessionStore, numberOfRetries) return } } @@ -215,14 +222,14 @@ export function processSessionStoreOperations(operations: Operations, sessionSto persistSession(processedSession) } } - if (storeAccessOptions.lockEnabled) { + if (lockEnabled) { // correctly handle lock around expiration would require to handle this case properly at several levels // since we don't have evidence of lock issues around expiration, let's just not do the corruption check for it if (!(processedSession && isSessionInExpiredState(processedSession))) { // if lock corrupted after persist, retry later currentSession = retrieveSession() if (currentSession.lock !== currentLock!) { - retryLater(operations, sessionStore, numberOfRetries, storeAccessOptions.lockRetryDelay) + retryLater(operations, sessionStore, numberOfRetries) return } delete currentSession.lock @@ -236,15 +243,16 @@ export function processSessionStoreOperations(operations: Operations, sessionSto next(sessionStore) } -function retryLater( - operations: Operations, - sessionStore: SessionStore, - currentNumberOfRetries: number, - retryDelay: number -) { +/** + * Lock strategy allows mitigating issues due to concurrent access to cookie. + * This issue concerns only chromium browsers and enabling this on firefox increases cookie write failures. + */ +export const isLockEnabled = () => isChromium() + +function retryLater(operations: Operations, sessionStore: SessionStore, currentNumberOfRetries: number) { setTimeout(() => { processSessionStoreOperations(operations, sessionStore, currentNumberOfRetries + 1) - }, retryDelay) + }, LOCK_RETRY_DELAY) } function next(sessionStore: SessionStore) { From bc24152408c885047566aeeca2dd39975f1df68c Mon Sep 17 00:00:00 2001 From: Yannick Adam Date: Thu, 1 Jun 2023 09:21:17 +0200 Subject: [PATCH 09/13] =?UTF-8?q?=F0=9F=91=8C=20Extract=20sessionStoreOper?= =?UTF-8?q?ations=20in=20its=20own=20file?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../session/sessionStoreManager.spec.ts | 251 +----------------- .../src/domain/session/sessionStoreManager.ts | 112 +------- .../session/sessionStoreOperations.spec.ts | 248 +++++++++++++++++ .../domain/session/sessionStoreOperations.ts | 107 ++++++++ 4 files changed, 362 insertions(+), 356 deletions(-) create mode 100644 packages/core/src/domain/session/sessionStoreOperations.spec.ts create mode 100644 packages/core/src/domain/session/sessionStoreOperations.ts diff --git a/packages/core/src/domain/session/sessionStoreManager.spec.ts b/packages/core/src/domain/session/sessionStoreManager.spec.ts index 532bf51509..53b6faf90d 100644 --- a/packages/core/src/domain/session/sessionStoreManager.spec.ts +++ b/packages/core/src/domain/session/sessionStoreManager.spec.ts @@ -1,19 +1,11 @@ import type { Clock } from '../../../test' -import { stubCookie, mockClock } from '../../../test' +import { mockClock } from '../../../test' import type { CookieOptions } from '../../browser/cookie' import { getCookie, setCookie, COOKIE_ACCESS_DELAY } from '../../browser/cookie' -import { isChromium } from '../../tools/utils/browserDetection' import type { SessionStoreManager } from './sessionStoreManager' -import { - LOCK_MAX_TRIES, - LOCK_RETRY_DELAY, - isLockEnabled, - processSessionStoreOperations, - startSessionStoreManager, -} from './sessionStoreManager' -import { SESSION_COOKIE_NAME, initCookieStore, toSessionString } from './sessionCookieStore' +import { startSessionStoreManager } from './sessionStoreManager' +import { SESSION_COOKIE_NAME } from './sessionCookieStore' import { SESSION_EXPIRATION_DELAY, SESSION_TIME_OUT_DELAY } from './sessionConstants' -import type { SessionState, SessionStore } from './sessionStore' const enum FakeTrackingType { TRACKED = 'tracked', @@ -359,241 +351,4 @@ describe('session store', () => { }) }) }) - - describe('process operations mechanism', () => { - const COOKIE_OPTIONS = {} - let initialSession: SessionState - let otherSession: SessionState - let processSpy: jasmine.Spy - let afterSpy: jasmine.Spy - let cookie: ReturnType - let cookieStorage: SessionStore - - beforeEach(() => { - cookieStorage = initCookieStore(COOKIE_OPTIONS) - initialSession = { id: '123', created: '0' } - otherSession = { id: '456', created: '100' } - processSpy = jasmine.createSpy('process') - afterSpy = jasmine.createSpy('after') - cookie = stubCookie() - }) - - describe('with cookie-lock disabled', () => { - beforeEach(() => { - isChromium() && pending('cookie-lock only disabled on non chromium browsers') - }) - - it('should persist session when process returns a value', () => { - cookieStorage.persistSession(initialSession) - processSpy.and.returnValue({ ...otherSession }) - - processSessionStoreOperations({ process: processSpy, after: afterSpy }, cookieStorage) - - expect(processSpy).toHaveBeenCalledWith(initialSession) - const expectedSession = { ...otherSession, expire: jasmine.any(String) } - expect(cookieStorage.retrieveSession()).toEqual(expectedSession) - expect(afterSpy).toHaveBeenCalledWith(expectedSession) - }) - - it('should clear session when process return an empty value', () => { - cookieStorage.persistSession(initialSession) - processSpy.and.returnValue({}) - - processSessionStoreOperations({ process: processSpy, after: afterSpy }, cookieStorage) - - expect(processSpy).toHaveBeenCalledWith(initialSession) - const expectedSession = {} - expect(cookieStorage.retrieveSession()).toEqual(expectedSession) - expect(afterSpy).toHaveBeenCalledWith(expectedSession) - }) - - it('should not persist session when process return undefined', () => { - cookieStorage.persistSession(initialSession) - processSpy.and.returnValue(undefined) - - processSessionStoreOperations({ process: processSpy, after: afterSpy }, cookieStorage) - - expect(processSpy).toHaveBeenCalledWith(initialSession) - expect(cookieStorage.retrieveSession()).toEqual(initialSession) - expect(afterSpy).toHaveBeenCalledWith(initialSession) - }) - }) - - describe('with cookie-lock enabled', () => { - beforeEach(() => { - !isChromium() && pending('cookie-lock only enabled on chromium browsers') - }) - - it('should persist session when process return a value', () => { - cookieStorage.persistSession(initialSession) - processSpy.and.callFake((session) => ({ ...otherSession, lock: session.lock })) - - processSessionStoreOperations({ process: processSpy, after: afterSpy }, cookieStorage) - - expect(processSpy).toHaveBeenCalledWith({ ...initialSession, lock: jasmine.any(String) }) - const expectedSession = { ...otherSession, expire: jasmine.any(String) } - expect(cookieStorage.retrieveSession()).toEqual(expectedSession) - expect(afterSpy).toHaveBeenCalledWith(expectedSession) - }) - - it('should clear session when process return an empty value', () => { - cookieStorage.persistSession(initialSession) - processSpy.and.returnValue({}) - - processSessionStoreOperations({ process: processSpy, after: afterSpy }, cookieStorage) - - expect(processSpy).toHaveBeenCalledWith({ ...initialSession, lock: jasmine.any(String) }) - const expectedSession = {} - expect(cookieStorage.retrieveSession()).toEqual(expectedSession) - expect(afterSpy).toHaveBeenCalledWith(expectedSession) - }) - - it('should not persist session when process return undefined', () => { - cookieStorage.persistSession(initialSession) - processSpy.and.returnValue(undefined) - - processSessionStoreOperations({ process: processSpy, after: afterSpy }, cookieStorage) - - expect(processSpy).toHaveBeenCalledWith({ ...initialSession, lock: jasmine.any(String) }) - expect(cookieStorage.retrieveSession()).toEqual(initialSession) - expect(afterSpy).toHaveBeenCalledWith(initialSession) - }) - - type OnLockCheck = () => { currentState: SessionState; retryState: SessionState } - - function lockScenario({ - onInitialLockCheck, - onAcquiredLockCheck, - onPostProcessLockCheck, - onPostPersistLockCheck, - }: { - onInitialLockCheck?: OnLockCheck - onAcquiredLockCheck?: OnLockCheck - onPostProcessLockCheck?: OnLockCheck - onPostPersistLockCheck?: OnLockCheck - }) { - const onLockChecks = [onInitialLockCheck, onAcquiredLockCheck, onPostProcessLockCheck, onPostPersistLockCheck] - cookie.getSpy.and.callFake(() => { - const currentOnLockCheck = onLockChecks.shift() - if (!currentOnLockCheck) { - return cookie.currentValue() - } - const { currentState, retryState } = currentOnLockCheck() - cookie.setCurrentValue(buildSessionString(retryState)) - return buildSessionString(currentState) - }) - } - - function buildSessionString(currentState: SessionState) { - return `${SESSION_COOKIE_NAME}=${toSessionString(currentState)}` - } - - ;[ - { - description: 'should wait for lock to be free', - lockConflict: 'onInitialLockCheck', - }, - { - description: 'should retry if lock was acquired before process', - lockConflict: 'onAcquiredLockCheck', - }, - { - description: 'should retry if lock was acquired after process', - lockConflict: 'onPostProcessLockCheck', - }, - { - description: 'should retry if lock was acquired after persist', - lockConflict: 'onPostPersistLockCheck', - }, - ].forEach(({ description, lockConflict }) => { - it(description, (done) => { - lockScenario({ - [lockConflict]: () => ({ - currentState: { ...initialSession, lock: 'locked' }, - retryState: { ...initialSession, other: 'other' }, - }), - }) - initialSession.expire = String(Date.now() + SESSION_EXPIRATION_DELAY) - cookieStorage.persistSession(initialSession) - processSpy.and.callFake((session) => ({ ...session, processed: 'processed' } as SessionState)) - - processSessionStoreOperations( - { - process: processSpy, - after: (afterSession) => { - // session with 'other' value on process - expect(processSpy).toHaveBeenCalledWith({ - ...initialSession, - other: 'other', - lock: jasmine.any(String), - expire: jasmine.any(String), - }) - - // end state with session 'other' and 'processed' value - const expectedSession = { - ...initialSession, - other: 'other', - processed: 'processed', - expire: jasmine.any(String), - } - expect(cookieStorage.retrieveSession()).toEqual(expectedSession) - expect(afterSession).toEqual(expectedSession) - done() - }, - }, - cookieStorage - ) - }) - }) - - it('should abort after a max number of retry', () => { - const clock = mockClock() - - cookieStorage.persistSession(initialSession) - cookie.setSpy.calls.reset() - - cookie.getSpy.and.returnValue(buildSessionString({ ...initialSession, lock: 'locked' })) - processSessionStoreOperations({ process: processSpy, after: afterSpy }, cookieStorage) - - const lockMaxTries = isLockEnabled() ? LOCK_MAX_TRIES : 0 - const lockRetryDelay = isLockEnabled() ? LOCK_RETRY_DELAY : 0 - - clock.tick(lockMaxTries * lockRetryDelay) - expect(processSpy).not.toHaveBeenCalled() - expect(afterSpy).not.toHaveBeenCalled() - expect(cookie.setSpy).not.toHaveBeenCalled() - - clock.cleanup() - }) - - it('should execute cookie accesses in order', (done) => { - lockScenario({ - onInitialLockCheck: () => ({ - currentState: { ...initialSession, lock: 'locked' }, // force to retry the first access later - retryState: initialSession, - }), - }) - cookieStorage.persistSession(initialSession) - - processSessionStoreOperations( - { - process: (session) => ({ ...session, value: 'foo' }), - after: afterSpy, - }, - cookieStorage - ) - processSessionStoreOperations( - { - process: (session) => ({ ...session, value: `${session.value || ''}bar` }), - after: (session) => { - expect(session.value).toBe('foobar') - expect(afterSpy).toHaveBeenCalled() - done() - }, - }, - cookieStorage - ) - }) - }) - }) }) diff --git a/packages/core/src/domain/session/sessionStoreManager.ts b/packages/core/src/domain/session/sessionStoreManager.ts index 01bc61228d..4cfe639431 100644 --- a/packages/core/src/domain/session/sessionStoreManager.ts +++ b/packages/core/src/domain/session/sessionStoreManager.ts @@ -1,13 +1,12 @@ -import { clearInterval, setInterval, setTimeout } from '../../tools/timer' +import { clearInterval, setInterval } from '../../tools/timer' import { Observable } from '../../tools/observable' import { ONE_SECOND, dateNow } from '../../tools/utils/timeUtils' import { throttle } from '../../tools/utils/functionUtils' import { generateUUID } from '../../tools/utils/stringUtils' -import { isChromium } from '../../tools/utils/browserDetection' -import { SESSION_EXPIRATION_DELAY, SESSION_TIME_OUT_DELAY } from './sessionConstants' +import { SESSION_TIME_OUT_DELAY } from './sessionConstants' import { initCookieStore } from './sessionCookieStore' -import type { SessionState, SessionStore, StoreInitOptions } from './sessionStore' -import { isSessionInExpiredState } from './sessionStore' +import type { SessionState, StoreInitOptions } from './sessionStore' +import { processSessionStoreOperations } from './sessionStoreOperations' export interface SessionStoreManager { expandOrRenewSession: () => void @@ -159,106 +158,3 @@ export function startSessionStoreManager( }, } } - -type Operations = { - process: (sessionState: SessionState) => SessionState | undefined - after?: (sessionState: SessionState) => void -} - -export const LOCK_RETRY_DELAY = 10 -export const LOCK_MAX_TRIES = 100 - -const bufferedOperations: Operations[] = [] -let ongoingOperations: Operations | undefined - -export function processSessionStoreOperations(operations: Operations, sessionStore: SessionStore, numberOfRetries = 0) { - const { retrieveSession, persistSession, clearSession } = sessionStore - const lockEnabled = isLockEnabled() - - if (!ongoingOperations) { - ongoingOperations = operations - } - if (operations !== ongoingOperations) { - bufferedOperations.push(operations) - return - } - if (lockEnabled && numberOfRetries >= LOCK_MAX_TRIES) { - next(sessionStore) - return - } - let currentLock: string - let currentSession = retrieveSession() - if (lockEnabled) { - // if someone has lock, retry later - if (currentSession.lock) { - retryLater(operations, sessionStore, numberOfRetries) - return - } - // acquire lock - currentLock = generateUUID() - currentSession.lock = currentLock - persistSession(currentSession) - // if lock is not acquired, retry later - currentSession = retrieveSession() - if (currentSession.lock !== currentLock) { - retryLater(operations, sessionStore, numberOfRetries) - return - } - } - let processedSession = operations.process(currentSession) - if (lockEnabled) { - // if lock corrupted after process, retry later - currentSession = retrieveSession() - if (currentSession.lock !== currentLock!) { - retryLater(operations, sessionStore, numberOfRetries) - return - } - } - if (processedSession) { - if (isSessionInExpiredState(processedSession)) { - clearSession() - } else { - processedSession.expire = String(dateNow() + SESSION_EXPIRATION_DELAY) - persistSession(processedSession) - } - } - if (lockEnabled) { - // correctly handle lock around expiration would require to handle this case properly at several levels - // since we don't have evidence of lock issues around expiration, let's just not do the corruption check for it - if (!(processedSession && isSessionInExpiredState(processedSession))) { - // if lock corrupted after persist, retry later - currentSession = retrieveSession() - if (currentSession.lock !== currentLock!) { - retryLater(operations, sessionStore, numberOfRetries) - return - } - delete currentSession.lock - persistSession(currentSession) - processedSession = currentSession - } - } - // call after even if session is not persisted in order to perform operations on - // up-to-date session state value => the value could have been modified by another tab - operations.after?.(processedSession || currentSession) - next(sessionStore) -} - -/** - * Lock strategy allows mitigating issues due to concurrent access to cookie. - * This issue concerns only chromium browsers and enabling this on firefox increases cookie write failures. - */ -export const isLockEnabled = () => isChromium() - -function retryLater(operations: Operations, sessionStore: SessionStore, currentNumberOfRetries: number) { - setTimeout(() => { - processSessionStoreOperations(operations, sessionStore, currentNumberOfRetries + 1) - }, LOCK_RETRY_DELAY) -} - -function next(sessionStore: SessionStore) { - ongoingOperations = undefined - const nextOperations = bufferedOperations.shift() - if (nextOperations) { - processSessionStoreOperations(nextOperations, sessionStore) - } -} diff --git a/packages/core/src/domain/session/sessionStoreOperations.spec.ts b/packages/core/src/domain/session/sessionStoreOperations.spec.ts new file mode 100644 index 0000000000..5fe587fbb2 --- /dev/null +++ b/packages/core/src/domain/session/sessionStoreOperations.spec.ts @@ -0,0 +1,248 @@ +import { stubCookie, mockClock } from '../../../test' +import { isChromium } from '../../tools/utils/browserDetection' +import { SESSION_EXPIRATION_DELAY } from './sessionConstants' +import { initCookieStore, SESSION_COOKIE_NAME, toSessionString } from './sessionCookieStore' +import type { SessionState, SessionStore } from './sessionStore' +import { + processSessionStoreOperations, + isLockEnabled, + LOCK_MAX_TRIES, + LOCK_RETRY_DELAY, +} from './sessionStoreOperations' + +describe('process operations mechanism', () => { + const COOKIE_OPTIONS = {} + let initialSession: SessionState + let otherSession: SessionState + let processSpy: jasmine.Spy + let afterSpy: jasmine.Spy + let cookie: ReturnType + let cookieStorage: SessionStore + + beforeEach(() => { + cookieStorage = initCookieStore(COOKIE_OPTIONS) + initialSession = { id: '123', created: '0' } + otherSession = { id: '456', created: '100' } + processSpy = jasmine.createSpy('process') + afterSpy = jasmine.createSpy('after') + cookie = stubCookie() + }) + + describe('with cookie-lock disabled', () => { + beforeEach(() => { + isChromium() && pending('cookie-lock only disabled on non chromium browsers') + }) + + it('should persist session when process returns a value', () => { + cookieStorage.persistSession(initialSession) + processSpy.and.returnValue({ ...otherSession }) + + processSessionStoreOperations({ process: processSpy, after: afterSpy }, cookieStorage) + + expect(processSpy).toHaveBeenCalledWith(initialSession) + const expectedSession = { ...otherSession, expire: jasmine.any(String) } + expect(cookieStorage.retrieveSession()).toEqual(expectedSession) + expect(afterSpy).toHaveBeenCalledWith(expectedSession) + }) + + it('should clear session when process return an empty value', () => { + cookieStorage.persistSession(initialSession) + processSpy.and.returnValue({}) + + processSessionStoreOperations({ process: processSpy, after: afterSpy }, cookieStorage) + + expect(processSpy).toHaveBeenCalledWith(initialSession) + const expectedSession = {} + expect(cookieStorage.retrieveSession()).toEqual(expectedSession) + expect(afterSpy).toHaveBeenCalledWith(expectedSession) + }) + + it('should not persist session when process return undefined', () => { + cookieStorage.persistSession(initialSession) + processSpy.and.returnValue(undefined) + + processSessionStoreOperations({ process: processSpy, after: afterSpy }, cookieStorage) + + expect(processSpy).toHaveBeenCalledWith(initialSession) + expect(cookieStorage.retrieveSession()).toEqual(initialSession) + expect(afterSpy).toHaveBeenCalledWith(initialSession) + }) + }) + + describe('with cookie-lock enabled', () => { + beforeEach(() => { + !isChromium() && pending('cookie-lock only enabled on chromium browsers') + }) + + it('should persist session when process return a value', () => { + cookieStorage.persistSession(initialSession) + processSpy.and.callFake((session) => ({ ...otherSession, lock: session.lock })) + + processSessionStoreOperations({ process: processSpy, after: afterSpy }, cookieStorage) + + expect(processSpy).toHaveBeenCalledWith({ ...initialSession, lock: jasmine.any(String) }) + const expectedSession = { ...otherSession, expire: jasmine.any(String) } + expect(cookieStorage.retrieveSession()).toEqual(expectedSession) + expect(afterSpy).toHaveBeenCalledWith(expectedSession) + }) + + it('should clear session when process return an empty value', () => { + cookieStorage.persistSession(initialSession) + processSpy.and.returnValue({}) + + processSessionStoreOperations({ process: processSpy, after: afterSpy }, cookieStorage) + + expect(processSpy).toHaveBeenCalledWith({ ...initialSession, lock: jasmine.any(String) }) + const expectedSession = {} + expect(cookieStorage.retrieveSession()).toEqual(expectedSession) + expect(afterSpy).toHaveBeenCalledWith(expectedSession) + }) + + it('should not persist session when process return undefined', () => { + cookieStorage.persistSession(initialSession) + processSpy.and.returnValue(undefined) + + processSessionStoreOperations({ process: processSpy, after: afterSpy }, cookieStorage) + + expect(processSpy).toHaveBeenCalledWith({ ...initialSession, lock: jasmine.any(String) }) + expect(cookieStorage.retrieveSession()).toEqual(initialSession) + expect(afterSpy).toHaveBeenCalledWith(initialSession) + }) + + type OnLockCheck = () => { currentState: SessionState; retryState: SessionState } + + function lockScenario({ + onInitialLockCheck, + onAcquiredLockCheck, + onPostProcessLockCheck, + onPostPersistLockCheck, + }: { + onInitialLockCheck?: OnLockCheck + onAcquiredLockCheck?: OnLockCheck + onPostProcessLockCheck?: OnLockCheck + onPostPersistLockCheck?: OnLockCheck + }) { + const onLockChecks = [onInitialLockCheck, onAcquiredLockCheck, onPostProcessLockCheck, onPostPersistLockCheck] + cookie.getSpy.and.callFake(() => { + const currentOnLockCheck = onLockChecks.shift() + if (!currentOnLockCheck) { + return cookie.currentValue() + } + const { currentState, retryState } = currentOnLockCheck() + cookie.setCurrentValue(buildSessionString(retryState)) + return buildSessionString(currentState) + }) + } + + function buildSessionString(currentState: SessionState) { + return `${SESSION_COOKIE_NAME}=${toSessionString(currentState)}` + } + + ;[ + { + description: 'should wait for lock to be free', + lockConflict: 'onInitialLockCheck', + }, + { + description: 'should retry if lock was acquired before process', + lockConflict: 'onAcquiredLockCheck', + }, + { + description: 'should retry if lock was acquired after process', + lockConflict: 'onPostProcessLockCheck', + }, + { + description: 'should retry if lock was acquired after persist', + lockConflict: 'onPostPersistLockCheck', + }, + ].forEach(({ description, lockConflict }) => { + it(description, (done) => { + lockScenario({ + [lockConflict]: () => ({ + currentState: { ...initialSession, lock: 'locked' }, + retryState: { ...initialSession, other: 'other' }, + }), + }) + initialSession.expire = String(Date.now() + SESSION_EXPIRATION_DELAY) + cookieStorage.persistSession(initialSession) + processSpy.and.callFake((session) => ({ ...session, processed: 'processed' } as SessionState)) + + processSessionStoreOperations( + { + process: processSpy, + after: (afterSession) => { + // session with 'other' value on process + expect(processSpy).toHaveBeenCalledWith({ + ...initialSession, + other: 'other', + lock: jasmine.any(String), + expire: jasmine.any(String), + }) + + // end state with session 'other' and 'processed' value + const expectedSession = { + ...initialSession, + other: 'other', + processed: 'processed', + expire: jasmine.any(String), + } + expect(cookieStorage.retrieveSession()).toEqual(expectedSession) + expect(afterSession).toEqual(expectedSession) + done() + }, + }, + cookieStorage + ) + }) + }) + + it('should abort after a max number of retry', () => { + const clock = mockClock() + + cookieStorage.persistSession(initialSession) + cookie.setSpy.calls.reset() + + cookie.getSpy.and.returnValue(buildSessionString({ ...initialSession, lock: 'locked' })) + processSessionStoreOperations({ process: processSpy, after: afterSpy }, cookieStorage) + + const lockMaxTries = isLockEnabled() ? LOCK_MAX_TRIES : 0 + const lockRetryDelay = isLockEnabled() ? LOCK_RETRY_DELAY : 0 + + clock.tick(lockMaxTries * lockRetryDelay) + expect(processSpy).not.toHaveBeenCalled() + expect(afterSpy).not.toHaveBeenCalled() + expect(cookie.setSpy).not.toHaveBeenCalled() + + clock.cleanup() + }) + + it('should execute cookie accesses in order', (done) => { + lockScenario({ + onInitialLockCheck: () => ({ + currentState: { ...initialSession, lock: 'locked' }, // force to retry the first access later + retryState: initialSession, + }), + }) + cookieStorage.persistSession(initialSession) + + processSessionStoreOperations( + { + process: (session) => ({ ...session, value: 'foo' }), + after: afterSpy, + }, + cookieStorage + ) + processSessionStoreOperations( + { + process: (session) => ({ ...session, value: `${session.value || ''}bar` }), + after: (session) => { + expect(session.value).toBe('foobar') + expect(afterSpy).toHaveBeenCalled() + done() + }, + }, + cookieStorage + ) + }) + }) +}) diff --git a/packages/core/src/domain/session/sessionStoreOperations.ts b/packages/core/src/domain/session/sessionStoreOperations.ts new file mode 100644 index 0000000000..cfdd3852d0 --- /dev/null +++ b/packages/core/src/domain/session/sessionStoreOperations.ts @@ -0,0 +1,107 @@ +import { setTimeout } from '../../tools/timer' +import { dateNow } from '../../tools/utils/timeUtils' +import { generateUUID } from '../../tools/utils/stringUtils' +import { isChromium } from '../../tools/utils/browserDetection' +import { SESSION_EXPIRATION_DELAY } from './sessionConstants' +import type { SessionState, SessionStore } from './sessionStore' +import { isSessionInExpiredState } from './sessionStore' + +type Operations = { + process: (sessionState: SessionState) => SessionState | undefined + after?: (sessionState: SessionState) => void +} + +export const LOCK_RETRY_DELAY = 10 +export const LOCK_MAX_TRIES = 100 +const bufferedOperations: Operations[] = [] +let ongoingOperations: Operations | undefined + +export function processSessionStoreOperations(operations: Operations, sessionStore: SessionStore, numberOfRetries = 0) { + const { retrieveSession, persistSession, clearSession } = sessionStore + const lockEnabled = isLockEnabled() + + if (!ongoingOperations) { + ongoingOperations = operations + } + if (operations !== ongoingOperations) { + bufferedOperations.push(operations) + return + } + if (lockEnabled && numberOfRetries >= LOCK_MAX_TRIES) { + next(sessionStore) + return + } + let currentLock: string + let currentSession = retrieveSession() + if (lockEnabled) { + // if someone has lock, retry later + if (currentSession.lock) { + retryLater(operations, sessionStore, numberOfRetries) + return + } + // acquire lock + currentLock = generateUUID() + currentSession.lock = currentLock + persistSession(currentSession) + // if lock is not acquired, retry later + currentSession = retrieveSession() + if (currentSession.lock !== currentLock) { + retryLater(operations, sessionStore, numberOfRetries) + return + } + } + let processedSession = operations.process(currentSession) + if (lockEnabled) { + // if lock corrupted after process, retry later + currentSession = retrieveSession() + if (currentSession.lock !== currentLock!) { + retryLater(operations, sessionStore, numberOfRetries) + return + } + } + if (processedSession) { + if (isSessionInExpiredState(processedSession)) { + clearSession() + } else { + processedSession.expire = String(dateNow() + SESSION_EXPIRATION_DELAY) + persistSession(processedSession) + } + } + if (lockEnabled) { + // correctly handle lock around expiration would require to handle this case properly at several levels + // since we don't have evidence of lock issues around expiration, let's just not do the corruption check for it + if (!(processedSession && isSessionInExpiredState(processedSession))) { + // if lock corrupted after persist, retry later + currentSession = retrieveSession() + if (currentSession.lock !== currentLock!) { + retryLater(operations, sessionStore, numberOfRetries) + return + } + delete currentSession.lock + persistSession(currentSession) + processedSession = currentSession + } + } + // call after even if session is not persisted in order to perform operations on + // up-to-date session state value => the value could have been modified by another tab + operations.after?.(processedSession || currentSession) + next(sessionStore) +} +/** + * Lock strategy allows mitigating issues due to concurrent access to cookie. + * This issue concerns only chromium browsers and enabling this on firefox increases cookie write failures. + */ + +export const isLockEnabled = () => isChromium() +function retryLater(operations: Operations, sessionStore: SessionStore, currentNumberOfRetries: number) { + setTimeout(() => { + processSessionStoreOperations(operations, sessionStore, currentNumberOfRetries + 1) + }, LOCK_RETRY_DELAY) +} +function next(sessionStore: SessionStore) { + ongoingOperations = undefined + const nextOperations = bufferedOperations.shift() + if (nextOperations) { + processSessionStoreOperations(nextOperations, sessionStore) + } +} From 97f9e7230796824c0b14c8756a5c6fad94886d79 Mon Sep 17 00:00:00 2001 From: Yannick Adam Date: Thu, 1 Jun 2023 09:27:23 +0200 Subject: [PATCH 10/13] =?UTF-8?q?=F0=9F=91=8C=20Renamed=20expandOrRenewCoo?= =?UTF-8?q?kie?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/src/domain/session/sessionStoreManager.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/src/domain/session/sessionStoreManager.ts b/packages/core/src/domain/session/sessionStoreManager.ts index 4cfe639431..5e70f68476 100644 --- a/packages/core/src/domain/session/sessionStoreManager.ts +++ b/packages/core/src/domain/session/sessionStoreManager.ts @@ -46,7 +46,7 @@ export function startSessionStoreManager( { process: (sessionState) => { const synchronizedSession = synchronizeSession(sessionState) - isTracked = expandOrRenewCookie(synchronizedSession) + isTracked = expandOrRenewSessionState(synchronizedSession) return synchronizedSession }, after: (sessionState) => { @@ -98,7 +98,7 @@ export function startSessionStoreManager( return sessionState } - function expandOrRenewCookie(sessionState: SessionState) { + function expandOrRenewSessionState(sessionState: SessionState) { const { trackingType, isTracked } = computeSessionState(sessionState[productKey]) sessionState[productKey] = trackingType if (isTracked && !sessionState.id) { From 72f4303e8fd667d417cbb616dd9707f0cb70bbd2 Mon Sep 17 00:00:00 2001 From: Yannick Adam Date: Thu, 1 Jun 2023 16:22:44 +0200 Subject: [PATCH 11/13] =?UTF-8?q?=F0=9F=91=8C=20Added=20tests=20to=20oldCo?= =?UTF-8?q?okiesMigration?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/domain/session/oldCookiesMigration.spec.ts | 11 +++++++++-- .../core/src/domain/session/oldCookiesMigration.ts | 4 +--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/core/src/domain/session/oldCookiesMigration.spec.ts b/packages/core/src/domain/session/oldCookiesMigration.spec.ts index faf61babf2..732df81c6c 100644 --- a/packages/core/src/domain/session/oldCookiesMigration.spec.ts +++ b/packages/core/src/domain/session/oldCookiesMigration.spec.ts @@ -13,11 +13,11 @@ describe('old cookies migration', () => { const options: CookieOptions = {} it('should not touch current cookie', () => { - setCookie(SESSION_COOKIE_NAME, 'id=abcde&rum=0&logs=1', SESSION_EXPIRATION_DELAY) + setCookie(SESSION_COOKIE_NAME, 'id=abcde&rum=0&logs=1&expire=1234567890', SESSION_EXPIRATION_DELAY) tryOldCookiesMigration(options) - expect(getCookie(SESSION_COOKIE_NAME)).toBe('id=abcde&rum=0&logs=1') + expect(getCookie(SESSION_COOKIE_NAME)).toBe('id=abcde&rum=0&logs=1&expire=1234567890') }) it('should create new cookie from old cookie values', () => { @@ -30,6 +30,7 @@ describe('old cookies migration', () => { expect(getCookie(SESSION_COOKIE_NAME)).toContain('id=abcde') expect(getCookie(SESSION_COOKIE_NAME)).toContain('rum=0') expect(getCookie(SESSION_COOKIE_NAME)).toContain('logs=1') + expect(getCookie(SESSION_COOKIE_NAME)).toMatch(/expire=\d+/) }) it('should create new cookie from a single old cookie', () => { @@ -39,5 +40,11 @@ describe('old cookies migration', () => { expect(getCookie(SESSION_COOKIE_NAME)).not.toContain('id=') expect(getCookie(SESSION_COOKIE_NAME)).toContain('rum=0') + expect(getCookie(SESSION_COOKIE_NAME)).toMatch(/expire=\d+/) + }) + + it('should not create a new cookie if no old cookie is present', () => { + tryOldCookiesMigration(options) + expect(getCookie(SESSION_COOKIE_NAME)).toBeUndefined() }) }) diff --git a/packages/core/src/domain/session/oldCookiesMigration.ts b/packages/core/src/domain/session/oldCookiesMigration.ts index fd1e0a256f..6b9a547a18 100644 --- a/packages/core/src/domain/session/oldCookiesMigration.ts +++ b/packages/core/src/domain/session/oldCookiesMigration.ts @@ -3,7 +3,7 @@ import { getCookie } from '../../browser/cookie' import { dateNow } from '../../tools/utils/timeUtils' import type { SessionState } from './sessionStore' import { isSessionInExpiredState } from './sessionStore' -import { SESSION_COOKIE_NAME, deleteSessionCookie, persistSessionCookie } from './sessionCookieStore' +import { SESSION_COOKIE_NAME, persistSessionCookie } from './sessionCookieStore' import { SESSION_EXPIRATION_DELAY } from './sessionConstants' export const OLD_SESSION_COOKIE_NAME = '_dd' @@ -38,8 +38,6 @@ export function tryOldCookiesMigration(options: CookieOptions) { if (!isSessionInExpiredState(session)) { session.expire = String(dateNow() + SESSION_EXPIRATION_DELAY) persistSessionCookie(options)(session) - } else { - deleteSessionCookie(options)() } } } From fdc1996591fb4fbebecb4009e48478f1247a0e0b Mon Sep 17 00:00:00 2001 From: Yannick Adam Date: Mon, 5 Jun 2023 16:10:38 +0200 Subject: [PATCH 12/13] =?UTF-8?q?=F0=9F=91=8C=20Updated=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/src/domain/session/sessionCookieStore.spec.ts | 2 ++ packages/core/src/domain/session/sessionManager.spec.ts | 4 ++-- packages/core/src/domain/session/sessionStoreOperations.ts | 2 ++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/core/src/domain/session/sessionCookieStore.spec.ts b/packages/core/src/domain/session/sessionCookieStore.spec.ts index 214c46693b..b80526482a 100644 --- a/packages/core/src/domain/session/sessionCookieStore.spec.ts +++ b/packages/core/src/domain/session/sessionCookieStore.spec.ts @@ -21,6 +21,7 @@ describe('session cookie store', () => { cookieStorage.persistSession(sessionState) const session = cookieStorage.retrieveSession() expect(session).toEqual({ ...sessionState }) + expect(document.cookie).toMatch(/_dd_s=.*id=.*created/) }) it('should delete the cookie holding the session', () => { @@ -28,6 +29,7 @@ describe('session cookie store', () => { cookieStorage.clearSession() const session = cookieStorage.retrieveSession() expect(session).toEqual({}) + expect(document.cookie).not.toMatch(/_dd_s=/) }) it('should return an empty object if session string is invalid', () => { diff --git a/packages/core/src/domain/session/sessionManager.spec.ts b/packages/core/src/domain/session/sessionManager.spec.ts index 90c27f9f79..a2cbc72d52 100644 --- a/packages/core/src/domain/session/sessionManager.spec.ts +++ b/packages/core/src/domain/session/sessionManager.spec.ts @@ -19,12 +19,12 @@ const enum FakeTrackingType { const TRACKED_SESSION_STATE = { isTracked: true, trackingType: FakeTrackingType.TRACKED, -} as const +} const NOT_TRACKED_SESSION_STATE = { isTracked: false, trackingType: FakeTrackingType.NOT_TRACKED, -} as const +} describe('startSessionManager', () => { const DURATION = 123456 diff --git a/packages/core/src/domain/session/sessionStoreOperations.ts b/packages/core/src/domain/session/sessionStoreOperations.ts index cfdd3852d0..a430c945ae 100644 --- a/packages/core/src/domain/session/sessionStoreOperations.ts +++ b/packages/core/src/domain/session/sessionStoreOperations.ts @@ -93,11 +93,13 @@ export function processSessionStoreOperations(operations: Operations, sessionSto */ export const isLockEnabled = () => isChromium() + function retryLater(operations: Operations, sessionStore: SessionStore, currentNumberOfRetries: number) { setTimeout(() => { processSessionStoreOperations(operations, sessionStore, currentNumberOfRetries + 1) }, LOCK_RETRY_DELAY) } + function next(sessionStore: SessionStore) { ongoingOperations = undefined const nextOperations = bufferedOperations.shift() From d176b798c0b12a96a016990eee139f60435eb766 Mon Sep 17 00:00:00 2001 From: Yannick Adam Date: Tue, 6 Jun 2023 11:55:04 +0200 Subject: [PATCH 13/13] =?UTF-8?q?=F0=9F=91=8C=20Added=20test=20for=20MAX?= =?UTF-8?q?=5FLOCK=5FTRIES=20+=20used=20getCookie=20to=20check=20session?= =?UTF-8?q?=20persistence?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/domain/session/sessionCookieStore.spec.ts | 6 +++--- .../domain/session/sessionStoreOperations.spec.ts | 12 ++++++++++++ .../src/domain/session/sessionStoreOperations.ts | 2 +- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/packages/core/src/domain/session/sessionCookieStore.spec.ts b/packages/core/src/domain/session/sessionCookieStore.spec.ts index b80526482a..d5d653e439 100644 --- a/packages/core/src/domain/session/sessionCookieStore.spec.ts +++ b/packages/core/src/domain/session/sessionCookieStore.spec.ts @@ -1,5 +1,5 @@ import type { CookieOptions } from '../../browser/cookie' -import { setCookie, deleteCookie } from '../../browser/cookie' +import { getCookie, setCookie, deleteCookie } from '../../browser/cookie' import { SESSION_COOKIE_NAME, initCookieStore } from './sessionCookieStore' import type { SessionState, SessionStore } from './sessionStore' @@ -21,7 +21,7 @@ describe('session cookie store', () => { cookieStorage.persistSession(sessionState) const session = cookieStorage.retrieveSession() expect(session).toEqual({ ...sessionState }) - expect(document.cookie).toMatch(/_dd_s=.*id=.*created/) + expect(getCookie(SESSION_COOKIE_NAME)).toBe('id=123&created=0') }) it('should delete the cookie holding the session', () => { @@ -29,7 +29,7 @@ describe('session cookie store', () => { cookieStorage.clearSession() const session = cookieStorage.retrieveSession() expect(session).toEqual({}) - expect(document.cookie).not.toMatch(/_dd_s=/) + expect(getCookie(SESSION_COOKIE_NAME)).toBeUndefined() }) it('should return an empty object if session string is invalid', () => { diff --git a/packages/core/src/domain/session/sessionStoreOperations.spec.ts b/packages/core/src/domain/session/sessionStoreOperations.spec.ts index 5fe587fbb2..464e135a96 100644 --- a/packages/core/src/domain/session/sessionStoreOperations.spec.ts +++ b/packages/core/src/domain/session/sessionStoreOperations.spec.ts @@ -67,6 +67,18 @@ describe('process operations mechanism', () => { expect(cookieStorage.retrieveSession()).toEqual(initialSession) expect(afterSpy).toHaveBeenCalledWith(initialSession) }) + + it('LOCK_MAX_TRIES value should not influence the behavior when lock mechanism is not enabled', () => { + cookieStorage.persistSession(initialSession) + processSpy.and.returnValue({ ...otherSession }) + + processSessionStoreOperations({ process: processSpy, after: afterSpy }, cookieStorage, LOCK_MAX_TRIES) + + expect(processSpy).toHaveBeenCalledWith(initialSession) + const expectedSession = { ...otherSession, expire: jasmine.any(String) } + expect(cookieStorage.retrieveSession()).toEqual(expectedSession) + expect(afterSpy).toHaveBeenCalledWith(expectedSession) + }) }) describe('with cookie-lock enabled', () => { diff --git a/packages/core/src/domain/session/sessionStoreOperations.ts b/packages/core/src/domain/session/sessionStoreOperations.ts index a430c945ae..a47558c0e0 100644 --- a/packages/core/src/domain/session/sessionStoreOperations.ts +++ b/packages/core/src/domain/session/sessionStoreOperations.ts @@ -87,11 +87,11 @@ export function processSessionStoreOperations(operations: Operations, sessionSto operations.after?.(processedSession || currentSession) next(sessionStore) } + /** * Lock strategy allows mitigating issues due to concurrent access to cookie. * This issue concerns only chromium browsers and enabling this on firefox increases cookie write failures. */ - export const isLockEnabled = () => isChromium() function retryLater(operations: Operations, sessionStore: SessionStore, currentNumberOfRetries: number) {