From 476c18f0ff650c661379e6247b579e290be684fc Mon Sep 17 00:00:00 2001 From: korelstar Date: Thu, 20 May 2021 19:39:22 +0200 Subject: [PATCH] API: allow requesting notes list in chunks --- docs/api/v1.md | 13 +++-- lib/Controller/ChunkCursor.php | 41 +++++++++++++ lib/Controller/Helper.php | 63 +++++++++++++++----- lib/Controller/NotesApiController.php | 40 ++++++++++--- lib/Controller/NotesController.php | 16 +++--- lib/Service/MetaNote.php | 19 ++++++ lib/Service/MetaService.php | 12 ++-- lib/Service/NotesService.php | 10 ++-- src/NotesService.js | 4 +- src/store/notes.js | 11 +--- tests/api/APIv1Test.php | 83 +++++++++++++++++++++++++++ tests/api/AbstractAPITest.php | 11 ++++ 12 files changed, 265 insertions(+), 58 deletions(-) create mode 100644 lib/Controller/ChunkCursor.php create mode 100644 lib/Service/MetaNote.php diff --git a/docs/api/v1.md b/docs/api/v1.md index 70a34916..08ec5892 100644 --- a/docs/api/v1.md +++ b/docs/api/v1.md @@ -18,7 +18,7 @@ In this document, the Notes API major version 1 and all its minor versions are d The app and the API is mainly about notes. So, let's have a look about the attributes of a note. The description of endpoints and operations will refer to this attribute definition. | Attribute | Type | Description | since API version | -|:----------|:-----|:-------------------------|:-------------------| +|:----------|:-----|:------------|:------------------| | `id` | integer (read‑only) | Every note has a unique identifier which is created by the server. It can be used to query and update a specific note. | 1.0 | | `etag` | string (read‑only) | The note's entity tag (ETag) indicates if a note's attribute has changed. I.e., if the note changes, the ETag changes, too. Clients can use the ETag for detecting if the local note has to be updated from server and for optimistic concurrency control (see section [Preventing lost updates and conflict solution](#preventing-lost-updates-and-conflict-solution)). | 1.2 | | `readonly` | boolean (read‑only) | Indicates if the note is read-only. This is `true`, e.g., if a file or folder was shared by another user without allowing editing. If this attribute is `true`, then all read/write attributes become read-only; except for the `favorite` attribute. | 1.2 | @@ -56,15 +56,20 @@ All defined routes in the specification are appended to this url. To access all #### Request parameters | Parameter | Type | Description | since API version | -|:------|:-----|:-----|:-----| -| `category` | string, optional | Filter the result by category name, e.g. `?category=recipes`. Notes with another category are not included in the result. *Compatibility note:* in API v1.0, this parameter is ignored; i.e., the result contains all notes regardless of this parameter. | 1.1 | +|:----------|:-----|:------------|:------------------| +| `category` | string, optional | Filter the result by category name, e.g. `?category=recipes`. Notes with another category are not included in the result. *Compatibility note:* before API v1.1, this parameter is ignored; i.e., the result contains all notes regardless of this parameter. | 1.1 | | `exclude` | string, optional | Fields which should be excluded from response, seperated with a comma e.g.: `?exclude=content,title`. You can use this in order to reduce transferred data size if you are interested in specific attributes, only. | 1.0 | | `pruneBefore` | integer, optional | All notes without change before of this Unix timestamp are purged from the response, i.e. only the attribute `id` is included. You should use the Unix timestamp value from the last request's HTTP response header `Last-Modified` in order to reduce transferred data size. | 1.0 | +| `chunkSize` | integer, optional | The response will contain no more than the given number of full notes. If there are more notes, then the result is chunked and the HTTP response header `X-Notes-Chunk-Cursor` is sent with a string value. In order to request the next chunk, a new request have to be made with parameter `chunkCursor` filled with that string value. *Compatibility note:* before API v1.2, this parameter is ignored; i.e., the result contains all notes regardless of this parameter. | 1.2 | +| `chunkCursor` | string, optional | To be used together with the parameter `chunkSize`. You must use the string value from the last request's HTTP response header `X-Notes-Chunk-Cursor` in order to get the next chunk of notes. Don't use this parameter for requesting the first chunk. *Compatibility note:* before API v1.2, this parameter is ignored; i.e., the result contains all notes regardless of this parameter. | 1.2 | | `If-None-Match` | HTTP header, optional | Use this in order to reduce transferred data size (see [HTTP ETag](https://en.wikipedia.org/wiki/HTTP_ETag)). You should use the value from the last request's HTTP response header `ETag`. | 1.0 | #### Response ##### 200 OK -- **HTTP Header**: `ETag` (see [HTTP ETag](https://en.wikipedia.org/wiki/HTTP_ETag)). +- **HTTP Header**: + - `ETag` (see [HTTP ETag](https://en.wikipedia.org/wiki/HTTP_ETag)). + - `X-Notes-Chunk-Cursor`: Only if `chunkSize` is provided and not `0` and if the response does not contain all remaining notes. In this case, the response does not contain pruned notes. In order to get the next chunk, you will have to make a new request and use this header value as request parameter `chunkCursor`. The last chunk response will not contain this header but it will contain all pruned notes. In summary: a client have to repeatedly request the notes list from server with the desired `chunkSize` and with updated `chunkCursor` until the response does not contain any `X-Notes-Chunk-Cursor` HTTP header – only this last request can be used to check for deleted notes. + - `X-Notes-Chunk-Pending`: number of pending notes that have to be requested using the chunk cursor provided in the HTTP response header `X-Notes-Chunk-Cursor`. - **Body**: list of notes (see section [Note attributes](#note-attributes)), example: ```js [ diff --git a/lib/Controller/ChunkCursor.php b/lib/Controller/ChunkCursor.php new file mode 100644 index 00000000..d46b00a3 --- /dev/null +++ b/lib/Controller/ChunkCursor.php @@ -0,0 +1,41 @@ +timeStart = new \DateTime(); + $cc->timeStart->setTimestamp((int)$matches[1]); + $cc->noteLastUpdate = (int)$matches[2]; + $cc->noteId = (int)$matches[3]; + return $cc; + } else { + return null; + } + } + + public static function fromNote(\DateTime $timeStart, MetaNote $m) : ChunkCursor { + $cc = new static(); + $cc->timeStart = $timeStart; + $cc->noteLastUpdate = $m->meta->getLastUpdate(); + $cc->noteId = $m->note->getId(); + return $cc; + } + + public function toString() : string { + return $this->timeStart->getTimestamp() . '-' . $this->noteLastUpdate . '-' . $this->noteId; + } +} diff --git a/lib/Controller/Helper.php b/lib/Controller/Helper.php index b72839b0..d2f5640e 100644 --- a/lib/Controller/Helper.php +++ b/lib/Controller/Helper.php @@ -8,6 +8,7 @@ use OCA\Notes\AppInfo\Application; use OCA\Notes\Db\Meta; use OCA\Notes\Service\Note; use OCA\Notes\Service\NotesService; +use OCA\Notes\Service\MetaNote; use OCA\Notes\Service\MetaService; use OCA\Notes\Service\Util; @@ -71,28 +72,60 @@ class Helper { public function getNotesAndCategories( int $pruneBefore, array $exclude, - string $category = null + string $category = null, + int $chunkSize = 0, + string $chunkCursorStr = null ) : array { $userId = $this->getUID(); + $chunkCursor = $chunkCursorStr ? ChunkCursor::fromString($chunkCursorStr) : null; + $lastUpdate = $chunkCursor->timeStart ?? new \DateTime(); $data = $this->notesService->getAll($userId); - $notes = $data['notes']; - $metas = $this->metaService->updateAll($userId, $notes); + $metaNotes = $this->metaService->getAll($userId, $data['notes']); + + // if a category is requested, then ignore all other notes if ($category !== null) { - $notes = array_values(array_filter($notes, function ($note) use ($category) { - return $note->getCategory() === $category; - })); + $metaNotes = array_filter($metaNotes, function (MetaNote $m) use ($category) { + return $m->note->getCategory() === $category; + }); } - $notesData = array_map(function ($note) use ($metas, $pruneBefore, $exclude) { - $meta = $metas[$note->getId()]; - if ($pruneBefore && $meta->getLastUpdate() < $pruneBefore) { - return [ 'id' => $note->getId() ]; - } else { - return $this->getNoteData($note, $exclude, $meta); - } - }, $notes); + + // list of notes that should be sent to the client + $fullNotes = array_filter($metaNotes, function (MetaNote $m) use ($pruneBefore, $chunkCursor) { + $isPruned = $pruneBefore && $m->meta->getLastUpdate() < $pruneBefore; + $noteLastUpdate = (int)$m->meta->getLastUpdate(); + $isBeforeCursor = $chunkCursor && ( + $noteLastUpdate < $chunkCursor->noteLastUpdate + || ($noteLastUpdate === $chunkCursor->noteLastUpdate + && $m->note->getId() <= $chunkCursor->noteId) + ); + return !$isPruned && !$isBeforeCursor; + }); + + // sort the list for slicing the next chunk + uasort($fullNotes, function (MetaNote $a, MetaNote $b) { + return $a->meta->getLastUpdate() <=> $b->meta->getLastUpdate() + ?: $a->note->getId() <=> $b->note->getId(); + }); + + // slice the next chunk + $chunkedNotes = $chunkSize ? array_slice($fullNotes, 0, $chunkSize, true) : $fullNotes; + $numPendingNotes = count($fullNotes) - count($chunkedNotes); + + // if the chunk does not contain all remaining notes, then generate new chunk cursor + $newChunkCursor = $numPendingNotes ? ChunkCursor::fromNote($lastUpdate, end($chunkedNotes)) : null; + + // load data for the current chunk + $notesData = array_map(function (MetaNote $m) use ($exclude) { + return $this->getNoteData($m->note, $exclude, $m->meta); + }, $chunkedNotes); + return [ - 'notes' => $notesData, 'categories' => $data['categories'], + 'notesAll' => $metaNotes, + 'notesData' => $notesData, + 'lastUpdate' => $lastUpdate, + 'chunkCursor' => $newChunkCursor, + 'numPendingNotes' => $numPendingNotes, ]; } diff --git a/lib/Controller/NotesApiController.php b/lib/Controller/NotesApiController.php index 7f1e278e..6928603b 100644 --- a/lib/Controller/NotesApiController.php +++ b/lib/Controller/NotesApiController.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace OCA\Notes\Controller; use OCA\Notes\Service\NotesService; +use OCA\Notes\Service\MetaNote; use OCA\Notes\Service\MetaService; use OCA\Notes\Service\SettingsService; @@ -45,16 +46,37 @@ class NotesApiController extends ApiController { * @CORS * @NoCSRFRequired */ - public function index(?string $category = null, string $exclude = '', int $pruneBefore = 0) : JSONResponse { - return $this->helper->handleErrorResponse(function () use ($category, $exclude, $pruneBefore) { + public function index( + ?string $category = null, + string $exclude = '', + int $pruneBefore = 0, + int $chunkSize = 0, + string $chunkCursor = null + ) : JSONResponse { + return $this->helper->handleErrorResponse(function () use ( + $category, + $exclude, + $pruneBefore, + $chunkSize, + $chunkCursor + ) { $exclude = explode(',', $exclude); - $now = new \DateTime(); // this must be before loading notes if there are concurrent changes possible - $data = $this->helper->getNotesAndCategories($pruneBefore, $exclude, $category); - $notesData = $data['notes']; - $etag = md5(json_encode($notesData)); - return (new JSONResponse($notesData)) - ->setLastModified($now) - ->setETag($etag); + $data = $this->helper->getNotesAndCategories($pruneBefore, $exclude, $category, $chunkSize, $chunkCursor); + $notesData = $data['notesData']; + if (!$data['chunkCursor']) { + // if last chunk, then send all notes (pruned) + $notesData += array_map(function (MetaNote $m) { + return [ 'id' => $m->note->getId() ]; + }, $data['notesAll']); + } + $response = new JSONResponse(array_values($notesData)); + $response->setLastModified($data['lastUpdate']); + $response->setETag(md5(json_encode($notesData))); + if ($data['chunkCursor']) { + $response->addHeader('X-Notes-Chunk-Cursor', $data['chunkCursor']->toString()); + $response->addHeader('X-Notes-Chunk-Pending', $data['numPendingNotes']); + } + return $response; }); } diff --git a/lib/Controller/NotesController.php b/lib/Controller/NotesController.php index 22af341e..5315dcb6 100644 --- a/lib/Controller/NotesController.php +++ b/lib/Controller/NotesController.php @@ -55,7 +55,6 @@ class NotesController extends Controller { public function index(int $pruneBefore = 0) : JSONResponse { return $this->helper->handleErrorResponse(function () use ($pruneBefore) { $userId = $this->helper->getUID(); - $now = new \DateTime(); // this must be before loading notes if there are concurrent changes possible $settings = $this->settingsService->getAll($userId); $lastViewedNote = (int) $this->settings->getUserValue( @@ -64,32 +63,33 @@ class NotesController extends Controller { 'notesLastViewedNote' ); $errorMessage = null; - $notes = null; - $categories = null; + $nac = null; try { $nac = $this->helper->getNotesAndCategories($pruneBefore, [ 'etag', 'content' ]); - [ 'notes' => $notes, 'categories' => $categories ] = $nac; } catch (\Throwable $e) { $this->helper->logException($e); $errorMessage = $this->l10n->t('Reading notes from filesystem has failed.').' ('.get_class($e).')'; } - if ($errorMessage === null && $lastViewedNote && is_array($notes) && !count($notes)) { + if ($errorMessage === null && $lastViewedNote + && is_array($nac) && is_array($nac['notesAll']) && !count($nac['notesAll']) + ) { $this->settings->deleteUserValue($userId, $this->appName, 'notesLastViewedNote'); $lastViewedNote = 0; } $result = [ - 'notes' => $notes, - 'categories' => $categories, + 'notesData' => array_values($nac['notesData'] ?? null), + 'noteIds' => array_keys($nac['notesAll']), + 'categories' => $nac['categories'] ?? null, 'settings' => $settings, 'lastViewedNote' => $lastViewedNote, 'errorMessage' => $errorMessage, ]; $etag = md5(json_encode($result)); return (new JSONResponse($result)) - ->setLastModified($now) + ->setLastModified($nac['lastUpdate'] ?? null) ->setETag($etag) ; }); diff --git a/lib/Service/MetaNote.php b/lib/Service/MetaNote.php new file mode 100644 index 00000000..848bfb0d --- /dev/null +++ b/lib/Service/MetaNote.php @@ -0,0 +1,19 @@ +note = $note; + $this->meta = $meta; + } +} diff --git a/lib/Service/MetaService.php b/lib/Service/MetaService.php index 34fb46fe..7a27fe51 100644 --- a/lib/Service/MetaService.php +++ b/lib/Service/MetaService.php @@ -26,7 +26,7 @@ use OCA\Notes\Db\MetaMapper; * time and the next synchronization time. However, this is totally sufficient * for this purpose. * - * Therefore, on synchronization, the method `MetaService.updateAll` is called. + * Therefore, on synchronization, the method `MetaService.getAll` is called. * It generates an ETag for each note and compares it with the ETag from * `notes_meta` database table in order to detect changes (or creates an entry * if not existent). If there are changes, the ETag is updated and `LastUpdate` @@ -61,18 +61,17 @@ class MetaService { $this->metaMapper->deleteByNote($id); } - public function updateAll(string $userId, array $notes, bool $forceUpdate = false) : array { + public function getAll(string $userId, array $notes, bool $forceUpdate = false) : array { // load data $metas = $this->metaMapper->getAll($userId); $metas = $this->getIndexedArray($metas, 'fileId'); - $notes = $this->getIndexedArray($notes, 'id'); + $result = []; // delete obsolete notes foreach ($metas as $id => $meta) { if (!array_key_exists($id, $notes)) { // DELETE obsolete notes $this->metaMapper->delete($meta); - unset($metas[$id]); } } @@ -80,7 +79,7 @@ class MetaService { foreach ($notes as $id => $note) { if (!array_key_exists($id, $metas)) { // INSERT new notes - $metas[$note->getId()] = $this->createMeta($userId, $note); + $meta = $this->createMeta($userId, $note); } else { // UPDATE changed notes $meta = $metas[$id]; @@ -88,8 +87,9 @@ class MetaService { $this->metaMapper->update($meta); } } + $result[$id] = new MetaNote($note, $meta); } - return $metas; + return $result; } public function update(string $userId, Note $note) : Meta { diff --git a/lib/Service/NotesService.php b/lib/Service/NotesService.php index ef7aef37..267c22bb 100644 --- a/lib/Service/NotesService.php +++ b/lib/Service/NotesService.php @@ -26,9 +26,7 @@ class NotesService { public function getAll(string $userId) : array { $notesFolder = $this->getNotesFolder($userId); $data = $this->gatherNoteFiles($notesFolder); - $fileIds = array_map(function (File $file) : int { - return $file->getId(); - }, $data['files']); + $fileIds = array_keys($data['files']); // pre-load tags for all notes (performance improvement) $this->noteUtil->getTagService()->loadTags($fileIds); $notes = array_map(function (File $file) use ($notesFolder) : Note { @@ -159,10 +157,10 @@ class NotesService { $subCategory = $categoryPrefix . $node->getName(); $data['categories'][] = $subCategory; $data_sub = self::gatherNoteFiles($node, $subCategory . '/'); - $data['files'] = array_merge($data['files'], $data_sub['files']); - $data['categories'] = array_merge($data['categories'], $data_sub['categories']); + $data['files'] = $data['files'] + $data_sub['files']; + $data['categories'] = $data['categories'] + $data_sub['categories']; } elseif (self::isNote($node)) { - $data['files'][] = $node; + $data['files'][$node->getId()] = $node; } } return $data; diff --git a/src/NotesService.js b/src/NotesService.js index d220b4c5..4d166cf5 100644 --- a/src/NotesService.js +++ b/src/NotesService.js @@ -74,8 +74,8 @@ export const fetchNotes = () => { .then(response => { store.commit('setSettings', response.data.settings) store.commit('setCategories', response.data.categories) - if (response.data.notes !== null) { - store.dispatch('updateNotes', response.data.notes) + if (response.data.noteIds !== null) { + store.dispatch('updateNotes', { noteIds: response.data.noteIds, notes: response.data.notesData }) } if (response.data.errorMessage) { showError(t('notes', 'Error from Nextcloud server: {msg}', { msg: response.data.errorMessage })) diff --git a/src/store/notes.js b/src/store/notes.js index a1a9f452..7d2897eb 100644 --- a/src/store/notes.js +++ b/src/store/notes.js @@ -123,20 +123,15 @@ const mutations = { } const actions = { - updateNotes(context, notes) { - const noteIds = {} + updateNotes(context, { noteIds, notes }) { // add/update new notes for (const note of notes) { - noteIds[note.id] = true // TODO check for parallel (local) changes! - // only update, if note has changes (see API "pruneBefore") - if (note.title !== undefined) { - context.commit('updateNote', note) - } + context.commit('updateNote', note) } // remove deleted notes context.state.notes.forEach(note => { - if (noteIds[note.id] === undefined) { + if (!noteIds.includes(note.id)) { context.commit('removeNote', note.id) } }) diff --git a/tests/api/APIv1Test.php b/tests/api/APIv1Test.php index d60cf0a6..c9e40268 100644 --- a/tests/api/APIv1Test.php +++ b/tests/api/APIv1Test.php @@ -70,6 +70,89 @@ class APIv1Test extends CommonAPITest { ); } + protected function checkGetChunkNotes( + array $indexedRefNotes, + int $chunkSize, + string $messagePrefix, + string $chunkCursor = null, + array $collectedNotes = [] + ) : array { + $requestCount = 0; + $previousChunkCursor = null; + do { + $requestCount++; + $previousChunkCursor = $chunkCursor; + $query = '?chunkSize='.$chunkSize; + if ($chunkCursor) { + $query .= '&chunkCursor='.$chunkCursor; + } + $response = $this->http->request('GET', 'notes'.$query); + $chunkCursor = $response->getHeaderLine('X-Notes-Chunk-Cursor'); + $this->checkResponse($response, $messagePrefix.'Check response '.$requestCount, 200); + $notes = json_decode($response->getBody()->getContents()); + if ($chunkCursor) { + $this->assertIsArray($notes, $messagePrefix.'Response '.$requestCount); + $this->assertLessThanOrEqual( + $chunkSize, + count($notes), + $messagePrefix.'Notes of response '.$requestCount + ); + foreach ($notes as $note) { + $this->assertArrayNotHasKey( + $note->id, + $collectedNotes, + $messagePrefix.'Note ID of response '.$requestCount.' in collectedNotes' + ); + $this->assertArrayHasKey( + $note->id, + $indexedRefNotes, + $messagePrefix.'Note ID of response '.$requestCount.' in refNotes' + ); + $this->checkReferenceNote( + $indexedRefNotes[$note->id], + $note, + $messagePrefix.'Note in response '.$requestCount + ); + $collectedNotes[$note->id] = $note; + } + } else { + $leftIds = array_diff(array_keys($indexedRefNotes), array_keys($collectedNotes)); + $this->checkReferenceNotes( + $indexedRefNotes, + $notes, + $messagePrefix.'Notes of response '.$requestCount, + [], + $leftIds + ); + } + } while ($chunkCursor && $requestCount < 100); + $this->assertEmpty($chunkCursor, $messagePrefix.'Last response Chunk Cursor'); + return [ + 'previousChunkCursor' => $previousChunkCursor, + 'collectedNotes' => $collectedNotes, + ]; + } + + /** + * @depends testCheckForReferenceNotes + * @depends testCreateNotes + */ + public function testGetChunkedNotes(array $refNotes, array $testNotes) : void { + sleep(1); // wait for 'Last-Modified' to be >= Last-change + 1 + $indexedRefNotes = $this->getNotesIdMap(array_merge($refNotes, $testNotes), 'RefNotes'); + $l = $this->checkGetChunkNotes($indexedRefNotes, 2, 'Test1: '); + + $note = $testNotes[0]; + $rn1 = $this->updateNote($note, (object)[ + 'category' => 'ChunkedNote', + ], (object)[]); + + $collectedNotes = $l['collectedNotes']; + $this->assertArrayHasKey($note->id, $collectedNotes, 'Updated note is not in last chunk.'); + unset($collectedNotes[$note->id]); + $this->checkGetChunkNotes($indexedRefNotes, 2, 'Test2: ', $l['previousChunkCursor'], $collectedNotes); + } + public function testGetSettings() : \stdClass { $response = $this->http->request('GET', 'settings'); $this->checkResponse($response, 'Get settings', 200); diff --git a/tests/api/AbstractAPITest.php b/tests/api/AbstractAPITest.php index fbf04478..ceb35074 100644 --- a/tests/api/AbstractAPITest.php +++ b/tests/api/AbstractAPITest.php @@ -75,6 +75,17 @@ abstract class AbstractAPITest extends TestCase { $response = $this->http->request('GET', 'notes' . $param); $this->checkResponse($response, $messagePrefix, 200); $notes = json_decode($response->getBody()->getContents()); + $this->checkReferenceNotes($refNotes, $notes, $messagePrefix, $expectExclude, $expectedFullNotes); + } + + protected function checkReferenceNotes( + array $refNotes, + array $notes, + string $messagePrefix, + array $expectExclude = [], + array $expectedFullNotes = null + ) : void { + $this->assertIsArray($notes, $messagePrefix); $notesMap = $this->getNotesIdMap($notes, $messagePrefix); $this->assertEquals(count($refNotes), count($notes), $messagePrefix.': Number of notes'); foreach ($refNotes as $refNote) {