From 1a699a9c1aeb0734d1caab94739d2b541966aa08 Mon Sep 17 00:00:00 2001 From: AndriySvyryd Date: Tue, 22 Oct 2019 14:21:40 -0700 Subject: [PATCH] Reset relationship uniqueness when removing the navigation to dependent. Fixes #15512 --- .../Metadata/Internal/ConventionAnnotation.cs | 2 +- src/EFCore/Metadata/Internal/ForeignKey.cs | 10 ++-- src/EFCore/Metadata/Internal/Index.cs | 2 +- .../Internal/InternalRelationshipBuilder.cs | 5 ++ src/EFCore/Metadata/Internal/Key.cs | 2 +- src/EFCore/Metadata/Internal/Property.cs | 2 +- src/EFCore/Metadata/Internal/TypeBase.cs | 2 +- .../DataAnnotationTestBase.cs | 56 ++++++++++++++++--- .../ModelBuilding/OneToOneTestBase.cs | 3 + 9 files changed, 67 insertions(+), 17 deletions(-) diff --git a/src/EFCore/Metadata/Internal/ConventionAnnotation.cs b/src/EFCore/Metadata/Internal/ConventionAnnotation.cs index 93ecf1b5b5b..6d894a3e9d1 100644 --- a/src/EFCore/Metadata/Internal/ConventionAnnotation.cs +++ b/src/EFCore/Metadata/Internal/ConventionAnnotation.cs @@ -43,6 +43,6 @@ public ConventionAnnotation([NotNull] string name, [CanBeNull] object value, Con /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public virtual ConfigurationSource UpdateConfigurationSource(ConfigurationSource configurationSource) - => _configurationSource = _configurationSource.Max(configurationSource); + => _configurationSource = configurationSource.Max(_configurationSource); } } diff --git a/src/EFCore/Metadata/Internal/ForeignKey.cs b/src/EFCore/Metadata/Internal/ForeignKey.cs index b440d46221d..24c068dd789 100644 --- a/src/EFCore/Metadata/Internal/ForeignKey.cs +++ b/src/EFCore/Metadata/Internal/ForeignKey.cs @@ -478,19 +478,19 @@ public virtual bool IsUnique /// public virtual ForeignKey SetIsUnique(bool? unique, ConfigurationSource configurationSource) { - var isChanging = IsUnique != unique; + var oldUnique = IsUnique; _isUnique = unique; if (unique == null) { _isUniqueConfigurationSource = null; } - + else { UpdateIsUniqueConfigurationSource(configurationSource); } - return isChanging + return IsUnique != oldUnique ? (ForeignKey)DeclaringEntityType.Model.ConventionDispatcher.OnForeignKeyUniquenessChanged(Builder)?.Metadata : this; } @@ -652,7 +652,7 @@ public virtual bool IsOwnership /// public virtual ForeignKey SetIsOwnership(bool? ownership, ConfigurationSource configurationSource) { - var isChanging = IsOwnership != ownership; + var oldIsOwnership = IsOwnership; _isOwnership = ownership; if (_isOwnership == null) @@ -664,7 +664,7 @@ public virtual ForeignKey SetIsOwnership(bool? ownership, ConfigurationSource co UpdateIsOwnershipConfigurationSource(configurationSource); } - return isChanging + return IsOwnership != oldIsOwnership ? (ForeignKey)DeclaringEntityType.Model.ConventionDispatcher.OnForeignKeyOwnershipChanged(Builder)?.Metadata : this; } diff --git a/src/EFCore/Metadata/Internal/Index.cs b/src/EFCore/Metadata/Internal/Index.cs index d2afb23da78..9871b82c135 100644 --- a/src/EFCore/Metadata/Internal/Index.cs +++ b/src/EFCore/Metadata/Internal/Index.cs @@ -94,7 +94,7 @@ public virtual InternalIndexBuilder Builder /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public virtual void UpdateConfigurationSource(ConfigurationSource configurationSource) - => _configurationSource = _configurationSource.Max(configurationSource); + => _configurationSource = configurationSource.Max(_configurationSource); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to diff --git a/src/EFCore/Metadata/Internal/InternalRelationshipBuilder.cs b/src/EFCore/Metadata/Internal/InternalRelationshipBuilder.cs index 3fe8a2d7fbe..7075bd1cbca 100644 --- a/src/EFCore/Metadata/Internal/InternalRelationshipBuilder.cs +++ b/src/EFCore/Metadata/Internal/InternalRelationshipBuilder.cs @@ -310,10 +310,15 @@ private InternalRelationshipBuilder HasNavigations( { builder = this; Metadata.UpdateConfigurationSource(configurationSource); + if (shouldBeUnique.HasValue) { IsUnique(shouldBeUnique.Value, configurationSource); } + else + { + IsUnique(null, ConfigurationSource.Convention); + } if (navigationToPrincipal != null) { diff --git a/src/EFCore/Metadata/Internal/Key.cs b/src/EFCore/Metadata/Internal/Key.cs index a45b7d792c9..49ef898f262 100644 --- a/src/EFCore/Metadata/Internal/Key.cs +++ b/src/EFCore/Metadata/Internal/Key.cs @@ -95,7 +95,7 @@ public virtual InternalKeyBuilder Builder /// public virtual void UpdateConfigurationSource(ConfigurationSource configurationSource) { - _configurationSource = _configurationSource.Max(configurationSource); + _configurationSource = configurationSource.Max(_configurationSource); foreach (var property in Properties) { property.UpdateConfigurationSource(configurationSource); diff --git a/src/EFCore/Metadata/Internal/Property.cs b/src/EFCore/Metadata/Internal/Property.cs index 2fad2f0a679..7a1484307d6 100644 --- a/src/EFCore/Metadata/Internal/Property.cs +++ b/src/EFCore/Metadata/Internal/Property.cs @@ -120,7 +120,7 @@ public virtual InternalPropertyBuilder Builder public virtual bool UpdateConfigurationSource(ConfigurationSource configurationSource) { var oldConfigurationSource = _configurationSource; - _configurationSource = _configurationSource.Max(configurationSource); + _configurationSource = configurationSource.Max(_configurationSource); return _configurationSource != oldConfigurationSource; } diff --git a/src/EFCore/Metadata/Internal/TypeBase.cs b/src/EFCore/Metadata/Internal/TypeBase.cs index 47ac5ff07df..49f917ce675 100644 --- a/src/EFCore/Metadata/Internal/TypeBase.cs +++ b/src/EFCore/Metadata/Internal/TypeBase.cs @@ -104,7 +104,7 @@ private TypeBase([NotNull] Model model, ConfigurationSource configurationSource) /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public virtual void UpdateConfigurationSource(ConfigurationSource configurationSource) - => _configurationSource = _configurationSource.Max(configurationSource); + => _configurationSource = configurationSource.Max(_configurationSource); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to diff --git a/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs b/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs index 18be1118c85..6673f618705 100644 --- a/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs +++ b/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs @@ -52,7 +52,7 @@ public virtual ModelBuilder CreateModelBuilder() protected virtual IConventionSetBuilder CreateConventionSetBuilder(DbContext context) => context.GetService(); - protected virtual void Validate(ModelBuilder modelBuilder) + protected virtual IModel Validate(ModelBuilder modelBuilder) => modelBuilder.FinalizeModel(); protected class Person @@ -1304,17 +1304,17 @@ public virtual void ForeignKeyAttribute_configures_relationships_when_inverse_on modelBuilder.Entity(); modelBuilder.Entity(); - Validate(modelBuilder); + var model = modelBuilder.FinalizeModel(); - var fk1 = modelBuilder.Model.FindEntityType(typeof(PartialAnswer)).GetForeignKeys().Single(); + var fk1 = 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); + var fk2 = model.FindEntityType(typeof(PartialAnswerRepeating)).GetForeignKeys().Single(); + Assert.Equal(nameof(PartialAnswerRepeating.Answer), fk2.DependentToPrincipal.Name); + Assert.Equal(nameof(MultipleAnswersRepeating.Answers), fk2.PrincipalToDependent.Name); + Assert.Equal(nameof(PartialAnswerRepeating.AnswerId), fk2.Properties.Single().Name); } private abstract class Answer @@ -1349,6 +1349,48 @@ private class MultipleAnswersRepeating : Answer public virtual ICollection Answers { get; set; } } + [ConditionalFact] + public virtual void ForeignKeyAttribute_configures_two_self_referencing_relationships() + { + var modelBuilder = CreateModelBuilder(); + + modelBuilder.Entity(); + + var model = modelBuilder.FinalizeModel(); + + var entityType = model.FindEntityType(typeof(Comment)); + var fk1 = entityType.GetForeignKeys().Single(fk => fk.Properties.Single().Name == nameof(Comment.ParentCommentID)); + Assert.Equal(nameof(Comment.ParentComment), fk1.DependentToPrincipal.Name); + Assert.Null(fk1.PrincipalToDependent); + var index1 = entityType.FindIndex(fk1.Properties); + Assert.False(index1.IsUnique); + + var fk2 = entityType.GetForeignKeys().Single(fk => fk.Properties.Single().Name == nameof(Comment.ReplyCommentID)); + Assert.Equal(nameof(Comment.ReplyComment), fk2.DependentToPrincipal.Name); + Assert.Null(fk2.PrincipalToDependent); + var index2 = entityType.FindIndex(fk2.Properties); + Assert.False(index2.IsUnique); + + Assert.Equal(2, entityType.GetForeignKeys().Count()); + Assert.Equal(2, entityType.GetIndexes().Count()); + } + + private class Comment + { + [Key] + public long CommentID { get; set; } + + public long? ReplyCommentID { get; set; } + + public long? ParentCommentID { get; set; } + + [ForeignKey("ParentCommentID")] + public virtual Comment ParentComment { get; set; } + + [ForeignKey("ReplyCommentID")] + public virtual Comment ReplyComment { get; set; } + } + [ConditionalFact] public virtual ModelBuilder TableNameAttribute_affects_table_name_in_TPH() { diff --git a/test/EFCore.Tests/ModelBuilding/OneToOneTestBase.cs b/test/EFCore.Tests/ModelBuilding/OneToOneTestBase.cs index 37cb45090d4..4d3ba99280d 100644 --- a/test/EFCore.Tests/ModelBuilding/OneToOneTestBase.cs +++ b/test/EFCore.Tests/ModelBuilding/OneToOneTestBase.cs @@ -641,6 +641,7 @@ public virtual void Creates_relationship_with_specified_FK_with_navigation_to_pr Assert.Same(dependentKey, dependentType.GetKeys().Single()); Assert.Same(principalKey, principalType.FindPrimaryKey()); Assert.Same(dependentKey, dependentType.FindPrimaryKey()); + Assert.True(fk.IsUnique); Assert.Equal(dependentType.GetForeignKeys().Count(), dependentType.GetIndexes().Count()); Assert.True(fk.DeclaringEntityType.FindIndex(fk.Properties).IsUnique); Assert.True( @@ -1898,6 +1899,7 @@ public virtual void Creates_principal_key_when_specified_on_dependent_with_navig Assert.Same(dependentKey, dependentType.GetKeys().Single()); Assert.Same(principalKey, principalType.FindPrimaryKey()); Assert.Same(dependentKey, dependentType.FindPrimaryKey()); + Assert.True(fk.IsUnique); Assert.Equal(dependentType.GetForeignKeys().Count(), dependentType.GetIndexes().Count()); Assert.True(fk.DeclaringEntityType.FindIndex(fk.Properties).IsUnique); } @@ -2497,6 +2499,7 @@ public virtual void Creates_composite_FK_when_specified_on_principal_with_naviga Assert.Same(dependentKey, dependentType.GetKeys().Single()); Assert.Same(principalKey, principalType.FindPrimaryKey()); Assert.Same(dependentKey, dependentType.FindPrimaryKey()); + Assert.True(fk.IsUnique); Assert.Equal(dependentType.GetForeignKeys().Count(), dependentType.GetIndexes().Count()); Assert.True(fk.DeclaringEntityType.FindIndex(fk.Properties).IsUnique); Assert.True(