From f995c569ab9c7bacc8cd335a5056044d78c20c09 Mon Sep 17 00:00:00 2001 From: Matthew Ball Date: Sat, 20 Jun 2026 23:24:25 -0700 Subject: [PATCH 01/10] Added a deployment version service to check the running deployment vs the build --- frontend/.gitignore | 1 + frontend/build-version.js | 6 + frontend/src/app/app.component.ts | 12 +- .../deployment-version.service.spec.ts | 183 ++++++++++++++++++ .../deployment-version.service.ts | 70 +++++++ 5 files changed, 271 insertions(+), 1 deletion(-) create mode 100644 frontend/src/app/common/service/deployment-version/deployment-version.service.spec.ts create mode 100644 frontend/src/app/common/service/deployment-version/deployment-version.service.ts diff --git a/frontend/.gitignore b/frontend/.gitignore index d5db9ad9937..c3db340e81d 100644 --- a/frontend/.gitignore +++ b/frontend/.gitignore @@ -33,3 +33,4 @@ # Generated by build-version.js on prod builds. /src/environments/version.prod.ts +/src/assets/version.json diff --git a/frontend/build-version.js b/frontend/build-version.js index 089c0dcb455..6913f9f90d4 100644 --- a/frontend/build-version.js +++ b/frontend/build-version.js @@ -38,4 +38,10 @@ export const Version = { }; `, ); + +// Served at /assets/version.json so the running app can poll for the +// deployed build number and detect a new deployment at runtime. +const manifest = resolve(__dirname, "src", "assets", "version.json"); +writeFileSync(manifest, JSON.stringify({ buildNumber, version }) + "\n"); + console.log(`build-version: ${buildNumber}`); diff --git a/frontend/src/app/app.component.ts b/frontend/src/app/app.component.ts index 513b05b6985..f6dd1a4efee 100644 --- a/frontend/src/app/app.component.ts +++ b/frontend/src/app/app.component.ts @@ -19,6 +19,8 @@ import { Component } from "@angular/core"; import { GuiConfigService } from "./common/service/gui-config.service"; +import { DeploymentVersionService } from "./common/service/deployment-version/deployment-version.service"; +import { Version } from "../environments/version"; import { UntilDestroy } from "@ngneat/until-destroy"; @UntilDestroy() @@ -40,7 +42,10 @@ import { UntilDestroy } from "@ngneat/until-destroy"; export class AppComponent { configLoaded = false; - constructor(private config: GuiConfigService) { + constructor( + private config: GuiConfigService, + private deploymentVersion: DeploymentVersionService + ) { // determine whether configuration was successfully loaded by APP_INITIALIZER try { // accessing env will throw if not loaded @@ -49,6 +54,11 @@ export class AppComponent { } catch { this.configLoaded = false; } + + // Skip the "dev" placeholder build, where no deployments occur. + if (Version.buildNumber !== "dev") { + this.deploymentVersion.start(); + } } retry(): void { diff --git a/frontend/src/app/common/service/deployment-version/deployment-version.service.spec.ts b/frontend/src/app/common/service/deployment-version/deployment-version.service.spec.ts new file mode 100644 index 00000000000..dff89b44c8b --- /dev/null +++ b/frontend/src/app/common/service/deployment-version/deployment-version.service.spec.ts @@ -0,0 +1,183 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { HttpClientTestingModule, HttpTestingController } from "@angular/common/http/testing"; +import { TestBed, fakeAsync, tick } from "@angular/core/testing"; +import { NzNotificationDataOptions } from "ng-zorro-antd/notification"; +import { NotificationService } from "../notification/notification.service"; +import { DeploymentVersionService, VERSION_MANIFEST_URL, VERSION_POLL_INTERVAL_MS } from "./deployment-version.service"; + +// Records blank() calls so the prompt is observable without a spy framework. +class FakeNotificationService { + public blankCalls: { title: string; content: string; options: NzNotificationDataOptions }[] = []; + blank(title: string, content: string, options: NzNotificationDataOptions = {}): void { + this.blankCalls.push({ title, content, options }); + } +} + +describe("DeploymentVersionService", () => { + let service: DeploymentVersionService; + let httpMock: HttpTestingController; + let notification: FakeNotificationService; + + beforeEach(() => { + TestBed.configureTestingModule({ + imports: [HttpClientTestingModule], + providers: [{ provide: NotificationService, useClass: FakeNotificationService }], + }); + service = TestBed.inject(DeploymentVersionService); + httpMock = TestBed.inject(HttpTestingController); + notification = TestBed.inject(NotificationService) as unknown as FakeNotificationService; + }); + + afterEach(() => httpMock.verify()); + + function takeManifestRequest() { + return httpMock.expectOne(req => req.url === VERSION_MANIFEST_URL); + } + + function check(): { value: boolean | undefined } { + const out: { value: boolean | undefined } = { value: undefined }; + service.checkForUpdate().subscribe(v => (out.value = v)); + return out; + } + + describe("checkForUpdate (positive)", () => { + it("reports an update when the deployed build differs from the running one", () => { + const out = check(); + takeManifestRequest().flush({ buildNumber: "different-build-123" }); + expect(out.value).toBe(true); + }); + }); + + describe("checkForUpdate (no update / negative)", () => { + it("reports no update when the build matches the running one", () => { + const out = check(); + // Version.buildNumber is "dev" under test (the non-replaced version.ts). + takeManifestRequest().flush({ buildNumber: "dev" }); + expect(out.value).toBe(false); + }); + }); + + describe("checkForUpdate (malformed manifest)", () => { + it("ignores a manifest with no buildNumber field", () => { + const out = check(); + takeManifestRequest().flush({}); + expect(out.value).toBe(false); + }); + + it("ignores an empty-string buildNumber", () => { + const out = check(); + takeManifestRequest().flush({ buildNumber: "" }); + expect(out.value).toBe(false); + }); + + it("ignores a non-string buildNumber", () => { + const out = check(); + takeManifestRequest().flush({ buildNumber: 12345 }); + expect(out.value).toBe(false); + }); + + it("ignores a null response body", () => { + const out = check(); + takeManifestRequest().flush(null); + expect(out.value).toBe(false); + }); + }); + + describe("checkForUpdate (transport failures stay silent)", () => { + it("returns false on a network error", () => { + const out = check(); + takeManifestRequest().error(new ProgressEvent("error")); + expect(out.value).toBe(false); + }); + + it("returns false on a 404 (manifest not deployed)", () => { + const out = check(); + takeManifestRequest().flush("not found", { status: 404, statusText: "Not Found" }); + expect(out.value).toBe(false); + }); + + it("returns false on a 500 server error", () => { + const out = check(); + takeManifestRequest().flush("boom", { status: 500, statusText: "Server Error" }); + expect(out.value).toBe(false); + }); + }); + + describe("checkForUpdate (request shape)", () => { + it("requests the manifest with a cache-busting query param so a CDN/browser cache cannot mask a deploy", () => { + check(); + const req = takeManifestRequest(); + expect(req.request.method).toBe("GET"); + expect(req.request.params.has("t")).toBe(true); + expect(req.request.params.get("t")).toBeTruthy(); + req.flush({ buildNumber: "dev" }); + }); + }); + + describe("promptReload", () => { + it("shows exactly one sticky, dismissible notification with a refresh message", () => { + service.promptReload(); + expect(notification.blankCalls.length).toBe(1); + const call = notification.blankCalls[0]; + expect(call.options.nzDuration).toBe(0); + expect(call.title.length).toBeGreaterThan(0); + expect(call.content.toLowerCase()).toContain("refresh"); + }); + }); + + describe("start (polling)", () => { + it("polls after the interval and prompts once when a new deployment is detected", fakeAsync(() => { + const sub = service.start(1000); + expect(notification.blankCalls.length).toBe(0); // nothing before the first interval + tick(1000); + takeManifestRequest().flush({ buildNumber: "new-build" }); + expect(notification.blankCalls.length).toBe(1); + sub.unsubscribe(); + })); + + it("does not prompt while the deployed build is unchanged", fakeAsync(() => { + const sub = service.start(1000); + tick(1000); + takeManifestRequest().flush({ buildNumber: "dev" }); + expect(notification.blankCalls.length).toBe(0); + tick(1000); + takeManifestRequest().flush({ buildNumber: "dev" }); + expect(notification.blankCalls.length).toBe(0); + sub.unsubscribe(); + })); + + it("prompts only once and stops polling after an update is found", fakeAsync(() => { + const sub = service.start(1000); + tick(1000); + takeManifestRequest().flush({ buildNumber: "new-build" }); + expect(notification.blankCalls.length).toBe(1); + tick(1000); + // take(1) completed the stream: no further polling. + httpMock.expectNone(req => req.url === VERSION_MANIFEST_URL); + expect(notification.blankCalls.length).toBe(1); + sub.unsubscribe(); + })); + + it("uses a 5 minute default poll interval", () => { + expect(VERSION_POLL_INTERVAL_MS).toBe(5 * 60 * 1000); + }); + }); +}); diff --git a/frontend/src/app/common/service/deployment-version/deployment-version.service.ts b/frontend/src/app/common/service/deployment-version/deployment-version.service.ts new file mode 100644 index 00000000000..fba682f2af9 --- /dev/null +++ b/frontend/src/app/common/service/deployment-version/deployment-version.service.ts @@ -0,0 +1,70 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { HttpClient } from "@angular/common/http"; +import { Injectable } from "@angular/core"; +import { Observable, Subscription, of, timer } from "rxjs"; +import { catchError, filter, map, switchMap, take } from "rxjs/operators"; +import { Version } from "../../../../environments/version"; +import { NotificationService } from "../notification/notification.service"; + +export const VERSION_MANIFEST_URL = "assets/version.json"; +export const VERSION_POLL_INTERVAL_MS = 5 * 60 * 1000; + +@Injectable({ + providedIn: "root", +}) +export class DeploymentVersionService { + constructor( + private http: HttpClient, + private notification: NotificationService + ) {} + + // True when the deployed build's buildNumber differs from the running one. + checkForUpdate(): Observable { + return this.http + .get<{ buildNumber?: string }>(VERSION_MANIFEST_URL, { params: { t: Date.now().toString() } }) + .pipe( + map(manifest => { + const deployed = manifest?.buildNumber; + return typeof deployed === "string" && deployed.length > 0 && deployed !== Version.buildNumber; + }), + catchError(() => of(false)) + ); + } + + // Poll until a new deployment is detected, then prompt once. + start(intervalMs: number = VERSION_POLL_INTERVAL_MS): Subscription { + return timer(intervalMs, intervalMs) + .pipe( + switchMap(() => this.checkForUpdate()), + filter(updated => updated), + take(1) + ) + .subscribe(() => this.promptReload()); + } + + promptReload(): void { + this.notification.blank( + "New version available", + "A new version of Texera is available. Refresh the page to update.", + { nzDuration: 0 } + ); + } +} From 2c873f95a8ffc12942893f9e9215f2f0723a17e1 Mon Sep 17 00:00:00 2001 From: Matthew Ball Date: Sat, 20 Jun 2026 23:30:36 -0700 Subject: [PATCH 02/10] lint --- .../deployment-version.service.ts | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/frontend/src/app/common/service/deployment-version/deployment-version.service.ts b/frontend/src/app/common/service/deployment-version/deployment-version.service.ts index fba682f2af9..1bf636b822c 100644 --- a/frontend/src/app/common/service/deployment-version/deployment-version.service.ts +++ b/frontend/src/app/common/service/deployment-version/deployment-version.service.ts @@ -38,15 +38,13 @@ export class DeploymentVersionService { // True when the deployed build's buildNumber differs from the running one. checkForUpdate(): Observable { - return this.http - .get<{ buildNumber?: string }>(VERSION_MANIFEST_URL, { params: { t: Date.now().toString() } }) - .pipe( - map(manifest => { - const deployed = manifest?.buildNumber; - return typeof deployed === "string" && deployed.length > 0 && deployed !== Version.buildNumber; - }), - catchError(() => of(false)) - ); + return this.http.get<{ buildNumber?: string }>(VERSION_MANIFEST_URL, { params: { t: Date.now().toString() } }).pipe( + map(manifest => { + const deployed = manifest?.buildNumber; + return typeof deployed === "string" && deployed.length > 0 && deployed !== Version.buildNumber; + }), + catchError(() => of(false)) + ); } // Poll until a new deployment is detected, then prompt once. From 0317dce567b4f1a4234526e7e1927e96e00ca178 Mon Sep 17 00:00:00 2001 From: Matthew Ball Date: Sun, 21 Jun 2026 01:59:15 -0700 Subject: [PATCH 03/10] added test cases and only poll when a new deployment is detected --- frontend/src/app/app.component.spec.ts | 139 ++++++++++++++++++ .../deployment-version.service.spec.ts | 48 ++++++ .../deployment-version.service.ts | 12 +- 3 files changed, 197 insertions(+), 2 deletions(-) create mode 100644 frontend/src/app/app.component.spec.ts diff --git a/frontend/src/app/app.component.spec.ts b/frontend/src/app/app.component.spec.ts new file mode 100644 index 00000000000..4e906ede038 --- /dev/null +++ b/frontend/src/app/app.component.spec.ts @@ -0,0 +1,139 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { CommonModule } from "@angular/common"; +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { RouterTestingModule } from "@angular/router/testing"; +import { AppComponent } from "./app.component"; +import { GuiConfigService } from "./common/service/gui-config.service"; +import { DeploymentVersionService } from "./common/service/deployment-version/deployment-version.service"; +import { Version } from "../environments/version"; + +// GuiConfigService stub whose env getter either returns a value or throws, +// mirroring "config loaded" vs "config failed to load by APP_INITIALIZER". +class StubGuiConfigService { + shouldThrow = false; + get env(): unknown { + if (this.shouldThrow) { + throw new Error("config not loaded"); + } + return {}; + } +} + +// Records start() invocations without a spy framework. +class FakeDeploymentVersionService { + startCalls = 0; + start(): void { + this.startCalls++; + } +} + +describe("AppComponent", () => { + let config: StubGuiConfigService; + let deployment: FakeDeploymentVersionService; + + beforeEach(() => { + Version.buildNumber = "dev"; + config = new StubGuiConfigService(); + deployment = new FakeDeploymentVersionService(); + + TestBed.configureTestingModule({ + imports: [CommonModule, RouterTestingModule], + declarations: [AppComponent], + providers: [ + { provide: GuiConfigService, useValue: config }, + { provide: DeploymentVersionService, useValue: deployment }, + ], + }); + }); + + // Version is a shared module singleton; restore the dev default so a test + // that flips buildNumber cannot leak into other suites in the same worker. + afterEach(() => { + Version.buildNumber = "dev"; + }); + + function create(): ComponentFixture { + return TestBed.createComponent(AppComponent); + } + + describe("config-loaded detection", () => { + it("marks config as loaded when env is accessible", () => { + config.shouldThrow = false; + const component = create().componentInstance; + expect(component.configLoaded).toBe(true); + }); + + it("marks config as not loaded when accessing env throws", () => { + config.shouldThrow = true; + const component = create().componentInstance; + expect(component.configLoaded).toBe(false); + }); + + it("renders the configuration-error panel when config is not loaded", () => { + config.shouldThrow = true; + const fixture = create(); + fixture.detectChanges(); + const el: HTMLElement = fixture.nativeElement; + expect(el.querySelector("#config-error")).not.toBeNull(); + }); + + it("does not render the configuration-error panel when config is loaded", () => { + config.shouldThrow = false; + const fixture = create(); + fixture.detectChanges(); + const el: HTMLElement = fixture.nativeElement; + expect(el.querySelector("#config-error")).toBeNull(); + }); + }); + + describe("deployment-version polling guard", () => { + it("does not start polling for the 'dev' placeholder build", () => { + Version.buildNumber = "dev"; + create(); + expect(deployment.startCalls).toBe(0); + }); + + it("starts polling for a real (non-'dev') build", () => { + Version.buildNumber = "prod-build-123"; + create(); + expect(deployment.startCalls).toBe(1); + }); + }); + + describe("retry", () => { + it("reloads the page", () => { + const reload = vi.fn(); + // location.reload itself is non-configurable, so swap the whole + // location object for the duration of the test. + const original = window.location; + Object.defineProperty(window, "location", { + configurable: true, + value: { ...original, reload }, + }); + try { + create().componentInstance.retry(); + expect(reload).toHaveBeenCalledTimes(1); + } finally { + Object.defineProperty(window, "location", { configurable: true, value: original }); + } + }); + }); +}); diff --git a/frontend/src/app/common/service/deployment-version/deployment-version.service.spec.ts b/frontend/src/app/common/service/deployment-version/deployment-version.service.spec.ts index dff89b44c8b..0cfccc4870f 100644 --- a/frontend/src/app/common/service/deployment-version/deployment-version.service.spec.ts +++ b/frontend/src/app/common/service/deployment-version/deployment-version.service.spec.ts @@ -179,5 +179,53 @@ describe("DeploymentVersionService", () => { it("uses a 5 minute default poll interval", () => { expect(VERSION_POLL_INTERVAL_MS).toBe(5 * 60 * 1000); }); + + it("does not poll before the default 5 minute interval elapses", fakeAsync(() => { + const sub = service.start(); + tick(VERSION_POLL_INTERVAL_MS - 1); + httpMock.expectNone(req => req.url === VERSION_MANIFEST_URL); + sub.unsubscribe(); + })); + + it("keeps polling and still prompts after a transient request failure", fakeAsync(() => { + const sub = service.start(1000); + tick(1000); + // First poll fails at the transport level: the stream must survive it. + takeManifestRequest().error(new ProgressEvent("error")); + expect(notification.blankCalls.length).toBe(0); + tick(1000); + takeManifestRequest().flush({ buildNumber: "new-build" }); + expect(notification.blankCalls.length).toBe(1); + sub.unsubscribe(); + })); + }); + + describe("start (idempotency)", () => { + it("returns the in-flight subscription instead of stacking a second poller", fakeAsync(() => { + const first = service.start(1000); + const second = service.start(1000); + expect(second).toBe(first); + tick(1000); + // Only one poller is active, so only one manifest request is issued. + takeManifestRequest().flush({ buildNumber: "new-build" }); + expect(notification.blankCalls.length).toBe(1); + first.unsubscribe(); + })); + + it("starts a fresh poller once the previous run has completed", fakeAsync(() => { + const first = service.start(1000); + tick(1000); + // take(1) completes the first run after the update is detected. + takeManifestRequest().flush({ buildNumber: "new-build" }); + expect(notification.blankCalls.length).toBe(1); + + // A subsequent start() is no longer a no-op: the prior run is closed. + const second = service.start(1000); + expect(second).not.toBe(first); + tick(1000); + takeManifestRequest().flush({ buildNumber: "another-new-build" }); + expect(notification.blankCalls.length).toBe(2); + second.unsubscribe(); + })); }); }); diff --git a/frontend/src/app/common/service/deployment-version/deployment-version.service.ts b/frontend/src/app/common/service/deployment-version/deployment-version.service.ts index 1bf636b822c..add60faa8f6 100644 --- a/frontend/src/app/common/service/deployment-version/deployment-version.service.ts +++ b/frontend/src/app/common/service/deployment-version/deployment-version.service.ts @@ -31,6 +31,8 @@ export const VERSION_POLL_INTERVAL_MS = 5 * 60 * 1000; providedIn: "root", }) export class DeploymentVersionService { + private polling?: Subscription; + constructor( private http: HttpClient, private notification: NotificationService @@ -47,15 +49,21 @@ export class DeploymentVersionService { ); } - // Poll until a new deployment is detected, then prompt once. + // Poll until a new deployment is detected, then prompt once. Idempotent: + // a second call while already polling returns the in-flight subscription + // rather than stacking a duplicate poller (and a duplicate prompt). start(intervalMs: number = VERSION_POLL_INTERVAL_MS): Subscription { - return timer(intervalMs, intervalMs) + if (this.polling && !this.polling.closed) { + return this.polling; + } + this.polling = timer(intervalMs, intervalMs) .pipe( switchMap(() => this.checkForUpdate()), filter(updated => updated), take(1) ) .subscribe(() => this.promptReload()); + return this.polling; } promptReload(): void { From 07f2c7944f572794934a47e6efb0073e120c7b76 Mon Sep 17 00:00:00 2001 From: Matthew Ball Date: Sun, 21 Jun 2026 02:31:04 -0700 Subject: [PATCH 04/10] added build version types --- frontend/build-version.d.ts | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 frontend/build-version.d.ts diff --git a/frontend/build-version.d.ts b/frontend/build-version.d.ts new file mode 100644 index 00000000000..379241c3b63 --- /dev/null +++ b/frontend/build-version.d.ts @@ -0,0 +1,24 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// Types for build-version.js. +export function renderVersionArtifacts( + version: string, + buildNumber?: string +): { buildNumber: string; prodTs: string; manifestJson: string }; From 786d24736f2880e011720c36ba7dffb96a15e250 Mon Sep 17 00:00:00 2001 From: Matthew Ball Date: Sun, 21 Jun 2026 02:32:00 -0700 Subject: [PATCH 05/10] apply auto generated version --- frontend/build-version.js | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/frontend/build-version.js b/frontend/build-version.js index 6913f9f90d4..e66586d5c0e 100644 --- a/frontend/build-version.js +++ b/frontend/build-version.js @@ -23,25 +23,32 @@ // Dev builds (`yarn start`) keep the static "dev" string in version.ts. const { generate } = require("build-number-generator"); -const { version } = require("./package.json"); const { resolve } = require("path"); const { writeFileSync } = require("fs"); -const buildNumber = generate(version); -const out = resolve(__dirname, "src", "environments", "version.prod.ts"); -writeFileSync( - out, - `// AUTO-GENERATED by build-version.js — do not edit or commit. +// Returns the contents of version.prod.ts and version.json for a version. +function renderVersionArtifacts(version, buildNumber = generate(version)) { + const prodTs = `// AUTO-GENERATED by build-version.js — do not edit or commit. export const Version = { buildNumber: ${JSON.stringify(buildNumber)}, version: ${JSON.stringify(version)}, }; -`, -); +`; + const manifestJson = JSON.stringify({ buildNumber, version }) + "\n"; + return { buildNumber, prodTs, manifestJson }; +} -// Served at /assets/version.json so the running app can poll for the -// deployed build number and detect a new deployment at runtime. -const manifest = resolve(__dirname, "src", "assets", "version.json"); -writeFileSync(manifest, JSON.stringify({ buildNumber, version }) + "\n"); +function main() { + const { version } = require("./package.json"); + const { buildNumber, prodTs, manifestJson } = renderVersionArtifacts(version); + writeFileSync(resolve(__dirname, "src", "environments", "version.prod.ts"), prodTs); + writeFileSync(resolve(__dirname, "src", "assets", "version.json"), manifestJson); + console.log(`build-version: ${buildNumber}`); +} -console.log(`build-version: ${buildNumber}`); +module.exports = { renderVersionArtifacts }; + +// Write files only when run directly, not when required by a test. +if (require.main === module) { + main(); +} From 88a7e8ace652c005b55a22be882741fd36013fcd Mon Sep 17 00:00:00 2001 From: Matthew Ball Date: Sun, 21 Jun 2026 02:32:06 -0700 Subject: [PATCH 06/10] added test cases --- frontend/src/build-version.spec.ts | 79 ++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 frontend/src/build-version.spec.ts diff --git a/frontend/src/build-version.spec.ts b/frontend/src/build-version.spec.ts new file mode 100644 index 00000000000..a27f1f6f29b --- /dev/null +++ b/frontend/src/build-version.spec.ts @@ -0,0 +1,79 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { renderVersionArtifacts } from "../build-version"; + +describe("build-version: renderVersionArtifacts", () => { + const VERSION = "1.2.3-incubating"; + const BUILD = "1.2.3-incubating.250101010"; + + describe("with an explicit build number (deterministic)", () => { + it("embeds both the build number and version into version.prod.ts", () => { + const { prodTs } = renderVersionArtifacts(VERSION, BUILD); + expect(prodTs).toContain("export const Version"); + expect(prodTs).toContain(`buildNumber: "${BUILD}"`); + expect(prodTs).toContain(`version: "${VERSION}"`); + }); + + it("marks version.prod.ts as auto-generated so it is not hand-edited", () => { + const { prodTs } = renderVersionArtifacts(VERSION, BUILD); + expect(prodTs).toContain("AUTO-GENERATED"); + expect(prodTs.endsWith("\n")).toBe(true); + }); + + it("produces a manifest that the running app can parse, carrying the same build number", () => { + const { manifestJson, buildNumber } = renderVersionArtifacts(VERSION, BUILD); + expect(buildNumber).toBe(BUILD); + expect(manifestJson.endsWith("\n")).toBe(true); + expect(JSON.parse(manifestJson)).toEqual({ buildNumber: BUILD, version: VERSION }); + }); + + it("keeps the bundle's build number and the manifest's build number identical (the comparison the service relies on)", () => { + const { prodTs, manifestJson } = renderVersionArtifacts(VERSION, BUILD); + expect(prodTs).toContain(`"${BUILD}"`); + expect(JSON.parse(manifestJson).buildNumber).toBe(BUILD); + }); + + it("emits a TypeScript module that can be evaluated to the expected Version object", () => { + const { prodTs } = renderVersionArtifacts(VERSION, BUILD); + const evaluated = new Function(`${prodTs.replace("export ", "")} return Version;`)(); + expect(evaluated).toEqual({ buildNumber: BUILD, version: VERSION }); + }); + }); + + describe("string-safety of interpolated values", () => { + it("JSON-escapes a version containing quotes so the generated module stays valid", () => { + const tricky = '1.0.0"; throw new Error("x'; + const { prodTs } = renderVersionArtifacts(tricky, BUILD); + const evaluated = new Function(`${prodTs.replace("export ", "")} return Version;`)(); + expect(evaluated.version).toBe(tricky); + }); + }); + + describe("with a generated build number (default argument)", () => { + it("derives a non-empty build number from the version when none is supplied", () => { + const { buildNumber, manifestJson } = renderVersionArtifacts(VERSION); + expect(typeof buildNumber).toBe("string"); + expect(buildNumber.length).toBeGreaterThan(0); + // build-number-generator prefixes the generated number with the version. + expect(buildNumber.startsWith(VERSION)).toBe(true); + expect(JSON.parse(manifestJson).buildNumber).toBe(buildNumber); + }); + }); +}); From bf95cab23d5e469434f2b9435860e51b7fb7c30b Mon Sep 17 00:00:00 2001 From: Matthew Ball Date: Mon, 22 Jun 2026 01:30:43 -0700 Subject: [PATCH 07/10] added deployment version check enabled that is set to off by default --- common/config/src/main/resources/gui.conf | 5 ++++ .../texera/common/config/GuiConfig.scala | 2 ++ .../service/resource/ConfigResource.scala | 1 + .../resource/ConfigResourceAuthSpec.scala | 1 + frontend/src/app/app.component.spec.ts | 27 ++++++++++++++++--- frontend/src/app/app.component.ts | 6 +++-- .../common/service/gui-config.service.mock.ts | 1 + frontend/src/app/common/type/gui-config.ts | 1 + 8 files changed, 38 insertions(+), 6 deletions(-) diff --git a/common/config/src/main/resources/gui.conf b/common/config/src/main/resources/gui.conf index a0673c9a3fa..87b341dfe5c 100644 --- a/common/config/src/main/resources/gui.conf +++ b/common/config/src/main/resources/gui.conf @@ -124,4 +124,9 @@ gui { # whether to show the "Powered by Texera" attribution link in the sidebar attribution-enabled = false attribution-enabled = ${?GUI_ATTRIBUTION_ENABLED} + + # whether to poll for new deployments and prompt the user to reload when a + # newer build is detected. Off by default; enable per-deployment. + deployment-version-check-enabled = false + deployment-version-check-enabled = ${?GUI_DEPLOYMENT_VERSION_CHECK_ENABLED} } \ No newline at end of file diff --git a/common/config/src/main/scala/org/apache/texera/common/config/GuiConfig.scala b/common/config/src/main/scala/org/apache/texera/common/config/GuiConfig.scala index 3b378de1275..e0a580ae78b 100644 --- a/common/config/src/main/scala/org/apache/texera/common/config/GuiConfig.scala +++ b/common/config/src/main/scala/org/apache/texera/common/config/GuiConfig.scala @@ -75,6 +75,8 @@ object GuiConfig { conf.getInt("gui.workflow-workspace.limit-columns") val guiAttributionEnabled: Boolean = conf.getBoolean("gui.attribution-enabled") + val guiDeploymentVersionCheckEnabled: Boolean = + conf.getBoolean("gui.deployment-version-check-enabled") val guiWorkflowWorkspacePythonNotebookMigrationEnabled: Boolean = conf.getBoolean("gui.workflow-workspace.python-notebook-migration-enabled") } diff --git a/config-service/src/main/scala/org/apache/texera/service/resource/ConfigResource.scala b/config-service/src/main/scala/org/apache/texera/service/resource/ConfigResource.scala index 55dc386a3ea..d3eecb3bab8 100644 --- a/config-service/src/main/scala/org/apache/texera/service/resource/ConfigResource.scala +++ b/config-service/src/main/scala/org/apache/texera/service/resource/ConfigResource.scala @@ -49,6 +49,7 @@ class ConfigResource { "password" -> GuiConfig.guiLoginDefaultLocalUserPassword ), "attributionEnabled" -> GuiConfig.guiAttributionEnabled, + "deploymentVersionCheckEnabled" -> GuiConfig.guiDeploymentVersionCheckEnabled, "inviteOnly" -> UserSystemConfig.inviteOnly ) diff --git a/config-service/src/test/scala/org/apache/texera/service/resource/ConfigResourceAuthSpec.scala b/config-service/src/test/scala/org/apache/texera/service/resource/ConfigResourceAuthSpec.scala index d5418ea0f70..8b24b812775 100644 --- a/config-service/src/test/scala/org/apache/texera/service/resource/ConfigResourceAuthSpec.scala +++ b/config-service/src/test/scala/org/apache/texera/service/resource/ConfigResourceAuthSpec.scala @@ -101,6 +101,7 @@ class ConfigResourceAuthSpec extends AnyFlatSpec with Matchers with BeforeAndAft "googleLogin", "defaultLocalUser", "attributionEnabled", + "deploymentVersionCheckEnabled", "inviteOnly" ) } diff --git a/frontend/src/app/app.component.spec.ts b/frontend/src/app/app.component.spec.ts index 4e906ede038..764bdcd0ddc 100644 --- a/frontend/src/app/app.component.spec.ts +++ b/frontend/src/app/app.component.spec.ts @@ -29,11 +29,12 @@ import { Version } from "../environments/version"; // mirroring "config loaded" vs "config failed to load by APP_INITIALIZER". class StubGuiConfigService { shouldThrow = false; + deploymentVersionCheckEnabled = true; get env(): unknown { if (this.shouldThrow) { throw new Error("config not loaded"); } - return {}; + return { deploymentVersionCheckEnabled: this.deploymentVersionCheckEnabled }; } } @@ -106,12 +107,28 @@ describe("AppComponent", () => { describe("deployment-version polling guard", () => { it("does not start polling for the 'dev' placeholder build", () => { + config.deploymentVersionCheckEnabled = true; Version.buildNumber = "dev"; create(); expect(deployment.startCalls).toBe(0); }); - it("starts polling for a real (non-'dev') build", () => { + it("does not start polling when the config flag is disabled", () => { + config.deploymentVersionCheckEnabled = false; + Version.buildNumber = "prod-build-123"; + create(); + expect(deployment.startCalls).toBe(0); + }); + + it("does not start polling when config failed to load", () => { + config.shouldThrow = true; + Version.buildNumber = "prod-build-123"; + create(); + expect(deployment.startCalls).toBe(0); + }); + + it("starts polling for a real build when the config flag is enabled", () => { + config.deploymentVersionCheckEnabled = true; Version.buildNumber = "prod-build-123"; create(); expect(deployment.startCalls).toBe(1); @@ -121,8 +138,10 @@ describe("AppComponent", () => { describe("retry", () => { it("reloads the page", () => { const reload = vi.fn(); - // location.reload itself is non-configurable, so swap the whole - // location object for the duration of the test. + // location.reload is a non-writable, non-configurable own property (and is + // absent from Location.prototype) under this jsdom build, so it cannot be + // spied or reassigned directly. window.location itself is configurable, + // so swap the whole object for the test, then restore it. const original = window.location; Object.defineProperty(window, "location", { configurable: true, diff --git a/frontend/src/app/app.component.ts b/frontend/src/app/app.component.ts index f6dd1a4efee..183952c3b1e 100644 --- a/frontend/src/app/app.component.ts +++ b/frontend/src/app/app.component.ts @@ -55,8 +55,10 @@ export class AppComponent { this.configLoaded = false; } - // Skip the "dev" placeholder build, where no deployments occur. - if (Version.buildNumber !== "dev") { + // Poll for new deployments only when the config opts in (off by default), + // config actually loaded, and this isn't the "dev" placeholder build where + // no deployments occur. + if (this.configLoaded && this.config.env.deploymentVersionCheckEnabled && Version.buildNumber !== "dev") { this.deploymentVersion.start(); } } diff --git a/frontend/src/app/common/service/gui-config.service.mock.ts b/frontend/src/app/common/service/gui-config.service.mock.ts index 0324b10eb67..9efbd75a12f 100644 --- a/frontend/src/app/common/service/gui-config.service.mock.ts +++ b/frontend/src/app/common/service/gui-config.service.mock.ts @@ -54,6 +54,7 @@ export class MockGuiConfigService { limitColumns: 15, attributionEnabled: false, pythonNotebookMigrationEnabled: false, + deploymentVersionCheckEnabled: false, }; get env(): GuiConfig { diff --git a/frontend/src/app/common/type/gui-config.ts b/frontend/src/app/common/type/gui-config.ts index abdb4067b2c..28df71b3654 100644 --- a/frontend/src/app/common/type/gui-config.ts +++ b/frontend/src/app/common/type/gui-config.ts @@ -45,6 +45,7 @@ export interface GuiConfig { limitColumns: number; attributionEnabled: boolean; pythonNotebookMigrationEnabled: boolean; + deploymentVersionCheckEnabled: boolean; } export interface SidebarTabs { From 6501b80819631086bcf5bd66ff0d2ac74b41474e Mon Sep 17 00:00:00 2001 From: Matthew Ball Date: Fri, 26 Jun 2026 01:54:45 -0700 Subject: [PATCH 08/10] address review feedback on version check refactor(frontend): address review feedback on deployment version check - Test the real NotificationService/DeploymentVersionService with vi.spyOn instead of hand-rolled fakes, stubbing only ng-zorro's lower-level services - Make the reload notification actionable: blank() returns the NzNotificationRef and promptReload() reloads the page on click (guarded with take(1)) - Rename start() -> startPollingForUpdates() for clearer intent - Rename AppComponent constructor params to configService/deploymentVersionService --- frontend/src/app/app.component.spec.ts | 33 ++++--- frontend/src/app/app.component.ts | 10 +- .../deployment-version.service.spec.ts | 98 +++++++++++-------- .../deployment-version.service.ts | 14 ++- .../notification/notification.service.ts | 9 +- 5 files changed, 96 insertions(+), 68 deletions(-) diff --git a/frontend/src/app/app.component.spec.ts b/frontend/src/app/app.component.spec.ts index 764bdcd0ddc..1b0e7e14a65 100644 --- a/frontend/src/app/app.component.spec.ts +++ b/frontend/src/app/app.component.spec.ts @@ -17,12 +17,14 @@ * under the License. */ +import { HttpClientTestingModule } from "@angular/common/http/testing"; import { CommonModule } from "@angular/common"; import { ComponentFixture, TestBed } from "@angular/core/testing"; import { RouterTestingModule } from "@angular/router/testing"; import { AppComponent } from "./app.component"; import { GuiConfigService } from "./common/service/gui-config.service"; import { DeploymentVersionService } from "./common/service/deployment-version/deployment-version.service"; +import { NotificationService } from "./common/service/notification/notification.service"; import { Version } from "../environments/version"; // GuiConfigService stub whose env getter either returns a value or throws, @@ -38,31 +40,30 @@ class StubGuiConfigService { } } -// Records start() invocations without a spy framework. -class FakeDeploymentVersionService { - startCalls = 0; - start(): void { - this.startCalls++; - } -} - describe("AppComponent", () => { let config: StubGuiConfigService; - let deployment: FakeDeploymentVersionService; + // The real DeploymentVersionService, with its polling entry point spied so + // the test asserts on the wiring without kicking off real HTTP polling. + let startPollingSpy: ReturnType; beforeEach(() => { Version.buildNumber = "dev"; config = new StubGuiConfigService(); - deployment = new FakeDeploymentVersionService(); TestBed.configureTestingModule({ - imports: [CommonModule, RouterTestingModule], + imports: [CommonModule, RouterTestingModule, HttpClientTestingModule], declarations: [AppComponent], providers: [ { provide: GuiConfigService, useValue: config }, - { provide: DeploymentVersionService, useValue: deployment }, + DeploymentVersionService, + // NotificationService is a transitive dependency of DeploymentVersionService. + { provide: NotificationService, useValue: { blank: vi.fn() } }, ], }); + const deploymentVersionService = TestBed.inject(DeploymentVersionService); + startPollingSpy = vi + .spyOn(deploymentVersionService, "startPollingForUpdates") + .mockReturnValue({ unsubscribe: () => undefined } as never); }); // Version is a shared module singleton; restore the dev default so a test @@ -110,28 +111,28 @@ describe("AppComponent", () => { config.deploymentVersionCheckEnabled = true; Version.buildNumber = "dev"; create(); - expect(deployment.startCalls).toBe(0); + expect(startPollingSpy).not.toHaveBeenCalled(); }); it("does not start polling when the config flag is disabled", () => { config.deploymentVersionCheckEnabled = false; Version.buildNumber = "prod-build-123"; create(); - expect(deployment.startCalls).toBe(0); + expect(startPollingSpy).not.toHaveBeenCalled(); }); it("does not start polling when config failed to load", () => { config.shouldThrow = true; Version.buildNumber = "prod-build-123"; create(); - expect(deployment.startCalls).toBe(0); + expect(startPollingSpy).not.toHaveBeenCalled(); }); it("starts polling for a real build when the config flag is enabled", () => { config.deploymentVersionCheckEnabled = true; Version.buildNumber = "prod-build-123"; create(); - expect(deployment.startCalls).toBe(1); + expect(startPollingSpy).toHaveBeenCalledTimes(1); }); }); diff --git a/frontend/src/app/app.component.ts b/frontend/src/app/app.component.ts index 183952c3b1e..aae87fe4a93 100644 --- a/frontend/src/app/app.component.ts +++ b/frontend/src/app/app.component.ts @@ -43,13 +43,13 @@ export class AppComponent { configLoaded = false; constructor( - private config: GuiConfigService, - private deploymentVersion: DeploymentVersionService + private configService: GuiConfigService, + private deploymentVersionService: DeploymentVersionService ) { // determine whether configuration was successfully loaded by APP_INITIALIZER try { // accessing env will throw if not loaded - void this.config.env; + void this.configService.env; this.configLoaded = true; } catch { this.configLoaded = false; @@ -58,8 +58,8 @@ export class AppComponent { // Poll for new deployments only when the config opts in (off by default), // config actually loaded, and this isn't the "dev" placeholder build where // no deployments occur. - if (this.configLoaded && this.config.env.deploymentVersionCheckEnabled && Version.buildNumber !== "dev") { - this.deploymentVersion.start(); + if (this.configLoaded && this.configService.env.deploymentVersionCheckEnabled && Version.buildNumber !== "dev") { + this.deploymentVersionService.startPollingForUpdates(); } } diff --git a/frontend/src/app/common/service/deployment-version/deployment-version.service.spec.ts b/frontend/src/app/common/service/deployment-version/deployment-version.service.spec.ts index 0cfccc4870f..6ebbece48b4 100644 --- a/frontend/src/app/common/service/deployment-version/deployment-version.service.spec.ts +++ b/frontend/src/app/common/service/deployment-version/deployment-version.service.spec.ts @@ -19,31 +19,41 @@ import { HttpClientTestingModule, HttpTestingController } from "@angular/common/http/testing"; import { TestBed, fakeAsync, tick } from "@angular/core/testing"; -import { NzNotificationDataOptions } from "ng-zorro-antd/notification"; +import { NzMessageService } from "ng-zorro-antd/message"; +import { NzNotificationRef, NzNotificationService } from "ng-zorro-antd/notification"; +import { Subject } from "rxjs"; import { NotificationService } from "../notification/notification.service"; import { DeploymentVersionService, VERSION_MANIFEST_URL, VERSION_POLL_INTERVAL_MS } from "./deployment-version.service"; -// Records blank() calls so the prompt is observable without a spy framework. -class FakeNotificationService { - public blankCalls: { title: string; content: string; options: NzNotificationDataOptions }[] = []; - blank(title: string, content: string, options: NzNotificationDataOptions = {}): void { - this.blankCalls.push({ title, content, options }); - } -} - describe("DeploymentVersionService", () => { let service: DeploymentVersionService; let httpMock: HttpTestingController; - let notification: FakeNotificationService; + // The real NotificationService, with its single side-effecting method spied. + let notification: NotificationService; + let blankSpy: ReturnType; + // Drives the onClick of the ref returned by the spied blank() call. + let notificationClick: Subject; beforeEach(() => { + notificationClick = new Subject(); TestBed.configureTestingModule({ imports: [HttpClientTestingModule], - providers: [{ provide: NotificationService, useClass: FakeNotificationService }], + providers: [ + NotificationService, + // ng-zorro's lower-level services are not under test; stub them so the + // real NotificationService can be constructed and spied on. + { provide: NzNotificationService, useValue: { blank: vi.fn(), remove: vi.fn() } }, + { provide: NzMessageService, useValue: {} }, + ], }); service = TestBed.inject(DeploymentVersionService); httpMock = TestBed.inject(HttpTestingController); - notification = TestBed.inject(NotificationService) as unknown as FakeNotificationService; + notification = TestBed.inject(NotificationService); + blankSpy = vi.spyOn(notification, "blank").mockReturnValue({ + onClick: notificationClick, + onClose: new Subject(), + messageId: "test", + } as unknown as NzNotificationRef); }); afterEach(() => httpMock.verify()); @@ -135,44 +145,52 @@ describe("DeploymentVersionService", () => { describe("promptReload", () => { it("shows exactly one sticky, dismissible notification with a refresh message", () => { service.promptReload(); - expect(notification.blankCalls.length).toBe(1); - const call = notification.blankCalls[0]; - expect(call.options.nzDuration).toBe(0); - expect(call.title.length).toBeGreaterThan(0); - expect(call.content.toLowerCase()).toContain("refresh"); + expect(blankSpy).toHaveBeenCalledTimes(1); + const [title, content, options] = blankSpy.mock.calls[0] as [string, string, { nzDuration?: number }]; + expect(options.nzDuration).toBe(0); + expect(title.length).toBeGreaterThan(0); + expect(content.toLowerCase()).toContain("refresh"); + }); + + it("reloads the page when the notification is clicked", () => { + const reloadSpy = vi.spyOn(service, "reload").mockImplementation(() => undefined); + service.promptReload(); + expect(reloadSpy).not.toHaveBeenCalled(); + notificationClick.next(new MouseEvent("click")); + expect(reloadSpy).toHaveBeenCalledTimes(1); }); }); - describe("start (polling)", () => { + describe("startPollingForUpdates", () => { it("polls after the interval and prompts once when a new deployment is detected", fakeAsync(() => { - const sub = service.start(1000); - expect(notification.blankCalls.length).toBe(0); // nothing before the first interval + const sub = service.startPollingForUpdates(1000); + expect(blankSpy).not.toHaveBeenCalled(); // nothing before the first interval tick(1000); takeManifestRequest().flush({ buildNumber: "new-build" }); - expect(notification.blankCalls.length).toBe(1); + expect(blankSpy).toHaveBeenCalledTimes(1); sub.unsubscribe(); })); it("does not prompt while the deployed build is unchanged", fakeAsync(() => { - const sub = service.start(1000); + const sub = service.startPollingForUpdates(1000); tick(1000); takeManifestRequest().flush({ buildNumber: "dev" }); - expect(notification.blankCalls.length).toBe(0); + expect(blankSpy).not.toHaveBeenCalled(); tick(1000); takeManifestRequest().flush({ buildNumber: "dev" }); - expect(notification.blankCalls.length).toBe(0); + expect(blankSpy).not.toHaveBeenCalled(); sub.unsubscribe(); })); it("prompts only once and stops polling after an update is found", fakeAsync(() => { - const sub = service.start(1000); + const sub = service.startPollingForUpdates(1000); tick(1000); takeManifestRequest().flush({ buildNumber: "new-build" }); - expect(notification.blankCalls.length).toBe(1); + expect(blankSpy).toHaveBeenCalledTimes(1); tick(1000); // take(1) completed the stream: no further polling. httpMock.expectNone(req => req.url === VERSION_MANIFEST_URL); - expect(notification.blankCalls.length).toBe(1); + expect(blankSpy).toHaveBeenCalledTimes(1); sub.unsubscribe(); })); @@ -181,50 +199,50 @@ describe("DeploymentVersionService", () => { }); it("does not poll before the default 5 minute interval elapses", fakeAsync(() => { - const sub = service.start(); + const sub = service.startPollingForUpdates(); tick(VERSION_POLL_INTERVAL_MS - 1); httpMock.expectNone(req => req.url === VERSION_MANIFEST_URL); sub.unsubscribe(); })); it("keeps polling and still prompts after a transient request failure", fakeAsync(() => { - const sub = service.start(1000); + const sub = service.startPollingForUpdates(1000); tick(1000); // First poll fails at the transport level: the stream must survive it. takeManifestRequest().error(new ProgressEvent("error")); - expect(notification.blankCalls.length).toBe(0); + expect(blankSpy).not.toHaveBeenCalled(); tick(1000); takeManifestRequest().flush({ buildNumber: "new-build" }); - expect(notification.blankCalls.length).toBe(1); + expect(blankSpy).toHaveBeenCalledTimes(1); sub.unsubscribe(); })); }); - describe("start (idempotency)", () => { + describe("startPollingForUpdates (idempotency)", () => { it("returns the in-flight subscription instead of stacking a second poller", fakeAsync(() => { - const first = service.start(1000); - const second = service.start(1000); + const first = service.startPollingForUpdates(1000); + const second = service.startPollingForUpdates(1000); expect(second).toBe(first); tick(1000); // Only one poller is active, so only one manifest request is issued. takeManifestRequest().flush({ buildNumber: "new-build" }); - expect(notification.blankCalls.length).toBe(1); + expect(blankSpy).toHaveBeenCalledTimes(1); first.unsubscribe(); })); it("starts a fresh poller once the previous run has completed", fakeAsync(() => { - const first = service.start(1000); + const first = service.startPollingForUpdates(1000); tick(1000); // take(1) completes the first run after the update is detected. takeManifestRequest().flush({ buildNumber: "new-build" }); - expect(notification.blankCalls.length).toBe(1); + expect(blankSpy).toHaveBeenCalledTimes(1); - // A subsequent start() is no longer a no-op: the prior run is closed. - const second = service.start(1000); + // A subsequent startPollingForUpdates() is no longer a no-op: the prior run is closed. + const second = service.startPollingForUpdates(1000); expect(second).not.toBe(first); tick(1000); takeManifestRequest().flush({ buildNumber: "another-new-build" }); - expect(notification.blankCalls.length).toBe(2); + expect(blankSpy).toHaveBeenCalledTimes(2); second.unsubscribe(); })); }); diff --git a/frontend/src/app/common/service/deployment-version/deployment-version.service.ts b/frontend/src/app/common/service/deployment-version/deployment-version.service.ts index add60faa8f6..2d4287b6a6a 100644 --- a/frontend/src/app/common/service/deployment-version/deployment-version.service.ts +++ b/frontend/src/app/common/service/deployment-version/deployment-version.service.ts @@ -52,7 +52,7 @@ export class DeploymentVersionService { // Poll until a new deployment is detected, then prompt once. Idempotent: // a second call while already polling returns the in-flight subscription // rather than stacking a duplicate poller (and a duplicate prompt). - start(intervalMs: number = VERSION_POLL_INTERVAL_MS): Subscription { + startPollingForUpdates(intervalMs: number = VERSION_POLL_INTERVAL_MS): Subscription { if (this.polling && !this.polling.closed) { return this.polling; } @@ -67,10 +67,18 @@ export class DeploymentVersionService { } promptReload(): void { - this.notification.blank( + const ref = this.notification.blank( "New version available", - "A new version of Texera is available. Refresh the page to update.", + "A new version of Texera is available. Click here to refresh, or reload the page when convenient.", { nzDuration: 0 } ); + // Clicking the notification reloads the page so the user does not have to + // find the browser refresh button. take(1) guards against a double reload. + ref.onClick.pipe(take(1)).subscribe(() => this.reload()); + } + + // Indirection over window.location.reload so it can be spied in tests. + reload(): void { + window.location.reload(); } } diff --git a/frontend/src/app/common/service/notification/notification.service.ts b/frontend/src/app/common/service/notification/notification.service.ts index 54debba3777..956ff054f59 100644 --- a/frontend/src/app/common/service/notification/notification.service.ts +++ b/frontend/src/app/common/service/notification/notification.service.ts @@ -19,7 +19,7 @@ import { Injectable } from "@angular/core"; import { NzMessageDataOptions, NzMessageService } from "ng-zorro-antd/message"; -import { NzNotificationDataOptions, NzNotificationService } from "ng-zorro-antd/notification"; +import { NzNotificationDataOptions, NzNotificationRef, NzNotificationService } from "ng-zorro-antd/notification"; /** * NotificationService is an entry service for sending notifications @@ -33,9 +33,10 @@ export class NotificationService { private notification: NzNotificationService ) {} - // Only blank can be removed manually - blank(title: string, content: string, options: NzNotificationDataOptions = {}): void { - this.notification.blank(title, content, options); + // Only blank can be removed manually. Returns the notification ref so callers + // can react to clicks (e.g. an actionable "click to refresh" notification). + blank(title: string, content: string, options: NzNotificationDataOptions = {}): NzNotificationRef { + return this.notification.blank(title, content, options); } // Remove current blank notification only From 7502d560dfb8706f6f8dcf15b1b8a17e17506cf4 Mon Sep 17 00:00:00 2001 From: Matthew Ball Date: Fri, 26 Jun 2026 12:04:28 -0700 Subject: [PATCH 09/10] remove indempodent test case --- .../deployment-version.service.spec.ts | 29 ------------------- .../deployment-version.service.ts | 13 ++------- 2 files changed, 3 insertions(+), 39 deletions(-) diff --git a/frontend/src/app/common/service/deployment-version/deployment-version.service.spec.ts b/frontend/src/app/common/service/deployment-version/deployment-version.service.spec.ts index 6ebbece48b4..25de07b1290 100644 --- a/frontend/src/app/common/service/deployment-version/deployment-version.service.spec.ts +++ b/frontend/src/app/common/service/deployment-version/deployment-version.service.spec.ts @@ -217,33 +217,4 @@ describe("DeploymentVersionService", () => { sub.unsubscribe(); })); }); - - describe("startPollingForUpdates (idempotency)", () => { - it("returns the in-flight subscription instead of stacking a second poller", fakeAsync(() => { - const first = service.startPollingForUpdates(1000); - const second = service.startPollingForUpdates(1000); - expect(second).toBe(first); - tick(1000); - // Only one poller is active, so only one manifest request is issued. - takeManifestRequest().flush({ buildNumber: "new-build" }); - expect(blankSpy).toHaveBeenCalledTimes(1); - first.unsubscribe(); - })); - - it("starts a fresh poller once the previous run has completed", fakeAsync(() => { - const first = service.startPollingForUpdates(1000); - tick(1000); - // take(1) completes the first run after the update is detected. - takeManifestRequest().flush({ buildNumber: "new-build" }); - expect(blankSpy).toHaveBeenCalledTimes(1); - - // A subsequent startPollingForUpdates() is no longer a no-op: the prior run is closed. - const second = service.startPollingForUpdates(1000); - expect(second).not.toBe(first); - tick(1000); - takeManifestRequest().flush({ buildNumber: "another-new-build" }); - expect(blankSpy).toHaveBeenCalledTimes(2); - second.unsubscribe(); - })); - }); }); diff --git a/frontend/src/app/common/service/deployment-version/deployment-version.service.ts b/frontend/src/app/common/service/deployment-version/deployment-version.service.ts index 2d4287b6a6a..dd8d13b5581 100644 --- a/frontend/src/app/common/service/deployment-version/deployment-version.service.ts +++ b/frontend/src/app/common/service/deployment-version/deployment-version.service.ts @@ -31,8 +31,6 @@ export const VERSION_POLL_INTERVAL_MS = 5 * 60 * 1000; providedIn: "root", }) export class DeploymentVersionService { - private polling?: Subscription; - constructor( private http: HttpClient, private notification: NotificationService @@ -49,21 +47,16 @@ export class DeploymentVersionService { ); } - // Poll until a new deployment is detected, then prompt once. Idempotent: - // a second call while already polling returns the in-flight subscription - // rather than stacking a duplicate poller (and a duplicate prompt). + // Poll until a new deployment is detected, then prompt once and stop + // (take(1)). Called a single time from AppComponent on startup. startPollingForUpdates(intervalMs: number = VERSION_POLL_INTERVAL_MS): Subscription { - if (this.polling && !this.polling.closed) { - return this.polling; - } - this.polling = timer(intervalMs, intervalMs) + return timer(intervalMs, intervalMs) .pipe( switchMap(() => this.checkForUpdate()), filter(updated => updated), take(1) ) .subscribe(() => this.promptReload()); - return this.polling; } promptReload(): void { From d94ac5a2059f44f95f8ad42e4ce6a939905ba657 Mon Sep 17 00:00:00 2001 From: Matthew Ball Date: Fri, 26 Jun 2026 16:10:09 -0700 Subject: [PATCH 10/10] rerun-ci