Skip to content

Commit

Permalink
ModelValidator: Throw for unmapped initialized collection navigation
Browse files Browse the repository at this point in the history
We were not checking collection navigations which are initialized inline and without setter.
This worked fine for most part as
- One sided collection were always added by convention
- When there were multiple pairs and convention failed, opposite side had reference navigation which would end up throwing
- Many to Many brings a twist that there is no reference on other side so in case we fail to add many to many skip navigations by convention, we would not throw.
  • Loading branch information
smitpatel committed Jul 31, 2020
1 parent 2720e3f commit 1279224
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 1 deletion.
16 changes: 15 additions & 1 deletion src/EFCore/Infrastructure/ModelValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ namespace Microsoft.EntityFrameworkCore.Infrastructure
/// </summary>
public class ModelValidator : IModelValidator
{
private static readonly IEnumerable<string> _dictionaryProperties =
typeof(IDictionary<string, object>).GetRuntimeProperties().Select(e => e.Name);

/// <summary>
/// Creates a new instance of <see cref="ModelValidator" />.
/// </summary>
Expand Down Expand Up @@ -169,13 +172,17 @@ protected virtual void ValidatePropertyMapping(

clrProperties.UnionWith(
runtimeProperties.Values
.Where(pi => pi.IsCandidateProperty())
.Where(pi => pi.IsCandidateProperty(needsWrite: false))
.Select(pi => pi.GetSimpleMemberName()));

clrProperties.ExceptWith(entityType.GetProperties().Select(p => p.Name));
clrProperties.ExceptWith(entityType.GetNavigations().Select(p => p.Name));
clrProperties.ExceptWith(entityType.GetSkipNavigations().Select(p => p.Name));
clrProperties.ExceptWith(entityType.GetServiceProperties().Select(p => p.Name));
if (entityType.IsPropertyBag)
{
clrProperties.ExceptWith(_dictionaryProperties);
}
clrProperties.RemoveWhere(p => entityType.FindIgnoredConfigurationSource(p) != null);

if (clrProperties.Count <= 0)
Expand Down Expand Up @@ -205,6 +212,13 @@ protected virtual void ValidatePropertyMapping(

var targetType = FindCandidateNavigationPropertyType(actualProperty);

if ((targetType == null // Not a navigation
|| targetSequenceType == null) // Not a collection navigation
&& actualProperty.FindSetterProperty() == null) // No setter property
{
continue;
}

var isTargetWeakOrOwned
= targetType != null
&& (conventionModel.HasEntityTypeWithDefiningNavigation(targetType)
Expand Down
13 changes: 13 additions & 0 deletions test/EFCore.Tests/ModelBuilding/ManyToManyTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,19 @@ public virtual void UsingEntity_with_shared_type_passes_when_configured_as_share
Assert.Equal("Shared", joinEntityType.Name);
Assert.Equal(typeof(ManyToManyJoinWithFields), joinEntityType.ClrType);
}

[ConditionalFact]
public virtual void Unconfigured_many_to_many_navigations_throw()
{
var modelBuilder = CreateModelBuilder();

modelBuilder.Entity<AmbigiousManyToManyImplicitLeft>();

Assert.Equal(
CoreStrings.NavigationNotAdded(typeof(AmbigiousManyToManyImplicitLeft).DisplayName(fullName: false), "Navigation1",
typeof(List<AmbigiousManyToManyImplicitRight>).DisplayName(fullName: false)),
Assert.Throws<InvalidOperationException>(() => modelBuilder.FinalizeModel()).Message);
}
}
}
}
14 changes: 14 additions & 0 deletions test/EFCore.Tests/ModelBuilding/TestModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1106,5 +1106,19 @@ protected class CollectionNavigationToSharedType
public int Id { get; set; }
public List<Dictionary<string, object>> Navigation { get; set; }
}

protected class AmbigiousManyToManyImplicitLeft
{
public int Id { get; set; }
public List<AmbigiousManyToManyImplicitRight> Navigation1 { get; } = new List<AmbigiousManyToManyImplicitRight>();
public List<AmbigiousManyToManyImplicitRight> Navigation2 { get; } = new List<AmbigiousManyToManyImplicitRight>();
}

protected class AmbigiousManyToManyImplicitRight
{
public int Id { get; set; }
public List<AmbigiousManyToManyImplicitLeft> Navigation1 { get; } = new List<AmbigiousManyToManyImplicitLeft>();
public List<AmbigiousManyToManyImplicitLeft> Navigation2 { get; } = new List<AmbigiousManyToManyImplicitLeft>();
}
}
}

0 comments on commit 1279224

Please sign in to comment.