diff --git a/packages/browser/README.md b/packages/browser/README.md index 44d1e9d0..287dace8 100644 --- a/packages/browser/README.md +++ b/packages/browser/README.md @@ -311,8 +311,10 @@ Reapplying the same configuration preserves deduplication. Neither provider emit `createdAt` is a configuration timestamp, not an experiment revision. This behavior does not depend on it changing on every request. Exposure caches retain up to 50,000 entries per scope, matching the Node provider's limit. -Exposure checks use a bounded in-memory cache. IndexedDB stores snapshots of that cache in the background. -The least recently used entries are removed when the cache reaches its limit. Evicted entries can produce another exposure. +Exposure checks use a bounded in-memory cache. Background IndexedDB transactions merge newly recorded exposures +with the stored entries, so instances sharing a scope preserve each other's writes without replaying their loaded snapshots. +Memory removes the least recently used entries; persistence removes the oldest written entries when it reaches its limit. +Evicted entries can produce another exposure. If IndexedDB is unavailable or a storage operation fails, evaluation and in-memory deduplication continue. An abrupt page exit can lose an unfinished write and permit a repeat exposure after reload. diff --git a/packages/browser/src/cache/indexeddb-assignment-cache.ts b/packages/browser/src/cache/indexeddb-assignment-cache.ts index b1510fdf..1b3edff2 100644 --- a/packages/browser/src/cache/indexeddb-assignment-cache.ts +++ b/packages/browser/src/cache/indexeddb-assignment-cache.ts @@ -1,15 +1,16 @@ -import type { AssignmentCacheEntry } from '@datadog/flagging-core' +import { type AssignmentCacheEntry, assignmentCacheKeyToString } from '@datadog/flagging-core' import { MAX_EXPOSURE_CACHE_ENTRIES } from './constants' import { withStore } from './indexeddb-store' import SimpleAssignmentCache from './simple-assignment-cache' -/** Keep exposure checks synchronous. Persist bounded snapshots without blocking evaluation. */ +/** Keep exposure checks synchronous. Merge pending entries into storage without blocking evaluation. */ export class IndexedDBAssignmentCache extends SimpleAssignmentCache { private readonly storageKey: string private operations: Promise = Promise.resolve() private initialization?: Promise private persistScheduled = false private generation = 0 + private pendingKeys = new Set() constructor(storageKeySuffix: string) { super() @@ -37,12 +38,20 @@ export class IndexedDBAssignmentCache extends SimpleAssignmentCache { set(entry: AssignmentCacheEntry): void { void this.init() super.set(entry) + const key = assignmentCacheKeyToString(entry) + this.pendingKeys.delete(key) + this.pendingKeys.add(key) + // Keep the pending key index bounded even if storage is unavailable. + if (this.pendingKeys.size > MAX_EXPOSURE_CACHE_ENTRIES) { + this.pendingKeys.delete(this.pendingKeys.values().next().value!) + } this.persist() } clear(): Promise { super.clear() this.generation++ + this.pendingKeys.clear() // New entries need a write after the delete, even if an older write is queued. this.persistScheduled = false return this.enqueue(async () => { @@ -60,10 +69,52 @@ export class IndexedDBAssignmentCache extends SimpleAssignmentCache { private persist(): void { if (this.persistScheduled) return this.persistScheduled = true + const generation = this.generation void Promise.resolve().then(() => { void this.enqueue(async () => { + // A queued write from before clear must not consume post-clear entries. + if (generation !== this.generation) return this.persistScheduled = false - await withStore('readwrite', (store) => store.put(Array.from(this.entries()), this.storageKey)) + const pendingKeys = this.pendingKeys + this.pendingKeys = new Set() + const entries = Array.from(this.entries()).filter(([key]) => pendingKeys.has(key)) + try { + await withStore('readwrite', (store) => { + // IndexedDB serializes read/write transactions across connections and tabs. + // Merge only this batch's changes, never the instance's stale loaded snapshot. + const request = store.get(this.storageKey) + request.onsuccess = () => { + try { + const stored = new Map( + Array.isArray(request.result) ? request.result.filter(isEntry) : [] + ) + for (const [key, value] of entries) { + stored.delete(key) + stored.set(key, value) + } + for (const key of stored.keys()) { + if (stored.size <= MAX_EXPOSURE_CACHE_ENTRIES) break + stored.delete(key) + } + store.put(Array.from(stored), this.storageKey) + } catch { + // Exceptions in a request callback must abort the whole merge. + store.transaction.abort() + } + } + return request + }) + } catch { + if (generation === this.generation) { + // Retry failed entries on a later write, retaining only keys still in memory. + // Read their current values then, so retries cannot overwrite newer assignments. + const newerKeys = this.pendingKeys + this.pendingKeys = new Set() + for (const [key] of this.entries()) { + if (pendingKeys.has(key) || newerKeys.has(key)) this.pendingKeys.add(key) + } + } + } }) }) } diff --git a/packages/browser/test/cache/exposure-cache-capacity.spec.ts b/packages/browser/test/cache/exposure-cache-capacity.spec.ts index 3183bc0d..22901962 100644 --- a/packages/browser/test/cache/exposure-cache-capacity.spec.ts +++ b/packages/browser/test/cache/exposure-cache-capacity.spec.ts @@ -56,4 +56,26 @@ describe('exposure cache capacity', () => { expect(stored.has('key-1')).toBe(false) expect(cache.has(exposure('new'))).toBe(true) }) + + it("bounds the merged store without discarding another instance's recent writes", async () => { + await withStore('readwrite', (store) => store.put(entries(), 'assignments-scope')) + const first = new IndexedDBAssignmentCache('scope') + const second = new IndexedDBAssignmentCache('scope') + await Promise.all([first.init(), second.init()]) + const written = nextWrite(2) + first.set(exposure('first')) + second.set(exposure('second')) + await written + + const stored = new Map(await withStore('readonly', (store) => store.get('assignments-scope'))) + expect(stored.size).toBe(MAX_EXPOSURE_CACHE_ENTRIES) + expect(stored.has('key-0')).toBe(false) + expect(stored.has('key-1')).toBe(false) + expect(stored.get(assignmentCacheKeyToString(exposure('first')))).toBe( + assignmentCacheValueToString(exposure('first')) + ) + expect(stored.get(assignmentCacheKeyToString(exposure('second')))).toBe( + assignmentCacheValueToString(exposure('second')) + ) + }) }) diff --git a/packages/browser/test/cache/indexeddb-assignment-cache.spec.ts b/packages/browser/test/cache/indexeddb-assignment-cache.spec.ts index 0b99ca3e..68a197a6 100644 --- a/packages/browser/test/cache/indexeddb-assignment-cache.spec.ts +++ b/packages/browser/test/cache/indexeddb-assignment-cache.spec.ts @@ -42,6 +42,61 @@ describe('IndexedDBAssignmentCache', () => { expect(reloaded.has(b)).toBe(true) }) + it('preserves committed entries from another instance of the same scope', async () => { + const other = new IndexedDBAssignmentCache('scope') + await Promise.all([cache.init(), other.init()]) + let written = nextWrite() + cache.set(a) + await written + written = nextWrite() + other.set(b) + await written + + const reloaded = new IndexedDBAssignmentCache('scope') + await reloaded.init() + expect(reloaded.has(a)).toBe(true) + expect(reloaded.has(b)).toBe(true) + }) + + it('merges concurrent writes from separate instances atomically', async () => { + const other = new IndexedDBAssignmentCache('scope') + await Promise.all([cache.init(), other.init()]) + const written = nextWrite(2) + cache.set(a) + other.set(b) + await written + expect(new Map(await read())).toEqual(new Map([serialized(a), serialized(b)])) + }) + + it("does not restore cleared entries from another instance's loaded snapshot", async () => { + await seed([serialized(a)]) + const other = new IndexedDBAssignmentCache('scope') + await Promise.all([cache.init(), other.init()]) + await cache.clear() + const written = nextWrite() + other.set(b) + await written + + const reloaded = new IndexedDBAssignmentCache('scope') + await reloaded.init() + expect(reloaded.has(a)).toBe(false) + expect(reloaded.has(b)).toBe(true) + }) + + it("does not overwrite another instance's updated assignment with a loaded value", async () => { + await seed([serialized(a)]) + const other = new IndexedDBAssignmentCache('scope') + await Promise.all([cache.init(), other.init()]) + const updated = { ...a, variant: { key: 'treatment' } } + let written = nextWrite() + cache.set(updated) + await written + written = nextWrite() + other.set(b) + await written + expect(await read()).toEqual([serialized(updated), serialized(b)]) + }) + it('initializes once without replacing an updated assignment with persisted data', async () => { const loading = deferred() const started = deferred() @@ -176,6 +231,42 @@ describe('IndexedDBAssignmentCache', () => { expect(await read()).toEqual([serialized(a), serialized(b)]) }) + it('retries the latest assignment after a failed write without replaying loaded entries', async () => { + await seed([serialized(b)]) + await cache.init() + const attempted = deferred() + jest.spyOn(storage, 'withStore').mockImplementationOnce(async () => { + attempted.resolve() + throw new Error('write failed') + }) + cache.set(a) + await attempted.promise + const other = new IndexedDBAssignmentCache('scope') + await other.clear() + const updated = { ...a, variant: { key: 'treatment' } } + const written = nextWrite() + cache.set(updated) + await written + expect(await read()).toEqual([serialized(updated)]) + }) + + it('aborts and retries when put throws inside the merge callback', async () => { + await cache.init() + const aborted = deferred() + const put = jest.spyOn(IDBObjectStore.prototype, 'put').mockImplementationOnce(function (this: IDBObjectStore) { + this.transaction.addEventListener('abort', () => aborted.resolve()) + throw new Error('quota') + }) + cache.set(a) + await aborted.promise + put.mockRestore() + expect(cache.has(a)).toBe(true) + const written = nextWrite() + cache.set(b) + await written + expect(await read()).toEqual([serialized(a), serialized(b)]) + }) + it('settles a read transaction that aborts after its request succeeds', async () => { const get = IDBObjectStore.prototype.get jest.spyOn(IDBObjectStore.prototype, 'get').mockImplementationOnce(function (this: IDBObjectStore, key) { diff --git a/packages/browser/test/cache/indexeddb-test-helpers.ts b/packages/browser/test/cache/indexeddb-test-helpers.ts index 99d73a89..ac10058e 100644 --- a/packages/browser/test/cache/indexeddb-test-helpers.ts +++ b/packages/browser/test/cache/indexeddb-test-helpers.ts @@ -1,18 +1,25 @@ import { IDBObjectStore } from 'fake-indexeddb' /** Observe an actual commit instead of sleeping for an assumed write duration. */ -export function nextWrite(): Promise { +export function nextWrite(count = 1): Promise { const put = IDBObjectStore.prototype.put return new Promise((resolve, reject) => { + let started = 0 + let completed = 0 const spy = jest.spyOn(IDBObjectStore.prototype, 'put').mockImplementation(function ( this: IDBObjectStore, ...args: Parameters ) { const request = put.apply(this, args) if (!String(args[1]).startsWith('assignments-')) return request - spy.mockRestore() - request.transaction!.addEventListener('complete', () => resolve()) - request.transaction!.addEventListener('abort', () => reject(request.transaction!.error)) + if (++started === count) spy.mockRestore() + request.transaction!.addEventListener('complete', () => { + if (++completed === count) resolve() + }) + request.transaction!.addEventListener('abort', () => { + spy.mockRestore() + reject(request.transaction!.error) + }) return request }) }) diff --git a/packages/browser/test/openfeature/exposures.spec.ts b/packages/browser/test/openfeature/exposures.spec.ts index f1d94e1c..0f85a68a 100644 --- a/packages/browser/test/openfeature/exposures.spec.ts +++ b/packages/browser/test/openfeature/exposures.spec.ts @@ -5,6 +5,7 @@ import type { FlaggingInitConfiguration } from '../../src/domain/configuration' import { createDatadogExposureLoggingHook } from '../../src/openfeature/exposures' import { DatadogProvider } from '../../src/openfeature/provider' import type { DDRum } from '../../src/openfeature/rumIntegration' +import { nextWrite } from '../cache/indexeddb-test-helpers' import precomputedServerResponse from '../data/precomputed-v1.json' describe('Exposures End-to-End', () => { @@ -860,8 +861,10 @@ describe('Exposures End-to-End', () => { await OpenFeature.setProviderAndWait(provider1) const client1 = OpenFeature.getClient() + const written = nextWrite() client1.getStringValue('string-flag', 'default') triggerBatch() + await written // Verify first exposure was logged expect(getExposuresCalls()).toHaveLength(1) diff --git a/packages/browser/test/openfeature/obfuscation.spec.ts b/packages/browser/test/openfeature/obfuscation.spec.ts index 87df3d9f..b72c7960 100644 --- a/packages/browser/test/openfeature/obfuscation.spec.ts +++ b/packages/browser/test/openfeature/obfuscation.spec.ts @@ -12,7 +12,7 @@ import { DatadogCoreProvider, } from '../../src/rules-based' import { fetchPrecomputedConfiguration } from '../../src/transport/fetchConfiguration' -import { deferred } from '../cache/indexeddb-test-helpers' +import { deferred, nextWrite } from '../cache/indexeddb-test-helpers' const context = { targetingKey: 'athlete-123' } const key = 'new-route-planner' @@ -282,18 +282,20 @@ describe('browser flag-key obfuscation', () => { ) await install() - const evaluate = () => { + const evaluate = async (expectWrite = false) => { + const written = expectWrite ? nextWrite() : undefined expect(client.getBooleanValue(key, false)).toBe(true) jest.advanceTimersByTime(31_000) + await written } - evaluate() - evaluate() + await evaluate(true) + await evaluate() expect(exposureEvents()).toHaveLength(1) // createdAt may stay unchanged across requests. Reusing the same snapshot // must preserve deduplication, including a portable serialization round trip. await refresh() - evaluate() + await evaluate() expect(exposureEvents()).toHaveLength(1) // Timestamp changes permit another exposure with identical assignment IDs. @@ -303,8 +305,8 @@ describe('browser flag-key obfuscation', () => { await refresh() jest.advanceTimersByTime(31_000) expect(exposureEvents()).toHaveLength(index + 1) - evaluate() - evaluate() + await evaluate(true) + await evaluate() expect(exposureEvents()).toHaveLength(index + 2) } @@ -312,7 +314,7 @@ describe('browser flag-key obfuscation', () => { for (const [index, publicSalt] of ['f'.repeat(32), '', salt].entries()) { payload = response(publicSalt, true, `2026-09-30T00:0${index + 2}:00Z`) await refresh() - evaluate() + await evaluate(true) expect(exposureEvents()).toHaveLength(index + 4) } @@ -325,7 +327,7 @@ describe('browser flag-key obfuscation', () => { ].entries()) { Object.assign(assignment, change) await refresh() - evaluate() + await evaluate(true) expect(exposureEvents()).toHaveLength(index + 7) } expect(exposureEvents()[8]).toMatchObject({ @@ -340,7 +342,7 @@ describe('browser flag-key obfuscation', () => { await tracking?.shutdown() await OpenFeature.clearProviders() await install() - evaluate() + await evaluate() expect(exposureEvents()).toHaveLength(9) client.clearHooks() await tracking?.shutdown()