diff --git a/src/EFCore.Design/Scaffolding/Internal/CSharpRuntimeModelCodeGenerator.cs b/src/EFCore.Design/Scaffolding/Internal/CSharpRuntimeModelCodeGenerator.cs index 01cdde9a365..84813be8bd9 100644 --- a/src/EFCore.Design/Scaffolding/Internal/CSharpRuntimeModelCodeGenerator.cs +++ b/src/EFCore.Design/Scaffolding/Internal/CSharpRuntimeModelCodeGenerator.cs @@ -899,6 +899,12 @@ private void Create( return (Type?)annotation.Value; } + if (!Property.UseOldBehavior32422) + { + return ((Property)property).GetConversion(throwOnProviderClrTypeConflict: false, throwOnValueConverterConflict: false) + .ValueConverterType; + } + var principalProperty = property; var i = 0; for (; i < ForeignKey.LongestFkChainAllowedLength; i++) diff --git a/src/EFCore/Metadata/Internal/Property.cs b/src/EFCore/Metadata/Internal/Property.cs index 6cc5ee1dbaf..a8fcb4eee96 100644 --- a/src/EFCore/Metadata/Internal/Property.cs +++ b/src/EFCore/Metadata/Internal/Property.cs @@ -31,6 +31,15 @@ public class Property : PropertyBase, IMutableProperty, IConventionProperty, IPr private ConfigurationSource? _valueGeneratedConfigurationSource; private ConfigurationSource? _typeMappingConfigurationSource; + /// + /// 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. + /// + public static readonly bool UseOldBehavior32422 = + AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue32422", out var enabled32422) && enabled32422; + /// /// 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 @@ -744,6 +753,12 @@ private object? DefaultSentinel return (ValueConverter?)annotation.Value; } + if (!UseOldBehavior32422) + { + return GetConversion(throwOnProviderClrTypeConflict: FindAnnotation(CoreAnnotationNames.ProviderClrType) == null) + .ValueConverter; + } + var property = this; var i = 0; for (; i < ForeignKey.LongestFkChainAllowedLength; i++) @@ -836,6 +851,12 @@ private object? DefaultSentinel return (Type?)annotation.Value; } + if (!UseOldBehavior32422) + { + return GetConversion(throwOnValueConverterConflict: FindAnnotation(CoreAnnotationNames.ValueConverter) == null) + .ProviderClrType; + } + var property = this; var i = 0; for (; i < ForeignKey.LongestFkChainAllowedLength; i++) @@ -880,6 +901,214 @@ private object? DefaultSentinel : null; } + /// + /// 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. + /// + public virtual (ValueConverter? ValueConverter, Type? ValueConverterType, Type? ProviderClrType) GetConversion( + bool throwOnValueConverterConflict = true, + bool throwOnProviderClrTypeConflict = true) + { + Queue<(Property CurrentProperty, Property CycleBreakingPropert, int CyclePosition, int MaxCycleLength)>? queue = null; + (Property CurrentProperty, Property CycleBreakingPropert, int CyclePosition, int MaxCycleLength)? currentNode = + (this, this, 0, 2); + + ValueConverter? valueConverter = null; + Type? valueConverterType = null; + Type? providerClrType = null; + while (currentNode is not null || queue is { Count: > 0 }) + { + var (property, cycleBreakingProperty, cyclePosition, maxCycleLength) = currentNode ?? queue!.Dequeue(); + currentNode = null; + if (cyclePosition >= ForeignKey.LongestFkChainAllowedLength) + { + throw new InvalidOperationException( + CoreStrings.RelationshipCycle(DeclaringType.DisplayName(), Name, "ValueConverter")); + } + + foreach (var foreignKey in property.GetContainingForeignKeys()) + { + for (var propertyIndex = 0; propertyIndex < foreignKey.Properties.Count; propertyIndex++) + { + if (property != foreignKey.Properties[propertyIndex]) + { + continue; + } + + var principalProperty = foreignKey.PrincipalKey.Properties[propertyIndex]; + if (principalProperty == cycleBreakingProperty) + { + break; + } + + var annotationFound = GetConversion( + principalProperty, + throwOnValueConverterConflict, + throwOnProviderClrTypeConflict, + ref valueConverter, + ref valueConverterType, + ref providerClrType); + if (!annotationFound) + { + if (currentNode != null) + { + queue = new(); + queue.Enqueue(currentNode.Value); + } + + if (cyclePosition == maxCycleLength - 1) + { + // We need to use different primes to ensure a different cycleBreakingProperty is selected + // each time when traversing properties that participate in multiple relationship cycles + currentNode = (principalProperty, property, 0, HashHelpers.GetPrime(maxCycleLength << 1)); + } + else + { + currentNode = (principalProperty, cycleBreakingProperty, cyclePosition + 1, maxCycleLength); + } + + if (queue != null) + { + queue.Enqueue(currentNode.Value); + currentNode = null; + } + } + break; + } + } + } + + return (valueConverter, valueConverterType, providerClrType); + + bool GetConversion( + Property principalProperty, + bool throwOnValueConverterConflict, + bool throwOnProviderClrTypeConflict, + ref ValueConverter? valueConverter, + ref Type? valueConverterType, + ref Type? providerClrType) + { + var annotationFound = false; + var valueConverterAnnotation = principalProperty.FindAnnotation(CoreAnnotationNames.ValueConverter); + if (valueConverterAnnotation != null) + { + var annotationValue = (ValueConverter?)valueConverterAnnotation.Value; + if (annotationValue != null) + { + if (valueConverter != null + && annotationValue.GetType() != valueConverter.GetType()) + { + throw new InvalidOperationException( + CoreStrings.ConflictingRelationshipConversions( + DeclaringType.DisplayName(), Name, + valueConverter.GetType().ShortDisplayName(), annotationValue.GetType().ShortDisplayName())); + } + + if (valueConverterType != null + && annotationValue.GetType() != valueConverterType) + { + throw new InvalidOperationException( + CoreStrings.ConflictingRelationshipConversions( + DeclaringType.DisplayName(), Name, + valueConverterType.ShortDisplayName(), annotationValue.GetType().ShortDisplayName())); + } + + if (providerClrType != null + && throwOnProviderClrTypeConflict) + { + throw new InvalidOperationException( + CoreStrings.ConflictingRelationshipConversions( + DeclaringType.DisplayName(), Name, + providerClrType.ShortDisplayName(), annotationValue.GetType().ShortDisplayName())); + } + + valueConverter = annotationValue; + } + annotationFound = true; + } + + var valueConverterTypeAnnotation = principalProperty.FindAnnotation(CoreAnnotationNames.ValueConverterType); + if (valueConverterTypeAnnotation != null) + { + var annotationValue = (Type?)valueConverterTypeAnnotation.Value; + if (annotationValue != null) + { + if (valueConverter != null + && valueConverter.GetType() != annotationValue) + { + throw new InvalidOperationException( + CoreStrings.ConflictingRelationshipConversions( + DeclaringType.DisplayName(), Name, + valueConverter.GetType().ShortDisplayName(), annotationValue.ShortDisplayName())); + } + + if (valueConverterType != null + && valueConverterType != annotationValue) + { + throw new InvalidOperationException( + CoreStrings.ConflictingRelationshipConversions( + DeclaringType.DisplayName(), Name, + valueConverterType.ShortDisplayName(), annotationValue.ShortDisplayName())); + } + + if (providerClrType != null + && throwOnProviderClrTypeConflict) + { + throw new InvalidOperationException( + CoreStrings.ConflictingRelationshipConversions( + DeclaringType.DisplayName(), Name, + providerClrType.ShortDisplayName(), annotationValue.ShortDisplayName())); + } + + valueConverterType = annotationValue; + } + annotationFound = true; + } + + var providerClrTypeAnnotation = principalProperty.FindAnnotation(CoreAnnotationNames.ProviderClrType); + if (providerClrTypeAnnotation != null) + { + var annotationValue = (Type?)providerClrTypeAnnotation.Value; + if (annotationValue != null) + { + if (providerClrType != null + && annotationValue != providerClrType) + { + throw new InvalidOperationException( + CoreStrings.ConflictingRelationshipConversions( + DeclaringType.DisplayName(), Name, + providerClrType.ShortDisplayName(), annotationValue.ShortDisplayName())); + } + + if (valueConverter != null + && throwOnValueConverterConflict) + { + throw new InvalidOperationException( + CoreStrings.ConflictingRelationshipConversions( + DeclaringType.DisplayName(), Name, + valueConverter.GetType().ShortDisplayName(), annotationValue.ShortDisplayName())); + } + + if (valueConverterType != null + && throwOnValueConverterConflict) + { + throw new InvalidOperationException( + CoreStrings.ConflictingRelationshipConversions( + DeclaringType.DisplayName(), Name, + valueConverterType.ShortDisplayName(), annotationValue.ShortDisplayName())); + } + + providerClrType = annotationValue; + } + annotationFound = true; + } + + return annotationFound; + } + } + /// /// 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 diff --git a/src/EFCore/Properties/CoreStrings.Designer.cs b/src/EFCore/Properties/CoreStrings.Designer.cs index 9546ff18313..fd7a93208b0 100644 --- a/src/EFCore/Properties/CoreStrings.Designer.cs +++ b/src/EFCore/Properties/CoreStrings.Designer.cs @@ -5,6 +5,7 @@ using System.Resources; using System.Threading; using Microsoft.EntityFrameworkCore.Diagnostics; +using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Internal; using Microsoft.Extensions.Logging; @@ -622,6 +623,14 @@ public static string ConflictingPropertyOrNavigation(object? member, object? typ GetString("ConflictingPropertyOrNavigation", nameof(member), nameof(type), nameof(conflictingType)), member, type, conflictingType); + /// + /// The property '{entityType}.{property}' participates in several relationship chains that have conflicting conversions: '{valueConversion}' and '{conflictingValueConversion}'. + /// + public static string ConflictingRelationshipConversions(object? entityType, object? property, object? valueConversion, object? conflictingValueConversion) + => string.Format( + GetString("ConflictingRelationshipConversions", nameof(entityType), nameof(property), nameof(valueConversion), nameof(conflictingValueConversion)), + entityType, property, valueConversion, conflictingValueConversion); + /// /// 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'. /// diff --git a/src/EFCore/Properties/CoreStrings.resx b/src/EFCore/Properties/CoreStrings.resx index d5fc18b31e8..45b45251ed8 100644 --- a/src/EFCore/Properties/CoreStrings.resx +++ b/src/EFCore/Properties/CoreStrings.resx @@ -342,6 +342,9 @@ 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. + + The property '{entityType}.{property}' participates in several relationship chains that have conflicting conversions: '{valueConversion}' and '{conflictingValueConversion}'. + 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'. diff --git a/src/Shared/HashHelpers.cs b/src/Shared/HashHelpers.cs new file mode 100644 index 00000000000..f9055d6211a --- /dev/null +++ b/src/Shared/HashHelpers.cs @@ -0,0 +1,117 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#nullable enable + +namespace Microsoft.EntityFrameworkCore.Utilities +{ + internal static partial class HashHelpers + { + internal static int PowerOf2(int v) + { + if ((v & (v - 1)) == 0) + { + return v; + } + + var i = 2; + while (i < v) + { + i <<= 1; + } + + return i; + } + + // must never be written to + internal static readonly int[] SizeOneIntArray = new int[1]; + + public const int HashCollisionThreshold = 100; + + // This is the maximum prime smaller than Array.MaxArrayLength + public const int MaxPrimeArrayLength = 0x7FEFFFFD; + + public const int HashPrime = 101; + + // Table of prime numbers to use as hash table sizes. + // A typical resize algorithm would pick the smallest prime number in this array + // that is larger than twice the previous capacity. + // Suppose our Hashtable currently has capacity x and enough elements are added + // such that a resize needs to occur. Resizing first computes 2x then finds the + // first prime in the table greater than 2x, i.e. if primes are ordered + // p_1, p_2, ..., p_i, ..., it finds p_n such that p_n-1 < 2x < p_n. + // Doubling is important for preserving the asymptotic complexity of the + // hashtable operations such as add. Having a prime guarantees that double + // hashing does not lead to infinite loops. IE, your hash function will be + // h1(key) + i*h2(key), 0 <= i < size. h2 and the size must be relatively prime. + // We prefer the low computation costs of higher prime numbers over the increased + // memory allocation of a fixed prime number i.e. when right sizing a HashSet. + public static readonly int[] primes = { + 3, 7, 11, 17, 23, 29, 37, 47, 59, 71, 89, 107, 131, 163, 197, 239, 293, 353, 431, 521, 631, 761, 919, + 1103, 1327, 1597, 1931, 2333, 2801, 3371, 4049, 4861, 5839, 7013, 8419, 10103, 12143, 14591, + 17519, 21023, 25229, 30293, 36353, 43627, 52361, 62851, 75431, 90523, 108631, 130363, 156437, + 187751, 225307, 270371, 324449, 389357, 467237, 560689, 672827, 807403, 968897, 1162687, 1395263, + 1674319, 2009191, 2411033, 2893249, 3471899, 4166287, 4999559, 5999471, 7199369 }; + + public static bool IsPrime(int candidate) + { + if ((candidate & 1) != 0) + { + var limit = (int)Math.Sqrt(candidate); + for (var divisor = 3; divisor <= limit; divisor += 2) + { + if ((candidate % divisor) == 0) + { + return false; + } + } + return true; + } + return candidate == 2; + } + + public static int GetPrime(int min) + { + if (min < 0) + { + throw new ArgumentException("Hashtable's capacity overflowed and went negative. Check load factor, capacity and the current size of the table."); + } + + for (var i = 0; i < primes.Length; i++) + { + var prime = primes[i]; + if (prime >= min) + { + return prime; + } + } + + //outside of our predefined table. + //compute the hard way. + for (var i = (min | 1); i < int.MaxValue; i += 2) + { + if (IsPrime(i) && ((i - 1) % HashPrime != 0)) + { + return i; + } + } + return min; + } + + // Returns size of hashtable to grow to. + public static int ExpandPrime(int oldSize) + { + var newSize = 2 * oldSize; + + // Allow the hashtables to grow to maximum possible size (~2G elements) before encountering capacity overflow. + // Note that this check works even when _items.Length overflowed thanks to the (uint) cast + if ((uint)newSize > MaxPrimeArrayLength && MaxPrimeArrayLength > oldSize) + { + Debug.Assert(MaxPrimeArrayLength == GetPrime(MaxPrimeArrayLength), "Invalid MaxPrimeArrayLength"); + return MaxPrimeArrayLength; + } + + return GetPrime(newSize); + } + } +} diff --git a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs index 2619bb2e72a..26a5bd3fce4 100644 --- a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs +++ b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs @@ -726,7 +726,7 @@ public virtual void Detects_shared_table_root_cycle() modelBuilder.Entity().HasOne().WithOne().HasForeignKey(a => a.Id).HasPrincipalKey(b => b.Id).IsRequired(); modelBuilder.Entity().ToTable("Table"); - VerifyError(CoreStrings.RelationshipCycle("B", "AId", "ValueConverter"), modelBuilder); + VerifyError(CoreStrings.IdentifyingRelationshipCycle("A -> B"), modelBuilder); } [ConditionalFact] diff --git a/test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs b/test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs index 671d77d5201..88d0ee7d707 100644 --- a/test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs +++ b/test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs @@ -593,28 +593,90 @@ public virtual void Detects_identifying_relationship_cycle() 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(); - VerifyError(CoreStrings.RelationshipCycle("B", "AId", "ValueConverter"), modelBuilder); + VerifyError(CoreStrings.IdentifyingRelationshipCycle("A -> B -> C"), modelBuilder); } [ConditionalFact] - public virtual void Detects_relationship_cycle_for_property_configuration() + public virtual void Passes_on_relationship_cycle_for_property_configuration() { var modelBuilder = base.CreateConventionModelBuilder(); modelBuilder.Entity().HasBaseType((string)null); + 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(); + modelBuilder.Entity().HasOne().WithOne().HasForeignKey(c => c.Id).HasPrincipalKey(a => a.Id).IsRequired(); + modelBuilder.Entity().HasOne().WithOne().HasForeignKey(b => b.Id).HasPrincipalKey(c => c.Id).IsRequired(); + modelBuilder.Entity().HasOne().WithOne().HasForeignKey(d => d.Id).HasPrincipalKey(b => b.Id).IsRequired(); + + var dId = modelBuilder.Model.FindEntityType(typeof(D)).FindProperty(nameof(D.Id)); + + Assert.Null(dId.GetValueConverter()); + Assert.Null(dId.GetProviderClrType()); + } + + [ConditionalFact] + public virtual void Passes_on_multiple_relationship_cycles_for_property_configuration() + { + var modelBuilder = base.CreateConventionModelBuilder(); + + modelBuilder.Entity().HasBaseType((string)null); 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(c => c.Id).HasPrincipalKey(a => a.Id).IsRequired(); + modelBuilder.Entity().HasOne().WithOne().HasForeignKey(b => b.Id).HasPrincipalKey(c => c.Id).IsRequired(); + modelBuilder.Entity().HasOne().WithOne().HasForeignKey(d => d.Id).HasPrincipalKey(c => c.Id).IsRequired(); + modelBuilder.Entity().HasOne().WithOne().HasForeignKey(e => e.Id).HasPrincipalKey(d => d.Id).IsRequired(); + modelBuilder.Entity().HasOne().WithOne().HasForeignKey(c => c.Id).HasPrincipalKey(e => e.Id).IsRequired(); + + var aId = modelBuilder.Model.FindEntityType(typeof(A)).FindProperty(nameof(A.Id)); + + Assert.Null(aId.GetValueConverter()); + Assert.Null(aId.GetProviderClrType()); + } + + [ConditionalFact] + public virtual void Detects_conflicting_converter_and_provider_type_with_relationship_cycle() + { + var modelBuilder = base.CreateConventionModelBuilder(); + + modelBuilder.Entity().HasBaseType((string)null); + modelBuilder.Entity().HasBaseType((string)null); + modelBuilder.Entity().Property(b => b.Id).HasConversion(); + modelBuilder.Entity().Property(b => b.Id).HasConversion>(); + + modelBuilder.Entity().HasOne().WithOne().HasForeignKey(b => b.Id).HasPrincipalKey(c => c.Id).IsRequired(); + modelBuilder.Entity().HasOne().WithOne().HasForeignKey(c => c.Id).HasPrincipalKey(b => b.Id).IsRequired(); + modelBuilder.Entity().HasOne().WithOne().HasForeignKey(d => d.Id).HasPrincipalKey(a => a.Id).IsRequired(); + modelBuilder.Entity().HasOne().WithOne().HasForeignKey(d => d.Id).HasPrincipalKey(c => c.Id).IsRequired(); var dId = modelBuilder.Model.FindEntityType(typeof(D)).FindProperty(nameof(D.Id)); - Assert.Equal( - CoreStrings.RelationshipCycle(nameof(D), nameof(D.Id), "ValueConverter"), + Assert.Equal(CoreStrings.ConflictingRelationshipConversions("D", "Id", "string", "CastingConverter"), Assert.Throws(dId.GetValueConverter).Message); - Assert.Equal( - CoreStrings.RelationshipCycle(nameof(D), nameof(D.Id), "ProviderClrType"), + Assert.Equal(CoreStrings.ConflictingRelationshipConversions("D", "Id", "string", "CastingConverter"), + Assert.Throws(dId.GetProviderClrType).Message); + } + + [ConditionalFact] + public virtual void Detects_conflicting_provider_types_with_relationship_cycle() + { + var modelBuilder = base.CreateConventionModelBuilder(); + + modelBuilder.Entity().HasBaseType((string)null); + modelBuilder.Entity().HasBaseType((string)null); + modelBuilder.Entity().Property(c => c.Id).HasConversion(); + modelBuilder.Entity().Property(a => a.Id).HasConversion(); + + modelBuilder.Entity().HasOne().WithOne().HasForeignKey(b => b.Id).HasPrincipalKey(c => c.Id).IsRequired(); + modelBuilder.Entity().HasOne().WithOne().HasForeignKey(c => c.Id).HasPrincipalKey(b => b.Id).IsRequired(); + modelBuilder.Entity().HasOne().WithOne().HasForeignKey(d => d.Id).HasPrincipalKey(a => a.Id).IsRequired(); + modelBuilder.Entity().HasOne().WithOne().HasForeignKey(d => d.Id).HasPrincipalKey(c => c.Id).IsRequired(); + + var dId = modelBuilder.Model.FindEntityType(typeof(D)).FindProperty(nameof(D.Id)); + + Assert.Equal(CoreStrings.ConflictingRelationshipConversions("D", "Id", "string", "long"), + Assert.Throws(dId.GetValueConverter).Message); + Assert.Equal(CoreStrings.ConflictingRelationshipConversions("D", "Id", "string", "long"), Assert.Throws(dId.GetProviderClrType).Message); }