From 7181efde8d0944795f869a6f67a101fda34a67ca Mon Sep 17 00:00:00 2001 From: Gavin Barron Date: Mon, 20 Nov 2023 12:26:17 -0800 Subject: [PATCH] fix: subscription caching (#2864) adds a transaction helper function to the CacheStore uses transactions to ensure atomic read and update when updating subscription cache data --- .../Caching/SubscriptionCache.ts | 60 +++++++++++-------- packages/mgt-element/src/utils/CacheStore.ts | 14 ++++- .../mgt-element/src/utils/CustomElement.ts | 1 - 3 files changed, 47 insertions(+), 28 deletions(-) diff --git a/packages/mgt-chat/src/statefulClient/Caching/SubscriptionCache.ts b/packages/mgt-chat/src/statefulClient/Caching/SubscriptionCache.ts index 1e064257a0..7065f8a774 100644 --- a/packages/mgt-chat/src/statefulClient/Caching/SubscriptionCache.ts +++ b/packages/mgt-chat/src/statefulClient/Caching/SubscriptionCache.ts @@ -5,10 +5,10 @@ * ------------------------------------------------------------------------------------------- */ -import { CacheItem, CacheSchema, CacheService, CacheStore, schemas } from '@microsoft/mgt-react'; +import { CacheItem, CacheSchema, CacheService, CacheStore, log, schemas } from '@microsoft/mgt-react'; import { Subscription } from '@microsoft/microsoft-graph-types'; import { isConversationCacheEnabled } from './isConversationCacheEnabled'; -import { cacheEntryIsValid } from './cacheEntryIsValid'; +import { IDBPObjectStore } from 'idb'; type CachedSubscriptionData = CacheItem & { chatId: string; @@ -28,38 +28,46 @@ export class SubscriptionsCache { public async loadSubscriptions(chatId: string, sessionId: string): Promise { if (isConversationCacheEnabled()) { const cacheKey = buildCacheKey(chatId, sessionId); - const data = await this.cache.getValue(cacheKey); - if (data && cacheEntryIsValid(data)) { - data.lastAccessDateTime = new Date().toISOString(); - await this.cache.putValue(cacheKey, data); - return data; - } + let data; + await this.cache.transaction(async (store: IDBPObjectStore) => { + data = (await store.get(cacheKey)) as CachedSubscriptionData | undefined; + if (data) { + data.lastAccessDateTime = new Date().toISOString(); + await store.put(data, cacheKey); + } + }); + return data || undefined; } return undefined; } public async cacheSubscription(chatId: string, sessionId: string, subscriptionRecord: Subscription): Promise { - let cacheEntry = await this.loadSubscriptions(chatId, sessionId); - if (cacheEntry && cacheEntry.chatId === chatId) { - const subIndex = cacheEntry.subscriptions.findIndex(s => s.resource === subscriptionRecord.resource); - if (subIndex !== -1) { - cacheEntry.subscriptions[subIndex] = subscriptionRecord; + await this.cache.transaction(async (store: IDBPObjectStore) => { + log('cacheSubscription', subscriptionRecord); + const cacheKey = buildCacheKey(chatId, sessionId); + + let cacheEntry = (await store.get(cacheKey)) as CachedSubscriptionData | undefined; + if (cacheEntry && cacheEntry.chatId === chatId) { + const subIndex = cacheEntry.subscriptions.findIndex(s => s.resource === subscriptionRecord.resource); + if (subIndex !== -1) { + cacheEntry.subscriptions[subIndex] = subscriptionRecord; + } else { + cacheEntry.subscriptions.push(subscriptionRecord); + } } else { - cacheEntry.subscriptions.push(subscriptionRecord); + cacheEntry = { + chatId, + sessionId, + subscriptions: [subscriptionRecord], + // we're cheating a bit here to ensure that we have a defined lastAccessDateTime + // but we're updating the value for all cases before storing it. + lastAccessDateTime: '' + }; } - } else { - cacheEntry = { - chatId, - sessionId, - subscriptions: [subscriptionRecord], - // we're cheating a bit here to ensure that we have a defined lastAccessDateTime - // but we're updating the value for all cases before storing it. - lastAccessDateTime: '' - }; - } - cacheEntry.lastAccessDateTime = new Date().toISOString(); + cacheEntry.lastAccessDateTime = new Date().toISOString(); - await this.cache.putValue(buildCacheKey(chatId, sessionId), cacheEntry); + await store.put(cacheEntry, cacheKey); + }); } public deleteCachedSubscriptions(chatId: string, sessionId: string): Promise { diff --git a/packages/mgt-element/src/utils/CacheStore.ts b/packages/mgt-element/src/utils/CacheStore.ts index d4a545ef36..190d725d01 100644 --- a/packages/mgt-element/src/utils/CacheStore.ts +++ b/packages/mgt-element/src/utils/CacheStore.ts @@ -5,7 +5,7 @@ * ------------------------------------------------------------------------------------------- */ -import { openDB } from 'idb'; +import { IDBPObjectStore, openDB } from 'idb'; import { Providers } from '../providers/Providers'; import { CacheItem, CacheSchema, Index, dbListKey } from './CacheService'; @@ -151,4 +151,16 @@ export class CacheStore { const db = await this.getDb(); return (await db.getAllFromIndex(this.store, indexName, query)) as T[]; } + + /** + * Helper function to get a wrapping transaction for an action function + * @param action a function that takes an object store uses it to make changes to the cache + */ + public async transaction(action: (store: IDBPObjectStore) => Promise) { + const db = await this.getDb(); + const tx = db.transaction(this.store, 'readwrite'); + const store = tx.objectStore(this.store); + await action(store); + await tx.done; + } } diff --git a/packages/mgt-element/src/utils/CustomElement.ts b/packages/mgt-element/src/utils/CustomElement.ts index ca97334a76..799ccb6ab4 100644 --- a/packages/mgt-element/src/utils/CustomElement.ts +++ b/packages/mgt-element/src/utils/CustomElement.ts @@ -26,7 +26,6 @@ export const customElement = (tagName: string): ((classOrDescriptor: unknown) => ((element as any).packageVersion || unknownVersion) as string; if (mgtElement) { return (classOrDescriptor: CustomElementConstructor) => { - // eslint-disable-next-line no-console error( `Tag name ${mgtTagName} is already defined using class ${mgtElement.name} version ${version(mgtElement)}\n`, `Currently registering class ${classOrDescriptor.name} with version ${version(classOrDescriptor)}\n`,