Initialization of Db from INIT-currentschema script (Fixes #395)

CHANGELOG.md

@@ -11,6 +11,8 @@
 - When an invalid value is submitted in one of the settings forms, RCMCardDAV will now abort with an error. Previous
   versions of RCMCardDAV would ignore the value when possible an instead keep the existing value or use a default value.
 - Backend will refuse Frontend request to remove user account created from preset
+- On initial installation of RCMCardDAV, the database schema is created from a single script with the current schema
+  version. Previous versions incrementally executed all migration scripts since the very beginning (Fixes #395)
 
 ## Version 5.0.0-beta1 (to 4.4.4)
 

Makefile

@@ -5,19 +5,23 @@ ROUNDCUBEDIR=roundcubemail
 # For each DBTYPE listed here, the following macros need to be defined:
 # - TESTDB_$(DBTYPE): Name of the test database
 # - MIGTESTDB_$(DBTYPE): Name of the database for the schema migrations test
+# - INITTESTDB_$(DBTYPE): Name of the database for the schema initialization test
 # - EXECDBSCRIPT_$(DBTYPE): Command to execute an SQL script
 # - CREATEDB_$(DBTYPE): Command to create a database
 # - DUMPTBL_$(DBTYPE): Command to dump the schema of the rcmcarddav tables
 DBTYPES=postgres sqlite3 mysql
 
 TESTDB_sqlite3=testreports/test.db
 MIGTESTDB_sqlite3=testreports/migtest.db
+INITTESTDB_sqlite3=testreports/inittest.db
 
 TESTDB_mysql=rcmcarddavtest
 MIGTESTDB_mysql=rcmcarddavmigtest
+INITTESTDB_mysql=rcmcarddavinittest
 
 TESTDB_postgres=rcmcarddavtest
 MIGTESTDB_postgres=rcmcarddavmigtest
+INITTESTDB_postgres=rcmcarddavinittest
 
 # A list of the DB tables of the rcmcarddav plugin
 CD_TABLES=$(foreach tbl,accounts addressbooks contacts groups group_user xsubtypes migrations,carddav_$(tbl))
@@ -105,6 +109,9 @@ $(call EXECDBSCRIPT_postgres,$(TESTDB_postgres),$(ROUNDCUBEDIR)/SQL/postgres.ini
 $(PG_DROPDB) --if-exists $(MIGTESTDB_postgres)
 $(PG_CREATEDB) -E UNICODE $(MIGTESTDB_postgres)
 $(call EXECDBSCRIPT_postgres,$(MIGTESTDB_postgres),$(ROUNDCUBEDIR)/SQL/postgres.initial.sql)
+$(PG_DROPDB) --if-exists $(INITTESTDB_postgres)
+$(PG_CREATEDB) -E UNICODE $(INITTESTDB_postgres)
+$(call EXECDBSCRIPT_postgres,$(INITTESTDB_postgres),$(ROUNDCUBEDIR)/SQL/postgres.initial.sql)
 endef
 define CREATEDB_mysql
 $(MYSQL) --show-warnings -e 'DROP DATABASE IF EXISTS $(TESTDB_mysql);'
@@ -113,13 +120,16 @@ $(call EXECDBSCRIPT_mysql,$(TESTDB_mysql),$(ROUNDCUBEDIR)/SQL/mysql.initial.sql)
 $(MYSQL) --show-warnings -e 'DROP DATABASE IF EXISTS $(MIGTESTDB_mysql);'
 $(MYSQL) --show-warnings -e 'CREATE DATABASE $(MIGTESTDB_mysql) /*!40101 CHARACTER SET utf8 COLLATE utf8_general_ci */;'
 $(call EXECDBSCRIPT_mysql,$(MIGTESTDB_mysql),$(ROUNDCUBEDIR)/SQL/mysql.initial.sql)
+$(MYSQL) --show-warnings -e 'DROP DATABASE IF EXISTS $(INITTESTDB_mysql);'
+$(MYSQL) --show-warnings -e 'CREATE DATABASE $(INITTESTDB_mysql) /*!40101 CHARACTER SET utf8 COLLATE utf8_general_ci */;'
+$(call EXECDBSCRIPT_mysql,$(INITTESTDB_mysql),$(ROUNDCUBEDIR)/SQL/mysql.initial.sql)
 endef
 define CREATEDB_sqlite3
-mkdir -p $(dir $(TESTDB_sqlite3))
-mkdir -p $(dir $(MIGTESTDB_sqlite3))
-rm -f $(TESTDB_sqlite3) $(MIGTESTDB_sqlite3)
+mkdir -p $(dir $(TESTDB_sqlite3)) $(dir $(MIGTESTDB_sqlite3)) $(dir $(INITTESTDB_sqlite3))
+rm -f $(TESTDB_sqlite3) $(MIGTESTDB_sqlite3) $(INITTESTDB_sqlite3)
 $(call EXECDBSCRIPT_sqlite3,$(TESTDB_sqlite3),$(ROUNDCUBEDIR)/SQL/sqlite.initial.sql)
 $(call EXECDBSCRIPT_sqlite3,$(MIGTESTDB_sqlite3),$(ROUNDCUBEDIR)/SQL/sqlite.initial.sql)
+$(call EXECDBSCRIPT_sqlite3,$(INITTESTDB_sqlite3),$(ROUNDCUBEDIR)/SQL/sqlite.initial.sql)
 endef
 
 define DUMPTBL_postgres
@@ -153,6 +163,9 @@ tests-$(1): tests/DBInteroperability/phpunit-$(1).xml tests/DBInteroperability/D
 	@echo Performing schema comparison of initial schema to schema resulting from migrations
 	$$(call DUMPTBL_$(1),$(MIGTESTDB_$(1)),testreports/$(1)-mig.sql)
 	diff testreports/$(1)-mig.sql testreports/$(1)-init.sql
+	@echo Performing schema comparison of initial schema to schema resulting from initialization
+	$$(call DUMPTBL_$(1),$(INITTESTDB_$(1)),testreports/$(1)-inittest.sql)
+	diff testreports/$(1)-inittest.sql testreports/$(1)-init.sql
 endef
 
 $(foreach dbtype,$(DBTYPES),$(eval $(call EXEC_DBTESTS,$(dbtype))))

src/Db/Database.php

@@ -193,53 +193,60 @@ public function checkMigrations(string $dbPrefix, string $scriptDir): void
             }
         }
 
-        // (2) Determine which migration scripts have already been executed. This must handle the initial case that
-        //     the plugin's database tables to not exist yet, in which case they will be initialized.
-        try {
-            $dbh->set_option('ignore_key_errors', true);
+        // (2) Check if the carddav_migrations table is available. If not, no RCMCardDAV tables are present yet and
+        // instead of performing migration we will initialize the RCMCardDAV with the current schema
+        if (in_array($dbh->table_name('carddav_migrations'), $dbh->list_tables())) {
+            // (2.a.2) Determine which migration scripts have already been executed.
             /** @var list<array{filename: string}> $migrations */
             $migrationsDone = $this->get([], ['filename'], 'migrations');
             $migrationsDone = array_column($migrationsDone, 'filename');
-        } catch (DatabaseException $e) {
-            $migrationsDone = [];
-        }
-        $dbh->set_option('ignore_key_errors', null);
-
-        // (3) Execute the migration scripts that have not been executed before
-        foreach ($migrationsAvailable as $migration) {
-            // skip migrations that have already been done
-            if (in_array($migration, $migrationsDone)) {
-                continue;
-            }
-
-            $logger->notice("In migration: $migration");
 
-            $phpMigrationScript = "$scriptDir/$migration/migrate.php";
-            $sqlMigrationScript = "$scriptDir/$migration/$db_backend.sql";
-
-            if (file_exists($phpMigrationScript)) {
-                $migrationClass = "\MStilkerich\RCMCardDAV\DBMigrations\Migration"
-                    . substr($migration, 0, 4); // the 4-digit number
-
-                /**
-                 * @psalm-suppress InvalidStringClass
-                 * @var DBMigrationInterface $migrationObj
-                 */
-                $migrationObj = new $migrationClass();
-                if ($migrationObj->migrate($dbh, $logger) === false) {
-                    return; // error already logged
+            // (2.a.3) Execute the migration scripts that have not been executed before
+            foreach ($migrationsAvailable as $migration) {
+                // skip migrations that have already been done
+                if (in_array($migration, $migrationsDone)) {
+                    continue;
                 }
-            } elseif (file_exists($sqlMigrationScript)) {
-                if ($this->performSqlMigration($sqlMigrationScript, $dbPrefix) === false) {
-                    return; // error already logged
+
+                $logger->notice("In migration: $migration");
+
+                $phpMigrationScript = "$scriptDir/$migration/migrate.php";
+                $sqlMigrationScript = "$scriptDir/$migration/$db_backend.sql";
+
+                if (file_exists($phpMigrationScript)) {
+                    $migrationClass = '\MStilkerich\RCMCardDAV\DBMigrations\Migration'
+                        . substr($migration, 0, 4); // the 4-digit number
+
+                    /**
+                     * @psalm-suppress InvalidStringClass
+                     * @var DBMigrationInterface $migrationObj
+                     */
+                    $migrationObj = new $migrationClass();
+                    if ($migrationObj->migrate($dbh, $logger) === false) {
+                        return; // error already logged
+                    }
+                } else {
+                    if ($this->performSqlMigration($sqlMigrationScript, $dbPrefix) === false) {
+                        return; // error already logged
+                    }
                 }
-            } else {
-                $logger->warning("No migration script found for: $migration");
-                // do not continue with other scripts that may depend on this one
-                return;
-            }
 
-            $this->insert('migrations', ['filename'], [ [$migration] ]);
+                $this->insert('migrations', ['filename'], [ [$migration] ]);
+            }
+        } else {
+            // (2.b) Initial creation of RCMCardDAV tables
+            $logger->notice("Performing initial creation of RCMCardDAV tables");
+
+            $sqlMigrationScript = "$scriptDir/INIT-currentschema/$db_backend.sql";
+            if ($this->performSqlMigration($sqlMigrationScript, $dbPrefix) === true) {
+                $allMigRows = array_map(
+                    function (string $mig): array {
+                        return [$mig];
+                    },
+                    $migrationsAvailable
+                );
+                $this->insert('migrations', ['filename'], $allMigRows);
+            }
         }
     }
 
@@ -253,9 +260,8 @@ private function performSqlMigration(string $migrationScript, string $dbPrefix):
     {
         $dbh = $this->dbHandle;
         $logger = $this->logger;
-        $queries_raw = file_get_contents($migrationScript);
 
-        if ($queries_raw === false) {
+        if (file_exists($migrationScript) !== true || ($queries_raw = file_get_contents($migrationScript)) === false) {
             $logger->error("Failed to read migration script: $migrationScript - aborting");
             return false;
         }

tests/DBInteroperability/DatabaseAccounts.php.dist

@@ -14,14 +14,17 @@ final class DatabaseAccounts
         "sqlite3" => [
             "sqlite:///" . __DIR__ . "/../../testreports/test.db?mode=0640",
             "sqlite:///" . __DIR__ . "/../../testreports/migtest.db?mode=0640",
+            "sqlite:///" . __DIR__ . "/../../testreports/inittest.db?mode=0640",
         ],
         "postgres" => [
             "pgsql://postgres:[email protected]/rcmcarddavtest",
             "pgsql://postgres:[email protected]/rcmcarddavmigtest",
+            "pgsql://postgres:[email protected]/rcmcarddavinittest",
         ],
         "mysql" => [
             "mysql://root:[email protected]/rcmcarddavtest",
             "mysql://root:[email protected]/rcmcarddavmigtest",
+            "mysql://root:[email protected]/rcmcarddavinittest",
         ],
     ];
 }

tests/DBInteroperability/DatabaseInitializationTest.php

@@ -0,0 +1,110 @@
+<?php
+
+/*
+ * RCMCardDAV - CardDAV plugin for Roundcube webmail
+ *
+ * Copyright (C) 2011-2022 Benjamin Schieder <[email protected]>,
+ *                         Michael Stilkerich <[email protected]>
+ *
+ * This file is part of RCMCardDAV.
+ *
+ * RCMCardDAV is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * RCMCardDAV is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with RCMCardDAV. If not, see <https://www.gnu.org/licenses/>.
+ */
+
+declare(strict_types=1);
+
+namespace MStilkerich\Tests\RCMCardDAV\DBInteroperability;
+
+use MStilkerich\Tests\RCMCardDAV\TestInfrastructure;
+use PHPUnit\Framework\TestCase;
+use MStilkerich\RCMCardDAV\Db\AbstractDatabase;
+
+final class DatabaseInitializationTest extends TestCase
+{
+    private const SCRIPTDIR = __DIR__ . "/../../dbmigrations";
+
+    /** @var AbstractDatabase Database used for the schema initialization test */
+    private static $db;
+
+    public static function setUpBeforeClass(): void
+    {
+        self::resetRcubeDb();
+    }
+
+    public function setUp(): void
+    {
+    }
+
+    public function tearDown(): void
+    {
+        TestInfrastructure::logger()->reset();
+
+        // We need to create a new rcube_db instance after each test (technically, each test that creates or drops
+        // tables) so that the rcube_db::list_tables() cache is cleared. This is particularly important after the first
+        // migration which creates the carddav tables.
+        self::resetRcubeDb();
+    }
+
+    private static function resetRcubeDb(): void
+    {
+        $dbsettings = TestInfrastructureDB::dbSettings();
+        $initdb_dsnw = $dbsettings[2];
+        self::$db = TestInfrastructureDB::initDatabase($initdb_dsnw);
+        TestInfrastructure::init(self::$db);
+    }
+
+    /**
+     * Tests the schema initialization.
+     *
+     * This test is invoked once on an empty database (init database, only used for this test). The makefiles dump the
+     * schema and compare it to a schema created from script-based execution of the init script.
+     */
+    public function testSchemaInitializationWorks(): void
+    {
+        $db = self::$db;
+
+        // check preconditions
+        $this->assertDirectoryExists(self::SCRIPTDIR . "/INIT-currentschema");
+
+        // Perform the initialization
+        $db->checkMigrations("", self::SCRIPTDIR);
+
+        // After the initialization, the migrations table must contain all the available migrations
+        $migsavail = array_map(
+            function (string $s): string {
+                return basename($s);
+            },
+            glob(self::SCRIPTDIR . "/0???-*")
+        );
+        $migsDone = $this->getDoneMigrations();
+        $this->assertSame($migsavail, $migsDone);
+    }
+
+    /**
+     * Gets a sorted list of migrations that have been recorded as done in the carddav_migrations table.
+     *
+     * @return list<string>
+     */
+    private function getDoneMigrations(): array
+    {
+        $db = self::$db;
+        /** @var list<array{filename: string}> $rows */
+        $rows = $db->get([], ['filename'], 'migrations');
+        $migsDone = array_column($rows, 'filename');
+        sort($migsDone);
+        return $migsDone;
+    }
+}
+
+// vim: ts=4:sw=4:expandtab:fenc=utf8:ff=unix:tw=120