Skip to content

Commit

Permalink
Pull down relationship to the derived type when setting a navigation …
Browse files Browse the repository at this point in the history 
…targeting the derived type.

Remove ambiguous navigations when the target type is ignored or a base type is set that has the navigation ignored.
Remove ignored properties on the derived entity types when the configuration source of the base property is changed.
Throw correct exception when the collection navigation type doesn't match the dependent entity type.

Fixes #16762
  • Loading branch information
@AndriySvyryd
AndriySvyryd committed Jul 31, 2019
1 parent 8411b60 commit 01fb859
Show file tree
Hide file tree
Showing 12 changed files with 251 additions and 87 deletions.
25 changes: 25 additions & 0 deletions src/EFCore/Extensions/ConventionEntityTypeExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
using JetBrains.Annotations;
Expand Down Expand Up @@ -52,6 +53,30 @@ public static IEnumerable<IConventionEntityType> GetDerivedTypesInclusive([NotNu
public static IEnumerable<IConventionEntityType> GetDirectlyDerivedTypes([NotNull] this IConventionEntityType entityType)
=> ((EntityType)entityType).GetDirectlyDerivedTypes();

/// <summary>
/// Returns all base types of the given <see cref="IEntityType" />, including the type itself, top to bottom.
/// </summary>
/// <param name="entityType"> The entity type. </param>
/// <returns> Base types. </returns>
public static IEnumerable<IConventionEntityType> GetAllBaseTypesInclusive([NotNull] this IConventionEntityType entityType)
=> GetAllBaseTypesInclusiveAscending(entityType).Reverse();

/// <summary>
/// Returns all base types of the given <see cref="IEntityType" />, including the type itself, bottom to top.
/// </summary>
/// <param name="entityType"> The entity type. </param>
/// <returns> Base types. </returns>
public static IEnumerable<IConventionEntityType> GetAllBaseTypesInclusiveAscending([NotNull] this IConventionEntityType entityType)
{
Check.NotNull(entityType, nameof(entityType));

while (entityType != null)
{
yield return entityType;
entityType = entityType.BaseType;
}
}

/// <summary>
/// <para>
/// Gets all keys declared on the given <see cref="IEntityType" />.
Expand Down
14 changes: 2 additions & 12 deletions src/EFCore/Metadata/Builders/ReferenceNavigationBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,23 +178,13 @@ private InternalRelationshipBuilder WithManyBuilder(MemberIdentity collection)
ThrowForConflictingNavigation(foreignKey, collectionName, false);
}

return RelatedEntityType != foreignKey.PrincipalEntityType
? collection.MemberInfo == null && ReferenceMember == null
return collection.MemberInfo == null || ReferenceMember == null
? builder.HasNavigations(
ReferenceName, collection.Name,
(EntityType)RelatedEntityType, (EntityType)DeclaringEntityType, ConfigurationSource.Explicit)
: builder.HasNavigations(
ReferenceMember, collection.MemberInfo,
(EntityType)RelatedEntityType, (EntityType)DeclaringEntityType, ConfigurationSource.Explicit)
: collection.MemberInfo != null
? builder.HasNavigation(
collection.MemberInfo,
pointsToPrincipal: false,
ConfigurationSource.Explicit)
: builder.HasNavigation(
collection.Name,
pointsToPrincipal: false,
ConfigurationSource.Explicit);
(EntityType)RelatedEntityType, (EntityType)DeclaringEntityType, ConfigurationSource.Explicit);
}

/// <summary>
Expand Down
1 change: 1 addition & 0 deletions src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ public virtual ConventionSet CreateConventionSet()
conventionSet.EntityTypeAddedConventions.Add(new DerivedTypeDiscoveryConvention(Dependencies));

conventionSet.EntityTypeIgnoredConventions.Add(inversePropertyAttributeConvention);
conventionSet.EntityTypeIgnoredConventions.Add(relationshipDiscoveryConvention);

var discriminatorConvention = new DiscriminatorConvention(Dependencies);
conventionSet.EntityTypeRemovedConventions.Add(new OwnedTypesConvention(Dependencies));
Expand Down
101 changes: 69 additions & 32 deletions src/EFCore/Metadata/Conventions/RelationshipDiscoveryConvention.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Conventions
/// </summary>
public class RelationshipDiscoveryConvention :
IEntityTypeAddedConvention,
IEntityTypeIgnoredConvention,
IEntityTypeBaseTypeChangedConvention,
INavigationRemovedConvention,
IEntityTypeMemberIgnoredConvention,
Expand Down Expand Up @@ -770,15 +771,24 @@ public virtual void ProcessEntityTypeBaseTypeChanged(
DiscoverRelationships(oldBaseTypeBuilder, context);
}

if (entityTypeBuilder.Metadata.BaseType != newBaseType)
var entityType = entityTypeBuilder.Metadata;
if (entityType.BaseType != newBaseType)
{
return;
}

ApplyOnRelatedEntityTypes(entityTypeBuilder.Metadata, context);
foreach (var entityType in entityTypeBuilder.Metadata.GetDerivedTypesInclusive())
if (newBaseType != null)
{
DiscoverRelationships(entityType.Builder, context);
foreach (var ignoredMember in newBaseType.GetAllBaseTypesInclusive().SelectMany(et => et.GetIgnoredMembers()))
{
ProcessEntityTypeMemberIgnoredOnBase(entityType, ignoredMember);
}
}

ApplyOnRelatedEntityTypes(entityType, context);
foreach (var derivedEntityType in entityType.GetDerivedTypesInclusive())
{
DiscoverRelationships(derivedEntityType.Builder, context);
}
}

Expand Down Expand Up @@ -849,6 +859,28 @@ private static bool IsCandidateNavigationProperty(
&& (!sourceEntityTypeBuilder.Metadata.IsKeyless
|| (memberInfo as PropertyInfo)?.PropertyType.TryGetSequenceType() == null);

/// <summary>
/// Called after an entity type is ignored.
/// </summary>
/// <param name="modelBuilder"> The builder for the model. </param>
/// <param name="name"> The name of the ignored entity type. </param>
/// <param name="type"> The ignored entity type. </param>
/// <param name="context"> Additional information associated with convention execution. </param>
public virtual void ProcessEntityTypeIgnored(
IConventionModelBuilder modelBuilder, string name, Type type, IConventionContext<string> context)
{
foreach (var entityType in modelBuilder.Metadata.GetEntityTypes())
{
// Only run the convention if an ambiguity might have been removed
var ambiguityRemoved = RemoveAmbiguous(entityType, type);

if (ambiguityRemoved)
{
DiscoverRelationships(entityType.Builder, context);
}
}
}

/// <summary>
/// Called after an entity type member is ignored.
/// </summary>
Expand All @@ -861,43 +893,48 @@ public virtual void ProcessEntityTypeMemberIgnored(
var anyAmbiguityRemoved = false;
foreach (var derivedEntityType in entityTypeBuilder.Metadata.GetDerivedTypesInclusive())
{
var ambiguousNavigations = GetAmbiguousNavigations(derivedEntityType);
if (ambiguousNavigations == null)
{
continue;
}

KeyValuePair<MemberInfo, Type>? ambiguousNavigation = null;
foreach (var navigation in ambiguousNavigations)
{
if (navigation.Key.GetSimpleMemberName() == name)
{
ambiguousNavigation = navigation;
}
}

if (ambiguousNavigation == null)
{
continue;
}
anyAmbiguityRemoved |= ProcessEntityTypeMemberIgnoredOnBase(derivedEntityType, name);
}

anyAmbiguityRemoved = true;
if (anyAmbiguityRemoved)
{
DiscoverRelationships(entityTypeBuilder, context);
}
}

var targetClrType = ambiguousNavigation.Value.Value;
RemoveAmbiguous(derivedEntityType, targetClrType);
private bool ProcessEntityTypeMemberIgnoredOnBase(IConventionEntityType entityType, string name)
{
var ambiguousNavigations = GetAmbiguousNavigations(entityType);
if (ambiguousNavigations == null)
{
return false;
}

var targetType = ((InternalEntityTypeBuilder)entityTypeBuilder)
.GetTargetEntityTypeBuilder(targetClrType, ambiguousNavigation.Value.Key, null)?.Metadata;
if (targetType != null)
KeyValuePair<MemberInfo, Type>? ambiguousNavigation = null;
foreach (var navigation in ambiguousNavigations)
{
if (navigation.Key.GetSimpleMemberName() == name)
{
RemoveAmbiguous(targetType, derivedEntityType.ClrType);
ambiguousNavigation = navigation;
}
}

if (anyAmbiguityRemoved)
if (ambiguousNavigation == null)
{
DiscoverRelationships(entityTypeBuilder, context);
return false;
}

var targetClrType = ambiguousNavigation.Value.Value;
RemoveAmbiguous(entityType, targetClrType);

var targetType = ((InternalEntityTypeBuilder)entityType.Builder)
.GetTargetEntityTypeBuilder(targetClrType, ambiguousNavigation.Value.Key, null)?.Metadata;
if (targetType != null)
{
RemoveAmbiguous(targetType, entityType.ClrType);
}

return true;
}

/// <summary>
Expand Down
18 changes: 15 additions & 3 deletions src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ private InternalPropertyBuilder Property(
ConfigurationSource? typeConfigurationSource,
ConfigurationSource? configurationSource)
{
IEnumerable<Property> propertiesToDetach = null;
List<Property> propertiesToDetach = null;
var existingProperty = Metadata.FindProperty(propertyName);
if (existingProperty != null)
{
Expand All @@ -456,7 +456,7 @@ private InternalPropertyBuilder Property(
{
if (configurationSource.Overrides(existingProperty.GetConfigurationSource()))
{
propertiesToDetach = new[]
propertiesToDetach = new List<Property>
{
existingProperty
};
Expand Down Expand Up @@ -527,7 +527,19 @@ private InternalPropertyBuilder Property(

Metadata.RemoveIgnored(propertyName);

propertiesToDetach = Metadata.FindDerivedProperties(propertyName);
foreach (var derivedType in Metadata.GetDerivedTypes())
{
var derivedProperty = derivedType.FindDeclaredProperty(propertyName);
if (derivedProperty != null)
{
if (propertiesToDetach == null)
{
propertiesToDetach = new List<Property>();
}

propertiesToDetach.Add(derivedProperty);
}
}
}

InternalPropertyBuilder builder;
Expand Down
49 changes: 43 additions & 6 deletions src/EFCore/Metadata/Internal/InternalRelationshipBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,8 @@ private InternalRelationshipBuilder HasNavigations(
out var shouldInvert,
out var shouldBeUnique,
out var removeOppositeNavigation,
out var removeConflictingNavigations))
out var removeConflictingNavigations,
out var changeRelatedTypes))
{
return null;
}
Expand Down Expand Up @@ -280,7 +281,8 @@ private InternalRelationshipBuilder HasNavigations(

InternalRelationshipBuilder builder;
if (shouldInvert == true
|| removeConflictingNavigations)
|| removeConflictingNavigations
|| changeRelatedTypes)
{
builder = ReplaceForeignKey(
configurationSource,
Expand All @@ -291,7 +293,7 @@ private InternalRelationshipBuilder HasNavigations(
dependentProperties,
principalProperties: principalProperties,
isUnique: shouldBeUnique,
removeCurrent: shouldInvert ?? false,
removeCurrent: shouldInvert == true || changeRelatedTypes,
principalEndConfigurationSource: shouldInvert != null ? configurationSource : (ConfigurationSource?)null,
oldRelationshipInverted: shouldInvert == true);

Expand Down Expand Up @@ -546,7 +548,8 @@ private bool CanSetNavigations(
out shouldInvert,
out shouldBeUnique,
out removeOppositeNavigation,
out removeConflictingNavigations);
out removeConflictingNavigations,
out _);

private bool CanSetNavigations(
MemberIdentity? navigationToPrincipal,
Expand All @@ -559,12 +562,14 @@ private bool CanSetNavigations(
out bool? shouldInvert,
out bool? shouldBeUnique,
out bool removeOppositeNavigation,
out bool removeConflictingNavigations)
out bool removeConflictingNavigations,
out bool changeRelatedTypes)
{
shouldInvert = null;
shouldBeUnique = null;
removeOppositeNavigation = false;
removeConflictingNavigations = false;
changeRelatedTypes = false;

if ((navigationToPrincipal == null
|| navigationToPrincipal.Value.Name == Metadata.DependentToPrincipal?.Name)
Expand Down Expand Up @@ -736,6 +741,38 @@ private bool CanSetNavigations(
dependentProperties: null,
principalProperties: null).Where(r => r.Metadata != Metadata).Distinct().Any();

if (shouldInvert == false
&& !removeConflictingNavigations
&& (principalEntityType != Metadata.PrincipalEntityType
|| dependentEntityType != Metadata.DeclaringEntityType))
{
if (navigationToPrincipalProperty != null
&& !IsCompatible(
navigationToPrincipalProperty,
pointsToPrincipal: true,
Metadata.DeclaringEntityType.ClrType,
Metadata.PrincipalEntityType.ClrType,
shouldThrow: false,
out _))
{
changeRelatedTypes = true;
return true;
}

if (navigationToDependentProperty != null
&& !IsCompatible(
navigationToDependentProperty,
pointsToPrincipal: false,
Metadata.DeclaringEntityType.ClrType,
Metadata.PrincipalEntityType.ClrType,
shouldThrow: false,
out _))
{
changeRelatedTypes = true;
return true;
}
}

return true;
}

Expand Down Expand Up @@ -775,7 +812,7 @@ private static bool IsCompatible(
navigationProperty,
principalType,
dependentType,
shouldBeCollection: false,
shouldBeCollection: null,
shouldThrow: true);
}

Expand Down
Loading

0 comments on commit 01fb859

Please sign in to comment.