From cc412a226e06d8b59e1c67618484cf49a4500cd5 Mon Sep 17 00:00:00 2001 From: Git'Fellow <12234510+solracsf@users.noreply.github.com> Date: Thu, 3 Sep 2026 13:49:45 +0200 Subject: [PATCH 1/3] fix(filecache): announce every removed entry so metadata is cleaned up Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com> --- lib/private/Files/Cache/Cache.php | 54 ++++++++-- tests/lib/Files/Cache/CacheTest.php | 153 ++++++++++++++++++++++++++++ 2 files changed, 200 insertions(+), 7 deletions(-) diff --git a/lib/private/Files/Cache/Cache.php b/lib/private/Files/Cache/Cache.php index a6f01a594e079..df6e8151eeb7c 100644 --- a/lib/private/Files/Cache/Cache.php +++ b/lib/private/Files/Cache/Cache.php @@ -608,7 +608,9 @@ public function remove($file) { $this->removeChildren($entry); } - $this->eventDispatcher->dispatchTyped(new CacheEntryRemovedEvent($this->storage, $entry->getPath(), $entry->getId(), $this->getNumericStorageId())); + $event = new CacheEntryRemovedEvent($this->storage, $entry->getPath(), $entry->getId(), $this->getNumericStorageId()); + $this->eventDispatcher->dispatchTyped($event); + $this->eventDispatcher->dispatchTyped(new CacheEntriesRemovedEvent([$event])); } } @@ -679,8 +681,8 @@ private function removeChildren(ICacheEntry $entry) { $query->executeStatement(); } - $cacheEntryRemovedEvents = []; - foreach (array_chunk(array_combine($deletedIds, $deletedPaths), IQueryBuilder::MAX_IN_PARAMETERS) as $chunk) { + foreach (array_chunk(array_combine($deletedIds, $deletedPaths), IQueryBuilder::MAX_IN_PARAMETERS, true) as $chunk) { + $cacheEntryRemovedEvents = []; /** @var array $chunk */ foreach ($chunk as $fileId => $filePath) { $cacheEntryRemovedEvents[] = new CacheEntryRemovedEvent( @@ -900,15 +902,53 @@ private function getChildIds(int $storageId, string $path): array { * remove all entries for files that are stored on the storage from the cache */ public function clear() { - $query = $this->getQueryBuilder(); - $query->delete('filecache') - ->whereStorageId($this->getNumericStorageId()); - $query->executeStatement(); + $storageId = $this->getNumericStorageId(); + $exception = null; + + // removed in batches so a storage with many entries does not have to be held in memory at once + while (true) { + $query = $this->getQueryBuilder(); + $query->select('fileid', 'path') + ->from('filecache') + ->whereStorageId($storageId) + ->setMaxResults(IQueryBuilder::MAX_IN_PARAMETERS); + $rows = $query->executeQuery()->fetchAll(); + if ($rows === []) { + break; + } + + $fileIds = array_map(static fn (array $row): int => (int)$row['fileid'], $rows); + + $query = $this->getQueryBuilder(); + $query->delete('filecache') + ->whereStorageId($storageId) + ->andWhere($query->expr()->in('fileid', $query->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY))); + $query->executeStatement(); + + $cacheEntryRemovedEvents = []; + foreach ($rows as $row) { + $cacheEntryRemovedEvents[] = new CacheEntryRemovedEvent($this->storage, (string)$row['path'], (int)$row['fileid'], $storageId); + } + + // a listener must not be able to leave the storage half cleared + try { + $this->eventDispatcher->dispatchTyped(new CacheEntriesRemovedEvent($cacheEntryRemovedEvents)); + foreach ($cacheEntryRemovedEvents as $cacheEntryRemovedEvent) { + $this->eventDispatcher->dispatchTyped($cacheEntryRemovedEvent); + } + } catch (\Exception $e) { + $exception ??= $e; + } + } $query = $this->connection->getQueryBuilder(); $query->delete('storages') ->where($query->expr()->eq('id', $query->createNamedParameter($this->storageId))); $query->executeStatement(); + + if ($exception !== null) { + throw $exception; + } } /** diff --git a/tests/lib/Files/Cache/CacheTest.php b/tests/lib/Files/Cache/CacheTest.php index c072250a5ab1b..d1d75006e4d6a 100644 --- a/tests/lib/Files/Cache/CacheTest.php +++ b/tests/lib/Files/Cache/CacheTest.php @@ -9,20 +9,30 @@ namespace Test\Files\Cache; use OC\Files\Cache\Cache; +use OC\Files\Cache\CacheDependencies; use OC\Files\Cache\CacheEntry; +use OC\Files\Cache\QuerySearchHelper; use OC\Files\Cache\Wrapper\CacheJail; use OC\Files\Search\SearchComparison; use OC\Files\Search\SearchQuery; use OC\Files\Storage\Temporary; +use OC\SystemConfig; +use OC\User\DisplayNameCache; use OC\User\User; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\EventDispatcher\IEventDispatcher; +use OCP\Files\Cache\CacheEntriesRemovedEvent; use OCP\Files\Cache\ICacheEntry; +use OCP\Files\IMimeTypeLoader; use OCP\Files\Search\ISearchComparison; +use OCP\Files\Storage\IStorage; +use OCP\FilesMetadata\IFilesMetadataManager; use OCP\IDBConnection; use OCP\ITagManager; use OCP\IUser; use OCP\IUserManager; use OCP\Server; +use Psr\Log\LoggerInterface; class LongId extends Temporary { #[\Override] @@ -245,6 +255,149 @@ public function testRemoveRecursive(): void { } } + /** + * @return array{0: Cache, 1: \Closure(): list} + */ + private function cacheWithRecordedEvents(IStorage $storage): array { + $events = []; + $dispatcher = $this->createMock(IEventDispatcher::class); + $dispatcher->method('dispatchTyped') + ->willReturnCallback(function (object $event) use (&$events): void { + if ($event instanceof CacheEntriesRemovedEvent) { + $events[] = $event; + } + }); + + $dependencies = new CacheDependencies( + Server::get(IMimeTypeLoader::class), + Server::get(IDBConnection::class), + $dispatcher, + Server::get(QuerySearchHelper::class), + Server::get(SystemConfig::class), + Server::get(LoggerInterface::class), + Server::get(IFilesMetadataManager::class), + Server::get(DisplayNameCache::class), + ); + + return [new Cache($storage, $dependencies), function () use (&$events): array { + return $events; + }]; + } + + /** + * @param list $events + * @return list + */ + private function announcedFileIds(array $events): array { + $fileIds = []; + foreach ($events as $event) { + foreach ($event->getCacheEntryRemovedEvents() as $removed) { + $fileIds[] = $removed->getFileId(); + } + } + return $fileIds; + } + + public function testRemoveSingleFileAnnouncesBatchEvent(): void { + $storage = new Temporary([]); + [$cache, $recorded] = $this->cacheWithRecordedEvents($storage); + $cache->insert('', ['size' => 0, 'mtime' => 0, 'mimetype' => ICacheEntry::DIRECTORY_MIMETYPE]); + $fileId = $cache->put('foo.txt', ['size' => 1, 'mtime' => 20, 'mimetype' => 'text/plain']); + + $cache->remove('foo.txt'); + + $this->assertEquals([$fileId], $this->announcedFileIds($recorded())); + } + + public function testRemoveRecursiveAnnouncesRealFileIds(): void { + $storage = new Temporary([]); + [$cache, $recorded] = $this->cacheWithRecordedEvents($storage); + $cache->insert('', ['size' => 0, 'mtime' => 0, 'mimetype' => ICacheEntry::DIRECTORY_MIMETYPE]); + $folderData = ['size' => 100, 'mtime' => 50, 'mimetype' => ICacheEntry::DIRECTORY_MIMETYPE]; + $fileData = ['size' => 1000, 'mtime' => 20, 'mimetype' => 'text/plain']; + + $expected = [$cache->put('folder', $folderData)]; + $expected[] = $cache->put('folder/sub', $folderData); + $expected[] = $cache->put('folder/foo.txt', $fileData); + $expected[] = $cache->put('folder/sub/bar.txt', $fileData); + + $cache->remove('folder'); + + $announced = $this->announcedFileIds($recorded()); + sort($expected); + sort($announced); + $this->assertEquals($expected, $announced); + } + + public function testRemoveRecursiveAnnouncesEveryChildExactlyOnce(): void { + $storage = new Temporary([]); + [$cache, $recorded] = $this->cacheWithRecordedEvents($storage); + $cache->insert('', ['size' => 0, 'mtime' => 0, 'mimetype' => ICacheEntry::DIRECTORY_MIMETYPE]); + $fileData = ['size' => 1, 'mtime' => 20, 'mimetype' => 'text/plain']; + + $expected = [$cache->put('folder', ['size' => 0, 'mtime' => 50, 'mimetype' => ICacheEntry::DIRECTORY_MIMETYPE])]; + for ($i = 0; $i < IQueryBuilder::MAX_IN_PARAMETERS + 1; $i++) { + $expected[] = $cache->insert("folder/child$i.txt", $fileData); + } + + $cache->remove('folder'); + + $announced = $this->announcedFileIds($recorded()); + $this->assertCount(count($expected), $announced, 'every removed entry is announced exactly once'); + sort($expected); + sort($announced); + $this->assertEquals($expected, $announced); + } + + public function testClearEmptiesTheStorageEvenIfAListenerThrows(): void { + $storage = new Temporary([]); + $dispatcher = $this->createMock(IEventDispatcher::class); + $dispatcher->method('dispatchTyped') + ->willReturnCallback(function (object $event): void { + if ($event instanceof CacheEntriesRemovedEvent) { + throw new \RuntimeException('listener blew up'); + } + }); + $dependencies = new CacheDependencies( + Server::get(IMimeTypeLoader::class), + Server::get(IDBConnection::class), + $dispatcher, + Server::get(QuerySearchHelper::class), + Server::get(SystemConfig::class), + Server::get(LoggerInterface::class), + Server::get(IFilesMetadataManager::class), + Server::get(DisplayNameCache::class), + ); + $cache = new Cache($storage, $dependencies); + $cache->insert('', ['size' => 0, 'mtime' => 0, 'mimetype' => ICacheEntry::DIRECTORY_MIMETYPE]); + $cache->put('foo.txt', ['size' => 1, 'mtime' => 20, 'mimetype' => 'text/plain']); + + try { + $cache->clear(); + $this->fail('the listener exception should surface'); + } catch (\RuntimeException $e) { + $this->assertEquals('listener blew up', $e->getMessage()); + } + + $this->assertFalse($cache->inCache('foo.txt')); + $this->assertFalse($cache->inCache('')); + } + + public function testClearAnnouncesRemovedEntries(): void { + $storage = new Temporary([]); + [$cache, $recorded] = $this->cacheWithRecordedEvents($storage); + $expected = [$cache->insert('', ['size' => 0, 'mtime' => 0, 'mimetype' => ICacheEntry::DIRECTORY_MIMETYPE])]; + $expected[] = $cache->put('foo.txt', ['size' => 1, 'mtime' => 20, 'mimetype' => 'text/plain']); + $expected[] = $cache->put('bar.txt', ['size' => 1, 'mtime' => 20, 'mimetype' => 'text/plain']); + + $cache->clear(); + + $announced = $this->announcedFileIds($recorded()); + sort($expected); + sort($announced); + $this->assertEquals($expected, $announced); + } + public static function folderDataProvider(): array { return [ ['folder'], From c178ac873fb08a6bd9806345ad4305d4f7ee5c47 Mon Sep 17 00:00:00 2001 From: Git'Fellow <12234510+solracsf@users.noreply.github.com> Date: Thu, 10 Sep 2026 18:26:59 +0200 Subject: [PATCH 2/3] fix(filecache): clear companion rows in clear() without per-entry events clear() no longer dispatches removal events for every entry of the storage. With admin_audit enabled that wrote one log line per file of a deleted user. It now removes the filecache_extended and file metadata rows batch by batch, before deleting the filecache rows themselves. Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com> --- lib/private/Files/Cache/Cache.php | 35 +++++------------ tests/lib/Files/Cache/CacheTest.php | 59 ++++++++--------------------- 2 files changed, 26 insertions(+), 68 deletions(-) diff --git a/lib/private/Files/Cache/Cache.php b/lib/private/Files/Cache/Cache.php index df6e8151eeb7c..924d850cb6699 100644 --- a/lib/private/Files/Cache/Cache.php +++ b/lib/private/Files/Cache/Cache.php @@ -903,52 +903,37 @@ private function getChildIds(int $storageId, string $path): array { */ public function clear() { $storageId = $this->getNumericStorageId(); - $exception = null; - // removed in batches so a storage with many entries does not have to be held in memory at once while (true) { $query = $this->getQueryBuilder(); - $query->select('fileid', 'path') + $query->select('fileid') ->from('filecache') ->whereStorageId($storageId) ->setMaxResults(IQueryBuilder::MAX_IN_PARAMETERS); - $rows = $query->executeQuery()->fetchAll(); - if ($rows === []) { + $fileIds = array_map(intval(...), $query->executeQuery()->fetchFirstColumn()); + if ($fileIds === []) { break; } - $fileIds = array_map(static fn (array $row): int => (int)$row['fileid'], $rows); + $query = $this->getQueryBuilder(); + $query->delete('filecache_extended') + ->where($query->expr()->in('fileid', $query->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY))) + ->hintShardKey('storage', $storageId); + $query->executeStatement(); + + $this->metadataManager->deleteMetadataForFiles($storageId, $fileIds); $query = $this->getQueryBuilder(); $query->delete('filecache') ->whereStorageId($storageId) ->andWhere($query->expr()->in('fileid', $query->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY))); $query->executeStatement(); - - $cacheEntryRemovedEvents = []; - foreach ($rows as $row) { - $cacheEntryRemovedEvents[] = new CacheEntryRemovedEvent($this->storage, (string)$row['path'], (int)$row['fileid'], $storageId); - } - - // a listener must not be able to leave the storage half cleared - try { - $this->eventDispatcher->dispatchTyped(new CacheEntriesRemovedEvent($cacheEntryRemovedEvents)); - foreach ($cacheEntryRemovedEvents as $cacheEntryRemovedEvent) { - $this->eventDispatcher->dispatchTyped($cacheEntryRemovedEvent); - } - } catch (\Exception $e) { - $exception ??= $e; - } } $query = $this->connection->getQueryBuilder(); $query->delete('storages') ->where($query->expr()->eq('id', $query->createNamedParameter($this->storageId))); $query->executeStatement(); - - if ($exception !== null) { - throw $exception; - } } /** diff --git a/tests/lib/Files/Cache/CacheTest.php b/tests/lib/Files/Cache/CacheTest.php index d1d75006e4d6a..e58f41344f24c 100644 --- a/tests/lib/Files/Cache/CacheTest.php +++ b/tests/lib/Files/Cache/CacheTest.php @@ -349,53 +349,26 @@ public function testRemoveRecursiveAnnouncesEveryChildExactlyOnce(): void { $this->assertEquals($expected, $announced); } - public function testClearEmptiesTheStorageEvenIfAListenerThrows(): void { - $storage = new Temporary([]); - $dispatcher = $this->createMock(IEventDispatcher::class); - $dispatcher->method('dispatchTyped') - ->willReturnCallback(function (object $event): void { - if ($event instanceof CacheEntriesRemovedEvent) { - throw new \RuntimeException('listener blew up'); - } - }); - $dependencies = new CacheDependencies( - Server::get(IMimeTypeLoader::class), - Server::get(IDBConnection::class), - $dispatcher, - Server::get(QuerySearchHelper::class), - Server::get(SystemConfig::class), - Server::get(LoggerInterface::class), - Server::get(IFilesMetadataManager::class), - Server::get(DisplayNameCache::class), - ); - $cache = new Cache($storage, $dependencies); - $cache->insert('', ['size' => 0, 'mtime' => 0, 'mimetype' => ICacheEntry::DIRECTORY_MIMETYPE]); - $cache->put('foo.txt', ['size' => 1, 'mtime' => 20, 'mimetype' => 'text/plain']); - - try { - $cache->clear(); - $this->fail('the listener exception should surface'); - } catch (\RuntimeException $e) { - $this->assertEquals('listener blew up', $e->getMessage()); - } - - $this->assertFalse($cache->inCache('foo.txt')); - $this->assertFalse($cache->inCache('')); - } + public function testClearRemovesExtendedAndMetadataEntries(): void { + $cache = new Cache($this->storage); + $fileId = $cache->put('foo', ['size' => 100, 'mtime' => 50, 'mimetype' => 'text/plain', 'upload_time' => 30]); - public function testClearAnnouncesRemovedEntries(): void { - $storage = new Temporary([]); - [$cache, $recorded] = $this->cacheWithRecordedEvents($storage); - $expected = [$cache->insert('', ['size' => 0, 'mtime' => 0, 'mimetype' => ICacheEntry::DIRECTORY_MIMETYPE])]; - $expected[] = $cache->put('foo.txt', ['size' => 1, 'mtime' => 20, 'mimetype' => 'text/plain']); - $expected[] = $cache->put('bar.txt', ['size' => 1, 'mtime' => 20, 'mimetype' => 'text/plain']); + $metadataManager = Server::get(IFilesMetadataManager::class); + $metadata = $metadataManager->getMetadata($fileId, true); + $metadata->setString('test-key', 'value', true); + $metadataManager->saveMetadata($metadata); $cache->clear(); - $announced = $this->announcedFileIds($recorded()); - sort($expected); - sort($announced); - $this->assertEquals($expected, $announced); + $this->assertFalse($cache->inCache('foo')); + $connection = Server::get(IDBConnection::class); + foreach (['filecache_extended' => 'fileid', 'files_metadata' => 'file_id', 'files_metadata_index' => 'file_id'] as $table => $column) { + $query = $connection->getQueryBuilder(); + $query->select($column) + ->from($table) + ->where($query->expr()->eq($column, $query->createNamedParameter($fileId, IQueryBuilder::PARAM_INT))); + $this->assertSame([], $query->executeQuery()->fetchFirstColumn(), "$table rows of the cleared storage remain"); + } } public static function folderDataProvider(): array { From fbda405f292b1495f26fca24b35e2c5c84afcda5 Mon Sep 17 00:00:00 2001 From: Git'Fellow <12234510+solracsf@users.noreply.github.com> Date: Thu, 10 Sep 2026 18:50:10 +0200 Subject: [PATCH 3/3] fix(files): clean up filecache companion rows of removed storages cleanByMountId() deleted the filecache rows of an unmounted storage but left their filecache_extended and file metadata rows behind. It now goes through Storage::removeFileCacheEntries(), the same path clear() uses. files:cleanup also removes filecache_extended, files_metadata and files_metadata_index rows whose file is no longer in the filecache, so rows orphaned before this fix are cleaned up too. Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com> --- .../files/lib/Command/DeleteOrphanedFiles.php | 89 ++++++++--------- .../tests/Command/DeleteOrphanedFilesTest.php | 99 +++++++++++++++++-- lib/private/Files/Cache/Cache.php | 28 +----- lib/private/Files/Cache/Storage.php | 46 +++++++-- tests/lib/Files/Cache/StorageTest.php | 67 +++++++++++++ 5 files changed, 235 insertions(+), 94 deletions(-) create mode 100644 tests/lib/Files/Cache/StorageTest.php diff --git a/apps/files/lib/Command/DeleteOrphanedFiles.php b/apps/files/lib/Command/DeleteOrphanedFiles.php index 0563584d75be2..be97f3bd88fa0 100644 --- a/apps/files/lib/Command/DeleteOrphanedFiles.php +++ b/apps/files/lib/Command/DeleteOrphanedFiles.php @@ -18,12 +18,13 @@ use OCP\IDBConnection; /** - * Delete all file entries that have no matching entries in the storage table. + * Delete all file entries that have no matching entries in the storage table, + * and the rows keyed by file id that have no matching file entry. */ #[AsCommand( name: 'files:cleanup', description: 'Clean up orphaned filecache and mount entries', - help: 'Deletes orphaned filecache and mount entries (those without an existing storage).', + help: 'Deletes orphaned filecache and mount entries (those without an existing storage), and filecache_extended and file metadata entries without a filecache entry.', )] class DeleteOrphanedFiles { public const int CHUNK_SIZE = 200; @@ -38,23 +39,22 @@ public function __invoke( #[Option(name: 'skip-filecache-extended', description: 'don\'t remove orphaned entries from filecache_extended')] bool $skipFilecacheExtended = false, ): ExitCode { - $fileIdsByStorage = []; - $deletedStorages = array_diff($this->getReferencedStorages(), $this->getExistingStorages()); - $deleteExtended = !$skipFilecacheExtended; - if ($deleteExtended) { - $fileIdsByStorage = $this->getFileIdsForStorages($deletedStorages); - } - $deletedEntries = $this->cleanupOrphanedFileCache($deletedStorages); $output->writeln("$deletedEntries orphaned file cache entries deleted"); - if ($deleteExtended) { - $deletedFileCacheExtended = $this->cleanupOrphanedFileCacheExtended($fileIdsByStorage); + if (!$skipFilecacheExtended) { + $deletedFileCacheExtended = $this->cleanupEntriesWithoutFileCache('filecache_extended', 'fileid'); $output->writeln("$deletedFileCacheExtended orphaned file cache extended entries deleted"); } + $deletedMetadata = $this->cleanupEntriesWithoutFileCache('files_metadata', 'file_id'); + $output->writeln("$deletedMetadata orphaned file metadata entries deleted"); + + $deletedMetadataIndex = $this->cleanupEntriesWithoutFileCache('files_metadata_index', 'file_id'); + $output->writeln("$deletedMetadataIndex orphaned file metadata index entries deleted"); + $deletedMounts = $this->cleanupOrphanedMounts(); $output->writeln("$deletedMounts orphaned mount entries deleted"); @@ -78,28 +78,6 @@ private function getExistingStorages(): array { return $query->executeQuery()->fetchFirstColumn(); } - /** - * @param int[] $storageIds - * @return array - */ - private function getFileIdsForStorages(array $storageIds): array { - $query = $this->connection->getQueryBuilder(); - $query->select('storage', 'fileid') - ->from('filecache') - ->where($query->expr()->in('storage', $query->createParameter('storage_ids'))); - - $result = []; - $storageIdChunks = array_chunk($storageIds, self::CHUNK_SIZE); - foreach ($storageIdChunks as $storageIdChunk) { - $query->setParameter('storage_ids', $storageIdChunk, IQueryBuilder::PARAM_INT_ARRAY); - $chunk = $query->executeQuery()->fetchAllAssociative(); - foreach ($chunk as $row) { - $result[$row['storage']][] = $row['fileid']; - } - } - return $result; - } - private function cleanupOrphanedFileCache(array $deletedStorages): int { $deletedEntries = 0; @@ -116,27 +94,38 @@ private function cleanupOrphanedFileCache(array $deletedStorages): int { return $deletedEntries; } - /** - * @param array $fileIdsByStorage - * @return int - */ - private function cleanupOrphanedFileCacheExtended(array $fileIdsByStorage): int { + private function cleanupEntriesWithoutFileCache(string $table, string $fileIdColumn): int { $deletedEntries = 0; + $lastFileId = 0; + + while (true) { + $query = $this->connection->getQueryBuilder(); + $query->select($fileIdColumn) + ->from($table) + ->where($query->expr()->gt($fileIdColumn, $query->createNamedParameter($lastFileId, IQueryBuilder::PARAM_INT))) + ->orderBy($fileIdColumn) + ->setMaxResults(IQueryBuilder::MAX_IN_PARAMETERS) + ->runAcrossAllShards(); + $fileIds = array_unique(array_map(intval(...), $query->executeQuery()->fetchFirstColumn())); + if ($fileIds === []) { + return $deletedEntries; + } - $deleteQuery = $this->connection->getQueryBuilder(); - $deleteQuery->delete('filecache_extended') - ->where($deleteQuery->expr()->in('fileid', $deleteQuery->createParameter('file_ids'))); - - foreach ($fileIdsByStorage as $storageId => $fileIds) { - $deleteQuery->hintShardKey('storage', $storageId, true); - $fileChunks = array_chunk($fileIds, self::CHUNK_SIZE); - foreach ($fileChunks as $fileChunk) { - $deleteQuery->setParameter('file_ids', $fileChunk, IQueryBuilder::PARAM_INT_ARRAY); - $deletedEntries += $deleteQuery->executeStatement(); + $query = $this->connection->getQueryBuilder(); + $query->select('fileid') + ->from('filecache') + ->where($query->expr()->in('fileid', $query->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY))); + $missingFileIds = array_diff($fileIds, $query->executeQuery()->fetchFirstColumn()); + + if ($missingFileIds !== []) { + $query = $this->connection->getQueryBuilder(); + $query->delete($table) + ->where($query->expr()->in($fileIdColumn, $query->createNamedParameter($missingFileIds, IQueryBuilder::PARAM_INT_ARRAY))); + $deletedEntries += $query->executeStatement(); } - } - return $deletedEntries; + $lastFileId = max($fileIds); + } } private function cleanupOrphanedMounts(): int { diff --git a/apps/files/tests/Command/DeleteOrphanedFilesTest.php b/apps/files/tests/Command/DeleteOrphanedFilesTest.php index 4745669cebeec..b51d683157d34 100644 --- a/apps/files/tests/Command/DeleteOrphanedFilesTest.php +++ b/apps/files/tests/Command/DeleteOrphanedFilesTest.php @@ -9,11 +9,15 @@ namespace OCA\Files\Tests\Command; +use OC\Files\Storage\Temporary; use OC\Files\View; use OCA\Files\Command\DeleteOrphanedFiles; use OCP\Console\IOutput; +use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\Files\Cache\ICacheEntry; use OCP\Files\IRootFolder; use OCP\Files\StorageNotAvailableException; +use OCP\FilesMetadata\IFilesMetadataManager; use OCP\IDBConnection; use OCP\IUserManager; use OCP\Server; @@ -73,6 +77,31 @@ protected function getMountsCount(int $storageId): int { return (int)$query->executeQuery()->fetchOne(); } + /** + * @param list $fileIds + */ + protected function countRows(string $table, string $column, array $fileIds): int { + // selecting rows instead of COUNT(*), which a sharded query answers once per shard + $query = $this->connection->getQueryBuilder(); + $query->select($column) + ->from($table) + ->where($query->expr()->in($column, $query->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY))); + return count($query->executeQuery()->fetchFirstColumn()); + } + + /** + * @param list $calls + */ + protected function expectOutput(IOutput&\PHPUnit\Framework\MockObject\MockObject $output, array $calls): void { + $output + ->expects($this->exactly(count($calls))) + ->method('writeln') + ->willReturnCallback(function (string $message) use (&$calls): void { + $expected = array_shift($calls); + $this->assertSame($expected, $message); + }); + } + /** * Test clearing orphaned files */ @@ -102,6 +131,16 @@ public function testClearFiles(): void { $this->assertCount(1, $this->getFile($fileInfo->getId()), 'Asserts that file is still available'); $this->assertEquals(1, $this->getMountsCount($numericStorageId), 'Asserts that mount is still available'); + $qb = $this->connection->getQueryBuilder(); + $storageFileIds = array_map('intval', $qb->select('fileid') + ->from('filecache') + ->where($qb->expr()->eq('storage', $qb->createNamedParameter($numericStorageId, IQueryBuilder::PARAM_INT))) + ->executeQuery() + ->fetchFirstColumn()); + $extendedEntries = $this->countRows('filecache_extended', 'fileid', $storageFileIds); + $metadataEntries = $this->countRows('files_metadata', 'file_id', $storageFileIds); + $metadataIndexEntries = $this->countRows('files_metadata_index', 'file_id', $storageFileIds); + $qb = $this->connection->getQueryBuilder(); $deletedRows = $qb->delete('storages') ->where($qb->expr()->eq('id', $qb->createNamedParameter($storageId))) @@ -110,18 +149,13 @@ public function testClearFiles(): void { $this->assertSame(1, $deletedRows, 'Asserts that storage got deleted'); // parent folder, `files`, ´test` and `welcome.txt` => 4 elements - $calls = [ + $this->expectOutput($output, [ '3 orphaned file cache entries deleted', - '0 orphaned file cache extended entries deleted', + "$extendedEntries orphaned file cache extended entries deleted", + "$metadataEntries orphaned file metadata entries deleted", + "$metadataIndexEntries orphaned file metadata index entries deleted", '1 orphaned mount entries deleted', - ]; - $output - ->expects($this->exactly(3)) - ->method('writeln') - ->willReturnCallback(function (string $message) use (&$calls): void { - $expected = array_shift($calls); - $this->assertSame($expected, $message); - }); + ]); ($this->command)($output); @@ -136,4 +170,49 @@ public function testClearFiles(): void { } catch (StorageNotAvailableException $e) { } } + + public function testClearEntriesWithoutFileCacheEntry(): void { + // remove orphans left behind by other tests so that the counts below only cover this test + ($this->command)($this->createMock(IOutput::class)); + + $storage = new Temporary([]); + $cache = $storage->getCache(); + $cache->put('', ['size' => 0, 'mtime' => 0, 'mimetype' => ICacheEntry::DIRECTORY_MIMETYPE]); + $data = ['size' => 1, 'mtime' => 1, 'mimetype' => 'text/plain', 'upload_time' => 25]; + $orphanId = $cache->put('orphan.txt', $data); + $keptId = $cache->put('kept.txt', $data); + + $metadataManager = Server::get(IFilesMetadataManager::class); + foreach ([$orphanId, $keptId] as $fileId) { + $metadata = $metadataManager->getMetadata($fileId, true); + $metadata->setString('test-key', 'value', true); + $metadataManager->saveMetadata($metadata); + } + + $qb = $this->connection->getQueryBuilder(); + $qb->delete('filecache') + ->where($qb->expr()->eq('fileid', $qb->createNamedParameter($orphanId, IQueryBuilder::PARAM_INT))) + ->executeStatement(); + + $output = $this->createMock(IOutput::class); + $this->expectOutput($output, [ + '0 orphaned file cache entries deleted', + '1 orphaned file cache extended entries deleted', + '1 orphaned file metadata entries deleted', + '1 orphaned file metadata index entries deleted', + '0 orphaned mount entries deleted', + ]); + + ($this->command)($output); + + $this->assertSame(0, $this->countRows('filecache_extended', 'fileid', [$orphanId])); + $this->assertSame(0, $this->countRows('files_metadata', 'file_id', [$orphanId])); + $this->assertSame(0, $this->countRows('files_metadata_index', 'file_id', [$orphanId])); + + $this->assertSame(1, $this->countRows('filecache_extended', 'fileid', [$keptId])); + $this->assertSame(1, $this->countRows('files_metadata', 'file_id', [$keptId])); + $this->assertSame(1, $this->countRows('files_metadata_index', 'file_id', [$keptId])); + + $cache->clear(); + } } diff --git a/lib/private/Files/Cache/Cache.php b/lib/private/Files/Cache/Cache.php index 924d850cb6699..eec26ecc96608 100644 --- a/lib/private/Files/Cache/Cache.php +++ b/lib/private/Files/Cache/Cache.php @@ -902,33 +902,7 @@ private function getChildIds(int $storageId, string $path): array { * remove all entries for files that are stored on the storage from the cache */ public function clear() { - $storageId = $this->getNumericStorageId(); - - while (true) { - $query = $this->getQueryBuilder(); - $query->select('fileid') - ->from('filecache') - ->whereStorageId($storageId) - ->setMaxResults(IQueryBuilder::MAX_IN_PARAMETERS); - $fileIds = array_map(intval(...), $query->executeQuery()->fetchFirstColumn()); - if ($fileIds === []) { - break; - } - - $query = $this->getQueryBuilder(); - $query->delete('filecache_extended') - ->where($query->expr()->in('fileid', $query->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY))) - ->hintShardKey('storage', $storageId); - $query->executeStatement(); - - $this->metadataManager->deleteMetadataForFiles($storageId, $fileIds); - - $query = $this->getQueryBuilder(); - $query->delete('filecache') - ->whereStorageId($storageId) - ->andWhere($query->expr()->in('fileid', $query->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY))); - $query->executeStatement(); - } + Storage::removeFileCacheEntries($this->getNumericStorageId()); $query = $this->connection->getQueryBuilder(); $query->delete('storages') diff --git a/lib/private/Files/Cache/Storage.php b/lib/private/Files/Cache/Storage.php index 4b9a074db5b73..7f8869d69bbb1 100644 --- a/lib/private/Files/Cache/Storage.php +++ b/lib/private/Files/Cache/Storage.php @@ -13,6 +13,7 @@ use OC\DB\Exceptions\DbalException; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\Storage\IStorage; +use OCP\FilesMetadata\IFilesMetadataManager; use OCP\IDBConnection; use OCP\Server; use Psr\Log\LoggerInterface; @@ -170,14 +171,11 @@ public static function cleanByMountId(int $mountId): void { $query->select('storage_id') ->from('mounts') ->where($query->expr()->eq('mount_id', $query->createNamedParameter($mountId, IQueryBuilder::PARAM_INT))); - $storageIds = $query->executeQuery()->fetchFirstColumn(); - $storageIds = array_unique($storageIds); + $storageIds = array_unique(array_map(intval(...), $query->executeQuery()->fetchFirstColumn())); - $query = $db->getQueryBuilder(); - $query->delete('filecache') - ->where($query->expr()->in('storage', $query->createNamedParameter($storageIds, IQueryBuilder::PARAM_INT_ARRAY))) - ->runAcrossAllShards() - ->executeStatement(); + foreach ($storageIds as $storageId) { + self::removeFileCacheEntries($storageId); + } $query = $db->getQueryBuilder(); $query->delete('storages') @@ -195,4 +193,38 @@ public static function cleanByMountId(int $mountId): void { throw $exception; } } + + /** + * Remove the filecache entries of a storage together with their filecache_extended and metadata rows + */ + public static function removeFileCacheEntries(int $numericStorageId): void { + $db = Server::get(IDBConnection::class); + $metadataManager = Server::get(IFilesMetadataManager::class); + + while (true) { + $query = $db->getQueryBuilder(); + $query->select('fileid') + ->from('filecache') + ->where($query->expr()->eq('storage', $query->createNamedParameter($numericStorageId, IQueryBuilder::PARAM_INT))) + ->setMaxResults(IQueryBuilder::MAX_IN_PARAMETERS); + $fileIds = array_map(intval(...), $query->executeQuery()->fetchFirstColumn()); + if ($fileIds === []) { + return; + } + + $query = $db->getQueryBuilder(); + $query->delete('filecache_extended') + ->where($query->expr()->in('fileid', $query->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY))) + ->hintShardKey('storage', $numericStorageId) + ->executeStatement(); + + $metadataManager->deleteMetadataForFiles($numericStorageId, $fileIds); + + $query = $db->getQueryBuilder(); + $query->delete('filecache') + ->where($query->expr()->eq('storage', $query->createNamedParameter($numericStorageId, IQueryBuilder::PARAM_INT))) + ->andWhere($query->expr()->in('fileid', $query->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY))) + ->executeStatement(); + } + } } diff --git a/tests/lib/Files/Cache/StorageTest.php b/tests/lib/Files/Cache/StorageTest.php new file mode 100644 index 0000000000000..7d8dfe98b8ff0 --- /dev/null +++ b/tests/lib/Files/Cache/StorageTest.php @@ -0,0 +1,67 @@ +getCache(); + $rootId = $cache->put('', ['size' => 0, 'mtime' => 0, 'mimetype' => ICacheEntry::DIRECTORY_MIMETYPE]); + $fileId = $cache->put('foo.txt', ['size' => 1, 'mtime' => 1, 'mimetype' => 'text/plain', 'upload_time' => 25]); + + $metadataManager = Server::get(IFilesMetadataManager::class); + $metadata = $metadataManager->getMetadata($fileId, true); + $metadata->setString('test-key', 'value', true); + $metadataManager->saveMetadata($metadata); + + $mountId = random_int(100000000, 999999999); + $mountPoint = '/' . $this->getUniqueID('user') . '/files/mount/'; + $qb = Server::get(IDBConnection::class)->getQueryBuilder(); + $qb->insert('mounts') + ->values([ + 'storage_id' => $qb->createNamedParameter($cache->getNumericStorageId(), IQueryBuilder::PARAM_INT), + 'root_id' => $qb->createNamedParameter($rootId, IQueryBuilder::PARAM_INT), + 'user_id' => $qb->createNamedParameter('test'), + 'mount_point' => $qb->createNamedParameter($mountPoint), + 'mount_point_hash' => $qb->createNamedParameter(hash('xxh128', $mountPoint)), + 'mount_id' => $qb->createNamedParameter($mountId, IQueryBuilder::PARAM_INT), + ]) + ->executeStatement(); + + $this->assertSame(1, $this->countRows('filecache_extended', 'fileid', $fileId)); + $this->assertSame(1, $this->countRows('files_metadata', 'file_id', $fileId)); + $this->assertSame(1, $this->countRows('files_metadata_index', 'file_id', $fileId)); + + Storage::cleanByMountId($mountId); + + $this->assertSame(0, $this->countRows('filecache', 'fileid', $fileId)); + $this->assertSame(0, $this->countRows('filecache_extended', 'fileid', $fileId)); + $this->assertSame(0, $this->countRows('files_metadata', 'file_id', $fileId)); + $this->assertSame(0, $this->countRows('files_metadata_index', 'file_id', $fileId)); + } + + private function countRows(string $table, string $column, int $fileId): int { + $qb = Server::get(IDBConnection::class)->getQueryBuilder(); + $qb->select($qb->func()->count()) + ->from($table) + ->where($qb->expr()->eq($column, $qb->createNamedParameter($fileId, IQueryBuilder::PARAM_INT))); + return (int)$qb->executeQuery()->fetchOne(); + } +}