Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

System.Text.Json: Add TimeSpanConverter #54186

Merged
merged 11 commits into from
Jul 7, 2021
1 change: 1 addition & 0 deletions src/libraries/System.Text.Json/ref/System.Text.Json.cs
Original file line number Diff line number Diff line change
Expand Up @@ -887,6 +887,7 @@ public static partial class JsonMetadataServices
public static System.Text.Json.Serialization.JsonConverter<sbyte> SByteConverter { get { throw null; } }
public static System.Text.Json.Serialization.JsonConverter<float> SingleConverter { get { throw null; } }
public static System.Text.Json.Serialization.JsonConverter<string> StringConverter { get { throw null; } }
public static System.Text.Json.Serialization.JsonConverter<System.TimeSpan> TimeSpanConverter { get { throw null; } }
[System.CLSCompliantAttribute(false)]
public static System.Text.Json.Serialization.JsonConverter<ushort> UInt16Converter { get { throw null; } }
[System.CLSCompliantAttribute(false)]
Expand Down
3 changes: 3 additions & 0 deletions src/libraries/System.Text.Json/src/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,9 @@
<data name="FormatDateTimeOffset" xml:space="preserve">
<value>The JSON value is not in a supported DateTimeOffset format.</value>
</data>
<data name="FormatTimeSpan" xml:space="preserve">
<value>The JSON value is not in a supported TimeSpan format.</value>
</data>
<data name="FormatGuid" xml:space="preserve">
<value>The JSON value is not in a supported Guid format.</value>
</data>
Expand Down
1 change: 1 addition & 0 deletions src/libraries/System.Text.Json/src/System.Text.Json.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@
<Compile Include="System\Text\Json\Serialization\Converters\Value\SByteConverter.cs" />
<Compile Include="System\Text\Json\Serialization\Converters\Value\SingleConverter.cs" />
<Compile Include="System\Text\Json\Serialization\Converters\Value\StringConverter.cs" />
<Compile Include="System\Text\Json\Serialization\Converters\Value\TimeSpanConverter.cs" />
<Compile Include="System\Text\Json\Serialization\Converters\Value\UInt16Converter.cs" />
<Compile Include="System\Text\Json\Serialization\Converters\Value\UInt32Converter.cs" />
<Compile Include="System\Text\Json\Serialization\Converters\Value\UInt64Converter.cs" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Buffers;
using System.Buffers.Text;
using System.Diagnostics;

namespace System.Text.Json.Serialization.Converters
{
internal sealed class TimeSpanConverter : JsonConverter<TimeSpan>
{
private const int MinimumTimeSpanFormatLength = 7; // h:mm:ss
private const int MaximumTimeSpanFormatLength = 26; // -dddddddd.hh:mm:ss.fffffff

public override TimeSpan Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
if (reader.TokenType != JsonTokenType.String)
{
throw ThrowHelper.GetInvalidOperationException_ExpectedString(reader.TokenType);
}

ReadOnlySpan<byte> source = stackalloc byte[0];

if (reader.HasValueSequence)
{
ReadOnlySequence<byte> valueSequence = reader.ValueSequence;
long sequenceLength = valueSequence.Length;

if (!JsonHelpers.IsInRangeInclusive(sequenceLength, MinimumTimeSpanFormatLength, MaximumTimeSpanFormatLength))
{
throw ThrowHelper.GetFormatException();
}

Span<byte> stackSpan = stackalloc byte[(int)sequenceLength];
CodeBlanch marked this conversation as resolved.
Show resolved Hide resolved

valueSequence.CopyTo(stackSpan);
source = stackSpan;
}
else
{
source = reader.ValueSpan;

if (!JsonHelpers.IsInRangeInclusive(source.Length, MinimumTimeSpanFormatLength, MaximumTimeSpanFormatLength))
{
throw ThrowHelper.GetFormatException();
}
}

bool result = Utf8Parser.TryParse(source, out TimeSpan tmpValue, out int bytesConsumed, 'c');

// Note: Utf8Parser.TryParse will return true for invalid input so
// long as it starts with an integer. Example: "2021-06-18" or
// "1$$$$$$$$$$". We need to check bytesConsumed to know if the
// entire source was actually valid.

if (result && source.Length == bytesConsumed)
{
return tmpValue;
}

result = Utf8Parser.TryParse(source, out tmpValue, out bytesConsumed, 'g');
layomia marked this conversation as resolved.
Show resolved Hide resolved

if (result && source.Length == bytesConsumed)
{
return tmpValue;
}

throw ThrowHelper.GetFormatException();
}

public override void Write(Utf8JsonWriter writer, TimeSpan value, JsonSerializerOptions options)
{
Span<byte> output = stackalloc byte[MaximumTimeSpanFormatLength];

bool result = Utf8Formatter.TryFormat(value, output, out int bytesWritten, 'c');
Debug.Assert(result);

writer.WriteStringValue(output.Slice(0, bytesWritten));
Copy link
Contributor

@layomia layomia Jul 1, 2021

Choose a reason for hiding this comment

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

[Non-blocking for this PR] When #54254 goes in, we should consider updating this to use the new Utf8JsonWriter.WriteRawValue API so we can avoid logic that checks whether the input payload needs to be escaped. We'd first need to quote the formatted value.

}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ private void RootBuiltInConverters()

private static Dictionary<Type, JsonConverter> GetDefaultSimpleConverters()
{
const int NumberOfSimpleConverters = 23;
const int NumberOfSimpleConverters = 24;
var converters = new Dictionary<Type, JsonConverter>(NumberOfSimpleConverters);

// Use a dictionary for simple converters.
Expand All @@ -72,6 +72,7 @@ private void RootBuiltInConverters()
Add(JsonMetadataServices.SByteConverter);
Add(JsonMetadataServices.SingleConverter);
Add(JsonMetadataServices.StringConverter);
Add(JsonMetadataServices.TimeSpanConverter);
Add(JsonMetadataServices.UInt16Converter);
Add(JsonMetadataServices.UInt32Converter);
Add(JsonMetadataServices.UInt64Converter);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,12 @@ public static partial class JsonMetadataServices
public static JsonConverter<string> StringConverter => s_stringConverter ??= new StringConverter();
private static JsonConverter<string>? s_stringConverter;

/// <summary>
/// Returns a <see cref="JsonConverter{T}"/> instance that converts <see cref="TimeSpan"/> values.
/// </summary>
public static JsonConverter<TimeSpan> TimeSpanConverter => s_timeSpanConverter ??= new TimeSpanConverter();
layomia marked this conversation as resolved.
Show resolved Hide resolved
private static JsonConverter<TimeSpan>? s_timeSpanConverter;

/// <summary>
/// Returns a <see cref="JsonConverter{T}"/> instance that converts <see cref="ushort"/> values.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,9 @@ public static FormatException GetFormatException(DataType dateType)
case DataType.DateTimeOffset:
message = SR.FormatDateTimeOffset;
break;
case DataType.TimeSpan:
message = SR.FormatTimeSpan;
break;
CodeBlanch marked this conversation as resolved.
Show resolved Hide resolved
case DataType.Base64String:
message = SR.CannotDecodeInvalidBase64;
break;
Expand Down Expand Up @@ -723,6 +726,7 @@ internal enum DataType
Boolean,
DateTime,
DateTimeOffset,
TimeSpan,
Base64String,
Guid,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Globalization;
using System.Runtime.CompilerServices;
using Newtonsoft.Json;
using Xunit;

namespace System.Text.Json.Serialization.Tests
Expand Down Expand Up @@ -80,6 +81,7 @@ public static void ReadPrimitivesFail()

Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<DateTime>("\"abc\""));
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<DateTimeOffset>("\"abc\""));
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<TimeSpan>("\"abc\""));
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<Guid>("\"abc\""));

Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<byte>("\"abc\""));
Expand Down Expand Up @@ -118,6 +120,7 @@ public static void ReadPrimitivesFail()
[InlineData(typeof(sbyte))]
[InlineData(typeof(float))]
[InlineData(typeof(string))]
[InlineData(typeof(TimeSpan))]
[InlineData(typeof(ushort))]
[InlineData(typeof(uint))]
[InlineData(typeof(ulong))]
Expand Down Expand Up @@ -303,6 +306,9 @@ public static void ValueFail()
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<DateTimeOffset>(unexpectedString));
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<DateTimeOffset?>(unexpectedString));

Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<TimeSpan>(unexpectedString));
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<TimeSpan?>(unexpectedString));
Copy link
Contributor

Choose a reason for hiding this comment

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

Due to other logic, we should be fine, but a few more nullable TimeSpan tests would be good. E.g. deserializing null into a non-nullable TimeSpan should fail, but should be fine for nullable TimeSpan?.


Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<string>("1"));

Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<char>("1"));
Expand Down Expand Up @@ -442,5 +448,64 @@ private static void DeserializeLongJsonString(int stringLength)
string str = JsonSerializer.Deserialize<string>(json);
Assert.True(json.AsSpan(1, json.Length - 2).SequenceEqual(str.AsSpan()));
}

[Theory]
[InlineData("23:59:59")]
[InlineData("23:59:59.9", "23:59:59.9000000")]
[InlineData("23:59:59.9999999")]
[InlineData("1:00:00", "01:00:00")] // 'g' Format
[InlineData("1:2:00:00", "1.02:00:00")] // 'g' Format
[InlineData("9999999.23:59:59.9999999")]
[InlineData("-9999999.23:59:59.9999999")]
CodeBlanch marked this conversation as resolved.
Show resolved Hide resolved
[InlineData("10675199.02:48:05.4775807")] // TimeSpan.MaxValue
[InlineData("-10675199.02:48:05.4775808")] // TimeSpan.MinValue
public static void TimeSpan_Read_Success(string json, string? actual = null)
CodeBlanch marked this conversation as resolved.
Show resolved Hide resolved
{
TimeSpan value = JsonSerializer.Deserialize<TimeSpan>($"\"{json}\"");

Assert.Equal(TimeSpan.Parse(actual ?? json), value);
Assert.Equal(value, JsonConvert.DeserializeObject<TimeSpan>($"\"{json}\""));
}

[Fact]
public static void TimeSpan_Read_KnownDifferences()
{
string value = "24:00:00";

// 24:00:00 should be invalid because hours can only be up to 23.
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<TimeSpan>($"\"{value}\""));

TimeSpan expectedValue = TimeSpan.Parse("24.00:00:00");

// TimeSpan.Parse has a quirk where it treats 24:00:00 as 24.00:00:00.
Assert.Equal(expectedValue, TimeSpan.Parse(value));

// Newtonsoft uses TimeSpan.Parse so it is subject to the quirk.
Assert.Equal(expectedValue, JsonConvert.DeserializeObject<TimeSpan>($"\"{value}\""));
}

[Theory]
[InlineData("24:00:00")]
[InlineData("00:60:00")]
[InlineData("00:00:60")]
[InlineData("00:00:00.00000009")]
[InlineData("900000000.00:00:00")]
[InlineData("+00:00:00")]
[InlineData("2021-06-18")]
[InlineData("1$")]
CodeBlanch marked this conversation as resolved.
Show resolved Hide resolved
[InlineData("10675199.02:48:05.4775808")] // TimeSpan.MaxValue + 1
[InlineData("-10675199.02:48:05.4775809")] // TimeSpan.MinValue - 1
[InlineData("1234", false)]
[InlineData("{}", false)]
[InlineData("[]", false)]
[InlineData("true", false)]
[InlineData("null", false)]
public static void TimeSpan_Read_Failure(string json, bool addQuotes = true)
{
if (addQuotes)
json = $"\"{json}\"";

Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<TimeSpan>(json));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -112,5 +112,24 @@ public static void WritePrimitives()
Assert.Equal(@"""1.2.3.4""", JsonSerializer.Serialize(version));
}
}

[Theory]
[InlineData("1:59:59", "01:59:59")]
[InlineData("23:59:59")]
[InlineData("23:59:59.9", "23:59:59.9000000")]
[InlineData("23:59:59.9999999")]
[InlineData("1.23:59:59")]
[InlineData("9999999.23:59:59.9999999")]
[InlineData("-9999999.23:59:59.9999999")]
CodeBlanch marked this conversation as resolved.
Show resolved Hide resolved
[InlineData("10675199.02:48:05.4775807")] // TimeSpan.MaxValue
[InlineData("-10675199.02:48:05.4775808")] // TimeSpan.MinValue
public static void TimeSpan_Write_Success(string value, string? expectedValue = null)
{
TimeSpan ts = TimeSpan.Parse(value);
string json = JsonSerializer.Serialize(ts);

Assert.Equal($"\"{expectedValue ?? value}\"", json);
Assert.Equal(json, JsonConvert.SerializeObject(ts));
}
}
}