Skip to content

Commit b80e0d9

Browse files
fix: clean attachment sharing records after permanent deleted
Signed-off-by: Luka Trovic <[email protected]>
1 parent 382c05e commit b80e0d9

File tree

4 files changed

+61
-2
lines changed

4 files changed

+61
-2
lines changed

lib/Cron/DeleteCron.php

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use OCA\Deck\Db\StackMapper;
1515
use OCA\Deck\InvalidAttachmentType;
1616
use OCA\Deck\Service\AttachmentService;
17+
use OCA\Deck\Sharing\DeckShareProvider;
1718
use OCP\AppFramework\Utility\ITimeFactory;
1819
use OCP\BackgroundJob\IJob;
1920
use OCP\BackgroundJob\TimedJob;
@@ -30,14 +31,25 @@ class DeleteCron extends TimedJob {
3031
private $attachmentMapper;
3132
/** @var StackMapper */
3233
private $stackMapper;
34+
/** @var DeckShareProvider */
35+
private $deckShareProvider;
3336

34-
public function __construct(ITimeFactory $time, BoardMapper $boardMapper, CardMapper $cardMapper, AttachmentService $attachmentService, AttachmentMapper $attachmentMapper, StackMapper $stackMapper) {
37+
public function __construct(
38+
ITimeFactory $time,
39+
BoardMapper $boardMapper,
40+
CardMapper $cardMapper,
41+
AttachmentService $attachmentService,
42+
AttachmentMapper $attachmentMapper,
43+
StackMapper $stackMapper,
44+
DeckShareProvider $deckShareProvider,
45+
) {
3546
parent::__construct($time);
3647
$this->boardMapper = $boardMapper;
3748
$this->cardMapper = $cardMapper;
3849
$this->attachmentService = $attachmentService;
3950
$this->attachmentMapper = $attachmentMapper;
4051
$this->stackMapper = $stackMapper;
52+
$this->deckShareProvider = $deckShareProvider;
4153

4254
$this->setInterval(60 * 60 * 24);
4355
$this->setTimeSensitivity(IJob::TIME_INSENSITIVE);
@@ -70,6 +82,12 @@ protected function run($argument) {
7082
$this->attachmentMapper->delete($attachment);
7183
}
7284

85+
// Delete orphaned attachment shares
86+
$shares = $this->deckShareProvider->getOrphanedAttachmentShares();
87+
foreach ($shares as $share) {
88+
$this->deckShareProvider->delete($share);
89+
}
90+
7391
$stacks = $this->stackMapper->findToDelete();
7492
foreach ($stacks as $stack) {
7593
$this->stackMapper->delete($stack);

lib/Db/CardMapper.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -645,4 +645,16 @@ public function remapCardOwner(int $boardId, string $userId, string $newUserId):
645645

646646
$result->closeCursor();
647647
}
648+
649+
public function getAllCardIds(): array {
650+
$qb = $this->db->getQueryBuilder();
651+
$qb->select('id')
652+
->from('deck_cards');
653+
$result = $qb->executeQuery();
654+
$ids = [];
655+
while ($row = $result->fetch()) {
656+
$ids[] = (int)$row['id'];
657+
}
658+
return $ids;
659+
}
648660
}

lib/Sharing/DeckShareProvider.php

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,4 +1046,21 @@ public function getAllShares(): iterable {
10461046
}
10471047
$cursor->closeCursor();
10481048
}
1049+
1050+
public function getOrphanedAttachmentShares(): array {
1051+
$allCardIds = $this->cardMapper->getAllCardIds();
1052+
$qb = $this->dbConnection->getQueryBuilder();
1053+
$qb->select('*')
1054+
->from('share', 's')
1055+
->where($qb->expr()->eq('s.share_type', $qb->createNamedParameter(IShare::TYPE_DECK)))
1056+
->andWhere($qb->expr()->notIn('s.share_with', $qb->createNamedParameter($allCardIds, IQueryBuilder::PARAM_STR_ARRAY)));
1057+
1058+
$cursor = $qb->execute();
1059+
$shares = [];
1060+
while ($data = $cursor->fetch()) {
1061+
$shares[] = $this->createShareObject($data);
1062+
}
1063+
1064+
return $shares;
1065+
}
10491066
}

tests/unit/Cron/DeleteCronTest.php

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
use OCA\Deck\InvalidAttachmentType;
3636
use OCA\Deck\Service\AttachmentService;
3737
use OCA\Deck\Service\IAttachmentService;
38+
use OCA\Deck\Sharing\DeckShareProvider;
3839
use OCP\AppFramework\Utility\ITimeFactory;
3940
use PHPUnit\Framework\MockObject\MockObject;
4041
use Test\TestCase;
@@ -53,6 +54,8 @@ class DeleteCronTest extends TestCase {
5354
private $attachmentMapper;
5455
/** @var StackMapper|MockObject */
5556
private $stackMapper;
57+
/** @var DeckShareProvider */
58+
private $deckShareProvider;
5659
/** @var DeleteCron */
5760
protected $deleteCron;
5861

@@ -64,7 +67,16 @@ public function setUp(): void {
6467
$this->attachmentService = $this->createMock(AttachmentService::class);
6568
$this->attachmentMapper = $this->createMock(AttachmentMapper::class);
6669
$this->stackMapper = $this->createMock(StackMapper::class);
67-
$this->deleteCron = new DeleteCron($this->timeFactory, $this->boardMapper, $this->cardMapper, $this->attachmentService, $this->attachmentMapper, $this->stackMapper);
70+
$this->deckShareProvider = $this->createMock(DeckShareProvider::class);
71+
$this->deleteCron = new DeleteCron(
72+
$this->timeFactory,
73+
$this->boardMapper,
74+
$this->cardMapper,
75+
$this->attachmentService,
76+
$this->attachmentMapper,
77+
$this->stackMapper,
78+
$this->deckShareProvider,
79+
);
6880
}
6981

7082
protected function getBoard($id) {

0 commit comments

Comments
 (0)