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

DateTimeOffset ignores offset during deserialisation, uses LOCAL TIME ZONE offset #235

Closed
GengeT opened this issue Apr 3, 2017 · 21 comments

Comments

@GengeT
Copy link

GengeT commented Apr 3, 2017

We have the following poco in document Db, used to split out a date into JSON ticks and the ISO8601 date;

"When": { "Date": "2017-04-03T10:57:56+01:00", "Ticks": 1491213476000 },

The Date field uses a direct set to an internal field on the GET, simple stuff.

However, when reading the document using the DocumentClient.CreateDocumentQuery it deserialises the date time offset without the offset, but instead uses the LOCAL TIME ZONE of the server to set the offset from UTC. Completely mind boggling.

Why the hell aren't you guys just doing a DateTimeOffset.Parse, or even better just use JSON.NET!

@GengeT
Copy link
Author

GengeT commented Apr 3, 2017

It looks like this is a problem on the boundary of Json.NET and your SDK.

JsonConverter.DefaultSettings are being ignored when set in code, so you must have an explicit set of settings going into your JsonConvert.

If I deserialize from the document file locally there isn't a problem (with the correct JsonConverter default settings defined). However, DocumentClient.CreateDocumentQuery doesn't appear to respect the defaults set for JsonConverter.

@GengeT
Copy link
Author

GengeT commented Apr 3, 2017

Sure, I could go to the ballache of storing a TimeZoneInfo and changing this now UTC date to the correct time zone, but that's processing I don't need to do if all I want to do is show the source data local time. That's already stored in the datetimeoffset... its kind of the point. If I was displaying in a TZ not of the source data then indeed I would expect to use a TimeZoneInfo (for example, if the user wants to view it in their time zone).

@GengeT
Copy link
Author

GengeT commented Apr 4, 2017

Well, that's very odd. It now seems to be deserialising correctly, with no change to our code.

@GengeT GengeT closed this as completed Apr 4, 2017
@GengeT
Copy link
Author

GengeT commented Apr 4, 2017

So we did actually implement a TimeZoneInfo.ConvertTime using the sites time in the view model, which fixed it in our web UI. This is still as issue for DocumentDb however; DateTimeOffset should be treated as a DateTimeOffset.

@GengeT GengeT reopened this Apr 4, 2017
@GengeT
Copy link
Author

GengeT commented Apr 10, 2017

The bizarre thing is why has the community not made a big stink about this? Is no one else using DateTimeOffset? Is no one else doing anything remotely complex or date based (possibly due to poor Range based partition support) in DocumentDb? Maybe no one is actually using DocumentDb?

Just completely perplexing, and yet REALLY EASY to fix. Just get the DateTimeOffset deserialisation using the JsonConvert to NOT specify Json settings, so it inherits the default global settings.

@arramac
Copy link
Contributor

arramac commented Apr 13, 2017

@jackbond, @GengeT

I'm from the DocumentDB team. Sincere apologies for the delay in responding and any gaps in communication. Let me explain what's happening here, our plans for addressing this, and how you can workaround this issue.

DocumentDB uses JSON.net for serialization and deserialization. Normally, JSON.net preserves the TimeZone offset when you do this across the conversion. However, when you are working with dynamic objects, JSON.net decides on whether a string is DateTime, DateTimeOffset, or string based on your serializer settings. By default, when a string parses to a DateTime or a DateTimeOffset, it chooses DateTime - which means that the offset information is dropped. See snippet below with just JSON.net:

public class Event
{
    [JsonProperty("id")]
    public String Id { get; set; }

    [JsonProperty("ts")]
    public DateTimeOffset TS { get; set; }
}

Event myEvent = new Event { Id = "Zero", TS = new DateTimeOffset(2017, 1, 1, 0, 0, 0, TimeSpan.Zero) };
string json = JsonConvert.SerializeObject(myEvent);


// You have the value of TS in local time
Event newEvent2 = JsonConvert.DeserializeObject<Event>(json);

// You have the correct value of TS with the UTC offset
Event newEvent = JsonConvert.DeserializeObject<Event>(json, new JsonSerializerSettings { DateParseHandling = DateParseHandling.DateTimeOffset });

Since DocumentDB works with any object type, internally all Documents are parsed as dynamic with the default Json serializer settings, which means that it is treated as DateTime. Note this is only on deserialization, the data as stored in DocumentDB will have the original timestamp with Offset. This is not mutated or changed by DocumentDB (you can verify via Fiddler). The change you see is only on read.

We are actively working on a change that will allow users to plug in custom Json serializer settings to the DocumentDB client. Since this is a fairly large change since DocumentDB uses JSON conversion in a number of places including metadata for collections and databases, unfortunately we cannot provide a stop-gap hotfix for DateTimeOffset parsing.

However, there are a couple of workarounds we had suggested to your questions - 1. storing the DateTimeOffset as a string and 2. implementing a custom JsonConverter that ensures the offsets are deserialized as you want it.

Thanks again for using DocumentDB. And once again, sorry about the delay. Please email us at askdocdb@microsoft.com, and we can discuss in more detail over a call and help you with a solution.

@GengeT
Copy link
Author

GengeT commented Apr 13, 2017

Thanks @arramac. We've had to implement our own date wrapper to handle storing as JSON / Unix based ticks, to allow range lookups anyway. The actual DateTimeOffset is stored as a property of this wrapper; so we updated this today to store offset under another property, used in conjunction with the Ticks when reading into our code. It does the job for now.

Re: the fix, it seems to me you could just set the JsonSerializerSettings to parse the date time strings as DateTimeOffset, using "RoundTrip" as opposed to "Local"? Any chump still using DateTime in their data model (rather than DateTimeOffset) deserves everything they get!

We do love DocumentDb; just so easy to scale, takes away a lot of the pain of storage as long as the data is suitable (unstructured, high volume). And OMG TimeToLive is a real time saver / worker maintenance saver. The new auto-partition / 250GB collection abstraction is cool (rather expensive) but a bit limited for most data models, unless you have clear partition key candidates which are hashes (and not ranges).

@GengeT GengeT closed this as completed Apr 13, 2017
@arramac
Copy link
Contributor

arramac commented Apr 13, 2017

@GengeT glad you have a workaround, and happy to hear you link DocumentDB :)

We did consider changing the default to DateTimeOffset, but as you can imagine, it's a change that can break existing applications (the SDK is downloaded ~900k times on Nuget), so we want to be careful and address in a backwards-compatible way.

Hash is more elastic and supports high throughput because we can distribute data evenly, and this evenness + parallelism generally outweighs the benefits of range partitioning. You can also devise hybrid schemes where a bucket of range values hash to a single value. Happy to chat offline about partition key schemes and how you can map between range and hash partitioning.

@GengeT
Copy link
Author

GengeT commented Aug 8, 2017

@jackbond I wonder if you and I are the only ones who use DocumentDb! Or at least use it for anything more exciting that very simple data models.

@ealsur
Copy link
Member

ealsur commented Aug 8, 2017

@jackbond The SDK version changes are updated on the SDK docs pages. This is the one for .Net Core and this for .Net Full Framework.

Hope it helps 😄

@pcoppney
Copy link

pcoppney commented Dec 29, 2017

@arramac Has there been any progress on this?

Creating a property like this
[JsonProperty(PropertyName = "createDateTime")]
public string CreateDateTime { get; set; }

And populating like this
DateTimeOffset startDate = new DateTimeOffset(2000, 5, 15, 12, 0, 0, new TimeSpan(0, 0, 0));
CreateDateTime = startDate.ToString(CultureInfo.InvariantCulture);

results in the document client saving this
"createDateTime": "05/15/2000 12:00:00 +00:00"

Which is something like the ISO format, but not really. If I send in a string with the datetimeoffset nothing is saved.

This
CreateDateTime = startDate.ToUnixTimeSeconds().ToString();

is saved as
"createDateTime": "958392000"

Which is a serviceable work around, but using Unix constructs in C# feels a bit less than optimal.

@arramac
Copy link
Contributor

arramac commented Dec 29, 2017

@pcoppney custom JsonSerializerSettings is supported as of SDK 1.16.0. Can you please try setting that when you read/write these objects. You need something like: new JsonSerializerSettings { DateParseHandling = DateParseHandling.DateTimeOffset })

@pcoppney
Copy link

pcoppney commented Jan 3, 2018

@arramac I am using .net core and version 1.7.1 of Microsoft.Azure.DocumentDB.Core. Which SDK are you talking about?

@jcageman
Copy link

jcageman commented May 8, 2019

I have had this issue now on multiple places in azure... azure functions, cosmos.. Why is the datetimeoffset problem not being taken seriously? Everytime the problem is in the json serializers..

@pvanroos
Copy link

pvanroos commented Jan 3, 2020

What is the workaround for Microsoft.Azure.Cosmos lib v3.5.1? As everyone has mentioned, it serializes in Cosmos correctly, it's only when you pull the document back that you get it as a DateTime.

@JohnLTaylor
Copy link

Have you tried the following:
JsonConvert.DefaultSettings = () => new JsonSerializerSettings
{
DateFormatHandling = DateFormatHandling.IsoDateFormat,
DateFormatString = "yyyy'-'MM'-'dd'T'HH':'mm':'ss.fffffffK",
DateParseHandling = DateParseHandling.DateTimeOffset
};

This should work and without the DateParseHandling it will not parse correctly

@dluc
Copy link
Member

dluc commented Mar 20, 2020

I experienced the same issue today. The issue has been reported 3 years ago... please fix this, e.g. make UTC the default behavior, allowing to switch to locale on the view logic.

For now this workaround is helping (global settings could be easily conflict with other application areas though, so take it with caution)

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants