From bed3becc64ad8cfc58b06f1b03031280cbebd291 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 19 Sep 2024 10:40:57 +0200 Subject: [PATCH] fix: add sharding compatible version of share reminder job Signed-off-by: Robin Appelman --- apps/files_sharing/lib/SharesReminderJob.php | 103 ++++++++++++++++-- .../tests/SharesReminderJobTest.php | 4 +- 2 files changed, 94 insertions(+), 13 deletions(-) diff --git a/apps/files_sharing/lib/SharesReminderJob.php b/apps/files_sharing/lib/SharesReminderJob.php index 6780ee77fe3f3..0dbfe817fe558 100644 --- a/apps/files_sharing/lib/SharesReminderJob.php +++ b/apps/files_sharing/lib/SharesReminderJob.php @@ -13,6 +13,8 @@ use OCP\DB\Exception; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Defaults; +use OCP\Files\Cache\ICacheEntry; +use OCP\Files\IMimeTypeLoader; use OCP\Files\NotFoundException; use OCP\IDBConnection; use OCP\IL10N; @@ -33,6 +35,7 @@ class SharesReminderJob extends TimedJob { private const SECONDS_BEFORE_REMINDER = 86400; private const CHUNK_SIZE = 1000; + private int $folderMimeTypeId; public function __construct( ITimeFactory $time, @@ -44,9 +47,11 @@ public function __construct( private readonly IFactory $l10nFactory, private readonly IMailer $mailer, private readonly Defaults $defaults, + IMimeTypeLoader $mimeTypeLoader, ) { parent::__construct($time); $this->setInterval(3600); + $this->folderMimeTypeId = $mimeTypeLoader->getId(ICacheEntry::DIRECTORY_MIMETYPE); } @@ -74,6 +79,30 @@ public function run(mixed $argument): void { * @throws Exception if a database error occurs */ private function getShares(): array|\Iterator { + if ($this->db->getShardDefinition('filecache')) { + $sharesResult = $this->getSharesDataSharded(); + } else { + $sharesResult = $this->getSharesData(); + } + foreach($sharesResult as $share) { + if ((int)$share['share_type'] === IShare::TYPE_EMAIL) { + $id = "ocMailShare:$share[id]"; + } else { + $id = "ocinternal:$share[id]"; + } + + try { + yield $this->shareManager->getShareById($id); + } catch (ShareNotFound) { + $this->logger->error("Share with ID $id not found."); + } + } + } + + /** + * @return array{id: int, share_type: int}[] + */ + private function getSharesData(): array { $minDate = new \DateTime(); $maxDate = new \DateTime(); $maxDate->setTimestamp($maxDate->getTimestamp() + self::SECONDS_BEFORE_REMINDER); @@ -103,21 +132,71 @@ private function getShares(): array|\Iterator { ) ->setMaxResults(SharesReminderJob::CHUNK_SIZE); - $sharesResult = $qb->executeQuery(); - while ($share = $sharesResult->fetch()) { - if ((int)$share['share_type'] === IShare::TYPE_EMAIL) { - $id = "ocMailShare:$share[id]"; - } else { - $id = "ocinternal:$share[id]"; - } + return $qb->executeQuery()->fetchAll(); + } - try { - yield $this->shareManager->getShareById($id); - } catch (ShareNotFound) { - $this->logger->error("Share with ID $id not found."); + /** + * Sharding compatible version of getSharesData + * + * @return array{id: mixed, share_type: mixed}[] + */ + private function getSharesDataSharded(): array|\Iterator { + $minDate = new \DateTime(); + $maxDate = new \DateTime(); + $maxDate->setTimestamp($maxDate->getTimestamp() + self::SECONDS_BEFORE_REMINDER); + + $qb = $this->db->getQueryBuilder(); + $qb->select('s.id', 's.share_type', 's.file_source') + ->from('share', 's') + ->where( + $qb->expr()->andX( + $qb->expr()->orX( + $qb->expr()->eq('s.share_type', $qb->expr()->literal(IShare::TYPE_USER)), + $qb->expr()->eq('s.share_type', $qb->expr()->literal(IShare::TYPE_EMAIL)) + ), + $qb->expr()->eq('s.item_type', $qb->expr()->literal('folder')), + $qb->expr()->gte('s.expiration', $qb->createNamedParameter($minDate, IQueryBuilder::PARAM_DATE)), + $qb->expr()->lte('s.expiration', $qb->createNamedParameter($maxDate, IQueryBuilder::PARAM_DATE)), + $qb->expr()->eq('s.reminder_sent', $qb->createNamedParameter( + false, IQueryBuilder::PARAM_BOOL + )), + $qb->expr()->eq( + $qb->expr()->bitwiseAnd('s.permissions', Constants::PERMISSION_CREATE), + $qb->createNamedParameter(Constants::PERMISSION_CREATE, IQueryBuilder::PARAM_INT) + ), + ) + ); + + $shares = $qb->executeQuery()->fetchAll(); + return $this->filterSharesWithEmptyFolders($shares, self::CHUNK_SIZE); + } + + /** + * Check which of the supplied file ids is an empty folder until there are `$maxResults` folders + * @param array{id: mixed, share_type: mixed, file_source: mixed}[] $shares + */ + private function filterSharesWithEmptyFolders(array $shares, int $maxResults): array { + $query = $this->db->getQueryBuilder(); + $query->select('fileid') + ->from('filecache') + ->where($query->expr()->eq('size', $query->createNamedParameter(0), IQueryBuilder::PARAM_INT_ARRAY)) + ->andWhere($query->expr()->eq('mimetype', $query->createNamedParameter($this->folderMimeTypeId, IQueryBuilder::PARAM_INT))) + ->andWhere($query->expr()->in('fileid', $query->createParameter('fileids'))); + $chunks = array_chunk($shares, SharesReminderJob::CHUNK_SIZE); + $results = []; + foreach ($chunks as $chunk) { + $chunkFileIds = array_map(fn ($share) => $share['file_source'], $chunk); + $chunkByFileId = array_combine($chunkFileIds, $chunk); + $query->setParameter('fileids', $chunkFileIds, IQueryBuilder::PARAM_INT_ARRAY); + $chunkResults = $query->executeQuery()->fetchAll(\PDO::FETCH_COLUMN); + foreach ($chunkResults as $folderId) { + $results[] = $chunkByFileId[$folderId]; + } + if (count($results) >= $maxResults) { + break; } } - $sharesResult->closeCursor(); + return $results; } /** diff --git a/apps/files_sharing/tests/SharesReminderJobTest.php b/apps/files_sharing/tests/SharesReminderJobTest.php index 5f01f4850e9e8..9889ff47c5366 100644 --- a/apps/files_sharing/tests/SharesReminderJobTest.php +++ b/apps/files_sharing/tests/SharesReminderJobTest.php @@ -12,6 +12,7 @@ use OCP\AppFramework\Utility\ITimeFactory; use OCP\Constants; use OCP\Defaults; +use OCP\Files\IMimeTypeLoader; use OCP\Files\IRootFolder; use OCP\IDBConnection; use OCP\IURLGenerator; @@ -71,6 +72,7 @@ protected function setUp(): void { \OC::$server->get(IFactory::class), $this->mailer, \OC::$server->get(Defaults::class), + \OC::$server->get(IMimeTypeLoader::class), ); } @@ -158,7 +160,7 @@ public function testSharesReminder( $testFolder = $user1Folder->newFolder('test'); if (!$isEmpty) { - $testFolder->newFile('some_file.txt'); + $testFolder->newFile('some_file.txt', 'content'); } $share = $this->shareManager->newShare();