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 31, 2020
1 parent d87837a commit d276575
Show file tree
Hide file tree
Showing 15 changed files with 351 additions and 297 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
127 changes: 77 additions & 50 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,57 @@ 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) == null)
{
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;
}
// TODO: Should something be done for skip navigation?
}

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

return null;
}

Expand All @@ -206,21 +211,43 @@ 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 inverseTargetType = inverseNavigationPropertyInfo.PropertyType.TryGetSequenceType();
if (navigationTargetType == targetClrType
&& inverseTargetType == entityType.ClrType)
{
entityTypeBuilder.HasSkipNavigation(
navigationPropertyInfo, targetEntityTypeBuilder.Metadata,
inverseNavigationPropertyInfo, collections: true, onDependent: false, fromDataAnnotation: true);
}
}

return newForeignKeyBuilder;
}
}

/// <summary>
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
2 changes: 2 additions & 0 deletions test/EFCore.Specification.Tests/TestModels/ManyToManyModel/EntityThree.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 Down Expand Up @@ -36,6 +37,7 @@ public class EntityThree
public virtual ICollection<JoinThreeToCompositeKeyFull> JoinCompositeKeyFull { get; }
= new ObservableCollection<JoinThreeToCompositeKeyFull>(); // #21684

[InverseProperty("ThreeSkipShared")]
public virtual ICollection<EntityRoot> RootSkipShared { get; } = new ObservableCollection<EntityRoot>(); // #21684
}
}
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
4 changes: 2 additions & 2 deletions test/EFCore.Specification.Tests/TestModels/ManyToManyModel/JoinOneToBranch.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ namespace Microsoft.EntityFrameworkCore.TestModels.ManyToManyModel
{
public class JoinOneToBranch
{
public virtual int OneId { get; set; }
public virtual int BranchId { get; set; }
public virtual int EntityOneId { get; set; }
public virtual int EntityBranchId { get; set; }
}
}
8 changes: 4 additions & 4 deletions test/EFCore.Specification.Tests/TestModels/ManyToManyModel/ManyToManyData.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@ public ManyToManyData()

foreach (var joinEntity in _joinOneToBranches)
{
var one = _ones.First(o => o.Id == joinEntity.OneId);
var branch = _roots.OfType<EntityBranch>().First(t => t.Id == joinEntity.BranchId);
var one = _ones.First(o => o.Id == joinEntity.EntityOneId);
var branch = _roots.OfType<EntityBranch>().First(t => t.Id == joinEntity.EntityBranchId);
one.BranchSkip.Add(branch);
branch.OneSkip.Add(one);
}
Expand Down Expand Up @@ -629,8 +629,8 @@ private static JoinOneToBranch CreateJoinOneToBranch(
=> CreateInstance(
context?.Set<JoinOneToBranch>(), e =>
{
e.OneId = oneId;
e.BranchId = branchId;
e.EntityOneId = oneId;
e.EntityBranchId = branchId;
});

private static JoinOneToThreePayloadFull[] CreateJoinOneToThreePayloadFulls(ManyToManyContext context)
Expand Down
Loading

0 comments on commit d276575

Please sign in to comment.