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

Support TimeUnit in the @Scheduled annotation #27309

Closed
wants to merge 11 commits into from
Expand Up @@ -22,6 +22,7 @@
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.util.concurrent.TimeUnit;

import org.springframework.scheduling.config.ScheduledTaskRegistrar;

Expand Down Expand Up @@ -101,52 +102,64 @@
String zone() default "";

/**
* Execute the annotated method with a fixed period in milliseconds between the
* Execute the annotated method with a fixed period between the
* end of the last invocation and the start of the next.
* @return the delay in milliseconds
* Using milliseconds by default with timeUnit().
* @return the delay
*/
long fixedDelay() default -1;

/**
* Execute the annotated method with a fixed period in milliseconds between the
* Execute the annotated method with a fixed period between the
* end of the last invocation and the start of the next.
* @return the delay in milliseconds as a String value, e.g. a placeholder
* Using milliseconds by default with fixedDelayTimeUnit().
* @return the delay as a String value, e.g. a placeholder
* or a {@link java.time.Duration#parse java.time.Duration} compliant value
* @since 3.2.2
*/
String fixedDelayString() default "";

/**
* Execute the annotated method with a fixed period in milliseconds between
* Execute the annotated method with a fixed period between
* invocations.
* @return the period in milliseconds
* Using milliseconds by default with timeUnit().
* @return the period
*/
long fixedRate() default -1;

/**
* Execute the annotated method with a fixed period in milliseconds between
* Execute the annotated method with a fixed period between
* invocations.
* @return the period in milliseconds as a String value, e.g. a placeholder
* Using milliseconds by default with fixedRateTimeUnit().
* @return the period as a String value, e.g. a placeholder
* or a {@link java.time.Duration#parse java.time.Duration} compliant value
* @since 3.2.2
*/
String fixedRateString() default "";

/**
* Number of milliseconds to delay before the first execution of a
* Number to delay before the first execution of a
* {@link #fixedRate} or {@link #fixedDelay} task.
* @return the initial delay in milliseconds
* Using milliseconds by default with timeUnit().
* @return the initial
* @since 3.2
*/
long initialDelay() default -1;

/**
* Number of milliseconds to delay before the first execution of a
* Number to delay before the first execution of a
* {@link #fixedRate} or {@link #fixedDelay} task.
* Using milliseconds by default with initialDelayTimeUnit().
* @return the initial delay in milliseconds as a String value, e.g. a placeholder
* or a {@link java.time.Duration#parse java.time.Duration} compliant value
* @since 3.2.2
*/
String initialDelayString() default "";

/**
* Specify the {@link TimeUnit} to use for initialDelay, fixedRate and fixedDelay values.
* @return the {@link TimeUnit}, by default milliseconds will be used.
*/
TimeUnit timeUnit() default TimeUnit.MILLISECONDS;

}
Expand Up @@ -30,6 +30,7 @@
import java.util.TimeZone;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
Expand Down Expand Up @@ -398,7 +399,7 @@ protected void processScheduled(Scheduled scheduled, Method method, Object bean)
Set<ScheduledTask> tasks = new LinkedHashSet<>(4);

// Determine initial delay
long initialDelay = scheduled.initialDelay();
long initialDelay = TimeUnit.MILLISECONDS.convert(scheduled.initialDelay(), scheduled.timeUnit());
String initialDelayString = scheduled.initialDelayString();
if (StringUtils.hasText(initialDelayString)) {
Assert.isTrue(initialDelay < 0, "Specify 'initialDelay' or 'initialDelayString', not both");
Expand All @@ -407,7 +408,7 @@ protected void processScheduled(Scheduled scheduled, Method method, Object bean)
}
if (StringUtils.hasLength(initialDelayString)) {
try {
initialDelay = parseDelayAsLong(initialDelayString);
initialDelay = TimeUnit.MILLISECONDS.convert(parseDelayAsLong(initialDelayString), scheduled.timeUnit());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While performing a final review, I noticed that this introduces a 🐛 when the String is a java.time.Duration string, since the value gets converted to milliseconds and then converted again using the configured time unit.

I've fixed this locally, and you can see the result once I've merged the changes into main.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh ok, good to know ! Thank you.

}
catch (RuntimeException ex) {
throw new IllegalArgumentException(
Expand Down Expand Up @@ -446,12 +447,13 @@ protected void processScheduled(Scheduled scheduled, Method method, Object bean)
}

// Check fixed delay
long fixedDelay = scheduled.fixedDelay();
long fixedDelay = TimeUnit.MILLISECONDS.convert(scheduled.fixedDelay(), scheduled.timeUnit());
if (fixedDelay >= 0) {
Assert.isTrue(!processedSchedule, errorMessage);
processedSchedule = true;
tasks.add(this.registrar.scheduleFixedDelayTask(new FixedDelayTask(runnable, fixedDelay, initialDelay)));
}

String fixedDelayString = scheduled.fixedDelayString();
if (StringUtils.hasText(fixedDelayString)) {
if (this.embeddedValueResolver != null) {
Expand All @@ -461,7 +463,7 @@ protected void processScheduled(Scheduled scheduled, Method method, Object bean)
Assert.isTrue(!processedSchedule, errorMessage);
processedSchedule = true;
try {
fixedDelay = parseDelayAsLong(fixedDelayString);
fixedDelay = TimeUnit.MILLISECONDS.convert(parseDelayAsLong(fixedDelayString), scheduled.timeUnit());
}
catch (RuntimeException ex) {
throw new IllegalArgumentException(
Expand All @@ -472,7 +474,7 @@ protected void processScheduled(Scheduled scheduled, Method method, Object bean)
}

// Check fixed rate
long fixedRate = scheduled.fixedRate();
long fixedRate = TimeUnit.MILLISECONDS.convert(scheduled.fixedRate(), scheduled.timeUnit());
if (fixedRate >= 0) {
Assert.isTrue(!processedSchedule, errorMessage);
processedSchedule = true;
Expand All @@ -487,7 +489,7 @@ protected void processScheduled(Scheduled scheduled, Method method, Object bean)
Assert.isTrue(!processedSchedule, errorMessage);
processedSchedule = true;
try {
fixedRate = parseDelayAsLong(fixedRateString);
fixedRate = TimeUnit.MILLISECONDS.convert(parseDelayAsLong(fixedRateString), scheduled.timeUnit());
}
catch (RuntimeException ex) {
throw new IllegalArgumentException(
Expand Down