Skip to content

Commit

Permalink
Add IConventionEntityTypeBuilder.HasSkipNavigation that takes an inverse
Browse files Browse the repository at this point in the history 
Fixes #21486
  • Loading branch information
@AndriySvyryd
AndriySvyryd committed Jul 30, 2020
1 parent 2d5c580 commit 943dff1
Show file tree
Hide file tree
Showing 5 changed files with 144 additions and 84 deletions.
34 changes: 30 additions & 4 deletions src/EFCore/Metadata/Builders/IConventionEntityTypeBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,32 @@ bool CanAddNavigation([NotNull] string navigationName, bool fromDataAnnotation =
/// <returns> <see langword="true" /> if the configuration can be applied. </returns>
bool CanHaveNavigation([NotNull] string navigationName, bool fromDataAnnotation = false);

/// <summary>
/// Configures a skip navigation and the inverse between this and the target entity type.
/// </summary>
/// <param name="targetEntityType"> The entity type that this relationship targets. </param>
/// <param name="navigationToTarget"> The navigation property on this entity type that is part of the relationship. </param>
/// <param name="inverseNavigation">
/// The navigation property on the target entity type that is part of the relationship. If <see langword="null" />
/// is specified, the relationship will be configured without a navigation property on the target end.
/// </param>
/// <param name="collections"> Whether both of the navigation properties are collections or aren't collections. </param>
/// <param name="onDependent">
/// Whether both of the navigation property are defined on the dependent side of the underlying foreign keys.
/// </param>
/// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
/// <returns>
/// An object that can be used to configure the relationship if it exists on the entity type,
/// <see langword="null" /> otherwise.
/// </returns>
IConventionSkipNavigationBuilder HasSkipNavigation(
[NotNull] MemberInfo navigationToTarget,
[NotNull] IConventionEntityType targetEntityType,
[NotNull] MemberInfo inverseNavigation,
bool? collections = null,
bool? onDependent = null,
bool fromDataAnnotation = false);

/// <summary>
/// Configures a skip navigation between this and the target entity type.
/// </summary>
Expand All @@ -636,8 +662,8 @@ bool CanAddNavigation([NotNull] string navigationName, bool fromDataAnnotation =
IConventionSkipNavigationBuilder HasSkipNavigation(
[NotNull] MemberInfo navigationToTarget,
[NotNull] IConventionEntityType targetEntityType,
bool collection = true,
bool onDependent = false,
bool? collection = null,
bool? onDependent = null,
bool fromDataAnnotation = false);

/// <summary>
Expand All @@ -657,8 +683,8 @@ IConventionSkipNavigationBuilder HasSkipNavigation(
IConventionSkipNavigationBuilder HasSkipNavigation(
[NotNull] string navigationName,
[NotNull] IConventionEntityType targetEntityType,
bool collection = true,
bool onDependent = false,
bool? collection = null,
bool? onDependent = null,
bool fromDataAnnotation = false);

/// <summary>
Expand Down
49 changes: 7 additions & 42 deletions src/EFCore/Metadata/Conventions/RelationshipDiscoveryConvention.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -664,12 +664,15 @@ private void CreateRelationships(
{
entityTypeBuilder.HasOwnership(targetEntityType.ClrType, navigation, inverse);
}
else
else if (entityTypeBuilder.HasRelationship(targetEntityType, navigation, inverse) == null)
{
if (entityTypeBuilder.HasRelationship(targetEntityType, navigation, inverse) == null)
var navigationTargetType = navigation.PropertyType.TryGetSequenceType();
var inverseTargetType = inverse.PropertyType.TryGetSequenceType();
if (navigationTargetType == targetEntityType.ClrType
&& inverseTargetType == entityType.ClrType)
{
HasManyToManyRelationship(
entityTypeBuilder, relationshipCandidate.TargetTypeBuilder, navigation, inverse);
entityTypeBuilder.HasSkipNavigation(
navigation, targetEntityType, inverse, collections: true, onDependent: false);
}
}
}
Expand Down Expand Up @@ -750,44 +753,6 @@ private void RemoveNavigation(
}
}

private void HasManyToManyRelationship(
IConventionEntityTypeBuilder entityTypeBuilder,
IConventionEntityTypeBuilder targetEntityTypeBuilder,
PropertyInfo navigation,
PropertyInfo inverse)
{
var navigationTargetType = navigation.PropertyType.TryGetSequenceType();
var inverseTargetType = inverse.PropertyType.TryGetSequenceType();
if (navigationTargetType == null
|| inverseTargetType == null)
{
return;
}

var entityType = entityTypeBuilder.Metadata;
var targetEntityType = targetEntityTypeBuilder.Metadata;
if (navigationTargetType != targetEntityType.ClrType
|| inverseTargetType != entityType.ClrType)
{
return;
}

var skipNavigationBuilder = entityTypeBuilder.HasSkipNavigation(navigation, targetEntityType);
if (skipNavigationBuilder == null)
{
return;
}

var inverseSkipNavigationBuilder = targetEntityTypeBuilder.HasSkipNavigation(inverse, entityType);
if (inverseSkipNavigationBuilder == null)
{
entityTypeBuilder.HasNoSkipNavigation(skipNavigationBuilder.Metadata);
return;
}

skipNavigationBuilder.HasInverse(inverseSkipNavigationBuilder.Metadata, fromDataAnnotation: false);
}

/// <inheritdoc />
public virtual void ProcessEntityTypeAdded(
IConventionEntityTypeBuilder entityTypeBuilder, IConventionContext<IConventionEntityTypeBuilder> context)
Expand Down
8 changes: 8 additions & 0 deletions src/EFCore/Metadata/Internal/EntityType.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1663,6 +1663,14 @@ public virtual SkipNavigation AddSkipNavigation(
shouldThrow: true);
}

Navigation.IsCompatible(
name,
memberInfo,
this,
targetEntityType,
collection,
shouldThrow: true);

var skipNavigation = new SkipNavigation(
name,
memberInfo as PropertyInfo,
Expand Down
121 changes: 89 additions & 32 deletions src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3519,6 +3519,38 @@ private ForeignKey SetOrAddForeignKey(
return foreignKey;
}

/// <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 InternalSkipNavigationBuilder HasSkipNavigation(
MemberIdentity navigationToTarget,
[NotNull] EntityType targetEntityType,
MemberIdentity inverseNavigation,
ConfigurationSource configurationSource,
bool? collections = null,
bool? onDependent = null)
{
var skipNavigationBuilder = HasSkipNavigation(
navigationToTarget, targetEntityType, configurationSource, collections, onDependent);
if (skipNavigationBuilder == null)
{
return null;
}

var inverseSkipNavigationBuilder = targetEntityType.Builder.HasSkipNavigation(
inverseNavigation, Metadata, configurationSource, collections, onDependent);
if (inverseSkipNavigationBuilder == null)
{
HasNoSkipNavigation(skipNavigationBuilder.Metadata, configurationSource);
return null;
}

return skipNavigationBuilder.HasInverse(inverseSkipNavigationBuilder.Metadata, configurationSource);
}

/// <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 All @@ -3529,8 +3561,8 @@ public virtual InternalSkipNavigationBuilder HasSkipNavigation(
MemberIdentity navigationProperty,
[NotNull] EntityType targetEntityType,
ConfigurationSource? configurationSource,
bool collection = true,
bool onDependent = false)
bool? collection = null,
bool? onDependent = null)
{
List<SkipNavigation> navigationsToDetach = null;
List<(InternalSkipNavigationBuilder Navigation, InternalSkipNavigationBuilder Inverse)> detachedNavigations = null;
Expand All @@ -3542,11 +3574,11 @@ public virtual InternalSkipNavigationBuilder HasSkipNavigation(
Check.DebugAssert(memberInfo == null || memberInfo.IsSameAs(existingNavigation.GetIdentifyingMemberInfo()),
"Expected memberInfo to be the same on the existing navigation");

Check.DebugAssert(collection == existingNavigation.IsCollection,
"Only collection skip navigations are currently supported");
Check.DebugAssert(collection == null || collection == existingNavigation.IsCollection,
"Expected existing navigation to have the same cardinality");

Check.DebugAssert(onDependent == existingNavigation.IsOnDependent,
"Only skip navigations on principals are currently supported");
Check.DebugAssert(onDependent == null || onDependent == existingNavigation.IsOnDependent,
"Expected existing navigation to be on the same side");

if (existingNavigation.DeclaringEntityType != Metadata)
{
Expand All @@ -3563,39 +3595,47 @@ public virtual InternalSkipNavigationBuilder HasSkipNavigation(

return existingNavigation.Builder;
}
else

if (!configurationSource.HasValue
|| IsIgnored(navigationName, configurationSource))
{
if (!configurationSource.HasValue
|| IsIgnored(navigationName, configurationSource))
return null;
}

foreach (var conflictingMember in Metadata.FindPropertiesInHierarchy(navigationName).Cast<PropertyBase>()
.Concat(Metadata.FindNavigationsInHierarchy(navigationName))
.Concat(Metadata.FindServicePropertiesInHierarchy(navigationName)))
{
if (!configurationSource.Overrides(conflictingMember.GetConfigurationSource()))
{
return null;
}
}

foreach (var conflictingMember in Metadata.FindPropertiesInHierarchy(navigationName).Cast<PropertyBase>()
.Concat(Metadata.FindNavigationsInHierarchy(navigationName))
.Concat(Metadata.FindServicePropertiesInHierarchy(navigationName)))
foreach (var derivedType in Metadata.GetDerivedTypes())
{
var conflictingNavigation = derivedType.FindDeclaredSkipNavigation(navigationName);
if (conflictingNavigation != null)
{
if (!configurationSource.Overrides(conflictingMember.GetConfigurationSource()))
if (navigationsToDetach == null)
{
return null;
navigationsToDetach = new List<SkipNavigation>();
}
}

foreach (var derivedType in Metadata.GetDerivedTypes())
{
var conflictingNavigation = derivedType.FindDeclaredSkipNavigation(navigationName);
if (conflictingNavigation != null)
{
if (navigationsToDetach == null)
{
navigationsToDetach = new List<SkipNavigation>();
}

navigationsToDetach.Add(conflictingNavigation);
}
navigationsToDetach.Add(conflictingNavigation);
}
}

if (collection == null
&& navigationProperty.MemberInfo != null)
{
var navigationType = navigationProperty.MemberInfo.GetMemberType();
var navigationTargetClrType = navigationType.TryGetSequenceType();
collection = navigationTargetClrType != null
&& navigationType != targetEntityType.ClrType
&& navigationTargetClrType.IsAssignableFrom(targetEntityType.ClrType);
}

InternalSkipNavigationBuilder builder;
using (ModelBuilder.Metadata.ConventionDispatcher.DelayConventions())
{
Expand Down Expand Up @@ -3650,7 +3690,7 @@ public virtual InternalSkipNavigationBuilder HasSkipNavigation(

builder = Metadata.AddSkipNavigation(
navigationName, navigationProperty.MemberInfo,
targetEntityType, collection, onDependent, configurationSource.Value).Builder;
targetEntityType, collection ?? true, onDependent ?? false, configurationSource.Value).Builder;

if (detachedNavigations != null)
{
Expand Down Expand Up @@ -4703,6 +4743,23 @@ IConventionForeignKeyBuilder IConventionEntityTypeBuilder.HasRelationship(
fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention,
setTargetAsPrincipal);

/// <inheritdoc />
[DebuggerStepThrough]
IConventionSkipNavigationBuilder IConventionEntityTypeBuilder.HasSkipNavigation(
MemberInfo navigationToTarget,
IConventionEntityType targetEntityType,
MemberInfo inverseNavigation,
bool? collections,
bool? onDependent,
bool fromDataAnnotation)
=> HasSkipNavigation(
MemberIdentity.Create(navigationToTarget),
(EntityType)targetEntityType,
MemberIdentity.Create(inverseNavigation),
collections,
onDependent,
fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention);

/// <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 @@ -4819,8 +4876,8 @@ bool IConventionEntityTypeBuilder.CanHaveNavigation(string navigationName, bool
IConventionSkipNavigationBuilder IConventionEntityTypeBuilder.HasSkipNavigation(
MemberInfo navigationToTarget,
IConventionEntityType targetEntityType,
bool collection,
bool onDependent,
bool? collection,
bool? onDependent,
bool fromDataAnnotation)
=> HasSkipNavigation(
MemberIdentity.Create(navigationToTarget),
Expand All @@ -4834,8 +4891,8 @@ IConventionSkipNavigationBuilder IConventionEntityTypeBuilder.HasSkipNavigation(
IConventionSkipNavigationBuilder IConventionEntityTypeBuilder.HasSkipNavigation(
string navigationName,
IConventionEntityType targetEntityType,
bool collection,
bool onDependent,
bool? collection,
bool? onDependent,
bool fromDataAnnotation)
=> HasSkipNavigation(
MemberIdentity.Create(navigationName),
Expand Down
16 changes: 10 additions & 6 deletions src/EFCore/Metadata/Internal/SkipNavigation.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -336,60 +336,64 @@ public virtual DebugView DebugView
[DebuggerStepThrough]
public override string ToString() => this.ToDebugString(MetadataDebugStringOptions.SingleLineDefault);

/// <inheritdoc />
IConventionSkipNavigationBuilder IConventionSkipNavigation.Builder
{
[DebuggerStepThrough]
get => Builder;
}

/// <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>
/// <inheritdoc />
IConventionAnnotatableBuilder IConventionAnnotatable.Builder
{
[DebuggerStepThrough]
get => Builder;
}

/// <inheritdoc />
IEntityType INavigationBase.DeclaringEntityType
{
[DebuggerStepThrough]
get => DeclaringEntityType;
}

/// <inheritdoc />
IEntityType INavigationBase.TargetEntityType
{
[DebuggerStepThrough]
get => TargetEntityType;
}

/// <inheritdoc />
IForeignKey ISkipNavigation.ForeignKey
{
[DebuggerStepThrough]
get => ForeignKey;
}

/// <inheritdoc />
[DebuggerStepThrough]
void IMutableSkipNavigation.SetForeignKey([CanBeNull] IMutableForeignKey foreignKey)
=> SetForeignKey((ForeignKey)foreignKey, ConfigurationSource.Explicit);

/// <inheritdoc />
[DebuggerStepThrough]
IConventionForeignKey IConventionSkipNavigation.SetForeignKey([CanBeNull] IConventionForeignKey foreignKey, bool fromDataAnnotation)
=> SetForeignKey((ForeignKey)foreignKey, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention);

/// <inheritdoc />
ISkipNavigation ISkipNavigation.Inverse
{
[DebuggerStepThrough]
get => Inverse;
}

/// <inheritdoc />
[DebuggerStepThrough]
IMutableSkipNavigation IMutableSkipNavigation.SetInverse([CanBeNull] IMutableSkipNavigation inverse)
=> SetInverse((SkipNavigation)inverse, ConfigurationSource.Explicit);

/// <inheritdoc />
[DebuggerStepThrough]
IConventionSkipNavigation IConventionSkipNavigation.SetInverse([CanBeNull] IConventionSkipNavigation inverse, bool fromDataAnnotation)
=> SetInverse((SkipNavigation)inverse, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention);
Expand Down

0 comments on commit 943dff1

Please sign in to comment.