diff --git a/lib/Controller/BoardApiController.php b/lib/Controller/BoardApiController.php index 4953c1b989..3e4bd3b568 100644 --- a/lib/Controller/BoardApiController.php +++ b/lib/Controller/BoardApiController.php @@ -8,6 +8,7 @@ namespace OCA\Deck\Controller; use OCA\Deck\Db\Board; +use OCA\Deck\Db\ChangeHelper; use OCA\Deck\Service\BoardService; use OCA\Deck\StatusException; use OCP\AppFramework\ApiController; @@ -32,6 +33,7 @@ public function __construct( $appName, IRequest $request, private BoardService $boardService, + private ChangeHelper $changeHelper, private $userId, ) { parent::__construct($appName, $request); @@ -57,7 +59,11 @@ public function index(bool $details = false): DataResponse { } $response = new DataResponse($boards, HTTP::STATUS_OK); $response->setETag(md5(json_encode(array_map(function (Board $board) { - return $board->getId() . '-' . $board->getETag(); + $etag = $this->changeHelper->getEtag(ChangeHelper::TYPE_BOARD, $board->getId()); + if ($etag === '') { + $etag = $board->getETag(); + } + return $board->getId() . '-' . $etag; }, $boards)))); return $response; } @@ -69,9 +75,14 @@ public function index(bool $details = false): DataResponse { #[NoCSRFRequired] #[CORS] public function get(): DataResponse { - $board = $this->boardService->find($this->request->getParam('boardId')); + $boardId = (int)$this->request->getParam('boardId'); + $board = $this->boardService->find($boardId); $response = new DataResponse($board, HTTP::STATUS_OK); - $response->setETag($board->getEtag()); + $etag = $this->changeHelper->getEtag(ChangeHelper::TYPE_BOARD, $boardId); + if ($etag === '') { + $etag = $board->getEtag(); + } + $response->setETag($etag); return $response; } @@ -93,7 +104,10 @@ public function create(string $title, string $color): DataResponse { #[NoCSRFRequired] #[CORS] public function update(string $title, string $color, bool $archived = false): DataResponse { - $board = $this->boardService->update($this->request->getParam('boardId'), $title, $color, $archived); + $boardId = (int)$this->request->getParam('boardId'); + $board = $this->boardService->find($boardId, false); + $this->changeHelper->checkIfMatch(ChangeHelper::TYPE_BOARD, $boardId, $board->getETag()); + $board = $this->boardService->update($boardId, $title, $color, $archived); return new DataResponse($board, HTTP::STATUS_OK); } @@ -104,7 +118,10 @@ public function update(string $title, string $color, bool $archived = false): Da #[NoCSRFRequired] #[CORS] public function delete(): DataResponse { - $board = $this->boardService->delete($this->request->getParam('boardId')); + $boardId = (int)$this->request->getParam('boardId'); + $board = $this->boardService->find($boardId, false); + $this->changeHelper->checkIfMatch(ChangeHelper::TYPE_BOARD, $boardId, $board->getETag()); + $board = $this->boardService->delete($boardId); return new DataResponse($board, HTTP::STATUS_OK); } diff --git a/lib/Controller/CardApiController.php b/lib/Controller/CardApiController.php index b3f3fd173d..c0947a5e31 100644 --- a/lib/Controller/CardApiController.php +++ b/lib/Controller/CardApiController.php @@ -7,6 +7,7 @@ namespace OCA\Deck\Controller; +use OCA\Deck\Db\ChangeHelper; use OCA\Deck\Model\OptionalNullableValue; use OCA\Deck\Service\AssignmentService; use OCA\Deck\Service\CardService; @@ -37,6 +38,7 @@ public function __construct( IRequest $request, private CardService $cardService, private AssignmentService $assignmentService, + private ChangeHelper $changeHelper, private $userId, ) { parent::__construct($appName, $request); @@ -50,9 +52,14 @@ public function __construct( * Get a specific card. */ public function get() { - $card = $this->cardService->find($this->request->getParam('cardId')); + $cardId = (int)$this->request->getParam('cardId'); + $card = $this->cardService->find($cardId); $response = new DataResponse($card, HTTP::STATUS_OK); - $response->setETag($card->getEtag()); + $etag = $this->changeHelper->getEtag(ChangeHelper::TYPE_CARD, $cardId); + if ($etag === '') { + $etag = $card->getEtag(); + } + $response->setETag($etag); return $response; } @@ -89,9 +96,12 @@ public function create($title, $type = 'plain', $order = 999, $description = '', #[CORS] #[NoCSRFRequired] public function update(string $title, $type, string $owner, string $description = '', int $order = 0, $duedate = null, $startdate = null, $archived = null): DataResponse { + $cardId = (int)$this->request->getParam('cardId'); + $card = $this->cardService->find($cardId); + $this->changeHelper->checkIfMatch(ChangeHelper::TYPE_CARD, $cardId, $card->getETag()); $done = array_key_exists('done', $this->request->getParams()) ? new OptionalNullableValue($this->request->getParam('done', null)) : null; $color = array_key_exists('color', $this->request->getParams()) ? new OptionalNullableValue($this->request->getParam('color', null)) : null; - $card = $this->cardService->update($this->request->getParam('cardId'), $title, $this->request->getParam('stackId'), $type, $owner, $description, $order, $duedate, 0, $archived, $done, $startdate, $color); + $card = $this->cardService->update($cardId, $title, $this->request->getParam('stackId'), $type, $owner, $description, $order, $duedate, 0, $archived, $done, $startdate, $color); return new DataResponse($card, HTTP::STATUS_OK); } @@ -102,7 +112,10 @@ public function update(string $title, $type, string $owner, string $description #[CORS] #[NoCSRFRequired] public function delete(): DataResponse { - $card = $this->cardService->delete($this->request->getParam('cardId')); + $cardId = (int)$this->request->getParam('cardId'); + $card = $this->cardService->find($cardId); + $this->changeHelper->checkIfMatch(ChangeHelper::TYPE_CARD, $cardId, $card->getETag()); + $card = $this->cardService->delete($cardId); return new DataResponse($card, HTTP::STATUS_OK); } @@ -113,7 +126,10 @@ public function delete(): DataResponse { #[CORS] #[NoCSRFRequired] public function assignLabel(int $labelId): DataResponse { - $card = $this->cardService->assignLabel($this->request->getParam('cardId'), $labelId); + $cardId = (int)$this->request->getParam('cardId'); + $card = $this->cardService->find($cardId); + $this->changeHelper->checkIfMatch(ChangeHelper::TYPE_CARD, $cardId, $card->getETag()); + $card = $this->cardService->assignLabel($cardId, $labelId); return new DataResponse($card, HTTP::STATUS_OK); } @@ -124,7 +140,10 @@ public function assignLabel(int $labelId): DataResponse { #[CORS] #[NoCSRFRequired] public function removeLabel(int $labelId): DataResponse { - $card = $this->cardService->removeLabel($this->request->getParam('cardId'), $labelId); + $cardId = (int)$this->request->getParam('cardId'); + $card = $this->cardService->find($cardId); + $this->changeHelper->checkIfMatch(ChangeHelper::TYPE_CARD, $cardId, $card->getETag()); + $card = $this->cardService->removeLabel($cardId, $labelId); return new DataResponse($card, HTTP::STATUS_OK); } @@ -135,6 +154,8 @@ public function removeLabel(int $labelId): DataResponse { #[CORS] #[NoCSRFRequired] public function assignUser(int $cardId, string $userId, int $type = 0): DataResponse { + $card = $this->cardService->find($cardId); + $this->changeHelper->checkIfMatch(ChangeHelper::TYPE_CARD, $cardId, $card->getETag()); $card = $this->assignmentService->assignUser($cardId, $userId, $type); return new DataResponse($card, HTTP::STATUS_OK); } @@ -146,6 +167,8 @@ public function assignUser(int $cardId, string $userId, int $type = 0): DataResp #[CORS] #[NoCSRFRequired] public function unassignUser(int $cardId, string $userId, int $type = 0): DataResponse { + $card = $this->cardService->find($cardId); + $this->changeHelper->checkIfMatch(ChangeHelper::TYPE_CARD, $cardId, $card->getETag()); $card = $this->assignmentService->unassignUser($cardId, $userId, $type); return new DataResponse($card, HTTP::STATUS_OK); } @@ -179,6 +202,8 @@ public function removeDependentCard(int $cardId, int $dependentCardId): DataResp #[CORS] #[NoCSRFRequired] public function archive(int $cardId): DataResponse { + $card = $this->cardService->find($cardId); + $this->changeHelper->checkIfMatch(ChangeHelper::TYPE_CARD, $cardId, $card->getETag()); $card = $this->cardService->archive($cardId); return new DataResponse($card, HTTP::STATUS_OK); } @@ -190,6 +215,8 @@ public function archive(int $cardId): DataResponse { #[CORS] #[NoCSRFRequired] public function unarchive(int $cardId): DataResponse { + $card = $this->cardService->find($cardId); + $this->changeHelper->checkIfMatch(ChangeHelper::TYPE_CARD, $cardId, $card->getETag()); $card = $this->cardService->unarchive($cardId); return new DataResponse($card, HTTP::STATUS_OK); } @@ -201,7 +228,10 @@ public function unarchive(int $cardId): DataResponse { #[CORS] #[NoCSRFRequired] public function reorder(int $stackId, int $order): DataResponse { - $card = $this->cardService->reorder((int)$this->request->getParam('cardId'), $stackId, $order); + $cardId = (int)$this->request->getParam('cardId'); + $card = $this->cardService->find($cardId); + $this->changeHelper->checkIfMatch(ChangeHelper::TYPE_CARD, $cardId, $card->getETag()); + $card = $this->cardService->reorder($cardId, $stackId, $order); return new DataResponse($card, HTTP::STATUS_OK); } } diff --git a/lib/Controller/LabelApiController.php b/lib/Controller/LabelApiController.php index 5e9b2d665f..3f85d48c54 100644 --- a/lib/Controller/LabelApiController.php +++ b/lib/Controller/LabelApiController.php @@ -7,6 +7,7 @@ namespace OCA\Deck\Controller; +use OCA\Deck\Db\ChangeHelper; use OCA\Deck\Service\LabelService; use OCP\AppFramework\ApiController; use OCP\AppFramework\Http; @@ -29,6 +30,7 @@ public function __construct( $appName, IRequest $request, private LabelService $labelService, + private ChangeHelper $changeHelper, ) { parent::__construct($appName, $request); } @@ -40,8 +42,15 @@ public function __construct( #[NoCSRFRequired] #[CORS] public function get(): DataResponse { - $label = $this->labelService->find($this->request->getParam('labelId')); - return new DataResponse($label, HTTP::STATUS_OK); + $labelId = (int)$this->request->getParam('labelId'); + $label = $this->labelService->find($labelId); + $response = new DataResponse($label, HTTP::STATUS_OK); + $etag = $this->changeHelper->getEtag(ChangeHelper::TYPE_LABEL, $labelId); + if ($etag === '') { + $etag = $label->getETag(); + } + $response->setETag($etag); + return $response; } /** @@ -62,7 +71,10 @@ public function create(string $title, string $color): DataResponse { #[NoCSRFRequired] #[CORS] public function update(string $title, string $color): DataResponse { - $label = $this->labelService->update($this->request->getParam('labelId'), $title, $color); + $labelId = (int)$this->request->getParam('labelId'); + $label = $this->labelService->find($labelId); + $this->changeHelper->checkIfMatch(ChangeHelper::TYPE_LABEL, $labelId, $label->getETag()); + $label = $this->labelService->update($labelId, $title, $color); return new DataResponse($label, HTTP::STATUS_OK); } @@ -73,7 +85,10 @@ public function update(string $title, string $color): DataResponse { #[NoCSRFRequired] #[CORS] public function delete(): DataResponse { - $label = $this->labelService->delete($this->request->getParam('labelId')); + $labelId = (int)$this->request->getParam('labelId'); + $label = $this->labelService->find($labelId); + $this->changeHelper->checkIfMatch(ChangeHelper::TYPE_LABEL, $labelId, $label->getETag()); + $label = $this->labelService->delete($labelId); return new DataResponse($label, HTTP::STATUS_OK); } } diff --git a/lib/Controller/StackApiController.php b/lib/Controller/StackApiController.php index 06570dc8b1..586c0c4522 100644 --- a/lib/Controller/StackApiController.php +++ b/lib/Controller/StackApiController.php @@ -7,6 +7,8 @@ namespace OCA\Deck\Controller; +use OCA\Deck\Db\ChangeHelper; +use OCA\Deck\Db\Stack; use OCA\Deck\Service\StackService; use OCA\Deck\StatusException; use OCP\AppFramework\ApiController; @@ -31,6 +33,7 @@ public function __construct( $appName, IRequest $request, private StackService $stackService, + private ChangeHelper $changeHelper, ) { parent::__construct($appName, $request); } @@ -52,7 +55,15 @@ public function index(): DataResponse { $since = $date->getTimestamp(); } $stacks = $this->stackService->findAll($this->request->getParam('boardId'), $since); - return new DataResponse($stacks, HTTP::STATUS_OK); + $response = new DataResponse($stacks, HTTP::STATUS_OK); + $response->setETag(md5(json_encode(array_map(function (Stack $stack) { + $etag = $this->changeHelper->getEtag(ChangeHelper::TYPE_STACK, $stack->getId()); + if ($etag === '') { + $etag = $stack->getETag(); + } + return $stack->getId() . '-' . $etag; + }, $stacks)))); + return $response; } /** @@ -62,9 +73,14 @@ public function index(): DataResponse { #[CORS] #[NoCSRFRequired] public function get(): DataResponse { - $stack = $this->stackService->find($this->request->getParam('stackId')); + $stackId = (int)$this->request->getParam('stackId'); + $stack = $this->stackService->find($stackId); $response = new DataResponse($stack, HTTP::STATUS_OK); - $response->setETag($stack->getETag()); + $etag = $this->changeHelper->getEtag(ChangeHelper::TYPE_STACK, $stackId); + if ($etag === '') { + $etag = $stack->getETag(); + } + $response->setETag($etag); return $response; } @@ -86,7 +102,10 @@ public function create(string $title, int $order): DataResponse { #[CORS] #[NoCSRFRequired] public function update(string $title, int $order) { - $stack = $this->stackService->update($this->request->getParam('stackId'), $title, $this->request->getParam('boardId'), $order, 0); + $stackId = (int)$this->request->getParam('stackId'); + $stack = $this->stackService->find($stackId); + $this->changeHelper->checkIfMatch(ChangeHelper::TYPE_STACK, $stackId, $stack->getETag()); + $stack = $this->stackService->update($stackId, $title, $this->request->getParam('boardId'), $order, 0); return new DataResponse($stack, HTTP::STATUS_OK); } @@ -97,7 +116,10 @@ public function update(string $title, int $order) { #[CORS] #[NoCSRFRequired] public function delete(): DataResponse { - $stack = $this->stackService->delete($this->request->getParam('stackId')); + $stackId = (int)$this->request->getParam('stackId'); + $stack = $this->stackService->find($stackId); + $this->changeHelper->checkIfMatch(ChangeHelper::TYPE_STACK, $stackId, $stack->getETag()); + $stack = $this->stackService->delete($stackId); return new DataResponse($stack, HTTP::STATUS_OK); } diff --git a/lib/Db/BoardMapper.php b/lib/Db/BoardMapper.php index d9d7d2c4b8..660abf38b0 100644 --- a/lib/Db/BoardMapper.php +++ b/lib/Db/BoardMapper.php @@ -471,6 +471,20 @@ public function findAll(): array { return $this->findEntities($qb); } + public function insert(Entity $entity): Entity { + if (!isset($entity->getUpdatedFields()['lastModified'])) { + $entity->setLastModified(time()); + } + return parent::insert($entity); + } + + public function update(Entity $entity): Entity { + $entity->setLastModified(time()); + $result = parent::update($entity); + $this->boardCache[(string)$entity->getId()] = $result; + return $result; + } + public function findToDelete(int $timeLimit) { $qb = $this->db->getQueryBuilder(); $qb->select('id', 'title', 'owner', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') diff --git a/lib/Db/ChangeHelper.php b/lib/Db/ChangeHelper.php index 8cdc3da5d0..6a79ef1f96 100644 --- a/lib/Db/ChangeHelper.php +++ b/lib/Db/ChangeHelper.php @@ -7,6 +7,7 @@ namespace OCA\Deck\Db; +use OCA\Deck\Exceptions\PreconditionFailedException; use OCP\ICache; use OCP\ICacheFactory; use OCP\IDBConnection; @@ -15,6 +16,8 @@ class ChangeHelper { public const TYPE_BOARD = 'boardChanged'; public const TYPE_CARD = 'cardChanged'; + public const TYPE_STACK = 'stackChanged'; + public const TYPE_LABEL = 'labelChanged'; private IDBConnection $db; private ICache $cache; @@ -54,17 +57,55 @@ public function cardChanged($cardId, $updateCard = true) { $result = $this->db->executeQuery($sql, [$cardId]); if ($row = $result->fetch()) { $this->boardChanged($row['id']); - $this->stackChanged($row['stack_id']); + $this->stackChanged($row['stack_id'], true, false); } } - public function stackChanged($stackId, $updateBoard = true) { + public function stackChanged($stackId, $updateStack = true, $updateBoard = true) { $time = time(); $etag = md5($time . microtime()); - $this->cache->set(self::TYPE_CARD . '-' . $stackId, $etag); - if ($updateBoard) { + $this->cache->set(self::TYPE_STACK . '-' . $stackId, $etag); + if ($updateStack) { $sql = 'UPDATE `*PREFIX*deck_stacks` SET `last_modified` = ? WHERE `id` = ?'; - $this->db->executeUpdate($sql, [time(), $stackId]); + $this->db->executeUpdate($sql, [$time, $stackId]); + } + + if ($updateBoard) { + $sql = 'SELECT board_id FROM `*PREFIX*deck_stacks` WHERE id = ?'; + $result = $this->db->executeQuery($sql, [$stackId]); + if ($row = $result->fetch()) { + $this->boardChanged($row['board_id']); + } + } + } + + public function labelChanged($labelId, $updateLabel = true) { + $time = time(); + $etag = md5($time . microtime()); + $this->cache->set(self::TYPE_LABEL . '-' . $labelId, $etag); + if ($updateLabel) { + $sql = 'UPDATE `*PREFIX*deck_labels` SET `last_modified` = ? WHERE `id` = ?'; + $this->db->executeUpdate($sql, [$time, $labelId]); + } + + $sql = 'SELECT board_id FROM `*PREFIX*deck_labels` WHERE id = ?'; + $result = $this->db->executeQuery($sql, [$labelId]); + if ($row = $result->fetch()) { + $this->boardChanged($row['board_id']); + } + } + + public function checkIfMatch($type, $id, $fallbackEtag = null) { + $ifMatch = $this->request->getHeader('If-Match'); + if ($ifMatch === '') { + return; + } + $etag = $this->getEtag($type, $id); + if ($etag === '' && $fallbackEtag !== null) { + $etag = $fallbackEtag; + } + if ($ifMatch !== '*' && $ifMatch !== $etag && $ifMatch !== '"' . $etag . '"') { + throw new PreconditionFailedException('If-Match header does not match the current ETag'); } } diff --git a/lib/Db/StackMapper.php b/lib/Db/StackMapper.php index bbbd6e94bc..9efd8b22b6 100644 --- a/lib/Db/StackMapper.php +++ b/lib/Db/StackMapper.php @@ -138,7 +138,15 @@ public function findDeleted(int $boardId, ?int $limit = null, int $offset = 0): return $this->findEntities($qb); } + public function insert(Entity $entity): Entity { + if (!isset($entity->getUpdatedFields()['lastModified'])) { + $entity->setLastModified(time()); + } + return parent::insert($entity); + } + public function update(Entity $entity): Entity { + $entity->setLastModified(time()); $result = parent::update($entity); $this->stackCache[(string)$entity->getId()] = $result; return $result; diff --git a/lib/Exceptions/PreconditionFailedException.php b/lib/Exceptions/PreconditionFailedException.php new file mode 100644 index 0000000000..aa7e45e6a9 --- /dev/null +++ b/lib/Exceptions/PreconditionFailedException.php @@ -0,0 +1,17 @@ +setTitle($title); $label->setColor($color); $label->setBoardId($boardId); - $this->changeHelper->boardChanged($boardId); + $label = $this->labelMapper->insert($label); + $this->changeHelper->labelChanged($label->getId(), false); - return $this->labelMapper->insert($label); + return $label; } public function cloneLabelIfNotExists(int $labelId, int $targetBoardId): Label { @@ -92,7 +93,7 @@ public function delete(int $id): Label { throw new StatusException('Operation not allowed. This board is archived.'); } $label = $this->labelMapper->delete($this->find($id)); - $this->changeHelper->boardChanged($label->getBoardId()); + $this->changeHelper->labelChanged($label->getId(), false); return $label; } @@ -121,8 +122,9 @@ public function update(int $id, string $title, string $color): Label { $label->setTitle($title); $label->setColor($color); - $this->changeHelper->boardChanged($label->getBoardId()); + $label = $this->labelMapper->update($label); + $this->changeHelper->labelChanged($label->getId(), false); - return $this->labelMapper->update($label); + return $label; } } diff --git a/lib/Service/StackService.php b/lib/Service/StackService.php index 16e8d8b415..4c26435bf3 100644 --- a/lib/Service/StackService.php +++ b/lib/Service/StackService.php @@ -194,7 +194,7 @@ public function create(string $title, int $boardId, int $order): Stack { $this->activityManager->triggerEvent( ActivityManager::DECK_OBJECT_BOARD, $stack, ActivityManager::SUBJECT_STACK_CREATE, [], $this->permissionService->getUserId() ); - $this->changeHelper->boardChanged($boardId); + $this->changeHelper->stackChanged($stack->getId(), false); $this->eventDispatcher->dispatchTyped(new BoardUpdatedEvent($boardId)); return $stack; @@ -217,7 +217,7 @@ public function delete(int $id): Stack { $this->activityManager->triggerEvent( ActivityManager::DECK_OBJECT_BOARD, $stack, ActivityManager::SUBJECT_STACK_DELETE, [], $this->permissionService->getUserId() ); - $this->changeHelper->boardChanged($stack->getBoardId()); + $this->changeHelper->stackChanged($stack->getId(), false); $this->eventDispatcher->dispatchTyped(new BoardUpdatedEvent($stack->getBoardId())); $this->enrichStacksWithCards([$stack]); @@ -252,7 +252,7 @@ public function update(int $id, string $title, int $boardId, int $order, ?int $d $this->activityManager->triggerUpdateEvents( ActivityManager::DECK_OBJECT_BOARD, $changes, ActivityManager::SUBJECT_STACK_UPDATE ); - $this->changeHelper->boardChanged($stack->getBoardId()); + $this->changeHelper->stackChanged($stack->getId(), false); $this->eventDispatcher->dispatchTyped(new BoardUpdatedEvent($stack->getBoardId())); return $stack; @@ -287,6 +287,7 @@ public function reorder(int $id, int $order): array { $stack->setOrder($i++); } $stack = $this->stackMapper->update($stack); + $this->changeHelper->stackChanged($stack->getId(), false, false); $result[$stack->getOrder()] = $stack; } $this->changeHelper->boardChanged($stackToSort->getBoardId()); @@ -329,7 +330,7 @@ public function setDoneStack(int $stackId, int $boardId, bool $isDone): void { } $this->stackMapper->setIsDoneColumn($stackId, $isDone); - $this->changeHelper->boardChanged($boardId); + $this->changeHelper->stackChanged($stackId, true); $this->eventDispatcher->dispatchTyped(new BoardUpdatedEvent($boardId)); } } diff --git a/tests/integration/features/api/boards.feature b/tests/integration/features/api/boards.feature index 46ea0e3abe..fa0ea33ae2 100644 --- a/tests/integration/features/api/boards.feature +++ b/tests/integration/features/api/boards.feature @@ -98,6 +98,31 @@ Feature: REST API - Boards | archived | true | Then the response should have a status code "200" And the response value "title" should be "Updated board" + + Scenario: PUT /boards/{boardId} - Update board details fails if If-Match doesn't match + Given sending "POST" to the API endpoint "/boards" with body: + | title | Board to update | + | color | ff0000 | + And the response value "id" is stored as "boardId" + When sending "PUT" to the API endpoint "/boards/" with the header "If-Match" set to "invalid" and body: + | title | Updated board | + | color | 00ff00 | + | archived | true | + Then the response should have a status code "412" + + Scenario: PUT /boards/{boardId} - Update board details succeeds if If-Match matches + Given sending "POST" to the API endpoint "/boards" with body: + | title | Board to update | + | color | ff0000 | + And the response value "id" is stored as "boardId" + And sending "GET" to the API endpoint "/boards/" + And the response header "ETag" is stored as "boardEtag" + When sending "PUT" to the API endpoint "/boards/" with the header "If-Match" set to "" and body: + | title | Updated board | + | color | 00ff00 | + | archived | true | + Then the response should have a status code "200" + And the response value "title" should be "Updated board" And the response value "color" should be "00ff00" And the response value "archived" should be "true" When sending "GET" to the API endpoint "/boards/" diff --git a/tests/integration/features/bootstrap/DeckApiContext.php b/tests/integration/features/bootstrap/DeckApiContext.php index 473bd056db..1687dfe443 100644 --- a/tests/integration/features/bootstrap/DeckApiContext.php +++ b/tests/integration/features/bootstrap/DeckApiContext.php @@ -77,9 +77,13 @@ public function sendingToTheOcsApiEndpoint(string $method, string $endpoint, ?Ta /** * @When /^sending "([^"]*)" to the API endpoint "([^"]*)" with the header "([^"]*)" set to "([^"]*)"$/ + * @When /^sending "([^"]*)" to the API endpoint "([^"]*)" with the header "([^"]*)" set to "([^"]*)" and body:$/ */ - public function sendingToTheApiEndpointWithHeader(string $method, string $endpoint, string $header, string $value): void { - $this->sendRequest($method, $endpoint, [], [$header => $this->resolve($value)]); + public function sendingToTheApiEndpointWithHeader(string $method, string $endpoint, string $header, string $value, ?TableNode $body = null): void { + $options = $body === null + ? [] + : ['json' => $this->parseBody($body)]; + $this->sendRequest($method, $endpoint, $options, [$header => $this->resolve($value)]); } /** diff --git a/tests/unit/Db/ChangeHelperTest.php b/tests/unit/Db/ChangeHelperTest.php new file mode 100644 index 0000000000..ed82eb1c2a --- /dev/null +++ b/tests/unit/Db/ChangeHelperTest.php @@ -0,0 +1,96 @@ + + * + * @author Julius Härtl + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\Deck\Db; + +use OCA\Deck\Exceptions\PreconditionFailedException; +use OCP\ICache; +use OCP\ICacheFactory; +use OCP\IDBConnection; +use OCP\IRequest; +use PHPUnit\Framework\TestCase; + +class ChangeHelperTest extends TestCase { + + private $db; + private $cache; + private $cacheFactory; + private $request; + private $changeHelper; + + protected function setUp(): void { + parent::setUp(); + $this->db = $this->createMock(IDBConnection::class); + $this->cache = $this->createMock(ICache::class); + $this->cacheFactory = $this->createMock(ICacheFactory::class); + $this->cacheFactory->method('createDistributed')->willReturn($this->cache); + $this->request = $this->createMock(IRequest::class); + $this->changeHelper = new ChangeHelper( + $this->db, + $this->cacheFactory, + $this->request, + 'user1' + ); + } + + public function testCheckIfMatchNoHeader() { + $this->request->method('getHeader')->with('If-Match')->willReturn(''); + $this->changeHelper->checkIfMatch(ChangeHelper::TYPE_BOARD, 1); + $this->addToAssertionCount(1); + } + + public function testCheckIfMatchSuccess() { + $this->request->method('getHeader')->with('If-Match')->willReturn('my-etag'); + $this->cache->method('get')->with('boardChanged-1')->willReturn('my-etag'); + $this->changeHelper->checkIfMatch(ChangeHelper::TYPE_BOARD, 1); + $this->addToAssertionCount(1); + } + + public function testCheckIfMatchQuotedSuccess() { + $this->request->method('getHeader')->with('If-Match')->willReturn('"my-etag"'); + $this->cache->method('get')->with('boardChanged-1')->willReturn('my-etag'); + $this->changeHelper->checkIfMatch(ChangeHelper::TYPE_BOARD, 1); + $this->addToAssertionCount(1); + } + + public function testCheckIfMatchWildcardSuccess() { + $this->request->method('getHeader')->with('If-Match')->willReturn('*'); + $this->changeHelper->checkIfMatch(ChangeHelper::TYPE_BOARD, 1); + $this->addToAssertionCount(1); + } + + public function testCheckIfMatchMismatch() { + $this->request->method('getHeader')->with('If-Match')->willReturn('wrong-etag'); + $this->cache->method('get')->with('boardChanged-1')->willReturn('correct-etag'); + $this->expectException(PreconditionFailedException::class); + $this->changeHelper->checkIfMatch(ChangeHelper::TYPE_BOARD, 1); + } + + public function testCheckIfMatchFallbackSuccess() { + $this->request->method('getHeader')->with('If-Match')->willReturn('fallback-etag'); + $this->cache->method('get')->with('boardChanged-1')->willReturn('null'); + $this->changeHelper->checkIfMatch(ChangeHelper::TYPE_BOARD, 1, 'fallback-etag'); + $this->addToAssertionCount(1); + } +} diff --git a/tests/unit/controller/BoardApiControllerTest.php b/tests/unit/controller/BoardApiControllerTest.php index 0e57d1a081..713eaa3a04 100644 --- a/tests/unit/controller/BoardApiControllerTest.php +++ b/tests/unit/controller/BoardApiControllerTest.php @@ -25,6 +25,7 @@ namespace OCA\Deck\Controller; use OCA\Deck\Db\Board; +use OCA\Deck\Db\ChangeHelper; use OCA\Deck\Service\BoardService; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; @@ -36,6 +37,7 @@ class BoardApiControllerTest extends \Test\TestCase { private $userId = 'admin'; private $controller; private $boardService; + private $changeHelper; private $exampleBoard; private $deniedBoard; @@ -43,11 +45,13 @@ public function setUp(): void { parent::setUp(); $this->request = $this->createMock(IRequest::class); $this->boardService = $this->createMock(BoardService::class); + $this->changeHelper = $this->createMock(ChangeHelper::class); $this->controller = new BoardApiController( $this->appName, $this->request, $this->boardService, + $this->changeHelper, $this->userId ); @@ -86,6 +90,11 @@ public function testGet() { ->method('find') ->willReturn($board); + $this->changeHelper->expects($this->once()) + ->method('getEtag') + ->with(ChangeHelper::TYPE_BOARD, $boardId) + ->willReturn(''); + $this->request->expects($this->any()) ->method('getParam') ->with('boardId') diff --git a/tests/unit/controller/CardApiControllerTest.php b/tests/unit/controller/CardApiControllerTest.php index 1b643222f8..83d13b5e7b 100644 --- a/tests/unit/controller/CardApiControllerTest.php +++ b/tests/unit/controller/CardApiControllerTest.php @@ -27,6 +27,7 @@ namespace OCA\Deck\Controller; use OCA\Deck\Db\Card; +use OCA\Deck\Db\ChangeHelper; use OCA\Deck\Service\AssignmentService; use OCA\Deck\Service\CardService; use OCP\AppFramework\Http; @@ -38,6 +39,7 @@ class CardApiControllerTest extends \Test\TestCase { private CardApiController $controller; private IRequest&MockObject $request; private CardService&MockObject $cardService; + private ChangeHelper&MockObject $changeHelper; private string $userId = 'admin'; private array $cardExample; private array $stackExample; @@ -49,6 +51,7 @@ public function setUp(): void { $this->request = $this->createMock(IRequest::class); $this->cardService = $this->createMock(CardService::class); $this->assignmentService = $this->createMock(AssignmentService::class); + $this->changeHelper = $this->createMock(ChangeHelper::class); $this->cardExample['id'] = 1; $this->stackExample['id'] = 1; @@ -58,6 +61,7 @@ public function setUp(): void { $this->request, $this->cardService, $this->assignmentService, + $this->changeHelper, $this->userId ); } @@ -75,6 +79,11 @@ public function testGet(): void { ->method('find') ->willReturn($card); + $this->changeHelper->expects($this->once()) + ->method('getEtag') + ->with(ChangeHelper::TYPE_CARD, $this->cardExample['id']) + ->willReturn(''); + $expected = new DataResponse($card, HTTP::STATUS_OK); $expected->setETag($card->getETag()); $actual = $this->controller->get(); diff --git a/tests/unit/controller/LabelApiControllerTest.php b/tests/unit/controller/LabelApiControllerTest.php index 97f86eae68..45436734bc 100644 --- a/tests/unit/controller/LabelApiControllerTest.php +++ b/tests/unit/controller/LabelApiControllerTest.php @@ -24,18 +24,21 @@ namespace OCA\Deck\Controller; +use OCA\Deck\Db\ChangeHelper; use OCA\Deck\Db\Label; use OCA\Deck\Service\LabelService; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; use OCP\IRequest; +use PHPUnit\Framework\MockObject\MockObject; class LabelApiControllerTest extends \Test\TestCase { - private $controller; - private $request; - private $labelService; - private $userId = 'admin'; - private $exampleLabel = [ + private LabelApiController $controller; + private IRequest&MockObject $request; + private LabelService&MockObject $labelService; + private ChangeHelper&MockObject $changeHelper; + private string $userId = 'admin'; + private array $exampleLabel = [ 'id' => 123 ]; @@ -43,12 +46,12 @@ public function setUp(): void { parent::setUp(); $this->request = $this->createMock(IRequest::class); $this->labelService = $this->createMock(LabelService::class); - $this->exampleLabel['id']; + $this->changeHelper = $this->createMock(ChangeHelper::class); $this->controller = new LabelApiController( 'deck', $this->request, $this->labelService, - $this->userId + $this->changeHelper ); } @@ -65,7 +68,13 @@ public function testGet() { ->method('find') ->willReturn($label); + $this->changeHelper->expects($this->once()) + ->method('getEtag') + ->with(ChangeHelper::TYPE_LABEL, $this->exampleLabel['id']) + ->willReturn(''); + $expected = new DataResponse($label, HTTP::STATUS_OK); + $expected->setETag($label->getETag()); $actual = $this->controller->get(); $this->assertEquals($expected, $actual); } @@ -97,8 +106,18 @@ public function testUpdate() { ->with('labelId') ->will($this->returnValue($this->exampleLabel['id'])); + $this->labelService->expects($this->once()) + ->method('find') + ->with($this->exampleLabel['id']) + ->willReturn($label); + + $this->changeHelper->expects($this->once()) + ->method('checkIfMatch') + ->with(ChangeHelper::TYPE_LABEL, $this->exampleLabel['id'], $label->getETag()); + $this->labelService->expects($this->once()) ->method('update') + ->with($this->exampleLabel['id'], 'title', '000000') ->will($this->returnValue($label)); $expected = new DataResponse($label, HTTP::STATUS_OK); @@ -115,8 +134,18 @@ public function testDelete() { ->with('labelId') ->will($this->returnValue($this->exampleLabel['id'])); + $this->labelService->expects($this->once()) + ->method('find') + ->with($this->exampleLabel['id']) + ->willReturn($label); + + $this->changeHelper->expects($this->once()) + ->method('checkIfMatch') + ->with(ChangeHelper::TYPE_LABEL, $this->exampleLabel['id'], $label->getETag()); + $this->labelService->expects($this->once()) ->method('delete') + ->with($this->exampleLabel['id']) ->willReturn($label); $expected = new DataResponse($label, HTTP::STATUS_OK); diff --git a/tests/unit/controller/StackApiControllerTest.php b/tests/unit/controller/StackApiControllerTest.php index 37a5bcef09..17fe74987e 100644 --- a/tests/unit/controller/StackApiControllerTest.php +++ b/tests/unit/controller/StackApiControllerTest.php @@ -24,6 +24,7 @@ namespace OCA\Deck\Controller; +use OCA\Deck\Db\ChangeHelper; use OCA\Deck\Db\Stack; use OCA\Deck\Service\BoardService; use OCA\Deck\Service\StackService; @@ -38,6 +39,7 @@ class StackApiControllerTest extends \Test\TestCase { private $controller; private $boardService; private $stackService; + private $changeHelper; private $exampleStack = []; private $exampleBoard = []; @@ -46,6 +48,7 @@ public function setUp(): void { $this->request = $this->createMock(IRequest::class); $this->boardService = $this->createMock(BoardService::class); $this->stackService = $this->createMock(StackService::class); + $this->changeHelper = $this->createMock(ChangeHelper::class); $this->exampleBoard['boardId'] = '89'; @@ -58,7 +61,7 @@ public function setUp(): void { $this->appName, $this->request, $this->stackService, - $this->boardService + $this->changeHelper ); } @@ -94,6 +97,11 @@ public function testGet() { ->method('find') ->willReturn($stack); + $this->changeHelper->expects($this->once()) + ->method('getEtag') + ->with(ChangeHelper::TYPE_STACK, $this->exampleStack['id']) + ->willReturn(''); + $this->request->expects($this->once()) ->method('getParam') ->with('stackId')