Skip to content

Commit 119c872

Browse files
AndriySvyrydAndriySvyryd
authored
Improve the relationship cycle breaking logic to detect cycles even when not starting on one. (#32598)
Additionally, detect multiple relationship chains that end with incompatible conversions. Fixes #32422
1 parent 08da61e commit 119c872

File tree

7 files changed

+436
-10
lines changed

7 files changed

+436
-10
lines changed

‎src/EFCore.Design/Scaffolding/Internal/CSharpRuntimeModelCodeGenerator.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -899,6 +899,12 @@ private void Create(
899899
return (Type?)annotation.Value;
900900
}
901901

902+
if (!Property.UseOldBehavior32422)
903+
{
904+
return ((Property)property).GetConversion(throwOnProviderClrTypeConflict: false, throwOnValueConverterConflict: false)
905+
.ValueConverterType;
906+
}
907+
902908
var principalProperty = property;
903909
var i = 0;
904910
for (; i < ForeignKey.LongestFkChainAllowedLength; i++)

‎src/EFCore/Metadata/Internal/Property.cs

Lines changed: 229 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,15 @@ public class Property : PropertyBase, IMutableProperty, IConventionProperty, IPr
3131
private ConfigurationSource? _valueGeneratedConfigurationSource;
3232
private ConfigurationSource? _typeMappingConfigurationSource;
3333

34+
/// <summary>
35+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
36+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
37+
/// any release. You should only use it directly in your code with extreme caution and knowing that
38+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
39+
/// </summary>
40+
public static readonly bool UseOldBehavior32422 =
41+
AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue32422", out var enabled32422) && enabled32422;
42+
3443
/// <summary>
3544
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
3645
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
@@ -744,6 +753,12 @@ private object? DefaultSentinel
744753
return (ValueConverter?)annotation.Value;
745754
}
746755

756+
if (!UseOldBehavior32422)
757+
{
758+
return GetConversion(throwOnProviderClrTypeConflict: FindAnnotation(CoreAnnotationNames.ProviderClrType) == null)
759+
.ValueConverter;
760+
}
761+
747762
var property = this;
748763
var i = 0;
749764
for (; i < ForeignKey.LongestFkChainAllowedLength; i++)
@@ -836,6 +851,12 @@ private object? DefaultSentinel
836851
return (Type?)annotation.Value;
837852
}
838853

854+
if (!UseOldBehavior32422)
855+
{
856+
return GetConversion(throwOnValueConverterConflict: FindAnnotation(CoreAnnotationNames.ValueConverter) == null)
857+
.ProviderClrType;
858+
}
859+
839860
var property = this;
840861
var i = 0;
841862
for (; i < ForeignKey.LongestFkChainAllowedLength; i++)
@@ -880,6 +901,214 @@ private object? DefaultSentinel
880901
: null;
881902
}
882903

904+
/// <summary>
905+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
906+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
907+
/// any release. You should only use it directly in your code with extreme caution and knowing that
908+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
909+
/// </summary>
910+
public virtual (ValueConverter? ValueConverter, Type? ValueConverterType, Type? ProviderClrType) GetConversion(
911+
bool throwOnValueConverterConflict = true,
912+
bool throwOnProviderClrTypeConflict = true)
913+
{
914+
Queue<(Property CurrentProperty, Property CycleBreakingPropert, int CyclePosition, int MaxCycleLength)>? queue = null;
915+
(Property CurrentProperty, Property CycleBreakingPropert, int CyclePosition, int MaxCycleLength)? currentNode =
916+
(this, this, 0, 2);
917+
918+
ValueConverter? valueConverter = null;
919+
Type? valueConverterType = null;
920+
Type? providerClrType = null;
921+
while (currentNode is not null || queue is { Count: > 0 })
922+
{
923+
var (property, cycleBreakingProperty, cyclePosition, maxCycleLength) = currentNode ?? queue!.Dequeue();
924+
currentNode = null;
925+
if (cyclePosition >= ForeignKey.LongestFkChainAllowedLength)
926+
{
927+
throw new InvalidOperationException(
928+
CoreStrings.RelationshipCycle(DeclaringType.DisplayName(), Name, "ValueConverter"));
929+
}
930+
931+
foreach (var foreignKey in property.GetContainingForeignKeys())
932+
{
933+
for (var propertyIndex = 0; propertyIndex < foreignKey.Properties.Count; propertyIndex++)
934+
{
935+
if (property != foreignKey.Properties[propertyIndex])
936+
{
937+
continue;
938+
}
939+
940+
var principalProperty = foreignKey.PrincipalKey.Properties[propertyIndex];
941+
if (principalProperty == cycleBreakingProperty)
942+
{
943+
break;
944+
}
945+
946+
var annotationFound = GetConversion(
947+
principalProperty,
948+
throwOnValueConverterConflict,
949+
throwOnProviderClrTypeConflict,
950+
ref valueConverter,
951+
ref valueConverterType,
952+
ref providerClrType);
953+
if (!annotationFound)
954+
{
955+
if (currentNode != null)
956+
{
957+
queue = new();
958+
queue.Enqueue(currentNode.Value);
959+
}
960+
961+
if (cyclePosition == maxCycleLength - 1)
962+
{
963+
// We need to use different primes to ensure a different cycleBreakingProperty is selected
964+
// each time when traversing properties that participate in multiple relationship cycles
965+
currentNode = (principalProperty, property, 0, HashHelpers.GetPrime(maxCycleLength << 1));
966+
}
967+
else
968+
{
969+
currentNode = (principalProperty, cycleBreakingProperty, cyclePosition + 1, maxCycleLength);
970+
}
971+
972+
if (queue != null)
973+
{
974+
queue.Enqueue(currentNode.Value);
975+
currentNode = null;
976+
}
977+
}
978+
break;
979+
}
980+
}
981+
}
982+
983+
return (valueConverter, valueConverterType, providerClrType);
984+
985+
bool GetConversion(
986+
Property principalProperty,
987+
bool throwOnValueConverterConflict,
988+
bool throwOnProviderClrTypeConflict,
989+
ref ValueConverter? valueConverter,
990+
ref Type? valueConverterType,
991+
ref Type? providerClrType)
992+
{
993+
var annotationFound = false;
994+
var valueConverterAnnotation = principalProperty.FindAnnotation(CoreAnnotationNames.ValueConverter);
995+
if (valueConverterAnnotation != null)
996+
{
997+
var annotationValue = (ValueConverter?)valueConverterAnnotation.Value;
998+
if (annotationValue != null)
999+
{
1000+
if (valueConverter != null
1001+
&& annotationValue.GetType() != valueConverter.GetType())
1002+
{
1003+
throw new InvalidOperationException(
1004+
CoreStrings.ConflictingRelationshipConversions(
1005+
DeclaringType.DisplayName(), Name,
1006+
valueConverter.GetType().ShortDisplayName(), annotationValue.GetType().ShortDisplayName()));
1007+
}
1008+
1009+
if (valueConverterType != null
1010+
&& annotationValue.GetType() != valueConverterType)
1011+
{
1012+
throw new InvalidOperationException(
1013+
CoreStrings.ConflictingRelationshipConversions(
1014+
DeclaringType.DisplayName(), Name,
1015+
valueConverterType.ShortDisplayName(), annotationValue.GetType().ShortDisplayName()));
1016+
}
1017+
1018+
if (providerClrType != null
1019+
&& throwOnProviderClrTypeConflict)
1020+
{
1021+
throw new InvalidOperationException(
1022+
CoreStrings.ConflictingRelationshipConversions(
1023+
DeclaringType.DisplayName(), Name,
1024+
providerClrType.ShortDisplayName(), annotationValue.GetType().ShortDisplayName()));
1025+
}
1026+
1027+
valueConverter = annotationValue;
1028+
}
1029+
annotationFound = true;
1030+
}
1031+
1032+
var valueConverterTypeAnnotation = principalProperty.FindAnnotation(CoreAnnotationNames.ValueConverterType);
1033+
if (valueConverterTypeAnnotation != null)
1034+
{
1035+
var annotationValue = (Type?)valueConverterTypeAnnotation.Value;
1036+
if (annotationValue != null)
1037+
{
1038+
if (valueConverter != null
1039+
&& valueConverter.GetType() != annotationValue)
1040+
{
1041+
throw new InvalidOperationException(
1042+
CoreStrings.ConflictingRelationshipConversions(
1043+
DeclaringType.DisplayName(), Name,
1044+
valueConverter.GetType().ShortDisplayName(), annotationValue.ShortDisplayName()));
1045+
}
1046+
1047+
if (valueConverterType != null
1048+
&& valueConverterType != annotationValue)
1049+
{
1050+
throw new InvalidOperationException(
1051+
CoreStrings.ConflictingRelationshipConversions(
1052+
DeclaringType.DisplayName(), Name,
1053+
valueConverterType.ShortDisplayName(), annotationValue.ShortDisplayName()));
1054+
}
1055+
1056+
if (providerClrType != null
1057+
&& throwOnProviderClrTypeConflict)
1058+
{
1059+
throw new InvalidOperationException(
1060+
CoreStrings.ConflictingRelationshipConversions(
1061+
DeclaringType.DisplayName(), Name,
1062+
providerClrType.ShortDisplayName(), annotationValue.ShortDisplayName()));
1063+
}
1064+
1065+
valueConverterType = annotationValue;
1066+
}
1067+
annotationFound = true;
1068+
}
1069+
1070+
var providerClrTypeAnnotation = principalProperty.FindAnnotation(CoreAnnotationNames.ProviderClrType);
1071+
if (providerClrTypeAnnotation != null)
1072+
{
1073+
var annotationValue = (Type?)providerClrTypeAnnotation.Value;
1074+
if (annotationValue != null)
1075+
{
1076+
if (providerClrType != null
1077+
&& annotationValue != providerClrType)
1078+
{
1079+
throw new InvalidOperationException(
1080+
CoreStrings.ConflictingRelationshipConversions(
1081+
DeclaringType.DisplayName(), Name,
1082+
providerClrType.ShortDisplayName(), annotationValue.ShortDisplayName()));
1083+
}
1084+
1085+
if (valueConverter != null
1086+
&& throwOnValueConverterConflict)
1087+
{
1088+
throw new InvalidOperationException(
1089+
CoreStrings.ConflictingRelationshipConversions(
1090+
DeclaringType.DisplayName(), Name,
1091+
valueConverter.GetType().ShortDisplayName(), annotationValue.ShortDisplayName()));
1092+
}
1093+
1094+
if (valueConverterType != null
1095+
&& throwOnValueConverterConflict)
1096+
{
1097+
throw new InvalidOperationException(
1098+
CoreStrings.ConflictingRelationshipConversions(
1099+
DeclaringType.DisplayName(), Name,
1100+
valueConverterType.ShortDisplayName(), annotationValue.ShortDisplayName()));
1101+
}
1102+
1103+
providerClrType = annotationValue;
1104+
}
1105+
annotationFound = true;
1106+
}
1107+
1108+
return annotationFound;
1109+
}
1110+
}
1111+
8831112
/// <summary>
8841113
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
8851114
/// the same compatibility standards as public APIs. It may be changed or removed without notice in

‎src/EFCore/Properties/CoreStrings.Designer.cs

Lines changed: 9 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/EFCore/Properties/CoreStrings.resx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,9 @@
342342
<data name="ConflictingPropertyOrNavigation" xml:space="preserve">
343343
<value>The property or navigation '{member}' cannot be added to the '{type}' type because a property or navigation with the same name already exists on the '{conflictingType}' type.</value>
344344
</data>
345+
<data name="ConflictingRelationshipConversions" xml:space="preserve">
346+
<value>The property '{entityType}.{property}' participates in several relationship chains that have conflicting conversions: '{valueConversion}' and '{conflictingValueConversion}'.</value>
347+
</data>
345348
<data name="ConflictingRelationshipNavigation" xml:space="preserve">
346349
<value>Cannot create a relationship between '{newPrincipalNavigationSpecification}' and '{newDependentNavigationSpecification}' because a relationship already exists between '{existingPrincipalNavigationSpecification}' and '{existingDependentNavigationSpecification}'. Navigations can only participate in a single relationship. If you want to override an existing relationship call 'Ignore' on the navigation '{newDependentNavigationSpecification}' first in 'OnModelCreating'.</value>
347350
</data>

0 commit comments

Comments
 (0)