This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-8464-93e2982a7405c7adf5b832d2c18a2d3a0b28dacf in repository https://gitbox.apache.org/repos/asf/texera.git
commit fbf6b2ca99fd2f75dadb642e0f0dbf9fa2715c7a Author: Xinyuan Lin <[email protected]> AuthorDate: Fri Sep 25 04:10:59 2026 +0000 chore(frontend): remove the unused DatasetVersionFileTreeManager (#8464) ### What changes were proposed in this PR? Deletes `DatasetVersionFileTreeManager`, which builds a client-side hash map of dataset file paths that nothing in production constructs. Pure deletion, no behaviour change: **−287 lines**. ### History | | | | --- | --- | | **Introduced by** | #2413 (2024-02-26) — "Introduce Dataset GUI" | | **Usage removed by** | #3296 (2025-03-10) — "Add FileService as a standalone microservice, LakeFS+S3 as dataset storage" deleted the real usages from `files-uploader.component.ts` (`previouslyUploadFilesManager`, `newUploadFileTreeManager`) once LakeFS began serving the file tree. The leftover dangling import was swept away by #3848 (2025-10-11, the `org.apache` rename) | Dead for about a year and a half. > Reviewer note: the rest of `datasetVersionFileTree.ts` stays — `DatasetFileNode` and `getFullPathFromDatasetFileNode` have live consumers, and the spec keeps its coverage of both. Only the class and its own `describe` block are removed. ### Any related issues, documentation, discussions? Closes #8462 ### How was this PR tested? Existing tests only — this PR removes a class and the spec block that covered it. From `frontend/`: - `npx ng test --watch=false --include='**/datasetVersionFileTree.spec.ts'` — 10 tests pass (the surviving `DatasetFileNode` / `getFullPathFromDatasetFileNode` coverage). - `yarn --cwd frontend format:ci` — clean. Verification, re-runnable by a reviewer: ``` git grep -n DatasetVersionFileTreeManager # only the deleted class and its tests git grep -n getFullPathFromDatasetFileNode # the live sibling, untouched ``` ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5) --- .../app/common/type/datasetVersionFileTree.spec.ts | 151 --------------------- .../src/app/common/type/datasetVersionFileTree.ts | 136 ------------------- 2 files changed, 287 deletions(-) diff --git a/frontend/src/app/common/type/datasetVersionFileTree.spec.ts b/frontend/src/app/common/type/datasetVersionFileTree.spec.ts index 207182e5fd..1ee6b94da1 100644 --- a/frontend/src/app/common/type/datasetVersionFileTree.spec.ts +++ b/frontend/src/app/common/type/datasetVersionFileTree.spec.ts @@ -18,7 +18,6 @@ import { DatasetFileNode, - DatasetVersionFileTreeManager, getFullPathFromDatasetFileNode, getPathsUnderOrEqualDatasetFileNode, getRelativePathFromDatasetFileNode, @@ -101,153 +100,3 @@ describe("getPathsUnderOrEqualDatasetFileNode", () => { expect(getPathsUnderOrEqualDatasetFileNode(noChildrenProp)).toEqual([]); }); }); - -describe("DatasetVersionFileTreeManager", () => { - describe("addNodeWithPath", () => { - it("starts with no root nodes", () => { - const manager = new DatasetVersionFileTreeManager(); - expect(manager.getRootNodes()).toEqual([]); - }); - - it("builds the intermediate directory structure and returns the leaf file node", () => { - const manager = new DatasetVersionFileTreeManager(); - const leaf = manager.addNodeWithPath("/a/b/c.txt"); - - expect(leaf.name).toBe("c.txt"); - expect(leaf.type).toBe("file"); - expect(getFullPathFromDatasetFileNode(leaf)).toBe("/a/b/c.txt"); - - const roots = manager.getRootNodes(); - expect(roots.length).toBe(1); - expect(roots[0].name).toBe("a"); - expect(roots[0].type).toBe("directory"); - - const dirB = roots[0].children![0]; - expect(dirB.name).toBe("b"); - expect(dirB.type).toBe("directory"); - expect(dirB.children![0]).toBe(leaf); - }); - - it("is idempotent when adding the same path twice", () => { - const manager = new DatasetVersionFileTreeManager(); - const first = manager.addNodeWithPath("/a/b/c.txt"); - const second = manager.addNodeWithPath("/a/b/c.txt"); - - expect(second).toBe(first); - expect(manager.getRootNodes().length).toBe(1); - const dirB = manager.getRootNodes()[0].children![0]; - expect(dirB.children!.length).toBe(1); - }); - - it("adds siblings under an existing directory", () => { - const manager = new DatasetVersionFileTreeManager(); - manager.addNodeWithPath("/a/b/c.txt"); - manager.addNodeWithPath("/a/b/d.txt"); - - const dirB = manager.getRootNodes()[0].children![0]; - expect(dirB.children!.map(child => child.name).sort()).toEqual(["c.txt", "d.txt"]); - }); - - it("handles paths without a leading slash", () => { - const manager = new DatasetVersionFileTreeManager(); - const leaf = manager.addNodeWithPath("x/y.txt"); - expect(getFullPathFromDatasetFileNode(leaf)).toBe("/x/y.txt"); - expect(manager.getRootNodes()[0].name).toBe("x"); - }); - }); - - describe("initializeWithRootNodes / constructor", () => { - it("exposes provided root nodes", () => { - const dir: DatasetFileNode = { - name: "dir", - type: "directory", - parentDir: "/", - children: [{ name: "f.txt", type: "file", parentDir: "/dir" }], - }; - const manager = new DatasetVersionFileTreeManager([dir]); - expect(manager.getRootNodes()).toEqual([dir]); - }); - }); - - describe("removeNode", () => { - it("removes a leaf node found by identity via BFS", () => { - const manager = new DatasetVersionFileTreeManager(); - const leaf = manager.addNodeWithPath("/a/b/c.txt"); - const dirB = manager.getRootNodes()[0].children![0]; - - manager.removeNode(leaf); - expect(dirB.children).toEqual([]); - }); - - it("removes a whole subtree when removing an inner directory", () => { - const manager = new DatasetVersionFileTreeManager(); - manager.addNodeWithPath("/a/b/c.txt"); - const rootA = manager.getRootNodes()[0]; - - manager.removeNode(rootA); - expect(manager.getRootNodes()).toEqual([]); - }); - - it("does nothing for a node that is not present in the tree", () => { - const manager = new DatasetVersionFileTreeManager(); - manager.addNodeWithPath("/a/b/c.txt"); - const stranger: DatasetFileNode = { name: "z.txt", type: "file", parentDir: "/q" }; - - manager.removeNode(stranger); - expect(manager.getRootNodes().length).toBe(1); - expect(manager.getRootNodes()[0].children![0].children!.length).toBe(1); - }); - - it("refuses to remove the synthetic root node", () => { - const manager = new DatasetVersionFileTreeManager(); - manager.addNodeWithPath("/a/b/c.txt"); - const fakeRoot: DatasetFileNode = { name: "/", type: "directory", parentDir: "" }; - - manager.removeNode(fakeRoot); - expect(manager.getRootNodes().length).toBe(1); - }); - }); - - describe("removeNodeWithPath", () => { - it("removes a node from its parent's children and the internal map", () => { - const file1: DatasetFileNode = { name: "file1.txt", type: "file", parentDir: "/dir" }; - const file2: DatasetFileNode = { name: "file2.txt", type: "file", parentDir: "/dir" }; - const dir: DatasetFileNode = { name: "dir", type: "directory", parentDir: "/", children: [file1, file2] }; - const manager = new DatasetVersionFileTreeManager([dir]); - - manager.removeNodeWithPath("/dir/file1.txt"); - expect(dir.children!.map(child => child.name)).toEqual(["file2.txt"]); - - // A second removal of the same (now absent) path is a no-op. - manager.removeNodeWithPath("/dir/file1.txt"); - expect(dir.children!.map(child => child.name)).toEqual(["file2.txt"]); - }); - - it("removes a whole subtree when removing a directory path", () => { - const file: DatasetFileNode = { name: "f.txt", type: "file", parentDir: "/dir/sub" }; - const subDir: DatasetFileNode = { name: "sub", type: "directory", parentDir: "/dir", children: [file] }; - const dir: DatasetFileNode = { name: "dir", type: "directory", parentDir: "/", children: [subDir] }; - const manager = new DatasetVersionFileTreeManager([dir]); - - manager.removeNodeWithPath("/dir/sub"); - expect(dir.children).toEqual([]); - - // Removing a descendant path after the subtree is gone is also a no-op. - manager.removeNodeWithPath("/dir/sub/f.txt"); - expect(dir.children).toEqual([]); - }); - - it("does nothing for an unknown path", () => { - const dir: DatasetFileNode = { - name: "dir", - type: "directory", - parentDir: "/", - children: [{ name: "f.txt", type: "file", parentDir: "/dir" }], - }; - const manager = new DatasetVersionFileTreeManager([dir]); - - manager.removeNodeWithPath("/does/not/exist"); - expect(dir.children!.length).toBe(1); - }); - }); -}); diff --git a/frontend/src/app/common/type/datasetVersionFileTree.ts b/frontend/src/app/common/type/datasetVersionFileTree.ts index efbb875128..91160b38dd 100644 --- a/frontend/src/app/common/type/datasetVersionFileTree.ts +++ b/frontend/src/app/common/type/datasetVersionFileTree.ts @@ -61,139 +61,3 @@ export function getPathsUnderOrEqualDatasetFileNode(node: DatasetFileNode): stri return gatherPaths(node); } - -// This class convert a list of DatasetVersionTreeNode into a hash map, recursively containing all the paths -export class DatasetVersionFileTreeManager { - private root: DatasetFileNode = { name: "/", type: "directory", children: [], parentDir: "" }; - private treeNodesMap: Map<string, DatasetFileNode> = new Map<string, DatasetFileNode>(); - - constructor(nodes: DatasetFileNode[] = []) { - this.treeNodesMap.set("/", this.root); - if (nodes.length > 0) this.initializeWithRootNodes(nodes); - } - - private updateTreeMapWithPath(path: string): DatasetFileNode { - const pathParts = path.startsWith("/") ? path.slice(1).split("/") : path.split("/"); - let currentPath = "/"; - let currentNode = this.root; - - pathParts.forEach((part, index) => { - const previousPath = currentPath; - currentPath += part + (index < pathParts.length - 1 ? "/" : ""); // Don't add trailing slash for last part - - if (!this.treeNodesMap.has(currentPath)) { - const isLastPart = index === pathParts.length - 1; - const newNode: DatasetFileNode = { - name: part, - type: isLastPart ? "file" : "directory", - parentDir: previousPath.endsWith("/") ? previousPath.slice(0, -1) : previousPath, // Store the full path for parentDir - ...(isLastPart ? {} : { children: [] }), // Only add 'children' for directories - }; - this.treeNodesMap.set(currentPath, newNode); - currentNode.children = currentNode.children ?? []; // Ensure 'children' is initialized - currentNode.children.push(newNode); - } - currentNode = this.treeNodesMap.get(currentPath)!; // Get the node for the next iteration - }); - - return currentNode; - } - - private removeNodeAndDescendants(node: DatasetFileNode): void { - if (node.type === "directory" && node.children) { - node.children.forEach(child => { - const childPath = - node.parentDir === "/" ? `/${node.name}/${child.name}` : `${node.parentDir}/${node.name}/${child.name}`; - this.removeNodeAndDescendants(child); - this.treeNodesMap.delete(childPath); // Remove the child from the map - }); - } - // Now that all children are removed, clear the current node's children array - node.children = []; - } - - addNodeWithPath(path: string): DatasetFileNode { - return this.updateTreeMapWithPath(path); - } - - initializeWithRootNodes(rootNodes: DatasetFileNode[]) { - // Clear existing nodes in map except the root - this.treeNodesMap.clear(); - this.treeNodesMap.set("/", this.root); - - // Helper function to add nodes recursively - const addNodeRecursively = (node: DatasetFileNode, parentDir: string) => { - const nodePath = parentDir === "/" ? `/${node.name}` : `${parentDir}/${node.name}`; - this.treeNodesMap.set(nodePath, node); - - // If the node is a directory, recursively add its children - if (node.type === "directory" && node.children) { - node.children.forEach(child => addNodeRecursively(child, nodePath)); - } - }; - - // Add each root node and their children to the tree and map - rootNodes.forEach(node => { - if (!this.root.children) { - this.root.children = []; - } - this.root.children.push(node); - addNodeRecursively(node, "/"); - }); - } - - removeNode(targetNode: DatasetFileNode): void { - if (targetNode.parentDir === "" && targetNode.name === "/") { - // Can't remove root - return; - } - - // Queue for BFS - const queue: DatasetFileNode[] = [this.root]; - - while (queue.length > 0) { - const node = queue.shift()!; - - // Check if the current node is the parent of the target node - if (node.children && node.children.some(child => child === targetNode)) { - // Remove the target node and its descendants - this.removeNodeAndDescendants(targetNode); - - // Remove the target node from the current node's children - node.children = node.children.filter(child => child !== targetNode); - - // Construct the full path of the target node to remove it from the map - const pathToRemove = getFullPathFromDatasetFileNode(targetNode); - this.treeNodesMap.delete(pathToRemove); - - return; // Node found and removed, exit the function - } - - // If not found, add the children of the current node to the queue - if (node.children) { - queue.push(...node.children); - } - } - } - - removeNodeWithPath(path: string): void { - const nodeToRemove = this.treeNodesMap.get(path); - if (nodeToRemove) { - // First, recursively remove all descendants of the node - this.removeNodeAndDescendants(nodeToRemove); - - // Then, remove the node from its parent's children array - const parentNode = this.treeNodesMap.get(nodeToRemove.parentDir); - if (parentNode && parentNode.children) { - parentNode.children = parentNode.children.filter(child => child.name !== nodeToRemove.name); - } - - // Finally, remove the node from the map - this.treeNodesMap.delete(path); - } - } - - getRootNodes(): DatasetFileNode[] { - return this.root.children ?? []; - } -}
