From c38991f938bddc46791253a55b5b2c3a0a49c37f Mon Sep 17 00:00:00 2001
From: Martin Kouba <mkouba@redhat.com>
Date: Tue, 22 Jun 2021 08:11:35 +0200
Subject: [PATCH] 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
---
 .../deployment/SchedulerProcessor.java        | 62 +++++++++----------
 .../test/InvalidDelayedExpressionTest.java    |  4 +-
 2 files changed, 31 insertions(+), 35 deletions(-)

diff --git a/extensions/scheduler/deployment/src/main/java/io/quarkus/scheduler/deployment/SchedulerProcessor.java b/extensions/scheduler/deployment/src/main/java/io/quarkus/scheduler/deployment/SchedulerProcessor.java
index ad34c15f83748..10d9c0a7a5005 100644
--- a/extensions/scheduler/deployment/src/main/java/io/quarkus/scheduler/deployment/SchedulerProcessor.java
+++ b/extensions/scheduler/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/extensions/scheduler/deployment/src/test/java/io/quarkus/scheduler/test/InvalidDelayedExpressionTest.java b/extensions/scheduler/deployment/src/test/java/io/quarkus/scheduler/test/InvalidDelayedExpressionTest.java
index 2cf49f2e77325..e42bfb30cc65a 100644
--- a/extensions/scheduler/deployment/src/test/java/io/quarkus/scheduler/test/InvalidDelayedExpressionTest.java
+++ b/extensions/scheduler/deployment/src/test/java/io/quarkus/scheduler/test/InvalidDelayedExpressionTest.java
@@ -16,7 +16,7 @@ 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 {
@@ -24,7 +24,7 @@ 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() {
         }