From 95184dadc75f9a7d9183c44426de2fa6da0b32b0 Mon Sep 17 00:00:00 2001 From: Dillard Blom Date: Wed, 21 Oct 2026 19:51:46 +0200 Subject: [PATCH] Index: an empty listing is checked (config present, listed once more) before it is taken, so a repository another server emptied is accepted and a hiccup recovers --- lib/Repository/BlobIndex.php | 38 +++++++---- tests/Unit/Repository/IndexListingTest.php | 76 ++++++++++++++++++---- 2 files changed, 91 insertions(+), 23 deletions(-) diff --git a/lib/Repository/BlobIndex.php b/lib/Repository/BlobIndex.php index fc8e844..e0d9c87 100644 --- a/lib/Repository/BlobIndex.php +++ b/lib/Repository/BlobIndex.php @@ -99,7 +99,8 @@ final class BlobIndex { $this->downloaded = 0; $order = []; // pack ids in listing order: where a blob is in two packs, the last listed wins $fetch = []; - foreach ($this->backend->listFiles('index') as $path) { + $known = $hadIndex || $cached !== [] || ($verify && ($this->cache?->all($this->repositoryId) ?? []) !== []); + foreach ($this->listIndex($known) as $path) { $packId = basename($path); $present[$packId] = true; $order[] = $packId; @@ -107,13 +108,7 @@ final class BlobIndex { $fetch[] = $path; } } - // A repository never loses all its index files: an empty listing is a connection that - // failed quietly. Taken as is, it would empty the index and the cache (or make a check - // report every blob missing, or a cleanup wipe the cache). - if ($present === [] && ($hadIndex || $cached !== [] || ($verify && ($this->cache?->all($this->repositoryId) ?? []) !== []))) { - throw new BackendException('The location listed no index files, while some are known: not trusting that listing'); - } - // only now: an empty listing that was refused above leaves the index as it was + // only now: an empty listing that listIndex() refused leaves the index as it was $this->blobs = []; $this->loaded = $present; // The rest is downloaded several at a time where the location allows it: on a large @@ -213,15 +208,12 @@ final class BlobIndex { public function refresh(): void { $fetch = []; $listed = 0; - foreach ($this->backend->listFiles('index') as $path) { + foreach ($this->listIndex($this->loaded !== []) as $path) { $listed++; if (!isset($this->loaded[basename($path)])) { $fetch[] = $path; } } - if ($listed === 0 && $this->loaded !== []) { - throw new BackendException('The location listed no index files, while ' . count($this->loaded) . ' are known: not trusting that listing'); - } $toCache = []; foreach ($this->backend->getMany($fetch) as $path => $data) { $packId = basename($path); @@ -237,6 +229,28 @@ final class BlobIndex { } } + /** + * The index files on the location. An empty listing while index files are known ($known) is + * usually a connection that failed quietly; taken as is, it would empty the index and the + * cache, make a check report every blob missing, or make a cleanup wipe the cache. So the + * connection is checked (the repository's config must be there) and the folder listed once + * more: empty again on a working connection, the repository really is empty (another server + * removed its last snapshot and cleaned up). + * + * @return list + * @throws BackendException when the location does not answer as it should + */ + private function listIndex(bool $known): array { + $paths = $this->backend->listFiles('index'); + if ($paths !== [] || !$known) { + return $paths; + } + if (!$this->backend->exists('config')) { + throw new BackendException('The location listed no index files and has no repository config: not trusting that listing'); + } + return $this->backend->listFiles('index'); + } + /** @return array> packId => blobs currently located in that pack */ public function packs(): array { $packs = []; diff --git a/tests/Unit/Repository/IndexListingTest.php b/tests/Unit/Repository/IndexListingTest.php index b014fb4..a0f3f97 100644 --- a/tests/Unit/Repository/IndexListingTest.php +++ b/tests/Unit/Repository/IndexListingTest.php @@ -38,6 +38,10 @@ class IndexListingTest extends TestCase { $this->snapshot = $repo->backupDirectory($this->src)['snapshot']; } + private static function cache(): IIndexCache { + return indexCache(); + } + public function testARestoreGoesOnAfterAPackReadFailedAndTheListingCameBackEmpty(): void { $backend = new FlakyBackend(new LocalBackend($this->root)); $repo = Repository::openWithKey($backend, $this->keys); @@ -68,7 +72,7 @@ class IndexListingTest extends TestCase { public function testACheckWithAnEmptyListingIsAnErrorNotEveryBlobMissing(): void { $backend = new FlakyBackend(new LocalBackend($this->root)); $repo = Repository::openWithKey($backend, $this->keys); - $backend->emptyIndexListings = 1; + $backend->dead = true; $this->expectException(BackendException::class); $this->expectExceptionMessage('listed no index files'); @@ -86,21 +90,54 @@ class IndexListingTest extends TestCase { $this->assertSame(0, $index->downloaded, 'every index file is loaded already, also those whose blobs are elsewhere too'); } + public function testARestoreGoesOnWhenTheLocationCannotBeListedAfterAFailedRead(): void { + $backend = new FlakyBackend(new LocalBackend($this->root)); + $repo = Repository::openWithKey($backend, $this->keys); + $backend->failPackReads = 1; + $backend->dead = true; // listing and config look gone; reading packs works again + $target = $this->tempDir(); + + $this->assertSame(12, $repo->restore($this->snapshot, $target)); + + $this->assertSame(self::hashTree($this->src), self::hashTree($target)); + } + + public function testAnEmptyListingThatWasAHiccupIsListedAgain(): void { + $cache = self::cache(); + $backend = new FlakyBackend(new LocalBackend($this->root)); + Repository::openWithKey($backend, $this->keys, $cache); + $cached = $cache->data; + $backend->emptyIndexListings = 1; + + $again = Repository::openWithKey($backend, $this->keys, $cache); + + $this->assertSame($cached, $cache->data); + $this->assertSame(12, $again->restore($this->snapshot, $this->tempDir())); + } + + public function testARepositoryThatReallyBecameEmptyIsTakenAsEmpty(): void { + $cache = self::cache(); + $backend = new FlakyBackend(new LocalBackend($this->root)); + Repository::openWithKey($backend, $this->keys, $cache); + $this->assertNotSame([], array_filter($cache->data)); + // another server forgot the last snapshot and cleaned up: no packs and index files left + foreach (array_merge($backend->list('index'), $backend->list('packs')) as $path) { + $backend->delete($path); + } + + Repository::openWithKey($backend, $this->keys, $cache); + + $this->assertSame([], array_filter($cache->data), 'the stale cache rows are gone'); + } + public function testOpeningWithAnEmptyListingKeepsTheCachedIndex(): void { - $cache = new class implements IIndexCache { - public array $data = []; - public function all(string $r): array { return $this->data[$r] ?? []; } - public function put(string $r, string $p, array $e): void { $this->data[$r][$p] = $e; } - public function remove(string $r, array $ps): void { foreach ($ps as $p) { unset($this->data[$r][$p]); } } - public function putMany(string $r, array $packs): void { foreach ($packs as $p => $e) { $this->put($r, (string)$p, $e); } } - public function replaceAll(string $r, array $packs): void { $this->data[$r] = []; $this->putMany($r, $packs); } - }; + $cache = self::cache(); $backend = new FlakyBackend(new LocalBackend($this->root)); Repository::openWithKey($backend, $this->keys, $cache); $cached = $cache->data; $this->assertNotSame([], $cached); - $backend->emptyIndexListings = 1; + $backend->dead = true; try { Repository::openWithKey($backend, $this->keys, $cache); $this->fail('an empty listing of a repository with an index must not be taken as is'); @@ -111,12 +148,26 @@ class IndexListingTest extends TestCase { } } +/** @internal */ +function indexCache(): IIndexCache { + return new class implements IIndexCache { + public array $data = []; + public function all(string $r): array { return $this->data[$r] ?? []; } + public function put(string $r, string $p, array $e): void { $this->data[$r][$p] = $e; } + public function remove(string $r, array $ps): void { foreach ($ps as $p) { unset($this->data[$r][$p]); } } + public function putMany(string $r, array $packs): void { foreach ($packs as $p => $e) { $this->put($r, (string)$p, $e); } } + public function replaceAll(string $r, array $packs): void { $this->data[$r] = []; $this->putMany($r, $packs); } + }; +} + /** A location whose pack reads fail, and whose index listings come back empty, a set number of times. */ final class FlakyBackend implements IBackend { use \OCA\NgBackup\Backend\GetsOneByOne; public int $failPackReads = 0; public int $emptyIndexListings = 0; + /** The connection is gone: also the repository config looks missing. */ + public bool $dead = false; public function __construct( private IBackend $inner, @@ -144,7 +195,7 @@ final class FlakyBackend implements IBackend { } public function exists(string $path): bool { - return $this->inner->exists($path); + return !$this->dead && $this->inner->exists($path); } public function move(string $from, string $to): void { @@ -156,6 +207,9 @@ final class FlakyBackend implements IBackend { } public function listFiles(string $dir): array { + if ($dir === 'index' && $this->dead) { + return []; + } if ($dir === 'index' && $this->emptyIndexListings > 0) { $this->emptyIndexListings--; return [];