You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Christian P. MOMON (JIRA)" <ji...@apache.org> on 2015/04/08 07:48:12 UTC

[jira] [Commented] (LANG-916) CLONE - DateFormatUtils.format does not correctly change Calendar TimeZone in certain situations

    [ https://issues.apache.org/jira/browse/LANG-916?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14484761#comment-14484761 ] 

Christian P. MOMON commented on LANG-916:
-----------------------------------------

Here are the results of my investigations.

A) Thomas Neidhart patch is really good.
I confirm that the patch from Thomas Neidhart is the good way. 
It is easy to verify:

FastDateFormat.getInstance("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'", TimeZone.getTimeZone("Asia/Kolkata")).format(cal)

1-> FastDateFormat.getInstance("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'", TimeZone.getTimeZone("Asia/Kolkata"))
2--> .format(cal) 
3---> return  printer.format(calendar)
4----> return format(calendar, new StringBuffer(this.mMaxLengthEstimate)).toString()
5-----> return applyRules(calendar, buf);

Step 1: store the TimeZone parameter in a FastDateFormat instance and build rules from the pattern to display.
Step 5: apply previously build rules to the parameter 'calendar'. So, there is no use of the TimeZone parameter stored in step 1 => **BUG**

In his patch, Thomas Neidhart calls the newCalendar(); method which build a new calendar using the TimeZone parameter stored in step 1. Then, this new calendar is used to apply rules.

It is exactly what it is done in other methods called "format" too but with a different parameter type (Date, long...). For each, there is a comment "// hard code GregorianCalendar".

The method "format" must build a new calendar with the timezone previously stored.


B) Test errors come from bad test.
Almost all test errors come from a flawed test. In FastDatePrinterTimeZonesTest.testCalendarTimezoneRespected, we can see:

final String actualValue = FastDateFormat.getInstance(PATTERN).format(cal);

As the TimeZone is not setted, then the TimeZone.getDefault() is set and so it is different than the original timezone. So the displayed result is not the same.

Fix: final String actualValue = FastDateFormat.getInstance(PATTERN, this.timeZone).format(cal);

It must also be true of other tests.


So, definitively, the patch from Thomas Neidhart is the good way.

I will try to provide patches for tests.

> CLONE - DateFormatUtils.format does not correctly change Calendar TimeZone in certain situations
> ------------------------------------------------------------------------------------------------
>
>                 Key: LANG-916
>                 URL: https://issues.apache.org/jira/browse/LANG-916
>             Project: Commons Lang
>          Issue Type: Bug
>          Components: lang.time.*
>    Affects Versions: 3.1
>         Environment: Sun JDK 1.6.0_45 and 1.7.0_21 on Fedora 17 (Linux 3.9.10-100.fc17.i686.PAE).
>            Reporter: Christian P. MOMON
>              Labels: patch, time
>             Fix For: Review Patch
>
>         Attachments: LANG-916.patch
>
>
> In LANG-538 issue, there is an unit test:
> {noformat}
>   public void testFormat_CalendarIsoMsZulu() {
>     final String dateTime = "2009-10-16T16:42:16.000Z";
>     GregorianCalendar cal = new GregorianCalendar(TimeZone.getTimeZone("GMT-8"));
>     cal.clear();
>     cal.set(2009, 9, 16, 8, 42, 16);
>     cal.getTime();
>     FastDateFormat format = FastDateFormat.getInstance("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'", TimeZone.getTimeZone("GMT"));
>     assertEquals("dateTime", dateTime, format.format(cal));
>   }
> {noformat}
> This test passes successfully in lang-2.6 but failed in lang3-3.1:
> {noformat}
> org.junit.ComparisonFailure: dateTime expected:<2009-10-16T[16]:42:16.000Z> but was:<2009-10-16T[08]:42:16.000Z>
> {noformat}
> Reproduced whit Sun Java version: 1.6.0_45 and 1.7.0_21 on Fedora 17 (Linux 3.9.10-100.fc17.i686.PAE).
> Moreover, I wrote another unit test showing that the timeZone parameter seems to be ignored :
> {noformat}
> public void test() {
> 	Calendar cal = Calendar.getInstance(TimeZone.getTimeZone("Europe/Paris"));
> 	cal.set(2009, 9, 16, 8, 42, 16);
> 	// System.out.println(DateFormatUtils.ISO_DATETIME_TIME_ZONE_FORMAT.format(cal));
> 	System.out.println("long");
> 	System.out.println(DateFormatUtils.format(cal.getTimeInMillis(), DateFormatUtils.ISO_DATETIME_TIME_ZONE_FORMAT.getPattern(), TimeZone.getDefault()));
> 	System.out.println(DateFormatUtils.format(cal.getTimeInMillis(), DateFormatUtils.ISO_DATETIME_TIME_ZONE_FORMAT.getPattern(),
> 			TimeZone.getTimeZone("Asia/Kolkata")));
> 	System.out.println(DateFormatUtils.format(cal.getTimeInMillis(), DateFormatUtils.ISO_DATETIME_TIME_ZONE_FORMAT.getPattern(),
> 			TimeZone.getTimeZone("Europe/London")));
> 	System.out.println("calendar");
> 	System.out.println(DateFormatUtils.format(cal, DateFormatUtils.ISO_DATETIME_TIME_ZONE_FORMAT.getPattern(), TimeZone.getDefault()));
> 	System.out.println(DateFormatUtils.format(cal, DateFormatUtils.ISO_DATETIME_TIME_ZONE_FORMAT.getPattern(), TimeZone.getTimeZone("Asia/Kolkata")));
> 	System.out.println(DateFormatUtils.format(cal, DateFormatUtils.ISO_DATETIME_TIME_ZONE_FORMAT.getPattern(), TimeZone.getTimeZone("Europe/London")));
> 	System.out.println("calendar fast");
> 	System.out.println(FastDateFormat.getInstance("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'", TimeZone.getTimeZone("Europe/Paris")).format(cal));
> 	System.out.println(FastDateFormat.getInstance("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'", TimeZone.getTimeZone("Asia/Kolkata")).format(cal));
> 	System.out.println(FastDateFormat.getInstance("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'", TimeZone.getTimeZone("Europe/London")).format(cal));
> }
> {noformat}
> Gives the following console logs:
> {noformat}
> long
> 2009-10-16T08:42:16+02:00
> 2009-10-16T12:12:16+05:30
> 2009-10-16T07:42:16+01:00
> calendar
> 2009-10-16T08:42:16+02:00
> 2009-10-16T08:42:16+02:00
> 2009-10-16T08:42:16+02:00
> calendar fast
> 2009-10-16T08:42:16.975Z
> 2009-10-16T08:42:16.975Z
> 2009-10-16T08:42:16.975Z
> {noformat}
> When DateFormatUtils.format takes a long parameter, the time string is good.
> When DateFormatUtils.format takes a Calendar parameter, the time string is wrong, the timezone parameter is IGNORED.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)