From ee406c481e27072e444091081b1984d19fd4d994 Mon Sep 17 00:00:00 2001 From: Dillard Blom Date: Fri, 6 Nov 2026 18:27:00 +0100 Subject: [PATCH] Restore: go on after a partial write; created tables stay once the database is committed --- lib/Db/DbDumper.php | 30 ++++++++++----------- lib/Repository/Repository.php | 9 +++++-- tests/Unit/Repository/RestoreTargetTest.php | 30 +++++++++++++++++++++ 3 files changed, 52 insertions(+), 17 deletions(-) diff --git a/lib/Db/DbDumper.php b/lib/Db/DbDumper.php index ef4bc72..2a532d2 100644 --- a/lib/Db/DbDumper.php +++ b/lib/Db/DbDumper.php @@ -225,21 +225,6 @@ final class DbDumper { $this->restoreSequences($manifest['sequences'] ?? [], $report['missing'], array_merge($report['restored'], $report['skipped'])); $this->resumeForeignKeys($foreignKeys); $this->db->commit(); - // Foreign keys of the tables created above, now their rows are in. Not inside the - // transaction: MySQL commits on DDL. A failure is reported, not fatal: the data is - // restored either way. - foreach ($report['created'] as $table) { - $structure = $manifest['tables'][$table]['schema'] ?? []; - foreach ($structure['foreignKeys'] ?? [] as $fk) { - try { - $this->db->createSchemaManager()->createForeignKey( - new \Doctrine\DBAL\Schema\ForeignKeyConstraint($fk['columns'], $fk['table'], $fk['foreign'], $fk['name'], $fk['options'] ?? []), - $table); - } catch (\Throwable $e) { - $report['foreignKeyErrors'][] = $table . '.' . $fk['name'] . ': ' . $e->getMessage(); - } - } - } } catch (\Throwable $e) { $this->db->rollBack(); // PostgreSQL rolls the dropped foreign keys back with the transaction; MySQL's session @@ -250,6 +235,21 @@ final class DbDumper { $this->dropCreatedTables($report); throw $e; } + // Committed. Foreign keys of the tables created above, now their rows are in. Not inside the + // transaction: MySQL commits on DDL. A failure is reported, not fatal: the data is restored + // either way, and the created tables stay. + foreach ($report['created'] as $table) { + $structure = $manifest['tables'][$table]['schema'] ?? []; + foreach ($structure['foreignKeys'] ?? [] as $fk) { + try { + $this->db->createSchemaManager()->createForeignKey( + new \Doctrine\DBAL\Schema\ForeignKeyConstraint($fk['columns'], $fk['table'], $fk['foreign'], $fk['name'], $fk['options'] ?? []), + $table); + } catch (\Throwable $e) { + $report['foreignKeyErrors'][] = $table . '.' . $fk['name'] . ': ' . $e->getMessage(); + } + } + } return $total; } diff --git a/lib/Repository/Repository.php b/lib/Repository/Repository.php index 947365d..e900b25 100644 --- a/lib/Repository/Repository.php +++ b/lib/Repository/Repository.php @@ -451,12 +451,17 @@ final class Repository { /** * Write all of $data, or stop: a short write leaves a file that looks damaged only by its size. + * A write may take only part of the data and still be fine; one that takes nothing is not. * * @param resource $out */ public static function writeAll($out, string $data, string $path): void { - if (@fwrite($out, $data) !== strlen($data)) { - throw new RestoreTargetException('Cannot write ' . $path . ' to the restore directory (disk full?)'); + while ($data !== '') { + $written = @fwrite($out, $data); + if ($written === false || $written === 0) { + throw new RestoreTargetException('Cannot write ' . $path . ' to the restore directory (disk full?)'); + } + $data = substr($data, $written); } } diff --git a/tests/Unit/Repository/RestoreTargetTest.php b/tests/Unit/Repository/RestoreTargetTest.php index 81e47fc..c04d8c6 100644 --- a/tests/Unit/Repository/RestoreTargetTest.php +++ b/tests/Unit/Repository/RestoreTargetTest.php @@ -12,6 +12,24 @@ use OCA\NgBackup\Repository\RestoreTargetException; use OCA\NgBackup\Tests\Unit\TempDirTrait; use PHPUnit\Framework\TestCase; +/** A stream that takes at most 3 bytes per write. */ +class PartialWriteStream { + public static string $written = ''; + /** @var resource|null set by PHP */ + public $context; + + public function stream_open(string $path, string $mode, int $options, ?string &$opened): bool { + self::$written = ''; + return true; + } + + public function stream_write(string $data): int { + $part = substr($data, 0, 3); + self::$written .= $part; + return strlen($part); + } +} + /** * What goes wrong where a restore writes to is not damage in the backup: it stops the restore, also * with --salvage, and the message does not point at --salvage. @@ -48,6 +66,18 @@ class RestoreTargetTest extends TestCase { Repository::writeAll($out, 'data', 'dir/a.bin'); } + public function testAWriteThatTakesPartOfTheDataGoesOn(): void { + stream_wrapper_register('ngbpartial', PartialWriteStream::class); + try { + $out = fopen('ngbpartial://x', 'wb'); + Repository::writeAll($out, 'abcdefgh', 'dir/a.bin'); + fclose($out); + $this->assertSame('abcdefgh', PartialWriteStream::$written); + } finally { + stream_wrapper_unregister('ngbpartial'); + } + } + public function testATargetErrorDoesNotLookLikeDamage(): void { $this->assertFalse(Restore::backupLooksDamaged(new RestoreTargetException('Cannot write'))); }