Various stuff from EFCore.PG JSON work

dotnet · Jun 3, 2024 · 114ef46 · 114ef46
1 parent 16acc46
commit 114ef46
Show file tree

Hide file tree

Showing 11 changed files with 100 additions and 107 deletions.
diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs
diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx
@@ -571,6 +571,9 @@
   <data name="JsonRequiredEntityWithNullJson" xml:space="preserve">
     <value>Entity {entity} is required but the JSON element containing it is null.</value>
   </data>
+  <data name="JsonValueReadWriterMissingOnTypeMapping" xml:space="preserve">
+    <value>Type mapping type '{typeMapping}', which is being used on property '{property}' on entity type '{entityType}' in a JSON document, has not defined a JsonValueReaderWriter.</value>
+  </data>
   <data name="KeylessMappingStrategy" xml:space="preserve">
     <value>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.</value>
   </data>

diff --git 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
@@ -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
@@ -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<T>(InternalEntityEntry entry, IProperty property, int index)
-        {
-            if (index == -1)
-            {
-                throw new InvalidOperationException(
-                    CoreStrings.OriginalValueNotTracked(property.Name, property.DeclaringType.DisplayName()));
-            }
-
-            return IsEmpty ? entry.GetCurrentValue<T>(property) : _values.GetValue<T>(index);
-        }
+            => index == -1
+                ? throw new InvalidOperationException(
+                    CoreStrings.OriginalValueNotTracked(property.Name, property.DeclaringType.DisplayName()))
+                : IsEmpty
+                    ? entry.GetCurrentValue<T>(property)
+                    : _values.GetValue<T>(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
@@ -958,8 +958,7 @@ public T ReadTemporaryValue<T>(int storeGeneratedIndex)
         => _temporaryValues.GetValue<T>(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
@@ -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
@@ -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<EnumU64CollectionType, List<EnumU64>>(
+            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<NullableEnumU64CollectionType, List<EnumU64?>>(
+            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<string, object?>? 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
@@ -15,14 +15,7 @@ public Func<DbContext> GetContextCreator()
         => () => CreateContext();
 
     public virtual ISetSource GetExpectedData()
-    {
-        if (_expectedData == null)
-        {
-            _expectedData = new JsonQueryData();
-        }
-
-        return _expectedData;
-    }
+        => _expectedData ??= new JsonQueryData();
 
     public IReadOnlyDictionary<Type, object> EntitySorters { get; } = new Dictionary<Type, Func<object, object>>
     {

diff --git 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<Type, object> EntitySorters { get; } = new Dictionary<Type, Func<object, object>>
         {

diff --git 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<EnumU64CollectionType, List<EnumU64>>(
-            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<NullableEnumU64CollectionType, List<EnumU64?>>(
-            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;
-Original file line number
+Diff line change
@@ Expand Up @@
                 }
             }
+            // 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
@@ Expand Down @@