Skip to content

Commit 9223f03

Browse files
j3k0j3k0
committed
fix: address senior review — blockers + API surface improvements
Blockers: - Rebuild www/store.js (review fix was never shipped in 463f6ee) - clear() no longer calls store.off() — verified listener stays registered, so persistence continues working after logout. Removed the verifiedCallback field entirely (reviewer's option A). - Added test: clear() + ready() + verified event still persists. API surface improvements (permanent before ship): - Rename token_invalid → entitlement_missing, token_expired → expired (no tokens in phase 1; entitlement_missing fires for the normal case of checking a product the user never bought) - Dedup events per productId: repeated isOwned() calls with the same state no longer re-fire the same event - refresh() now returns Promise<void> so callers can await completion - Scope clock rollback to subscriptions only (non-consumables have no time component — a lifetime purchaser fixing a wrong clock should not be locked out) - log.warn on pre-ready isOwned() calls (top integration footgun) New tests (22 total, up from 16): - clear() + verified event still persists - event dedup (same type fires once; different types both fire) - ready() idempotency - corrupted JSON in storage - schema version mismatch Closes FOV-882
1 parent 463f6ee commit 9223f03

4 files changed

Lines changed: 227 additions & 49 deletions

File tree

‎src/ts/offline-entitlements.ts‎

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ namespace CdvPurchase {
2121

2222
/** Event emitted by {@link OfflineEntitlements} when evaluating ownership offline. */
2323
export interface OfflineEntitlementEvent {
24-
type: 'grace' | 'readonly' | 'clock_rollback' | 'token_invalid' | 'token_expired';
24+
type: 'grace' | 'readonly' | 'clock_rollback' | 'entitlement_missing' | 'expired';
2525
productId: string;
2626
message: string;
2727
}
@@ -82,8 +82,8 @@ namespace CdvPurchase {
8282
/** Event callbacks. */
8383
private eventCallbacks: Internal.Callbacks<OfflineEntitlementEvent>;
8484

85-
/** The callback registered on `store.when().verified(...)`, kept so we can `off()` it on `clear()`. */
86-
private verifiedCallback: Callback<VerifiedReceipt>;
85+
/** Last event fired per productId, to deduplicate events on repeated isOwned() calls. */
86+
private lastEventPerProduct: { [productId: string]: OfflineEntitlementEvent['type'] } = {};
8787

8888
constructor(store: Store, options: OfflineEntitlementsOptions = {}) {
8989
this.store = store;
@@ -92,8 +92,7 @@ namespace CdvPurchase {
9292
this.onExpiredOffline = options.onExpiredOffline ?? 'readonly';
9393
this.detectClockRollback = options.detectClockRollback ?? false;
9494
this.eventCallbacks = new Internal.Callbacks<OfflineEntitlementEvent>(store.log, 'OfflineEntitlements');
95-
this.verifiedCallback = (receipt: VerifiedReceipt) => { void this.onVerified(receipt); };
96-
this.store.when().verified(this.verifiedCallback);
95+
this.store.when().verified((receipt: VerifiedReceipt) => { void this.onVerified(receipt); });
9796
}
9897

9998
/** Wrap the global `localStorage` as an async `OfflineStorageAdapter`. */
@@ -112,9 +111,9 @@ namespace CdvPurchase {
112111
this.isReady = true;
113112
}
114113

115-
/** Reload from storage and re-evaluate. Call after reconnecting or manually. */
116-
refresh(): void {
117-
void this.loadFromStorage();
114+
/** Reload from storage and re-evaluate. Resolves when the reload is complete. Call after reconnecting or manually. */
115+
refresh(): Promise<void> {
116+
return this.loadFromStorage();
118117
}
119118

120119
/** Register a callback for {@link OfflineEntitlementEvent}s. */
@@ -124,10 +123,10 @@ namespace CdvPurchase {
124123

125124
/** Remove all persisted entitlements from storage and clear the in-memory cache. For user logout. */
126125
async clear(): Promise<void> {
127-
this.store.off(this.verifiedCallback);
128126
this.cache = {};
129127
this.lastSeenTimestamp = 0;
130128
this.isReady = false;
129+
this.lastEventPerProduct = {};
131130
await this.storage.removeItem(OfflineEntitlements.STORAGE_KEY);
132131
}
133132

@@ -138,34 +137,36 @@ namespace CdvPurchase {
138137
* Returns `false` for all products until `ready()` has resolved.
139138
*/
140139
isOwned(productId: string): boolean {
141-
if (!this.isReady) return false;
140+
if (!this.isReady) {
141+
this.store.log.warn('OfflineEntitlements.isOwned("' + productId + '") called before ready() — returning false. Call await offline.ready() at startup.');
142+
return false;
143+
}
142144

143145
// 1. If store.verifiedReceipts has a valid (non-expired) entry, return true.
144146
const online = Internal.VerifiedReceipts.isOwned(this.store.verifiedReceipts, { id: productId });
145147
if (online) return true;
146148

147149
const now = +new Date();
148150

149-
// 4. Clock rollback detection.
150-
if (this.detectClockRollback && this.lastSeenTimestamp > now) {
151-
this.fireEvent('clock_rollback', productId, 'Clock appears to have rolled back; denying offline entitlement.');
152-
return false;
153-
}
154-
155151
// 2. Find persisted purchase for this productId across all platforms.
156152
const persisted = this.findPersisted(productId);
157153
if (!persisted) {
158-
// 5. No persisted entitlement.
159-
this.fireEvent('token_invalid', productId, 'No persisted entitlement for this product.');
154+
// No persisted entitlement.
155+
this.fireEvent('entitlement_missing', productId, 'No persisted entitlement for this product.');
160156
return false;
161157
}
162158

163159
// 3. Branch by product type.
164160
// Subscriptions have an expiryDate; non-consumables don't.
165161
if (persisted.expiryDate !== undefined && persisted.expiryDate !== null) {
166162
// Subscription
163+
// Clock rollback detection — scoped to subscriptions (non-consumables have no time component).
164+
if (this.detectClockRollback && this.lastSeenTimestamp > now) {
165+
this.fireEvent('clock_rollback', persisted.id, 'Clock appears to have rolled back; denying offline entitlement.');
166+
return false;
167+
}
167168
if (persisted.isExpired) {
168-
this.fireEvent('token_expired', persisted.id, 'Subscription is marked as expired.');
169+
this.fireEvent('expired', persisted.id, 'Subscription is marked as expired.');
169170
return false;
170171
}
171172
if (now < persisted.expiryDate) {
@@ -188,7 +189,7 @@ namespace CdvPurchase {
188189
return false;
189190
}
190191
else {
191-
// Non-consumable: never hard-expires.
192+
// Non-consumable: never hard-expires, no clock rollback check.
192193
return true;
193194
}
194195
}
@@ -274,8 +275,10 @@ namespace CdvPurchase {
274275
}
275276
}
276277

277-
/** Fire an event to all registered callbacks. */
278+
/** Fire an event to all registered callbacks, deduplicating per productId. */
278279
private fireEvent(type: OfflineEntitlementEvent['type'], productId: string, message: string): void {
280+
if (this.lastEventPerProduct[productId] === type) return;
281+
this.lastEventPerProduct[productId] = type;
279282
this.eventCallbacks.trigger({ type, productId, message }, 'offline_entitlements');
280283
}
281284
}

‎tests/offline-entitlements.test.ts‎

Lines changed: 174 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ describe('OfflineEntitlements', () => {
238238

239239
expect(oe.isOwned(productId)).toBe(false);
240240
await flushTimers();
241-
expect(events.some(e => e.type === 'token_expired' && e.productId === productId)).toBe(true);
241+
expect(events.some(e => e.type === 'expired' && e.productId === productId)).toBe(true);
242242
});
243243
});
244244

@@ -271,15 +271,15 @@ describe('OfflineEntitlements', () => {
271271
});
272272

273273
describe('isOwned — no persisted entitlement', () => {
274-
test('returns false + token_invalid event', async () => {
274+
test('returns false + entitlement_missing event', async () => {
275275
const storage = mockStorage();
276276
const oe = new CdvPurchase.OfflineEntitlements(CdvPurchase.store, { storage });
277277
const events = collectEvents(oe);
278278
await oe.ready();
279279

280280
expect(oe.isOwned('unknown-product')).toBe(false);
281281
await flushTimers();
282-
expect(events.some(e => e.type === 'token_invalid' && e.productId === 'unknown-product')).toBe(true);
282+
expect(events.some(e => e.type === 'entitlement_missing' && e.productId === 'unknown-product')).toBe(true);
283283
});
284284
});
285285

@@ -460,9 +460,8 @@ describe('OfflineEntitlements', () => {
460460
};
461461
await storage.setItem('cdvpurchase.offline_entitlements', JSON.stringify(payload));
462462

463-
// refresh() triggers a reload (async), wait for it to settle
464-
oe.refresh();
465-
await new Promise(r => setTimeout(r, 10));
463+
// refresh() now returns a promise — await it directly
464+
await oe.refresh();
466465

467466
expect(oe.isOwned('refreshed')).toBe(true);
468467
});
@@ -494,4 +493,173 @@ describe('OfflineEntitlements', () => {
494493
expect(oe.isOwned('coins_pack')).toBe(false);
495494
});
496495
});
496+
497+
describe('clear() does not break subsequent verified events', () => {
498+
test('after clear() + ready(), verified events still persist', async () => {
499+
const storage = mockStorage();
500+
const oe = new CdvPurchase.OfflineEntitlements(CdvPurchase.store, { storage });
501+
await oe.ready();
502+
503+
// Clear data (e.g., user logout)
504+
await oe.clear();
505+
506+
// Re-ready
507+
await oe.ready();
508+
509+
// Simulate a verified receipt for the new user
510+
const now = Date.now();
511+
const receipt = makeVerifiedReceipt(Platform.APPLE_APPSTORE, {
512+
id: 'new_user_sub',
513+
platform: Platform.APPLE_APPSTORE,
514+
expiryDate: now + 30 * 24 * 60 * 60 * 1000,
515+
isExpired: false,
516+
renewalIntent: RenewalIntent.RENEW,
517+
purchaseDate: now - 1000,
518+
});
519+
520+
// @ts-ignore - accessing private property for testing
521+
CdvPurchase.store.verifiedCallbacks.trigger(receipt, 'test');
522+
await flushTimers();
523+
await new Promise(r => setTimeout(r, 10));
524+
525+
expect(oe.isOwned('new_user_sub')).toBe(true);
526+
});
527+
});
528+
529+
describe('event deduplication', () => {
530+
test('same event type for same product fires only once', async () => {
531+
const storage = mockStorage();
532+
const oe = new CdvPurchase.OfflineEntitlements(CdvPurchase.store, { storage });
533+
const events = collectEvents(oe);
534+
await oe.ready();
535+
536+
// No entitlement → entitlement_missing on every call, but deduped
537+
oe.isOwned('dedup_product');
538+
await flushTimers();
539+
oe.isOwned('dedup_product');
540+
await flushTimers();
541+
oe.isOwned('dedup_product');
542+
await flushTimers();
543+
544+
const missing = events.filter(e => e.type === 'entitlement_missing' && e.productId === 'dedup_product');
545+
expect(missing.length).toBe(1);
546+
});
547+
548+
test('different event types for same product both fire', async () => {
549+
const storage = mockStorage();
550+
const now = Date.now();
551+
552+
// First: a valid subscription → no event, returns true
553+
const payloadValid = {
554+
receipts: {
555+
[Platform.APPLE_APPSTORE + ':dedup2']: {
556+
id: 'dedup2',
557+
platform: Platform.APPLE_APPSTORE,
558+
expiryDate: now + 30 * 24 * 60 * 60 * 1000,
559+
isExpired: false,
560+
renewalIntent: RenewalIntent.RENEW,
561+
},
562+
},
563+
lastSeenTimestamp: now,
564+
schemaVersion: 1,
565+
};
566+
await storage.setItem('cdvpurchase.offline_entitlements', JSON.stringify(payloadValid));
567+
568+
const oe = new CdvPurchase.OfflineEntitlements(CdvPurchase.store, { storage });
569+
const events = collectEvents(oe);
570+
await oe.ready();
571+
572+
expect(oe.isOwned('dedup2')).toBe(true);
573+
await flushTimers();
574+
575+
// Now overwrite with an expired subscription → grace event
576+
const payloadGrace = {
577+
receipts: {
578+
[Platform.APPLE_APPSTORE + ':dedup2']: {
579+
id: 'dedup2',
580+
platform: Platform.APPLE_APPSTORE,
581+
expiryDate: now - 5 * 24 * 60 * 60 * 1000,
582+
isExpired: false,
583+
renewalIntent: RenewalIntent.RENEW,
584+
},
585+
},
586+
lastSeenTimestamp: now,
587+
schemaVersion: 1,
588+
};
589+
await storage.setItem('cdvpurchase.offline_entitlements', JSON.stringify(payloadGrace));
590+
await oe.refresh();
591+
592+
expect(oe.isOwned('dedup2')).toBe(true);
593+
await flushTimers();
594+
595+
const grace = events.filter(e => e.type === 'grace' && e.productId === 'dedup2');
596+
expect(grace.length).toBe(1);
597+
});
598+
});
599+
600+
describe('ready() idempotency', () => {
601+
test('second ready() call is a no-op', async () => {
602+
const storage = mockStorage();
603+
const now = Date.now();
604+
const payload = {
605+
receipts: {
606+
[Platform.APPLE_APPSTORE + ':idem']: {
607+
id: 'idem',
608+
platform: Platform.APPLE_APPSTORE,
609+
expiryDate: now + 30 * 24 * 60 * 60 * 1000,
610+
isExpired: false,
611+
renewalIntent: RenewalIntent.RENEW,
612+
},
613+
},
614+
lastSeenTimestamp: now,
615+
schemaVersion: 1,
616+
};
617+
await storage.setItem('cdvpurchase.offline_entitlements', JSON.stringify(payload));
618+
619+
const oe = new CdvPurchase.OfflineEntitlements(CdvPurchase.store, { storage });
620+
await oe.ready();
621+
// Second call should resolve immediately
622+
await oe.ready();
623+
624+
expect(oe.isOwned('idem')).toBe(true);
625+
});
626+
});
627+
628+
describe('loadFromStorage error handling', () => {
629+
test('corrupted JSON in storage → graceful fallback to empty cache', async () => {
630+
const storage = mockStorage();
631+
await storage.setItem('cdvpurchase.offline_entitlements', '{not valid json');
632+
633+
const oe = new CdvPurchase.OfflineEntitlements(CdvPurchase.store, { storage });
634+
await oe.ready();
635+
636+
// Should not throw, should return false
637+
expect(oe.isOwned('any-product')).toBe(false);
638+
});
639+
640+
test('schema version mismatch → ignores cached data', async () => {
641+
const storage = mockStorage();
642+
const now = Date.now();
643+
const payload = {
644+
receipts: {
645+
[Platform.APPLE_APPSTORE + ':mismatch']: {
646+
id: 'mismatch',
647+
platform: Platform.APPLE_APPSTORE,
648+
expiryDate: now + 30 * 24 * 60 * 60 * 1000,
649+
isExpired: false,
650+
renewalIntent: RenewalIntent.RENEW,
651+
},
652+
},
653+
lastSeenTimestamp: now,
654+
schemaVersion: 999,
655+
};
656+
await storage.setItem('cdvpurchase.offline_entitlements', JSON.stringify(payload));
657+
658+
const oe = new CdvPurchase.OfflineEntitlements(CdvPurchase.store, { storage });
659+
await oe.ready();
660+
661+
// Data should be ignored — no entitlement
662+
expect(oe.isOwned('mismatch')).toBe(false);
663+
});
664+
});
497665
});

‎www/store.d.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -899,7 +899,7 @@ declare namespace CdvPurchase {
899899
}
900900
/** Event emitted by {@link OfflineEntitlements} when evaluating ownership offline. */
901901
interface OfflineEntitlementEvent {
902-
type: 'grace' | 'readonly' | 'clock_rollback' | 'token_invalid' | 'token_expired';
902+
type: 'grace' | 'readonly' | 'clock_rollback' | 'entitlement_missing' | 'expired';
903903
productId: string;
904904
message: string;
905905
}
@@ -950,15 +950,15 @@ declare namespace CdvPurchase {
950950
private isReady;
951951
/** Event callbacks. */
952952
private eventCallbacks;
953-
/** The callback registered on `store.when().verified(...)`, kept so we can `off()` it on `clear()`. */
954-
private verifiedCallback;
953+
/** Last event fired per productId, to deduplicate events on repeated isOwned() calls. */
954+
private lastEventPerProduct;
955955
constructor(store: Store, options?: OfflineEntitlementsOptions);
956956
/** Wrap the global `localStorage` as an async `OfflineStorageAdapter`. */
957957
private static createLocalStorageAdapter;
958958
/** Load persisted entitlements from storage into the in-memory cache. Idempotent. */
959959
ready(): Promise<void>;
960-
/** Reload from storage and re-evaluate. Call after reconnecting or manually. */
961-
refresh(): void;
960+
/** Reload from storage and re-evaluate. Resolves when the reload is complete. Call after reconnecting or manually. */
961+
refresh(): Promise<void>;
962962
/** Register a callback for {@link OfflineEntitlementEvent}s. */
963963
onEvent(callback: Callback<OfflineEntitlementEvent>): void;
964964
/** Remove all persisted entitlements from storage and clear the in-memory cache. For user logout. */
@@ -978,7 +978,7 @@ declare namespace CdvPurchase {
978978
private loadFromStorage;
979979
/** Serialize the in-memory cache to storage. */
980980
private saveToStorage;
981-
/** Fire an event to all registered callbacks. */
981+
/** Fire an event to all registered callbacks, deduplicating per productId. */
982982
private fireEvent;
983983
}
984984
}

0 commit comments

Comments
 (0)