Skip to content

Commit

Permalink
Be more resilient when using the model from the model snapshot
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ajcvickers committed Aug 17, 2019
1 parent 53073a2 commit d020b62
Show file tree
Hide file tree
Showing 16 changed files with 55 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,7 @@ public virtual IModel Process(IModel model)
}
}

return (model is IMutableModel mutableModel)
? mutableModel.FinalizeModel()
: model;
return model;
}

private void ProcessCollection(IEnumerable<IAnnotatable> metadata, string version)
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.InMemory/Storage/Internal/InMemoryTable.cs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ private static List<ValueComparer> GetStructuralComparers(IEnumerable<IProperty>
=> properties.Select(GetStructuralComparer).ToList();

private static ValueComparer GetStructuralComparer(IProperty p)
=> p.GetStructuralValueComparer() ?? p.GetTypeMapping().StructuralComparer;
=> p.GetStructuralValueComparer() ?? p.FindTypeMapping()?.StructuralComparer;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
10 changes: 6 additions & 4 deletions src/EFCore.Relational/Infrastructure/ModelSnapshot.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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();
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1814,8 +1814,8 @@ protected virtual void DiffData(
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()
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore/ChangeTracking/Internal/ChangeDetector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ protected virtual bool TryCreateFromEntry(
/// </summary>
protected static IEqualityComparer<object[]> CreateEqualityComparer([NotNull] IReadOnlyList<IProperty> 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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore/ChangeTracking/Internal/StateManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion src/EFCore/Metadata/Internal/EntityType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2328,7 +2328,7 @@ public virtual IEnumerable<IDictionary<string, object>> GetSeedData(bool provide
{
if (propertyBase is IProperty property)
{
valueConverter = property.GetTypeMapping().Converter
valueConverter = property.FindTypeMapping()?.Converter
?? property.GetValueConverter();
}

Expand Down
2 changes: 1 addition & 1 deletion src/EFCore/Metadata/Internal/EntityTypeExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ public static PropertyCounts GetCounts([NotNull] this IEntityType entityType)
/// </summary>
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;
Expand Down
11 changes: 10 additions & 1 deletion src/EFCore/Metadata/Internal/Model.cs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,15 @@ public Model([NotNull] ConventionSet conventions)
/// </summary>
public virtual ConventionDispatcher ConventionDispatcher { [DebuggerStepThrough] get; private set; }

/// <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 virtual bool IsReadonly
=> ConventionDispatcher == null;

/// <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
Expand Down Expand Up @@ -824,7 +833,7 @@ protected override IConventionAnnotation OnAnnotationSet(
public virtual IModel FinalizeModel()
{
var finalModel = (Model)ConventionDispatcher.OnModelFinalized(Builder)?.Metadata;
return finalModel?.MakeReadonly();
return finalModel?.MakeReadonly() ?? this;
}

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore/ValueGeneration/ValueGeneratorSelector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
9 changes: 9 additions & 0 deletions src/Shared/Check.cs
Original file line number Diff line number Diff line change
Expand Up @@ -93,5 +93,14 @@ public static IReadOnlyList<T> HasNoNulls<T>(IReadOnlyList<T> 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}");
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3642,7 +3642,7 @@ protected void Test(Action<ModelBuilder> buildModel, string expectedCode, Action
Activator.CreateInstance(factoryType),
new object[] { builder });

var modelFromSnapshot = new SnapshotModelProcessor(new TestOperationReporter()).Process(builder.Model);
var modelFromSnapshot = new SnapshotModelProcessor(new TestOperationReporter()).Process(builder.Model.FinalizeModel());

assert(modelFromSnapshot, model);
}
Expand Down
16 changes: 16 additions & 0 deletions test/EFCore.SqlServer.FunctionalTests/MigrationsSqlServerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -1349,6 +1356,15 @@ protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
=> optionsBuilder.UseSqlServer(@"Server=(localdb)\mssqllocaldb;Database=Test;ConnectRetryCount=0");

public DbSet<Blog> Blogs { get; set; }

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.Entity<Blog>().HasData(
new Blog
{
Id = 1, Name = "HalfADonkey"
});
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,11 @@ public void OnModelFinalized_calls_conventions_in_order(bool useBuilder)

if (useBuilder)
{
Assert.Null(new InternalModelBuilder(model).Metadata.FinalizeModel());
Assert.Same(model, new InternalModelBuilder(model).Metadata.FinalizeModel());
}
else
{
Assert.Null(model.FinalizeModel());
Assert.Same(model, model.FinalizeModel());
}

Assert.Equal(1, convention1.Calls);
Expand Down

0 comments on commit d020b62

Please sign in to comment.