Add a test for #16704

Some cleanup in relationship builders Resolves #16704
dotnet · Aug 19, 2019 · 353152a · 353152a
1 parent 1fd2f52
commit 353152a
Show file tree

Hide file tree

Showing 4 changed files with 88 additions and 50 deletions.
diff --git a/src/EFCore/Metadata/Builders/CollectionNavigationBuilder.cs b/src/EFCore/Metadata/Builders/CollectionNavigationBuilder.cs
@@ -178,26 +178,15 @@ private InternalRelationshipBuilder WithOneBuilder(MemberIdentity reference)
                         foreignKey.DependentToPrincipal.Name));
             }
 
-            return referenceName != null
-                   && RelatedEntityType != foreignKey.DeclaringEntityType
-                ? reference.MemberInfo == null && CollectionMember == null
-                    ? Builder.HasNavigations(
-                        reference.Name, CollectionName,
-                        (EntityType)DeclaringEntityType, (EntityType)RelatedEntityType,
-                        ConfigurationSource.Explicit)
-                    : Builder.HasNavigations(
-                        reference.MemberInfo, CollectionMember,
-                        (EntityType)DeclaringEntityType, (EntityType)RelatedEntityType,
-                        ConfigurationSource.Explicit)
-                : reference.MemberInfo == null
-                    ? Builder.HasNavigation(
-                        reference.Name,
-                        pointsToPrincipal: true,
-                        ConfigurationSource.Explicit)
-                    : Builder.HasNavigation(
-                        reference.MemberInfo,
-                        pointsToPrincipal: true,
-                        ConfigurationSource.Explicit);
+            return reference.MemberInfo == null || CollectionMember == null
+                ? Builder.HasNavigations(
+                    reference.Name, CollectionName,
+                    (EntityType)DeclaringEntityType, (EntityType)RelatedEntityType,
+                    ConfigurationSource.Explicit)
+                : Builder.HasNavigations(
+                    reference.MemberInfo, CollectionMember,
+                    (EntityType)DeclaringEntityType, (EntityType)RelatedEntityType,
+                    ConfigurationSource.Explicit);
         }
 
         #region Hidden System.Object members

diff --git a/src/EFCore/Metadata/Builders/ReferenceNavigationBuilder.cs b/src/EFCore/Metadata/Builders/ReferenceNavigationBuilder.cs
@@ -274,42 +274,26 @@ private InternalRelationshipBuilder WithOneBuilder(MemberIdentity reference)
                 }
 
                 var referenceProperty = reference.MemberInfo;
-                if (referenceName != null
-                    && pointsToPrincipal
-                    && RelatedEntityType != foreignKey.DeclaringEntityType)
+                if (pointsToPrincipal)
                 {
-                    builder = referenceProperty == null && ReferenceMember == null
+                    builder = referenceProperty == null || ReferenceMember == null
                         ? builder.HasNavigations(
                             referenceName, ReferenceName,
                             (EntityType)DeclaringEntityType, (EntityType)RelatedEntityType, ConfigurationSource.Explicit)
                         : builder.HasNavigations(
                             referenceProperty, ReferenceMember,
                             (EntityType)DeclaringEntityType, (EntityType)RelatedEntityType, ConfigurationSource.Explicit);
                 }
-                else if (referenceName != null
-                         && !pointsToPrincipal
-                         && RelatedEntityType != foreignKey.PrincipalEntityType)
+                else
                 {
-                    builder = referenceProperty == null && ReferenceMember == null
+                    builder = referenceProperty == null || ReferenceMember == null
                         ? builder.HasNavigations(
                             ReferenceName, referenceName,
                             (EntityType)RelatedEntityType, (EntityType)DeclaringEntityType, ConfigurationSource.Explicit)
                         : builder.HasNavigations(
                             ReferenceMember, referenceProperty,
                             (EntityType)RelatedEntityType, (EntityType)DeclaringEntityType, ConfigurationSource.Explicit);
                 }
-                else
-                {
-                    builder = referenceProperty != null
-                        ? builder.HasNavigation(
-                            referenceProperty,
-                            pointsToPrincipal,
-                            ConfigurationSource.Explicit)
-                        : builder.HasNavigation(
-                            referenceName,
-                            pointsToPrincipal,
-                            ConfigurationSource.Explicit);
-                }
 
                 return batch.Run(builder);
             }

diff --git a/src/EFCore/Metadata/Internal/InternalRelationshipBuilder.cs b/src/EFCore/Metadata/Internal/InternalRelationshipBuilder.cs
@@ -2551,8 +2551,10 @@ private InternalRelationshipBuilder GetOrCreateRelationshipBuilder(
                 navigationToPrincipal,
                 navigationToDependent,
                 dependentProperties,
-                principalProperties ?? principalEntityType.FindPrimaryKey()?.Properties);
-            matchingRelationships = matchingRelationships.Where(r => r.Metadata != Metadata).Distinct().ToList();
+                principalProperties ?? principalEntityType.FindPrimaryKey()?.Properties)
+                .Where(r => r.Metadata != Metadata)
+                .Distinct()
+                .ToList();
 
             var unresolvableRelationships = new List<InternalRelationshipBuilder>();
             var resolvableRelationships = new List<Tuple<InternalRelationshipBuilder, bool, Resolution, bool>>();
@@ -2803,7 +2805,7 @@ private InternalRelationshipBuilder GetOrCreateRelationshipBuilder(
 
             // This workaround prevents the properties to be cleaned away before the new FK is created,
             // this should be replaced with reference counting
-            // Issue #214
+            // Issue #15898
             var temporaryProperties = dependentProperties?.Where(
                 p => p.GetConfigurationSource() == ConfigurationSource.Convention
                      && p.IsShadowProperty()).ToList();
@@ -2892,13 +2894,13 @@ private InternalRelationshipBuilder GetOrCreateRelationshipBuilder(
                     continue;
                 }
 
-                var navigationLessForeignKey = resolvableRelationship.Metadata;
-                if (navigationLessForeignKey.DependentToPrincipal == null
-                    && navigationLessForeignKey.PrincipalToDependent == null
-                    && navigationLessForeignKey.DeclaringEntityType.Builder.HasNoRelationship(
-                        navigationLessForeignKey, ConfigurationSource.Convention) != null)
+                var navigationlessForeignKey = resolvableRelationship.Metadata;
+                if (navigationlessForeignKey.DependentToPrincipal == null
+                    && navigationlessForeignKey.PrincipalToDependent == null
+                    && navigationlessForeignKey.DeclaringEntityType.Builder.HasNoRelationship(
+                        navigationlessForeignKey, ConfigurationSource.Convention) != null)
                 {
-                    removedForeignKeys.Add(navigationLessForeignKey);
+                    removedForeignKeys.Add(navigationlessForeignKey);
                 }
 
                 if (resolution.HasFlag(Resolution.ResetDependentProperties))

diff --git a/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs b/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs
@@ -951,7 +951,6 @@ protected class Profile2
         }
 
         [ConditionalFact]
-        // Regression test for Dev11 Bug 94993
         public virtual void Required_to_Required_and_ForeignKey()
         {
             var modelBuilder = CreateModelBuilder();
@@ -1300,6 +1299,70 @@ public class ProfileDetails12
             public int Id { get; set; }
         }
 
+        [ConditionalFact]
+        public virtual void ForeignKeyAttribute_configures_relationships_when_inverse_on_derived()
+        {
+            var modelBuilder = CreateModelBuilder();
+
+            modelBuilder.Entity<Answer>();
+            modelBuilder.Entity<MultipleAnswers>()
+                .HasMany(m => m.Answers)
+                .WithOne(p => (MultipleAnswers)p.Answer)
+                .HasForeignKey(p => p.AnswerId);
+            modelBuilder.Entity<MultipleAnswersRepeating>()
+                .HasMany(m => m.Answers)
+                .WithOne(p => (MultipleAnswersRepeating)p.Answer)
+                .HasForeignKey(p => p.AnswerId);
+
+            modelBuilder.Entity<PartialAnswerBase>();
+            modelBuilder.Entity<PartialAnswer>();
+            modelBuilder.Entity<PartialAnswerRepeating>();
+
+            Validate(modelBuilder);
+
+            var fk1 = modelBuilder.Model.FindEntityType(typeof(PartialAnswer)).GetForeignKeys().Single();
+            Assert.Equal(nameof(PartialAnswer.Answer), fk1.DependentToPrincipal.Name);
+            Assert.Equal(nameof(MultipleAnswers.Answers), fk1.PrincipalToDependent.Name);
+            Assert.Equal(nameof(PartialAnswer.AnswerId), fk1.Properties.Single().Name);
+
+            var fk2 = modelBuilder.Model.FindEntityType(typeof(PartialAnswerRepeating)).GetForeignKeys().Single();
+            Assert.Equal(nameof(PartialAnswerRepeating.Answer), fk1.DependentToPrincipal.Name);
+            Assert.Equal(nameof(MultipleAnswersRepeating.Answers), fk1.PrincipalToDependent.Name);
+            Assert.Equal(nameof(PartialAnswerRepeating.AnswerId), fk1.Properties.Single().Name);
+        }
+
+        public abstract class Answer
+        {
+            public int Id { get; set; }
+        }
+
+        public class PartialAnswerBase
+        {
+            public int Id { get; set; }
+            public int AnswerId { get; set; }
+
+            [ForeignKey("AnswerId")]
+            public virtual Answer Answer { get; set; }
+        }
+
+        public class PartialAnswer : PartialAnswerBase
+        {
+        }
+
+        public class PartialAnswerRepeating : PartialAnswerBase
+        {
+        }
+
+        public class MultipleAnswers : Answer
+        {
+            public virtual ICollection<PartialAnswer> Answers { get; set; }
+        }
+
+        public class MultipleAnswersRepeating : Answer
+        {
+            public virtual ICollection<PartialAnswerRepeating> Answers { get; set; }
+        }
+
         [ConditionalFact]
         public virtual ModelBuilder TableNameAttribute_affects_table_name_in_TPH()
         {