Skip to content

Commit cfac737

Browse files
rose-arose-a
authored
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
1 parent d422bfd commit cfac737

File tree

15 files changed

+84
-34
lines changed

15 files changed

+84
-34
lines changed

‎Directory.Build.targets‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
<GeneratePackageOnBuild>true</GeneratePackageOnBuild> <!--TODO: remove? since generated on CI-->
77
</PropertyGroup>
88

9-
<PropertyGroup Condition="'$(IsPackable)' == 'true' AND '$(GitVersion_SemVer)' != ''">
9+
<!-- <PropertyGroup Condition="'$(IsPackable)' == 'true' AND '$(GitVersion_SemVer)' != ''">
1010
<GetVersion>false</GetVersion>
1111
<WriteVersionInfoToBuildLog>false</WriteVersionInfoToBuildLog>
1212
<UpdateAssemblyInfo>false</UpdateAssemblyInfo>
@@ -23,7 +23,7 @@
2323
<FileVersion Condition=" '$(FileVersion)' == '' ">$(GitVersion_AssemblySemFileVer)</FileVersion>
2424
<RepositoryBranch Condition=" '$(RepositoryBranch)' == '' ">$(GitVersion_BranchName)</RepositoryBranch>
2525
<RepositoryCommit Condition=" '$(RepositoryCommit)' == '' ">$(GitVersion_Sha)</RepositoryCommit>
26-
</PropertyGroup>
26+
</PropertyGroup> -->
2727

2828
<PropertyGroup Condition="'$(IsPackable)' != 'true'">
2929
<NoWarn>$(NoWarn);1591</NoWarn>
@@ -33,10 +33,10 @@
3333
<None Include="..\..\assets\logo.64x64.png" Pack="true" PackagePath="\" Visible="false" />
3434
<None Include="..\..\README.md" Pack="true" PackagePath="\" Visible="false" />
3535
<None Include="..\..\LICENSE.txt" Pack="true" PackagePath="\" Visible="false" />
36-
<PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.1.1" PrivateAssets="All" />
37-
<PackageReference Update="GitVersionTask" Version="5.6.7">
36+
<!-- <PackageReference Include="Microsoft.SourceLink.GitHub" Version="8.0.0" PrivateAssets="All" /> -->
37+
<PackageReference Include="GitVersion.MsBuild" Version="5.12.0">
3838
<PrivateAssets>all</PrivateAssets>
39-
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
39+
<IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
4040
</PackageReference>
4141
</ItemGroup>
4242

‎dotnet-tools.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
]
99
},
1010
"gitversion.tool": {
11-
"version": "5.10.3",
11+
"version": "5.12.0",
1212
"commands": [
1313
"dotnet-gitversion"
1414
]

‎examples/GraphQL.Client.Example/GraphQL.Client.Example.csproj‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
<PropertyGroup>
44
<OutputType>Exe</OutputType>
5-
<TargetFramework>net7</TargetFramework>
5+
<TargetFramework>net8</TargetFramework>
66
<IsPackable>false</IsPackable>
77
</PropertyGroup>
88

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,15 @@ public override void WriteJson(JsonWriter writer, Map value, JsonSerializer seri
99
throw new NotImplementedException(
1010
"This converter currently is only intended to be used to read a JSON object into a strongly-typed representation.");
1111

12-
public override Map ReadJson(JsonReader reader, Type objectType, Map existingValue, bool hasExistingValue, JsonSerializer serializer)
12+
public override Map? ReadJson(JsonReader reader, Type objectType, Map existingValue, bool hasExistingValue, JsonSerializer serializer)
1313
{
1414
var rootToken = JToken.ReadFrom(reader);
15-
if (rootToken is JObject)
15+
return rootToken.Type switch
1616
{
17-
return (Map)ReadDictionary(rootToken, new Map());
18-
}
19-
else
20-
throw new ArgumentException("This converter can only parse when the root element is a JSON Object.");
17+
JTokenType.Object => (Map)ReadDictionary(rootToken, new Map()),
18+
JTokenType.Null => null,
19+
_ => throw new ArgumentException("This converter can only parse when the root element is a JSON Object.")
20+
};
2121
}
2222

2323
private object? ReadToken(JToken? token) =>

‎src/GraphQL.Client/GraphQLHttpClient.cs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
11
using System.Diagnostics;
2+
#pragma warning disable IDE0005
3+
// see https://learn.microsoft.com/en-us/dotnet/core/compatibility/sdk/8.0/implicit-global-using-netfx
4+
using System.Net.Http;
5+
#pragma warning restore IDE0005
26
using GraphQL.Client.Abstractions;
37
using GraphQL.Client.Abstractions.Websocket;
48
using GraphQL.Client.Http.Websocket;

‎src/GraphQL.Client/GraphQLHttpClientOptions.cs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
11
using System.Net;
2+
#pragma warning disable IDE0005
3+
// see https://learn.microsoft.com/en-us/dotnet/core/compatibility/sdk/8.0/implicit-global-using-netfx
4+
using System.Net.Http;
5+
#pragma warning restore IDE0005
26
using System.Net.Http.Headers;
37
using System.Net.WebSockets;
48
using GraphQL.Client.Http.Websocket;

‎src/GraphQL.Client/GraphQLHttpRequest.cs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
#pragma warning disable IDE0005
2+
// see https://learn.microsoft.com/en-us/dotnet/core/compatibility/sdk/8.0/implicit-global-using-netfx
3+
using System.Net.Http;
4+
#pragma warning restore IDE0005
15
using System.Net.Http.Headers;
26
using System.Text;
37
using GraphQL.Client.Abstractions;

‎src/GraphQL.Client/Websocket/GraphQLHttpWebSocket.cs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
11
using System.Diagnostics;
2+
#pragma warning disable IDE0005
3+
// see https://learn.microsoft.com/en-us/dotnet/core/compatibility/sdk/8.0/implicit-global-using-netfx
4+
using System.Net.Http;
5+
#pragma warning restore IDE0005
26
using System.Net.WebSockets;
37
using System.Reactive;
48
using System.Reactive.Disposables;

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

Lines changed: 49 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,8 @@ namespace GraphQL.Client.Serializer.Tests;
99

1010
public class ConsistencyTests
1111
{
12-
[Fact]
13-
public void MapConvertersShouldBehaveConsistent()
14-
{
15-
const string json = @"{
12+
[Theory]
13+
[InlineData(@"{
1614
""array"": [
1715
""some stuff"",
1816
""something else""
@@ -27,7 +25,26 @@ public void MapConvertersShouldBehaveConsistent()
2725
{""number"": 1234.567},
2826
{""number"": 567.8}
2927
]
30-
}";
28+
}")]
29+
[InlineData("null")]
30+
public void MapConvertersShouldBehaveConsistent(string json)
31+
{
32+
//const string json = @"{
33+
// ""array"": [
34+
// ""some stuff"",
35+
// ""something else""
36+
// ],
37+
// ""string"": ""this is a string"",
38+
// ""boolean"": true,
39+
// ""number"": 1234.567,
40+
// ""nested object"": {
41+
// ""prop1"": false
42+
// },
43+
// ""arrayOfObjects"": [
44+
// {""number"": 1234.567},
45+
// {""number"": 567.8}
46+
// ]
47+
// }";
3148

3249
var newtonsoftSerializer = new NewtonsoftJsonSerializer();
3350
var systemTextJsonSerializer = new SystemTextJsonSerializer();
@@ -45,16 +62,33 @@ public void MapConvertersShouldBehaveConsistent()
4562
.RespectingRuntimeTypes());
4663
}
4764

48-
private void CompareMaps(Dictionary<string, object> first, Dictionary<string, object> second)
65+
/// <summary>
66+
/// Regression test for https://github.com/graphql-dotnet/graphql-client/issues/601
67+
/// </summary>
68+
[Fact]
69+
public void MapConvertersShouldBeAbleToDeserializeNullValues()
4970
{
50-
foreach (var keyValuePair in first)
51-
{
52-
second.Should().ContainKey(keyValuePair.Key);
53-
second[keyValuePair.Key].Should().BeOfType(keyValuePair.Value.GetType());
54-
if (keyValuePair.Value is Dictionary<string, object> map)
55-
CompareMaps(map, (Dictionary<string, object>)second[keyValuePair.Key]);
56-
else
57-
keyValuePair.Value.Should().BeEquivalentTo(second[keyValuePair.Key]);
58-
}
71+
var newtonsoftSerializer = new NewtonsoftJsonSerializer();
72+
var systemTextJsonSerializer = new SystemTextJsonSerializer();
73+
string json = "null";
74+
75+
JsonConvert.DeserializeObject<Map>(json, newtonsoftSerializer.JsonSerializerSettings).Should().BeNull();
76+
System.Text.Json.JsonSerializer.Deserialize<Map>(json, systemTextJsonSerializer.Options).Should().BeNull();
77+
}
78+
79+
private void CompareMaps(Dictionary<string, object>? first, Dictionary<string, object>? second)
80+
{
81+
if (first is null)
82+
second.Should().BeNull();
83+
else
84+
foreach (var keyValuePair in first)
85+
{
86+
second.Should().ContainKey(keyValuePair.Key);
87+
second[keyValuePair.Key].Should().BeOfType(keyValuePair.Value.GetType());
88+
if (keyValuePair.Value is Dictionary<string, object> map)
89+
CompareMaps(map, (Dictionary<string, object>)second[keyValuePair.Key]);
90+
else
91+
keyValuePair.Value.Should().BeEquivalentTo(second[keyValuePair.Key]);
92+
}
5993
}
6094
}

‎tests/GraphQL.Client.Serializer.Tests/GraphQL.Client.Serializer.Tests.csproj‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
<Import Project="../tests.props" />
44

55
<PropertyGroup>
6-
<TargetFramework>net7</TargetFramework>
6+
<TargetFramework>net8</TargetFramework>
77
</PropertyGroup>
88

99
<ItemGroup>

0 commit comments

Comments
 (0)