[DBAL-204] Sane refactoring of the previous idea. Removed a bunch of code...

[DBAL-204] Sane refactoring of the previous idea. Removed a bunch of code again and implemented proper schema namespacing. Also got the wording right (hopefully) and fixed the algorithms.

lib/Doctrine/DBAL/Schema/AbstractAsset.php

@@ -39,6 +39,16 @@ abstract class AbstractAsset
      */
     protected $_name;
 
+    /**
+     * Namespace of the asset. If none isset the default namespace is assumed.
+     *
+     * @var string
+     */
+    protected $_namespace;
+
+    /**
+     * @var bool
+     */
     protected $_quoted = false;
 
     /**
@@ -52,9 +62,73 @@ abstract class AbstractAsset
             $this->_quoted = true;
             $name = $this->trimQuotes($name);
         }
+        if (strpos($name, ".") !== false) {
+            $parts = explode(".", $name);
+            $this->_namespace = $parts[0];
+            $name = $parts[1];
+        }
         $this->_name = $name;
     }
 
+    /**
+     * Is this asset in the default namespace?
+     *
+     * @param string $defaultNamespaceName
+     * @return bool
+     */
+    public function isInDefaultNamespace($defaultNamespaceName)
+    {
+        return $this->_namespace == $defaultNamespaceName || $this->_namespace === null;
+    }
+
+    /**
+     * Get namespace name of this asset.
+     *
+     * If NULL is returned this means the default namespace is used.
+     *
+     * @return string
+     */
+    public function getNamespaceName()
+    {
+        return $this->_namespace;
+    }
+
+    /**
+     * The shortest name is stripped of the default namespace. All other
+     * namespaced elements are returned as full-qualified names.
+     *
+     * @param string
+     * @return string
+     */
+    public function getShortestName($defaultNamespaceName)
+    {
+        $shortestName = $this->getName();
+        if ($this->_namespace == $defaultNamespaceName) {
+            $shortestName = $this->_name;
+        }
+        return strtolower($shortestName);
+    }
+
+    /**
+     * The normalized name is full-qualified and lowerspaced. Lowerspacing is
+     * actually wrong, but we have to do it to keep our sanity. If you are
+     * using database objects that only differentiate in the casing (FOO vs
+     * Foo) then you will NOT be able to use Doctrine Schema abstraction.
+     *
+     * Every non-namespaced element is prefixed with the default namespace
+     * name which is passed as argument to this method.
+     *
+     * @return string
+     */
+    public function getFullQualifiedName($defaultNamespaceName)
+    {
+        $name = $this->getName();
+        if (!$this->_namespace) {
+            $name = $defaultNamespaceName . "." . $name;
+        }
+        return strtolower($name);
+    }
+
     /**
      * Check if this identifier is quoted.
      *
@@ -84,6 +158,9 @@ abstract class AbstractAsset
      */
     public function getName()
     {
+        if ($this->_namespace) {
+            return $this->_namespace . "." . $this->_name;
+        }
         return $this->_name;
     }
 
@@ -97,7 +174,7 @@ abstract class AbstractAsset
     public function getQuotedName(AbstractPlatform $platform)
     {
         $keywords = $platform->getReservedKeywordsList();
-        $parts = explode(".", $this->_name);
+        $parts = explode(".", $this->getName());
         foreach ($parts AS $k => $v) {
             $parts[$k] = ($this->_quoted || $keywords->isKeyword($v)) ? $platform->quoteIdentifier($v) : $v;
         }

lib/Doctrine/DBAL/Schema/AbstractSchemaManager.php

@@ -804,7 +804,8 @@ abstract class AbstractSchemaManager
         }
         $tables = $this->listTables();
 
-        return new Schema($tables, $sequences, $this->createSchemaConfig());
+        $schema = new Schema($tables, $sequences, $this->createSchemaConfig());
+        return $schema;
     }
 
     /**
@@ -816,7 +817,11 @@ abstract class AbstractSchemaManager
     {
         $schemaConfig = new SchemaConfig();
         $schemaConfig->setMaxIdentifierLength($this->_platform->getMaxIdentifierLength());
-        $schemaConfig->setSearchPaths($this->getSchemaSearchPaths());
+
+        $searchPaths = $this->getSchemaSearchPaths();
+        if (isset($searchPaths[0])) {
+            $schemaConfig->setName($searchPaths[0]);
+        }
 
         return $schemaConfig;
     }

lib/Doctrine/DBAL/Schema/Comparator.php

@@ -61,11 +61,12 @@ class Comparator
 
         $foreignKeysToTable = array();
 
-        foreach ( $toSchema->getFullQualifiedTableNames() AS $tableName ) {
-            if ( !$fromSchema->hasFullQualifiedTable($tableName)) {
-                $diff->newTables[$tableName] = $toSchema->getFullQualifiedTable($tableName);
+        foreach ( $toSchema->getTables() AS $table ) {
+            $tableName = $table->getShortestName($toSchema->getName());
+            if ( !$fromSchema->hasTable($tableName)) {
+                $diff->newTables[$tableName] = $toSchema->getTable($tableName);
             } else {
-                $tableDifferences = $this->diffTable( $fromSchema->getFullQualifiedTable($tableName), $toSchema->getFullQualifiedTable($tableName) );
+                $tableDifferences = $this->diffTable( $fromSchema->getTable($tableName), $toSchema->getTable($tableName) );
                 if ( $tableDifferences !== false ) {
                     $diff->changedTables[$tableName] = $tableDifferences;
                 }
@@ -73,9 +74,11 @@ class Comparator
         }
 
         /* Check if there are tables removed */
-        foreach ( $fromSchema->getFullQualifiedTableNames() AS $tableName ) {
-            $table = $fromSchema->getFullQualifiedTable($tableName);
-            if ( !$toSchema->hasFullQualifiedTable($tableName) ) {
+        foreach ( $fromSchema->getTables() AS $table ) {
+            $tableName = $table->getShortestName($fromSchema->getName());
+
+            $table = $fromSchema->getTable($tableName);
+            if ( !$toSchema->hasTable($tableName) ) {
                 $diff->removedTables[$tableName] = $table;
             }
 
@@ -95,7 +98,8 @@ class Comparator
             }
         }
 
-        foreach ( $toSchema->getSequences() AS $sequenceName => $sequence) {
+        foreach ( $toSchema->getSequences() AS $sequence) {
+            $sequenceName = $sequence->getShortestName($toSchema->getName());
             if (!$fromSchema->hasSequence($sequenceName)) {
                 $diff->newSequences[] = $sequence;
             } else {
@@ -105,7 +109,8 @@ class Comparator
             }
         }
 
-        foreach ($fromSchema->getSequences() AS $sequenceName => $sequence) {
+        foreach ($fromSchema->getSequences() AS $sequence) {
+            $sequenceName = $sequence->getShortestName($fromSchema->getName());
             if (!$toSchema->hasSequence($sequenceName)) {
                 $diff->removedSequences[] = $sequence;
             }

lib/Doctrine/DBAL/Schema/Schema.php

 <?php
 /*
- *  $Id$
- *
  * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
  * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
  * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
@@ -28,10 +26,30 @@ use Doctrine\DBAL\Schema\Visitor\Visitor;
 /**
  * Object representation of a database schema
  *
+ * Different vendors have very inconsistent naming with regard to the concept
+ * of a "schema". Doctrine understands a schema as the entity that conceptually
+ * wraps a set of database objects such as tables, sequences, indexes and
+ * foreign keys that belong to each other into a namespace. A Doctrine Schema
+ * has nothing to do with the "SCHEMA" defined as in PostgreSQL, it is more
+ * related to the concept of "DATABASE" that exists in MySQL and PostgreSQL.
+ *
+ * Every asset in the doctrine schema has a name. A name consists of either a
+ * namespace.local name pair or just a local unqualified name.
+ *
+ * The abstraction layer that covers a PostgreSQL schema is the namespace of an
+ * database object (asset). A schema can have a name, which will be used as
+ * default namespace for the unqualified database objects that are created in
+ * the schema.
+ *
+ * In the case of MySQL where cross-database queries are allowed this leads to
+ * databases being "misinterpreted" as namespaces. This is intentional, however
+ * the CREATE/DROP SQL visitors will just filter this queries and do not
+ * execute them. Only the queries for the currently connected database are
+ * executed.
+ *
  * @license http://www.opensource.org/licenses/lgpl-license.php LGPL
  * @link    www.doctrine-project.org
  * @since   2.0
- * @version $Revision$
  * @author  Benjamin Eberlei <kontakt@beberlei.de>
  */
 class Schema extends AbstractAsset
@@ -64,6 +82,7 @@ class Schema extends AbstractAsset
             $schemaConfig = new SchemaConfig();
         }
         $this->_schemaConfig = $schemaConfig;
+        $this->_setName($schemaConfig->getName() ?: 'public');
 
         foreach ($tables AS $table) {
             $this->_addTable($table);
@@ -81,17 +100,12 @@ class Schema extends AbstractAsset
         return $this->_schemaConfig->hasExplicitForeignKeyIndexes();
     }
 
-    public function getName()
-    {
-        return $this->_schemaConfig->getName();
-    }
-
     /**
      * @param Table $table
      */
     protected function _addTable(Table $table)
     {
-        $tableName = strtolower($table->getName());
+        $tableName = $table->getFullQualifiedName($this->getName());
         if(isset($this->_tables[$tableName])) {
             throw SchemaException::tableAlreadyExists($tableName);
         }
@@ -105,7 +119,7 @@ class Schema extends AbstractAsset
      */
     protected function _addSequence(Sequence $sequence)
     {
-        $seqName = strtolower($sequence->getName());
+        $seqName = $sequence->getFullQualifiedName($this->getName());
         if (isset($this->_sequences[$seqName])) {
             throw SchemaException::sequenceAlreadyExists($seqName);
         }
@@ -128,7 +142,7 @@ class Schema extends AbstractAsset
      */
     public function getTable($tableName)
     {
-        $tableName = strtolower($tableName);
+        $tableName = $this->getFullQualifiedAssetName($tableName);
         if (!isset($this->_tables[$tableName])) {
             throw SchemaException::tableDoesNotExist($tableName);
         }
@@ -136,6 +150,17 @@ class Schema extends AbstractAsset
         return $this->_tables[$tableName];
     }
 
+    /**
+     * @return string
+     */
+    private function getFullQualifiedAssetName($name)
+    {
+        if (strpos($name, ".") === false) {
+            $name = $this->getName() . "." . $name;
+        }
+        return strtolower($name);
+    }
+
     /**
      * Does this schema have a table with the given name?
      *
@@ -144,7 +169,7 @@ class Schema extends AbstractAsset
      */
     public function hasTable($tableName)
     {
-        $tableName = strtolower($tableName);
+        $tableName = $this->getFullQualifiedAssetName($tableName);
         return isset($this->_tables[$tableName]);
     }
 
@@ -154,74 +179,14 @@ class Schema extends AbstractAsset
      *
      * @return array
      */
-    public function getFullQualifiedTableNames()
-    {
-        $names = array();
-        foreach ($this->_tables as $table) {
-            $names[] = $table->getFullQualifiedTableName($this->_schemaConfig->getName());
-        }
-        return $names;
-    }
-
-    /**
-     * Does this schema have a table with the given FQN?
-     *
-     * @return bool
-     */
-    public function hasFullQualifiedTable($fqTableName)
+    public function getTableNames()
     {
-        $fqTableName = strtolower($fqTableName);
-        if (strpos($fqTableName, ".") === false) {
-            $shortTableName = $fqTableName;
-        } else {
-            $parts = explode(".", $fqTableName);
-            $shortTableName = $fqTableName[1];
+        return array_keys($this->_tables);
     }
 
-        foreach ($this->_tables as $table) {
-            foreach ($this->_schemaConfig->getSearchPaths() as $searchPathSchema) {
-                if (strtolower($table->getFullQualifiedTableName($searchPathSchema)) == $fqTableName) {
-                    return true;
-                }
-            }
-            if (strtolower($table->getName()) == $shortTableName || strtolower($table->getName()) == $fqTableName) {
-                return true;
-            }
-        }
-        return false;
-    }
-
-    public function getFullQualifiedTable($fqTableName)
-    {
-        $fqTableName = strtolower($fqTableName);
-        if (strpos($fqTableName, ".") === false) {
-            $shortTableName = $fqTableName;
-        } else {
-            $parts = explode(".", $fqTableName);
-            $shortTableName = $fqTableName[1];
-        }
-
-        foreach ($this->_tables as $table) {
-            foreach ($this->_schemaConfig->getSearchPaths() as $searchPathSchema) {
-                if (strtolower($table->getFullQualifiedTableName($searchPathSchema)) == $fqTableName) {
-                    return $table;
-                }
-            }
-            if (strtolower($table->getName()) == $shortTableName) {
-                return $table;
-            }
-        }
-
-        throw SchemaException::tableDoesNotExist($fqTableName);
-    }
-
-    /**
-     * @param  string $sequenceName
-     * @return bool
-     */
     public function hasSequence($sequenceName)
     {
-        $sequenceName = strtolower($sequenceName);
+        $sequenceName = $this->getFullQualifiedAssetName($sequenceName);
         return isset($this->_sequences[$sequenceName]);
     }
 
@@ -232,7 +197,7 @@ class Schema extends AbstractAsset
      */
     public function getSequence($sequenceName)
     {
-        $sequenceName = strtolower($sequenceName);
+        $sequenceName = $this->getFullQualifiedAssetName($sequenceName);
         if(!$this->hasSequence($sequenceName)) {
             throw SchemaException::sequenceDoesNotExist($sequenceName);
         }
@@ -285,7 +250,7 @@ class Schema extends AbstractAsset
      */
     public function dropTable($tableName)
     {
-        $tableName = strtolower($tableName);
+        $tableName = $this->getFullQualifiedAssetName($tableName);
         $table = $this->getTable($tableName);
         unset($this->_tables[$tableName]);
         return $this;
@@ -312,7 +277,7 @@ class Schema extends AbstractAsset
      */
     public function dropSequence($sequenceName)
     {
-        $sequenceName = strtolower($sequenceName);
+        $sequenceName = $this->getFullQualifiedAssetName($sequenceName);
         unset($this->_sequences[$sequenceName]);
         return $this;
     }

lib/Doctrine/DBAL/Schema/SchemaConfig.php

@@ -40,9 +40,9 @@ class SchemaConfig
     protected $_maxIdentifierLength = 63;
 
     /**
-     * @var array
+     * @var string
      */
-    protected $_searchPaths = array();
+    protected $_name;
 
     /**
      * @return bool
@@ -76,21 +76,23 @@ class SchemaConfig
         return $this->_maxIdentifierLength;
     }
 
-    public function setSearchPaths($paths)
-    {
-        $this->_searchPaths = $paths;
-    }
-
-    public function getSearchPaths()
+    /**
+     * Get default namespace of schema objects.
+     *
+     * @return string
+     */
+    public function getName()
     {
-        return $this->_searchPaths;
+        return $this->_name;
     }
 
-    public function getName()
+    /**
+     * set default namespace name of schema objects.
+     *
+     * @param _name the value to set.
+     */
+    public function setName($name)
     {
-        if ($this->_searchPaths) {
-            return $this->_searchPaths[0];
-        }
-        return "";
+        $this->_name = $name;
     }
 }

lib/Doctrine/DBAL/Schema/Table.php

@@ -596,27 +596,6 @@ class Table extends AbstractAsset
         return $this->_options;
     }
 
-    /**
-     * Get the Fully-Qualified Table Name.
-     *
-     * The full-qualified table name is "schema.name". If the table name
-     * already has a schema, then the schema passed to the method is ignored.
-     * If no schema is set, then the passed schema is prepended as the table is
-     * in the current default schema.
-     *
-     * @param string $schemaName - Current schema this table is in.
-     * @return string
-     */
-    public function getFullQualifiedTableName($schemaName)
-    {
-        if (strpos($this->_name, ".") !== false) {
-            return $this->_name;
-        } else if ($schemaName) {
-            return $schemaName . "." . $this->_name;
-        }
-        return $this->_name;
-    }
-
     /**
      * @param Visitor $visitor
      */

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

@@ -366,7 +366,6 @@ class SchemaManagerFunctionalTestCase extends \Doctrine\Tests\DbalFunctionalTest
         $this->createTestTable('test_table');
 
         $schema = $this->_sm->createSchema();
-
         $this->assertTrue($schema->hasTable('test_table'));
     }
 

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

@@ -709,11 +709,12 @@ class ComparatorTest extends \PHPUnit_Framework_TestCase
     public function testFqnSchemaComparision()
     {
         $config = new SchemaConfig();
-        $config->setSearchPaths(array("foo"));
+        $config->setName("foo");
+
         $oldSchema = new Schema(array(), array(), $config);
         $oldSchema->createTable('bar');
 
-        $newSchema = new Schema();
+        $newSchema= new Schema(array(), array(), $config);
         $newSchema->createTable('foo.bar');
 
         $c = new Comparator();
@@ -723,21 +724,20 @@ class ComparatorTest extends \PHPUnit_Framework_TestCase
     /**
      * @group DBAL-204
      */
-    public function testFqnSchemaComparisionDifferent()
+    public function testFqnSchemaComparisionDifferentSchemaNameButSameTableNoDiff()
     {
         $config = new SchemaConfig();
-        $config->setSearchPaths(array("foo"));
+        $config->setName("foo");
+
         $oldSchema = new Schema(array(), array(), $config);
-        $oldSchema->createTable('bar');
+        $oldSchema->createTable('foo.bar');
 
         $newSchema = new Schema();
-        $newSchema->createTable('bar.bar');
+        $newSchema->createTable('bar');
 
         $c = new Comparator();
         $diff = $c->compare($oldSchema, $newSchema);
-
-        $this->assertTrue(isset($diff->newTables["bar.bar"]));
-        $this->assertTrue(isset($diff->removedTables["foo.bar"]));
+        $this->assertEquals(new SchemaDiff(), $c->compare($oldSchema, $newSchema));
     }
 
     /**
@@ -746,7 +746,7 @@ class ComparatorTest extends \PHPUnit_Framework_TestCase
     public function testFqnSchemaComparisionNoSchemaSame()
     {
         $config = new SchemaConfig();
-        $config->setSearchPaths(array("foo"));
+        $config->setName("foo");
         $oldSchema = new Schema(array(), array(), $config);
         $oldSchema->createTable('bar');
 

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

@@ -12,7 +12,7 @@ class SchemaTest extends \PHPUnit_Framework_TestCase
 {
     public function testAddTable()
     {
-        $tableName = "foo";
+        $tableName = "public.foo";
         $table = new Table($tableName);
 
         $schema = new Schema(array($table));
@@ -107,7 +107,7 @@ class SchemaTest extends \PHPUnit_Framework_TestCase
         $this->assertInstanceOf('Doctrine\DBAL\Schema\Sequence', $schema->getSequence("a_seq"));
 
         $sequences = $schema->getSequences();
-        $this->assertArrayHasKey('a_seq', $sequences);
+        $this->assertArrayHasKey('public.a_seq', $sequences);
     }
 
     public function testSequenceAccessCaseInsensitive()
@@ -145,7 +145,7 @@ class SchemaTest extends \PHPUnit_Framework_TestCase
         $this->assertInstanceOf('Doctrine\DBAL\Schema\Sequence', $schema->getSequence("a_seq"));
 
         $sequences = $schema->getSequences();
-        $this->assertArrayHasKey('a_seq', $sequences);
+        $this->assertArrayHasKey('public.a_seq', $sequences);
     }
 
     public function testDropSequence()

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

@@ -490,11 +490,11 @@ class TableTest extends \Doctrine\Tests\DbalTestCase
     public function testFullQualifiedTableName()
     {
         $table = new Table("`test`.`test`");
-        $this->assertEquals('test.test', $table->getFullQualifiedTableName("test"));
-        $this->assertEquals('test.test', $table->getFullQualifiedTableName("other"));
+        $this->assertEquals('test.test', $table->getFullQualifiedName("test"));
+        $this->assertEquals('test.test', $table->getFullQualifiedName("other"));
 
         $table = new Table("test");
-        $this->assertEquals('test.test', $table->getFullQualifiedTableName("test"));
-        $this->assertEquals('other.test', $table->getFullQualifiedTableName("other"));
+        $this->assertEquals('test.test', $table->getFullQualifiedName("test"));
+        $this->assertEquals('other.test', $table->getFullQualifiedName("other"));
     }
 }