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-7038-f28d8ec35c6fc5828a46157ffcec8c696e0060ae in repository https://gitbox.apache.org/repos/asf/texera.git
commit fe9c4900234f535ae80fb002138f8c9386e68bb8 Author: Xinyuan Lin <[email protected]> AuthorDate: Wed Jul 29 19:36:07 2026 -0700 refactor(frontend): remove dead getPrevStats and the redundant search-cache index (#7038) ### What changes were proposed in this PR? Removes two pieces of frontend state that are maintained but not needed. No behaviour change. **1. `OperatorPaginationResultService.getPrevStats` is dead API.** `getPrevStats()` has no caller in `src/` — not from TypeScript, not from a template. Its backing field `prevStatsCache` exists only to feed it, so the whole rotate-into-the-previous-slot dance in `handleStatsUpdate` serves a reader that does not exist. The `if (!this.statsCache)` branch is also unreachable: `statsCache` is initialised to `{}` and only ever reassigned to another object. ```ts // before public handleStatsUpdate(statsUpdate): void { if (!this.statsCache) { this.statsCache = statsUpdate; this.prevStatsCache = statsUpdate; } else { this.prevStatsCache = this.statsCache; this.statsCache = statsUpdate; } } // after public handleStatsUpdate(statsUpdate): void { this.statsCache = statsUpdate; } ``` This does **not** touch `WorkflowResultService.getResultTableStats()`, which is the mechanism actually used to deliver previous/current stats pairs to the UI (via rxjs `pairwise()`). **2. `SearchBarComponent.queryOrder` duplicates `Map` insertion order.** The search cache kept a parallel `string[]` purely to know which key to evict next. A JavaScript `Map` already iterates in insertion order, and `addToCache` is only reached on a cache **miss** (a hit returns early at `getSearchResults`), so the array can never diverge from the Map's own key order — the oldest key is always `searchCache.keys().next().value`. ```ts // before if (this.queryOrder.length >= 20) { const oldestQuery = this.queryOrder.shift(); this.searchCache.delete(oldestQuery!); } this.queryOrder.push(query); this.searchCache.set(query, results); // after if (this.searchCache.size >= 20) { const oldestQuery = this.searchCache.keys().next().value; this.searchCache.delete(oldestQuery!); } this.searchCache.set(query, results); ``` Both existing tests were kept and re-pointed at the surviving API rather than dropped, so eviction order and stats replacement are still pinned. −23 lines, +13. ### Any related issues, documentation, discussions? Closes #7037 ### How was this PR tested? Existing tests, updated in place to assert the same behaviour through the surviving API: - `search-bar.component.spec.ts` — the eviction test now reads `[...cache.keys()]` instead of the removed `queryOrder` array. Same assertions: `q0` evicted, `q1` oldest, `q20` newest, size stays 20. - `workflow-result.service.spec.ts` — `handleStatsUpdate` is now asserted to replace the current stats (it no longer has a previous slot to rotate into). Locally: - `npx tsc --noEmit -p frontend/tsconfig.json` — exit 0. - `ng test --watch=false --include='**/search-bar.component.spec.ts' --include='**/workflow-result.service.spec.ts'` — 39 tests, all pass. - `prettier-eslint --list-different "src/**/*.{ts,js,html,scss,less,json}"` — no differences. Verification that nothing references the removed members: ``` grep -rn "prevStatsCache\|getPrevStats\|queryOrder" frontend/src ``` ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5) --- .../component/user/search-bar/search-bar.component.spec.ts | 9 +++++---- .../component/user/search-bar/search-bar.component.ts | 8 ++++---- .../service/workflow-result/workflow-result.service.spec.ts | 6 +++--- .../service/workflow-result/workflow-result.service.ts | 13 +------------ 4 files changed, 13 insertions(+), 23 deletions(-) diff --git a/frontend/src/app/dashboard/component/user/search-bar/search-bar.component.spec.ts b/frontend/src/app/dashboard/component/user/search-bar/search-bar.component.spec.ts index 2540e89d2b..71b7573cf5 100644 --- a/frontend/src/app/dashboard/component/user/search-bar/search-bar.component.spec.ts +++ b/frontend/src/app/dashboard/component/user/search-bar/search-bar.component.spec.ts @@ -149,22 +149,23 @@ describe("SearchBarComponent", () => { it("addToCache evicts the oldest entry once 20 queries are cached", () => { const cache = (component as any).searchCache as Map<string, string[]>; - const order = (component as any).queryOrder as string[]; for (let i = 0; i < 20; i++) { (component as any).addToCache(`q${i}`, [`r${i}`]); } expect(cache.size).toBe(20); expect(cache.has("q0")).toBe(true); - expect(order[0]).toBe("q0"); + expect([...cache.keys()][0]).toBe("q0"); (component as any).addToCache("q20", ["r20"]); expect(cache.size).toBe(20); expect(cache.has("q0")).toBe(false); expect(cache.has("q20")).toBe(true); - expect(order[0]).toBe("q1"); - expect(order[order.length - 1]).toBe("q20"); + // The Map's own iteration order is the eviction order. + const keys = [...cache.keys()]; + expect(keys[0]).toBe("q1"); + expect(keys[keys.length - 1]).toBe("q20"); }); describe("convertToName", () => { diff --git a/frontend/src/app/dashboard/component/user/search-bar/search-bar.component.ts b/frontend/src/app/dashboard/component/user/search-bar/search-bar.component.ts index 638a2f7e7b..83cabf089a 100644 --- a/frontend/src/app/dashboard/component/user/search-bar/search-bar.component.ts +++ b/frontend/src/app/dashboard/component/user/search-bar/search-bar.component.ts @@ -69,7 +69,6 @@ export class SearchBarComponent { }; private searchCache = new Map<string, string[]>(); - private queryOrder: string[] = []; constructor( private router: Router, @@ -119,12 +118,13 @@ export class SearchBarComponent { } } + // A Map iterates in insertion order, and addToCache is only reached on a cache + // miss, so the oldest key is always the first one. private addToCache(query: string, results: string[]): void { - if (this.queryOrder.length >= 20) { - const oldestQuery = this.queryOrder.shift(); + if (this.searchCache.size >= 20) { + const oldestQuery = this.searchCache.keys().next().value; this.searchCache.delete(oldestQuery!); } - this.queryOrder.push(query); this.searchCache.set(query, results); } diff --git a/frontend/src/app/workspace/service/workflow-result/workflow-result.service.spec.ts b/frontend/src/app/workspace/service/workflow-result/workflow-result.service.spec.ts index 8ed9d41b4a..e73f28b39a 100644 --- a/frontend/src/app/workspace/service/workflow-result/workflow-result.service.spec.ts +++ b/frontend/src/app/workspace/service/workflow-result/workflow-result.service.spec.ts @@ -421,17 +421,17 @@ describe("OperatorPaginationResultService", () => { }); describe("handleStatsUpdate", () => { - it("rotates the current stats into the previous slot on each update", () => { + it("replaces the current stats on each update", () => { const first = { colA: { count: 1 } }; const second = { colA: { count: 2 } }; + expect(service.getStats()).toEqual({}); + service.handleStatsUpdate(first); expect(service.getStats()).toEqual(first); - expect(service.getPrevStats()).toEqual({}); service.handleStatsUpdate(second); expect(service.getStats()).toEqual(second); - expect(service.getPrevStats()).toEqual(first); }); }); diff --git a/frontend/src/app/workspace/service/workflow-result/workflow-result.service.ts b/frontend/src/app/workspace/service/workflow-result/workflow-result.service.ts index ffd9b43b91..26133a636e 100644 --- a/frontend/src/app/workspace/service/workflow-result/workflow-result.service.ts +++ b/frontend/src/app/workspace/service/workflow-result/workflow-result.service.ts @@ -273,7 +273,6 @@ export class OperatorResultService { export class OperatorPaginationResultService { private pendingRequests: Map<string, Subject<PaginatedResultEvent>> = new Map(); private resultCache: Map<number, ReadonlyArray<object>> = new Map(); - private prevStatsCache: Record<string, Record<string, number>> = {}; private statsCache: Record<string, Record<string, number>> = {}; private currentPageIndex: number = 1; private currentTotalNumTuples: number = 0; @@ -293,10 +292,6 @@ export class OperatorPaginationResultService { return this.statsCache; } - public getPrevStats(): Record<string, Record<string, number>> { - return this.prevStatsCache; - } - public getCurrentPageIndex(): number { return this.currentPageIndex; } @@ -378,13 +373,7 @@ export class OperatorPaginationResultService { } public handleStatsUpdate(statsUpdate: Record<string, Record<string, number>>): void { - if (!this.statsCache) { - this.statsCache = statsUpdate; - this.prevStatsCache = statsUpdate; - } else { - this.prevStatsCache = this.statsCache; - this.statsCache = statsUpdate; - } + this.statsCache = statsUpdate; } private handlePaginationResult(res: PaginatedResultEvent): void {
