From 2565b7addb7be82e7b070eb41495f3df42a517d3 Mon Sep 17 00:00:00 2001 From: shawon9324 Date: Mon, 21 Sep 2026 11:58:06 +0600 Subject: [PATCH] fix(files_metadata): chunk file id list in getMetadataFromFileIds getMetadataFromFileIds() passed the full file id array into a single IN() expression. QueryBuilder rejects lists above MAX_IN_PARAMETERS, so PROPFIND on directories with more than 1000 files failed with a query exception. Query the ids in chunks, matching dropMetadataForFiles(). Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: shawon9324 --- .../Service/MetadataRequestService.php | 34 +++++---- .../FilesMetadataManagerTest.php | 71 +++++++++++++++++++ 2 files changed, 90 insertions(+), 15 deletions(-) diff --git a/lib/private/FilesMetadata/Service/MetadataRequestService.php b/lib/private/FilesMetadata/Service/MetadataRequestService.php index 06aa54d4eec0c..2bc29a05b465e 100644 --- a/lib/private/FilesMetadata/Service/MetadataRequestService.php +++ b/lib/private/FilesMetadata/Service/MetadataRequestService.php @@ -107,25 +107,29 @@ public function getMetadataFromFileId(int $fileId): IFilesMetadata { * @psalm-return array */ public function getMetadataFromFileIds(array $fileIds): array { - $qb = $this->dbConnection->getQueryBuilder(); - $qb->select('file_id', 'json', 'sync_token') - ->from(self::TABLE_METADATA) - ->where($qb->expr()->in('file_id', $qb->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY))) - ->runAcrossAllShards(); + $chunks = array_chunk($fileIds, IQueryBuilder::MAX_IN_PARAMETERS); $list = []; - $result = $qb->executeQuery(); - while ($data = $result->fetchAssociative()) { - $fileId = (int)$data['file_id']; - $metadata = new FilesMetadata($fileId); - try { - $metadata->importFromDatabase($data); - } catch (FilesMetadataNotFoundException) { - continue; + foreach ($chunks as $chunk) { + $qb = $this->dbConnection->getQueryBuilder(); + $qb->select('file_id', 'json', 'sync_token') + ->from(self::TABLE_METADATA) + ->where($qb->expr()->in('file_id', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY))) + ->runAcrossAllShards(); + + $result = $qb->executeQuery(); + while ($data = $result->fetchAssociative()) { + $fileId = (int)$data['file_id']; + $metadata = new FilesMetadata($fileId); + try { + $metadata->importFromDatabase($data); + } catch (FilesMetadataNotFoundException) { + continue; + } + $list[$fileId] = $metadata; } - $list[$fileId] = $metadata; + $result->closeCursor(); } - $result->closeCursor(); return $list; } diff --git a/tests/lib/FilesMetadata/FilesMetadataManagerTest.php b/tests/lib/FilesMetadata/FilesMetadataManagerTest.php index 31bf2f0f9852c..a44591102da81 100644 --- a/tests/lib/FilesMetadata/FilesMetadataManagerTest.php +++ b/tests/lib/FilesMetadata/FilesMetadataManagerTest.php @@ -13,6 +13,7 @@ use OC\FilesMetadata\FilesMetadataManager; use OC\FilesMetadata\Service\IndexRequestService; use OC\FilesMetadata\Service\MetadataRequestService; +use OCP\DB\IResult; use OCP\DB\QueryBuilder\IExpressionBuilder; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\EventDispatcher\Event; @@ -129,4 +130,74 @@ public function testDropMetadataForFilesChunking(): void { $this->assertSame($expectedChunks, $boundChunks); } + + public function testGetMetadataFromFileIdsChunking(): void { + $connection = $this->createMock(IDBConnection::class); + $qb = $this->createMock(IQueryBuilder::class); + $expr = $this->createMock(IExpressionBuilder::class); + $result = $this->createMock(IResult::class); + + $connection->method('getQueryBuilder')->willReturn($qb); + $qb->method('expr')->willReturn($expr); + $qb->method('select')->willReturnSelf(); + $qb->method('from')->willReturnSelf(); + $qb->method('where')->willReturnSelf(); + $qb->method('runAcrossAllShards')->willReturnSelf(); + $qb->method('executeQuery')->willReturn($result); + $result->method('fetchAssociative')->willReturn(false); + + $fileIds = range(1, IQueryBuilder::MAX_IN_PARAMETERS * 2 + 1); + $expectedChunks = array_chunk($fileIds, IQueryBuilder::MAX_IN_PARAMETERS); + $boundChunks = []; + + $qb->expects($this->exactly(count($expectedChunks))) + ->method('createNamedParameter') + ->willReturnCallback(function (array $chunk, $type) use (&$boundChunks): string { + $this->assertSame(IQueryBuilder::PARAM_INT_ARRAY, $type); + $boundChunks[] = $chunk; + return ':param'; + }); + + $service = new MetadataRequestService($connection, $this->logger); + $service->getMetadataFromFileIds($fileIds); + + $this->assertSame($expectedChunks, $boundChunks); + } + + public function testGetMetadataFromFileIdsMergesResultsAcrossChunks(): void { + $connection = $this->createMock(IDBConnection::class); + $qb = $this->createMock(IQueryBuilder::class); + $expr = $this->createMock(IExpressionBuilder::class); + $result = $this->createMock(IResult::class); + + $connection->method('getQueryBuilder')->willReturn($qb); + $qb->method('expr')->willReturn($expr); + $qb->method('select')->willReturnSelf(); + $qb->method('from')->willReturnSelf(); + $qb->method('where')->willReturnSelf(); + $qb->method('runAcrossAllShards')->willReturnSelf(); + $qb->method('createNamedParameter')->willReturn(':param'); + $qb->method('executeQuery')->willReturn($result); + + $fileIds = range(1, IQueryBuilder::MAX_IN_PARAMETERS + 2); + $firstId = $fileIds[0]; + $lastId = end($fileIds); + $row = static fn (int $fileId): array => [ + 'file_id' => (string)$fileId, + 'json' => '{}', + 'sync_token' => 'token', + ]; + + // one row then end-of-result per chunk, so both chunks contribute + $fetches = [$row($firstId), false, $row($lastId), false]; + $result->method('fetchAssociative') + ->willReturnCallback(function () use (&$fetches) { + return array_shift($fetches); + }); + + $service = new MetadataRequestService($connection, $this->logger); + $metadata = $service->getMetadataFromFileIds($fileIds); + + $this->assertSame([$firstId, $lastId], array_keys($metadata)); + } }