This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/texera.git
The following commit(s) were added to refs/heads/main by this push:
new 957b6c9656 fix(workflow): stop the workspace destroying itself on
beforeunload (#8600)
957b6c9656 is described below
commit 957b6c965640c07ace034eb9edcae53ef974324a
Author: yangzhang75 <[email protected]>
AuthorDate: Sat Sep 19 00:26:43 2026 +0000
fix(workflow): stop the workspace destroying itself on beforeunload (#8600)
### What changes were proposed in this PR?
Switching between the operator canvas and the Form View is a full-page
navigation, and both pages ran their entire teardown from a
`beforeunload` host binding: clear the graph, destroy the Yjs shared
document, disconnect the computing unit, reset the execution state and
the results.
A browser does not always discard a document it navigates away from.
Chrome may keep it in the back/forward cache, and going back restores
the JavaScript state exactly as it was left, re-running nothing. What
came back was the page these methods had already gutted:
- an empty canvas that answered no clicks, because the graph had been
cleared;
- a workflow id reset to `DEFAULT_WORKFLOW`'s, which the share dialog
then asked the backend about;
- an autosave still subscribed to that reset metadata, which wrote the
default out as a brand-new blank workflow, so the workflow list grew by
one on every trip.
There was never anything to tear down there. A document that really is
discarded takes its websockets and its graph with it, and a document
that comes back needs them. So `beforeunload` now only saves, and the
teardown stays in `ngOnDestroy`, which runs when the page is genuinely
replaced. Both views change the same way.
Three more defects turned this into a silent failure, and all are fixed
here:
- `WorkflowResource.getWorkflowType` read
`workflowDao.fetchOneByWid(wid)` and dereferenced it, so an id matching
no row answered 500 with a stack trace rather than 404.
- The share dialog's publish-state subscription had no error handler. A
failed request left `isPublic` null, and the template hides the
Private/Public choice on exactly that (`*ngIf="isPublic !== null"`), so
the dialog looked complete while offering one control fewer and the only
way to find out was the network tab. `ngOnInit` also doubles as a
refresh after an access change, so a failed second read used to keep the
value from the first: the buttons stayed on screen showing a state
nothing had confirmed, while the toast said the choice was not shown. It
is dropped now.
- The Share button had no gate. `ngAfterViewInit` calls
`resetAsNewWorkflow()`, so the metadata sits at `DEFAULT_WORKFLOW` (wid
0) on every canvas load and the real id only arrives with the workflow;
the menu renders outside the loading spinner's container, so a click in
that window opened a dialog that asked `GET /workflow/type/0` and came
back without the Private/Public choice. The button is disabled until the
id arrives and the handler refuses the same window. This is the second
route to the symptom, and it is the gesture the issue reports.
#### Before
<!-- Drop the recording here: open a saved workflow, switch to the Form
View, press the browser's Back button. The canvas comes back blank and
unclickable, the share dialog has no Private/Public choice, and the
workflow list has gained a blank workflow. -->
#### After
<!-- Drop the recording here: the same steps on this branch. The canvas
comes back live, the share dialog keeps both buttons, and no blank
workflow is created. -->
**One behaviour changes deliberately.** The shared document is no longer
destroyed on unload, so a co-editor is no longer told explicitly that
you left; the room notices when the socket closes with the document.
Destroying it on unload is what made a restored page unusable, and a
restored page needs its room.
### Any related issues, documentation, discussions?
Closes #8599.
### How was this PR tested?
Reproduced first, on a local instance running plain `main`
(`e7d1676e1`): open a saved workflow, switch to the Form View, press the
browser's Back button, and the canvas comes back blank and unclickable,
the share dialog has no Private/Public choice, and the workflow list has
gained a blank workflow. With this branch deployed to the same instance,
none of the three happens.
The endpoint was checked against that instance directly: `GET
/api/workflow/type/0` and `/type/999999` answered 500 before and answer
404 after.
Unit tests:
- `workspace.component.spec` and `workflow-form.component.spec`:
`beforeunload` saves and tears nothing down; the existing tests that
`ngOnDestroy` still tears everything down are unchanged and still pass.
The workspace test dispatches a real `beforeunload` DOM event rather
than calling the handler, so the host binding being removed or miswired
is caught too; `workflow-form.rendered.spec` already did this for the
form.
- `menu.component.spec`: the share dialog is not opened for wid 0 or for
no id at all, and the Share button is disabled until the id arrives.
- `share-access.component.spec`: a failed publish-state request reports
itself instead of hiding the choice silently, and a failed refresh drops
the value the previous read left behind.
- `WorkflowResourceCoverSpec`: `getWorkflowType` reports the publish
state, and throws `NotFoundException` for an id that matches no
workflow.
Each new guard was deletion-checked: restoring the teardown on
`beforeunload` in either view, dropping the error handler or the
`isPublic` reset, removing the `[disabled]` on the Share button, or
removing the handler's own guard each turns exactly the intended tests
red. Removing the `@HostListener` itself now turns a test red, which it
would not have before.
Full frontend suite: 224 files, 6126 passed, 1 skipped (pre-existing), 0
failed. `WorkflowExecutionService/testOnly ...
WorkflowResourceCoverSpec`: 16 passed. `ng build
--configuration=production` (AOT), `eslint`, `prettier --check`,
`scalafmtCheck` on main and test sources: all clean.
**Deliberately out of scope.** Grepping for the same shape found two
more pages that act on `beforeunload`, and neither is on the path this
issue reports, so both are left alone and filed instead of widened into
here: the Hub's workflow detail page clears the graph there, and
`AgentPanelComponent` deactivates the current agent there. The app's
other four `beforeunload` bindings only write panel geometry to
`localStorage` and are unaffected.
**The in-page button, the gesture the issue reports,** reaches the same
symptom by the second route above rather than through the cache, which
is why it did not reproduce for me on the Back button's steps: it needs
the click to land before the workflow does. Found by @mengw15 in review.
The Back-button path is the one I reproduced in a running instance; the
button gate that closes this one is covered by tests rather than by a
manual repro, since it is a race against the workflow fetch.
### Was this PR authored or co-authored using generative AI tooling?
Yes. Generated-by: Claude Code (Claude Opus 5, Anthropic). Co-authored
with Claude; the author reviewed the change line by line and reproduced
both the failure and the fix in a running instance before submission.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY
---------
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
---
.../dashboard/user/workflow/WorkflowResource.scala | 6 ++++
.../user/workflow/WorkflowResourceCoverSpec.scala | 14 +++++++++
.../share-access/share-access.component.spec.ts | 27 ++++++++++++++++
.../user/share-access/share-access.component.ts | 16 +++++++++-
.../workspace/component/menu/menu.component.html | 1 +
.../component/menu/menu.component.spec.ts | 36 ++++++++++++++++++++++
.../app/workspace/component/menu/menu.component.ts | 9 ++++++
.../workflow-form/workflow-form.component.spec.ts | 16 ++++++++++
.../workflow-form/workflow-form.component.ts | 16 +++++++++-
.../workflow-form/workflow-form.rendered.spec.ts | 10 ++++--
.../component/workspace.component.spec.ts | 31 +++++++++++++++++++
.../app/workspace/component/workspace.component.ts | 31 ++++++++++++++++---
12 files changed, 204 insertions(+), 9 deletions(-)
diff --git
a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala
b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala
index 17cd7a11fa..46fad49580 100644
---
a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala
+++
b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala
@@ -849,7 +849,13 @@ class WorkflowResource extends LazyLogging {
@RolesAllowed(Array("REGULAR", "ADMIN"))
@Path("/type/{wid}")
def getWorkflowType(@PathParam("wid") wid: Integer): String = {
+ // fetchOneByWid answers null for an id that matches no row, and
dereferencing that answered
+ // every such request with a 500 and a stack trace, which reads as a
broken server rather than
+ // as a workflow that is not there.
val workflow: Workflow = workflowDao.fetchOneByWid(wid)
+ if (workflow == null) {
+ throw new NotFoundException(s"Workflow with id $wid not found")
+ }
if (workflow.getIsPublic) {
"Public"
} else {
diff --git
a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResourceCoverSpec.scala
b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResourceCoverSpec.scala
index 3d830dfe09..fc116d9c92 100644
---
a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResourceCoverSpec.scala
+++
b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResourceCoverSpec.scala
@@ -249,4 +249,18 @@ class WorkflowResourceCoverSpec
// The cover must still be present after the rejected delete.
resource.getCoverImage(testWid, session(owner)).image shouldBe sampleImage
}
+
+ "getWorkflowType" should "report the workflow's publish state" in {
+ resource.getWorkflowType(testWid) shouldBe "Private"
+ }
+
+ // fetchOneByWid answers null for an id that matches no row, and
dereferencing it answered with a
+ // 500 and a stack trace. Its one caller is the share dialog, which asks
with whatever id the page
+ // believes it is on, so a page that has lost its id turned a missing
workflow into a server error
+ // and, having no error handler, silently dropped the Private/Public choice
(issue #8599).
+ it should "throw NotFoundException for an id that matches no workflow" in {
+ assertThrows[NotFoundException] {
+ resource.getWorkflowType(testWid + 999999)
+ }
+ }
}
diff --git
a/frontend/src/app/dashboard/component/user/share-access/share-access.component.spec.ts
b/frontend/src/app/dashboard/component/user/share-access/share-access.component.spec.ts
index 202465b25f..94f20b2cbd 100644
---
a/frontend/src/app/dashboard/component/user/share-access/share-access.component.spec.ts
+++
b/frontend/src/app/dashboard/component/user/share-access/share-access.component.spec.ts
@@ -181,6 +181,33 @@ describe("ShareAccessComponent", () => {
expect(c.isPublic).toBe(false);
});
+ // isPublic staying null hides the Private/Public choice, which is right
for a kind that cannot
+ // be published and wrong when the request merely failed: the dialog then
looked complete while
+ // silently offering one control fewer, and the only way to find out was
the network tab.
+ it("says so when the publish state cannot be read, instead of hiding the
choice silently", () => {
+ workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(throwError(()
=> new Error("boom")));
+
+ const c = setupComponent({ type: "workflow", id: 9 });
+
+ expect(c.isPublic).toBeNull();
+ expect(notificationSpy.error).toHaveBeenCalled();
+ });
+
+ // ngOnInit is re-entered as a refresh after an access change, so a value
from the previous read
+ // is still here when the second one fails. Keeping it would leave the
buttons on screen showing
+ // a state nothing has confirmed, while the toast says the choice is not
shown.
+ it("drops a previously read publish state when the refresh fails, rather
than leaving it stale", () => {
+ workflowPublished = true;
+ const c = setupComponent({ type: "workflow", id: 9 });
+ expect(c.isPublic).toBe(true);
+
+ workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(throwError(()
=> new Error("boom")));
+ c.ngOnInit();
+
+ expect(c.isPublic).toBeNull();
+ expect(notificationSpy.error).toHaveBeenCalled();
+ });
+
it("loads publish state for dataset via DatasetService.getDataset", () => {
datasetPublished = true;
const c = setupComponent({ type: "dataset", id: 12 });
diff --git
a/frontend/src/app/dashboard/component/user/share-access/share-access.component.ts
b/frontend/src/app/dashboard/component/user/share-access/share-access.component.ts
index d5ebb4c595..b5baf5d9db 100644
---
a/frontend/src/app/dashboard/component/user/share-access/share-access.component.ts
+++
b/frontend/src/app/dashboard/component/user/share-access/share-access.component.ts
@@ -137,10 +137,24 @@ export class ShareAccessComponent implements OnInit,
OnDestroy {
this.owner = name;
});
// Stays null for kinds that cannot be published, which is what hides the
publish buttons.
+ // A failed request leaves it null too, and the buttons are equally gone,
so say so: without
+ // this the dialog looked complete while quietly offering one control
fewer, and the only way
+ // to find out was the network tab.
this.descriptor
?.isPublic?.(this.id)
.pipe(untilDestroyed(this))
- .subscribe(isPublic => (this.isPublic = isPublic));
+ .subscribe({
+ next: isPublic => (this.isPublic = isPublic),
+ error: () => {
+ // `ngOnInit` is re-entered as a refresh after an access change, so
a value from the
+ // previous read may still be here. Drop it: keeping it would leave
the buttons on
+ // screen, showing a state nothing has confirmed, while the toast
says they are gone.
+ this.isPublic = null;
+ this.notificationService.error(
+ `Could not read whether this ${this.type} is public, so that
choice is not shown.`
+ );
+ },
+ });
}
ngOnDestroy(): void {
diff --git a/frontend/src/app/workspace/component/menu/menu.component.html
b/frontend/src/app/workspace/component/menu/menu.component.html
index fe8e50601c..0cb65293bf 100644
--- a/frontend/src/app/workspace/component/menu/menu.component.html
+++ b/frontend/src/app/workspace/component/menu/menu.component.html
@@ -420,6 +420,7 @@
<button
nz-button
id="share-button"
+ [disabled]="!workflowId"
(click)="onClickOpenShareAccess()">
<span
nz-icon
diff --git a/frontend/src/app/workspace/component/menu/menu.component.spec.ts
b/frontend/src/app/workspace/component/menu/menu.component.spec.ts
index baea315819..1214f513af 100644
--- a/frontend/src/app/workspace/component/menu/menu.component.spec.ts
+++ b/frontend/src/app/workspace/component/menu/menu.component.spec.ts
@@ -906,6 +906,7 @@ describe("MenuComponent", () => {
vi.spyOn(modalService, "create").mockReturnValue(fakeModalRef);
const router = TestBed.inject(Router);
const navigateSpy = vi.spyOn(router, "navigate").mockResolvedValue(true);
+ component.workflowId = 7;
await component.onClickOpenShareAccess();
@@ -919,11 +920,30 @@ describe("MenuComponent", () => {
vi.spyOn(modalService, "create").mockReturnValue(fakeModalRef);
const router = TestBed.inject(Router);
const navigateSpy = vi.spyOn(router, "navigate").mockResolvedValue(true);
+ component.workflowId = 7;
await component.onClickOpenShareAccess();
expect(navigateSpy).not.toHaveBeenCalled();
});
+
+ // The canvas resets to DEFAULT_WORKFLOW (wid 0) on every load and the
real id only arrives with
+ // the workflow, so a click landing in that window used to open a dialog
that asked the backend
+ // about workflow 0 and came back without a Private/Public choice (issue
#8599).
+ it.each([
+ ["the workflow has not loaded yet (wid 0)", 0],
+ ["there is no workflow id at all", undefined],
+ ])("does not open the share dialog while %s", async (_case, wid) => {
+ vi.spyOn(workflowPersistService,
"retrieveOwners").mockReturnValue(of([]));
+ const createSpy = vi
+ .spyOn(modalService, "create")
+ .mockReturnValue({ afterClose: of(undefined) } as unknown as
NzModalRef);
+ component.workflowId = wid;
+
+ await component.onClickOpenShareAccess();
+
+ expect(createSpy).not.toHaveBeenCalled();
+ });
});
it("onClickCreateNewWorkflow resets the graph and navigates back to root",
() => {
@@ -1445,6 +1465,22 @@ describe("MenuComponent", () => {
vi.restoreAllMocks();
});
+ // Until the workflow arrives the id is DEFAULT_WORKFLOW's, and the dialog
opened on it asks the
+ // backend about workflow 0 and comes back without a Private/Public choice
(issue #8599).
+ it("disables the Share button until the workflow id arrives", () => {
+ component.workflowId = undefined;
+ fixture.detectChanges();
+ expect(q("#share-button").nativeElement.disabled).toBe(true);
+
+ component.workflowId = 0;
+ fixture.detectChanges();
+ expect(q("#share-button").nativeElement.disabled).toBe(true);
+
+ component.workflowId = 7;
+ fixture.detectChanges();
+ expect(q("#share-button").nativeElement.disabled).toBe(false);
+ });
+
describe("view switch", () => {
const flag = (formViewEnabled: boolean) =>
(TestBed.inject(GuiConfigService) as unknown as
MockGuiConfigService).setConfig({ formViewEnabled });
diff --git a/frontend/src/app/workspace/component/menu/menu.component.ts
b/frontend/src/app/workspace/component/menu/menu.component.ts
index 2ad2572ddd..cc1fa2c727 100644
--- a/frontend/src/app/workspace/component/menu/menu.component.ts
+++ b/frontend/src/app/workspace/component/menu/menu.component.ts
@@ -344,7 +344,16 @@ export class MenuComponent implements OnInit, OnDestroy {
});
}
+ /**
+ * The workflow id only arrives with the workflow: the canvas resets to
`DEFAULT_WORKFLOW` (wid 0)
+ * on every load, so until the fetch lands there is nothing to share.
Opening the dialog in that
+ * window asked the backend about workflow 0 and came back without a
Private/Public choice, which
+ * is the gesture issue #8599 reports. The button is disabled for the same
window.
+ */
public async onClickOpenShareAccess(): Promise<void> {
+ if (!this.workflowId) {
+ return;
+ }
const modalRef = this.modalService.create({
nzContent: ShareAccessComponent,
nzData: {
diff --git
a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts
b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts
index 0c3e5e1f8a..18088fdaf7 100644
---
a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts
+++
b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts
@@ -256,6 +256,22 @@ describe("WorkflowFormComponent", () => {
expect(h.workflowConsoleService.clearConsoleMessages).toHaveBeenCalled();
expect(h.workflowResultService.clearResults).toHaveBeenCalled();
});
+
+ // The canvas switch is a full-page navigation, and the browser may keep
this document in its
+ // back/forward cache. Coming back restores the JavaScript state as it was
left and re-runs
+ // nothing, so anything torn down on the way out would stay torn down on a
page that still
+ // looks live (issue #8599).
+ it("tears nothing down on beforeunload, so a page restored from the cache
still works", () => {
+ build(formViewWorkflow).ngOnInit();
+
+ component.onBeforeUnload();
+
+ expect(workflowActionService.clearWorkflow).not.toHaveBeenCalled();
+ expect(h.computingUnitStatusService.disconnect).not.toHaveBeenCalled();
+
expect(h.executeWorkflowService.resetExecutionAndWorkers).not.toHaveBeenCalled();
+
expect(h.workflowConsoleService.clearConsoleMessages).not.toHaveBeenCalled();
+ expect(h.workflowResultService.clearResults).not.toHaveBeenCalled();
+ });
});
describe("title bar and saving", () => {
diff --git
a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts
b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts
index 0c3c182b12..95b58b0b8d 100644
---
a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts
+++
b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts
@@ -1977,13 +1977,27 @@ export class WorkflowFormComponent implements OnInit,
OnDestroy {
}
}
+ /**
+ * The browser is leaving this document: save, and change nothing else.
+ *
+ * The canvas switch is a full-page navigation, and the browser may keep
this document in its
+ * back/forward cache rather than discarding it. Coming back restores the
JavaScript state as it
+ * was left, and nothing re-runs, so anything torn down here would stay torn
down on a page that
+ * looks live. A document that really is discarded takes its websockets and
its graph with it, so
+ * there is nothing to tear down on the way out either way.
+ */
+ @HostListener("window:beforeunload")
+ onBeforeUnload(): void {
+ // The queue is deliberately left open: a document restored from the cache
goes on using it.
+ this.save();
+ }
+
/**
* Tear down exactly what the operator canvas tears down: both views drive
the same
* singleton services, so anything left bound here follows the user to the
next page
* (the symptom was a frozen canvas after a visit -- the old shared model
still attached).
* On the way out, save once more so a last edit is not lost.
*/
- @HostListener("window:beforeunload")
ngOnDestroy(): void {
this.destroyed = true;
// The final save joins the queue behind anything still in flight, then
the queue is closed: the
diff --git
a/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts
b/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts
index 0cbc713d49..8dc429dc27 100644
---
a/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts
+++
b/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts
@@ -869,14 +869,20 @@ describe("WorkflowFormComponent (rendered template)", ()
=> {
expect(el("texera-property-editor")!.hasAttribute("inert")).toBe(false);
});
- it("tears the workflow down when the browser unloads (the beforeunload host
binding)", () => {
+ // Dispatching the DOM event, rather than calling the handler, is what would
catch the host
+ // binding being removed or miswired. What it must do is save and nothing
else: the browser may
+ // keep this document in its back/forward cache, and coming back re-runs
nothing, so a teardown
+ // here would leave a page that looks live and is not (issue #8599).
+ it("saves and tears nothing down when the browser unloads (the beforeunload
host binding)", () => {
fixture.detectChanges();
finishLoad();
const workflowActionService: any = TestBed.inject(WorkflowActionService);
+ const save = vi.spyOn(fixture.componentInstance as any, "save");
window.dispatchEvent(new Event("beforeunload"));
- expect(workflowActionService.clearWorkflow).toHaveBeenCalled();
+ expect(save).toHaveBeenCalled();
+ expect(workflowActionService.clearWorkflow).not.toHaveBeenCalled();
});
// The held rebuild is drained by a real blur: a focusout bubbling up from a
control inside the
diff --git a/frontend/src/app/workspace/component/workspace.component.spec.ts
b/frontend/src/app/workspace/component/workspace.component.spec.ts
index 839358b854..c651f37daa 100644
--- a/frontend/src/app/workspace/component/workspace.component.spec.ts
+++ b/frontend/src/app/workspace/component/workspace.component.spec.ts
@@ -497,6 +497,37 @@ describe("WorkspaceComponent", () => {
expect(workflowResultService.clearResults).toHaveBeenCalled();
});
+ // A full-page navigation away fires beforeunload, and the browser may
then keep this document
+ // in its back/forward cache instead of discarding it. Coming back
restores the JavaScript
+ // state as it was left and re-runs nothing, so anything torn down here
stays torn down: the
+ // graph came back empty, the workflow id came back as the default, and
the still-subscribed
+ // autosave then wrote that default out as a new, blank workflow (issue
#8599).
+ // Dispatching the DOM event, rather than calling the handler, is what
would catch the host
+ // binding being removed or miswired.
+ it("saves on beforeunload and tears nothing down, so a page restored from
the cache still works", async () => {
+ await createFixture();
+ fixture.detectChanges();
+
+ window.dispatchEvent(new Event("beforeunload"));
+
+
expect(workflowPersistService.persistWorkflow).toHaveBeenCalledWith(stubWorkflow);
+ expect(workflowActionService.clearWorkflow).not.toHaveBeenCalled();
+ expect(computingUnitStatusService.disconnect).not.toHaveBeenCalled();
+
expect(executeWorkflowService.resetExecutionAndWorkers).not.toHaveBeenCalled();
+
expect(workflowConsoleService.clearConsoleMessages).not.toHaveBeenCalled();
+ expect(workflowResultService.clearResults).not.toHaveBeenCalled();
+ });
+
+ it("skips even the save on beforeunload when the user is not signed in",
async () => {
+ await createFixture();
+ fixture.detectChanges();
+ userService.isLogin.mockReturnValue(false);
+
+ component.onBeforeUnload();
+
+ expect(workflowPersistService.persistWorkflow).not.toHaveBeenCalled();
+ });
+
it("clears the workflow session state when the computing unit is switched
in-canvas (issue #3120)", async () => {
await createFixture();
fixture.detectChanges();
diff --git a/frontend/src/app/workspace/component/workspace.component.ts
b/frontend/src/app/workspace/component/workspace.component.ts
index a4ba9baa5d..f83f43eae5 100644
--- a/frontend/src/app/workspace/component/workspace.component.ts
+++ b/frontend/src/app/workspace/component/workspace.component.ts
@@ -173,13 +173,27 @@ export class WorkspaceComponent implements AfterViewInit,
OnInit, OnDestroy {
this.codeEditorService.vc = this.codeEditorViewRef;
}
+ /**
+ * The browser is leaving this document: save the workflow, and change
nothing else.
+ *
+ * Tearing the session down here was the cause of a page that came back
dead. A full-page
+ * navigation away (the Form View switch is one) fires this, and the browser
may then keep the
+ * document in its back/forward cache rather than discarding it. Coming back
restores the
+ * JavaScript state exactly as it was left, so whatever this method had
already destroyed stayed
+ * destroyed: an empty graph on a canvas that answered no clicks, and a
workflow id reset to the
+ * default, which the share dialog then asked the backend about and got an
error for. Nothing
+ * re-runs on a restore, because the component was never re-created.
+ *
+ * There is nothing to tear down on the way out anyway. A document that is
really discarded takes
+ * its websockets and its graph with it, and a document that comes back
needs them.
+ */
@HostListener("window:beforeunload")
- ngOnDestroy() {
- if (this.userService.isLogin() &&
this.workflowPersistService.isWorkflowPersistEnabled()) {
- const workflow = this.workflowActionService.getWorkflow();
-
this.workflowPersistService.persistWorkflow(workflow).pipe(untilDestroyed(this)).subscribe();
- }
+ onBeforeUnload(): void {
+ this.persistBeforeLeaving();
+ }
+ ngOnDestroy() {
+ this.persistBeforeLeaving();
this.codeEditorViewRef.clear();
this.workflowActionService.clearWorkflow();
// Tear down the connection and all websocket-derived session state so a
@@ -188,6 +202,13 @@ export class WorkspaceComponent implements AfterViewInit,
OnInit, OnDestroy {
this.resetWorkflowSessionState();
}
+ private persistBeforeLeaving(): void {
+ if (this.userService.isLogin() &&
this.workflowPersistService.isWorkflowPersistEnabled()) {
+ const workflow = this.workflowActionService.getWorkflow();
+
this.workflowPersistService.persistWorkflow(workflow).pipe(untilDestroyed(this)).subscribe();
+ }
+ }
+
/**
* Clear websocket-derived session state (execution status, console,
results).
* Shared by workspace teardown and in-canvas unit switches.
