From 114ef46f26ebe75f4fe21d9037422785046b0646 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Fri, 27 Oct 2023 23:10:27 +0200 Subject: [PATCH] Various stuff from EFCore.PG JSON work --- .../Properties/RelationalStrings.Designer.cs | 8 ++++ .../Properties/RelationalStrings.resx | 3 ++ .../Update/ModificationCommand.cs | 10 +++- ...yableMethodTranslatingExpressionVisitor.cs | 1 + .../InternalEntityEntry.OriginalValues.cs | 31 +++++------- .../Internal/InternalEntityEntry.cs | 3 +- .../Internal/SnapshotFactoryFactory.cs | 38 +++++++-------- .../JsonTypesRelationalTestBase.cs | 47 ++++++++++++++++++ .../Query/JsonQueryFixtureBase.cs | 9 +--- .../Query/Ef6GroupByTestBase.cs | 9 +--- .../JsonTypesSqlServerTestBase.cs | 48 ------------------- 11 files changed, 100 insertions(+), 107 deletions(-) diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs index d7b638387fb..a142ac2f071 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs +++ b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs @@ -1197,6 +1197,14 @@ public static string JsonRequiredEntityWithNullJson(object? entity) GetString("JsonRequiredEntityWithNullJson", nameof(entity)), entity); + /// + /// Type mapping type '{typeMapping}', which is being used on property '{property}' on entity type '{entityType}' in a JSON document, has not defined a JsonValueReaderWriter. + /// + public static string JsonValueReadWriterMissingOnTypeMapping(object? typeMapping, object? property, object? entityType) + => string.Format( + GetString("JsonValueReadWriterMissingOnTypeMapping", nameof(typeMapping), nameof(property), nameof(entityType)), + typeMapping, property, entityType); + /// /// The mapping strategy '{mappingStrategy}' used for '{entityType}' is not supported for keyless entity types. See https://go.microsoft.com/fwlink/?linkid=2130430 for more information. /// diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx index fc80d7d8f27..299cb0d9e44 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -571,6 +571,9 @@ Entity {entity} is required but the JSON element containing it is null. + + Type mapping type '{typeMapping}', which is being used on property '{property}' on entity type '{entityType}' in a JSON document, has not defined a JsonValueReaderWriter. + The mapping strategy '{mappingStrategy}' used for '{entityType}' is not supported for keyless entity types. See https://go.microsoft.com/fwlink/?linkid=2130430 for more information. diff --git a/src/EFCore.Relational/Update/ModificationCommand.cs b/src/EFCore.Relational/Update/ModificationCommand.cs index f60d0369268..388db3c6a14 100644 --- a/src/EFCore.Relational/Update/ModificationCommand.cs +++ b/src/EFCore.Relational/Update/ModificationCommand.cs @@ -902,7 +902,15 @@ private void WriteJson( if (value is not null) { - (property.GetJsonValueReaderWriter() ?? property.GetTypeMapping().JsonValueReaderWriter)!.ToJson(writer, value); + var jsonValueReaderWriter = property.GetJsonValueReaderWriter() ?? property.GetTypeMapping().JsonValueReaderWriter; + if (jsonValueReaderWriter is null) + { + throw new InvalidOperationException( + RelationalStrings.JsonValueReadWriterMissingOnTypeMapping( + property.GetTypeMapping().GetType().Name, property.Name, entityType.DisplayName())); + } + + jsonValueReaderWriter.ToJson(writer, value); } else { diff --git a/src/EFCore.SqlServer/Query/Internal/SqlServerQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.SqlServer/Query/Internal/SqlServerQueryableMethodTranslatingExpressionVisitor.cs index 64a14a34e26..90227493636 100644 --- a/src/EFCore.SqlServer/Query/Internal/SqlServerQueryableMethodTranslatingExpressionVisitor.cs +++ b/src/EFCore.SqlServer/Query/Internal/SqlServerQueryableMethodTranslatingExpressionVisitor.cs @@ -350,6 +350,7 @@ protected override ShapedQueryExpression TransformJsonQueryToTable(JsonQueryExpr } } + // Navigations represent nested JSON owned entities, which we also add to the OPENJSON WITH clause, but with AS JSON. foreach (var navigation in jsonQueryExpression.EntityType.GetNavigationsInHierarchy() .Where( n => n.ForeignKey.IsOwnership diff --git a/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.OriginalValues.cs b/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.OriginalValues.cs index 8aad9eb21dc..1e1b15bb8de 100644 --- a/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.OriginalValues.cs +++ b/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.OriginalValues.cs @@ -17,27 +17,20 @@ public OriginalValues(InternalEntityEntry entry) } public object? GetValue(InternalEntityEntry entry, IProperty property) - { - var index = property.GetOriginalValueIndex(); - if (index == -1) - { - throw new InvalidOperationException( - CoreStrings.OriginalValueNotTracked(property.Name, property.DeclaringType.DisplayName())); - } - - return IsEmpty ? entry[property] : _values[index]; - } + => property.GetOriginalValueIndex() is var index && index == -1 + ? throw new InvalidOperationException( + CoreStrings.OriginalValueNotTracked(property.Name, property.DeclaringType.DisplayName())) + : IsEmpty + ? entry[property] + : _values[index]; public T GetValue(InternalEntityEntry entry, IProperty property, int index) - { - if (index == -1) - { - throw new InvalidOperationException( - CoreStrings.OriginalValueNotTracked(property.Name, property.DeclaringType.DisplayName())); - } - - return IsEmpty ? entry.GetCurrentValue(property) : _values.GetValue(index); - } + => index == -1 + ? throw new InvalidOperationException( + CoreStrings.OriginalValueNotTracked(property.Name, property.DeclaringType.DisplayName())) + : IsEmpty + ? entry.GetCurrentValue(property) + : _values.GetValue(index); public void SetValue(IProperty property, object? value, int index) { diff --git a/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs b/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs index e142cc7f93d..2e634d75542 100644 --- a/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs +++ b/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs @@ -958,8 +958,7 @@ public T ReadTemporaryValue(int storeGeneratedIndex) => _temporaryValues.GetValue(storeGeneratedIndex); private static readonly MethodInfo GetCurrentValueMethod - = typeof(InternalEntityEntry).GetTypeInfo().GetDeclaredMethods(nameof(GetCurrentValue)).Single( - m => m.IsGenericMethod); + = typeof(InternalEntityEntry).GetTypeInfo().GetDeclaredMethods(nameof(GetCurrentValue)).Single(m => m.IsGenericMethod); [UnconditionalSuppressMessage( "ReflectionAnalysis", "IL2060", diff --git a/src/EFCore/ChangeTracking/Internal/SnapshotFactoryFactory.cs b/src/EFCore/ChangeTracking/Internal/SnapshotFactoryFactory.cs index d7cae14cec3..92fe7ef24bf 100644 --- a/src/EFCore/ChangeTracking/Internal/SnapshotFactoryFactory.cs +++ b/src/EFCore/ChangeTracking/Internal/SnapshotFactoryFactory.cs @@ -115,29 +115,25 @@ protected virtual Expression CreateSnapshotExpression( for (var i = 0; i < count; i++) { var propertyBase = propertyBases[i]; - if (propertyBase == null) - { - arguments[i] = Expression.Constant(null); - types[i] = typeof(object); - continue; - } - - if (propertyBase is IProperty property) - { - arguments[i] = CreateSnapshotValueExpression(CreateReadValueExpression(parameter, property), property); - continue; - } - - if (propertyBase is IComplexProperty complexProperty) - { - arguments[i] = CreateSnapshotValueExpression(CreateReadValueExpression(parameter, complexProperty), complexProperty); - continue; - } - if (propertyBase.IsShadowProperty()) + switch (propertyBase) { - arguments[i] = CreateSnapshotValueExpression(CreateReadShadowValueExpression(parameter, propertyBase), propertyBase); - continue; + case null: + arguments[i] = Expression.Constant(null); + types[i] = typeof(object); + continue; + + case IProperty property: + arguments[i] = CreateSnapshotValueExpression(CreateReadValueExpression(parameter, property), property); + continue; + + case IComplexProperty complexProperty: + arguments[i] = CreateSnapshotValueExpression(CreateReadValueExpression(parameter, complexProperty), complexProperty); + continue; + + case var _ when propertyBase.IsShadowProperty(): + arguments[i] = CreateSnapshotValueExpression(CreateReadShadowValueExpression(parameter, propertyBase), propertyBase); + continue; } var memberInfo = propertyBase.GetMemberInfo(forMaterialization: false, forSet: false); diff --git a/test/EFCore.Relational.Specification.Tests/JsonTypesRelationalTestBase.cs b/test/EFCore.Relational.Specification.Tests/JsonTypesRelationalTestBase.cs index d3b76f5c073..0e67e0c66fe 100644 --- a/test/EFCore.Relational.Specification.Tests/JsonTypesRelationalTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/JsonTypesRelationalTestBase.cs @@ -43,6 +43,53 @@ public virtual Task Can_read_write_collection_of_ASCII_string_JSON_values(object { RelationalAnnotationNames.StoreType, storeType }, { CoreAnnotationNames.Unicode, false } }); + public override Task Can_read_write_ulong_enum_JSON_values(EnumU64 value, string json) + { + if (value == EnumU64.Max) + { + json = """{"Prop":-1}"""; // Because ulong is converted to long on SQL Server + } + + return base.Can_read_write_ulong_enum_JSON_values(value, json); + } + + public override Task Can_read_write_nullable_ulong_enum_JSON_values(object? value, string json) + { + if (Equals(value, ulong.MaxValue)) + { + json = """{"Prop":-1}"""; // Because ulong is converted to long on SQL Server + } + + return base.Can_read_write_nullable_ulong_enum_JSON_values(value, json); + } + + public override Task Can_read_write_collection_of_ulong_enum_JSON_values() + => Can_read_and_write_JSON_value>( + nameof(EnumU64CollectionType.EnumU64), + [ + EnumU64.Min, + EnumU64.Max, + EnumU64.Default, + EnumU64.One, + (EnumU64)8 + ], + """{"Prop":[0,-1,0,1,8]}""", // Because ulong is converted to long on SQL Server + mappedCollection: true); + + public override Task Can_read_write_collection_of_nullable_ulong_enum_JSON_values() + => Can_read_and_write_JSON_value>( + nameof(NullableEnumU64CollectionType.EnumU64), + [ + EnumU64.Min, + null, + EnumU64.Max, + EnumU64.Default, + EnumU64.One, + (EnumU64?)8 + ], + """{"Prop":[0,null,-1,0,1,8]}""", // Because ulong is converted to long on SQL Server + mappedCollection: true); + protected override void AssertElementFacets(IElementType element, Dictionary? facets) { base.AssertElementFacets(element, facets); diff --git a/test/EFCore.Relational.Specification.Tests/Query/JsonQueryFixtureBase.cs b/test/EFCore.Relational.Specification.Tests/Query/JsonQueryFixtureBase.cs index a755d47755d..0b1f9f47519 100644 --- a/test/EFCore.Relational.Specification.Tests/Query/JsonQueryFixtureBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Query/JsonQueryFixtureBase.cs @@ -15,14 +15,7 @@ public Func GetContextCreator() => () => CreateContext(); public virtual ISetSource GetExpectedData() - { - if (_expectedData == null) - { - _expectedData = new JsonQueryData(); - } - - return _expectedData; - } + => _expectedData ??= new JsonQueryData(); public IReadOnlyDictionary EntitySorters { get; } = new Dictionary> { diff --git a/test/EFCore.Specification.Tests/Query/Ef6GroupByTestBase.cs b/test/EFCore.Specification.Tests/Query/Ef6GroupByTestBase.cs index 1b719b54393..99a1d53a1a6 100644 --- a/test/EFCore.Specification.Tests/Query/Ef6GroupByTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/Ef6GroupByTestBase.cs @@ -813,14 +813,7 @@ protected override Task SeedAsync(ArubaContext context) } public virtual ISetSource GetExpectedData() - { - if (_expectedData == null) - { - _expectedData = new ArubaData(); - } - - return _expectedData; - } + => _expectedData ??= new ArubaData(); public IReadOnlyDictionary EntitySorters { get; } = new Dictionary> { diff --git a/test/EFCore.SqlServer.FunctionalTests/JsonTypesSqlServerTestBase.cs b/test/EFCore.SqlServer.FunctionalTests/JsonTypesSqlServerTestBase.cs index ec53cb56dc0..554f444ef78 100644 --- a/test/EFCore.SqlServer.FunctionalTests/JsonTypesSqlServerTestBase.cs +++ b/test/EFCore.SqlServer.FunctionalTests/JsonTypesSqlServerTestBase.cs @@ -5,60 +5,12 @@ namespace Microsoft.EntityFrameworkCore; public abstract class JsonTypesSqlServerTestBase : JsonTypesRelationalTestBase { - public override Task Can_read_write_ulong_enum_JSON_values(EnumU64 value, string json) - { - if (value == EnumU64.Max) - { - json = """{"Prop":-1}"""; // Because ulong is converted to long on SQL Server - } - - return base.Can_read_write_ulong_enum_JSON_values(value, json); - } - - public override Task Can_read_write_nullable_ulong_enum_JSON_values(object? value, string json) - { - if (Equals(value, ulong.MaxValue)) - { - json = """{"Prop":-1}"""; // Because ulong is converted to long on SQL Server - } - - return base.Can_read_write_nullable_ulong_enum_JSON_values(value, json); - } - - public override Task Can_read_write_collection_of_ulong_enum_JSON_values() - => Can_read_and_write_JSON_value>( - nameof(EnumU64CollectionType.EnumU64), - [ - EnumU64.Min, - EnumU64.Max, - EnumU64.Default, - EnumU64.One, - (EnumU64)8 - ], - """{"Prop":[0,-1,0,1,8]}""", // Because ulong is converted to long on SQL Server - mappedCollection: true); - - public override Task Can_read_write_collection_of_nullable_ulong_enum_JSON_values() - => Can_read_and_write_JSON_value>( - nameof(NullableEnumU64CollectionType.EnumU64), - [ - EnumU64.Min, - null, - EnumU64.Max, - EnumU64.Default, - EnumU64.One, - (EnumU64?)8 - ], - """{"Prop":[0,null,-1,0,1,8]}""", // Because ulong is converted to long on SQL Server - mappedCollection: true); - public override Task Can_read_write_collection_of_fixed_length_string_JSON_values(object? storeType) => base.Can_read_write_collection_of_fixed_length_string_JSON_values("nchar(32)"); public override Task Can_read_write_collection_of_ASCII_string_JSON_values(object? storeType) => base.Can_read_write_collection_of_ASCII_string_JSON_values("varchar(max)"); - protected override ITestStoreFactory TestStoreFactory => SqlServerTestStoreFactory.Instance;