[
https://issues.apache.org/jira/browse/LANG-1791?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18033046#comment-18033046
]
Igor Rodchenkov commented on LANG-1791:
---------------------------------------
[~ggregory] , thank you for looking into this.
Please correct me if I didn't understand. You mean the expected behavior (my
test above) in 3.0.1 was in fact a bug, but in 3.18.0 (or at some point after
3.0.1) it's "fixed", thus now breaks existing code, right? So the test from the
Jira description will keep failing with all next 3.x.x lib versions (so we'd
better switch to e.g. java.time instead commons-lang3 and Calendar, refactor
our legacy code, rather than expect FastDateFormat works as it did in 3.0.1)?
I saw your commit on Github. You basically fixed the unit test instead of
fixing the bug in new FastDateFormat. I know that it works as expected when you
use the second arg here: FastDateFormat.getInstance(pattern, TimeZones.GMT);
But this issue was all about existing code where it's
FastDateFormat.getInstance(pattern); ans so the Calendar's time zone (GMT in my
case) should be used (which worked fine in 3.0.1 but broke after...)
Also, please look closely at this part (in FastDatePrinter) -
{code:java}
// do not pass in calendar directly, this will cause TimeZone of
FastDatePrinter to be ignored
if (!calendar.getTimeZone().equals(timeZone)) {
calendar = (Calendar) calendar.clone();
calendar.setTimeZone(timeZone);
} {code}
which did not exist in 3.0.1 and it is obviously wrong - {*}comment says one
thing but the code block does exactly opposite{*}! Basically, we now always use
the Formatter's/Printer's timeZone (even if null) here and ignore Calendar's
one (unless these two zones were the same).
So, what should be the decision here? Are we going to keep current v3.18.0,
v3.19.0 behavior (completely ignores Calendar's time zone, regardless) or fix
it and restore backward compatibility with 3.0.1? Did we indeed wanted to beak
commons-lang3 lib backward compatibility after 3.0.1 without bumping the major
version (bad practice)?
> FastDateFormat behavior changed between 3.0.1 and e.g. 3.18.0 and later
> -----------------------------------------------------------------------
>
> Key: LANG-1791
> URL: https://issues.apache.org/jira/browse/LANG-1791
> Project: Commons Lang
> Issue Type: Bug
> Environment: Linux (Ubuntu 24.04), temurin-17.0.16 JDK, run in
> Toronto.
> Reporter: Igor Rodchenkov
> Priority: Major
>
> Lately, we discovered a bug which is better demonstrate with this test case
> (it's not perfect because always passes in GMT zone, but e.g. in Toronto it
> passes when commons-lang3 v3.0.1 used and fails when v3.18.0 or 3.19.0 is
> used; and yes, in this special case, we have somewhat legacy java code using
> Calendar etc., but that was what broke in our app by this library upgrade,
> which supoosed to be backward compatible):
>
>
> {code:java}
> /*
> * org.apache.commons.lang3.time.FastDateFormat
> * In dateFormatter.format(timeCal), when time was not set explicitly, would
> make e.g.
> * the following timestamps in Toronto at 10 am on 2025-09-17:
> * "2025091714" - GMT, as expected, when using commons-lang3 v3.0.1
> * "2025091710" - EDT, when using commons-lang3 v3.18.0 or 3.19.0 (likely some
> other versions <3.18.0 too, but I did not test that)
> */
> @Test
> public void fastDateFormatFormatterUsingCalendarShouldMakeGmtTimestamp() {
>
> Instant now = Instant.now();
> ZoneId zoneId = ZoneId.systemDefault(); //e.g. America/Toronto
> ZoneOffset offset = zoneId.getRules().getOffset(now);
> System.out.printf("Current time: %s, zone: %s, offset: %sh\n", now, zoneId,
> offset);
> //some legacy code that should still work the same after commons-lang3
> minor ver. upgrade but it does not:
> FastDateFormat dateFormatter = FastDateFormat.getInstance("yyyyMMddHH");
> TimeZone timeZone = TimeZone.getTimeZone("GMT");
> Calendar timeCal = Calendar.getInstance(timeZone);
> String timestamp = dateFormatter.format(timeCal); //makes local zone
> timestamp when commons-lang3 v3.18.0, 3.19.0 is used (ignores Calendar's
> zone).
> //expected GMT timestamp
> String expected =
> DateTimeFormatter.ofPattern("yyyyMMddHH").withZone(ZoneId.of("GMT")).format(now);
> assertEquals(expected, timestamp);
> }{code}
>
> I think the bug is in 3.18.0 the FastDatePrinter here -
> {code:java}
> @Override
> public <B extends Appendable> B format(Calendar calendar, final B buf) {
> // do not pass in calendar directly, this will cause TimeZone of
> FastDatePrinter to be ignored
> if (!calendar.getTimeZone().equals(timeZone)) {
> calendar = (Calendar) calendar.clone();
> calendar.setTimeZone(timeZone);
> }
> return applyRules(calendar, buf);
> }{code}
> - note misleading comment, - it's calendar's timeZone is in fact there
> ignored instead of date printer's timeZone!
> - we cannot require this "do{color:#808080} not pass in calendar
> directly{color}" nor change the behavior here, not in v3.x.x (perhaps in
> v4.x.x - aye), according to Semantic versioning rules.
>
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
