From b8b1589e961d18d3bdcc71f56f93d96eb9e8c777 Mon Sep 17 00:00:00 2001 From: Martin Kouba Date: Thu, 15 Feb 2024 10:24:14 +0100 Subject: [PATCH] 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 --- .../main/asciidoc/scheduler-reference.adoc | 44 ++++++++++++++++--- .../java/io/quarkus/scheduler/Scheduled.java | 5 +++ .../deployment/SchedulerProcessor.java | 37 ++++++++++++---- .../NonStaticScheduledAbstractClassTest.java | 32 ++++++++++++++ .../test/NonStaticScheduledInterfaceTest.java | 31 +++++++++++++ .../ScheduledStaticMethodTest.java | 27 +++++++++++- 6 files changed, 159 insertions(+), 17 deletions(-) create mode 100644 extensions/scheduler/deployment/src/test/java/io/quarkus/scheduler/test/NonStaticScheduledAbstractClassTest.java create mode 100644 extensions/scheduler/deployment/src/test/java/io/quarkus/scheduler/test/NonStaticScheduledInterfaceTest.java diff --git a/docs/src/main/asciidoc/scheduler-reference.adoc b/docs/src/main/asciidoc/scheduler-reference.adoc index 25fa988a58315..134e3bef9407a 100644 --- 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 index 493478260ad40..0353c8c401bbe 100644 --- 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} or {@code io.smallrye.mutiny.Uni}, or is annotated with * {@link io.smallrye.common.annotation.NonBlocking} is executed on the event loop. * + *

Inheritance of metadata

+ * 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/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 df1c88ec65e55..1d1f180fb6d96 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 @@ -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 bean = Arc.container().bean("foo1"); diff --git a/extensions/scheduler/deployment/src/test/java/io/quarkus/scheduler/test/NonStaticScheduledAbstractClassTest.java b/extensions/scheduler/deployment/src/test/java/io/quarkus/scheduler/test/NonStaticScheduledAbstractClassTest.java new file mode 100644 index 0000000000000..47c89cbfd16e6 --- /dev/null +++ b/extensions/scheduler/deployment/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/extensions/scheduler/deployment/src/test/java/io/quarkus/scheduler/test/NonStaticScheduledInterfaceTest.java b/extensions/scheduler/deployment/src/test/java/io/quarkus/scheduler/test/NonStaticScheduledInterfaceTest.java new file mode 100644 index 0000000000000..23cce9c6058a0 --- /dev/null +++ b/extensions/scheduler/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/extensions/scheduler/deployment/src/test/java/io/quarkus/scheduler/test/staticmethod/ScheduledStaticMethodTest.java b/extensions/scheduler/deployment/src/test/java/io/quarkus/scheduler/test/staticmethod/ScheduledStaticMethodTest.java index ac0777024fd5b..90355cd15979d 100644 --- a/extensions/scheduler/deployment/src/test/java/io/quarkus/scheduler/test/staticmethod/ScheduledStaticMethodTest.java +++ b/extensions/scheduler/deployment/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(); + } + } }