Scheduler - fix a regression in validation

- if a scheduled method has a config-based value in every() or cron() the rest of the values is not validated at build time
gsmet · Jun 22, 2021 · c38991f · c38991f
1 parent fa018e6
commit c38991f
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 35 deletions.
diff --git a/...cheduler/deployment/src/main/java/io/quarkus/scheduler/deployment/SchedulerProcessor.java b/...cheduler/deployment/src/main/java/io/quarkus/scheduler/deployment/SchedulerProcessor.java
@@ -331,33 +331,30 @@ private Throwable validateScheduled(CronParser parser, AnnotationInstance schedu
         AnnotationValue everyValue = schedule.value("every");
         if (cronValue != null && !cronValue.asString().trim().isEmpty()) {
             String cron = cronValue.asString().trim();
-            if (SchedulerUtils.isConfigValue(cron)) {
-                // Don't validate config property
-                return null;
-            }
-            try {
-                parser.parse(cron).validate();
-            } catch (IllegalArgumentException e) {
-                return new IllegalStateException("Invalid cron() expression on: " + schedule, e);
-            }
-            if (everyValue != null && !everyValue.asString().trim().isEmpty()) {
-                LOGGER.warnf(
-                        "%s declared on %s#%s() defines both cron() and every() - the cron expression takes precedence",
-                        schedule, method.declaringClass().name(), method.name());
+            if (!SchedulerUtils.isConfigValue(cron)) {
+                try {
+                    parser.parse(cron).validate();
+                } catch (IllegalArgumentException e) {
+                    return new IllegalStateException("Invalid cron() expression on: " + schedule, e);
+                }
+                if (everyValue != null && !everyValue.asString().trim().isEmpty()) {
+                    LOGGER.warnf(
+                            "%s declared on %s#%s() defines both cron() and every() - the cron expression takes precedence",
+                            schedule, method.declaringClass().name(), method.name());
+                }
             }
         } else {
             if (everyValue != null && !everyValue.asString().trim().isEmpty()) {
                 String every = everyValue.asString().trim();
-                if (SchedulerUtils.isConfigValue(every)) {
-                    return null;
-                }
-                if (Character.isDigit(every.charAt(0))) {
-                    every = "PT" + every;
-                }
-                try {
-                    Duration.parse(every);
-                } catch (Exception e) {
-                    return new IllegalStateException("Invalid every() expression on: " + schedule, e);
+                if (!SchedulerUtils.isConfigValue(every)) {
+                    if (Character.isDigit(every.charAt(0))) {
+                        every = "PT" + every;
+                    }
+                    try {
+                        Duration.parse(every);
+                    } catch (Exception e) {
+                        return new IllegalStateException("Invalid every() expression on: " + schedule, e);
+                    }
                 }
             } else {
                 return new IllegalStateException("@Scheduled must declare either cron() or every(): " + schedule);
@@ -368,16 +365,15 @@ private Throwable validateScheduled(CronParser parser, AnnotationInstance schedu
         if (delay == null || delay.asLong() <= 0) {
             if (delayedValue != null && !delayedValue.asString().trim().isEmpty()) {
                 String delayed = delayedValue.asString().trim();
-                if (SchedulerUtils.isConfigValue(delayed)) {
-                    return null;
-                }
-                if (Character.isDigit(delayed.charAt(0))) {
-                    delayed = "PT" + delayed;
-                }
-                try {
-                    Duration.parse(delayed);
-                } catch (Exception e) {
-                    return new IllegalStateException("Invalid delayed() expression on: " + schedule, e);
+                if (!SchedulerUtils.isConfigValue(delayed)) {
+                    if (Character.isDigit(delayed.charAt(0))) {
+                        delayed = "PT" + delayed;
+                    }
+                    try {
+                        Duration.parse(delayed);
+                    } catch (Exception e) {
+                        return new IllegalStateException("Invalid delayed() expression on: " + schedule, e);
+                    }
                 }
             }
         } else {

diff --git a/...uler/deployment/src/test/java/io/quarkus/scheduler/test/InvalidDelayedExpressionTest.java b/...uler/deployment/src/test/java/io/quarkus/scheduler/test/InvalidDelayedExpressionTest.java
@@ -16,15 +16,15 @@ public class InvalidDelayedExpressionTest {
     static final QuarkusUnitTest test = new QuarkusUnitTest()
             .setExpectedException(DeploymentException.class)
             .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
-                    .addClasses(InvalidDelayedExpressionTest.InvalidBean.class));
+                    .addClasses(InvalidBean.class));
 
     @Test
     public void test() throws InterruptedException {
     }
 
     static class InvalidBean {
 
-        @Scheduled(delayed = "for 10 seconds")
+        @Scheduled(every = "${my.every.expr:off}", delayed = "for 10 seconds")
         void wrong() {
         }