API: check note's ETag (HTTP If-Match / 412)

docs/api/v1.md

@@ -20,7 +20,7 @@ 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 (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](https://en.wikipedia.org/wiki/Optimistic_concurrency_control). | 1.2 |
+| `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 |
@@ -152,6 +152,7 @@ Not enough free storage for saving the note's content.
 | Parameter | Type | Description |
 |:------|:-----|:-----|
 | `id` | integer, required (path) | ID of the note to update. |
+| `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
@@ -167,6 +168,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>
@@ -249,3 +255,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

@@ -13,6 +13,7 @@ use OCA\Notes\Service\Util;
 
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\JSONResponse;
+use OCP\IRequest;
 use OCP\IUserSession;
 
 use Psr\Log\LoggerInterface;
@@ -44,6 +45,20 @@ class Helper {
 		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);
@@ -96,6 +111,8 @@ class Helper {
 		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

@@ -143,7 +143,7 @@ class NotesApiController extends ApiController {
 			$category,
 			$favorite
 		) {
-			$note = $this->service->get($this->helper->getUID(), $id);
+			$note = $this->helper->getNoteWithETagCheck($id, $this->request);
 			if ($content !== null) {
 				$note->setContent($content);
 			}

lib/Controller/NotesController.php

@@ -222,7 +222,7 @@ class NotesController extends Controller {
 	 */
 	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);
 			return $this->helper->getNoteData($note);
 		});

tests/api/AbstractAPITest.php

@@ -188,9 +188,19 @@ abstract class AbstractAPITest extends TestCase {
 		return $note;
 	}
 
-	protected function updateNote(\stdClass &$note, \stdClass $request, \stdClass $expected) {
+	protected function updateNote(
-		$response = $this->http->request('PUT', 'notes/'.$note->id, [ 'json' => $request ]);
+		\stdClass &$note,
-		$this->checkResponse($response, 'Update note '.$note->title, 200);
+		\stdClass $request,
+		\stdClass $expected,
+		string $etag = null,
+		int $statusExp = 200
+	) {
+		$requestOptions = [ 'json' => $request ];
+		if ($etag !== null) {
+			$requestOptions['headers'] = [ 'If-Match' => '"'.$etag.'"' ];
+		}
+		$response = $this->http->request('PUT', 'notes/'.$note->id, $requestOptions);
+		$this->checkResponse($response, 'Update note '.$note->title, $statusExp);
 		$responseNote = json_decode($response->getBody()->getContents());
 		foreach (get_object_vars($request) as $key => $val) {
 			$note->$key = $val;

tests/api/CommonAPITest.php

@@ -197,7 +197,13 @@ abstract class CommonAPITest extends AbstractAPITest {
 		// test update note with single attributes
 		$rn2 = $this->updateNote($note, (object)[
 			'category' => 'Test/Third Category',
-		], (object)[]);
+		], (object)[], $rn1->etag);
+		// test update note with wrong ETag
+		$this->updateNote($note, (object)[
+			'category' => 'New Fail',
+		], (object)[
+			'category' => 'Test/Third Category',
+		], 'wrongetag', 412);
 		// TODO test update category with read-only folder (target category)
 		$rn3 = $this->updateNote($note, (object)[
 			'favorite' => true,