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

Make HttpApiData representation for ZonedTime consistent with aeson #41

Closed
fizruk opened this issue Dec 27, 2016 · 6 comments · Fixed by #53
Closed

Make HttpApiData representation for ZonedTime consistent with aeson #41

fizruk opened this issue Dec 27, 2016 · 6 comments · Fixed by #53
Labels

Comments

@fizruk
Copy link
Owner

fizruk commented Dec 27, 2016

Currently they encode slightly differently:

>>> zt :: ZonedTime
2016-12-31 01:00:00 +0000
>>> encode zt
"\"2016-12-31T01:00:00Z\""
>>> toQueryParam zt
"2016-12-31T01:00:00+0000"

And parseQueryParam can't decode aeson's variant:

>>> decode "\"2016-12-31T01:00:00Z\"" :: Maybe ZonedTime
Just 2016-12-31 01:00:00 +0000
>>> parseQueryParamMaybe "2016-12-31T01:00:00Z" :: Maybe ZonedTime
Nothing

While aeson can decode http-api-data variant:

>>> decode "\"2016-12-31T01:00:00+0000\"" :: Maybe ZonedTime
Just 2016-12-31 01:00:00 +0000
>>> parseQueryParamMaybe "2016-12-31T01:00:00+0000" :: Maybe ZonedTime
Just 2016-12-31 01:00:00 +0000

aeson also can decode +hh suffix (when there's no minutes offset):

>>> decode "\"2016-12-31T01:00:00+03\"" :: Maybe ZonedTime
Just 2016-12-31 01:00:00 +0300
>>> parseQueryParamMaybe "2016-12-31T01:00:00+03" :: Maybe ZonedTime
Nothing
@fizruk fizruk added the bug label Dec 27, 2016
@phadej
Copy link
Collaborator

phadej commented Jan 13, 2017

Would be nice to use http://hackage.haskell.org/package/time-parsers, it brings

--- old.deps	2017-01-13 11:13:55.005936844 +0200
+++ new.deps	2017-01-13 11:13:23.069778531 +0200
@@ -33,7 +33,11 @@
 UnitID "scientific-0.3.4.10-f39ad0aeb8547e08817b75b25d9d0aaff566f77a5436cdf7bf03ae89623d9fcd"
 UnitID "attoparsec-0.13.1.0-6afbd013b7fa7b9a37edf5d02f39849114773c370e49f06d78698cd720e65bc1"
 UnitID "base-compat-0.9.1-e8fbf792eb6b9aced2eebeeb3f65d3c31b75f8e34a9a6f93fe27a21c8db60903"
+UnitID "base-orphans-0.5.4-221596738ac95aba5b36b0b0362492907162ea1af94cf8f335da4c27a8bef6e9"
 UnitID "blaze-builder-0.4.0.2-8f01425b3efd14a168afedf0708436daddf067f3a72c01f5acd56c526bf9f3fb"
+UnitID "semigroups-0.18.2-819b431f6fdb68d2f106b52bdc71dfc6238084c22c6859c4ef083ecf0fecdc72"
+UnitID "unordered-containers-0.2.7.2-6c218f0d131a613b23028e9da022362405619eacbe95b866e121e37c100af565"
+UnitID "charset-0.3.7.1-5a9dc2144743ee760e3adb90efeddc08741db708aa89c6cb57b574cfc6361b39"
 UnitID "cryptohash-md5-0.11.100.1-ffa13147105f188ef8822a15eecbf5020998892385f21b714bf638447070763e"
 UnitID "cryptohash-sha1-0.11.100.1-46edb95630d8a667378fb335fd7e74c3cb861c21c9830da185e201d9442b295b"
 UnitID "ghc-boot-8.0.1"
@@ -56,9 +60,12 @@
 UnitID "hspec-discover-2.3.2-c542bdb36178610b3e28a623226922d75a26ea1d72f79160c86b667f4ed9d17c"
 UnitID "hspec-2.3.2-bcd8185e78713a27a2e839454247305cf32c068d6576aba43f513298eb388f5d"
 UnitID "hspec-discover-2.3.2-12b60877cc4e02e77cf25b18158fdd4d72efc9ba65672cc3fc6aa7e0ef2fdd8e"
-UnitID "unordered-containers-0.2.7.2-6c218f0d131a613b23028e9da022362405619eacbe95b866e121e37c100af565"
 UnitID "quickcheck-instances-0.3.12-5426b51228fdf1d0715340c6dd85944cb5a534ee67f9151a7c2bc2be251f9aa3"
 UnitID "time-locale-compat-0.1.1.3-253f9cf0c042c095c3ca556456d14a271bafd7895e757aa784f30be1f225c8c9"
+UnitID "mtl-2.2.1-189152f9e249689c10662b8c73dfcee93b11c6711307beb4138946627c3531be"
+UnitID "parsec-3.1.11-db8940de22deb4d6b5e7cd5e0a12e9cd288a104666210b5e75722156bfad8653"
+UnitID "parsers-0.12.4-172130ec44b1745ee0b3f1a7d7fdb6f8f267f8007d6e8c20522cc297c0d4c47c"
+UnitID "time-parsers-0.1.2.0-b69c870a68f84ad0a0178208fd4f964992fd18548f37d828c2e046706decd519"
 UnitID "uri-bytestring-0.2.2.1-03f740070db1b84f62c234eca7e9c084aff052086000f1ec72597c51600c00d6"
 UnitID "network-info-0.2.0.8-0d7b62b336f3ca432f9a9c41e8b66457e3b3edd07b84365f27de01acbd40c397"
 UnitID "uuid-types-1.0.3-87ae4f0665f418a39cebac091ba5908d463c9cd30a47a979f761dbafb397d938"

@phadej
Copy link
Collaborator

phadej commented Jan 13, 2017

(not sure what happened to unordered-containers)

EDIT: ah, it just moved up

@fizruk
Copy link
Owner Author

fizruk commented Jan 13, 2017

I personally am ok with it, but I think this library is used in web frameworks that don't generally like extra deps (because they already have a lot of those). So far we have dependencies that are there anyway for at least major web frameworks.

It seems that parsec and parsers would break that and will hence introduce these extra dependencies down the line.

@phadej
Copy link
Collaborator

phadej commented Jan 13, 2017

Looks like snap depends on parsec thru heist and xmlhtml. But that's true that e.g. wai-extra transitive dependency closure doesn't include parsec.

Would be great not to copy-paste those parsers around for different parsers, but may I have just eat it and
write time-parsers-attoparsec?

@fizruk
Copy link
Owner Author

fizruk commented Apr 10, 2017

Judging by haskell/aeson#530 it looks like there's going to be attoparsec-iso8601 package.
I guess we wait till then?

@fizruk
Copy link
Owner Author

fizruk commented Apr 20, 2017

Now that attoparsec-iso8601 is out, we can finally fix this issue (and #48)!

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

Successfully merging a pull request may close this issue.

2 participants