DiffGenerator: align generated diff with ORM SchemaTool

Author: cristi-contiu

phpstan.neon.dist

@@ -34,10 +34,6 @@ parameters:
             message: '~^Call to an undefined method Doctrine\\DBAL\\Connection\:\:getEventManager\(\)\.$~'
             path: src/DependencyFactory.php
 
-        -
-            message: '~^Strict comparison using !== between callable\(\)\: mixed and null will always evaluate to true\.$~'
-            path: src/Generator/DiffGenerator.php
-
         -
             message: '~Doctrine\\ORM\\Tools\\Console\\Helper\\EntityManagerHelper~'
             path: src/Tools/Console/ConsoleRunner.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
@@ -46,14 +44,22 @@ public function generate(
         bool $checkDbPlatform = true,
         bool $fromEmptySchema = false,
     ): string {
+        $toSchema = $this->createToSchema();
+
         if ($filterExpression !== null) {
+            // whitelist assets we already know about in $toSchema, use the existing $filterExpression otherwise
+            // @see https://github.com/doctrine/orm/pull/7875
             $this->dbalConfiguration->setSchemaAssetsFilter(
-                static function ($assetName) use ($filterExpression) {
+                static function ($assetName) use ($filterExpression, $toSchema): bool {
                     if ($assetName instanceof AbstractAsset) {
                         $assetName = $assetName->getName();
                     }
 
-                    return preg_match($filterExpression, $assetName);
+                    if ($toSchema->hasTable($assetName) || $toSchema->hasSequence($assetName)) {
+                        return true;
+                    }
+
+                    return (bool) preg_match($filterExpression, $assetName);
                 },
             );
         }
@@ -62,8 +68,6 @@ static function ($assetName) use ($filterExpression) {
             ? $this->createEmptySchema()
             : $this->createFromSchema();
 
-        $toSchema = $this->createToSchema();
-
         // 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
         if (
@@ -119,35 +123,6 @@ private function createFromSchema(): Schema
 
     private function createToSchema(): Schema
     {
-        $toSchema = $this->schemaProvider->createSchema();
-
-        $schemaAssetsFilter = $this->dbalConfiguration->getSchemaAssetsFilter();
-
-        if ($schemaAssetsFilter !== null) {
-            foreach ($toSchema->getTables() as $table) {
-                $tableName = $table->getName();
-
-                if ($schemaAssetsFilter($this->resolveTableName($tableName))) {
-                    continue;
-                }
-
-                $toSchema->dropTable($tableName);
-            }
-        }
-
-        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
-    {
-        $pos = strpos($name, '.');
-
-        return $pos === false ? $name : substr($name, $pos + 1);
+        return $this->schemaProvider->createSchema();
     }
 }

tests/Generator/DiffGeneratorTest.php

@@ -38,32 +38,11 @@ public function testGenerate(): void
         $toSchema   = $this->createMock(Schema::class);
 
         $this->dbalConfiguration->expects(self::once())
-            ->method('setSchemaAssetsFilter');
-
-        $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]);
+            ->method('setSchemaAssetsFilter')
+            ->willReturnCallback(static function (callable $schemaAssetsFilter): void {
+                // also test the code inside the closures provided to setSchemaAssetsFilter
+                self::assertTrue($schemaAssetsFilter('table_name1'));
+            });
 
         $this->emptySchemaProvider->expects(self::never())
             ->method('createSchema');
@@ -76,10 +55,6 @@ public function testGenerate(): void
             ->method('createSchema')
             ->willReturn($toSchema);
 
-        $toSchema->expects(self::exactly(2))
-            ->method('dropTable')
-            ->willReturnSelf();
-
         $schemaDiff = self::createStub(SchemaDiff::class);
 
         $this->platform->method('getAlterSchemaSQL')->willReturnCallback(static function (): array {
@@ -126,10 +101,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')]);