chore(refactor): split open api functions

Two api endpoints deserve two different functions
in particular as they also expect different params.

This also allows narrowing down the types a little more.

Signed-off-by: Max <max@nextcloud.com>

Author: max-nextcloud

cypress/e2e/api/SessionApi.spec.js

@@ -36,14 +36,14 @@ describe('The session Api', function() {
 		})
 
 		it('returns connection', function() {
-			cy.openConnection({ fileId }).then(({ connection }) => {
+			cy.openFileConnection({ fileId }).then(({ connection }) => {
 				cy.wrap(connection).its('documentId').should('equal', fileId)
 				cy.closeConnection(connection)
 			})
 		})
 
 		it('provides initial content', function() {
-			cy.openConnection({ fileId, filePath }).then(({ connection, data }) => {
+			cy.openFileConnection({ fileId, filePath }).then(({ connection, data }) => {
 				cy.wrap(data).its('content').should('eql', '## Hello world\n')
 				cy.closeConnection(connection)
 			})
@@ -63,7 +63,7 @@ describe('The session Api', function() {
 
 		beforeEach(function() {
 			cy.uploadTestFile()
-				.then((fileId) => cy.openConnection({ fileId }))
+				.then((fileId) => cy.openFileConnection({ fileId }))
 				.then(({ connection: con }) => {
 					connection = con
 				})
@@ -116,7 +116,7 @@ describe('The session Api', function() {
 			cy.uploadTestFile()
 				.then((id) => {
 					fileId = id
-					return cy.openConnection({ fileId, filePath })
+					return cy.openFileConnection({ fileId, filePath })
 				})
 				.then(({ connection: con }) => {
 					connection = con
@@ -151,7 +151,7 @@ describe('The session Api', function() {
 				documentState,
 				manualSave: true,
 			})
-			cy.openConnection({ fileId, filePath })
+			cy.openFileConnection({ fileId, filePath })
 				.as('joining')
 				.its('data.documentState')
 				.should('eql', documentState)
@@ -183,7 +183,7 @@ describe('The session Api', function() {
 				.then(() => cy.clearCookies())
 				.then(() => {
 					return cy
-						.openConnection({ filePath: '', token: shareToken })
+						.openShareConnection({ filePath: '', token: shareToken })
 						.then(({ connection: con }) => {
 							connection = con
 						})
@@ -223,7 +223,7 @@ describe('The session Api', function() {
 				documentState,
 				manualSave: true,
 			})
-			cy.openConnection({ filePath: '', token: shareToken })
+			cy.openShareConnection({ filePath: '', token: shareToken })
 				.as('joining')
 				.its('data.documentState')
 				.should('eql', documentState)
@@ -247,15 +247,15 @@ describe('The session Api', function() {
 					cy.log(token)
 					shareToken = token
 					cy.clearCookies()
-					cy.openConnection({ filePath: '', token: shareToken }).then(({ connection: con }) => {
+					cy.openShareConnection({ filePath: '', token: shareToken }).then(({ connection: con }) => {
 						connection = con
 					})
 				})
 		})
 
 		it('does not send initial content if other session is alive but did not push any steps', function() {
 			let joining
-			cy.openConnection({ filePath: '', token: shareToken })
+			cy.openShareConnection({ filePath: '', token: shareToken })
 				.then(({ connection: con, data }) => {
 					joining = con
 					return data
@@ -271,7 +271,7 @@ describe('The session Api', function() {
 			cy.pushSteps({ connection, steps: [messages.update], version })
 				.its('version')
 				.should('eql', 0)
-			cy.openConnection({ filePath: '', token: shareToken })
+			cy.openShareConnection({ filePath: '', token: shareToken })
 				.then(({ connection: con, data }) => {
 					joining = con
 					return data
@@ -312,7 +312,7 @@ describe('The session Api', function() {
 				.its('version')
 				.should('eql', 0)
 			cy.log('Other user creates session')
-			cy.openConnection({ filePath: '', token: shareToken }).then(({ connection: con }) => {
+			cy.openShareConnection({ filePath: '', token: shareToken }).then(({ connection: con }) => {
 				joining = con
 			})
 			cy.log('Initial user closes session')
@@ -330,7 +330,7 @@ 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.openConnection({ filePath: '', token: shareToken })
+			cy.openShareConnection({ filePath: '', token: shareToken })
 				.then(({ connection: con, data }) => {
 					connection = con
 					return data

cypress/e2e/api/UsersApi.spec.js

@@ -16,7 +16,7 @@ describe('The user mention API', function() {
 		cy.login(user)
 		cy.uploadTestFile('test.md')
 			.as('fileId')
-			.then((fileId) => cy.openConnection({ fileId }))
+			.then((fileId) => cy.openFileConnection({ fileId }))
 			.its('connection')
 			.as('connection')
 	})

cypress/support/sessions.js

@@ -4,20 +4,21 @@
  */
 
 import axios from '@nextcloud/axios'
-import { close, open } from '../../src/apis/connect.ts'
+import { close, openFile, openShare } from '../../src/apis/connect.ts'
 import { save } from '../../src/apis/save.ts'
 import { push, sync } from '../../src/apis/sync.ts'
 
 const url = Cypress.config('baseUrl').replace(/\/index.php\/?$/g, '')
 
-Cypress.Commands.add('openConnection', open)
+Cypress.Commands.add('openFileConnection', openFile)
+Cypress.Commands.add('openShareConnection', openShare)
 
 Cypress.Commands.add('closeConnection', close)
 
 Cypress.Commands.add(
 	'failToCreateTextSession',
 	(fileId, baseVersionEtag = null, options = {}) => {
-		return open({ fileId, ...options, baseVersionEtag }).then(
+		return openFile({ fileId, ...options, baseVersionEtag }).then(
 			() => {
 				throw new Error('Expected request to fail - but it succeeded!')
 			},

src/apis/connect.ts

@@ -10,10 +10,15 @@ import axios from '@nextcloud/axios'
 import { generateUrl } from '@nextcloud/router'
 
 export interface OpenParams {
-	fileId?: number
+	fileId: number
+	filePath: string // not send to the api but included in the connection
 	baseVersionEtag?: string
+}
+
+export interface OpenShareParams {
+	token: string
 	filePath: string
-	token?: string
+	baseVersionEtag?: string
 	guestName?: string
 }
 
@@ -27,16 +32,31 @@ export interface OpenData {
 	hasOwner: boolean
 }
 
+/**
+ * Open editing connection to a file when logged in
+ *
+ * @param params Parameters identifying the document
+ */
+export async function openFile(params: OpenParams): Promise<{ connection: Connection, data: OpenData }> {
+	const url = generateUrl(`/apps/text/session/${params.fileId}/create`)
+	const response = await axios.put(url, params)
+	const { document, session } = response.data
+	const connection = {
+		documentId: document.id,
+		sessionId: session.id,
+		sessionToken: session.token,
+		baseVersionEtag: document.baseVersionEtag,
+	}
+	return { connection, data: response.data }
+}
+
 /**
  * Open editing connection to the document
  *
  * @param params Parameters identifying the document
  */
-export async function open(params: OpenParams): Promise<{ connection: Connection, data: OpenData }> {
-	const _baseUrl = params.token
-		? generateUrl('/apps/text/public')
-		: generateUrl('/apps/text')
-	const url = `${_baseUrl}/session/${params.fileId}/create`
+export async function openShare(params: OpenShareParams): Promise<{ connection: Connection, data: OpenData }> {
+	const url = generateUrl('/apps/text/public/session/123/create')
 	const response = await axios.put(url, params)
 	const { document, session } = response.data
 	const connection = {

src/composables/useConnection.ts

@@ -8,7 +8,7 @@ import type { OpenData } from '../apis/connect.ts'
 import type { Document, Session } from '../services/SyncService.ts'
 
 import { inject, provide, shallowRef } from 'vue'
-import { open } from '../apis/connect.ts'
+import * as api from '../apis/connect.ts'
 
 export interface Connection {
 	documentId: number
@@ -65,13 +65,8 @@ export function provideConnection(
 		const guestName = localStorage.getItem('nick') ?? ''
 		const { connection: opened, data }
 			= openInitialSession(props, baseVersionEtag)
-				|| (await open({
-					fileId: props.fileId,
-					guestName,
-					token: props.shareToken,
-					filePath: props.relativePath,
-					baseVersionEtag,
-				}))
+				|| await openShare(props, baseVersionEtag, guestName)
+				|| await openFile(props, baseVersionEtag)
 		await setBaseVersionEtag(data.document.baseVersionEtag)
 		connection.value = opened
 		openData.value = data
@@ -134,3 +129,56 @@ function openInitialSession(
 		return { connection, data: props.initialSession }
 	}
 }
+
+/**
+ * Get the connection and additional data from the initial session if available.
+ *
+ * @param props Props of the editor component
+ * @param props.relativePath Relative path to the file.
+ * @param props.shareToken Share token of the file.
+ * @param props.fileId id of the file
+ * @param baseVersionEtag Etag from the last editing session.
+ * @param guestName to be shown to other participants.
+ */
+async function openShare(
+	props: {
+		fileId: number
+		relativePath: string
+		shareToken?: string
+	},
+	baseVersionEtag: string | undefined,
+	guestName: string | undefined,
+) {
+	if (props.shareToken) {
+		return api.openShare({
+			guestName,
+			token: props.shareToken,
+			filePath: props.relativePath,
+			fileId: props.fileId,
+			baseVersionEtag,
+		})
+	}
+}
+
+/**
+ * Get the connection and additional data from the initial session if available.
+ *
+ * @param props Props of the editor component
+ * @param props.fileId id of the file
+ * @param props.relativePath Relative path to the file.
+ * @param baseVersionEtag Etag from the last editing session.
+ */
+async function openFile(
+	props: {
+		fileId: number
+		relativePath: string
+	},
+	baseVersionEtag: string | undefined,
+) {
+	return api.openFile({
+		type: 'file',
+		id: props.fileId,
+		filePath: props.relativePath,
+		baseVersionEtag,
+	})
+}

src/tests/services/SyncService.spec.ts

@@ -54,7 +54,7 @@ describe('Sync service', () => {
 			setBaseVersionEtag,
 		)
 		vi.mock('../../apis/connect')
-		vi.mocked(connect.open).mockResolvedValue(openResult)
+		vi.mocked(connect.openFile).mockResolvedValue(openResult)
 		const openHandler = vi.fn()
 		const service = new SyncService({ connection, openConnection })
 		service.bus.on('opened', openHandler)