[
https://issues.apache.org/jira/browse/LANG-916?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14484761#comment-14484761
]
Christian P. MOMON edited comment on LANG-916 at 4/8/15 5:49 AM:
-----------------------------------------------------------------
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 hard code GregorianCalendar.
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.
was (Author: cpm):
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)
