Merge pull request #225 from graphql-dotnet/fix-inconsistency-in-mapconverters

Fix inconsistency in mapconverters

Author: rose-a

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

@@ -1,6 +1,6 @@
 using System;
 using System.Collections.Generic;
-using System.Runtime.InteropServices;
+using System.Linq;
 using Newtonsoft.Json;
 using Newtonsoft.Json.Linq;
 
@@ -24,11 +24,11 @@ public override Map ReadJson(JsonReader reader, Type objectType, Map existingVal
                 throw new ArgumentException("This converter can only parse when the root element is a JSON Object.");
         }
 
-        private object ReadToken(JToken? token) =>
+        private object? ReadToken(JToken? token) =>
             token switch
             {
                 JObject jObject => ReadDictionary<Dictionary<string, object>>(jObject),
-                JArray jArray => ReadArray(jArray),
+                JArray jArray => ReadArray(jArray).ToList(),
                 JValue jValue => jValue.Value,
                 JConstructor _ => throw new ArgumentOutOfRangeException(nameof(token.Type),
                     "cannot deserialize a JSON constructor"),
@@ -51,7 +51,7 @@ private TDictionary ReadDictionary<TDictionary>(JToken element) where TDictionar
             return result;
         }
 
-        private IEnumerable<object> ReadArray(JArray element)
+        private IEnumerable<object?> ReadArray(JArray element)
         {
             foreach (var item in element)
             {

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

@@ -41,15 +41,15 @@ private TDictionary ReadDictionary<TDictionary>(JsonElement element) where TDict
             return result;
         }
 
-        private IEnumerable<object> ReadArray(JsonElement value)
+        private IEnumerable<object?> ReadArray(JsonElement value)
         {
             foreach (var item in value.EnumerateArray())
             {
                 yield return ReadValue(item);
             }
         }
 
-        private object ReadValue(JsonElement value)
+        private object? ReadValue(JsonElement value)
             => value.ValueKind switch
             {
                 JsonValueKind.Array => ReadArray(value).ToList(),
@@ -65,15 +65,15 @@ private object ReadValue(JsonElement value)
 
         private object ReadNumber(JsonElement value)
         {
-            if (value.TryGetInt32(out var i))
+            if (value.TryGetInt32(out int i))
                 return i;
-            else if (value.TryGetInt64(out var l))
+            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 var d))
+            else if (value.TryGetDouble(out double d))
                 return d;
-            else if (value.TryGetDecimal(out var dd))
+            else if (value.TryGetDecimal(out decimal dd))
                 return dd;
 
             throw new NotImplementedException($"Unexpected Number value. Raw text was: {value.GetRawText()}");

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

@@ -0,0 +1,62 @@
+using System.Collections.Generic;
+using FluentAssertions;
+using FluentAssertions.Execution;
+using GraphQL.Client.Serializer.Newtonsoft;
+using GraphQL.Client.Serializer.SystemTextJson;
+using Newtonsoft.Json;
+using Xunit;
+
+namespace GraphQL.Client.Serializer.Tests
+{
+    public class ConsistencyTests
+    {
+        [Fact]
+        public void MapConvertersShouldBehaveConsistent()
+        {
+            const string json = @"{
+                ""array"": [
+                  ""some stuff"",
+                  ""something else""
+                ],
+                ""string"": ""this is a string"",
+                ""boolean"": true,
+                ""number"": 1234.567,
+                ""nested object"": {
+                    ""prop1"": false
+                },
+                ""arrayOfObjects"": [
+                  {""number"": 1234.567},
+                  {""number"": 567.8}
+                ]
+            }";
+            
+            var newtonsoftSerializer = new NewtonsoftJsonSerializer();
+            var systemTextJsonSerializer = new SystemTextJsonSerializer();
+
+            var newtonsoftMap = JsonConvert.DeserializeObject<Map>(json, newtonsoftSerializer.JsonSerializerSettings);
+            var systemTextJsonMap = System.Text.Json.JsonSerializer.Deserialize<Map>(json, systemTextJsonSerializer.Options);
+
+
+            using(new AssertionScope())
+            {
+                CompareMaps(newtonsoftMap, systemTextJsonMap);
+            }
+
+            newtonsoftMap.Should().BeEquivalentTo(systemTextJsonMap, options => options
+                .RespectingRuntimeTypes());
+        }
+
+        private void CompareMaps(Dictionary<string, object> first, Dictionary<string, object> second)
+        {
+            foreach (var keyValuePair in first)
+            {
+                second.Should().ContainKey(keyValuePair.Key);
+                second[keyValuePair.Key].Should().BeOfType(keyValuePair.Value.GetType());
+                if(keyValuePair.Value is Dictionary<string, object> map)
+                    CompareMaps(map, (Dictionary<string, object>)second[keyValuePair.Key]);
+                else
+                    keyValuePair.Value.Should().BeEquivalentTo(second[keyValuePair.Key]);
+            }
+        }
+    }
+}