Skip to content

Commit

Permalink
Reset relationship uniqueness when removing the navigation to dependent.
Browse files Browse the repository at this point in the history
Fixes #15512
  • Loading branch information
AndriySvyryd committed Oct 23, 2019
1 parent cdfff74 commit 1a699a9
Show file tree
Hide file tree
Showing 9 changed files with 67 additions and 17 deletions.
2 changes: 1 addition & 1 deletion src/EFCore/Metadata/Internal/ConventionAnnotation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
/// </summary>
public virtual ConfigurationSource UpdateConfigurationSource(ConfigurationSource configurationSource)
=> _configurationSource = _configurationSource.Max(configurationSource);
=> _configurationSource = configurationSource.Max(_configurationSource);
}
}
10 changes: 5 additions & 5 deletions src/EFCore/Metadata/Internal/ForeignKey.cs
Original file line number Diff line number Diff line change
Expand Up @@ -478,19 +478,19 @@ public virtual bool IsUnique
/// </summary>
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;
}
Expand Down Expand Up @@ -652,7 +652,7 @@ public virtual bool IsOwnership
/// </summary>
public virtual ForeignKey SetIsOwnership(bool? ownership, ConfigurationSource configurationSource)
{
var isChanging = IsOwnership != ownership;
var oldIsOwnership = IsOwnership;
_isOwnership = ownership;

if (_isOwnership == null)
Expand All @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore/Metadata/Internal/Index.cs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public virtual InternalIndexBuilder Builder
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual void UpdateConfigurationSource(ConfigurationSource configurationSource)
=> _configurationSource = _configurationSource.Max(configurationSource);
=> _configurationSource = configurationSource.Max(_configurationSource);

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
5 changes: 5 additions & 0 deletions src/EFCore/Metadata/Internal/InternalRelationshipBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore/Metadata/Internal/Key.cs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public virtual InternalKeyBuilder Builder
/// </summary>
public virtual void UpdateConfigurationSource(ConfigurationSource configurationSource)
{
_configurationSource = _configurationSource.Max(configurationSource);
_configurationSource = configurationSource.Max(_configurationSource);
foreach (var property in Properties)
{
property.UpdateConfigurationSource(configurationSource);
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore/Metadata/Internal/Property.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion src/EFCore/Metadata/Internal/TypeBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
/// </summary>
public virtual void UpdateConfigurationSource(ConfigurationSource configurationSource)
=> _configurationSource = _configurationSource.Max(configurationSource);
=> _configurationSource = configurationSource.Max(_configurationSource);

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
56 changes: 49 additions & 7 deletions test/EFCore.Specification.Tests/DataAnnotationTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public virtual ModelBuilder CreateModelBuilder()
protected virtual IConventionSetBuilder CreateConventionSetBuilder(DbContext context)
=> context.GetService<IConventionSetBuilder>();

protected virtual void Validate(ModelBuilder modelBuilder)
protected virtual IModel Validate(ModelBuilder modelBuilder)
=> modelBuilder.FinalizeModel();

protected class Person
Expand Down Expand Up @@ -1304,17 +1304,17 @@ public virtual void ForeignKeyAttribute_configures_relationships_when_inverse_on
modelBuilder.Entity<PartialAnswer>();
modelBuilder.Entity<PartialAnswerRepeating>();

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
Expand Down Expand Up @@ -1349,6 +1349,48 @@ private class MultipleAnswersRepeating : Answer
public virtual ICollection<PartialAnswerRepeating> Answers { get; set; }
}

[ConditionalFact]
public virtual void ForeignKeyAttribute_configures_two_self_referencing_relationships()
{
var modelBuilder = CreateModelBuilder();

modelBuilder.Entity<Comment>();

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()
{
Expand Down
3 changes: 3 additions & 0 deletions test/EFCore.Tests/ModelBuilding/OneToOneTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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(
Expand Down

0 comments on commit 1a699a9

Please sign in to comment.