Merge pull request #7875 from nicolas-grekas/schema-tool

Whilelist existing assets we know about from metadata in SchemaTool::getUpdateSchemaSql()

UPGRADE.md

@@ -10,6 +10,12 @@ Method `Doctrine\ORM\AbstractQuery#useResultCache()` which could be used for bot
 To optimize DB interaction, `Doctrine\ORM\Tools\Pagination\Paginator` no longer fetches identifiers to be able to
 perform the pagination with join collections when max results isn't set in the query.
 
+## Minor BC BREAK: tables filtered with `schema_filter` are no longer created
+
+When generating schema diffs, if a source table is filtered out by a `schema_filter` expression, then a `CREATE TABLE` was
+always generated, even if the table already existed. This has been changed in this release and the table will no longer
+be created.
+
 ## Deprecated number unaware `Doctrine\ORM\Mapping\UnderscoreNamingStrategy`
 
 In the last patch of the `v2.6.x` series, we fixed a bug that was not converting names properly when they had numbers

lib/Doctrine/ORM/Tools/SchemaTool.php

@@ -19,7 +19,7 @@
 
 namespace Doctrine\ORM\Tools;
 
-use Doctrine\ORM\ORMException;
+use Doctrine\DBAL\Schema\AbstractAsset;
 use Doctrine\DBAL\Schema\Comparator;
 use Doctrine\DBAL\Schema\Index;
 use Doctrine\DBAL\Schema\Schema;
@@ -28,6 +28,7 @@
 use Doctrine\DBAL\Schema\Visitor\RemoveNamespacedAssets;
 use Doctrine\ORM\EntityManagerInterface;
 use Doctrine\ORM\Mapping\ClassMetadata;
+use Doctrine\ORM\ORMException;
 use Doctrine\ORM\Tools\Event\GenerateSchemaTableEventArgs;
 use Doctrine\ORM\Tools\Event\GenerateSchemaEventArgs;
 
@@ -891,10 +892,8 @@ public function updateSchema(array $classes, $saveMode = false)
      */
     public function getUpdateSchemaSql(array $classes, $saveMode = false)
     {
-        $sm = $this->em->getConnection()->getSchemaManager();
-
-        $fromSchema = $sm->createSchema();
         $toSchema = $this->getSchemaFromMetadata($classes);
+        $fromSchema = $this->createSchemaForComparison($toSchema);
 
         $comparator = new Comparator();
         $schemaDiff = $comparator->compare($fromSchema, $toSchema);
@@ -905,4 +904,35 @@ public function getUpdateSchemaSql(array $classes, $saveMode = false)
 
         return $schemaDiff->toSql($this->platform);
     }
+
+    /**
+     * Creates the schema from the database, ensuring tables from the target schema are whitelisted for comparison.
+     */
+    private function createSchemaForComparison(Schema $toSchema) : Schema
+    {
+        $connection    = $this->em->getConnection();
+        $schemaManager = $connection->getSchemaManager();
+
+        // backup schema assets filter
+        $config         = $connection->getConfiguration();
+        $previousFilter = $config->getSchemaAssetsFilter();
+
+        if ($previousFilter === null) {
+            return $schemaManager->createSchema();
+        }
+
+        // whitelist assets we already know about in $toSchema, use the existing filter otherwise
+        $config->setSchemaAssetsFilter(static function ($asset) use ($previousFilter, $toSchema) : bool {
+            $assetName = $asset instanceof AbstractAsset ? $asset->getName() : $asset;
+
+            return $toSchema->hasTable($assetName) || $toSchema->hasSequence($assetName) || $previousFilter($asset);
+        });
+
+        try {
+            return $schemaManager->createSchema();
+        } finally {
+            // restore schema assets filter
+            $config->setSchemaAssetsFilter($previousFilter);
+        }
+    }
 }

tests/Doctrine/Tests/ORM/Functional/Ticket/GH7875Test.php

@@ -0,0 +1,136 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\Tests\ORM\Functional\Ticket;
+
+use Doctrine\DBAL\Configuration;
+use Doctrine\DBAL\Platforms\PostgreSqlPlatform;
+use Doctrine\ORM\Tools\SchemaTool;
+use Doctrine\Tests\OrmFunctionalTestCase;
+use function array_filter;
+use function current;
+use function method_exists;
+use function sprintf;
+use function strpos;
+
+/** @group GH7875 */
+final class GH7875Test extends OrmFunctionalTestCase
+{
+    /** @after */
+    public function cleanUpSchema() : void
+    {
+        $connection = $this->_em->getConnection();
+
+        $connection->exec('DROP TABLE IF EXISTS gh7875_my_entity');
+        $connection->exec('DROP TABLE IF EXISTS gh7875_my_other_entity');
+
+        if ($connection->getDatabasePlatform() instanceof PostgreSqlPlatform) {
+            $connection->exec('DROP SEQUENCE IF EXISTS gh7875_my_entity_id_seq');
+            $connection->exec('DROP SEQUENCE IF EXISTS gh7875_my_other_entity_id_seq');
+        }
+    }
+
+    /**
+     * @param string[] $sqls
+     *
+     * @return string[]
+     */
+    private function filterCreateTable(array $sqls, string $tableName) : array
+    {
+        return array_filter($sqls, static function (string $sql) use ($tableName) : bool {
+            return strpos($sql, sprintf('CREATE TABLE %s (', $tableName)) === 0;
+        });
+    }
+
+    public function testUpdateSchemaSql() : void
+    {
+        $classes = [$this->_em->getClassMetadata(GH7875MyEntity::class)];
+
+        $tool = new SchemaTool($this->_em);
+        $sqls = $this->filterCreateTable($tool->getUpdateSchemaSql($classes), 'gh7875_my_entity');
+
+        self::assertCount(1, $sqls);
+
+        $this->_em->getConnection()->exec(current($sqls));
+
+        $sqls = array_filter($tool->getUpdateSchemaSql($classes), static function (string $sql) : bool {
+            return strpos($sql, ' gh7875_my_entity ') !== false;
+        });
+
+        self::assertSame([], $sqls);
+
+        $classes[] = $this->_em->getClassMetadata(GH7875MyOtherEntity::class);
+
+        $sqls = $tool->getUpdateSchemaSql($classes);
+
+        self::assertCount(0, $this->filterCreateTable($sqls, 'gh7875_my_entity'));
+        self::assertCount(1, $this->filterCreateTable($sqls, 'gh7875_my_other_entity'));
+    }
+
+    /**
+     * @return array<array<string|callable|null>>
+     */
+    public function provideUpdateSchemaSqlWithSchemaAssetFilter() : array
+    {
+        return [
+            ['/^(?!my_enti)/', null],
+            [
+                null,
+                static function ($assetName) : bool {
+                    return $assetName !== 'gh7875_my_entity';
+                },
+            ],
+        ];
+    }
+
+    /** @dataProvider provideUpdateSchemaSqlWithSchemaAssetFilter */
+    public function testUpdateSchemaSqlWithSchemaAssetFilter(?string $filterRegex, ?callable $filterCallback) : void
+    {
+        if ($filterRegex && ! method_exists(Configuration::class, 'setFilterSchemaAssetsExpression')) {
+            self::markTestSkipped(sprintf('Test require %s::setFilterSchemaAssetsExpression method', Configuration::class));
+        }
+
+        $classes = [$this->_em->getClassMetadata(GH7875MyEntity::class)];
+
+        $tool = new SchemaTool($this->_em);
+        $tool->createSchema($classes);
+
+        $config = $this->_em->getConnection()->getConfiguration();
+        if ($filterRegex) {
+            $config->setFilterSchemaAssetsExpression($filterRegex);
+        } else {
+            $config->setSchemaAssetsFilter($filterCallback);
+        }
+
+        $previousFilter = $config->getSchemaAssetsFilter();
+
+        $sqls = $tool->getUpdateSchemaSql($classes);
+        $sqls = array_filter($sqls, static function (string $sql) : bool {
+            return strpos($sql, ' gh7875_my_entity ') !== false;
+        });
+
+        self::assertCount(0, $sqls);
+        self::assertSame($previousFilter, $config->getSchemaAssetsFilter());
+    }
+}
+
+/**
+ * @Entity
+ * @Table(name="gh7875_my_entity")
+ */
+class GH7875MyEntity
+{
+    /** @Id @Column(type="integer") @GeneratedValue(strategy="AUTO") */
+    public $id;
+}
+
+/**
+ * @Entity
+ * @Table(name="gh7875_my_other_entity")
+ */
+class GH7875MyOtherEntity
+{
+    /** @Id @Column(type="integer") @GeneratedValue(strategy="AUTO") */
+    public $id;
+}