diff --git a/src/EFCore/Infrastructure/ModelValidator.cs b/src/EFCore/Infrastructure/ModelValidator.cs index 4aaafd6b5b4..7c69678ba8d 100644 --- a/src/EFCore/Infrastructure/ModelValidator.cs +++ b/src/EFCore/Infrastructure/ModelValidator.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -463,50 +463,42 @@ protected virtual void ValidateNoCycles( { Check.NotNull(model, nameof(model)); - var typesToValidate = new Queue(); - var reachableTypes = new HashSet(); - var unvalidatedEntityTypes = new HashSet(model.GetEntityTypes()); - while (unvalidatedEntityTypes.Count > 0) + var useOldBehavior = AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue23289", out var isEnabled) + && isEnabled; + if (useOldBehavior) { - var rootEntityType = unvalidatedEntityTypes.First(); - reachableTypes.Clear(); - reachableTypes.Add(rootEntityType); - typesToValidate.Enqueue(rootEntityType); + return; + } - while (typesToValidate.Count > 0) + var graph = new Multigraph(); + foreach (var entityType in model.GetEntityTypes()) + { + var primaryKey = entityType.FindPrimaryKey(); + if (primaryKey == null) { - var entityType = typesToValidate.Dequeue(); - var primaryKey = entityType.FindPrimaryKey(); - if (primaryKey == null) - { - continue; - } + continue; + } - foreach (var foreignKey in entityType.GetForeignKeys()) + foreach (var foreignKey in entityType.GetForeignKeys()) + { + var principalType = foreignKey.PrincipalEntityType; + if (!foreignKey.PrincipalKey.IsPrimaryKey() + || !PropertyListComparer.Instance.Equals(foreignKey.Properties, primaryKey.Properties) + || foreignKey.PrincipalEntityType.IsAssignableFrom(entityType)) { - var principalType = foreignKey.PrincipalEntityType; - if (!foreignKey.PrincipalKey.IsPrimaryKey() - || !unvalidatedEntityTypes.Contains(principalType) - || foreignKey.PrincipalEntityType.IsAssignableFrom(entityType) - || !PropertyListComparer.Instance.Equals(foreignKey.Properties, primaryKey.Properties)) - { - continue; - } - - if (!reachableTypes.Add(principalType)) - { - throw new InvalidOperationException(CoreStrings.IdentifyingRelationshipCycle(rootEntityType.DisplayName())); - } - - typesToValidate.Enqueue(principalType); + continue; } - } - foreach (var entityType in reachableTypes) - { - unvalidatedEntityTypes.Remove(entityType); + graph.AddVertex(entityType); + graph.AddVertex(principalType); + graph.AddEdge(entityType, principalType, foreignKey); } } + + graph.TopologicalSort( + tryBreakEdge: null, + formatCycle: c => c.Select(d => d.Item1.DisplayName()).Join(" -> "), + c => CoreStrings.IdentifyingRelationshipCycle(c)); } /// diff --git a/src/EFCore/Properties/CoreStrings.Designer.cs b/src/EFCore/Properties/CoreStrings.Designer.cs index ea842ac919b..b11444e5371 100644 --- a/src/EFCore/Properties/CoreStrings.Designer.cs +++ b/src/EFCore/Properties/CoreStrings.Designer.cs @@ -997,7 +997,7 @@ public static string HiLoBadBlockSize => GetString("HiLoBadBlockSize"); /// - /// The entity type '{entityType}' is part of a relationship cycle involving its primary key. + /// A relationship cycle involving the primary keys of the following enitity types was detected: '{entityType}'. This would prevent any entity to be inserted without violating the store constraints. Review the foreign keys defined on the primary keys and either remove or use other properties for at least one of them. /// public static string IdentifyingRelationshipCycle([CanBeNull] object entityType) => string.Format( diff --git a/src/EFCore/Properties/CoreStrings.resx b/src/EFCore/Properties/CoreStrings.resx index 34943f46ce4..732774c5b5f 100644 --- a/src/EFCore/Properties/CoreStrings.resx +++ b/src/EFCore/Properties/CoreStrings.resx @@ -490,7 +490,7 @@ The block size used for Hi-Lo value generation must be positive. When the Hi-Lo generator is backed by a SQL sequence this means that the sequence increment must be positive. - The entity type '{entityType}' is part of a relationship cycle involving its primary key. + A relationship cycle involving the primary keys of the following enitity types was detected: '{entityType}'. This would prevent any entity to be inserted without violating the store constraints. Review the foreign keys defined on the primary keys and either remove or use other properties for at least one of them. The instance of entity type '{entityType}' cannot be tracked because another instance with the same key value for {keyProperties} is already being tracked. When attaching existing entities, ensure that only one entity instance with a given key value is attached. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see the conflicting key values. diff --git a/src/Shared/Multigraph.cs b/src/Shared/Multigraph.cs index 1119bc7648a..109e631424d 100644 --- a/src/Shared/Multigraph.cs +++ b/src/Shared/Multigraph.cs @@ -135,7 +135,8 @@ public IReadOnlyList TopologicalSort( public IReadOnlyList TopologicalSort( [CanBeNull] Func, bool> tryBreakEdge, - [CanBeNull] Func>>, string> formatCycle) + [CanBeNull] Func>>, string> formatCycle, + Func formatException = null) { var sortedQueue = new List(); var predecessorCounts = new Dictionary(); @@ -248,7 +249,7 @@ public IReadOnlyList TopologicalSort( cycle.Reverse(); - ThrowCycle(cycle, formatCycle); + ThrowCycle(cycle, formatCycle, formatException); } } } @@ -256,7 +257,10 @@ public IReadOnlyList TopologicalSort( return sortedQueue; } - private void ThrowCycle(List cycle, Func>>, string> formatCycle) + private void ThrowCycle( + List cycle, + Func>>, string> formatCycle, + Func formatException = null) { string cycleString; if (formatCycle == null) @@ -277,7 +281,8 @@ private void ThrowCycle(List cycle, Func().ToTable("Table"); VerifyError( - CoreStrings.IdentifyingRelationshipCycle(nameof(A)), + CoreStrings.IdentifyingRelationshipCycle("A -> B"), modelBuilder.Model); } diff --git a/test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs b/test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs index b4bdd1ac72d..53791ab46f6 100644 --- a/test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs +++ b/test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -321,10 +321,25 @@ public virtual void Detects_relationship_cycle() modelBuilder.Entity().HasOne().WithOne().HasForeignKey(a => a.Id).HasPrincipalKey(b => b.Id).IsRequired(); VerifyError( - CoreStrings.IdentifyingRelationshipCycle(nameof(A)), + CoreStrings.IdentifyingRelationshipCycle("A -> B -> C"), modelBuilder.Model); } + [ConditionalFact] + public virtual void Passes_on_multiple_relationship_paths() + { + var modelBuilder = base.CreateConventionalModelBuilder(); + + modelBuilder.Entity(); + modelBuilder.Entity(); + modelBuilder.Entity().HasBaseType((string)null); + modelBuilder.Entity().HasOne().WithOne().HasForeignKey(a => a.Id).HasPrincipalKey(b => b.Id).IsRequired(); + modelBuilder.Entity().HasOne().WithOne().HasForeignKey(a => a.Id).HasPrincipalKey(b => b.Id).IsRequired(); + modelBuilder.Entity().HasOne().WithOne().HasForeignKey(a => a.Id).HasPrincipalKey(b => b.Id).IsRequired(); + + Validate(modelBuilder.Model); + } + [ConditionalFact] public virtual void Passes_on_redundant_foreign_key() {