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
microsoftgraph · Nov 20, 2023 · 7181efd · 7181efd
1 parent e8fbbea
commit 7181efd
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 28 deletions.
diff --git 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<CachedSubscriptionData | undefined> {
     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<unknown, [string], string, 'readwrite'>) => {
+        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<void> {
-    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<unknown, [string], string, 'readwrite'>) => {
+      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<void> {

diff --git 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<T extends CacheItem> {
     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<unknown, [string], string, 'readwrite'>) => Promise<void>) {
+    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
@@ -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`,