Scheduler - validate cron() and every() expressions

extensions/arc/deployment/src/test/java/org/jboss/shamrock/arc/test/config/ConfigPropertyInjectionValidationTest.java

@@ -23,7 +23,7 @@
 import javax.inject.Inject;
 
 import org.eclipse.microprofile.config.inject.ConfigProperty;
-import org.jboss.shamrock.test.BuildShouldFailWith;
+import org.jboss.shamrock.test.ShouldFail;
 import org.jboss.shamrock.test.Deployment;
 import org.jboss.shamrock.test.ShamrockUnitTest;
 import org.jboss.shrinkwrap.api.ShrinkWrap;
@@ -34,7 +34,7 @@
 @RunWith(ShamrockUnitTest.class)
 public class ConfigPropertyInjectionValidationTest {
 
-    @BuildShouldFailWith(DeploymentException.class)
+    @ShouldFail(DeploymentException.class)
     @Deployment
     public static JavaArchive deploy() {
         return ShrinkWrap.create(JavaArchive.class)

extensions/arc/deployment/src/test/java/org/jboss/shamrock/arc/test/interceptor/InterceptorNoBindingsTest.java

@@ -22,7 +22,7 @@
 import javax.interceptor.Interceptor;
 import javax.interceptor.InvocationContext;
 
-import org.jboss.shamrock.test.BuildShouldFailWith;
+import org.jboss.shamrock.test.ShouldFail;
 import org.jboss.shamrock.test.Deployment;
 import org.jboss.shamrock.test.ShamrockUnitTest;
 import org.jboss.shrinkwrap.api.ShrinkWrap;
@@ -33,7 +33,7 @@
 @RunWith(ShamrockUnitTest.class)
 public class InterceptorNoBindingsTest {
 
-    @BuildShouldFailWith(DefinitionException.class)
+    @ShouldFail(DefinitionException.class)
     @Deployment
     public static JavaArchive deploy() {
         return ShrinkWrap.create(JavaArchive.class)

extensions/scheduler/deployment/src/main/java/org/jboss/shamrock/scheduler/deployment/SchedulerProcessor.java

@@ -17,6 +17,8 @@
 
 import static org.jboss.shamrock.annotations.ExecutionTime.STATIC_INIT;
 
+import java.text.ParseException;
+import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
@@ -40,6 +42,7 @@
 import org.jboss.protean.arc.processor.AnnotationStore;
 import org.jboss.protean.arc.processor.AnnotationsTransformer;
 import org.jboss.protean.arc.processor.BeanDeploymentValidator;
+import org.jboss.protean.arc.processor.BeanDeploymentValidator.ValidationContext;
 import org.jboss.protean.arc.processor.BeanInfo;
 import org.jboss.protean.arc.processor.DotNames;
 import org.jboss.protean.arc.processor.ScopeInfo;
@@ -62,8 +65,10 @@
 import org.jboss.shamrock.scheduler.api.Scheduleds;
 import org.jboss.shamrock.scheduler.runtime.QuartzScheduler;
 import org.jboss.shamrock.scheduler.runtime.ScheduledInvoker;
+import org.jboss.shamrock.scheduler.runtime.ScheduledLiteral;
 import org.jboss.shamrock.scheduler.runtime.SchedulerConfiguration;
 import org.jboss.shamrock.scheduler.runtime.SchedulerDeploymentTemplate;
+import org.quartz.CronExpression;
 import org.quartz.simpl.CascadingClassLoadHelper;
 import org.quartz.simpl.RAMJobStore;
 import org.quartz.simpl.SimpleThreadPool;
@@ -167,8 +172,11 @@ public void validate(ValidationContext validationContext) {
                                 if (!method.returnType().kind().equals(Type.Kind.VOID)) {
                                     throw new IllegalStateException(String.format("Scheduled business method must return void [method: %s, bean:%s", method.returnType(), method, bean));
                                 }
+                                // Validate cron() and every() expressions
+                                for (AnnotationInstance scheduled : schedules) {
+                                    validateScheduled(validationContext, scheduled);
+                                }
                                 scheduledBusinessMethods.produce(new ScheduledBusinessMethodItem(bean, method, schedules));
-                                // TODO: validate cron and period expressions
                                 LOGGER.debugf("Found scheduled business method %s declared on %s", method, bean);
                             }
                         }
@@ -260,6 +268,40 @@ private Object getValue(AnnotationValue annotationValue) {
         }
     }
 
+    private void validateScheduled(ValidationContext validationContext, AnnotationInstance schedule) {
+        AnnotationValue cronValue = schedule.value("cron");
+        if (cronValue != null && !cronValue.asString().trim().isEmpty()) {
+            String cron = cronValue.asString().trim();
+            if (ScheduledLiteral.isConfigValue(cron)) {
+                // Don't validate config property
+                return;
+            }
+            try {
+                new CronExpression(cron);
+            } catch (ParseException e) {
+                validationContext.addDeploymentProblem(new IllegalStateException("Invalid cron() expression on: " + schedule, e));
+            }
+        } else {
+            AnnotationValue everyValue = schedule.value("every");
+            if (everyValue != null && !everyValue.asString().trim().isEmpty()) {
+                String every = everyValue.asString().trim();
+                if (ScheduledLiteral.isConfigValue(every)) {
+                    return;
+                }
+                if (Character.isDigit(every.charAt(0))) {
+                    every = "PT" + every;
+                }
+                try {
+                    Duration.parse(every);
+                } catch (Exception e) {
+                    validationContext.addDeploymentProblem(new IllegalStateException("Invalid every() expression on: " + schedule, e));
+                }
+            } else {
+                validationContext.addDeploymentProblem(new IllegalStateException("@Scheduled must declare either cron() or every(): " + schedule));
+            }
+        }
+    }
+
     static final class ProcessorClassOutput implements ClassOutput {
         private final BuildProducer<GeneratedClassBuildItem> producer;
 

extensions/scheduler/deployment/src/test/java/org/jboss/shamrock/scheduler/test/InvalidCronExpressionTest.java

@@ -0,0 +1,52 @@
+/*
+ * Copyright 2018 Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jboss.shamrock.scheduler.test;
+
+import javax.enterprise.inject.spi.DeploymentException;
+
+import org.jboss.shamrock.scheduler.api.Scheduled;
+import org.jboss.shamrock.test.ShouldFail;
+import org.jboss.shamrock.test.Deployment;
+import org.jboss.shamrock.test.ShamrockUnitTest;
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.spec.JavaArchive;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+@RunWith(ShamrockUnitTest.class)
+public class InvalidCronExpressionTest {
+
+    @ShouldFail(DeploymentException.class)
+    @Deployment
+    public static JavaArchive deploy() {
+        return ShrinkWrap.create(JavaArchive.class)
+                .addClasses(InvalidBean.class);
+    }
+
+    @Test
+    public void test() throws InterruptedException {
+    }
+
+    static class InvalidBean {
+
+
+        @Scheduled(cron="0 0 0 ????")
+        void wrong() {
+        }
+
+    }
+
+}

extensions/scheduler/deployment/src/test/java/org/jboss/shamrock/scheduler/test/InvalidEveryExpressionTest.java

@@ -0,0 +1,52 @@
+/*
+ * Copyright 2018 Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jboss.shamrock.scheduler.test;
+
+import javax.enterprise.inject.spi.DeploymentException;
+
+import org.jboss.shamrock.scheduler.api.Scheduled;
+import org.jboss.shamrock.test.ShouldFail;
+import org.jboss.shamrock.test.Deployment;
+import org.jboss.shamrock.test.ShamrockUnitTest;
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.spec.JavaArchive;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+@RunWith(ShamrockUnitTest.class)
+public class InvalidEveryExpressionTest {
+
+    @ShouldFail(DeploymentException.class)
+    @Deployment
+    public static JavaArchive deploy() {
+        return ShrinkWrap.create(JavaArchive.class)
+                .addClasses(InvalidBean.class);
+    }
+
+    @Test
+    public void test() throws InterruptedException {
+    }
+
+    static class InvalidBean {
+
+
+        @Scheduled(every="call me every other day")
+        void wrong() {
+        }
+
+    }
+
+}

extensions/scheduler/deployment/src/test/java/org/jboss/shamrock/scheduler/test/MissingConfigCronExpressionTest.java

@@ -0,0 +1,48 @@
+/*
+ * Copyright 2018 Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jboss.shamrock.scheduler.test;
+
+import org.jboss.shamrock.scheduler.api.Scheduled;
+import org.jboss.shamrock.test.Deployment;
+import org.jboss.shamrock.test.ShamrockUnitTest;
+import org.jboss.shamrock.test.ShouldFail;
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.spec.JavaArchive;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+@RunWith(ShamrockUnitTest.class)
+public class MissingConfigCronExpressionTest {
+
+    @ShouldFail(IllegalStateException.class)
+    @Deployment
+    public static JavaArchive deploy() {
+        return ShrinkWrap.create(JavaArchive.class).addClasses(InvalidBean.class);
+    }
+
+    @Test
+    public void test() throws InterruptedException {
+    }
+
+    static class InvalidBean {
+
+        @Scheduled(cron = "{my.cron}")
+        void wrong() {
+        }
+
+    }
+
+}

extensions/scheduler/deployment/src/test/java/org/jboss/shamrock/scheduler/test/NoExpressionTest.java

@@ -0,0 +1,52 @@
+/*
+ * Copyright 2018 Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jboss.shamrock.scheduler.test;
+
+import javax.enterprise.inject.spi.DeploymentException;
+
+import org.jboss.shamrock.scheduler.api.Scheduled;
+import org.jboss.shamrock.test.ShouldFail;
+import org.jboss.shamrock.test.Deployment;
+import org.jboss.shamrock.test.ShamrockUnitTest;
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.spec.JavaArchive;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+@RunWith(ShamrockUnitTest.class)
+public class NoExpressionTest {
+
+    @ShouldFail(DeploymentException.class)
+    @Deployment
+    public static JavaArchive deploy() {
+        return ShrinkWrap.create(JavaArchive.class)
+                .addClasses(InvalidBean.class);
+    }
+
+    @Test
+    public void test() throws InterruptedException {
+    }
+
+    static class InvalidBean {
+
+
+        @Scheduled
+        void wrong() {
+        }
+
+    }
+
+}

extensions/scheduler/runtime/src/main/java/org/jboss/shamrock/scheduler/runtime/QuartzScheduler.java

@@ -171,21 +171,21 @@ public Job newJob(TriggerFiredBundle bundle, org.quartz.Scheduler scheduler) thr
                                 .usingJobData(SchedulerDeploymentTemplate.INVOKER_KEY, entry.getKey());
                         ScheduleBuilder<?> scheduleBuilder;
 
-                        String cron = scheduled.cron();
+                        String cron = scheduled.cron().trim();
                         if (!cron.isEmpty()) {
                             try {
-                                if (cron.startsWith("{") && cron.endsWith("}")) {
-                                    cron = config.getValue(cron.substring(1, cron.length() - 1), String.class);
+                                if (ScheduledLiteral.isConfigValue(cron)) {
+                                    cron = config.getValue(ScheduledLiteral.getConfigProperty(cron), String.class);
                                 }
                                 scheduleBuilder = CronScheduleBuilder.cronSchedule(cron);
                             } catch (RuntimeException e) {
-                                LOGGER.warnf(e, "Invalid CRON expression: %s", cron);
-                                continue;
+                                // This should only happen for config-based expressions
+                                throw new IllegalStateException("Invalid cron() expression on: " + scheduled, e);
                             }
                         } else if (!scheduled.every().isEmpty()) {
-                            String every = scheduled.every();
-                            if (every.startsWith("{") && every.endsWith("}")) {
-                                every = config.getValue(every.substring(1, every.length() - 1), String.class);
+                            String every = scheduled.every().trim();
+                            if (ScheduledLiteral.isConfigValue(every)) {
+                                every = config.getValue(ScheduledLiteral.getConfigProperty(every), String.class);
                             }
                             if (Character.isDigit(every.charAt(0))) {
                                 every = "PT" + every;
@@ -194,8 +194,8 @@ public Job newJob(TriggerFiredBundle bundle, org.quartz.Scheduler scheduler) thr
                             try {
                                 duration = Duration.parse(every);
                             } catch (Exception e) {
-                                LOGGER.warnf(e, "Invalid period expression: %s", scheduled.every());
-                                continue;
+                                // This should only happen for config-based expressions
+                                throw new IllegalStateException("Invalid every() expression on: " + scheduled, e);
                             }
                             scheduleBuilder = SimpleScheduleBuilder.simpleSchedule().withIntervalInMilliseconds(duration.toMillis()).repeatForever();
                         } else {

extensions/scheduler/runtime/src/main/java/org/jboss/shamrock/scheduler/runtime/ScheduledLiteral.java

@@ -105,4 +105,13 @@ Scheduled build() {
 
     }
 
+    public static boolean isConfigValue(String val) {
+    	val = val.trim();
+    	return val.startsWith("{") && val.endsWith("}");
+    }
+
+    public static String getConfigProperty(String val) {
+        return val.substring(1, val.length() - 1);
+    }
+
 }