From 10727098399a41074236a9a37cbc6cfa8a85a854 Mon Sep 17 00:00:00 2001 From: Arthur Vickers Date: Mon, 27 Nov 2023 20:54:14 +0000 Subject: [PATCH 1/2] Create correct JSON reader/writer for double nested value conversions Fixes #32376 --- src/EFCore/Storage/CoreTypeMapping.cs | 19 ++++-- .../JsonTypesTestBase.cs | 66 +++++++++++++++++++ .../Baselines/BigModel/ManyTypesEntityType.cs | 24 ++----- .../ManyTypesEntityType.cs | 24 ++----- .../Baselines/BigModel/ManyTypesEntityType.cs | 16 ++--- .../ManyTypesEntityType.cs | 16 ++--- 6 files changed, 101 insertions(+), 64 deletions(-) diff --git a/src/EFCore/Storage/CoreTypeMapping.cs b/src/EFCore/Storage/CoreTypeMapping.cs index dba6280b10b..9239d2c5b05 100644 --- a/src/EFCore/Storage/CoreTypeMapping.cs +++ b/src/EFCore/Storage/CoreTypeMapping.cs @@ -4,6 +4,7 @@ using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using Microsoft.EntityFrameworkCore.Internal; +using Microsoft.EntityFrameworkCore.Storage.Internal; using Microsoft.EntityFrameworkCore.Storage.Json; namespace Microsoft.EntityFrameworkCore.Storage; @@ -134,11 +135,21 @@ public CoreTypeMappingParameters WithComposedConverter( ?? (converter == null || JsonValueReaderWriter == null ? JsonValueReaderWriter : RuntimeFeature.IsDynamicCodeSupported - ? (JsonValueReaderWriter)Activator.CreateInstance( - typeof(JsonConvertedValueReaderWriter<,>).MakeGenericType( - converter.ModelClrType, JsonValueReaderWriter.ValueType), - JsonValueReaderWriter, converter)! + ? CreateReaderWriter(converter, JsonValueReaderWriter) : throw new InvalidOperationException(CoreStrings.NativeAotNoCompiledModel))); + + static JsonValueReaderWriter CreateReaderWriter(ValueConverter converter, JsonValueReaderWriter readerWriter) + { + if (readerWriter is IJsonConvertedValueReaderWriter convertedValueReaderWriter) + { + readerWriter = convertedValueReaderWriter.InnerReaderWriter; + } + + return (JsonValueReaderWriter)Activator.CreateInstance( + typeof(JsonConvertedValueReaderWriter<,>).MakeGenericType( + converter.ModelClrType, readerWriter.ValueType), + readerWriter, converter)!; + } } } diff --git a/test/EFCore.Specification.Tests/JsonTypesTestBase.cs b/test/EFCore.Specification.Tests/JsonTypesTestBase.cs index cfa60109507..d36f85685db 100644 --- a/test/EFCore.Specification.Tests/JsonTypesTestBase.cs +++ b/test/EFCore.Specification.Tests/JsonTypesTestBase.cs @@ -1667,6 +1667,72 @@ protected class NullableDddIdType public DddId? DddId { get; set; } } + [ConditionalTheory] + [InlineData(EnumProperty.FieldA, """{"Prop":"A"}""")] + [InlineData(EnumProperty.FieldB, """{"Prop":"B"}""")] + public virtual void Can_read_write_enum_char_converted_type_JSON_values(int value, string json) + => Can_read_and_write_JSON_value( + b => b.Entity().HasNoKey().Property(e => e.EnumProperty), + b => b.Properties().HaveConversion>(), + nameof(EnumCharType.EnumProperty), + (EnumProperty)value, + json); + + protected class EnumValueConverter() : ValueConverter( + p => p.ToChar(null), p => (T)Enum.Parse(typeof(T), Convert.ToInt32(p).ToString())) + where T : Enum, IConvertible; + + protected class EnumCharType + { + public EnumProperty EnumProperty { get; set; } + } + + protected enum EnumProperty + { + FieldA = 'A', + FieldB = 'B', + FieldC = 'C', + } + + [ConditionalTheory] + [InlineData("127.0.0.1", """{"Prop":"127.0.0.1"}""")] + [InlineData("0.0.0.0", """{"Prop":"0.0.0.0"}""")] + [InlineData("255.255.255.255", """{"Prop":"255.255.255.255"}""")] + [InlineData("192.168.1.156", """{"Prop":"192.168.1.156"}""")] + [InlineData("::1", """{"Prop":"::1"}""")] + [InlineData("::", """{"Prop":"::"}""")] + [InlineData("2a00:23c7:c60f:4f01:ba43:6d5a:e648:7577", """{"Prop":"2a00:23c7:c60f:4f01:ba43:6d5a:e648:7577"}""")] + public virtual void Can_read_write_custom_converted_type_JSON_values(string value, string json) + => Can_read_and_write_JSON_value( + b => b.Entity().HasNoKey().Property(e => e.Address), + b => b.Properties().HaveConversion(), + nameof(IpAddressType.Address), + new(IPAddress.Parse(value)), + json); + + protected class IpAddressConverter() : ValueConverter( + v => v.Address, + v => new IpAddress(v)); + + protected class IpAddressType + { + public IpAddress? Address { get; set; } + } + + protected class IpAddress(IPAddress address) + { + public IPAddress Address { get; } = address; + + protected bool Equals(IpAddress other) + => Address.Equals(other.Address); + + public override bool Equals(object? obj) + => !ReferenceEquals(null, obj) && (ReferenceEquals(this, obj) || obj.GetType() == GetType() && Equals((IpAddress)obj)); + + public override int GetHashCode() + => Address.GetHashCode(); + } + [ConditionalFact] public virtual void Can_read_write_collection_of_sbyte_JSON_values() => Can_read_and_write_JSON_value>( diff --git a/test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/BigModel/ManyTypesEntityType.cs b/test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/BigModel/ManyTypesEntityType.cs index f5f7dbe11fb..eec96037a75 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/BigModel/ManyTypesEntityType.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/BigModel/ManyTypesEntityType.cs @@ -8574,12 +8574,8 @@ public static RuntimeEntityType Create(RuntimeModel model, RuntimeEntityType bas converter: new ValueConverter( (string v) => string.Format(CultureInfo.InvariantCulture, "{0}", (object)(v.Length < 1 ? '\0' : v[0])), (string v) => string.Format(CultureInfo.InvariantCulture, "{0}", (object)(v.Length < 1 ? '\0' : v[0]))), - jsonValueReaderWriter: new JsonConvertedValueReaderWriter( - new JsonConvertedValueReaderWriter( - JsonStringReaderWriter.Instance, - new ValueConverter( - (char v) => string.Format(CultureInfo.InvariantCulture, "{0}", (object)v), - (string v) => v.Length < 1 ? '\0' : v[0])), + jsonValueReaderWriter: new JsonConvertedValueReaderWriter( + JsonStringReaderWriter.Instance, new ValueConverter( (string v) => string.Format(CultureInfo.InvariantCulture, "{0}", (object)(v.Length < 1 ? '\0' : v[0])), (string v) => string.Format(CultureInfo.InvariantCulture, "{0}", (object)(v.Length < 1 ? '\0' : v[0]))))); @@ -8762,12 +8758,8 @@ public static RuntimeEntityType Create(RuntimeModel model, RuntimeEntityType bas converter: new ValueConverter( (string v) => (long)StringEnumConverter.ConvertToEnum(v), (long value) => ((CompiledModelTestBase.EnumU32)value).ToString()), - jsonValueReaderWriter: new JsonConvertedValueReaderWriter( - new JsonConvertedValueReaderWriter( - JsonInt64ReaderWriter.Instance, - new ValueConverter( - (CompiledModelTestBase.EnumU32 value) => (long)value, - (long value) => (CompiledModelTestBase.EnumU32)value)), + jsonValueReaderWriter: new JsonConvertedValueReaderWriter( + JsonInt64ReaderWriter.Instance, new ValueConverter( (string v) => (long)StringEnumConverter.ConvertToEnum(v), (long value) => ((CompiledModelTestBase.EnumU32)value).ToString()))); @@ -8916,12 +8908,8 @@ public static RuntimeEntityType Create(RuntimeModel model, RuntimeEntityType bas (string v) => new Uri(v, UriKind.RelativeOrAbsolute).ToString(), (string v) => new Uri(v, UriKind.RelativeOrAbsolute).ToString()), storeTypePostfix: StoreTypePostfix.None, - jsonValueReaderWriter: new JsonConvertedValueReaderWriter( - new JsonConvertedValueReaderWriter( - JsonStringReaderWriter.Instance, - new ValueConverter( - (Uri v) => v.ToString(), - (string v) => new Uri(v, UriKind.RelativeOrAbsolute))), + jsonValueReaderWriter: new JsonConvertedValueReaderWriter( + JsonStringReaderWriter.Instance, new ValueConverter( (string v) => new Uri(v, UriKind.RelativeOrAbsolute).ToString(), (string v) => new Uri(v, UriKind.RelativeOrAbsolute).ToString()))); diff --git a/test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/BigModel_with_JSON_columns/ManyTypesEntityType.cs b/test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/BigModel_with_JSON_columns/ManyTypesEntityType.cs index f5f7dbe11fb..eec96037a75 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/BigModel_with_JSON_columns/ManyTypesEntityType.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/BigModel_with_JSON_columns/ManyTypesEntityType.cs @@ -8574,12 +8574,8 @@ public static RuntimeEntityType Create(RuntimeModel model, RuntimeEntityType bas converter: new ValueConverter( (string v) => string.Format(CultureInfo.InvariantCulture, "{0}", (object)(v.Length < 1 ? '\0' : v[0])), (string v) => string.Format(CultureInfo.InvariantCulture, "{0}", (object)(v.Length < 1 ? '\0' : v[0]))), - jsonValueReaderWriter: new JsonConvertedValueReaderWriter( - new JsonConvertedValueReaderWriter( - JsonStringReaderWriter.Instance, - new ValueConverter( - (char v) => string.Format(CultureInfo.InvariantCulture, "{0}", (object)v), - (string v) => v.Length < 1 ? '\0' : v[0])), + jsonValueReaderWriter: new JsonConvertedValueReaderWriter( + JsonStringReaderWriter.Instance, new ValueConverter( (string v) => string.Format(CultureInfo.InvariantCulture, "{0}", (object)(v.Length < 1 ? '\0' : v[0])), (string v) => string.Format(CultureInfo.InvariantCulture, "{0}", (object)(v.Length < 1 ? '\0' : v[0]))))); @@ -8762,12 +8758,8 @@ public static RuntimeEntityType Create(RuntimeModel model, RuntimeEntityType bas converter: new ValueConverter( (string v) => (long)StringEnumConverter.ConvertToEnum(v), (long value) => ((CompiledModelTestBase.EnumU32)value).ToString()), - jsonValueReaderWriter: new JsonConvertedValueReaderWriter( - new JsonConvertedValueReaderWriter( - JsonInt64ReaderWriter.Instance, - new ValueConverter( - (CompiledModelTestBase.EnumU32 value) => (long)value, - (long value) => (CompiledModelTestBase.EnumU32)value)), + jsonValueReaderWriter: new JsonConvertedValueReaderWriter( + JsonInt64ReaderWriter.Instance, new ValueConverter( (string v) => (long)StringEnumConverter.ConvertToEnum(v), (long value) => ((CompiledModelTestBase.EnumU32)value).ToString()))); @@ -8916,12 +8908,8 @@ public static RuntimeEntityType Create(RuntimeModel model, RuntimeEntityType bas (string v) => new Uri(v, UriKind.RelativeOrAbsolute).ToString(), (string v) => new Uri(v, UriKind.RelativeOrAbsolute).ToString()), storeTypePostfix: StoreTypePostfix.None, - jsonValueReaderWriter: new JsonConvertedValueReaderWriter( - new JsonConvertedValueReaderWriter( - JsonStringReaderWriter.Instance, - new ValueConverter( - (Uri v) => v.ToString(), - (string v) => new Uri(v, UriKind.RelativeOrAbsolute))), + jsonValueReaderWriter: new JsonConvertedValueReaderWriter( + JsonStringReaderWriter.Instance, new ValueConverter( (string v) => new Uri(v, UriKind.RelativeOrAbsolute).ToString(), (string v) => new Uri(v, UriKind.RelativeOrAbsolute).ToString()))); diff --git a/test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/BigModel/ManyTypesEntityType.cs b/test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/BigModel/ManyTypesEntityType.cs index 5a842ffbbfa..6c5cb015ebc 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/BigModel/ManyTypesEntityType.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/BigModel/ManyTypesEntityType.cs @@ -7630,12 +7630,8 @@ public static RuntimeEntityType Create(RuntimeModel model, RuntimeEntityType bas converter: new ValueConverter( (string v) => (uint)StringEnumConverter.ConvertToEnum(v), (uint value) => ((CompiledModelTestBase.EnumU32)value).ToString()), - jsonValueReaderWriter: new JsonConvertedValueReaderWriter( - new JsonConvertedValueReaderWriter( - JsonUInt32ReaderWriter.Instance, - new ValueConverter( - (CompiledModelTestBase.EnumU32 value) => (uint)value, - (uint value) => (CompiledModelTestBase.EnumU32)value)), + jsonValueReaderWriter: new JsonConvertedValueReaderWriter( + JsonUInt32ReaderWriter.Instance, new ValueConverter( (string v) => (uint)StringEnumConverter.ConvertToEnum(v), (uint value) => ((CompiledModelTestBase.EnumU32)value).ToString()))); @@ -7761,12 +7757,8 @@ public static RuntimeEntityType Create(RuntimeModel model, RuntimeEntityType bas converter: new ValueConverter( (string v) => new Uri(v, UriKind.RelativeOrAbsolute).ToString(), (string v) => new Uri(v, UriKind.RelativeOrAbsolute).ToString()), - jsonValueReaderWriter: new JsonConvertedValueReaderWriter( - new JsonConvertedValueReaderWriter( - JsonStringReaderWriter.Instance, - new ValueConverter( - (Uri v) => v.ToString(), - (string v) => new Uri(v, UriKind.RelativeOrAbsolute))), + jsonValueReaderWriter: new JsonConvertedValueReaderWriter( + JsonStringReaderWriter.Instance, new ValueConverter( (string v) => new Uri(v, UriKind.RelativeOrAbsolute).ToString(), (string v) => new Uri(v, UriKind.RelativeOrAbsolute).ToString()))); diff --git a/test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/BigModel_with_JSON_columns/ManyTypesEntityType.cs b/test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/BigModel_with_JSON_columns/ManyTypesEntityType.cs index 5a842ffbbfa..6c5cb015ebc 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/BigModel_with_JSON_columns/ManyTypesEntityType.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/BigModel_with_JSON_columns/ManyTypesEntityType.cs @@ -7630,12 +7630,8 @@ public static RuntimeEntityType Create(RuntimeModel model, RuntimeEntityType bas converter: new ValueConverter( (string v) => (uint)StringEnumConverter.ConvertToEnum(v), (uint value) => ((CompiledModelTestBase.EnumU32)value).ToString()), - jsonValueReaderWriter: new JsonConvertedValueReaderWriter( - new JsonConvertedValueReaderWriter( - JsonUInt32ReaderWriter.Instance, - new ValueConverter( - (CompiledModelTestBase.EnumU32 value) => (uint)value, - (uint value) => (CompiledModelTestBase.EnumU32)value)), + jsonValueReaderWriter: new JsonConvertedValueReaderWriter( + JsonUInt32ReaderWriter.Instance, new ValueConverter( (string v) => (uint)StringEnumConverter.ConvertToEnum(v), (uint value) => ((CompiledModelTestBase.EnumU32)value).ToString()))); @@ -7761,12 +7757,8 @@ public static RuntimeEntityType Create(RuntimeModel model, RuntimeEntityType bas converter: new ValueConverter( (string v) => new Uri(v, UriKind.RelativeOrAbsolute).ToString(), (string v) => new Uri(v, UriKind.RelativeOrAbsolute).ToString()), - jsonValueReaderWriter: new JsonConvertedValueReaderWriter( - new JsonConvertedValueReaderWriter( - JsonStringReaderWriter.Instance, - new ValueConverter( - (Uri v) => v.ToString(), - (string v) => new Uri(v, UriKind.RelativeOrAbsolute))), + jsonValueReaderWriter: new JsonConvertedValueReaderWriter( + JsonStringReaderWriter.Instance, new ValueConverter( (string v) => new Uri(v, UriKind.RelativeOrAbsolute).ToString(), (string v) => new Uri(v, UriKind.RelativeOrAbsolute).ToString()))); From b77f343aafb320faa9e8854be624549f9d9126cb Mon Sep 17 00:00:00 2001 From: Arthur Vickers Date: Wed, 29 Nov 2023 09:32:01 +0000 Subject: [PATCH 2/2] Review update --- src/EFCore/Storage/CoreTypeMapping.cs | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/EFCore/Storage/CoreTypeMapping.cs b/src/EFCore/Storage/CoreTypeMapping.cs index 9239d2c5b05..dbbaf41c730 100644 --- a/src/EFCore/Storage/CoreTypeMapping.cs +++ b/src/EFCore/Storage/CoreTypeMapping.cs @@ -131,15 +131,20 @@ public CoreTypeMappingParameters WithComposedConverter( ProviderValueComparer, ValueGeneratorFactory, elementMapping ?? ElementTypeMapping, - jsonValueReaderWriter - ?? (converter == null || JsonValueReaderWriter == null - ? JsonValueReaderWriter - : RuntimeFeature.IsDynamicCodeSupported - ? CreateReaderWriter(converter, JsonValueReaderWriter) - : throw new InvalidOperationException(CoreStrings.NativeAotNoCompiledModel))); - - static JsonValueReaderWriter CreateReaderWriter(ValueConverter converter, JsonValueReaderWriter readerWriter) + jsonValueReaderWriter ?? CreateReaderWriter(converter, JsonValueReaderWriter)); + + static JsonValueReaderWriter? CreateReaderWriter(ValueConverter? converter, JsonValueReaderWriter? readerWriter) { + if (converter == null || readerWriter == null) + { + return readerWriter; + } + + if (!RuntimeFeature.IsDynamicCodeSupported) + { + throw new InvalidOperationException(CoreStrings.NativeAotNoCompiledModel); + } + if (readerWriter is IJsonConvertedValueReaderWriter convertedValueReaderWriter) { readerWriter = convertedValueReaderWriter.InnerReaderWriter;