Skip to content

Commit d71dce8

Browse files
author
desperateCoder
committed
feat: add If-Match support and fix stack ETag synchronization
- Implement Optimistic Concurrency Control via If-Match header for Boards, Stacks, Cards, and Labels. - Fix ETag propagation when renaming or reordering stacks (Fixes #2866). - Ensure automatic last_modified updates in BoardMapper and StackMapper. - Add unit and integration tests for OCC and ETag logic. Fixes #2866
1 parent b0f6943 commit d71dce8

17 files changed

Lines changed: 392 additions & 45 deletions

‎lib/Controller/BoardApiController.php‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
namespace OCA\Deck\Controller;
99

1010
use OCA\Deck\Db\Board;
11+
use OCA\Deck\Db\ChangeHelper;
1112
use OCA\Deck\Service\BoardService;
1213
use OCA\Deck\StatusException;
1314
use OCP\AppFramework\ApiController;
@@ -32,6 +33,7 @@ public function __construct(
3233
$appName,
3334
IRequest $request,
3435
private BoardService $boardService,
36+
private ChangeHelper $changeHelper,
3537
private $userId,
3638
) {
3739
parent::__construct($appName, $request);
@@ -57,7 +59,11 @@ public function index(bool $details = false): DataResponse {
5759
}
5860
$response = new DataResponse($boards, HTTP::STATUS_OK);
5961
$response->setETag(md5(json_encode(array_map(function (Board $board) {
60-
return $board->getId() . '-' . $board->getETag();
62+
$etag = $this->changeHelper->getEtag(ChangeHelper::TYPE_BOARD, $board->getId());
63+
if ($etag === '') {
64+
$etag = $board->getETag();
65+
}
66+
return $board->getId() . '-' . $etag;
6167
}, $boards))));
6268
return $response;
6369
}
@@ -69,9 +75,14 @@ public function index(bool $details = false): DataResponse {
6975
#[NoCSRFRequired]
7076
#[CORS]
7177
public function get(): DataResponse {
72-
$board = $this->boardService->find($this->request->getParam('boardId'));
78+
$boardId = (int)$this->request->getParam('boardId');
79+
$board = $this->boardService->find($boardId);
7380
$response = new DataResponse($board, HTTP::STATUS_OK);
74-
$response->setETag($board->getEtag());
81+
$etag = $this->changeHelper->getEtag(ChangeHelper::TYPE_BOARD, $boardId);
82+
if ($etag === '') {
83+
$etag = $board->getEtag();
84+
}
85+
$response->setETag($etag);
7586
return $response;
7687
}
7788

@@ -93,7 +104,10 @@ public function create(string $title, string $color): DataResponse {
93104
#[NoCSRFRequired]
94105
#[CORS]
95106
public function update(string $title, string $color, bool $archived = false): DataResponse {
96-
$board = $this->boardService->update($this->request->getParam('boardId'), $title, $color, $archived);
107+
$boardId = (int)$this->request->getParam('boardId');
108+
$board = $this->boardService->find($boardId, false);
109+
$this->changeHelper->checkIfMatch(ChangeHelper::TYPE_BOARD, $boardId, $board->getETag());
110+
$board = $this->boardService->update($boardId, $title, $color, $archived);
97111
return new DataResponse($board, HTTP::STATUS_OK);
98112
}
99113

@@ -104,7 +118,10 @@ public function update(string $title, string $color, bool $archived = false): Da
104118
#[NoCSRFRequired]
105119
#[CORS]
106120
public function delete(): DataResponse {
107-
$board = $this->boardService->delete($this->request->getParam('boardId'));
121+
$boardId = (int)$this->request->getParam('boardId');
122+
$board = $this->boardService->find($boardId, false);
123+
$this->changeHelper->checkIfMatch(ChangeHelper::TYPE_BOARD, $boardId, $board->getETag());
124+
$board = $this->boardService->delete($boardId);
108125
return new DataResponse($board, HTTP::STATUS_OK);
109126
}
110127

‎lib/Controller/CardApiController.php‎

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
namespace OCA\Deck\Controller;
99

10+
use OCA\Deck\Db\ChangeHelper;
1011
use OCA\Deck\Model\OptionalNullableValue;
1112
use OCA\Deck\Service\AssignmentService;
1213
use OCA\Deck\Service\CardService;
@@ -37,6 +38,7 @@ public function __construct(
3738
IRequest $request,
3839
private CardService $cardService,
3940
private AssignmentService $assignmentService,
41+
private ChangeHelper $changeHelper,
4042
private $userId,
4143
) {
4244
parent::__construct($appName, $request);
@@ -50,9 +52,14 @@ public function __construct(
5052
* Get a specific card.
5153
*/
5254
public function get() {
53-
$card = $this->cardService->find($this->request->getParam('cardId'));
55+
$cardId = (int)$this->request->getParam('cardId');
56+
$card = $this->cardService->find($cardId);
5457
$response = new DataResponse($card, HTTP::STATUS_OK);
55-
$response->setETag($card->getEtag());
58+
$etag = $this->changeHelper->getEtag(ChangeHelper::TYPE_CARD, $cardId);
59+
if ($etag === '') {
60+
$etag = $card->getEtag();
61+
}
62+
$response->setETag($etag);
5663
return $response;
5764
}
5865

@@ -89,9 +96,12 @@ public function create($title, $type = 'plain', $order = 999, $description = '',
8996
#[CORS]
9097
#[NoCSRFRequired]
9198
public function update(string $title, $type, string $owner, string $description = '', int $order = 0, $duedate = null, $startdate = null, $archived = null): DataResponse {
99+
$cardId = (int)$this->request->getParam('cardId');
100+
$card = $this->cardService->find($cardId);
101+
$this->changeHelper->checkIfMatch(ChangeHelper::TYPE_CARD, $cardId, $card->getETag());
92102
$done = array_key_exists('done', $this->request->getParams()) ? new OptionalNullableValue($this->request->getParam('done', null)) : null;
93103
$color = array_key_exists('color', $this->request->getParams()) ? new OptionalNullableValue($this->request->getParam('color', null)) : null;
94-
$card = $this->cardService->update($this->request->getParam('cardId'), $title, $this->request->getParam('stackId'), $type, $owner, $description, $order, $duedate, 0, $archived, $done, $startdate, $color);
104+
$card = $this->cardService->update($cardId, $title, $this->request->getParam('stackId'), $type, $owner, $description, $order, $duedate, 0, $archived, $done, $startdate, $color);
95105
return new DataResponse($card, HTTP::STATUS_OK);
96106
}
97107

@@ -102,7 +112,10 @@ public function update(string $title, $type, string $owner, string $description
102112
#[CORS]
103113
#[NoCSRFRequired]
104114
public function delete(): DataResponse {
105-
$card = $this->cardService->delete($this->request->getParam('cardId'));
115+
$cardId = (int)$this->request->getParam('cardId');
116+
$card = $this->cardService->find($cardId);
117+
$this->changeHelper->checkIfMatch(ChangeHelper::TYPE_CARD, $cardId, $card->getETag());
118+
$card = $this->cardService->delete($cardId);
106119
return new DataResponse($card, HTTP::STATUS_OK);
107120
}
108121

@@ -113,7 +126,10 @@ public function delete(): DataResponse {
113126
#[CORS]
114127
#[NoCSRFRequired]
115128
public function assignLabel(int $labelId): DataResponse {
116-
$card = $this->cardService->assignLabel($this->request->getParam('cardId'), $labelId);
129+
$cardId = (int)$this->request->getParam('cardId');
130+
$card = $this->cardService->find($cardId);
131+
$this->changeHelper->checkIfMatch(ChangeHelper::TYPE_CARD, $cardId, $card->getETag());
132+
$card = $this->cardService->assignLabel($cardId, $labelId);
117133
return new DataResponse($card, HTTP::STATUS_OK);
118134
}
119135

@@ -124,7 +140,10 @@ public function assignLabel(int $labelId): DataResponse {
124140
#[CORS]
125141
#[NoCSRFRequired]
126142
public function removeLabel(int $labelId): DataResponse {
127-
$card = $this->cardService->removeLabel($this->request->getParam('cardId'), $labelId);
143+
$cardId = (int)$this->request->getParam('cardId');
144+
$card = $this->cardService->find($cardId);
145+
$this->changeHelper->checkIfMatch(ChangeHelper::TYPE_CARD, $cardId, $card->getETag());
146+
$card = $this->cardService->removeLabel($cardId, $labelId);
128147
return new DataResponse($card, HTTP::STATUS_OK);
129148
}
130149

@@ -135,6 +154,8 @@ public function removeLabel(int $labelId): DataResponse {
135154
#[CORS]
136155
#[NoCSRFRequired]
137156
public function assignUser(int $cardId, string $userId, int $type = 0): DataResponse {
157+
$card = $this->cardService->find($cardId);
158+
$this->changeHelper->checkIfMatch(ChangeHelper::TYPE_CARD, $cardId, $card->getETag());
138159
$card = $this->assignmentService->assignUser($cardId, $userId, $type);
139160
return new DataResponse($card, HTTP::STATUS_OK);
140161
}
@@ -146,6 +167,8 @@ public function assignUser(int $cardId, string $userId, int $type = 0): DataResp
146167
#[CORS]
147168
#[NoCSRFRequired]
148169
public function unassignUser(int $cardId, string $userId, int $type = 0): DataResponse {
170+
$card = $this->cardService->find($cardId);
171+
$this->changeHelper->checkIfMatch(ChangeHelper::TYPE_CARD, $cardId, $card->getETag());
149172
$card = $this->assignmentService->unassignUser($cardId, $userId, $type);
150173
return new DataResponse($card, HTTP::STATUS_OK);
151174
}
@@ -179,6 +202,8 @@ public function removeDependentCard(int $cardId, int $dependentCardId): DataResp
179202
#[CORS]
180203
#[NoCSRFRequired]
181204
public function archive(int $cardId): DataResponse {
205+
$card = $this->cardService->find($cardId);
206+
$this->changeHelper->checkIfMatch(ChangeHelper::TYPE_CARD, $cardId, $card->getETag());
182207
$card = $this->cardService->archive($cardId);
183208
return new DataResponse($card, HTTP::STATUS_OK);
184209
}
@@ -190,6 +215,8 @@ public function archive(int $cardId): DataResponse {
190215
#[CORS]
191216
#[NoCSRFRequired]
192217
public function unarchive(int $cardId): DataResponse {
218+
$card = $this->cardService->find($cardId);
219+
$this->changeHelper->checkIfMatch(ChangeHelper::TYPE_CARD, $cardId, $card->getETag());
193220
$card = $this->cardService->unarchive($cardId);
194221
return new DataResponse($card, HTTP::STATUS_OK);
195222
}
@@ -201,7 +228,10 @@ public function unarchive(int $cardId): DataResponse {
201228
#[CORS]
202229
#[NoCSRFRequired]
203230
public function reorder(int $stackId, int $order): DataResponse {
204-
$card = $this->cardService->reorder((int)$this->request->getParam('cardId'), $stackId, $order);
231+
$cardId = (int)$this->request->getParam('cardId');
232+
$card = $this->cardService->find($cardId);
233+
$this->changeHelper->checkIfMatch(ChangeHelper::TYPE_CARD, $cardId, $card->getETag());
234+
$card = $this->cardService->reorder($cardId, $stackId, $order);
205235
return new DataResponse($card, HTTP::STATUS_OK);
206236
}
207237
}

‎lib/Controller/LabelApiController.php‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
namespace OCA\Deck\Controller;
99

10+
use OCA\Deck\Db\ChangeHelper;
1011
use OCA\Deck\Service\LabelService;
1112
use OCP\AppFramework\ApiController;
1213
use OCP\AppFramework\Http;
@@ -29,6 +30,7 @@ public function __construct(
2930
$appName,
3031
IRequest $request,
3132
private LabelService $labelService,
33+
private ChangeHelper $changeHelper,
3234
) {
3335
parent::__construct($appName, $request);
3436
}
@@ -40,8 +42,15 @@ public function __construct(
4042
#[NoCSRFRequired]
4143
#[CORS]
4244
public function get(): DataResponse {
43-
$label = $this->labelService->find($this->request->getParam('labelId'));
44-
return new DataResponse($label, HTTP::STATUS_OK);
45+
$labelId = (int)$this->request->getParam('labelId');
46+
$label = $this->labelService->find($labelId);
47+
$response = new DataResponse($label, HTTP::STATUS_OK);
48+
$etag = $this->changeHelper->getEtag(ChangeHelper::TYPE_LABEL, $labelId);
49+
if ($etag === '') {
50+
$etag = $label->getETag();
51+
}
52+
$response->setETag($etag);
53+
return $response;
4554
}
4655

4756
/**
@@ -62,7 +71,10 @@ public function create(string $title, string $color): DataResponse {
6271
#[NoCSRFRequired]
6372
#[CORS]
6473
public function update(string $title, string $color): DataResponse {
65-
$label = $this->labelService->update($this->request->getParam('labelId'), $title, $color);
74+
$labelId = (int)$this->request->getParam('labelId');
75+
$label = $this->labelService->find($labelId);
76+
$this->changeHelper->checkIfMatch(ChangeHelper::TYPE_LABEL, $labelId, $label->getETag());
77+
$label = $this->labelService->update($labelId, $title, $color);
6678
return new DataResponse($label, HTTP::STATUS_OK);
6779
}
6880

@@ -73,7 +85,10 @@ public function update(string $title, string $color): DataResponse {
7385
#[NoCSRFRequired]
7486
#[CORS]
7587
public function delete(): DataResponse {
76-
$label = $this->labelService->delete($this->request->getParam('labelId'));
88+
$labelId = (int)$this->request->getParam('labelId');
89+
$label = $this->labelService->find($labelId);
90+
$this->changeHelper->checkIfMatch(ChangeHelper::TYPE_LABEL, $labelId, $label->getETag());
91+
$label = $this->labelService->delete($labelId);
7792
return new DataResponse($label, HTTP::STATUS_OK);
7893
}
7994
}

‎lib/Controller/StackApiController.php‎

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77

88
namespace OCA\Deck\Controller;
99

10+
use OCA\Deck\Db\ChangeHelper;
11+
use OCA\Deck\Db\Stack;
1012
use OCA\Deck\Service\StackService;
1113
use OCA\Deck\StatusException;
1214
use OCP\AppFramework\ApiController;
@@ -31,6 +33,7 @@ public function __construct(
3133
$appName,
3234
IRequest $request,
3335
private StackService $stackService,
36+
private ChangeHelper $changeHelper,
3437
) {
3538
parent::__construct($appName, $request);
3639
}
@@ -52,7 +55,15 @@ public function index(): DataResponse {
5255
$since = $date->getTimestamp();
5356
}
5457
$stacks = $this->stackService->findAll($this->request->getParam('boardId'), $since);
55-
return new DataResponse($stacks, HTTP::STATUS_OK);
58+
$response = new DataResponse($stacks, HTTP::STATUS_OK);
59+
$response->setETag(md5(json_encode(array_map(function (Stack $stack) {
60+
$etag = $this->changeHelper->getEtag(ChangeHelper::TYPE_STACK, $stack->getId());
61+
if ($etag === '') {
62+
$etag = $stack->getETag();
63+
}
64+
return $stack->getId() . '-' . $etag;
65+
}, $stacks))));
66+
return $response;
5667
}
5768

5869
/**
@@ -62,9 +73,14 @@ public function index(): DataResponse {
6273
#[CORS]
6374
#[NoCSRFRequired]
6475
public function get(): DataResponse {
65-
$stack = $this->stackService->find($this->request->getParam('stackId'));
76+
$stackId = (int)$this->request->getParam('stackId');
77+
$stack = $this->stackService->find($stackId);
6678
$response = new DataResponse($stack, HTTP::STATUS_OK);
67-
$response->setETag($stack->getETag());
79+
$etag = $this->changeHelper->getEtag(ChangeHelper::TYPE_STACK, $stackId);
80+
if ($etag === '') {
81+
$etag = $stack->getETag();
82+
}
83+
$response->setETag($etag);
6884
return $response;
6985
}
7086

@@ -86,7 +102,10 @@ public function create(string $title, int $order): DataResponse {
86102
#[CORS]
87103
#[NoCSRFRequired]
88104
public function update(string $title, int $order) {
89-
$stack = $this->stackService->update($this->request->getParam('stackId'), $title, $this->request->getParam('boardId'), $order, 0);
105+
$stackId = (int)$this->request->getParam('stackId');
106+
$stack = $this->stackService->find($stackId);
107+
$this->changeHelper->checkIfMatch(ChangeHelper::TYPE_STACK, $stackId, $stack->getETag());
108+
$stack = $this->stackService->update($stackId, $title, $this->request->getParam('boardId'), $order, 0);
90109
return new DataResponse($stack, HTTP::STATUS_OK);
91110
}
92111

@@ -97,7 +116,10 @@ public function update(string $title, int $order) {
97116
#[CORS]
98117
#[NoCSRFRequired]
99118
public function delete(): DataResponse {
100-
$stack = $this->stackService->delete($this->request->getParam('stackId'));
119+
$stackId = (int)$this->request->getParam('stackId');
120+
$stack = $this->stackService->find($stackId);
121+
$this->changeHelper->checkIfMatch(ChangeHelper::TYPE_STACK, $stackId, $stack->getETag());
122+
$stack = $this->stackService->delete($stackId);
101123
return new DataResponse($stack, HTTP::STATUS_OK);
102124
}
103125

‎lib/Db/BoardMapper.php‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -471,6 +471,20 @@ public function findAll(): array {
471471
return $this->findEntities($qb);
472472
}
473473

474+
public function insert(Entity $entity): Entity {
475+
if (!isset($entity->getUpdatedFields()['lastModified'])) {
476+
$entity->setLastModified(time());
477+
}
478+
return parent::insert($entity);
479+
}
480+
481+
public function update(Entity $entity): Entity {
482+
$entity->setLastModified(time());
483+
$result = parent::update($entity);
484+
$this->boardCache[(string)$entity->getId()] = $result;
485+
return $result;
486+
}
487+
474488
public function findToDelete(int $timeLimit) {
475489
$qb = $this->db->getQueryBuilder();
476490
$qb->select('id', 'title', 'owner', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token')

0 commit comments

Comments
 (0)