Skip to content
This repository has been archived by the owner on Jan 24, 2021. It is now read-only.

Improved ISO8601 support #1485

Merged
merged 5 commits into from
Mar 25, 2014
Merged

Conversation

dot-i
Copy link
Contributor

@dot-i dot-i commented Mar 9, 2014

Improved the ISO8601 format to use the round-trip format to supports milliseconds, timezones and UTC time.

Added three unit tests to make sure the serialization has the required result for all different kinds of datetimes (Local, UTC and Unspecficied) in both formats, ISO8601 and WCF.

Also made the ISO8601DateFormat locally overridable on the DefaultJsonSerializer, just like the RetainCasing JSON setting.

@dot-i dot-i mentioned this pull request Mar 9, 2014
@dot-i
Copy link
Contributor Author

dot-i commented Mar 9, 2014

OK, now I'm really confused... unit tests still fail on Travis, but universal time in timezone +0000 gives different milliseconds as UTC time during test-run. This is normally not possible, or it's getting too late and I'm not thinking clearly... :-(

This just demonstrates how hard it is to get this timezone stuff 100% right ;-)

@dot-i
Copy link
Contributor Author

dot-i commented Mar 9, 2014

OK, not too late yet -- figured out why my test was still wrong ;-) Hopefully fixing for real this time...

@jrsconfitto
Copy link
Contributor

i think it's a good idea to implement this change. Before using the better Date serialization in 0.22, i was using ToString("o") (the round-trip specifier) to serialize dates and i think this is a better format.

Also, MSDN says that the round-trip specifier is informed by ISO 8601 as well as the sortable specifier:

The pattern for this specifier reflects a defined standard (ISO 8601).

@dot-i
Copy link
Contributor Author

dot-i commented Mar 11, 2014

Let me add these examples of the new output to clarify the improvement.

Fact: I am currently in the GMT+1 timezone.

Output Description
2014-03-09T11:41:20.0719047+01:00 11:41 in my GMT+1 timezone
2014-03-09T11:41:20.0000000+00:00 11:41 in GMT timezone (UK)
2014-03-09T10:41:20.0719047Z 10:41 in UTC (aka 11:41 in my timezone)

This is what things used to look like before the changes in this pull request:

Output Description
2014-03-09T11:41:20 will be parsed OK for my timezone
2014-03-09T11:41:20 will be parsed incorrectly, should have given 12:41 for my timezone
2014-03-09T10:41:20 will be parsed incorrectly as it will appear as 10:41 Local time

@jrsconfitto
Copy link
Contributor

Another point i'd like to mention is that the round-trip specifier allows for more precision (AFAIK) than the one we're currently using.

@jrsconfitto
Copy link
Contributor

Alright, @dot-i already mentioned that up top! Cast my vote in for more precision, then.

@hlesesne
Copy link
Contributor

+1 I agree that this would be a welcome addition.


Assert.Equal(String.Format("{{\"createdOn\":\"2013-12-25T12:10:30.0000000{0}\"}}", GetTimezoneSuffix(DateTime.Now)), model);
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra blank line.. splitting hairs, but @thecodejunkie will shout if I pull it in with such things in there ;)

@grumpydev
Copy link
Member

Other than a random blank line it looks good to me 👍

@dot-i
Copy link
Contributor Author

dot-i commented Mar 25, 2014

I just fixed the blank line ;)

@grumpydev
Copy link
Member

Awesome 😁

@grumpydev grumpydev added this to the 0.23 milestone Mar 25, 2014
grumpydev added a commit that referenced this pull request Mar 25, 2014
@grumpydev grumpydev merged commit 3d8798d into NancyFx:master Mar 25, 2014
@dot-i dot-i deleted the finetune-iso8601-format branch March 25, 2014 20:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants