From 55526f4a202da20a278fbbdf9a5a9bd59474e711 Mon Sep 17 00:00:00 2001 From: Sahana Bogar Date: Fri, 26 Jun 2026 11:49:18 +0530 Subject: [PATCH 1/2] compare weeks by day distance, not calendar year, in compare() AbstractCalendarValidator.compare ordered WEEK_OF_YEAR on Calendar.YEAR then the week number, but a week belongs to its own week-year, which at the year boundary differs from the calendar year (31 December can be week 1 of the following year). So compareWeeks reported 31 December and 1 January of the same calendar year as the same week, and treated two dates that share one week across the boundary as different weeks. WEEK_OF_MONTH resets in the same way at the start of a month. Order both week fields by the distance in days first: dates a week or more apart are always in different weeks, and nearer dates share a week only when the week number also matches. This drops the week-year lookup and its unsupported-calendar fallback. WEEK_OF_MONTH keeps its previous results; only the WEEK_OF_YEAR boundary case changes. Tests cover both fields across the year boundary. --- .../routines/AbstractCalendarValidator.java | 59 +++++++++++++++---- .../routines/CalendarValidatorTest.java | 46 +++++++++++++++ 2 files changed, 95 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/apache/commons/validator/routines/AbstractCalendarValidator.java b/src/main/java/org/apache/commons/validator/routines/AbstractCalendarValidator.java index a2b2e9a2b..a06694f3b 100644 --- a/src/main/java/org/apache/commons/validator/routines/AbstractCalendarValidator.java +++ b/src/main/java/org/apache/commons/validator/routines/AbstractCalendarValidator.java @@ -23,6 +23,7 @@ import java.util.Calendar; import java.util.Locale; import java.util.TimeZone; +import java.util.concurrent.TimeUnit; import org.apache.commons.validator.GenericValidator; @@ -38,6 +39,12 @@ public abstract class AbstractCalendarValidator extends AbstractFormatValidator private static final long serialVersionUID = -1410008585975827379L; + /** Number of days in a week, beyond which two dates cannot share a week. */ + private static final int DAYS_PER_WEEK = 7; + + /** Number of milliseconds in a day, used to convert an instant to a day count. */ + private static final long MILLIS_PER_DAY = TimeUnit.DAYS.toMillis(1); + /** * The date style to use for Locale validation. */ @@ -117,17 +124,20 @@ protected int compare(final Calendar value, final Calendar compare, final int fi int result; + // Week of Year and Week of Month numbers repeat across the boundaries they reset on, and a + // week can belong to a different calendar year or month than its number suggests (for + // example 31 December may fall in week 1 of the following year), so the week is compared by + // day distance and week number rather than by comparing the calendar year first. + if (field == Calendar.WEEK_OF_YEAR || field == Calendar.WEEK_OF_MONTH) { + return compareWeek(value, compare, field); + } + // Compare Year result = calculateCompareResult(value, compare, Calendar.YEAR); if (result != 0 || field == Calendar.YEAR) { return result; } - // Compare Week of Year - if (field == Calendar.WEEK_OF_YEAR) { - return calculateCompareResult(value, compare, Calendar.WEEK_OF_YEAR); - } - // Compare Day of the Year if (field == Calendar.DAY_OF_YEAR) { return calculateCompareResult(value, compare, Calendar.DAY_OF_YEAR); @@ -139,11 +149,6 @@ protected int compare(final Calendar value, final Calendar compare, final int fi return result; } - // Compare Week of Month - if (field == Calendar.WEEK_OF_MONTH) { - return calculateCompareResult(value, compare, Calendar.WEEK_OF_MONTH); - } - // Compare Date result = calculateCompareResult(value, compare, Calendar.DATE); if (result != 0 || field == Calendar.DATE || @@ -416,4 +421,38 @@ protected Object parse(String value, final String pattern, final Locale locale, */ @Override protected abstract Object processParsedValue(Object value, Format formatter); + + /** + * Compares the week two calendars fall in, ordering by the actual week rather than by the + * {@code WEEK_OF_YEAR} or {@code WEEK_OF_MONTH} number alone. Those numbers repeat across the + * boundaries they reset on (for example 31 December may be week 1 of the following year, and + * the first week of a month can hold days carried over from the previous month), so the day + * distance is checked first: dates a week or more apart are always in different weeks, and + * nearer dates share a week only when the week number also matches. + * + * @param value The Calendar value. + * @param compare The {@link Calendar} to check the value against. + * @param field {@code Calendar.WEEK_OF_YEAR} or {@code Calendar.WEEK_OF_MONTH}. + * @return Zero if both calendars are in the same week, -1 or +1 otherwise. + */ + private int compareWeek(final Calendar value, final Calendar compare, final int field) { + final long days = epochDay(value) - epochDay(compare); + if (Math.abs(days) >= DAYS_PER_WEEK || calculateCompareResult(value, compare, field) != 0) { + return Long.signum(days); + } + return 0; + } + + /** + * Returns the number of whole days from the epoch to the calendar's local date, so two + * calendars can be compared by date regardless of the time of day. + * + * @param calendar The calendar to convert. + * @return the day count from the epoch in the calendar's own time zone. + */ + private static long epochDay(final Calendar calendar) { + final long localMillis = calendar.getTimeInMillis() + + calendar.get(Calendar.ZONE_OFFSET) + calendar.get(Calendar.DST_OFFSET); + return Math.floorDiv(localMillis, MILLIS_PER_DAY); + } } diff --git a/src/test/java/org/apache/commons/validator/routines/CalendarValidatorTest.java b/src/test/java/org/apache/commons/validator/routines/CalendarValidatorTest.java index 2d835aff0..a28105aca 100644 --- a/src/test/java/org/apache/commons/validator/routines/CalendarValidatorTest.java +++ b/src/test/java/org/apache/commons/validator/routines/CalendarValidatorTest.java @@ -232,6 +232,52 @@ void testCompare() { assertEquals("Invalid field: -1", e.getMessage(), "check message"); } + /** + * Test compareWeeks() across the year boundary, where WEEK_OF_YEAR repeats: 31 December can be + * week 1 of the next year while 1 January of the same calendar year is also week 1. + */ + @Test + @DefaultLocale(country = "US", language = "en") + void testCompareWeeksAcrossYearBoundary() { + final int noon = 120000; + // US locale: week starts Sunday, so Sun 30 Dec 2018 to Sat 5 Jan 2019 is a single week + final Calendar dec30y2018 = createCalendar(TimeZones.GMT, 20181230, noon); + final Calendar dec31y2018 = createCalendar(TimeZones.GMT, 20181231, noon); + final Calendar jan01y2018 = createCalendar(TimeZones.GMT, 20180101, noon); + final Calendar jan01y2019 = createCalendar(TimeZones.GMT, 20190101, noon); + final Calendar jan05y2019 = createCalendar(TimeZones.GMT, 20190105, noon); + + // both are calendar year 2018 and WEEK_OF_YEAR 1, but lie ~52 weeks apart + assertEquals(1, calValidator.compareWeeks(dec31y2018, jan01y2018), "Dec 31 2018 is weeks after Jan 1 2018"); + assertEquals(-1, calValidator.compareWeeks(jan01y2018, dec31y2018), "Jan 1 2018 is weeks before Dec 31 2018"); + + // different calendar years but the same week + assertEquals(0, calValidator.compareWeeks(dec31y2018, jan01y2019), "Dec 31 2018 is the same week as Jan 1 2019"); + + // the whole Sun 30 Dec 2018 to Sat 5 Jan 2019 span is one week + assertEquals(0, calValidator.compareWeeks(dec30y2018, jan05y2019), "Dec 30 2018 is the same week as Jan 5 2019"); + } + + /** + * Test WEEK_OF_MONTH comparison across a month/year boundary, where the week number resets and + * the first week of a month can hold days carried over from the previous month. + */ + @Test + @DefaultLocale(country = "US", language = "en") + void testCompareWeekOfMonthAcrossBoundary() { + final int noon = 120000; + final Calendar dec30y2018 = createCalendar(TimeZones.GMT, 20181230, noon); + final Calendar dec31y2018 = createCalendar(TimeZones.GMT, 20181231, noon); + final Calendar jan01y2019 = createCalendar(TimeZones.GMT, 20190101, noon); + + // 30 and 31 Dec 2018 fall in the same week of December + assertEquals(0, calValidator.compare(dec30y2018, dec31y2018, Calendar.WEEK_OF_MONTH), "Dec 30 and 31 2018 are the same week of month"); + + // 31 Dec 2018 and 1 Jan 2019 are one day apart but lie in different weeks of their months + assertEquals(-1, calValidator.compare(dec31y2018, jan01y2019, Calendar.WEEK_OF_MONTH), "Dec 31 2018 is before Jan 1 2019 by week of month"); + assertEquals(1, calValidator.compare(jan01y2019, dec31y2018, Calendar.WEEK_OF_MONTH), "Jan 1 2019 is after Dec 31 2018 by week of month"); + } + /** * Test Date/Time style Validator (there isn't an implementation for this) */ From 827b27922afde910d95d2a9ed450db3869277dc5 Mon Sep 17 00:00:00 2001 From: Sahana Bogar Date: Sat, 27 Jun 2026 12:18:51 +0530 Subject: [PATCH 2/2] Compare week instants by milliseconds in compareWeek() --- .../routines/AbstractCalendarValidator.java | 32 +++++-------------- 1 file changed, 8 insertions(+), 24 deletions(-) diff --git a/src/main/java/org/apache/commons/validator/routines/AbstractCalendarValidator.java b/src/main/java/org/apache/commons/validator/routines/AbstractCalendarValidator.java index a06694f3b..be88d07be 100644 --- a/src/main/java/org/apache/commons/validator/routines/AbstractCalendarValidator.java +++ b/src/main/java/org/apache/commons/validator/routines/AbstractCalendarValidator.java @@ -39,11 +39,8 @@ public abstract class AbstractCalendarValidator extends AbstractFormatValidator private static final long serialVersionUID = -1410008585975827379L; - /** Number of days in a week, beyond which two dates cannot share a week. */ - private static final int DAYS_PER_WEEK = 7; - - /** Number of milliseconds in a day, used to convert an instant to a day count. */ - private static final long MILLIS_PER_DAY = TimeUnit.DAYS.toMillis(1); + /** Number of milliseconds in a week, beyond which two instants cannot share a week. */ + private static final long MILLIS_PER_WEEK = TimeUnit.DAYS.toMillis(7); /** * The date style to use for Locale validation. @@ -426,9 +423,9 @@ protected Object parse(String value, final String pattern, final Locale locale, * Compares the week two calendars fall in, ordering by the actual week rather than by the * {@code WEEK_OF_YEAR} or {@code WEEK_OF_MONTH} number alone. Those numbers repeat across the * boundaries they reset on (for example 31 December may be week 1 of the following year, and - * the first week of a month can hold days carried over from the previous month), so the day - * distance is checked first: dates a week or more apart are always in different weeks, and - * nearer dates share a week only when the week number also matches. + * the first week of a month can hold days carried over from the previous month), so the gap + * between the two instants is checked first: dates a week or more apart are always in different + * weeks, and nearer dates share a week only when the week number also matches. * * @param value The Calendar value. * @param compare The {@link Calendar} to check the value against. @@ -436,23 +433,10 @@ protected Object parse(String value, final String pattern, final Locale locale, * @return Zero if both calendars are in the same week, -1 or +1 otherwise. */ private int compareWeek(final Calendar value, final Calendar compare, final int field) { - final long days = epochDay(value) - epochDay(compare); - if (Math.abs(days) >= DAYS_PER_WEEK || calculateCompareResult(value, compare, field) != 0) { - return Long.signum(days); + final long millis = value.getTimeInMillis() - compare.getTimeInMillis(); + if (Math.abs(millis) >= MILLIS_PER_WEEK || calculateCompareResult(value, compare, field) != 0) { + return Long.signum(millis); } return 0; } - - /** - * Returns the number of whole days from the epoch to the calendar's local date, so two - * calendars can be compared by date regardless of the time of day. - * - * @param calendar The calendar to convert. - * @return the day count from the epoch in the calendar's own time zone. - */ - private static long epochDay(final Calendar calendar) { - final long localMillis = calendar.getTimeInMillis() - + calendar.get(Calendar.ZONE_OFFSET) + calendar.get(Calendar.DST_OFFSET); - return Math.floorDiv(localMillis, MILLIS_PER_DAY); - } }