BUG Fix issues preventing a site from being migrated from SS3 to SS4 (#104)

* BUG Enum value change wasn't being detected by alterTableAlterColumn because backslashes were not accounting

* BUG Update renameTable to also rename constraints

* BUG Add unit tests to cover requireTable and renameTable

* Fix liniting errors

* MINOR Update build to use xenial and add extra PHP version

.travis.yml

@@ -1,6 +1,10 @@
 language: php
 
-dist: trusty
+dist: xenial
+
+services:
+  - postgresql
+  -
 
 cache:
   directories:
@@ -19,6 +23,12 @@ matrix:
     - php: 7.1
       env:
         - PHPUNIT_TEST=framework
+    - php: 7.2
+      env:
+        - PHPUNIT_TEST=framework
+    - php: 7.3
+      env:
+        - PHPUNIT_TEST=framework
     - php: 7.1
       env:
         - PHPUNIT_TEST=postgresql

code/PostgreSQLSchemaManager.php

@@ -494,9 +494,15 @@ class PostgreSQLSchemaManager extends DBSchemaManager
         // First, we split the column specifications into parts
         // TODO: this returns an empty array for the following string: int(11) not null auto_increment
         //         on second thoughts, why is an auto_increment field being passed through?
-
+        $pattern = '/^([\w(\,)]+)\s?((?:not\s)?null)?\s?(default\s[\w\.\'\\\\]+)?\s?(check\s[\w()\'",\s\\\\]+)?$/i';
-        $pattern = '/^([\w(\,)]+)\s?((?:not\s)?null)?\s?(default\s[\w\.\']+)?\s?(check\s[\w()\'",\s]+)?$/i';
         preg_match($pattern, $colSpec, $matches);
+        // example value this regex is expected to parse:
+        // varchar(255) not null default 'SS\Test\Player' check ("ClassName" in ('SS\Test\Player', 'Player', null))
+        // split into:
+        // * varchar(255)
+        // * not null
+        // * default 'SS\Test\Player'
+        // * check ("ClassName" in ('SS\Test\Player', 'Player', null))
 
         if (sizeof($matches) == 0) {
             return '';
@@ -537,8 +543,8 @@ class PostgreSQLSchemaManager extends DBSchemaManager
                 if ($this->hasTable("{$tableName}_Live")) {
                     $updateConstraint .= "UPDATE \"{$tableName}_Live\" SET \"$colName\"='$default' WHERE \"$colName\" NOT IN ($constraint_values);";
                 }
-                if ($this->hasTable("{$tableName}_versions")) {
+                if ($this->hasTable("{$tableName}_Versions")) {
-                    $updateConstraint .= "UPDATE \"{$tableName}_versions\" SET \"$colName\"='$default' WHERE \"$colName\" NOT IN ($constraint_values);";
+                    $updateConstraint .= "UPDATE \"{$tableName}_Versions\" SET \"$colName\"='$default' WHERE \"$colName\" NOT IN ($constraint_values);";
                 }
 
                 $this->query($updateConstraint);
@@ -560,8 +566,17 @@ class PostgreSQLSchemaManager extends DBSchemaManager
 
     public function renameTable($oldTableName, $newTableName)
     {
+        $constraints = $this->getConstraintForTable($oldTableName);
         $this->query("ALTER TABLE \"$oldTableName\" RENAME TO \"$newTableName\"");
+
+        if ($constraints) {
+            foreach ($constraints as $old) {
+                $new = preg_replace('/^' . $oldTableName . '/', $newTableName, $old);
+                $this->query("ALTER TABLE \"$newTableName\" RENAME CONSTRAINT \"$old\" TO \"$new\";");
+            }
+        }
         unset(self::$cached_fieldlists[$oldTableName]);
+        unset(self::$cached_constraints[$oldTableName]);
     }
 
     public function checkAndRepairTable($tableName)
@@ -961,6 +976,28 @@ class PostgreSQLSchemaManager extends DBSchemaManager
         return self::$cached_constraints[$constraint];
     }
 
+    /**
+     * Retrieve a list of constraints for the provided table name.
+     * @param string $tableName
+     * @return array
+     */
+    private function getConstraintForTable($tableName)
+    {
+        // Note the PostgreSQL `like` operator is case sensitive
+        $constraints = $this->preparedQuery(
+            "
+            SELECT conname
+            FROM pg_catalog.pg_constraint r
+            INNER JOIN pg_catalog.pg_namespace n
+              ON r.connamespace = n.oid
+            WHERE r.contype = 'c' AND conname like ? AND n.nspname = ?
+            ORDER BY 1;",
+            array($tableName . '_%', $this->database->currentSchema())
+        )->column('conname');
+
+        return $constraints;
+    }
+
     /**
      * A function to return the field names and datatypes for the particular table
      *

tests/PostgreSQLSchemaManagerTest.php

@@ -0,0 +1,138 @@
+<?php
+
+namespace SilverStripe\PostgreSQL\Tests;
+
+use SilverStripe\Core\ClassInfo;
+use SilverStripe\Dev\SapphireTest;
+use SilverStripe\ORM\Connect\Database;
+use SilverStripe\ORM\Connect\DatabaseException;
+use SilverStripe\ORM\DB;
+use SilverStripe\PostgreSQL\PostgreSQLConnector;
+use SilverStripe\PostgreSQL\PostgreSQLSchemaManager;
+
+class PostgreSQLSchemaManagerTest extends SapphireTest
+{
+
+    protected $usesTransactions = false;
+
+    public function testAlterTable()
+    {
+        try {
+            /** @var PostgreSQLSchemaManager $dbSchema */
+            $dbSchema = DB::get_schema();
+            $dbSchema->quiet();
+
+            $this->createSS3Table();
+
+            try {
+                DB::query('INSERT INTO "ClassNamesUpgrade" ("ClassName") VALUES (\'App\MySite\FooBar\')');
+                $this->assertFalse(true, 'SS3 Constaint should have blocked the previous insert.');
+            } catch (DatabaseException $ex) {
+            }
+
+            $dbSchema->schemaUpdate(function () use ($dbSchema) {
+                $dbSchema->requireTable(
+                    'ClassNamesUpgrade',
+                    [
+                        'ID' => 'PrimaryKey',
+                        'ClassName' => 'Enum(array("App\\\\MySite\\\\FooBar"))',
+                    ]
+                );
+            });
+
+            DB::query('INSERT INTO "ClassNamesUpgrade" ("ClassName") VALUES (\'App\MySite\FooBar\')');
+            $count = DB::query('SELECT count(*) FROM "ClassNamesUpgrade" WHERE "ClassName" = \'App\MySite\FooBar\'')
+                ->value();
+
+            $this->assertEquals(1, $count);
+        } finally {
+            DB::query('DROP TABLE IF EXISTS "ClassNamesUpgrade"');
+            DB::query('DROP SEQUENCE IF EXISTS "ClassNamesUpgrade_ID_seq"');
+        }
+    }
+
+    private function createSS3Table()
+    {
+        DB::query(<<<SQL
+CREATE SEQUENCE "ClassNamesUpgrade_ID_seq" start 1 increment 1;       
+CREATE TABLE "ClassNamesUpgrade"
+(
+  "ID" bigint NOT NULL DEFAULT nextval('"ClassNamesUpgrade_ID_seq"'::regclass),
+  "ClassName" character varying(255) DEFAULT 'ClassNamesUpgrade'::character varying,
+  CONSTRAINT "ClassNamesUpgrade_pkey" PRIMARY KEY ("ID"),
+  CONSTRAINT "ClassNamesUpgrade_ClassName_check" CHECK ("ClassName"::text = ANY (ARRAY['FooBar'::character varying::text]))
+)
+WITH (
+  OIDS=FALSE
+);
+SQL
+        );
+    }
+
+    public function testRenameTable()
+    {
+        try {
+            /** @var PostgreSQLSchemaManager $dbSchema */
+            $dbSchema = DB::get_schema();
+            $dbSchema->quiet();
+
+            $this->createSS3VersionedTable();
+
+            $this->assertConstraintCount(1, 'ClassNamesUpgrade_versioned_ClassName_check');
+
+            $dbSchema->schemaUpdate(function () use ($dbSchema) {
+                $dbSchema->renameTable(
+                    'ClassNamesUpgrade_versioned',
+                    'ClassNamesUpgrade_Versioned'
+                );
+            });
+
+            $this->assertTableCount(0, 'ClassNamesUpgrade_versioned');
+            $this->assertTableCount(1, 'ClassNamesUpgrade_Versioned');
+            $this->assertConstraintCount(0, 'ClassNamesUpgrade_versioned_ClassName_check');
+            $this->assertConstraintCount(1, 'ClassNamesUpgrade_Versioned_ClassName_check');
+        } finally {
+            DB::query('DROP TABLE IF EXISTS "ClassNamesUpgrade_Versioned"');
+            DB::query('DROP TABLE IF EXISTS "ClassNamesUpgrade_versioned"');
+            DB::query('DROP SEQUENCE IF EXISTS "ClassNamesUpgrade_versioned_ID_seq"');
+        }
+    }
+
+    private function assertConstraintCount($expected, $constraintName)
+    {
+        $count = DB::prepared_query(
+            'SELECT count(*) FROM pg_catalog.pg_constraint WHERE conname like ?',
+            [$constraintName]
+        )->value();
+
+        $this->assertEquals($expected, $count);
+    }
+
+    private function assertTableCount($expected, $tableName)
+    {
+        $count = DB::prepared_query(
+            'SELECT count(*) FROM pg_catalog.pg_tables WHERE "tablename" like ?',
+            [$tableName]
+        )->value();
+
+        $this->assertEquals($expected, $count);
+    }
+
+    private function createSS3VersionedTable()
+    {
+        DB::query(<<<SQL
+CREATE SEQUENCE "ClassNamesUpgrade_versioned_ID_seq" start 1 increment 1;       
+CREATE TABLE "ClassNamesUpgrade_versioned"
+(
+  "ID" bigint NOT NULL DEFAULT nextval('"ClassNamesUpgrade_versioned_ID_seq"'::regclass),
+  "ClassName" character varying(255) DEFAULT 'ClassNamesUpgrade'::character varying,
+  CONSTRAINT "ClassNamesUpgrade_pkey" PRIMARY KEY ("ID"),
+  CONSTRAINT "ClassNamesUpgrade_versioned_ClassName_check" CHECK ("ClassName"::text = ANY (ARRAY['FooBar'::character varying::text]))
+)
+WITH (
+  OIDS=FALSE
+);
+SQL
+        );
+    }
+}