You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Dan Collens (JIRA)" <ji...@apache.org> on 2012/09/06 19:49:07 UTC

[jira] [Created] (LANG-818) FastDateFormat's "z" pattern does not respect timezone of Calendar instances passed to format()

Dan Collens created LANG-818:
--------------------------------

             Summary: FastDateFormat's "z" pattern does not respect timezone of Calendar instances passed to format()
                 Key: LANG-818
                 URL: https://issues.apache.org/jira/browse/LANG-818
             Project: Commons Lang
          Issue Type: Bug
          Components: lang.time.*
    Affects Versions: 3.x
            Reporter: Dan Collens


The work on LANG-462 has introduced a time zone formatting bug in FastDateFormat in commons-lang3.

The problem can be seen by this snippet:
{code}
// Always prints timezone name of machine's default timezone, ignoring TZ
// set on calendar, even though the printed time itself respects calendar's TZ.
Calendar myCal = Calendar.getInstance(TimeZone.getTimeZone("US/Central"));
System.out.println(FastDateFormat.getInstance("h:mma z").format(myCal));
{code}

If you happen to be in US/Central, this will print the right thing, but just try it with US/Eastern, US/Pacific, etc.  It will print the time in the correct timezone, but the timezone name at the end (the "z" pattern) will always be the system default timezone.  This is a regression against commons-lang 2.x.

Basically, when the "forced time zone" code was removed, the TimeZoneNameRule class stopped respecting the Calendar instance's timezone, and instead now always uses the mTimeZone of the FastDateFormat instance itself (which is only supposed to be used when formatting timezone-less objects such as Date or long).

The removal of the forced time zone stuff is surely the right thing to do (it was a mess).  I think the fix is to change the TimeZoneNameRule inner class to not take a TimeZone instance, but rather to use the TimeZone on the Calendar instance passed into appendTo(), just like TimeZoneNumberRule does.  Presumably then for efficiency, one would use the getTimeZoneDisplay() package-static method to quickly retrieve the required timezone's display name.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Resolved] (LANG-818) FastDateFormat's "z" pattern does not respect timezone of Calendar instances passed to format()

Posted by "Sebb (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/LANG-818?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Sebb resolved LANG-818.
-----------------------

       Resolution: Fixed
    Fix Version/s: 3.2

Thanks for the code patch and test case.
Test case fails before code patch is applied and succeeds afterwards (as it should!).
Applied:

URL: http://svn.apache.org/viewvc?rev=1390980&view=rev
Log:
LANG-818 FastDateFormat's "z" pattern does not respect timezone of Calendar instances passed to format()

Modified:
    commons/proper/lang/trunk/src/changes/changes.xml
    commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/time/FastDatePrinter.java
    commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/time/FastDatePrinterTest.java

                
> FastDateFormat's "z" pattern does not respect timezone of Calendar instances passed to format()
> -----------------------------------------------------------------------------------------------
>
>                 Key: LANG-818
>                 URL: https://issues.apache.org/jira/browse/LANG-818
>             Project: Commons Lang
>          Issue Type: Bug
>          Components: lang.time.*
>    Affects Versions: 3.x
>            Reporter: Dan Collens
>             Fix For: 3.2
>
>         Attachments: commons-lang3-LANG-818.patch
>
>
> The work on LANG-462 has introduced a time zone formatting bug in FastDateFormat in commons-lang3.
> The problem can be seen by this snippet:
> {code}
> // Always prints timezone name of machine's default timezone, ignoring TZ
> // set on calendar, even though the printed time itself respects calendar's TZ.
> Calendar myCal = Calendar.getInstance(TimeZone.getTimeZone("US/Central"));
> System.out.println(FastDateFormat.getInstance("h:mma z").format(myCal));
> {code}
> If you happen to be in US/Central, this will print the right thing, but just try it with US/Eastern, US/Pacific, etc.  It will print the time in the correct timezone, but the timezone name at the end (the "z" pattern) will always be the system default timezone.  This is a regression against commons-lang 2.x.
> Basically, when the "forced time zone" code was removed, the TimeZoneNameRule class stopped respecting the Calendar instance's timezone, and instead now always uses the mTimeZone of the FastDateFormat instance itself (which is only supposed to be used when formatting timezone-less objects such as Date or long).
> The removal of the forced time zone stuff is surely the right thing to do (it was a mess).  I think the fix is to change the TimeZoneNameRule inner class to not take a TimeZone instance, but rather to use the TimeZone on the Calendar instance passed into appendTo(), just like TimeZoneNumberRule does.  Presumably then for efficiency, one would use the getTimeZoneDisplay() package-static method to quickly retrieve the required timezone's display name.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Updated] (LANG-818) FastDateFormat's "z" pattern does not respect timezone of Calendar instances passed to format()

Posted by "Duncan Jones (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/LANG-818?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Duncan Jones updated LANG-818:
------------------------------

    Attachment: commons-lang3-LANG-818.patch

Slightly tweaked the patch - in the test case I had imported assertNotNull from a different package to the other static imports.
                
> FastDateFormat's "z" pattern does not respect timezone of Calendar instances passed to format()
> -----------------------------------------------------------------------------------------------
>
>                 Key: LANG-818
>                 URL: https://issues.apache.org/jira/browse/LANG-818
>             Project: Commons Lang
>          Issue Type: Bug
>          Components: lang.time.*
>    Affects Versions: 3.x
>            Reporter: Dan Collens
>         Attachments: commons-lang3-LANG-818.patch
>
>
> The work on LANG-462 has introduced a time zone formatting bug in FastDateFormat in commons-lang3.
> The problem can be seen by this snippet:
> {code}
> // Always prints timezone name of machine's default timezone, ignoring TZ
> // set on calendar, even though the printed time itself respects calendar's TZ.
> Calendar myCal = Calendar.getInstance(TimeZone.getTimeZone("US/Central"));
> System.out.println(FastDateFormat.getInstance("h:mma z").format(myCal));
> {code}
> If you happen to be in US/Central, this will print the right thing, but just try it with US/Eastern, US/Pacific, etc.  It will print the time in the correct timezone, but the timezone name at the end (the "z" pattern) will always be the system default timezone.  This is a regression against commons-lang 2.x.
> Basically, when the "forced time zone" code was removed, the TimeZoneNameRule class stopped respecting the Calendar instance's timezone, and instead now always uses the mTimeZone of the FastDateFormat instance itself (which is only supposed to be used when formatting timezone-less objects such as Date or long).
> The removal of the forced time zone stuff is surely the right thing to do (it was a mess).  I think the fix is to change the TimeZoneNameRule inner class to not take a TimeZone instance, but rather to use the TimeZone on the Calendar instance passed into appendTo(), just like TimeZoneNumberRule does.  Presumably then for efficiency, one would use the getTimeZoneDisplay() package-static method to quickly retrieve the required timezone's display name.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (LANG-818) FastDateFormat's "z" pattern does not respect timezone of Calendar instances passed to format()

Posted by "Dan Collens (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/LANG-818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13457058#comment-13457058 ] 

Dan Collens commented on LANG-818:
----------------------------------

That patch looks good to me.
                
> FastDateFormat's "z" pattern does not respect timezone of Calendar instances passed to format()
> -----------------------------------------------------------------------------------------------
>
>                 Key: LANG-818
>                 URL: https://issues.apache.org/jira/browse/LANG-818
>             Project: Commons Lang
>          Issue Type: Bug
>          Components: lang.time.*
>    Affects Versions: 3.x
>            Reporter: Dan Collens
>         Attachments: commons-lang3-LANG-818.patch
>
>
> The work on LANG-462 has introduced a time zone formatting bug in FastDateFormat in commons-lang3.
> The problem can be seen by this snippet:
> {code}
> // Always prints timezone name of machine's default timezone, ignoring TZ
> // set on calendar, even though the printed time itself respects calendar's TZ.
> Calendar myCal = Calendar.getInstance(TimeZone.getTimeZone("US/Central"));
> System.out.println(FastDateFormat.getInstance("h:mma z").format(myCal));
> {code}
> If you happen to be in US/Central, this will print the right thing, but just try it with US/Eastern, US/Pacific, etc.  It will print the time in the correct timezone, but the timezone name at the end (the "z" pattern) will always be the system default timezone.  This is a regression against commons-lang 2.x.
> Basically, when the "forced time zone" code was removed, the TimeZoneNameRule class stopped respecting the Calendar instance's timezone, and instead now always uses the mTimeZone of the FastDateFormat instance itself (which is only supposed to be used when formatting timezone-less objects such as Date or long).
> The removal of the forced time zone stuff is surely the right thing to do (it was a mess).  I think the fix is to change the TimeZoneNameRule inner class to not take a TimeZone instance, but rather to use the TimeZone on the Calendar instance passed into appendTo(), just like TimeZoneNumberRule does.  Presumably then for efficiency, one would use the getTimeZoneDisplay() package-static method to quickly retrieve the required timezone's display name.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Updated] (LANG-818) FastDateFormat's "z" pattern does not respect timezone of Calendar instances passed to format()

Posted by "Duncan Jones (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/LANG-818?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Duncan Jones updated LANG-818:
------------------------------

    Attachment: commons-lang3-LANG-818.patch

I've attached a patch that I believe addresses this issue, plus a unit test to prove correctness.

The only minor issue is that the estimated length provided by TimeZoneNameRule.estimateLength() is now based on the wrong timezone. I can't see a way to avoid this.

Perhaps it would be better to artificially increase the returned estimated length value on the basis that over-provisioning a StringBuffer is better than under-provisioning?
                
> FastDateFormat's "z" pattern does not respect timezone of Calendar instances passed to format()
> -----------------------------------------------------------------------------------------------
>
>                 Key: LANG-818
>                 URL: https://issues.apache.org/jira/browse/LANG-818
>             Project: Commons Lang
>          Issue Type: Bug
>          Components: lang.time.*
>    Affects Versions: 3.x
>            Reporter: Dan Collens
>         Attachments: commons-lang3-LANG-818.patch
>
>
> The work on LANG-462 has introduced a time zone formatting bug in FastDateFormat in commons-lang3.
> The problem can be seen by this snippet:
> {code}
> // Always prints timezone name of machine's default timezone, ignoring TZ
> // set on calendar, even though the printed time itself respects calendar's TZ.
> Calendar myCal = Calendar.getInstance(TimeZone.getTimeZone("US/Central"));
> System.out.println(FastDateFormat.getInstance("h:mma z").format(myCal));
> {code}
> If you happen to be in US/Central, this will print the right thing, but just try it with US/Eastern, US/Pacific, etc.  It will print the time in the correct timezone, but the timezone name at the end (the "z" pattern) will always be the system default timezone.  This is a regression against commons-lang 2.x.
> Basically, when the "forced time zone" code was removed, the TimeZoneNameRule class stopped respecting the Calendar instance's timezone, and instead now always uses the mTimeZone of the FastDateFormat instance itself (which is only supposed to be used when formatting timezone-less objects such as Date or long).
> The removal of the forced time zone stuff is surely the right thing to do (it was a mess).  I think the fix is to change the TimeZoneNameRule inner class to not take a TimeZone instance, but rather to use the TimeZone on the Calendar instance passed into appendTo(), just like TimeZoneNumberRule does.  Presumably then for efficiency, one would use the getTimeZoneDisplay() package-static method to quickly retrieve the required timezone's display name.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Updated] (LANG-818) FastDateFormat's "z" pattern does not respect timezone of Calendar instances passed to format()

Posted by "Duncan Jones (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/LANG-818?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Duncan Jones updated LANG-818:
------------------------------

    Attachment:     (was: commons-lang3-LANG-818.patch)
    
> FastDateFormat's "z" pattern does not respect timezone of Calendar instances passed to format()
> -----------------------------------------------------------------------------------------------
>
>                 Key: LANG-818
>                 URL: https://issues.apache.org/jira/browse/LANG-818
>             Project: Commons Lang
>          Issue Type: Bug
>          Components: lang.time.*
>    Affects Versions: 3.x
>            Reporter: Dan Collens
>
> The work on LANG-462 has introduced a time zone formatting bug in FastDateFormat in commons-lang3.
> The problem can be seen by this snippet:
> {code}
> // Always prints timezone name of machine's default timezone, ignoring TZ
> // set on calendar, even though the printed time itself respects calendar's TZ.
> Calendar myCal = Calendar.getInstance(TimeZone.getTimeZone("US/Central"));
> System.out.println(FastDateFormat.getInstance("h:mma z").format(myCal));
> {code}
> If you happen to be in US/Central, this will print the right thing, but just try it with US/Eastern, US/Pacific, etc.  It will print the time in the correct timezone, but the timezone name at the end (the "z" pattern) will always be the system default timezone.  This is a regression against commons-lang 2.x.
> Basically, when the "forced time zone" code was removed, the TimeZoneNameRule class stopped respecting the Calendar instance's timezone, and instead now always uses the mTimeZone of the FastDateFormat instance itself (which is only supposed to be used when formatting timezone-less objects such as Date or long).
> The removal of the forced time zone stuff is surely the right thing to do (it was a mess).  I think the fix is to change the TimeZoneNameRule inner class to not take a TimeZone instance, but rather to use the TimeZone on the Calendar instance passed into appendTo(), just like TimeZoneNumberRule does.  Presumably then for efficiency, one would use the getTimeZoneDisplay() package-static method to quickly retrieve the required timezone's display name.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira