Fix bug where annotations added by initial migration are not removed when migration is reverted. (#3714)

Adds NpgsqlMigrationsAnnotationProvider to implement ForRemove(IRelationalModel).

Fixes #3604

Author: brianpursley

src/EFCore.PG/Extensions/NpgsqlServiceCollectionExtensions.cs

@@ -94,6 +94,7 @@ public static IServiceCollection AddEntityFrameworkNpgsql(this IServiceCollectio
             .TryAdd<IRelationalTypeMappingSource, NpgsqlTypeMappingSource>()
             .TryAdd<ISqlGenerationHelper, NpgsqlSqlGenerationHelper>()
             .TryAdd<IRelationalAnnotationProvider, NpgsqlAnnotationProvider>()
+            .TryAdd<IMigrationsAnnotationProvider, NpgsqlMigrationsAnnotationProvider>()
             .TryAdd<IModelValidator, NpgsqlModelValidator>()
             .TryAdd<IMigrator, NpgsqlMigrator>()
             .TryAdd<IProviderConventionSetBuilder, NpgsqlConventionSetBuilder>()

src/EFCore.PG/Metadata/Internal/NpgsqlAnnotationHelper.cs

@@ -0,0 +1,10 @@
+namespace Npgsql.EntityFrameworkCore.PostgreSQL.Metadata.Internal;
+
+internal static class NpgsqlAnnotationHelper
+{
+    internal static bool IsRelationalModelAnnotation(IAnnotation annotation)
+        => annotation.Name.StartsWith(NpgsqlAnnotationNames.PostgresExtensionPrefix, StringComparison.Ordinal)
+            || annotation.Name.StartsWith(NpgsqlAnnotationNames.EnumPrefix, StringComparison.Ordinal)
+            || annotation.Name.StartsWith(NpgsqlAnnotationNames.RangePrefix, StringComparison.Ordinal)
+            || annotation.Name.StartsWith(NpgsqlAnnotationNames.CollationDefinitionPrefix, StringComparison.Ordinal);
+}

src/EFCore.PG/Metadata/Internal/NpgsqlAnnotationProvider.cs

@@ -257,11 +257,6 @@ public override IEnumerable<IAnnotation> For(IRelationalModel model, bool design
             return [];
         }
 
-        return model.Model.GetAnnotations().Where(
-            a =>
-                a.Name.StartsWith(NpgsqlAnnotationNames.PostgresExtensionPrefix, StringComparison.Ordinal)
-                || a.Name.StartsWith(NpgsqlAnnotationNames.EnumPrefix, StringComparison.Ordinal)
-                || a.Name.StartsWith(NpgsqlAnnotationNames.RangePrefix, StringComparison.Ordinal)
-                || a.Name.StartsWith(NpgsqlAnnotationNames.CollationDefinitionPrefix, StringComparison.Ordinal));
+        return model.Model.GetAnnotations().Where(NpgsqlAnnotationHelper.IsRelationalModelAnnotation);
     }
 }

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

@@ -0,0 +1,34 @@
+using Npgsql.EntityFrameworkCore.PostgreSQL.Metadata.Internal;
+
+namespace Npgsql.EntityFrameworkCore.PostgreSQL.Migrations.Internal;
+
+/// <summary>
+///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+///     any release. You should only use it directly in your code with extreme caution and knowing that
+///     doing so can result in application failures when updating to a new Entity Framework Core release.
+/// </summary>
+public class NpgsqlMigrationsAnnotationProvider : MigrationsAnnotationProvider
+{
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+#pragma warning disable EF1001 // Internal EF Core API usage.
+    public NpgsqlMigrationsAnnotationProvider(MigrationsAnnotationProviderDependencies dependencies)
+#pragma warning restore EF1001
+        : base(dependencies)
+    {
+    }
+
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    public override IEnumerable<IAnnotation> ForRemove(IRelationalModel model)
+        => model.Model.GetAnnotations().Where(NpgsqlAnnotationHelper.IsRelationalModelAnnotation);
+}

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

@@ -0,0 +1,50 @@
+using Microsoft.EntityFrameworkCore.Metadata.Internal;
+using Npgsql.EntityFrameworkCore.PostgreSQL.Metadata.Internal;
+using Npgsql.EntityFrameworkCore.PostgreSQL.Migrations.Internal;
+
+namespace Npgsql.EntityFrameworkCore.PostgreSQL.Migrations;
+
+public class NpgsqlMigrationsAnnotationProviderTest
+{
+    private readonly NpgsqlMigrationsAnnotationProvider _migrationsAnnotationProvider = new(new MigrationsAnnotationProviderDependencies());
+
+    [Fact]
+    public virtual void ForRemove_returns_relational_model_annotations()
+    {
+        var modelBuilder = new ModelBuilder()
+            .HasPostgresExtension("test_extension")
+            .HasPostgresExtension("test_schema2", "test_extension2")
+            .HasPostgresEnum("test_enum", ["A", "B", "C"])
+            .HasPostgresEnum("test_schema2", "test_enum2", ["A", "B", "C"])
+            .HasPostgresRange("test_range", "test_subtype")
+            .HasPostgresRange("test_schema2", "test_range2", "test_subtype2")
+            .HasCollation("test_collation", "test_locale")
+            .HasCollation("test_schema2", "test_collation2", "test_locale2", "test_provider2", false);
+
+        // Define a sequence, so we can verify that the annotation it creates is excluded from the ForRemove() result.
+        modelBuilder.HasSequence("test_sequence");
+
+        var model = new RelationalModel(modelBuilder.FinalizeModel());
+        var allAnnotations = model.Model.GetAnnotations().ToList();
+
+        var annotations = _migrationsAnnotationProvider.ForRemove(model).ToList();
+
+        Assert.Equal(9, allAnnotations.Count);
+        Assert.Equal(8, annotations.Count);
+        Assert.Contains(annotations, a => a.Name == $"{NpgsqlAnnotationNames.PostgresExtensionPrefix}test_extension");
+        Assert.Contains(annotations, a => a.Name == $"{NpgsqlAnnotationNames.PostgresExtensionPrefix}test_schema2.test_extension2");
+        Assert.Contains(annotations, a => a.Name == $"{NpgsqlAnnotationNames.EnumPrefix}test_enum");
+        Assert.Contains(annotations, a => a.Name == $"{NpgsqlAnnotationNames.EnumPrefix}test_schema2.test_enum2");
+        Assert.Contains(annotations, a => a.Name == $"{NpgsqlAnnotationNames.RangePrefix}test_range");
+        Assert.Contains(annotations, a => a.Name == $"{NpgsqlAnnotationNames.RangePrefix}test_schema2.test_range2");
+        Assert.Contains(annotations, a => a.Name == $"{NpgsqlAnnotationNames.CollationDefinitionPrefix}test_collation");
+        Assert.Contains(annotations, a => a.Name == $"{NpgsqlAnnotationNames.CollationDefinitionPrefix}test_schema2.test_collation2");
+    }
+
+    [Fact]
+    public virtual void ForRemove_handles_no_annotations()
+    {
+        var model = new RelationalModel(new Model());
+        Assert.Empty(_migrationsAnnotationProvider.ForRemove(model));
+    }
+}