diff --git a/src/pageViewStorage.ts b/src/pageViewStorage.ts index 35297a2..3a041be 100644 --- a/src/pageViewStorage.ts +++ b/src/pageViewStorage.ts @@ -17,6 +17,10 @@ export interface PageEvent { activeTimeOnPage?: number; } +function capPageViews(views: PageEvent[]): PageEvent[] { + return views.slice(-PAGE_VIEWS_MAX_COUNT); +} + export function migrateLegacyPageViewStorage(loggingService: LoggingService | null): void { const legacyViews = readJSON(LEGACY_PAGE_VIEWS_KEY); if (legacyViews === null) { @@ -50,7 +54,7 @@ export function loadPageViews(loggingService: LoggingService | null): PageEvent[ } export function writePageViews(pageViews: PageEvent[]): boolean { - return writeNamespacedField(LS_NAMESPACE_KEY, LS_PAGE_VIEWS_FIELD, pageViews.slice(-PAGE_VIEWS_MAX_COUNT)); + return writeNamespacedField(LS_NAMESPACE_KEY, LS_PAGE_VIEWS_FIELD, capPageViews(pageViews)); } export function clearPageViews(): void { @@ -58,7 +62,7 @@ export function clearPageViews(): void { } export function buildPageEvents(pageViews: PageEvent[]): PageEvent[] { - const views = pageViews.slice(-PAGE_VIEWS_MAX_COUNT); + const views = capPageViews(pageViews); return views.map((pageView, index) => { const activeTimeOnSite = pageView.activeTimeOnSite; const hasActiveTime = activeTimeOnSite !== undefined && Number.isFinite(activeTimeOnSite); diff --git a/src/storage.ts b/src/storage.ts index 536e718..10b60d4 100644 --- a/src/storage.ts +++ b/src/storage.ts @@ -63,35 +63,3 @@ export function removeNamespacedField(namespaceKey: string, field: string): void writeJSON(namespaceKey, next); } } - -export function writeNamespacedFieldWithinBudget( - namespaceKey: string, - field: string, - records: unknown[], - maxLength: number, -): boolean { - // Operate on a copy so the caller's array isn't trimmed as a side effect. - const remaining = records.slice(); - - const evictOldest = (): boolean => { - if (remaining.length <= 1) { - return false; - } - remaining.shift(); - return true; - }; - - // Two limits: our own soft cap (maxLength), then the browser's hard quota, - // which is shared across the origin and only surfaces when setItem throws. - let overBudget = JSON.stringify(remaining).length > maxLength; - while (overBudget && evictOldest()) { - overBudget = JSON.stringify(remaining).length > maxLength; - } - - let written = writeNamespacedField(namespaceKey, field, remaining); - while (!written && evictOldest()) { - written = writeNamespacedField(namespaceKey, field, remaining); - } - - return written; -} diff --git a/test/src/pageViewStorage.spec.ts b/test/src/pageViewStorage.spec.ts index e1a0574..84af00c 100644 --- a/test/src/pageViewStorage.spec.ts +++ b/test/src/pageViewStorage.spec.ts @@ -154,8 +154,6 @@ describe('pageViewStorage', () => { })); const result = buildPageEvents(views); expect(result).toHaveLength(25); - // First record in the capped window should have activeTimeOnPage derived from - // the next record within the slice, not from the unsliced original array. expect(result[0].activeTimeOnPage).toBe(1000); expect(result[24].activeTimeOnPage).toBeUndefined(); }); diff --git a/test/src/storage.spec.ts b/test/src/storage.spec.ts index 598143b..821333e 100644 --- a/test/src/storage.spec.ts +++ b/test/src/storage.spec.ts @@ -7,7 +7,6 @@ import { readNamespacedField, writeNamespacedField, removeNamespacedField, - writeNamespacedFieldWithinBudget, } from '../../src/storage'; describe('storage: key-agnostic localStorage helpers', () => { @@ -156,68 +155,6 @@ describe('storage: key-agnostic localStorage helpers', () => { expect(readJSON(NAMESPACE_KEY)).toEqual({ other: 1 }); }); }); - - describe('writeNamespacedFieldWithinBudget', () => { - const NAMESPACE_KEY = 'mp-rokt-kit'; - const BUDGET = 1024; - - it('writes all records unchanged when under budget', () => { - const records = [{ id: 1 }, { id: 2 }]; - expect(writeNamespacedFieldWithinBudget(NAMESPACE_KEY, 'pageViews', records, BUDGET)).toBe(true); - expect(records).toHaveLength(2); - expect(readNamespacedField(NAMESPACE_KEY, 'pageViews')).toEqual([{ id: 1 }, { id: 2 }]); - }); - - it('evicts oldest-first until the serialized size is within budget', () => { - const big = 'x'.repeat(300); - const records = [ - { id: 'a', v: big }, - { id: 'b', v: big }, - { id: 'c', v: big }, - { id: 'd', v: big }, - ]; - expect(writeNamespacedFieldWithinBudget(NAMESPACE_KEY, 'pageViews', records, BUDGET)).toBe(true); - - const stored = readNamespacedField(NAMESPACE_KEY, 'pageViews') as { id: string }[]; - expect(JSON.stringify(stored).length).toBeLessThanOrEqual(BUDGET); - expect(stored[stored.length - 1].id).toBe('d'); - expect(stored.map((r) => r.id)).not.toContain('a'); - }); - - it('keeps at least the newest record even when it alone exceeds the budget', () => { - const records = [{ id: 'newest', v: 'x'.repeat(BUDGET * 2) }]; - expect(writeNamespacedFieldWithinBudget(NAMESPACE_KEY, 'pageViews', records, BUDGET)).toBe(true); - expect(readNamespacedField(NAMESPACE_KEY, 'pageViews')).toEqual([{ id: 'newest', v: 'x'.repeat(BUDGET * 2) }]); - }); - - it('evicts and retries when writes fail, then persists', () => { - let calls = 0; - const realSetItem = Storage.prototype.setItem; - vi.spyOn(Storage.prototype, 'setItem').mockImplementation(function (this: Storage, key: string, value: string) { - calls += 1; - if (calls <= 2) { - throw new DOMException('quota', 'QuotaExceededError'); - } - return realSetItem.call(this, key, value); - }); - - const records = [{ id: 'a' }, { id: 'b' }, { id: 'c' }, { id: 'd' }]; - expect(writeNamespacedFieldWithinBudget(NAMESPACE_KEY, 'pageViews', records, BUDGET)).toBe(true); - - const stored = readNamespacedField(NAMESPACE_KEY, 'pageViews') as { id: string }[]; - // 2 failed writes evict the oldest twice (a, then b), leaving [c, d]. - expect(stored.map((r) => r.id)).toEqual(['c', 'd']); - }); - - it('returns false when even a single record cannot be written', () => { - vi.spyOn(Storage.prototype, 'setItem').mockImplementation(() => { - throw new DOMException('quota', 'QuotaExceededError'); - }); - const records = [{ id: 'a' }, { id: 'b' }]; - expect(writeNamespacedFieldWithinBudget(NAMESPACE_KEY, 'pageViews', records, BUDGET)).toBe(false); - expect(records).toEqual([{ id: 'a' }, { id: 'b' }]); - }); - }); }); describe('isLocalStorageAvailable', () => {