👷 remove watch_cookie_without_lock experimental feature flag (#3773)

Author: thomas-lebeau

packages/core/src/domain/session/sessionStore.spec.ts

@@ -1,8 +1,7 @@
 import type { Clock } from '../../../test'
-import { mockClock, createFakeSessionStoreStrategy, mockExperimentalFeatures } from '../../../test'
+import { mockClock, createFakeSessionStoreStrategy } from '../../../test'
 import type { InitConfiguration, Configuration } from '../configuration'
 import { display } from '../../tools/display'
-import { ExperimentalFeature } from '../../tools/experimentalFeatures'
 import type { SessionStore } from './sessionStore'
 import { STORAGE_POLL_DELAY, startSessionStore, selectSessionStoreStrategyType } from './sessionStore'
 import {
@@ -466,115 +465,94 @@ describe('session store', () => {
     })
 
     describe('regular watch', () => {
-      ;[true, false].forEach((useExperimentalFeature) => {
-        describe(`when 'watch_cookie_without_lock' experimental feature is ${useExperimentalFeature}`, () => {
-          beforeEach(() => {
-            if (useExperimentalFeature) {
-              mockExperimentalFeatures([ExperimentalFeature.WATCH_COOKIE_WITHOUT_LOCK])
-            }
-          })
-
-          it('when session not in cache and session not in store, should store the expired session', () => {
-            setupSessionStore()
-
-            clock.tick(STORAGE_POLL_DELAY)
-
-            expectSessionToBeExpiredInStore()
-            expect(sessionStoreManager.getSession().id).toBeUndefined()
-            expect(expireSpy).not.toHaveBeenCalled()
-            expect(sessionStoreStrategy.persistSession).toHaveBeenCalled()
-          })
-
-          it('when session in cache and session not in store, should expire session', () => {
-            setupSessionStore(createSessionState(FakeTrackingType.TRACKED, FIRST_ID))
-            resetSessionInStore()
-
-            clock.tick(STORAGE_POLL_DELAY)
-
-            expect(sessionStoreManager.getSession().id).toBeUndefined()
-            expectSessionToBeExpiredInStore()
-            expect(expireSpy).toHaveBeenCalled()
-            expect(sessionStoreStrategy.persistSession).toHaveBeenCalled()
-          })
-
-          it('when session not in cache and session in store, should do nothing', () => {
-            setupSessionStore()
-            setSessionInStore(createSessionState(FakeTrackingType.TRACKED, FIRST_ID))
-
-            clock.tick(STORAGE_POLL_DELAY)
-
-            expect(sessionStoreManager.getSession().id).toBeUndefined()
-            expect(expireSpy).not.toHaveBeenCalled()
-            if (useExperimentalFeature) {
-              expect(sessionStoreStrategy.persistSession).not.toHaveBeenCalled()
-            }
-          })
-
-          it('when session in cache is same session than in store, should synchronize session', () => {
-            setupSessionStore(createSessionState(FakeTrackingType.TRACKED, FIRST_ID))
-            setSessionInStore(
-              createSessionState(FakeTrackingType.TRACKED, FIRST_ID, Date.now() + SESSION_TIME_OUT_DELAY + 10)
-            )
-
-            clock.tick(STORAGE_POLL_DELAY)
-
-            expect(sessionStoreManager.getSession().id).toBe(FIRST_ID)
-            expect(sessionStoreManager.getSession().expire).toBe(getStoreExpiration())
-            expect(expireSpy).not.toHaveBeenCalled()
-            if (useExperimentalFeature) {
-              expect(sessionStoreStrategy.persistSession).not.toHaveBeenCalled()
-            }
-          })
-
-          it('when session id in cache is different than session id in store, should expire session and not touch the store', () => {
-            setupSessionStore(createSessionState(FakeTrackingType.TRACKED, FIRST_ID))
-            setSessionInStore(createSessionState(FakeTrackingType.TRACKED, SECOND_ID))
-
-            clock.tick(STORAGE_POLL_DELAY)
-
-            expect(sessionStoreManager.getSession().id).toBeUndefined()
-            expect(expireSpy).toHaveBeenCalled()
-            if (useExperimentalFeature) {
-              expect(sessionStoreStrategy.persistSession).not.toHaveBeenCalled()
-            }
-          })
-
-          it('when session in store is expired first and then get updated by another tab, should expire session in cache and not touch the store', () => {
-            setupSessionStore(createSessionState(FakeTrackingType.TRACKED, FIRST_ID))
-            resetSessionInStore()
-
-            // Simulate a new session being written to the store by another tab during the watch
-            // when the experimental feature is enabled, watch is reading the cookie twice so we need to plan the write of the cookie at the right index
-            sessionStoreStrategy.planRetrieveSession(
-              useExperimentalFeature ? 1 : 0,
-              createSessionState(FakeTrackingType.TRACKED, SECOND_ID)
-            )
-
-            clock.tick(STORAGE_POLL_DELAY)
-
-            // expires session in cache
-            expect(sessionStoreManager.getSession().id).toBeUndefined()
-            expect(expireSpy).toHaveBeenCalled()
-
-            // Does not touch the store
-            // The two calls to persist session are for the lock management, these can be ignored
-            expect(sessionStoreStrategy.persistSession).toHaveBeenCalledTimes(2)
-            expect(sessionStoreStrategy.expireSession).not.toHaveBeenCalled()
-          })
-
-          it('when session type in cache is different than session type in store, should expire session and not touch the store', () => {
-            setupSessionStore(createSessionState(FakeTrackingType.NOT_TRACKED, FIRST_ID))
-            setSessionInStore(createSessionState(FakeTrackingType.TRACKED, FIRST_ID))
-
-            clock.tick(STORAGE_POLL_DELAY)
-
-            expect(sessionStoreManager.getSession().id).toBeUndefined()
-            expect(expireSpy).toHaveBeenCalled()
-            if (useExperimentalFeature) {
-              expect(sessionStoreStrategy.persistSession).not.toHaveBeenCalled()
-            }
-          })
-        })
+      it('when session not in cache and session not in store, should store the expired session', () => {
+        setupSessionStore()
+
+        clock.tick(STORAGE_POLL_DELAY)
+
+        expectSessionToBeExpiredInStore()
+        expect(sessionStoreManager.getSession().id).toBeUndefined()
+        expect(expireSpy).not.toHaveBeenCalled()
+        expect(sessionStoreStrategy.persistSession).toHaveBeenCalled()
+      })
+
+      it('when session in cache and session not in store, should expire session', () => {
+        setupSessionStore(createSessionState(FakeTrackingType.TRACKED, FIRST_ID))
+        resetSessionInStore()
+
+        clock.tick(STORAGE_POLL_DELAY)
+
+        expect(sessionStoreManager.getSession().id).toBeUndefined()
+        expectSessionToBeExpiredInStore()
+        expect(expireSpy).toHaveBeenCalled()
+        expect(sessionStoreStrategy.persistSession).toHaveBeenCalled()
+      })
+
+      it('when session not in cache and session in store, should do nothing', () => {
+        setupSessionStore()
+        setSessionInStore(createSessionState(FakeTrackingType.TRACKED, FIRST_ID))
+
+        clock.tick(STORAGE_POLL_DELAY)
+
+        expect(sessionStoreManager.getSession().id).toBeUndefined()
+        expect(expireSpy).not.toHaveBeenCalled()
+        expect(sessionStoreStrategy.persistSession).not.toHaveBeenCalled()
+      })
+
+      it('when session in cache is same session than in store, should synchronize session', () => {
+        setupSessionStore(createSessionState(FakeTrackingType.TRACKED, FIRST_ID))
+        setSessionInStore(
+          createSessionState(FakeTrackingType.TRACKED, FIRST_ID, Date.now() + SESSION_TIME_OUT_DELAY + 10)
+        )
+
+        clock.tick(STORAGE_POLL_DELAY)
+
+        expect(sessionStoreManager.getSession().id).toBe(FIRST_ID)
+        expect(sessionStoreManager.getSession().expire).toBe(getStoreExpiration())
+        expect(expireSpy).not.toHaveBeenCalled()
+        expect(sessionStoreStrategy.persistSession).not.toHaveBeenCalled()
+      })
+
+      it('when session id in cache is different than session id in store, should expire session and not touch the store', () => {
+        setupSessionStore(createSessionState(FakeTrackingType.TRACKED, FIRST_ID))
+        setSessionInStore(createSessionState(FakeTrackingType.TRACKED, SECOND_ID))
+
+        clock.tick(STORAGE_POLL_DELAY)
+
+        expect(sessionStoreManager.getSession().id).toBeUndefined()
+        expect(expireSpy).toHaveBeenCalled()
+        expect(sessionStoreStrategy.persistSession).not.toHaveBeenCalled()
+      })
+
+      it('when session in store is expired first and then get updated by another tab, should expire session in cache and not touch the store', () => {
+        setupSessionStore(createSessionState(FakeTrackingType.TRACKED, FIRST_ID))
+        resetSessionInStore()
+
+        // Simulate a new session being written to the store by another tab during the watch.
+        // Watch is reading the cookie twice so we need to plan the write of the cookie at the right index
+        sessionStoreStrategy.planRetrieveSession(1, createSessionState(FakeTrackingType.TRACKED, SECOND_ID))
+
+        clock.tick(STORAGE_POLL_DELAY)
+
+        // expires session in cache
+        expect(sessionStoreManager.getSession().id).toBeUndefined()
+        expect(expireSpy).toHaveBeenCalled()
+
+        // Does not touch the store
+        // The two calls to persist session are for the lock management, these can be ignored
+        expect(sessionStoreStrategy.persistSession).toHaveBeenCalledTimes(2)
+        expect(sessionStoreStrategy.expireSession).not.toHaveBeenCalled()
+      })
+
+      it('when session type in cache is different than session type in store, should expire session and not touch the store', () => {
+        setupSessionStore(createSessionState(FakeTrackingType.NOT_TRACKED, FIRST_ID))
+        setSessionInStore(createSessionState(FakeTrackingType.TRACKED, FIRST_ID))
+
+        clock.tick(STORAGE_POLL_DELAY)
+
+        expect(sessionStoreManager.getSession().id).toBeUndefined()
+        expect(expireSpy).toHaveBeenCalled()
+        expect(sessionStoreStrategy.persistSession).not.toHaveBeenCalled()
       })
     })
 

packages/core/src/domain/session/sessionStore.ts

@@ -5,7 +5,6 @@ import { throttle } from '../../tools/utils/functionUtils'
 import { generateUUID } from '../../tools/utils/stringUtils'
 import type { InitConfiguration, Configuration } from '../configuration'
 import { display } from '../../tools/display'
-import { ExperimentalFeature, isExperimentalFeatureEnabled } from '../../tools/experimentalFeatures'
 import { selectCookieStrategy, initCookieStrategy } from './storeStrategies/sessionInCookie'
 import type { SessionStoreStrategy, SessionStoreStrategyType } from './storeStrategies/sessionStoreStrategy'
 import type { SessionState } from './sessionState'
@@ -135,21 +134,19 @@ export function startSessionStore<TrackingType extends string>(
    * - if the session is not active, clear the session store and expire the session cache
    */
   function watchSession() {
-    const sessionStoreOperation = {
-      process: (sessionState: SessionState) =>
-        isSessionInExpiredState(sessionState) ? getExpiredSessionState(sessionState, configuration) : undefined,
-      after: synchronizeSession,
-    } as const
-
-    if (isExperimentalFeatureEnabled(ExperimentalFeature.WATCH_COOKIE_WITHOUT_LOCK)) {
-      const sessionState = sessionStoreStrategy.retrieveSession()
-      if (isSessionInExpiredState(sessionState)) {
-        processSessionStoreOperations(sessionStoreOperation, sessionStoreStrategy)
-      } else {
-        synchronizeSession(sessionState)
-      }
+    const sessionState = sessionStoreStrategy.retrieveSession()
+
+    if (isSessionInExpiredState(sessionState)) {
+      processSessionStoreOperations(
+        {
+          process: (sessionState: SessionState) =>
+            isSessionInExpiredState(sessionState) ? getExpiredSessionState(sessionState, configuration) : undefined,
+          after: synchronizeSession,
+        },
+        sessionStoreStrategy
+      )
     } else {
-      processSessionStoreOperations(sessionStoreOperation, sessionStoreStrategy)
+      synchronizeSession(sessionState)
     }
   }
 

packages/core/src/tools/experimentalFeatures.ts

@@ -16,7 +16,6 @@ import { objectHasValue } from './utils/objectUtils'
 export enum ExperimentalFeature {
   TRACK_INTAKE_REQUESTS = 'track_intake_requests',
   WRITABLE_RESOURCE_GRAPHQL = 'writable_resource_graphql',
-  WATCH_COOKIE_WITHOUT_LOCK = 'watch_cookie_without_lock',
 }
 
 const enabledExperimentalFeatures: Set<ExperimentalFeature> = new Set()