Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Quartz-style Nth Day of Week cron expressions can overflow to other month #34360

Closed
RingoDev opened this issue Feb 3, 2025 · 2 comments
Closed
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: backported An issue that has been backported to maintenance branches type: bug A general bug
Milestone

Comments

@RingoDev
Copy link

RingoDev commented Feb 3, 2025

The issue

We are experiencing a weird issue with Cron Expressions. We are using the @Scheduled annotation to run scheduled jobs every monday.

We have a use case where we want to run a schedule differently if it is the first Monday of the month, which is why we split our schedules up into 2 CRON expressions, using the "day of week" notation (documented here)

  1. 0 0 8 ? * MON#1 - run every first Monday of the month
  2. 0 0 8 ? * MON#2,MON#3,MON#4,MON#5 - run on all other Mondays of the month

This worked fine until we observed weird behavior today on Monday the 3rd February 2025. The second CRON expression fired even though it should not fire on the first Monday of a month.

I run a test for the next 10000 invocations and these are the 10 dates that this unexpected overlap will happen next:

[
2025-02-03T08:00Z (java.time.OffsetDateTime),
2026-02-02T08:00Z (java.time.OffsetDateTime),
2027-02-01T08:00Z (java.time.OffsetDateTime),
2030-02-04T08:00Z (java.time.OffsetDateTime),
2031-02-03T08:00Z (java.time.OffsetDateTime),
2037-02-02T08:00Z (java.time.OffsetDateTime),
2038-02-01T08:00Z (java.time.OffsetDateTime),
2041-02-04T08:00Z (java.time.OffsetDateTime),
2042-02-03T08:00Z (java.time.OffsetDateTime),
2043-02-02T08:00Z (java.time.OffsetDateTime),
2047-02-04T08:00Z (java.time.OffsetDateTime),
]

Conclusion

It seems as if the day of week expression (e.g. MON#5) overflows only from January to February in the years where January has less than 5 Mondays. (This is why 31.1.2028 is not up there for example).

Reproduction

This can be reproduced with spring-context-6.2.2 with following test case:

package com.dynatrace.security.vulnerabilityreminderservice.scheduler;

import org.junit.jupiter.api.Test;
import org.springframework.scheduling.support.CronExpression;

import java.time.OffsetDateTime;
import java.time.ZoneOffset;
import java.util.ArrayList;
import java.util.List;

import static org.assertj.core.api.Assertions.assertThat;

public class CronExpressionTest {

   @Test
   public void ensureNoOverlap() {
       int numberOfInvocations = 10000;

       var seed = OffsetDateTime.of(2025, 1, 1, 0, 0, 0, 0, ZoneOffset.UTC);

       var monthly = CronExpression.parse("0 0 8 ? * MON#1");
       var weekly = CronExpression.parse("0 0 8 ? * MON#2,MON#3,MON#4,MON#5");

       var nextMonthlyInvocations = getNextInvocationsFromSeed(numberOfInvocations, monthly, seed);
       var nextWeeklyInvocations = getNextInvocationsFromSeed(numberOfInvocations, weekly, seed);


       List<OffsetDateTime> matches = new ArrayList<>();
       // Assert that there are no overlaps between the next invocations
       for (var invocation : nextWeeklyInvocations) {
           if (nextMonthlyInvocations.contains(invocation)) {
               matches.add(invocation);
           }

       }
       assertThat(matches).isEmpty();
   }

   private List<OffsetDateTime> getNextInvocationsFromSeed(int numberOfInvocations, CronExpression expression, OffsetDateTime seed) {
       var next = seed;
       List<OffsetDateTime> nextInvocations = new ArrayList<>();
       for (int i = 0; i < numberOfInvocations; i++) {
           next = expression.next(next);
           nextInvocations.add(next);
       }
       return nextInvocations;
   }
}
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 3, 2025
@bclozel bclozel added the in: core Issues in core modules (aop, beans, core, context, expression) label Feb 4, 2025
@RingoDev
Copy link
Author

RingoDev commented Feb 4, 2025

I created a simplified test case that fits into the spring-context/src/test/java/org/springframework/scheduling/support/CronExpressionTests.java test class. This test should pass in my opinion, but doesn't:

@Test
void fifthOccurrenceOfDayOfWeekInMonth() {
	CronExpression expression = CronExpression.parse("0 0 0 ? * MON#5");

	LocalDateTime last = LocalDateTime.of(2025, 1, 1, 0, 0, 0);

        // first occurrence of 5 mondays in a month from last
	LocalDateTime expected = LocalDateTime.of(2025, 3, 31, 0, 0, 0);
	LocalDateTime actual = expression.next(last);
	assertThat(actual).isNotNull();
	assertThat(actual).isEqualTo(expected);
	assertThat(actual.getDayOfWeek()).isEqualTo(MONDAY);
}

@bclozel bclozel added the type: bug A general bug label Feb 4, 2025
@bclozel bclozel added this to the 6.2.x milestone Feb 4, 2025
@snicoll snicoll removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 5, 2025
@bclozel bclozel self-assigned this Feb 6, 2025
@bclozel bclozel changed the title Day of Week miscalculation in scheduling.support.CronExpression Quartz-style Nth Day of Week cron expressions can overflow to other month Feb 6, 2025
@bclozel bclozel added the for: backport-to-6.1.x Marks an issue as a candidate for backport to 6.1.x label Feb 6, 2025
@github-actions github-actions bot added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-6.1.x Marks an issue as a candidate for backport to 6.1.x labels Feb 6, 2025
@bclozel bclozel modified the milestones: 6.2.x, 6.2.3 Feb 6, 2025
@bclozel bclozel closed this as completed in 174d0e4 Feb 6, 2025
@bclozel
Copy link
Member

bclozel commented Feb 6, 2025

Thanks for the detailed test case @RingoDev , this helped a lot and I've pushed a fix for the next 6.2.x and 6.1.x maintenance releases due next week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: backported An issue that has been backported to maintenance branches type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants