Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reset relationship uniqueness when removing the navigation to dependent. #18521

Merged
merged 1 commit into from
Oct 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No functional impact, just consistency

}
}
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