Scheduler: inheritance of metadata

- turn the warning in the docs into a subsection - mention the inheritance rules in the javadoc of the Scheduled annotation - fail the build if an abstract class/interface declares a method annotated with Scheduled - add support for static interface scheduled methods (so far only static methods declared on a class were supported) - fixes #38781
quarkusio · Feb 15, 2024 · b8b1589 · b8b1589
1 parent 754f4e4
commit b8b1589
Show file tree

Hide file tree

Showing 6 changed files with 159 additions and 17 deletions.
diff --git a/docs/src/main/asciidoc/scheduler-reference.adoc b/docs/src/main/asciidoc/scheduler-reference.adoc
@@ -34,12 +34,16 @@ Furthermore, the annotated method must return `void` and either declare no param
 
 TIP: The annotation is repeatable so a single method could be scheduled multiple times.
 
-[WARNING]
-====
-Subclasses never inherit the metadata of a `@Scheduled` method declared on a superclass. In the following example, the `everySecond()` method is only invoked upon the instance of `Jobs`.
+=== Inheritance of metadata
+
+A subclass never inherits the metadata of a `@Scheduled` method declared on a superclass. 
+For example, suppose the class `org.amce.Foo` is extended by the class `org.amce.Bar`. 
+If `Foo` declares a non-static method annotated with `@Scheduled` then `Bar` does not inherit the metadata of the scheduled method.
+In the following example, the `everySecond()` method is only invoked upon the instance of `Foo`.
+
 [source,java]
 ----
-class Jobs {
+class Foo {
 
    @Scheduled(every = "1s")
    void everySecond() {
@@ -48,12 +52,38 @@ class Jobs {
 }
 
 @Singleton
-class MyJobs extends Jobs {
+class Bar extends Foo {
 }
 ----
-====
 
-A CDI event of type `io.quarkus.scheduler.SuccessfulExecution` is fired synchronously and asynchronously when an execution of a scheduled method is successful. A CDI event of type `io.quarkus.scheduler.FailedExecution` is fired synchronously and asynchronously  when an execution of a scheduled method throws an exception.
+=== CDI events
+
+Some CDI events are fired synchronously and asynchronously when specific events occur.
+
+|===
+|Type |Event description
+
+|`io.quarkus.scheduler.SuccessfulExecution`
+|An execution of a scheduled job completed successfuly.
+
+|`io.quarkus.scheduler.FailedExecution`
+|An execution of a scheduled job completed with an exception.
+
+|`io.quarkus.scheduler.SkippedExecution`
+|An execution of a scheduled job was skipped.
+
+|`io.quarkus.scheduler.SchedulerPaused`
+|The scheduler was paused.
+
+|`io.quarkus.scheduler.SchedulerResumed`
+|The scheduler was resumed.
+
+|`io.quarkus.scheduler.ScheduledJobPaused`
+|A scheduled job was paused.
+
+|`io.quarkus.scheduler.ScheduledJobResumed`
+|A scheduled job was resumed.
+|===
 
 === Triggers
 

diff --git a/extensions/scheduler/api/src/main/java/io/quarkus/scheduler/Scheduled.java b/extensions/scheduler/api/src/main/java/io/quarkus/scheduler/Scheduled.java
@@ -45,6 +45,11 @@
  * {@code java.util.concurrent.CompletionStage<Void>} or {@code io.smallrye.mutiny.Uni<Void>}, or is annotated with
  * {@link io.smallrye.common.annotation.NonBlocking} is executed on the event loop.
  *
+ * <h2>Inheritance of metadata</h2>
+ * A subclass never inherits the metadata of a {@link Scheduled} method declared on a superclass. For example, suppose the class
+ * {@code org.amce.Foo} is extended by the class {@code org.amce.Bar}. If {@code Foo} declares a non-static method annotated
+ * with {@link Scheduled} then {@code Bar} does not inherit the metadata of the scheduled method.
+ *
  * @see ScheduledExecution
  */
 @Target(METHOD)

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
@@ -132,14 +132,21 @@ void collectScheduledMethods(BeanArchiveIndexBuildItem beanArchives, BeanDiscove
         }
         for (AnnotationInstance annotationInstance : schedules) {
             if (annotationInstance.target().kind() != METHOD) {
-                continue;
+                continue; // This should never happen as the annotation has @Target(METHOD)
             }
             MethodInfo method = annotationInstance.target().asMethod();
+            ClassInfo declaringClass = method.declaringClass();
+            if (!Modifier.isStatic(method.flags())
+                    && (Modifier.isAbstract(declaringClass.flags()) || declaringClass.isInterface())) {
+                throw new IllegalStateException(String.format(
+                        "Non-static @Scheduled methods may not be declared on abstract classes and interfaces: %s() declared on %s",
+                        method.name(), declaringClass.name()));
+            }
             if (Modifier.isStatic(method.flags()) && !KotlinUtil.isSuspendMethod(method)) {
                 scheduledBusinessMethods.produce(new ScheduledBusinessMethodItem(null, method, schedules,
                         transformedAnnotations.hasAnnotation(method, SchedulerDotNames.NON_BLOCKING),
                         transformedAnnotations.hasAnnotation(method, SchedulerDotNames.RUN_ON_VIRTUAL_THREAD)));
-                LOGGER.debugf("Found scheduled static method %s declared on %s", method, method.declaringClass().name());
+                LOGGER.debugf("Found scheduled static method %s declared on %s", method, declaringClass.name());
             }
         }
 
@@ -425,14 +432,26 @@ private String generateInvoker(ScheduledBusinessMethodItem scheduledMethod, Clas
         String returnTypeStr = DescriptorUtils.typeToString(method.returnType());
         ResultHandle res;
         if (isStatic) {
-            if (method.parameterTypes().isEmpty()) {
-                res = tryBlock.invokeStaticMethod(
-                        MethodDescriptor.ofMethod(implClazz.name().toString(), method.name(), returnTypeStr));
+            if (implClazz.isInterface()) {
+                if (method.parameterTypes().isEmpty()) {
+                    res = tryBlock.invokeStaticInterfaceMethod(
+                            MethodDescriptor.ofMethod(implClazz.name().toString(), method.name(), returnTypeStr));
+                } else {
+                    res = tryBlock.invokeStaticInterfaceMethod(
+                            MethodDescriptor.ofMethod(implClazz.name().toString(), method.name(), returnTypeStr,
+                                    ScheduledExecution.class),
+                            tryBlock.getMethodParam(0));
+                }
             } else {
-                res = tryBlock.invokeStaticMethod(
-                        MethodDescriptor.ofMethod(implClazz.name().toString(), method.name(), returnTypeStr,
-                                ScheduledExecution.class),
-                        tryBlock.getMethodParam(0));
+                if (method.parameterTypes().isEmpty()) {
+                    res = tryBlock.invokeStaticMethod(
+                            MethodDescriptor.ofMethod(implClazz.name().toString(), method.name(), returnTypeStr));
+                } else {
+                    res = tryBlock.invokeStaticMethod(
+                            MethodDescriptor.ofMethod(implClazz.name().toString(), method.name(), returnTypeStr,
+                                    ScheduledExecution.class),
+                            tryBlock.getMethodParam(0));
+                }
             }
         } else {
             // InjectableBean<Foo> bean = Arc.container().bean("foo1");

diff --git a/...ployment/src/test/java/io/quarkus/scheduler/test/NonStaticScheduledAbstractClassTest.java b/...ployment/src/test/java/io/quarkus/scheduler/test/NonStaticScheduledAbstractClassTest.java
@@ -0,0 +1,32 @@
+package io.quarkus.scheduler.test;
+
+import static org.junit.jupiter.api.Assertions.fail;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.scheduler.Scheduled;
+import io.quarkus.test.QuarkusUnitTest;
+
+public class NonStaticScheduledAbstractClassTest {
+
+    @RegisterExtension
+    static final QuarkusUnitTest test = new QuarkusUnitTest()
+            .setExpectedException(IllegalStateException.class)
+            .withApplicationRoot(root -> root
+                    .addClasses(AbstractClassWitchScheduledMethod.class));
+
+    @Test
+    public void test() {
+        fail();
+    }
+
+    static abstract class AbstractClassWitchScheduledMethod {
+
+        @Scheduled(every = "1s")
+        void everySecond() {
+        }
+
+    }
+
+}
diff --git a/...r/deployment/src/test/java/io/quarkus/scheduler/test/NonStaticScheduledInterfaceTest.java b/...r/deployment/src/test/java/io/quarkus/scheduler/test/NonStaticScheduledInterfaceTest.java
@@ -0,0 +1,31 @@
+package io.quarkus.scheduler.test;
+
+import static org.junit.jupiter.api.Assertions.fail;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.scheduler.Scheduled;
+import io.quarkus.test.QuarkusUnitTest;
+
+public class NonStaticScheduledInterfaceTest {
+
+    @RegisterExtension
+    static final QuarkusUnitTest test = new QuarkusUnitTest()
+            .setExpectedException(IllegalStateException.class)
+            .withApplicationRoot(root -> root
+                    .addClasses(InterfaceWitchScheduledMethod.class));
+
+    @Test
+    public void test() {
+        fail();
+    }
+
+    interface InterfaceWitchScheduledMethod {
+
+        @Scheduled(every = "1s")
+        void everySecond();
+
+    }
+
+}
diff --git a/...yment/src/test/java/io/quarkus/scheduler/test/staticmethod/ScheduledStaticMethodTest.java b/...yment/src/test/java/io/quarkus/scheduler/test/staticmethod/ScheduledStaticMethodTest.java
@@ -16,11 +16,13 @@ public class ScheduledStaticMethodTest {
     @RegisterExtension
     static final QuarkusUnitTest test = new QuarkusUnitTest()
             .withApplicationRoot((jar) -> jar
-                    .addClasses(Jobs.class));
+                    .addClasses(Jobs.class, AbstractJobs.class, InterfaceJobs.class));
 
     @Test
     public void testSimpleScheduledJobs() throws InterruptedException {
         assertTrue(Jobs.LATCH.await(5, TimeUnit.SECONDS));
+        assertTrue(AbstractJobs.LATCH.await(5, TimeUnit.SECONDS));
+        assertTrue(InterfaceJobs.LATCH.await(5, TimeUnit.SECONDS));
     }
 
     static class Jobs {
@@ -31,6 +33,29 @@ static class Jobs {
         static void everySecond() {
             LATCH.countDown();
         }
+
+    }
+
+    static abstract class AbstractJobs {
+
+        static final CountDownLatch LATCH = new CountDownLatch(1);
+
+        @Scheduled(every = "1s")
+        static void everySecond() {
+            LATCH.countDown();
+        }
+
+    }
+
+    interface InterfaceJobs {
+
+        CountDownLatch LATCH = new CountDownLatch(1);
+
+        @Scheduled(every = "1s")
+        static void everySecond() {
+            LATCH.countDown();
+        }
+
     }
 
 }