From 9f924e5f71c2638c00c1b34f50c65dc3bd4dacfd Mon Sep 17 00:00:00 2001 From: Arthur Vickers Date: Fri, 16 Aug 2019 11:40:48 -0700 Subject: [PATCH] Be more resilient when using the model from the model snapshot Fixes #17145 but see also #17205 The fix here allows the state manager to work, which is required when the model snapshot has model data in it. --- .../Internal/SnapshotModelProcessor.cs | 4 +--- .../Storage/Internal/InMemoryTable.cs | 2 +- .../Infrastructure/ModelSnapshot.cs | 10 ++++++---- .../Migrations/Internal/MigrationsModelDiffer.cs | 4 ++-- .../ChangeTracking/Internal/ChangeDetector.cs | 2 +- .../Internal/CompositeValueFactory.cs | 2 +- .../Internal/SimplePrincipalKeyValueFactory.cs | 2 +- .../ChangeTracking/Internal/StateManager.cs | 2 +- src/EFCore/Metadata/Internal/EntityType.cs | 2 +- .../Metadata/Internal/EntityTypeExtensions.cs | 2 +- src/EFCore/Metadata/Internal/Model.cs | 11 ++++++++++- .../ValueGeneration/ValueGeneratorSelector.cs | 2 +- src/Shared/Check.cs | 9 +++++++++ .../MigrationsSqlServerTest.cs | 16 ++++++++++++++++ 14 files changed, 52 insertions(+), 18 deletions(-) diff --git a/src/EFCore.Design/Migrations/Internal/SnapshotModelProcessor.cs b/src/EFCore.Design/Migrations/Internal/SnapshotModelProcessor.cs index a5455729970..58d11f5e642 100644 --- a/src/EFCore.Design/Migrations/Internal/SnapshotModelProcessor.cs +++ b/src/EFCore.Design/Migrations/Internal/SnapshotModelProcessor.cs @@ -76,9 +76,7 @@ public virtual IModel Process(IModel model) } } - return (model is IMutableModel mutableModel) - ? mutableModel.FinalizeModel() - : model; + return model; } private void ProcessCollection(IEnumerable metadata, string version) diff --git a/src/EFCore.InMemory/Storage/Internal/InMemoryTable.cs b/src/EFCore.InMemory/Storage/Internal/InMemoryTable.cs index 71208bf0416..0c786610b3f 100644 --- a/src/EFCore.InMemory/Storage/Internal/InMemoryTable.cs +++ b/src/EFCore.InMemory/Storage/Internal/InMemoryTable.cs @@ -89,7 +89,7 @@ private static List GetStructuralComparers(IEnumerable => properties.Select(GetStructuralComparer).ToList(); private static ValueComparer GetStructuralComparer(IProperty p) - => p.GetStructuralValueComparer() ?? p.GetTypeMapping().StructuralComparer; + => p.GetStructuralValueComparer() ?? p.FindTypeMapping()?.StructuralComparer; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to diff --git a/src/EFCore.Relational/Infrastructure/ModelSnapshot.cs b/src/EFCore.Relational/Infrastructure/ModelSnapshot.cs index 42766e8f21d..260255f5ff0 100644 --- a/src/EFCore.Relational/Infrastructure/ModelSnapshot.cs +++ b/src/EFCore.Relational/Infrastructure/ModelSnapshot.cs @@ -3,7 +3,7 @@ using JetBrains.Annotations; using Microsoft.EntityFrameworkCore.Metadata; -using Microsoft.EntityFrameworkCore.Metadata.Conventions; +using Microsoft.EntityFrameworkCore.Metadata.Internal; namespace Microsoft.EntityFrameworkCore.Infrastructure { @@ -14,12 +14,14 @@ public abstract class ModelSnapshot { private IModel _model; - private IMutableModel CreateModel() + private IModel CreateModel() { - var modelBuilder = new ModelBuilder(new ConventionSet()); + var model = new Model(); + var modelBuilder = new ModelBuilder(model); + BuildModel(modelBuilder); - return modelBuilder.Model; + return model.FinalizeModel(); } /// diff --git a/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs b/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs index 5b2e4f5b550..968b24b5bea 100644 --- a/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs +++ b/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs @@ -1814,8 +1814,8 @@ protected virtual IEnumerable Remove([NotNull] ISequence sou var targetValue = entry.GetCurrentValue(targetProperty); var comparer = targetProperty.GetValueComparer() ?? sourceProperty.GetValueComparer() ?? - targetProperty.GetTypeMapping().Comparer ?? - sourceProperty.GetTypeMapping().Comparer; + targetProperty.FindTypeMapping()?.Comparer ?? + sourceProperty.FindTypeMapping()?.Comparer; var modelValuesChanged = sourceProperty.ClrType.UnwrapNullableType() == targetProperty.ClrType.UnwrapNullableType() diff --git a/src/EFCore/ChangeTracking/Internal/ChangeDetector.cs b/src/EFCore/ChangeTracking/Internal/ChangeDetector.cs index 4b5f9e0d0ae..57f44663b2a 100644 --- a/src/EFCore/ChangeTracking/Internal/ChangeDetector.cs +++ b/src/EFCore/ChangeTracking/Internal/ChangeDetector.cs @@ -252,7 +252,7 @@ private void DetectKeyChange(InternalEntityEntry entry, IProperty property) var currentValue = entry[property]; var comparer = property.GetKeyValueComparer() - ?? property.GetTypeMapping().KeyComparer; + ?? property.FindTypeMapping()?.KeyComparer; // Note that mutation of a byte[] key is not supported or detected, but two different instances // of byte[] with the same content must be detected as equal. diff --git a/src/EFCore/ChangeTracking/Internal/CompositeValueFactory.cs b/src/EFCore/ChangeTracking/Internal/CompositeValueFactory.cs index 2f273b25b26..a58e1ee0807 100644 --- a/src/EFCore/ChangeTracking/Internal/CompositeValueFactory.cs +++ b/src/EFCore/ChangeTracking/Internal/CompositeValueFactory.cs @@ -133,7 +133,7 @@ public virtual bool TryCreateFromRelationshipSnapshot(InternalEntityEntry entry, /// protected static IEqualityComparer CreateEqualityComparer([NotNull] IReadOnlyList properties) { - var comparers = properties.Select(p => p.GetKeyValueComparer() ?? p.GetTypeMapping().KeyComparer).ToList(); + var comparers = properties.Select(p => p.GetKeyValueComparer() ?? p.FindTypeMapping()?.KeyComparer).ToList(); return comparers.All(c => c != null) ? new CompositeCustomComparer(comparers) diff --git a/src/EFCore/ChangeTracking/Internal/SimplePrincipalKeyValueFactory.cs b/src/EFCore/ChangeTracking/Internal/SimplePrincipalKeyValueFactory.cs index 38fd86bc40a..b8fa8a82eda 100644 --- a/src/EFCore/ChangeTracking/Internal/SimplePrincipalKeyValueFactory.cs +++ b/src/EFCore/ChangeTracking/Internal/SimplePrincipalKeyValueFactory.cs @@ -35,7 +35,7 @@ public SimplePrincipalKeyValueFactory([NotNull] IProperty property) _propertyAccessors = _property.GetPropertyAccessors(); var comparer = property.GetKeyValueComparer() - ?? property.GetTypeMapping().KeyComparer; + ?? property.FindTypeMapping()?.KeyComparer; EqualityComparer = comparer != null diff --git a/src/EFCore/ChangeTracking/Internal/StateManager.cs b/src/EFCore/ChangeTracking/Internal/StateManager.cs index 07183b24343..b5db82689b4 100644 --- a/src/EFCore/ChangeTracking/Internal/StateManager.cs +++ b/src/EFCore/ChangeTracking/Internal/StateManager.cs @@ -1067,7 +1067,7 @@ private static bool KeysEqual(InternalEntityEntry entry, IForeignKey fk, Interna private static bool KeyValuesEqual(IProperty property, object value, object currentValue) => (property.GetKeyValueComparer() - ?? property.GetTypeMapping().KeyComparer) + ?? property.FindTypeMapping()?.KeyComparer) ?.Equals(currentValue, value) ?? Equals(currentValue, value); diff --git a/src/EFCore/Metadata/Internal/EntityType.cs b/src/EFCore/Metadata/Internal/EntityType.cs index 869aeb4acec..2e7693f3e2e 100644 --- a/src/EFCore/Metadata/Internal/EntityType.cs +++ b/src/EFCore/Metadata/Internal/EntityType.cs @@ -2328,7 +2328,7 @@ public override void OnTypeMemberIgnored(string name) { if (propertyBase is IProperty property) { - valueConverter = property.GetTypeMapping().Converter + valueConverter = property.FindTypeMapping()?.Converter ?? property.GetValueConverter(); } diff --git a/src/EFCore/Metadata/Internal/EntityTypeExtensions.cs b/src/EFCore/Metadata/Internal/EntityTypeExtensions.cs index dcabc49c3d3..f72b7754bca 100644 --- a/src/EFCore/Metadata/Internal/EntityTypeExtensions.cs +++ b/src/EFCore/Metadata/Internal/EntityTypeExtensions.cs @@ -299,7 +299,7 @@ public static PropertyCounts GetCounts([NotNull] this IEntityType entityType) /// public static PropertyCounts CalculateCounts([NotNull] this EntityType entityType) { - Debug.Assert(entityType.Model.ConventionDispatcher == null, "Should not be called on a mutable model"); + Check.DebugAssert(entityType.Model.IsReadonly, "Should not be called on a mutable model"); var index = 0; var navigationIndex = 0; diff --git a/src/EFCore/Metadata/Internal/Model.cs b/src/EFCore/Metadata/Internal/Model.cs index a564d9f58e9..209bc1f87be 100644 --- a/src/EFCore/Metadata/Internal/Model.cs +++ b/src/EFCore/Metadata/Internal/Model.cs @@ -83,6 +83,15 @@ public Model([NotNull] ConventionSet conventions) /// public virtual ConventionDispatcher ConventionDispatcher { [DebuggerStepThrough] get; private set; } + /// + /// 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. + /// + public virtual bool IsReadonly + => ConventionDispatcher == null; + /// /// 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 @@ -824,7 +833,7 @@ public virtual void SetPropertyAccessMode(PropertyAccessMode? propertyAccessMode public virtual IModel FinalizeModel() { var finalModel = (Model)ConventionDispatcher.OnModelFinalized(Builder)?.Metadata; - return finalModel?.MakeReadonly(); + return finalModel?.MakeReadonly() ?? this; } /// diff --git a/src/EFCore/ValueGeneration/ValueGeneratorSelector.cs b/src/EFCore/ValueGeneration/ValueGeneratorSelector.cs index 21fb0bd96f3..f8cd33eb321 100644 --- a/src/EFCore/ValueGeneration/ValueGeneratorSelector.cs +++ b/src/EFCore/ValueGeneration/ValueGeneratorSelector.cs @@ -73,7 +73,7 @@ private static ValueGenerator CreateFromFactory(IProperty property, IEntityType if (factory == null) { - var mapping = property.GetTypeMapping(); + var mapping = property.FindTypeMapping(); factory = mapping?.ValueGeneratorFactory; if (factory == null) diff --git a/src/Shared/Check.cs b/src/Shared/Check.cs index a5d167f17b6..c974f3e3915 100644 --- a/src/Shared/Check.cs +++ b/src/Shared/Check.cs @@ -93,5 +93,14 @@ public static IReadOnlyList HasNoNulls(IReadOnlyList value, [InvokerPar return value; } + + [Conditional("DEBUG")] + public static void DebugAssert([System.Diagnostics.CodeAnalysis.DoesNotReturnIf(false)] bool condition, string message) + { + if (!condition) + { + throw new Exception($"Check.DebugAssert failed: {message}"); + } + } } } diff --git a/test/EFCore.SqlServer.FunctionalTests/MigrationsSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/MigrationsSqlServerTest.cs index f3df4812505..971a4f20fcb 100644 --- a/test/EFCore.SqlServer.FunctionalTests/MigrationsSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/MigrationsSqlServerTest.cs @@ -528,6 +528,13 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.HasKey("Id"); b.ToTable("Blogs"); + + b.HasData( + new + { + Id = 1, + Name = "HalfADonkey" + }); }); modelBuilder.Entity( @@ -1349,6 +1356,15 @@ protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) => optionsBuilder.UseSqlServer(@"Server=(localdb)\mssqllocaldb;Database=Test;ConnectRetryCount=0"); public DbSet Blogs { get; set; } + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity().HasData( + new Blog + { + Id = 1, Name = "HalfADonkey" + }); + } } }