From 61933483151e40421c30fb71b73536559b1d5984 Mon Sep 17 00:00:00 2001 From: Anna Larch Date: Wed, 3 Jun 2026 13:34:35 +0200 Subject: [PATCH 1/2] fix(files): chunk filecache IN queries to respect Oracle's 1000-element limit Oracle raises ORA-01795 when an IN list exceeds 1000 elements. Both getByFileIds and getByFileIdsInStorage passed the full input array directly into an IN clause with no upper bound, so any caller supplying more than 1000 file IDs would get a hard DB error on Oracle. files_sharing/Updater collects all of a user's share node IDs without limiting the count before calling getByFileIdsInStorage, making this a realistic failure path for power users. Chunk both methods at 1000 and merge results, keeping the method signatures and return types identical. Add an early return for empty input to avoid issuing a vacuous IN query. Assisted-by: ClaudeCode:claude-sonnet-4-6 Signed-off-by: Anna Larch --- lib/private/Files/Cache/FileAccess.php | 33 +++++++----- tests/lib/Files/Cache/FileAccessTest.php | 65 ++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 12 deletions(-) diff --git a/lib/private/Files/Cache/FileAccess.php b/lib/private/Files/Cache/FileAccess.php index 016ff6de46ec7..6de080e7fc704 100644 --- a/lib/private/Files/Cache/FileAccess.php +++ b/lib/private/Files/Cache/FileAccess.php @@ -81,11 +81,16 @@ private function rowsToEntries(array $rows): array { */ #[\Override] public function getByFileIds(array $fileIds): array { - $query = $this->getQuery()->selectFileCache(); - $query->andWhere($query->expr()->in('filecache.fileid', $query->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY))); - - $rows = $query->executeQuery()->fetchAll(); - return $this->rowsToEntries($rows); + if (empty($fileIds)) { + return []; + } + $result = []; + foreach (array_chunk($fileIds, 1000) as $chunk) { + $query = $this->getQuery()->selectFileCache(); + $query->andWhere($query->expr()->in('filecache.fileid', $query->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY))); + $result += $this->rowsToEntries($query->executeQuery()->fetchAll()); + } + return $result; } /** @@ -95,13 +100,17 @@ public function getByFileIds(array $fileIds): array { */ #[\Override] public function getByFileIdsInStorage(array $fileIds, int $storageId): array { - $fileIds = array_values($fileIds); - $query = $this->getQuery()->selectFileCache(); - $query->andWhere($query->expr()->in('filecache.fileid', $query->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY))); - $query->andWhere($query->expr()->eq('filecache.storage', $query->createNamedParameter($storageId, IQueryBuilder::PARAM_INT))); - - $rows = $query->executeQuery()->fetchAll(); - return $this->rowsToEntries($rows); + if (empty($fileIds)) { + return []; + } + $result = []; + foreach (array_chunk(array_values($fileIds), 1000) as $chunk) { + $query = $this->getQuery()->selectFileCache(); + $query->andWhere($query->expr()->in('filecache.fileid', $query->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY))); + $query->andWhere($query->expr()->eq('filecache.storage', $query->createNamedParameter($storageId, IQueryBuilder::PARAM_INT))); + $result += $this->rowsToEntries($query->executeQuery()->fetchAll()); + } + return $result; } #[\Override] diff --git a/tests/lib/Files/Cache/FileAccessTest.php b/tests/lib/Files/Cache/FileAccessTest.php index 169a07180eeca..5355a7a9baff5 100644 --- a/tests/lib/Files/Cache/FileAccessTest.php +++ b/tests/lib/Files/Cache/FileAccessTest.php @@ -303,6 +303,71 @@ private function setUpTestDatabaseForGetByAncestorInStorage(): void { ->executeStatement(); } + public function testGetByFileIds(): void { + $result = $this->fileAccess->getByFileIds([1, 2, 3]); + + $this->assertCount(3, $result); + $this->assertArrayHasKey(1, $result); + $this->assertArrayHasKey(2, $result); + $this->assertArrayHasKey(3, $result); + } + + public function testGetByFileIdsWithEmptyArray(): void { + $result = $this->fileAccess->getByFileIds([]); + $this->assertCount(0, $result); + } + + public function testGetByFileIdsWithNonExistentIds(): void { + $result = $this->fileAccess->getByFileIds([9998, 9999]); + $this->assertCount(0, $result); + } + + public function testGetByFileIdsSpanningChunkBoundary(): void { + // 1001 IDs exceeds Oracle's 1000-element IN limit — verifies chunked queries return correct results + $ids = range(1, 1001); + $result = $this->fileAccess->getByFileIds($ids); + + // Only fileids 1–7 exist in the test data + $this->assertCount(7, $result); + foreach (range(1, 7) as $id) { + $this->assertArrayHasKey($id, $result); + } + } + + public function testGetByFileIdsInStorage(): void { + $result = $this->fileAccess->getByFileIdsInStorage([1, 2, 3], 1); + + $this->assertCount(3, $result); + $this->assertArrayHasKey(1, $result); + $this->assertArrayHasKey(2, $result); + $this->assertArrayHasKey(3, $result); + } + + public function testGetByFileIdsInStorageFiltersStorage(): void { + // fileids 6 and 7 belong to storage 2, not storage 1 + $result = $this->fileAccess->getByFileIdsInStorage([1, 6, 7], 1); + + $this->assertCount(1, $result); + $this->assertArrayHasKey(1, $result); + } + + public function testGetByFileIdsInStorageWithEmptyArray(): void { + $result = $this->fileAccess->getByFileIdsInStorage([], 1); + $this->assertCount(0, $result); + } + + public function testGetByFileIdsInStorageSpanningChunkBoundary(): void { + // 1001 IDs exceeds Oracle's 1000-element IN limit — verifies chunked queries return correct results + $ids = range(1, 1001); + $result = $this->fileAccess->getByFileIdsInStorage($ids, 1); + + // fileids 1–5 are in storage 1; 6 and 7 are in storage 2 + $this->assertCount(5, $result); + foreach (range(1, 5) as $id) { + $this->assertArrayHasKey($id, $result); + } + } + /** * Test fetching files by ancestor in storage. */ From bd1f48c16e934f5d61a681e852baa57ce815f1de Mon Sep 17 00:00:00 2001 From: Anna Larch Date: Wed, 3 Jun 2026 13:36:11 +0200 Subject: [PATCH 2/2] refactor(files): remove manual chunking in SetupManager now redundant getByFileIds now chunks internally, so the array_chunk loop in SetupManager::setupForPath was doing a double chunk. Replace with a direct call. Assisted-by: ClaudeCode:claude-sonnet-4-6 Signed-off-by: Anna Larch --- lib/private/Files/SetupManager.php | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/lib/private/Files/SetupManager.php b/lib/private/Files/SetupManager.php index a8c46a20bd31a..184a9fc412924 100644 --- a/lib/private/Files/SetupManager.php +++ b/lib/private/Files/SetupManager.php @@ -604,12 +604,7 @@ public function setupForPath(string $path, bool $includeChildren = false): void array_merge(...array_values($authoritativeCachedMounts)), ); - $rootsMetadata = []; - foreach (array_chunk($rootIds, 1000) as $chunk) { - foreach ($this->fileAccess->getByFileIds($chunk) as $id => $fileMetadata) { - $rootsMetadata[$id] = $fileMetadata; - } - } + $rootsMetadata = $this->fileAccess->getByFileIds($rootIds); $this->setupMountProviderPaths[$mountPoint] = self::SETUP_WITH_CHILDREN; foreach ($authoritativeCachedMounts as $providerClass => $cachedMounts) { $providerArgs = array_values(array_filter(array_map(