Fixing columns order in Oracle (and many other small issues)

Author: moufmouf

composer.json

@@ -35,7 +35,7 @@
         "doctrine/annotations": "^1.10",
         "zendframework/zend-code": "^3.4",
         "psr/container": "^1",
-        "brain-diminished/schema-version-control": "^1.0.2",
+        "brain-diminished/schema-version-control": "dev-master",
         "ext-PDO": "*",
         "ext-json": "*",
         "ext-hash": "*",

phpunit.oracle.xml

@@ -7,7 +7,7 @@
          convertNoticesToExceptions="true"
          convertWarningsToExceptions="true"
          processIsolation="false"
-         stopOnFailure="true"
+         stopOnFailure="false"
          bootstrap="vendor/autoload.php"
 >
     <testsuites>

src/SchemaLockFileDumper.php

@@ -10,6 +10,7 @@
 use Doctrine\DBAL\Schema\Schema;
 use Doctrine\DBAL\Schema\Table;
 use Mouf\Database\SchemaAnalyzer\SchemaAnalyzer;
+use TheCodingMachine\TDBM\Utils\ColumnsReorderer;
 use TheCodingMachine\TDBM\Utils\ImmutableCaster;
 use function array_map;
 use function array_values;
@@ -92,6 +93,7 @@ public function getSchema(bool $ignoreCache = false): Schema
             } else {
                 $this->schema = $this->schemaVersionControlService->loadSchemaFile();
                 ImmutableCaster::castSchemaToImmutable($this->schema);
+                ColumnsReorderer::reorderTableColumns($this->schema);
                 $this->cache->save($cacheKey, $this->schema);
             }
         }

src/Utils/ColumnsReorderer.php

@@ -0,0 +1,80 @@
+<?php
+
+
+namespace TheCodingMachine\TDBM\Utils;
+
+use Doctrine\DBAL\Schema\Column;
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\DBAL\Schema\Table;
+use ReflectionClass;
+use function array_merge;
+use function in_array;
+
+/**
+ * The sole purpose of this class is to work around a DBAL bug related to Oracle that
+ * does not return the columns in the same order as the other databases.
+ * In order to have consistent results (and consistent tests), we need those columns in
+ * the same order in all databases.
+ */
+class ColumnsReorderer
+{
+    public static function reorderTableColumns(Schema $schema): void
+    {
+        foreach ($schema->getTables() as $table) {
+            self::reorderColumnsForTable($table);
+        }
+    }
+
+    private static function reorderColumnsForTable(Table $table)
+    {
+        $columns = self::getColumnsInExpectedOrder($table);
+
+        // Note: the only way to rewrite columns order is to tap into a PROTECTED method.
+        // This is bad BUT!
+        // - we only do this for Oracle databases
+        // - we know Doctrine consider protected methods as part of the API for BC changes so risk is limited.
+        $refClass = new ReflectionClass(Table::class);
+        $addColumnMethod = $refClass->getMethod('_addColumn');
+        $addColumnMethod->setAccessible(true);
+
+        foreach ($columns as $column) {
+            $table->dropColumn($column->getName());
+            $addColumnMethod->invoke($table, $column);
+            //$table->_addColumn($column);
+        }
+    }
+
+    /**
+     * @param Table $table
+     * @return Column[]
+     */
+    private static function getColumnsInExpectedOrder(Table $table): array {
+        if ($table->hasPrimaryKey()) {
+            $pkColumns = $table->getPrimaryKey()->getUnquotedColumns();
+        } else {
+            $pkColumns = [];
+        }
+        $fks = $table->getForeignKeys();
+
+        $fkColumns = [];
+
+        foreach ($fks as $fk) {
+            $fkColumns = array_merge($fkColumns, $fk->getUnquotedLocalColumns());
+        }
+
+        $first = [];
+        $second = [];
+        $last = [];
+
+        foreach ($table->getColumns() as $column) {
+            if (in_array($column->getName(), $pkColumns, true)) {
+                $first[] = $column;
+            } elseif (in_array($column->getName(), $fkColumns, true)) {
+                $second[] = $column;
+            } else {
+                $last[] = $column;
+            }
+        }
+        return array_merge($first, $second, $last);
+    }
+}

tests/Dao/TestCountryDao.php

@@ -21,7 +21,7 @@ public function getCountriesByUserCount()
 SELECT country.*
 FROM country
 LEFT JOIN users ON users.country_id = country.id
-GROUP BY country.id
+GROUP BY country.id, country.label
 ORDER BY COUNT(users.id) DESC
 SQL;
 

tests/SchemaLockFileDumperTest.php

@@ -24,6 +24,7 @@
 use Doctrine\Common\Cache\ArrayCache;
 use Mouf\Database\SchemaAnalyzer\SchemaAnalyzer;
 use TheCodingMachine\TDBM\Utils\ImmutableCaster;
+use function sort;
 
 class SchemaLockFileDumperTest extends TDBMAbstractServiceTest
 {
@@ -45,16 +46,21 @@ public function testSchemaLock(): void
         //lock file doesn't save the database name so we have to replace it manually.
         ImmutableCaster::castSchemaToImmutable($schemaFromConnec);
         foreach ($schemaFromConnec->getTableNames() as $tableName) {
-            $tableNames[] = str_replace(['tdbm_testcase', 'postgres'], 'public', $tableName);
+            $tableNames[] = str_replace(['tdbm_testcase', 'postgres', 'tdbm'], 'public', $tableName);
         }
 
         $cache = new ArrayCache();
         $schemaLockFileDumper = new SchemaLockFileDumper(self::getConnection(), $cache, Configuration::getDefaultLockFilePath());
 
         $schemaFromAnalyser = $schemaLockFileDumper->getSchema(true);
         $schemaFromAnalyserCached = $schemaLockFileDumper->getSchema();
-        $this->assertEquals($tableNames, $schemaFromAnalyser->getTableNames());
-        $this->assertEquals($schemaFromAnalyser->getTableNames(), $schemaFromAnalyserCached->getTableNames());
+        sort($tableNames);
+        $tablesFromAnalyser = $schemaFromAnalyser->getTableNames();
+        sort($tablesFromAnalyser);
+        $tablesFromAnalyserCached = $schemaFromAnalyserCached->getTableNames();
+        sort($tablesFromAnalyserCached);
+        $this->assertEquals($tableNames, $tablesFromAnalyser);
+        $this->assertEquals($tablesFromAnalyser, $tablesFromAnalyserCached);
     }
 
     public function testGetSchema(): void