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 Aug 1, 2020
1 parent 2d0d999 commit d6acf4d
Show file tree
Hide file tree
Showing 17 changed files with 429 additions and 307 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 navigation base can be added to this entity type.
/// </summary>
/// <param name="navigationBaseName"> The name of the navigation base. </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 CanHaveNavigationBase([NotNull] string navigationBaseName, bool fromDataAnnotation = false);

/// <summary>
/// Configures a skip navigation and the inverse between this and the target entity type.
/// </summary>
Expand Down
162 changes: 111 additions & 51 deletions src/EFCore/Metadata/Conventions/InversePropertyAttributeConvention.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ private void Process(
IConventionEntityTypeBuilder entityTypeBuilder, MemberInfo navigationMemberInfo, Type targetClrType,
InversePropertyAttribute attribute)
{
if (!entityTypeBuilder.CanHaveNavigation(
if (!entityTypeBuilder.CanHaveNavigationBase(
navigationMemberInfo.GetSimpleMemberName(), fromDataAnnotation: true))
{
return;
Expand Down Expand Up @@ -137,53 +137,88 @@ 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 (entityType.FindSkipNavigation(navigationMemberInfo) is IConventionSkipNavigation existingSkipNavigation)
{
var fk = existingInverse.ForeignKey;
if (fk.IsOwnership
|| fk.DeclaringEntityType.Builder.HasNoRelationship(fk, fromDataAnnotation: true) == null)
var inverseSkipNavigation = targetEntityTypeBuilder.Metadata.FindSkipNavigation(inverseNavigationPropertyInfo);
if (inverseSkipNavigation != null)
{
fk.Builder.HasNavigation(
(string)null,
existingInverse.IsOnDependent,
fromDataAnnotation: true);
if (inverseSkipNavigation.Inverse == null
|| inverseSkipNavigation.Inverse.Equals(existingSkipNavigation))
{
existingSkipNavigation.Builder.HasInverse(inverseSkipNavigation, fromDataAnnotation: true);
}
else
{
var existingInverseSkipnavigation = inverseSkipNavigation.Inverse;

entityType.Builder.HasNoSkipNavigation(existingSkipNavigation, fromDataAnnotation: true);
inverseSkipNavigation.DeclaringEntityType.Builder.HasNoSkipNavigation(
inverseSkipNavigation, fromDataAnnotation: true);
existingInverseSkipnavigation.DeclaringEntityType.Builder.HasNoSkipNavigation(
existingInverseSkipnavigation, 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 ambiguousInverseSkipNavigation = FindActualEntityType(ambiguousInverse.Value.Item2)
.FindSkipNavigation(ambiguousInverse.Value.Item1);
if (ambiguousInverseSkipNavigation != null)
{
fk.Builder.HasNavigation(
(string)null,
existingNavigation.IsOnDependent,
fromDataAnnotation: true);
ambiguousInverseSkipNavigation.DeclaringEntityType.Builder.HasNoSkipNavigation(
ambiguousInverseSkipNavigation, fromDataAnnotation: true);
}
}

var existingAmbiguousNavigation = FindActualEntityType(ambiguousInverse.Value.Item2)
.FindNavigation(ambiguousInverse.Value.Item1);
if (existingAmbiguousNavigation != null)
throw new InvalidOperationException("Issue#21890");
}
else
{
var fk = existingAmbiguousNavigation.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,
existingAmbiguousNavigation.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)
{
fk.Builder.HasNavigation(
(string)null,
existingNavigation.IsOnDependent,
fromDataAnnotation: true);
}
}
}

return entityType.FindNavigation(navigationMemberInfo)?.ForeignKey.Builder;
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)
{
fk.Builder.HasNavigation(
(string)null,
existingAmbiguousNavigation.IsOnDependent,
fromDataAnnotation: true);
}
}

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

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

return null;
}

Expand All @@ -206,21 +242,45 @@ 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
{
var newForeignKeyBuilder = targetEntityTypeBuilder.HasRelationship(
entityType,
inverseNavigationPropertyInfo,
navigationMemberInfo,
fromDataAnnotation: true);

if (newForeignKeyBuilder == null
&& navigationMemberInfo is PropertyInfo navigationPropertyInfo)
{
var navigationTargetType = navigationPropertyInfo.PropertyType.TryGetSequenceType();
var inverseNavigationTargetType = inverseNavigationPropertyInfo.PropertyType.TryGetSequenceType();
if (navigationTargetType != null
&& inverseNavigationTargetType != null
&& navigationTargetType.IsAssignableFrom(targetClrType)
&& inverseNavigationTargetType.IsAssignableFrom(entityType.ClrType))
{
entityTypeBuilder.HasSkipNavigation(
navigationPropertyInfo, targetEntityTypeBuilder.Metadata,
inverseNavigationPropertyInfo, collections: true, onDependent: false, fromDataAnnotation: true);
}
}

return newForeignKeyBuilder;
}
}

/// <summary>
Expand Down Expand Up @@ -472,9 +532,9 @@ private static (MemberInfo, IConventionEntityType)? FindAmbiguousInverse(
foreach (var referencingTuple in referencingNavigationsWithAttribute)
{
var inverseTargetEntityType = FindActualEntityType(referencingTuple.TargetEntityType);
if ((inverseTargetEntityType?.Builder.IsIgnored(
if (inverseTargetEntityType?.Builder.IsIgnored(
referencingTuple.Inverse.GetSimpleMemberName(), fromDataAnnotation: true)
!= false))
!= false)
{
if (tuplesToRemove == null)
{
Expand Down
0 ...yToManyAssociationEntityTypeConvention.cs → ...ons/ManyToManyJoinEntityTypeConvention.cs
View file
File renamed without changes.
24 changes: 24 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,18 @@ 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 CanHaveNavigationBase([NotNull] string skipNavigationName, ConfigurationSource? configurationSource)
=> !IsIgnored(skipNavigationName, configurationSource)
&& Metadata.FindPropertiesInHierarchy(skipNavigationName).Cast<IConventionPropertyBase>()
.Concat(Metadata.FindServicePropertiesInHierarchy(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 +4883,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.CanHaveNavigationBase(string navigationBaseName, bool fromDataAnnotation)
=> CanHaveNavigationBase(
navigationBaseName,
fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention);

/// <inheritdoc />
[DebuggerStepThrough]
IConventionSkipNavigationBuilder IConventionEntityTypeBuilder.HasSkipNavigation(
Expand Down
23 changes: 6 additions & 17 deletions test/EFCore.Specification.Tests/Query/ManyToManyQueryFixtureBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,20 +171,15 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
.WithMany(e => e.OneSkip)
.UsingEntity<JoinOneToTwo>(
r => r.HasOne<EntityTwo>().WithMany().HasForeignKey(e => e.TwoId),
l => l.HasOne<EntityOne>().WithMany().HasForeignKey(e => e.OneId))
.HasKey(e => new { e.OneId, e.TwoId });
l => l.HasOne<EntityOne>().WithMany().HasForeignKey(e => e.OneId));

// Nav:6 Payload:Yes Join:Concrete Extra:None
modelBuilder.Entity<EntityOne>()
.HasMany(e => e.ThreeSkipPayloadFull)
.WithMany(e => e.OneSkipPayloadFull)
.UsingEntity<JoinOneToThreePayloadFull>(
r => r.HasOne(x => x.Three).WithMany(e => e.JoinOnePayloadFull),
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);
l => l.HasOne(x => x.One).WithMany(e => e.JoinThreePayloadFull));

// Nav:4 Payload:Yes Join:Shared Extra:None
modelBuilder.Entity<EntityOne>()
Expand All @@ -202,17 +197,15 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
.WithMany(e => e.SelfSkipPayloadRight)
.UsingEntity<JoinOneSelfPayload>(
l => l.HasOne(x => x.Left).WithMany(x => x.JoinSelfPayloadLeft),
r => r.HasOne(x => x.Right).WithMany(x => x.JoinSelfPayloadRight).OnDelete(DeleteBehavior.ClientCascade))
.HasKey(e => new { e.LeftId, e.RightId });
r => r.HasOne(x => x.Right).WithMany(x => x.JoinSelfPayloadRight).OnDelete(DeleteBehavior.ClientCascade));

// Nav:2 Payload:No Join:Concrete Extra:Inheritance
modelBuilder.Entity<EntityOne>()
.HasMany(e => e.BranchSkip)
.WithMany(e => e.OneSkip)
.UsingEntity<JoinOneToBranch>(
r => r.HasOne<EntityBranch>().WithMany().HasForeignKey(e => e.BranchId),
l => l.HasOne<EntityOne>().WithMany().HasForeignKey(e => e.OneId))
.HasKey(e => new { e.BranchId, e.OneId });
r => r.HasOne<EntityBranch>().WithMany(),
l => l.HasOne<EntityOne>().WithMany());

modelBuilder.Entity<EntityTwo>()
.HasOne(e => e.Reference)
Expand All @@ -230,8 +223,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
.WithMany(e => e.TwoSkipFull)
.UsingEntity<JoinTwoToThree>(
r => r.HasOne(x => x.Three).WithMany(e => e.JoinTwoFull),
l => l.HasOne(x => x.Two).WithMany(e => e.JoinThreeFull))
.HasKey(e => new { e.TwoId, e.ThreeId });
l => l.HasOne(x => x.Two).WithMany(e => e.JoinThreeFull));

// Nav:2 Payload:No Join:Shared Extra:Self-ref
modelBuilder.Entity<EntityTwo>()
Expand Down Expand Up @@ -261,9 +253,6 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
r => r.HasOne(x => x.Three).WithMany(x => x.JoinCompositeKeyFull))
.HasKey(e => new { e.ThreeId, e.CompositeId1, e.CompositeId2, e.CompositeId3 });

// Nav:2 Payload:No Join:Shared Extra:Inheritance
modelBuilder.Entity<EntityThree>().HasMany(e => e.RootSkipShared).WithMany(e => e.ThreeSkipShared);

// TODO: convert to shared type
// Nav:2 Payload:No Join:Shared Extra:Inheritance,CompositeKey
modelBuilder.Entity<EntityCompositeKey>()
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
Loading

0 comments on commit d6acf4d

Please sign in to comment.