Merge pull request #940 from doctrine/no-schema-table

Better handling of the table initialization
doctrine · Mar 6, 2020 · 05628ad · 05628ad
2 parents f87b808 + 3e6768c
commit 05628ad
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 22 deletions.
diff --git a/lib/Doctrine/Migrations/DependencyFactory.php b/lib/Doctrine/Migrations/DependencyFactory.php
@@ -21,7 +21,6 @@
 use Doctrine\Migrations\Generator\Generator;
 use Doctrine\Migrations\Generator\SqlGenerator;
 use Doctrine\Migrations\Metadata\Storage\MetadataStorage;
-use Doctrine\Migrations\Metadata\Storage\MetadataStorageConfiguration;
 use Doctrine\Migrations\Metadata\Storage\TableMetadataStorage;
 use Doctrine\Migrations\Metadata\Storage\TableMetadataStorageConfiguration;
 use Doctrine\Migrations\Provider\DBALSchemaDiffProvider;
@@ -307,19 +306,12 @@ public function setService(string $id, $service) : void
         $this->dependencies[$id] = $service;
     }
 
-    private function getMetadataStorageConfiguration() : MetadataStorageConfiguration
-    {
-        return $this->getDependency(MetadataStorageConfiguration::class, static function () : MetadataStorageConfiguration {
-            return new TableMetadataStorageConfiguration();
-        });
-    }
-
     public function getMetadataStorage() : MetadataStorage
     {
         return $this->getDependency(MetadataStorage::class, function () : MetadataStorage {
             return new TableMetadataStorage(
                 $this->getConnection(),
-                $this->getMetadataStorageConfiguration(),
+                $this->getConfiguration()->getMetadataStorageConfiguration(),
                 $this->getMigrationRepository()
             );
         });

diff --git a/lib/Doctrine/Migrations/Metadata/Storage/TableMetadataStorage.php b/lib/Doctrine/Migrations/Metadata/Storage/TableMetadataStorage.php
@@ -67,6 +67,10 @@ public function __construct(
 
     public function getExecutedMigrations() : ExecutedMigrationsSet
     {
+        if (! $this->isInitialized()) {
+            return new ExecutedMigrationsSet([]);
+        }
+
         $this->checkInitialization();
         $rows = $this->connection->fetchAll(sprintf('SELECT * FROM %s', $this->configuration->getTableName()));
 
@@ -132,15 +136,15 @@ public function complete(ExecutionResult $result) : void
 
     public function ensureInitialized() : void
     {
-        $expectedSchemaChangelog = $this->getExpectedTable();
-
-        if (! $this->isInitialized($expectedSchemaChangelog)) {
+        if (! $this->isInitialized()) {
+            $expectedSchemaChangelog = $this->getExpectedTable();
             $this->schemaManager->createTable($expectedSchemaChangelog);
 
             return;
         }
 
-        $diff = $this->needsUpdate($expectedSchemaChangelog);
+        $expectedSchemaChangelog = $this->getExpectedTable();
+        $diff                    = $this->needsUpdate($expectedSchemaChangelog);
         if ($diff === null) {
             return;
         }
@@ -158,23 +162,23 @@ private function needsUpdate(Table $expectedTable) : ?TableDiff
         return $diff instanceof TableDiff ? $diff : null;
     }
 
-    private function isInitialized(Table $expectedTable) : bool
+    private function isInitialized() : bool
     {
         if ($this->connection instanceof MasterSlaveConnection) {
             $this->connection->connect('master');
         }
 
-        return $this->schemaManager->tablesExist([$expectedTable->getName()]);
+        return $this->schemaManager->tablesExist([$this->configuration->getTableName()]);
     }
 
     private function checkInitialization() : void
     {
-        $expectedTable = $this->getExpectedTable();
-
-        if (! $this->isInitialized($expectedTable)) {
+        if (! $this->isInitialized()) {
             throw MetadataStorageError::notInitialized();
         }
 
+        $expectedTable = $this->getExpectedTable();
+
         if ($this->needsUpdate($expectedTable) !== null) {
             throw MetadataStorageError::notUpToDate();
         }

diff --git a/tests/Doctrine/Migrations/Tests/DependencyFactoryTest.php b/tests/Doctrine/Migrations/Tests/DependencyFactoryTest.php
@@ -5,6 +5,7 @@
 namespace Doctrine\Migrations\Tests;
 
 use Doctrine\DBAL\Connection;
+use Doctrine\DBAL\DriverManager;
 use Doctrine\Migrations\Configuration\Configuration;
 use Doctrine\Migrations\Configuration\Connection\ExistingConnection;
 use Doctrine\Migrations\Configuration\EntityManager\ExistingEntityManager;
@@ -14,6 +15,7 @@
 use Doctrine\Migrations\Exception\MissingDependency;
 use Doctrine\Migrations\Finder\GlobFinder;
 use Doctrine\Migrations\Finder\RecursiveRegexFinder;
+use Doctrine\Migrations\Metadata\Storage\TableMetadataStorageConfiguration;
 use Doctrine\ORM\EntityManager;
 use PHPUnit\Framework\MockObject\MockObject;
 use Psr\Log\LoggerInterface;
@@ -123,4 +125,18 @@ public function testOverrideCustomLogger() : void
         $di->setService(LoggerInterface::class, $anotherLogger);
         self::assertSame($anotherLogger, $di->getLogger());
     }
+
+    public function testMetadataConfigurationIsPassedToTableStorage() : void
+    {
+        $connection     = DriverManager::getConnection(['driver' => 'pdo_sqlite', 'memory' => true]);
+        $metadataConfig = new TableMetadataStorageConfiguration();
+        $metadataConfig->setTableName('foo');
+        $configuration = new Configuration();
+        $configuration->setMetadataStorageConfiguration($metadataConfig);
+
+        $di = DependencyFactory::fromConnection(new ExistingConfiguration($configuration), new ExistingConnection($connection));
+        $di->getMetadataStorage()->ensureInitialized();
+
+        self::assertTrue($connection->getSchemaManager()->tablesExist(['foo']));
+    }
 }
diff --git a/tests/Doctrine/Migrations/Tests/Metadata/Storage/TableMetadataStorageTest.php b/tests/Doctrine/Migrations/Tests/Metadata/Storage/TableMetadataStorageTest.php
@@ -66,11 +66,12 @@ public function testDifferentTableNotUpdatedOnRead() : void
         $this->storage->getExecutedMigrations();
     }
 
-    public function testTableNotCreatedOnRead() : void
+    public function testTableNotCreatedOnReadButReadingWorks() : void
     {
-        $this->expectException(MetadataStorageError::class);
-        $this->expectExceptionMessage('The metadata storage is not initialized, please run the sync-metadata-storage command to fix this issue.');
-        $this->storage->getExecutedMigrations();
+        $executedMigrations = $this->storage->getExecutedMigrations();
+
+        self::assertSame([], $executedMigrations->getItems());
+        self::assertFalse($this->schemaManager->tablesExist([$this->config->getTableName()]));
     }
 
     public function testTableStructureUpdate() : void
@@ -98,6 +99,27 @@ public function testTableStructureUpdate() : void
         self::assertInstanceOf(IntegerType::class, $table->getColumn('d')->getType());
     }
 
+    public function testTableNotUpToDateTriggersExcepton() : void
+    {
+        $this->expectException(MetadataStorageError::class);
+        $this->expectExceptionMessage('The metadata storage is not up to date, please run the sync-metadata-storage command to fix this issue.');
+
+        $config = new TableMetadataStorageConfiguration();
+        $config->setTableName('a');
+        $config->setVersionColumnName('b');
+        $config->setVersionColumnLength(199);
+        $config->setExecutedAtColumnName('c');
+        $config->setExecutionTimeColumnName('d');
+
+        $table = new Table($config->getTableName());
+        $table->addColumn($config->getVersionColumnName(), 'string', ['notnull' => true, 'length' => 10]);
+        $table->setPrimaryKey([$config->getVersionColumnName()]);
+        $this->schemaManager->createTable($table);
+
+        $storage = new TableMetadataStorage($this->connection, $config);
+        $storage->getExecutedMigrations();
+    }
+
     public function testTableStructure() : void
     {
         $config = new TableMetadataStorageConfiguration();