Skip to content

Commit

Permalink
Make InverseProperty work on skip navigations
Browse files Browse the repository at this point in the history
  • Loading branch information
@smitpatel
smitpatel committed Jul 30, 2020
1 parent d1139af commit 00c1ff7
Show file tree
Hide file tree
Showing 6 changed files with 137 additions and 56 deletions.
8 changes: 8 additions & 0 deletions src/EFCore/Metadata/Builders/IConventionEntityTypeBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,14 @@ 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>
/// Returns a value indicating whether the given skinavigation can be added to this entity type.
/// </summary>
/// <param name="skipNavigationName"> The name of the skip navigation. </param>
/// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
/// <returns> <see langword="true" /> if the configuration can be applied. </returns>
bool CanHaveSkipNavigation([NotNull] string skipNavigationName, bool fromDataAnnotation = false);

/// <summary>
/// Configures a skip navigation and the inverse between this and the target entity type.
/// </summary>
Expand Down
153 changes: 100 additions & 53 deletions src/EFCore/Metadata/Conventions/InversePropertyAttributeConvention.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,11 @@ private void Process(
IConventionEntityTypeBuilder entityTypeBuilder, MemberInfo navigationMemberInfo, Type targetClrType,
InversePropertyAttribute attribute)
{
if (!entityTypeBuilder.CanHaveNavigation(
navigationMemberInfo.GetSimpleMemberName(), fromDataAnnotation: true))
var canBeNavigation = entityTypeBuilder.CanHaveNavigation(
navigationMemberInfo.GetSimpleMemberName(), fromDataAnnotation: true);
var canBeSkipNavigation = entityTypeBuilder.CanHaveSkipNavigation(
navigationMemberInfo.GetSimpleMemberName(), fromDataAnnotation: true);
if (!canBeNavigation && !canBeSkipNavigation)
{
return;
}
Expand All @@ -67,14 +70,17 @@ private void Process(
return;
}

ConfigureInverseNavigation(entityTypeBuilder, navigationMemberInfo, targetEntityTypeBuilder, attribute);
ConfigureInverseNavigation(
entityTypeBuilder, navigationMemberInfo, targetEntityTypeBuilder, attribute, canBeNavigation, canBeSkipNavigation);
}

private IConventionForeignKeyBuilder ConfigureInverseNavigation(
IConventionEntityTypeBuilder entityTypeBuilder,
MemberInfo navigationMemberInfo,
IConventionEntityTypeBuilder targetEntityTypeBuilder,
InversePropertyAttribute attribute)
InversePropertyAttribute attribute,
bool canBeNavigation,
bool canBeSkipNavigation)
{
var entityType = entityTypeBuilder.Metadata;
var targetClrType = targetEntityTypeBuilder.Metadata.ClrType;
Expand Down Expand Up @@ -137,53 +143,61 @@ private IConventionForeignKeyBuilder ConfigureInverseNavigation(

if (ambiguousInverse != null)
{
var existingInverse = targetEntityTypeBuilder.Metadata.FindNavigation(inverseNavigationPropertyInfo)?.Inverse;
var existingInverseType = existingInverse?.DeclaringEntityType;
if (existingInverse != null
&& IsAmbiguousInverse(
existingInverse.GetIdentifyingMemberInfo(), existingInverseType, referencingNavigationsWithAttribute))
if (canBeNavigation)
{
var fk = existingInverse.ForeignKey;
if (fk.IsOwnership
|| fk.DeclaringEntityType.Builder.HasNoRelationship(fk, fromDataAnnotation: true) == null)
var existingInverse = targetEntityTypeBuilder.Metadata.FindNavigation(inverseNavigationPropertyInfo)?.Inverse;
var existingInverseType = existingInverse?.DeclaringEntityType;
if (existingInverse != null
&& IsAmbiguousInverse(
existingInverse.GetIdentifyingMemberInfo(), existingInverseType, referencingNavigationsWithAttribute))
{
fk.Builder.HasNavigation(
(string)null,
existingInverse.IsOnDependent,
fromDataAnnotation: true);
var fk = existingInverse.ForeignKey;
if (fk.IsOwnership
|| fk.DeclaringEntityType.Builder.HasNoRelationship(fk, fromDataAnnotation: true) == null)
{
fk.Builder.HasNavigation(
(string)null,
existingInverse.IsOnDependent,
fromDataAnnotation: true);
}
}
}

var existingNavigation = entityType.FindNavigation(navigationMemberInfo);
if (existingNavigation != null)
{
var fk = existingNavigation.ForeignKey;
if (fk.IsOwnership
|| fk.DeclaringEntityType.Builder.HasNoRelationship(fk, fromDataAnnotation: true) == null)
var existingNavigation = entityType.FindNavigation(navigationMemberInfo);
if (existingNavigation != null)
{
fk.Builder.HasNavigation(
(string)null,
existingNavigation.IsOnDependent,
fromDataAnnotation: true);
var fk = existingNavigation.ForeignKey;
if (fk.IsOwnership
|| fk.DeclaringEntityType.Builder.HasNoRelationship(fk, fromDataAnnotation: true) == null)
{
fk.Builder.HasNavigation(
(string)null,
existingNavigation.IsOnDependent,
fromDataAnnotation: true);
}
}
}

var existingAmbiguousNavigation = FindActualEntityType(ambiguousInverse.Value.Item2)
.FindNavigation(ambiguousInverse.Value.Item1);
if (existingAmbiguousNavigation != null)
{
var fk = existingAmbiguousNavigation.ForeignKey;
if (fk.IsOwnership
|| fk.DeclaringEntityType.Builder.HasNoRelationship(fk, fromDataAnnotation: true) == null)
var existingAmbiguousNavigation = FindActualEntityType(ambiguousInverse.Value.Item2)
.FindNavigation(ambiguousInverse.Value.Item1);
if (existingAmbiguousNavigation != null)
{
fk.Builder.HasNavigation(
(string)null,
existingAmbiguousNavigation.IsOnDependent,
fromDataAnnotation: true);
var fk = existingAmbiguousNavigation.ForeignKey;
if (fk.IsOwnership
|| fk.DeclaringEntityType.Builder.HasNoRelationship(fk, fromDataAnnotation: true) == null)
{
fk.Builder.HasNavigation(
(string)null,
existingAmbiguousNavigation.IsOnDependent,
fromDataAnnotation: true);
}
}

return entityType.FindNavigation(navigationMemberInfo)?.ForeignKey.Builder;
}

return entityType.FindNavigation(navigationMemberInfo)?.ForeignKey.Builder;
if (canBeSkipNavigation)
{
// TODO: Not sure what to do
}
}

var ownership = entityType.FindOwnership();
Expand All @@ -195,6 +209,7 @@ private IConventionForeignKeyBuilder ConfigureInverseNavigation(
entityType, navigationMemberInfo,
targetEntityTypeBuilder.Metadata, inverseNavigationPropertyInfo,
ownership.PrincipalToDependent?.GetIdentifyingMemberInfo());

return null;
}

Expand All @@ -206,21 +221,51 @@ private IConventionForeignKeyBuilder ConfigureInverseNavigation(
entityType, navigationMemberInfo,
targetEntityTypeBuilder.Metadata, inverseNavigationPropertyInfo,
entityType.DefiningEntityType.GetRuntimeProperties()[entityType.DefiningNavigationName]);

return null;
}

return entityType.Model.FindIsOwnedConfigurationSource(entityType.ClrType) != null
&& !entityType.IsInOwnershipPath(targetEntityTypeBuilder.Metadata)
? targetEntityTypeBuilder.HasOwnership(
entityTypeBuilder.Metadata.ClrType,
inverseNavigationPropertyInfo,
navigationMemberInfo,
fromDataAnnotation: true)
: targetEntityTypeBuilder.HasRelationship(
entityType,
inverseNavigationPropertyInfo,
navigationMemberInfo,
fromDataAnnotation: true);
if (entityType.Model.FindIsOwnedConfigurationSource(entityType.ClrType) != null
&& !entityType.IsInOwnershipPath(targetEntityTypeBuilder.Metadata))
{
return targetEntityTypeBuilder.HasOwnership(
entityTypeBuilder.Metadata.ClrType,
inverseNavigationPropertyInfo,
navigationMemberInfo,
fromDataAnnotation: true);
}
else
{
if (canBeNavigation)
{
var newForeignKeyBuilder = targetEntityTypeBuilder.HasRelationship(
entityType,
inverseNavigationPropertyInfo,
navigationMemberInfo,
fromDataAnnotation: true);

if (newForeignKeyBuilder != null)
{
return newForeignKeyBuilder;
}
}

if (canBeSkipNavigation
&& navigationMemberInfo is PropertyInfo navigationPropertyInfo)
{
var navigationTargetType = navigationPropertyInfo.PropertyType.TryGetSequenceType();
var inverseTargetType = inverseNavigationPropertyInfo.PropertyType.TryGetSequenceType();
if (navigationTargetType == targetClrType
&& inverseTargetType == entityType.ClrType)
{
entityTypeBuilder.HasSkipNavigation(
navigationPropertyInfo, targetEntityTypeBuilder.Metadata,
inverseNavigationPropertyInfo, collections: true, onDependent: false);
}
}
}

return null;
}

/// <summary>
Expand Down Expand Up @@ -276,7 +321,9 @@ public override void ProcessNavigationAdded(
navigation.DeclaringEntityType.Builder,
navigation.GetIdentifyingMemberInfo(),
navigation.TargetEntityType.Builder,
attribute);
attribute,
canBeNavigation: true,
canBeSkipNavigation: false);

if (newRelationshipBuilder == null)
{
Expand Down
25 changes: 25 additions & 0 deletions src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -968,6 +968,19 @@ public virtual bool CanHaveNavigation([NotNull] string navigationName, Configura
.Concat(Metadata.FindSkipNavigationsInHierarchy(navigationName))
.All(m => configurationSource.Overrides(m.GetConfigurationSource()));

/// <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 CanHaveSkipNavigation([NotNull] string skipNavigationName, ConfigurationSource? configurationSource)
=> !IsIgnored(skipNavigationName, configurationSource)
&& Metadata.FindPropertiesInHierarchy(skipNavigationName).Cast<IConventionPropertyBase>()
.Concat(Metadata.FindServicePropertiesInHierarchy(skipNavigationName))
.Concat(Metadata.FindNavigationsInHierarchy(skipNavigationName))
.All(m => configurationSource.Overrides(m.GetConfigurationSource()));

/// <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 @@ -4871,6 +4884,18 @@ bool IConventionEntityTypeBuilder.CanHaveNavigation(string navigationName, bool
navigationName,
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
/// 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>
[DebuggerStepThrough]
bool IConventionEntityTypeBuilder.CanHaveSkipNavigation(string skipNavigationName, bool fromDataAnnotation)
=> CanHaveSkipNavigation(
skipNavigationName,
fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention);

/// <inheritdoc />
[DebuggerStepThrough]
IConventionSkipNavigationBuilder IConventionEntityTypeBuilder.HasSkipNavigation(
Expand Down
3 changes: 0 additions & 3 deletions test/EFCore.Specification.Tests/Query/ManyToManyQueryFixtureBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,9 +183,6 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
l => l.HasOne(x => x.One).WithMany(e => e.JoinThreePayloadFull))
.HasKey(e => new { e.OneId, e.ThreeId });

// Nav:2 Payload:No Join:Shared Extra:None
modelBuilder.Entity<EntityOne>().HasMany(e => e.TwoSkipShared).WithMany(e => e.OneSkipShared);

// Nav:4 Payload:Yes Join:Shared Extra:None
modelBuilder.Entity<EntityOne>()
.HasMany(e => e.ThreeSkipPayloadFullShared)
Expand Down
2 changes: 2 additions & 0 deletions test/EFCore.Specification.Tests/TestModels/ManyToManyModel/EntityOne.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.ComponentModel.DataAnnotations.Schema;

namespace Microsoft.EntityFrameworkCore.TestModels.ManyToManyModel
{
Expand All @@ -21,6 +22,7 @@ public class EntityOne
public virtual ICollection<JoinOneToThreePayloadFull> JoinThreePayloadFull { get; }
= new ObservableCollection<JoinOneToThreePayloadFull>(); // #21684

[InverseProperty("OneSkipShared")]
public virtual ICollection<EntityTwo> TwoSkipShared { get; } = new ObservableCollection<EntityTwo>(); // #21684

public virtual ICollection<EntityThree> ThreeSkipPayloadFullShared { get; } = new ObservableCollection<EntityThree>(); // #21684
Expand Down
2 changes: 2 additions & 0 deletions test/EFCore.Specification.Tests/TestModels/ManyToManyModel/EntityTwo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.ComponentModel.DataAnnotations.Schema;

namespace Microsoft.EntityFrameworkCore.TestModels.ManyToManyModel
{
Expand All @@ -25,6 +26,7 @@ public class EntityTwo
public virtual ICollection<EntityTwo> SelfSkipSharedLeft { get; } = new ObservableCollection<EntityTwo>(); // #21684
public virtual ICollection<EntityTwo> SelfSkipSharedRight { get; } = new ObservableCollection<EntityTwo>(); // #21684

[InverseProperty("TwoSkipShared")]
public virtual ICollection<EntityOne> OneSkipShared { get; } = new ObservableCollection<EntityOne>(); // #21684

public virtual ICollection<EntityCompositeKey> CompositeKeySkipShared { get; }
Expand Down

0 comments on commit 00c1ff7

Please sign in to comment.