Skip to content

Commit

Permalink
Support for nullable with System.Text.Json source generator
Browse files Browse the repository at this point in the history
Converters inheriting from `JsonConverter<T>` can only handle `T`, they can't also handle
`Nullable<T>` directly if `T` for value types because they don't inherit from
`JsonConverter<Nullable<T>>`. Instead, we need to wrap these in a `NodaNullableConverter`
that handles the null cases.

Fixes #127
  • Loading branch information
brantburnett authored and jskeet committed Nov 20, 2024
1 parent 1817df2 commit f386a3e
Show file tree
Hide file tree
Showing 4 changed files with 127 additions and 10 deletions.
43 changes: 43 additions & 0 deletions src/NodaTime.Serialization.SystemTextJson/NodaNullableConverter.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Copyright 2023 The Noda Time Authors. All rights reserved.
// Use of this source code is governed by the Apache License 2.0,
// as found in the LICENSE.txt file.

using System;
using System.Text.Json;
using System.Text.Json.Serialization;

namespace NodaTime.Serialization.SystemTextJson;

internal class NodaNullableConverter<T> : JsonConverter<T?> where T : struct
{
private readonly JsonConverter<T> _innerConverter;

public NodaNullableConverter(JsonConverter<T> innerConverter)
{
Preconditions.CheckNotNull(innerConverter, nameof(innerConverter));

_innerConverter = innerConverter;
}

public override T? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
if (reader.TokenType == JsonTokenType.Null)
{
return null;
}

return _innerConverter.Read(ref reader, typeToConvert, options);
}

public override void Write(Utf8JsonWriter writer, T? value, JsonSerializerOptions options)
{
if (value is null)
{
writer.WriteNullValue();
}
else
{
_innerConverter.Write(writer, value.Value, options);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,26 +37,33 @@ private static Dictionary<Type, JsonConverter> CreateConverterDictionary()
var converters = new Dictionary<Type, JsonConverter>()
{
{ typeof(AnnualDate), NodaConverters.AnnualDateConverter },
{ typeof(AnnualDate?), CreateNullableConverter(NodaConverters.AnnualDateConverter) },
{ typeof(DateInterval), NodaConverters.DateIntervalConverter },
{ typeof(DateTimeZone), NodaConverters.CreateDateTimeZoneConverter(DateTimeZoneProviders.Tzdb) },
{ typeof(Duration), NodaConverters.DurationConverter },
{ typeof(Duration?), CreateNullableConverter((JsonConverter<Duration>) NodaConverters.DurationConverter) },
{ typeof(Instant), NodaConverters.InstantConverter },
{ typeof(Instant?), CreateNullableConverter(NodaConverters.InstantConverter) },
{ typeof(Interval), NodaConverters.IntervalConverter },
{ typeof(Interval?), CreateNullableConverter(NodaConverters.IntervalConverter) },
{ typeof(LocalDate), NodaConverters.LocalDateConverter },
{ typeof(LocalDate?), CreateNullableConverter(NodaConverters.LocalDateConverter) },
{ typeof(LocalDateTime), NodaConverters.LocalDateTimeConverter },
{ typeof(LocalDateTime?), CreateNullableConverter(NodaConverters.LocalDateTimeConverter) },
{ typeof(LocalTime), NodaConverters.LocalTimeConverter },
{ typeof(LocalTime?), CreateNullableConverter(NodaConverters.LocalTimeConverter) },
{ typeof(Offset), NodaConverters.OffsetConverter },
{ typeof(Offset?), CreateNullableConverter(NodaConverters.OffsetConverter) },
{ typeof(OffsetDate), NodaConverters.OffsetDateConverter },
{ typeof(OffsetDate?), CreateNullableConverter(NodaConverters.OffsetDateConverter) },
{ typeof(OffsetDateTime), NodaConverters.OffsetDateTimeConverter },
{ typeof(OffsetDateTime?), CreateNullableConverter(NodaConverters.OffsetDateTimeConverter) },
{ typeof(OffsetTime), NodaConverters.OffsetTimeConverter },
{ typeof(OffsetTime?), CreateNullableConverter(NodaConverters.OffsetTimeConverter) },
{ typeof(Period), NodaConverters.RoundtripPeriodConverter },
{ typeof(ZonedDateTime), NodaConverters.CreateZonedDateTimeConverter(DateTimeZoneProviders.Tzdb) }
{ typeof(ZonedDateTime), NodaConverters.CreateZonedDateTimeConverter(DateTimeZoneProviders.Tzdb) },
{ typeof(ZonedDateTime?), CreateNullableConverter(NodaConverters.CreateZonedDateTimeConverter(DateTimeZoneProviders.Tzdb)) }
};
// Use the same converter for Nullable<T> as T.
foreach (var entry in converters.Where(pair => pair.Key.IsValueType).ToList())
{
converters[typeof(Nullable<>).MakeGenericType(entry.Key)] = entry.Value;
}
return converters;
}

Expand All @@ -74,4 +81,10 @@ internal static JsonConverter GetConverter(Type typeToConvert) =>
/// <inheritdoc />
public override JsonConverter CreateConverter(Type typeToConvert, JsonSerializerOptions options) =>
GetConverter(typeToConvert);

/// <summary>
/// Helper to construct a <see cref="NodaNullableConverter{T}"/> with generic type inference at the call site.
/// </summary>
private static NodaNullableConverter<T> CreateNullableConverter<T>(JsonConverter<T> innerConverter) where T : struct
=> new NodaNullableConverter<T>(innerConverter);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Copyright 2019 The Noda Time Authors. All rights reserved.
// Use of this source code is governed by the Apache License 2.0,
// as found in the LICENSE.txt file.

using System.Text.Json;
using NodaTime.Serialization.SystemTextJson;
using NUnit.Framework;
using static NodaTime.Serialization.Test.SystemText.TestHelper;

namespace NodaTime.Serialization.Test.SystemText
{
/// <summary>
/// Tests for the converters exposed in NodaConverters.
/// </summary>
public class NodaNullableConverterTest
{
[Test]
public void InstantConverter_NotNull()
{
Instant? value = Instant.FromUtc(2012, 1, 2, 3, 4, 5);
string json = "\"2012-01-02T03:04:05Z\"";
var converter = new NodaTimeDefaultJsonConverterFactory().CreateConverter(typeof(Instant?), new JsonSerializerOptions());
AssertConversions(value, json, converter);
}

[Test]
public void InstantConverter_Null()
{
Instant? value = null;
string json = "null";
var converter = new NodaTimeDefaultJsonConverterFactory().CreateConverter(typeof(Instant?), new JsonSerializerOptions());
AssertConversions(value, json, converter);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,7 @@ public void ConvertibleTypes(Type type)
Assert.IsTrue(factory.CanConvert(type));
var converter = factory.CreateConverter(type, default);
Assert.NotNull(converter);
// The converter doesn't "advertise" that it handles nullable value types,
// unlike the Newtonsoft.Json version.
var typeToCheckForCanConvert = Nullable.GetUnderlyingType(type) ?? type;
Assert.IsTrue(converter.CanConvert(typeToCheckForCanConvert));
Assert.IsTrue(converter.CanConvert(type));
}

[Test]
Expand All @@ -89,13 +86,42 @@ public void SourceGenerationCompatibility()
Assert.AreEqual(expected, actual);
}

// See https://github.com/nodatime/nodatime.serialization/issues/127 and
// https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/source-generation
[Test]
public void SourceGenerationCompatibility_Nullable_NotNull()
{
var sample = new SampleDataNullable { Foo = Instant.FromUtc(2023, 8, 6, 12, 40, 12) };
byte[] utf8Json = JsonSerializer.SerializeToUtf8Bytes(sample, SampleJsonContext.Default.SampleDataNullable);
string actual = Encoding.UTF8.GetString(utf8Json);
string expected = "{\"Foo\":\"2023-08-06T12:40:12Z\"}";
Assert.AreEqual(expected, actual);
}

[Test]
public void SourceGenerationCompatibility_Nullable_IsNull()
{
var sample = new SampleDataNullable { Foo = null };
byte[] utf8Json = JsonSerializer.SerializeToUtf8Bytes(sample, SampleJsonContext.Default.SampleDataNullable);
string actual = Encoding.UTF8.GetString(utf8Json);
string expected = "{\"Foo\":null}";
Assert.AreEqual(expected, actual);
}

public class SampleData
{
[JsonConverter(typeof(NodaTimeDefaultJsonConverterFactory))]
public Instant Foo { get; set; }
}

public class SampleDataNullable
{
[JsonConverter(typeof(NodaTimeDefaultJsonConverterFactory))]
public Instant? Foo { get; set; }
}

[JsonSerializable(typeof(SampleData))]
[JsonSerializable(typeof(SampleDataNullable))]
public partial class SampleJsonContext : JsonSerializerContext
{
}
Expand Down

0 comments on commit f386a3e

Please sign in to comment.