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

@@ -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
+		// only now: an empty listing that listIndex() refused leaves the index as it was
-		// 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
 		$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<string>
+	 * @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<string, list<array{id:string, offset:int, length:int}>> packId => blobs currently located in that pack */
 	public function packs(): array {
 		$packs = [];

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 {
+		$cache = self::cache();
-			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); }
-		};
 		$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 [];