[PM-19127] - Nested Traverse Optimization (#14881)

* Draft optimization of getNestedCollectionTree

* Added feature flag to wrap nestedTraverse_vNext. added the old implementation back in for feature flagging.

* Correction from CR

* Copied tests over for the vNext method.

---------

Co-authored-by: Thomas Rittson <trittson@bitwarden.com>

libs/common/src/enums/feature-flag.enum.ts

@@ -13,6 +13,7 @@ export enum FeatureFlag {
   /* Admin Console Team */
   LimitItemDeletion = "pm-15493-restrict-item-deletion-to-can-manage-permission",
   SeparateCustomRolePermissions = "pm-19917-separate-custom-role-permissions",
+  OptimizeNestedTraverseTypescript = "pm-21695-optimize-nested-traverse-typescript",
 
   /* Auth */
   PM16117_ChangeExistingPasswordRefactor = "pm-16117-change-existing-password-refactor",
@@ -82,6 +83,7 @@ export const DefaultFeatureFlagValue = {
   /* Admin Console Team */
   [FeatureFlag.LimitItemDeletion]: FALSE,
   [FeatureFlag.SeparateCustomRolePermissions]: FALSE,
+  [FeatureFlag.OptimizeNestedTraverseTypescript]: FALSE,
 
   /* Autofill */
   [FeatureFlag.BlockBrowserInjectionsByDomain]: FALSE,

libs/common/src/vault/service-utils.spec.ts

@@ -36,6 +36,24 @@ describe("serviceUtils", () => {
     });
   });
 
+  describe("nestedTraverse_vNext", () => {
+    it("should traverse a tree and add a node at the correct position given a valid path", () => {
+      const nodeToBeAdded: FakeObject = { id: "1.2.1", name: "1.2.1" };
+      const path = ["1", "1.2", "1.2.1"];
+
+      ServiceUtils.nestedTraverse_vNext(nodeTree, 0, path, nodeToBeAdded, null, "/");
+      expect(nodeTree[0].children[1].children[0].node).toEqual(nodeToBeAdded);
+    });
+
+    it("should combine the path for missing nodes and use as the added node name given an invalid path", () => {
+      const nodeToBeAdded: FakeObject = { id: "blank", name: "blank" };
+      const path = ["3", "3.1", "3.1.1"];
+
+      ServiceUtils.nestedTraverse_vNext(nodeTree, 0, path, nodeToBeAdded, null, "/");
+      expect(nodeTree[2].children[0].node.name).toEqual("3.1/3.1.1");
+    });
+  });
+
   describe("getTreeNodeObject", () => {
     it("should return a matching node given a single tree branch and a valid id", () => {
       const id = "1.1.1";

libs/common/src/vault/service-utils.ts

@@ -3,15 +3,6 @@
 import { ITreeNodeObject, TreeNode } from "./models/domain/tree-node";
 
 export class ServiceUtils {
-  /**
-   * Recursively adds a node to nodeTree
-   * @param {TreeNode<ITreeNodeObject>[]} nodeTree - An array of TreeNodes that the node will be added to
-   * @param {number} partIndex - Index of the `parts` array that is being processed
-   * @param {string[]} parts - Array of strings that represent the path to the `obj` node
-   * @param {ITreeNodeObject} obj - The node to be added to the tree
-   * @param {ITreeNodeObject} parent - The parent node of the `obj` node
-   * @param {string} delimiter - The delimiter used to split the path string, will be used to combine the path for missing nodes
-   */
   static nestedTraverse(
     nodeTree: TreeNode<ITreeNodeObject>[],
     partIndex: number,
@@ -70,11 +61,75 @@ export class ServiceUtils {
     }
   }
 
+  /**
+   * Recursively adds a node to nodeTree
+   * @param {TreeNode<ITreeNodeObject>[]} nodeTree - An array of TreeNodes that the node will be added to
+   * @param {number} partIndex - Index of the `parts` array that is being processed
+   * @param {string[]} parts - Array of strings that represent the path to the `obj` node
+   * @param {ITreeNodeObject} obj - The node to be added to the tree
+   * @param {ITreeNodeObject} parent - The parent node of the `obj` node
+   * @param {string} delimiter - The delimiter used to split the path string, will be used to combine the path for missing nodes
+   */
+  static nestedTraverse_vNext(
+    nodeTree: TreeNode<ITreeNodeObject>[],
+    partIndex: number,
+    parts: string[],
+    obj: ITreeNodeObject,
+    parent: TreeNode<ITreeNodeObject> | undefined,
+    delimiter: string,
+  ) {
+    if (parts.length <= partIndex) {
+      return;
+    }
+
+    // 'end' indicates we've traversed as far as we can based on the object name
+    const end: boolean = partIndex === parts.length - 1;
+    const partName: string = parts[partIndex];
+
+    // If we're at the end, just add the node - it doesn't matter what else is here
+    if (end) {
+      nodeTree.push(new TreeNode(obj, parent, partName));
+      return;
+    }
+
+    // Get matching nodes at this level by name
+    // NOTE: this is effectively a loop so we only want to do it once
+    const matchingNodes = nodeTree.filter((n) => n.node.name === partName);
+
+    // If there are no matching nodes...
+    if (matchingNodes.length === 0) {
+      // And we're not at the end of the path (because we didn't trigger the early return above),
+      // combine the current name with the next name.
+      // 1, *1.2, 1.2.1 becomes
+      // 1, *1.2/1.2.1
+      const newPartName = partName + delimiter + parts[partIndex + 1];
+      ServiceUtils.nestedTraverse_vNext(
+        nodeTree,
+        0,
+        [newPartName, ...parts.slice(partIndex + 2)],
+        obj,
+        parent,
+        delimiter,
+      );
+    } else {
+      // There is a node here with the same name, descend into it
+      ServiceUtils.nestedTraverse_vNext(
+        matchingNodes[0].children,
+        partIndex + 1,
+        parts,
+        obj,
+        matchingNodes[0],
+        delimiter,
+      );
+      return;
+    }
+  }
+
   /**
    * Searches a tree for a node with a matching `id`
-   * @param {TreeNode<T>} nodeTree - A single TreeNode branch that will be searched
+   * @param {TreeNode<T extends ITreeNodeObject>} nodeTree - A single TreeNode branch that will be searched
    * @param {string} id - The id of the node to be found
-   * @returns {TreeNode<T>} The node with a matching `id`
+   * @returns {TreeNode<T extends ITreeNodeObject>} The node with a matching `id`
    */
   static getTreeNodeObject<T extends ITreeNodeObject>(
     nodeTree: TreeNode<T>,
@@ -96,9 +151,9 @@ export class ServiceUtils {
 
   /**
    * Searches an array of tree nodes for a node with a matching `id`
-   * @param {TreeNode<T>} nodeTree - An array of TreeNode branches that will be searched
+   * @param {TreeNode<T extends ITreeNodeObject>} nodeTree - An array of TreeNode branches that will be searched
    * @param {string} id - The id of the node to be found
-   * @returns {TreeNode<T>} The node with a matching `id`
+   * @returns {TreeNode<T extends ITreeNodeObject>} The node with a matching `id`
    */
   static getTreeNodeObjectFromList<T extends ITreeNodeObject>(
     nodeTree: TreeNode<T>[],