From 165000f80ccf84aaf85ce6e62c3096b7c53ffa3e Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 14 Jul 2026 09:29:51 +0200 Subject: [PATCH 01/15] fix(autosave): every time the typing stops Debounce will delay the execution of the function every time it is called. So autosave was waiting until no updates occured for 30 seconds. That's a long time and does not feel responsive. Try to autosave every time the user stops typing for at least one second. Signed-off-by: Max --- src/services/SaveService.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/services/SaveService.ts b/src/services/SaveService.ts index f0088ded779..021c379a4d6 100644 --- a/src/services/SaveService.ts +++ b/src/services/SaveService.ts @@ -18,7 +18,7 @@ import { ERROR_TYPE } from './SyncService.ts' * * time in ms */ -const AUTOSAVE_INTERVAL = 30000 +const AUTOSAVE_DEBOUNCE = 1000 class SaveService { connection: ShallowRef @@ -42,7 +42,7 @@ class SaveService { this.syncService = syncService this.serialize = serialize this.getDocumentState = getDocumentState - this.autosave = debounce(this._autosave.bind(this), AUTOSAVE_INTERVAL) + this.autosave = debounce(this._autosave.bind(this), AUTOSAVE_DEBOUNCE) this.syncService.bus.on('close', () => { this.autosave.clear() }) @@ -115,6 +115,7 @@ class SaveService { } _autosave() { + logger.debug('_autosave') return this.save({ manualSave: false }).catch((error) => { logger.error('Failed to autosave document.', { error }) // retry in 30 seconds From 5d3c7d82726a9aa3c0c27e448cff6811865ad40d Mon Sep 17 00:00:00 2001 From: Max Date: Sat, 25 Jul 2026 22:06:33 +0200 Subject: [PATCH 02/15] chore(refactor): document tracking into provideSyncService Also removed the document attribute from the `sync` event load. It is already included in the `change` event load. Signed-off-by: Max --- src/components/CollaborativeEditor.vue | 16 +++++----------- src/composables/useSyncService.ts | 23 +++++++++++++++++++++-- src/services/PollingBackend.ts | 12 ++++++------ src/services/SyncService.ts | 5 +---- 4 files changed, 33 insertions(+), 23 deletions(-) diff --git a/src/components/CollaborativeEditor.vue b/src/components/CollaborativeEditor.vue index 35345dc44e2..8504896200a 100644 --- a/src/components/CollaborativeEditor.vue +++ b/src/components/CollaborativeEditor.vue @@ -286,7 +286,7 @@ export default defineComponent({ getBaseVersionEtag, setBaseVersionEtag, ) - const { syncService } = provideSyncService(connection, openConnection) + const { document, syncService } = provideSyncService(connection, openConnection) const extensions = [ Autofocus.configure({ fileId: props.fileId }), Collaboration.configure({ document: ydoc }), @@ -343,6 +343,7 @@ export default defineComponent({ clearIndexedDb, connection, dirty, + document, editor, editorReady, el, @@ -371,7 +372,6 @@ export default defineComponent({ return { IDLE_TIMEOUT, - document: null, fileNode: null, idle: false, @@ -614,8 +614,7 @@ export default defineComponent({ this.idle = false }, - onOpened({ document, session, content, documentState, readOnly }) { - this.document = document + onOpened({ session, content, documentState, readOnly }) { this.readOnly = readOnly this.editMode = !readOnly && !this.openReadOnlyEnabled this.hasConnectionIssue = false @@ -666,9 +665,7 @@ export default defineComponent({ this.updateUser(session) }, - onChange({ document }) { - this.document = document - + onChange() { this.syncError = null this.setEditable(this.editMode) }, @@ -690,7 +687,7 @@ export default defineComponent({ }) }, - onSync({ document }) { + onSync() { this.hasConnectionIssue = this.syncService.backend.fetcher === 0 || !this.syncProvider?.wsconnected @@ -702,9 +699,6 @@ export default defineComponent({ this.$nextTick(() => { this.$emit('syncService:sync') }) - if (document) { - this.document = document - } }, onError({ type, data }) { diff --git a/src/composables/useSyncService.ts b/src/composables/useSyncService.ts index 3f216af3179..9d3016575ed 100644 --- a/src/composables/useSyncService.ts +++ b/src/composables/useSyncService.ts @@ -4,9 +4,10 @@ */ import type { InjectionKey, ShallowRef } from 'vue' +import type { Document } from '../services/SyncService.ts' import type { Connection, InitialData } from './useConnection.ts' -import { inject, provide } from 'vue' +import { inject, onUnmounted, provide, ref } from 'vue' import { SyncService } from '../services/SyncService.ts' const syncServiceKey = Symbol('text:sync') as InjectionKey @@ -26,7 +27,25 @@ export function provideSyncService( openConnection, }) provide(syncServiceKey, syncService) - return { syncService } + + const document = ref() + /** + * Update the document ref based on the event provided + * + * @param event that triggered the update + * @param event.document latest state of the document + */ + function updateDocument({ document: current }: { document: Document }) { + document.value = current + } + syncService.bus.on('opened', updateDocument) + syncService.bus.on('change', updateDocument) + onUnmounted(() => { + syncService.bus.off('opened', updateDocument) + syncService.bus.off('change', updateDocument) + }) + + return { document, syncService } } /** diff --git a/src/services/PollingBackend.ts b/src/services/PollingBackend.ts index 2e8c4181f42..ff6fe94e826 100644 --- a/src/services/PollingBackend.ts +++ b/src/services/PollingBackend.ts @@ -140,23 +140,23 @@ class PollingBackend { } _handleResponse({ data }: { data: PollData }) { - const { document, sessions } = data + const { document, readOnly, sessions, steps } = data this.#fetchRetryCounter = 0 - if (data.readOnly !== undefined && data.readOnly !== this.#readOnly) { - this.#readOnly = data.readOnly + if (readOnly !== undefined && readOnly !== this.#readOnly) { + this.#readOnly = readOnly this.#syncService.bus.emit('permissionChange', { readOnly: this.#readOnly, }) - if (data.readOnly) { + if (readOnly) { this.maximumReadOnlyTimer() } } this.#syncService.bus.emit('change', { document, sessions }) - this.#syncService.receiveSteps(data) + this.#syncService.receiveSteps({ sessions, steps }) - if (data.steps.length === 0) { + if (steps.length === 0) { if (!this.#initialLoadingFinished) { this.#initialLoadingFinished = true } diff --git a/src/services/SyncService.ts b/src/services/SyncService.ts index 5a6f7c06e51..3ccd10107cb 100644 --- a/src/services/SyncService.ts +++ b/src/services/SyncService.ts @@ -110,7 +110,7 @@ export declare type EventTypes = { opened: OpenData /* received new steps */ - sync: { document?: object, steps: Step[] } + sync: { steps: Step[] } /* state changed (dirty) */ stateChange: { initialLoading?: boolean, dirty?: boolean } @@ -294,17 +294,14 @@ class SyncService { receiveSteps({ steps, - document, sessions = [], }: { steps: Step[] - document?: object sessions?: Session[] }) { const versionAfter = Math.max(this.version, ...steps.map((s) => s.version)) this.bus.emit('sync', { steps: [...awarenessSteps(sessions), ...steps], - document, }) if (this.version < versionAfter) { // Steps up to version where emitted but it looks like they were not processed. From 19d7403cfea0fa5a75d52d1308277894b617d3b1 Mon Sep 17 00:00:00 2001 From: Max Date: Sat, 25 Jul 2026 22:45:23 +0200 Subject: [PATCH 03/15] fix(autosave): only save when server is ready The server will only accept autosaves every 10 seconds. `document` contains the last saved timestamp. Compute the time to autosave next. Add a small random delay (up to 3 seconds) to avoid all connected clients from saving at the same time. Signed-off-by: Max --- src/apis/save.ts | 2 +- src/composables/useSaveService.ts | 7 +++++-- src/composables/useSyncService.ts | 2 ++ src/services/SaveService.ts | 35 +++++++++++++++++++++---------- src/services/SyncService.ts | 2 +- 5 files changed, 33 insertions(+), 15 deletions(-) diff --git a/src/apis/save.ts b/src/apis/save.ts index 3a3ab8f3636..4e610358548 100644 --- a/src/apis/save.ts +++ b/src/apis/save.ts @@ -21,7 +21,7 @@ interface SaveData { } interface SaveResponse { - data: Document + data: { document: Document } } /** diff --git a/src/composables/useSaveService.ts b/src/composables/useSaveService.ts index dccfc1c0853..fd6816caae7 100644 --- a/src/composables/useSaveService.ts +++ b/src/composables/useSaveService.ts @@ -3,9 +3,9 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -import type { InjectionKey, ShallowRef } from 'vue' +import type { InjectionKey, Ref, ShallowRef } from 'vue' import type { Doc } from 'yjs' -import type { SyncService } from '../services/SyncService.ts' +import type { Document, SyncService } from '../services/SyncService.ts' import type { Connection } from './useConnection.ts' import { inject, provide } from 'vue' @@ -17,18 +17,21 @@ const saveServiceKey = Symbol('text:save') as InjectionKey /** * * @param connection to the api + * @param document as tracked by the sync service * @param syncService mostly used for the event bus and events * @param serialize to extract the document markdown content * @param ydoc to extract the document state from */ export function provideSaveService( connection: ShallowRef, + document: Ref, syncService: SyncService, serialize: () => string, ydoc: Doc, ) { const saveService = new SaveService({ connection, + document, syncService, serialize, getDocumentState: () => getDocumentState(ydoc), diff --git a/src/composables/useSyncService.ts b/src/composables/useSyncService.ts index 9d3016575ed..e69ca06ed73 100644 --- a/src/composables/useSyncService.ts +++ b/src/composables/useSyncService.ts @@ -40,9 +40,11 @@ export function provideSyncService( } syncService.bus.on('opened', updateDocument) syncService.bus.on('change', updateDocument) + syncService.bus.on('save', updateDocument) onUnmounted(() => { syncService.bus.off('opened', updateDocument) syncService.bus.off('change', updateDocument) + syncService.bus.off('save', updateDocument) }) return { document, syncService } diff --git a/src/services/SaveService.ts b/src/services/SaveService.ts index 021c379a4d6..c185cdf857f 100644 --- a/src/services/SaveService.ts +++ b/src/services/SaveService.ts @@ -3,9 +3,9 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -import type { ShallowRef } from 'vue' +import type { Ref, ShallowRef } from 'vue' import type { Connection } from '../composables/useConnection.ts' -import type { SyncService } from './SyncService.ts' +import type { Document, SyncService } from './SyncService.ts' import { showError } from '@nextcloud/dialogs' import debounce from 'debounce' @@ -13,15 +13,17 @@ import { save, saveViaSendBeacon } from '../apis/save.ts' import { logger } from '../helpers/logger.js' import { ERROR_TYPE } from './SyncService.ts' -/** - * Interval to save the serialized document and the document state - * - * time in ms - */ -const AUTOSAVE_DEBOUNCE = 1000 +// Time constants in seconds: +// Only autosave after 1 second typing breaks +const AUTOSAVE_DEBOUNCE = 1 +// Server only accepts auutosaves every 10 seconds +const SERVER_AUTOSAVE_INTERVAL = 10 +// Randomize save times to prevent all clients saving at the same time. +const MAX_RANDOM_AUTOSAVE_DELAY = 3 class SaveService { connection: ShallowRef + document: Ref syncService serialize getDocumentState @@ -29,20 +31,23 @@ class SaveService { constructor({ connection, + document, syncService, serialize, getDocumentState, }: { connection: ShallowRef + document: Ref syncService: SyncService serialize: () => string getDocumentState: () => string }) { this.connection = connection + this.document = document this.syncService = syncService this.serialize = serialize this.getDocumentState = getDocumentState - this.autosave = debounce(this._autosave.bind(this), AUTOSAVE_DEBOUNCE) + this.autosave = debounce(this._autosave.bind(this), AUTOSAVE_DEBOUNCE * 1000) this.syncService.bus.on('close', () => { this.autosave.clear() }) @@ -70,7 +75,6 @@ class SaveService { force, manualSave, }) - this.emit('stateChange', { dirty: false }) logger.debug('[SaveService] saved', { response }) this.emit('save', response.data) this.autosave.clear() @@ -115,7 +119,16 @@ class SaveService { } _autosave() { - logger.debug('_autosave') + const lastSave = this.document.value?.lastSavedVersionTime ?? 0 + const now = Date.now() / 1000 + // Server won't accept autosaves yet + if (now < lastSave + SERVER_AUTOSAVE_INTERVAL) { + logger.debug('Not autosaving as last save is recent', { lastSave, now }) + const nextSave = lastSave + SERVER_AUTOSAVE_INTERVAL + Math.random() * MAX_RANDOM_AUTOSAVE_DELAY + setTimeout(() => this.autosave(), (nextSave - now) * 1000) + return + } + logger.debug('Autosaving') return this.save({ manualSave: false }).catch((error) => { logger.error('Failed to autosave document.', { error }) // retry in 30 seconds diff --git a/src/services/SyncService.ts b/src/services/SyncService.ts index 3ccd10107cb..b455b3051b6 100644 --- a/src/services/SyncService.ts +++ b/src/services/SyncService.ts @@ -122,7 +122,7 @@ export declare type EventTypes = { change: { sessions: Session[], document: Document } /* Emitted after successful save */ - save: object + save: { document: Document } /* Emitted once a document becomes idle */ idle: void From b3d7743343c20e12804a4221c89cf36c54436b93 Mon Sep 17 00:00:00 2001 From: Max Date: Sun, 26 Jul 2026 19:01:16 +0200 Subject: [PATCH 04/15] fix(autosave): better dirty tracking with versions Keep track of the version that has our changes and compare it to the last saved version on the server. This allows fixing two scenarios: * Server response happily to save but does not actually save. The server will only save new versions every 10 seconds. If the latest save just happened it will still respond with 200 but list the outdated version in the response. Comparing the versions shows that our changes have not been saved yet. * Other user already saved the file. our changes. So far dirty would stay true until WE save our changes. Comparing the versions also shows the file was saved when it was saved by someone else. Signed-off-by: Max --- src/components/CollaborativeEditor.vue | 20 ++++++++---------- src/composables/useSyncService.ts | 28 ++++++++++++++++++++++---- src/services/SyncService.ts | 13 ++++++++---- 3 files changed, 41 insertions(+), 20 deletions(-) diff --git a/src/components/CollaborativeEditor.vue b/src/components/CollaborativeEditor.vue index 8504896200a..7dd1562e9ff 100644 --- a/src/components/CollaborativeEditor.vue +++ b/src/components/CollaborativeEditor.vue @@ -286,7 +286,7 @@ export default defineComponent({ getBaseVersionEtag, setBaseVersionEtag, ) - const { document, syncService } = provideSyncService(connection, openConnection) + const { document, syncService } = provideSyncService(connection, openConnection, setDirty) const extensions = [ Autofocus.configure({ fileId: props.fileId }), Collaboration.configure({ document: ydoc }), @@ -320,6 +320,7 @@ export default defineComponent({ const { saveService } = provideSaveService( connection, + document, syncService, serialize, ydoc, @@ -591,6 +592,7 @@ export default defineComponent({ bus.on('idle', this.onIdle) bus.on('save', this.onSave) bus.on('permissionChange', this.onPermissionChange) + bus.on('changesPushed', this.onChangesPushed) }, unlistenSyncServiceEvents() { @@ -603,6 +605,7 @@ export default defineComponent({ bus.off('idle', this.onIdle) bus.off('save', this.onSave) bus.off('permissionChange', this.onPermissionChange) + bus.off('changesPushed', this.onChangesPushed) }, reconnect() { @@ -752,17 +755,6 @@ export default defineComponent({ } this.$emit('ready') } - if (Object.hasOwn(state, 'dirty')) { - if (state.dirty) { - // ignore initial loading and other automated changes before first user change - if (this.editor.can().undo() || this.editor.can().redo()) { - this.setDirty(state.dirty) - this.saveService.autosave() - } - } else { - this.setDirty(state.dirty) - } - } }, onIdle() { @@ -802,6 +794,10 @@ export default defineComponent({ } }, + onChangesPushed() { + this.saveService.autosave() + }, + onFocus() { this.$emit('focus') }, diff --git a/src/composables/useSyncService.ts b/src/composables/useSyncService.ts index e69ca06ed73..b759b068bdc 100644 --- a/src/composables/useSyncService.ts +++ b/src/composables/useSyncService.ts @@ -7,7 +7,7 @@ import type { InjectionKey, ShallowRef } from 'vue' import type { Document } from '../services/SyncService.ts' import type { Connection, InitialData } from './useConnection.ts' -import { inject, onUnmounted, provide, ref } from 'vue' +import { computed, inject, onUnmounted, provide, ref, watch } from 'vue' import { SyncService } from '../services/SyncService.ts' const syncServiceKey = Symbol('text:sync') as InjectionKey @@ -17,10 +17,12 @@ const syncServiceKey = Symbol('text:sync') as InjectionKey * * @param connection Connection to the text api. * @param openConnection Function to open the connection. + * @param setDirty to udpate the dirty state. */ export function provideSyncService( connection: ShallowRef, openConnection: () => Promise, + setDirty: (val: boolean) => Promise, ) { const syncService = new SyncService({ connection, @@ -35,8 +37,8 @@ export function provideSyncService( * @param event that triggered the update * @param event.document latest state of the document */ - function updateDocument({ document: current }: { document: Document }) { - document.value = current + function updateDocument(event: { document: Document }) { + document.value = event.document } syncService.bus.on('opened', updateDocument) syncService.bus.on('change', updateDocument) @@ -47,7 +49,25 @@ export function provideSyncService( syncService.bus.off('save', updateDocument) }) - return { document, syncService } + const versionWithChanges = ref(0) + /** + * Update the tracked version based on the one in the event + * + * @param event that triggered the update + * @param event.version with changes pushed to the server + */ + function updateVersionWithChanges(event: { version: number }) { + versionWithChanges.value = Math.max(event.version, versionWithChanges.value) + } + syncService.bus.on('changesPushed', updateVersionWithChanges) + onUnmounted(() => { + syncService.bus.off('changesPushed', updateVersionWithChanges) + }) + + const dirty = computed(() => (document.value?.lastSavedVersion ?? 0) < versionWithChanges.value) + watch(dirty, setDirty) + + return { dirty, document, syncService } } /** diff --git a/src/services/SyncService.ts b/src/services/SyncService.ts index b455b3051b6..04243bf2c9d 100644 --- a/src/services/SyncService.ts +++ b/src/services/SyncService.ts @@ -113,7 +113,10 @@ export declare type EventTypes = { sync: { steps: Step[] } /* state changed (dirty) */ - stateChange: { initialLoading?: boolean, dirty?: boolean } + stateChange: { initialLoading?: boolean } + + /* local yjs changes have been pushed to the server */ + changesPushed: { version: number } /* error */ error: { type: ErrorType, data?: object } @@ -232,9 +235,7 @@ class SyncService { this.#sending = true clearInterval(this.#sendIntervalId) this.#sendIntervalId = undefined - if (this.#outbox.hasUpdate) { - this.bus.emit('stateChange', { dirty: true }) - } + const hadUpdate = this.#outbox.hasUpdate if (!this.hasActiveConnection()) { return } @@ -257,6 +258,10 @@ class SyncService { this.#sending = false if (steps?.length > 0) { this.receiveSteps({ steps }) + if (hadUpdate) { + // this.version has been increased in receiveSteps + this.bus.emit('changesPushed', { version: this.version }) + } } }) .catch((err) => { From b114c264d7638a650b5317b6e8a9a1418ba64837 Mon Sep 17 00:00:00 2001 From: Max Date: Mon, 27 Jul 2026 06:50:17 +0200 Subject: [PATCH 05/15] chore(refactor): Save service with its own bus Signed-off-by: Max --- src/components/CollaborativeEditor.vue | 18 ++++----- src/composables/useSaveService.ts | 53 +++++++++++++++++++++++--- src/composables/useSyncService.ts | 44 +-------------------- src/services/SaveService.ts | 20 +++++++--- src/services/SyncService.ts | 3 -- 5 files changed, 71 insertions(+), 67 deletions(-) diff --git a/src/components/CollaborativeEditor.vue b/src/components/CollaborativeEditor.vue index 7dd1562e9ff..14d1231d9af 100644 --- a/src/components/CollaborativeEditor.vue +++ b/src/components/CollaborativeEditor.vue @@ -286,7 +286,7 @@ export default defineComponent({ getBaseVersionEtag, setBaseVersionEtag, ) - const { document, syncService } = provideSyncService(connection, openConnection, setDirty) + const { syncService } = provideSyncService(connection, openConnection) const extensions = [ Autofocus.configure({ fileId: props.fileId }), Collaboration.configure({ document: ydoc }), @@ -318,12 +318,12 @@ export default defineComponent({ ? () => createMarkdownSerializer(editor.schema).serialize(editor.state.doc) : () => serializePlainText(editor.state.doc) - const { saveService } = provideSaveService( + const { document, saveService } = provideSaveService( connection, - document, syncService, serialize, ydoc, + setDirty, ) const syncProvider = shallowRef(null) @@ -590,9 +590,9 @@ export default defineComponent({ bus.on('error', this.onError) bus.on('stateChange', this.onStateChange) bus.on('idle', this.onIdle) - bus.on('save', this.onSave) bus.on('permissionChange', this.onPermissionChange) - bus.on('changesPushed', this.onChangesPushed) + this.saveService.bus.on('error', this.onError) + this.saveService.bus.on('save', this.onSave) }, unlistenSyncServiceEvents() { @@ -603,9 +603,9 @@ export default defineComponent({ bus.off('error', this.onError) bus.off('stateChange', this.onStateChange) bus.off('idle', this.onIdle) - bus.off('save', this.onSave) bus.off('permissionChange', this.onPermissionChange) - bus.off('changesPushed', this.onChangesPushed) + this.saveService.bus.off('error', this.onError) + this.saveService.bus.off('save', this.onSave) }, reconnect() { @@ -794,10 +794,6 @@ export default defineComponent({ } }, - onChangesPushed() { - this.saveService.autosave() - }, - onFocus() { this.$emit('focus') }, diff --git a/src/composables/useSaveService.ts b/src/composables/useSaveService.ts index fd6816caae7..dd825f60c18 100644 --- a/src/composables/useSaveService.ts +++ b/src/composables/useSaveService.ts @@ -3,12 +3,12 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -import type { InjectionKey, Ref, ShallowRef } from 'vue' +import type { InjectionKey, ShallowRef } from 'vue' import type { Doc } from 'yjs' import type { Document, SyncService } from '../services/SyncService.ts' import type { Connection } from './useConnection.ts' -import { inject, provide } from 'vue' +import { computed, inject, onUnmounted, provide, ref, watch } from 'vue' import { getDocumentState } from '../helpers/yjs.ts' import { SaveService } from '../services/SaveService.ts' @@ -17,18 +17,19 @@ const saveServiceKey = Symbol('text:save') as InjectionKey /** * * @param connection to the api - * @param document as tracked by the sync service * @param syncService mostly used for the event bus and events * @param serialize to extract the document markdown content * @param ydoc to extract the document state from + * @param setDirty set the dirty state for the editor */ export function provideSaveService( connection: ShallowRef, - document: Ref, syncService: SyncService, serialize: () => string, ydoc: Doc, + setDirty: (val: boolean) => Promise, ) { + const document = ref() const saveService = new SaveService({ connection, document, @@ -36,8 +37,50 @@ export function provideSaveService( serialize, getDocumentState: () => getDocumentState(ydoc), }) + + syncService.bus.on('changesPushed', saveService.autosave) + onUnmounted(() => { + syncService.bus.off('changesPushed', saveService.autosave) + }) + + /** + * Update the document ref based on the event provided + * + * @param event that triggered the update + * @param event.document latest state of the document + */ + function updateDocument(event: { document: Document }) { + document.value = event.document + } + syncService.bus.on('opened', updateDocument) + syncService.bus.on('change', updateDocument) + saveService.bus.on('save', updateDocument) + onUnmounted(() => { + syncService.bus.off('opened', updateDocument) + syncService.bus.off('change', updateDocument) + saveService.bus.off('save', updateDocument) + }) + + const versionWithChanges = ref(0) + /** + * Update the tracked version based on the one in the event + * + * @param event that triggered the update + * @param event.version with changes pushed to the server + */ + function updateVersionWithChanges(event: { version: number }) { + versionWithChanges.value = Math.max(event.version, versionWithChanges.value) + } + syncService.bus.on('changesPushed', updateVersionWithChanges) + onUnmounted(() => { + syncService.bus.off('changesPushed', updateVersionWithChanges) + }) + + const dirty = computed(() => (document.value?.lastSavedVersion ?? 0) < versionWithChanges.value) + watch(dirty, setDirty) + provide(saveServiceKey, saveService) - return { saveService } + return { document, saveService } } /** diff --git a/src/composables/useSyncService.ts b/src/composables/useSyncService.ts index b759b068bdc..3a612f08c46 100644 --- a/src/composables/useSyncService.ts +++ b/src/composables/useSyncService.ts @@ -4,10 +4,9 @@ */ import type { InjectionKey, ShallowRef } from 'vue' -import type { Document } from '../services/SyncService.ts' import type { Connection, InitialData } from './useConnection.ts' -import { computed, inject, onUnmounted, provide, ref, watch } from 'vue' +import { inject, provide } from 'vue' import { SyncService } from '../services/SyncService.ts' const syncServiceKey = Symbol('text:sync') as InjectionKey @@ -17,12 +16,10 @@ const syncServiceKey = Symbol('text:sync') as InjectionKey * * @param connection Connection to the text api. * @param openConnection Function to open the connection. - * @param setDirty to udpate the dirty state. */ export function provideSyncService( connection: ShallowRef, openConnection: () => Promise, - setDirty: (val: boolean) => Promise, ) { const syncService = new SyncService({ connection, @@ -30,44 +27,7 @@ export function provideSyncService( }) provide(syncServiceKey, syncService) - const document = ref() - /** - * Update the document ref based on the event provided - * - * @param event that triggered the update - * @param event.document latest state of the document - */ - function updateDocument(event: { document: Document }) { - document.value = event.document - } - syncService.bus.on('opened', updateDocument) - syncService.bus.on('change', updateDocument) - syncService.bus.on('save', updateDocument) - onUnmounted(() => { - syncService.bus.off('opened', updateDocument) - syncService.bus.off('change', updateDocument) - syncService.bus.off('save', updateDocument) - }) - - const versionWithChanges = ref(0) - /** - * Update the tracked version based on the one in the event - * - * @param event that triggered the update - * @param event.version with changes pushed to the server - */ - function updateVersionWithChanges(event: { version: number }) { - versionWithChanges.value = Math.max(event.version, versionWithChanges.value) - } - syncService.bus.on('changesPushed', updateVersionWithChanges) - onUnmounted(() => { - syncService.bus.off('changesPushed', updateVersionWithChanges) - }) - - const dirty = computed(() => (document.value?.lastSavedVersion ?? 0) < versionWithChanges.value) - watch(dirty, setDirty) - - return { dirty, document, syncService } + return { syncService } } /** diff --git a/src/services/SaveService.ts b/src/services/SaveService.ts index c185cdf857f..e23d2930af4 100644 --- a/src/services/SaveService.ts +++ b/src/services/SaveService.ts @@ -9,6 +9,7 @@ import type { Document, SyncService } from './SyncService.ts' import { showError } from '@nextcloud/dialogs' import debounce from 'debounce' +import mitt from 'mitt' import { save, saveViaSendBeacon } from '../apis/save.ts' import { logger } from '../helpers/logger.js' import { ERROR_TYPE } from './SyncService.ts' @@ -21,7 +22,18 @@ const SERVER_AUTOSAVE_INTERVAL = 10 // Randomize save times to prevent all clients saving at the same time. const MAX_RANDOM_AUTOSAVE_DELAY = 3 +type ErrorType = (typeof ERROR_TYPE)[keyof typeof ERROR_TYPE] + +export declare type EventTypes = { + /* error */ + error: { type: ErrorType, data?: object } + + /* Emitted after successful save */ + save: { document: Document } +} + class SaveService { + bus = mitt() connection: ShallowRef document: Ref syncService @@ -57,10 +69,6 @@ class SaveService { return this.syncService.version } - get emit() { - return this.syncService.bus.emit - } - async save({ force = false, manualSave = true } = {}) { logger.debug('[SaveService] saving', { force, manualSave }) if (!this.connection.value) { @@ -76,7 +84,7 @@ class SaveService { manualSave, }) logger.debug('[SaveService] saved', { response }) - this.emit('save', response.data) + this.bus.emit('save', response.data) this.autosave.clear() } catch (e) { logger.error('Failed to save document.', { error: e }) @@ -88,7 +96,7 @@ class SaveService { return } if (response?.status === 412) { - this.emit('error', { + this.bus.emit('error', { type: ERROR_TYPE.LOAD_ERROR, data: response, }) diff --git a/src/services/SyncService.ts b/src/services/SyncService.ts index 04243bf2c9d..47ee29d1599 100644 --- a/src/services/SyncService.ts +++ b/src/services/SyncService.ts @@ -124,9 +124,6 @@ export declare type EventTypes = { /* Events for session and document meta data */ change: { sessions: Session[], document: Document } - /* Emitted after successful save */ - save: { document: Document } - /* Emitted once a document becomes idle */ idle: void From 8ec98b7b8effc6229a700453ac2c7b5f1b8b9eb8 Mon Sep 17 00:00:00 2001 From: Max Date: Mon, 27 Jul 2026 07:15:29 +0200 Subject: [PATCH 06/15] chore(refactor): separate SaveService from SyncService and ydoc * `getSaveData` now prepares the data to send to the server. * `provideSaveService` now handles all the sync service events calling functions on `saveService` where needed. Signed-off-by: Max --- src/apis/save.ts | 9 +++++--- src/composables/useSaveService.ts | 19 +++++++++++++--- src/services/SaveService.ts | 38 ++++++++----------------------- 3 files changed, 32 insertions(+), 34 deletions(-) diff --git a/src/apis/save.ts b/src/apis/save.ts index 4e610358548..554861736b3 100644 --- a/src/apis/save.ts +++ b/src/apis/save.ts @@ -12,10 +12,13 @@ import axios from '@nextcloud/axios' import { generateUrl } from '@nextcloud/router' import { unref } from 'vue' -interface SaveData { +export interface SaveData { version: number autosaveContent: string documentState: string +} + +export interface SaveOptions { force: boolean manualSave: boolean } @@ -32,7 +35,7 @@ interface SaveResponse { */ export function save( connection: ShallowRef | Connection, - data: SaveData, + data: SaveData & SaveOptions, ): Promise { const con = unref(connection) const pub = con.shareToken ? '/public' : '' @@ -61,7 +64,7 @@ export function save( */ export function saveViaSendBeacon( connection: Connection, - data: Omit, + data: SaveData, ): boolean { const con = unref(connection) const pub = con.shareToken ? '/public' : '' diff --git a/src/composables/useSaveService.ts b/src/composables/useSaveService.ts index dd825f60c18..6c0aeddbb9a 100644 --- a/src/composables/useSaveService.ts +++ b/src/composables/useSaveService.ts @@ -5,6 +5,7 @@ import type { InjectionKey, ShallowRef } from 'vue' import type { Doc } from 'yjs' +import type { SaveData } from '../apis/save.ts' import type { Document, SyncService } from '../services/SyncService.ts' import type { Connection } from './useConnection.ts' @@ -30,17 +31,29 @@ export function provideSaveService( setDirty: (val: boolean) => Promise, ) { const document = ref() + + /** + * Get the data for the save request + */ + function getSaveData(): SaveData { + return { + version: syncService.version, + autosaveContent: serialize(), + documentState: getDocumentState(ydoc), + } + } + const saveService = new SaveService({ connection, document, - syncService, - serialize, - getDocumentState: () => getDocumentState(ydoc), + getSaveData, }) syncService.bus.on('changesPushed', saveService.autosave) + syncService.bus.on('close', saveService.clear) onUnmounted(() => { syncService.bus.off('changesPushed', saveService.autosave) + syncService.bus.off('close', saveService.clear) }) /** diff --git a/src/services/SaveService.ts b/src/services/SaveService.ts index e23d2930af4..d3b3d2734d0 100644 --- a/src/services/SaveService.ts +++ b/src/services/SaveService.ts @@ -4,8 +4,9 @@ */ import type { Ref, ShallowRef } from 'vue' +import type { SaveData } from '../apis/save.ts' import type { Connection } from '../composables/useConnection.ts' -import type { Document, SyncService } from './SyncService.ts' +import type { Document } from './SyncService.ts' import { showError } from '@nextcloud/dialogs' import debounce from 'debounce' @@ -36,37 +37,24 @@ class SaveService { bus = mitt() connection: ShallowRef document: Ref - syncService - serialize - getDocumentState + getSaveData autosave + clear constructor({ connection, document, - syncService, - serialize, - getDocumentState, + getSaveData, }: { connection: ShallowRef document: Ref - syncService: SyncService - serialize: () => string - getDocumentState: () => string + getSaveData: () => SaveData }) { this.connection = connection this.document = document - this.syncService = syncService - this.serialize = serialize - this.getDocumentState = getDocumentState + this.getSaveData = getSaveData this.autosave = debounce(this._autosave.bind(this), AUTOSAVE_DEBOUNCE * 1000) - this.syncService.bus.on('close', () => { - this.autosave.clear() - }) - } - - get version() { - return this.syncService.version + this.clear = this.autosave.clear.bind(this.autosave) } async save({ force = false, manualSave = true } = {}) { @@ -77,9 +65,7 @@ class SaveService { } try { const response = await save(this.connection.value, { - version: this.version, - autosaveContent: this.serialize(), - documentState: this.getDocumentState(), + ...this.getSaveData(), force, manualSave, }) @@ -112,11 +98,7 @@ class SaveService { if (!this.connection.value) { return } - const success = saveViaSendBeacon(this.connection.value, { - version: this.version, - autosaveContent: this.serialize(), - documentState: this.getDocumentState(), - }) + const success = saveViaSendBeacon(this.connection.value, this.getSaveData()) if (success) { logger.debug('[SaveService] saved using sendBeacon') } From 141bab81b1cf46d64a0ccbd8629ec2a83fbab9e6 Mon Sep 17 00:00:00 2001 From: Max Date: Mon, 27 Jul 2026 07:55:07 +0200 Subject: [PATCH 07/15] fix(autosave): exponential delay for retries on error Signed-off-by: Max --- src/services/SaveService.ts | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/services/SaveService.ts b/src/services/SaveService.ts index d3b3d2734d0..953103d1139 100644 --- a/src/services/SaveService.ts +++ b/src/services/SaveService.ts @@ -22,6 +22,8 @@ const AUTOSAVE_DEBOUNCE = 1 const SERVER_AUTOSAVE_INTERVAL = 10 // Randomize save times to prevent all clients saving at the same time. const MAX_RANDOM_AUTOSAVE_DELAY = 3 +// First retry on error - interval will double with every retry. +const RETRY_TIMEOUT = SERVER_AUTOSAVE_INTERVAL type ErrorType = (typeof ERROR_TYPE)[keyof typeof ERROR_TYPE] @@ -37,6 +39,7 @@ class SaveService { bus = mitt() connection: ShallowRef document: Ref + autosaveErrorCount = 0 getSaveData autosave clear @@ -115,15 +118,21 @@ class SaveService { if (now < lastSave + SERVER_AUTOSAVE_INTERVAL) { logger.debug('Not autosaving as last save is recent', { lastSave, now }) const nextSave = lastSave + SERVER_AUTOSAVE_INTERVAL + Math.random() * MAX_RANDOM_AUTOSAVE_DELAY - setTimeout(() => this.autosave(), (nextSave - now) * 1000) + setTimeout(this.autosave, (nextSave - now) * 1000) return } logger.debug('Autosaving') - return this.save({ manualSave: false }).catch((error) => { - logger.error('Failed to autosave document.', { error }) - // retry in 30 seconds - this.autosave() - }) + return this.save({ manualSave: false }) + .then(() => { + this.autosaveErrorCount = 0 + }) + .catch((error) => { + logger.error('Failed to autosave document.', { error }) + // double the delay on every failed attempt + const delay = Math.pow(2, this.autosaveErrorCount) * RETRY_TIMEOUT + setTimeout(this.autosave, delay * 1000) + this.autosaveErrorCount++ + }) } } From e5ca902d3216b6389486970e62475871e57f6b45 Mon Sep 17 00:00:00 2001 From: Max Date: Mon, 27 Jul 2026 08:11:45 +0200 Subject: [PATCH 08/15] fix(autosave): retry if server throttled save request The server will only perform one actual save every 10 seconds. Trigger another autosave if the save attempt was throttled. `document` is udpated by throttled save attempts. Rely on its `lastSavedVersionTime` for the retry. Signed-off-by: Max --- src/services/SaveService.ts | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/src/services/SaveService.ts b/src/services/SaveService.ts index 953103d1139..7536e8a989f 100644 --- a/src/services/SaveService.ts +++ b/src/services/SaveService.ts @@ -60,21 +60,36 @@ class SaveService { this.clear = this.autosave.clear.bind(this.autosave) } + /** + * Save the current state + * + * @param options for saving + * @param options.force force save for handling conflicts + * @param options.manualSave user initiated the saving - not autosave + * @return true on success, false if autosave was throttled by the server + */ async save({ force = false, manualSave = true } = {}) { logger.debug('[SaveService] saving', { force, manualSave }) if (!this.connection.value) { logger.warn('Could not save due to missing connection') return } + const data = this.getSaveData() try { const response = await save(this.connection.value, { - ...this.getSaveData(), + ...data, force, manualSave, }) - logger.debug('[SaveService] saved', { response }) + // update the document - even if the save was throttled this.bus.emit('save', response.data) + if (response.data.document.lastSavedVersion < data.version) { + logger.debug('[SaveService] Server throttled save request.', { response }) + return false + } + logger.debug('[SaveService] saved', { response }) this.autosave.clear() + return true } catch (e) { logger.error('Failed to save document.', { error: e }) const response = ( @@ -122,9 +137,14 @@ class SaveService { return } logger.debug('Autosaving') - return this.save({ manualSave: false }) - .then(() => { + this.save({ manualSave: false }) + .then((saved) => { this.autosaveErrorCount = 0 + // server did not save due to throttling + if (saved === false) { + // document has been updated - let autosave handle the delay. + this.autosave() + } }) .catch((error) => { logger.error('Failed to autosave document.', { error }) From 35083d34eca0b26d37761115726f2a432b646394 Mon Sep 17 00:00:00 2001 From: Max Date: Mon, 27 Jul 2026 08:21:34 +0200 Subject: [PATCH 09/15] fix(autosave): handle out of sync clocks * If the server claims to have saved the doc in the future use the current timestamp instead. Autosave happens at least ten seconds after `lastSavedVersionTime`. So if that was far into the future it would never happen. * If the server is living in the past delay the autosave retries without relying on `lastSavedVersionTime`. Signed-off-by: Max --- src/composables/useSaveService.ts | 10 +++++++++- src/services/SaveService.ts | 4 ++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/composables/useSaveService.ts b/src/composables/useSaveService.ts index 6c0aeddbb9a..7d1b3bfa9d0 100644 --- a/src/composables/useSaveService.ts +++ b/src/composables/useSaveService.ts @@ -63,7 +63,15 @@ export function provideSaveService( * @param event.document latest state of the document */ function updateDocument(event: { document: Document }) { - document.value = event.document + // Limit lastSavedVersionTime to now. No saving from the future. + const lastSavedVersionTime = Math.min( + event.document.lastSavedVersionTime, + Math.ceil(Date.now() / 1000), + ) + document.value = { + ...event.document, + lastSavedVersionTime, + } } syncService.bus.on('opened', updateDocument) syncService.bus.on('change', updateDocument) diff --git a/src/services/SaveService.ts b/src/services/SaveService.ts index 7536e8a989f..7202e555e61 100644 --- a/src/services/SaveService.ts +++ b/src/services/SaveService.ts @@ -142,8 +142,8 @@ class SaveService { this.autosaveErrorCount = 0 // server did not save due to throttling if (saved === false) { - // document has been updated - let autosave handle the delay. - this.autosave() + // Make sure to not hammer the server if clocks are out of sync. + setTimeout(this.autosave, SERVER_AUTOSAVE_INTERVAL * 1000) } }) .catch((error) => { From f07b93ac7a2721646717af56b0539ffc8bfee507 Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 28 Jul 2026 10:10:30 +0200 Subject: [PATCH 10/15] chore(test): always retry autosave after 10 seconds Adding a random offset makes determenistic testing harder. Now we also do not depend on in sync clocks. Signed-off-by: Max --- playwright/e2e/autosave.spec.ts | 52 +++++++++++++++------- playwright/support/fixtures/upload-file.ts | 6 ++- src/components/CollaborativeEditor.vue | 4 +- 3 files changed, 41 insertions(+), 21 deletions(-) diff --git a/playwright/e2e/autosave.spec.ts b/playwright/e2e/autosave.spec.ts index 328183e4a9f..a134229f9aa 100644 --- a/playwright/e2e/autosave.spec.ts +++ b/playwright/e2e/autosave.spec.ts @@ -14,39 +14,55 @@ const test = mergeTests(editorTest, offlineTest, uploadFileTest) // we cannot run tests in parallel. test.describe.configure({ mode: 'serial' }) +// Files were created 10 seconds ago so there's no throttling to begin with. +test.use({ mtime: Date.now() / 1000 - 10 }) + test.beforeEach(async ({ open }) => { await open() }) -test('saves after 30 seconds', async ({ editor, page }) => { - await page.clock.install() +test('saves after 1 second', async ({ editor }) => { await expect(editor.el).toBeVisible() await editor.typeHeading('Hello world') await expect(editor.saveIndicator).toHaveAccessibleName(/Unsaved changes/) - await page.clock.fastForward(30_000) await expect(editor.saveIndicator).not.toHaveAccessibleName(/Unsaved changes/) // TODO: Why does this not work? await expect(await file.getContent()).toBe('## Hello world') }) -test('saves after being disconnected for 20 sec.', async ({ +/* + * 1 second autosave debounce + * 10 seconds waiting for server to be ready again + * 1 second for the save request + */ +test('saves again within 12 seconds', async ({ editor }) => { + test.slow() + await expect(editor.el).toBeVisible() + await editor.typeHeading('Hello') + await expect(editor.saveIndicator).toHaveAccessibleName(/Unsaved changes/) + await expect(editor.saveIndicator).not.toHaveAccessibleName(/Unsaved changes/) + await editor.type(' again') + await expect(editor.saveIndicator).toHaveAccessibleName(/Unsaved changes/) + await expect(editor.saveIndicator).not.toHaveAccessibleName(/Unsaved changes/, { timeout: 12_000 }) +}) + +test('saves after being disconnected for 5 sec.', async ({ editor, - page, setOffline, setOnline, }) => { - await page.clock.install() await expect(editor.el).toBeVisible() - await editor.typeHeading('Hello world') + await editor.typeHeading('Hello') + await expect(editor.saveIndicator).not.toHaveAccessibleName(/Unsaved changes/) + await editor.type(' again') await expect(editor.saveIndicator).toHaveAccessibleName(/Unsaved changes/) await setOffline() - await page.clock.fastForward(20_000) + await new Promise((resolve) => setTimeout(resolve, 5_000)) await setOnline() - await page.clock.fastForward(20_000) - await expect(editor.saveIndicator).not.toHaveAccessibleName(/Unsaved changes/) - // TODO: Why does this not work? await expect(await file.getContent()).toBe('## Hello world') + await expect(editor.saveIndicator).toHaveAccessibleName(/Unsaved changes/) + await expect(editor.saveIndicator).not.toHaveAccessibleName(/Unsaved changes/, { timeout: 10_000 }) }) -test('saves after being disconnected for 2 minutes', async ({ +test('saves after being disconnected for 2 minutes.', async ({ editor, page, setOffline, @@ -54,12 +70,16 @@ test('saves after being disconnected for 2 minutes', async ({ }) => { await page.clock.install() await expect(editor.el).toBeVisible() - await editor.typeHeading('Hello world') + await editor.typeHeading('Hello') + await expect(editor.saveIndicator).not.toHaveAccessibleName(/Unsaved changes/) + await editor.type(' again') await expect(editor.saveIndicator).toHaveAccessibleName(/Unsaved changes/) await setOffline() - await page.clock.fastForward(120_000) + await new Promise((resolve) => setTimeout(resolve, 5_000)) + await page.clock.fastForward(110_000) await setOnline() - await page.clock.fastForward(40_000) + await new Promise((resolve) => setTimeout(resolve, 5_000)) + await page.clock.fastForward(5_000) + await expect(editor.saveIndicator).toHaveAccessibleName(/Unsaved changes/) await expect(editor.saveIndicator).not.toHaveAccessibleName(/Unsaved changes/) - // TODO: Why does this not work? await expect(await file.getContent()).toBe('## Hello world') }) diff --git a/playwright/support/fixtures/upload-file.ts b/playwright/support/fixtures/upload-file.ts index 5dce3417ee8..f736e7487f6 100644 --- a/playwright/support/fixtures/upload-file.ts +++ b/playwright/support/fixtures/upload-file.ts @@ -12,6 +12,7 @@ export interface UploadFileFixture { file: Node fileName: string fileContent: string + mtime: number oldVersions: { content?: string, mtime: number }[] open: () => Promise close: () => Promise @@ -25,15 +26,16 @@ export interface UploadFileFixture { export const test = base.extend({ fileContent: ['', { option: true }], fileName: ['empty.md', { option: true }], + mtime: [undefined, { option: true }], oldVersions: [[], { option: true }], - file: async ({ fileContent, fileName, oldVersions, user }, use) => { + file: async ({ fileContent, fileName, oldVersions, mtime, user }, use) => { const uploadVersion = (opts: { content?: string, mtime?: number }) => user.uploadFile({ name: fileName, ...opts }) for (const version of oldVersions) { await uploadVersion(version) } - const file = await uploadVersion({ content: fileContent }) + const file = await uploadVersion({ content: fileContent, mtime }) await use(file) }, diff --git a/src/components/CollaborativeEditor.vue b/src/components/CollaborativeEditor.vue index 14d1231d9af..6651cba52e2 100644 --- a/src/components/CollaborativeEditor.vue +++ b/src/components/CollaborativeEditor.vue @@ -659,9 +659,7 @@ export default defineComponent({ // Save and push unsaved changes from offline editing session. Promise.all([this.whenSynced, this.editorReady]).then(() => { if (this.dirty) { - this.saveService - .save() - .catch((err) => logger.error('Failed to save offline changes', { err })) + // the update will trigger an autosave this.syncProvider.sendUpdateFromDoc('offline', this.ydoc) } }) From ed62629c35f5d80982ff4f203b06822d3d9cc7db Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 28 Jul 2026 10:12:17 +0200 Subject: [PATCH 11/15] fix(indexed-db): rely on autosave when recovering Autosave is triggered by the push of the steps. Saving before that delays the autosave because of server throttling. Signed-off-by: Max --- src/services/SaveService.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/services/SaveService.ts b/src/services/SaveService.ts index 7202e555e61..6ba2fdfc50d 100644 --- a/src/services/SaveService.ts +++ b/src/services/SaveService.ts @@ -20,8 +20,6 @@ import { ERROR_TYPE } from './SyncService.ts' const AUTOSAVE_DEBOUNCE = 1 // Server only accepts auutosaves every 10 seconds const SERVER_AUTOSAVE_INTERVAL = 10 -// Randomize save times to prevent all clients saving at the same time. -const MAX_RANDOM_AUTOSAVE_DELAY = 3 // First retry on error - interval will double with every retry. const RETRY_TIMEOUT = SERVER_AUTOSAVE_INTERVAL @@ -131,9 +129,8 @@ class SaveService { const now = Date.now() / 1000 // Server won't accept autosaves yet if (now < lastSave + SERVER_AUTOSAVE_INTERVAL) { - logger.debug('Not autosaving as last save is recent', { lastSave, now }) - const nextSave = lastSave + SERVER_AUTOSAVE_INTERVAL + Math.random() * MAX_RANDOM_AUTOSAVE_DELAY - setTimeout(this.autosave, (nextSave - now) * 1000) + logger.debug('Just saved, will try again in 10 seconds.', { lastSave, now }) + setTimeout(this.autosave, (SERVER_AUTOSAVE_INTERVAL - AUTOSAVE_DEBOUNCE) * 1000) return } logger.debug('Autosaving') @@ -143,7 +140,7 @@ class SaveService { // server did not save due to throttling if (saved === false) { // Make sure to not hammer the server if clocks are out of sync. - setTimeout(this.autosave, SERVER_AUTOSAVE_INTERVAL * 1000) + setTimeout(this.autosave, (SERVER_AUTOSAVE_INTERVAL - AUTOSAVE_DEBOUNCE) * 1000) } }) .catch((error) => { From 6dc751d8a378adf79b26cd29070535886a96b361 Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 28 Jul 2026 10:35:25 +0200 Subject: [PATCH 12/15] chore(CI): playwright test timeout 45 seconds Some of our runners are slow. Give them more time to finish the runs. Also separate local and CI config in the config file. Signed-off-by: Max --- playwright.config.ts | 38 +++++++++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/playwright.config.ts b/playwright.config.ts index 6d510c8d68d..8fbd1516ed9 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -3,24 +3,40 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ +import type { ReporterDescription } from '@playwright/test' + import { defineConfig, devices } from '@playwright/test' +/** + * Used locally - i.e. if `CI` is not set as an environment variable. + */ +const LOCAL_CONFIG = { + // Just the html report with the traces + reporter: 'list', +} as const + +/** + * Used on CI - i.e. if `CI` is set as an environment variable. + */ +const CI_CONFIG = { + // ensure no `test.only` is left in the code causing false positives + forbidOnly: true, + // blob (so we can merge reports and download them for inspection), + // dot (so we have a quick overview in the logs while the tests are running) + // github (to have annotations in the PR) + reporter: [['blob'], ['line'], ['github']] as ReporterDescription[], + retries: 1, + timeout: 45_000, + // we shard to speed up the tests so no parallelism in workers + workers: 1, +} as const + /** * See https://playwright.dev/docs/test-configuration. */ export default defineConfig({ testDir: './playwright', - // ensure no `test.only` is left in the code causing false positives - forbidOnly: !!process.env.CI, - // retry on CI only - retries: process.env.CI ? 1 : 0, - // we shard on CI to speed up the tests so no parallelism in workers - workers: process.env.CI ? 1 : undefined, - // on CI we want to have blob (so we can merge reports and download them for inspection), - // line (so we have a quick overview in the logs while the tests are running) - // github (to have annotations in the PR) - // locally we just want the html report with the traces - reporter: process.env.CI ? [['blob'], ['line'], ['github']] : 'list', + ...(process.env.CI ? CI_CONFIG : LOCAL_CONFIG), use: { // Base URL to use in actions like `await page.goto('./')`. baseURL: process.env.baseURL ?? 'http://localhost:8089/index.php/', From 9bacc1ca9171169a577be64dddd18f8634b411ac Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 28 Jul 2026 10:42:16 +0200 Subject: [PATCH 13/15] chore(test): mtime is optional in upload file fixture Signed-off-by: Max --- playwright/support/fixtures/upload-file.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/playwright/support/fixtures/upload-file.ts b/playwright/support/fixtures/upload-file.ts index f736e7487f6..2ff391ea57c 100644 --- a/playwright/support/fixtures/upload-file.ts +++ b/playwright/support/fixtures/upload-file.ts @@ -12,7 +12,7 @@ export interface UploadFileFixture { file: Node fileName: string fileContent: string - mtime: number + mtime?: number oldVersions: { content?: string, mtime: number }[] open: () => Promise close: () => Promise From 3f8b96aa0fc93c7f599be9b46c938901a1031edc Mon Sep 17 00:00:00 2001 From: Max Date: Wed, 29 Jul 2026 08:08:00 +0200 Subject: [PATCH 14/15] fix(autosave): throttle to SERVER_AUTOSAVE_INTERVAL Only consider the local clock. Avoid problems if clocks are out of sync or if the server is unresponsive and `lastSavedVersionTime` does not get updated Signed-off-by: Max --- src/services/SaveService.ts | 40 +++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/src/services/SaveService.ts b/src/services/SaveService.ts index 6ba2fdfc50d..0992ea12d71 100644 --- a/src/services/SaveService.ts +++ b/src/services/SaveService.ts @@ -20,8 +20,6 @@ import { ERROR_TYPE } from './SyncService.ts' const AUTOSAVE_DEBOUNCE = 1 // Server only accepts auutosaves every 10 seconds const SERVER_AUTOSAVE_INTERVAL = 10 -// First retry on error - interval will double with every retry. -const RETRY_TIMEOUT = SERVER_AUTOSAVE_INTERVAL type ErrorType = (typeof ERROR_TYPE)[keyof typeof ERROR_TYPE] @@ -37,7 +35,8 @@ class SaveService { bus = mitt() connection: ShallowRef document: Ref - autosaveErrorCount = 0 + lastSaveAttempt = 0 + pendingAutosave = 0 getSaveData autosave clear @@ -55,7 +54,7 @@ class SaveService { this.document = document this.getSaveData = getSaveData this.autosave = debounce(this._autosave.bind(this), AUTOSAVE_DEBOUNCE * 1000) - this.clear = this.autosave.clear.bind(this.autosave) + this.clear = this.clearAutosave.bind(this) } /** @@ -74,6 +73,7 @@ class SaveService { } const data = this.getSaveData() try { + this.lastSaveAttempt = Date.now() const response = await save(this.connection.value, { ...data, force, @@ -86,7 +86,7 @@ class SaveService { return false } logger.debug('[SaveService] saved', { response }) - this.autosave.clear() + this.clearAutosave() return true } catch (e) { logger.error('Failed to save document.', { error: e }) @@ -125,32 +125,38 @@ class SaveService { } _autosave() { - const lastSave = this.document.value?.lastSavedVersionTime ?? 0 - const now = Date.now() / 1000 + const now = Date.now() + const nextSaveAttempt = this.lastSaveAttempt + SERVER_AUTOSAVE_INTERVAL * 1000 // Server won't accept autosaves yet - if (now < lastSave + SERVER_AUTOSAVE_INTERVAL) { - logger.debug('Just saved, will try again in 10 seconds.', { lastSave, now }) - setTimeout(this.autosave, (SERVER_AUTOSAVE_INTERVAL - AUTOSAVE_DEBOUNCE) * 1000) + if (now < nextSaveAttempt) { + if (!this.pendingAutosave) { + const wait = nextSaveAttempt - now + logger.debug(`Just saved, will try again in ${Math.ceil(wait)} seconds.`) + this.pendingAutosave = window.setTimeout(this.autosave, wait) + } return } logger.debug('Autosaving') this.save({ manualSave: false }) .then((saved) => { - this.autosaveErrorCount = 0 // server did not save due to throttling if (saved === false) { - // Make sure to not hammer the server if clocks are out of sync. - setTimeout(this.autosave, (SERVER_AUTOSAVE_INTERVAL - AUTOSAVE_DEBOUNCE) * 1000) + this.autosave() } }) .catch((error) => { logger.error('Failed to autosave document.', { error }) - // double the delay on every failed attempt - const delay = Math.pow(2, this.autosaveErrorCount) * RETRY_TIMEOUT - setTimeout(this.autosave, delay * 1000) - this.autosaveErrorCount++ + this.autosave() }) } + + clearAutosave() { + this.autosave.clear() + if (this.pendingAutosave) { + window.clearTimeout(this.pendingAutosave) + this.pendingAutosave = 0 + } + } } export { SaveService } From 9f9454273ee65a3b4b12c9fc65969913d623cba9 Mon Sep 17 00:00:00 2001 From: Max Date: Wed, 29 Jul 2026 10:11:40 +0200 Subject: [PATCH 15/15] chore(tests): fix autosave test with new timing Signed-off-by: Max --- playwright/e2e/autosave.spec.ts | 9 ++++++--- src/services/SyncService.ts | 1 + 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/playwright/e2e/autosave.spec.ts b/playwright/e2e/autosave.spec.ts index a134229f9aa..46ed77deebf 100644 --- a/playwright/e2e/autosave.spec.ts +++ b/playwright/e2e/autosave.spec.ts @@ -71,15 +71,18 @@ test('saves after being disconnected for 2 minutes.', async ({ await page.clock.install() await expect(editor.el).toBeVisible() await editor.typeHeading('Hello') + await expect(editor.saveIndicator).toHaveAccessibleName(/Unsaved changes/) await expect(editor.saveIndicator).not.toHaveAccessibleName(/Unsaved changes/) await editor.type(' again') await expect(editor.saveIndicator).toHaveAccessibleName(/Unsaved changes/) await setOffline() - await new Promise((resolve) => setTimeout(resolve, 5_000)) + // Wait long enough for the server throttling to be over. + await new Promise((resolve) => setTimeout(resolve, 10_000)) await page.clock.fastForward(110_000) await setOnline() - await new Promise((resolve) => setTimeout(resolve, 5_000)) - await page.clock.fastForward(5_000) + await expect(editor.offlineState).not.toBeVisible() await expect(editor.saveIndicator).toHaveAccessibleName(/Unsaved changes/) + // Be sure to trigger at least one autosave + await page.clock.fastForward(15_000) await expect(editor.saveIndicator).not.toHaveAccessibleName(/Unsaved changes/) }) diff --git a/src/services/SyncService.ts b/src/services/SyncService.ts index 47ee29d1599..11ac87f4e1a 100644 --- a/src/services/SyncService.ts +++ b/src/services/SyncService.ts @@ -258,6 +258,7 @@ class SyncService { if (hadUpdate) { // this.version has been increased in receiveSteps this.bus.emit('changesPushed', { version: this.version }) + logger.debug('changesPushed', { version: this.version }) } } })