Skip to content

Commit

Permalink
enh: save api request separate from sync
Browse files Browse the repository at this point in the history 
Save will only save the doc and doc state.
Sync will only fetch steps from the server.

Signed-off-by: Max <max@nextcloud.com>
  • Loading branch information
@max-nextcloud
max-nextcloud committed Feb 20, 2023
1 parent 4f0ec37 commit 742edb3
Show file tree
Hide file tree
Showing 12 changed files with 143 additions and 99 deletions.
2 changes: 2 additions & 0 deletions appinfo/routes.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'],

Expand Down
12 changes: 7 additions & 5 deletions cypress/e2e/SessionApi.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
})
Expand Down Expand Up @@ -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')
Expand All @@ -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,
Expand Down Expand Up @@ -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')
Expand All @@ -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,
Expand Down
16 changes: 10 additions & 6 deletions cypress/e2e/sync.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
5 changes: 5 additions & 0 deletions cypress/support/sessions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
16 changes: 12 additions & 4 deletions lib/Controller/PublicSessionController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
17 changes: 13 additions & 4 deletions lib/Controller/SessionController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
36 changes: 23 additions & 13 deletions lib/Service/ApiService.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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) {
Expand All @@ -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
}
Expand All @@ -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
]);
}

/**
Expand Down
22 changes: 8 additions & 14 deletions lib/Service/DocumentService.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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 {
Expand Down
31 changes: 19 additions & 12 deletions src/components/Editor.vue
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ export default {
forceRecreate: false,
menubarLoaded: false,
draggedOver: false,
version: 0,

contentWrapper: null,
}
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -527,7 +528,7 @@ export default {
}),
Keymap.configure({
'Mod-s': () => {
this.$syncService.save()
this.$syncService.save(this.version)
return true
},
}),
Expand All @@ -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 }) {
Expand Down Expand Up @@ -619,7 +626,7 @@ export default {
) {
this.dirty = state.dirty
if (this.dirty) {
this.$syncService.autosave()
this.$syncService.autosave(this.version)
}
}
}
Expand Down
Loading

0 comments on commit 742edb3

Please sign in to comment.