DBAL-91 - Fix auto-generation of asset names (foreign keys and indexes) which...

DBAL-91 - Fix auto-generation of asset names (foreign keys and indexes) which could lead to exceptions due to auto-quoting of PostgreSQL for example. Now hashing the names with crc32+dechex

lib/Doctrine/DBAL/Schema/AbstractAsset.php

@@ -51,19 +51,35 @@ abstract class AbstractAsset
      */
     protected function _setName($name)
     {
-        if (strlen($name)) {
-            // TODO: find more elegant way to solve this issue.
-            if ($name[0] == '`') {
-                $this->_quoted = true;
-                $name = trim($name, '`');
-            } else if ($name[0] == '"') {
-                $this->_quoted = true;
-                $name = trim($name, '"');
-            }
+        if ($this->isQuoted($name)) {
+            $this->_quoted = true;
+            $name = $this->trimQuotes($name);
         }
         $this->_name = $name;
     }
 
+    /**
+     * Check if this identifier is quoted.
+     *
+     * @param  string $identifier
+     * @return bool
+     */
+    protected function isQuoted($identifier)
+    {
+        return (isset($identifier[0]) && ($identifier[0] == '`' || $identifier[0] == '"'));
+    }
+
+    /**
+     * Trim quotes from the identifier.
+     * 
+     * @param  string $identifier
+     * @return string
+     */
+    protected function trimQuotes($identifier)
+    {
+        return trim($identifier, '`"');
+    }
+
     /**
      * Return name of this schema asset.
      * 
@@ -94,13 +110,13 @@ abstract class AbstractAsset
      * very long names.
      *
      * @param  array $columnNames
-     * @param  string $postfix
+     * @param  string $prefix
      * @param  int $maxSize
      * @return string
      */
-    protected function _generateIdentifierName($columnNames, $postfix='', $maxSize=30)
+    protected function _generateIdentifierName($columnNames, $prefix='', $maxSize=30)
     {
-        $columnCount = count($columnNames);
+        /*$columnCount = count($columnNames);
         $postfixLen = strlen($postfix);
         $parts = array_map(function($columnName) use($columnCount, $postfixLen, $maxSize) {
             return substr($columnName, -floor(($maxSize-$postfixLen)/$columnCount - 1));
@@ -116,6 +132,12 @@ abstract class AbstractAsset
             $identifier = "i" . substr($identifier, 0, strlen($identifier)-1);
         }
 
-        return $identifier;
+        return $identifier;*/
+
+
+        $hash = implode("", array_map(function($column) {
+            return dechex(crc32($column));
+        }, $columnNames));
+        return substr(strtoupper($prefix . "_" . $hash), 0, $maxSize);
     }
 }
\ No newline at end of file

lib/Doctrine/DBAL/Schema/PostgreSqlSchemaManager.php

@@ -46,6 +46,8 @@ class PostgreSqlSchemaManager extends AbstractSchemaManager
         }
 
         if (preg_match('/FOREIGN KEY \((.+)\) REFERENCES (.+)\((.+)\)/', $tableForeignKey['condef'], $values)) {
+            // PostgreSQL returns identifiers that are keywords with quotes, we need them later, don't get
+            // the idea to trim them here.
             $localColumns = array_map('trim', explode(",", $values[1]));
             $foreignColumns = array_map('trim', explode(",", $values[3]));
             $foreignTable = $values[2];

lib/Doctrine/DBAL/Schema/Table.php

@@ -496,7 +496,7 @@ class Table extends AbstractAsset
      */
     public function hasColumn($columnName)
     {
-        $columnName = strtolower($columnName);
+        $columnName = $this->trimQuotes(strtolower($columnName));
         return isset($this->_columns[$columnName]);
     }
 
@@ -508,7 +508,7 @@ class Table extends AbstractAsset
      */
     public function getColumn($columnName)
     {
-        $columnName = strtolower($columnName);
+        $columnName = strtolower($this->trimQuotes($columnName));
         if (!$this->hasColumn($columnName)) {
             throw SchemaException::columnDoesNotExist($columnName, $this->_name);
         }

tests/Doctrine/Tests/DBAL/Functional/Schema/PostgreSqlSchemaManagerTest.php

@@ -132,6 +132,29 @@ class PostgreSqlSchemaManagerTest extends SchemaManagerFunctionalTestCase
         $relatedFk = array_pop($relatedFks);
         $this->assertEquals("nested.schemarelated", $relatedFk->getForeignTableName());
     }
+
+    /**
+     * @group DBAL-91
+     * @group DBAL-88
+     */
+    public function testReturnQuotedAssets()
+    {
+        $sql = 'create table dbal91_something ( id integer  CONSTRAINT id_something PRIMARY KEY NOT NULL  ,"table"   integer );';
+        $this->_conn->exec($sql);
+
+        $sql = 'ALTER TABLE dbal91_something ADD CONSTRAINT something_input FOREIGN KEY( "table" ) REFERENCES dbal91_something ON UPDATE CASCADE;';
+        $this->_conn->exec($sql);
+
+        $table = $this->_sm->listTableDetails('dbal91_something');
+
+        $this->assertEquals(
+            array(
+                "CREATE TABLE dbal91_something (id INT NOT NULL, table INT DEFAULT NULL, PRIMARY KEY(id))",
+                "CREATE INDEX IDX_A9401304ECA7352B ON dbal91_something (\"table\")",
+            ),
+            $this->_conn->getDatabasePlatform()->getCreateTableSQL($table)
+        );
+    }
 }
 
 class MoneyType extends Type

tests/Doctrine/Tests/DBAL/Platforms/MsSqlPlatformTest.php

@@ -24,7 +24,7 @@ class MsSqlPlatformTest extends AbstractPlatformTestCase
     {
         return array(
             'CREATE TABLE test (foo NVARCHAR(255) DEFAULT NULL, bar NVARCHAR(255) DEFAULT NULL)',
-            'CREATE UNIQUE INDEX test_foo_bar_uniq ON test (foo, bar) WHERE foo IS NOT NULL AND bar IS NOT NULL'
+            'CREATE UNIQUE INDEX UNIQ_D87F7E0C8C73652176FF8CAA ON test (foo, bar) WHERE foo IS NOT NULL AND bar IS NOT NULL'
         );
     }
 

tests/Doctrine/Tests/DBAL/Platforms/MySqlPlatformTest.php

@@ -31,7 +31,7 @@ class MySqlPlatformTest extends AbstractPlatformTestCase
     public function getGenerateTableWithMultiColumnUniqueIndexSql()
     {
         return array(
-            'CREATE TABLE test (foo VARCHAR(255) DEFAULT NULL, bar VARCHAR(255) DEFAULT NULL, UNIQUE INDEX test_foo_bar_uniq (foo, bar)) ENGINE = InnoDB'
+            'CREATE TABLE test (foo VARCHAR(255) DEFAULT NULL, bar VARCHAR(255) DEFAULT NULL, UNIQUE INDEX UNIQ_D87F7E0C8C73652176FF8CAA (foo, bar)) ENGINE = InnoDB'
         );
     }
 

tests/Doctrine/Tests/DBAL/Platforms/OraclePlatformTest.php

@@ -23,7 +23,7 @@ class OraclePlatformTest extends AbstractPlatformTestCase
     {
         return array(
             'CREATE TABLE test (foo VARCHAR2(255) DEFAULT NULL, bar VARCHAR2(255) DEFAULT NULL)',
-            'CREATE UNIQUE INDEX test_foo_bar_uniq ON test (foo, bar)',
+            'CREATE UNIQUE INDEX UNIQ_D87F7E0C8C73652176FF8CAA ON test (foo, bar)',
         );
     }
 

tests/Doctrine/Tests/DBAL/Platforms/PostgreSqlPlatformTest.php

@@ -23,7 +23,7 @@ class PostgreSqlPlatformTest extends AbstractPlatformTestCase
     {
         return array(
             'CREATE TABLE test (foo VARCHAR(255) DEFAULT NULL, bar VARCHAR(255) DEFAULT NULL)',
-            'CREATE UNIQUE INDEX test_foo_bar_uniq ON test (foo, bar)'
+            'CREATE UNIQUE INDEX UNIQ_D87F7E0C8C73652176FF8CAA ON test (foo, bar)'
         );
     }
 

tests/Doctrine/Tests/DBAL/Platforms/SqlitePlatformTest.php

@@ -23,7 +23,7 @@ class SqlitePlatformTest extends AbstractPlatformTestCase
     {
         return array(
             'CREATE TABLE test (foo VARCHAR(255) DEFAULT NULL, bar VARCHAR(255) DEFAULT NULL)',
-            'CREATE UNIQUE INDEX test_foo_bar_uniq ON test (foo, bar)',
+            'CREATE UNIQUE INDEX UNIQ_D87F7E0C8C73652176FF8CAA ON test (foo, bar)',
         );
     }
 

tests/Doctrine/Tests/DBAL/Schema/SchemaTest.php

@@ -170,14 +170,15 @@ class SchemaTest extends \PHPUnit_Framework_TestCase
     public function testConfigMaxIdentifierLength()
     {
         $schemaConfig = new \Doctrine\DBAL\Schema\SchemaConfig();
-        $schemaConfig->setMaxIdentifierLength(10);
+        $schemaConfig->setMaxIdentifierLength(5);
 
         $schema = new Schema(array(), array(), $schemaConfig);
         $table = $schema->createTable("smalltable");
         $table->addColumn('long_id', 'integer');
         $table->addIndex(array('long_id'));
 
-        $this->assertTrue($table->hasIndex('le_id_idx'));
+        $index = current($table->getIndexes());
+        $this->assertEquals(5, strlen($index->getName()));
     }
 
     public function testDeepClone()

tests/Doctrine/Tests/DBAL/Schema/TableTest.php

@@ -112,8 +112,8 @@ class TableTest extends \PHPUnit_Framework_TestCase
         $columns = array(new Column("foo", $type), new Column("bar", $type), new Column("baz", $type));
         $table = new Table("foo", $columns);
         
-        $table->addIndex(array("foo", "bar"));
-        $table->addUniqueIndex(array("bar", "baz"));
+        $table->addIndex(array("foo", "bar"), "foo_foo_bar_idx");
+        $table->addUniqueIndex(array("bar", "baz"), "foo_bar_baz_uniq");
 
         $this->assertTrue($table->hasIndex("foo_foo_bar_idx"));
         $this->assertTrue($table->hasIndex("foo_bar_baz_uniq"));
@@ -311,11 +311,12 @@ class TableTest extends \PHPUnit_Framework_TestCase
 
         $constraints = $table->getForeignKeys();
         $this->assertEquals(1, count($constraints));
-        $this->assertType('Doctrine\DBAL\Schema\ForeignKeyConstraint', $constraints["foo_id_fk"]);
+        $constraint = current($constraints);
+        
+        $this->assertInstanceOf('Doctrine\DBAL\Schema\ForeignKeyConstraint', $constraint);
 
-        $this->assertEquals("foo_id_fk", $constraints["foo_id_fk"]->getName());
-        $this->assertTrue($constraints["foo_id_fk"]->hasOption("foo"));
-        $this->assertEquals("bar", $constraints["foo_id_fk"]->getOption("foo"));
+        $this->assertTrue($constraint->hasOption("foo"));
+        $this->assertEquals("bar", $constraint->getOption("foo"));
     }
 
     public function testAddIndexWithCaseSensitiveColumnProblem()
@@ -351,7 +352,7 @@ class TableTest extends \PHPUnit_Framework_TestCase
         $table->addColumn('baz', 'integer', array());
         $table->addIndex(array('baz'));
 
-        $this->assertTrue($table->hasIndex('foo_bar_baz_idx'));
+        $this->assertEquals(1, count($table->getIndexes()));
     }
 
     /**
@@ -380,9 +381,12 @@ class TableTest extends \PHPUnit_Framework_TestCase
 
         $table->addForeignKeyConstraint($foreignTable, array("id"), array("id"), array("foo" => "bar"));
 
-        $this->assertEquals(1, count($table->getIndexes()));
-        $this->assertTrue($table->hasIndex("foo_id_idx"));
-        $this->assertEquals(array('id'), $table->getIndex('foo_id_idx')->getColumns());
+        $indexes = $table->getIndexes();
+        $this->assertEquals(1, count($indexes));
+        $index = current($indexes);
+
+        $this->assertTrue($table->hasIndex($index->getName()));
+        $this->assertEquals(array('id'), $index->getColumns());
     }
 
     /**
@@ -393,13 +397,14 @@ class TableTest extends \PHPUnit_Framework_TestCase
         $table = new Table("bar");
         $table->addColumn('baz', 'integer', array());
         $table->addIndex(array('baz'));
-        $this->assertEquals(1, count($table->getIndexes()));
-        $this->assertTrue($table->hasIndex('bar_baz_idx'));
+
+        $indexes = $table->getIndexes();
+        $this->assertEquals(1, count($indexes));
+        $index = current($indexes);
 
         $table->addUniqueIndex(array('baz'));
         $this->assertEquals(1, count($table->getIndexes()));
-        $this->assertFalse($table->hasIndex('bar_baz_idx'));
-        $this->assertTrue($table->hasIndex('bar_baz_uniq'));
+        $this->assertFalse($table->hasIndex($index->getName()));
     }
 
     /**
@@ -416,4 +421,41 @@ class TableTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals('`bar`', $table->getQuotedName($mysqlPlatform));
         $this->assertEquals('"bar"', $table->getQuotedName($sqlitePlatform));
     }
-}
\ No newline at end of file
+
+    /**
+     * @group DBAL-79
+     */
+    public function testTableHasPrimaryKey()
+    {
+        $table = new Table("test");
+
+        $this->assertFalse($table->hasPrimaryKey());
+
+        $table->addColumn("foo", "integer");
+        $table->setPrimaryKey(array("foo"));
+
+        $this->assertTrue($table->hasPrimaryKey());
+    }
+
+    /**
+     * @group DBAL-91
+     */
+    public function testAddIndexWithQuotedColumns()
+    {
+        $table = new Table("test");
+        $table->addColumn('"foo"', 'integer');
+        $table->addColumn('bar', 'integer');
+        $table->addIndex(array('"foo"', '"bar"'));
+    }
+
+    /**
+     * @group DBAL-91
+     */
+    public function testAddForeignKeyWithQuotedColumnsAndTable()
+    {
+        $table = new Table("test");
+        $table->addColumn('"foo"', 'integer');
+        $table->addColumn('bar', 'integer');
+        $table->addForeignKeyConstraint('"boing"', array('"foo"', '"bar"'), array("id"));
+    }
+}