Prevent lost updates by using ETags for notes

docs/api/v1.md

@@ -19,12 +19,13 @@ The app and the API is mainly about notes. So, let's have a look about the attri
 
 | Attribute | Type | Description | since API version |
 |:----------|:-----|:-------------------------|:-------------------|
-| `id` | integer | 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 |
-| `content` | string | Notes can contain arbitrary text. Formatting should be done using Markdown, but not every markup can be supported by every client. Therefore, markup should be used with care. | 1.0 |
-| `title` | string | The note's title is also used as filename for the note's file. Therefore, some special characters are automatically removed and a sequential number is added if a note with the same title in the same category exists. When saving a title, the sanitized value is returned and should be adopted by your client. | 1.0 |
-| `category` | string | Every note is assigned to a category. By default, the category is an empty string (not null), which means the note is uncategorized. Categories are mapped to folders in the file backend. Illegal characters are automatically removed and the respective folder is automatically created. Sub-categories (mapped to sub-folders) can be created by using `/` as delimiter. | 1.0 |
-| `favorite` | boolean | If a note is marked as favorite, it is displayed at the top of the notes' list. Default is `false`. | 1.0 |
-| `modified` | integer | Unix timestamp for the last modified date/time of the note. If not provided on note creation or content update, the current time is used. | 1.0 |
+| `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 |
+| `content` | string (read/write) | Notes can contain arbitrary text. Formatting should be done using Markdown, but not every markup can be supported by every client. Therefore, markup should be used with care. | 1.0 |
+| `title` | string (read/write) | The note's title is also used as filename for the note's file. Therefore, some special characters are automatically removed and a sequential number is added if a note with the same title in the same category exists. When saving a title, the sanitized value is returned and should be adopted by your client. | 1.0 |
+| `category` | string (read/write) | Every note is assigned to a category. By default, the category is an empty string (not null), which means the note is uncategorized. Categories are mapped to folders in the file backend. Illegal characters are automatically removed and the respective folder is automatically created. Sub-categories (mapped to sub-folders) can be created by using `/` as delimiter. | 1.0 |
+| `favorite` | boolean (read/write) | If a note is marked as favorite, it is displayed at the top of the notes' list. Default is `false`. | 1.0 |
+| `modified` | integer (read/write) | Unix timestamp for the last modified date/time of the note. If not provided on note creation or content update, the current time is used. | 1.0 |
 
 
 ## Settings
@@ -68,6 +69,7 @@ All defined routes in the specification are appended to this url. To access all
 [
     {
         "id": 76,
+        "etag": "be284e00488c61c101ee28309d235e0b",
         "modified": 1376753464,
         "title": "New note",
         "category": "sub-directory",
@@ -89,13 +91,16 @@ No valid authentication credentials supplied.
 | Parameter | Type | Description |
 |:------|:-----|:-----|
 | `id` | integer, required (path) | ID of the note to query. |
+| `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 note's attribute `etag` or from the last request's HTTP response header `ETag`. | 1.2 |
 
 #### Response
 ##### 200 OK
+- **HTTP Header**: `ETag` (see [HTTP ETag](https://en.wikipedia.org/wiki/HTTP_ETag)). The value is identical to the note's attribute `etag` (see section [Note attributes](#note-attributes)).
 - **Body**: note (see section [Note attributes](#note-attributes)), example:
 ```js
 {
     "id": 76,
+    "etag": "be284e00488c61c101ee28309d235e0b",
     "modified": 1376753464,
     "title": "New note",
     "category": "sub-directory",
@@ -118,7 +123,7 @@ Note not found.
 <details><summary>Details</summary>
 
 #### Request parameters
-- **Body**: See section [Note attributes](#note-attributes) (except for `id`).  All attributes are optional. Example: 
+- **Body**: some or all "read/write" attributes (see section [Note attributes](#note-attributes)), example: 
 ```js
 {
     "title": "New note",
@@ -149,7 +154,8 @@ Not enough free storage for saving the note's content.
 | Parameter | Type | Description |
 |:------|:-----|:-----|
 | `id` | integer, required (path) | ID of the note to update. |
-- **Body**: See section [Note attributes](#note-attributes) (except for `id`).  All attributes are optional. Example see section [Create note](#create-note-post-notes).
+| `If-Match` | HTTP header, optional | Use this for optimistic concurrency control (optional, but strongly recommended in order to prevent lost updates). As value of this HTTP header, the client has to use the last known note's etag (see section [Note attributes](#note-attributes)). If the note has changed in the meanwhile (concurrent change), the update request is blocked with HTTP status 412 (see below). Otherwise, the request will be processed normally. | 1.2 |
+- **Body**: some or all "read/write" attributes (see section [Note attributes](#note-attributes)), example see section [Create note](#create-note-post-notes).
 
 #### Response
 ##### 200 OK
@@ -164,6 +170,11 @@ No valid authentication credentials supplied.
 ##### 404 Not Found
 Note not found.
 
+##### 412 Precondition Failed
+*(since API v1.2)*
+Update cannot be performed since the note has been changed on the server in the meanwhile (concurrent change). The body contains the current note's state from server (see section [Note attributes](#note-attributes)), example see section [Get single note](#get-single-note-get-notesid). The client should use this response data in order to perform a conflict solution (see section [Preventing lost updates and conflict solution](#preventing-lost-updates-and-conflict-solution)).
+
+
 ##### 507 Insufficient Storage
 Not enough free storage for saving the note's content.
 </details>
@@ -246,3 +257,26 @@ Endpoint not supported by installed notes app version (requires API version 1.2)
 ##### 401 Unauthorized
 No valid authentication credentials supplied.
 </details>
+
+
+
+## Preventing lost updates and conflict solution
+
+While changing a note using a Notes client, the same note may be changed by another client.
+In order to prevent lost updates of those concurrent changes, the notes API uses a well established mechanism called [optimistic concurrency control](https://en.wikipedia.org/wiki/Optimistic_concurrency_control).
+For this purpose, notes have the attribute `etag` which is an identifier that changes if (and only if) the note changes on the server.
+Clients have to store the `etag` for every note and send its value with every update request (HTTP header `If-Match`, see section [Update note](#update-note-put-notesid)).
+If there was no parallel change on the server (i.e., the `etag` on server is the same as the one send from the client), the update request is performed as usual.
+But if there was a parallel change, the `etag` on the server has changed and the server will refuse the update request.
+
+In this case, the client has to perform a conflict resolution, i.e. the local changes have to be merged with the remote changes.
+In order to compare local changes with remote changes, it is useful that the client stores the full note's state as reference state before performing any local updates.
+If an update conflict occurs, the client can use this reference state in order to merge all changes attribute-wise:
+- Attributes, that have changed only locally or remotely, can be merged by picking the (local resp. remote) change.
+- Attributes, that have changed both localy and remotely, have to be merged (see below).
+
+There are several options on how to merge an attribute:
+- a) *Let the user decide*: ask the user whether i) overwrite local changes, ii) overwrite remote changes, or iii) save local (or remote) changes as new note.
+- b) *Let the user merge*: provide an interface which allows for merging the files (you know it from your version control).
+- c) *Try to merge automatically*: merge all changes automatically, e.g. for the `content` attribute using the [google-diff-match-patch](https://code.google.com/p/google-diff-match-patch/) ([Demo](https://neil.fraser.name/software/diff_match_patch/svn/trunk/demos/demo_patch.html), [Code](https://github.com/bystep15/google-diff-match-patch)) library.
+

lib/Controller/ETagDoesNotMatchException.php

@@ -0,0 +1,17 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OCA\Notes\Controller;
+
+use OCA\Notes\Service\Note;
+
+use Exception;
+
+class ETagDoesNotMatchException extends Exception {
+	public $note;
+
+	public function __construct(Note $note) {
+		$this->note = $note;
+	}
+}

lib/Controller/Helper.php

@@ -5,25 +5,38 @@
 namespace OCA\Notes\Controller;
 
 use OCA\Notes\AppInfo\Application;
+use OCA\Notes\Db\Meta;
+use OCA\Notes\Service\Note;
+use OCA\Notes\Service\NotesService;
+use OCA\Notes\Service\MetaService;
 use OCA\Notes\Service\Util;
 
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\JSONResponse;
+use OCP\IRequest;
 use OCP\IUserSession;
 
 use Psr\Log\LoggerInterface;
 
 class Helper {
 
+	/** @var NotesService */
+	private $notesService;
+	/** @var MetaService */
+	private $metaService;
 	/** @var LoggerInterface */
 	public $logger;
 	/** @var IUserSession */
 	private $userSession;
 
 	public function __construct(
+		NotesService $notesService,
+		MetaService $metaService,
 		IUserSession $userSession,
 		LoggerInterface $logger
 	) {
+		$this->notesService = $notesService;
+		$this->metaService = $metaService;
 		$this->userSession = $userSession;
 		$this->logger = $logger;
 	}
@@ -32,6 +45,57 @@ public function getUID() : string {
 		return $this->userSession->getUser()->getUID();
 	}
 
+	public function getNoteWithETagCheck(int $id, IRequest $request) : Note {
+		$userId = $this->getUID();
+		$note = $this->notesService->get($userId, $id);
+		$ifMatch = $request->getHeader('If-Match');
+		if ($ifMatch) {
+			$matchEtags = preg_split('/,\s*/', $ifMatch);
+			$meta = $this->metaService->update($userId, $note);
+			if (!in_array('"'.$meta->getEtag().'"', $matchEtags)) {
+				throw new ETagDoesNotMatchException($note);
+			}
+		}
+		return $note;
+	}
+
+	public function getNoteData(Note $note, array $exclude = [], Meta $meta = null) : array {
+		if ($meta === null) {
+			$meta = $this->metaService->update($this->getUID(), $note);
+		}
+		$data = $note->getData($exclude);
+		$data['etag'] = $meta->getEtag();
+		return $data;
+	}
+
+	public function getNotesAndCategories(
+		int $pruneBefore,
+		array $exclude,
+		string $category = null
+	) : array {
+		$userId = $this->getUID();
+		$data = $this->notesService->getAll($userId);
+		$notes = $data['notes'];
+		$metas = $this->metaService->updateAll($userId, $notes);
+		if ($category !== null) {
+			$notes = array_values(array_filter($notes, function ($note) use ($category) {
+				return $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);
+		return [
+			'notes' => $notesData,
+			'categories' => $data['categories'],
+		];
+	}
+
 	public function logException(\Throwable $e) : void {
 		$this->logger->error('Controller failed with '.get_class($e), [ 'exception' => $e ]);
 	}
@@ -47,6 +111,8 @@ public function handleErrorResponse(callable $respond) : JSONResponse {
 		try {
 			$data = Util::retryIfLocked($respond);
 			$response = $data instanceof JSONResponse ? $data : new JSONResponse($data);
+		} catch (\OCA\Notes\Controller\ETagDoesNotMatchException $e) {
+			$response = new JSONResponse($this->getNoteData($e->note), Http::STATUS_PRECONDITION_FAILED);
 		} catch (\OCA\Notes\Service\NoteDoesNotExistException $e) {
 			$this->logException($e);
 			$response = $this->createErrorResponse($e, Http::STATUS_NOT_FOUND);

lib/Controller/NotesApiController.php

@@ -49,21 +49,8 @@ public function index(?string $category = null, string $exclude = '', int $prune
 		return $this->helper->handleErrorResponse(function () use ($category, $exclude, $pruneBefore) {
 			$exclude = explode(',', $exclude);
 			$now = new \DateTime(); // this must be before loading notes if there are concurrent changes possible
-			$notes = $this->service->getAll($this->helper->getUID())['notes'];
-			$metas = $this->metaService->updateAll($this->helper->getUID(), $notes);
-			if ($category !== null) {
-				$notes = array_values(array_filter($notes, function ($note) use ($category) {
-					return $note->getCategory() === $category;
-				}));
-			}
-			$notesData = array_map(function ($note) use ($metas, $pruneBefore, $exclude) {
-				$lastUpdate = $metas[$note->getId()]->getLastUpdate();
-				if ($pruneBefore && $lastUpdate < $pruneBefore) {
-					return [ 'id' => $note->getId() ];
-				} else {
-					return $note->getData($exclude);
-				}
-			}, $notes);
+			$data = $this->helper->getNotesAndCategories($pruneBefore, $exclude, $category);
+			$notesData = $data['notes'];
 			$etag = md5(json_encode($notesData));
 			return (new JSONResponse($notesData))
 				->setLastModified($now)
@@ -81,7 +68,10 @@ public function get(int $id, string $exclude = '') : JSONResponse {
 		return $this->helper->handleErrorResponse(function () use ($id, $exclude) {
 			$exclude = explode(',', $exclude);
 			$note = $this->service->get($this->helper->getUID(), $id);
-			return $note->getData($exclude);
+			$noteData = $this->helper->getNoteData($note, $exclude);
+			return (new JSONResponse($noteData))
+				->setETag($noteData['etag'])
+			;
 		});
 	}
 
@@ -113,7 +103,7 @@ public function create(
 				$this->service->delete($this->helper->getUID(), $note->getId());
 				throw $e;
 			}
-			return $note->getData();
+			return $this->helper->getNoteData($note);
 		});
 	}
 
@@ -156,7 +146,7 @@ public function update(
 			$category,
 			$favorite
 		) {
-			$note = $this->service->get($this->helper->getUID(), $id);
+			$note = $this->helper->getNoteWithETagCheck($id, $this->request);
 			if ($content !== null) {
 				$note->setContent($content);
 			}
@@ -171,8 +161,7 @@ public function update(
 			if ($favorite !== null) {
 				$note->setFavorite($favorite);
 			}
-			$this->metaService->update($this->helper->getUID(), $note);
-			return $note->getData();
+			return $this->helper->getNoteData($note);
 		});
 	}
 

lib/Controller/NotesController.php

@@ -49,24 +49,6 @@ public function __construct(
 		$this->l10n = $l10n;
 	}
 
-	private function getNotesAndCategories(string $userId, int $pruneBefore) : array {
-		$data = $this->notesService->getAll($userId);
-		$metas = $this->metaService->updateAll($userId, $data['notes']);
-		$notes = array_map(function ($note) use ($metas, $pruneBefore) {
-			$lastUpdate = $metas[$note->getId()]->getLastUpdate();
-			if ($pruneBefore && $lastUpdate < $pruneBefore) {
-				return [ 'id' => $note->getId() ];
-			} else {
-				return $note->getData([ 'content' ]);
-			}
-		}, $data['notes']);
-		return [
-			'notes' => $notes,
-			'categories' => $data['categories'],
-		];
-	}
-
-
 	/**
 	 * @NoAdminRequired
 	 */
@@ -86,7 +68,7 @@ public function index(int $pruneBefore = 0) : JSONResponse {
 			$categories = null;
 
 			try {
-				$nac = $this->getNotesAndCategories($userId, $pruneBefore);
+				$nac = $this->helper->getNotesAndCategories($pruneBefore, [ 'etag', 'content' ]);
 				[ 'notes' => $notes, 'categories' => $categories ] = $nac;
 			} catch (\Throwable $e) {
 				$this->helper->logException($e);
@@ -161,10 +143,9 @@ public function get(int $id) : JSONResponse {
 				strval($id)
 			);
 
-			$result = $note->getData();
-			$etag = md5(json_encode($result));
-			return (new JSONResponse($result))
-				->setETag($etag)
+			$noteData = $this->helper->getNoteData($note);
+			return (new JSONResponse($noteData))
+				->setETag($noteData['etag'])
 			;
 		});
 	}
@@ -176,7 +157,7 @@ public function get(int $id) : JSONResponse {
 	public function create(string $category) : JSONResponse {
 		return $this->helper->handleErrorResponse(function () use ($category) {
 			$note = $this->notesService->create($this->helper->getUID(), '', $category);
-			return $note->getData();
+			return $this->helper->getNoteData($note);
 		});
 	}
 
@@ -203,7 +184,7 @@ public function undo(
 			try {
 				// check if note still exists
 				$note = $this->notesService->get($this->helper->getUID(), $id);
-				$noteData = $note->getData();
+				$noteData = $this->helper->getNoteData($note);
 				if ($noteData['error']) {
 					throw new \Exception();
 				}
@@ -214,7 +195,7 @@ public function undo(
 				$note->setContent($content);
 				$note->setModified($modified);
 				$note->setFavorite($favorite);
-				return $note->getData();
+				return $this->helper->getNoteData($note);
 			}
 		});
 	}
@@ -241,10 +222,9 @@ public function autotitle(int $id) : JSONResponse {
 	 */
 	public function update(int $id, string $content) : JSONResponse {
 		return $this->helper->handleErrorResponse(function () use ($id, $content) {
-			$note = $this->notesService->get($this->helper->getUID(), $id);
+			$note = $this->helper->getNoteWithETagCheck($id, $this->request);
 			$note->setContent($content);
-			$this->metaService->update($this->helper->getUID(), $note);
-			return $note->getData();
+			return $this->helper->getNoteData($note);
 		});
 	}
 

lib/Service/MetaService.php

@@ -92,17 +92,18 @@ public function updateAll(string $userId, array $notes, bool $forceUpdate = fals
 		return $metas;
 	}
 
-	public function update(string $userId, Note $note) : void {
+	public function update(string $userId, Note $note) : Meta {
 		$meta = null;
 		try {
 			$meta = $this->metaMapper->findById($userId, $note->getId());
 		} catch (\OCP\AppFramework\Db\DoesNotExistException $e) {
 		}
 		if ($meta === null) {
-			$this->createMeta($userId, $note);
+			$meta = $this->createMeta($userId, $note);
 		} elseif ($this->updateIfNeeded($meta, $note, true)) {
 			$this->metaMapper->update($meta);
 		}
+		return $meta;
 	}
 
 	private function getIndexedArray(array $data, string $property) : array {