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-8351-506397a5c2d8f392a1f8ceb85dc5f7a5416169b8 in repository https://gitbox.apache.org/repos/asf/texera.git
commit ce1b1023d9c5343c64591e41e6776cd8b07e26e2 Author: yangzhang75 <[email protected]> AuthorDate: Mon Sep 28 21:42:44 2026 +0000 fix(workflow): keep workflowSettings and formBinding in the shared model (#8351) ### What changes were proposed in this PR? `workflowSettings` and the Form View definition (`formBinding`) were held as per-client private fields on `WorkflowActionService`, outside the Yjs shared document. Because autosave persists the whole workflow content, a co-editor whose private copy was stale would overwrite another editor's change on their next canvas edit -- a lost update (#8315). This moves both into the shared model: * A shared `contentMetaMap` (`Y.Map`, keyed `"settings"` / `"formBinding"`) is added to `SharedModel`, alongside the graph maps, so it syncs live and every autosave writes the current value rather than a stale copy. It is deliberately kept out of the undo manager, matching the previous (non-undoable) behavior of these values. * `getWorkflowSettings` / `setWorkflowSettings` / `getFormBinding` / `setFormBinding` / `hydrateFormBinding` / `getWorkflowContent` now read and write the shared map. * An observer republishes a change -- from a local edit or a co-editor -- on `formBindingChanged$` for the definition, merged into `workflowChanged()` so the Form View re-renders and the existing autosave picks it up, and on the new `workflowSettingsChanged$` for the settings, which the settings panel refreshes from alongside `workflowChanged()`. The settings stream is not merged into `workflowChanged()`: the panel persists a settings change itself, and the autosave would save it a second time, cutting a second version; a later autosave carries the current settings anyway, since the content reads them from the shared map. * Opening a workflow seeds the database's copy into the document, and that seed is careful: it waits for the provider's first sync (immediately when the document is offline or already synced) and writes only when the document says nothing about the key yet, leaving whatever arrived from the room alone -- a co-editor's value, or the mark that the key was cleared. Absence is explicit: a workflow opened or reloaded without a value marks the key cleared (`null`) rather than deleting it, so a room that cleared a key is told from one no client has seeded yet, and a joining client whose database copy has not caught up with the clearing cannot put the value back; opening a workflow that carries no definition does not take a co-editor's away either. Writing before the sync would have been a concurrent whole-value write that Yjs settles by client id rather than by recency, which is the lost update this PR exists to prevent. While the seed waits, the database copy is held and read from, so the page shows it and an autosave in that window carries it instead of an empty one, presence included (a definition that is present but empty stays in the content). The wait is not bounded: a copy written before the sync would be the concurrent write above, on a slow room as much as on an unreachable one, and never writing it loses nothing, since nobody else was in that document either. A local edit of the key while the seed waits cancels the seed and is held the same way: before the first sync a write into the document would be one more concurrent whole-value write, settled by client id, so the edit is read from and announced at once and written after the sync, where it follows the room's state and replaces it, as an edit should. The seed is not announced as an edit, so opening a workflow does not save; clearing the page writes nothing into the destroyed document, and a blank workflow starts on a fresh document, whose seed state (waiting copies, seeded keys) starts empty. The room's precedence is for the first seed of a key on a document, the one the workflow is opened with; a later reload into the same document (a version shown or returned from, an agent's rewrite) replaces the value, the room's included, or marks it cleared for a version that carries none. A seed that waits listens for the sync and stops listening when it lands, is superseded or is cancelled. Whole-value last-writer-wins is accepted for now; field-level Y-types are a possible future refinement. ### Any related issues, documentation, discussions? Closes #8315. ### How was this PR tested? Unit tests (vitest): `workflow-action.service.spec`, driving the real `SharedModel`. Beyond the move itself (a co-editor's write to the shared map reaches `formBindingChanged$` and `workflowChanged()`; the same for settings; an unchanged value is not rewritten; opening a workflow stays silent, driven through the real reload path), the seed is covered by a co-editor's value surviving both a seed and an open that carries none, by reads falling back to the database copy while the seed waits, by the copy staying out of the document until the first sync however long that takes, reads and saves carrying it meanwhile, by a definition written while the seed of a workflow opened without one waited surviving the sync, by a present but empty definition staying in the content while the seed waits, by clearing the page announcing nothing and writing nothing, by a later reload replacing and then clearing a value the room owned, by a cleared mark from the room keeping a stale database copy out, by an edit made before the first sync being held, shown, announced exactly once, and written after the sync over what the room held, by a settings change reaching `workflowSettingsChanged$` and not `workflowChanged()`, and by the settings panel re-syncing from that stream without persisting, by one sync listener per waiting key that goes with the seed, by the deferred seed staying silent, by the seed going in at once into a document that is not connecting or has already synced, by a later open superseding a seed still waiting for the sync, and by a map change made under the reloading flag staying silent. Each new guard was deletion-checked (removing it turns the corresponding test red). The full frontend suite, eslint, prettier and the production (AOT) build pass; every line this PR adds is covered. To verify by hand, with two browser windows on one workflow: in A, open the Form View, enter edit mode and expose a field (or change a setting in the canvas's Settings panel), and wait for the save; in B, drag an operator on the canvas and wait for the autosave (about 5 s); reload A. Before this change the exposed field (the setting) is gone, B's autosave having stored its stale copy; after it, the field is still there, and B's Form View or Settings panel showed the change live. A side-by-side before/after video is to be added here. ### Was this PR authored or co-authored using generative AI tooling? Yes. Generated-by: Claude Code (Claude Opus 5, Anthropic). Co-authored with Claude, reviewed line by line by the author before submission. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY --------- Co-authored-by: Claude Opus 4.8 <[email protected]> --- .../left-panel/settings/settings.component.spec.ts | 19 + .../left-panel/settings/settings.component.ts | 8 +- .../service/workflow-graph/model/shared-model.ts | 20 + .../model/workflow-action.service.spec.ts | 430 ++++++++++++++++++++- .../model/workflow-action.service.ts | 303 +++++++++++++-- 5 files changed, 733 insertions(+), 47 deletions(-) diff --git a/frontend/src/app/workspace/component/left-panel/settings/settings.component.spec.ts b/frontend/src/app/workspace/component/left-panel/settings/settings.component.spec.ts index ab13c540f3..5a6e0e4904 100644 --- a/frontend/src/app/workspace/component/left-panel/settings/settings.component.spec.ts +++ b/frontend/src/app/workspace/component/left-panel/settings/settings.component.spec.ts @@ -42,6 +42,8 @@ class StubWorkflowActionService { executionMode: ExecutionMode.PIPELINED, }; private workflowChangedSubject = new Subject<unknown>(); + private workflowSettingsChangedSubject = new Subject<unknown>(); + readonly workflowSettingsChanged$ = this.workflowSettingsChangedSubject.asObservable(); getWorkflowSettings(): WorkflowSettings { return this.settings; @@ -73,6 +75,10 @@ class StubWorkflowActionService { emitWorkflowChanged(): void { this.workflowChangedSubject.next(undefined); } + + emitWorkflowSettingsChanged(): void { + this.workflowSettingsChangedSubject.next(undefined); + } } describe("SettingsComponent", () => { @@ -200,6 +206,19 @@ describe("SettingsComponent", () => { expect(workflowPersistSpy.persistWorkflow).not.toHaveBeenCalled(); }); + it("re-syncs the form when a co-editor's settings arrive, without saving them again", () => { + // A settings change travels on its own stream, not on workflowChanged, so that the autosave + // behind workflowChanged does not save what this panel (or a co-editor's) already persisted. + workflowActionService.setWorkflowDataTransferBatchSize(555); + workflowActionService.updateExecutionMode(ExecutionMode.MATERIALIZED); + + workflowActionService.emitWorkflowSettingsChanged(); + + expect(component.settingsForm.get("dataTransferBatchSize")!.value).toBe(555); + expect(component.settingsForm.get("executionMode")!.value).toBe(ExecutionMode.MATERIALIZED); + expect(workflowPersistSpy.persistWorkflow).not.toHaveBeenCalled(); + }); + it("should ignore form value changes that fail validation", () => { const setBatchSizeSpy = vi.spyOn(workflowActionService, "setWorkflowDataTransferBatchSize"); diff --git a/frontend/src/app/workspace/component/left-panel/settings/settings.component.ts b/frontend/src/app/workspace/component/left-panel/settings/settings.component.ts index e70761916e..74d35fa125 100644 --- a/frontend/src/app/workspace/component/left-panel/settings/settings.component.ts +++ b/frontend/src/app/workspace/component/left-panel/settings/settings.component.ts @@ -20,6 +20,7 @@ import { Component, OnInit } from "@angular/core"; import { FormBuilder, FormGroup, Validators, FormsModule, ReactiveFormsModule } from "@angular/forms"; import { UntilDestroy, untilDestroyed } from "@ngneat/until-destroy"; +import { merge } from "rxjs"; import { WorkflowActionService } from "../../../service/workflow-graph/model/workflow-action.service"; import { WorkflowPersistService } from "src/app/common/service/workflow-persist/workflow-persist.service"; import { UserService } from "../../../../common/service/user/user.service"; @@ -71,8 +72,11 @@ export class SettingsComponent implements OnInit { this.updateExecutionMode(mode); }); - this.workflowActionService - .workflowChanged() + // The settings can move without the form: undo/redo, a reloaded workflow (workflowChanged), or + // a co-editor's edit, which arrives on workflowSettingsChanged$ alone -- a settings change is + // not part of workflowChanged, since this panel persists its own and the autosave behind + // workflowChanged would save it a second time. + merge(this.workflowActionService.workflowChanged(), this.workflowActionService.workflowSettingsChanged$) .pipe(untilDestroyed(this)) .subscribe(() => { this.settingsForm.patchValue( diff --git a/frontend/src/app/workspace/service/workflow-graph/model/shared-model.ts b/frontend/src/app/workspace/service/workflow-graph/model/shared-model.ts index f9546a64a8..c3179d6d5a 100644 --- a/frontend/src/app/workspace/service/workflow-graph/model/shared-model.ts +++ b/frontend/src/app/workspace/service/workflow-graph/model/shared-model.ts @@ -28,10 +28,21 @@ import { Point, } from "../../../types/workflow-common.interface"; import { CoeditorState, User } from "../../../../common/type/user"; +import { FormBindingConfig, WorkflowSettings } from "../../../../common/type/workflow"; import { getWebsocketUrl } from "../../../../common/util/url"; import { v4 as uuid } from "uuid"; import { YType } from "../../../types/shared-editing.interface"; +/** + * The values `contentMetaMap` holds: the non-graph pieces of a workflow's content that sync in + * the shared doc. Keyed "settings" (WorkflowSettings) and "formBinding" (the Form View + * definition); the union keeps the map off `unknown` and rejects unrelated values. + */ +export type ContentMetaValue = WorkflowSettings | FormBindingConfig; + +/** The keys `contentMetaMap` holds. */ +export type ContentMetaKey = "settings" | "formBinding"; + /** * SharedModel encapsulates everything related to real-time shared editing for the current workflow. * Most of the yjs-related implementations are within this class. @@ -45,6 +56,14 @@ export class SharedModel { public operatorLinkMap: Y.Map<OperatorLink>; public elementPositionMap: Y.Map<Point>; public debugState: Y.Map<Y.Map<BreakpointInfo>>; + // Non-graph workflow content that used to live as a per-client private field and so was + // silently overwritten by another editor's whole-content autosave (workflowSettings, and + // the Form View definition). Kept in the shared doc, keyed "settings"/"formBinding", so it + // syncs live like the graph and every autosave writes the current value, not a stale copy. + // `null` marks a key the room has cleared -- a workflow opened or reloaded without the value -- + // as opposed to one no client has seeded yet: the difference decides whether a joining client's + // database copy goes in (see WorkflowActionService.seedContentMeta). + public contentMetaMap: Y.Map<ContentMetaValue | null>; public undoManager: Y.UndoManager; public clientId: string; @@ -68,6 +87,7 @@ export class SharedModel { this.commentBoxMap = this.yDoc.getMap("commentBoxMap"); this.operatorLinkMap = this.yDoc.getMap("operatorLinkMap"); this.elementPositionMap = this.yDoc.getMap("elementPositionMap"); + this.contentMetaMap = this.yDoc.getMap("contentMeta"); // Initialize Y-undo manager by aggregating intended Y-structures. Only structures included here will be undoable. this.undoManager = new Y.UndoManager( diff --git a/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.spec.ts b/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.spec.ts index d02862d1e4..984ff04ff9 100644 --- a/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.spec.ts +++ b/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.spec.ts @@ -37,11 +37,18 @@ import { } from "./mock-workflow-data"; import { inject, TestBed } from "@angular/core/testing"; +import * as Y from "yjs"; import { DEFAULT_WORKFLOW, DEFAULT_WORKFLOW_NAME, WorkflowActionService } from "./workflow-action.service"; import { LogicalPort, OperatorPredicate } from "../../../types/workflow-common.interface"; import { WorkflowUtilService } from "../util/workflow-util.service"; import { commonTestProviders } from "../../../../common/testing/test-utils"; -import { ExecutionMode, Workflow, WorkflowSettings } from "../../../../common/type/workflow"; +import { + ExecutionMode, + FormBindingConfig, + getDefaultFormBinding, + Workflow, + WorkflowSettings, +} from "../../../../common/type/workflow"; import { WorkflowMetadata } from "../../../../dashboard/type/workflow-metadata.interface"; describe("WorkflowActionService", () => { @@ -71,6 +78,18 @@ describe("WorkflowActionService", () => { jointGraph = (service as any).jointGraph; }); + /** + * The shared document has finished its first sync with the room. A seed of the workflow's + * settings or form definition waits for this, so that a co-editor's newer value is already in + * the map and is not overwritten by the copy the database handed us (see seedContentMeta). + * No y-websocket server answers in a unit test, so the tests say when the sync lands. Set the + * way the provider sets it, so the document counts as synced afterwards (a seed of a later + * reload then goes in at once) and the event is emitted for the seeds waiting on it. + */ + function syncSharedDoc(): void { + texeraGraph.sharedModel.wsProvider.synced = true; + } + it("should be created", inject([WorkflowActionService], (injectedService: WorkflowActionService) => { expect(injectedService).toBeTruthy(); })); @@ -832,12 +851,88 @@ describe("WorkflowActionService", () => { expect(events).toEqual([true, false]); }); - it("should not replace the workflow settings object when given the same reference", () => { - const current = service.getWorkflowSettings(); + it("does not write a redundant shared-model update when the settings value is unchanged", () => { + syncSharedDoc(); // an edit after the sync goes straight into the document + service.setWorkflowSettings({ dataTransferBatchSize: 123, executionMode: ExecutionMode.PIPELINED }); + const stored = texeraGraph.sharedModel.contentMetaMap.get("settings"); + expect(stored).toEqual({ dataTransferBatchSize: 123, executionMode: ExecutionMode.PIPELINED }); - service.setWorkflowSettings(current); + // An equal value (a different object) must be skipped, or every collaborator gets a redundant + // Yjs update; the stored object is therefore left in place. + service.setWorkflowSettings({ dataTransferBatchSize: 123, executionMode: ExecutionMode.PIPELINED }); - expect(service.getWorkflowSettings()).toBe(current); + expect(texeraGraph.sharedModel.contentMetaMap.get("settings")).toBe(stored); + }); + + // A co-editor's settings change has to reach this client's settings panel, which refreshes from + // workflowSettingsChanged$. Not through workflowChanged: the panel persists a settings change + // itself, and the autosave behind workflowChanged would save it a second time, cutting a second + // version. Writing to the shared map stands in for the remote update, as it does for the form binding. + it("announces a settings change from the shared model on workflowSettingsChanged$, not on workflowChanged", () => { + const changed: unknown[] = []; + const settings: WorkflowSettings = { dataTransferBatchSize: 77, executionMode: ExecutionMode.MATERIALIZED }; + const settingsSeen: unknown[] = []; + const subA = service.workflowChanged().subscribe(v => changed.push(v)); + const subB = service.workflowSettingsChanged$.subscribe(v => settingsSeen.push(v)); + + texeraGraph.sharedModel.contentMetaMap.set("settings", settings); + + expect(settingsSeen).toEqual([settings]); + expect(changed).toEqual([]); + expect(service.getWorkflowSettings()).toEqual(settings); + subA.unsubscribe(); + subB.unsubscribe(); + }); + + // A workflow opened without settings marks the key cleared rather than leaving it absent, so the + // room can tell "cleared" from "nobody has seeded this yet"; getWorkflowSettings then falls back + // to the defaults on the mark. + it("marks the settings cleared when a workflow with no settings is opened into a fresh room", () => { + service.hydrateSettings(undefined); + syncSharedDoc(); + + expect(texeraGraph.sharedModel.contentMetaMap.get("settings")).toBeNull(); + expect(service.getWorkflowSettings()).toEqual(service["getDefaultSettings"]()); + expect(service.getWorkflowContent().settings).toEqual(service["getDefaultSettings"]()); + }); + + it("leaves a co-editor's settings alone when the workflow being opened has none", () => { + const coeditors = new Y.Doc(); + const live: WorkflowSettings = { dataTransferBatchSize: 99, executionMode: ExecutionMode.MATERIALIZED }; + coeditors.getMap("contentMeta").set("settings", live); + Y.applyUpdate(texeraGraph.sharedModel.yDoc, Y.encodeStateAsUpdate(coeditors)); + + service.hydrateSettings(undefined); + syncSharedDoc(); + + expect(service.getWorkflowSettings()).toEqual(live); + }); + + // Clearing the page reports no edit and writes nothing: the document is destroyed, and the + // settings go back to the defaults through the blank reload's seed, under the reloading flag, + // the same way the form definition does. + it("clears non-default settings without announcing an edit or writing into the destroyed document", () => { + service.setWorkflowSettings({ dataTransferBatchSize: 1, executionMode: ExecutionMode.MATERIALIZED }); + const changed: unknown[] = []; + const sub = service.workflowChanged().subscribe(v => changed.push(v)); + + service.clearWorkflow(); + + expect(changed).toEqual([]); + expect(service.getWorkflowSettings()).toEqual(service["getDefaultSettings"]()); + sub.unsubscribe(); + }); + + // The copy a seed holds for the previous workflow is that workflow's: a blank one opened before + // the sync came must not read it, or carry it into its first save. + it("does not carry the previous workflow's waiting settings copy into a blank workflow", () => { + service.hydrateSettings({ dataTransferBatchSize: 5, executionMode: ExecutionMode.MATERIALIZED }); + expect(service.getWorkflowSettings().dataTransferBatchSize).toBe(5); + + service.clearWorkflow(); + + expect(service.getWorkflowSettings()).toEqual(service["getDefaultSettings"]()); + expect(service.getWorkflowContent().settings).toEqual(service["getDefaultSettings"]()); }); it("should assemble workflow content and the full workflow from graph state", () => { @@ -904,6 +999,7 @@ describe("WorkflowActionService", () => { }; service.reloadWorkflow(workflow, false, false); + syncSharedDoc(); expect(service.getFormBinding()).toEqual(config); }); @@ -925,6 +1021,7 @@ describe("WorkflowActionService", () => { false, false ); + syncSharedDoc(); expect(service.getFormBinding()).toEqual({ fields: [] }); }); @@ -969,13 +1066,331 @@ describe("WorkflowActionService", () => { sub.unsubscribe(); }); - // Opening a workflow is not an edit; announcing it would save on every open. - it("should stay silent while a workflow is being opened", () => { + // The definition lives in the shared model (#8315), so a change to the shared map -- a local + // edit and a co-editor's remote update flow through the same observer -- is picked up and + // republished, and this client's next autosave carries the current value instead of a stale + // private copy. Writing to the map directly here stands in for either source. + it("should pick up a change to the shared model", () => { + const seen: unknown[] = []; + const sub = service.formBindingChanged$.subscribe(v => seen.push(v)); + + texeraGraph.sharedModel.contentMetaMap.set("formBinding", config); + + expect(seen).toEqual([config]); + expect(service.getFormBinding()).toEqual(config); + sub.unsubscribe(); + }); + + // The seed a workflow opens with must not overwrite what the room already holds: the document + // is connected a moment before, so a write that goes in before the first sync is concurrent + // with the co-editor's, and Yjs settles that by client id, not by which is newer. The value + // the room sends is the authoritative one. + it("leaves a co-editor's definition alone instead of seeding the database copy over it", () => { + const coeditors = new Y.Doc(); + const live: FormBindingConfig = { fields: [], instruction: { title: "live", body: "from a co-editor" } }; + coeditors.getMap("contentMeta").set("formBinding", live); + // The room's state reaches this client as a remote update, exactly as the first sync delivers it. + Y.applyUpdate(texeraGraph.sharedModel.yDoc, Y.encodeStateAsUpdate(coeditors)); + + service.hydrateFormBinding(config); + syncSharedDoc(); + + expect(service.getFormBinding()).toEqual(live); + }); + + // Opening a workflow that carries no definition must not take the room's away either: the + // clear is for the document this client is reusing, not for a co-editor's live value. + it("leaves a co-editor's definition alone when the workflow being opened has none", () => { + const coeditors = new Y.Doc(); + const live: FormBindingConfig = { fields: [], instruction: { title: "live", body: "from a co-editor" } }; + coeditors.getMap("contentMeta").set("formBinding", live); + Y.applyUpdate(texeraGraph.sharedModel.yDoc, Y.encodeStateAsUpdate(coeditors)); + + service.hydrateFormBinding(undefined); + syncSharedDoc(); + + expect(service.getFormBinding()).toEqual(live); + }); + + // The seed lands after the open has finished (it waits for the sync), so the reloading flag is + // no longer up: it has to keep itself quiet, or every open would announce an edit and save. + it("does not announce the seed that lands once the document syncs", () => { + service.reloadWorkflow( + { + ...DEFAULT_WORKFLOW, + content: { + operators: [], + operatorPositions: {}, + links: [], + commentBoxes: [], + settings: undefined as any, + formBinding: config, + }, + }, + false, + false + ); const seen: unknown[] = []; const sub = service.formBindingChanged$.subscribe(v => seen.push(v)); + syncSharedDoc(); + + expect(seen).toEqual([]); + expect(service.getFormBinding()).toEqual(config); + sub.unsubscribe(); + }); + + // Until the sync says whether the room holds one, the database copy still has to be the + // workflow's: the page renders it, and an autosave that fires meanwhile carries it rather + // than an empty definition that would wipe the author's setup. + it("reads the database copy while the seed waits for the first sync", () => { service.hydrateFormBinding(config); + expect(texeraGraph.sharedModel.contentMetaMap.has("formBinding")).toBe(false); + expect(service.getFormBinding()).toEqual(config); + expect(service.getWorkflowContent().formBinding).toEqual(config); + }); + + // A room that is slow to answer must not be written into before it has: the copy would be the + // concurrent write the wait exists to avoid. Reads and saves carry the copy for as long as it + // takes, and the document gets it when the sync comes. + it("holds the database copy out of the document until the first sync, however long that takes", () => { + vi.useFakeTimers(); + try { + service.hydrateFormBinding(config); + vi.advanceTimersByTime(60_000); + + expect(texeraGraph.sharedModel.contentMetaMap.has("formBinding")).toBe(false); + expect(service.getFormBinding()).toEqual(config); + expect(service.getWorkflowContent().formBinding).toEqual(config); + + syncSharedDoc(); + expect(texeraGraph.sharedModel.contentMetaMap.get("formBinding")).toEqual(config); + } finally { + vi.useRealTimers(); + } + }); + + // A workflow opened without a definition seeds a deletion once the sync lands. An author who + // writes one before then has the document carry theirs, and the seed must not take it away. + it("does not delete a definition written while the seed of a workflow opened without one was waiting", () => { + service.hydrateFormBinding(undefined); + service.setFormBinding(config); + + syncSharedDoc(); + + expect(texeraGraph.sharedModel.contentMetaMap.get("formBinding")).toEqual(config); + expect(service.getFormBinding()).toEqual(config); + }); + + // A stored definition can be present but empty (an author added fields and removed them all). + // While the seed waits, the copy counts as present too: an autosave in that window keeps + // carrying it, where dropping it would cut a version without the key. + it("keeps a present but empty definition in the content while the seed waits", () => { + const presentButEmpty: FormBindingConfig = { fields: [] }; + service.hydrateFormBinding(presentButEmpty); + + expect(texeraGraph.sharedModel.contentMetaMap.has("formBinding")).toBe(false); + expect(service.getWorkflowContent().formBinding).toEqual(presentButEmpty); + }); + + // A document that is not connecting to a room has no sync to wait for, and one that has + // already synced (a version reloaded into the open document) has nothing more to learn from + // it: the seed goes in at once, so the value is in the document and not only held here. + it("seeds at once into a document that is not connecting to a room", () => { + texeraGraph.sharedModel.wsProvider.shouldConnect = false; + + service.hydrateFormBinding(config); + + expect(texeraGraph.sharedModel.contentMetaMap.get("formBinding")).toEqual(config); + }); + + it("seeds at once into a document that has already synced", () => { + texeraGraph.sharedModel.wsProvider.synced = true; + + service.hydrateFormBinding(config); + + expect(texeraGraph.sharedModel.contentMetaMap.get("formBinding")).toEqual(config); + }); + + // A second open before the first seed has landed (a version reloaded into the document while + // it was still syncing) supersedes it: only the later copy may go in, or the earlier one would + // take the key first and the later one, finding it present, would leave it there. + it("lets a later open supersede a seed still waiting for the sync", () => { + const earlier: FormBindingConfig = { fields: [], instruction: { title: "earlier", body: "superseded" } }; + service.hydrateFormBinding(earlier); + service.hydrateFormBinding(config); + + syncSharedDoc(); + + expect(texeraGraph.sharedModel.contentMetaMap.get("formBinding")).toEqual(config); + }); + + // The room's value wins over the copy a workflow is opened with, and over that only: a later + // reload into the same document (a version shown, an agent's rewrite) is the intent, and + // replaces the value, the room's included -- or clears it, for a version that carries none. + it("replaces a value the room owned when a later reload into the document brings another", () => { + const coeditors = new Y.Doc(); + const live: FormBindingConfig = { fields: [], instruction: { title: "live", body: "from a co-editor" } }; + coeditors.getMap("contentMeta").set("formBinding", live); + Y.applyUpdate(texeraGraph.sharedModel.yDoc, Y.encodeStateAsUpdate(coeditors)); + service.hydrateFormBinding(config); + syncSharedDoc(); + expect(service.getFormBinding()).toEqual(live); // the open: the room's value stands + + const version: FormBindingConfig = { fields: [], instruction: { title: "v3", body: "as saved back then" } }; + service.hydrateFormBinding(version); + expect(texeraGraph.sharedModel.contentMetaMap.get("formBinding")).toEqual(version); + + service.hydrateFormBinding(undefined); + expect(texeraGraph.sharedModel.contentMetaMap.get("formBinding")).toBeNull(); + expect(service.getFormBinding()).toEqual(getDefaultFormBinding()); + expect("formBinding" in service.getWorkflowContent()).toBe(false); + }); + + // A collaborator who opened or reloaded the workflow without a definition marked the key + // cleared. A client joining with a database copy the clearing has not reached yet must not + // put the definition back: the room's word, cleared included, beats the copy. + it("does not bring back a definition the room has cleared from a database copy that still has it", () => { + const coeditors = new Y.Doc(); + coeditors.getMap("contentMeta").set("formBinding", null); + Y.applyUpdate(texeraGraph.sharedModel.yDoc, Y.encodeStateAsUpdate(coeditors)); + + service.hydrateFormBinding(config); + syncSharedDoc(); + + expect(texeraGraph.sharedModel.contentMetaMap.get("formBinding")).toBeNull(); + expect(service.getFormBinding()).toEqual(getDefaultFormBinding()); + expect("formBinding" in service.getWorkflowContent()).toBe(false); + }); + + // A seed that waits listens for the sync; a document that never syncs must not collect a + // listener per reload, so the listening ends with the seed: when it is superseded by a later + // one, cancelled by an edit, or lands. + it("keeps one sync listener per waiting key, and none once it has landed", () => { + const listeners = () => (texeraGraph.sharedModel.wsProvider as any)._observers.get("sync")?.size ?? 0; + const before = listeners(); + + service.hydrateFormBinding(config); + service.hydrateFormBinding({ fields: [], instruction: { title: "later", body: "supersedes" } }); + expect(listeners()).toBe(before + 1); + + service.setFormBinding(config); // an edit before the sync takes the seed's place: still one + expect(listeners()).toBe(before + 1); + + syncSharedDoc(); // lands + expect(listeners()).toBe(before); + }); + + // Before the first sync every write into the document is concurrent with the room's state and + // settled by client id, so an edit typed right after opening could lose to a value the room + // holds. The edit is held instead -- shown and announced at once, carried by an autosave -- + // and written after the sync, where it follows the room's state and replaces it. + it("holds an edit made before the first sync and writes it after, over what the room held", () => { + service.hydrateFormBinding(config); // the open's seed, waiting + const seen: FormBindingConfig[] = []; + const sub = service.formBindingChanged$.subscribe(v => seen.push(v)); + const mine: FormBindingConfig = { fields: [], instruction: { title: "mine", body: "typed right after opening" } }; + + service.setFormBinding(mine); + + expect(seen).toEqual([mine]); + expect(service.getFormBinding()).toEqual(mine); + expect(service.getWorkflowContent().formBinding).toEqual(mine); + expect(texeraGraph.sharedModel.contentMetaMap.has("formBinding")).toBe(false); // not a concurrent write + + // The room's state arrives with the sync, holding a co-editor's definition. + const coeditors = new Y.Doc(); + const live: FormBindingConfig = { fields: [], instruction: { title: "live", body: "from a co-editor" } }; + coeditors.getMap("contentMeta").set("formBinding", live); + Y.applyUpdate(texeraGraph.sharedModel.yDoc, Y.encodeStateAsUpdate(coeditors)); + expect(service.getFormBinding()).toEqual(mine); // the edit is what this page shows throughout + syncSharedDoc(); + + expect(texeraGraph.sharedModel.contentMetaMap.get("formBinding")).toEqual(mine); + expect(seen).toEqual([mine]); // once: not again when the room's value arrived, nor on landing + sub.unsubscribe(); + }); + + it("holds a settings edit made before the first sync the same way", () => { + const seen: WorkflowSettings[] = []; + const sub = service.workflowSettingsChanged$.subscribe(v => seen.push(v)); + const mine: WorkflowSettings = { dataTransferBatchSize: 7, executionMode: ExecutionMode.MATERIALIZED }; + + service.setWorkflowSettings(mine); + + expect(seen).toEqual([mine]); + expect(service.getWorkflowSettings()).toEqual(mine); + expect(texeraGraph.sharedModel.contentMetaMap.has("settings")).toBe(false); + syncSharedDoc(); + expect(texeraGraph.sharedModel.contentMetaMap.get("settings")).toEqual(mine); + sub.unsubscribe(); + }); + + // The graph observers stay quiet while a workflow is being opened, and this one does the same: + // a map write made under the reloading flag is part of the open, not an edit to announce. + it("does not announce a map change made while a workflow is reloading", () => { + const seen: unknown[] = []; + const sub = service.formBindingChanged$.subscribe(v => seen.push(v)); + service.getJointGraphWrapper().setReloadingWorkflow(true); + try { + texeraGraph.sharedModel.contentMetaMap.set("formBinding", config); + } finally { + service.getJointGraphWrapper().setReloadingWorkflow(false); + } + + expect(seen).toEqual([]); + sub.unsubscribe(); + }); + + // Setting the same value again must not write, or every collaborator gets a redundant Yjs + // update and the observer re-fires for a change that is not one. + it("writes an edit straight into the document once it has synced, and announces it from there", () => { + syncSharedDoc(); + const seen: unknown[] = []; + const sub = service.formBindingChanged$.subscribe(v => seen.push(v)); + + service.setFormBinding(config); + + expect(texeraGraph.sharedModel.contentMetaMap.get("formBinding")).toEqual(config); + expect(seen).toEqual([config]); + sub.unsubscribe(); + }); + + it("does not re-announce a form binding that is unchanged", () => { + syncSharedDoc(); + service.setFormBinding(config); + const seen: unknown[] = []; + const sub = service.formBindingChanged$.subscribe(v => seen.push(v)); + + service.setFormBinding({ ...config }); + + expect(seen).toEqual([]); + sub.unsubscribe(); + }); + + // Opening a workflow is not an edit; announcing it would save on every open. The seed + // runs under the reloading flag, so the shared-map observer skips it. + it("should stay silent while a workflow is being opened", () => { + const seen: unknown[] = []; + const sub = service.formBindingChanged$.subscribe(v => seen.push(v)); + + service.reloadWorkflow( + { + ...DEFAULT_WORKFLOW, + content: { + operators: [mockScanPredicate], + operatorPositions: { [mockScanPredicate.operatorID]: mockPoint }, + links: [], + commentBoxes: [], + settings: { dataTransferBatchSize: 400, executionMode: ExecutionMode.PIPELINED }, + formBinding: config, + }, + }, + false, + false + ); + expect(seen.length).toEqual(0); expect(service.getFormBinding()).toEqual(config); sub.unsubscribe(); @@ -1002,6 +1417,7 @@ describe("WorkflowActionService", () => { }; service.reloadWorkflow(workflow, false, false); + syncSharedDoc(); expect(texeraGraph.hasCommentBox("commentBox-old")).toBeFalsy(); expect(texeraGraph.hasOperator(mockScanPredicate.operatorID)).toBeTruthy(); diff --git a/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.ts b/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.ts index b4aba75013..f73b97c89a 100644 --- a/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.ts +++ b/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.ts @@ -20,6 +20,8 @@ import { Injectable } from "@angular/core"; import * as joint from "jointjs"; +import * as Y from "yjs"; +import { isEqual } from "lodash-es"; import { BehaviorSubject, merge, Observable, Subject } from "rxjs"; import { ExecutionMode, @@ -50,8 +52,15 @@ import { filter } from "rxjs/operators"; import { isDefined } from "../../../../common/util/predicate"; import { User } from "../../../../common/type/user"; import { SharedModelChangeHandler } from "./shared-model-change-handler"; +import { ContentMetaKey, ContentMetaValue } from "./shared-model"; import { GuiConfigService } from "../../../../common/service/gui-config.service"; +/** A content-meta seed waiting to land (see seedContentMeta): its identity, and how to stop + * listening for the sync it waits on once it is superseded or cancelled. */ +interface ContentMetaSeed { + detach?: () => void; +} + export const DEFAULT_WORKFLOW_NAME = "Untitled Workflow"; export const DEFAULT_WORKFLOW = { name: DEFAULT_WORKFLOW_NAME, @@ -102,19 +111,34 @@ export class WorkflowActionService { private resultPanelOpenSubject = new Subject<boolean>(); public readonly resultPanelOpen$: Observable<boolean> = this.resultPanelOpenSubject.asObservable(); - private workflowSettings: WorkflowSettings; private workflowResetSubject = new Subject<void>(); - // The Form View definition. Presentation, not structure, so it stays out of the shared - // graph (no collaborative merge) and is handled like workflowSettings -- hydrated by - // reloadWorkflow, emitted by getWorkflowContent. - private formBinding: FormBindingConfig = getDefaultFormBinding(); - // Whether the opened workflow's content carried a formBinding. Kept so getWorkflowContent - // re-emits the key only when it was there (or an author has since populated it), leaving a - // plain workflow's content byte-identical -- the same rule agent-service follows. - private formBindingLoaded = false; + // The Form View definition. Presentation, not structure, but it still lives in the shared + // doc (shared-model contentMetaMap, key "formBinding") so a co-editor sees it live and no + // collaborator's whole-content autosave overwrites it with a stale copy. workflowSettings + // is kept there too (key "settings") for the same reason. private formBindingChangeSubject = new Subject<FormBindingConfig>(); public readonly formBindingChanged$: Observable<FormBindingConfig> = this.formBindingChangeSubject.asObservable(); + // A settings change, this client's or a co-editor's, for the settings panel to refresh from. Not + // part of workflowChanged, unlike the form definition (see observeContentMeta). + private workflowSettingsChangeSubject = new Subject<WorkflowSettings>(); + public readonly workflowSettingsChanged$: Observable<WorkflowSettings> = + this.workflowSettingsChangeSubject.asObservable(); + // The shared content-map observer, tracked so re-attaching on a new doc detaches the old one. + private contentMetaObserver?: (event: Y.YMapEvent<ContentMetaValue | null>) => void; + private observedContentMetaMap?: Y.Map<ContentMetaValue | null>; + // Per document, all reset when a new one is loaded (see observeContentMeta): database copies, + // and edits, waiting for the document's first sync (see seedContentMeta), read from meanwhile; the seed + // each key is waiting on, which a later reload replaces and a local edit of the key cancels; + // the keys a seed is writing right now, which is not an edit and so is not announced (an edit + // held for the sync is announced when it is made, see writeContentMeta); the keys + // whose entry came from the room, which the first seed of the key yields to; and the keys seeded + // on this document already, whose next seed is a reload that replaces the value. + private pendingContentMeta = new Map<ContentMetaKey, { value: ContentMetaValue; edit: boolean }>(); + private contentMetaSeeds = new Map<ContentMetaKey, ContentMetaSeed>(); + private seedingContentMetaKeys = new Set<ContentMetaKey>(); + private roomOwnedContentMeta = new Set<ContentMetaKey>(); + private seededContentMetaKeys = new Set<ContentMetaKey>(); constructor( private operatorMetadataService: OperatorMetadataService, @@ -136,12 +160,69 @@ export class WorkflowActionService { ); this.sharedModelChangeHandler.setConfigService(this.config); this.workflowMetadata = DEFAULT_WORKFLOW; - this.workflowSettings = this.getDefaultSettings(); this.undoRedoService.setUndoManager(this.texeraGraph.sharedModel.undoManager); + // Watch the shared content map, re-attaching whenever the shared model is recreated + // (opening another workflow), the same way SharedModelChangeHandler re-attaches its + // graph observers. A formBinding change from a local edit or a co-editor is republished + // on formBindingChanged$ so the Form View re-renders and the existing autosave picks it + // up. The reload seed is skipped -- like the graph seed -- so opening a workflow is not + // announced as an edit and does not save on every open. + this.observeContentMeta(); + this.texeraGraph.newYDocLoadedSubject.subscribe(() => this.observeContentMeta()); + this.handleJointElementDrag(); } + private observeContentMeta(): void { + // Detach the observer from the previous shared model before attaching to the new one, so + // re-attaching on each opened workflow does not stack listeners on the same map. + this.observedContentMetaMap?.unobserve(this.contentMetaObserver!); + // A new document starts with no seed state: nothing in it came from the previous room, no key + // has been seeded on it yet, and whatever copy the previously open workflow left waiting is + // not this one's -- dropped here, so it cannot leak into the next save of a workflow opened + // (or a blank one started) on this document. + for (const key of [...this.contentMetaSeeds.keys()]) { + this.cancelContentMetaSeed(key); // drops the copy it holds, and stops its listening + } + this.roomOwnedContentMeta.clear(); + this.seededContentMetaKeys.clear(); + const contentMetaMap = this.texeraGraph.sharedModel.contentMetaMap; + this.contentMetaObserver = event => { + if (!event.transaction.local) { + // An entry that arrived from the room belongs to the room, a co-editor's value or its + // cleared mark alike: the seed a workflow is opened with yields to it (see seedContentMeta). + // The first sync delivers the room's state as a remote transaction, which is how an + // opening client learns what the room already holds. + for (const key of event.changes.keys.keys()) { + this.roomOwnedContentMeta.add(key as ContentMetaKey); + } + } + if (this.jointGraphWrapper.getReloadingWorkflow()) { + return; + } + if (event.changes.keys.has("formBinding") && this.announces("formBinding")) { + this.formBindingChangeSubject.next(this.getFormBinding()); + } + // Settings are announced the same way, so a co-editor's change reaches the settings panel, + // which refreshes from this stream, instead of sitting stale until an unrelated edit. Not + // through workflowChanged, though: the panel persists a settings change itself, and the + // autosave behind workflowChanged would save it a second time, cutting a second version. + if (event.changes.keys.has("settings") && this.announces("settings")) { + this.workflowSettingsChangeSubject.next(this.getWorkflowSettings()); + } + }; + contentMetaMap.observe(this.contentMetaObserver); + this.observedContentMetaMap = contentMetaMap; + } + + /** Whether a change to a key in the shared map is announced: not a seed's own write, which is an + * open and not an edit; and not while an edit held for the first sync owns the key -- that edit + * was announced when it was made, reads answer it, and its landing is a seed's write. */ + private announces(key: ContentMetaKey): boolean { + return !this.seedingContentMetaKeys.has(key) && !this.pendingContentMeta.get(key)?.edit; + } + private getDefaultSettings(): WorkflowSettings { return { dataTransferBatchSize: this.config.env.defaultDataTransferBatchSize, @@ -710,15 +791,15 @@ export class WorkflowActionService { this.jointGraphWrapper.jointGraph.clear(); if (workflow === undefined) { - // A blank workflow carries no Form View definition; drop any left over from the - // previously open one so it cannot leak into this workflow's next save. - this.hydrateFormBinding(undefined); + // A blank workflow starts on a fresh document, which carries no settings and no Form View + // definition; a copy the previously open workflow left waiting goes with the old document + // (see observeContentMeta), so neither can leak into this workflow's next save. this.setNewSharedModel(); return; } const workflowContent: WorkflowContent = workflow.content; - this.workflowSettings = workflowContent.settings || this.getDefaultSettings(); + this.hydrateSettings(workflowContent.settings); this.hydrateFormBinding(workflowContent.formBinding); let operatorsAndPositions: { op: OperatorPredicate; pos: Point }[] = []; @@ -797,25 +878,162 @@ export class WorkflowActionService { } public setWorkflowSettings(workflowSettings: WorkflowSettings | undefined): void { - if (this.workflowSettings === workflowSettings) { + const newSettings = workflowSettings === undefined ? this.getDefaultSettings() : workflowSettings; + // Skip a redundant write: setting the same value would still cut a Yjs update and observer + // churn for every collaborator. + if (isEqual(this.getWorkflowSettings(), newSettings)) { return; } - - const newSettings = workflowSettings === undefined ? this.getDefaultSettings() : workflowSettings; - this.workflowSettings = newSettings; + this.writeContentMeta("settings", newSettings, () => this.workflowSettingsChangeSubject.next(newSettings)); } public getWorkflowSettings(): WorkflowSettings { - return this.workflowSettings; + return (this.readContentMeta("settings") as WorkflowSettings) ?? this.getDefaultSettings(); } /** - * Load a definition without announcing an edit. Used while opening a workflow, so - * that merely reading one does not look like a change and trigger a save. + * Seed workflowSettings into the shared model while opening a workflow (see seedContentMeta). + * Called under the reloading flag, so the seed is not announced as an edit. + */ + public hydrateSettings(workflowSettings: WorkflowSettings | undefined): void { + this.seedContentMeta("settings", workflowSettings); + } + + /** + * Load a definition into the shared model while opening a workflow (see seedContentMeta). + * Called under the reloading flag, so the shared-map observer skips this seed and opening a + * workflow is not announced as an edit. */ public hydrateFormBinding(formBinding: FormBindingConfig | undefined): void { - this.formBindingLoaded = formBinding !== undefined; - this.formBinding = formBinding ?? getDefaultFormBinding(); + this.seedContentMeta("formBinding", formBinding); + } + + /** + * Put the database's copy of a content-meta value into the shared model as a workflow opens, + * without taking a co-editor's newer one away. + * + * The document is created and connected just before this runs, so the provider has usually not + * synced yet. Writing straight away is a concurrent whole-value write, and Yjs resolves those + * by client id rather than by recency: the copy the database handed us could win over an edit a + * co-editor has not saved yet -- the lost update this all exists to prevent. So wait for the + * first sync, then write only when the key is still absent; a value that came from the room is + * authoritative, whether it is a co-editor's edit or our own from another tab. + * + * Meanwhile the value is not lost: reads fall back to the copy held here, so the page shows it + * and an autosave that fires before the sync carries it rather than an empty one. The wait is + * not bounded: a copy published before the sync would be the concurrent write above, on a room + * that is slow to answer as much as on one that never does. For as long as the room stays out + * of reach the copy is read from here and every save carries it, and it goes into the document + * when the sync eventually comes -- or never, which loses nothing, since nobody else was in that + * document either. + * + * A local edit of the key while the seed waits replaces it (see writeContentMeta): the edit is + * what goes in then, and the copy -- or the clearing, for a workflow opened without a value -- + * must neither write over it nor undo it. + * + * The room's precedence is for the first seed of a key on a document, the one a workflow is + * opened with: it yields to what the room put there by then, a co-editor's value or the mark + * that the key was cleared (`null`, written by whoever opened or reloaded a workflow that + * carries none). The mark is what lets a joining client see the clearing at all -- a deletion + * would leave nothing in the room's state to learn from, and a client whose database copy has + * not caught up with the clearing would put the value back. Anything else the document holds, + * a value this client wrote before the reload included, the seed replaces. A later reload into + * the same document -- a version shown, or returned from; an agent's rewrite -- is the intent, + * and its seed replaces whatever the document holds, a co-editor's value included; `undefined` + * then marks the key cleared, so a version that carries no settings or no form definition does + * not keep the open workflow's. + * + * An edit made before the first sync (`edit`) is held the same way: written straight into the + * document it would be one more concurrent whole-value write, settled by client id, that could + * lose to a value the room holds or take it over unseen. Held, it is read from and announced at + * once (see writeContentMeta), and goes in after the sync as a write that follows the room's + * state and replaces it -- which is what an edit means -- so it yields to nothing. + */ + private seedContentMeta(key: ContentMetaKey, value: ContentMetaValue | undefined, edit = false): void { + // This seed replaces any the key was still waiting on (and stops its listening); it has an + // identity of its own, since a cleared key (undefined) has no value to be told apart by. + this.cancelContentMetaSeed(key); + const seed: ContentMetaSeed = {}; + this.contentMetaSeeds.set(key, seed); + if (value !== undefined) { + this.pendingContentMeta.set(key, { value, edit }); + } + const shared = this.texeraGraph.sharedModel; + // Runs at once or when the sync comes, and then for this seed only: a later seed of the key, a + // local edit of it and the loading of another document all detach the listening before they + // would make this seed stale (see cancelContentMetaSeed, observeContentMeta). + const land = () => { + this.cancelContentMetaSeed(key); + const first = !this.seededContentMetaKeys.has(key); + this.seededContentMetaKeys.add(key); + // Over the first seed the room's word wins, a value or the cleared mark, whether a + // co-editor's or this client's from another tab; a later reload, or an edit, replaces it. + if (first && !edit && this.roomOwnedContentMeta.has(key)) { + return; + } + const stored = value ?? null; + // Opening a workflow is not an edit, so the seed is not announced (see observeContentMeta). + this.seedingContentMetaKeys.add(key); + try { + if (!isEqual(shared.contentMetaMap.get(key), stored)) { + shared.contentMetaMap.set(key, stored); + } + } finally { + this.seedingContentMetaKeys.delete(key); + } + }; + if (!shared.wsProvider.shouldConnect || shared.wsProvider.synced) { + land(); + return; + } + // Listening is undone by land itself (through cancelContentMetaSeed) and by whatever supersedes + // or cancels the seed, so a document that never syncs does not collect a listener per reload. + const onSync = () => land(); + seed.detach = () => shared.wsProvider.off("sync", onSync); + shared.wsProvider.on("sync", onSync); + } + + /** Forget the seed a key is waiting on, stop its listening, and drop the copy it holds; the + * document's value stands. */ + private cancelContentMetaSeed(key: ContentMetaKey): void { + this.contentMetaSeeds.get(key)?.detach?.(); + this.contentMetaSeeds.delete(key); + this.pendingContentMeta.delete(key); + } + + /** An edit still waiting for the first sync, which is going to replace whatever the document + * holds; else the shared document's value for a key -- none, if the document marks it cleared + * -- or, for a key the document says nothing about yet, the database copy still waiting to go in. */ + private readContentMeta(key: ContentMetaKey): ContentMetaValue | undefined { + const pending = this.pendingContentMeta.get(key); + if (pending?.edit) { + return pending.value; + } + const held = this.texeraGraph.sharedModel.contentMetaMap.get(key); + if (held !== undefined) { + return held ?? undefined; + } + return pending?.value; + } + + /** + * An edit of a content-meta value: into the document, where the map's observer announces it -- + * unless the document has not had its first sync yet, when the edit is held and announced here + * instead, and goes in after the sync (see seedContentMeta). + */ + private writeContentMeta(key: ContentMetaKey, value: ContentMetaValue, announce: () => void): void { + const shared = this.texeraGraph.sharedModel; + if (shared.wsProvider.shouldConnect && !shared.wsProvider.synced) { + this.seedContentMeta(key, value, true); + announce(); + return; + } + shared.contentMetaMap.set(key, value); + } + + /** Whether the workflow has a value for a key: in the document, or still waiting to go in. */ + private hasContentMeta(key: ContentMetaKey): boolean { + return this.readContentMeta(key) !== undefined; } /** A form binding worth persisting: an author populated it (fields, a chosen result list -- an @@ -826,16 +1044,20 @@ export class WorkflowActionService { } /** - * Replace the definition as an edit: announced on `formBindingChanged$`, which - * feeds workflowChanged() and so reaches the existing autosave. + * Replace the definition as an edit. The shared map's observer republishes it on + * `formBindingChanged$`, which feeds workflowChanged() and so reaches the existing autosave. */ public setFormBinding(formBinding: FormBindingConfig): void { - this.formBinding = formBinding; - this.formBindingChangeSubject.next(this.formBinding); + // Skip a redundant write (as setWorkflowSettings does): an unchanged value would still cut a + // Yjs update and re-fire the observer for every collaborator. + if (isEqual(this.getFormBinding(), formBinding)) { + return; + } + this.writeContentMeta("formBinding", formBinding, () => this.formBindingChangeSubject.next(formBinding)); } public getFormBinding(): FormBindingConfig { - return this.formBinding; + return (this.readContentMeta("formBinding") as FormBindingConfig) ?? getDefaultFormBinding(); } public getWorkflowMetadata(): WorkflowMetadata { @@ -849,7 +1071,9 @@ export class WorkflowActionService { const links = texeraGraph.getAllLinks(); const operatorPositions: { [key: string]: Point } = {}; const commentBoxes = texeraGraph.getAllCommentBoxes(); - const settings = this.workflowSettings; + const settings = this.getWorkflowSettings(); + // Read the binding once so the has-check and the value are the same snapshot. + const formBinding = this.getFormBinding(); texeraGraph .getAllOperators() @@ -865,12 +1089,13 @@ export class WorkflowActionService { links, commentBoxes, settings, - // Carry formBinding only when the workflow has one (loaded with it, or an author - // populated it), so a plain workflow's content is unchanged and its save cuts no - // needless version. - ...(this.formBindingLoaded || this.isFormBindingNonEmpty(this.formBinding) - ? { formBinding: this.formBinding } - : {}), + // Carry formBinding only when the workflow has one (opened with it, or an author + // populated it since), so a plain workflow's content is unchanged and its save cuts no + // needless version. hasContentMeta stands in for the old "loaded" flag: hydrate sets the + // key (or holds the copy until the sync) for a workflow opened with a binding and deletes + // it for one without. Presence and value come from the same place, so a binding that is + // present but empty is carried while the seed waits, not dropped. + ...(this.hasContentMeta("formBinding") || this.isFormBindingNonEmpty(formBinding) ? { formBinding } : {}), }; } @@ -915,18 +1140,20 @@ export class WorkflowActionService { public setWorkflowDataTransferBatchSize(size: number): void { if (size > 0 && size != null) { - this.setWorkflowSettings({ ...this.workflowSettings, dataTransferBatchSize: size }); + this.setWorkflowSettings({ ...this.getWorkflowSettings(), dataTransferBatchSize: size }); } } public updateExecutionMode(mode: ExecutionMode): void { - this.setWorkflowSettings({ ...this.workflowSettings, executionMode: mode }); + this.setWorkflowSettings({ ...this.getWorkflowSettings(), executionMode: mode }); } public clearWorkflow(): void { this.destroySharedModel(); this.setWorkflowMetadata(undefined); - this.setWorkflowSettings(undefined); + // The settings go back to the defaults with the fresh document the blank reload creates, like + // the form definition: written here they would go into the document just destroyed and be + // announced as an edit. this.reloadWorkflow(undefined); this.setHighlightingEnabled(false); }
