Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions src/NodaTime.Serialization.SystemTextJson/Extensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -103,25 +103,27 @@ internal static string ResolvePropertyName(this JsonSerializerOptions serializer
/// Retrieves the <see cref="JsonConverter"/> from <paramref name="serializerOptions"/> and deserializes the object as <typeparamref name="T"/>.
/// </summary>
/// <param name="serializerOptions">The serializer options to use.</param>
/// <param name="explicitConverter">An explicit converter to use instead of the one in the options, if not null.</param>
/// <param name="reader">Json reader.</param>
/// <typeparam name="T">The type of object to read.</typeparam>
/// <returns>The deserialized object of type <typeparamref name="T"/>.</returns>
internal static T ReadType<T>(this JsonSerializerOptions serializerOptions, ref Utf8JsonReader reader)
internal static T ReadType<T>(this JsonSerializerOptions serializerOptions, JsonConverter<T> explicitConverter, ref Utf8JsonReader reader)
{
var converter = (JsonConverter<T>)serializerOptions.GetConverter(typeof(T));
var converter = explicitConverter ?? (JsonConverter<T>)serializerOptions.GetConverter(typeof(T));
return converter.Read(ref reader, typeof(T), serializerOptions);
}

/// <summary>
/// Retrieves the <see cref="JsonConverter"/> from <paramref name="serializerOptions"/> and serializes the object as <typeparamref name="T"/>.
/// </summary>
/// <param name="serializerOptions">The serializer options to use.</param>
/// <param name="explicitConverter">An explicit converter to use instead of the one in the options, if not null.</param>
/// <param name="writer">Json writer.</param>
/// <param name="value">The value to serialize</param>
/// <typeparam name="T">The type of object to write.</typeparam>
internal static void WriteType<T>(this JsonSerializerOptions serializerOptions, Utf8JsonWriter writer, T value)
internal static void WriteType<T>(this JsonSerializerOptions serializerOptions, JsonConverter<T> explicitConverter, Utf8JsonWriter writer, T value)
{
var converter = (JsonConverter<T>)serializerOptions.GetConverter(typeof(T));
var converter = explicitConverter ?? (JsonConverter<T>)serializerOptions.GetConverter(typeof(T));
converter.Write(writer, value, serializerOptions);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

using System;
using System.Text.Json;
using System.Text.Json.Serialization;
using NodaTime.Utility;

namespace NodaTime.Serialization.SystemTextJson
Expand All @@ -15,6 +16,20 @@ namespace NodaTime.Serialization.SystemTextJson
/// </summary>
internal sealed class NodaDateIntervalConverter : NodaConverterBase<DateInterval>
{
/// <summary>
/// LocalDate converter to use, overriding whatever is in JsonSerializerOptions.
/// </summary>
private readonly JsonConverter<LocalDate> localDateConverter;

internal NodaDateIntervalConverter() : this(null)
{
}

internal NodaDateIntervalConverter(JsonConverter<LocalDate> localDateConverter)
{
this.localDateConverter = localDateConverter;
}

/// <summary>
/// Reads Start and End properties for the start and end of a date interval, converting them to local dates
/// using the given serializer.
Expand Down Expand Up @@ -44,13 +59,13 @@ protected override DateInterval ReadJsonImpl(ref Utf8JsonReader reader, JsonSeri
var startPropertyName = options.ResolvePropertyName(nameof(Interval.Start));
if (string.Equals(propertyName, startPropertyName, caseSensitivity))
{
startLocalDate = options.ReadType<LocalDate>(ref reader);
startLocalDate = options.ReadType<LocalDate>(localDateConverter, ref reader);
}

var endPropertyName = options.ResolvePropertyName(nameof(Interval.End));
if (string.Equals(propertyName, endPropertyName, caseSensitivity))
{
endLocalDate = options.ReadType<LocalDate>(ref reader);
endLocalDate = options.ReadType<LocalDate>(localDateConverter, ref reader);
}
}

Expand Down Expand Up @@ -79,11 +94,11 @@ protected override void WriteJsonImpl(Utf8JsonWriter writer, DateInterval value,

var startPropertyName = options.ResolvePropertyName(nameof(Interval.Start));
writer.WritePropertyName(startPropertyName);
options.WriteType(writer, value.Start);
options.WriteType(localDateConverter, writer, value.Start);

var endPropertyName = options.ResolvePropertyName(nameof(Interval.End));
writer.WritePropertyName(endPropertyName);
options.WriteType(writer, value.End);
options.WriteType(localDateConverter, writer, value.End);

writer.WriteEndObject();
}
Expand Down
23 changes: 19 additions & 4 deletions src/NodaTime.Serialization.SystemTextJson/NodaIntervalConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

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

namespace NodaTime.Serialization.SystemTextJson
{
Expand All @@ -14,6 +15,20 @@ namespace NodaTime.Serialization.SystemTextJson
/// </summary>
internal sealed class NodaIntervalConverter : NodaConverterBase<Interval>
{
/// <summary>
/// Instant converter to use, overriding whatever is in JsonSerializerOptions.
/// </summary>
private readonly JsonConverter<Instant> instantConverter;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I'm surprised this doesn't need to be declared as nullable; but also later I'm also asking if we need to accept null here.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're not using nullable reference types in this project yet. (I should probably do so at some point.)


internal NodaIntervalConverter() : this(null)
{
}

internal NodaIntervalConverter(JsonConverter<Instant> instantConverter)
{
this.instantConverter = instantConverter;
}

/// <summary>
/// Reads Start and End properties for the start and end of an interval, converting them to instants
/// using the given serializer.
Expand Down Expand Up @@ -45,13 +60,13 @@ protected override Interval ReadJsonImpl(ref Utf8JsonReader reader, JsonSerializ
var startPropertyName = options.ResolvePropertyName(nameof(Interval.Start));
if (string.Equals(propertyName, startPropertyName, caseSensitivity))
{
startInstant = options.ReadType<Instant>(ref reader);
startInstant = options.ReadType<Instant>(instantConverter, ref reader);
}

var endPropertyName = options.ResolvePropertyName(nameof(Interval.End));
if (string.Equals(propertyName, endPropertyName, caseSensitivity))
{
endInstant = options.ReadType<Instant>(ref reader);
endInstant = options.ReadType<Instant>(instantConverter, ref reader);
}
}

Expand All @@ -72,13 +87,13 @@ protected override void WriteJsonImpl(Utf8JsonWriter writer, Interval value, Jso
{
var startPropertyName = options.ResolvePropertyName(nameof(Interval.Start));
writer.WritePropertyName(startPropertyName);
options.WriteType(writer, value.Start);
options.WriteType(instantConverter, writer, value.Start);
}
if (value.HasEnd)
{
var endPropertyName = options.ResolvePropertyName(nameof(Interval.End));
writer.WritePropertyName(endPropertyName);
options.WriteType(writer, value.End);
options.WriteType(instantConverter, writer, value.End);
}
writer.WriteEndObject();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ private static Dictionary<Type, JsonConverter> CreateConverterDictionary()
Add(NodaConverters.AnnualDateConverter);
Add(NodaConverters.DurationConverterImpl);
Add(NodaConverters.InstantConverter);
Add(NodaConverters.IntervalConverter);
Add(new NodaIntervalConverter(NodaConverters.InstantConverter));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I must admit that I'm not entirely following this change, but should this change be made to the NodaConverters.IntervalConverter property itself?

(That would also obviate the need for two separate constructors for NodaIntervalConverter etc, I think.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so - because if someone creates a set of options and specifies a non-default Instant converter (e.g. new NodaJsonSettings { IntervalConverter = ...}, that should be used for converting the instants in an interval, IMO.

Add(NodaConverters.LocalDateConverter);
Add(NodaConverters.LocalDateTimeConverter);
Add(NodaConverters.LocalTimeConverter);
Expand All @@ -51,7 +51,7 @@ private static Dictionary<Type, JsonConverter> CreateConverterDictionary()
Add(NodaConverters.CreateZonedDateTimeConverter(DateTimeZoneProviders.Tzdb));

// Reference types
converters[typeof(DateInterval)] = NodaConverters.DateIntervalConverter;
converters[typeof(DateInterval)] = new NodaDateIntervalConverter(NodaConverters.LocalDateConverter);
converters[typeof(DateTimeZone)] = NodaConverters.CreateDateTimeZoneConverter(DateTimeZoneProviders.Tzdb);
converters[typeof(Period)] = NodaConverters.RoundtripPeriodConverter;
return converters;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// as found in the LICENSE.txt file.

using NodaTime.Serialization.SystemTextJson;
using NodaTime.Text;
using NUnit.Framework;
using System.Text.Json;

Expand All @@ -13,21 +14,50 @@ public class NodaTimeDefaultJsonConverterAttributeTest
[Test]
public void Roundtrip()
{
var options = new JsonSerializerOptions
{
// Converter that only serializes to the minute. Won't round-trip our sample value, and
// the result can't be parsed with the default converter.
// The fact that we *do* roundtrip shows that the custom options are ignored in favor of the
// attribute saying "use the default".
Converters = { new NodaPatternConverter<Instant>(InstantPattern.CreateWithInvariantCulture("uuuu'-'MM'-'dd'T'HH':'mm'Z'")) },
};
AssertRoundtrip(null, null);
AssertRoundtrip(null, options);
AssertRoundtrip(options, null);
AssertRoundtrip(options, options);
}

private void AssertRoundtrip(JsonSerializerOptions serializationOptions, JsonSerializerOptions deserializationOptions)
{
var options = new JsonSerializerOptions
{
// Converter that only serializes to the minute. Won't round-trip our sample value, and
// the result can't be parsed with the default converter.
// The fact that we *do* roundtrip shows that the custom options are ignored in favor of the
// attribute saying "use the default".
Converters = { new NodaPatternConverter<Instant>(InstantPattern.CreateWithInvariantCulture("uuuu'-'MM'-'dd'T'HH':'mm'Z'")) },
};

var obj = new SampleClass
{
NonNullableInstant = Instant.FromUtc(2023, 5, 29, 14, 11, 23),
NullableInstant1 = Instant.FromUtc(2025, 1, 2, 3, 4, 5),
NullableInstant2 = null,
ZonedDateTime = Instant.FromUtc(2023, 5, 29, 14, 11, 23).InZone(DateTimeZoneProviders.Tzdb["Europe/London"])
ZonedDateTime = Instant.FromUtc(2023, 5, 29, 14, 11, 23).InZone(DateTimeZoneProviders.Tzdb["Europe/London"]),
Interval = new(Instant.FromUtc(2023, 5, 29, 14, 11, 23), Instant.FromUtc(2025, 1, 2, 3, 4, 5)),
DateInterval = new(new LocalDate(2025, 2, 13), new LocalDate(2025, 2, 15))
};

string json = JsonSerializer.Serialize(obj);
var result = JsonSerializer.Deserialize<SampleClass>(json);
string json = JsonSerializer.Serialize(obj, serializationOptions);
var result = JsonSerializer.Deserialize<SampleClass>(json, deserializationOptions);

Assert.AreEqual(obj.NonNullableInstant, result.NonNullableInstant);
Assert.AreEqual(obj.NullableInstant1, result.NullableInstant1);
Assert.Null(result.NullableInstant2);
Assert.AreEqual(obj.ZonedDateTime, result.ZonedDateTime);
Assert.AreEqual(obj.Interval, result.Interval);
Assert.AreEqual(obj.DateInterval, result.DateInterval);
}

// Just enough properties to check that the custom converters are being used.
Expand All @@ -41,5 +71,9 @@ public class SampleClass
public Instant? NullableInstant2 { get; set; }
[NodaTimeDefaultJsonConverter]
public ZonedDateTime ZonedDateTime { get; set; }
[NodaTimeDefaultJsonConverter]
public Interval Interval { get; set; }
[NodaTimeDefaultJsonConverter]
public DateInterval DateInterval { get; set; }
}
}