From b74efb9734e349c1673eb7aa9710d563b49b0ffe Mon Sep 17 00:00:00 2001 From: Michal Polacik Date: Fri, 27 Aug 2021 23:26:25 +0200 Subject: [PATCH 1/8] Permanently delete deck cards marked as deleted after 5 min in a cron job --- lib/Cron/DeleteCron.php | 10 +++++++++- lib/Db/CardMapper.php | 8 ++++++++ tests/unit/Cron/DeleteCronTest.php | 19 ++++++++++++++++++- 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/lib/Cron/DeleteCron.php b/lib/Cron/DeleteCron.php index 49a26d136..c87d7f290 100644 --- a/lib/Cron/DeleteCron.php +++ b/lib/Cron/DeleteCron.php @@ -34,13 +34,16 @@ class DeleteCron extends Job { /** @var BoardMapper */ private $boardMapper; + /** @var CardMapper */ + private $cardMapper; /** @var AttachmentService */ private $attachmentService; /** @var AttachmentMapper */ private $attachmentMapper; - public function __construct(BoardMapper $boardMapper, AttachmentService $attachmentService, AttachmentMapper $attachmentMapper) { + public function __construct(BoardMapper $boardMapper, CardMapper $cardMapper, AttachmentService $attachmentService, AttachmentMapper $attachmentMapper) { $this->boardMapper = $boardMapper; + $this->cardMapper = $cardMapper; $this->attachmentService = $attachmentService; $this->attachmentMapper = $attachmentMapper; } @@ -55,6 +58,11 @@ protected function run($argument) { $this->boardMapper->delete($board); } + $cards = $this->cardMapper->findToDelete(); + foreach ($cards as $card) { + $this->cardMapper->delete($card); + } + $attachments = $this->attachmentMapper->findToDelete(); foreach ($attachments as $attachment) { try { diff --git a/lib/Db/CardMapper.php b/lib/Db/CardMapper.php index e416d1fb9..1d298edda 100644 --- a/lib/Db/CardMapper.php +++ b/lib/Db/CardMapper.php @@ -165,6 +165,14 @@ public function queryCardsByBoards(array $boardIds): IQueryBuilder { return $qb; } + public function findToDelete() { + // add buffer of 5 min + $timeLimit = time() - (60 * 5); + $sql = 'SELECT id, title, owner, archived, deleted_at, last_modified FROM `*PREFIX*deck_cards` ' . + 'WHERE `deleted_at` > 0 AND `deleted_at` < ?'; + return $this->findEntities($sql, [$timeLimit]); + } + public function findDeleted($boardId, $limit = null, $offset = null) { $qb = $this->queryCardsByBoard($boardId); $qb->andWhere($qb->expr()->neq('c.deleted_at', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))) diff --git a/tests/unit/Cron/DeleteCronTest.php b/tests/unit/Cron/DeleteCronTest.php index e7ba1c6b9..774ac298d 100644 --- a/tests/unit/Cron/DeleteCronTest.php +++ b/tests/unit/Cron/DeleteCronTest.php @@ -35,6 +35,8 @@ class DeleteCronTest extends \Test\TestCase { /** @var BoardMapper|\PHPUnit\Framework\MockObject\MockObject */ protected $boardMapper; + /** @var CardMapper|\PHPUnit\Framework\MockObject\MockObject */ + protected $cardMapper; /** @var AttachmentService|\PHPUnit\Framework\MockObject\MockObject */ private $attachmentService; /** @var AttachmentMapper|\PHPUnit\Framework\MockObject\MockObject */ @@ -45,9 +47,10 @@ class DeleteCronTest extends \Test\TestCase { public function setUp(): void { parent::setUp(); $this->boardMapper = $this->createMock(BoardMapper::class); + $this->cardMapper = $this->createMock(CardMapper::class); $this->attachmentService = $this->createMock(AttachmentService::class); $this->attachmentMapper = $this->createMock(AttachmentMapper::class); - $this->deleteCron = new DeleteCron($this->boardMapper, $this->attachmentService, $this->attachmentMapper); + $this->deleteCron = new DeleteCron($this->boardMapper, $this->cardMapper, $this->attachmentService, $this->attachmentMapper); } protected function getBoard($id) { @@ -56,6 +59,12 @@ protected function getBoard($id) { return $board; } + protected function getCard($id) { + $card = new Card(); + $card->setId($id); + return $card; + } + public function testDeleteCron() { $boards = [ $this->getBoard(1), @@ -79,6 +88,14 @@ public function testDeleteCron() { ->method('delete') ->with($boards[3]); + $cards = [ $this->getBoard(10) ]; + $this->cardMapper->expects($this->once()) + ->method('findToDelete') + ->willReturn($cards); + $this->cardMapper->expects($this->once()) + ->method('delete') + ->with($cards[0]); + $attachment = new Attachment(); $attachment->setType('deck_file'); $this->attachmentMapper->expects($this->once()) From 552418dcd66d662b9815c5f6c1ba4ed00bd41dd4 Mon Sep 17 00:00:00 2001 From: Michal Polacik Date: Wed, 15 Sep 2021 19:04:32 +0200 Subject: [PATCH 2/8] Limit deleted cards in one cron job run to 500 --- lib/Cron/DeleteCron.php | 9 +++++---- lib/Db/CardMapper.php | 15 +++++++++------ 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/lib/Cron/DeleteCron.php b/lib/Cron/DeleteCron.php index c87d7f290..9d4eb7410 100644 --- a/lib/Cron/DeleteCron.php +++ b/lib/Cron/DeleteCron.php @@ -58,10 +58,11 @@ protected function run($argument) { $this->boardMapper->delete($board); } - $cards = $this->cardMapper->findToDelete(); - foreach ($cards as $card) { - $this->cardMapper->delete($card); - } + $timeLimit = time() - (60 * 5); // 5 min buffer + $cards = $this->cardMapper->findToDelete($timeLimit, 500); + foreach ($cards as $card) { + $this->cardMapper->delete($card); + } $attachments = $this->attachmentMapper->findToDelete(); foreach ($attachments as $attachment) { diff --git a/lib/Db/CardMapper.php b/lib/Db/CardMapper.php index 1d298edda..199469595 100644 --- a/lib/Db/CardMapper.php +++ b/lib/Db/CardMapper.php @@ -165,12 +165,15 @@ public function queryCardsByBoards(array $boardIds): IQueryBuilder { return $qb; } - public function findToDelete() { - // add buffer of 5 min - $timeLimit = time() - (60 * 5); - $sql = 'SELECT id, title, owner, archived, deleted_at, last_modified FROM `*PREFIX*deck_cards` ' . - 'WHERE `deleted_at` > 0 AND `deleted_at` < ?'; - return $this->findEntities($sql, [$timeLimit]); + public function findToDelete($timeLimit, $limit = null) { + $qb = $this->db->getQueryBuilder(); + $qb->select('id','title','owner','archived','deleted_at','last_modified') + ->from('deck_cards') + ->where($qb->expr()->gt('deleted_at', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->lt('deleted_at', $qb->createNamedParameter($timeLimit, IQueryBuilder::PARAM_INT_ARRAY))) + ->orderBy('deleted_at') + ->setMaxResults($limit); + return $qb; } public function findDeleted($boardId, $limit = null, $offset = null) { From ce5ca44afaf436ff748083c61ce8a03250b2055f Mon Sep 17 00:00:00 2001 From: Michal Polacik Date: Wed, 6 Oct 2021 16:22:19 +0200 Subject: [PATCH 3/8] Converted spaces to tabs --- lib/Db/CardMapper.php | 14 +++++++------- tests/unit/Cron/DeleteCronTest.php | 18 +++++++++--------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/lib/Db/CardMapper.php b/lib/Db/CardMapper.php index 199469595..9c3708aea 100644 --- a/lib/Db/CardMapper.php +++ b/lib/Db/CardMapper.php @@ -167,13 +167,13 @@ public function queryCardsByBoards(array $boardIds): IQueryBuilder { public function findToDelete($timeLimit, $limit = null) { $qb = $this->db->getQueryBuilder(); - $qb->select('id','title','owner','archived','deleted_at','last_modified') - ->from('deck_cards') - ->where($qb->expr()->gt('deleted_at', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))) - ->andWhere($qb->expr()->lt('deleted_at', $qb->createNamedParameter($timeLimit, IQueryBuilder::PARAM_INT_ARRAY))) - ->orderBy('deleted_at') - ->setMaxResults($limit); - return $qb; + $qb->select('id','title','owner','archived','deleted_at','last_modified') + ->from('deck_cards') + ->where($qb->expr()->gt('deleted_at', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->lt('deleted_at', $qb->createNamedParameter($timeLimit, IQueryBuilder::PARAM_INT_ARRAY))) + ->orderBy('deleted_at') + ->setMaxResults($limit); + return $qb; } public function findDeleted($boardId, $limit = null, $offset = null) { diff --git a/tests/unit/Cron/DeleteCronTest.php b/tests/unit/Cron/DeleteCronTest.php index 774ac298d..7ef8e81c5 100644 --- a/tests/unit/Cron/DeleteCronTest.php +++ b/tests/unit/Cron/DeleteCronTest.php @@ -35,8 +35,8 @@ class DeleteCronTest extends \Test\TestCase { /** @var BoardMapper|\PHPUnit\Framework\MockObject\MockObject */ protected $boardMapper; - /** @var CardMapper|\PHPUnit\Framework\MockObject\MockObject */ - protected $cardMapper; + /** @var CardMapper|\PHPUnit\Framework\MockObject\MockObject */ + protected $cardMapper; /** @var AttachmentService|\PHPUnit\Framework\MockObject\MockObject */ private $attachmentService; /** @var AttachmentMapper|\PHPUnit\Framework\MockObject\MockObject */ @@ -88,13 +88,13 @@ public function testDeleteCron() { ->method('delete') ->with($boards[3]); - $cards = [ $this->getBoard(10) ]; - $this->cardMapper->expects($this->once()) - ->method('findToDelete') - ->willReturn($cards); - $this->cardMapper->expects($this->once()) - ->method('delete') - ->with($cards[0]); + $cards = [ $this->getBoard(10) ]; + $this->cardMapper->expects($this->once()) + ->method('findToDelete') + ->willReturn($cards); + $this->cardMapper->expects($this->once()) + ->method('delete') + ->with($cards[0]); $attachment = new Attachment(); $attachment->setType('deck_file'); From e5b3ae0ae12b29d3c2fd11dd3e1b286fe9667511 Mon Sep 17 00:00:00 2001 From: Michal Polacik Date: Mon, 8 Nov 2021 17:56:56 +0100 Subject: [PATCH 4/8] Added missing import for CardMapper class --- tests/unit/Cron/DeleteCronTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/Cron/DeleteCronTest.php b/tests/unit/Cron/DeleteCronTest.php index 7ef8e81c5..534ddc76b 100644 --- a/tests/unit/Cron/DeleteCronTest.php +++ b/tests/unit/Cron/DeleteCronTest.php @@ -27,6 +27,7 @@ use OCA\Deck\Db\AttachmentMapper; use OCA\Deck\Db\Board; use OCA\Deck\Db\BoardMapper; +use OCA\Deck\Db\CardMapper; use OCA\Deck\InvalidAttachmentType; use OCA\Deck\Service\AttachmentService; use OCA\Deck\Service\IAttachmentService; From 49b3289f53c26e8d46650cbafc9ca319f96f7893 Mon Sep 17 00:00:00 2001 From: Michal Polacik Date: Mon, 8 Nov 2021 18:16:33 +0100 Subject: [PATCH 5/8] Added another missing import for CardMapper class --- lib/Cron/DeleteCron.php | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/Cron/DeleteCron.php b/lib/Cron/DeleteCron.php index 9d4eb7410..d49be6e40 100644 --- a/lib/Cron/DeleteCron.php +++ b/lib/Cron/DeleteCron.php @@ -27,6 +27,7 @@ use OC\BackgroundJob\Job; use OCA\Deck\Db\AttachmentMapper; use OCA\Deck\Db\BoardMapper; +use OCA\Deck\Db\CardMapper; use OCA\Deck\InvalidAttachmentType; use OCA\Deck\Service\AttachmentService; From 31f99aa9d0373ed502b9aeeaea557cc992d750a1 Mon Sep 17 00:00:00 2001 From: Michal Polacik Date: Wed, 10 Nov 2021 21:48:02 +0100 Subject: [PATCH 6/8] Fixed response object in findToDelete method + fixed 2 misspellings in API.md --- docs/API.md | 4 ++-- lib/Db/CardMapper.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/API.md b/docs/API.md index 376720a50..02c77a8e0 100644 --- a/docs/API.md +++ b/docs/API.md @@ -1,7 +1,7 @@ The REST API provides access for authenticated users to their data inside the Deck app. To get a better understanding of Decks data models and their relations, please have a look at the [data structure](structure.md) documentation. -# Prequisited +# Prerequisites - All requests require a `OCS-APIRequest` HTTP header to be set to `true` and a `Content-Type` of `application/json`. - The API is located at https://nextcloud.local/index.php/apps/deck/api/v1.0 @@ -9,7 +9,7 @@ The REST API provides access for authenticated users to their data inside the De ## Naming -- Board is the the project like grouping of tasks that can be shared to different users and groups +- Board is the project like grouping of tasks that can be shared to different users and groups - Stack is the grouping of cards which is rendered in vertical columns in the UI diff --git a/lib/Db/CardMapper.php b/lib/Db/CardMapper.php index 9c3708aea..3f8a029a0 100644 --- a/lib/Db/CardMapper.php +++ b/lib/Db/CardMapper.php @@ -173,7 +173,7 @@ public function findToDelete($timeLimit, $limit = null) { ->andWhere($qb->expr()->lt('deleted_at', $qb->createNamedParameter($timeLimit, IQueryBuilder::PARAM_INT_ARRAY))) ->orderBy('deleted_at') ->setMaxResults($limit); - return $qb; + return $this->findEntities($qb); } public function findDeleted($boardId, $limit = null, $offset = null) { From b34434e8c6947988085a1cd0c234444f9ac6e621 Mon Sep 17 00:00:00 2001 From: Michal Polacik Date: Mon, 24 Jan 2022 17:16:04 +0100 Subject: [PATCH 7/8] Fixed invalid parameter type --- lib/Db/CardMapper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Db/CardMapper.php b/lib/Db/CardMapper.php index 3f8a029a0..d07acabde 100644 --- a/lib/Db/CardMapper.php +++ b/lib/Db/CardMapper.php @@ -170,7 +170,7 @@ public function findToDelete($timeLimit, $limit = null) { $qb->select('id','title','owner','archived','deleted_at','last_modified') ->from('deck_cards') ->where($qb->expr()->gt('deleted_at', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))) - ->andWhere($qb->expr()->lt('deleted_at', $qb->createNamedParameter($timeLimit, IQueryBuilder::PARAM_INT_ARRAY))) + ->andWhere($qb->expr()->lt('deleted_at', $qb->createNamedParameter($timeLimit, IQueryBuilder::PARAM_INT))) ->orderBy('deleted_at') ->setMaxResults($limit); return $this->findEntities($qb); From e4a383b9d35bb263c053ccf2954f7d1e972b7f0d Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Thu, 27 Oct 2022 13:35:55 +0200 Subject: [PATCH 8/8] Fix DeleteCronTest Signed-off-by: Marcel Klehr --- tests/unit/Cron/DeleteCronTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/unit/Cron/DeleteCronTest.php b/tests/unit/Cron/DeleteCronTest.php index 07e7ee252..83a65e85e 100644 --- a/tests/unit/Cron/DeleteCronTest.php +++ b/tests/unit/Cron/DeleteCronTest.php @@ -27,6 +27,7 @@ use OCA\Deck\Db\AttachmentMapper; use OCA\Deck\Db\Board; use OCA\Deck\Db\BoardMapper; +use OCA\Deck\Db\Card; use OCA\Deck\Db\CardMapper; use OCA\Deck\InvalidAttachmentType; use OCA\Deck\Service\AttachmentService; @@ -91,7 +92,7 @@ public function testDeleteCron() { [$boards[3]] ); - $cards = [ $this->getBoard(10) ]; + $cards = [ $this->getCard(10) ]; $this->cardMapper->expects($this->once()) ->method('findToDelete') ->willReturn($cards);