Filter out extension-related operations in creation scripts of history table (#3713)

Fixes #3496

Co-authored-by: Shay Rojansky <roji@roji.org>
(cherry picked from commit 8237cba)

Author: dkottis

src/EFCore.PG/Migrations/Internal/NpgsqlHistoryRepository.cs

@@ -1,4 +1,5 @@
 using Npgsql.EntityFrameworkCore.PostgreSQL.Metadata.Conventions;
+using Npgsql.EntityFrameworkCore.PostgreSQL.Metadata.Internal;
 
 namespace Npgsql.EntityFrameworkCore.PostgreSQL.Migrations.Internal;
 
@@ -101,14 +102,40 @@ protected override IReadOnlyList<MigrationCommand> GetCreateCommands()
     {
         // TODO: This is all a hack around https://github.com/dotnet/efcore/issues/34991: we have provider-specific conventions which add
         // enums and extensions to the model, and the default EF logic causes them to be created at this point, when the history table is
-        // being created.
+        // being created. This causes problems when:
+        // (a) we create an enum here when creating the history table, and then try to create it again when the actual migration
+        //     runs (#3324), and
+        // (b) we shouldn't be creating extensions at this early point either, and doing so can cause issues (e.g. #3496).
+        //
+        // So we filter out any extension/enum migration operations.
+        // Note that the approach in EF is to remove specific conventions (e.g. DbSetFindingConvention), but we don't want to hardcode
+        // specific conventions here; for example, the NetTopologySuite plugin has its NpgsqlNetTopologySuiteExtensionAddingConvention
+        // which adds PostGIS. So we just filter out the annotations on the operations themselves.
         var model = EnsureModel();
 
         var operations = Dependencies.ModelDiffer.GetDifferences(null, model.GetRelationalModel());
-        var commandList = Dependencies.MigrationsSqlGenerator.Generate(operations, model);
-        return commandList;
+
+        foreach (var operation in operations)
+        {
+            if (operation is not AlterDatabaseOperation alterDatabaseOperation)
+            {
+                continue;
+            }
+
+            foreach (var annotation in alterDatabaseOperation.GetAnnotations())
+            {
+                if (annotation.Name.StartsWith(NpgsqlAnnotationNames.PostgresExtensionPrefix, StringComparison.Ordinal)
+                    || annotation.Name.StartsWith(NpgsqlAnnotationNames.EnumPrefix, StringComparison.Ordinal))
+                {
+                    alterDatabaseOperation.RemoveAnnotation(annotation.Name);
+                }
+            }
+        }
+
+        return Dependencies.MigrationsSqlGenerator.Generate(operations, model);
     }
 
+    // Copied as-is from EF's HistoryRepository, since it's private (see https://github.com/dotnet/efcore/issues/34991)
     private IModel EnsureModel()
     {
         if (_model == null)
@@ -117,16 +144,13 @@ private IModel EnsureModel()
 
             conventionSet.Remove(typeof(DbSetFindingConvention));
             conventionSet.Remove(typeof(RelationalDbFunctionAttributeConvention));
-            // TODO: this whole method exists only so we can remove this convention (https://github.com/dotnet/efcore/issues/34991)
-            conventionSet.Remove(typeof(NpgsqlPostgresModelFinalizingConvention));
 
             var modelBuilder = new ModelBuilder(conventionSet);
-            modelBuilder.Entity<HistoryRow>(
-                x =>
-                {
-                    ConfigureTable(x);
-                    x.ToTable(TableName, TableSchema);
-                });
+            modelBuilder.Entity<HistoryRow>(x =>
+            {
+                ConfigureTable(x);
+                x.ToTable(TableName, TableSchema);
+            });
 
             _model = Dependencies.ModelRuntimeInitializer.Initialize(
                 (IModel)modelBuilder.Model, designTime: true, validationLogger: null);

test/EFCore.PG.Tests/Migrations/NpgsqlHistoryRepositoryTest.cs

@@ -77,6 +77,73 @@ IF NOT EXISTS(SELECT 1 FROM pg_namespace WHERE nspname = 'my') THEN
     CONSTRAINT "PK___EFMigrationsHistory" PRIMARY KEY ("MigrationId")
 );
 
+""", sql, ignoreLineEndingDifferences: true);
+    }
+
+    [ConditionalFact]
+    public void GetCreateIfNotExistsScript_does_not_include_extensions()
+    {
+        var historyRepository = new TestDbContext(
+                new DbContextOptionsBuilder()
+                    .UseInternalServiceProvider(
+                        NpgsqlTestHelpers.Instance.CreateServiceProvider(
+                            new ServiceCollection().AddEntityFrameworkNpgsqlNetTopologySuite()))
+                    .UseNpgsql(
+                        new NpgsqlConnection("Host=localhost;Database=DummyDatabase"),
+                        b => b.MigrationsHistoryTable(HistoryRepository.DefaultTableName, "some_schema")
+                            .UseNetTopologySuite())
+                    .Options)
+            .GetService<IHistoryRepository>();;
+
+        var sql = historyRepository.GetCreateIfNotExistsScript();
+
+        Assert.Equal(
+            """
+DO $EF$
+BEGIN
+    IF NOT EXISTS(SELECT 1 FROM pg_namespace WHERE nspname = 'some_schema') THEN
+        CREATE SCHEMA some_schema;
+    END IF;
+END $EF$;
+CREATE TABLE IF NOT EXISTS some_schema."__EFMigrationsHistory" (
+    "MigrationId" character varying(150) NOT NULL,
+    "ProductVersion" character varying(32) NOT NULL,
+    CONSTRAINT "PK___EFMigrationsHistory" PRIMARY KEY ("MigrationId")
+);
+
+""", sql, ignoreLineEndingDifferences: true);
+    }
+
+    enum TestEnum { Value1, Value2 }
+
+    [ConditionalFact]
+    public void GetCreateIfNotExistsScript_does_not_include_enums()
+    {
+        var historyRepository = new TestDbContext(
+                new DbContextOptionsBuilder()
+                    .UseNpgsql(
+                        new NpgsqlConnection("Host=localhost;Database=DummyDatabase"),
+                        b => b.MigrationsHistoryTable(HistoryRepository.DefaultTableName, "some_schema")
+                            .MapEnum<TestEnum>("test_enum"))
+                    .Options)
+            .GetService<IHistoryRepository>();;
+
+        var sql = historyRepository.GetCreateIfNotExistsScript();
+
+        Assert.Equal(
+            """
+DO $EF$
+BEGIN
+    IF NOT EXISTS(SELECT 1 FROM pg_namespace WHERE nspname = 'some_schema') THEN
+        CREATE SCHEMA some_schema;
+    END IF;
+END $EF$;
+CREATE TABLE IF NOT EXISTS some_schema."__EFMigrationsHistory" (
+    "MigrationId" character varying(150) NOT NULL,
+    "ProductVersion" character varying(32) NOT NULL,
+    CONSTRAINT "PK___EFMigrationsHistory" PRIMARY KEY ("MigrationId")
+);
+
 """, sql, ignoreLineEndingDifferences: true);
     }