Skip to content

Commit

Permalink
Add a pre-validation exception
Browse files Browse the repository at this point in the history
  • Loading branch information
AndriySvyryd committed Dec 12, 2022
1 parent 1f12a1e commit f5bc9b0
Show file tree
Hide file tree
Showing 8 changed files with 117 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -855,8 +855,15 @@ private void Create(
return type;
}

var annotation = property.FindAnnotation(CoreAnnotationNames.ValueConverterType);
if (annotation != null)
{
return (Type?)annotation.Value;
}

var principalProperty = property;
for (var i = 0; i < 10000; i++)
var i = 0;
for (; i < ForeignKey.LongestFkChainAllowedLength; i++)
{
IProperty? nextProperty = null;
foreach (var foreignKey in principalProperty.GetContainingForeignKeys())
Expand All @@ -872,10 +879,10 @@ private void Create(
break;
}

type = (Type?)newPrincipalProperty[CoreAnnotationNames.ValueConverterType];
if (type != null)
annotation = newPrincipalProperty.FindAnnotation(CoreAnnotationNames.ValueConverterType);
if (annotation != null)
{
return type;
return (Type?)annotation.Value;
}

nextProperty = newPrincipalProperty;
Expand All @@ -891,7 +898,10 @@ private void Create(
principalProperty = nextProperty;
}

return null;
return i == ForeignKey.LongestFkChainAllowedLength
? throw new InvalidOperationException(CoreStrings.RelationshipCycle(
property.DeclaringEntityType.DisplayName(), property.Name, "ValueConverterType"))
: null;
}

private void PropertyBaseParameters(
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore/Infrastructure/ModelValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,7 @@ protected virtual void ValidateNoCycles(
graph.TopologicalSort(
tryBreakEdge: null,
formatCycle: c => c.Select(d => d.Item1.DisplayName()).Join(" -> "),
c => CoreStrings.IdentifyingRelationshipCycle(c));
CoreStrings.IdentifyingRelationshipCycle);
}

/// <summary>
Expand Down
8 changes: 8 additions & 0 deletions src/EFCore/Metadata/Internal/ForeignKey.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@ public class ForeignKey : ConventionAnnotatable, IMutableForeignKey, IConvention
private IDependentKeyValueFactory? _dependentKeyValueFactory;
private Func<IDependentsMap>? _dependentsMapFactory;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public static readonly int LongestFkChainAllowedLength = 10000;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
Expand Down
46 changes: 27 additions & 19 deletions src/EFCore/Metadata/Internal/Property.cs
Original file line number Diff line number Diff line change
Expand Up @@ -613,7 +613,7 @@ public virtual PropertySaveBehavior GetAfterSaveBehavior()
}

RemoveAnnotation(CoreAnnotationNames.ValueConverterType);
return (ValueConverter?)SetOrRemoveAnnotation(CoreAnnotationNames.ValueConverter, converter, configurationSource)?.Value;
return (ValueConverter?)SetAnnotation(CoreAnnotationNames.ValueConverter, converter, configurationSource)?.Value;
}

/// <summary>
Expand Down Expand Up @@ -648,7 +648,7 @@ public virtual PropertySaveBehavior GetAfterSaveBehavior()
}

SetValueConverter(converter, configurationSource);
SetOrRemoveAnnotation(CoreAnnotationNames.ValueConverterType, converterType, configurationSource);
SetAnnotation(CoreAnnotationNames.ValueConverterType, converterType, configurationSource);

return converterType;
}
Expand All @@ -661,14 +661,15 @@ public virtual PropertySaveBehavior GetAfterSaveBehavior()
/// </summary>
public virtual ValueConverter? GetValueConverter()
{
var converter = (ValueConverter?)this[CoreAnnotationNames.ValueConverter];
if (converter != null)
var annotation = FindAnnotation(CoreAnnotationNames.ValueConverter);
if (annotation != null)
{
return converter;
return (ValueConverter?)annotation.Value;
}

var property = this;
for (var i = 0; i < 10000; i++)
var i = 0;
for (; i < ForeignKey.LongestFkChainAllowedLength; i++)
{
Property? nextProperty = null;
foreach (var foreignKey in property.GetContainingForeignKeys())
Expand All @@ -684,10 +685,10 @@ public virtual PropertySaveBehavior GetAfterSaveBehavior()
break;
}

converter = (ValueConverter?)principalProperty[CoreAnnotationNames.ValueConverter];
if (converter != null)
annotation = principalProperty.FindAnnotation(CoreAnnotationNames.ValueConverter);
if (annotation != null)
{
return converter;
return (ValueConverter?)annotation.Value;
}

nextProperty = principalProperty;
Expand All @@ -703,7 +704,10 @@ public virtual PropertySaveBehavior GetAfterSaveBehavior()
property = nextProperty;
}

return null;
return i == ForeignKey.LongestFkChainAllowedLength
? throw new InvalidOperationException(CoreStrings.RelationshipCycle(
DeclaringEntityType.DisplayName(), Name, "ValueConverter"))
: null;
}

/// <summary>
Expand Down Expand Up @@ -738,7 +742,7 @@ public virtual PropertySaveBehavior GetAfterSaveBehavior()
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual Type? SetProviderClrType(Type? providerClrType, ConfigurationSource configurationSource)
=> (Type?)SetOrRemoveAnnotation(CoreAnnotationNames.ProviderClrType, providerClrType, configurationSource)?.Value;
=> (Type?)SetAnnotation(CoreAnnotationNames.ProviderClrType, providerClrType, configurationSource)?.Value;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand All @@ -748,14 +752,15 @@ public virtual PropertySaveBehavior GetAfterSaveBehavior()
/// </summary>
public virtual Type? GetProviderClrType()
{
var type = (Type?)this[CoreAnnotationNames.ProviderClrType];
if (type != null)
var annotation = FindAnnotation(CoreAnnotationNames.ProviderClrType);
if (annotation != null)
{
return type;
return (Type?)annotation.Value;
}

var property = this;
for (var i = 0; i < 10000; i++)
var i = 0;
for (; i < ForeignKey.LongestFkChainAllowedLength; i++)
{
Property? nextProperty = null;
foreach (var foreignKey in property.GetContainingForeignKeys())
Expand All @@ -771,10 +776,10 @@ public virtual PropertySaveBehavior GetAfterSaveBehavior()
break;
}

type = (Type?)principalProperty[CoreAnnotationNames.ProviderClrType];
if (type != null)
annotation = principalProperty.FindAnnotation(CoreAnnotationNames.ProviderClrType);
if (annotation != null)
{
return type;
return (Type?)annotation.Value;
}

nextProperty = principalProperty;
Expand All @@ -790,7 +795,10 @@ public virtual PropertySaveBehavior GetAfterSaveBehavior()
property = nextProperty;
}

return null;
return i == ForeignKey.LongestFkChainAllowedLength
? throw new InvalidOperationException(CoreStrings.RelationshipCycle(
DeclaringEntityType.DisplayName(), Name, "ProviderClrType"))
: null;
}

/// <summary>
Expand Down
10 changes: 9 additions & 1 deletion src/EFCore/Properties/CoreStrings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion src/EFCore/Properties/CoreStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -1307,6 +1307,9 @@
<data name="RelationshipConceptualNullSensitive" xml:space="preserve">
<value>The association between entities '{firstType}' and '{secondType}' with the key value '{secondKeyValue}' has been severed, but the relationship is either marked as required or is implicitly required because the foreign key is not nullable. If the dependent/child entity should be deleted when a required relationship is severed, configure the relationship to use cascade deletes.</value>
</data>
<data name="RelationshipCycle" xml:space="preserve">
<value>A relationship cycle involving the property '{entityType}.{property}' was detected. This prevents Entity Framework from determining the correct configuration. Review the foreign keys defined on the property and the corresponding principal property and either remove one of them or specify '{configuration}' explicitly on one of the properties.</value>
</data>
<data name="RequiredSkipNavigation" xml:space="preserve">
<value>'{entityType}.{navigation}' cannot be configured as required since it represents a skip navigation.</value>
</data>
Expand Down Expand Up @@ -1505,4 +1508,4 @@
<data name="WrongStateManager" xml:space="preserve">
<value>Cannot start tracking the entry for entity type '{entityType}' because it was created by a different StateManager instance.</value>
</data>
</root>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -1396,7 +1396,13 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
eb.HasKey(l => new { l.GameId, l.Id });
});

modelBuilder.Entity<Container>();
modelBuilder.Entity<Container>(
eb =>
{
eb.HasMany(c => c.Items)
.WithOne()
.HasForeignKey("GameId", "ContainerId");
});

modelBuilder.Entity<Game>(
eb =>
Expand Down
49 changes: 46 additions & 3 deletions test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -472,16 +472,59 @@ public virtual void Detects_key_property_with_value_generated_on_add_or_update()
}

[ConditionalFact]
public virtual void Detects_relationship_cycle()
public virtual void Detects_identifying_relationship_cycle()
{
var modelBuilder = base.CreateConventionModelBuilder();

modelBuilder.Entity<C>().HasBaseType((string)null);
modelBuilder.Entity<A>().HasOne<B>().WithOne().HasForeignKey<A>(a => a.Id).HasPrincipalKey<B>(b => b.Id).IsRequired();
modelBuilder.Entity<A>().HasOne<C>().WithOne().HasForeignKey<C>(a => a.Id).HasPrincipalKey<A>(b => b.Id).IsRequired();
modelBuilder.Entity<C>().HasOne<B>().WithOne().HasForeignKey<B>(a => a.Id).HasPrincipalKey<C>(b => b.Id).IsRequired();

VerifyError(
CoreStrings.IdentifyingRelationshipCycle("A -> B -> C"),
modelBuilder);
}

[ConditionalFact]
public virtual void Detects_relationship_cycle_for_property_configuration()
{
var modelBuilder = base.CreateConventionModelBuilder();

modelBuilder.Entity<A>();
modelBuilder.Entity<B>();
modelBuilder.Entity<C>().HasBaseType((string)null);
modelBuilder.Entity<A>().HasOne<B>().WithOne().HasForeignKey<A>(a => a.Id).HasPrincipalKey<B>(b => b.Id).IsRequired();
modelBuilder.Entity<A>().HasOne<C>().WithOne().HasForeignKey<C>(a => a.Id).HasPrincipalKey<A>(b => b.Id).IsRequired();
modelBuilder.Entity<C>().HasOne<B>().WithOne().HasForeignKey<B>(a => a.Id).HasPrincipalKey<C>(b => b.Id).IsRequired();
modelBuilder.Entity<D>().HasBaseType((string)null);
modelBuilder.Entity<D>().HasOne<B>().WithOne().HasForeignKey<D>(a => a.Id).HasPrincipalKey<B>(b => b.Id).IsRequired();

var dId = modelBuilder.Model.FindEntityType(typeof(D)).FindProperty(nameof(D.Id));

Assert.Equal(CoreStrings.RelationshipCycle(nameof(D), nameof(D.Id), "ValueConverter"),
Assert.Throws<InvalidOperationException>(dId.GetValueConverter).Message);
Assert.Equal(CoreStrings.RelationshipCycle(nameof(D), nameof(D.Id), "ProviderClrType"),
Assert.Throws<InvalidOperationException>(dId.GetProviderClrType).Message);
}

[ConditionalFact]
public virtual void Detects_relationship_cycle_for_explicit_property_configuration()
{
var modelBuilder = base.CreateConventionModelBuilder();

modelBuilder.Entity<C>().HasBaseType((string)null);
modelBuilder.Entity<A>().HasOne<B>().WithOne().HasForeignKey<A>(a => a.Id).HasPrincipalKey<B>(b => b.Id).IsRequired();
modelBuilder.Entity<A>().HasOne<C>().WithOne().HasForeignKey<C>(a => a.Id).HasPrincipalKey<A>(b => b.Id).IsRequired();
modelBuilder.Entity<C>().HasOne<B>().WithOne().HasForeignKey<B>(a => a.Id).HasPrincipalKey<C>(b => b.Id).IsRequired();
modelBuilder.Entity<D>().HasBaseType((string)null);
modelBuilder.Entity<D>().HasOne<B>().WithOne().HasForeignKey<D>(a => a.Id).HasPrincipalKey<B>(b => b.Id).IsRequired();

var aId = modelBuilder.Model.FindEntityType(typeof(A)).FindProperty(nameof(A.Id));
aId.SetValueConverter((ValueConverter)null);
aId.SetProviderClrType(null);

var dId = modelBuilder.Model.FindEntityType(typeof(D)).FindProperty(nameof(D.Id));
Assert.Null(dId.GetValueConverter());
Assert.Null(dId.GetProviderClrType());

VerifyError(
CoreStrings.IdentifyingRelationshipCycle("A -> B -> C"),
Expand Down

0 comments on commit f5bc9b0

Please sign in to comment.