Fixed #477, Fixed #607, Fixed #604, Fixed #610

propelorm · Apr 15, 2014 · ef1e4eb · ef1e4eb
1 parent e3810e0
commit ef1e4eb
Show file tree

Hide file tree

Showing 51 changed files with 881 additions and 583 deletions.
diff --git a/src/Propel/Generator/Behavior/Archivable/ArchivableBehavior.php b/src/Propel/Generator/Behavior/Archivable/ArchivableBehavior.php
@@ -102,7 +102,6 @@ protected function addArchiveTable()
             // copy the indices
             foreach ($table->getIndices() as $index) {
                 $copiedIndex = clone $index;
-                $copiedIndex->setName('');
                 $archiveTable->addIndex($copiedIndex);
             }
             // copy unique indices to indices

diff --git a/src/Propel/Generator/Behavior/ConcreteInheritance/ConcreteInheritanceBehavior.php b/src/Propel/Generator/Behavior/ConcreteInheritance/ConcreteInheritanceBehavior.php
@@ -10,6 +10,7 @@
 
 namespace Propel\Generator\Behavior\ConcreteInheritance;
 
+use Propel\Generator\Exception\InvalidArgumentException;
 use Propel\Generator\Model\Behavior;
 use Propel\Generator\Model\ForeignKey;
 
@@ -66,7 +67,9 @@ public function modifyTable()
             if ($column->isPrimaryKey() && $this->isCopyData()) {
                 $fk = new ForeignKey();
                 $fk->setForeignTableCommonName($column->getTable()->getCommonName());
-                $fk->setForeignSchemaName($column->getTable()->getSchema());
+                if ($table->guessSchemaName() != $column->getTable()->guessSchemaName()) {
+                    $fk->setForeignSchemaName($column->getTable()->guessSchemaName());
+                }
                 $fk->setOnDelete('CASCADE');
                 $fk->setOnUpdate(null);
                 $fk->addReference($copiedColumn, $column);
@@ -123,7 +126,11 @@ protected function getParentTable()
             $tableName = $this->getParameter('schema').$database->getPlatform()->getSchemaDelimiter().$tableName;
         }
 
-        return $database->getTable($tableName);
+        if (!$table = $database->getTable($tableName)) {
+            throw new InvalidArgumentException(sprintf('Table "%s" used in the concrete_inheritance behavior at table "%s" not exist.', $tableName, $this->getTable()->getName()));
+        }
+
+        return $table;
     }
 
     protected function isCopyData()

diff --git a/src/Propel/Generator/Builder/Util/SchemaReader.php b/src/Propel/Generator/Builder/Util/SchemaReader.php
@@ -232,11 +232,13 @@ public function startElement($parser, $name, $attributes)
                     break;
 
                 case 'index':
-                    $this->currIndex = $this->currTable->addIndex($attributes);
+                    $this->currIndex = new Index();
+                    $this->currIndex->loadMapping($attributes);
                     break;
 
                 case 'unique':
-                    $this->currUnique = $this->currTable->addUnique($attributes);
+                    $this->currUnique = new Unique();
+                    $this->currUnique->loadMapping($attributes);
                     break;
 
                 case 'vendor':
@@ -359,6 +361,12 @@ protected function _throwInvalidTagException($parser, $tag_name)
 
     public function endElement($parser, $name)
     {
+        if ('index' === $name) {
+            $this->currTable->addIndex($this->currIndex);
+        } else if ('unique' === $name) {
+            $this->currTable->addUnique($this->currUnique);
+        }
+
         if (self::DEBUG) {
             print('endElement(' . $name . ") called\n");
         }

diff --git a/src/Propel/Generator/Command/MigrationDiffCommand.php b/src/Propel/Generator/Command/MigrationDiffCommand.php
@@ -41,6 +41,7 @@ protected function configure()
             ->addOption('output-dir',       null, InputOption::VALUE_REQUIRED,  'The output directory', self::DEFAULT_OUTPUT_DIRECTORY)
             ->addOption('migration-table',  null, InputOption::VALUE_REQUIRED,  'Migration table name', self::DEFAULT_MIGRATION_TABLE)
             ->addOption('connection',       null, InputOption::VALUE_IS_ARRAY | InputOption::VALUE_REQUIRED, 'Connection to use', array())
+            ->addOption('table-renaming',   null, InputOption::VALUE_NONE,  'Detect table renaming', null)
             ->addOption('editor',           null, InputOption::VALUE_OPTIONAL,  'The text editor to use to open diff files', null)
             ->setName('migration:diff')
             ->setAliases(array('diff'))
@@ -85,8 +86,15 @@ protected function execute(InputInterface $input, OutputInterface $output)
         }
 
         $totalNbTables = 0;
-        $schema = new Schema();
-        foreach ($connections as $name => $params) {
+        $reversedSchema = new Schema();
+
+        foreach ($manager->getDatabases() as $appDatabase) {
+
+            $name = $appDatabase->getName();
+            if (!$params = @$connections[$name]) {
+                $output->writeln(sprintf('<info>No connection configured for database "%s"</info>', $name));
+            }
+
             if ($input->getOption('verbose')) {
                 $output->writeln(sprintf('Connecting to database "%s" using DSN "%s"', $name, $params['dsn']));
             }
@@ -99,14 +107,22 @@ protected function execute(InputInterface $input, OutputInterface $output)
                 continue;
             }
 
+            $additionalTables = [];
+            foreach ($appDatabase->getTables() as $table) {
+                if ($table->getSchema() && $table->getSchema() != $appDatabase->getSchema()) {
+                    $additionalTables[] = $table;
+                }
+            }
+
             $database = new Database($name);
             $database->setPlatform($platform);
+            $database->setSchema($appDatabase->getSchema());
             $database->setDefaultIdMethod(IdMethod::NATIVE);
 
             $parser   = $generatorConfig->getConfiguredSchemaParser($conn);
-            $nbTables = $parser->parse($database, $this);
+            $nbTables = $parser->parse($database, $additionalTables);
 
-            $schema->addDatabase($database);
+            $reversedSchema->addDatabase($database);
             $totalNbTables += $nbTables;
 
             if ($input->getOption('verbose')) {
@@ -120,27 +136,24 @@ protected function execute(InputInterface $input, OutputInterface $output)
             $output->writeln('No table found in all databases');
         }
 
-        $appDatasFromXml = $manager->getDataModels();
-        $appDataFromXml  = array_pop($appDatasFromXml);
-
         // comparing models
         $output->writeln('Comparing models...');
+        $tableRenaming = $input->getOption('table-renaming');
 
         $migrationsUp   = array();
         $migrationsDown = array();
-        foreach ($schema->getDatabases() as $database) {
+        foreach ($reversedSchema->getDatabases() as $database) {
             $name = $database->getName();
 
             if ($input->getOption('verbose')) {
                 $output->writeln(sprintf('Comparing database "%s"', $name));
             }
 
-            if (!$appDataFromXml->hasDatabase($name)) {
-                // FIXME: tables present in database but not in XML
+            if (!$appDataDatabase = $manager->getDatabase($name)) {
                 continue;
             }
 
-            $databaseDiff = DatabaseComparator::computeDiff($database, $appDataFromXml->getDatabase($name));
+            $databaseDiff = DatabaseComparator::computeDiff($database, $appDataDatabase, false, $tableRenaming);
 
             if (!$databaseDiff) {
                 if ($input->getOption('verbose')) {
@@ -151,6 +164,13 @@ protected function execute(InputInterface $input, OutputInterface $output)
 
             $output->writeln(sprintf('Structure of database was modified in datasource "%s": %s', $name, $databaseDiff->getDescription()));
 
+            foreach ($databaseDiff->getPossibleRenamedTables() as $fromTableName => $toTableName) {
+                $output->writeln(sprintf(
+                    '<info>Possible table renaming detected: "%s" to "%s". It will be deleted and recreated. Use --table-renaming to only rename it.</info>',
+                        $fromTableName, $toTableName
+                ));
+            }
+
             $platform               = $generatorConfig->getConfiguredPlatform(null, $name);
             $migrationsUp[$name]    = $platform->getModifyDatabaseDDL($databaseDiff);
             $migrationsDown[$name]  = $platform->getModifyDatabaseDDL($databaseDiff->getReverseDiff());
@@ -185,8 +205,11 @@ protected function execute(InputInterface $input, OutputInterface $output)
      */
     protected function getReverseClass(InputInterface $input)
     {
-        $reverse = strstr($input->getOption('platform'), 'Platform', true);
-        $reverse = 'Propel\\Generator\\Reverse\\'.$reverse.'SchemaParser';
+        $reverse = $input->getOption('platform');
+        if (false !== strpos($reverse, 'Platform')) {
+            $reverse = strstr($input->getOption('platform'), 'Platform', true);
+        }
+        $reverse = sprintf('Propel\\Generator\\Reverse\\%sSchemaParser', ucfirst($reverse));
 
         return $reverse;
     }

diff --git a/src/Propel/Generator/Command/MigrationUpCommand.php b/src/Propel/Generator/Command/MigrationUpCommand.php
@@ -10,6 +10,7 @@
 
 namespace Propel\Generator\Command;
 
+use Propel\Runtime\Exception\RuntimeException;
 use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
@@ -112,9 +113,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
                     $stmt->execute();
                     $res++;
                 } catch (\PDOException $e) {
-                    $output->writeln(sprintf('<error>Failed to execute SQL "%s". Aborting migration.</error>', $statement));
-
-                    return false;
+                    throw new RuntimeException(sprintf('<error>Failed to execute SQL "%s". Aborting migration.</error>', $statement), 0, $e);
                 }
             }
             if (!$res) {

diff --git a/src/Propel/Generator/Manager/AbstractManager.php b/src/Propel/Generator/Manager/AbstractManager.php
@@ -14,6 +14,7 @@
 use Propel\Generator\Config\GeneratorConfigInterface;
 use Propel\Generator\Exception\BuildException;
 use Propel\Generator\Exception\EngineException;
+use Propel\Generator\Model\Database;
 use Propel\Generator\Model\Schema;
 
 /**
@@ -31,6 +32,11 @@ abstract class AbstractManager
      */
     protected $dataModels = array();
 
+    /**
+     * @var Database[]
+     */
+    protected $databases;
+
     /**
      * Map of data model name to database name.
      * Should probably stick to the convention
@@ -162,6 +168,44 @@ public function getDataModelDbMap()
         return $this->dataModelDbMap;
     }
 
+    /**
+     * @return Database[]
+     */
+    public function getDatabases()
+    {
+        if (null === $this->databases) {
+            $databases = array();
+            foreach ($this->getDataModels() as $dataModel) {
+                foreach ($dataModel->getDatabases() as $database) {
+                    if (!isset($databases[$database->getName()])) {
+                        $databases[$database->getName()] = $database;
+                    } else {
+                        $tables = $database->getTables();
+                        // Merge tables from different schema.xml to the same database
+                        foreach ($tables as $table) {
+                            if (!$databases[$database->getName()]->hasTable($table->getName(), true)) {
+                                $databases[$database->getName()]->addTable($table);
+                            }
+                        }
+                    }
+                }
+            }
+            $this->databases = $databases;
+        }
+
+        return $this->databases;
+    }
+
+    /**
+     * @param  string $name
+     * @return Database|null
+     */
+    public function getDatabase($name)
+    {
+        $dbs = $this->getDatabases();
+        return @$dbs[$name];
+    }
+
     /**
      * Sets whether to perform validation on the datamodel schema.xml file(s).
      *
@@ -233,11 +277,8 @@ protected function loadDataModels()
             $dom = new \DOMDocument('1.0', 'UTF-8');
             $dom->load($dmFilename);
 
-
-
             $this->includeExternalSchemas($dom, $schema->getPath());
 
-
             // normalize (or transform) the XML document using XSLT
             if ($this->getGeneratorConfig()->getBuildProperty('schemaTransform') && $this->xsl) {
                 $this->log('Transforming ' . $dmFilename . ' using stylesheet ' . $this->xsl->getPath());

diff --git a/src/Propel/Generator/Manager/SqlManager.php b/src/Propel/Generator/Manager/SqlManager.php
@@ -67,34 +67,6 @@ public function getConnection($datasource)
         return $this->connections[$datasource];
     }
 
-    /**
-     * @return array
-     */
-    public function getDatabases()
-    {
-        if (null === $this->databases) {
-            $databases = array();
-            foreach ($this->getDataModels() as $dataModel) {
-                foreach ($dataModel->getDatabases() as $database) {
-                    if (!isset($databases[$database->getName()])) {
-                        $databases[$database->getName()] = $database;
-                    } else {
-                        $tables = $database->getTables();
-                        // Merge tables from different schema.xml to the same database
-                        foreach ($tables as $table) {
-                            if (!$databases[$database->getName()]->hasTable($table->getName(), true)) {
-                                $databases[$database->getName()]->addTable($table);
-                            }
-                        }
-                    }
-                }
-            }
-            $this->databases = $databases;
-        }
-
-        return $this->databases;
-    }
-
     /**
      * @return string
      */

diff --git a/src/Propel/Generator/Model/Database.php b/src/Propel/Generator/Model/Database.php
@@ -460,7 +460,6 @@ public function addTable($table)
         if (!$table instanceof Table) {
             $tbl = new Table();
             $tbl->setDatabase($this);
-            $tbl->setSchema($this->getSchema());
             $tbl->loadMapping($table);
 
             return $this->addTable($tbl);
@@ -472,10 +471,6 @@ public function addTable($table)
             throw new EngineException(sprintf('Table "%s" declared twice', $table->getName()));
         }
 
-        if (null === $table->getSchema()) {
-            $table->setSchema($this->getSchema());
-        }
-
         $this->tables[] = $table;
         $this->tablesByName[$table->getName()] = $table;
         $this->tablesByLowercaseName[strtolower($table->getName())] = $table;
@@ -798,7 +793,6 @@ protected function registerBehavior(Behavior $behavior)
     protected function setupTableReferrers()
     {
         foreach ($this->tables as $table) {
-            $table->doNaming();
             $table->setupReferrers();
         }
     }
@@ -823,8 +817,10 @@ public function __toString()
 
             $fks = [];
             foreach ($table->getForeignKeys() as $fk) {
-                $fks[] = sprintf("      %s (%s => %s)",
+                $fks[] = sprintf("      %s to %s.%s (%s => %s)",
                     $fk->getName(),
+                    $fk->getForeignSchemaName(),
+                    $fk->getForeignTableCommonName(),
                     join(', ', $fk->getLocalColumns()),
                     join(', ', $fk->getForeignColumns())
                 );