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 default DateTimeZoneHandling #1566

Open
btecu opened this issue Oct 1, 2019 · 27 comments
Open

System.Text.Json default DateTimeZoneHandling #1566

btecu opened this issue Oct 1, 2019 · 27 comments
Labels
area-System.Text.Json wishlist Issue we would like to prioritize, but we can't commit we will get to it yet
Milestone

Comments

@btecu
Copy link

btecu commented Oct 1, 2019

In JSON.NET you can:

JsonConvert.DefaultSettings = () => new JsonSerializerSettings {
    DateTimeZoneHandling = DateTimeZoneHandling.Utc
};

Is there anything similar in System.Text.Json?
If there is not an option to set the default, can it be passed in manually?

Also, what is the default (Local, Utc, Unspecified)?

@steveharter
Copy link
Member

cc @layomia on dates

Since a DateTime can be set in each of those formats (e.g. with and without the timezone\offset), the reader\writer (and thus the serializer) uses what is available and uses\assumes an ISO 8601 format to represent that.

@layomia
Copy link
Contributor

layomia commented Nov 8, 2019

@btecu the ISO 8601 profile is used by default in the serializer, e.g. 2019-07-26T16:59:57-05:00.

For deserializing, if the datetime offset is given as "hh:mm", a DateTime with DateTimeKind.Local is created. "Z" will give DateTimeKind.Utc. If no offset is given, then a DateTime with DateTimeKind.Unspecified is created.
The reverse mapping applies to serialization: writing a DateTime with DateTimeKind.Local will yield an ISO representation with an "hh:mm"-formatted offset, and so on.

For more on DateTime and DateTimeOffset support in System.Text.Json, and how you can implement custom parsing or formatting, see https://docs.microsoft.com/en-us/dotnet/standard/datetime/system-text-json-support.

@layomia layomia changed the title System.Text.Json default date handling System.Text.Json default DateTimeZoneHandling Nov 23, 2019
@ericstj ericstj transferred this issue from dotnet/corefx Jan 9, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Text.Json untriaged New issue has not been triaged by the area owner labels Jan 9, 2020
@ericstj ericstj added this to To do in System.Text.Json - 6.0 Jan 9, 2020
@ahsonkhan ahsonkhan removed the untriaged New issue has not been triaged by the area owner label Feb 20, 2020
@ahsonkhan ahsonkhan added this to the Future milestone Feb 20, 2020
@layomia layomia removed this from To do in System.Text.Json - 6.0 Feb 20, 2020
@mpashkovskiy
Copy link

@btecu unfortunately the only way, I think, to have the similar option in System.Text.Json is to do:

services.AddControllers()
    .AddJsonOptions(options =>
     {
         options.JsonSerializerOptions.Converters.Add(new DateTimeConverter());
     });

and implement DateTimeConverter like that

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

    public override void Write(Utf8JsonWriter writer, DateTime value, JsonSerializerOptions options)
    {
         writer.WriteStringValue(value.ToUniversalTime().ToString("yyyy'-'MM'-'dd'T'HH':'mm':'ssZ"));
    }
}

@dalle
Copy link

dalle commented Dec 15, 2020

This extends @mpashkovskiy solution and relies on the functionality in Utf8JsonReader/Utf8JsonWriter for actual parsing/formatting.

public class DateTimeConverter : JsonConverter<DateTime>
{
    public override DateTime Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        return reader.GetDateTime().ToUniversalTime();
    }

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

@danilobreda
Copy link

danilobreda commented Feb 2, 2021

This solution should come natively so that we have the best possible performance. JsonConverter creates problems when poorly implemented.
Sometimes we can't control which Kind of datetime because of its origin being an ORM for example ... needing a .ToUniversalTime() just to satisfy json deserealization by adding a +00:00 or "Z"
This solution should be reviewed. :(

@amay5027
Copy link

amay5027 commented May 6, 2021

Just adding to @mpashkovskiy solution, you could use the SpecifyKind method in .Net to do this as follows:

    public override DateTime Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        return DateTime.Parse(reader.GetString());
    }

    public override void Write(Utf8JsonWriter writer, DateTime value, JsonSerializerOptions options)
    {
        writer.WriteStringValue(DateTime.SpecifyKind(value, DateTimeKind.Utc));
    }

@maulik-modi
Copy link

maulik-modi commented Jun 18, 2021

@davidfowl and @JamesNK , what do you guys recommend here? We are building up new project using System.Text.json, want to ensure we use the right guidance and get benefit from system.text.json.

@ysbakker
Copy link

Just adding to @mpashkovskiy solution, you could use the SpecifyKind method in .Net to do this as follows:

    public override DateTime Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        return DateTime.Parse(reader.GetString());
    }

    public override void Write(Utf8JsonWriter writer, DateTime value, JsonSerializerOptions options)
    {
        writer.WriteStringValue(DateTime.SpecifyKind(value, DateTimeKind.Utc));
    }

Exactly what I was looking for! I save all DateTime as UTC in the database because I have different timezones in development and production. I then parse the UTC time to local time in the client, but by default the json serializer didn't append a "Z" so I had to do some ugly string concatenation to make it work. This solves that issue by specifying that the date is already in UTC and will therefore return a nice ISO string that javascript also understands. The issue with using DateTime.ToUniversalTime() in my case is that it assumes I save the timestamps in my local timezone.

@maulik-modi
Copy link

@ysbakker , javascript is not sending string ending with "Z"?

@ysbakker
Copy link

@ysbakker , javascript is not sending string ending with "Z"?

Date.prototype.toISOString() creates an ISO-compliant string, so this includes a "Z". You can try (new Date()).toISOString(), it should automatically convert to UTC too.

@xenod
Copy link

xenod commented Jun 29, 2021

Is there still no built in feature for forcing deserializer to add 'Z' to the end? I also need this feature.

We used this in Json.NET:

jsonSerializerSettings.DateTimeZoneHandling = DateTimeZoneHandling.Utc; //Make sure that Json dates are formatted with a 'Z' on the end! jsonSerializerSettings.DateFormatHandling = DateFormatHandling.IsoDateFormat;

@backnotprop
Copy link

public override DateTime Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
return DateTime.Parse(reader.GetString());
}

public override void Write(Utf8JsonWriter writer, DateTime value, JsonSerializerOptions options)
{
    writer.WriteStringValue(DateTime.SpecifyKind(value, DateTimeKind.Utc));
}

Same, except it was @dalle's solution that worked for me. This has been a world of pain for a globally distributed system.

@maulik-modi
Copy link

maulik-modi commented Aug 25, 2021

@backnotprop , @dalle , @amay5027 and @davidfowl ,

We decided not to use JsonConverter and instead pass DateTime as string for two reasons:

  1. We want to provide friendly message in case date time format is incorrect, we expect ISO8601 format for all incoming datetime inputs
  2. return DateTime.Parse(reader.GetString()); provides datetime in local timezone, we instead store UTC DateTime

Our solution was to wrap in an extension method:

static class DateTimeExtensions
{
const string ISO8601DateTimeSecondsFormat = "yyyy-MM-ddTHH:mm:ssZ";
public static DateTime? ToDateTime(this string eventDateTime)
{
if(DateTime.TryParseExact(eventDateTime, ISO8601DateTimeSecondsFormat,
                                             CultureInfo.InvariantCulture,
                                             DateTimeStyles.RoundtripKind,
                                             out DateTime dateTimeWithSeconds))
  return dateTimeWithSeconds;
else
 return null;
}

Your thoughts?

@killerwife
Copy link

Also stumbled upon this issue, since every other system expects and ISO string in json. This does sound like an unnecessary hack.

@maulik-modi
Copy link

@rbhanda , Do we have an option to set this globally like Json.NET in .NET 6 ? Can you add it to backlog of .NET 7 otherwise?

JsonConvert.DefaultSettings = () => new JsonSerializerSettings {
    DateTimeZoneHandling = DateTimeZoneHandling.Utc
};

@seekingtheoptimal
Copy link

In almost every typical client-server web system the server uses and stores dates in UTC (default behavior for SQL DateTime mapping with EF Core for example), while browsers should show the local timezone as users are from all around the internet. Having this feature in a similar fashion to json.net - even if not making it the default behavior to break things - would just make a lot of sense.

@eiriktsarpalis
Copy link
Member

@amay5027 Note that the DateTime.SpecifyKind method does not convert the date to the new kind, it will simply swap the DateTimeKind and keep the same ticks value, which is not what most users would expect (it's certainly not how DateTimeZoneHandling works in Json.NET). I would recommend using the workaround as proposed in #1566 (comment) instead.

This solution should come natively so that we have the best possible performance. JsonConverter creates problems when poorly implemented.

@danilobreda what are the performance issues you are identified with the proposed workarounds? It would seem to me that the solution proposed in #1566 (comment) would be as fast as a native feature.

Also stumbled upon this issue, since every other system expects and ISO string in json. This does sound like an unnecessary hack.

@killerwife the default DateTime serialization uses ISO 8601. I believe the ask here is whether we should introduce a feature that automatically converts the TZ offsets on the serialization layer.

@eiriktsarpalis eiriktsarpalis added the wishlist Issue we would like to prioritize, but we can't commit we will get to it yet label Oct 26, 2021
@danilobreda
Copy link

danilobreda commented Oct 26, 2021

@eiriktsarpalis I won't know what the performance issues are, and that's the idea. Using a solution that comes natively doesn't bring that kind of questioning.
This was already solved by newtonsoft, a configuration should exist for which way the conversion should take place. For UTC Always, or use Kind of property (default). #1566 (comment)

@eiriktsarpalis
Copy link
Member

I won't know what the performance issues are, and that's the idea. Using a solution that comes natively doesn't bring that kind of questioning.

I personally wouldn't agree with this line of thinking. Ultimately not every requested feature will find its way as an OOTB feature (or it may take multiple releases before it does). As such I wholeheartedly encourage building successful third-party extension libraries (there are quite a few quality ones already out there). As a maintainer I would prioritize addressing extensibility concerns in the core infrastructure over complete feature parity with other offerings.

Bottom line in this case, I would argue that the custom converter proposed here as a workaround would be just as fast (and likely faster) than a configurable built-in converter.

This was already solved by newtonsoft, a configuration should exist for which way the conversion should take place. For UTC Always, or use Kind of property (default).

I believe that the Newtonsoft offering might simply reflect the fact that earlier versions of the library didn't use ISO 8601 when serializing dates. This has been the only supported format in STJ from day 1, and as such any TZ information is always accurately represented in the underlying JSON.

I guess I'm trying to better understand the motivating use cases, and I can only think of the following:

  1. The application uses DateTime instead of DateTimeOffset when representing dates, resulting in bugs when comparing dates of different kinds.
  2. The application uses DateTime.Now instead of DateTime.UtcNow and the host TZ itself is not UTC, resulting in local TZ offsets leaking in server responses.

Am I forgetting something?

@danilobreda
Copy link

@eiriktsarpalis Applications that used JSON.NET with DateTimeZoneHandling = DateTimeZoneHandling.Utc and migrated to System.Text.Json.

@AndiRudi
Copy link

This is a really annoying thing when you want to upgrade to .net 6 because the default de-serialiser of a view model will add the timezone given by the frontend. If you forward this to a Postgres database you will get an error like Cannot write DateTimeOffset with Offset=02:00:00 to PostgreSQL type .... Or am I missing something?

@eithe
Copy link

eithe commented Sep 7, 2022

Edit: Please do not use this, see replies from @eiriktsarpalis and @OskarKlintrot below.

For anyone who uses DateTime?, here is a nullable variant of @dalle's solution:

public class NullableDateTimeConverter : JsonConverter<DateTime?>
{
    public override DateTime? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        return reader.TryGetDateTime(out var value) ? value.ToUniversalTime() : null;
    }

    public override void Write(Utf8JsonWriter writer, DateTime? value, JsonSerializerOptions options)
    {
        if (value.HasValue)
        {
            writer.WriteStringValue(value.Value.ToUniversalTime());
        }
        else
        {
            writer.WriteNullValue();
        }
    }
}

@eiriktsarpalis
Copy link
Member

For anyone who uses DateTime?

Writing dedicated converters for nullable types is not recommended. Authoring a custom converter for the underlying struct should suffice, the generic nullable converter will compose with that.

@OskarKlintrot
Copy link

And you can access the other converters from inside your converter (to avoid doing the convertion yourself if there's already a perfectly good converter at your disposal once you done with type checks or whatever you want to do):

options.GetConverter(typeof(DateTime))

@eithe
Copy link

eithe commented Sep 7, 2022

Thanks @eiriktsarpalis and @OskarKlintrot, will edit.

@HarelM
Copy link

HarelM commented Jan 29, 2023

Would be great to have some build it converter in .Net for these kind of scenarios.
I'm trying to migrate my project from Newtonsoft.JSON to system.text.json and a lot of places are failing, some related to this, some related to other issues.
I don't want to write a generic datetime converter that can parse a date well.
I have an external API that is written in python that has a date of the format: 2021-08-30 23:26:46 for example. Previously it just worked.
When I migrated to STJ this started failing. While I get the performance advantages of assuming everything is ISO, I still think there should be an easy way to say: "hey, converter, this date isn't well formatted, but let's make sure you are able to parse it" without writing a converter that was already written hundreds of times...

@Astral100
Copy link

If the goal is to get pure local time (by stripping timezone part from the datetime), then one solution I found is to change the contract of the received object properties to DateTimeOffset as described here (in EDIT2 part): https://stackoverflow.com/questions/78450474/how-to-strip-off-timezone-part-from-datetime-to-leave-only-local-datetime-part/78450521?noredirect=1#comment138306952_78450521

And then you could use .DateTime on the DateTimeOffset property.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Text.Json wishlist Issue we would like to prioritize, but we can't commit we will get to it yet
Projects
None yet
Development

No branches or pull requests