Skip to content

Support HALF_DAY periodicity for AM/PM date patterns - #1051

Open
seonwooj0810 wants to merge 1 commit into
qos-ch:masterfrom
seonwooj0810:fix/issue-976-half-day-periodicity
Open

Support HALF_DAY periodicity for AM/PM date patterns#1051
seonwooj0810 wants to merge 1 commit into
qos-ch:masterfrom
seonwooj0810:fix/issue-976-half-day-periodicity

Conversation

@seonwooj0810

Copy link
Copy Markdown
Contributor

Fixes #976

Root cause

The HALF_DAY periodicity type and its RollingCalendar.printPeriodicity() message ("Roll-over at midday and midnight.") have existed since LBCORE-11 (2008), but HALF_DAY was never added to PeriodicityType.VALID_ORDERED_LIST, and RollingCalendar has no HALF_DAY branch in its switch statements. So computePeriodicityType() can never select it: a TimeBasedRollingPolicy whose date pattern carries the AM/PM token but no finer time token (e.g. %d{yyyy-MM-dd-a}) falls through to TOP_OF_DAY and rolls over only at midnight, logging "Roll-over at midnight." instead of rotating at both 00:00 and 12:00.

Change

  • PeriodicityType: insert HALF_DAY into VALID_ORDERED_LIST between TOP_OF_HOUR and TOP_OF_DAY. Because detection returns the shortest matching period, this only affects patterns that distinguish 00:00 from 12:00 but nothing finer (i.e. the a token) — patterns that already match at TOP_OF_HOUR or below are unchanged.
  • RollingCalendar.innerGetEndOfNextNthPeriod: add the HALF_DAY case — floor to the current half-day boundary (00:00 or 12:00) and advance in 12h steps, mirroring the existing TOP_OF_DAY flooring.
  • RollingCalendar.periodBarriersCrossed: add HALF_DAY (previously it hit the default branch and threw IllegalStateException).
  • RollingCalendar.isCollisionFree: add HALF_DAY — an isolated a without a date component repeats every 24h, so a 1-day collision check flags it.

Test evidence

Added regression coverage to RollingCalendarTest (all pass; each fails on current master before the fix):

  • testPeriodicity: yyyy-MM-dd-a and yyyy-MM-dd a now resolve to HALF_DAY (previously TOP_OF_DAY).
  • testVaryingNumberOfHalfDailyPeriods: getEndOfNextNthPeriod advances in exact 12h boundaries across a range of periods.
  • testBarrierCrossingComputation: 3 half-day barriers over 36h.
  • testCollisionFreenes: yyyy-MM-dd-a is collision-free; isolated a is not.

Verification done: mvn -pl logback-core -am test -Dtest=RollingCalendarTest → 10/10 pass. Ran the full ch.qos.logback.core.rolling.* suite: all rolling tests pass; the only errors are 3 pre-existing InaccessibleObjectExceptions in RollingFileAppenderTest (JPMS reflection into ch.qos.logback.core.appender), which are unrelated to this change and reproduce identically on a clean master checkout when the test JVM is JDK 25.

The HALF_DAY periodicity type and its printPeriodicity() message have
existed since LBCORE-11 (2008), but HALF_DAY was never added to
PeriodicityType.VALID_ORDERED_LIST and no HALF_DAY branch was wired into
RollingCalendar. As a result a TimeBasedRollingPolicy whose date pattern
carries the AM/PM token (e.g. %d{yyyy-MM-dd-a}) is detected as TOP_OF_DAY
and rolls over only at midnight instead of at 00:00 and 12:00.

Add HALF_DAY to the ordered detection list and implement the missing
HALF_DAY cases in innerGetEndOfNextNthPeriod (floor to the current
half-day boundary, advance in 12h steps), periodBarriersCrossed, and
isCollisionFree. Add regression tests to RollingCalendarTest.

Fixes qos-ch#976

Signed-off-by: seonwoo_jung <79202163+seonwooj0810@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TimeBasedRollingPolicy does not honor HALF_DAY periodicity for pattern %d{yyyy-MM-dd-a} — rolls over only at midnight instead of 00:00 and 12:00

1 participant