Various stuff from EFCore.PG JSON work

dotnet · Nov 8, 2023 · d01866a · d01866a
1 parent 20c1795
commit d01866a
Show file tree

Hide file tree

Showing 14 changed files with 158 additions and 137 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
@@ -565,6 +565,9 @@
   <data name="JsonReaderInvalidTokenType" xml:space="preserve">
     <value>Invalid token type: '{tokenType}'.</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="JsonRequiredEntityWithNullJson" xml:space="preserve">
     <value>Entity {entity} is required but the JSON element containing it is null.</value>
   </data>

diff --git a/...uery/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs b/...uery/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs
@@ -2528,8 +2528,15 @@ private Expression CreateReadJsonPropertyValueExpression(
             var nullable = property.IsNullable;
             var typeMapping = property.GetTypeMapping();
 
-            var jsonReaderWriterExpression = Constant(
-                property.GetJsonValueReaderWriter() ?? property.GetTypeMapping().JsonValueReaderWriter!);
+            var jsonValueReaderWriter = property.GetJsonValueReaderWriter() ?? property.GetTypeMapping().JsonValueReaderWriter;
+            if (jsonValueReaderWriter is null)
+            {
+                throw new InvalidOperationException(
+                    RelationalStrings.JsonValueReadWriterMissingOnTypeMapping(
+                        property.GetTypeMapping().GetType().Name, property.Name, property.DeclaringType.DisplayName()));
+            }
+
+            var jsonReaderWriterExpression = Constant(jsonValueReaderWriter);
 
             var fromJsonMethod = jsonReaderWriterExpression.Type.GetMethod(
                 nameof(JsonValueReaderWriter<object>.FromJsonTyped),

diff --git a/src/EFCore.Relational/Update/ModificationCommand.cs b/src/EFCore.Relational/Update/ModificationCommand.cs
@@ -873,7 +873,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
@@ -250,6 +250,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 GetAllNavigationsInHierarchy(jsonQueryExpression.EntityType)
                      .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
@@ -922,8 +922,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
@@ -106,29 +106,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
@@ -47,6 +47,59 @@ public virtual void Can_read_write_collection_of_ASCII_string_JSON_values(object
                 { RelationalAnnotationNames.StoreType, storeType }, { CoreAnnotationNames.Unicode, false }
             });
 
+    public override void Can_read_write_ulong_enum_JSON_values(EnumU64 value, string json)
+    {
+        // Relational databases don't support unsigned numeric types, so ulong is value-converted to long
+        if (value == EnumU64.Max)
+        {
+            json = """{"Prop":-1}""";
+        }
+
+        base.Can_read_write_ulong_enum_JSON_values(value, json);
+    }
+
+    public override void Can_read_write_nullable_ulong_enum_JSON_values(object? value, string json)
+    {
+        // Relational databases don't support unsigned numeric types, so ulong is value-converted to long
+        if (Equals(value, ulong.MaxValue))
+        {
+            json = """{"Prop":-1}""";
+        }
+
+        base.Can_read_write_nullable_ulong_enum_JSON_values(value, json);
+    }
+
+    public override void Can_read_write_collection_of_ulong_enum_JSON_values()
+        => Can_read_and_write_JSON_value<EnumU64CollectionType, List<EnumU64>>(
+            nameof(EnumU64CollectionType.EnumU64),
+            new List<EnumU64>
+            {
+                EnumU64.Min,
+                EnumU64.Max,
+                EnumU64.Default,
+                EnumU64.One,
+                (EnumU64)8
+            },
+            // Relational databases don't support unsigned numeric types, so ulong is value-converted to long
+            """{"Prop":[0,-1,0,1,8]}""",
+            mappedCollection: true);
+
+    public override void Can_read_write_collection_of_nullable_ulong_enum_JSON_values()
+        => Can_read_and_write_JSON_value<NullableEnumU64CollectionType, List<EnumU64?>>(
+            nameof(NullableEnumU64CollectionType.EnumU64),
+            new List<EnumU64?>
+            {
+                EnumU64.Min,
+                null,
+                EnumU64.Max,
+                EnumU64.Default,
+                EnumU64.One,
+                (EnumU64?)8
+            },
+            // Relational databases don't support unsigned numeric types, so ulong is value-converted to long
+            """{"Prop":[0,null,-1,0,1,8]}""",
+            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
@@ -13,14 +13,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.Relational.Specification.Tests/TestUtilities/TestSqlLoggerFactory.cs b/test/EFCore.Relational.Specification.Tests/TestUtilities/TestSqlLoggerFactory.cs
@@ -254,7 +254,7 @@ void RewriteSourceWithNewBaseline(string fileName, int lineNumber)
                         indentBuilder.Append("    ");
                         var indent = indentBuilder.ToString();
                         var newBaseLine = $@"Assert{(forUpdate ? "ExecuteUpdate" : "")}Sql(
-{string.Join("," + Environment.NewLine + indent + "//" + Environment.NewLine, SqlStatements.Skip(offset).Take(count).Select(sql => "\"\"\"" + Environment.NewLine + sql + Environment.NewLine + "\"\"\""))})";
+{string.Join("," + Environment.NewLine + indent + "//" + Environment.NewLine, SqlStatements.Skip(offset).Take(count).Select(sql => indent + "\"\"\"" + Environment.NewLine + sql + Environment.NewLine + "\"\"\""))})";
                         var numNewlinesInRewritten = newBaseLine.Count(c => c is '\n' or '\r');
 
                         writer.Write(newBaseLine);

diff --git a/test/EFCore.Specification.Tests/Query/Ef6GroupByTestBase.cs b/test/EFCore.Specification.Tests/Query/Ef6GroupByTestBase.cs
@@ -808,14 +808,7 @@ protected override void Seed(ArubaContext context)
             => new ArubaData(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
@@ -7,55 +7,6 @@ namespace Microsoft.EntityFrameworkCore;
 
 public abstract class JsonTypesSqlServerTestBase : JsonTypesRelationalTestBase
 {
-    public override void 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
-        }
-
-        base.Can_read_write_ulong_enum_JSON_values(value, json);
-    }
-
-    public override void 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
-        }
-
-        base.Can_read_write_nullable_ulong_enum_JSON_values(value, json);
-    }
-
-    public override void Can_read_write_collection_of_ulong_enum_JSON_values()
-        => Can_read_and_write_JSON_value<EnumU64CollectionType, List<EnumU64>>(
-            nameof(EnumU64CollectionType.EnumU64),
-            new List<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 void Can_read_write_collection_of_nullable_ulong_enum_JSON_values()
-        => Can_read_and_write_JSON_value<NullableEnumU64CollectionType, List<EnumU64?>>(
-            nameof(NullableEnumU64CollectionType.EnumU64),
-            new List<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 void 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)");
-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 GetAllNavigationsInHierarchy(jsonQueryExpression.EntityType)
                          .Where(
                              n => n.ForeignKey.IsOwnership
@@ Expand Down @@