Skip to content

Commit 358cc4c

Browse files
authored
Fix indexDB putObject concurrency issue and cache. (#4710)
* Fix indexDB putObject concurrency issue and cache. * Fix spurious triggers.
1 parent c1f07f1 commit 358cc4c

File tree

4 files changed

+34
-35
lines changed

4 files changed

+34
-35
lines changed

packages-exp/auth-exp/src/platform_browser/auth.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ describe('core/auth/auth_impl', () => {
7171
apiHost: DefaultConfig.API_HOST,
7272
apiScheme: DefaultConfig.API_SCHEME,
7373
tokenApiHost: DefaultConfig.TOKEN_API_HOST,
74+
clientPlatform: ClientPlatform.BROWSER,
7475
sdkClientVersion: 'v'
7576
});
7677

@@ -137,6 +138,7 @@ describe('core/auth/initializeAuth', () => {
137138
apiScheme: DefaultConfig.API_SCHEME,
138139
tokenApiHost: DefaultConfig.TOKEN_API_HOST,
139140
authDomain,
141+
clientPlatform: ClientPlatform.BROWSER,
140142
sdkClientVersion: _getClientVersion(ClientPlatform.BROWSER)
141143
});
142144

@@ -312,6 +314,7 @@ describe('core/auth/initializeAuth', () => {
312314
apiHost: DefaultConfig.API_HOST,
313315
apiScheme: DefaultConfig.API_SCHEME,
314316
tokenApiHost: DefaultConfig.TOKEN_API_HOST,
317+
clientPlatform: ClientPlatform.BROWSER,
315318
sdkClientVersion: _getClientVersion(ClientPlatform.BROWSER)
316319
});
317320
});

packages-exp/auth-exp/src/platform_browser/persistence/indexed_db.test.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,11 @@ describe('platform_browser/persistence/indexed_db', () => {
128128
clock.restore();
129129
});
130130

131+
it('should not trigger a listener when there are no changes', async () => {
132+
await waitUntilPoll(clock);
133+
expect(callback).not.to.have.been.called;
134+
});
135+
131136
it('should trigger a listener when the key changes', async () => {
132137
await _putObject(db, key, newValue);
133138

packages-exp/auth-exp/src/platform_browser/persistence/indexed_db.ts

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -127,20 +127,11 @@ export async function _putObject(
127127
key: string,
128128
value: PersistenceValue | string
129129
): Promise<void> {
130-
const getRequest = getObjectStore(db, false).get(key);
131-
const data = await new DBPromise<DBObject | null>(getRequest).toPromise();
132-
if (data) {
133-
// Force an index signature on the user object
134-
data.value = value as PersistedBlob;
135-
const request = getObjectStore(db, true).put(data);
136-
return new DBPromise<void>(request).toPromise();
137-
} else {
138-
const request = getObjectStore(db, true).add({
139-
[DB_DATA_KEYPATH]: key,
140-
value
141-
});
142-
return new DBPromise<void>(request).toPromise();
143-
}
130+
const request = getObjectStore(db, true).put({
131+
[DB_DATA_KEYPATH]: key,
132+
value
133+
});
134+
return new DBPromise<void>(request).toPromise();
144135
}
145136

146137
async function getObject(
@@ -382,7 +373,7 @@ class IndexedDBLocalPersistence implements InternalPersistence {
382373
}
383374
}
384375
for (const localKey of Object.keys(this.localCache)) {
385-
if (!keysInResult.has(localKey)) {
376+
if (this.localCache[localKey] && !keysInResult.has(localKey)) {
386377
// Deleted
387378
this.notifyListeners(localKey, null);
388379
keys.push(localKey);
@@ -395,12 +386,12 @@ class IndexedDBLocalPersistence implements InternalPersistence {
395386
key: string,
396387
newValue: PersistenceValue | null
397388
): void {
398-
if (!this.listeners[key]) {
399-
return;
400-
}
401389
this.localCache[key] = newValue;
402-
for (const listener of Array.from(this.listeners[key])) {
403-
listener(newValue);
390+
const listeners = this.listeners[key];
391+
if (listeners) {
392+
for (const listener of Array.from(listeners)) {
393+
listener(newValue);
394+
}
404395
}
405396
}
406397

@@ -424,7 +415,11 @@ class IndexedDBLocalPersistence implements InternalPersistence {
424415
if (Object.keys(this.listeners).length === 0) {
425416
this.startPolling();
426417
}
427-
this.listeners[key] = this.listeners[key] || new Set();
418+
if (!this.listeners[key]) {
419+
this.listeners[key] = new Set();
420+
// Populate the cache to avoid spuriously triggering on first poll.
421+
void this._get(key); // This can happen in the background async and we can return immediately.
422+
}
428423
this.listeners[key].add(listener);
429424
}
430425

@@ -434,7 +429,6 @@ class IndexedDBLocalPersistence implements InternalPersistence {
434429

435430
if (this.listeners[key].size === 0) {
436431
delete this.listeners[key];
437-
delete this.localCache[key];
438432
}
439433
}
440434

packages-exp/auth-exp/src/platform_browser/persistence/local_storage.ts

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,6 @@ class BrowserLocalPersistence
9999

100100
const key = event.key;
101101

102-
// Ignore keys that have no listeners.
103-
if (!this.listeners[key]) {
104-
return;
105-
}
106-
107102
// Check the mechanism how this event was detected.
108103
// The first event will dictate the mechanism to be used.
109104
if (poll) {
@@ -165,12 +160,12 @@ class BrowserLocalPersistence
165160
}
166161

167162
private notifyListeners(key: string, value: string | null): void {
168-
if (!this.listeners[key]) {
169-
return;
170-
}
171163
this.localCache[key] = value;
172-
for (const listener of Array.from(this.listeners[key])) {
173-
listener(value ? JSON.parse(value) : value);
164+
const listeners = this.listeners[key];
165+
if (listeners) {
166+
for (const listener of Array.from(listeners)) {
167+
listener(value ? JSON.parse(value) : value);
168+
}
174169
}
175170
}
176171

@@ -209,7 +204,6 @@ class BrowserLocalPersistence
209204
}
210205

211206
_addListener(key: string, listener: StorageEventListener): void {
212-
this.localCache[key] = this.storage.getItem(key);
213207
if (Object.keys(this.listeners).length === 0) {
214208
// Whether browser can detect storage event when it had already been pushed to the background.
215209
// This may happen in some mobile browsers. A localStorage change in the foreground window
@@ -221,7 +215,11 @@ class BrowserLocalPersistence
221215
this.attachListener();
222216
}
223217
}
224-
this.listeners[key] = this.listeners[key] || new Set();
218+
if (!this.listeners[key]) {
219+
this.listeners[key] = new Set();
220+
// Populate the cache to avoid spuriously triggering on first poll.
221+
this.localCache[key] = this.storage.getItem(key);
222+
}
225223
this.listeners[key].add(listener);
226224
}
227225

@@ -231,7 +229,6 @@ class BrowserLocalPersistence
231229

232230
if (this.listeners[key].size === 0) {
233231
delete this.listeners[key];
234-
delete this.localCache[key];
235232
}
236233
}
237234

0 commit comments

Comments
 (0)