From cfac737d07bca0a067488d62a6336361b7f2a6a8 Mon Sep 17 00:00:00 2001 From: Alexander Rose Date: Thu, 23 Nov 2023 09:30:34 +0100 Subject: [PATCH] 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 --- Directory.Build.targets | 10 +-- dotnet-tools.json | 2 +- .../GraphQL.Client.Example.csproj | 2 +- .../MapConverter.cs | 12 ++-- src/GraphQL.Client/GraphQLHttpClient.cs | 4 ++ .../GraphQLHttpClientOptions.cs | 4 ++ src/GraphQL.Client/GraphQLHttpRequest.cs | 4 ++ .../Websocket/GraphQLHttpWebSocket.cs | 4 ++ .../ConsistencyTests.cs | 64 ++++++++++++++----- .../GraphQL.Client.Serializer.Tests.csproj | 2 +- .../GraphQL.Client.Tests.Common.csproj | 2 +- .../GraphQL.Integration.Tests.csproj | 2 +- .../GraphQL.Primitives.Tests.csproj | 2 +- .../GraphQL.Server.Test.csproj | 2 +- .../IntegrationTestServer.csproj | 2 +- 15 files changed, 84 insertions(+), 34 deletions(-) diff --git a/Directory.Build.targets b/Directory.Build.targets index a75e6500..f09b31ba 100644 --- a/Directory.Build.targets +++ b/Directory.Build.targets @@ -6,7 +6,7 @@ true - + $(NoWarn);1591 @@ -33,10 +33,10 @@ - - + + all - runtime; build; native; contentfiles; analyzers; buildtransitive + runtime; build; native; contentfiles; analyzers diff --git a/dotnet-tools.json b/dotnet-tools.json index 7254cc09..e5146732 100644 --- 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 index 28c2c457..07866dfc 100644 --- a/examples/GraphQL.Client.Example/GraphQL.Client.Example.csproj +++ b/examples/GraphQL.Client.Example/GraphQL.Client.Example.csproj @@ -2,7 +2,7 @@ Exe - net7 + net8 false diff --git a/src/GraphQL.Client.Serializer.Newtonsoft/MapConverter.cs b/src/GraphQL.Client.Serializer.Newtonsoft/MapConverter.cs index 90b22ed8..5bc53686 100644 --- 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 index 7ffea50b..dd03334a 100644 --- 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 index 169260f0..6c3b30d1 100644 --- 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 index d03b9f8e..986f1dd5 100644 --- 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 index 32925567..4be55bc7 100644 --- 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 index 200c0acf..7a3f8f55 100644 --- 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 first, Dictionary second) + /// + /// Regression test for https://github.com/graphql-dotnet/graphql-client/issues/601 + /// + [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 map) - CompareMaps(map, (Dictionary)second[keyValuePair.Key]); - else - keyValuePair.Value.Should().BeEquivalentTo(second[keyValuePair.Key]); - } + var newtonsoftSerializer = new NewtonsoftJsonSerializer(); + var systemTextJsonSerializer = new SystemTextJsonSerializer(); + string json = "null"; + + JsonConvert.DeserializeObject(json, newtonsoftSerializer.JsonSerializerSettings).Should().BeNull(); + System.Text.Json.JsonSerializer.Deserialize(json, systemTextJsonSerializer.Options).Should().BeNull(); + } + + private void CompareMaps(Dictionary? first, Dictionary? 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 map) + CompareMaps(map, (Dictionary)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 index ed2da1ab..7273de9a 100644 --- 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 @@ - net7 + net8 diff --git a/tests/GraphQL.Client.Tests.Common/GraphQL.Client.Tests.Common.csproj b/tests/GraphQL.Client.Tests.Common/GraphQL.Client.Tests.Common.csproj index 138f4c67..460fe724 100644 --- 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 @@ - net7.0 + net8.0 false diff --git a/tests/GraphQL.Integration.Tests/GraphQL.Integration.Tests.csproj b/tests/GraphQL.Integration.Tests/GraphQL.Integration.Tests.csproj index d6f082fe..fdc033fa 100644 --- a/tests/GraphQL.Integration.Tests/GraphQL.Integration.Tests.csproj +++ b/tests/GraphQL.Integration.Tests/GraphQL.Integration.Tests.csproj @@ -3,7 +3,7 @@ - net7 + net8 diff --git a/tests/GraphQL.Primitives.Tests/GraphQL.Primitives.Tests.csproj b/tests/GraphQL.Primitives.Tests/GraphQL.Primitives.Tests.csproj index 235c025a..5210f214 100644 --- a/tests/GraphQL.Primitives.Tests/GraphQL.Primitives.Tests.csproj +++ b/tests/GraphQL.Primitives.Tests/GraphQL.Primitives.Tests.csproj @@ -3,7 +3,7 @@ - net7 + net8 diff --git a/tests/GraphQL.Server.Test/GraphQL.Server.Test.csproj b/tests/GraphQL.Server.Test/GraphQL.Server.Test.csproj index 7220da61..4b81bde3 100644 --- a/tests/GraphQL.Server.Test/GraphQL.Server.Test.csproj +++ b/tests/GraphQL.Server.Test/GraphQL.Server.Test.csproj @@ -1,7 +1,7 @@ - net7 + net8 false diff --git a/tests/IntegrationTestServer/IntegrationTestServer.csproj b/tests/IntegrationTestServer/IntegrationTestServer.csproj index ad1e5255..85b003c9 100644 --- a/tests/IntegrationTestServer/IntegrationTestServer.csproj +++ b/tests/IntegrationTestServer/IntegrationTestServer.csproj @@ -1,7 +1,7 @@  - net7 + net8 IntegrationTestServer.Program false