diff --git a/src/java/org/apache/commons/lang/time/DurationFormatUtils.java b/src/java/org/apache/commons/lang/time/DurationFormatUtils.java index ae55474f2..7f68080fa 100644 --- a/src/java/org/apache/commons/lang/time/DurationFormatUtils.java +++ b/src/java/org/apache/commons/lang/time/DurationFormatUtils.java @@ -273,11 +273,12 @@ public class DurationFormatUtils { public static String formatPeriod(long startMillis, long endMillis, String format, boolean padWithZeros, TimeZone timezone) { - long millis = endMillis - startMillis; - if (millis < 28 * DateUtils.MILLIS_PER_DAY) { - return formatDuration(millis, format, padWithZeros); - } - + // Used to optimise for differences under 28 days and + // called formatDuration(millis, format); however this did not work + // over leap years. + // TODO: Compare performance to see if anything was lost by + // losing this optimisation. + Token[] tokens = lexx(format); // timezones get funky around 0, so normalizing everything to GMT @@ -313,66 +314,73 @@ public class DurationFormatUtils { hours += 24; days -= 1; } - // TODO: Create a test to see if this should be while. ie) one that makes hours above - // overflow and pushes this above the maximum # of days in a month? - int leapDays = 0; - if (days < 0) { - days += start.getActualMaximum(Calendar.DAY_OF_MONTH); - // Multiple answers possible. - // For example, for Jan 15th to March 10th. If I count days-first it is - // 1 month and 26 days, but if I count month-first then it is 1 month and 23 days. - // Here we choose the former. - months -= 1; - start.add(Calendar.MONTH, 1); - } - while (months < 0) { - months += 12; - years -= 1; - if (start instanceof GregorianCalendar) { - if ( ((GregorianCalendar) start).isLeapYear(start.get(Calendar.YEAR) + 1) && - ( end.get(Calendar.MONTH) > 1) ) - { - leapDays += 1; - } + + if (Token.containsTokenWithValue(tokens, M)) { + while (days < 0) { + days += start.getActualMaximum(Calendar.DAY_OF_MONTH); + months -= 1; + start.add(Calendar.MONTH, 1); } - if (end instanceof GregorianCalendar) { - if ( ((GregorianCalendar) end).isLeapYear(end.get(Calendar.YEAR)) && - ( end.get(Calendar.MONTH) < 1) ) - { - leapDays -= 1; - } - } - start.add(Calendar.YEAR, 1); - } - // This rest of this code adds in values that - // aren't requested. This allows the user to ask for the - // number of months and get the real count and not just 0->11. - - if (!Token.containsTokenWithValue(tokens, y) && years != 0) { - if (Token.containsTokenWithValue(tokens, M)) { - months += 12 * years; - years = 0; - } else { - while ( (start.get(Calendar.YEAR) != end.get(Calendar.YEAR))) { - days += start.getActualMaximum(Calendar.DAY_OF_YEAR); - start.add(Calendar.YEAR, 1); + while (months < 0) { + months += 12; + years -= 1; + } + + if (!Token.containsTokenWithValue(tokens, y) && years != 0) { + while (years != 0) { + months += 12 * years; + years = 0; } + } + } else { + // there are no M's in the format string + + if( !Token.containsTokenWithValue(tokens, y) ) { + int target = end.get(Calendar.YEAR); + if (months < 0) { + // target is end-year -1 + target -= 1; + } + + while ( (start.get(Calendar.YEAR) != target)) { + days += start.getActualMaximum(Calendar.DAY_OF_YEAR) - start.get(Calendar.DAY_OF_YEAR); + + // Not sure I grok why this is needed, but the brutal tests show it is + if(start instanceof GregorianCalendar) { + if( (start.get(Calendar.MONTH) == Calendar.FEBRUARY) && + (start.get(Calendar.DAY_OF_MONTH) == 29 ) ) + { + days += 1; + } + } + + start.add(Calendar.YEAR, 1); + + days += start.get(Calendar.DAY_OF_YEAR); + } + years = 0; } - } - start.set(Calendar.YEAR, end.get(Calendar.YEAR)); - - if (!Token.containsTokenWithValue(tokens, M) && months != 0) { - while(start.get(Calendar.MONTH) != end.get(Calendar.MONTH)) { - String date = start.getTime().toString(); + + while( start.get(Calendar.MONTH) != end.get(Calendar.MONTH) ) { days += start.getActualMaximum(Calendar.DAY_OF_MONTH); start.add(Calendar.MONTH, 1); } - days += leapDays; + months = 0; + + while (days < 0) { + days += start.getActualMaximum(Calendar.DAY_OF_MONTH); + months -= 1; + start.add(Calendar.MONTH, 1); + } + } - start.set(Calendar.MONTH, end.get(Calendar.MONTH)); + + // The rest of this code adds in values that + // aren't requested. This allows the user to ask for the + // number of months and get the real count and not just 0->11. if (!Token.containsTokenWithValue(tokens, d)) { hours += 24 * days; diff --git a/src/test/org/apache/commons/lang/time/DurationFormatUtilsTest.java b/src/test/org/apache/commons/lang/time/DurationFormatUtilsTest.java index f4e317b84..f22466b2d 100644 --- a/src/test/org/apache/commons/lang/time/DurationFormatUtilsTest.java +++ b/src/test/org/apache/commons/lang/time/DurationFormatUtilsTest.java @@ -469,31 +469,77 @@ public class DurationFormatUtilsTest extends TestCase { assertEqualDuration( "365", new int[] { 2007, 2, 2, 0, 0, 0 }, new int[] { 2008, 2, 1, 0, 0, 0 }, "dd"); - // assertEqualDuration( "333", new int[] { 2007, 1, 2, 0, 0, 0 }, - // new int[] { 2008, 0, 1, 0, 0, 0 }, "dd"); + assertEqualDuration( "333", new int[] { 2007, 1, 2, 0, 0, 0 }, + new int[] { 2008, 0, 1, 0, 0, 0 }, "dd"); + + assertEqualDuration( "28", new int[] { 2008, 1, 2, 0, 0, 0 }, + new int[] { 2008, 2, 1, 0, 0, 0 }, "dd"); + assertEqualDuration( "393", new int[] { 2007, 1, 2, 0, 0, 0 }, + new int[] { 2008, 2, 1, 0, 0, 0 }, "dd"); + + assertEqualDuration( "369", new int[] { 2004, 0, 29, 0, 0, 0 }, + new int[] { 2005, 1, 1, 0, 0, 0 }, "dd"); + + assertEqualDuration( "338", new int[] { 2004, 1, 29, 0, 0, 0 }, + new int[] { 2005, 1, 1, 0, 0, 0 }, "dd"); + + assertEqualDuration( "28", new int[] { 2004, 2, 8, 0, 0, 0 }, + new int[] { 2004, 3, 5, 0, 0, 0 }, "dd"); + + assertEqualDuration( "48", new int[] { 1992, 1, 29, 0, 0, 0 }, + new int[] { 1996, 1, 29, 0, 0, 0 }, "M"); + + + // this seems odd - and will fail if I throw it in as a brute force + // below as it expects the answer to be 12. It's a tricky edge case + assertEqualDuration( "11", new int[] { 1996, 1, 29, 0, 0, 0 }, + new int[] { 1997, 1, 28, 0, 0, 0 }, "M"); + // again - this seems odd + assertEqualDuration( "11 28", new int[] { 1996, 1, 29, 0, 0, 0 }, + new int[] { 1997, 1, 28, 0, 0, 0 }, "M d"); + } public void testDurationsByBruteForce() { - bruteForce(2006, 0, 1); - bruteForce(2006, 0, 2); - // bruteForce(2007, 1, 2); + bruteForce(2006, 0, 1, "d", Calendar.DAY_OF_MONTH); + bruteForce(2006, 0, 2, "d", Calendar.DAY_OF_MONTH); + bruteForce(2007, 1, 2, "d", Calendar.DAY_OF_MONTH); + bruteForce(2004, 1, 29, "d", Calendar.DAY_OF_MONTH); + bruteForce(1996, 1, 29, "d", Calendar.DAY_OF_MONTH); + + bruteForce(1969, 1, 28, "M", Calendar.MONTH); // tests for 48 years + //bruteForce(1996, 1, 29, "M", Calendar.MONTH); // this will fail } - - private void bruteForce(int year, int month, int day) { + + private int FOUR_YEARS = 365 * 3 + 366; + + // Takes a minute to run, so generally turned off +// public void testBrutally() { +// Calendar c = Calendar.getInstance(); +// c.set(2004, 0, 1, 0, 0, 0); +// for (int i=0; i < FOUR_YEARS; i++) { +// bruteForce(c.get(Calendar.YEAR), c.get(Calendar.MONTH), c.get(Calendar.DAY_OF_MONTH), "d", Calendar.DAY_OF_MONTH ); +// c.add(Calendar.DAY_OF_MONTH, 1); +// } +// } + + private void bruteForce(int year, int month, int day, String format, int calendarType) { String msg = year + "-" + month + "-" + day + " to "; Calendar c = Calendar.getInstance(); c.set(year, month, day, 0, 0, 0); int[] array1 = new int[] { year, month, day, 0, 0, 0 }; int[] array2 = new int[] { year, month, day, 0, 0, 0 }; - for (int i=0; i < 1500; i++) { + for (int i=0; i < FOUR_YEARS; i++) { array2[0] = c.get(Calendar.YEAR); array2[1] = c.get(Calendar.MONTH); array2[2] = c.get(Calendar.DAY_OF_MONTH); String tmpMsg = msg + array2[0] + "-" + array2[1] + "-" + array2[2] + " at "; - assertEqualDuration( tmpMsg + i, Integer.toString(i), array1, array2, "d" ); - c.add(Calendar.DAY_OF_MONTH, 1); + assertEqualDuration( tmpMsg + i, Integer.toString(i), array1, array2, format ); + c.add(calendarType, 1); } } + + private void assertEqualDuration(String expected, int[] start, int[] end, String format) { assertEqualDuration(null, expected, start, end, format);