Merge pull request #235 from graphql-dotnet/fix-immutableconverter

Fix ImmutableConverter and ErrorPath

src/GraphQL.Client.Serializer.SystemTextJson/ConverterHelperExtensions.cs

@@ -0,0 +1,37 @@
+using System;
+using System.Buffers;
+using System.Numerics;
+using System.Text;
+using System.Text.Json;
+
+namespace GraphQL.Client.Serializer.SystemTextJson
+{
+    public static class ConverterHelperExtensions
+    {
+        public static object ReadNumber(this ref Utf8JsonReader reader)
+        {
+            if (reader.TryGetInt32(out int i))
+                return i;
+            else if (reader.TryGetInt64(out long l))
+                return l;
+            else if (reader.TryGetDouble(out double d))
+                return reader.TryGetBigInteger(out var bi) && bi != new BigInteger(d)
+                    ? bi
+                    : (object)d;
+            else if (reader.TryGetDecimal(out decimal dd))
+                return reader.TryGetBigInteger(out var bi) && bi != new BigInteger(dd)
+                    ? bi
+                    : (object)dd;
+
+            throw new NotImplementedException($"Unexpected Number value. Raw text was: {reader.GetRawString()}");
+        }
+
+        public static bool TryGetBigInteger(this ref Utf8JsonReader reader, out BigInteger bi) => BigInteger.TryParse(reader.GetRawString(), out bi);
+
+        public static string GetRawString(this ref Utf8JsonReader reader)
+        {
+            var byteArray = reader.HasValueSequence ? reader.ValueSequence.ToArray() : reader.ValueSpan.ToArray();
+            return Encoding.UTF8.GetString(byteArray);
+        }
+    }
+}

src/GraphQL.Client.Serializer.SystemTextJson/ErrorPathConverter.cs

@@ -0,0 +1,51 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text.Json;
+using System.Text.Json.Serialization;
+
+namespace GraphQL.Client.Serializer.SystemTextJson
+{
+    public class ErrorPathConverter : JsonConverter<ErrorPath>
+    {
+
+        public override ErrorPath Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) =>
+            new ErrorPath(ReadArray(ref reader));
+
+        public override void Write(Utf8JsonWriter writer, ErrorPath value, JsonSerializerOptions options)
+            => throw new NotImplementedException(
+                "This converter currently is only intended to be used to read a JSON object into a strongly-typed representation.");
+
+        private IEnumerable<object?> ReadArray(ref Utf8JsonReader reader)
+        {
+            if (reader.TokenType != JsonTokenType.StartArray)
+            {
+                throw new JsonException("This converter can only parse when the root element is a JSON Array.");
+            }
+
+            var array = new List<object?>();
+
+            while (reader.Read())
+            {
+                if (reader.TokenType == JsonTokenType.EndArray)
+                    break;
+
+                array.Add(ReadValue(ref reader));
+            }
+
+            return array;
+        }
+
+        private object? ReadValue(ref Utf8JsonReader reader)
+            => reader.TokenType switch
+            {
+                JsonTokenType.None => null,
+                JsonTokenType.String => reader.GetString(),
+                JsonTokenType.Number => reader.ReadNumber(),
+                JsonTokenType.True => true,
+                JsonTokenType.False => false,
+                JsonTokenType.Null => null,
+                _ => throw new InvalidOperationException($"Unexpected token type: {reader.TokenType}")
+            };
+    }
+}

src/GraphQL.Client.Serializer.SystemTextJson/ImmutableConverter.cs

@@ -15,6 +15,13 @@ public class ImmutableConverter : JsonConverter<object>
     {
         public override bool CanConvert(Type typeToConvert)
         {
+            if (typeToConvert.IsPrimitive)
+                return false;
+
+            var nullableUnderlyingType = Nullable.GetUnderlyingType(typeToConvert);
+            if (nullableUnderlyingType != null && nullableUnderlyingType.IsPrimitive)
+                return false;
+
             bool result;
             var constructors = typeToConvert.GetConstructors(BindingFlags.Public | BindingFlags.Instance);
             if (constructors.Length != 1)

src/GraphQL.Client.Serializer.SystemTextJson/MapConverter.cs

@@ -15,68 +15,70 @@ namespace GraphQL.Client.Serializer.SystemTextJson
     /// </remarks>
     public class MapConverter : JsonConverter<Map>
     {
-        public override Map Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
-        {
-            using var doc = JsonDocument.ParseValue(ref reader);
-
-            if (doc?.RootElement == null || doc?.RootElement.ValueKind != JsonValueKind.Object)
-            {
-                throw new ArgumentException("This converter can only parse when the root element is a JSON Object.");
-            }
-
-            return ReadDictionary<Map>(doc.RootElement);
-        }
+        public override Map Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) => ReadDictionary(ref reader, new Map());
 
         public override void Write(Utf8JsonWriter writer, Map value, JsonSerializerOptions options)
             => throw new NotImplementedException(
                 "This converter currently is only intended to be used to read a JSON object into a strongly-typed representation.");
 
-        private TDictionary ReadDictionary<TDictionary>(JsonElement element) where TDictionary : Dictionary<string, object>
+        private static TDictionary ReadDictionary<TDictionary>(ref Utf8JsonReader reader, TDictionary result)
+            where TDictionary : Dictionary<string, object>
         {
-            var result = Activator.CreateInstance<TDictionary>();
-            foreach (var property in element.EnumerateObject())
+            if (reader.TokenType != JsonTokenType.StartObject)
+                throw new JsonException();
+
+            while (reader.Read())
             {
-                result[property.Name] = ReadValue(property.Value);
+                if (reader.TokenType == JsonTokenType.EndObject)
+                    break;
+
+                if (reader.TokenType != JsonTokenType.PropertyName)
+                    throw new JsonException();
+
+                string key = reader.GetString();
+
+                // move to property value
+                if (!reader.Read())
+                    throw new JsonException();
+
+                result.Add(key, ReadValue(ref reader));
             }
+
             return result;
         }
 
-        private IEnumerable<object?> ReadArray(JsonElement value)
+        private static List<object> ReadArray(ref Utf8JsonReader reader)
         {
-            foreach (var item in value.EnumerateArray())
+            if (reader.TokenType != JsonTokenType.StartArray)
+                throw new JsonException();
+
+            var result = new List<object>();
+
+            while (reader.Read())
             {
-                yield return ReadValue(item);
+                if (reader.TokenType == JsonTokenType.EndArray)
+                    break;
+
+                result.Add(ReadValue(ref reader));
             }
-        }
 
-        private object? ReadValue(JsonElement value)
-            => value.ValueKind switch
+            return result;
+        }
+
+        private static object? ReadValue(ref Utf8JsonReader reader)
+            => reader.TokenType switch
             {
-                JsonValueKind.Array => ReadArray(value).ToList(),
-                JsonValueKind.Object => ReadDictionary<Dictionary<string, object>>(value),
-                JsonValueKind.Number => ReadNumber(value),
-                JsonValueKind.True => true,
-                JsonValueKind.False => false,
-                JsonValueKind.String => value.GetString(),
-                JsonValueKind.Null => null,
-                JsonValueKind.Undefined => null,
-                _ => throw new InvalidOperationException($"Unexpected value kind: {value.ValueKind}")
+                JsonTokenType.StartArray => ReadArray(ref reader).ToList(),
+                JsonTokenType.StartObject => ReadDictionary(ref reader, new Dictionary<string, object>()),
+                JsonTokenType.Number => reader.ReadNumber(),
+                JsonTokenType.True => true,
+                JsonTokenType.False => false,
+                JsonTokenType.String => reader.GetString(),
+                JsonTokenType.Null => null,
+                JsonTokenType.None => null,
+                _ => throw new InvalidOperationException($"Unexpected value kind: {reader.TokenType}")
             };
 
-        private object ReadNumber(JsonElement value)
-        {
-            if (value.TryGetInt32(out int i))
-                return i;
-            else if (value.TryGetInt64(out long l))
-                return l;
-            else if (BigInteger.TryParse(value.GetRawText(), out var bi))
-                return bi;
-            else if (value.TryGetDouble(out double d))
-                return d;
-            else if (value.TryGetDecimal(out decimal dd))
-                return dd;
-
-            throw new NotImplementedException($"Unexpected Number value. Raw text was: {value.GetRawText()}");
-        }
+
     }
 }

src/GraphQL.Client.Serializer.SystemTextJson/SystemTextJsonSerializer.cs

@@ -30,6 +30,7 @@ public SystemTextJsonSerializer(JsonSerializerOptions options)
         private void ConfigureMandatorySerializerOptions()
         {
             // deserialize extensions to Dictionary<string, object>
+            Options.Converters.Insert(0, new ErrorPathConverter());
             Options.Converters.Insert(0, new MapConverter());
             // allow the JSON field "data" to match the property "Data" even without JsonNamingPolicy.CamelCase
             Options.PropertyNameCaseInsensitive = true;

src/GraphQL.Primitives/ErrorPath.cs

@@ -0,0 +1,15 @@
+using System.Collections.Generic;
+
+namespace GraphQL
+{
+    public class ErrorPath : List<object>
+    {
+        public ErrorPath()
+        {
+        }
+
+        public ErrorPath(IEnumerable<object> collection) : base(collection)
+        {
+        }
+    }
+}

src/GraphQL.Primitives/GraphQLError.cs

@@ -26,7 +26,7 @@ public class GraphQLError : IEquatable<GraphQLError?>
         /// The Path of the error
         /// </summary>
         [DataMember(Name = "path")]
-        public object[]? Path { get; set; }
+        public ErrorPath? Path { get; set; }
 
         /// <summary>
         /// The extensions of the error

tests/GraphQL.Client.Serializer.Tests/BaseSerializerTest.cs

@@ -1,7 +1,12 @@
+using System;
 using System.IO;
+using System.Linq;
+using System.Reflection;
 using System.Text;
 using System.Threading;
+using System.Threading.Tasks;
 using FluentAssertions;
+using FluentAssertions.Execution;
 using GraphQL.Client.Abstractions;
 using GraphQL.Client.Abstractions.Websocket;
 using GraphQL.Client.LocalExecution;
@@ -48,14 +53,31 @@ public void SerializeToBytesTest(string expectedJson, GraphQLWebSocketRequest re
 
         [Theory]
         [ClassData(typeof(DeserializeResponseTestData))]
-        public async void DeserializeFromUtf8StreamTest(string json, GraphQLResponse<object> expectedResponse)
+        public async void DeserializeFromUtf8StreamTest(string json, IGraphQLResponse expectedResponse)
         {
             var jsonBytes = Encoding.UTF8.GetBytes(json);
             await using var ms = new MemoryStream(jsonBytes);
-            var response = await Serializer.DeserializeFromUtf8StreamAsync<GraphQLResponse<object>>(ms, CancellationToken.None);
+            var response = await DeserializeToUnknownType(expectedResponse.Data?.GetType() ?? typeof(object), ms);
+            //var response = await Serializer.DeserializeFromUtf8StreamAsync<object>(ms, CancellationToken.None);
 
-            response.Data.Should().BeEquivalentTo(expectedResponse.Data);
-            response.Errors.Should().Equal(expectedResponse.Errors);
+            response.Data.Should().BeEquivalentTo(expectedResponse.Data, options => options.WithAutoConversion());
+
+            if (expectedResponse.Errors is null)
+                response.Errors.Should().BeNull();
+            else {
+                using (new AssertionScope())
+                {
+                    response.Errors.Should().NotBeNull();
+                    response.Errors.Should().HaveSameCount(expectedResponse.Errors);
+                    for (int i = 0; i < expectedResponse.Errors.Length; i++)
+                    {
+                        response.Errors[i].Message.Should().BeEquivalentTo(expectedResponse.Errors[i].Message);
+                        response.Errors[i].Locations.Should().BeEquivalentTo(expectedResponse.Errors[i].Locations?.ToList());
+                        response.Errors[i].Path.Should().BeEquivalentTo(expectedResponse.Errors[i].Path);
+                        response.Errors[i].Extensions.Should().BeEquivalentTo(expectedResponse.Errors[i].Extensions);
+                    }
+                }
+            }
 
             if (expectedResponse.Extensions == null)
                 response.Extensions.Should().BeNull();
@@ -69,6 +91,17 @@ public async void DeserializeFromUtf8StreamTest(string json, GraphQLResponse<obj
             }
         }
 
+        public async Task<IGraphQLResponse> DeserializeToUnknownType(Type dataType, Stream stream)
+        {
+            MethodInfo mi = Serializer.GetType().GetMethod("DeserializeFromUtf8StreamAsync", BindingFlags.Instance | BindingFlags.Public);
+            MethodInfo mi2 = mi.MakeGenericMethod(dataType);
+            var task = (Task) mi2.Invoke(Serializer, new object[] { stream, CancellationToken.None });
+            await task;
+            var resultProperty = task.GetType().GetProperty("Result", BindingFlags.Public | BindingFlags.Instance);
+            var result = resultProperty.GetValue(task);
+            return (IGraphQLResponse)result;
+        }
+
         [Fact]
         public async void CanDeserializeExtensions()
         {
@@ -118,5 +151,26 @@ public async void CanDoSerializationWithPredefinedTypes()
 
             Assert.Equal(message, response.Data.AddMessage.Content);
         }
+
+
+        public class WithNullable
+        {
+            public int? NullableInt { get; set; }
+        }
+
+        [Fact]
+        public void CanSerializeNullableInt()
+        {
+            Action action = () => Serializer.SerializeToString(new GraphQLRequest
+            {
+                Query = "{}",
+                Variables = new WithNullable
+                {
+                    NullableInt = 2
+                }
+            });
+
+            action.Should().NotThrow();
+        }
     }
 }