search

fizruk / http-api-data

Converting to/from HTTP API data like URL pieces, headers and query parameters.
http://hackage.haskell.org/package/http-api-data
Other
52 stars 42 forks source link

Make HttpApiData representation for ZonedTime consistent with aeson #41

Closed fizruk closed 7 years ago

fizruk commented 7 years ago

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
phadej commented 7 years ago

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 commented 7 years ago

(not sure what happened to unordered-containers)

EDIT: ah, it just moved up

fizruk commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

Judging by https://github.com/bos/aeson/pull/530 it looks like there's going to be attoparsec-iso8601 package. I guess we wait till then?

fizruk commented 7 years ago

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