JsonSerializer support for TimeSpan in 3.0?

[martincostello]

EDIT @eiriktsarpalis: See #29932 (comment) for the API proposal.

Apologies if this is already answered somewhere else, but if it was my search abilities failed me.

I'm trying out the preview 6 of 3.0 with some existing production application code, and one of the issues that's come out of testing is that some of our existing objects we use in our API contracts use properties which are TimeSpan values, which are then represented as strings.

Is support for TimeSpan properties planned for 3.0 for the new System.Text.Json APIs?

If it's not going to be that gives us notice to do some refactoring ahead of September to change them to strings so we can use the new serializer, where as if it's planned but just not implemented yet then we just need to wait for a later preview to get this working.

Below is a minimal repro unit test that demonstrates the failure for TimeSpan to be handled compared to our existing JSON .NET code.

using System;
using System.Text.Json.Serialization;
using Xunit;
using JsonConvert = Newtonsoft.Json.JsonConvert;
using JsonSerializer = System.Text.Json.Serialization.JsonSerializer;

namespace JsonSerializerTimeSpanNotSupportedException
{
    public static class Repro
    {
        [Fact]
        public static void Can_Deserialize_Object_With_SystemTextJson()
        {
            // Arrange
            string json = "{\"child\":{\"value\":\"00:10:00\"}}";

            // Act (fails in preview 6, throws: System.Text.Json.JsonException : The JSON value could not be converted to System.TimeSpan. Path: $.child.value | LineNumber: 0 | BytePositionInLine: 28.)
            var actual = JsonSerializer.Parse<Parent>(json, new JsonSerializerOptions() { PropertyNamingPolicy = JsonNamingPolicy.CamelCase });

            // Assert
            Assert.NotNull(actual);
            Assert.NotNull(actual.Child);
            Assert.Equal(TimeSpan.FromMinutes(10), actual.Child.Value);
        }

        [Fact]
        public static void Can_Deserialize_Object_With_NewtonsoftJson()
        {
            // Arrange
            string json = "{\"child\":{\"value\":\"00:10:00\"}}";

            var actual = JsonConvert.DeserializeObject<Parent>(json);

            // Assert
            Assert.NotNull(actual);
            Assert.NotNull(actual.Child);
            Assert.Equal(TimeSpan.FromMinutes(10), actual.Child.Value);
        }

        private sealed class Parent
        {
            public Child Child { get; set; }
        }

        private sealed class Child
        {
            public TimeSpan Value { get; set; }
        }
    }
}

[ahsonkhan]

We currently do not have plans to support TimeSpan and would be added in the future but I can do some investigation to see how much work is involved. Alternatively, you could create your own JsonConverter to support TimeSpan as a workaround. I'll give an update by end of next week. Thanks.

If we were to add it, we would also want to add TimeSpan APIs to the reader/writer/JsonElement, which would have to go through api review.

[martincostello]

Thanks - a custom converter would also be an easy enough way to make it work for our app for 3.0. Are those capabilities planned to ship with preview 7?

[steveharter]

Yes the custom converter support is now in master and thus will be in preview 7.

However, since TimeSpan is a common BCL type we should still provide a default converter.

[terrajobst]

We reviewed this today:

• There is an argument in favor of it, because it's a value type. In principle we could optimize parsing to be allocation free (i.e. avoid going through a string).
• It's unclear whether there is an standard encoding for time spans.
• For now, close. This can be reopened if there is enough customer evidence that this needed.

[khellang]

• It's unclear whether there is an standard encoding for time spans.

The standard encoding for time spans would be ISO8601's "duration". This is also what XML use to represent TimeSpan and is already implemented in XmlConvert and XsdDuration:

https://github.com/dotnet/corefx/blob/6d723b8e5ae3129c0a94252292322fc19673478f/src/System.Private.Xml/src/System/Xml/XmlConvert.cs#L1128-L1146

https://github.com/dotnet/corefx/blob/6d723b8e5ae3129c0a94252292322fc19673478f/src/System.Private.Xml/src/System/Xml/Schema/XsdDuration.cs#L229-L236

Now, arguably, TimeSpan itself should support parsing and formatting ISO8601 duration formats. Maybe it would be better, as a first step, to add a new standard TimeSpan format string, similar to DateTime[Offset]'s round-trip ("O", "o") format specifier? Adding a converter after that would be really simple.

[yhvicey]

This also changed default behavior of how AspNetCore handing returning TimeSpan type in API, see dotnet/aspnetcore#11724. This might be a breaking change.

[rezabay]

The simplest solution is to create a custom converter:

public class TimeSpanConverter : JsonConverter<TimeSpan>
    {
        public override TimeSpan Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
        {
            return TimeSpan.Parse(reader.GetString());
        }

        public override void Write(Utf8JsonWriter writer, TimeSpan value, JsonSerializerOptions options)
        {
            writer.WriteStringValue(value.ToString());
        }
    }

[FForthmann]

Talking about customer evidence: for our software-development support for TimeSpan is highly necessary.

[IGx89]

Ran into this today porting an app to .NET Core 3.0 as well. Because this is closed does that mean Microsoft has no plans to add native support? @khellang's comment seems a pretty convincing argument to me that it should be on a roadmap somewhere...

[steveharter]

Re-opening for 5.0 based on additional asks. The ISO8601 duration is likely the best representation although compat with Newtonsoft should also be considered.

[mfeingol]

Just ran into this issue today. The default behavior is worse than previous behavior and was entirely unexpected. We should fix it, either by using ISO8601 or being compatible with Newtonsoft.

[khellang]

The default behavior is worse than previous behavior and was entirely unexpected.

@mfeingol What behavior? That it simply fails?

We should fix it, either by using ISO8601 or being compatible with Newtonsoft.

It's really easy to just add the workaround @rezabayesteh mentioned.