dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.27k stars 4.73k forks source link

System.Text.Json can't deserialize ISO dateTime to DateOnly #102594

Open marcinjahn opened 5 months ago

marcinjahn commented 5 months ago

Description

It looks like System.Text.Json cannot deserialize from ISO 8601 datetime string to DateOnly, which I find odd, since DateOnly.Parse("2024-05-01T00:00:00") works perfectly fine.

Reproduction Steps

Here's the code:

record MyRecord(DateOnly Date);

var serialized = "{ \"Date\": \"2025-05-01T00:00:00\" }";

System.Text.Json.JsonSerializer.Deserialize<MyRecord>(serialized); // throws

Expected behavior

I'd expect no exception to be thrown and an isntance of MyRecord to be created.

Actual behavior

An exception is thrown:

The JSON value could not be converted to Submission#1+MyRecord. Path: $.Date | LineNumber: 0 | BytePositionInLine: 31.

Regression?

No response

Known Workarounds

Use DateTime instead of DateOnly.

Configuration

dotnet: 8.0.204 OS: Fedora 40 arch: x64

Other information

No response

dotnet-policy-service[bot] commented 5 months ago

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis See info in area-owners.md if you want to be subscribed.

gregsdennis commented 5 months ago

Possible duplicate of #85545 (maybe just related)

eiriktsarpalis commented 5 months ago

I believe this was a deliberate decision for the built-in to avoid loss of information (same is true for TimeOnly). I wouldn't be opposed to a PR that extends this to full ISO dates, if you're willing to put up a PR.

cc @Jozkee

marcinjahn commented 5 months ago

I think it's expected that the time portion of the datetime string will be lost when you're deserializing to a DateOnly. It's kind of similar to how you might have a type:

record (int PropOne, int PropB);

and then you try to deserialize:

{
  "PropOne": 1,
  "PropTwo": 2,
  "PropThree": 3
}

The PropThree field gets lots, and it's expected.

I will try to prepare a PR for it. I will see if I manage to build it locally. If not, I guess I could rely on PR tests.

JaimeStill commented 4 months ago

I ran into this issue just yesterday and want to share my experience in hopes of being able to accommodate native deserialization of ISO strings to DateOnly properties. I concur with @marcinjahn that the expectation was that the time portion of the ISO string would be lost in the deserialization and that this behavior should be supported.

Our web apps are written in Angular and make use of the Angular Material library. Our forms use the Datepicker component with the native date adapter. All of this to say that the native date adapter does not support the ability to set the parse format, which would allow us to adjust selected dates to a format that could deserialize to DateOnly on the API side. The format that it provides by default is the ISO datetime string. I could use a different date adapter, but that would require adding a whole new unnecessary npm dependency.

See the note below the tables in Customizing the parse and display formats.

What I ultimately ended up doing is creating my own DateOnly converter as follows:

public class DateOnlyJsonConverter : JsonConverter<DateOnly>
{
    public override DateOnly Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        return DateOnly.FromDateTime(
            DateTime.Parse(
                reader.GetString()
                ?? DateTime.MinValue.ToString()
            )
        );
    }

    public override void Write(Utf8JsonWriter writer, DateOnly value, JsonSerializerOptions options)
    {
        writer.WriteStringValue(value.ToString());
    }
}
julealgon commented 4 months ago

@eiriktsarpalis

I believe this was a deliberate decision for the built-in to avoid loss of information

What about introducing a setting in the serializer options to allow or prevent loss of information then?

It seems to me that both scenarios could be valid: either someone actively wants for it to "just work" and trim the data, or they might actively want to block the behavior and make their API more strict.