From a5f1dceef9aae6005b164285b5ac8ed6134007dc Mon Sep 17 00:00:00 2001 From: xhaktm00 <153787023+xhaktm00@users.noreply.github.com> Date: Wed, 9 Sep 2026 15:50:14 +0900 Subject: [PATCH 1/2] [ZEPPELIN-6697] Keep live NOTE_UPDATED out of an open revision snapshot updateNote() broadcasts NOTE_UPDATED to every socket associated with the note, and a socket that navigated to a saved revision keeps that association. The Angular handler applied the payload unconditionally, so a look and feel change made on the live note rewrote the historical snapshot the revision view was showing. Guard the handler with revisionView, which is what the paragraph handlers in the same component already do, and what ZEPPELIN-2452 did for the classic UI before the guard was only partly carried over. The new Playwright spec opens a note, saves a revision, enters it on the same socket, then changes look and feel from an independent browser context. It waits until the revision view has actually received NOTE_UPDATED before asserting, so it fails without this guard rather than passing by checking too early. Co-Authored-By: Claude Opus 5 (1M context) --- .../revision/revision-isolation.spec.ts | 116 ++++++++++++++++++ .../workspace/notebook/notebook.component.ts | 4 +- 2 files changed, 119 insertions(+), 1 deletion(-) create mode 100644 zeppelin-web-angular/e2e/tests/notebook/revision/revision-isolation.spec.ts 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 00000000000..17c635eaa80 --- /dev/null +++ b/zeppelin-web-angular/e2e/tests/notebook/revision/revision-isolation.spec.ts @@ -0,0 +1,116 @@ +/* + * 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, + skipWhenAuthenticationIsStillRequired, + waitForNotebookLinks, + waitForZeppelinReady +} from '../../../utils'; + +const prepareWorkspace = async (page: Page): Promise => { + await page.goto('/#/'); + await waitForZeppelinReady(page); + await performLoginIfRequired(page); + await skipWhenAuthenticationIsStillRequired(page); + await waitForNotebookLinks(page); +}; + +const openNotebook = async (page: Page, noteId: string): Promise => { + 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 => { + 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 => { + await lookAndFeelButton(page).click(); + await page.locator('li[nz-menu-item]').filter({ hasText: value }).last().click(); +}; + +test.describe('Revision isolation', () => { + addPageAnnotationBeforeEach(PAGES.WORKSPACE.NOTEBOOK); + + // Both 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 { 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() }); + const livePage = await liveContext.newPage(); + + try { + await livePage.goto('/#/'); + await waitForZeppelinReady(livePage); + await performLoginIfRequired(livePage); + await skipWhenAuthenticationIsStillRequired(livePage); + await openNotebook(livePage, noteId); + await expect(lookAndFeelButton(livePage)).toContainText('default', { timeout: 15000 }); + + // Change the live note from an independent browser context; this broadcasts NOTE_UPDATED. + await setLookAndFeel(livePage, 'simple'); + + // The live follower applies it, which is how we know the broadcast actually went out and + // that the revision view has had the same chance to receive it. + await expect(lookAndFeelButton(livePage)).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'); + } 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 6f5e24974c8..3dd6e739607 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) { From 91083e55def688feb83f52beb0a45560b250ca90 Mon Sep 17 00:00:00 2001 From: YONGJAE LEE Date: Wed, 9 Sep 2026 23:42:40 +0900 Subject: [PATCH 2/2] [ZEPPELIN-6697] Test revision isolation with Git storage and a follower --- .github/workflows/frontend.yml | 21 ++++++++++ .../revision/revision-isolation.spec.ts | 40 +++++++++++++------ 2 files changed, 49 insertions(+), 12 deletions(-) diff --git a/.github/workflows/frontend.yml b/.github/workflows/frontend.yml index 9e59901692f..cd1adee8aff 100644 --- a/.github/workflows/frontend.yml +++ b/.github/workflows/frontend.yml @@ -115,6 +115,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() @@ -123,6 +143,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 index 17c635eaa80..811d733f697 100644 --- a/zeppelin-web-angular/e2e/tests/notebook/revision/revision-isolation.spec.ts +++ b/zeppelin-web-angular/e2e/tests/notebook/revision/revision-isolation.spec.ts @@ -17,7 +17,6 @@ import { createTestNotebookWithName, PAGES, performLoginIfRequired, - skipWhenAuthenticationIsStillRequired, waitForNotebookLinks, waitForZeppelinReady } from '../../../utils'; @@ -26,7 +25,6 @@ const prepareWorkspace = async (page: Page): Promise => { await page.goto('/#/'); await waitForZeppelinReady(page); await performLoginIfRequired(page); - await skipWhenAuthenticationIsStillRequired(page); await waitForNotebookLinks(page); }; @@ -56,12 +54,22 @@ const setLookAndFeel = async (page: Page, value: string): Promise => { test.describe('Revision isolation', () => { addPageAnnotationBeforeEach(PAGES.WORKSPACE.NOTEBOOK); - // Both viewers share one principal (same storageState), matching the collaborative-mode spec. + // 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); @@ -89,26 +97,34 @@ test.describe('Revision isolation', () => { }); const liveContext = await browser.newContext({ storageState: await page.context().storageState() }); - const livePage = await liveContext.newPage(); - try { - await livePage.goto('/#/'); - await waitForZeppelinReady(livePage); - await performLoginIfRequired(livePage); - await skipWhenAuthenticationIsStillRequired(livePage); + 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'); - // The live follower applies it, which is how we know the broadcast actually went out and - // that the revision view has had the same chance to receive it. - await expect(lookAndFeelButton(livePage)).toContainText('simple', { timeout: 15000 }); + // 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(); }