This is an automated email from the ASF dual-hosted git repository.
voidmatcha pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git
The following commit(s) were added to refs/heads/master by this push:
new 5e36932e24 [ZEPPELIN-6697] Prevent live NOTE_UPDATED from mutating
revision snapshots
5e36932e24 is described below
commit 5e36932e24cabac0224bba235659ca9f7e0aeeb4
Author: Lee SuJung <[email protected]>
AuthorDate: Sat Sep 12 23:12:54 2026 +0900
[ZEPPELIN-6697] Prevent live NOTE_UPDATED from mutating revision snapshots
### What is this PR for?
`NotebookServer.updateNote()` broadcasts `NOTE_UPDATED` to every socket
associated with the note, and a socket that navigates to a saved revision keeps
that association. The Angular handler applied the payload unconditionally:
```typescript
<at>MessageListener(OP.NOTE_UPDATED)
noteUpdated(data) {
if (!this.note) {
return;
}
this.note.config = data.config; // overwrites the historical snapshot
```
So changing the live note's look and feel from another browser rewrote the
snapshot the revision view was showing.
The guard already exists elsewhere in the same component —
`removeParagraph`, `addParagraph` and `moveParagraph` all check `revisionView`,
and ZEPPELIN-2452 added the same check to the classic UI. It was only partly
carried over during the migration, and `noteUpdated` was the one left out. This
adds it there.
Per the issue, this stays out of `noteId`, request correlation and socket
generation: the reproduction only proves a revision-isolation defect, and
nothing here required a wire change. The Shared Notebook Core revision adapter
is not covered yet, since that slice has not landed.
### What type of PR is it?
Bug Fix
### Todos
* [x] Guard `noteUpdated` with `revisionView`
* [x] Add a retained Playwright regression for the same-tab
live-to-revision route
### What is the Jira issue?
* [ZEPPELIN-6697](https://issues.apache.org/jira/browse/ZEPPELIN-6697)
### How should this be tested?
New spec `e2e/tests/notebook/revision/revision-isolation.spec.ts` opens a
note, saves a revision, enters it on the same socket, then changes look and
feel from an independent browser context. It asserts the live follower applies
the update, and that the revision view still shows the value it was saved with.
```
cd zeppelin-web-angular && npx playwright test
e2e/tests/notebook/revision/revision-isolation.spec.ts --project=chromium
--reporter=list
```
Result: `2 passed (22.5s)`. `--reporter=list` is worth passing locally,
since the configured HTML reporter opens a report server afterwards and the
command does not return on its own.
Reverting only the guard makes it fail, so it does catch the defect rather
than passing by construction:
```
Error: expect(locator).toContainText(expected) failed
Expected string: "default"
Received string: " simple "
```
Two details the spec depends on, both found the hard way:
- **Look and feel, not rename.** Renaming goes through `renameNote`, which
answers with `broadcastNote()` (`OP.NOTE`) and never emits `NOTE_UPDATED`.
`setLookAndFeel()` is what calls `updateNote()`, which is why the issue names
it in the reproduction.
- **Waiting for the event.** Asserting the snapshot straight after the live
change passes trivially, because the assertion resolves before the broadcast
arrives. The spec first polls the revision page's console for `Receive:
NOTE_UPDATED`, then asserts.
### Screenshots (if appropriate)
N/A
### Questions:
* Does the license files need to update? No
* Is there breaking changes for older versions? No — a live follower still
applies the same update exactly once; only a revision view ignores it
* Does this needs documentation? No
Closes #5468 from xhaktm00/ZEPPELIN-6697.
Signed-off-by: YONGJAE LEE <[email protected]>
---
.github/workflows/frontend.yml | 21 ++++
.../notebook/revision/revision-isolation.spec.ts | 132 +++++++++++++++++++++
.../pages/workspace/notebook/notebook.component.ts | 4 +-
3 files changed, 156 insertions(+), 1 deletion(-)
diff --git a/.github/workflows/frontend.yml b/.github/workflows/frontend.yml
index 69bc73cf5d..08af6bf699 100644
--- a/.github/workflows/frontend.yml
+++ b/.github/workflows/frontend.yml
@@ -121,6 +121,26 @@ jobs:
- name: Run headless E2E test with Maven
# Classic UI e2e runs only on the anonymous leg, like the legacy
Protractor suite
run: xvfb-run --auto-servernum --server-args="-screen 0 1024x768x24"
./mvnw verify -pl zeppelin-web-angular -Pweb-e2e -Dweb.e2e.classic.disabled=${{
matrix.mode != 'anonymous' }} ${MAVEN_ARGS}
+ - name: Run revision isolation E2E test with Git storage
+ env:
+ CI: 'true'
+ ZEPPELIN_NOTEBOOK_STORAGE:
org.apache.zeppelin.notebook.repo.GitNotebookRepo
+ ZEPPELIN_E2E_REQUIRE_REVISION: 'true'
+ PLAYWRIGHT_HTML_OUTPUT_DIR: playwright-report-revision
+ run: |
+ revision_notebook_dir="$(mktemp -d
"${RUNNER_TEMP}/zeppelin-revision-notebooks.XXXXXX")"
+ zeppelin_daemon="${GITHUB_WORKSPACE}/bin/zeppelin-daemon.sh"
+ cleanup() {
+ "$zeppelin_daemon" stop || true
+ rm -rf -- "$revision_notebook_dir"
+ }
+ trap cleanup EXIT
+ export ZEPPELIN_NOTEBOOK_DIR="$revision_notebook_dir"
+ # This step owns notebook cleanup instead of Playwright's global
directory reset.
+ unset ZEPPELIN_E2E_TEST_NOTEBOOK_DIR
+ "$zeppelin_daemon" start
+ cd zeppelin-web-angular
+ xvfb-run --auto-servernum --server-args="-screen 0 1024x768x24"
./node/npm run e2e -- tests/notebook/revision/revision-isolation.spec.ts
--output=test-results-revision --reporter=github,html
- name: Upload Playwright Report
uses: actions/upload-artifact@v6
if: always()
@@ -129,6 +149,7 @@ jobs:
path: |
zeppelin-web-angular/playwright-report/
zeppelin-web-angular/playwright-report-classic/
+ zeppelin-web-angular/playwright-report-revision/
retention-days: 3
- name: Print Zeppelin logs
if: always()
diff --git
a/zeppelin-web-angular/e2e/tests/notebook/revision/revision-isolation.spec.ts
b/zeppelin-web-angular/e2e/tests/notebook/revision/revision-isolation.spec.ts
new file mode 100644
index 0000000000..811d733f69
--- /dev/null
+++
b/zeppelin-web-angular/e2e/tests/notebook/revision/revision-isolation.spec.ts
@@ -0,0 +1,132 @@
+/*
+ * Licensed 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 { expect, Locator, Page, test } from '@playwright/test';
+
+import {
+ addPageAnnotationBeforeEach,
+ createTestNotebookWithName,
+ PAGES,
+ performLoginIfRequired,
+ waitForNotebookLinks,
+ waitForZeppelinReady
+} from '../../../utils';
+
+const prepareWorkspace = async (page: Page): Promise<void> => {
+ await page.goto('/#/');
+ await waitForZeppelinReady(page);
+ await performLoginIfRequired(page);
+ await waitForNotebookLinks(page);
+};
+
+const openNotebook = async (page: Page, noteId: string): Promise<void> => {
+ await page.goto(`/#/notebook/${noteId}`);
+ await waitForZeppelinReady(page);
+};
+
+/** Saves a revision. The commit control is an icon-only button that opens a
popover. */
+const commitRevision = async (page: Page, message: string): Promise<void> => {
+ await page.locator('button:has(i[nzType="to-top"])').click();
+ const commitInput = page.getByPlaceholder('commit message');
+ await expect(commitInput).toBeVisible({ timeout: 15000 });
+ await commitInput.fill(message);
+ await page.getByRole('button', { name: 'commit', exact: true }).click();
+ await expect(commitInput).toBeHidden({ timeout: 15000 });
+};
+
+/** The look and feel dropdown is labelled with the note's current value. */
+const lookAndFeelButton = (page: Page): Locator =>
page.locator('button[nz-dropdown]:has(i[nzType="down"])').last();
+
+const setLookAndFeel = async (page: Page, value: string): Promise<void> => {
+ await lookAndFeelButton(page).click();
+ await page.locator('li[nz-menu-item]').filter({ hasText: value
}).last().click();
+};
+
+test.describe('Revision isolation', () => {
+ addPageAnnotationBeforeEach(PAGES.WORKSPACE.NOTEBOOK);
+
+ // All viewers share one principal (same storageState), matching the
collaborative-mode spec.
+ // Look and feel is what drives NOTE_UPDATE, and so the NOTE_UPDATED
broadcast this covers;
+ // renaming goes through a different op that resends the whole note.
+ test('keeps a live NOTE_UPDATED from mutating an open revision snapshot',
async ({ page, browser }) => {
+ await prepareWorkspace(page);
+
+ const capabilities = await page.request.get('/api/notebook/capabilities');
+ expect(capabilities.ok()).toBe(true);
+ const { body } = await capabilities.json();
+ expect(typeof body.isRevisionSupported).toBe('boolean');
+ expect(
+ body.isRevisionSupported || process.env.ZEPPELIN_E2E_REQUIRE_REVISION
!== 'true',
+ 'The revision CI run requires versioned notebook storage'
+ ).toBe(true);
+ test.skip(!body.isRevisionSupported, 'The configured notebook storage does
not support revisions');
+
+ const { noteId } = await createTestNotebookWithName(page, { namePrefix:
'RevisionIsolation' });
+ await openNotebook(page, noteId);
+
+ // The snapshot has to capture the original look and feel, before the live
change below.
+ await expect(lookAndFeelButton(page)).toContainText('default', { timeout:
15000 });
+ await commitRevision(page, 'snapshot before look and feel change');
+
+ // The dropdown is labelled with the current revision, which is "Head"
until one is chosen.
+ await page.getByRole('button', { name: 'Head', exact: true }).click();
+ const revisionItem = page.locator('li[nz-menu-item]').filter({ hasText:
'snapshot before look' });
+ await expect(revisionItem).toBeVisible({ timeout: 15000 });
+ await revisionItem.click();
+
+ // Same socket, now showing the historical snapshot.
+ await expect(page).toHaveURL(/\/revision\//, { timeout: 15000 });
+ await expect(lookAndFeelButton(page)).toContainText('default', { timeout:
15000 });
+
+ // The assertion below has to run after the revision view has actually
received the event,
+ // otherwise it would pass simply by checking too early. Message.receive
logs every op.
+ let sawNoteUpdated = false;
+ page.on('console', message => {
+ if (message.text().includes('Receive: NOTE_UPDATED')) {
+ sawNoteUpdated = true;
+ }
+ });
+
+ const liveContext = await browser.newContext({ storageState: await
page.context().storageState() });
+ try {
+ const livePage = await liveContext.newPage();
+ const followerPage = await liveContext.newPage();
+ let followerMessagesReceived = 0;
+ followerPage.on('console', message => {
+ if (message.text().includes('Receive: NOTE_UPDATED')) {
+ followerMessagesReceived++;
+ }
+ });
+
+ await prepareWorkspace(livePage);
+ await openNotebook(livePage, noteId);
+ await expect(lookAndFeelButton(livePage)).toContainText('default', {
timeout: 15000 });
+
+ await openNotebook(followerPage, noteId);
+ await expect(lookAndFeelButton(followerPage)).toContainText('default', {
timeout: 15000 });
+
+ // Change the live note from an independent browser context; this
broadcasts NOTE_UPDATED.
+ await setLookAndFeel(livePage, 'simple');
+
+ // This viewer performs no local edit, so its change must come from the
broadcast.
+ await expect(lookAndFeelButton(followerPage)).toContainText('simple', {
timeout: 15000 });
+ await expect.poll(() => sawNoteUpdated, { timeout: 15000 }).toBe(true);
+
+ // The revision view got the event and must still show the snapshot as
it was saved.
+ await expect(lookAndFeelButton(page)).toContainText('default');
+ // Count received broadcasts; the UI assertion above verifies their
visible effect.
+ expect(followerMessagesReceived).toBe(1);
+ } finally {
+ await liveContext.close();
+ }
+ });
+});
diff --git
a/zeppelin-web-angular/src/app/pages/workspace/notebook/notebook.component.ts
b/zeppelin-web-angular/src/app/pages/workspace/notebook/notebook.component.ts
index 6f5e24974c..3dd6e73960 100644
---
a/zeppelin-web-angular/src/app/pages/workspace/notebook/notebook.component.ts
+++
b/zeppelin-web-angular/src/app/pages/workspace/notebook/notebook.component.ts
@@ -241,7 +241,9 @@ export class NotebookComponent extends
MessageListenersManager implements OnInit
@MessageListener(OP.NOTE_UPDATED)
noteUpdated(data: MessageReceiveDataTypeMap[OP.NOTE_UPDATED]) {
- if (!this.note) {
+ // NOTE_UPDATED carries the live note, so applying it while a revision is
open would
+ // overwrite the historical snapshot with current values.
+ if (!this.note || this.revisionView) {
return;
}
if (data.name !== this.note.name) {
