diff --git a/lib/Backend/IBackend.php b/lib/Backend/IBackend.php index 131b293..2d7eb6d 100644 --- a/lib/Backend/IBackend.php +++ b/lib/Backend/IBackend.php @@ -37,5 +37,15 @@ interface IBackend { /** @return list object paths below $prefix (recursive) */ public function list(string $prefix): array; + /** + * Object paths directly in $dir, for a folder that only holds objects (index/, snapshots/, + * catalog/, trash/info/). One listing, without checking the type of every entry: on a remote + * storage list() costs a round trip per entry, which for index/ (one file per pack) grows + * with the size of the repository. + * + * @return list + */ + public function listFiles(string $dir): array; + public function delete(string $path): void; } diff --git a/lib/Backend/LocalBackend.php b/lib/Backend/LocalBackend.php index 1805768..7fdb122 100644 --- a/lib/Backend/LocalBackend.php +++ b/lib/Backend/LocalBackend.php @@ -107,6 +107,22 @@ final class LocalBackend implements IBackend { return $result; } + public function listFiles(string $dir): array { + $base = $this->abs($dir); + if (!is_dir($base)) { + return []; + } + $result = []; + $names = scandir($base); + foreach ($names === false ? [] : $names as $name) { + if ($name !== '.' && $name !== '..' && !StorageBackend::isTemporary($name)) { + $result[] = trim($dir, '/') . '/' . $name; + } + } + sort($result); + return $result; + } + public function delete(string $path): void { @unlink($this->abs($path)); } diff --git a/lib/Backend/StorageBackend.php b/lib/Backend/StorageBackend.php index aff055f..e273888 100644 --- a/lib/Backend/StorageBackend.php +++ b/lib/Backend/StorageBackend.php @@ -133,6 +133,27 @@ final class StorageBackend implements IBackend { } } + public function listFiles(string $dir): array { + $abs = $this->abs($dir); + if (!$this->storage->is_dir($abs)) { + return []; // not created yet + } + $dh = $this->storage->opendir($abs); + if ($dh === false) { + return []; + } + $cut = $this->base === '' ? 0 : strlen($this->base) + 1; + $result = []; + while (($name = readdir($dh)) !== false) { + if ($name !== '.' && $name !== '..' && !self::isTemporary($name)) { + $result[] = substr($abs . '/' . $name, $cut); + } + } + closedir($dh); + sort($result); + return $result; + } + private function listInto(string $dir, array &$result): void { if (!$this->storage->is_dir($dir)) { return; diff --git a/lib/Repository/BlobIndex.php b/lib/Repository/BlobIndex.php index 8831e49..eb7269e 100644 --- a/lib/Repository/BlobIndex.php +++ b/lib/Repository/BlobIndex.php @@ -92,7 +92,7 @@ final class BlobIndex { $cached = $verify ? [] : ($this->cache?->all($this->repositoryId) ?? []); $present = []; $this->downloaded = 0; - foreach ($this->backend->list('index') as $path) { + foreach ($this->backend->listFiles('index') as $path) { $packId = basename($path); $present[$packId] = true; if (isset($cached[$packId])) { diff --git a/lib/Repository/Catalog.php b/lib/Repository/Catalog.php index b8a3c47..7dae152 100644 --- a/lib/Repository/Catalog.php +++ b/lib/Repository/Catalog.php @@ -128,7 +128,7 @@ final class Catalog { /** @return list */ private function generations(): array { - return array_map(fn ($p) => (int)basename($p), $this->backend->list('catalog')); + return array_map(fn ($p) => (int)basename($p), $this->backend->listFiles('catalog')); } private function path(int $gen): string { diff --git a/lib/Repository/Repository.php b/lib/Repository/Repository.php index 8c98ce3..d6c4f1d 100644 --- a/lib/Repository/Repository.php +++ b/lib/Repository/Repository.php @@ -92,8 +92,8 @@ final class Repository { private function loadCatalog(?ICatalogAnchor $anchor): void { $this->catalog = new Catalog($this->backend, $this->cipher, $this->repositoryId, $anchor); $this->catalog->load( - fn () => array_map('basename', $this->backend->list('snapshots')), - fn () => array_map('basename', $this->backend->list('trash/info')), + fn () => array_map('basename', $this->backend->listFiles('snapshots')), + fn () => array_map('basename', $this->backend->listFiles('trash/info')), fn () => $this->backend->list('users')); } @@ -643,7 +643,7 @@ final class Repository { * @return list */ public function listSnapshots(): array { - $present = array_flip(array_map('basename', $this->backend->list('snapshots'))); + $present = array_flip(array_map('basename', $this->backend->listFiles('snapshots'))); $listed = array_keys($this->catalog->snapshots); foreach ($listed as $id) { if (!isset($present[$id])) { diff --git a/tests/Unit/Backend/StorageBackendListTest.php b/tests/Unit/Backend/StorageBackendListTest.php new file mode 100644 index 0000000..8c4deee --- /dev/null +++ b/tests/Unit/Backend/StorageBackendListTest.php @@ -0,0 +1,47 @@ +tempDir(); + foreach (['b1', 'a2', 'c3.part-0123abcd'] as $name) { + touch("$dir/$name"); + } + $isDirCalls = 0; + $storage = $this->createMock(IStorage::class); + $storage->method('instanceOfStorage')->willReturn(false); + $storage->method('is_dir')->willReturnCallback(function (string $path) use (&$isDirCalls): bool { + $isDirCalls++; + return $path === 'repo' || $path === 'repo/index'; + }); + $storage->method('opendir')->willReturnCallback(fn (string $path) => $path === 'repo/index' ? opendir($dir) : false); + + $backend = new StorageBackend($storage, 'repo'); + $isDirCalls = 0; // the constructor checks the base folder + $this->assertSame(['index/a2', 'index/b1'], $backend->listFiles('index')); + $this->assertSame(1, $isDirCalls); + + $isDirCalls = 0; + $this->assertSame(['index/a2', 'index/b1'], $backend->list('index')); + $this->assertSame(4, $isDirCalls, 'list() checks every entry: the reason listFiles() exists'); + } + + public function testListFilesOfAMissingFolderIsEmpty(): void { + $storage = $this->createMock(IStorage::class); + $storage->method('instanceOfStorage')->willReturn(false); + $storage->method('is_dir')->willReturnCallback(fn (string $path): bool => $path === 'repo'); + $storage->expects($this->never())->method('opendir'); + $this->assertSame([], (new StorageBackend($storage, 'repo'))->listFiles('snapshots')); + } +} diff --git a/tests/Unit/Repository/VerifyTest.php b/tests/Unit/Repository/VerifyTest.php index 117321f..fccdcb7 100644 --- a/tests/Unit/Repository/VerifyTest.php +++ b/tests/Unit/Repository/VerifyTest.php @@ -166,6 +166,10 @@ final class FlakyRangeBackend implements IBackend { return $this->inner->list($prefix); } + public function listFiles(string $dir): array { + return $this->inner->listFiles($dir); + } + public function delete(string $path): void { $this->inner->delete($path); } diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 06fdcb4..63f664b 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -3,3 +3,13 @@ declare(strict_types=1); require __DIR__ . '/../vendor/autoload.php'; + +// nextcloud/ocp ships the public interfaces without an autoloader; tests that mock them need one. +spl_autoload_register(static function (string $class): void { + if (str_starts_with($class, 'OCP\\')) { + $file = __DIR__ . '/../vendor/nextcloud/ocp/' . str_replace('\\', '/', $class) . '.php'; + if (is_file($file)) { + require $file; + } + } +});