Skip to content

Commit

Permalink
fix: Default JSON serializer instance is not affected by any global s…
Browse files Browse the repository at this point in the history
…ettings.

This partially superceeds #2610 and it's a better approach overall as our serializar is not created using the global settings at all, so we don't need to override global settings.
Note that JsonConvert.Deserialize uses JsonSerializer.CreateDefault that relies on global settings, and then applies any specific settings. In turn we use JsonSerializer.Create that only applies specific settings. By not using JsonConvert we have stopped being affected by global settings at all.
The test from #2610 remains and proves this approach works.

Possibly fixes #2611
  • Loading branch information
amanda-tarafa committed Nov 16, 2023
1 parent 1f551fa commit 5775c73
Showing 1 changed file with 16 additions and 18 deletions.
34 changes: 16 additions & 18 deletions Src/Support/Google.Apis.Core/Json/NewtonsoftJsonSerializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ You may obtain a copy of the License at
limitations under the License.
*/

using Google.Apis.Util;
using Newtonsoft.Json;
using Google.Apis.Util;
using Newtonsoft.Json;
using Newtonsoft.Json.Serialization;
using System;
using System.IO;
using System.Reflection;
using System.Linq;
using System;
using System.IO;
using System.Linq;
using System.Reflection;
using System.Threading;
using System.Threading.Tasks;

Expand Down Expand Up @@ -110,7 +110,6 @@ protected override JsonContract CreateContract(Type objectType)
/// <summary>Class for serialization and deserialization of JSON documents using the Newtonsoft Library.</summary>
public class NewtonsoftJsonSerializer : IJsonSerializer
{
private readonly JsonSerializerSettings settings;
private readonly JsonSerializer serializer;

/// <summary>The default instance of the Newtonsoft JSON Serializer, with default settings.</summary>
Expand All @@ -127,12 +126,8 @@ public NewtonsoftJsonSerializer() : this(CreateDefaultSettings())
/// Constructs a new instance with the given settings.
/// </summary>
/// <param name="settings">The settings to apply when serializing and deserializing. Must not be null.</param>
public NewtonsoftJsonSerializer(JsonSerializerSettings settings)
{
Utilities.ThrowIfNull(settings, nameof(settings));
this.settings = settings;
serializer = JsonSerializer.Create(settings);
}
public NewtonsoftJsonSerializer(JsonSerializerSettings settings) =>
serializer = JsonSerializer.Create(Utilities.ThrowIfNull(settings, nameof(settings)));

/// <summary>
/// Creates a new instance of <see cref="JsonSerializerSettings"/> with the same behavior
Expand All @@ -146,7 +141,6 @@ public static JsonSerializerSettings CreateDefaultSettings() =>
NullValueHandling = NullValueHandling.Ignore,
MetadataPropertyHandling = MetadataPropertyHandling.Ignore,
ContractResolver = new NewtonsoftJsonContractResolver(),
DefaultValueHandling = DefaultValueHandling.Include,
};

/// <inheritdoc/>
Expand Down Expand Up @@ -184,9 +178,9 @@ public T Deserialize<T>(string input)
{
if (string.IsNullOrEmpty(input))
{
return default(T);
return default;
}
return JsonConvert.DeserializeObject<T>(input, settings);
return (T)Deserialize(input, typeof(T));
}

/// <inheritdoc/>
Expand All @@ -196,7 +190,11 @@ public object Deserialize(string input, Type type)
{
return null;
}
return JsonConvert.DeserializeObject(input, type, settings);

using (JsonTextReader reader = new JsonTextReader(new StringReader(input)))
{
return serializer.Deserialize(reader, type);
}
}

/// <inheritdoc/>
Expand Down Expand Up @@ -229,4 +227,4 @@ public async Task<T> DeserializeAsync<T>(Stream input, CancellationToken cancell
}
}
}
}
}

0 comments on commit 5775c73

Please sign in to comment.