diff --git a/src/EFCore.Cosmos/Storage/Internal/CosmosTypeMapping.cs b/src/EFCore.Cosmos/Storage/Internal/CosmosTypeMapping.cs index cb62bd9a9e5..821ebe33886 100644 --- a/src/EFCore.Cosmos/Storage/Internal/CosmosTypeMapping.cs +++ b/src/EFCore.Cosmos/Storage/Internal/CosmosTypeMapping.cs @@ -63,9 +63,10 @@ protected CosmosTypeMapping(CoreTypeMappingParameters parameters) public override CoreTypeMapping Clone( ValueConverter? converter, ValueComparer? comparer = null, + ValueComparer? keyComparer = null, CoreTypeMapping? elementMapping = null, JsonValueReaderWriter? jsonValueReaderWriter = null) - => new CosmosTypeMapping(Parameters.WithComposedConverter(converter, comparer, elementMapping, jsonValueReaderWriter)); + => new CosmosTypeMapping(Parameters.WithComposedConverter(converter, comparer, keyComparer, elementMapping, jsonValueReaderWriter)); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to diff --git a/src/EFCore.InMemory/Storage/Internal/InMemoryTypeMapping.cs b/src/EFCore.InMemory/Storage/Internal/InMemoryTypeMapping.cs index bfd8126ac82..e29aa4b0fa0 100644 --- a/src/EFCore.InMemory/Storage/Internal/InMemoryTypeMapping.cs +++ b/src/EFCore.InMemory/Storage/Internal/InMemoryTypeMapping.cs @@ -56,9 +56,10 @@ private InMemoryTypeMapping(CoreTypeMappingParameters parameters) public override CoreTypeMapping Clone( ValueConverter? converter, ValueComparer? comparer = null, + ValueComparer? keyComparer = null, CoreTypeMapping? elementMapping = null, JsonValueReaderWriter? jsonValueReaderWriter = null) - => new InMemoryTypeMapping(Parameters.WithComposedConverter(converter, comparer, elementMapping, jsonValueReaderWriter)); + => new InMemoryTypeMapping(Parameters.WithComposedConverter(converter, comparer, keyComparer, elementMapping, jsonValueReaderWriter)); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs index 9ff0b64c9e8..0ad1dfc7ec8 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs @@ -808,8 +808,7 @@ when entityType.IsMappedToJson(): // (OPENJSON, json_each, etc), but we can't use it for distinct, as it would warp the results. // Instead, we will treat every non-key property as identifier. - // TODO: hack/workaround, see #31398 - foreach (var property in entityType.GetDeclaredProperties().Where(p => !p.IsPrimaryKey() && p.GetRelationalTypeMapping().ElementTypeMapping == null)) + foreach (var property in entityType.GetDeclaredProperties().Where(p => !p.IsPrimaryKey())) { typeProjectionIdentifiers.Add(entityProjection.BindProperty(property)); typeProjectionValueComparers.Add(property.GetKeyValueComparer()); @@ -826,8 +825,7 @@ when entityType.IsMappedToJson(): // entity type would have wiped the identifiers when generating the join. Check.DebugAssert(primaryKey != null, "primary key is null."); - // TODO: hack/workaround, see #31398 - foreach (var property in primaryKey.Properties.Where(x => x.GetRelationalTypeMapping().ElementTypeMapping == null)) + foreach (var property in primaryKey.Properties) { typeProjectionIdentifiers.Add(entityProjection.BindProperty(property)); typeProjectionValueComparers.Add(property.GetKeyValueComparer()); @@ -1851,7 +1849,7 @@ ConstantExpression AddStructuralTypeProjection(StructuralTypeProjectionExpressio { ownerEntity = ownership.PrincipalEntityType; } - } + } while (ownerEntity.IsMappedToJson()); var keyPropertyCount = ownerEntity.FindPrimaryKey()!.Properties.Count; diff --git a/src/EFCore.Relational/Storage/RelationalTypeMapping.cs b/src/EFCore.Relational/Storage/RelationalTypeMapping.cs index 1f12134f7b3..8d79b96d2cc 100644 --- a/src/EFCore.Relational/Storage/RelationalTypeMapping.cs +++ b/src/EFCore.Relational/Storage/RelationalTypeMapping.cs @@ -231,16 +231,18 @@ public RelationalTypeMappingParameters WithScale(int? scale) /// /// The converter. /// The comparer. + /// The key comparer. /// The element mapping, or for non-collection mappings. /// The JSON reader/writer, or to leave unchanged. /// The new parameter object. public RelationalTypeMappingParameters WithComposedConverter( ValueConverter? converter, ValueComparer? comparer, + ValueComparer? keyComparer, CoreTypeMapping? elementMapping, JsonValueReaderWriter? jsonValueReaderWriter) => new( - CoreParameters.WithComposedConverter(converter, comparer, elementMapping, jsonValueReaderWriter), + CoreParameters.WithComposedConverter(converter, comparer, keyComparer, elementMapping, jsonValueReaderWriter), StoreType, StoreTypePostfix, DbType, @@ -431,9 +433,10 @@ public virtual RelationalTypeMapping Clone(int? precision, int? scale) public override CoreTypeMapping Clone( ValueConverter? converter, ValueComparer? comparer = null, + ValueComparer? keyComparer = null, CoreTypeMapping? elementMapping = null, JsonValueReaderWriter? jsonValueReaderWriter = null) - => Clone(Parameters.WithComposedConverter(converter, comparer, elementMapping, jsonValueReaderWriter)); + => Clone(Parameters.WithComposedConverter(converter, comparer, keyComparer, elementMapping, jsonValueReaderWriter)); /// /// Clones the type mapping to update facets from the mapping info, if needed. diff --git a/src/EFCore.Relational/Storage/RelationalTypeMappingSource.cs b/src/EFCore.Relational/Storage/RelationalTypeMappingSource.cs index cc5cb6fe9f1..c49945a32de 100644 --- a/src/EFCore.Relational/Storage/RelationalTypeMappingSource.cs +++ b/src/EFCore.Relational/Storage/RelationalTypeMappingSource.cs @@ -231,6 +231,12 @@ protected override CoreTypeMapping FindMapping(in TypeMappingInfo mappingInfo) { var elementType = modelType.TryGetElementType(typeof(IEnumerable<>))!; + var comparer = (ValueComparer?)Activator.CreateInstance( + elementType.IsNullableValueType() + ? typeof(NullableValueTypeListComparer<>).MakeGenericType(elementType.UnwrapNullableType()) + : typeof(ListComparer<>).MakeGenericType(elementMapping!.Comparer.Type), + elementMapping!.Comparer); + return (RelationalTypeMapping)FindMapping( info.WithConverter( // Note that the converter info is only used temporarily here and never creates an instance. @@ -238,11 +244,8 @@ protected override CoreTypeMapping FindMapping(in TypeMappingInfo mappingInfo) .Clone( (ValueConverter)Activator.CreateInstance( typeof(CollectionToJsonStringConverter<>).MakeGenericType(elementType), collectionReaderWriter!)!, - (ValueComparer?)Activator.CreateInstance( - elementType.IsNullableValueType() - ? typeof(NullableValueTypeListComparer<>).MakeGenericType(elementType.UnwrapNullableType()) - : typeof(ListComparer<>).MakeGenericType(elementMapping!.Comparer.Type), - elementMapping!.Comparer), + comparer, + comparer, elementMapping, collectionReaderWriter); } diff --git a/src/EFCore/Storage/CoreTypeMapping.cs b/src/EFCore/Storage/CoreTypeMapping.cs index f84fa623bab..cf4a0030871 100644 --- a/src/EFCore/Storage/CoreTypeMapping.cs +++ b/src/EFCore/Storage/CoreTypeMapping.cs @@ -111,12 +111,14 @@ public CoreTypeMappingParameters( /// /// The converter. /// The comparer. + /// The key comparer. /// The element mapping, or for non-collection mappings. /// The JSON reader/writer, or to leave unchanged. /// The new parameter object. public CoreTypeMappingParameters WithComposedConverter( ValueConverter? converter, ValueComparer? comparer, + ValueComparer? keyComparer, CoreTypeMapping? elementMapping, JsonValueReaderWriter? jsonValueReaderWriter) { @@ -126,7 +128,7 @@ public CoreTypeMappingParameters WithComposedConverter( ClrType, converter ?? Converter, comparer ?? Comparer, - KeyComparer, + keyComparer?? KeyComparer, ProviderValueComparer, ValueGeneratorFactory, elementMapping ?? ElementTypeMapping, @@ -266,12 +268,14 @@ public virtual ValueComparer ProviderValueComparer /// /// The converter to use. /// The comparer to use, or for to keep the default. + /// The comparer to use when the value is a key, or for to keep the default. /// The element mapping, or for non-collection mappings. /// The JSON reader/writer, or to leave unchanged. /// A new type mapping public abstract CoreTypeMapping Clone( ValueConverter? converter, ValueComparer? comparer = null, + ValueComparer? keyComparer = null, CoreTypeMapping? elementMapping = null, JsonValueReaderWriter? jsonValueReaderWriter = null); diff --git a/src/EFCore/Storage/TypeMappingSource.cs b/src/EFCore/Storage/TypeMappingSource.cs index 855ba977af8..4edbd91de83 100644 --- a/src/EFCore/Storage/TypeMappingSource.cs +++ b/src/EFCore/Storage/TypeMappingSource.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Concurrent; +using ValueComparer = Microsoft.EntityFrameworkCore.ChangeTracking.ValueComparer; namespace Microsoft.EntityFrameworkCore.Storage; @@ -182,25 +183,30 @@ protected TypeMappingSource(TypeMappingSourceDependencies dependencies) Type? providerType, CoreTypeMapping? elementMapping) { - var elementType = modelType.TryGetElementType(typeof(IEnumerable<>))!; + if (TryFindJsonCollectionMapping( + info, modelType, providerType, ref elementMapping, out var collectionReaderWriter)) + { + var elementType = modelType.TryGetElementType(typeof(IEnumerable<>))!; + var comparer = (ValueComparer?)Activator.CreateInstance( + elementType.IsNullableValueType() + ? typeof(NullableValueTypeListComparer<>).MakeGenericType(elementType.UnwrapNullableType()) + : typeof(ListComparer<>).MakeGenericType(elementMapping!.Comparer.Type), + elementMapping!.Comparer); - return TryFindJsonCollectionMapping( - info, modelType, providerType, ref elementMapping, out var collectionReaderWriter) - ? FindMapping( + return FindMapping( info.WithConverter( // Note that the converter info is only used temporarily here and never creates an instance. new ValueConverterInfo(modelType, typeof(string), _ => null!)))! .Clone( (ValueConverter)Activator.CreateInstance( typeof(CollectionToJsonStringConverter<>).MakeGenericType(elementType), collectionReaderWriter!)!, - (ValueComparer?)Activator.CreateInstance( - elementType.IsNullableValueType() - ? typeof(NullableValueTypeListComparer<>).MakeGenericType(elementType.UnwrapNullableType()) - : typeof(ListComparer<>).MakeGenericType(elementMapping!.Comparer.Type), - elementMapping!.Comparer), + comparer, + comparer, elementMapping, - collectionReaderWriter) - : null; + collectionReaderWriter); + } + + return null; } /// diff --git a/test/EFCore.SqlServer.FunctionalTests/JsonTypesCustomMappingSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/JsonTypesCustomMappingSqlServerTest.cs index 4992c1549d7..6cee8193491 100644 --- a/test/EFCore.SqlServer.FunctionalTests/JsonTypesCustomMappingSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/JsonTypesCustomMappingSqlServerTest.cs @@ -55,6 +55,11 @@ private class TestSqlServerTypeMappingSource( info.CoreTypeMappingInfo, modelType, providerType, ref elementMapping, out var collectionReaderWriter)) { var elementType = TryGetElementType(modelType, typeof(IEnumerable<>))!; + var comparer = (ValueComparer?)Activator.CreateInstance( + IsNullableValueType(elementType) + ? typeof(NullableValueTypeListComparer<>).MakeGenericType(UnwrapNullableType(elementType)) + : typeof(ListComparer<>).MakeGenericType(elementMapping!.Comparer.Type), + elementMapping!.Comparer); return (RelationalTypeMapping)FindMapping( info.WithConverter( @@ -63,11 +68,8 @@ private class TestSqlServerTypeMappingSource( .Clone( (ValueConverter)Activator.CreateInstance( typeof(CollectionToJsonStringConverter<>).MakeGenericType(elementType), collectionReaderWriter!)!, - (ValueComparer?)Activator.CreateInstance( - IsNullableValueType(elementType) - ? typeof(NullableValueTypeListComparer<>).MakeGenericType(UnwrapNullableType(elementType)) - : typeof(ListComparer<>).MakeGenericType(elementMapping!.Comparer.Type), - elementMapping!.Comparer), + comparer, + comparer, elementMapping, collectionReaderWriter); } diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/JsonQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/JsonQuerySqlServerTest.cs index cbdddc497aa..85b72f084a9 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/JsonQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/JsonQuerySqlServerTest.cs @@ -1338,7 +1338,7 @@ [OwnedCollectionBranch] nvarchar(max) '$.OwnedCollectionBranch' AS JSON, [OwnedReferenceBranch] nvarchar(max) '$.OwnedReferenceBranch' AS JSON ) AS [o] ) AS [t] -ORDER BY [j].[Id], [t].[Name] +ORDER BY [j].[Id], [t].[Name], [t].[Names], [t].[Number] """); } @@ -1400,7 +1400,7 @@ WHERE CAST(JSON_VALUE([o2].[value], '$.Date') AS datetime2) <> '2000-01-01T00:00 ) AS [t2] ) AS [t1] LEFT JOIN [JsonEntitiesBasicForCollection] AS [j0] ON [j].[Id] = [j0].[ParentId] -ORDER BY [j].[Id], [t].[c], [t].[key], [t0].[Name], [t0].[Number], [t1].[c1], [t1].[key], [t1].[c10], [t1].[key0] +ORDER BY [j].[Id], [t].[c], [t].[key], [t0].[Name], [t0].[Names], [t0].[Number], [t0].[Numbers], [t1].[c1], [t1].[key], [t1].[c10], [t1].[key0] """); } @@ -1426,7 +1426,7 @@ [OwnedReferenceLeaf] nvarchar(max) '$.OwnedReferenceLeaf' AS JSON ) AS [o] ) AS [t] LEFT JOIN [JsonEntitiesBasicForCollection] AS [j0] ON [j].[Id] = [j0].[ParentId] -ORDER BY [j].[Id], [t].[Date], [t].[Enum], [t].[Fraction], [t].[NullableEnum] +ORDER BY [j].[Id], [t].[Date], [t].[Enum], [t].[Enums], [t].[Fraction], [t].[NullableEnum], [t].[NullableEnums] """); } diff --git a/test/EFCore.Tests/ChangeTracking/Internal/ChangeDetectorTest.cs b/test/EFCore.Tests/ChangeTracking/Internal/ChangeDetectorTest.cs index 7b9c501aee6..747fb8a21ec 100644 --- a/test/EFCore.Tests/ChangeTracking/Internal/ChangeDetectorTest.cs +++ b/test/EFCore.Tests/ChangeTracking/Internal/ChangeDetectorTest.cs @@ -314,9 +314,11 @@ public ConcreteTypeMapping(Type clrType, ValueConverter converter, ValueComparer public override CoreTypeMapping Clone( ValueConverter converter, ValueComparer comparer = null, + ValueComparer keyComparer = null, CoreTypeMapping elementMapping = null, JsonValueReaderWriter jsonValueReaderWriter = null) - => new ConcreteTypeMapping(Parameters.WithComposedConverter(converter, comparer, elementMapping, jsonValueReaderWriter)); + => new ConcreteTypeMapping(Parameters.WithComposedConverter( + converter, comparer, keyComparer, elementMapping, jsonValueReaderWriter)); protected override CoreTypeMapping Clone(CoreTypeMappingParameters parameters) => new ConcreteTypeMapping(parameters);