Upgrade the snapshot when rolling back to a migration from a previous version (#32690)

AndriySvyryd · web-flow · commit 9fdf1c433ea1 · 2024-01-04T10:19:42.000-08:00
Fixes #32555
diff --git a/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs b/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs
@@ -124,13 +124,18 @@ protected virtual void GenerateEntityType(
     {
         var ownership = entityType.FindOwnership();
         var ownerNavigation = ownership?.PrincipalToDependent!.Name;
-
         var entityTypeName = entityType.Name;
         if (ownerNavigation != null
-            && entityType.HasSharedClrType
-            && entityTypeName == ownership!.PrincipalEntityType.GetOwnedName(entityType.ClrType.ShortDisplayName(), ownerNavigation))
+            && entityType.HasSharedClrType)
         {
-            entityTypeName = entityType.ClrType.DisplayName();
+            if (entityTypeName == ownership!.PrincipalEntityType.GetOwnedName(entityType.ClrType.ShortDisplayName(), ownerNavigation))
+            {
+                entityTypeName = entityType.ClrType.DisplayName();
+            }
+            else if (entityTypeName == ownership!.PrincipalEntityType.GetOwnedName(entityType.ShortName(), ownerNavigation))
+            {
+                entityTypeName = entityType.ShortName();
+            }
         }
 
         var entityTypeBuilderName = GenerateNestedBuilderName(builderName);
@@ -436,8 +441,8 @@ protected virtual void GenerateProperty(
         IProperty property,
         IndentedStringBuilder stringBuilder)
     {
-        var clrType = FindValueConverter(property)?.ProviderClrType.MakeNullable(property.IsNullable)
-            ?? property.ClrType;
+        var clrType = (FindValueConverter(property)?.ProviderClrType ?? property.ClrType)
+            .MakeNullable(property.IsNullable);
 
         var propertyBuilderName = $"{entityTypeBuilderName}.Property<{Code.Reference(clrType)}>({Code.Literal(property.Name)})";
 
diff --git a/src/EFCore.Design/Migrations/Design/MigrationsCodeGenerator.cs b/src/EFCore.Design/Migrations/Design/MigrationsCodeGenerator.cs
@@ -196,13 +196,36 @@ private static IEnumerable<IAnnotatable> GetAnnotatables(IModel model)
             foreach (var property in entityType.GetDeclaredProperties())
             {
                 yield return property;
+
+                foreach (var @override in property.GetOverrides())
+                {
+                    yield return @override;
+                }
+            }
+
+            foreach (var property in entityType.GetDeclaredComplexProperties())
+            {
+                foreach (var annotatable in GetAnnotatables(property))
+                {
+                    yield return annotatable;
+                }
             }
 
             foreach (var key in entityType.GetDeclaredKeys())
             {
                 yield return key;
             }
 
+            foreach (var navigation in entityType.GetDeclaredNavigations())
+            {
+                yield return navigation;
+            }
+
+            foreach (var navigation in entityType.GetDeclaredSkipNavigations())
+            {
+                yield return navigation;
+            }
+
             foreach (var foreignKey in entityType.GetDeclaredForeignKeys())
             {
                 yield return foreignKey;
@@ -212,6 +235,45 @@ private static IEnumerable<IAnnotatable> GetAnnotatables(IModel model)
             {
                 yield return index;
             }
+
+            foreach (var checkConstraint in entityType.GetDeclaredCheckConstraints())
+            {
+                yield return checkConstraint;
+            }
+
+            foreach (var trigger in entityType.GetDeclaredTriggers())
+            {
+                yield return trigger;
+            }
+
+            foreach (var fragment in entityType.GetMappingFragments())
+            {
+                yield return fragment;
+            }
+        }
+
+        foreach (var sequence in model.GetSequences())
+        {
+            yield return sequence;
+        }
+    }
+
+    private static IEnumerable<IAnnotatable> GetAnnotatables(IComplexProperty complexProperty)
+    {
+        yield return complexProperty;
+        yield return complexProperty.ComplexType;
+
+        foreach (var property in complexProperty.ComplexType.GetDeclaredProperties())
+        {
+            yield return property;
+        }
+
+        foreach (var property in complexProperty.ComplexType.GetDeclaredComplexProperties())
+        {
+            foreach (var annotatable in GetAnnotatables(property))
+            {
+                yield return annotatable;
+            }
         }
     }
 
diff --git a/src/EFCore.Design/Migrations/Design/MigrationsScaffolder.cs b/src/EFCore.Design/Migrations/Design/MigrationsScaffolder.cs
@@ -319,7 +319,7 @@ public virtual MigrationFiles RemoveMigration(
                 }
 
                 model = migrations.Count > 1
-                    ? Dependencies.SnapshotModelProcessor.Process(migrations[^2].TargetModel)
+                    ? Dependencies.SnapshotModelProcessor.Process(migrations[^2].TargetModel, resetVersion: true)
                     : null;
             }
             else
@@ -351,6 +351,7 @@ public virtual MigrationFiles RemoveMigration(
         {
             var modelSnapshotNamespace = modelSnapshot.GetType().Namespace;
             Check.DebugAssert(!string.IsNullOrEmpty(modelSnapshotNamespace), "modelSnapshotNamespace is null or empty");
+
             var modelSnapshotCode = codeGenerator.GenerateSnapshot(
                 modelSnapshotNamespace,
                 _contextType,
diff --git a/src/EFCore.Design/Migrations/Internal/ISnapshotModelProcessor.cs b/src/EFCore.Design/Migrations/Internal/ISnapshotModelProcessor.cs
@@ -20,5 +20,5 @@ public interface ISnapshotModelProcessor
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
     [return: NotNullIfNotNull("model")]
-    IModel? Process(IReadOnlyModel? model);
+    IModel? Process(IReadOnlyModel? model, bool resetVersion = false);
 }
diff --git a/src/EFCore.Design/Migrations/Internal/SnapshotModelProcessor.cs b/src/EFCore.Design/Migrations/Internal/SnapshotModelProcessor.cs
@@ -47,7 +47,7 @@ public SnapshotModelProcessor(
     ///     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 virtual IModel? Process(IReadOnlyModel? model)
+    public virtual IModel? Process(IReadOnlyModel? model, bool resetVersion = false)
     {
         if (model == null)
         {
@@ -76,6 +76,15 @@ public SnapshotModelProcessor(
             }
         }
 
+        if (model is IMutableModel mutableModel)
+        {
+            mutableModel.RemoveAnnotation("ChangeDetector.SkipDetectChanges");
+            if (resetVersion)
+            {
+                mutableModel.SetProductVersion(ProductInfo.GetVersion());
+            }
+        }
+
         return _modelRuntimeInitializer.Initialize((IModel)model, designTime: true, validationLogger: null);
     }
 
@@ -145,13 +154,17 @@ private static void UpdateSequences(IReadOnlyModel model, string version)
         var sequences = model.GetAnnotations()
 #pragma warning disable CS0618 // Type or member is obsolete
             .Where(a => a.Name.StartsWith(RelationalAnnotationNames.SequencePrefix, StringComparison.Ordinal))
-            .Select(a => new Sequence(model, a.Name));
+            .ToList();
 #pragma warning restore CS0618 // Type or member is obsolete
 
         var sequencesDictionary = new Dictionary<(string, string?), ISequence>();
-        foreach (var sequence in sequences)
+        foreach (var sequenceAnnotation in sequences)
         {
+#pragma warning disable CS0618 // Type or member is obsolete
+            var sequence = new Sequence(model, sequenceAnnotation.Name);
+#pragma warning restore CS0618 // Type or member is obsolete
             sequencesDictionary[(sequence.Name, sequence.ModelSchema)] = sequence;
+            mutableModel.RemoveAnnotation(sequenceAnnotation.Name);
         }
 
         if (sequencesDictionary.Count > 0)
diff --git a/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs b/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs
@@ -3,7 +3,6 @@
 
 using System.Collections;
 using System.Globalization;
-using Microsoft.EntityFrameworkCore.Metadata;
 using Microsoft.EntityFrameworkCore.Metadata.Internal;
 using Microsoft.EntityFrameworkCore.Update.Internal;
 
diff --git a/src/Shared/SharedTypeExtensions.cs b/src/Shared/SharedTypeExtensions.cs
@@ -597,7 +597,18 @@ public static IEnumerable<string> GetNamespaces(this Type type)
             yield break;
         }
 
-        yield return type.Namespace!;
+        if (type.IsConstructedGenericType
+            && type.GetGenericTypeDefinition() == typeof(Nullable<>))
+        {
+            foreach (var ns in type.UnwrapNullableType().GetNamespaces())
+            {
+                yield return ns;
+            }
+        }
+        else
+        {
+            yield return type.Namespace!;
+        }
 
         if (type.IsGenericType)
         {
diff --git a/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.ModelSnapshot.cs b/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.ModelSnapshot.cs
@@ -1062,7 +1062,6 @@ public virtual void Non_base_abstract_base_class_with_TPC()
             },
             """
 // <auto-generated />
-using System;
 using Microsoft.EntityFrameworkCore;
 using Microsoft.EntityFrameworkCore.Infrastructure;
 using Microsoft.EntityFrameworkCore.Metadata;
@@ -3557,7 +3556,7 @@ public virtual void Owned_types_are_stored_in_snapshot()
 
                     b.Navigation("Properties");
                 });
-""", usingSystem: true),
+"""),
             o =>
             {
                 var entityWithOneProperty = o.FindEntityType(typeof(EntityWithOneProperty));
@@ -3793,7 +3792,7 @@ public virtual void Owned_types_are_stored_in_snapshot_when_excluded()
 
                     b.Navigation("Properties");
                 });
-""", usingSystem: true),
+"""),
             o =>
             {
                 var entityWithOneProperty = o.FindEntityType(typeof(EntityWithOneProperty));
@@ -4881,7 +4880,7 @@ public virtual void Property_column_name_is_stored_in_snapshot_when_DefaultColum
                 {
                     b.Navigation("Bar");
                 });
-""", usingSystem: true),
+"""),
             model =>
             {
                 var entityType = model.FindEntityType(typeof(BarA).FullName);
@@ -5345,7 +5344,7 @@ public virtual void Property_of_enum_to_nullable()
 
                     b.ToTable("EntityWithEnumType", "DefaultSchema");
                 });
-""", usingSystem: true),
+"""),
             o => Assert.False(o.GetEntityTypes().First().FindProperty("Day").IsNullable));
 
     [ConditionalFact]
@@ -7214,7 +7213,7 @@ public virtual void Do_not_generate_entity_type_builder_again_if_no_foreign_key_
 
                     b.Navigation("Navigation");
                 });
-""", usingSystem: true),
+"""),
             o => { });
 
     [ConditionalFact]
diff --git a/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.cs b/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.cs
@@ -905,7 +905,7 @@ public void Multidimensional_array_warning_is_not_suppressed_for_unidimensional_
         Assert.DoesNotContain("#pragma warning disable CA1814", migration);
     }
 
-    private static IMigrationsCodeGenerator CreateMigrationsCodeGenerator()
+    public static IMigrationsCodeGenerator CreateMigrationsCodeGenerator()
     {
         var testAssembly = typeof(CSharpMigrationsGeneratorTest).Assembly;
         var reporter = new TestOperationReporter();
diff --git a/test/EFCore.Design.Tests/Migrations/Design/SnapshotModelProcessorTest.cs b/test/EFCore.Design.Tests/Migrations/Design/SnapshotModelProcessorTest.cs
@@ -191,19 +191,7 @@ public void Sets_owned_type_keys()
         public void Can_diff_against_older_ownership_model(Type snapshotType)
         {
             using var context = new OwnershipContext();
-            var differ = context.GetService<IMigrationsModelDiffer>();
-            var snapshot = (ModelSnapshot)Activator.CreateInstance(snapshotType);
-            var reporter = new TestOperationReporter();
-            var modelRuntimeInitializer =
-                SqlServerTestHelpers.Instance.CreateContextServices().GetRequiredService<IModelRuntimeInitializer>();
-            var processor = new SnapshotModelProcessor(reporter, modelRuntimeInitializer);
-            var model = processor.Process(snapshot.Model);
-
-            var differences = differ.GetDifferences(
-                model.GetRelationalModel(),
-                context.GetService<IDesignTimeModel>().Model.GetRelationalModel());
-
-            Assert.Empty(differences);
+            AssertSameSnapshot(snapshotType, context);
         }
 
         [ConditionalTheory]
@@ -213,17 +201,84 @@ public void Can_diff_against_older_ownership_model(Type snapshotType)
         public void Can_diff_against_older_sequence_model(Type snapshotType)
         {
             using var context = new SequenceContext();
+            AssertSameSnapshot(snapshotType, context);
+        }
+
+        private static void AssertSameSnapshot(Type snapshotType, DbContext context)
+        {
             var differ = context.GetService<IMigrationsModelDiffer>();
             var snapshot = (ModelSnapshot)Activator.CreateInstance(snapshotType);
             var reporter = new TestOperationReporter();
-            var setBuilder = SqlServerTestHelpers.Instance.CreateContextServices().GetRequiredService<IModelRuntimeInitializer>();
-            var processor = new SnapshotModelProcessor(reporter, setBuilder);
-            var model = processor.Process(snapshot.Model);
+            var modelRuntimeInitializer =
+                SqlServerTestHelpers.Instance.CreateContextServices().GetRequiredService<IModelRuntimeInitializer>();
+
+            var model = PreprocessModel(snapshot);
+            model = new SnapshotModelProcessor(reporter, modelRuntimeInitializer).Process(model, resetVersion: true);
+            var currentModel = context.GetService<IDesignTimeModel>().Model;
 
             var differences = differ.GetDifferences(
-                model.GetRelationalModel(), context.GetService<IDesignTimeModel>().Model.GetRelationalModel());
+                model.GetRelationalModel(),
+                currentModel.GetRelationalModel());
 
             Assert.Empty(differences);
+
+            var generator = CSharpMigrationsGeneratorTest.CreateMigrationsCodeGenerator();
+
+            var oldSnapshotCode = generator.GenerateSnapshot(
+                "MyNamespace",
+                context.GetType(),
+                "MySnapshot",
+                model);
+
+            var newSnapshotCode = generator.GenerateSnapshot(
+                "MyNamespace",
+                context.GetType(),
+                "MySnapshot",
+                currentModel);
+
+            Assert.Equal(newSnapshotCode, oldSnapshotCode);
+        }
+
+        private static IModel PreprocessModel(ModelSnapshot snapshot)
+        {
+            var model = snapshot.Model;
+            if (model.FindAnnotation(RelationalAnnotationNames.MaxIdentifierLength) == null)
+            {
+                ((Model)model)[RelationalAnnotationNames.MaxIdentifierLength] = 128;
+            }
+
+            foreach (EntityType entityType in model.GetEntityTypes())
+            {
+                var schemaAnnotation = entityType.FindAnnotation(RelationalAnnotationNames.Schema);
+                if (schemaAnnotation != null
+                    && schemaAnnotation.Value == null)
+                {
+                    entityType.RemoveAnnotation(RelationalAnnotationNames.Schema);
+                }
+
+                foreach (var property in entityType.GetProperties())
+                {
+                    if (property.IsForeignKey())
+                    {
+                        if (property.ValueGenerated != ValueGenerated.Never)
+                        {
+                            property.SetValueGenerated(null, ConfigurationSource.Explicit);
+                        }
+
+                        if (property.GetValueGenerationStrategy() != SqlServerValueGenerationStrategy.None)
+                        {
+                            property.SetValueGenerationStrategy(null);
+                        }
+                    }
+                    else if (property.GetValueGenerationStrategy() is SqlServerValueGenerationStrategy strategy
+                        && strategy != SqlServerValueGenerationStrategy.None)
+                    {
+                        property.SetValueGenerationStrategy(strategy);
+                    }
+                }
+            }
+
+            return model;
         }
 
         private void AddAnnotations(IMutableAnnotatable element)

Original file line number	Diff line number	Diff line change
`@@ -319,7 +319,7 @@ public virtual MigrationFiles RemoveMigration(`
`319`	`319`	`}`
`320`	`320`
`321`	`321`	`model = migrations.Count > 1`
`322`		`- ? Dependencies.SnapshotModelProcessor.Process(migrations[^2].TargetModel)`
	`322`	`+ ? Dependencies.SnapshotModelProcessor.Process(migrations[^2].TargetModel, resetVersion: true)`
`323`	`323`	`: null;`
`324`	`324`	`}`
`325`	`325`	`else`
`@@ -351,6 +351,7 @@ public virtual MigrationFiles RemoveMigration(`
`351`	`351`	`{`
`352`	`352`	`var modelSnapshotNamespace = modelSnapshot.GetType().Namespace;`
`353`	`353`	`Check.DebugAssert(!string.IsNullOrEmpty(modelSnapshotNamespace), "modelSnapshotNamespace is null or empty");`
	`354`	`+`
`354`	`355`	`var modelSnapshotCode = codeGenerator.GenerateSnapshot(`
`355`	`356`	`modelSnapshotNamespace,`
`356`	`357`	`_contextType,`
Original file line number	Diff line number	Diff line change
`@@ -20,5 +20,5 @@ public interface ISnapshotModelProcessor`
`20`	`20`	`/// doing so can result in application failures when updating to a new Entity Framework Core release.`
`21`	`21`	`/// </summary>`
`22`	`22`	`[return: NotNullIfNotNull("model")]`
`23`		`- IModel? Process(IReadOnlyModel? model);`
	`23`	`+ IModel? Process(IReadOnlyModel? model, bool resetVersion = false);`
`24`	`24`	`}`
Original file line number	Diff line number	Diff line change
`@@ -47,7 +47,7 @@ public SnapshotModelProcessor(`
`47`	`47`	`/// any release. You should only use it directly in your code with extreme caution and knowing that`
`48`	`48`	`/// doing so can result in application failures when updating to a new Entity Framework Core release.`
`49`	`49`	`/// </summary>`
`50`		`- public virtual IModel? Process(IReadOnlyModel? model)`
	`50`	`+ public virtual IModel? Process(IReadOnlyModel? model, bool resetVersion = false)`
`51`	`51`	`{`
`52`	`52`	`if (model == null)`
`53`	`53`	`{`
`@@ -76,6 +76,15 @@ public SnapshotModelProcessor(`
`76`	`76`	`}`
`77`	`77`	`}`
`78`	`78`
	`79`	`+ if (model is IMutableModel mutableModel)`
	`80`	`+ {`
	`81`	`+ mutableModel.RemoveAnnotation("ChangeDetector.SkipDetectChanges");`
	`82`	`+ if (resetVersion)`
	`83`	`+ {`
	`84`	`+ mutableModel.SetProductVersion(ProductInfo.GetVersion());`
	`85`	`+ }`
	`86`	`+ }`
	`87`	`+`
`79`	`88`	`return _modelRuntimeInitializer.Initialize((IModel)model, designTime: true, validationLogger: null);`
`80`	`89`	`}`
`81`	`90`
`@@ -145,13 +154,17 @@ private static void UpdateSequences(IReadOnlyModel model, string version)`
`145`	`154`	`var sequences = model.GetAnnotations()`
`146`	`155`	`#pragma warning disable CS0618 // Type or member is obsolete`
`147`	`156`	`.Where(a => a.Name.StartsWith(RelationalAnnotationNames.SequencePrefix, StringComparison.Ordinal))`
`148`		`- .Select(a => new Sequence(model, a.Name));`
	`157`	`+ .ToList();`
`149`	`158`	`#pragma warning restore CS0618 // Type or member is obsolete`
`150`	`159`
`151`	`160`	`var sequencesDictionary = new Dictionary<(string, string?), ISequence>();`
`152`		`- foreach (var sequence in sequences)`
	`161`	`+ foreach (var sequenceAnnotation in sequences)`
`153`	`162`	`{`
	`163`	`+#pragma warning disable CS0618 // Type or member is obsolete`
	`164`	`+ var sequence = new Sequence(model, sequenceAnnotation.Name);`
	`165`	`+#pragma warning restore CS0618 // Type or member is obsolete`
`154`	`166`	`sequencesDictionary[(sequence.Name, sequence.ModelSchema)] = sequence;`
	`167`	`+ mutableModel.RemoveAnnotation(sequenceAnnotation.Name);`
`155`	`168`	`}`
`156`	`169`
`157`	`170`	`if (sequencesDictionary.Count > 0)`
Original file line number	Diff line number	Diff line change
`@@ -905,7 +905,7 @@ public void Multidimensional_array_warning_is_not_suppressed_for_unidimensional_`
`905`	`905`	`Assert.DoesNotContain("#pragma warning disable CA1814", migration);`
`906`	`906`	`}`
`907`	`907`
`908`		`- private static IMigrationsCodeGenerator CreateMigrationsCodeGenerator()`
	`908`	`+ public static IMigrationsCodeGenerator CreateMigrationsCodeGenerator()`
`909`	`909`	`{`
`910`	`910`	`var testAssembly = typeof(CSharpMigrationsGeneratorTest).Assembly;`
`911`	`911`	`var reporter = new TestOperationReporter();`