DiffGenerator: align generated diff with ORM SchemaTool

Author: cristi-contiu

phpstan.neon.dist

@@ -35,7 +35,7 @@ parameters:
             path: src/DependencyFactory.php
 
         -
-            message: '~^Strict comparison using !== between callable\(\)\: mixed and null will always evaluate to true\.$~'
+            message: '~^Strict comparison using === between callable\(\)\: mixed and null will always evaluate to false\.$~'
             path: src/Generator/DiffGenerator.php
 
         -

src/Generator/DiffGenerator.php

@@ -14,8 +14,6 @@
 
 use function method_exists;
 use function preg_match;
-use function strpos;
-use function substr;
 
 /**
  * The DiffGenerator class is responsible for comparing two Doctrine\DBAL\Schema\Schema instances and generating a
@@ -53,16 +51,16 @@ static function ($assetName) use ($filterExpression) {
                         $assetName = $assetName->getName();
                     }
 
-                    return preg_match($filterExpression, $assetName);
+                    return (bool) preg_match($filterExpression, $assetName);
                 },
             );
         }
 
+        $toSchema = $this->createToSchema();
+
         $fromSchema = $fromEmptySchema
             ? $this->createEmptySchema()
-            : $this->createFromSchema();
-
-        $toSchema = $this->createToSchema();
+            : $this->createFromSchema($toSchema);
 
         // prior to DBAL 4.0, the schema name was set to the first element in the search path,
         // which is not necessarily the default schema name
@@ -112,42 +110,37 @@ private function createEmptySchema(): Schema
         return $this->emptySchemaProvider->createSchema();
     }
 
-    private function createFromSchema(): Schema
-    {
-        return $this->schemaManager->introspectSchema();
-    }
-
-    private function createToSchema(): Schema
+    /**
+     * Creates the schema from the database, ensuring tables from the target schema are whitelisted for comparison.
+     *
+     * @see https://github.com/doctrine/orm/pull/7875
+     */
+    private function createFromSchema(Schema $toSchema): Schema
     {
-        $toSchema = $this->schemaProvider->createSchema();
+        // backup schema assets filter
+        $previousFilter = $this->dbalConfiguration->getSchemaAssetsFilter();
 
-        $schemaAssetsFilter = $this->dbalConfiguration->getSchemaAssetsFilter();
+        if ($previousFilter === null) {
+            return $this->schemaManager->introspectSchema();
+        }
 
-        if ($schemaAssetsFilter !== null) {
-            foreach ($toSchema->getTables() as $table) {
-                $tableName = $table->getName();
+        // whitelist assets we already know about in $toSchema, use the existing filter otherwise
+        $this->dbalConfiguration->setSchemaAssetsFilter(static function ($asset) use ($previousFilter, $toSchema): bool {
+            $assetName = $asset instanceof AbstractAsset ? $asset->getName() : $asset;
 
-                if ($schemaAssetsFilter($this->resolveTableName($tableName))) {
-                    continue;
-                }
+            return $toSchema->hasTable($assetName) || $toSchema->hasSequence($assetName) || $previousFilter($asset);
+        });
 
-                $toSchema->dropTable($tableName);
-            }
+        try {
+            return $this->schemaManager->introspectSchema();
+        } finally {
+            // restore schema assets filter
+            $this->dbalConfiguration->setSchemaAssetsFilter($previousFilter);
         }
-
-        return $toSchema;
     }
 
-    /**
-     * Resolve a table name from its fully qualified name. The `$name` argument
-     * comes from Doctrine\DBAL\Schema\Table#getName which can sometimes return
-     * a namespaced name with the form `{namespace}.{tableName}`. This extracts
-     * the table name from that.
-     */
-    private function resolveTableName(string $name): string
+    private function createToSchema(): Schema
     {
-        $pos = strpos($name, '.');
-
-        return $pos === false ? $name : substr($name, $pos + 1);
+        return $this->schemaProvider->createSchema();
     }
 }

tests/Generator/DiffGeneratorTest.php

@@ -37,34 +37,21 @@ public function testGenerate(): void
         $fromSchema = $this->createMock(Schema::class);
         $toSchema   = $this->createMock(Schema::class);
 
-        $this->dbalConfiguration->expects(self::once())
-            ->method('setSchemaAssetsFilter');
+        // first time sets a regex check using the provided $filterExpression pattern
+        // second time sets a function wrapping the initial one so it does not remove assets provided in $toSchema
+        // third time reverts to the initial regex-only check
+        $this->dbalConfiguration->expects(self::exactly(3))
+            ->method('setSchemaAssetsFilter')
+            ->willReturnCallback(function (callable $newSchemaAssetsFilter) {
+                self::assertTrue($newSchemaAssetsFilter('table_name1'));
+            });
 
         $this->dbalConfiguration->expects(self::once())
             ->method('getSchemaAssetsFilter')
             ->willReturn(
                 static fn ($name): bool => $name === 'table_name1',
             );
 
-        $table1 = $this->createMock(Table::class);
-        $table1->expects(self::once())
-            ->method('getName')
-            ->willReturn('schema.table_name1');
-
-        $table2 = $this->createMock(Table::class);
-        $table2->expects(self::once())
-            ->method('getName')
-            ->willReturn('schema.table_name2');
-
-        $table3 = $this->createMock(Table::class);
-        $table3->expects(self::once())
-            ->method('getName')
-            ->willReturn('schema.table_name3');
-
-        $toSchema->expects(self::once())
-            ->method('getTables')
-            ->willReturn([$table1, $table2, $table3]);
-
         $this->emptySchemaProvider->expects(self::never())
             ->method('createSchema');
 
@@ -76,9 +63,13 @@ public function testGenerate(): void
             ->method('createSchema')
             ->willReturn($toSchema);
 
-        $toSchema->expects(self::exactly(2))
-            ->method('dropTable')
-            ->willReturnSelf();
+        $toSchema->expects(self::once())
+            ->method('hasTable')
+            ->willReturn(false);
+
+        $toSchema->expects(self::once())
+            ->method('hasSequence')
+            ->willReturn(false);
 
         $schemaDiff = self::createStub(SchemaDiff::class);
 
@@ -126,10 +117,6 @@ public function testGenerateFromEmptySchema(): void
         $this->dbalConfiguration->expects(self::never())
             ->method('setSchemaAssetsFilter');
 
-        $this->dbalConfiguration->expects(self::once())
-            ->method('getSchemaAssetsFilter')
-            ->willReturn(static fn () => true);
-
         $toSchema->method('getTables')
             ->willReturn([new Table('table_name')]);