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.

[mfeingol]

@khellang: what I observed with a relatively vanilla ASP.NET Core project is that it serializes a Timespan? as a HasValue field, and then each of the properties of the TimeSpan struct.

It's really easy to just add the workaround

I did, but that shouldn't be necessary for such a commonly used type.

[bashocz]

I've just found this issue today (reported by my customer) and have to switch back all of my aspnet webapi and apps to use Newtonsoft.Json serializer instead using configuration in Startup.cs:

services.AddControllers()
.AddNewtonsoftJson(options =>
{
options.SerializerSettings.NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore;
});

In my cases I use a few nullable TimeSpan (TimeSpan?) and System.Text.Json has serialized it as:

{
"hasValue": true,
"value": {
"tick":0,
"days": 0,
"hours": 0,
"milliseconds": 0,
"minutes": 0,
"seconds": 0,
"totalDays": 0,
"totalHours": 0,
"totalMilliseconds": 0,
"totalMinutes": 0,
"totalSeconds": 0
}
}

This causes a little problem for javascript objects in web browsers, as well as for various cross-platform (means various programming languages) deserializers consuming my apis.

I would expect same serialization result as Newtonsoft.Json serializer:

{
"timeSpan": "00:00:00.0000000",
"nullTimeSpan": null
}

[khellang]

I've just found this issue today (reported by my customer) and have to switch back all of my aspnet webapi and apps to use Newtonsoft.Json serializer instead

@bashocz Why won't the workaround mentioned in https://github.com/dotnet/corefx/issues/38641#issuecomment-540200476 work for you?

[bashocz]

Why won't the workaround mentioned in #38641 (comment) work for you?

@khellang I just want to highlight TimeSpan serialization is an issue for another developers, and give it an attention. I'd very like to switch to System.Text.Json, however there are some obstacles.

I'd investigated new System.Text.Json a few weeks ago and found it is not feature complete. I've raised an issue dotnet/corefx#41747 and was pointed to other dotnet/corefx#39031, dotnet/corefx#41325, dotnet/corefx#38650 related. Because of that all of our internal microservices still use Newtonsoft.Json.

For unknown reason I forgot to manage devs to fix it on public APIs and webapps as well.

BTW: I try to avoid workarounds as much as possible in production code.. it's hard to maintain and remove it in future.

[arisewanggithub]

@khellang , it's not that a workaround won't work. It's just such a basic thing shouldn't need developers to add a workaround. As a big feature introduced for .NET core 3, it shouldn't lack such basic implementations.

[martincostello]

@arisewanggithub There are global settings available to controllers. You can configure it via AddJsonOptions(), or you can pass a JsonSerializerOptions instance to the Json() controller method.

[dev-thinks]

Is this closed cause we are having workaround?!

[khellang]

Is this closed cause we are having workaround?!

The issue is still open, pending an API/behavior proposal, review and implementation. In the meantime, it's pretty easy to work around by using the converter mentioned in https://github.com/dotnet/corefx/issues/38641#issuecomment-540200476.

[CodeBlanch]

For anyone blocked by this, here's a NuGet package with a converter (JsonTimeSpanConverter) we can use ahead of the 5.0 drop: Macross.Json.Extensions

Supports TimeSpan & Nullable<TimeSpan> types.

[YohDeadfall]

@eiriktsarpalis, as I wrote in the PR, I feel burned out and resigning therefore. It would be better to not prevent others from taking this.

[eiriktsarpalis]

Take care @YohDeadfall, and thank you for working on the feature.

[CodeBlanch]

@eiriktsarpalis @layomia I'm happy to pick this up if that would be helpful.

[eiriktsarpalis]

Thanks @CodeBlanch, I've assigned the issue to you. @YohDeadfall's PR #51492 would be a good starting point, in particular the conversation around the serialization format.

[CodeBlanch]

@eiriktsarpalis I just read through the thread, it sounds like we want to go with standard TimeSpan format instead of ISO8601 can you confirm that for me, and then I'll get started?

[mattjohnsonpint]

The issue with using the ISO860 period format is that it can represent values that don't fit cleanly into a TimeSpan. So if one had in their JSON something like "P3M" (3 months), the best one could do for a TimeSpan would be to use an average month duration, such as 3 months of 30 days each of exactly 24 hours long. Of course, such values wouldn't survive a round trip either, because it could come out as "P90D" or "P2160H", or a variety of other ways.

This is why in Noda Time there are separate Period and Duration types. Period uses the ISO8601 format and preserves the units of measure (even when ambiguous), while Duration resolves to a specific length of time and uses a format similar to TimeSpan's default string format (except it separates days with a : instead of .). (.NET doesn't yet have a Period type or anything similar.)

FWIW, my advice is indeed to align to TimeSpan's constant (c) format, [-][d.]hh:mm:ss[.fffffff] as described here.

[eiriktsarpalis]

Yes, I think we go for the standard TimeSpan format and aim for roundtrip compatibility with Json.NET. cc @JamesNK who might have opinions on the matter.

[eiriktsarpalis]

Following #53539 this feature will be adding one new API to facilitate source generation:

namespace System.Text.Json.Serialization.Metadata
{
    public static class JsonMetadataServices
    {
        public static System.Text.Json.Serialization.JsonConverter<System.TimeSpan> TimeSpanConverter { get; }
    }
}

[KevinCathcart]

This will be added to the netstandard2.0 builds too, right? (Since it can be, as TimeSpan is not a new type.) Thus the additional discussions on #53539 with respect to that won’t apply here?

[terrajobst]

Video

• We believe it's an acceptable breaking change even though we serialized TimeSpan before as a POCO. If developers don't want the new behavior, they can register their own TimeSpan converter. We may want to provide a "generic" converter that provides POCO style-serialization for any type (e.g. DefaultPropertyConverter<T>). The source generator is new, so it's not a breaking change there regardless.
  • @layomia could you file a separate issue for this?
• There is no way to configure the emitted format of the the time span (we emit it as days:hours:minute:seconds). We could add an API to configure this later, but we don't believe it's needed.

namespace System.Text.Json.Serialization.Metadata
{
    public static class JsonMetadataServices
    {
        public static JsonConverter<TimeSpan> TimeSpanConverter { get; }
    }
}