diff options
| -rw-r--r-- | apps/files_sharing/lib/DeleteOrphanedSharesJob.php | 79 | ||||
| -rw-r--r-- | apps/files_sharing/tests/DeleteOrphanedSharesJobTest.php | 3 |
2 files changed, 69 insertions, 13 deletions
diff --git a/apps/files_sharing/lib/DeleteOrphanedSharesJob.php b/apps/files_sharing/lib/DeleteOrphanedSharesJob.php index a9452cb3dcc..6c6d5bfede5 100644 --- a/apps/files_sharing/lib/DeleteOrphanedSharesJob.php +++ b/apps/files_sharing/lib/DeleteOrphanedSharesJob.php @@ -1,4 +1,7 @@ <?php + +declare(strict_types=1); + /** * @copyright Copyright (c) 2016, ownCloud, Inc. * @@ -24,21 +27,45 @@ */ namespace OCA\Files_Sharing; +use OCP\AppFramework\Db\TTransactional; use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\TimedJob; +use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\IDBConnection; +use PDO; +use Psr\Log\LoggerInterface; +use function array_map; /** * Delete all share entries that have no matching entries in the file cache table. */ class DeleteOrphanedSharesJob extends TimedJob { + + use TTransactional; + + private const CHUNK_SIZE = 1000; + + private const INTERVAL = 24 * 60 * 60; // 1 day + + private IDBConnection $db; + + private LoggerInterface $logger; + /** * sets the correct interval for this timed job */ - public function __construct(ITimeFactory $time) { + public function __construct( + ITimeFactory $time, + IDBConnection $db, + LoggerInterface $logger + ) { parent::__construct($time); - $this->setInterval(24 * 60 * 60); // 1 day + $this->db = $db; + + $this->setInterval(self::INTERVAL); // 1 day $this->setTimeSensitivity(self::TIME_INSENSITIVE); + $this->logger = $logger; } /** @@ -47,15 +74,45 @@ class DeleteOrphanedSharesJob extends TimedJob { * @param array $argument unused argument */ public function run($argument) { - $connection = \OC::$server->getDatabaseConnection(); - $logger = \OC::$server->getLogger(); - - $sql = - 'DELETE FROM `*PREFIX*share` ' . - 'WHERE `item_type` in (\'file\', \'folder\') ' . - 'AND NOT EXISTS (SELECT `fileid` FROM `*PREFIX*filecache` WHERE `file_source` = `fileid`)'; + $qbSelect = $this->db->getQueryBuilder(); + $qbSelect->select('id') + ->from('share', 's') + ->leftJoin('s', 'filecache', 'fc', $qbSelect->expr()->eq('s.file_source', 'fc.fileid')) + ->where($qbSelect->expr()->isNull('fc.fileid')) + ->setMaxResults(self::CHUNK_SIZE); + $deleteQb = $this->db->getQueryBuilder(); + $deleteQb->delete('share') + ->where( + $deleteQb->expr()->in('id', $deleteQb->createParameter('ids'), IQueryBuilder::PARAM_INT_ARRAY) + ); - $deletedEntries = $connection->executeUpdate($sql); - $logger->debug("$deletedEntries orphaned share(s) deleted", ['app' => 'DeleteOrphanedSharesJob']); + /** + * Read a chunk of orphan rows and delete them. Continue as long as the + * chunk is filled and time before the next cron run does not run out. + * + * Note: With isolation level READ COMMITTED, the database will allow + * other transactions to delete rows between our SELECT and DELETE. In + * that (unlikely) case, our DELETE will have fewer affected rows than + * IDs passed for the WHERE IN. If this happens while processing a full + * chunk, the logic below will stop prematurely. + * Note: The queries below are optimized for low database locking. They + * could be combined into one single DELETE with join or sub query, but + * that has shown to (dead)lock often. + */ + $cutOff = $this->time->getTime() + self::INTERVAL; + do { + $deleted = $this->atomic(function () use ($qbSelect, $deleteQb) { + $result = $qbSelect->executeQuery(); + $ids = array_map('intval', $result->fetchAll(PDO::FETCH_COLUMN)); + $result->closeCursor(); + $deleteQb->setParameter('ids', $ids, IQueryBuilder::PARAM_INT_ARRAY); + $deleted = $deleteQb->executeStatement(); + $this->logger->debug("{deleted} orphaned share(s) deleted", [ + 'app' => 'DeleteOrphanedSharesJob', + 'deleted' => $deleted, + ]); + return $deleted; + }, $this->db); + } while ($deleted >= self::CHUNK_SIZE && $this->time->getTime() <= $cutOff); } } diff --git a/apps/files_sharing/tests/DeleteOrphanedSharesJobTest.php b/apps/files_sharing/tests/DeleteOrphanedSharesJobTest.php index 3de40215f15..0a39246e030 100644 --- a/apps/files_sharing/tests/DeleteOrphanedSharesJobTest.php +++ b/apps/files_sharing/tests/DeleteOrphanedSharesJobTest.php @@ -27,7 +27,6 @@ namespace OCA\Files_Sharing\Tests; use OCA\Files_Sharing\DeleteOrphanedSharesJob; -use OCP\AppFramework\Utility\ITimeFactory; use OCP\Share\IShare; /** @@ -94,7 +93,7 @@ class DeleteOrphanedSharesJobTest extends \Test\TestCase { \OC::registerShareHooks(\OC::$server->getSystemConfig()); - $this->job = new DeleteOrphanedSharesJob(\OCP\Server::get(ITimeFactory::class)); + $this->job = \OCP\Server::get(DeleteOrphanedSharesJob::class); } protected function tearDown(): void { |