From 6fb4576e00962b7d65147818a98fe0f6f27890a6 Mon Sep 17 00:00:00 2001 From: Arthur Vickers Date: Sun, 28 Jul 2019 15:59:19 -0700 Subject: [PATCH] Stop assuming model snapshot and model-behind will have generated type mappings Fixes #16737 Also manually tested add/remove migration. --- .../DesignTimeServiceCollectionExtensions.cs | 1 + .../Design/CSharpSnapshotGenerator.cs | 5 +-- .../CSharpSnapshotGeneratorDependencies.cs | 24 +++++++++++--- .../Design/MigrationsCodeGenerator.cs | 14 +++++--- .../MigrationsCodeGeneratorDependencies.cs | 18 +++++++++- .../Design/CSharpMigrationsGeneratorTest.cs | 33 +++++++++++-------- .../Design/MigrationScaffolderTest.cs | 11 ++++--- .../Migrations/ModelSnapshotSqlServerTest.cs | 15 +++++---- .../MigrationsTestBase.cs | 13 ++++++++ 9 files changed, 95 insertions(+), 39 deletions(-) diff --git a/src/EFCore.Design/Design/DesignTimeServiceCollectionExtensions.cs b/src/EFCore.Design/Design/DesignTimeServiceCollectionExtensions.cs index f08ce470b5d..d780ce4b8b6 100644 --- 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()) .AddTransient(_ => context.GetService()) .AddTransient(_ => context.GetService()) + .AddTransient(_ => context.GetService()) .AddTransient(_ => context.GetService()); } } diff --git a/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs b/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs index 87330bc0a90..8e55b8fd35f 100644 --- 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; /// /// Generates code for objects. diff --git a/src/EFCore.Design/Migrations/Design/CSharpSnapshotGeneratorDependencies.cs b/src/EFCore.Design/Migrations/Design/CSharpSnapshotGeneratorDependencies.cs index 69fd1e9104a..b610fac0429 100644 --- 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,14 +47,15 @@ public sealed class CSharpSnapshotGeneratorDependencies /// doing so can result in application failures when updating to a new Entity Framework Core release. /// /// - /// The C# helper. [EntityFrameworkInternal] public CSharpSnapshotGeneratorDependencies( - [NotNull] ICSharpHelper csharpHelper) + [NotNull] ICSharpHelper csharpHelper, + [NotNull] IRelationalTypeMappingSource relationalTypeMappingSource) { Check.NotNull(csharpHelper, nameof(csharpHelper)); CSharpHelper = csharpHelper; + RelationalTypeMappingSource = relationalTypeMappingSource; } /// @@ -61,13 +63,25 @@ public CSharpSnapshotGeneratorDependencies( /// public ICSharpHelper CSharpHelper { get; } + /// + /// The type mapper. + /// + public IRelationalTypeMappingSource RelationalTypeMappingSource { get; } + /// /// Clones this dependency parameter object with one service replaced. /// /// A replacement for the current dependency of this type. /// A new parameter object with the given service replaced. - public CSharpSnapshotGeneratorDependencies With( - [NotNull] ICSharpHelper csharpHelper) - => new CSharpSnapshotGeneratorDependencies(csharpHelper); + public CSharpSnapshotGeneratorDependencies With([NotNull] ICSharpHelper csharpHelper) + => new CSharpSnapshotGeneratorDependencies(csharpHelper, RelationalTypeMappingSource); + + /// + /// Clones this dependency parameter object with one service replaced. + /// + /// A replacement for the current dependency of this type. + /// A new parameter object with the given service replaced. + 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 index db846d83969..75608d04a5b 100644 --- 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 GetAnnotatables(IEnumerable 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 GetAnnotatables(IModel model) @@ -214,7 +214,7 @@ private static IEnumerable GetAnnotatables(IModel model) } } - private static IEnumerable GetAnnotationNamespaces(IEnumerable items) + private IEnumerable GetAnnotationNamespaces(IEnumerable items) { var ignoredAnnotations = new List { @@ -261,10 +261,14 @@ private static IEnumerable GetAnnotationNamespaces(IEnumerable 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 index ac74b871ea5..91c3b4a8de4 100644 --- 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 /// /// [EntityFrameworkInternal] - public MigrationsCodeGeneratorDependencies() + public MigrationsCodeGeneratorDependencies([NotNull] IRelationalTypeMappingSource relationalTypeMappingSource) { + RelationalTypeMappingSource = relationalTypeMappingSource; } + + /// + /// The type mapper. + /// + public IRelationalTypeMappingSource RelationalTypeMappingSource { get; } + + /// + /// Clones this dependency parameter object with one service replaced. + /// + /// A replacement for the current dependency of this type. + /// A new parameter object with the given service replaced. + 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 index 8d4801e75ba..764deaa6dca 100644 --- 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 test) { + var sqlServerTypeMappingSource = new SqlServerTypeMappingSource( + TestServiceFactory.Instance.Create(), + TestServiceFactory.Instance.Create()); + var codeHelper = new CSharpHelper( - new SqlServerTypeMappingSource( - TestServiceFactory.Instance.Create(), - TestServiceFactory.Instance.Create())); + 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,13 +300,15 @@ private class Derived : WithAnnotations [ConditionalFact] public void Snapshot_with_enum_discriminator_uses_converted_values() { + var sqlServerTypeMappingSource = new SqlServerTypeMappingSource( + TestServiceFactory.Instance.Create(), + TestServiceFactory.Instance.Create()); + var codeHelper = new CSharpHelper( - new SqlServerTypeMappingSource( - TestServiceFactory.Instance.Create(), - TestServiceFactory.Instance.Create())); + sqlServerTypeMappingSource); var generator = new CSharpMigrationsGenerator( - new MigrationsCodeGeneratorDependencies(), + new MigrationsCodeGeneratorDependencies(sqlServerTypeMappingSource), new CSharpMigrationsGeneratorDependencies( codeHelper, new CSharpMigrationOperationGenerator( @@ -312,7 +316,7 @@ public void Snapshot_with_enum_discriminator_uses_converted_values() 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(), - TestServiceFactory.Instance.Create())); + var sqlServerTypeMappingSource = new SqlServerTypeMappingSource( + TestServiceFactory.Instance.Create(), + TestServiceFactory.Instance.Create()); + + 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 index 4a5a89dcddf..35a92e41fae 100644 --- 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() { var currentContext = new CurrentDbContext(new TContext()); var idGenerator = new MigrationsIdGenerator(); + var sqlServerTypeMappingSource = new SqlServerTypeMappingSource( + TestServiceFactory.Instance.Create(), + TestServiceFactory.Instance.Create()); var code = new CSharpHelper( - new SqlServerTypeMappingSource( - TestServiceFactory.Instance.Create(), - TestServiceFactory.Instance.Create())); + sqlServerTypeMappingSource); var reporter = new TestOperationReporter(); var migrationAssembly = new MigrationsAssembly( @@ -84,7 +85,7 @@ var migrationAssembly new[] { new CSharpMigrationsGenerator( - new MigrationsCodeGeneratorDependencies(), + new MigrationsCodeGeneratorDependencies(sqlServerTypeMappingSource), new CSharpMigrationsGeneratorDependencies( code, new CSharpMigrationOperationGenerator( @@ -92,7 +93,7 @@ var migrationAssembly 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 index 28835147859..b058cde31cf 100644 --- a/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs +++ b/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs @@ -3534,15 +3534,16 @@ protected void Test(Action buildModel, string expectedCode, Action var model = modelBuilder.FinalizeModel(); + var sqlServerTypeMappingSource = new SqlServerTypeMappingSource( + TestServiceFactory.Instance.Create(), + new RelationalTypeMappingSourceDependencies( + new IRelationalTypeMappingSourcePlugin[] + { new SqlServerNetTopologySuiteTypeMappingSourcePlugin(NtsGeometryServices.Instance) })); var codeHelper = new CSharpHelper( - new SqlServerTypeMappingSource( - TestServiceFactory.Instance.Create(), - new RelationalTypeMappingSourceDependencies( - new IRelationalTypeMappingSourcePlugin[] - { new SqlServerNetTopologySuiteTypeMappingSourcePlugin(NtsGeometryServices.Instance) }))); + sqlServerTypeMappingSource); var generator = new CSharpMigrationsGenerator( - new MigrationsCodeGeneratorDependencies(), + new MigrationsCodeGeneratorDependencies(sqlServerTypeMappingSource), new CSharpMigrationsGeneratorDependencies( codeHelper, new CSharpMigrationOperationGenerator( @@ -3550,7 +3551,7 @@ protected void Test(Action buildModel, string expectedCode, Action 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 index 36468f1e89b..9051933c8d8 100644 --- 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(); + + 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(); var operations = modelDiffer.GetDifferences(sourceModel, targetModel);