Skip to content

feat: add If-Match support and fix stack ETag synchronization #8291

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
desperateCoder wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: add If-Match support and fix stack ETag synchronization #8291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 22 additions & 5 deletions lib/Controller/BoardApiController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -32,6 +33,7 @@ public function __construct(
$appName,
IRequest $request,
private BoardService $boardService,
private ChangeHelper $changeHelper,
private $userId,
) {
parent::__construct($appName, $request);
Expand All @@ -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;
}
Expand All @@ -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;
}

Expand All @@ -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);
}

Expand All @@ -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);
}

Expand Down
44 changes: 37 additions & 7 deletions lib/Controller/CardApiController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -37,6 +38,7 @@ public function __construct(
IRequest $request,
private CardService $cardService,
private AssignmentService $assignmentService,
private ChangeHelper $changeHelper,
private $userId,
) {
parent::__construct($appName, $request);
Expand All @@ -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;
}

Expand Down Expand Up @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -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);
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
}
23 changes: 19 additions & 4 deletions lib/Controller/LabelApiController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -29,6 +30,7 @@ public function __construct(
$appName,
IRequest $request,
private LabelService $labelService,
private ChangeHelper $changeHelper,
) {
parent::__construct($appName, $request);
}
Expand All @@ -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;
}

/**
Expand All @@ -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);
}

Expand All @@ -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);
}
}
32 changes: 27 additions & 5 deletions lib/Controller/StackApiController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -31,6 +33,7 @@ public function __construct(
$appName,
IRequest $request,
private StackService $stackService,
private ChangeHelper $changeHelper,
) {
parent::__construct($appName, $request);
}
Expand All @@ -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;
}

/**
Expand All @@ -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;
}

Expand All @@ -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);
}

Expand All @@ -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);
}

Expand Down
14 changes: 14 additions & 0 deletions lib/Db/BoardMapper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,20 @@
return $this->findEntities($qb);
}

public function insert(Entity $entity): Entity {

Check failure on line 474 in lib/Db/BoardMapper.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

InvalidReturnType 

lib/Db/BoardMapper.php:474:42: InvalidReturnType: The declared return type 'OCA\Deck\Db\Board&OCA\Deck\Db\Entity' for OCA\Deck\Db\BoardMapper::insert is incorrect, got 'OCA\Deck\Db\Board' (see https://psalm.dev/011)

Check failure on line 474 in lib/Db/BoardMapper.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

UndefinedClass 

lib/Db/BoardMapper.php:474:42: UndefinedClass: Class, interface or enum named OCA\Deck\Db\Entity does not exist (see https://psalm.dev/019)

Check failure on line 474 in lib/Db/BoardMapper.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

ImplementedParamTypeMismatch 

lib/Db/BoardMapper.php:474:32: ImplementedParamTypeMismatch: Argument 1 of OCA\Deck\Db\BoardMapper::insert has wrong type 'OCA\Deck\Db\Entity', expecting 'OCA\Deck\Db\Board' as defined by OCP\AppFramework\Db\QBMapper::insert (see https://psalm.dev/199)

Check failure on line 474 in lib/Db/BoardMapper.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

MethodSignatureMismatch 

lib/Db/BoardMapper.php:474:32: MethodSignatureMismatch: Argument 1 of OCA\Deck\Db\BoardMapper::insert has wrong type 'OCA\Deck\Db\Entity', expecting 'OCP\AppFramework\Db\Entity' as defined by OCP\AppFramework\Db\QBMapper::insert (see https://psalm.dev/042)

Check failure on line 474 in lib/Db/BoardMapper.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

MethodSignatureMismatch 

lib/Db/BoardMapper.php:474:2: MethodSignatureMismatch: Method OCA\Deck\Db\BoardMapper::insert with return type 'OCA\Deck\Db\Entity' is different to return type 'OCP\AppFramework\Db\Entity' of inherited method OCP\AppFramework\Db\QBMapper::insert (see https://psalm.dev/042)
if (!isset($entity->getUpdatedFields()['lastModified'])) {
$entity->setLastModified(time());
}
return parent::insert($entity);

Check failure on line 478 in lib/Db/BoardMapper.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

InvalidReturnStatement 

lib/Db/BoardMapper.php:478:10: InvalidReturnStatement: The inferred type 'OCA\Deck\Db\Board' does not match the declared return type 'OCA\Deck\Db\Board&OCA\Deck\Db\Entity' for OCA\Deck\Db\BoardMapper::insert (see https://psalm.dev/128)
}

public function update(Entity $entity): Entity {

Check failure on line 481 in lib/Db/BoardMapper.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

UndefinedClass 

lib/Db/BoardMapper.php:481:42: UndefinedClass: Class, interface or enum named OCA\Deck\Db\Entity does not exist (see https://psalm.dev/019)

Check failure on line 481 in lib/Db/BoardMapper.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

ImplementedParamTypeMismatch 

lib/Db/BoardMapper.php:481:32: ImplementedParamTypeMismatch: Argument 1 of OCA\Deck\Db\BoardMapper::update has wrong type 'OCA\Deck\Db\Entity', expecting 'OCA\Deck\Db\Board' as defined by OCP\AppFramework\Db\QBMapper::update (see https://psalm.dev/199)

Check failure on line 481 in lib/Db/BoardMapper.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

MethodSignatureMismatch 

lib/Db/BoardMapper.php:481:32: MethodSignatureMismatch: Argument 1 of OCA\Deck\Db\BoardMapper::update has wrong type 'OCA\Deck\Db\Entity', expecting 'OCP\AppFramework\Db\Entity' as defined by OCP\AppFramework\Db\QBMapper::update (see https://psalm.dev/042)

Check failure on line 481 in lib/Db/BoardMapper.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

MethodSignatureMismatch 

lib/Db/BoardMapper.php:481:2: MethodSignatureMismatch: Method OCA\Deck\Db\BoardMapper::update with return type 'OCA\Deck\Db\Entity' is different to return type 'OCP\AppFramework\Db\Entity' of inherited method OCP\AppFramework\Db\QBMapper::update (see https://psalm.dev/042)
$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')
Expand Down
Loading
Loading