Skip to content

Commit

Permalink
Scheduler: javadoc/docs clarifications
Browse files Browse the repository at this point in the history
- log a warning if a Scheduled#every() value is not supported by the
underlying scheduler impl
- fixes quarkusio#37567
  • Loading branch information
mkouba committed Dec 8, 2023
1 parent b12e61e commit 4cfd8d3
Show file tree
Hide file tree
Showing 12 changed files with 70 additions and 24 deletions.
2 changes: 2 additions & 0 deletions docs/src/main/asciidoc/scheduler-reference.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ So for example, `15m` can be used instead of `PT15M` and is parsed as "15 minute
void every15Mins() { }
----

WARNING: A value less than one second may not be supported by the underlying scheduler implementation. In that case a warning message is logged during build and application start.

The `every` attribute supports <<config-reference#property-expressions,Property Expressions>> including default values and nested
Property Expressions. (Note that `"{property.path}"` style expressions are still supported but don't offer the full functionality of Property Expressions.)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,15 @@
String cron() default "";

/**
* Defines a period between invocations.
* Defines the period between invocations.
* <p>
* The value is parsed with {@link Duration#parse(CharSequence)}. However, if an expression starts with a digit, "PT" prefix
* is added automatically, so for example, {@code 15m} can be used instead of {@code PT15M} and is parsed as "15 minutes".
* Note that the absolute value of the value is always used.
* <p>
* A value less than one second may not be supported by the underlying scheduler implementation. In that case a warning
* message is logged during build and application start.
* <p>
* The value can be a property expression. In this case, the scheduler attempts to use the configured value instead:
* {@code @Scheduled(every = "${myJob.everyExpression}")}.
* Additionally, the property expression can specify a default value: {@code @Scheduled(every =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,20 @@
import java.time.Instant;

/**
* Scheduled execution metadata.
* Execution metadata of a specific scheduled job.
*/
public interface ScheduledExecution {

/**
*
* @return the trigger that fired the event
* @return the trigger that fired the execution
*/
Trigger getTrigger();

/**
* Unlike {@link Trigger#getPreviousFireTime()} this method always returns the same value.
*
* @return the time the event was fired
* @return the time the associated trigger was fired
*/
Instant getFireTime();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,9 @@ interface JobDefinition {
* The schedule is defined either by {@link #setCron(String)} or by {@link #setInterval(String)}. If both methods are
* used, then the cron expression takes precedence.
* <p>
* A value less than one second may not be supported by the underlying scheduler implementation. In that case a warning
* message is logged immediately.
* <p>
* {@link Scheduled#every()}
*
* @param every
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,20 @@
import java.time.Instant;

/**
* Trigger is bound to a scheduled task.
* Trigger is bound to a scheduled job.
* <p>
* It represents the logic that is used to test if a scheduled job should be executed
* at a specific time, i.e. the trigger is "fired".
*
* @see Scheduled
*/
public interface Trigger {

/**
*
* @return the identifier
* @return the identifier of the job
* @see Scheduled#identity()
* @see Scheduler#newJob(String)
*/
String getId();

Expand Down
5 changes: 5 additions & 0 deletions extensions/scheduler/deployment/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@
<artifactId>quarkus-junit5-internal</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.rest-assured</groupId>
<artifactId>rest-assured</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,8 @@ private void collectScheduledMethods(IndexView index, TransformedAnnotationsBuil

@BuildStep
void validateScheduledBusinessMethods(SchedulerConfig config, List<ScheduledBusinessMethodItem> scheduledMethods,
ValidationPhaseBuildItem validationPhase, BuildProducer<ValidationErrorBuildItem> validationErrors) {
ValidationPhaseBuildItem validationPhase, BuildProducer<ValidationErrorBuildItem> validationErrors,
Capabilities capabilities) {
List<Throwable> errors = new ArrayList<>();
Map<String, AnnotationInstance> encounteredIdentities = new HashMap<>();
Set<String> methodDescriptions = new HashSet<>();
Expand Down Expand Up @@ -240,9 +241,11 @@ void validateScheduledBusinessMethods(SchedulerConfig config, List<ScheduledBusi
}
}
// Validate cron() and every() expressions
long checkPeriod = capabilities.isMissing(Capability.QUARTZ) ? SimpleScheduler.CHECK_PERIOD : 50;
CronParser parser = new CronParser(CronDefinitionBuilder.instanceDefinitionFor(config.cronType));
for (AnnotationInstance scheduled : scheduledMethod.getSchedules()) {
Throwable error = validateScheduled(parser, scheduled, encounteredIdentities, validationPhase.getContext());
Throwable error = validateScheduled(parser, scheduled, encounteredIdentities, validationPhase.getContext(),
checkPeriod);
if (error != null) {
errors.add(error);
}
Expand Down Expand Up @@ -509,7 +512,7 @@ private String generateInvoker(ScheduledBusinessMethodItem scheduledMethod, Clas

private Throwable validateScheduled(CronParser parser, AnnotationInstance schedule,
Map<String, AnnotationInstance> encounteredIdentities,
BeanDeploymentValidator.ValidationContext validationContext) {
BeanDeploymentValidator.ValidationContext validationContext, long checkPeriod) {
MethodInfo method = schedule.target().asMethod();
AnnotationValue cronValue = schedule.value("cron");
AnnotationValue everyValue = schedule.value("every");
Expand All @@ -519,7 +522,7 @@ private Throwable validateScheduled(CronParser parser, AnnotationInstance schedu
try {
parser.parse(cron).validate();
} catch (IllegalArgumentException e) {
return new IllegalStateException("Invalid cron() expression on: " + schedule, e);
return new IllegalStateException(errorMessage("Invalid cron() expression", schedule, method), e);
}
if (everyValue != null && !everyValue.asString().trim().isEmpty()) {
LOGGER.warnf(
Expand All @@ -535,7 +538,7 @@ private Throwable validateScheduled(CronParser parser, AnnotationInstance schedu
try {
ZoneId.of(timeZone);
} catch (Exception e) {
return new IllegalStateException("Invalid timeZone() on " + schedule, e);
return new IllegalStateException(errorMessage("Invalid timeZone()", schedule, method), e);
}
}
}
Expand All @@ -548,9 +551,14 @@ private Throwable validateScheduled(CronParser parser, AnnotationInstance schedu
every = "PT" + every;
}
try {
Duration.parse(every);
Duration period = Duration.parse(every);
if (period.toMillis() < checkPeriod) {
LOGGER.warnf(
"An every() value less than %s ms is not supported - the scheduled job will be executed with a delay: %s declared on %s#%s()",
checkPeriod, schedule, method.declaringClass().name(), method.name());
}
} catch (Exception e) {
return new IllegalStateException("Invalid every() expression on: " + schedule, e);
return new IllegalStateException(errorMessage("Invalid every() expression", schedule, method), e);
}
}
} else {
Expand All @@ -569,7 +577,7 @@ private Throwable validateScheduled(CronParser parser, AnnotationInstance schedu
try {
Duration.parse(delayed);
} catch (Exception e) {
return new IllegalStateException("Invalid delayed() expression on: " + schedule, e);
return new IllegalStateException(errorMessage("Invalid delayed() expression", schedule, method), e);
}
}

Expand Down Expand Up @@ -609,6 +617,10 @@ private Throwable validateScheduled(CronParser parser, AnnotationInstance schedu
return null;
}

private static String errorMessage(String base, AnnotationInstance scheduled, MethodInfo method) {
return String.format("%s: %s declared on %s#%s()", base, scheduled, method.declaringClass().name(), method.name());
}

@BuildStep
UnremovableBeanBuildItem unremoveableSkipPredicates() {
return new UnremovableBeanBuildItem(new UnremovableBeanBuildItem.BeanTypeExclusion(SchedulerDotNames.SKIP_PREDICATE));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package io.quarkus.scheduler.test;

import jakarta.enterprise.inject.spi.DeploymentException;
import static org.assertj.core.api.Assertions.assertThat;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
Expand All @@ -12,7 +12,10 @@ public class InvalidCronExpressionTest {

@RegisterExtension
static final QuarkusUnitTest test = new QuarkusUnitTest()
.setExpectedException(DeploymentException.class)
.assertException(t -> {
assertThat(t).cause().isInstanceOf(IllegalStateException.class)
.hasMessageContaining("Invalid cron() expression");
})
.withApplicationRoot((jar) -> jar
.addClasses(InvalidBean.class));

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package io.quarkus.scheduler.test;

import jakarta.enterprise.inject.spi.DeploymentException;
import static org.assertj.core.api.Assertions.assertThat;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
Expand All @@ -12,7 +12,10 @@ public class InvalidDelayedExpressionTest {

@RegisterExtension
static final QuarkusUnitTest test = new QuarkusUnitTest()
.setExpectedException(DeploymentException.class)
.assertException(t -> {
assertThat(t).cause().isInstanceOf(IllegalStateException.class)
.hasMessageContaining("Invalid delayed() expression");
})
.withApplicationRoot((jar) -> jar
.addClasses(InvalidBean.class));

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package io.quarkus.scheduler.test;

import jakarta.enterprise.inject.spi.DeploymentException;
import static org.assertj.core.api.Assertions.assertThat;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
Expand All @@ -12,7 +12,10 @@ public class InvalidEveryExpressionTest {

@RegisterExtension
static final QuarkusUnitTest test = new QuarkusUnitTest()
.setExpectedException(DeploymentException.class)
.assertException(t -> {
assertThat(t).cause().isInstanceOf(IllegalStateException.class)
.hasMessageContaining("Invalid every() expression");
})
.withApplicationRoot((jar) -> jar
.addClasses(InvalidEveryExpressionTest.InvalidBean.class));

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package io.quarkus.scheduler.test;

import jakarta.enterprise.inject.spi.DeploymentException;
import static org.assertj.core.api.Assertions.assertThat;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
Expand All @@ -12,7 +12,9 @@ public class InvalidTimeZoneTest {

@RegisterExtension
static final QuarkusUnitTest test = new QuarkusUnitTest()
.setExpectedException(DeploymentException.class)
.assertException(t -> {
assertThat(t).cause().isInstanceOf(IllegalStateException.class).hasMessageContaining("Invalid timeZone()");
})
.withApplicationRoot((jar) -> jar
.addClasses(InvalidBean.class));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public class SimpleScheduler implements Scheduler {
private static final Logger LOG = Logger.getLogger(SimpleScheduler.class);

// milliseconds
private static final long CHECK_PERIOD = 1000L;
public static final long CHECK_PERIOD = 1000L;

private final ScheduledExecutorService scheduledExecutor;
private final Vertx vertx;
Expand Down Expand Up @@ -351,7 +351,7 @@ Optional<SimpleTrigger> createTrigger(String id, String methodDescription, CronP
SchedulerUtils.parseCronTimeZone(scheduled), methodDescription));
} else if (!scheduled.every().isEmpty()) {
final OptionalLong everyMillis = SchedulerUtils.parseEveryAsMillis(scheduled);
if (!everyMillis.isPresent()) {
if (everyMillis.isEmpty()) {
return Optional.empty();
}
return Optional.of(new IntervalTrigger(id, start, everyMillis.getAsLong(),
Expand Down Expand Up @@ -505,6 +505,11 @@ static class IntervalTrigger extends SimpleTrigger {
super(id, start, description);
this.interval = interval;
this.gracePeriod = gracePeriod;
if (interval < CHECK_PERIOD) {
LOG.warnf(
"An every() value less than %s ms is not supported - the scheduled job will be executed with a delay: %s",
CHECK_PERIOD, description);
}
}

@Override
Expand Down

0 comments on commit 4cfd8d3

Please sign in to comment.