rombert / ereviewboard

A mylyn-based Eclipse integration for Review Board
46 stars 31 forks source link

Problem parsing dates from Review Board Web API JSON responses #116

Closed hddm closed 11 years ago

hddm commented 11 years ago

Hi, We are using Review Board 1.7.1 and the Web API JSON responses contains dates in ISO8601 while the plugin expects the dates to be formatted in "yyyy-MM-dd HH:mm:ss". We did not find a way to change Review Board server settings to match the expected format. The solution we found is to modify org.review_board.ereviewboard.core.util.ReviewboardUtil.java and replace the content of ReviewboardUtil.marshallDate with the following: public static Date marshallDate(String time) { Calendar cal = javax.xml.bind.DatatypeConverter.parseDate(time); cal.setTimeZone(TimeZone.getTimeZone("GMT")); return cal.getTime(); }

What do you think of it?

Thanks.

Hanh

rombert commented 11 years ago

Hi,

Can you tell me how you got this format? Do you run the server in a non-GMT time zone?

Robert

On Mon, Jan 21, 2013 at 4:21 AM, hddm notifications@github.com wrote:

Hi, We are using Review Board 1.7.1 and the Web API JSON responses contains dates in ISO8601 while the plugin expects the dates to be formatted in "yyyy-MM-dd HH:mm:ss". We did not find a way to change Review Board server settings to match the expected format. The solution we found is to modify org.review_board.ereviewboard.core.util.ReviewboardUtil.java and replace the content of ReviewboardUtil.marshallDate with the following: public static Date marshallDate(String time) { Calendar cal = javax.xml.bind.DatatypeConverter.parseDate(time); cal.setTimeZone(TimeZone.getTimeZone("GMT")); return cal.getTime(); }

What do you think of it?

Thanks.

Hanh

— Reply to this email directly or view it on GitHubhttps://github.com/rombert/ereviewboard/issues/116.

Sent from my (old) computer

bu5hm4nn commented 11 years ago

Same here. I get this message:

Failed performing query : Unparseable date: "2013-01-18T08:12:56Z"

I'm running ReviewBoard 1.7.2 with MySQL on Ubuntu 12.04 LTS with Locale: de_DE.UTF-8

ubuntu:~$ date
Mo 21. Jan 13:44:27 CET 2013
rombert commented 11 years ago

This seems to be 1.7-specific. Did you run 1.6 before?

On Mon, Jan 21, 2013 at 2:41 PM, bu5hm4nn notifications@github.com wrote:

Same here. I get this message:

Failed performing query : Unparseable date: "2013-01-18T08:12:56Z"

I'm running ReviewBoad 1.7.1 with MySQL on Ubuntu 12.04 LTS with Locale: de_DE.UTF-8

— Reply to this email directly or view it on GitHubhttps://github.com/rombert/ereviewboard/issues/116#issuecomment-12496174.

Sent from my (old) computer

bu5hm4nn commented 11 years ago

No, I haven't tried 1.6. This is a very fresh 1.7.2 install. Could you give me a hint how to debug your plugin in Eclipse RCP?

hddm commented 11 years ago

As for us, we are using Bangkok time zone and we haven't tried 1.6 either. I noticed that the Review Board online demo (1.7) is also returning dates in ISO8601 format. Look at this request for example:

http://demo.reviewboard.org/api/review-requests/?status=pending&max-results=50&start=0

bu5hm4nn commented 11 years ago

This behavior propably was introduced to reviewboard in 1.7 by moving to Django > 1.4 as that introduced TimeZone handling. http://www.reviewboard.org/docs/releasenotes/dev/reviewboard/1.7/ https://docs.djangoproject.com/en/dev/releases/1.4/

rombert commented 11 years ago

Thanks for taking the time to report and investigate the cause, and even provide fixes (!) . Indeed, the date format has changed between 1.6 and 1.7 and I need to account for both. The bad part is that since our baseline is Java 1.5 I can't use the provided JAXB code - that's only available with Java 5. Until one of the upstream projects moves to Java 1.6 I'll keep 1.5.

I've also reported this problem upstream at 1 but I'm not holding my breath ; it's probably a change they will stick to. Since parsing ISO8601 dates is apparently hard with Java's SimpleDateFormat, I"ve committed a pretty hackish fix for this.

An update site archive should be available soon at https://ereviewboard.ci.cloudbees.com/job/eReviewBoard/114/org.review_board.ereviewboard$org.review_board.ereviewboard.update/ ( download the -assembly.zip file ). It'd be good if you could give it a test so that I can release a new version soonish.

bu5hm4nn commented 11 years ago

Thank YOU for maintaining this useful plugin! Forget what I wrote last night, your hack works wonderfully here. Thanks for your quick response.

rombert commented 11 years ago

Hm, I have a unit test which verifies this sort of format [1] so this should work ( currently fails due to TimeZone differences on the server ) so this should work.

If it doesn't, I'll have to hunt through the bundles provided by Orbit [2] and see if I can find one which helps. Joda Time is not there unfortunately.

[1]: https://github.com/rombert/ereviewboard/commit/a66b6910ae629dfd175dc6d7c7f196b4b8f41da2#L1R30 [2]: http://download.eclipse.org/tools/orbit/downloads/drops/R20120526062928/

On Wed, Jan 23, 2013 at 12:28 AM, bu5hm4nn notifications@github.com wrote:

Thank YOU for maintaining this useful plugin! I was trying a hack as well today (had some trouble figuring out how to build the plugin) and found out, that sometimes (after posting a review through ereviewboard for example) the webapi returns a date with milliseconds as well ("2013-01-18T08:12:56.231Z"). So your hack would fail in that case (of course one could remove everything after the "." as well). I searched the web and came up with http://joda-time.sourceforge.net/. I couldn't quite figure out how to package it as a plugin yet, but I think it might work.

— Reply to this email directly or view it on GitHubhttps://github.com/rombert/ereviewboard/issues/116#issuecomment-12570487.

Sent from my (old) computer

bu5hm4nn commented 11 years ago

I edited my comment above because it works ;-) Orbit has javax.xml.bind if that helps.

hddm commented 11 years ago

I also confirm that the Eclipse plugin from the -assembly.zip works. Thanks a lot for fixing this issue!