From f354a6763791fc7b3279b9c68f0c7bcb8f3d4992 Mon Sep 17 00:00:00 2001 From: Max Date: Mon, 3 Nov 2025 18:01:08 +0100 Subject: [PATCH 01/37] chore(test): split setOnline and setOffline Signed-off-by: Max --- playwright/e2e/offline.spec.ts | 44 +++++++++++++++++----------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/playwright/e2e/offline.spec.ts b/playwright/e2e/offline.spec.ts index d3a34a395c3..ce667decec7 100644 --- a/playwright/e2e/offline.spec.ts +++ b/playwright/e2e/offline.spec.ts @@ -9,24 +9,24 @@ import { test as uploadFileTest } from '../support/fixtures/upload-file' const test = mergeTests(randomUserTest, uploadFileTest) -const setOnline = async (client: CDPSession, online: boolean): Promise => { - if (online) { - await client.send('Network.emulateNetworkConditions', { - offline: false, - latency: 0, - downloadThroughput: -1, - uploadThroughput: -1, - }) - await client.send('Network.disable') - } else { - await client.send('Network.enable') - await client.send('Network.emulateNetworkConditions', { - offline: true, - latency: 0, - downloadThroughput: 0, - uploadThroughput: 0, - }) - } +const setOnline = async (client: CDPSession): Promise => { + await client.send('Network.emulateNetworkConditions', { + offline: false, + latency: 0, + downloadThroughput: -1, + uploadThroughput: -1, + }) + await client.send('Network.disable') +} + +const setOffline = async (client: CDPSession): Promise => { + await client.send('Network.enable') + await client.send('Network.emulateNetworkConditions', { + offline: true, + latency: 0, + downloadThroughput: 0, + uploadThroughput: 0, + }) } test.beforeEach(async ({ page, file }) => { @@ -39,12 +39,12 @@ test.describe('Offline', () => { await expect(page.locator('.offline-state')).not.toBeVisible() const client = await context.newCDPSession(page) - await setOnline(client, false) + await setOffline(client) await expect(page.locator('.session-list')).not.toBeVisible() await expect(page.locator('.offline-state')).toBeVisible() - await setOnline(client, true) + await setOnline(client) }) test('Disabled upload and link file when offline', async ({ context, page }) => { @@ -58,7 +58,7 @@ test.describe('Offline', () => { ).toBeEnabled() const client = await context.newCDPSession(page) - await setOnline(client, false) + await setOffline(client) await page.locator('[data-text-action-entry="insert-link"]').click() await expect( @@ -69,6 +69,6 @@ test.describe('Offline', () => { page.locator('[data-text-action-entry="insert-attachment"] button'), ).toBeDisabled() - await setOnline(client, true) + await setOnline(client) }) }) From 79a075bfe2ae957fa495765dfc0d660283b1400b Mon Sep 17 00:00:00 2001 From: Max Date: Mon, 3 Nov 2025 18:18:10 +0100 Subject: [PATCH 02/37] chore(test): add offline fixture to playwright Signed-off-by: Max --- playwright/e2e/offline.spec.ts | 37 ++++------------------- playwright/support/fixtures/offline.ts | 42 ++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 31 deletions(-) create mode 100644 playwright/support/fixtures/offline.ts diff --git a/playwright/e2e/offline.spec.ts b/playwright/e2e/offline.spec.ts index ce667decec7..f294891f588 100644 --- a/playwright/e2e/offline.spec.ts +++ b/playwright/e2e/offline.spec.ts @@ -4,50 +4,28 @@ */ import { type CDPSession, expect, mergeTests } from '@playwright/test' +import { test as offlineTest } from '../support/fixtures/offline' import { test as randomUserTest } from '../support/fixtures/random-user' import { test as uploadFileTest } from '../support/fixtures/upload-file' -const test = mergeTests(randomUserTest, uploadFileTest) - -const setOnline = async (client: CDPSession): Promise => { - await client.send('Network.emulateNetworkConditions', { - offline: false, - latency: 0, - downloadThroughput: -1, - uploadThroughput: -1, - }) - await client.send('Network.disable') -} - -const setOffline = async (client: CDPSession): Promise => { - await client.send('Network.enable') - await client.send('Network.emulateNetworkConditions', { - offline: true, - latency: 0, - downloadThroughput: 0, - uploadThroughput: 0, - }) -} +const test = mergeTests(offlineTest, randomUserTest, uploadFileTest) test.beforeEach(async ({ page, file }) => { await page.goto(`f/${file.fileId}`) }) test.describe('Offline', () => { - test('Offline state indicator', async ({ context, page }) => { + test('Offline state indicator', async ({ context, page, setOffline }) => { await expect(page.locator('.session-list')).toBeVisible() await expect(page.locator('.offline-state')).not.toBeVisible() - const client = await context.newCDPSession(page) - await setOffline(client) + await setOffline() await expect(page.locator('.session-list')).not.toBeVisible() await expect(page.locator('.offline-state')).toBeVisible() - - await setOnline(client) }) - test('Disabled upload and link file when offline', async ({ context, page }) => { + test('Disabled upload and link file when offline', async ({ context, page, setOffline }) => { await page.locator('[data-text-action-entry="insert-link"]').click() await expect( page.locator('[data-text-action-entry="insert-link-file"] button'), @@ -57,8 +35,7 @@ test.describe('Offline', () => { page.locator('[data-text-action-entry="insert-attachment"] button'), ).toBeEnabled() - const client = await context.newCDPSession(page) - await setOffline(client) + await setOffline() await page.locator('[data-text-action-entry="insert-link"]').click() await expect( @@ -68,7 +45,5 @@ test.describe('Offline', () => { await expect( page.locator('[data-text-action-entry="insert-attachment"] button'), ).toBeDisabled() - - await setOnline(client) }) }) diff --git a/playwright/support/fixtures/offline.ts b/playwright/support/fixtures/offline.ts new file mode 100644 index 00000000000..da54ad7c50e --- /dev/null +++ b/playwright/support/fixtures/offline.ts @@ -0,0 +1,42 @@ +/** + * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import { test as base, type CDPSession } from '@playwright/test' + +interface OfflineFixture { + setOffline: () => Promise +} + +const setClientOnline = async (client: CDPSession): Promise => { + await client.send('Network.emulateNetworkConditions', { + offline: false, + latency: 0, + downloadThroughput: -1, + uploadThroughput: -1, + }) + await client.send('Network.disable') +} + +const setClientOffline = async (client: CDPSession): Promise => { + await client.send('Network.enable') + await client.send('Network.emulateNetworkConditions', { + offline: true, + latency: 0, + downloadThroughput: 0, + uploadThroughput: 0, + }) +} + +/** + * setOffline will turn the network off for the rest of the test and then on again. + */ +export const test = base.extend({ + // eslint-disable-next-line no-empty-pattern + setOffline: async ({ context, page }, use) => { + const client = await context.newCDPSession(page) + await use (() => setClientOffline(client)) + await setClientOnline(client) + }, +}) From c612b18b675b76fa82ca01aa834bedb3eee1074d Mon Sep 17 00:00:00 2001 From: Max Date: Mon, 3 Nov 2025 19:15:32 +0100 Subject: [PATCH 03/37] chore(test): get request token from /csrftoken Signed-off-by: Max --- playwright/support/fixtures/random-user.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/playwright/support/fixtures/random-user.ts b/playwright/support/fixtures/random-user.ts index 464bd3f4257..d9b2cffe3ca 100644 --- a/playwright/support/fixtures/random-user.ts +++ b/playwright/support/fixtures/random-user.ts @@ -21,15 +21,11 @@ export const test = base.extend({ const user = await createRandomUser() await use(user) }, - requestToken: async ({ page }, use) => { - // Navigate to get the page context and extract request token - await page.goto('/') - - // Get the request token from the page context - const token = await page.evaluate(() => { - // @ts-expect-error - OC is a global variable - return window.OC?.requestToken || '' + requestToken: async ({ page, request }, use) => { + const tokenResponse = await page.request.get('./csrftoken', { + failOnStatusCode: true, }) + const token = (await tokenResponse.json()).token await use(token) }, From f22c61c5ae0fc4bd40745219640a287b73b72975 Mon Sep 17 00:00:00 2001 From: Max Date: Mon, 3 Nov 2025 19:27:06 +0100 Subject: [PATCH 04/37] chore(test): split request token fixture from random user Signed-off-by: Max --- playwright/support/fixtures/random-user.ts | 9 -------- playwright/support/fixtures/request-token.ts | 24 ++++++++++++++++++++ playwright/support/fixtures/upload-file.ts | 7 +----- 3 files changed, 25 insertions(+), 15 deletions(-) create mode 100644 playwright/support/fixtures/request-token.ts diff --git a/playwright/support/fixtures/random-user.ts b/playwright/support/fixtures/random-user.ts index d9b2cffe3ca..2ee147cadfe 100644 --- a/playwright/support/fixtures/random-user.ts +++ b/playwright/support/fixtures/random-user.ts @@ -9,7 +9,6 @@ import { type User } from '@nextcloud/e2e-test-server' interface RandomUserFixture { user: User - requestToken: string } /** @@ -21,14 +20,6 @@ export const test = base.extend({ const user = await createRandomUser() await use(user) }, - requestToken: async ({ page, request }, use) => { - const tokenResponse = await page.request.get('./csrftoken', { - failOnStatusCode: true, - }) - const token = (await tokenResponse.json()).token - - await use(token) - }, page: async ({ browser, baseURL, user }, use) => { // Important: make sure we authenticate in a clean environment by unsetting storage state. const page = await browser.newPage({ diff --git a/playwright/support/fixtures/request-token.ts b/playwright/support/fixtures/request-token.ts new file mode 100644 index 00000000000..e2c9c37e811 --- /dev/null +++ b/playwright/support/fixtures/request-token.ts @@ -0,0 +1,24 @@ +/** + * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import { test as base } from '@playwright/test' + +export interface RequestTokenFixture { + requestToken: string +} + +/** + * This test fixture ensures a new random user is created and used for the test (current page) + */ +export const test = base.extend({ + requestToken: async ({ page }, use) => { + const tokenResponse = await page.request.get('./csrftoken', { + failOnStatusCode: true, + }) + const token = (await tokenResponse.json()).token + + await use(token) + }, +}) diff --git a/playwright/support/fixtures/upload-file.ts b/playwright/support/fixtures/upload-file.ts index e14755d3632..1ecca1f632e 100644 --- a/playwright/support/fixtures/upload-file.ts +++ b/playwright/support/fixtures/upload-file.ts @@ -3,10 +3,9 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -import { test as base } from '@playwright/test' +import { test as base } from './request-token' interface UploadMdFixture { - requestToken?: string file: { fileName: string fileId: number @@ -22,10 +21,6 @@ export const test = base.extend({ const fileName = 'empty.md' const fileContent = '' - if (!requestToken) { - throw new Error('requestToken is required. Make sure to merge with random-user fixture.') - } - // Upload file via WebDAV using page.request with requesttoken header const response = await page.request.put( `/remote.php/webdav/${fileName}`, From 2e96c26316d2e9b6e9ef8181a81b21e2218b4038 Mon Sep 17 00:00:00 2001 From: Max Date: Mon, 3 Nov 2025 19:46:42 +0100 Subject: [PATCH 05/37] chore(test): cleanup unused type and context Signed-off-by: Max --- playwright/e2e/offline.spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/playwright/e2e/offline.spec.ts b/playwright/e2e/offline.spec.ts index f294891f588..dc9d9ff6f8a 100644 --- a/playwright/e2e/offline.spec.ts +++ b/playwright/e2e/offline.spec.ts @@ -3,7 +3,7 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -import { type CDPSession, expect, mergeTests } from '@playwright/test' +import { expect, mergeTests } from '@playwright/test' import { test as offlineTest } from '../support/fixtures/offline' import { test as randomUserTest } from '../support/fixtures/random-user' import { test as uploadFileTest } from '../support/fixtures/upload-file' @@ -15,7 +15,7 @@ test.beforeEach(async ({ page, file }) => { }) test.describe('Offline', () => { - test('Offline state indicator', async ({ context, page, setOffline }) => { + test('Offline state indicator', async ({ page, setOffline }) => { await expect(page.locator('.session-list')).toBeVisible() await expect(page.locator('.offline-state')).not.toBeVisible() @@ -25,7 +25,7 @@ test.describe('Offline', () => { await expect(page.locator('.offline-state')).toBeVisible() }) - test('Disabled upload and link file when offline', async ({ context, page, setOffline }) => { + test('Disabled upload and link file when offline', async ({ page, setOffline }) => { await page.locator('[data-text-action-entry="insert-link"]').click() await expect( page.locator('[data-text-action-entry="insert-link-file"] button'), From 1df8359f08e9bff5c49564f8e9ff25738e6bfed7 Mon Sep 17 00:00:00 2001 From: Max Date: Mon, 3 Nov 2025 20:12:21 +0100 Subject: [PATCH 06/37] chore(test): mimetype change in playwright Signed-off-by: Max --- playwright/e2e/change-mime-type.spec.ts | 41 +++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 playwright/e2e/change-mime-type.spec.ts diff --git a/playwright/e2e/change-mime-type.spec.ts b/playwright/e2e/change-mime-type.spec.ts new file mode 100644 index 00000000000..e3cd8b03170 --- /dev/null +++ b/playwright/e2e/change-mime-type.spec.ts @@ -0,0 +1,41 @@ +/** + * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import { expect, mergeTests } from '@playwright/test' +import { test as randomUserTest } from '../support/fixtures/random-user' +import { test as uploadFileTest } from '../support/fixtures/upload-file' + +const test = mergeTests(randomUserTest, uploadFileTest) + +test.beforeEach(async ({ page, file }) => { + await page.goto(`f/${file.fileId}`) +}) + +test.describe('Changing mimetype from/to markdown resets document session', () => { + test('Rename from md to txt', async ({ page, file, requestToken }) => { + await page.getByRole('textbox').pressSequentially('## Hello world') + await expect(page.getByRole('heading', { name: 'Hello world' })) + .toBeVisible() + await page.getByRole('button', { name: 'Close', exact: true }).click() + await expect(page.getByRole('button', { name: 'Close', exact: true })) + .not.toBeVisible() + const destinationPath = 'test1.txt' + await page.request.fetch( + `/remote.php/webdav/${file.fileName}`, + { + headers: { + Destination: `/remote.php/webdav/${destinationPath}`, + 'requesttoken': requestToken, + }, + method: 'MOVE', + }) + await page.goto(`f/${file.fileId}`) + await expect(page.getByRole('textbox')) + .toBeVisible() + await expect(page.getByText('## Hello world')) + .toBeVisible() + + }) +}) From 5c00d5adbc87e88ec523aa1451883198610a78e1 Mon Sep 17 00:00:00 2001 From: Max Date: Mon, 3 Nov 2025 20:52:23 +0100 Subject: [PATCH 07/37] chore(test): basic editor page object model Signed-off-by: Max --- playwright/e2e/change-mime-type.spec.ts | 18 +++++++------- playwright/support/fixtures/editor.ts | 18 ++++++++++++++ playwright/support/sections/EditorSection.ts | 25 ++++++++++++++++++++ 3 files changed, 52 insertions(+), 9 deletions(-) create mode 100644 playwright/support/fixtures/editor.ts create mode 100644 playwright/support/sections/EditorSection.ts diff --git a/playwright/e2e/change-mime-type.spec.ts b/playwright/e2e/change-mime-type.spec.ts index e3cd8b03170..57fc83191e9 100644 --- a/playwright/e2e/change-mime-type.spec.ts +++ b/playwright/e2e/change-mime-type.spec.ts @@ -4,19 +4,20 @@ */ import { expect, mergeTests } from '@playwright/test' +import { test as editorTest } from '../support/fixtures/editor' import { test as randomUserTest } from '../support/fixtures/random-user' import { test as uploadFileTest } from '../support/fixtures/upload-file' -const test = mergeTests(randomUserTest, uploadFileTest) +const test = mergeTests(editorTest, randomUserTest, uploadFileTest) test.beforeEach(async ({ page, file }) => { await page.goto(`f/${file.fileId}`) }) test.describe('Changing mimetype from/to markdown resets document session', () => { - test('Rename from md to txt', async ({ page, file, requestToken }) => { - await page.getByRole('textbox').pressSequentially('## Hello world') - await expect(page.getByRole('heading', { name: 'Hello world' })) + test('Rename from md to txt', async ({ editor, page, file, requestToken }) => { + await editor.type('## Hello world') + await expect(editor.getHeading({ name: 'Hello world' })) .toBeVisible() await page.getByRole('button', { name: 'Close', exact: true }).click() await expect(page.getByRole('button', { name: 'Close', exact: true })) @@ -32,10 +33,9 @@ test.describe('Changing mimetype from/to markdown resets document session', () = method: 'MOVE', }) await page.goto(`f/${file.fileId}`) - await expect(page.getByRole('textbox')) - .toBeVisible() - await expect(page.getByText('## Hello world')) - .toBeVisible() - + await expect(editor.contentLocator) + .toHaveText('## Hello world') + await expect(editor.getHeading()) + .not.toBeVisible() }) }) diff --git a/playwright/support/fixtures/editor.ts b/playwright/support/fixtures/editor.ts new file mode 100644 index 00000000000..bb217bffa9a --- /dev/null +++ b/playwright/support/fixtures/editor.ts @@ -0,0 +1,18 @@ +/** + * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import { test as baseTest } from '@playwright/test' +import { EditorSection } from '../sections/EditorSection' + +interface EditorFixture { + editor: EditorSection +} + +export const test = baseTest.extend({ + editor: async ({ page }, use) => { + const editor = new EditorSection(page) + await use(editor) + }, +}) diff --git a/playwright/support/sections/EditorSection.ts b/playwright/support/sections/EditorSection.ts new file mode 100644 index 00000000000..39ac39bee7d --- /dev/null +++ b/playwright/support/sections/EditorSection.ts @@ -0,0 +1,25 @@ +/** + * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import type { Locator, Page } from '@playwright/test' + +export class EditorSection { + public readonly locator: Locator + public readonly contentLocator: Locator + + // eslint-disable-next-line no-useless-constructor + constructor(public readonly page: Page) { + this.locator = this.page.locator('.editor').first() + this.contentLocator = this.locator.getByRole('textbox') + } + + public async type(keys: string): Promise { + await this.contentLocator.pressSequentially(keys) + } + + public getHeading(options: {} = {}): Locator { + return this.contentLocator.getByRole('heading', options) + } +} From 8035dfb5614bd828732221961b05b737c5388667 Mon Sep 17 00:00:00 2001 From: Max Date: Mon, 3 Nov 2025 21:01:18 +0100 Subject: [PATCH 08/37] chore(test): file class for handling uploaded file Signed-off-by: Max --- playwright/e2e/change-mime-type.spec.ts | 6 ++-- playwright/e2e/offline.spec.ts | 2 +- playwright/support/fixtures/upload-file.ts | 34 +++++++++++++++------- 3 files changed, 28 insertions(+), 14 deletions(-) diff --git a/playwright/e2e/change-mime-type.spec.ts b/playwright/e2e/change-mime-type.spec.ts index 57fc83191e9..5797611da06 100644 --- a/playwright/e2e/change-mime-type.spec.ts +++ b/playwright/e2e/change-mime-type.spec.ts @@ -11,7 +11,7 @@ import { test as uploadFileTest } from '../support/fixtures/upload-file' const test = mergeTests(editorTest, randomUserTest, uploadFileTest) test.beforeEach(async ({ page, file }) => { - await page.goto(`f/${file.fileId}`) + await page.goto(`f/${file.id}`) }) test.describe('Changing mimetype from/to markdown resets document session', () => { @@ -24,7 +24,7 @@ test.describe('Changing mimetype from/to markdown resets document session', () = .not.toBeVisible() const destinationPath = 'test1.txt' await page.request.fetch( - `/remote.php/webdav/${file.fileName}`, + `/remote.php/webdav/${file.name}`, { headers: { Destination: `/remote.php/webdav/${destinationPath}`, @@ -32,7 +32,7 @@ test.describe('Changing mimetype from/to markdown resets document session', () = }, method: 'MOVE', }) - await page.goto(`f/${file.fileId}`) + await page.goto(`f/${file.id}`) await expect(editor.contentLocator) .toHaveText('## Hello world') await expect(editor.getHeading()) diff --git a/playwright/e2e/offline.spec.ts b/playwright/e2e/offline.spec.ts index dc9d9ff6f8a..1d12473931b 100644 --- a/playwright/e2e/offline.spec.ts +++ b/playwright/e2e/offline.spec.ts @@ -11,7 +11,7 @@ import { test as uploadFileTest } from '../support/fixtures/upload-file' const test = mergeTests(offlineTest, randomUserTest, uploadFileTest) test.beforeEach(async ({ page, file }) => { - await page.goto(`f/${file.fileId}`) + await page.goto(`f/${file.id}`) }) test.describe('Offline', () => { diff --git a/playwright/support/fixtures/upload-file.ts b/playwright/support/fixtures/upload-file.ts index 1ecca1f632e..3aafcaf3fb7 100644 --- a/playwright/support/fixtures/upload-file.ts +++ b/playwright/support/fixtures/upload-file.ts @@ -3,13 +3,11 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ +import type { Page } from '@playwright/test' import { test as base } from './request-token' interface UploadMdFixture { - file: { - fileName: string - fileId: number - } + file: File } /** @@ -18,12 +16,28 @@ interface UploadMdFixture { */ export const test = base.extend({ file: async ({ page, requestToken }, use) => { - const fileName = 'empty.md' + const file = new File('empty.md', page) const fileContent = '' + await file.upload(fileContent, requestToken) + await use(file) + }, +}) + +class File { + name: string + page: Page + id?: number + + constructor(name: string, page: Page) { + this.name = name + this.page = page + } + + async upload(fileContent: string, requestToken: string) { // Upload file via WebDAV using page.request with requesttoken header - const response = await page.request.put( - `/remote.php/webdav/${fileName}`, + const response = await this.page.request.put( + `/remote.php/webdav/${this.name}`, { data: fileContent, headers: { @@ -40,7 +54,7 @@ export const test = base.extend({ // Extract file ID from response headers const ocFileId = response.headers()['oc-fileid'] const fileId = ocFileId ? Number(ocFileId.split('oc')?.[0]) : 0 + this.id = fileId + } - await use({ fileName, fileId }) - }, -}) +} From a9f995f34ef136550585d23f9a320f3de80096a2 Mon Sep 17 00:00:00 2001 From: Max Date: Mon, 3 Nov 2025 21:04:08 +0100 Subject: [PATCH 09/37] chore(test): file.open() Signed-off-by: Max --- playwright/e2e/change-mime-type.spec.ts | 6 +++--- playwright/e2e/offline.spec.ts | 9 ++++++--- playwright/support/fixtures/upload-file.ts | 3 +++ 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/playwright/e2e/change-mime-type.spec.ts b/playwright/e2e/change-mime-type.spec.ts index 5797611da06..2947f4fdf3c 100644 --- a/playwright/e2e/change-mime-type.spec.ts +++ b/playwright/e2e/change-mime-type.spec.ts @@ -10,8 +10,8 @@ import { test as uploadFileTest } from '../support/fixtures/upload-file' const test = mergeTests(editorTest, randomUserTest, uploadFileTest) -test.beforeEach(async ({ page, file }) => { - await page.goto(`f/${file.id}`) +test.beforeEach(async ({ file }) => { + await file.open() }) test.describe('Changing mimetype from/to markdown resets document session', () => { @@ -32,7 +32,7 @@ test.describe('Changing mimetype from/to markdown resets document session', () = }, method: 'MOVE', }) - await page.goto(`f/${file.id}`) + await file.open() await expect(editor.contentLocator) .toHaveText('## Hello world') await expect(editor.getHeading()) diff --git a/playwright/e2e/offline.spec.ts b/playwright/e2e/offline.spec.ts index 1d12473931b..838e1df05e9 100644 --- a/playwright/e2e/offline.spec.ts +++ b/playwright/e2e/offline.spec.ts @@ -10,8 +10,8 @@ import { test as uploadFileTest } from '../support/fixtures/upload-file' const test = mergeTests(offlineTest, randomUserTest, uploadFileTest) -test.beforeEach(async ({ page, file }) => { - await page.goto(`f/${file.id}`) +test.beforeEach(async ({ file }) => { + await file.open() }) test.describe('Offline', () => { @@ -25,7 +25,10 @@ test.describe('Offline', () => { await expect(page.locator('.offline-state')).toBeVisible() }) - test('Disabled upload and link file when offline', async ({ page, setOffline }) => { + test('Disabled upload and link file when offline', async ({ + page, + setOffline, + }) => { await page.locator('[data-text-action-entry="insert-link"]').click() await expect( page.locator('[data-text-action-entry="insert-link-file"] button'), diff --git a/playwright/support/fixtures/upload-file.ts b/playwright/support/fixtures/upload-file.ts index 3aafcaf3fb7..f0f079389bd 100644 --- a/playwright/support/fixtures/upload-file.ts +++ b/playwright/support/fixtures/upload-file.ts @@ -57,4 +57,7 @@ class File { this.id = fileId } + async open() { + return this.page.goto(`f/${this.id}`) + } } From e2f52b38739510a46da354d11d9adeae9933d08e Mon Sep 17 00:00:00 2001 From: Max Date: Mon, 3 Nov 2025 21:09:39 +0100 Subject: [PATCH 10/37] chore(test): with file.move() and file.close() Signed-off-by: Max --- playwright/e2e/change-mime-type.spec.ts | 10 +----- playwright/support/fixtures/upload-file.ts | 37 ++++++++++++++++++---- 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/playwright/e2e/change-mime-type.spec.ts b/playwright/e2e/change-mime-type.spec.ts index 2947f4fdf3c..352392a5aa6 100644 --- a/playwright/e2e/change-mime-type.spec.ts +++ b/playwright/e2e/change-mime-type.spec.ts @@ -23,15 +23,7 @@ test.describe('Changing mimetype from/to markdown resets document session', () = await expect(page.getByRole('button', { name: 'Close', exact: true })) .not.toBeVisible() const destinationPath = 'test1.txt' - await page.request.fetch( - `/remote.php/webdav/${file.name}`, - { - headers: { - Destination: `/remote.php/webdav/${destinationPath}`, - 'requesttoken': requestToken, - }, - method: 'MOVE', - }) + await file.move(destinationPath) await file.open() await expect(editor.contentLocator) .toHaveText('## Hello world') diff --git a/playwright/support/fixtures/upload-file.ts b/playwright/support/fixtures/upload-file.ts index f0f079389bd..7f38a404d84 100644 --- a/playwright/support/fixtures/upload-file.ts +++ b/playwright/support/fixtures/upload-file.ts @@ -3,7 +3,7 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -import type { Page } from '@playwright/test' +import { expect, type Page } from '@playwright/test' import { test as base } from './request-token' interface UploadMdFixture { @@ -16,9 +16,9 @@ interface UploadMdFixture { */ export const test = base.extend({ file: async ({ page, requestToken }, use) => { - const file = new File('empty.md', page) + const file = new File('empty.md', page, requestToken) const fileContent = '' - await file.upload(fileContent, requestToken) + await file.upload(fileContent) await use(file) }, }) @@ -26,14 +26,16 @@ export const test = base.extend({ class File { name: string page: Page + requestToken: string id?: number - constructor(name: string, page: Page) { + constructor(name: string, page: Page, requestToken: string) { this.name = name this.page = page + this.requestToken = requestToken } - async upload(fileContent: string, requestToken: string) { + async upload(fileContent: string) { // Upload file via WebDAV using page.request with requesttoken header const response = await this.page.request.put( @@ -42,7 +44,7 @@ class File { data: fileContent, headers: { 'Content-Type': 'text/markdown', - 'requesttoken': requestToken, + 'requesttoken': this.requestToken, }, }, ) @@ -58,6 +60,27 @@ class File { } async open() { - return this.page.goto(`f/${this.id}`) + await this.page.goto(`f/${this.id}`) + await expect(this.page.getByLabel(this.name, { exact: true })) + .toBeVisible() + } + + async close() { + await this.page.getByRole('button', { name: 'Close', exact: true }).click() + await expect(this.page.getByLabel(this.name, { exact: true })) + .not.toBeVisible() + } + + async move(newName: string) { + await this.page.request.fetch( + `/remote.php/webdav/${this.name}`, + { + headers: { + Destination: `/remote.php/webdav/${newName}`, + 'requesttoken': this.requestToken, + }, + method: 'MOVE', + }) + this.name = newName } } From 93652d73c7287e798af05439b87f317b4c01ac9e Mon Sep 17 00:00:00 2001 From: Max Date: Mon, 3 Nov 2025 21:28:19 +0100 Subject: [PATCH 11/37] chore(test): with editor.typeHeading() Signed-off-by: Max --- playwright/e2e/change-mime-type.spec.ts | 19 ++++++------------- playwright/support/sections/EditorSection.ts | 8 ++++++-- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/playwright/e2e/change-mime-type.spec.ts b/playwright/e2e/change-mime-type.spec.ts index 352392a5aa6..1dafa78d0e3 100644 --- a/playwright/e2e/change-mime-type.spec.ts +++ b/playwright/e2e/change-mime-type.spec.ts @@ -15,19 +15,12 @@ test.beforeEach(async ({ file }) => { }) test.describe('Changing mimetype from/to markdown resets document session', () => { - test('Rename from md to txt', async ({ editor, page, file, requestToken }) => { - await editor.type('## Hello world') - await expect(editor.getHeading({ name: 'Hello world' })) - .toBeVisible() - await page.getByRole('button', { name: 'Close', exact: true }).click() - await expect(page.getByRole('button', { name: 'Close', exact: true })) - .not.toBeVisible() - const destinationPath = 'test1.txt' - await file.move(destinationPath) + test('Rename from md to txt', async ({ editor, file }) => { + await editor.typeHeading('Hello world') + await file.close() + await file.move('test1.txt') await file.open() - await expect(editor.contentLocator) - .toHaveText('## Hello world') - await expect(editor.getHeading()) - .not.toBeVisible() + await expect(editor.contentLocator).toHaveText('## Hello world') + await expect(editor.getHeading()).not.toBeVisible() }) }) diff --git a/playwright/support/sections/EditorSection.ts b/playwright/support/sections/EditorSection.ts index 39ac39bee7d..b96a09a2bf0 100644 --- a/playwright/support/sections/EditorSection.ts +++ b/playwright/support/sections/EditorSection.ts @@ -4,6 +4,7 @@ */ import type { Locator, Page } from '@playwright/test' +import { expect } from '@playwright/test' export class EditorSection { public readonly locator: Locator @@ -15,8 +16,11 @@ export class EditorSection { this.contentLocator = this.locator.getByRole('textbox') } - public async type(keys: string): Promise { - await this.contentLocator.pressSequentially(keys) + public async typeHeading(name: string): Promise { + await this.contentLocator.pressSequentially('## ') + await this.contentLocator.pressSequentially(name) + await expect(this.getHeading({ name })) + .toBeVisible() } public getHeading(options: {} = {}): Locator { From d8eb8f560c5588ac4882b6105ec1e09d65f8d02e Mon Sep 17 00:00:00 2001 From: Max Date: Mon, 3 Nov 2025 21:44:43 +0100 Subject: [PATCH 12/37] chore(test): change mimetype both ways Signed-off-by: Max --- playwright/e2e/change-mime-type.spec.ts | 19 +++++++++++-- playwright/support/fixtures/upload-file.ts | 29 +++++++++++--------- playwright/support/sections/EditorSection.ts | 13 +++++---- 3 files changed, 40 insertions(+), 21 deletions(-) diff --git a/playwright/e2e/change-mime-type.spec.ts b/playwright/e2e/change-mime-type.spec.ts index 1dafa78d0e3..2cc712a94eb 100644 --- a/playwright/e2e/change-mime-type.spec.ts +++ b/playwright/e2e/change-mime-type.spec.ts @@ -14,13 +14,26 @@ test.beforeEach(async ({ file }) => { await file.open() }) -test.describe('Changing mimetype from/to markdown resets document session', () => { - test('Rename from md to txt', async ({ editor, file }) => { +test.describe('Changing mimetype from markdown to plaintext', () => { + test('resets the document session and indexed db', async ({ editor, file }) => { await editor.typeHeading('Hello world') await file.close() - await file.move('test1.txt') + await file.move('test.txt') await file.open() await expect(editor.contentLocator).toHaveText('## Hello world') await expect(editor.getHeading()).not.toBeVisible() }) }) + +test.describe('Changing mimetype from plain to markdown', () => { + test.use({ fileName: 'empty.txt' }) + + test('resets the document session and indexed db', async ({ editor, file }) => { + await editor.type('## Hello world') + await expect(editor.contentLocator).toHaveText('## Hello world') + await file.close() + await file.move('test.md') + await file.open() + await expect(editor.getHeading({ name: 'Hello world' })).toBeVisible() + }) +}) diff --git a/playwright/support/fixtures/upload-file.ts b/playwright/support/fixtures/upload-file.ts index 7f38a404d84..b386793054f 100644 --- a/playwright/support/fixtures/upload-file.ts +++ b/playwright/support/fixtures/upload-file.ts @@ -8,21 +8,9 @@ import { test as base } from './request-token' interface UploadMdFixture { file: File + fileName: string } -/** - * This test fixture uploads the empty.md file to the user's root directory - * Note: This fixture requires the page to be authenticated (e.g., by merging with random-user fixture) - */ -export const test = base.extend({ - file: async ({ page, requestToken }, use) => { - const file = new File('empty.md', page, requestToken) - const fileContent = '' - await file.upload(fileContent) - await use(file) - }, -}) - class File { name: string page: Page @@ -84,3 +72,18 @@ class File { this.name = newName } } + +/** + * This test fixture uploads the empty.md file to the user's root directory + * Note: This fixture requires the page to be authenticated (e.g., by merging with random-user fixture) + */ +export const test = base.extend({ + file: async ({ page, requestToken, fileName }, use) => { + const file = new File(fileName, page, requestToken) + const fileContent = '' + await file.upload(fileContent) + await use(file) + }, + fileName: ['empty.md', {option: true}], +}) + diff --git a/playwright/support/sections/EditorSection.ts b/playwright/support/sections/EditorSection.ts index b96a09a2bf0..facc068a196 100644 --- a/playwright/support/sections/EditorSection.ts +++ b/playwright/support/sections/EditorSection.ts @@ -16,14 +16,17 @@ export class EditorSection { this.contentLocator = this.locator.getByRole('textbox') } + public async type(keys: string): Promise { + await this.contentLocator.pressSequentially(keys) + } + public async typeHeading(name: string): Promise { - await this.contentLocator.pressSequentially('## ') - await this.contentLocator.pressSequentially(name) - await expect(this.getHeading({ name })) - .toBeVisible() + await this.type('## ') + await this.type(name) + await expect(this.getHeading({ name })).toBeVisible() } - public getHeading(options: {} = {}): Locator { + public getHeading(options: object = {}): Locator { return this.contentLocator.getByRole('heading', options) } } From b227e3a999beac8ef3a06ffa1ff4f0391465f811 Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 4 Nov 2025 08:54:14 +0100 Subject: [PATCH 13/37] chore(test): store test-results folder as artifacts Signed-off-by: Max --- .github/workflows/playwright.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/playwright.yml b/.github/workflows/playwright.yml index d979566b380..c7806ba7cf2 100644 --- a/.github/workflows/playwright.yml +++ b/.github/workflows/playwright.yml @@ -54,5 +54,5 @@ jobs: if: always() with: name: playwright-report - path: playwright-report/ + path: test-results/ retention-days: 30 From 86ecbe394179b1b7b6ee4db2e0c8a64ffde6ae6e Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 4 Nov 2025 11:14:08 +0100 Subject: [PATCH 14/37] chore(test): move File class to separate file Signed-off-by: Max --- playwright/support/fixtures/File.ts | 68 ++++++++++++++++++++++ playwright/support/fixtures/upload-file.ts | 64 +------------------- 2 files changed, 69 insertions(+), 63 deletions(-) create mode 100644 playwright/support/fixtures/File.ts diff --git a/playwright/support/fixtures/File.ts b/playwright/support/fixtures/File.ts new file mode 100644 index 00000000000..6bed16da931 --- /dev/null +++ b/playwright/support/fixtures/File.ts @@ -0,0 +1,68 @@ +/** + * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import { expect, type Page } from '@playwright/test' + +export class File { + name: string + page: Page + requestToken: string + id?: number + + constructor(name: string, page: Page, requestToken: string) { + this.name = name + this.page = page + this.requestToken = requestToken + } + + async upload(fileContent: string) { + + // Upload file via WebDAV using page.request with requesttoken header + const response = await this.page.request.put( + `/remote.php/webdav/${this.name}`, + { + data: fileContent, + headers: { + 'Content-Type': 'text/markdown', + 'requesttoken': this.requestToken, + }, + }, + ) + + if (!response.ok()) { + throw new Error(`Failed to upload file: ${response.status()} ${response.statusText()}`) + } + + // Extract file ID from response headers + const ocFileId = response.headers()['oc-fileid'] + const fileId = ocFileId ? Number(ocFileId.split('oc')?.[0]) : 0 + this.id = fileId + } + + async open() { + await this.page.goto(`f/${this.id}`) + await expect(this.page.getByLabel(this.name, { exact: true })) + .toBeVisible() + } + + async close() { + await this.page.getByRole('button', { name: 'Close', exact: true }).click() + await expect(this.page.getByLabel(this.name, { exact: true })) + .not.toBeVisible() + } + + async move(newName: string) { + await this.page.request.fetch( + `/remote.php/webdav/${this.name}`, + { + headers: { + Destination: `/remote.php/webdav/${newName}`, + 'requesttoken': this.requestToken, + }, + method: 'MOVE', + }) + this.name = newName + } +} diff --git a/playwright/support/fixtures/upload-file.ts b/playwright/support/fixtures/upload-file.ts index b386793054f..a9f578ff2c7 100644 --- a/playwright/support/fixtures/upload-file.ts +++ b/playwright/support/fixtures/upload-file.ts @@ -3,76 +3,14 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -import { expect, type Page } from '@playwright/test' import { test as base } from './request-token' +import { File } from './File' interface UploadMdFixture { file: File fileName: string } -class File { - name: string - page: Page - requestToken: string - id?: number - - constructor(name: string, page: Page, requestToken: string) { - this.name = name - this.page = page - this.requestToken = requestToken - } - - async upload(fileContent: string) { - - // Upload file via WebDAV using page.request with requesttoken header - const response = await this.page.request.put( - `/remote.php/webdav/${this.name}`, - { - data: fileContent, - headers: { - 'Content-Type': 'text/markdown', - 'requesttoken': this.requestToken, - }, - }, - ) - - if (!response.ok()) { - throw new Error(`Failed to upload file: ${response.status()} ${response.statusText()}`) - } - - // Extract file ID from response headers - const ocFileId = response.headers()['oc-fileid'] - const fileId = ocFileId ? Number(ocFileId.split('oc')?.[0]) : 0 - this.id = fileId - } - - async open() { - await this.page.goto(`f/${this.id}`) - await expect(this.page.getByLabel(this.name, { exact: true })) - .toBeVisible() - } - - async close() { - await this.page.getByRole('button', { name: 'Close', exact: true }).click() - await expect(this.page.getByLabel(this.name, { exact: true })) - .not.toBeVisible() - } - - async move(newName: string) { - await this.page.request.fetch( - `/remote.php/webdav/${this.name}`, - { - headers: { - Destination: `/remote.php/webdav/${newName}`, - 'requesttoken': this.requestToken, - }, - method: 'MOVE', - }) - this.name = newName - } -} - /** * This test fixture uploads the empty.md file to the user's root directory * Note: This fixture requires the page to be authenticated (e.g., by merging with random-user fixture) From 4b6ebc0ff1fdcdb2c395d03a4366e17bc0b79155 Mon Sep 17 00:00:00 2001 From: Max Date: Fri, 7 Nov 2025 11:10:42 +0100 Subject: [PATCH 15/37] chore(test): use EditorSection in offline test Signed-off-by: Max --- playwright/e2e/change-mime-type.spec.ts | 4 +- playwright/e2e/offline.spec.ts | 39 ++++++++------------ playwright/support/sections/EditorSection.ts | 16 ++++++-- 3 files changed, 30 insertions(+), 29 deletions(-) diff --git a/playwright/e2e/change-mime-type.spec.ts b/playwright/e2e/change-mime-type.spec.ts index 2cc712a94eb..212c1bdae51 100644 --- a/playwright/e2e/change-mime-type.spec.ts +++ b/playwright/e2e/change-mime-type.spec.ts @@ -20,7 +20,7 @@ test.describe('Changing mimetype from markdown to plaintext', () => { await file.close() await file.move('test.txt') await file.open() - await expect(editor.contentLocator).toHaveText('## Hello world') + await expect(editor.content).toHaveText('## Hello world') await expect(editor.getHeading()).not.toBeVisible() }) }) @@ -30,7 +30,7 @@ test.describe('Changing mimetype from plain to markdown', () => { test('resets the document session and indexed db', async ({ editor, file }) => { await editor.type('## Hello world') - await expect(editor.contentLocator).toHaveText('## Hello world') + await expect(editor.content).toHaveText('## Hello world') await file.close() await file.move('test.md') await file.open() diff --git a/playwright/e2e/offline.spec.ts b/playwright/e2e/offline.spec.ts index 838e1df05e9..956aa18a841 100644 --- a/playwright/e2e/offline.spec.ts +++ b/playwright/e2e/offline.spec.ts @@ -4,49 +4,42 @@ */ import { expect, mergeTests } from '@playwright/test' +import { test as editorTest } from '../support/fixtures/editor' import { test as offlineTest } from '../support/fixtures/offline' import { test as randomUserTest } from '../support/fixtures/random-user' import { test as uploadFileTest } from '../support/fixtures/upload-file' -const test = mergeTests(offlineTest, randomUserTest, uploadFileTest) +const test = mergeTests(editorTest, offlineTest, randomUserTest, uploadFileTest) test.beforeEach(async ({ file }) => { await file.open() }) test.describe('Offline', () => { - test('Offline state indicator', async ({ page, setOffline }) => { - await expect(page.locator('.session-list')).toBeVisible() - await expect(page.locator('.offline-state')).not.toBeVisible() + test('Offline state indicator', async ({ editor, setOffline }) => { + await expect(editor.sessionList).toBeVisible() + await expect(editor.offlineState).not.toBeVisible() await setOffline() - await expect(page.locator('.session-list')).not.toBeVisible() - await expect(page.locator('.offline-state')).toBeVisible() + await expect(editor.sessionList).not.toBeVisible() + await expect(editor.offlineState).toBeVisible() }) test('Disabled upload and link file when offline', async ({ - page, + editor, setOffline, }) => { - await page.locator('[data-text-action-entry="insert-link"]').click() - await expect( - page.locator('[data-text-action-entry="insert-link-file"] button'), - ).toBeEnabled() - await page.locator('[data-text-action-entry="insert-link"]').click() - await expect( - page.locator('[data-text-action-entry="insert-attachment"] button'), - ).toBeEnabled() + await editor.getMenu('insert-link').click() + await expect(editor.getMenu('insert-link-file')).toBeEnabled() + await editor.getMenu('insert-link').click() + await expect(editor.getMenu('insert-attachment')).toBeEnabled() await setOffline() - await page.locator('[data-text-action-entry="insert-link"]').click() - await expect( - page.locator('[data-text-action-entry="insert-link-file"] button'), - ).toBeDisabled() - await page.locator('[data-text-action-entry="insert-link"]').click() - await expect( - page.locator('[data-text-action-entry="insert-attachment"] button'), - ).toBeDisabled() + await editor.getMenu('insert-link').click() + await expect(editor.getMenu('insert-link-file')).toBeDisabled() + await editor.getMenu('insert-link').click() + await expect(editor.getMenu('insert-attachment')).toBeDisabled() }) }) diff --git a/playwright/support/sections/EditorSection.ts b/playwright/support/sections/EditorSection.ts index facc068a196..529e44a604a 100644 --- a/playwright/support/sections/EditorSection.ts +++ b/playwright/support/sections/EditorSection.ts @@ -8,16 +8,20 @@ import { expect } from '@playwright/test' export class EditorSection { public readonly locator: Locator - public readonly contentLocator: Locator + public readonly content: Locator + public readonly sessionList: Locator + public readonly offlineState: Locator // eslint-disable-next-line no-useless-constructor constructor(public readonly page: Page) { this.locator = this.page.locator('.editor').first() - this.contentLocator = this.locator.getByRole('textbox') + this.content = this.locator.getByRole('textbox') + this.sessionList = this.locator.locator('.session-list') + this.offlineState = this.locator.locator('.offline-state') } public async type(keys: string): Promise { - await this.contentLocator.pressSequentially(keys) + await this.content.pressSequentially(keys) } public async typeHeading(name: string): Promise { @@ -26,7 +30,11 @@ export class EditorSection { await expect(this.getHeading({ name })).toBeVisible() } + public getMenu(name: string): Locator { + return this.locator.locator(`[data-text-action-entry="${name}"] button`) + } + public getHeading(options: object = {}): Locator { - return this.contentLocator.getByRole('heading', options) + return this.content.getByRole('heading', options) } } From e1749493035b5242cf8397d28a428f4e556e87f8 Mon Sep 17 00:00:00 2001 From: Max Date: Fri, 7 Nov 2025 11:55:59 +0100 Subject: [PATCH 16/37] chore(test) .withOpenMenu() to access submenus Signed-off-by: Max --- playwright/e2e/offline.spec.ts | 13 +++++++------ playwright/support/sections/EditorSection.ts | 9 +++++++++ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/playwright/e2e/offline.spec.ts b/playwright/e2e/offline.spec.ts index 956aa18a841..290c0645461 100644 --- a/playwright/e2e/offline.spec.ts +++ b/playwright/e2e/offline.spec.ts @@ -30,16 +30,17 @@ test.describe('Offline', () => { editor, setOffline, }) => { - await editor.getMenu('insert-link').click() - await expect(editor.getMenu('insert-link-file')).toBeEnabled() - await editor.getMenu('insert-link').click() + const linkToFile = editor.getMenu('insert-link-file') + await editor.withOpenMenu('insert-link', + () => expect(linkToFile).toBeEnabled() + ) await expect(editor.getMenu('insert-attachment')).toBeEnabled() await setOffline() - await editor.getMenu('insert-link').click() - await expect(editor.getMenu('insert-link-file')).toBeDisabled() - await editor.getMenu('insert-link').click() + await editor.withOpenMenu('insert-link', + () => expect(linkToFile).toBeDisabled() + ) await expect(editor.getMenu('insert-attachment')).toBeDisabled() }) }) diff --git a/playwright/support/sections/EditorSection.ts b/playwright/support/sections/EditorSection.ts index 529e44a604a..9d2fc04598a 100644 --- a/playwright/support/sections/EditorSection.ts +++ b/playwright/support/sections/EditorSection.ts @@ -34,6 +34,15 @@ export class EditorSection { return this.locator.locator(`[data-text-action-entry="${name}"] button`) } + public async withOpenMenu( + name: string, + fn: () => Promise, + ): Promise { + await this.getMenu(name).click() // open the menu + await fn() + await this.getMenu(name).click() // close the menu + } + public getHeading(options: object = {}): Locator { return this.content.getByRole('heading', options) } From 5c0da42530b88d36198b3a2d929704298b5f04c7 Mon Sep 17 00:00:00 2001 From: Max Date: Fri, 7 Nov 2025 12:36:33 +0100 Subject: [PATCH 17/37] chore(test): write offline and come back online Signed-off-by: Max --- playwright/e2e/offline.spec.ts | 18 ++++++++++++++++++ playwright/support/fixtures/offline.ts | 6 +++++- playwright/support/sections/EditorSection.ts | 2 ++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/playwright/e2e/offline.spec.ts b/playwright/e2e/offline.spec.ts index 290c0645461..8448e4e5f24 100644 --- a/playwright/e2e/offline.spec.ts +++ b/playwright/e2e/offline.spec.ts @@ -11,6 +11,10 @@ import { test as uploadFileTest } from '../support/fixtures/upload-file' const test = mergeTests(editorTest, offlineTest, randomUserTest, uploadFileTest) +// As we switch on and off the network +// we cannot run tests in parallel. +test.describe.configure({ mode: 'serial' }) + test.beforeEach(async ({ file }) => { await file.open() }) @@ -43,4 +47,18 @@ test.describe('Offline', () => { ) await expect(editor.getMenu('insert-attachment')).toBeDisabled() }) + + test('typing offline and coming back online', async ({ + editor, + setOffline, + setOnline, + }) => { + await expect(editor.locator).toBeVisible() + await setOffline() + await editor.typeHeading('Hello world') + await setOnline() + await expect(editor.offlineState).not.toBeVisible() + await expect(editor.saveIndicator).toHaveAttribute('title', /Unsaved changes/) + }) + }) diff --git a/playwright/support/fixtures/offline.ts b/playwright/support/fixtures/offline.ts index da54ad7c50e..0163aa9673b 100644 --- a/playwright/support/fixtures/offline.ts +++ b/playwright/support/fixtures/offline.ts @@ -7,6 +7,7 @@ import { test as base, type CDPSession } from '@playwright/test' interface OfflineFixture { setOffline: () => Promise + setOnline: () => Promise } const setClientOnline = async (client: CDPSession): Promise => { @@ -33,10 +34,13 @@ const setClientOffline = async (client: CDPSession): Promise => { * setOffline will turn the network off for the rest of the test and then on again. */ export const test = base.extend({ - // eslint-disable-next-line no-empty-pattern setOffline: async ({ context, page }, use) => { const client = await context.newCDPSession(page) await use (() => setClientOffline(client)) await setClientOnline(client) }, + setOnline: async ({ context, page }, use) => { + const client = await context.newCDPSession(page) + await use (() => setClientOnline(client)) + }, }) diff --git a/playwright/support/sections/EditorSection.ts b/playwright/support/sections/EditorSection.ts index 9d2fc04598a..464d19f7889 100644 --- a/playwright/support/sections/EditorSection.ts +++ b/playwright/support/sections/EditorSection.ts @@ -11,6 +11,7 @@ export class EditorSection { public readonly content: Locator public readonly sessionList: Locator public readonly offlineState: Locator + public readonly saveIndicator: Locator // eslint-disable-next-line no-useless-constructor constructor(public readonly page: Page) { @@ -18,6 +19,7 @@ export class EditorSection { this.content = this.locator.getByRole('textbox') this.sessionList = this.locator.locator('.session-list') this.offlineState = this.locator.locator('.offline-state') + this.saveIndicator = this.locator.locator('.save-status') } public async type(keys: string): Promise { From 3f211d5476733108f6105b05f2b70b5c04681013 Mon Sep 17 00:00:00 2001 From: Max Date: Fri, 7 Nov 2025 19:15:12 +0100 Subject: [PATCH 18/37] chore(test): reduce nesting in offline test Signed-off-by: Max --- playwright/e2e/offline.spec.ts | 65 +++++++++++++++------------------- 1 file changed, 29 insertions(+), 36 deletions(-) diff --git a/playwright/e2e/offline.spec.ts b/playwright/e2e/offline.spec.ts index 8448e4e5f24..cffb9660583 100644 --- a/playwright/e2e/offline.spec.ts +++ b/playwright/e2e/offline.spec.ts @@ -19,46 +19,39 @@ test.beforeEach(async ({ file }) => { await file.open() }) -test.describe('Offline', () => { - test('Offline state indicator', async ({ editor, setOffline }) => { - await expect(editor.sessionList).toBeVisible() - await expect(editor.offlineState).not.toBeVisible() +test('Offline state indicator', async ({ editor, setOffline }) => { + await expect(editor.sessionList).toBeVisible() + await expect(editor.offlineState).not.toBeVisible() - await setOffline() + await setOffline() - await expect(editor.sessionList).not.toBeVisible() - await expect(editor.offlineState).toBeVisible() - }) - - test('Disabled upload and link file when offline', async ({ - editor, - setOffline, - }) => { - const linkToFile = editor.getMenu('insert-link-file') - await editor.withOpenMenu('insert-link', - () => expect(linkToFile).toBeEnabled() - ) - await expect(editor.getMenu('insert-attachment')).toBeEnabled() + await expect(editor.sessionList).not.toBeVisible() + await expect(editor.offlineState).toBeVisible() +}) - await setOffline() +test('Disabled upload and link file when offline', async ({ + editor, + setOffline, +}) => { + const linkToFile = editor.getMenu('insert-link-file') + await editor.withOpenMenu('insert-link', () => expect(linkToFile).toBeEnabled()) + await expect(editor.getMenu('insert-attachment')).toBeEnabled() - await editor.withOpenMenu('insert-link', - () => expect(linkToFile).toBeDisabled() - ) - await expect(editor.getMenu('insert-attachment')).toBeDisabled() - }) + await setOffline() - test('typing offline and coming back online', async ({ - editor, - setOffline, - setOnline, - }) => { - await expect(editor.locator).toBeVisible() - await setOffline() - await editor.typeHeading('Hello world') - await setOnline() - await expect(editor.offlineState).not.toBeVisible() - await expect(editor.saveIndicator).toHaveAttribute('title', /Unsaved changes/) - }) + await editor.withOpenMenu('insert-link', () => expect(linkToFile).toBeDisabled()) + await expect(editor.getMenu('insert-attachment')).toBeDisabled() +}) +test('typing offline and coming back online', async ({ + editor, + setOffline, + setOnline, +}) => { + await expect(editor.locator).toBeVisible() + await setOffline() + await editor.typeHeading('Hello world') + await setOnline() + await expect(editor.offlineState).not.toBeVisible() + await expect(editor.saveIndicator).toHaveAttribute('title', /Unsaved changes/) }) From 46dde73825c119f7dfc8105e2b4b5e30f84a16e4 Mon Sep 17 00:00:00 2001 From: Max Date: Fri, 7 Nov 2025 19:15:45 +0100 Subject: [PATCH 19/37] chore(test): wait for close request when closing Signed-off-by: Max --- playwright/support/fixtures/File.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/playwright/support/fixtures/File.ts b/playwright/support/fixtures/File.ts index 6bed16da931..cb14d4e441a 100644 --- a/playwright/support/fixtures/File.ts +++ b/playwright/support/fixtures/File.ts @@ -49,6 +49,7 @@ export class File { async close() { await this.page.getByRole('button', { name: 'Close', exact: true }).click() + await this.page.waitForRequest(/close/) await expect(this.page.getByLabel(this.name, { exact: true })) .not.toBeVisible() } From cc7032416e38a3403cd1fe89e7dde0d2553bda83 Mon Sep 17 00:00:00 2001 From: Max Date: Fri, 13 Jun 2025 08:47:24 +0200 Subject: [PATCH 20/37] wip: try y-indexeddb For now text is sometimes duplicated Signed-off-by: Max --- package-lock.json | 29 +++++++++++++++++++++++++++++ package.json | 1 + src/components/Editor.vue | 17 +++++++++++------ 3 files changed, 41 insertions(+), 6 deletions(-) diff --git a/package-lock.json b/package-lock.json index c9a6318292f..36f0f25da25 100644 --- a/package-lock.json +++ b/package-lock.json @@ -86,6 +86,7 @@ "vue-click-outside": "^1.1.0", "vue-material-design-icons": "^5.3.1", "webdav": "^5.8.0", + "y-indexeddb": "^9.0.12", "y-prosemirror": "^1.3.7", "y-protocols": "^1.0.6", "yjs": "^13.6.27" @@ -20913,6 +20914,26 @@ "node": ">=0.4" } }, + "node_modules/y-indexeddb": { + "version": "9.0.12", + "resolved": "https://registry.npmjs.org/y-indexeddb/-/y-indexeddb-9.0.12.tgz", + "integrity": "sha512-9oCFRSPPzBK7/w5vOkJBaVCQZKHXB/v6SIT+WYhnJxlEC61juqG0hBrAf+y3gmSMLFLwICNH9nQ53uscuse6Hg==", + "license": "MIT", + "dependencies": { + "lib0": "^0.2.74" + }, + "engines": { + "node": ">=16.0.0", + "npm": ">=8.0.0" + }, + "funding": { + "type": "GitHub Sponsors ❤", + "url": "https://github.com/sponsors/dmonad" + }, + "peerDependencies": { + "yjs": "^13.0.0" + } + }, "node_modules/y-prosemirror": { "version": "1.3.7", "resolved": "https://registry.npmjs.org/y-prosemirror/-/y-prosemirror-1.3.7.tgz", @@ -35289,6 +35310,14 @@ "integrity": "sha512-LKYU1iAXJXUgAXn9URjiu+MWhyUXHsvfp7mcuYm9dSUKK0/CjtrUwFAxD82/mCWbtLsGjFIad0wIsod4zrTAEQ==", "dev": true }, + "y-indexeddb": { + "version": "9.0.12", + "resolved": "https://registry.npmjs.org/y-indexeddb/-/y-indexeddb-9.0.12.tgz", + "integrity": "sha512-9oCFRSPPzBK7/w5vOkJBaVCQZKHXB/v6SIT+WYhnJxlEC61juqG0hBrAf+y3gmSMLFLwICNH9nQ53uscuse6Hg==", + "requires": { + "lib0": "^0.2.74" + } + }, "y-prosemirror": { "version": "1.3.7", "resolved": "https://registry.npmjs.org/y-prosemirror/-/y-prosemirror-1.3.7.tgz", diff --git a/package.json b/package.json index 682ac4220a9..2945dbe1643 100644 --- a/package.json +++ b/package.json @@ -106,6 +106,7 @@ "vue-click-outside": "^1.1.0", "vue-material-design-icons": "^5.3.1", "webdav": "^5.8.0", + "y-indexeddb": "^9.0.12", "y-prosemirror": "^1.3.7", "y-protocols": "^1.0.6", "yjs": "^13.6.27" diff --git a/src/components/Editor.vue b/src/components/Editor.vue index 70190a9ca89..33b7685bcf2 100644 --- a/src/components/Editor.vue +++ b/src/components/Editor.vue @@ -87,7 +87,8 @@ import { File } from '@nextcloud/files' import { Collaboration } from '@tiptap/extension-collaboration' import { useElementSize } from '@vueuse/core' import { defineComponent, ref, shallowRef, watch } from 'vue' -import { Doc } from 'yjs' +import { IndexeddbPersistence } from 'y-indexeddb' +import { Doc, logUpdate } from 'yjs' import Autofocus from '../extensions/Autofocus.js' import { provideEditor } from '../composables/useEditor.ts' @@ -400,11 +401,15 @@ export default defineComponent({ exposeForDebugging(this) }, created() { + this.$indexedDbProvider = new IndexeddbPersistence(this.fileId, this.ydoc) + this.$indexedDbProvider.on('synced', (provider) => { + console.info('synced from indexeddb', provider) + }) // The following can be useful for debugging ydoc updates - // this.ydoc.on('update', function(update, origin, doc, tr) { - // console.debug('ydoc update', update, origin, doc, tr) - // Y.logUpdate(update) - // }); + this.ydoc.on('update', function (update, origin, doc, tr) { + console.debug('ydoc update', update, origin, doc, tr) + logUpdate(update) + }) this.$attachmentResolver = null if (this.active && this.hasDocumentParameters) { this.initSession() @@ -528,7 +533,7 @@ export default defineComponent({ this.document = document this.syncError = null - this.setEditable(this.editMode && !this.requireReconnect) + this.setEditable(this.editMode) // && !this.requireReconnect) }, onCreate({ editor }) { From 754ba13c6fb80e3c993abfd523e300aecd0911d8 Mon Sep 17 00:00:00 2001 From: Max Date: Thu, 4 Sep 2025 16:10:53 +0200 Subject: [PATCH 21/37] chore(split) useIndexedDbProvider from Editor.vue Signed-off-by: Max --- src/components/Editor.vue | 8 +++----- src/composables/useIndexedDbProvider.ts | 26 +++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 5 deletions(-) create mode 100644 src/composables/useIndexedDbProvider.ts diff --git a/src/components/Editor.vue b/src/components/Editor.vue index 33b7685bcf2..7e256acbeda 100644 --- a/src/components/Editor.vue +++ b/src/components/Editor.vue @@ -87,7 +87,6 @@ import { File } from '@nextcloud/files' import { Collaboration } from '@tiptap/extension-collaboration' import { useElementSize } from '@vueuse/core' import { defineComponent, ref, shallowRef, watch } from 'vue' -import { IndexeddbPersistence } from 'y-indexeddb' import { Doc, logUpdate } from 'yjs' import Autofocus from '../extensions/Autofocus.js' @@ -102,6 +101,7 @@ import { provideConnection } from '../composables/useConnection.ts' import { useDelayedFlag } from '../composables/useDelayedFlag.ts' import { useEditorMethods } from '../composables/useEditorMethods.ts' import { provideEditorWidth } from '../composables/useEditorWidth.ts' +import { useIndexedDbProvider } from '../composables/useIndexedDbProvider.ts' import { provideSaveService } from '../composables/useSaveService.ts' import { provideSyncService } from '../composables/useSyncService.ts' import { useSyntaxHighlighting } from '../composables/useSyntaxHighlighting.ts' @@ -229,6 +229,8 @@ export default defineComponent({ }) const ydoc = new Doc() const awareness = new Awareness(ydoc) + useIndexedDbProvider(props, ydoc) + const hasConnectionIssue = ref(false) const { delayed: requireReconnect } = useDelayedFlag(hasConnectionIssue) const { isPublic, isRichEditor, isRichWorkspace } = provideEditorFlags(props) @@ -401,10 +403,6 @@ export default defineComponent({ exposeForDebugging(this) }, created() { - this.$indexedDbProvider = new IndexeddbPersistence(this.fileId, this.ydoc) - this.$indexedDbProvider.on('synced', (provider) => { - console.info('synced from indexeddb', provider) - }) // The following can be useful for debugging ydoc updates this.ydoc.on('update', function (update, origin, doc, tr) { console.debug('ydoc update', update, origin, doc, tr) diff --git a/src/composables/useIndexedDbProvider.ts b/src/composables/useIndexedDbProvider.ts new file mode 100644 index 00000000000..12772086a34 --- /dev/null +++ b/src/composables/useIndexedDbProvider.ts @@ -0,0 +1,26 @@ +/** + * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import { IndexeddbPersistence } from 'y-indexeddb' +import type { Doc } from 'yjs' + +/** + * Initialize a indexed db provider for the given ydoc + * @param props Props of the editor component. + * @param props.fileId Fileid of the file. + * @param ydoc Document to sync via the provider + */ +export function useIndexedDbProvider( + props: { + fileId: number + }, + ydoc: Doc, +) { + const name = `${props.fileId}` + const indexedDbProvider = new IndexeddbPersistence(name, ydoc) + indexedDbProvider.on('synced', (provider: IndexeddbPersistence) => { + console.info('synced from indexeddb', provider) + }) +} From 46235fb0735fc911a2a379d0de419a94e09cc147 Mon Sep 17 00:00:00 2001 From: Max Date: Thu, 4 Sep 2025 16:57:06 +0200 Subject: [PATCH 22/37] fix(cron): do not reset document Keep the baseVersionEtag and the editing session around in case people who are offline connect again later. Signed-off-by: Max --- lib/Cron/Cleanup.php | 6 ------ 1 file changed, 6 deletions(-) diff --git a/lib/Cron/Cleanup.php b/lib/Cron/Cleanup.php index b7f29f6a4e7..a1490d08a59 100644 --- a/lib/Cron/Cleanup.php +++ b/lib/Cron/Cleanup.php @@ -11,7 +11,6 @@ namespace OCA\Text\Cron; -use OCA\Text\Exception\DocumentHasUnsavedChangesException; use OCA\Text\Service\AttachmentService; use OCA\Text\Service\DocumentService; use OCA\Text\Service\SessionService; @@ -42,11 +41,6 @@ protected function run($argument): void { // Inactive sessions will get removed further down and will trigger a reset next time continue; } - - try { - $this->documentService->resetDocument($document->getId()); - } catch (DocumentHasUnsavedChangesException) { - } $this->attachmentService->cleanupAttachments($document->getId()); } From 1f1b05dc5f37b6a75a6f5f1e40eeae474891670e Mon Sep 17 00:00:00 2001 From: Max Date: Thu, 4 Sep 2025 16:59:45 +0200 Subject: [PATCH 23/37] enh(yjs): store baseVersionEtag alongside doc ... and use it to check if the server is still on the same session. Signed-off-by: Max --- cypress/e2e/api/SyncServiceProvider.spec.js | 21 +++++---- src/components/Editor.vue | 11 ++++- src/composables/useConnection.ts | 47 +++++++++++++++------ src/composables/useIndexedDbProvider.ts | 20 +++++++++ src/tests/services/SyncService.spec.ts | 15 +++++-- 5 files changed, 86 insertions(+), 28 deletions(-) diff --git a/cypress/e2e/api/SyncServiceProvider.spec.js b/cypress/e2e/api/SyncServiceProvider.spec.js index 8844754aab0..257315af0e4 100644 --- a/cypress/e2e/api/SyncServiceProvider.spec.js +++ b/cypress/e2e/api/SyncServiceProvider.spec.js @@ -43,15 +43,20 @@ describe('Sync service provider', function () { */ function createProvider(ydoc) { const relativePath = '.' - const { connection, openConnection, baseVersionEtag } = provideConnection({ - fileId, - relativePath, - }) - const { syncService } = provideSyncService( - connection, - openConnection, - baseVersionEtag, + let baseVersionEtag + const setBaseVersionEtag = (val) => { + baseVersionEtag = val + } + const getBaseVersionEtag = () => baseVersionEtag + const { connection, openConnection } = provideConnection( + { + fileId, + relativePath, + }, + getBaseVersionEtag, + setBaseVersionEtag, ) + const { syncService } = provideSyncService(connection, openConnection) const queue = [] syncService.bus.on('opened', () => syncService.startSync()) return createSyncServiceProvider({ diff --git a/src/components/Editor.vue b/src/components/Editor.vue index 7e256acbeda..42f73479d8d 100644 --- a/src/components/Editor.vue +++ b/src/components/Editor.vue @@ -229,7 +229,10 @@ export default defineComponent({ }) const ydoc = new Doc() const awareness = new Awareness(ydoc) - useIndexedDbProvider(props, ydoc) + const { getBaseVersionEtag, setBaseVersionEtag } = useIndexedDbProvider( + props, + ydoc, + ) const hasConnectionIssue = ref(false) const { delayed: requireReconnect } = useDelayedFlag(hasConnectionIssue) @@ -238,7 +241,11 @@ export default defineComponent({ isRichEditor, props, ) - const { connection, openConnection } = provideConnection(props) + const { connection, openConnection } = provideConnection( + props, + getBaseVersionEtag, + setBaseVersionEtag, + ) const { syncService } = provideSyncService(connection, openConnection) const extensions = [ Autofocus.configure({ fileId: props.fileId }), diff --git a/src/composables/useConnection.ts b/src/composables/useConnection.ts index b09558968dc..1440cbc074f 100644 --- a/src/composables/useConnection.ts +++ b/src/composables/useConnection.ts @@ -41,20 +41,26 @@ export const openDataKey = Symbol('text:opendata') as InjectionKey< * @param props.relativePath Relative path to the file. * @param props.initialSession Initial session handed to the editor in direct editing * @param props.shareToken Share token of the file. + * @param getBaseVersionEtag Async getter function for the base version etag. + * @param setBaseVersionEtag Async setter function for the base version etag. */ -export function provideConnection(props: { - fileId: number - relativePath: string - initialSession?: InitialData - shareToken?: string -}) { - let baseVersionEtag: string | undefined +export function provideConnection( + props: { + fileId: number + relativePath: string + initialSession?: InitialData + shareToken?: string + }, + getBaseVersionEtag: () => Promise, + setBaseVersionEtag: (val: string) => Promise, +) { const connection = shallowRef(undefined) const openData = shallowRef(undefined) const openConnection = async () => { + const baseVersionEtag = await getBaseVersionEtag() const guestName = localStorage.getItem('nick') ?? '' const { connection: opened, data } = - openInitialSession(props) + openInitialSession(props, baseVersionEtag) || (await open({ fileId: props.fileId, guestName, @@ -62,7 +68,7 @@ export function provideConnection(props: { filePath: props.relativePath, baseVersionEtag, })) - baseVersionEtag = data.document.baseVersionEtag + await setBaseVersionEtag(data.document.baseVersionEtag) connection.value = opened openData.value = data return data @@ -84,14 +90,27 @@ export const useConnection = () => { * @param props.relativePath Relative path to the file. * @param props.initialSession Initial session handed to the editor in direct editing * @param props.shareToken Share token of the file. + * @param baseVersionEtag Etag from the last editing session. */ -function openInitialSession(props: { - relativePath: string - initialSession?: InitialData - shareToken?: string -}) { +function openInitialSession( + props: { + relativePath: string + initialSession?: InitialData + shareToken?: string + }, + baseVersionEtag: string | undefined, +) { if (props.initialSession) { const { document, session } = props.initialSession + if (baseVersionEtag && baseVersionEtag !== document.baseVersionEtag) { + throw new Error( + 'Base version etag did not match when opening initial session.', + ) + // In order to handle this properly we'd need to: + // * fetch the file content. + // * throw the same exception as a 409 response. + // * include the file content as `outsideChange` in the error. + } const connection = { documentId: document.id, sessionId: session.id, diff --git a/src/composables/useIndexedDbProvider.ts b/src/composables/useIndexedDbProvider.ts index 12772086a34..e6bf919171e 100644 --- a/src/composables/useIndexedDbProvider.ts +++ b/src/composables/useIndexedDbProvider.ts @@ -23,4 +23,24 @@ export function useIndexedDbProvider( indexedDbProvider.on('synced', (provider: IndexeddbPersistence) => { console.info('synced from indexeddb', provider) }) + + /** + * Get the base version etag the document had when it was edited last. + */ + function getBaseVersionEtag(): Promise { + return indexedDbProvider.get('baseVersionEtag') + } + + /** + * Set the base version etag for the current connection. + * @param val the base version etag as returned by open. + */ + function setBaseVersionEtag(val: string) { + return indexedDbProvider.set('baseVersionEtag', val) + } + + return { + getBaseVersionEtag, + setBaseVersionEtag, + } } diff --git a/src/tests/services/SyncService.spec.ts b/src/tests/services/SyncService.spec.ts index 412615d993c..4e4fbfad7d6 100644 --- a/src/tests/services/SyncService.spec.ts +++ b/src/tests/services/SyncService.spec.ts @@ -43,16 +43,23 @@ const openResult = { connection, data: initialData } describe('Sync service', () => { it('opens a connection', async () => { - const { connection, openConnection, openData } = provideConnection({ - fileId: 123, - relativePath: './', - }) + const getBaseVersionEtag = vi.fn() + const setBaseVersionEtag = vi.fn() + const { connection, openConnection, openData } = provideConnection( + { + fileId: 123, + relativePath: './', + }, + getBaseVersionEtag, + setBaseVersionEtag, + ) vi.mock('../../apis/connect') vi.mocked(connect.open).mockResolvedValue(openResult) const openHandler = vi.fn() const service = new SyncService({ connection, openConnection }) service.bus.on('opened', openHandler) await service.open() + expect(setBaseVersionEtag).toHaveBeenCalledWith('etag') expect(openHandler).toHaveBeenCalledWith( expect.objectContaining({ session: initialData.session }), ) From e4430a8863cb77e14367f0d30402130b06b8f884 Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 14 Oct 2025 11:11:40 +0200 Subject: [PATCH 24/37] fix(offline): persist dirty state in indexed db When reopening a document that was edited offline it will also be considered dirty now. Autosave will not kick in yet... As no steps are pushed. But when closing the file it will be saved. Signed-off-by: Max --- src/components/Editor.vue | 8 +++----- src/composables/useIndexedDbProvider.ts | 10 ++++++++++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/components/Editor.vue b/src/components/Editor.vue index 42f73479d8d..863eea5faf0 100644 --- a/src/components/Editor.vue +++ b/src/components/Editor.vue @@ -229,10 +229,8 @@ export default defineComponent({ }) const ydoc = new Doc() const awareness = new Awareness(ydoc) - const { getBaseVersionEtag, setBaseVersionEtag } = useIndexedDbProvider( - props, - ydoc, - ) + const { dirty, getBaseVersionEtag, setBaseVersionEtag } = + useIndexedDbProvider(props, ydoc) const hasConnectionIssue = ref(false) const { delayed: requireReconnect } = useDelayedFlag(hasConnectionIssue) @@ -286,6 +284,7 @@ export default defineComponent({ return { awareness, connection, + dirty, editor, el, hasConnectionIssue, @@ -314,7 +313,6 @@ export default defineComponent({ fileNode: null, idle: false, - dirty: false, contentLoaded: false, syncError: null, readOnly: true, diff --git a/src/composables/useIndexedDbProvider.ts b/src/composables/useIndexedDbProvider.ts index e6bf919171e..5423ed8dc5d 100644 --- a/src/composables/useIndexedDbProvider.ts +++ b/src/composables/useIndexedDbProvider.ts @@ -3,6 +3,7 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ +import { ref, watch } from 'vue' import { IndexeddbPersistence } from 'y-indexeddb' import type { Doc } from 'yjs' @@ -23,6 +24,14 @@ export function useIndexedDbProvider( indexedDbProvider.on('synced', (provider: IndexeddbPersistence) => { console.info('synced from indexeddb', provider) }) + const dirty = ref(false) + indexedDbProvider.get('dirty').then((val) => { + dirty.value = Boolean(val) + }) + + watch(dirty, (val) => { + indexedDbProvider.set('dirty', val ? 1 : 0) + }) /** * Get the base version etag the document had when it was edited last. @@ -40,6 +49,7 @@ export function useIndexedDbProvider( } return { + dirty, getBaseVersionEtag, setBaseVersionEtag, } From dde4d14b4f45bfcca360301c0fdd891d93de2f61 Mon Sep 17 00:00:00 2001 From: Max Date: Wed, 22 Oct 2025 19:28:08 +0200 Subject: [PATCH 25/37] chore(test): explore empty changesets Signed-off-by: Max --- src/tests/upstream/yjs.spec.ts | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/tests/upstream/yjs.spec.ts b/src/tests/upstream/yjs.spec.ts index 38834512559..dc87c35eb8a 100644 --- a/src/tests/upstream/yjs.spec.ts +++ b/src/tests/upstream/yjs.spec.ts @@ -42,4 +42,25 @@ describe('Yjs', function () { expect(targetMap.get('keyB')).to.be.eq('valueB') expect(targetMap.get('keyC')).to.be.eq('valueC') }) + + it('detect empty updates', function () { + const source = new Doc() + const update0 = encodeStateAsUpdate(source) + expect(update0).toMatchInlineSnapshot(` + Uint8Array [ + 0, + 0, + ] + `) + const sourceMap = source.getMap() + sourceMap.set('keyA', 'valueA') + const sourceVectorA = encodeStateVector(source) + const updateAA = encodeStateAsUpdate(source, sourceVectorA) + expect(updateAA).toMatchInlineSnapshot(` + Uint8Array [ + 0, + 0, + ] + `) + }) }) From c29d66e3f2e81d8177421bb3c0fc4645045561ec Mon Sep 17 00:00:00 2001 From: Max Date: Sun, 26 Oct 2025 20:19:53 +0100 Subject: [PATCH 26/37] chore(rename): use privateMethods for emitError and emitDocumentStateStep Signed-off-by: Max --- src/services/SyncService.ts | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/src/services/SyncService.ts b/src/services/SyncService.ts index 8d5572a50af..9f4dd951f51 100644 --- a/src/services/SyncService.ts +++ b/src/services/SyncService.ts @@ -164,7 +164,7 @@ class SyncService { if (this.hasActiveConnection()) { return } - const data = await this.#openConnection().catch((e) => this._emitError(e)) + const data = await this.#openConnection().catch((e) => this.#emitError(e)) if (!data) { // Error was already emitted above return @@ -178,7 +178,7 @@ class SyncService { this.bus.emit('opened', data) // Emit sync after opened, so websocket onmessage comes after onopen. if (data.documentState) { - this._emitDocumentStateStep( + this.#emitDocumentStateStep( data.documentState, data.document.lastSavedVersion, ) @@ -193,18 +193,15 @@ class SyncService { this.backend?.resetRefetchTimer() } - _emitError(error: { response?: object; code?: string }) { - if (!error.response || error.code === 'ECONNABORTED') { - this.bus.emit('error', { type: ERROR_TYPE.CONNECTION_FAILED, data: {} }) - } else { - this.bus.emit('error', { - type: ERROR_TYPE.LOAD_ERROR, - data: error.response, - }) - } + #emitError(error: { response?: object; code?: string }) { + const eventData = + !error.response || error.code === 'ECONNABORTED' + ? { type: ERROR_TYPE.CONNECTION_FAILED, data: {} } + : { type: ERROR_TYPE.LOAD_ERROR, data: error.response } + this.bus.emit('error', eventData) } - _emitDocumentStateStep(documentState: string, version: number) { + #emitDocumentStateStep(documentState: string, version: number) { const documentStateStep = documentStateToStep(documentState, version) this.bus.emit('sync', { steps: [documentStateStep], @@ -257,7 +254,7 @@ class SyncService { version: number } if (documentState) { - this._emitDocumentStateStep(documentState, version) + this.#emitDocumentStateStep(documentState, version) } this.pushError = 0 this.#sending = false From d7d029e1fdac4b01147d8cac4f39e766d1a6c099 Mon Sep 17 00:00:00 2001 From: Max Date: Sun, 26 Oct 2025 20:28:53 +0100 Subject: [PATCH 27/37] chore(cleanup): _getContent alias for serialize Signed-off-by: Max --- src/services/SaveService.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/services/SaveService.ts b/src/services/SaveService.ts index 4d3f4fa8555..621f2c865b6 100644 --- a/src/services/SaveService.ts +++ b/src/services/SaveService.ts @@ -54,10 +54,6 @@ class SaveService { return this.syncService.bus.emit } - _getContent() { - return this.serialize() - } - async save({ force = false, manualSave = true } = {}) { logger.debug('[SaveService] saving', { force, manualSave }) if (!this.connection.value) { @@ -67,7 +63,7 @@ class SaveService { try { const response = await save(this.connection.value, { version: this.version, - autosaveContent: this._getContent(), + autosaveContent: this.serialize(), documentState: this.getDocumentState(), force, manualSave, @@ -88,7 +84,7 @@ class SaveService { } saveViaSendBeacon(this.connection.value, { version: this.version, - autosaveContent: this._getContent(), + autosaveContent: this.serialize(), documentState: this.getDocumentState(), }) && logger.debug('[SaveService] saved using sendBeacon') } From ff9a63f33561d017487da46e82a1d9d8b9c5ef80 Mon Sep 17 00:00:00 2001 From: Max Date: Sun, 26 Oct 2025 20:47:12 +0100 Subject: [PATCH 28/37] chore(refactor): handle open data in websocket polyfill Signed-off-by: Max --- src/helpers/yjs.ts | 16 ++++++++++++++++ src/services/SyncService.ts | 7 ------- src/services/WebSocketPolyfill.ts | 6 +++++- 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/helpers/yjs.ts b/src/helpers/yjs.ts index 7e87ca249fb..ff74b522299 100644 --- a/src/helpers/yjs.ts +++ b/src/helpers/yjs.ts @@ -7,6 +7,7 @@ import * as decoding from 'lib0/decoding.js' import * as encoding from 'lib0/encoding.js' import * as syncProtocol from 'y-protocols/sync' import * as Y from 'yjs' +import type { OpenData } from '../apis/connect' import type { Step } from '../services/SyncService' import { messageSync } from '../services/y-websocket.js' import { decodeArrayBuffer, encodeArrayBuffer } from './base64' @@ -37,6 +38,21 @@ export function applyDocumentState( Y.applyUpdate(ydoc, update, origin) } +/** + * Create a steps from the open response + * i.e. create a sync protocol update message from the document state + * and encode it and wrap it in a step data structure. + * + * @param data - data returned by the open request + * @return steps extracted from the open data. + */ +export function stepsFromOpenData(data: OpenData): Step[] { + if (!data.documentState) { + return [] + } + return [documentStateToStep(data.documentState, data.document.lastSavedVersion)] +} + /** * Create a step from a document state * i.e. create a sync protocol update message from it diff --git a/src/services/SyncService.ts b/src/services/SyncService.ts index 9f4dd951f51..a46d3275c05 100644 --- a/src/services/SyncService.ts +++ b/src/services/SyncService.ts @@ -176,13 +176,6 @@ class SyncService { this.backend = new PollingBackend(this, this.connection.value, data) // Make sure to only emit this once the backend is in place. this.bus.emit('opened', data) - // Emit sync after opened, so websocket onmessage comes after onopen. - if (data.documentState) { - this.#emitDocumentStateStep( - data.documentState, - data.document.lastSavedVersion, - ) - } } startSync() { diff --git a/src/services/WebSocketPolyfill.ts b/src/services/WebSocketPolyfill.ts index 72b6f8ef813..1e0389ff1e3 100644 --- a/src/services/WebSocketPolyfill.ts +++ b/src/services/WebSocketPolyfill.ts @@ -3,8 +3,10 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ +import type { OpenData } from '../apis/connect' import { decodeArrayBuffer, encodeArrayBuffer } from '../helpers/base64' import { logger } from '../helpers/logger.js' +import { stepsFromOpenData } from '../helpers/yjs' import getNotifyBus from './NotifyService' import type { Step, SyncService } from './SyncService' @@ -35,10 +37,11 @@ export default function initWebSocketPolyfill( this.#url = url logger.debug('WebSocketPolyfill#constructor', { url, fileId }) - this.#onOpened = () => { + this.#onOpened = (data: OpenData) => { if (syncService.hasActiveConnection()) { this.onopen?.() } + this.#processSteps(stepsFromOpenData(data)) } syncService.bus.on('opened', this.#onOpened) @@ -104,6 +107,7 @@ export default function initWebSocketPolyfill( async close() { syncService.bus.off('sync', this.#onSync) + syncService.bus.off('opened', this.#onOpened) this.#notifyPushBus?.off('notify_push', this.#onNotifyPush.bind(this)) this.onclose?.(new CloseEvent('closing')) logger.debug('Websocket closed') From 207b4c5e64fd319f15fe40db866699c1f14a1b52 Mon Sep 17 00:00:00 2001 From: Max Date: Sun, 26 Oct 2025 20:59:26 +0100 Subject: [PATCH 29/37] fix(sync): only accept sync protocol and return sync step 2 Signed-off-by: Max --- cypress/e2e/api/SessionApi.spec.js | 51 ++++++++++++------------------ lib/Service/DocumentService.php | 8 +++-- 2 files changed, 27 insertions(+), 32 deletions(-) diff --git a/cypress/e2e/api/SessionApi.spec.js b/cypress/e2e/api/SessionApi.spec.js index ec52f020af9..66caf9c297d 100644 --- a/cypress/e2e/api/SessionApi.spec.js +++ b/cypress/e2e/api/SessionApi.spec.js @@ -73,23 +73,19 @@ describe('The session Api', function () { cy.closeConnection(connection) }) - // Echoes all message types but queries - Object.entries(messages) - .filter(([key, _value]) => key !== 'query') - .forEach(([type, sample]) => { - it(`echos ${type} messages`, function () { - const steps = [sample] - const version = 0 - cy.pushSteps({ connection, steps, version }) - .its('version') - .should('eql', 0) - cy.syncSteps(connection) - .its('steps[0].data') - .should('eql', steps) - }) + // Echoes updates and responses + ;['update', 'response'].forEach((type) => { + it(`echos ${type} messages`, function () { + const steps = [messages[type]] + const version = 0 + cy.pushSteps({ connection, steps, version }) + .its('version') + .should('eql', 0) + cy.syncSteps(connection).its('steps[0].data').should('eql', steps) }) + }) - it('responds to queries', function () { + it('responds to queries with updates and responses', function () { const version = 0 Object.entries(messages).forEach(([type, sample]) => { cy.pushSteps({ connection, steps: [sample], version }) @@ -97,10 +93,13 @@ describe('The session Api', function () { cy.pushSteps({ connection, steps: [messages.query], version }).then( (response) => { cy.wrap(response).its('version').should('eql', 0) - cy.wrap(response).its('steps.length').should('eql', 1) + cy.wrap(response).its('steps.length').should('eql', 2) cy.wrap(response) .its('steps[0].data') .should('eql', [messages.update]) + cy.wrap(response) + .its('steps[1].data') + .should('eql', [messages.response]) }, ) }) @@ -111,7 +110,6 @@ describe('The session Api', function () { let connection let fileId let filePath - let joining beforeEach(function () { cy.testName().then((name) => { @@ -155,13 +153,10 @@ describe('The session Api', function () { manualSave: true, }) cy.openConnection({ fileId, filePath }) - .then(({ connection: con, data }) => { - joining = con - return data - }) - .its('documentState') + .as('joining') + .its('data.documentState') .should('eql', documentState) - cy.closeConnection(joining) + cy.get('@joining').its('connection').then(cy.closeConnection) }) afterEach(function () { @@ -174,7 +169,6 @@ describe('The session Api', function () { let connection let filePath let shareToken - let joining beforeEach(function () { cy.testName().then((name) => { @@ -230,13 +224,10 @@ describe('The session Api', function () { manualSave: true, }) cy.openConnection({ filePath: '', token: shareToken }) - .then(({ connection: con, data }) => { - joining = con - return data - }) - .its('documentState') + .as('joining') + .its('data.documentState') .should('eql', documentState) - cy.closeConnection(joining) + cy.get('@joining').its('connection').then(cy.closeConnection) }) }) diff --git a/lib/Service/DocumentService.php b/lib/Service/DocumentService.php index 6204e0c08ed..258923657b2 100644 --- a/lib/Service/DocumentService.php +++ b/lib/Service/DocumentService.php @@ -216,8 +216,12 @@ public function addStep(Document $document, Session $session, array $steps, int if ($readOnly && $message->isUpdate()) { continue; } + // Only accept sync protocol + if ($message->getYjsMessageType() !== YjsMessage::YJS_MESSAGE_SYNC) { + continue; + } // Filter out query steps as they would just trigger clients to send their steps again - if ($message->getYjsMessageType() === YjsMessage::YJS_MESSAGE_SYNC && $message->getYjsSyncType() === YjsMessage::YJS_MESSAGE_SYNC_STEP1) { + if ($message->getYjsSyncType() === YjsMessage::YJS_MESSAGE_SYNC_STEP1) { $stepsIncludeQuery = true; } else { $stepsToInsert[] = $step; @@ -257,7 +261,7 @@ public function addStep(Document $document, Session $session, array $steps, int $stepsToReturn = []; foreach ($allSteps as $step) { $message = YjsMessage::fromBase64($step->getData()); - if ($message->getYjsMessageType() === YjsMessage::YJS_MESSAGE_SYNC && $message->getYjsSyncType() === YjsMessage::YJS_MESSAGE_SYNC_UPDATE) { + if ($message->isUpdate()) { $stepsToReturn[] = $step; } } From 4f0611722333630d1548f0e1404ebddc6da006c7 Mon Sep 17 00:00:00 2001 From: Max Date: Mon, 27 Oct 2025 21:14:48 +0100 Subject: [PATCH 30/37] enh(sync): recover automatically from outdated / renamed doc If no changes have been made offline clear the indexedDb cache and reload Editor.vue to load the latest editing session from the server. Signed-off-by: Max --- src/components/Editor.vue | 22 +++++++++++++++++++++- src/components/ViewerComponent.vue | 13 +++++++++++-- src/composables/useConnection.ts | 6 +++++- src/composables/useIndexedDbProvider.ts | 9 +++++++++ 4 files changed, 46 insertions(+), 4 deletions(-) diff --git a/src/components/Editor.vue b/src/components/Editor.vue index 863eea5faf0..4276fec6438 100644 --- a/src/components/Editor.vue +++ b/src/components/Editor.vue @@ -229,7 +229,7 @@ export default defineComponent({ }) const ydoc = new Doc() const awareness = new Awareness(ydoc) - const { dirty, getBaseVersionEtag, setBaseVersionEtag } = + const { dirty, getBaseVersionEtag, setBaseVersionEtag, clearIndexedDb } = useIndexedDbProvider(props, ydoc) const hasConnectionIssue = ref(false) @@ -283,6 +283,7 @@ export default defineComponent({ return { awareness, + clearIndexedDb, connection, dirty, editor, @@ -338,6 +339,13 @@ export default defineComponent({ hasDocumentParameters() { return this.fileId || this.shareToken || this.initialSession }, + hasOutdatedDocument() { + return ( + this.syncError + && this.syncError.type === ERROR_TYPE.LOAD_ERROR + && this.syncError.data.status === 412 + ) + }, currentDirectory() { return this.relativePath ? this.relativePath.split('/').slice(0, -1).join('/') @@ -392,6 +400,18 @@ export default defineComponent({ } this.setEditable(!val) }, + hasOutdatedDocument(val) { + if (!val) { + return + } + if (this.dirty) { + // handle conflict between active editing session and offline content + } else { + // clear the outdated cached content and reload without it. + this.clearIndexedDb() + this.emit('reload') + } + }, }, mounted() { if (!this.richWorkspace) { diff --git a/src/components/ViewerComponent.vue b/src/components/ViewerComponent.vue index d7a9837d185..2727db11920 100644 --- a/src/components/ViewerComponent.vue +++ b/src/components/ViewerComponent.vue @@ -5,7 +5,7 @@