SFTP: replace a dropped connection; write the index cache in one transaction

A Hetzner Storage Box drops an SFTP connection that carried a lot of data and then stayed quiet for a few minutes; files_external keeps using the dead client, on which every file looks missing. StorageBackend now checks the connection after a minute without requests, and replaces a connection that is dead after a failed request (once per request; put, move and delete are not repeated). SftpReadStream closes a connection it left in an unknown state. The verify cache refresh took more than 6 minutes on PostgreSQL for 27,547 packs, one autocommitted insert per pack: putMany()/replaceAll() write in one transaction. After a verify the cache holds the index files as they are, so a blob in two packs stays cached for both.
2026-10-12 01:58:54 +02:00
parent d659322e60
commit 9c06ab2841
9 changed files with 442 additions and 33 deletions
@@ -0,0 +1,146 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OCA\NgBackup\Tests\Unit\Backend;
+
+use OCA\NgBackup\Backend\BackendException;
+use OCA\NgBackup\Backend\StorageBackend;
+use OCP\Files\Storage\IStorage;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * A Hetzner Storage Box drops an SFTP connection that carried a lot of data and then stayed
+ * quiet for a few minutes; files_external keeps using the dead client, on which every file
+ * looks missing. The backend builds the storage anew instead.
+ */
+class StorageBackendReconnectTest extends TestCase {
+	/** @var list<ReconnectFakeSftp> */
+	private array $connections = [];
+	/** New connections come up dead (the location is gone). */
+	private bool $deadOnArrival = false;
+
+	private function storage(): IStorage {
+		$sftp = new ReconnectFakeSftp();
+		$sftp->dropped = $this->deadOnArrival;
+		$this->connections[] = $sftp;
+		$storage = $this->createMock(SftpLikeStorage::class);
+		$storage->method('instanceOfStorage')->willReturnCallback(fn (string $class): bool => $class === \OCA\Files_External\Lib\Storage\SFTP::class);
+		$storage->method('getConnection')->willReturn($sftp);
+		$storage->method('is_dir')->willReturn(true);
+		$storage->method('file_exists')->willReturnCallback(fn (string $path): bool => $sftp->request() && $path === 'repo/config');
+		$storage->method('fopen')->willReturnCallback(function (string $path) use ($sftp) {
+			if (!$sftp->request() || $path !== 'repo/config') {
+				return false;
+			}
+			$fh = fopen('php://memory', 'w+b');
+			fwrite($fh, 'config');
+			rewind($fh);
+			return $fh;
+		});
+		return $storage;
+	}
+
+	private function backend(bool $reconnect = true): StorageBackend {
+		return new StorageBackend($this->storage(), 'repo', false, $reconnect ? fn (): IStorage => $this->storage() : null);
+	}
+
+	private static function idle(StorageBackend $backend): void {
+		(new \ReflectionProperty($backend, 'lastUsed'))->setValue($backend, microtime(true) - StorageBackend::IDLE_CHECK - 1);
+	}
+
+	public function testADeadConnectionIsReplacedAfterAPause(): void {
+		$backend = $this->backend();
+		$this->connections[0]->dropped = true;
+		self::idle($backend);
+		$this->assertTrue($backend->exists('config'));
+		$this->assertSame(1, $backend->reconnects);
+		$this->assertSame(1, $this->connections[0]->requests, 'only the check went over the old connection');
+	}
+
+	public function testAPauseOnALiveConnectionCostsOneCheck(): void {
+		$backend = $this->backend();
+		self::idle($backend);
+		$this->assertTrue($backend->exists('config'));
+		$this->assertSame(0, $backend->reconnects);
+		$this->assertSame(2, $this->connections[0]->requests);
+		$this->assertTrue($backend->exists('config'));
+		$this->assertSame(3, $this->connections[0]->requests, 'no check without a pause');
+	}
+
+	public function testAFailureOnAConnectionThatJustDiedIsRepeated(): void {
+		$backend = $this->backend();
+		$this->connections[0]->dropped = true; // without a pause: not checked first
+		$this->assertTrue($backend->exists('config'));
+		$fh = $backend->get('config');
+		$this->assertSame('config', stream_get_contents($fh));
+		$this->assertSame(1, $backend->reconnects);
+	}
+
+	public function testAMissingFileOnALiveConnectionIsJustMissing(): void {
+		$backend = $this->backend();
+		$this->assertFalse($backend->exists('nothing'));
+		try {
+			$backend->get('nothing');
+			$this->fail('missing');
+		} catch (BackendException $e) {
+			$this->assertStringContainsString('Not found', $e->getMessage());
+		}
+		$this->assertSame(0, $backend->reconnects);
+		$this->assertCount(1, $this->connections);
+	}
+
+	public function testANewConnectionThatFailsToo(): void {
+		$backend = $this->backend();
+		$this->connections[0]->dropped = true;
+		$backend->exists('config'); // reconnects
+		$this->connections[1]->dropped = true;
+		$this->deadOnArrival = true;
+		self::idle($backend);
+		try {
+			$backend->get('config');
+			$this->fail('no connection');
+		} catch (BackendException) {
+		}
+		$this->assertSame(3, $backend->reconnects, 'once after the pause, once after the failure, no more');
+	}
+
+	public function testWithoutAWayToReconnectNothingChanges(): void {
+		$backend = $this->backend(false);
+		$this->connections[0]->dropped = true;
+		self::idle($backend);
+		$this->assertFalse($backend->exists('config'));
+		$this->assertCount(1, $this->connections);
+	}
+}
+
+interface SftpLikeStorage extends IStorage {
+	public function getConnection(): object;
+}
+
+/** The public phpseclib calls the backend uses to tell a dead connection. */
+final class ReconnectFakeSftp {
+	public bool $dropped = false;
+	private bool $connected = true;
+	public int $requests = 0;
+
+	public function isConnected(): bool {
+		return $this->connected;
+	}
+
+	public function clearStatCache(): void {
+	}
+
+	public function stat(string $path): array|false {
+		return $this->request() ? ['type' => 2] : false;
+	}
+
+	/** A request: false (and from then on disconnected) when the server has dropped the connection. */
+	public function request(): bool {
+		$this->requests++;
+		if ($this->dropped) {
+			$this->connected = false;
+		}
+		return $this->connected;
+	}
+}
@@ -20,6 +20,8 @@ class IndexCacheTest extends TestCase {
 			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); }
 		};
 		$src = $this->tempDir();
 		for ($i = 0; $i < 12; $i++) {
@@ -65,6 +67,8 @@ class IndexCacheTest extends TestCase {
 				$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 LocalBackend($this->tempDir());
 		$keys = KeyRing::generate();
@@ -88,4 +92,38 @@ class IndexCacheTest extends TestCase {
 		$this->assertCount(count($backend->list('index')), array_merge(...array_values($cache->data)));
 		$this->assertSame(0, Repository::openWithKey($backend, $keys, $cache)->indexDownloads());
 	}
+
+	/** After a check the cache holds every index file as it is, also a blob that is in two packs. */
+	public function testACheckCachesTheIndexFilesAsTheyAre(): void {
+		$cache = new class implements IIndexCache {
+			public array $data = [];
+			public int $replaced = 0;
+			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->replaced++; $this->data[$r] = $packs; }
+		};
+		$backend = new LocalBackend($this->tempDir());
+		$cipher = new \OCA\NgBackup\Crypto\StreamCipher(KeyRing::generate());
+		$index = new \OCA\NgBackup\Repository\BlobIndex($backend, $cipher);
+		$index->setCache($cache, 'r');
+		$blob = static fn (string $id, int $offset): array => ['id' => $id, 'offset' => $offset, 'length' => 10, 'raw' => 8, 'flags' => 0];
+		$index->addPack('pack1', [$blob('a', 0), $blob('shared', 10)]);
+		$index->addPack('pack2', [$blob('shared', 0), $blob('b', 10)]);
+		$cache->data = ['r' => ['gone' => [$blob('x', 0)]]];
+
+		$index->load(true);
+		$this->assertSame(1, $cache->replaced);
+		$this->assertSame(['pack1', 'pack2'], array_keys($cache->data['r']));
+		$this->assertCount(2, $cache->data['r']['pack1'], 'the shared blob stays in the row of its first pack');
+
+		// the second pack goes: the shared blob is still found in the first
+		$index->removePack('pack2');
+		$again = new \OCA\NgBackup\Repository\BlobIndex($backend, $cipher);
+		$again->setCache($cache, 'r');
+		$again->load();
+		$this->assertSame(0, $again->downloaded);
+		$this->assertSame('pack1', $again->get('shared')[0]);
+	}
 }