Refactor Polling Backend and Sync service

appinfo/routes.php

@@ -35,13 +35,15 @@
 		['name' => 'Attachment#getMediaFileMetadata', 'url' => '/mediaMetadata', 'verb' => 'GET'],
 
 		['name' => 'Session#create', 'url' => '/session/create', 'verb' => 'PUT'],
+		['name' => 'Session#save', 'url' => '/session/save', 'verb' => 'POST'],
 		['name' => 'Session#sync', 'url' => '/session/sync', 'verb' => 'POST'],
 		['name' => 'Session#push', 'url' => '/session/push', 'verb' => 'POST'],
 		['name' => 'Session#close', 'url' => '/session/close', 'verb' => 'POST'],
 		['name' => 'Session#mention', 'url' => '/session/mention', 'verb' => 'PUT'],
 
 		['name' => 'PublicSession#create', 'url' => '/public/session/create', 'verb' => 'PUT'],
 		['name' => 'PublicSession#updateSession', 'url' => '/public/session', 'verb' => 'POST'],
+		['name' => 'PublicSession#save', 'url' => '/public/session/save', 'verb' => 'POST'],
 		['name' => 'PublicSession#sync', 'url' => '/public/session/sync', 'verb' => 'POST'],
 		['name' => 'PublicSession#push', 'url' => '/public/session/push', 'verb' => 'POST'],
 

cypress/e2e/SessionApi.spec.js

@@ -130,12 +130,14 @@ describe('The session Api', function() {
 				})
 			cy.pushSteps({ connection, steps: [messages.query], version })
 				.then(response => {
+					// no steps inserted
 					cy.wrap(response)
 						.its('version')
 						.should('eql', 0)
+					// but returns all 3 other samples
 					cy.wrap(response)
 						.its('steps.length')
-						.should('eql', 0)
+						.should('eql', 3)
 				})
 		})
 	})
@@ -171,7 +173,7 @@ describe('The session Api', function() {
 			cy.pushSteps({ connection, steps: [messages.update], version })
 				.its('version')
 				.should('eql', 1)
-			cy.syncSteps(connection, { version: 1, autosaveContent: '# Heading 1', manualSave: true })
+			cy.saveDoc(connection, { version: 1, autosaveContent: '# Heading 1', manualSave: true })
 			cy.downloadFile(filePath)
 				.its('data')
 				.should('eql', '# Heading 1')
@@ -182,7 +184,7 @@ describe('The session Api', function() {
 			cy.pushSteps({ connection, steps: [messages.update], version })
 				.its('version')
 				.should('eql', 1)
-			cy.syncSteps(connection, {
+			cy.saveDoc(connection, {
 				version: 1,
 				autosaveContent: '# Heading 1',
 				documentState,
@@ -245,7 +247,7 @@ describe('The session Api', function() {
 			cy.pushSteps({ connection, steps: [messages.update], version })
 				.its('version')
 				.should('eql', 1)
-			cy.syncSteps(connection, { version: 1, autosaveContent: '# Heading 1', manualSave: true })
+			cy.saveDoc(connection, { version: 1, autosaveContent: '# Heading 1', manualSave: true })
 			cy.login(user)
 			cy.prepareSessionApi()
 			cy.downloadFile('saves.md')
@@ -258,7 +260,7 @@ describe('The session Api', function() {
 			cy.pushSteps({ connection, steps: [messages.update], version })
 				.its('version')
 				.should('eql', 1)
-			cy.syncSteps(connection, {
+			cy.saveDoc(connection, {
 				version: 1,
 				autosaveContent: '# Heading 1',
 				documentState,

cypress/e2e/sync.spec.js

@@ -37,25 +37,29 @@ describe('yjs document state', () => {
 		cy.openTestFile()
 		cy.getContent().find('h2').should('contain', 'Hello world')
 		cy.wait('@sync', { timeout: 7000 })
-		cy.getContent().type('* Saving the doc saves the doc state{enter}')
-		cy.wait('@sync', { timeout: 7000 })
 	})
 
-	it('saves the actual file and document state', () => {
+	it('saves the actual file', () => {
+		cy.getContent().type('* Saving the doc saves the file{enter}')
+		cy.wait('@sync', { timeout: 7000 })
+		cy.intercept({ method: 'POST', url: '**/apps/text/session/save' }).as('save')
 		cy.getContent().type('{ctrl+s}')
-		cy.wait('@sync').its('request.body')
+		cy.wait('@save').its('request.body')
 			.should('have.property', 'autosaveContent')
 			.should('not.be.empty')
 		cy.closeFile()
 		cy.testName()
 			.then(name => cy.downloadFile(`/${name}.md`))
 			.its('data')
-			.should('include', 'saves the doc state')
+			.should('include', 'saves the file')
 	})
 
 	it('passes the doc content from one session to the next', () => {
+		cy.getContent().type('* Saving the doc saves the doc state{enter}')
+		cy.wait('@sync', { timeout: 7000 })
+		cy.intercept({ method: 'POST', url: '**/apps/text/session/save' }).as('save')
 		cy.getContent().type('{ctrl+s}')
-		cy.wait('@sync').its('request.body')
+		cy.wait('@save').its('request.body')
 			.should('have.property', 'documentState')
 			.should('not.be.empty')
 		cy.closeFile()

cypress/support/sessions.js

@@ -51,3 +51,8 @@ Cypress.Commands.add('syncSteps', (connection, options = { version: 0 }) => {
 	return connection.sync(options)
 		.then(response => response.data)
 })
+
+Cypress.Commands.add('saveDoc', (connection, options = { }) => {
+	return connection.save(options)
+		.then(response => response.data)
+})

lib/Controller/PublicSessionController.php

@@ -82,16 +82,24 @@ public function close(int $documentId, int $sessionId, string $sessionToken): Da
 	 * @NoAdminRequired
 	 * @PublicPage
 	 */
-	public function push(int $documentId, int $sessionId, string $sessionToken, int $version, array $steps, string $awareness, string $token): DataResponse {
-		return $this->apiService->push($documentId, $sessionId, $sessionToken, $version, $steps, $awareness, $token);
+	public function push(int $documentId, int $sessionId, string $sessionToken, array $steps, string $awareness, string $token): DataResponse {
+		return $this->apiService->push($documentId, $sessionId, $sessionToken, $steps, $awareness, $token);
 	}
 
 	/**
 	 * @NoAdminRequired
 	 * @PublicPage
 	 */
-	public function sync(string $token, int $documentId, int $sessionId, string $sessionToken, int $version = 0, string $autosaveContent = null, string $documentState = null, bool $force = false, bool $manualSave = false): DataResponse {
-		return $this->apiService->sync($documentId, $sessionId, $sessionToken, $version, $autosaveContent, $documentState, $force, $manualSave, $token);
+	public function sync(string $token, int $documentId, int $sessionId, string $sessionToken, int $version = 0): DataResponse {
+		return $this->apiService->sync($documentId, $sessionId, $sessionToken, $version);
+	}
+
+	/**
+	 * @NoAdminRequired
+	 * @PublicPage
+	 */
+	public function save(string $token, int $documentId, int $sessionId, string $sessionToken, int $version = 0, string $autosaveContent = null, string $documentState = null, bool $force = false, bool $manualSave = false): DataResponse {
+		return $this->apiService->save($documentId, $sessionId, $sessionToken, $version, $autosaveContent, $documentState, $force, $manualSave, $token);
 	}
 
 	/**

lib/Controller/SessionController.php

@@ -69,18 +69,27 @@ public function close(int $documentId, int $sessionId, string $sessionToken): Da
 	 * @NoAdminRequired
 	 * @PublicPage
 	 */
-	public function push(int $documentId, int $sessionId, string $sessionToken, int $version, array $steps, string $awareness): DataResponse {
+	public function push(int $documentId, int $sessionId, string $sessionToken, array $steps, string $awareness): DataResponse {
 		$this->loginSessionUser($documentId, $sessionId, $sessionToken);
-		return $this->apiService->push($documentId, $sessionId, $sessionToken, $version, $steps, $awareness);
+		return $this->apiService->push($documentId, $sessionId, $sessionToken, $steps, $awareness);
 	}
 
 	/**
 	 * @NoAdminRequired
 	 * @PublicPage
 	 */
-	public function sync(int $documentId, int $sessionId, string $sessionToken, int $version = 0, string $autosaveContent = null, string $documentState = null, bool $force = false, bool $manualSave = false): DataResponse {
+	public function sync(int $documentId, int $sessionId, string $sessionToken, int $version = 0): DataResponse {
 		$this->loginSessionUser($documentId, $sessionId, $sessionToken);
-		return $this->apiService->sync($documentId, $sessionId, $sessionToken, $version, $autosaveContent, $documentState, $force, $manualSave);
+		return $this->apiService->sync($documentId, $sessionId, $sessionToken, $version);
+	}
+
+	/**
+	 * @NoAdminRequired
+	 * @PublicPage
+	 */
+	public function save(int $documentId, int $sessionId, string $sessionToken, int $version = 0, string $autosaveContent = null, string $documentState = null, bool $force = false, bool $manualSave = false): DataResponse {
+		$this->loginSessionUser($documentId, $sessionId, $sessionToken);
+		return $this->apiService->save($documentId, $sessionId, $sessionToken, $version, $autosaveContent, $documentState, $force, $manualSave);
 	}
 
 	/**

lib/Service/ApiService.php

@@ -189,7 +189,7 @@ public function close($documentId, $sessionId, $sessionToken): DataResponse {
 	 * @throws NotFoundException
 	 * @throws DoesNotExistException
 	 */
-	public function push($documentId, $sessionId, $sessionToken, $version, $steps, $awareness, $token = null): DataResponse {
+	public function push($documentId, $sessionId, $sessionToken, $steps, $awareness, $token = null): DataResponse {
 		if (!$this->sessionService->isValidSession($documentId, $sessionId, $sessionToken)) {
 			return new DataResponse([], 403);
 		}
@@ -201,7 +201,7 @@ public function push($documentId, $sessionId, $sessionToken, $version, $steps, $
 		$file = $this->documentService->getFileForSession($session, $token);
 		if (!$this->documentService->isReadOnly($file, $token)) {
 			try {
-				$result = $this->documentService->addStep($documentId, $sessionId, $steps, $version);
+				$result = $this->documentService->addStep($documentId, $sessionId, $steps);
 			} catch (InvalidArgumentException $e) {
 				return new DataResponse($e->getMessage(), 422);
 			}
@@ -210,18 +210,25 @@ public function push($documentId, $sessionId, $sessionToken, $version, $steps, $
 		return new DataResponse([], 403);
 	}
 
-	public function sync($documentId, $sessionId, $sessionToken, $version = 0, $autosaveContent = null, $documentState = null, bool $force = false, bool $manualSave = false, $token = null): DataResponse {
+	public function sync($documentId, $sessionId, $sessionToken, $version = 0): DataResponse {
 		if (!$this->sessionService->isValidSession($documentId, $sessionId, $sessionToken)) {
 			return new DataResponse([], 403);
 		}
 
-		try {
-			$result = [
-				'steps' => $this->documentService->getSteps($documentId, $version),
-				'sessions' => $this->sessionService->getAllSessions($documentId),
-				'document' => $this->documentService->get($documentId)
-			];
+		$result = [
+			'steps' => $this->documentService->getSteps($documentId, $version),
+			'sessions' => $this->sessionService->getAllSessions($documentId),
+		];
+		return new DataResponse($result, 200);
+	}
+
+	public function save($documentId, $sessionId, $sessionToken, $version = 0, $autosaveContent = null, $documentState = null, bool $force = false, bool $manualSave = false, $token = null): DataResponse {
+		$file = null;
+		if (!$this->sessionService->isValidSession($documentId, $sessionId, $sessionToken)) {
+			return new DataResponse([], 403);
+		}
 
+		try {
 			$session = $this->sessionService->getSession($documentId, $sessionId, $sessionToken);
 			$file = $this->documentService->getFileForSession($session, $token);
 		} catch (NotFoundException $e) {
@@ -237,10 +244,12 @@ public function sync($documentId, $sessionId, $sessionToken, $version = 0, $auto
 		}
 
 		try {
-			$result['document'] = $this->documentService->autosave($file, $documentId, $version, $autosaveContent, $documentState, $force, $manualSave, $token, $this->request->getParam('filePath'));
+			$document = $this->documentService->autosave($file, $documentId, $version, $autosaveContent, $documentState, $force, $manualSave, $token, $this->request->getParam('filePath'));
 		} catch (DocumentSaveConflictException $e) {
 			try {
-				$result['outsideChange'] = $file->getContent();
+				return new DataResponse([
+					'outsideChange' => $file->getContent()
+				], 409);
 			} catch (LockedException $e) {
 				// Ignore locked exception since it might happen due to an autosave action happening at the same time
 			}
@@ -252,8 +261,9 @@ public function sync($documentId, $sessionId, $sessionToken, $version = 0, $auto
 				'message' => 'Failed to autosave document'
 			], 500);
 		}
-
-		return new DataResponse($result, isset($result['outsideChange']) ? 409 : 200);
+		return new DataResponse([
+			'document' => $document
+		]);
 	}
 
 	/**

lib/Service/DocumentService.php

@@ -171,16 +171,15 @@ public function get($documentId) {
 	 * @param $documentId
 	 * @param $sessionId
 	 * @param $steps
-	 * @param $version
 	 * @return array
 	 * @throws DoesNotExistException
 	 * @throws InvalidArgumentException
 	 */
-	public function addStep($documentId, $sessionId, $steps, $version): array {
+	public function addStep($documentId, $sessionId, $steps): array {
 		$stepsToInsert = [];
 		$querySteps = [];
 		$getStepsSinceVersion = null;
-		$newVersion = $version;
+		$newVersion = 0;
 		foreach ($steps as $step) {
 			// Steps are base64 encoded messages of the yjs protocols
 			// https://github.com/yjs/y-protocols
@@ -193,17 +192,13 @@ public function addStep($documentId, $sessionId, $steps, $version): array {
 			}
 		}
 		if (sizeof($stepsToInsert) > 0) {
-			$newVersion = $this->insertSteps($documentId, $sessionId, $stepsToInsert, $version);
+			$newVersion = $this->insertSteps($documentId, $sessionId, $stepsToInsert);
 		}
 		// If there were any queries in the steps send the entire history
-		$getStepsSinceVersion = count($querySteps) > 0 ? 0 : $version;
-		$allSteps = $this->getSteps($documentId, $getStepsSinceVersion);
-		$stepsToReturn = [];
-		foreach ($allSteps as $step) {
-			if ($step < "AAQ") {
-				array_push($stepsToReturn, $step);
-			}
-		}
+		// TODO: make use of the saved document state here - otherwise it migth be too big.
+		$stepsToReturn = count($querySteps) === 0
+			? []
+			: $this->getSteps($documentId, 0);
 		return [
 			'steps' => $stepsToReturn,
 			'version' => $newVersion
@@ -214,12 +209,11 @@ public function addStep($documentId, $sessionId, $steps, $version): array {
 	 * @param $documentId
 	 * @param $sessionId
 	 * @param $steps
-	 * @param $version
 	 * @return int
 	 * @throws DoesNotExistException
 	 * @throws InvalidArgumentException
 	 */
-	private function insertSteps($documentId, $sessionId, $steps, $version): int {
+	private function insertSteps($documentId, $sessionId, $steps): int {
 		$document = null;
 		$stepsVersion = null;
 		try {

src/components/Editor.vue

@@ -241,6 +241,7 @@ export default {
 			forceRecreate: false,
 			menubarLoaded: false,
 			draggedOver: false,
+			version: 0,
 
 			contentWrapper: null,
 		}
@@ -421,15 +422,15 @@ export default {
 		},
 
 		resolveUseThisVersion() {
-			this.$syncService.forceSave()
+			this.$syncService.forceSave(this.version)
 			this.$editor.setOptions({ editable: !this.readOnly })
 		},
 
 		resolveUseServerVersion() {
 			const markdownItHtml = markdownit.render(this.syncError.data.outsideChange)
 			this.$editor.setOptions({ editable: !this.readOnly })
 			this.$editor.commands.setContent(markdownItHtml)
-			this.$syncService.forceSave()
+			this.$syncService.forceSave(this.version)
 		},
 
 		reconnect() {
@@ -527,7 +528,7 @@ export default {
 							}),
 							Keymap.configure({
 								'Mod-s': () => {
-									this.$syncService.save()
+									this.$syncService.save(this.version)
 									return true
 								},
 							}),
@@ -544,16 +545,22 @@ export default {
 
 		},
 
+		// TODO: split up in two handlers:
+		// - one that gets the session from PollingBackend
+		// - one that gets the document from SyncService
 		onChange({ document, sessions }) {
-			if (this.document.baseVersionEtag !== '' && document.baseVersionEtag !== this.document.baseVersionEtag) {
-				this.resolveUseServerVersion()
-				return
+			if (document) {
+				if (this.document.baseVersionEtag !== '' && document.baseVersionEtag !== this.document.baseVersionEtag) {
+					this.resolveUseServerVersion()
+					return
+				}
+				this.document = document
+				this.$editor.setOptions({ editable: !this.readOnly })
+			}
+			if (sessions) {
+				this.updateSessions.bind(this)(sessions)
+				this.syncError = null
 			}
-			this.updateSessions.bind(this)(sessions)
-			this.document = document
-
-			this.syncError = null
-			this.$editor.setOptions({ editable: !this.readOnly })
 		},
 
 		onSync({ steps, document }) {
@@ -619,7 +626,7 @@ export default {
 				) {
 					this.dirty = state.dirty
 					if (this.dirty) {
-						this.$syncService.autosave()
+						this.$syncService.autosave(this.version)
 					}
 				}
 			}