Merge pull request #14931 from craftcms/bugfix/db-backup-restore

DB backup/restore refinements
craftcms · May 4, 2024 · 208a6b2 · 208a6b2
2 parents 94b6fcd + e05fc36
commit 208a6b2
Show file tree

Hide file tree

Showing 5 changed files with 136 additions and 27 deletions.
diff --git a/CHANGELOG-WIP.md b/CHANGELOG-WIP.md
@@ -1 +1,11 @@
 # Release notes for Craft CMS 4.10 (WIP)
+
+### Administration
+- Added the `--format` option to the `db/backup` and `db/restore` commands for PostgreSQL installs. ([#14931](https://github.com/craftcms/cms/pull/14931))
+- The `db/restore` command now autodetects the backup format for PostgreSQL installs, if `--format` isn’t passed. ([#14931](https://github.com/craftcms/cms/pull/14931))
+
+### Extensibility
+- Added `craft\db\getBackupFormat()`.
+- Added `craft\db\getRestoreFormat()`.
+- Added `craft\db\setBackupFormat()`.
+- Added `craft\db\setRestoreFormat()`.
diff --git a/src/config/GeneralConfig.php b/src/config/GeneralConfig.php
@@ -451,7 +451,7 @@ class GeneralConfig extends BaseConfig
     public string|null|false|Closure $backupCommand = null;
 
     /**
-     * @var string|null The output format to pass to `pg_dump` when backing up the database.
+     * @var string|null The output format that database backups should use (PostgreSQL only).
      *
      * This setting has no effect with MySQL databases.
      *
@@ -3604,7 +3604,7 @@ public function backupCommand(string|null|false|Closure $value): self
     }
 
     /**
-     * The output format to pass to `pg_dump` when backing up the database.
+     * The output format that database backups should use (PostgreSQL only).
      *
      * This setting has no effect with MySQL databases.
      *

diff --git a/src/console/controllers/DbController.php b/src/console/controllers/DbController.php
@@ -34,6 +34,15 @@ class DbController extends Controller
      */
     public bool $zip = false;
 
+    /**
+     * @var string|null The output format that should be used (`custom`, `directory`, `tar`, or `plain`).
+     *
+     * The `backupCommandFormat` config setting will be used by default.
+     *
+     * @since 4.10.0
+     */
+    public ?string $format = null;
+
     /**
      * @var bool Whether to overwrite an existing backup file, if a specific file path is given.
      */
@@ -56,9 +65,17 @@ public function options($actionID): array
             case 'backup':
                 $options[] = 'zip';
                 $options[] = 'overwrite';
+
+                if (Craft::$app->getDb()->getIsPgsql()) {
+                    $options[] = 'format';
+                }
                 break;
             case 'restore':
                 $options[] = 'dropAllTables';
+
+                if (Craft::$app->getDb()->getIsPgsql()) {
+                    $options[] = 'format';
+                }
                 break;
         }
 
@@ -172,6 +189,10 @@ public function actionBackup(?string $path = null): int
         $this->stdout('Backing up the database ... ');
         $db = Craft::$app->getDb();
 
+        if (isset($this->format) && $db->getIsPgsql()) {
+            $db->getSchema()->setBackupFormat($this->format);
+        }
+
         if ($path !== null) {
             // Prefix with the working directory if a relative path or no path is given
             if (str_starts_with($path, '.') || !str_contains(FileHelper::normalizePath($path, '/'), '/')) {
@@ -224,8 +245,14 @@ public function actionBackup(?string $path = null): int
         }
 
         $this->stdout('done' . PHP_EOL, Console::FG_GREEN);
-        $size = Craft::$app->getFormatter()->asShortSize(filesize($path));
-        $this->stdout("Backup file: $path ($size)" . PHP_EOL);
+
+        if (is_dir($path)) {
+            $this->stdout("Backup directory: $path" . PHP_EOL);
+        } else {
+            $size = Craft::$app->getFormatter()->asShortSize(filesize($path));
+            $this->stdout("Backup file: $path ($size)" . PHP_EOL);
+        }
+
         return ExitCode::OK;
     }
 
@@ -243,12 +270,7 @@ public function actionBackup(?string $path = null): int
     public function actionRestore(?string $path = null): int
     {
         if (!is_readable($path)) {
-            if (!is_dir($path) && Craft::$app->getConfig()->getGeneral()->backupCommandFormat === 'directory') {
-                $this->stderr("Backup directory doesn't exist: $path" . PHP_EOL);
-            } else {
-                $this->stderr("Backup file doesn't exist: $path" . PHP_EOL);
-            }
-
+            $this->stderr("Backup path doesn't exist: $path" . PHP_EOL);
             return ExitCode::UNSPECIFIED_ERROR;
         }
 
@@ -296,7 +318,17 @@ public function actionRestore(?string $path = null): int
         $this->stdout('Restoring database backup ... ');
 
         try {
-            Craft::$app->getDb()->restore($path);
+            $db = Craft::$app->getDb();
+            if ($db->getIsPgsql()) {
+                $restoreFormat = $this->format ?? match (FileHelper::getMimeType($path)) {
+                    'application/octet-stream' => 'custom',
+                    'application/x-tar' => 'tar',
+                    'directory' => 'directory',
+                    default => null,
+                };
+                $db->getSchema()->setRestoreFormat($restoreFormat);
+            }
+            $db->restore($path);
         } catch (Throwable $e) {
             Craft::$app->getErrorHandler()->logException($e);
             $this->stderr('error: ' . $e->getMessage() . PHP_EOL, Console::FG_RED);

diff --git a/src/db/Connection.php b/src/db/Connection.php
@@ -450,6 +450,19 @@ public function onAfterTransaction(callable $callback): void
         }
     }
 
+    private function _getDumpExtension(): string
+    {
+        $backupFormat = $this->getIsPgsql()
+            ? $this->getSchema()->getBackupFormat()
+            : null;
+
+        return match ($backupFormat) {
+            'custom', 'directory' => '.dump',
+            'tar' => '.tar',
+            default => '.sql',
+        };
+    }
+
     /**
      * @inheritdoc
      */
@@ -467,11 +480,6 @@ public function trigger($name, Event $event = null)
         parent::trigger($name, $event);
     }
 
-    private function _getDumpExtension(): string
-    {
-        return $this->getIsPgsql() && $this->getSchema()->usePgRestore() ? '.dump' : '.sql';
-    }
-
     /**
      * Generates a FK, index, or PK name.
      *

diff --git a/src/db/pgsql/Schema.php b/src/db/pgsql/Schema.php
@@ -23,6 +23,17 @@
  */
 class Schema extends \yii\db\pgsql\Schema
 {
+    /**
+     * @see getBackupFormat()
+     * @see setBackupFormat()
+     */
+    private ?string $backupFormat = null;
+    /**
+     * @see getRestoreFormat()
+     * @see setRestoreFormat()
+     */
+    private ?string $restoreFormat = null;
+
     /**
      * @var int The maximum length that objects' names can be.
      */
@@ -131,7 +142,7 @@ public function getDefaultBackupCommand(?array $ignoreTables = null): string
             ->addArg('--schema=', '{schema}');
 
         $ignoreTables = $ignoreTables ?? Craft::$app->getDb()->getIgnoredBackupTables();
-        $format = Craft::$app->getConfig()->getGeneral()->backupCommandFormat;
+        $format = $this->getBackupFormat();
         $commandFromConfig = Craft::$app->getConfig()->getGeneral()->backupCommand;
 
         foreach ($ignoreTables as $table) {
@@ -164,14 +175,21 @@ public function getDefaultRestoreCommand(): string
             ->addArg('--username=', '{user}')
             ->addArg('--no-password');
 
-        $commandFromConfig = Craft::$app->getConfig()->getGeneral()->restoreCommand;
-
-
-        // If we're using pg_restore, we can't use STDIN, as it may be a directory
         if ($this->usePgRestore()) {
-            $command->addArg('{file}');
+            $command
+                ->addArg('--clean')
+                ->addArg('--if-exists')
+                ->addArg('--no-owner')
+                ->addArg('--no-acl')
+                ->addArg('--schema=', '{schema}')
+                ->addArg('--single-transaction')
+
+                // If we're using pg_restore, we can't use STDIN, as it may be a directory
+                ->addArg('{file}');
         }
 
+        $commandFromConfig = Craft::$app->getConfig()->getGeneral()->restoreCommand;
+
         if ($commandFromConfig instanceof \Closure) {
             $command = $commandFromConfig($command);
         }
@@ -240,17 +258,14 @@ public function loadTableSchema($name): ?TableSchema
     }
 
     /**
-     * Whether `pg_restore` should be used by default for the backup command.
+     * Whether `pg_restore` should be used for the restore command.
      *
      * @return bool
      * @since 4.9.0
      */
     public function usePgRestore(): bool
     {
-        return in_array(Craft::$app->getConfig()->getGeneral()->backupCommandFormat, [
-            'custom',
-            'directory',
-        ], true);
+        return isset($this->restoreFormat) && $this->restoreFormat !== 'plain';
     }
 
     /**
@@ -359,4 +374,48 @@ private function _pgpasswordCommand(): string
     {
         return Platform::isWindows() ? 'set PGPASSWORD="{password}" && ' : 'PGPASSWORD="{password}" ';
     }
+
+    /**
+     * Returns the backup format that should be used (`custom`, `directory`, `tar`, or `plain`).
+     *
+     * @return string|null
+     * @since 4.10.0
+     */
+    public function getBackupFormat(): ?string
+    {
+        return $this->backupFormat ?? Craft::$app->getConfig()->getGeneral()->backupCommandFormat;
+    }
+
+    /**
+     * Sets the backup format that should be used (`custom`, `directory`, `tar`, or `plain`).
+     *
+     * @param string|null $backupFormat
+     * @since 4.10.0
+     */
+    public function setBackupFormat(?string $backupFormat): void
+    {
+        $this->backupFormat = $backupFormat;
+    }
+
+    /**
+     * Returns the restore format that should be used (`custom`, `directory`, `tar`, or `plain`).
+     *
+     * @return string|null
+     * @since 4.10.0
+     */
+    public function getRestoreFormat(): ?string
+    {
+        return $this->restoreFormat;
+    }
+
+    /**
+     * Sets the restore format that should be used (`custom`, `directory`, `tar`, or `plain`).
+     *
+     * @param string|null $restoreFormat
+     * @since 4.10.0
+     */
+    public function setRestoreFormat(?string $restoreFormat): void
+    {
+        $this->restoreFormat = $restoreFormat;
+    }
 }