Merge pull request #1927 from Jose-Tejera/agent/verify-migration-writes

Make migration destination writes reliable

bin/migrate

@@ -110,7 +110,10 @@ function saveComment ($force_overwrite, $dryrun, $pasteid, $comment, $dststore)
         if (!$dryrun) {
             debug("Saving document ID " . $pasteid . ", parent id " .
                   $parentid . ", comment id " . $commentid);
-            $dststore->createComment($pasteid, $parentid, $commentid, $comment);
+            if (!$dststore->createComment($pasteid, $parentid, $commentid, $comment)) {
+                dieerr("Unable to save document ID " . $pasteid . ", parent id " .
+                       $parentid . ", comment id " . $commentid);
+            }
         } else {
             debug("Would save document ID " . $pasteid . ", parent id " .
                   $parentid . ", comment id " . $commentid);
@@ -119,7 +122,10 @@ function saveComment ($force_overwrite, $dryrun, $pasteid, $comment, $dststore)
         if (!$dryrun) {
             debug("Overwriting document ID " . $pasteid . ", parent id " .
                   $parentid . ", comment id " . $commentid);
-            $dststore->createComment($pasteid, $parentid, $commentid, $comment);
+            if (!$dststore->createComment($pasteid, $parentid, $commentid, $comment)) {
+                dieerr("Unable to overwrite document ID " . $pasteid . ", parent id " .
+                       $parentid . ", comment id " . $commentid);
+            }
         } else {
             debug("Would overwrite document ID " . $pasteid . ", parent id " .
                   $parentid . ", comment id " . $commentid);
@@ -140,14 +146,19 @@ function savePaste ($force_overwrite, $dryrun, $pasteid, $paste, $dststore)
     if (!$dststore->exists($pasteid)) {
         if (!$dryrun) {
             debug("Saving document ID " . $pasteid);
-            $dststore->create($pasteid, $paste);
+            if (!$dststore->create($pasteid, $paste)) {
+                dieerr("Unable to save document ID " . $pasteid);
+            }
         } else {
             debug("Would save document ID " . $pasteid);
         }
     } else if ($force_overwrite) {
         if (!$dryrun) {
             debug("Overwriting document ID " . $pasteid);
-            $dststore->create($pasteid, $paste);
+            $dststore->delete($pasteid);
+            if (!$dststore->create($pasteid, $paste)) {
+                dieerr("Unable to overwrite document ID " . $pasteid);
+            }
         } else {
             debug("Would overwrite document ID " . $pasteid);
         }

tst/MigrateForceOverwriteTest.php

@@ -0,0 +1,86 @@
+<?php declare(strict_types=1);
+use PHPUnit\Framework\TestCase;
+use PrivateBin\Data\Filesystem;
+
+class MigrateForceOverwriteTest extends TestCase
+{
+    private $_destination;
+
+    private $_path;
+
+    private $_source;
+
+    public function setUp(): void
+    {
+        $this->_path = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'privatebin_migrate_force';
+        Helper::rmDir($this->_path);
+        mkdir($this->_path);
+        mkdir($this->_path . DIRECTORY_SEPARATOR . 'source_cfg');
+        mkdir($this->_path . DIRECTORY_SEPARATOR . 'destination_cfg');
+
+        $options                         = parse_ini_file(CONF_SAMPLE, true);
+        $options['model_options']['dir'] = $this->_path . DIRECTORY_SEPARATOR . 'source';
+        $this->_source                   = new Filesystem($options['model_options']);
+        Helper::createIniFile(
+            $this->_path . DIRECTORY_SEPARATOR . 'source_cfg' . DIRECTORY_SEPARATOR . 'conf.php',
+            $options
+        );
+
+        $options['model_options']['dir'] = $this->_path . DIRECTORY_SEPARATOR . 'destination';
+        $this->_destination              = new Filesystem($options['model_options']);
+        Helper::createIniFile(
+            $this->_path . DIRECTORY_SEPARATOR . 'destination_cfg' . DIRECTORY_SEPARATOR . 'conf.php',
+            $options
+        );
+    }
+
+    public function tearDown(): void
+    {
+        Helper::rmDir($this->_path);
+    }
+
+    public function testForceReplacesPasteAndDiscussion()
+    {
+        $sourcePaste       = Helper::getPaste();
+        $sourcePaste['ct'] = 'source paste';
+        $this->_source->create(Helper::getPasteId(), $sourcePaste);
+        $sourceComment       = Helper::getComment();
+        $sourceComment['ct'] = 'source comment';
+        $this->_source->createComment(
+            Helper::getPasteId(),
+            Helper::getPasteId(),
+            Helper::getCommentId(),
+            $sourceComment
+        );
+
+        $destinationPaste       = Helper::getPaste();
+        $destinationPaste['ct'] = 'old paste';
+        $this->_destination->create(Helper::getPasteId(), $destinationPaste);
+        $destinationComment       = Helper::getComment();
+        $destinationComment['ct'] = 'old comment';
+        $this->_destination->createComment(
+            Helper::getPasteId(),
+            Helper::getPasteId(),
+            Helper::getCommentId(),
+            $destinationComment
+        );
+
+        $command = escapeshellarg(PHP_BINARY) . ' ' .
+            escapeshellarg(realpath(PATH . 'bin' . DIRECTORY_SEPARATOR . 'migrate')) .
+            ' -f --delete-after ' .
+            escapeshellarg($this->_path . DIRECTORY_SEPARATOR . 'source_cfg') . ' ' .
+            escapeshellarg($this->_path . DIRECTORY_SEPARATOR . 'destination_cfg') .
+            ' 2>&1';
+        exec($command, $output, $exitCode);
+
+        $this->assertSame(0, $exitCode, implode(PHP_EOL, $output));
+        $this->assertFalse($this->_source->exists(Helper::getPasteId()));
+        $this->assertSame(
+            'source paste',
+            $this->_destination->read(Helper::getPasteId())['ct']
+        );
+        $comments = array_values($this->_destination->readComments(Helper::getPasteId()));
+        $this->assertCount(1, $comments);
+        $this->assertSame('source comment', $comments[0]['ct']);
+    }
+}

tst/MigrateWriteFailureTest.php

@@ -0,0 +1,115 @@
+<?php declare(strict_types=1);
+use PHPUnit\Framework\TestCase;
+use PrivateBin\Data\Filesystem;
+
+class MigrateWriteFailureTest extends TestCase
+{
+    private $_path;
+
+    private $_destinationPath;
+
+    private $_source;
+
+    public function setUp(): void
+    {
+        $this->_path = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'privatebin_migrate_write_failure';
+        Helper::rmDir($this->_path);
+        mkdir($this->_path);
+        mkdir($this->_path . DIRECTORY_SEPARATOR . 'source_cfg');
+        mkdir($this->_path . DIRECTORY_SEPARATOR . 'destination_cfg');
+
+        $options                         = parse_ini_file(CONF_SAMPLE, true);
+        $options['model_options']['dir'] = $this->_path . DIRECTORY_SEPARATOR . 'source';
+        $this->_source                   = new Filesystem($options['model_options']);
+        Helper::createIniFile(
+            $this->_path . DIRECTORY_SEPARATOR . 'source_cfg' . DIRECTORY_SEPARATOR . 'conf.php',
+            $options
+        );
+
+        $this->_destinationPath = $this->_path . DIRECTORY_SEPARATOR . 'blocked';
+        file_put_contents($this->_destinationPath, 'not a directory');
+        $options['model_options']['dir'] = $this->_destinationPath;
+        Helper::createIniFile(
+            $this->_path . DIRECTORY_SEPARATOR . 'destination_cfg' . DIRECTORY_SEPARATOR . 'conf.php',
+            $options
+        );
+    }
+
+    public function tearDown(): void
+    {
+        Helper::rmDir($this->_path);
+    }
+
+    public function testSourceIsPreservedWhenDestinationWriteFails()
+    {
+        $paste = Helper::getPaste();
+        $this->_source->create(Helper::getPasteId(), $paste);
+
+        [$exitCode, $output] = $this->runMigration();
+
+        $this->assertSame(1, $exitCode, $output);
+        $this->assertStringContainsString(
+            'ERROR: Unable to save document ID ' . Helper::getPasteId(),
+            $output
+        );
+        $this->assertTrue($this->_source->exists(Helper::getPasteId()));
+    }
+
+    public function testSourceIsPreservedWhenDestinationCommentWriteFails()
+    {
+        unlink($this->_destinationPath);
+        mkdir(
+            $this->_destinationPath . DIRECTORY_SEPARATOR .
+            substr(Helper::getPasteId(), 0, 2) . DIRECTORY_SEPARATOR .
+            substr(Helper::getPasteId(), 2, 2),
+            0777,
+            true
+        );
+        file_put_contents(
+            $this->_destinationPath . DIRECTORY_SEPARATOR .
+            substr(Helper::getPasteId(), 0, 2) . DIRECTORY_SEPARATOR .
+            substr(Helper::getPasteId(), 2, 2) . DIRECTORY_SEPARATOR .
+            Helper::getPasteId() . '.discussion',
+            'not a directory'
+        );
+
+        $paste = Helper::getPaste();
+        $this->_source->create(Helper::getPasteId(), $paste);
+        $comment = Helper::getComment();
+        $this->_source->createComment(
+            Helper::getPasteId(),
+            Helper::getPasteId(),
+            Helper::getCommentId(),
+            $comment
+        );
+
+        [$exitCode, $output] = $this->runMigration();
+
+        $this->assertSame(1, $exitCode, $output);
+        $this->assertStringContainsString(
+            'ERROR: Unable to save document ID ' . Helper::getPasteId() .
+            ', parent id ' . Helper::getPasteId() .
+            ', comment id ' . Helper::getCommentId(),
+            $output
+        );
+        $this->assertTrue($this->_source->exists(Helper::getPasteId()));
+        $this->assertTrue($this->_source->existsComment(
+            Helper::getPasteId(),
+            Helper::getPasteId(),
+            Helper::getCommentId()
+        ));
+    }
+
+    private function runMigration()
+    {
+        $command = escapeshellarg(PHP_BINARY) . ' ' .
+            escapeshellarg(realpath(PATH . 'bin' . DIRECTORY_SEPARATOR . 'migrate')) .
+            ' --delete-after ' .
+            escapeshellarg($this->_path . DIRECTORY_SEPARATOR . 'source_cfg') . ' ' .
+            escapeshellarg($this->_path . DIRECTORY_SEPARATOR . 'destination_cfg') .
+            ' 2>&1';
+        exec($command, $output, $exitCode);
+
+        return [$exitCode, implode(PHP_EOL, $output)];
+    }
+}