From f34da5bdff6c3910bc69713d455a914bb27d81b6 Mon Sep 17 00:00:00 2001 From: Maurycy Markowski Date: Mon, 3 Apr 2023 13:34:24 -0700 Subject: [PATCH] Fix to #30330 - Json: updating property with conversion from string to other type fails on sql server When producing update statement, when deciding whether we need CAST around the result of JSON_VALUE, we need to look if the property has converter, rather than just look at it's clr type. If the clr type is string, but it gets converted to int/bool etc we still need a CAST. Fixes #30330 --- .../Internal/SqlServerUpdateSqlGenerator.cs | 18 +++++++- .../Query/JsonQueryFixtureBase.cs | 21 ++++++++++ .../Update/JsonUpdateFixtureBase.cs | 42 +++++++++++++++++++ .../Update/JsonUpdateSqlServerTest.cs | 24 +++++------ 4 files changed, 91 insertions(+), 14 deletions(-) diff --git a/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs b/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs index bf6827a2482..48661d21d52 100644 --- a/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs +++ b/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs @@ -15,6 +15,9 @@ namespace Microsoft.EntityFrameworkCore.SqlServer.Update.Internal; /// public class SqlServerUpdateSqlGenerator : UpdateAndSelectSqlGenerator, ISqlServerUpdateSqlGenerator { + private static readonly bool UseOldBehavior30330 + = AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue30330 ", out var enabled30330) && enabled30330; + /// /// 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 @@ -155,8 +158,19 @@ protected override void AppendUpdateColumnValue( if (columnModification.Property != null) { - var needsTypeConversion = columnModification.Property.ClrType.IsNumeric() - || columnModification.Property.ClrType == typeof(bool); + bool needsTypeConversion; + if (!UseOldBehavior30330) + { + var propertyClrType = columnModification.Property.GetTypeMapping().Converter?.ProviderClrType + ?? columnModification.Property.ClrType; + + needsTypeConversion = propertyClrType.IsNumeric() || propertyClrType == typeof(bool); + } + else + { + needsTypeConversion = columnModification.Property.ClrType.IsNumeric() + || columnModification.Property.ClrType == typeof(bool); + } if (needsTypeConversion) { diff --git a/test/EFCore.Relational.Specification.Tests/Query/JsonQueryFixtureBase.cs b/test/EFCore.Relational.Specification.Tests/Query/JsonQueryFixtureBase.cs index 6e9d16bba1d..ab71b2ffb76 100644 --- a/test/EFCore.Relational.Specification.Tests/Query/JsonQueryFixtureBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Query/JsonQueryFixtureBase.cs @@ -530,6 +530,27 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con { b.ToJson(); b.Property(x => x.TestDecimal).HasPrecision(18, 3); + b.Property(x => x.TestEnumWithIntConverter).HasConversion(); + b.Property(x => x.TestNullableEnumWithIntConverter).HasConversion(); + b.Property(x => x.TestNullableEnumWithConverterThatHandlesNulls).HasConversion( + new ValueConverter( + x => x == null + ? "Null" + : x == JsonEnum.One + ? "One" + : x == JsonEnum.Two + ? "Two" + : x == JsonEnum.Three + ? "Three" + : "INVALID", + x => x == "One" + ? JsonEnum.One + : x == "Two" + ? JsonEnum.Two + : x == "Three" + ? JsonEnum.Three + : null, + convertsNulls: true)); }); } } diff --git a/test/EFCore.Relational.Specification.Tests/Update/JsonUpdateFixtureBase.cs b/test/EFCore.Relational.Specification.Tests/Update/JsonUpdateFixtureBase.cs index b3fe4d6c2e5..f8b5e6e0576 100644 --- a/test/EFCore.Relational.Specification.Tests/Update/JsonUpdateFixtureBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Update/JsonUpdateFixtureBase.cs @@ -116,12 +116,54 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con { b.ToJson(); b.Property(x => x.TestDecimal).HasPrecision(18, 3); + b.Property(x => x.TestEnumWithIntConverter).HasConversion(); + b.Property(x => x.TestNullableEnumWithIntConverter).HasConversion(); + b.Property(x => x.TestNullableEnumWithConverterThatHandlesNulls).HasConversion( + new ValueConverter( + x => x == null + ? "Null" + : x == JsonEnum.One + ? "One" + : x == JsonEnum.Two + ? "Two" + : x == JsonEnum.Three + ? "Three" + : "INVALID", + x => x == "One" + ? JsonEnum.One + : x == "Two" + ? JsonEnum.Two + : x == "Three" + ? JsonEnum.Three + : null, + convertsNulls: true)); }); modelBuilder.Entity().OwnsMany( x => x.Collection, b => { b.ToJson(); b.Property(x => x.TestDecimal).HasPrecision(18, 3); + b.Property(x => x.TestEnumWithIntConverter).HasConversion(); + b.Property(x => x.TestNullableEnumWithIntConverter).HasConversion(); + b.Property(x => x.TestNullableEnumWithConverterThatHandlesNulls).HasConversion( + new ValueConverter( + x => x == null + ? "Null" + : x == JsonEnum.One + ? "One" + : x == JsonEnum.Two + ? "Two" + : x == JsonEnum.Three + ? "Three" + : "INVALID", + x => x == "One" + ? JsonEnum.One + : x == "Two" + ? JsonEnum.Two + : x == "Three" + ? JsonEnum.Three + : null, + convertsNulls: true)); }); base.OnModelCreating(modelBuilder, context); diff --git a/test/EFCore.SqlServer.FunctionalTests/Update/JsonUpdateSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Update/JsonUpdateSqlServerTest.cs index c332f9629ac..33303f02fa3 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Update/JsonUpdateSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Update/JsonUpdateSqlServerTest.cs @@ -558,7 +558,7 @@ public override async Task Edit_single_property_char() SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; -UPDATE [JsonEntitiesAllTypes] SET [Reference] = JSON_MODIFY([Reference], 'strict $.TestCharacter', CAST(JSON_VALUE(@p0, '$[0]') AS nvarchar(1))) +UPDATE [JsonEntitiesAllTypes] SET [Reference] = JSON_MODIFY([Reference], 'strict $.TestCharacter', JSON_VALUE(@p0, '$[0]')) OUTPUT 1 WHERE [Id] = @p1; """, @@ -984,13 +984,13 @@ public override async Task Edit_single_property_enum_with_int_converter() AssertSql( """ -@p0='["Three"]' (Nullable = false) (Size = 9) -@p1='["Three"]' (Nullable = false) (Size = 9) +@p0='[2]' (Nullable = false) (Size = 3) +@p1='[2]' (Nullable = false) (Size = 3) @p2='1' SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; -UPDATE [JsonEntitiesAllTypes] SET [Collection] = JSON_MODIFY([Collection], 'strict $[0].TestEnumWithIntConverter', JSON_VALUE(@p0, '$[0]')), [Reference] = JSON_MODIFY([Reference], 'strict $.TestEnumWithIntConverter', JSON_VALUE(@p1, '$[0]')) +UPDATE [JsonEntitiesAllTypes] SET [Collection] = JSON_MODIFY([Collection], 'strict $[0].TestEnumWithIntConverter', CAST(JSON_VALUE(@p0, '$[0]') AS int)), [Reference] = JSON_MODIFY([Reference], 'strict $.TestEnumWithIntConverter', CAST(JSON_VALUE(@p1, '$[0]') AS int)) OUTPUT 1 WHERE [Id] = @p2; """, @@ -1056,13 +1056,13 @@ public override async Task Edit_single_property_nullable_enum_with_int_converter AssertSql( """ -@p0='["One"]' (Nullable = false) (Size = 7) -@p1='["Three"]' (Nullable = false) (Size = 9) +@p0='[0]' (Nullable = false) (Size = 3) +@p1='[2]' (Nullable = false) (Size = 3) @p2='1' SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; -UPDATE [JsonEntitiesAllTypes] SET [Collection] = JSON_MODIFY([Collection], 'strict $[0].TestNullableEnumWithIntConverter', JSON_VALUE(@p0, '$[0]')), [Reference] = JSON_MODIFY([Reference], 'strict $.TestNullableEnumWithIntConverter', JSON_VALUE(@p1, '$[0]')) +UPDATE [JsonEntitiesAllTypes] SET [Collection] = JSON_MODIFY([Collection], 'strict $[0].TestNullableEnumWithIntConverter', CAST(JSON_VALUE(@p0, '$[0]') AS int)), [Reference] = JSON_MODIFY([Reference], 'strict $.TestNullableEnumWithIntConverter', CAST(JSON_VALUE(@p1, '$[0]') AS int)) OUTPUT 1 WHERE [Id] = @p2; """, @@ -1086,7 +1086,7 @@ public override async Task Edit_single_property_nullable_enum_with_int_converter SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; -UPDATE [JsonEntitiesAllTypes] SET [Collection] = JSON_MODIFY([Collection], 'strict $[0].TestNullableEnumWithIntConverter', JSON_VALUE(@p0, '$[0]')), [Reference] = JSON_MODIFY([Reference], 'strict $.TestNullableEnumWithIntConverter', JSON_VALUE(@p1, '$[0]')) +UPDATE [JsonEntitiesAllTypes] SET [Collection] = JSON_MODIFY([Collection], 'strict $[0].TestNullableEnumWithIntConverter', CAST(JSON_VALUE(@p0, '$[0]') AS int)), [Reference] = JSON_MODIFY([Reference], 'strict $.TestNullableEnumWithIntConverter', CAST(JSON_VALUE(@p1, '$[0]') AS int)) OUTPUT 1 WHERE [Id] = @p2; """, @@ -1128,8 +1128,8 @@ public override async Task Edit_single_property_nullable_enum_with_converter_tha AssertSql( """ -@p0='[null]' (Nullable = false) (Size = 6) -@p1='[null]' (Nullable = false) (Size = 6) +@p0='["Null"]' (Nullable = false) (Size = 8) +@p1='["Null"]' (Nullable = false) (Size = 8) @p2='1' SET IMPLICIT_TRANSACTIONS OFF; @@ -1152,8 +1152,8 @@ public override async Task Edit_two_properties_on_same_entity_updates_the_entire AssertSql( """ -@p0='{"TestBoolean":false,"TestByte":25,"TestCharacter":"h","TestDateTime":"2100-11-11T12:34:56","TestDateTimeOffset":"2200-11-11T12:34:56-05:00","TestDecimal":-123450.01,"TestDouble":-1.2345,"TestEnum":"One","TestEnumWithIntConverter":"Two","TestGuid":"00000000-0000-0000-0000-000000000000","TestInt16":-12,"TestInt32":32,"TestInt64":64,"TestNullableEnum":"One","TestNullableEnumWithConverterThatHandlesNulls":"Two","TestNullableEnumWithIntConverter":"Three","TestNullableInt32":90,"TestSignedByte":-18,"TestSingle":-1.4,"TestTimeSpan":"06:05:04.0030000","TestUnsignedInt16":12,"TestUnsignedInt32":12345,"TestUnsignedInt64":1234567867}' (Nullable = false) (Size = 631) -@p1='{"TestBoolean":true,"TestByte":255,"TestCharacter":"a","TestDateTime":"2000-01-01T12:34:56","TestDateTimeOffset":"2000-01-01T12:34:56-08:00","TestDecimal":-1234567890.01,"TestDouble":-1.23456789,"TestEnum":"One","TestEnumWithIntConverter":"Two","TestGuid":"12345678-1234-4321-7777-987654321000","TestInt16":-1234,"TestInt32":32,"TestInt64":64,"TestNullableEnum":"One","TestNullableEnumWithConverterThatHandlesNulls":"Three","TestNullableEnumWithIntConverter":"Two","TestNullableInt32":78,"TestSignedByte":-128,"TestSingle":-1.234,"TestTimeSpan":"10:09:08.0070000","TestUnsignedInt16":1234,"TestUnsignedInt32":1234565789,"TestUnsignedInt64":1234567890123456789}' (Nullable = false) (Size = 660) +@p0='{"TestBoolean":false,"TestByte":25,"TestCharacter":"h","TestDateTime":"2100-11-11T12:34:56","TestDateTimeOffset":"2200-11-11T12:34:56-05:00","TestDecimal":-123450.01,"TestDouble":-1.2345,"TestEnum":"One","TestEnumWithIntConverter":1,"TestGuid":"00000000-0000-0000-0000-000000000000","TestInt16":-12,"TestInt32":32,"TestInt64":64,"TestNullableEnum":"One","TestNullableEnumWithConverterThatHandlesNulls":"Two","TestNullableEnumWithIntConverter":2,"TestNullableInt32":90,"TestSignedByte":-18,"TestSingle":-1.4,"TestTimeSpan":"06:05:04.0030000","TestUnsignedInt16":12,"TestUnsignedInt32":12345,"TestUnsignedInt64":1234567867}' (Nullable = false) (Size = 621) +@p1='{"TestBoolean":true,"TestByte":255,"TestCharacter":"a","TestDateTime":"2000-01-01T12:34:56","TestDateTimeOffset":"2000-01-01T12:34:56-08:00","TestDecimal":-1234567890.01,"TestDouble":-1.23456789,"TestEnum":"One","TestEnumWithIntConverter":1,"TestGuid":"12345678-1234-4321-7777-987654321000","TestInt16":-1234,"TestInt32":32,"TestInt64":64,"TestNullableEnum":"One","TestNullableEnumWithConverterThatHandlesNulls":"Three","TestNullableEnumWithIntConverter":1,"TestNullableInt32":78,"TestSignedByte":-128,"TestSingle":-1.234,"TestTimeSpan":"10:09:08.0070000","TestUnsignedInt16":1234,"TestUnsignedInt32":1234565789,"TestUnsignedInt64":1234567890123456789}' (Nullable = false) (Size = 652) @p2='1' SET IMPLICIT_TRANSACTIONS OFF;