From 9b42eb17f96657f602b9b8bdb4c785d8778b2f5f Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 8 Jul 2025 15:11:04 +0200 Subject: [PATCH 1/6] chore(types): type polling backend and notify service Signed-off-by: Max --- .../{NotifyService.js => NotifyService.ts} | 14 ++- .../{PollingBackend.js => PollingBackend.ts} | 113 ++++++++++-------- src/services/SyncService.ts | 1 + src/services/WebSocketPolyfill.js | 2 +- 4 files changed, 77 insertions(+), 53 deletions(-) rename src/services/{NotifyService.js => NotifyService.ts} (68%) rename src/services/{PollingBackend.js => PollingBackend.ts} (74%) diff --git a/src/services/NotifyService.js b/src/services/NotifyService.ts similarity index 68% rename from src/services/NotifyService.js rename to src/services/NotifyService.ts index 8b846a9e555..0fb9bf5463d 100644 --- a/src/services/NotifyService.js +++ b/src/services/NotifyService.ts @@ -3,10 +3,20 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -import mitt from 'mitt' +import mitt, { type Emitter } from 'mitt' import { listen } from '@nextcloud/notify_push' import { loadState } from '@nextcloud/initial-state' +declare type EventTypes = { + notify_push: { messageType: unknown; messageBody: object } +} + +declare global { + interface Window { + _nc_text_notify?: Emitter + } +} + if (!window._nc_text_notify) { const isPushEnabled = loadState('text', 'notify_push', false) const useNotifyPush = isPushEnabled @@ -17,7 +27,7 @@ if (!window._nc_text_notify) { }) }) : undefined - window._nc_text_notify = useNotifyPush ? mitt() : null + window._nc_text_notify = useNotifyPush ? mitt() : undefined } export default () => { diff --git a/src/services/PollingBackend.js b/src/services/PollingBackend.ts similarity index 74% rename from src/services/PollingBackend.js rename to src/services/PollingBackend.ts index 43273d8574e..259b9794592 100644 --- a/src/services/PollingBackend.js +++ b/src/services/PollingBackend.ts @@ -3,42 +3,42 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ import { logger } from '../helpers/logger.js' -import { SyncService, ERROR_TYPE } from './SyncService.ts' -import { SessionConnection } from './SessionConnection.js' -import getNotifyBus from './NotifyService.js' +import { + type Session, + type Step, + type SyncService, + ERROR_TYPE, +} from './SyncService.js' +import { type SessionConnection } from './SessionConnection.js' +import getNotifyBus, { type EventTypes } from './NotifyService' +import type { Emitter } from 'mitt' /** - * Minimum inverval to refetch the document changes - * - * @type {number} time in ms + * Minimum inverval to refetch the document changes in ms. */ const FETCH_INTERVAL = 300 /** - * Maximum interval between refetches of document state if multiple users have joined - * - * @type {number} time in ms + * Maximum interval between refetches of document state in ms + * if multiple users have joined */ const FETCH_INTERVAL_MAX = 5000 /** - * Interval to check for changes when there is only one user joined - * - * @type {number} time in ms + * Interval to check for changes in ms + * when there is only one user joined */ const FETCH_INTERVAL_SINGLE_EDITOR = 5000 /** - * Interval to check for changes for read only users - * @type {number} + * Interval to check for changes in ms for read only users */ const FETCH_INTERVAL_READ_ONLY = 30000 /** - * Interval to fetch for changes when a browser window is considered invisible by the - * page visibility API https://developer.mozilla.org/de/docs/Web/API/Page_Visibility_API - * - * @type {number} time in ms + * Interval to fetch for changes in ms + * when a browser window is considered invisible by the page visibility API + * https://developer.mozilla.org/de/docs/Web/API/Page_Visibility_API */ const FETCH_INTERVAL_INVISIBLE = 30000 @@ -53,20 +53,36 @@ const MAX_RETRY_FETCH_COUNT = 5 */ const COLLABORATOR_DISCONNECT_TIME = FETCH_INTERVAL_INVISIBLE * 1.5 +interface PollData { + document: Document + sessions: Session[] + steps: Step[] +} + +interface ConflictData extends PollData { + outsideChange: string +} + class PollingBackend { - /** @type {SyncService} */ - #syncService - /** @type {SessionConnection} */ - #connection + #syncService: SyncService + #connection: SessionConnection #lastPoll - #fetchInterval - #fetchRetryCounter - #pollActive - #initialLoadingFinished - #notifyPushBus + #fetchInterval: number + #fetchRetryCounter: number + fetcher: NodeJS.Timeout | undefined + #pollActive = false + #initialLoadingFinished = false + #notifyPushBus: Emitter | undefined + visibilitychange = () => { + if (document.visibilityState === 'hidden') { + this.#fetchInterval = FETCH_INTERVAL_INVISIBLE + } else { + this.resetRefetchTimer() + } + } - constructor(syncService, connection) { + constructor(syncService: SyncService, connection: SessionConnection) { this.#syncService = syncService this.#connection = connection this.#fetchInterval = FETCH_INTERVAL @@ -75,16 +91,13 @@ class PollingBackend { } connect() { - if (this.fetcher > 0) { + if (this.fetcher) { console.error('Trying to connect, but already connected') return } this.#initialLoadingFinished = false this.fetcher = setInterval(this._fetchSteps.bind(this), 50) - document.addEventListener( - 'visibilitychange', - this.visibilitychange.bind(this), - ) + document.addEventListener('visibilitychange', this.visibilitychange) this.#notifyPushBus = getNotifyBus() } @@ -109,7 +122,9 @@ class PollingBackend { this.#pollActive = true - logger.debug('[PollingBackend] Fetching steps', this.#syncService.version) + logger.debug('[PollingBackend] Fetching steps', { + version: this.#syncService.version, + }) await this.#connection .sync({ version: this.#syncService.version, @@ -119,14 +134,20 @@ class PollingBackend { this.#pollActive = false } - handleNotifyPush({ messageType, messageBody }) { + handleNotifyPush({ + messageType: _, + messageBody, + }: { + messageType: unknown + messageBody: { documentId: number; response: PollData } + }) { if (messageBody.documentId !== this.#connection.document.id) { return } this._handleResponse({ data: messageBody.response }) } - _handleResponse({ data }) { + _handleResponse({ data }: { data: PollData }) { const { document, sessions } = data this.#fetchRetryCounter = 0 @@ -158,7 +179,10 @@ class PollingBackend { } } - _handleError(e) { + _handleError(e: { + response?: { status: number; data: ConflictData } + code?: string + }) { if (!e.response || e.code === 'ECONNABORTED') { if (this.#fetchRetryCounter++ >= MAX_RETRY_FETCH_COUNT) { logger.error( @@ -216,11 +240,8 @@ class PollingBackend { disconnect() { clearInterval(this.fetcher) - this.fetcher = 0 - document.removeEventListener( - 'visibilitychange', - this.visibilitychange.bind(this), - ) + this.fetcher = undefined + document.removeEventListener('visibilitychange', this.visibilitychange) } resetRefetchTimer() { @@ -250,14 +271,6 @@ class PollingBackend { maximumReadOnlyTimer() { this.#fetchInterval = FETCH_INTERVAL_READ_ONLY } - - visibilitychange() { - if (document.visibilityState === 'hidden') { - this.#fetchInterval = FETCH_INTERVAL_INVISIBLE - } else { - this.resetRefetchTimer() - } - } } export default PollingBackend diff --git a/src/services/SyncService.ts b/src/services/SyncService.ts index 3bc3dade582..0dc5b28493a 100644 --- a/src/services/SyncService.ts +++ b/src/services/SyncService.ts @@ -68,6 +68,7 @@ export interface Session { guestName?: string documentId: number displayName: string + clientId: number } export interface Document { diff --git a/src/services/WebSocketPolyfill.js b/src/services/WebSocketPolyfill.js index 047914aaf28..73aaea69396 100644 --- a/src/services/WebSocketPolyfill.js +++ b/src/services/WebSocketPolyfill.js @@ -5,7 +5,7 @@ import { logger } from '../helpers/logger.js' import { decodeArrayBuffer } from '../helpers/base64.ts' -import getNotifyBus from './NotifyService.js' +import getNotifyBus from './NotifyService.ts' /** * From b097f3987311b5c3808a8a20abfbb4c3c9d3c0e8 Mon Sep 17 00:00:00 2001 From: Max Date: Sun, 13 Jul 2025 10:11:43 +0200 Subject: [PATCH 2/6] refactor(connection): use sync api instead of SessionConnection Signed-off-by: Max --- cypress/support/sessions.js | 12 ++++----- src/apis/Sync.ts | 44 +++++++++++++++++++++++++++---- src/services/NotifyService.ts | 2 +- src/services/PollingBackend.ts | 17 ++++++------ src/services/SessionConnection.js | 9 ------- src/services/SyncService.ts | 7 ++++- 6 files changed, 60 insertions(+), 31 deletions(-) diff --git a/cypress/support/sessions.js b/cypress/support/sessions.js index 2fb7166ba96..113337af175 100644 --- a/cypress/support/sessions.js +++ b/cypress/support/sessions.js @@ -6,7 +6,7 @@ import axios from '@nextcloud/axios' import { SessionConnection } from '../../src/services/SessionConnection.js' import { open, close } from '../../src/apis/Connect.ts' -import { push } from '../../src/apis/Sync.ts' +import { push, sync } from '../../src/apis/Sync.ts' import { save } from '../../src/apis/Save.ts' const url = Cypress.config('baseUrl').replace(/\/index.php\/?$/g, '') @@ -45,14 +45,14 @@ Cypress.Commands.add('failToPushSteps', ({ connection: sessionConnection, steps, }, (err) => err.response) }) -Cypress.Commands.add('syncSteps', (connection, options = { version: 0 }) => { - return connection.sync(options) +Cypress.Commands.add('syncSteps', (sessionConnection, options = { version: 0 }) => { + return sync(sessionConnection.connection, options) .then(response => response.data) }) -Cypress.Commands.add('failToSyncSteps', (connection, options = { version: 0 }) => { - return connection.sync(options) - .then((response) => { +Cypress.Commands.add('failToSyncSteps', (sessionConnection, options = { version: 0 }) => { + return sync(sessionConnection.connection, options) + .then((_response) => { throw new Error('Expected request to fail - but it succeeded!') }, (err) => err.response) }) diff --git a/src/apis/Sync.ts b/src/apis/Sync.ts index 02896ceb389..b7384d0f737 100644 --- a/src/apis/Sync.ts +++ b/src/apis/Sync.ts @@ -7,15 +7,15 @@ import axios from '@nextcloud/axios' import type { Connection } from '../composables/useConnection.js' import { unref, type ShallowRef } from 'vue' import { generateUrl } from '@nextcloud/router' -import type { Step } from '../services/SyncService.js' +import type { Session, Step } from '../services/SyncService.js' -interface SyncData { +interface PushData { version: number steps: string[] awareness: string } -interface SyncResponse { +interface PushResponse { data: { steps: Step[] documentState: string @@ -30,8 +30,8 @@ interface SyncResponse { */ export function push( connection: ShallowRef | Connection, - data: SyncData, -): Promise { + data: PushData, +): Promise { const con = unref(connection) const pub = con.shareToken ? '/public' : '' const url = generateUrl(`apps/text${pub}/session/${con.documentId}/push`) @@ -46,3 +46,37 @@ export function push( awareness: data.awareness, }) } + +interface SyncResponse { + data: { + steps: Step[] + documentState: string + awareness: Record + document: Document + sessions: Session[] + } +} + +/** + * Receive updates from the server + * @param connection the active connection + * @param data data to push to the server + * @param data.version Changes up to this version are already known. + */ +export function sync( + connection: ShallowRef | Connection, + data: { version: number }, +): Promise { + const con = unref(connection) + const pub = con.shareToken ? '/public' : '' + const url = generateUrl(`apps/text${pub}/session/${con.documentId}/sync`) + return axios.post(url, { + documentId: con.documentId, + sessionId: con.sessionId, + sessionToken: con.sessionToken, + token: con.shareToken, + filePath: con.filePath, + baseVersionEtag: con.baseVersionEtag, + version: data.version, + }) +} diff --git a/src/services/NotifyService.ts b/src/services/NotifyService.ts index 0fb9bf5463d..32b629c14a9 100644 --- a/src/services/NotifyService.ts +++ b/src/services/NotifyService.ts @@ -7,7 +7,7 @@ import mitt, { type Emitter } from 'mitt' import { listen } from '@nextcloud/notify_push' import { loadState } from '@nextcloud/initial-state' -declare type EventTypes = { +export declare type EventTypes = { notify_push: { messageType: unknown; messageBody: object } } diff --git a/src/services/PollingBackend.ts b/src/services/PollingBackend.ts index 259b9794592..cd2fbc604ff 100644 --- a/src/services/PollingBackend.ts +++ b/src/services/PollingBackend.ts @@ -9,9 +9,10 @@ import { type SyncService, ERROR_TYPE, } from './SyncService.js' -import { type SessionConnection } from './SessionConnection.js' import getNotifyBus, { type EventTypes } from './NotifyService' import type { Emitter } from 'mitt' +import type { Connection } from '../composables/useConnection.js' +import { sync } from '../apis/Sync.js' /** * Minimum inverval to refetch the document changes in ms. @@ -65,7 +66,7 @@ interface ConflictData extends PollData { class PollingBackend { #syncService: SyncService - #connection: SessionConnection + #connection: Connection #lastPoll #fetchInterval: number @@ -82,7 +83,7 @@ class PollingBackend { } } - constructor(syncService: SyncService, connection: SessionConnection) { + constructor(syncService: SyncService, connection: Connection) { this.#syncService = syncService this.#connection = connection this.#fetchInterval = FETCH_INTERVAL @@ -125,11 +126,9 @@ class PollingBackend { logger.debug('[PollingBackend] Fetching steps', { version: this.#syncService.version, }) - await this.#connection - .sync({ - version: this.#syncService.version, - }) - .then(this._handleResponse.bind(this), this._handleError.bind(this)) + await sync(this.#connection, { + version: this.#syncService.version, + }).then(this._handleResponse.bind(this), this._handleError.bind(this)) this.#lastPoll = Date.now() this.#pollActive = false } @@ -141,7 +140,7 @@ class PollingBackend { messageType: unknown messageBody: { documentId: number; response: PollData } }) { - if (messageBody.documentId !== this.#connection.document.id) { + if (messageBody.documentId !== this.#connection.documentId) { return } this._handleResponse({ data: messageBody.response }) diff --git a/src/services/SessionConnection.js b/src/services/SessionConnection.js index 05ce63aac6f..4c9f24c9105 100644 --- a/src/services/SessionConnection.js +++ b/src/services/SessionConnection.js @@ -76,15 +76,6 @@ export class SessionConnection { } } - sync({ version }) { - return this.#post(this.#url(`session/${this.#document.id}/sync`), { - ...this.#defaultParams, - filePath: this.connection.filePath, - baseVersionEtag: this.#document.baseVersionEtag, - version, - }) - } - // TODO: maybe return a new connection here so connections have immutable state update(guestName) { return this.#post(this.#url(`session/${this.#document.id}/session`), { diff --git a/src/services/SyncService.ts b/src/services/SyncService.ts index 0dc5b28493a..ee35c71c3d2 100644 --- a/src/services/SyncService.ts +++ b/src/services/SyncService.ts @@ -166,8 +166,13 @@ class SyncService { return } this.sessionConnection = new SessionConnection(data, this.connection.value) - this.backend = new PollingBackend(this, this.sessionConnection) this.version = this.sessionConnection.docStateVersion + if (!this.connection.value) { + console.error('Opened the connection but now it is undefined') + return + } + this.backend = new PollingBackend(this, this.connection.value) + // Make sure to only emit this once the backend is in place. this.emit('opened', this.sessionConnection.state) } From 24665d2bc2b312b73025795413bbaef577c019e5 Mon Sep 17 00:00:00 2001 From: Max Date: Sun, 13 Jul 2025 11:36:51 +0200 Subject: [PATCH 3/6] chore(refactor): use api functions in all of cypress session handling Signed-off-by: Max --- cypress/e2e/api/SessionApi.spec.js | 98 +++++++++++++++--------------- cypress/e2e/api/UsersApi.spec.js | 15 ++--- cypress/support/sessions.js | 73 +++++++++------------- 3 files changed, 86 insertions(+), 100 deletions(-) diff --git a/cypress/e2e/api/SessionApi.spec.js b/cypress/e2e/api/SessionApi.spec.js index 98eb5a135a1..c7967c77c36 100644 --- a/cypress/e2e/api/SessionApi.spec.js +++ b/cypress/e2e/api/SessionApi.spec.js @@ -38,20 +38,20 @@ describe('The session Api', function() { }) it('returns connection', function() { - cy.createTextSession(fileId).then(connection => { + cy.openConnection({ fileId }).then(({ connection }) => { cy.wrap(connection) - .its('document.id') + .its('documentId') .should('equal', fileId) - cy.destroySession(connection) + cy.closeConnection(connection) }) }) it('provides initial content', function() { - cy.createTextSession(fileId, { filePath }).then(connection => { - cy.wrap(connection) - .its('state.documentSource') + cy.openConnection({fileId, filePath }).then(({ connection, data }) => { + cy.wrap(data) + .its('content') .should('eql', '## Hello world\n') - cy.destroySession(connection) + cy.closeConnection(connection) }) }) @@ -74,14 +74,14 @@ describe('The session Api', function() { beforeEach(function() { cy.uploadTestFile() - .then(cy.createTextSession) - .then(con => { + .then((fileId) => cy.openConnection({ fileId })) + .then(({ connection: con }) => { connection = con }) }) afterEach(function() { - cy.destroySession(connection) + cy.closeConnection(connection) }) // Echoes all message types but queries @@ -135,9 +135,9 @@ describe('The session Api', function() { cy.uploadTestFile() .then(id => { fileId = id - return cy.createTextSession(fileId, { filePath }) + return cy.openConnection({ fileId, filePath }) }) - .then(con => { + .then(({ connection: con }) => { connection = con }) }) @@ -169,18 +169,18 @@ describe('The session Api', function() { documentState, manualSave: true, }) - cy.createTextSession(fileId, { filePath }) - .then(con => { + cy.openConnection({ fileId, filePath }) + .then(({ connection: con, data }) => { joining = con - return joining + return data }) - .its('state.documentState') + .its('documentState') .should('eql', documentState) - .then(() => joining.close()) + cy.closeConnection(joining) }) afterEach(function() { - cy.destroySession(connection) + cy.closeConnection(connection) }) }) @@ -204,15 +204,15 @@ describe('The session Api', function() { }) .then(() => cy.clearCookies()) .then(() => { - return cy.createTextSession(undefined, { filePath: '', shareToken }) - .then(con => { + return cy.openConnection({ filePath: '', token: shareToken }) + .then(({ connection: con }) => { connection = con }) }) }) afterEach(function() { - cy.destroySession(connection) + cy.closeConnection(connection) }) it('starts empty public', function() { @@ -243,14 +243,14 @@ describe('The session Api', function() { documentState, manualSave: true, }) - cy.createTextSession(undefined, { filePath: '', shareToken }) - .then(con => { + cy.openConnection({ filePath: '', token: shareToken }) + .then(({ connection: con, data }) => { joining = con - return con + return data }) - .its('state.documentState') + .its('documentState') .should('eql', documentState) - .then(() => joining.close()) + cy.closeConnection(joining) }) }) @@ -269,8 +269,8 @@ describe('The session Api', function() { cy.log(token) shareToken = token cy.clearCookies() - cy.createTextSession(undefined, { filePath: '', shareToken }) - .then(con => { + cy.openConnection({ filePath: '', token: shareToken }) + .then(({ connection: con }) => { connection = con }) }) @@ -278,15 +278,15 @@ describe('The session Api', function() { it('does not send initial content if other session is alive but did not push any steps', function() { let joining - cy.createTextSession(undefined, { filePath: '', shareToken }) - .then(con => { + cy.openConnection({ filePath: '', token: shareToken }) + .then(({ connection: con, data }) => { joining = con - return con + return data }) - .its('state.documentSource') + .its('content') .should('eql', '## Hello world\n') - .then(() => cy.destroySession(joining)) - cy.destroySession(connection) + .then(() => cy.closeConnection(joining)) + cy.closeConnection(connection) }) it('does not send initial content if session is alive even without saved state', function() { @@ -294,15 +294,15 @@ describe('The session Api', function() { cy.pushSteps({ connection, steps: [messages.update], version }) .its('version') .should('be.at.least', 1) - cy.createTextSession(undefined, { filePath: '', shareToken }) - .then(con => { + cy.openConnection({ filePath: '', token: shareToken }) + .then(({ connection: con, data }) => { joining = con - return con + return data }) - .its('state.documentSource') + .its('content') .should('eql', '## Hello world\n') - .then(() => cy.destroySession(joining)) - cy.destroySession(connection) + .then(() => cy.closeConnection(joining)) + cy.closeConnection(connection) }) it('refuses create,push,sync,save with non-matching baseVersionEtag', function() { @@ -310,8 +310,7 @@ describe('The session Api', function() { .its('status') .should('eql', 412) - connection.setBaseVersionEtag('wrongBaseVersionEtag') - connection.connection.baseVersionEtag = 'wrongBaseVersionEtag' + connection.baseVersionEtag = 'wrongBaseVersionEtag' cy.failToPushSteps({ connection, steps: [messages.update], version }) .its('status') @@ -325,7 +324,7 @@ describe('The session Api', function() { .its('status') .should('equal', 412) - cy.destroySession(connection) + cy.closeConnection(connection) }) it('recovers session even if last person leaves right after create', function() { @@ -335,12 +334,12 @@ describe('The session Api', function() { .its('version') .should('be.at.least', 1) cy.log('Other user creates session') - cy.createTextSession(undefined, { filePath: '', shareToken }) - .then(con => { + cy.openConnection({ filePath: '', token: shareToken }) + .then(({ connection: con }) => { joining = con }) cy.log('Initial user closes session') - cy.destroySession(connection) + cy.closeConnection(connection) cy.log('Other user still finds the steps') .then(() => { cy.syncSteps(joining, { @@ -354,11 +353,12 @@ describe('The session Api', function() { // Skipped for now since the behaviour chanced by not cleaning up the state on close/create it.skip('ignores steps stored after close cleaned up', function() { cy.pushAndClose({ connection, steps: [messages.update], version }) - cy.createTextSession(undefined, { filePath: '', shareToken }) - .then(con => { + cy.openConnection({ filePath: '', token: shareToken }) + .then(({ connection: con, data }) => { connection = con + return data }) - .its('state.documentSource') + .its('content') .should('eql', '## Hello world\n') }) diff --git a/cypress/e2e/api/UsersApi.spec.js b/cypress/e2e/api/UsersApi.spec.js index ee9ee8cbe50..ebde91208f3 100644 --- a/cypress/e2e/api/UsersApi.spec.js +++ b/cypress/e2e/api/UsersApi.spec.js @@ -16,22 +16,22 @@ describe('The user mention API', function() { beforeEach(function() { cy.login(user) cy.uploadTestFile('test.md').as('fileId') - .then(cy.createTextSession).as('connection') - }) - - afterEach(function() { - cy.get('@connection').then(c => c.closed || c.close()) + .then((fileId) => cy.openConnection({ fileId })) + .its('connection') + .as('connection') }) it('has a valid connection', function() { cy.get('@connection') - .its('document.id') + .its('documentId') .should('equal', this.fileId) + cy.closeConnection(this.connection) }) it('fetches users with valid session', function() { cy.sessionUsers(this.connection) .its('status').should('eq', 200) + cy.closeConnection(this.connection) }) it('rejects invalid sessions', function() { @@ -41,10 +41,11 @@ describe('The user mention API', function() { .its('status').should('eq', 403) cy.sessionUsers(this.connection, { documentId: 0 }) .its('status').should('eq', 403) + cy.closeConnection(this.connection) }) it('rejects closed sessions', function() { - cy.destroySession(this.connection) + cy.closeConnection(this.connection) cy.sessionUsers(this.connection) .its('status').should('eq', 403) }) diff --git a/cypress/support/sessions.js b/cypress/support/sessions.js index 113337af175..0d4415734da 100644 --- a/cypress/support/sessions.js +++ b/cypress/support/sessions.js @@ -4,23 +4,15 @@ */ import axios from '@nextcloud/axios' -import { SessionConnection } from '../../src/services/SessionConnection.js' import { open, close } from '../../src/apis/Connect.ts' import { push, sync } from '../../src/apis/Sync.ts' import { save } from '../../src/apis/Save.ts' const url = Cypress.config('baseUrl').replace(/\/index.php\/?$/g, '') -Cypress.Commands.add('createTextSession', async (fileId, options = {}) => { - const { connection, data } = await open({ fileId, token: options.shareToken, ...options }) - return new SessionConnection(data, connection) -}) +Cypress.Commands.add('openConnection', open) -Cypress.Commands.add('destroySession', async (sessionConnection) => { - const { documentId, id, token } = sessionConnection.session - await close({ documentId, sessionId: id, sessionToken: token }) - sessionConnection.close() -}) +Cypress.Commands.add('closeConnection', close) Cypress.Commands.add('failToCreateTextSession', (fileId, baseVersionEtag = null, options = {}) => { return open({ fileId, ...options, baseVersionEtag }) @@ -29,55 +21,48 @@ Cypress.Commands.add('failToCreateTextSession', (fileId, baseVersionEtag = null, }, (err) => err.response) }) -Cypress.Commands.add('pushSteps', ({ connection: sessionConnection, steps, version, awareness = '' }) => { - return push( - sessionConnection.connection, - { steps, version, awareness } - ).then(response => response.data) +Cypress.Commands.add('pushSteps', ({ connection, steps, version, awareness = '' }) => { + return push(connection, { steps, version, awareness }) + .then(response => response.data) }) -Cypress.Commands.add('failToPushSteps', ({ connection: sessionConnection, steps, version, awareness = '' }) => { - return push( - sessionConnection.connection, - { steps, version, awareness } - ).then((_response) => { - throw new Error('Expected request to fail - but it succeeded!') - }, (err) => err.response) +Cypress.Commands.add('failToPushSteps', ({ connection, steps, version, awareness = '' }) => { + return push( connection, { steps, version, awareness }) + .then((_response) => { + throw new Error('Expected request to fail - but it succeeded!') + }, (err) => err.response) }) -Cypress.Commands.add('syncSteps', (sessionConnection, options = { version: 0 }) => { - return sync(sessionConnection.connection, options) +Cypress.Commands.add('syncSteps', (connection, options = { version: 0 }) => { + return sync(connection, options) .then(response => response.data) }) -Cypress.Commands.add('failToSyncSteps', (sessionConnection, options = { version: 0 }) => { - return sync(sessionConnection.connection, options) +Cypress.Commands.add('failToSyncSteps', (connection, options = { version: 0 }) => { + return sync(connection, options) .then((_response) => { throw new Error('Expected request to fail - but it succeeded!') }, (err) => err.response) }) -Cypress.Commands.add('save', (sessionConnection, options = { version: 0 }) => { - return save( - sessionConnection.connection, - options - ).then(response => response.data) +Cypress.Commands.add('save', (connection, options = { version: 0 }) => { + return save( connection, options) + .then(response => response.data) }) -Cypress.Commands.add('failToSave', (sessionConnection, options = { version: 0 }) => { - return save( - sessionConnection.connection, - options - ).then((response) => { - throw new Error('Expected request to fail - but it succeeded!') - }, (err) => err.response) +Cypress.Commands.add('failToSave', (connection, options = { version: 0 }) => { + return save( connection, options) + .then((_response) => { + throw new Error('Expected request to fail - but it succeeded!') + }, (err) => err.response) }) Cypress.Commands.add('sessionUsers', function(connection, bodyOptions = {}) { + const { documentId, sessionId, sessionToken } = connection const data = { - documentId: connection.document.id, - sessionId: connection.session.id, - sessionToken: connection.session.token, + documentId, + sessionId, + sessionToken, requesttoken: this.requesttoken, ...bodyOptions, } @@ -93,14 +78,14 @@ Cypress.Commands.add('sessionUsers', function(connection, bodyOptions = {}) { Cypress.Commands.add('pushAndClose', ({ connection, steps, version, awareness = '' }) => { cy.log('Race between push and close') .then(() => { - const push = connection.push({ steps, version, awareness }) + const pushed = push(connection, { steps, version, awareness }) .catch(error => { // handle 403 gracefully if (error.response?.status !== 403) { throw error } }) - const close = connection.close() - return Promise.all([push, close]) + const closed = close(connection) + return Promise.all([pushed, closed]) }) }) From f8e5ceb56fa31eef8f67efa2748e47d15897db9a Mon Sep 17 00:00:00 2001 From: Max Date: Sun, 13 Jul 2025 16:11:09 +0200 Subject: [PATCH 4/6] chore(refactor): use api functions for attachment handling Signed-off-by: Max --- src/apis/Attach.ts | 90 ++++++++++++++++++++++++++ src/components/Editor/MediaHandler.vue | 15 +++-- src/services/SessionConnection.js | 38 ----------- src/services/SyncService.ts | 21 ------ 4 files changed, 98 insertions(+), 66 deletions(-) create mode 100644 src/apis/Attach.ts diff --git a/src/apis/Attach.ts b/src/apis/Attach.ts new file mode 100644 index 00000000000..df174e33b23 --- /dev/null +++ b/src/apis/Attach.ts @@ -0,0 +1,90 @@ +/** + * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import axios from '@nextcloud/axios' +import { generateUrl } from '@nextcloud/router' +import type { Connection } from '../composables/useConnection.js' +import { unref, type ShallowRef } from 'vue' + +/** + * Upload an attachment to the server. + * @param connection the active connection. + * @param file upload this file. + */ +export function uploadAttachment( + connection: ShallowRef | Connection, + file: string | Blob, +) { + const { + documentId, + sessionId, + sessionToken, + shareToken: token, + } = unref(connection) + const params = Object.entries({ documentId, sessionId, sessionToken, token }) + .map(([k, v]) => v && [k, v.toString()]) // convert numbers to strings + .filter((pair) => !!pair) // leave out token if it's undefined. + const formData = new FormData() + formData.append('file', file) + const pub = token ? '/public' : '' + const url = + generateUrl(`apps/text${pub}/attachment/upload?`) + + new URLSearchParams(params) + return axios.post(url, formData, { + headers: { 'Content-Type': 'multipart/form-data' }, + }) +} + +/** + * Create a new attachment based on the given template + * @param connection the active connection + * @param template create the attachment based on this + * @param template.app app to create the attachment with + * @param template.extension extension to use + */ +export function createAttachment( + connection: ShallowRef | Connection, + template: { app: string; extension: string }, +) { + const { + documentId, + sessionId, + sessionToken, + shareToken: token, + } = unref(connection) + const pub = token ? '/public' : '' + const url = generateUrl(`apps/text${pub}/attachment/create`) + return axios.post(url, { + documentId, + sessionId, + sessionToken, + fileName: `${template.app}${template.extension}`, + }) +} + +/** + * Create a new attachment based on the given template + * @param connection the active connection + * @param filePath path to the file on the server. + */ +export function insertAttachmentFile( + connection: ShallowRef | Connection, + filePath: string, +) { + const { + documentId, + sessionId, + sessionToken, + shareToken: token, + } = unref(connection) + const pub = token ? '/public' : '' + const url = generateUrl(`apps/text${pub}/attachment/filepath`) + return axios.post(url, { + documentId, + sessionId, + sessionToken, + filePath, + }) +} diff --git a/src/components/Editor/MediaHandler.vue b/src/components/Editor/MediaHandler.vue index 7316a092c1f..e6811894510 100644 --- a/src/components/Editor/MediaHandler.vue +++ b/src/components/Editor/MediaHandler.vue @@ -28,8 +28,9 @@ import { getCurrentUser } from '@nextcloud/auth' import { showError } from '@nextcloud/dialogs' import { emit } from '@nextcloud/event-bus' import { generateUrl } from '@nextcloud/router' -import { logger } from '../../helpers/logger.js' import { useIsMobile } from '@nextcloud/vue/composables/useIsMobile' +import { logger } from '../../helpers/logger.js' +import { createAttachment, insertAttachmentFile, uploadAttachment } from '../../apis/Attach.ts' import { useFileMixin, @@ -42,7 +43,7 @@ import { ACTION_CREATE_ATTACHMENT, STATE_UPLOADING, } from './MediaHandler.provider.js' -import { useSyncService } from '../../composables/useSyncService.ts' +import { useConnection } from '../../composables/useConnection.ts' const getDir = (val) => val.split('/').slice(0, -1).join('/') @@ -70,11 +71,11 @@ export default { return val }, setup() { + const { connection } = useConnection() const isMobile = useIsMobile() const { editor } = useEditor() - const { syncService } = useSyncService() return { - editor, isMobile, syncService + connection, editor, isMobile } }, data() { @@ -134,7 +135,7 @@ export default { async uploadAttachmentFile(file, position = null) { this.state.isUploadingAttachments = true - return this.syncService.uploadAttachment(file) + return uploadAttachment(this.connection, file) .then((response) => { this.insertAttachment( response.data?.name, response.data?.id, file.type, @@ -168,7 +169,7 @@ export default { this.state.isUploadingAttachments = true - return this.syncService.insertAttachmentFile(filePath).then((response) => { + return insertAttachmentFile(this.connection, filePath).then((response) => { this.insertAttachment( response.data?.name, response.data?.id, response.data?.mimetype, null, response.data?.dirname, @@ -182,7 +183,7 @@ export default { }, createAttachment(template) { this.state.isUploadingAttachments = true - return this.syncService.createAttachment(template).then((response) => { + return createAttachment(this.connection, template).then((response) => { this.insertAttachmentPreview(response.data?.id) }).catch((error) => { logger.error('Failed to create attachment', { error }) diff --git a/src/services/SessionConnection.js b/src/services/SessionConnection.js index 4c9f24c9105..f07a9d67196 100644 --- a/src/services/SessionConnection.js +++ b/src/services/SessionConnection.js @@ -86,44 +86,6 @@ export class SessionConnection { }) } - uploadAttachment(file) { - const formData = new FormData() - formData.append('file', file) - const url = - _endpointUrl('attachment/upload') - + '?documentId=' - + encodeURIComponent(this.#document.id) - + '&sessionId=' - + encodeURIComponent(this.#session.id) - + '&sessionToken=' - + encodeURIComponent(this.#session.token) - + '&token=' - + encodeURIComponent(this.connection.shareToken || '') - return this.#post(url, formData, { - headers: { - 'Content-Type': 'multipart/form-data', - }, - }) - } - - createAttachment(template) { - return this.#post(_endpointUrl('attachment/create'), { - documentId: this.#document.id, - sessionId: this.#session.id, - sessionToken: this.#session.token, - fileName: `${template.app}${template.extension}`, - }) - } - - insertAttachmentFile(filePath) { - return this.#post(_endpointUrl('attachment/filepath'), { - documentId: this.#document.id, - sessionId: this.#session.id, - sessionToken: this.#session.token, - filePath, - }) - } - close() { this.closed = true } diff --git a/src/services/SyncService.ts b/src/services/SyncService.ts index ee35c71c3d2..a72622d573f 100644 --- a/src/services/SyncService.ts +++ b/src/services/SyncService.ts @@ -367,27 +367,6 @@ class SyncService { this.emit('close') } - uploadAttachment(file: object) { - if (!this.hasActiveConnection()) { - throw new Error('Not connected to server.') - } - return this.sessionConnection.uploadAttachment(file) - } - - insertAttachmentFile(filePath: string) { - if (!this.hasActiveConnection()) { - throw new Error('Not connected to server.') - } - return this.sessionConnection.insertAttachmentFile(filePath) - } - - createAttachment(template: object) { - if (!this.hasActiveConnection()) { - throw new Error('Not connected to server.') - } - return this.sessionConnection.createAttachment(template) - } - // For better typing use the bus directly: `syncService.bus.on()`. on(event: keyof EventTypes, callback: Handler) { this.bus.on(event, callback) From 7c77f4d337d11b31fd74544506321c3c41d6f071 Mon Sep 17 00:00:00 2001 From: Max Date: Sun, 13 Jul 2025 16:44:15 +0200 Subject: [PATCH 5/6] chore(rename): use lower case for api function modules Signed-off-by: Max --- cypress/support/sessions.js | 6 +++--- src/apis/{Attach.ts => attach.ts} | 0 src/apis/{Connect.ts => connect.ts} | 0 src/apis/{Mention.ts => mention.ts} | 0 src/apis/{Save.ts => save.ts} | 2 +- src/apis/{Sync.ts => sync.ts} | 0 src/components/Editor/MediaHandler.vue | 2 +- src/components/Suggestion/Mention/suggestions.js | 2 +- src/composables/useConnection.ts | 2 +- src/services/PollingBackend.ts | 2 +- src/services/SaveService.ts | 2 +- src/services/SyncService.ts | 4 ++-- src/tests/services/SyncService.spec.ts | 4 ++-- 13 files changed, 13 insertions(+), 13 deletions(-) rename src/apis/{Attach.ts => attach.ts} (100%) rename src/apis/{Connect.ts => connect.ts} (100%) rename src/apis/{Mention.ts => mention.ts} (100%) rename src/apis/{Save.ts => save.ts} (97%) rename src/apis/{Sync.ts => sync.ts} (100%) diff --git a/cypress/support/sessions.js b/cypress/support/sessions.js index 0d4415734da..f4d35776535 100644 --- a/cypress/support/sessions.js +++ b/cypress/support/sessions.js @@ -4,9 +4,9 @@ */ import axios from '@nextcloud/axios' -import { open, close } from '../../src/apis/Connect.ts' -import { push, sync } from '../../src/apis/Sync.ts' -import { save } from '../../src/apis/Save.ts' +import { open, close } from '../../src/apis/connect.ts' +import { push, sync } from '../../src/apis/sync.ts' +import { save } from '../../src/apis/save.ts' const url = Cypress.config('baseUrl').replace(/\/index.php\/?$/g, '') diff --git a/src/apis/Attach.ts b/src/apis/attach.ts similarity index 100% rename from src/apis/Attach.ts rename to src/apis/attach.ts diff --git a/src/apis/Connect.ts b/src/apis/connect.ts similarity index 100% rename from src/apis/Connect.ts rename to src/apis/connect.ts diff --git a/src/apis/Mention.ts b/src/apis/mention.ts similarity index 100% rename from src/apis/Mention.ts rename to src/apis/mention.ts diff --git a/src/apis/Save.ts b/src/apis/save.ts similarity index 97% rename from src/apis/Save.ts rename to src/apis/save.ts index cb94a3bdc23..f32959bf507 100644 --- a/src/apis/Save.ts +++ b/src/apis/save.ts @@ -8,7 +8,7 @@ import { getRequestToken } from '@nextcloud/auth' import type { Connection } from '../composables/useConnection.js' import { unref, type ShallowRef } from 'vue' import { generateUrl } from '@nextcloud/router' -import type { Document } from '../services/SyncService.ts' +import type { Document } from '../services/SyncService.js' interface SaveData { version: number diff --git a/src/apis/Sync.ts b/src/apis/sync.ts similarity index 100% rename from src/apis/Sync.ts rename to src/apis/sync.ts diff --git a/src/components/Editor/MediaHandler.vue b/src/components/Editor/MediaHandler.vue index e6811894510..60440b74045 100644 --- a/src/components/Editor/MediaHandler.vue +++ b/src/components/Editor/MediaHandler.vue @@ -30,7 +30,7 @@ import { emit } from '@nextcloud/event-bus' import { generateUrl } from '@nextcloud/router' import { useIsMobile } from '@nextcloud/vue/composables/useIsMobile' import { logger } from '../../helpers/logger.js' -import { createAttachment, insertAttachmentFile, uploadAttachment } from '../../apis/Attach.ts' +import { createAttachment, insertAttachmentFile, uploadAttachment } from '../../apis/attach.ts' import { useFileMixin, diff --git a/src/components/Suggestion/Mention/suggestions.js b/src/components/Suggestion/Mention/suggestions.js index 4a91a71b896..6a939e67194 100644 --- a/src/components/Suggestion/Mention/suggestions.js +++ b/src/components/Suggestion/Mention/suggestions.js @@ -5,7 +5,7 @@ import MentionList from './MentionList.vue' import createSuggestions from '../suggestions.js' -import { emitMention, getUsers } from '../../../apis/Mention.ts' +import { emitMention, getUsers } from '../../../apis/mention.ts' export default ({ connection, options }) => createSuggestions({ listComponent: MentionList, diff --git a/src/composables/useConnection.ts b/src/composables/useConnection.ts index 95f2de61e9c..0e1ae9beeaa 100644 --- a/src/composables/useConnection.ts +++ b/src/composables/useConnection.ts @@ -4,7 +4,7 @@ */ import { inject, provide, shallowRef, type InjectionKey, type ShallowRef } from 'vue' -import { open } from '../apis/Connect.js' +import { open } from '../apis/connect' import type { Document, Session } from '../services/SyncService.js' export interface Connection { diff --git a/src/services/PollingBackend.ts b/src/services/PollingBackend.ts index cd2fbc604ff..f863f31825a 100644 --- a/src/services/PollingBackend.ts +++ b/src/services/PollingBackend.ts @@ -12,7 +12,7 @@ import { import getNotifyBus, { type EventTypes } from './NotifyService' import type { Emitter } from 'mitt' import type { Connection } from '../composables/useConnection.js' -import { sync } from '../apis/Sync.js' +import { sync } from '../apis/sync' /** * Minimum inverval to refetch the document changes in ms. diff --git a/src/services/SaveService.ts b/src/services/SaveService.ts index d68f4f5bff1..578a25750ce 100644 --- a/src/services/SaveService.ts +++ b/src/services/SaveService.ts @@ -9,7 +9,7 @@ import type { ShallowRef } from 'vue' import { logger } from '../helpers/logger.js' import type { SyncService } from './SyncService.js' import type { Connection } from '../composables/useConnection.ts' -import { save, saveViaSendBeacon } from '../apis/Save' +import { save, saveViaSendBeacon } from '../apis/save' /** * Interval to save the serialized document and the document state diff --git a/src/services/SyncService.ts b/src/services/SyncService.ts index a72622d573f..4744ef8248f 100644 --- a/src/services/SyncService.ts +++ b/src/services/SyncService.ts @@ -14,8 +14,8 @@ import { documentStateToStep } from '../helpers/yjs.js' import { logger } from '../helpers/logger.js' import type { ShallowRef } from 'vue' import type { Connection } from '../composables/useConnection.js' -import { close, type OpenData } from '../apis/Connect.js' -import { push } from '../apis/Sync.js' +import { close, type OpenData } from '../apis/connect' +import { push } from '../apis/sync' /** * Timeout after which the editor will consider a document without changes being synced as idle diff --git a/src/tests/services/SyncService.spec.ts b/src/tests/services/SyncService.spec.ts index 88432112a2e..05febf5ef84 100644 --- a/src/tests/services/SyncService.spec.ts +++ b/src/tests/services/SyncService.spec.ts @@ -6,7 +6,7 @@ import { describe, it, vi, expect } from 'vitest' import { SyncService } from '../../services/SyncService.js' import { provideConnection } from '../../composables/useConnection.js' -import * as connect from '../../apis/Connect' +import * as connect from '../../apis/connect' const connection = { documentId: 123, sessionId: 345, sessionToken: 'sessionToken', filePath: './', baseVersionEtag: 'etag'} const initialData = { session: { id: 345 }, document: { id: 123, baseVersionEtag: 'etag' }, readOnly: false, content: '', hasOwner: true } @@ -16,7 +16,7 @@ const openData = { connection, data: initialData } describe('Sync service', () => { it('opens a connection', async () => { const { connection, openConnection } = provideConnection({ fileId: 123, relativePath: './', }) - vi.mock('../../apis/Connect.ts') + vi.mock('../../apis/connect') vi.mocked(connect.open).mockResolvedValue(openData) const openHandler = vi.fn() const service = new SyncService({ connection, openConnection }) From 889dd6987eabde56b9cbae015f103146926edb85 Mon Sep 17 00:00:00 2001 From: Max Date: Mon, 14 Jul 2025 08:34:59 +0200 Subject: [PATCH 6/6] chore(simplify): use axios params api It handles numbers and leaves out undefined params all by itself. :) Signed-off-by: Max --- src/apis/attach.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/apis/attach.ts b/src/apis/attach.ts index df174e33b23..9506da19fde 100644 --- a/src/apis/attach.ts +++ b/src/apis/attach.ts @@ -23,17 +23,13 @@ export function uploadAttachment( sessionToken, shareToken: token, } = unref(connection) - const params = Object.entries({ documentId, sessionId, sessionToken, token }) - .map(([k, v]) => v && [k, v.toString()]) // convert numbers to strings - .filter((pair) => !!pair) // leave out token if it's undefined. const formData = new FormData() formData.append('file', file) const pub = token ? '/public' : '' - const url = - generateUrl(`apps/text${pub}/attachment/upload?`) - + new URLSearchParams(params) + const url = generateUrl(`apps/text${pub}/attachment/upload?`) return axios.post(url, formData, { headers: { 'Content-Type': 'multipart/form-data' }, + params: { documentId, sessionId, sessionToken, token }, }) }