Address more #51528 feedback & do some clean up (#51767)

dotnet · Apr 26, 2021 · c9a15a5 · c9a15a5
1 parent c0d5122
commit c9a15a5
Show file tree

Hide file tree

Showing 15 changed files with 170 additions and 238 deletions.
diff --git a/src/libraries/System.Text.Json/src/System.Text.Json.csproj b/src/libraries/System.Text.Json/src/System.Text.Json.csproj
@@ -173,6 +173,7 @@
     <Compile Include="System\Text\Json\Serialization\JsonDefaultNamingPolicy.cs" />
     <Compile Include="System\Text\Json\Serialization\JsonNamingPolicy.cs" />
     <Compile Include="System\Text\Json\Serialization\JsonResumableConverterOfT.cs" />
+    <Compile Include="System\Text\Json\Serialization\JsonSerializer.Helpers.cs" />
     <Compile Include="System\Text\Json\Serialization\JsonSerializer.Read.HandleMetadata.cs" />
     <Compile Include="System\Text\Json\Serialization\JsonSerializer.Read.HandlePropertyName.cs" />
     <Compile Include="System\Text\Json\Serialization\JsonSerializer.Read.Helpers.cs" />

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/JsonHelpers.cs b/src/libraries/System.Text.Json/src/System/Text/Json/JsonHelpers.cs
@@ -6,8 +6,6 @@
 using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.Runtime.CompilerServices;
-using System.Text.Json.Serialization;
-using System.Text.Json.Serialization.Metadata;
 
 namespace System.Text.Json
 {
@@ -144,14 +142,6 @@ public static bool IsFinite(float value)
 #endif
         }
 
-        public static bool IsValidNumberHandlingValue(JsonNumberHandling handling) =>
-            IsInRangeInclusive((int)handling, 0,
-                (int)(
-                JsonNumberHandling.Strict |
-                JsonNumberHandling.AllowReadingFromString |
-                JsonNumberHandling.WriteAsString |
-                JsonNumberHandling.AllowNamedFloatingPointLiterals));
-
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static void ValidateInt32MaxArrayLength(uint length)
         {
@@ -160,19 +150,5 @@ public static void ValidateInt32MaxArrayLength(uint length)
                 ThrowHelper.ThrowOutOfMemoryException(length);
             }
         }
-
-        public static JsonTypeInfo GetTypeInfo(JsonSerializerContext context, Type type)
-        {
-            Debug.Assert(context != null);
-            Debug.Assert(type != null);
-
-            JsonTypeInfo? info = context.GetTypeInfo(type);
-            if (info == null)
-            {
-                ThrowHelper.ThrowInvalidOperationException_NoMetadataForType(type);
-            }
-
-            return info;
-        }
     }
 }
diff --git a/...em.Text.Json/src/System/Text/Json/Serialization/Attributes/JsonNumberHandlingAttribute.cs b/...em.Text.Json/src/System/Text/Json/Serialization/Attributes/JsonNumberHandlingAttribute.cs
@@ -20,7 +20,7 @@ public sealed class JsonNumberHandlingAttribute : JsonAttribute
         /// </summary>
         public JsonNumberHandlingAttribute(JsonNumberHandling handling)
         {
-            if (!JsonHelpers.IsValidNumberHandlingValue(handling))
+            if (!JsonSerializer.IsValidNumberHandlingValue(handling))
             {
                 throw new ArgumentOutOfRangeException(nameof(handling));
             }

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs
@@ -0,0 +1,41 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Diagnostics;
+using System.Text.Json.Serialization;
+using System.Text.Json.Serialization.Metadata;
+
+namespace System.Text.Json
+{
+    public static partial class JsonSerializer
+    {
+        private static JsonTypeInfo GetTypeInfo(Type runtimeType, JsonSerializerOptions? options)
+        {
+            options ??= JsonSerializerOptions.s_defaultOptions;
+            options.RootBuiltInConvertersAndTypeInfoCreator();
+            return options.GetOrAddClassForRootType(runtimeType);
+        }
+
+        private static JsonTypeInfo GetTypeInfo(JsonSerializerContext context, Type type)
+        {
+            Debug.Assert(context != null);
+            Debug.Assert(type != null);
+
+            JsonTypeInfo? info = context.GetTypeInfo(type);
+            if (info == null)
+            {
+                ThrowHelper.ThrowInvalidOperationException_NoMetadataForType(type);
+            }
+
+            return info;
+        }
+
+        internal static bool IsValidNumberHandlingValue(JsonNumberHandling handling) =>
+            JsonHelpers.IsInRangeInclusive((int)handling, 0,
+                (int)(
+                JsonNumberHandling.Strict |
+                JsonNumberHandling.AllowReadingFromString |
+                JsonNumberHandling.WriteAsString |
+                JsonNumberHandling.AllowNamedFloatingPointLiterals));
+    }
+}
diff --git a/...raries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Helpers.cs b/...raries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Helpers.cs
@@ -2,21 +2,13 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Diagnostics;
-using System.Diagnostics.CodeAnalysis;
 using System.Text.Json.Serialization;
+using System.Text.Json.Serialization.Metadata;
 
 namespace System.Text.Json
 {
     public static partial class JsonSerializer
     {
-        private static TValue? ReadCore<TValue>(ref Utf8JsonReader reader, Type returnType, JsonSerializerOptions options)
-        {
-            ReadStack state = default;
-            state.Initialize(returnType, options, supportContinuation: false);
-            JsonConverter jsonConverter = state.Current.JsonPropertyInfo!.ConverterBase;
-            return ReadCore<TValue>(jsonConverter, ref reader, options, ref state);
-        }
-
         private static TValue? ReadCore<TValue>(JsonConverter jsonConverter, ref Utf8JsonReader reader, JsonSerializerOptions options, ref ReadStack state)
         {
             if (jsonConverter is JsonConverter<TValue> converter)
@@ -30,5 +22,22 @@ public static partial class JsonSerializer
             Debug.Assert(value == null || value is TValue);
             return (TValue)value!;
         }
+
+        private static TValue? ReadUsingMetadata<TValue>(ReadOnlySpan<byte> utf8Json, JsonTypeInfo jsonTypeInfo, int? actualByteCount = null)
+        {
+            JsonSerializerOptions options = jsonTypeInfo.Options;
+
+            var readerState = new JsonReaderState(options.GetReaderOptions());
+            var reader = new Utf8JsonReader(utf8Json, isFinalBlock: true, readerState);
+
+            ReadStack state = default;
+            state.Initialize(jsonTypeInfo);
+
+            TValue? value = ReadCore<TValue>(jsonTypeInfo.PropertyInfoForTypeInfo.ConverterBase, ref reader, options, ref state);
+
+            // The reader should have thrown if we have remaining bytes.
+            Debug.Assert(reader.BytesConsumed == (actualByteCount ?? utf8Json.Length));
+            return value;
+        }
     }
 }
diff --git a/...libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Span.cs b/...libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Span.cs
@@ -25,19 +25,7 @@ public static partial class JsonSerializer
         /// for <typeparamref name="TValue"/> or its serializable members.
         /// </exception>
         public static TValue? Deserialize<[DynamicallyAccessedMembers(JsonHelpers.MembersAccessedOnRead)] TValue>(ReadOnlySpan<byte> utf8Json, JsonSerializerOptions? options = null)
-        {
-            if (options == null)
-            {
-                options = JsonSerializerOptions.s_defaultOptions;
-            }
-
-            options.RootBuiltInConvertersAndTypeInfoCreator();
-
-            var readerState = new JsonReaderState(options.GetReaderOptions());
-            var reader = new Utf8JsonReader(utf8Json, isFinalBlock: true, readerState);
-
-            return ReadCore<TValue>(ref reader, typeof(TValue), options);
-        }
+            => ReadUsingOptions<TValue>(utf8Json, typeof(TValue), options);
 
         /// <summary>
         /// Parse the UTF-8 encoded text representing a single JSON value into a <paramref name="returnType"/>.
@@ -65,17 +53,7 @@ public static partial class JsonSerializer
                 throw new ArgumentNullException(nameof(returnType));
             }
 
-            if (options == null)
-            {
-                options = JsonSerializerOptions.s_defaultOptions;
-            }
-
-            options.RootBuiltInConvertersAndTypeInfoCreator();
-
-            var readerState = new JsonReaderState(options.GetReaderOptions());
-            var reader = new Utf8JsonReader(utf8Json, isFinalBlock: true, readerState);
-
-            return ReadCore<object>(ref reader, returnType, options);
+            return ReadUsingOptions<object>(utf8Json, returnType, options);
         }
 
         /// <summary>
@@ -100,15 +78,7 @@ public static partial class JsonSerializer
                 throw new ArgumentNullException(nameof(jsonTypeInfo));
             }
 
-            JsonSerializerOptions options = jsonTypeInfo.Options;
-
-            var readerState = new JsonReaderState(options.GetReaderOptions());
-            var reader = new Utf8JsonReader(utf8Json, isFinalBlock: true, readerState);
-
-            ReadStack state = default;
-            state.Initialize(jsonTypeInfo);
-
-            return ReadCore<TValue>(jsonTypeInfo.PropertyInfoForTypeInfo.ConverterBase, ref reader, options, ref state);
+            return ReadUsingMetadata<TValue>(utf8Json, jsonTypeInfo);
         }
 
         /// <summary>
@@ -146,16 +116,14 @@ public static partial class JsonSerializer
                 throw new ArgumentNullException(nameof(context));
             }
 
-            JsonTypeInfo jsonTypeInfo = JsonHelpers.GetTypeInfo(context, returnType);
-            JsonSerializerOptions options = jsonTypeInfo.Options;
-
-            var readerState = new JsonReaderState(options.GetReaderOptions());
-            var reader = new Utf8JsonReader(utf8Json, isFinalBlock: true, readerState);
-
-            ReadStack state = default;
-            state.Initialize(jsonTypeInfo);
+            return ReadUsingMetadata<object?>(utf8Json, GetTypeInfo(context, returnType));
+        }
 
-            return ReadCore<object?>(jsonTypeInfo.PropertyInfoForTypeInfo.ConverterBase, ref reader, options, ref state);
+        private static TValue? ReadUsingOptions<TValue>(ReadOnlySpan<byte> utf8Json, Type returnType, JsonSerializerOptions? options)
+        {
+            options ??= JsonSerializerOptions.s_defaultOptions;
+            options.RootBuiltInConvertersAndTypeInfoCreator();
+            return ReadUsingMetadata<TValue>(utf8Json, GetTypeInfo(returnType, options));
         }
     }
 }
diff --git a/...braries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Stream.cs b/...braries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Stream.cs
@@ -82,26 +82,18 @@ public static partial class JsonSerializer
             CancellationToken cancellationToken = default)
         {
             if (utf8Json == null)
+            {
                 throw new ArgumentNullException(nameof(utf8Json));
+            }
 
             if (returnType == null)
+            {
                 throw new ArgumentNullException(nameof(returnType));
+            }
 
             return ReadAllUsingOptionsAsync<object>(utf8Json, returnType, options, cancellationToken);
         }
 
-        private static ValueTask<TValue?> ReadAllUsingOptionsAsync<TValue>(
-            Stream utf8Json,
-            Type returnType,
-            JsonSerializerOptions? options,
-            CancellationToken cancellationToken)
-        {
-            options ??= JsonSerializerOptions.s_defaultOptions;
-            options.RootBuiltInConvertersAndTypeInfoCreator();
-            JsonTypeInfo jsonTypeInfo = options.GetOrAddClassForRootType(returnType);
-            return ReadAllAsync<TValue>(utf8Json, returnType, jsonTypeInfo, cancellationToken);
-        }
-
         /// <summary>
         /// Read the UTF-8 encoded text representing a single JSON value into a <typeparamref name="TValue"/>.
         /// The Stream will be read to completion.
@@ -176,15 +168,21 @@ public static partial class JsonSerializer
             CancellationToken cancellationToken = default)
         {
             if (utf8Json == null)
+            {
                 throw new ArgumentNullException(nameof(utf8Json));
+            }
 
             if (returnType == null)
+            {
                 throw new ArgumentNullException(nameof(returnType));
+            }
 
             if (context == null)
+            {
                 throw new ArgumentNullException(nameof(context));
+            }
 
-            return ReadAllAsync<object>(utf8Json, returnType, JsonHelpers.GetTypeInfo(context, returnType), cancellationToken);
+            return ReadAllAsync<object>(utf8Json, returnType, GetTypeInfo(context, returnType), cancellationToken);
         }
 
         /// <summary>
@@ -391,6 +389,16 @@ internal static TValue ContinueDeserialize<TValue>(
             return value;
         }
 
+        private static ValueTask<TValue?> ReadAllUsingOptionsAsync<TValue>(
+            Stream utf8Json,
+            Type returnType,
+            JsonSerializerOptions? options,
+            CancellationToken cancellationToken)
+        {
+            JsonTypeInfo jsonTypeInfo = GetTypeInfo(returnType, options);
+            return ReadAllAsync<TValue>(utf8Json, returnType, jsonTypeInfo, cancellationToken);
+        }
+
         private static TValue ReadCore<TValue>(
             ref JsonReaderState readerState,
             bool isFinalBlock,