Skip to content

Commit

Permalink
fix: add sharding compatible version of share reminder job
Browse files Browse the repository at this point in the history 
Signed-off-by: Robin Appelman <[email protected]>
  • Loading branch information
@icewind1991
icewind1991 committed Sep 19, 2024
1 parent 6b8d331 commit 51623ad
Show file tree
Hide file tree
Showing 2 changed files with 104 additions and 13 deletions.
113 changes: 101 additions & 12 deletions apps/files_sharing/lib/SharesReminderJob.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -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);
}


Expand Down Expand Up @@ -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 ($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);
Expand Down Expand Up @@ -103,21 +132,81 @@ 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]";
}
$shares = $qb->executeQuery()->fetchAll();
return array_map(fn ($share): array => [
'id' => (int)$share['id'],
'share_type' => (int)$share['share_type'],
], $shares);
}

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: int, share_type: int}[]
*/
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();
$shares = array_map(fn ($share): array => [
'id' => (int)$share['id'],
'share_type' => (int)$share['share_type'],
'file_source' => (int)$share['file_source'],
], $shares);
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: int, share_type: int, file_source: int}[] $shares
* @return array{id: int, share_type: int}[]

Check failure on line 186 in apps/files_sharing/lib/SharesReminderJob.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

InvalidReturnType 

apps/files_sharing/lib/SharesReminderJob.php:186:13: InvalidReturnType: The declared return type 'array<array-key, array{id: int, share_type: int}>' for OCA\Files_Sharing\SharesReminderJob::filterSharesWithEmptyFolders is incorrect, got 'list<array{file_source: int, id: int, share_type: int}>' (see https://psalm.dev/011)

Check failure

Code scanning / Psalm

InvalidReturnType Error

The declared return type 'array<array-key, array{id: int, share_type: int}>' for OCA\Files_Sharing\SharesReminderJob::filterSharesWithEmptyFolders is incorrect, got 'list<array{file_source: int, id: int, share_type: int}>'
*/
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): int => $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;

Check failure on line 209 in apps/files_sharing/lib/SharesReminderJob.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

InvalidReturnStatement 

apps/files_sharing/lib/SharesReminderJob.php:209:10: InvalidReturnStatement: The inferred type 'list<array{file_source: int, id: int, share_type: int}>' does not match the declared return type 'array<array-key, array{id: int, share_type: int}>' for OCA\Files_Sharing\SharesReminderJob::filterSharesWithEmptyFolders (see https://psalm.dev/128)

Check failure

Code scanning / Psalm

InvalidReturnStatement Error

The inferred type 'list<array{file_source: int, id: int, share_type: int}>' does not match the declared return type 'array<array-key, array{id: int, share_type: int}>' for OCA\Files_Sharing\SharesReminderJob::filterSharesWithEmptyFolders
}

/**
Expand Down
4 changes: 3 additions & 1 deletion apps/files_sharing/tests/SharesReminderJobTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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),
);
}

Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit 51623ad

Please sign in to comment.