Stop assuming model snapshot and model-behind will have generated typ…

…e mappings Fixes #16737 Also manually tested add/remove migration.
dotnet · Jul 28, 2019 · 6ca581e · 6ca581e
1 parent ec02e5c
commit 6ca581e
Show file tree

Hide file tree

Showing 9 changed files with 95 additions and 39 deletions.
diff --git a/src/EFCore.Design/Design/DesignTimeServiceCollectionExtensions.cs b/src/EFCore.Design/Design/DesignTimeServiceCollectionExtensions.cs
@@ -105,6 +105,7 @@ public static IServiceCollection AddDbContextDesignTimeServices(
                 .AddTransient(_ => context.GetService<IMigrationsIdGenerator>())
                 .AddTransient(_ => context.GetService<IMigrationsModelDiffer>())
                 .AddTransient(_ => context.GetService<IMigrator>())
+                .AddTransient(_ => context.GetService<IRelationalTypeMappingSource>())
                 .AddTransient(_ => context.GetService<IModel>());
     }
 }
diff --git a/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs b/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs
@@ -504,8 +504,9 @@ protected virtual void GeneratePropertyAnnotations([NotNull] IProperty property,
             GenerateAnnotations(annotations, stringBuilder);
         }
 
-        private static ValueConverter FindValueConverter(IProperty property)
-            => property.GetTypeMapping().Converter;
+        private ValueConverter FindValueConverter(IProperty property)
+            => (property.FindMapping()
+                ?? Dependencies.RelationalTypeMappingSource.FindMapping(property))?.Converter;
 
         /// <summary>
         ///     Generates code for <see cref="IKey" /> objects.

diff --git a/src/EFCore.Design/Migrations/Design/CSharpSnapshotGeneratorDependencies.cs b/src/EFCore.Design/Migrations/Design/CSharpSnapshotGeneratorDependencies.cs
@@ -4,6 +4,7 @@
 using JetBrains.Annotations;
 using Microsoft.EntityFrameworkCore.Design;
 using Microsoft.EntityFrameworkCore.Infrastructure;
+using Microsoft.EntityFrameworkCore.Storage;
 using Microsoft.EntityFrameworkCore.Utilities;
 
 namespace Microsoft.EntityFrameworkCore.Migrations.Design
@@ -46,28 +47,41 @@ public sealed class CSharpSnapshotGeneratorDependencies
         ///         doing so can result in application failures when updating to a new Entity Framework Core release.
         ///     </para>
         /// </summary>
-        /// <param name="csharpHelper"> The C# helper. </param>
         [EntityFrameworkInternal]
         public CSharpSnapshotGeneratorDependencies(
-            [NotNull] ICSharpHelper csharpHelper)
+            [NotNull] ICSharpHelper csharpHelper,
+            [NotNull] IRelationalTypeMappingSource relationalTypeMappingSource)
         {
             Check.NotNull(csharpHelper, nameof(csharpHelper));
 
             CSharpHelper = csharpHelper;
+            RelationalTypeMappingSource = relationalTypeMappingSource;
         }
 
         /// <summary>
         ///     The C# helper.
         /// </summary>
         public ICSharpHelper CSharpHelper { get; }
 
+        /// <summary>
+        ///     The type mapper.
+        /// </summary>
+        public IRelationalTypeMappingSource RelationalTypeMappingSource { get; }
+
         /// <summary>
         ///     Clones this dependency parameter object with one service replaced.
         /// </summary>
         /// <param name="csharpHelper"> A replacement for the current dependency of this type. </param>
         /// <returns> A new parameter object with the given service replaced. </returns>
-        public CSharpSnapshotGeneratorDependencies With(
-            [NotNull] ICSharpHelper csharpHelper)
-            => new CSharpSnapshotGeneratorDependencies(csharpHelper);
+        public CSharpSnapshotGeneratorDependencies With([NotNull] ICSharpHelper csharpHelper)
+            => new CSharpSnapshotGeneratorDependencies(csharpHelper, RelationalTypeMappingSource);
+
+        /// <summary>
+        ///     Clones this dependency parameter object with one service replaced.
+        /// </summary>
+        /// <param name="relationalTypeMappingSource"> A replacement for the current dependency of this type. </param>
+        /// <returns> A new parameter object with the given service replaced. </returns>
+        public CSharpSnapshotGeneratorDependencies With([NotNull] IRelationalTypeMappingSource relationalTypeMappingSource)
+            => new CSharpSnapshotGeneratorDependencies(CSharpHelper, relationalTypeMappingSource);
     }
 }
diff --git a/src/EFCore.Design/Migrations/Design/MigrationsCodeGenerator.cs b/src/EFCore.Design/Migrations/Design/MigrationsCodeGenerator.cs
@@ -9,8 +9,8 @@
 using Microsoft.EntityFrameworkCore.Infrastructure;
 using Microsoft.EntityFrameworkCore.Internal;
 using Microsoft.EntityFrameworkCore.Metadata;
-using Microsoft.EntityFrameworkCore.Metadata.Internal;
 using Microsoft.EntityFrameworkCore.Migrations.Operations;
+using Microsoft.EntityFrameworkCore.Storage.ValueConversion;
 using Microsoft.EntityFrameworkCore.Utilities;
 
 namespace Microsoft.EntityFrameworkCore.Migrations.Design
@@ -181,7 +181,7 @@ private static IEnumerable<IAnnotatable> GetAnnotatables(IEnumerable<MigrationOp
         protected virtual IEnumerable<string> GetNamespaces([NotNull] IModel model)
             => model.GetEntityTypes().SelectMany(
                     e => e.GetDeclaredProperties()
-                        .SelectMany(p => (p.GetTypeMapping().Converter?.ProviderClrType ?? p.ClrType).GetNamespaces()))
+                        .SelectMany(p => (FindValueConverter(p)?.ProviderClrType ?? p.ClrType).GetNamespaces()))
                 .Concat(GetAnnotationNamespaces(GetAnnotatables(model)));
 
         private static IEnumerable<IAnnotatable> GetAnnotatables(IModel model)
@@ -214,7 +214,7 @@ private static IEnumerable<IAnnotatable> GetAnnotatables(IModel model)
             }
         }
 
-        private static IEnumerable<string> GetAnnotationNamespaces(IEnumerable<IAnnotatable> items)
+        private IEnumerable<string> GetAnnotationNamespaces(IEnumerable<IAnnotatable> items)
         {
             var ignoredAnnotations = new List<string>
             {
@@ -261,10 +261,14 @@ private static IEnumerable<string> GetAnnotationNamespaces(IEnumerable<IAnnotata
                     .SelectMany(a => GetProviderType(a.Annotatable, a.Annotation.Value.GetType()).GetNamespaces()));
         }
 
-        private static Type GetProviderType(IAnnotatable annotatable, Type valueType)
+        private ValueConverter FindValueConverter(IProperty property)
+            => (property.FindMapping()
+                ?? Dependencies.RelationalTypeMappingSource.FindMapping(property))?.Converter;
+
+        private Type GetProviderType(IAnnotatable annotatable, Type valueType)
             => annotatable is IProperty property
                && valueType.UnwrapNullableType() == property.ClrType.UnwrapNullableType()
-                ? property.GetTypeMapping().Converter?.ProviderClrType ?? valueType
+                ? FindValueConverter(property)?.ProviderClrType ?? valueType
                 : valueType;
     }
 }
diff --git a/src/EFCore.Design/Migrations/Design/MigrationsCodeGeneratorDependencies.cs b/src/EFCore.Design/Migrations/Design/MigrationsCodeGeneratorDependencies.cs
@@ -1,7 +1,9 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using JetBrains.Annotations;
 using Microsoft.EntityFrameworkCore.Infrastructure;
+using Microsoft.EntityFrameworkCore.Storage;
 
 namespace Microsoft.EntityFrameworkCore.Migrations.Design
 {
@@ -44,8 +46,22 @@ public sealed class MigrationsCodeGeneratorDependencies
         ///     </para>
         /// </summary>
         [EntityFrameworkInternal]
-        public MigrationsCodeGeneratorDependencies()
+        public MigrationsCodeGeneratorDependencies([NotNull] IRelationalTypeMappingSource relationalTypeMappingSource)
         {
+            RelationalTypeMappingSource = relationalTypeMappingSource;
         }
+
+        /// <summary>
+        ///     The type mapper.
+        /// </summary>
+        public IRelationalTypeMappingSource RelationalTypeMappingSource { get; }
+
+        /// <summary>
+        ///     Clones this dependency parameter object with one service replaced.
+        /// </summary>
+        /// <param name="relationalTypeMappingSource"> A replacement for the current dependency of this type. </param>
+        /// <returns> A new parameter object with the given service replaced. </returns>
+        public MigrationsCodeGeneratorDependencies With([NotNull] IRelationalTypeMappingSource relationalTypeMappingSource)
+            => new MigrationsCodeGeneratorDependencies(relationalTypeMappingSource);
     }
 }
diff --git a/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.cs b/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.cs
@@ -215,13 +215,15 @@ private static void MissingAnnotationCheck(
             string generationDefault,
             Action<TestCSharpSnapshotGenerator, IMutableAnnotatable, IndentedStringBuilder> test)
         {
+            var sqlServerTypeMappingSource = new SqlServerTypeMappingSource(
+                TestServiceFactory.Instance.Create<TypeMappingSourceDependencies>(),
+                TestServiceFactory.Instance.Create<RelationalTypeMappingSourceDependencies>());
+
             var codeHelper = new CSharpHelper(
-                new SqlServerTypeMappingSource(
-                    TestServiceFactory.Instance.Create<TypeMappingSourceDependencies>(),
-                    TestServiceFactory.Instance.Create<RelationalTypeMappingSourceDependencies>()));
+                sqlServerTypeMappingSource);
 
             var generator = new TestCSharpSnapshotGenerator(
-                new CSharpSnapshotGeneratorDependencies(codeHelper));
+                new CSharpSnapshotGeneratorDependencies(codeHelper, sqlServerTypeMappingSource));
 
             var coreAnnotations = typeof(CoreAnnotationNames).GetFields().Where(f => f.FieldType == typeof(string)).ToList();
 
@@ -298,21 +300,23 @@ private class Derived : WithAnnotations
         [ConditionalFact]
         public void Snapshot_with_enum_discriminator_uses_converted_values()
         {
+            var sqlServerTypeMappingSource = new SqlServerTypeMappingSource(
+                TestServiceFactory.Instance.Create<TypeMappingSourceDependencies>(),
+                TestServiceFactory.Instance.Create<RelationalTypeMappingSourceDependencies>());
+
             var codeHelper = new CSharpHelper(
-                new SqlServerTypeMappingSource(
-                    TestServiceFactory.Instance.Create<TypeMappingSourceDependencies>(),
-                    TestServiceFactory.Instance.Create<RelationalTypeMappingSourceDependencies>()));
+                sqlServerTypeMappingSource);
 
             var generator = new CSharpMigrationsGenerator(
-                new MigrationsCodeGeneratorDependencies(),
+                new MigrationsCodeGeneratorDependencies(sqlServerTypeMappingSource),
                 new CSharpMigrationsGeneratorDependencies(
                     codeHelper,
                     new CSharpMigrationOperationGenerator(
                         new CSharpMigrationOperationGeneratorDependencies(
                             codeHelper)),
                     new CSharpSnapshotGenerator(
                         new CSharpSnapshotGeneratorDependencies(
-                            codeHelper))));
+                            codeHelper, sqlServerTypeMappingSource))));
 
             var modelBuilder = RelationalTestHelpers.Instance.CreateConventionBuilder();
             modelBuilder.Model.RemoveAnnotation(CoreAnnotationNames.ProductVersion);
@@ -348,13 +352,14 @@ private static void AssertConverter(ValueConverter valueConverter, string expect
 
             modelBuilder.FinalizeModel();
 
-            var codeHelper = new CSharpHelper(
-                new SqlServerTypeMappingSource(
-                    TestServiceFactory.Instance.Create<TypeMappingSourceDependencies>(),
-                    TestServiceFactory.Instance.Create<RelationalTypeMappingSourceDependencies>()));
+            var sqlServerTypeMappingSource = new SqlServerTypeMappingSource(
+                TestServiceFactory.Instance.Create<TypeMappingSourceDependencies>(),
+                TestServiceFactory.Instance.Create<RelationalTypeMappingSourceDependencies>());
+
+            var codeHelper = new CSharpHelper(sqlServerTypeMappingSource);
 
             var generator = new TestCSharpSnapshotGenerator(
-                new CSharpSnapshotGeneratorDependencies(codeHelper));
+                new CSharpSnapshotGeneratorDependencies(codeHelper, sqlServerTypeMappingSource));
 
             var sb = new IndentedStringBuilder();
 

diff --git a/test/EFCore.Design.Tests/Migrations/Design/MigrationScaffolderTest.cs b/test/EFCore.Design.Tests/Migrations/Design/MigrationScaffolderTest.cs
@@ -51,10 +51,11 @@ private IMigrationsScaffolder CreateMigrationScaffolder<TContext>()
         {
             var currentContext = new CurrentDbContext(new TContext());
             var idGenerator = new MigrationsIdGenerator();
+            var sqlServerTypeMappingSource = new SqlServerTypeMappingSource(
+                TestServiceFactory.Instance.Create<TypeMappingSourceDependencies>(),
+                TestServiceFactory.Instance.Create<RelationalTypeMappingSourceDependencies>());
             var code = new CSharpHelper(
-                new SqlServerTypeMappingSource(
-                    TestServiceFactory.Instance.Create<TypeMappingSourceDependencies>(),
-                    TestServiceFactory.Instance.Create<RelationalTypeMappingSourceDependencies>()));
+                sqlServerTypeMappingSource);
             var reporter = new TestOperationReporter();
             var migrationAssembly
                 = new MigrationsAssembly(
@@ -84,15 +85,15 @@ var migrationAssembly
                         new[]
                         {
                             new CSharpMigrationsGenerator(
-                                new MigrationsCodeGeneratorDependencies(),
+                                new MigrationsCodeGeneratorDependencies(sqlServerTypeMappingSource),
                                 new CSharpMigrationsGeneratorDependencies(
                                     code,
                                     new CSharpMigrationOperationGenerator(
                                         new CSharpMigrationOperationGeneratorDependencies(
                                             code)),
                                     new CSharpSnapshotGenerator(
                                         new CSharpSnapshotGeneratorDependencies(
-                                            code))))
+                                            code, sqlServerTypeMappingSource))))
                         }),
                     historyRepository,
                     reporter,

diff --git a/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs b/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs
@@ -3534,23 +3534,24 @@ protected void Test(Action<ModelBuilder> buildModel, string expectedCode, Action
 
             var model = modelBuilder.FinalizeModel();
 
+            var sqlServerTypeMappingSource = new SqlServerTypeMappingSource(
+                TestServiceFactory.Instance.Create<TypeMappingSourceDependencies>(),
+                new RelationalTypeMappingSourceDependencies(
+                    new IRelationalTypeMappingSourcePlugin[]
+                        { new SqlServerNetTopologySuiteTypeMappingSourcePlugin(NtsGeometryServices.Instance) }));
             var codeHelper = new CSharpHelper(
-                new SqlServerTypeMappingSource(
-                    TestServiceFactory.Instance.Create<TypeMappingSourceDependencies>(),
-                    new RelationalTypeMappingSourceDependencies(
-                        new IRelationalTypeMappingSourcePlugin[]
-                            { new SqlServerNetTopologySuiteTypeMappingSourcePlugin(NtsGeometryServices.Instance) })));
+                sqlServerTypeMappingSource);
 
             var generator = new CSharpMigrationsGenerator(
-                new MigrationsCodeGeneratorDependencies(),
+                new MigrationsCodeGeneratorDependencies(sqlServerTypeMappingSource),
                 new CSharpMigrationsGeneratorDependencies(
                     codeHelper,
                     new CSharpMigrationOperationGenerator(
                         new CSharpMigrationOperationGeneratorDependencies(
                             codeHelper)),
                     new CSharpSnapshotGenerator(
                         new CSharpSnapshotGeneratorDependencies(
-                            codeHelper))));
+                            codeHelper, sqlServerTypeMappingSource))));
 
             var code = generator.GenerateSnapshot("RootNamespace", typeof(DbContext), "Snapshot", model);
 

diff --git a/test/EFCore.Relational.Specification.Tests/MigrationsTestBase.cs b/test/EFCore.Relational.Specification.Tests/MigrationsTestBase.cs
@@ -9,6 +9,7 @@
 using Microsoft.EntityFrameworkCore.Infrastructure;
 using Microsoft.EntityFrameworkCore.Internal;
 using Microsoft.EntityFrameworkCore.Migrations;
+using Microsoft.EntityFrameworkCore.Migrations.Internal;
 using Microsoft.EntityFrameworkCore.Storage;
 using Microsoft.EntityFrameworkCore.TestUtilities.Xunit;
 using Microsoft.Extensions.DependencyInjection;
@@ -371,6 +372,18 @@ protected virtual void DiffSnapshot(ModelSnapshot snapshot, DbContext context)
             var sourceModel = snapshot.Model;
             var targetModel = context.Model;
 
+            var typeMapper = context.GetService<IRelationalTypeMappingSource>();
+
+            foreach (var property in sourceModel.GetEntityTypes().SelectMany(e => e.GetDeclaredProperties()))
+            {
+                Assert.NotNull(typeMapper.FindMapping(property));
+            }
+
+            foreach (var property in targetModel.GetEntityTypes().SelectMany(e => e.GetDeclaredProperties()))
+            {
+                Assert.NotNull(typeMapper.FindMapping(property));
+            }
+
             var modelDiffer = context.GetService<IMigrationsModelDiffer>();
             var operations = modelDiffer.GetDifferences(sourceModel, targetModel);