Skip to content
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.

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

ETag support for API endpoints #2245

Merged
merged 6 commits into from
Nov 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 25 additions & 0 deletions docs/API.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,31 @@ curl -u admin:admin -X GET \
-H "If-Modified-Since: Mon, 05 Nov 2018 09:28:00 GMT"
```

### ETag

An ETag header is returned in order to determine if further child elements have been updated for the following endpoints:

- Fetch all user board `GET /api/v1.0/boards`
- Fetch a single board `GET /api/v1.0/boards/{boardId}`
- Fetch all stacks of a board `GET /api/v1.0/boards/{boardId}/stacks`
- Fetch a single stacks of a board `GET /api/v1.0/boards/{boardId}/stacks/{stackId}`
- Fetch a single card of a board `GET /api/v1.0/boards/{boardId}/stacks/{stackId}/cards/{cardId}`
- Fetch attachments of a card `GET /api/v1.0/boards/{boardId}/stacks/{stackId}/cards/{cardId}/attachments`

If a `If-None-Match` header is provided and the requested element has not changed a `304` Not Modified response will be returned.

Changes of child elements will propagate to their parents and also cause an update of the ETag which will be useful for determining if a sync is necessary on any client integration side. As an example, if a label is added to a card, the ETag of all related entities (the card, stack and board) will change.

If available the ETag will also be part of JSON response objects as shown below for a card:

```json
{
"id": 81,
"ETag": "bdb10fa2d2aeda092a2b6b469454dc90",
"title": "Test card"
}
```

# Changelog

## 1.0.0 (unreleased)
Expand Down
9 changes: 8 additions & 1 deletion lib/Activity/CommentEventHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
namespace OCA\Deck\Activity;

use OCA\Deck\Db\CardMapper;
use OCA\Deck\Db\ChangeHelper;
use OCA\Deck\Notification\NotificationHelper;
use OCP\Comments\CommentsEvent;
use OCP\Comments\IComment;
Expand All @@ -40,10 +41,14 @@ class CommentEventHandler implements ICommentsEventHandler {
/** @var CardMapper */
private $cardMapper;

public function __construct(ActivityManager $activityManager, NotificationHelper $notificationHelper, CardMapper $cardMapper) {
/** @var ChangeHelper */
private $changeHelper;

public function __construct(ActivityManager $activityManager, NotificationHelper $notificationHelper, CardMapper $cardMapper, ChangeHelper $changeHelper) {
$this->notificationHelper = $notificationHelper;
$this->activityManager = $activityManager;
$this->cardMapper = $cardMapper;
$this->changeHelper = $changeHelper;
}

/**
Expand All @@ -54,6 +59,8 @@ public function handle(CommentsEvent $event) {
return;
}

$this->changeHelper->cardChanged($event->getComment()->getObjectId());

$eventType = $event->getEvent();
if ($eventType === CommentsEvent::EVENT_ADD
) {
Expand Down
11 changes: 9 additions & 2 deletions lib/Controller/BoardApiController.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

namespace OCA\Deck\Controller;

use OCA\Deck\Db\Board;
use OCA\Deck\StatusException;
use OCP\AppFramework\ApiController;
use OCP\AppFramework\Http;
Expand Down Expand Up @@ -72,7 +73,11 @@ public function index($details = null) {
}
$boards = $this->boardService->findAll($date->getTimestamp(), $details);
}
return new DataResponse($boards, HTTP::STATUS_OK);
$response = new DataResponse($boards, HTTP::STATUS_OK);
$response->setETag(md5(json_encode(array_map(function (Board $board) {
return $board->getId() . '-' . $board->getETag();
}, $boards))));
return $response;
}

/**
Expand All @@ -85,7 +90,9 @@ public function index($details = null) {
*/
public function get() {
$board = $this->boardService->find($this->request->getParam('boardId'));
return new DataResponse($board, HTTP::STATUS_OK);
$response = new DataResponse($board, HTTP::STATUS_OK);
$response->setETag($board->getEtag());
return $response;
}

/**
Expand Down
4 changes: 3 additions & 1 deletion lib/Controller/CardApiController.php
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,9 @@ public function __construct($appName, IRequest $request, CardService $cardServic
*/
public function get() {
$card = $this->cardService->find($this->request->getParam('cardId'));
return new DataResponse($card, HTTP::STATUS_OK);
$response = new DataResponse($card, HTTP::STATUS_OK);
$response->setETag($card->getEtag());
return $response;
}

/**
Expand Down
4 changes: 3 additions & 1 deletion lib/Controller/StackApiController.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,9 @@ public function index() {
*/
public function get() {
$stack = $this->stackService->find($this->request->getParam('stackId'));
return new DataResponse($stack, HTTP::STATUS_OK);
$response = new DataResponse($stack, HTTP::STATUS_OK);
$response->setETag($stack->getETag());
return $response;
}

/**
Expand Down
4 changes: 4 additions & 0 deletions lib/Db/Board.php
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,8 @@ public function setAcl($acl) {
$this->acl[] = $a;
}
}

public function getETag() {
return md5((string)$this->getLastModified());
}
}
4 changes: 4 additions & 0 deletions lib/Db/Card.php
Original file line number Diff line number Diff line change
Expand Up @@ -155,4 +155,8 @@ public function getCalendarObject(): VCalendar {
public function getCalendarPrefix(): string {
return 'card';
}

public function getETag() {
return md5((string)$this->getLastModified());
}
}
4 changes: 4 additions & 0 deletions lib/Db/Label.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,8 @@ public function __construct() {
$this->addType('cardId', 'integer');
$this->addType('lastModified', 'integer');
}

public function getETag() {
return md5((string)$this->getLastModified());
}
}
3 changes: 3 additions & 0 deletions lib/Db/RelationalEntity.php
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ public function jsonSerialize() {
$json[$property] = $value;
}
}
if ($reflection->hasMethod('getETag')) {
$json['ETag'] = $this->getETag();
}
return $json;
}

Expand Down
4 changes: 4 additions & 0 deletions lib/Db/Stack.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,4 +65,8 @@ public function getCalendarObject(): VCalendar {
public function getCalendarPrefix(): string {
return 'stack';
}

public function getETag() {
return md5((string)$this->getLastModified());
}
}
4 changes: 2 additions & 2 deletions lib/Service/AssignmentService.php
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ public function assignUser($cardId, $userId, int $type = AssignedUsers::TYPE_USE
$assignment->setParticipant($userId);
$assignment->setType($type);
$assignment = $this->assignedUsersMapper->insert($assignment);
$this->changeHelper->cardChanged($cardId, false);
$this->changeHelper->cardChanged($cardId);
$this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_CARD, $card, ActivityManager::SUBJECT_CARD_USER_ASSIGN, ['assigneduser' => $userId]);

$this->eventDispatcher->dispatch(
Expand Down Expand Up @@ -185,7 +185,7 @@ public function unassignUser($cardId, $userId, $type = 0) {
$assignment = $this->assignedUsersMapper->delete($assignment);
$card = $this->cardMapper->find($cardId);
$this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_CARD, $card, ActivityManager::SUBJECT_CARD_USER_UNASSIGN, ['assigneduser' => $userId]);
$this->changeHelper->cardChanged($cardId, false);
$this->changeHelper->cardChanged($cardId);

$this->eventDispatcher->dispatch(
'\OCA\Deck\Card::onUpdate', new FTSEvent(null, ['id' => $cardId, 'card' => $card])
Expand Down
1 change: 1 addition & 0 deletions lib/Service/AttachmentService.php
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,7 @@ public function delete($attachmentId) {
if ($service->allowUndo()) {
$service->markAsDeleted($attachment);
$this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_CARD, $attachment, ActivityManager::SUBJECT_ATTACHMENT_DELETE);
$this->changeHelper->cardChanged($attachment->getCardId());
return $this->attachmentMapper->update($attachment);
}
$service->delete($attachment);
Expand Down
4 changes: 2 additions & 2 deletions lib/Service/CardService.php
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,7 @@ public function assignLabel($cardId, $labelId) {
}
$label = $this->labelMapper->find($labelId);
$this->cardMapper->assignLabel($cardId, $labelId);
$this->changeHelper->cardChanged($cardId, false);
$this->changeHelper->cardChanged($cardId);
$this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_CARD, $card, ActivityManager::SUBJECT_LABEL_ASSIGN, ['label' => $label]);

$this->eventDispatcher->dispatch(
Expand Down Expand Up @@ -574,7 +574,7 @@ public function removeLabel($cardId, $labelId) {
}
$label = $this->labelMapper->find($labelId);
$this->cardMapper->removeLabel($cardId, $labelId);
$this->changeHelper->cardChanged($cardId, false);
$this->changeHelper->cardChanged($cardId);
$this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_CARD, $card, ActivityManager::SUBJECT_LABEL_UNASSING, ['label' => $label]);

$this->eventDispatcher->dispatch(
Expand Down
5 changes: 4 additions & 1 deletion tests/unit/Activity/CommentEventHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

use OCA\Deck\Db\Card;
use OCA\Deck\Db\CardMapper;
use OCA\Deck\Db\ChangeHelper;
use OCA\Deck\Notification\NotificationHelper;
use OCP\Comments\CommentsEvent;
use OCP\Comments\IComment;
Expand All @@ -45,10 +46,12 @@ public function setUp(): void {
$this->activityManager = $this->createMock(ActivityManager::class);
$this->notificationHelper = $this->createMock(NotificationHelper::class);
$this->cardMapper = $this->createMock(CardMapper::class);
$this->changeHelper = $this->createMock(ChangeHelper::class);
$this->commentEventHandler = new CommentEventHandler(
$this->activityManager,
$this->notificationHelper,
$this->cardMapper
$this->cardMapper,
$this->changeHelper
);
}

Expand Down
3 changes: 3 additions & 0 deletions tests/unit/Db/BoardTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public function testJsonSerialize() {
'archived' => false,
'users' => ['user1', 'user2'],
'settings' => [],
'ETag' => $board->getETag(),
], $board->jsonSerialize());
}

Expand All @@ -52,6 +53,7 @@ public function testSetLabels() {
'archived' => false,
'users' => [],
'settings' => [],
'ETag' => $board->getETag(),
], $board->jsonSerialize());
}
public function testSetAcl() {
Expand Down Expand Up @@ -80,6 +82,7 @@ public function testSetShared() {
'shared' => 1,
'users' => [],
'settings' => [],
'ETag' => $board->getETag(),
], $board->jsonSerialize());
}
}
3 changes: 3 additions & 0 deletions tests/unit/Db/CardTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ public function testJsonSerialize() {
'deletedAt' => 0,
'commentsUnread' => 0,
'lastEditor' => null,
'ETag' => $card->getETag(),
], $card->jsonSerialize());
}
public function testJsonSerializeLabels() {
Expand All @@ -109,6 +110,7 @@ public function testJsonSerializeLabels() {
'deletedAt' => 0,
'commentsUnread' => 0,
'lastEditor' => null,
'ETag' => $card->getETag(),
], $card->jsonSerialize());
}

Expand Down Expand Up @@ -144,6 +146,7 @@ public function testJsonSerializeAsignedUsers() {
'deletedAt' => 0,
'commentsUnread' => 0,
'lastEditor' => null,
'ETag' => $card->getETag(),
], $card->jsonSerialize());
}
}
6 changes: 4 additions & 2 deletions tests/unit/Db/LabelTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ public function testJsonSerializeBoard() {
'boardId' => 123,
'cardId' => null,
'lastModified' => null,
'color' => '000000'
'color' => '000000',
'ETag' => $label->getETag(),
], $label->jsonSerialize());
}
public function testJsonSerializeCard() {
Expand All @@ -54,7 +55,8 @@ public function testJsonSerializeCard() {
'boardId' => null,
'cardId' => 123,
'lastModified' => null,
'color' => '000000'
'color' => '000000',
'ETag' => $label->getETag(),
], $label->jsonSerialize());
}
}
12 changes: 7 additions & 5 deletions tests/unit/Db/StackTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,21 @@ private function createStack() {
return $board;
}
public function testJsonSerialize() {
$board = $this->createStack();
$stack = $this->createStack();
$this->assertEquals([
'id' => 1,
'title' => "My Stack",
'order' => 1,
'boardId' => 1,
'deletedAt' => 0,
'lastModified' => 0,
], $board->jsonSerialize());
'ETag' => $stack->getETag(),
], $stack->jsonSerialize());
}
public function testJsonSerializeWithCards() {
$cards = ["foo", "bar"];
$board = $this->createStack();
$board->setCards($cards);
$stack = $this->createStack();
$stack->setCards($cards);
$this->assertEquals([
'id' => 1,
'title' => "My Stack",
Expand All @@ -55,6 +56,7 @@ public function testJsonSerializeWithCards() {
'cards' => ["foo", "bar"],
'deletedAt' => 0,
'lastModified' => 0,
], $board->jsonSerialize());
'ETag' => $stack->getETag(),
], $stack->jsonSerialize());
}
}
3 changes: 2 additions & 1 deletion tests/unit/controller/BoardApiControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public function testIndex() {

$expected = new DataResponse($boards, HTTP::STATUS_OK);
$actual = $this->controller->index();

$actual->setETag(null);
$this->assertEquals($expected, $actual);
}

Expand All @@ -90,6 +90,7 @@ public function testGet() {
->will($this->returnValue($boardId));

$expected = new DataResponse($board, HTTP::STATUS_OK);
$expected->setETag($board->getETag());
$actual = $this->controller->get();
$this->assertEquals($expected, $actual);
}
Expand Down
1 change: 1 addition & 0 deletions tests/unit/controller/CardApiControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ public function testGet() {
->willReturn($card);

$expected = new DataResponse($card, HTTP::STATUS_OK);
$expected->setETag($card->getETag());
$actual = $this->controller->get();
$this->assertEquals($expected, $actual);
}
Expand Down
2 changes: 2 additions & 0 deletions tests/unit/controller/StackApiControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ public function testIndex() {

$expected = new DataResponse($stacks, HTTP::STATUS_OK);
$actual = $this->controller->index();
$actual->setETag(null);
$this->assertEquals($expected, $actual);
}

Expand All @@ -97,6 +98,7 @@ public function testGet() {
->willReturn($this->exampleStack['id']);

$expected = new DataResponse($stack, HTTP::STATUS_OK);
$expected->setETag($stack->getETag());
$actual = $this->controller->get();
$this->assertEquals($expected, $actual);
}
Expand Down