Fix Newtonsoft.Json MapConverter (#602)

* extend test to cover issue * fix test * fix newtonsoft mapconverter * update gitversion * Try to fix breaking change regarding implicit using uf System.Net.Http * update sourcelink package to v 8.0.0 * rm package ref to sourcelink * add using System.Net.Http to GraphQLHttpWebSocket * upgrade net7 projects to net8
graphql-dotnet · Nov 23, 2023 · cfac737 · cfac737
1 parent d422bfd
commit cfac737
Show file tree

Hide file tree

Showing 15 changed files with 84 additions and 34 deletions.
diff --git a/Directory.Build.targets b/Directory.Build.targets
@@ -6,7 +6,7 @@
     <GeneratePackageOnBuild>true</GeneratePackageOnBuild> <!--TODO: remove? since generated on CI-->
   </PropertyGroup>
 
-  <PropertyGroup Condition="'$(IsPackable)' == 'true' AND '$(GitVersion_SemVer)' != ''">
+  <!-- <PropertyGroup Condition="'$(IsPackable)' == 'true' AND '$(GitVersion_SemVer)' != ''">
     <GetVersion>false</GetVersion>
     <WriteVersionInfoToBuildLog>false</WriteVersionInfoToBuildLog>
     <UpdateAssemblyInfo>false</UpdateAssemblyInfo>
@@ -23,7 +23,7 @@
     <FileVersion Condition=" '$(FileVersion)' == '' ">$(GitVersion_AssemblySemFileVer)</FileVersion>
     <RepositoryBranch Condition=" '$(RepositoryBranch)' == '' ">$(GitVersion_BranchName)</RepositoryBranch>
     <RepositoryCommit Condition=" '$(RepositoryCommit)' == '' ">$(GitVersion_Sha)</RepositoryCommit>
-  </PropertyGroup>
+  </PropertyGroup> -->
 
   <PropertyGroup Condition="'$(IsPackable)' != 'true'">
     <NoWarn>$(NoWarn);1591</NoWarn>
@@ -33,10 +33,10 @@
     <None Include="..\..\assets\logo.64x64.png" Pack="true" PackagePath="\" Visible="false" />
     <None Include="..\..\README.md" Pack="true" PackagePath="\" Visible="false" />
     <None Include="..\..\LICENSE.txt" Pack="true" PackagePath="\" Visible="false" />
-    <PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.1.1" PrivateAssets="All" />
-    <PackageReference Update="GitVersionTask" Version="5.6.7">
+    <!-- <PackageReference Include="Microsoft.SourceLink.GitHub" Version="8.0.0" PrivateAssets="All" /> -->
+    <PackageReference Include="GitVersion.MsBuild" Version="5.12.0">
       <PrivateAssets>all</PrivateAssets>
-      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
     </PackageReference>
   </ItemGroup>
 

diff --git a/dotnet-tools.json b/dotnet-tools.json
@@ -8,7 +8,7 @@
       ]
     },
     "gitversion.tool": {
-      "version": "5.10.3",
+      "version": "5.12.0",
       "commands": [
         "dotnet-gitversion"
       ]

diff --git a/examples/GraphQL.Client.Example/GraphQL.Client.Example.csproj b/examples/GraphQL.Client.Example/GraphQL.Client.Example.csproj
@@ -2,7 +2,7 @@
 
   <PropertyGroup>
     <OutputType>Exe</OutputType>
-    <TargetFramework>net7</TargetFramework>
+    <TargetFramework>net8</TargetFramework>
     <IsPackable>false</IsPackable>
   </PropertyGroup>
 

diff --git a/src/GraphQL.Client.Serializer.Newtonsoft/MapConverter.cs b/src/GraphQL.Client.Serializer.Newtonsoft/MapConverter.cs
@@ -9,15 +9,15 @@ public override void WriteJson(JsonWriter writer, Map value, JsonSerializer seri
         throw new NotImplementedException(
             "This converter currently is only intended to be used to read a JSON object into a strongly-typed representation.");
 
-    public override Map ReadJson(JsonReader reader, Type objectType, Map existingValue, bool hasExistingValue, JsonSerializer serializer)
+    public override Map? ReadJson(JsonReader reader, Type objectType, Map existingValue, bool hasExistingValue, JsonSerializer serializer)
     {
         var rootToken = JToken.ReadFrom(reader);
-        if (rootToken is JObject)
+        return rootToken.Type switch
         {
-            return (Map)ReadDictionary(rootToken, new Map());
-        }
-        else
-            throw new ArgumentException("This converter can only parse when the root element is a JSON Object.");
+            JTokenType.Object => (Map)ReadDictionary(rootToken, new Map()),
+            JTokenType.Null => null,
+            _ => throw new ArgumentException("This converter can only parse when the root element is a JSON Object.")
+        };
     }
 
     private object? ReadToken(JToken? token) =>

diff --git a/src/GraphQL.Client/GraphQLHttpClient.cs b/src/GraphQL.Client/GraphQLHttpClient.cs
@@ -1,4 +1,8 @@
 using System.Diagnostics;
+#pragma warning disable IDE0005
+// see https://learn.microsoft.com/en-us/dotnet/core/compatibility/sdk/8.0/implicit-global-using-netfx
+using System.Net.Http;
+#pragma warning restore IDE0005
 using GraphQL.Client.Abstractions;
 using GraphQL.Client.Abstractions.Websocket;
 using GraphQL.Client.Http.Websocket;

diff --git a/src/GraphQL.Client/GraphQLHttpClientOptions.cs b/src/GraphQL.Client/GraphQLHttpClientOptions.cs
@@ -1,4 +1,8 @@
 using System.Net;
+#pragma warning disable IDE0005
+// see https://learn.microsoft.com/en-us/dotnet/core/compatibility/sdk/8.0/implicit-global-using-netfx
+using System.Net.Http;
+#pragma warning restore IDE0005
 using System.Net.Http.Headers;
 using System.Net.WebSockets;
 using GraphQL.Client.Http.Websocket;

diff --git a/src/GraphQL.Client/GraphQLHttpRequest.cs b/src/GraphQL.Client/GraphQLHttpRequest.cs
@@ -1,3 +1,7 @@
+#pragma warning disable IDE0005
+// see https://learn.microsoft.com/en-us/dotnet/core/compatibility/sdk/8.0/implicit-global-using-netfx
+using System.Net.Http;
+#pragma warning restore IDE0005
 using System.Net.Http.Headers;
 using System.Text;
 using GraphQL.Client.Abstractions;

diff --git a/src/GraphQL.Client/Websocket/GraphQLHttpWebSocket.cs b/src/GraphQL.Client/Websocket/GraphQLHttpWebSocket.cs
@@ -1,4 +1,8 @@
 using System.Diagnostics;
+#pragma warning disable IDE0005
+// see https://learn.microsoft.com/en-us/dotnet/core/compatibility/sdk/8.0/implicit-global-using-netfx
+using System.Net.Http;
+#pragma warning restore IDE0005
 using System.Net.WebSockets;
 using System.Reactive;
 using System.Reactive.Disposables;

diff --git a/tests/GraphQL.Client.Serializer.Tests/ConsistencyTests.cs b/tests/GraphQL.Client.Serializer.Tests/ConsistencyTests.cs
@@ -9,10 +9,8 @@ namespace GraphQL.Client.Serializer.Tests;
 
 public class ConsistencyTests
 {
-    [Fact]
-    public void MapConvertersShouldBehaveConsistent()
-    {
-        const string json = @"{
+    [Theory]
+    [InlineData(@"{
                 ""array"": [
                   ""some stuff"",
                   ""something else""
@@ -27,7 +25,26 @@ public void MapConvertersShouldBehaveConsistent()
                   {""number"": 1234.567},
                   {""number"": 567.8}
                 ]
-            }";
+            }")]
+    [InlineData("null")]
+    public void MapConvertersShouldBehaveConsistent(string json)
+    {
+        //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();
@@ -45,16 +62,33 @@ public void MapConvertersShouldBehaveConsistent()
             .RespectingRuntimeTypes());
     }
 
-    private void CompareMaps(Dictionary<string, object> first, Dictionary<string, object> second)
+    /// <summary>
+    /// Regression test for https://github.com/graphql-dotnet/graphql-client/issues/601
+    /// </summary>
+    [Fact]
+    public void MapConvertersShouldBeAbleToDeserializeNullValues()
     {
-        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]);
-        }
+        var newtonsoftSerializer = new NewtonsoftJsonSerializer();
+        var systemTextJsonSerializer = new SystemTextJsonSerializer();
+        string json = "null";
+
+        JsonConvert.DeserializeObject<Map>(json, newtonsoftSerializer.JsonSerializerSettings).Should().BeNull();
+        System.Text.Json.JsonSerializer.Deserialize<Map>(json, systemTextJsonSerializer.Options).Should().BeNull();
+    }
+
+    private void CompareMaps(Dictionary<string, object>? first, Dictionary<string, object>? second)
+    {
+        if (first is null)
+            second.Should().BeNull();
+        else
+            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]);
+            }
     }
 }
diff --git a/tests/GraphQL.Client.Serializer.Tests/GraphQL.Client.Serializer.Tests.csproj b/tests/GraphQL.Client.Serializer.Tests/GraphQL.Client.Serializer.Tests.csproj
@@ -3,7 +3,7 @@
   <Import Project="../tests.props" />
 
   <PropertyGroup>
-    <TargetFramework>net7</TargetFramework>
+    <TargetFramework>net8</TargetFramework>
   </PropertyGroup>
 
   <ItemGroup>

diff --git a/tests/GraphQL.Client.Tests.Common/GraphQL.Client.Tests.Common.csproj b/tests/GraphQL.Client.Tests.Common/GraphQL.Client.Tests.Common.csproj
@@ -1,7 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <TargetFramework>net7.0</TargetFramework>
+    <TargetFramework>net8.0</TargetFramework>
     <IsPackable>false</IsPackable>
   </PropertyGroup>
 

diff --git a/tests/GraphQL.Integration.Tests/GraphQL.Integration.Tests.csproj b/tests/GraphQL.Integration.Tests/GraphQL.Integration.Tests.csproj
@@ -3,7 +3,7 @@
   <Import Project="../tests.props" />
 
   <PropertyGroup>
-    <TargetFramework>net7</TargetFramework>
+    <TargetFramework>net8</TargetFramework>
   </PropertyGroup>
 
   <ItemGroup>

diff --git a/tests/GraphQL.Primitives.Tests/GraphQL.Primitives.Tests.csproj b/tests/GraphQL.Primitives.Tests/GraphQL.Primitives.Tests.csproj
@@ -3,7 +3,7 @@
   <Import Project="../tests.props" />
 
   <PropertyGroup>
-    <TargetFramework>net7</TargetFramework>
+    <TargetFramework>net8</TargetFramework>
   </PropertyGroup>
 
   <ItemGroup>

diff --git a/tests/GraphQL.Server.Test/GraphQL.Server.Test.csproj b/tests/GraphQL.Server.Test/GraphQL.Server.Test.csproj
@@ -1,7 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk.Web">
 
   <PropertyGroup>
-    <TargetFramework>net7</TargetFramework>
+    <TargetFramework>net8</TargetFramework>
     <IsPackable>false</IsPackable>
   </PropertyGroup>
 

diff --git a/tests/IntegrationTestServer/IntegrationTestServer.csproj b/tests/IntegrationTestServer/IntegrationTestServer.csproj
@@ -1,7 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk.Web">
 
   <PropertyGroup>
-    <TargetFramework>net7</TargetFramework>
+    <TargetFramework>net8</TargetFramework>
     <StartupObject>IntegrationTestServer.Program</StartupObject>
     <IsPackable>false</IsPackable>
   </PropertyGroup>