Migrate Most Recent activity local storage to App State persistence (#1967)

* Rewrite MostRecentActivity to leverage AppStatePersistenceUtility.

* Fix format. Update type enum.

* Migrate Item enum to string enum

* Fix unit tests

* Fix build issue

src/Explorer/MostRecentActivity/MostRecentActivity.test.ts

@@ -1,13 +1,13 @@
+import { clear, collectionWasOpened, getItems, Type } from "Explorer/MostRecentActivity/MostRecentActivity";
 import { observable } from "knockout";
-import { mostRecentActivity } from "./MostRecentActivity";
 
 describe("MostRecentActivity", () => {
-  const accountId = "some account";
+  const accountName = "some account";
 
-  beforeEach(() => mostRecentActivity.clear(accountId));
+  beforeEach(() => clear(accountName));
 
   it("Has no items at first", () => {
-    expect(mostRecentActivity.getItems(accountId)).toStrictEqual([]);
+    expect(getItems(accountName)).toStrictEqual([]);
   });
 
   it("Can record collections being opened", () => {
@@ -18,9 +18,9 @@ describe("MostRecentActivity", () => {
       databaseId,
     };
 
-    mostRecentActivity.collectionWasOpened(accountId, collection);
+    collectionWasOpened(accountName, collection);
 
-    const activity = mostRecentActivity.getItems(accountId);
+    const activity = getItems(accountName);
     expect(activity).toEqual([
       expect.objectContaining({
         collectionId,
@@ -29,58 +29,24 @@ describe("MostRecentActivity", () => {
     ]);
   });
 
-  it("Can record notebooks being opened", () => {
-    const name = "some notebook";
-    const path = "some path";
-    const notebook = { name, path };
+  it("Does not store duplicate entries", () => {
+    const collectionId = "some collection";
+    const databaseId = "some database";
+    const collection = {
+      id: observable(collectionId),
+      databaseId,
+    };
 
-    mostRecentActivity.notebookWasItemOpened(accountId, notebook);
+    collectionWasOpened(accountName, collection);
+    collectionWasOpened(accountName, collection);
 
-    const activity = mostRecentActivity.getItems(accountId);
-    expect(activity).toEqual([expect.objectContaining(notebook)]);
-  });
-
-  it("Filters out duplicates", () => {
-    const name = "some notebook";
-    const path = "some path";
-    const notebook = { name, path };
-    const sameNotebook = { name, path };
-
-    mostRecentActivity.notebookWasItemOpened(accountId, notebook);
-    mostRecentActivity.notebookWasItemOpened(accountId, sameNotebook);
-
-    const activity = mostRecentActivity.getItems(accountId);
-    expect(activity.length).toEqual(1);
-    expect(activity).toEqual([expect.objectContaining(notebook)]);
-  });
-
-  it("Allows for multiple accounts", () => {
-    const name = "some notebook";
-    const path = "some path";
-    const notebook = { name, path };
-
-    const anotherNotebook = { name: "Another " + name, path };
-    const anotherAccountId = "Another " + accountId;
-
-    mostRecentActivity.notebookWasItemOpened(accountId, notebook);
-    mostRecentActivity.notebookWasItemOpened(anotherAccountId, anotherNotebook);
-
-    expect(mostRecentActivity.getItems(accountId)).toEqual([expect.objectContaining(notebook)]);
-    expect(mostRecentActivity.getItems(anotherAccountId)).toEqual([expect.objectContaining(anotherNotebook)]);
-  });
-
-  it("Can store multiple distinct elements, in FIFO order", () => {
-    const name = "some notebook";
-    const path = "some path";
-    const first = { name, path };
-    const second = { name: "Another " + name, path };
-    const third = { name, path: "Another " + path };
-
-    mostRecentActivity.notebookWasItemOpened(accountId, first);
-    mostRecentActivity.notebookWasItemOpened(accountId, second);
-    mostRecentActivity.notebookWasItemOpened(accountId, third);
-
-    const activity = mostRecentActivity.getItems(accountId);
-    expect(activity).toEqual([third, second, first].map(expect.objectContaining));
+    const activity = getItems(accountName);
+    expect(activity).toEqual([
+      expect.objectContaining({
+        type: Type.OpenCollection,
+        collectionId,
+        databaseId,
+      }),
+    ]);
   });
 });

src/Explorer/MostRecentActivity/MostRecentActivity.ts

@@ -1,10 +1,10 @@
+import { AppStateComponentNames, deleteState, loadState, saveState } from "Shared/AppStatePersistenceUtility";
 import { CollectionBase } from "../../Contracts/ViewModels";
-import { StorageKey, LocalStorageUtility } from "../../Shared/StorageUtility";
-import { NotebookContentItem } from "../Notebook/NotebookContentItem";
+import { LocalStorageUtility, StorageKey } from "../../Shared/StorageUtility";
 
 export enum Type {
-  OpenCollection,
-  OpenNotebook,
+  OpenCollection = "OpenCollection",
+  OpenNotebook = "OpenNotebook",
 }
 
 export interface OpenNotebookItem {
@@ -21,158 +21,174 @@ export interface OpenCollectionItem {
 
 type Item = OpenNotebookItem | OpenCollectionItem;
 
-// Update schemaVersion if you are going to change this interface
-interface StoredData {
-  schemaVersion: string;
-  itemsMap: { [accountId: string]: Item[] }; // FIFO
-}
+const itemsMaxNumber: number = 5;
 
 /**
- * Stores most recent activity
+ * Migrate old data to new AppState
  */
-class MostRecentActivity {
-  private static readonly schemaVersion: string = "2";
-  private static itemsMaxNumber: number = 5;
-  private storedData: StoredData;
-  constructor() {
-    // Retrieve from local storage
+const migrateOldData = () => {
   if (LocalStorageUtility.hasItem(StorageKey.MostRecentActivity)) {
+    const oldDataSchemaVersion: string = "2";
     const rawData = LocalStorageUtility.getEntryString(StorageKey.MostRecentActivity);
-
-      if (!rawData) {
-        this.storedData = MostRecentActivity.createEmptyData();
-      } else {
-        try {
-          this.storedData = JSON.parse(rawData);
-        } catch (e) {
-          console.error("Unable to parse stored most recent activity. Use empty data:", rawData);
-          this.storedData = MostRecentActivity.createEmptyData();
+    if (rawData) {
+      const oldData = JSON.parse(rawData);
+      if (oldData.schemaVersion === oldDataSchemaVersion) {
+        const itemsMap: Record<string, Item[]> = oldData.itemsMap;
+        Object.keys(itemsMap).forEach((accountId: string) => {
+          const accountName = accountId.split("/").pop();
+          if (accountName) {
+            saveState(
+              {
+                componentName: AppStateComponentNames.MostRecentActivity,
+                globalAccountName: accountName,
+              },
+              itemsMap[accountId].map((item) => {
+                if ((item.type as unknown as number) === 0) {
+                  item.type = Type.OpenCollection;
+                } else if ((item.type as unknown as number) === 1) {
+                  item.type = Type.OpenNotebook;
+                }
+                return item;
+              }),
+            );
+          }
+        });
+      }
     }
 
-        // If version doesn't match or schema broke, nuke it!
-        if (
-          !this.storedData.hasOwnProperty("schemaVersion") ||
-          this.storedData["schemaVersion"] !== MostRecentActivity.schemaVersion
-        ) {
-          LocalStorageUtility.removeEntry(StorageKey.MostRecentActivity);
-          this.storedData = MostRecentActivity.createEmptyData();
-        }
-      }
-    } else {
-      this.storedData = MostRecentActivity.createEmptyData();
-    }
-
-    for (let p in this.storedData.itemsMap) {
-      this.cleanupItems(p);
-    }
-    this.saveToLocalStorage();
-  }
-
-  private static createEmptyData(): StoredData {
-    return {
-      schemaVersion: MostRecentActivity.schemaVersion,
-      itemsMap: {},
-    };
-  }
-
-  private static isEmpty(object: any) {
-    return Object.keys(object).length === 0 && object.constructor === Object;
-  }
-
-  private saveToLocalStorage() {
-    if (MostRecentActivity.isEmpty(this.storedData.itemsMap)) {
-      if (LocalStorageUtility.hasItem(StorageKey.MostRecentActivity)) {
+    // Remove old data
     LocalStorageUtility.removeEntry(StorageKey.MostRecentActivity);
   }
-      // Don't save if empty
-      return;
-    }
+};
 
-    LocalStorageUtility.setEntryString(StorageKey.MostRecentActivity, JSON.stringify(this.storedData));
-  }
-
-  private addItem(accountId: string, newItem: Item): void {
+const addItem = (accountName: string, newItem: Item): void => {
   // When debugging, accountId is "undefined": most recent activity cannot be saved by account. Uncomment to disable.
   // if (!accountId) {
   //   return;
   // }
 
+  let items =
+    (loadState({
+      componentName: AppStateComponentNames.MostRecentActivity,
+      globalAccountName: accountName,
+    }) as Item[]) || [];
+
   // Remove duplicate
-    MostRecentActivity.removeDuplicate(newItem, this.storedData.itemsMap[accountId]);
+  items = removeDuplicate(newItem, items);
 
-    this.storedData.itemsMap[accountId] = this.storedData.itemsMap[accountId] || [];
-    this.storedData.itemsMap[accountId].unshift(newItem);
-    this.cleanupItems(accountId);
-    this.saveToLocalStorage();
+  items.unshift(newItem);
+  items = cleanupItems(items, accountName);
+  saveState(
+    {
+      componentName: AppStateComponentNames.MostRecentActivity,
+      globalAccountName: accountName,
+    },
+    items,
+  );
+};
+
+export const getItems = (accountName: string): Item[] => {
+  if (!accountName) {
+    return [];
   }
 
-  public getItems(accountId: string): Item[] {
-    return this.storedData.itemsMap[accountId] || [];
+  return (
+    (loadState({
+      componentName: AppStateComponentNames.MostRecentActivity,
+      globalAccountName: accountName,
+    }) as Item[]) || []
+  );
+};
+
+export const collectionWasOpened = (
+  accountName: string,
+  { id, databaseId }: Pick<CollectionBase, "id" | "databaseId">,
+) => {
+  if (accountName === undefined) {
+    return;
   }
 
-  public collectionWasOpened(accountId: string, { id, databaseId }: Pick<CollectionBase, "id" | "databaseId">) {
   const collectionId = id();
-    this.addItem(accountId, {
+  addItem(accountName, {
     type: Type.OpenCollection,
     databaseId,
     collectionId,
   });
-  }
+};
 
-  public notebookWasItemOpened(accountId: string, { name, path }: Pick<NotebookContentItem, "name" | "path">) {
-    this.addItem(accountId, {
-      type: Type.OpenNotebook,
-      name,
-      path,
-    });
-  }
-
-  public clear(accountId: string): void {
-    delete this.storedData.itemsMap[accountId];
-    this.saveToLocalStorage();
-  }
-
-  /**
-   * Find items by doing strict comparison and remove from array if duplicate is found
-   * @param item
-   */
-  private static removeDuplicate(item: Item, itemsArray: Item[]): void {
-    if (!itemsArray) {
+export const clear = (accountName: string): void => {
+  if (!accountName) {
     return;
   }
 
+  deleteState({
+    componentName: AppStateComponentNames.MostRecentActivity,
+    globalAccountName: accountName,
+  });
+};
+
+// Sort object by key
+const sortObjectKeys = (unordered: Record<string, unknown>): Record<string, unknown> => {
+  return Object.keys(unordered)
+    .sort()
+    .reduce((obj: Record<string, unknown>, key: string) => {
+      obj[key] = unordered[key];
+      return obj;
+    }, {});
+};
+
+/**
+ * Find items by doing strict comparison and remove from array if duplicate is found.
+ * Modifies the array.
+ * @param item
+ * @param itemsArray
+ * @returns new array
+ */
+const removeDuplicate = (item: Item, itemsArray: Item[]): Item[] => {
+  if (!itemsArray) {
+    return itemsArray;
+  }
+
+  const result: Item[] = [...itemsArray];
+
   let index = -1;
-    for (let i = 0; i < itemsArray.length; i++) {
-      const currentItem = itemsArray[i];
-      if (JSON.stringify(currentItem) === JSON.stringify(item)) {
+  for (let i = 0; i < result.length; i++) {
+    const currentItem = result[i];
+
+    if (
+      JSON.stringify(sortObjectKeys(currentItem as unknown as Record<string, unknown>)) ===
+      JSON.stringify(sortObjectKeys(item as unknown as Record<string, unknown>))
+    ) {
       index = i;
       break;
     }
   }
 
   if (index !== -1) {
-      itemsArray.splice(index, 1);
-    }
+    result.splice(index, 1);
   }
 
-  /**
+  return result;
+};
+
+/**
  * Remove unknown types
  * Limit items to max number
+ * Modifies the array.
  */
-  private cleanupItems(accountId: string): void {
-    if (!this.storedData.itemsMap.hasOwnProperty(accountId)) {
-      return;
+const cleanupItems = (items: Item[], accountName: string): Item[] => {
+  if (accountName === undefined) {
+    return [];
   }
 
-    const itemsArray = this.storedData.itemsMap[accountId]
-      .filter((item) => item.type in Type)
-      .slice(0, MostRecentActivity.itemsMaxNumber);
+  const itemsArray = items.filter((item) => item.type in Type).slice(0, itemsMaxNumber);
   if (itemsArray.length === 0) {
-      delete this.storedData.itemsMap[accountId];
-    } else {
-      this.storedData.itemsMap[accountId] = itemsArray;
+    deleteState({
+      componentName: AppStateComponentNames.MostRecentActivity,
+      globalAccountName: accountName,
+    });
   }
-  }
-}
+  return itemsArray;
+};
 
-export const mostRecentActivity = new MostRecentActivity();
+migrateOldData();

src/Explorer/SplashScreen/SplashScreen.tsx

@@ -114,7 +114,7 @@ export class SplashScreen extends React.Component<SplashScreenProps> {
   }
 
   private clearMostRecent = (): void => {
-    MostRecentActivity.mostRecentActivity.clear(userContext.databaseAccount?.id);
+    MostRecentActivity.clear(userContext.databaseAccount?.name);
     this.setState({});
   };
 
@@ -498,7 +498,7 @@ export class SplashScreen extends React.Component<SplashScreenProps> {
   }
 
   private createRecentItems(): SplashScreenItem[] {
-    return MostRecentActivity.mostRecentActivity.getItems(userContext.databaseAccount?.id).map((activity) => {
+    return MostRecentActivity.getItems(userContext.databaseAccount?.name).map((activity) => {
       switch (activity.type) {
         default: {
           const unknownActivity: never = activity;

src/Explorer/Tree/ResourceTreeAdapter.tsx

@@ -1,4 +1,5 @@
 import { TreeNodeMenuItem } from "Explorer/Controls/TreeComponent/TreeNodeComponent";
+import { collectionWasOpened } from "Explorer/MostRecentActivity/MostRecentActivity";
 import { shouldShowScriptNodes } from "Explorer/Tree/treeNodeUtil";
 import { getItemName } from "Utils/APITypeUtils";
 import * as ko from "knockout";
@@ -28,7 +29,6 @@ import { useDialog } from "../Controls/Dialog";
 import { LegacyTreeComponent, LegacyTreeNode } from "../Controls/TreeComponent/LegacyTreeComponent";
 import Explorer from "../Explorer";
 import { useCommandBar } from "../Menus/CommandBar/CommandBarComponentAdapter";
-import { mostRecentActivity } from "../MostRecentActivity/MostRecentActivity";
 import { NotebookContentItem, NotebookContentItemType } from "../Notebook/NotebookContentItem";
 import { NotebookUtil } from "../Notebook/NotebookUtil";
 import { useNotebook } from "../Notebook/useNotebook";
@@ -229,7 +229,7 @@ export class ResourceTreeAdapter implements ReactAdapter {
       onClick: () => {
         collection.openTab();
         // push to most recent
-        mostRecentActivity.collectionWasOpened(userContext.databaseAccount?.id, collection);
+        collectionWasOpened(userContext.databaseAccount?.name, collection);
       },
       isSelected: () =>
         useSelectedNode

src/Explorer/Tree/treeNodeUtil.tsx

@@ -1,5 +1,6 @@
 import { DatabaseRegular, DocumentMultipleRegular, SettingsRegular } from "@fluentui/react-icons";
 import { TreeNode } from "Explorer/Controls/TreeComponent/TreeNodeComponent";
+import { collectionWasOpened } from "Explorer/MostRecentActivity/MostRecentActivity";
 import TabsBase from "Explorer/Tabs/TabsBase";
 import StoredProcedure from "Explorer/Tree/StoredProcedure";
 import Trigger from "Explorer/Tree/Trigger";
@@ -17,7 +18,6 @@ import { userContext } from "../../UserContext";
 import * as ResourceTreeContextMenuButtonFactory from "../ContextMenuButtonFactory";
 import Explorer from "../Explorer";
 import { useCommandBar } from "../Menus/CommandBar/CommandBarComponentAdapter";
-import { mostRecentActivity } from "../MostRecentActivity/MostRecentActivity";
 import { useNotebook } from "../Notebook/useNotebook";
 import { useSelectedNode } from "../useSelectedNode";
 
@@ -98,7 +98,7 @@ export const createResourceTokenTreeNodes = (collection: ViewModels.CollectionBa
     onClick: () => {
       collection.onDocumentDBDocumentsClick();
       // push to most recent
-      mostRecentActivity.collectionWasOpened(userContext.databaseAccount?.id, collection);
+      collectionWasOpened(userContext.databaseAccount?.name, collection);
     },
     isSelected: () =>
       useSelectedNode
@@ -234,7 +234,7 @@ export const buildCollectionNode = (
       useSelectedNode.getState().setSelectedNode(collection);
       collection.openTab();
       // push to most recent
-      mostRecentActivity.collectionWasOpened(userContext.databaseAccount?.id, collection);
+      collectionWasOpened(userContext.databaseAccount?.name, collection);
     },
     onExpanded: async () => {
       // Rewritten version of expandCollapseCollection
@@ -282,7 +282,7 @@ const buildCollectionNodeChildren = (
     onClick: () => {
       collection.openTab();
       // push to most recent
-      mostRecentActivity.collectionWasOpened(userContext.databaseAccount?.id, collection);
+      collectionWasOpened(userContext.databaseAccount?.name, collection);
     },
     isSelected: () =>
       useSelectedNode

src/Shared/AppStatePersistenceUtility.ts

@@ -3,6 +3,7 @@ import { LocalStorageUtility, StorageKey } from "Shared/StorageUtility";
 // The component name whose state is being saved. Component name must not include special characters.
 export enum AppStateComponentNames {
   DocumentsTab = "DocumentsTab",
+  MostRecentActivity = "MostRecentActivity",
   QueryCopilot = "QueryCopilot",
 }
 
@@ -34,6 +35,7 @@ export const loadState = (path: StorePath): unknown => {
   const key = createKeyFromPath(path);
   return appState[key]?.data;
 };
+
 export const saveState = (path: StorePath, state: unknown): void => {
   // Retrieve state object
   const appState =
@@ -65,6 +67,10 @@ export const deleteState = (path: StorePath): void => {
   LocalStorageUtility.setEntryObject(StorageKey.AppState, appState);
 };
 
+export const hasState = (path: StorePath): boolean => {
+  return loadState(path) !== undefined;
+};
+
 // This is for high-frequency state changes
 let timeoutId: NodeJS.Timeout | undefined;
 export const saveStateDebounced = (path: StorePath, state: unknown, debounceDelayMs = 1000): void => {

src/Shared/StorageUtility.ts

@@ -24,7 +24,7 @@ export enum StorageKey {
   MaxDegreeOfParellism,
   IsGraphAutoVizDisabled,
   TenantId,
-  MostRecentActivity,
+  MostRecentActivity, // deprecated
   SetPartitionKeyUndefined,
   GalleryCalloutDismissed,
   VisitedAccounts,