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-8695-facdc2186d03f7efa87f02431e8a3b749830f94c in repository https://gitbox.apache.org/repos/asf/texera.git
commit fcaf27a561bc0d29bad052b191062019f75eafa1 Author: Xinyuan Lin <[email protected]> AuthorDate: Sun Sep 27 22:06:55 2026 +0000 refactor(frontend): remove the unused ObservableContextManager (#8695) ### What changes were proposed in this PR? Deletes `ObservableContextManager` and re-parents its only subclass, `JointGraphContext`, onto `ContextManager`. **+8/−223 lines.** There is no behaviour change: the removed layer only emitted enter/exit events on Subjects nothing subscribes to, and `JointGraphContext`'s own `enter` / `exit` overrides still call `super`. ### History | | | | --- | --- | | **Introduced by** | #1433 (2022-02-07), "Bugfix for Albert load workflow opt". `JointGraphContext` used the change stream to buffer JointJS events in async mode | | **Usage removed by** | #1810 (2023-01-27), "Fix buffering events in JointJS async context". It deleted both `getChangeContextStream()` subscribers | It has been dead for over three and a half years. Every paper-context enter/exit since then has emitted into streams with no reader. > Reviewer note: `context.spec.ts` goes from 23 cases to 12. The 11 stream-only cases are deleted. 4 others move into `describe("ContextManager")` with their stream assertions dropped, because they cover `ContextManager` paths the original 8 cases did not: object-identity contexts, nested unwinding when a callable throws, falsy return values, and re-entering the same value. ### Any related issues, documentation, discussions? Closes #8694 ### How was this PR tested? No new tests. The kept `ContextManager` cases cover the surviving code. From `frontend/`: - `npx ng test --watch=false --include='**/context.spec.ts' --include='**/joint-graph-wrapper.spec.ts' --include='**/workflow-editor.component.spec.ts'`: 3 files, 199 tests, all pass. - `yarn format:ci`: clean. - `npx ng build`: success. To re-check: ``` git grep -n "ObservableContextManager\|getChangeContextStream\|getEnterStream\|getExitStream" -- frontend # no hits ``` ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5.5) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- frontend/src/app/common/util/context.spec.ts | 186 +-------------------- frontend/src/app/common/util/context.ts | 41 ----- .../workflow-graph/model/joint-graph-wrapper.ts | 4 +- 3 files changed, 8 insertions(+), 223 deletions(-) diff --git a/frontend/src/app/common/util/context.spec.ts b/frontend/src/app/common/util/context.spec.ts index 7fd6a815ae..37511b77fd 100644 --- a/frontend/src/app/common/util/context.spec.ts +++ b/frontend/src/app/common/util/context.spec.ts @@ -17,7 +17,7 @@ * under the License. */ -import { ContextManager, ObservableContextManager } from "./context"; +import { ContextManager } from "./context"; describe("ContextManager", () => { it("should return the default context initially", () => { @@ -106,128 +106,15 @@ describe("ContextManager", () => { expect(managerB.getContext()).toBe("defaultB"); }); }); -}); - -describe("ObservableContextManager", () => { - it("should emit [exiting, entering] on the enter stream when a context is entered", () => { - const manager = ObservableContextManager<string>("default"); - const enterEvents: [string, string][] = []; - manager.getEnterStream().subscribe(event => enterEvents.push(event)); - - manager.withContext("inner", () => {}); - - expect(enterEvents).toEqual([["default", "inner"]]); - }); - - it("should emit [exiting, entering] on the exit stream when a context is exited", () => { - const manager = ObservableContextManager<string>("default"); - const exitEvents: [string, string][] = []; - manager.getExitStream().subscribe(event => exitEvents.push(event)); - - manager.withContext("inner", () => {}); - - expect(exitEvents).toEqual([["inner", "default"]]); - }); - - it("should emit the enter event after the context stack is updated", () => { - const manager = ObservableContextManager<string>("default"); - const contextsAtEmission: string[] = []; - manager.getEnterStream().subscribe(() => contextsAtEmission.push(manager.getContext())); - - manager.withContext("inner", () => {}); - - // the new context is already on the stack when the enter event fires - expect(contextsAtEmission).toEqual(["inner"]); - }); - - it("should emit the exit event after the context stack is popped", () => { - const manager = ObservableContextManager<string>("default"); - const contextsAtEmission: string[] = []; - manager.getExitStream().subscribe(() => contextsAtEmission.push(manager.getContext())); - - manager.withContext("inner", () => {}); - - // the exited context is already off the stack when the exit event fires - expect(contextsAtEmission).toEqual(["default"]); - }); - - it("should deliver both enter and exit events on the change context stream", () => { - const manager = ObservableContextManager<string>("default"); - const changeEvents: [string, string][] = []; - manager.getChangeContextStream().subscribe(event => changeEvents.push(event)); - - manager.withContext("inner", () => {}); - - expect(changeEvents).toEqual([ - ["default", "inner"], - ["inner", "default"], - ]); - }); - - it("should emit events in the correct order across nested withContext calls", () => { - const manager = ObservableContextManager<string>("default"); - const events: { type: string; event: [string, string] }[] = []; - manager.getEnterStream().subscribe(event => events.push({ type: "enter", event })); - manager.getExitStream().subscribe(event => events.push({ type: "exit", event })); - - manager.withContext("outer", () => { - manager.withContext("inner", () => {}); - }); - - expect(events).toEqual([ - { type: "enter", event: ["default", "outer"] }, - { type: "enter", event: ["outer", "inner"] }, - { type: "exit", event: ["inner", "outer"] }, - { type: "exit", event: ["outer", "default"] }, - ]); - }); - - it("should emit enter and exit events when the callable throws", () => { - const manager = ObservableContextManager<string>("default"); - const changeEvents: [string, string][] = []; - manager.getChangeContextStream().subscribe(event => changeEvents.push(event)); - - expect(() => - manager.withContext("inner", () => { - throw new Error("callable failure"); - }) - ).toThrowError("callable failure"); - - // the exit event still fires because withContext exits in a finally block - expect(changeEvents).toEqual([ - ["default", "inner"], - ["inner", "default"], - ]); - - expect(manager.getContext()).toBe("default"); - }); - - it("should still provide the basic ContextManager behavior", () => { - const manager = ObservableContextManager<string>("default"); - const result = manager.withContext("inner", () => { - expect(manager.getContext()).toBe("inner"); - expect(manager.prevContext()).toBe("default"); - return "value"; - }); - - expect(result).toBe("value"); - expect(manager.getContext()).toBe("default"); - }); - - it("should preserve object references through the stack and in emitted tuples", () => { + it("should preserve object references through the stack", () => { // mirrors the real JointGraphContextType usage where the context is an object interface ObjectContext { readonly name: string; } const defaultContext: ObjectContext = { name: "default" }; const innerContext: ObjectContext = { name: "inner" }; - const manager = ObservableContextManager<ObjectContext>(defaultContext); - - const enterEvents: [ObjectContext, ObjectContext][] = []; - const exitEvents: [ObjectContext, ObjectContext][] = []; - manager.getEnterStream().subscribe(event => enterEvents.push(event)); - manager.getExitStream().subscribe(event => exitEvents.push(event)); + const manager = ContextManager<ObjectContext>(defaultContext); // the same reference is returned for the default context expect(manager.getContext()).toBe(defaultContext); @@ -240,18 +127,10 @@ describe("ObservableContextManager", () => { // the default reference is restored after exit expect(manager.getContext()).toBe(defaultContext); - - // emitted tuples carry the original object references - expect(enterEvents[0][0]).toBe(defaultContext); - expect(enterEvents[0][1]).toBe(innerContext); - expect(exitEvents[0][0]).toBe(innerContext); - expect(exitEvents[0][1]).toBe(defaultContext); }); it("should unwind both levels via finally blocks when a nested callable throws", () => { - const manager = ObservableContextManager<string>("default"); - const changeEvents: [string, string][] = []; - manager.getChangeContextStream().subscribe(event => changeEvents.push(event)); + const manager = ContextManager<string>("default"); expect(() => manager.withContext("outer", () => @@ -263,73 +142,20 @@ describe("ObservableContextManager", () => { // the error propagated all the way out and the stack is fully restored expect(manager.getContext()).toBe("default"); - - // both levels' exit events fire during unwinding via their finally blocks - expect(changeEvents).toEqual([ - ["default", "outer"], - ["outer", "inner"], - ["inner", "outer"], - ["outer", "default"], - ]); - }); - - it("should not replay earlier events to a late subscriber", () => { - const manager = ObservableContextManager<string>("default"); - - // trigger a full enter/exit cycle before anyone subscribes - manager.withContext("inner", () => {}); - - const enterEvents: [string, string][] = []; - manager.getEnterStream().subscribe(event => enterEvents.push(event)); - - // the streams are plain Subjects (no replay), so the late subscriber sees nothing - expect(enterEvents).toEqual([]); - }); - - it("should allow prevContext() to be called inside the enter subscriber", () => { - const manager = ObservableContextManager<string>("default"); - const prevContexts: string[] = []; - - // at enter-emission time the new context is already pushed, so prevContext is valid - manager.getEnterStream().subscribe(() => prevContexts.push(manager.prevContext())); - - manager.withContext("inner", () => {}); - - expect(prevContexts).toEqual(["default"]); - }); - - it("should emit correctly for sequential sibling withContext calls", () => { - const manager = ObservableContextManager<string>("default"); - const changeEvents: [string, string][] = []; - manager.getChangeContextStream().subscribe(event => changeEvents.push(event)); - - manager.withContext("A", () => {}); - manager.withContext("B", () => {}); - - expect(changeEvents).toEqual([ - ["default", "A"], - ["A", "default"], - ["default", "B"], - ["B", "default"], - ]); }); it("should handle entering the same context value as the current one", () => { - const manager = ObservableContextManager<string>("default"); - const enterEvents: [string, string][] = []; - manager.getEnterStream().subscribe(event => enterEvents.push(event)); + const manager = ContextManager<string>("default"); manager.withContext("default", () => { // dedup is positional (stack depth), not value-based expect(manager.getContext()).toBe("default"); expect(manager.prevContext()).toBe("default"); }); - - expect(enterEvents).toEqual([["default", "default"]]); }); it("should return falsy values produced by the callable unchanged", () => { - const manager = ObservableContextManager<string>("default"); + const manager = ContextManager<string>("default"); // guards against an `|| fallback` regression in the return path expect(manager.withContext("inner", () => undefined)).toBeUndefined(); diff --git a/frontend/src/app/common/util/context.ts b/frontend/src/app/common/util/context.ts index b6f7a28b12..a122eb84ea 100644 --- a/frontend/src/app/common/util/context.ts +++ b/frontend/src/app/common/util/context.ts @@ -17,8 +17,6 @@ * under the License. */ -import { merge, Subject } from "rxjs"; - export function ContextManager<Context>(defaultContext: Context) { abstract class ContextManager { private static contextStack: Context[] = [defaultContext]; @@ -54,42 +52,3 @@ export function ContextManager<Context>(defaultContext: Context) { return ContextManager; } - -export function ObservableContextManager<Context>(defaultContext: Context) { - abstract class ObservableContextManager extends ContextManager(defaultContext) { - private static enterStream = new Subject<[exiting: Context, entering: Context]>(); - private static exitStream = new Subject<[exiting: Context, entering: Context]>(); - private static changeContextStream = ObservableContextManager.createChangeContextStream(); - - public static getEnterStream() { - return this.enterStream.asObservable(); - } - - public static getExitStream() { - return this.exitStream.asObservable(); - } - - public static getChangeContextStream() { - return this.changeContextStream; - } - - private static createChangeContextStream() { - return merge(this.getEnterStream(), this.getExitStream()); - } - - protected static enter(context: Context): void { - const oldContext = this.getContext(); - const newContext = context; - super.enter(context); - this.enterStream.next([oldContext, newContext]); - } - - protected static exit(): void { - const oldContext = this.getContext(); - super.exit(); - const newContext = this.getContext(); - this.exitStream.next([oldContext, newContext]); - } - } - return ObservableContextManager; -} diff --git a/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.ts b/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.ts index ee1fd6a6fd..acb31c4917 100644 --- a/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.ts +++ b/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.ts @@ -23,7 +23,7 @@ import { LogicalPort, Point } from "../../../types/workflow-common.interface"; import * as joint from "jointjs"; import * as dagre from "dagre"; import * as graphlib from "graphlib"; -import { ObservableContextManager } from "src/app/common/util/context"; +import { ContextManager } from "src/app/common/util/context"; import { Coeditor, User } from "../../../../common/type/user"; import { operatorCoeditorChangedPropertyClass, operatorCoeditorEditingClass } from "../../joint-ui/joint-ui.service"; import { HeatmapView } from "../../heatmap/heatmap-scoring"; @@ -817,7 +817,7 @@ export class JointGraphWrapper { } public static jointGraphContextFactory() { - class JointGraphContext extends ObservableContextManager<JointGraphContextType>(DefaultContext) { + class JointGraphContext extends ContextManager<JointGraphContextType>(DefaultContext) { private static jointPaper: joint.dia.Paper | undefined; public static async() {
