Scheduler - a more defensive design of SchedulerContext synthetic bean

mkouba · mkouba · commit 662458ae130d · 2023-03-02T09:24:31.000+01:00
- SchedulerContext#getScheduledMethods() should return immutable
metadata; it's an internal bean but can still be injected and modified
diff --git a/extensions/quartz/runtime/src/main/java/io/quarkus/quartz/runtime/QuartzSchedulerImpl.java b/extensions/quartz/runtime/src/main/java/io/quarkus/quartz/runtime/QuartzSchedulerImpl.java
@@ -74,7 +74,7 @@
 import io.quarkus.scheduler.common.runtime.AbstractJobDefinition;
 import io.quarkus.scheduler.common.runtime.DefaultInvoker;
 import io.quarkus.scheduler.common.runtime.ScheduledInvoker;
-import io.quarkus.scheduler.common.runtime.ScheduledMethodMetadata;
+import io.quarkus.scheduler.common.runtime.ScheduledMethod;
 import io.quarkus.scheduler.common.runtime.SchedulerContext;
 import io.quarkus.scheduler.common.runtime.SyntheticScheduled;
 import io.quarkus.scheduler.common.runtime.util.SchedulerUtils;
@@ -183,7 +183,7 @@ public QuartzSchedulerImpl(SchedulerContext context, QuartzSupport quartzSupport
                 if (transaction != null) {
                     transaction.begin();
                 }
-                for (ScheduledMethodMetadata method : context.getScheduledMethods()) {
+                for (ScheduledMethod method : context.getScheduledMethods()) {
                     int nameSequence = 0;
 
                     for (Scheduled scheduled : method.getSchedules()) {
diff --git a/extensions/scheduler/common/src/main/java/io/quarkus/scheduler/common/runtime/ImmutableScheduledMethod.java b/extensions/scheduler/common/src/main/java/io/quarkus/scheduler/common/runtime/ImmutableScheduledMethod.java
@@ -0,0 +1,39 @@
+package io.quarkus.scheduler.common.runtime;
+
+import java.util.List;
+import java.util.Objects;
+
+import io.quarkus.scheduler.Scheduled;
+
+public final class ImmutableScheduledMethod implements ScheduledMethod {
+
+    private final String invokerClassName;
+    private final String declaringClassName;
+    private final String methodName;
+    private final List<Scheduled> schedules;
+
+    public ImmutableScheduledMethod(String invokerClassName, String declaringClassName, String methodName,
+            List<Scheduled> schedules) {
+        this.invokerClassName = Objects.requireNonNull(invokerClassName);
+        this.declaringClassName = Objects.requireNonNull(declaringClassName);
+        this.methodName = Objects.requireNonNull(methodName);
+        this.schedules = List.copyOf(schedules);
+    }
+
+    public String getInvokerClassName() {
+        return invokerClassName;
+    }
+
+    public String getDeclaringClassName() {
+        return declaringClassName;
+    }
+
+    public String getMethodName() {
+        return methodName;
+    }
+
+    public List<Scheduled> getSchedules() {
+        return schedules;
+    }
+
+}
diff --git a/extensions/scheduler/common/src/main/java/io/quarkus/scheduler/common/runtime/MutableScheduledMethod.java b/extensions/scheduler/common/src/main/java/io/quarkus/scheduler/common/runtime/MutableScheduledMethod.java
@@ -4,7 +4,8 @@
 
 import io.quarkus.scheduler.Scheduled;
 
-public class ScheduledMethodMetadata {
+// This class is mutable so that it can be serialized in a recorder method
+public class MutableScheduledMethod implements ScheduledMethod {
 
     private String invokerClassName;
     private String declaringClassName;
@@ -19,10 +20,6 @@ public void setInvokerClassName(String invokerClassName) {
         this.invokerClassName = invokerClassName;
     }
 
-    public String getMethodDescription() {
-        return declaringClassName + "#" + methodName;
-    }
-
     public String getDeclaringClassName() {
         return declaringClassName;
     }
diff --git a/extensions/scheduler/common/src/main/java/io/quarkus/scheduler/common/runtime/ScheduledMethod.java b/extensions/scheduler/common/src/main/java/io/quarkus/scheduler/common/runtime/ScheduledMethod.java
@@ -0,0 +1,25 @@
+package io.quarkus.scheduler.common.runtime;
+
+import java.util.List;
+
+import io.quarkus.scheduler.Scheduled;
+
+/**
+ * Scheduled method metadata.
+ *
+ */
+public interface ScheduledMethod {
+
+    String getInvokerClassName();
+
+    String getDeclaringClassName();
+
+    String getMethodName();
+
+    List<Scheduled> getSchedules();
+
+    default String getMethodDescription() {
+        return getDeclaringClassName() + "#" + getMethodName();
+    }
+
+}
diff --git a/extensions/scheduler/common/src/main/java/io/quarkus/scheduler/common/runtime/SchedulerContext.java b/extensions/scheduler/common/src/main/java/io/quarkus/scheduler/common/runtime/SchedulerContext.java
@@ -9,7 +9,7 @@ public interface SchedulerContext {
 
     CronType getCronType();
 
-    List<ScheduledMethodMetadata> getScheduledMethods();
+    List<ScheduledMethod> getScheduledMethods();
 
     @SuppressWarnings("unchecked")
     default ScheduledInvoker createInvoker(String invokerClassName) {
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
@@ -83,7 +83,7 @@
 import io.quarkus.scheduler.ScheduledExecution;
 import io.quarkus.scheduler.Scheduler;
 import io.quarkus.scheduler.common.runtime.DefaultInvoker;
-import io.quarkus.scheduler.common.runtime.ScheduledMethodMetadata;
+import io.quarkus.scheduler.common.runtime.MutableScheduledMethod;
 import io.quarkus.scheduler.common.runtime.SchedulerContext;
 import io.quarkus.scheduler.common.runtime.util.SchedulerUtils;
 import io.quarkus.scheduler.runtime.SchedulerConfig;
@@ -279,7 +279,7 @@ public FeatureBuildItem build(SchedulerConfig config, BuildProducer<SyntheticBea
             BuildProducer<GeneratedClassBuildItem> generatedClasses, BuildProducer<ReflectiveClassBuildItem> reflectiveClass,
             AnnotationProxyBuildItem annotationProxy) {
 
-        List<ScheduledMethodMetadata> scheduledMetadata = new ArrayList<>();
+        List<MutableScheduledMethod> scheduledMetadata = new ArrayList<>();
         ClassOutput classOutput = new GeneratedClassGizmoAdaptor(generatedClasses, new Function<String, String>() {
             @Override
             public String apply(String name) {
@@ -296,9 +296,9 @@ public String apply(String name) {
         });
 
         for (ScheduledBusinessMethodItem scheduledMethod : scheduledMethods) {
-            ScheduledMethodMetadata metadata = new ScheduledMethodMetadata();
+            MutableScheduledMethod metadata = new MutableScheduledMethod();
             String invokerClass = generateInvoker(scheduledMethod, classOutput);
-            reflectiveClass.produce(new ReflectiveClassBuildItem(false, false, invokerClass));
+            reflectiveClass.produce(ReflectiveClassBuildItem.builder(invokerClass).constructors().methods().fields().build());
             metadata.setInvokerClassName(invokerClass);
             List<Scheduled> schedules = new ArrayList<>();
             for (AnnotationInstance scheduled : scheduledMethod.getSchedules()) {
diff --git a/extensions/scheduler/runtime/src/main/java/io/quarkus/scheduler/runtime/SchedulerRecorder.java b/extensions/scheduler/runtime/src/main/java/io/quarkus/scheduler/runtime/SchedulerRecorder.java
@@ -1,19 +1,24 @@
 package io.quarkus.scheduler.runtime;
 
+import java.util.ArrayList;
 import java.util.List;
 import java.util.function.Supplier;
 
 import com.cronutils.model.CronType;
 
 import io.quarkus.runtime.annotations.Recorder;
-import io.quarkus.scheduler.common.runtime.ScheduledMethodMetadata;
+import io.quarkus.scheduler.common.runtime.ImmutableScheduledMethod;
+import io.quarkus.scheduler.common.runtime.MutableScheduledMethod;
+import io.quarkus.scheduler.common.runtime.ScheduledMethod;
 import io.quarkus.scheduler.common.runtime.SchedulerContext;
 
 @Recorder
 public class SchedulerRecorder {
 
     public Supplier<Object> createContext(SchedulerConfig config,
-            List<ScheduledMethodMetadata> scheduledMethods) {
+            List<MutableScheduledMethod> scheduledMethods) {
+        // Defensive design - make an immutable copy of the scheduled method metadata
+        List<ScheduledMethod> metadata = immutableCopy(scheduledMethods);
         return new Supplier<Object>() {
             @Override
             public Object get() {
@@ -25,11 +30,20 @@ public CronType getCronType() {
                     }
 
                     @Override
-                    public List<ScheduledMethodMetadata> getScheduledMethods() {
-                        return scheduledMethods;
+                    public List<ScheduledMethod> getScheduledMethods() {
+                        return metadata;
                     }
                 };
             }
         };
     }
+
+    private List<ScheduledMethod> immutableCopy(List<MutableScheduledMethod> scheduledMethods) {
+        List<ScheduledMethod> metadata = new ArrayList<>(scheduledMethods.size());
+        for (ScheduledMethod scheduledMethod : scheduledMethods) {
+            metadata.add(new ImmutableScheduledMethod(scheduledMethod.getInvokerClassName(),
+                    scheduledMethod.getDeclaringClassName(), scheduledMethod.getMethodName(), scheduledMethod.getSchedules()));
+        }
+        return List.copyOf(metadata);
+    }
 }
diff --git a/extensions/scheduler/runtime/src/main/java/io/quarkus/scheduler/runtime/SimpleScheduler.java b/extensions/scheduler/runtime/src/main/java/io/quarkus/scheduler/runtime/SimpleScheduler.java
@@ -50,7 +50,7 @@
 import io.quarkus.scheduler.common.runtime.AbstractJobDefinition;
 import io.quarkus.scheduler.common.runtime.DefaultInvoker;
 import io.quarkus.scheduler.common.runtime.ScheduledInvoker;
-import io.quarkus.scheduler.common.runtime.ScheduledMethodMetadata;
+import io.quarkus.scheduler.common.runtime.ScheduledMethod;
 import io.quarkus.scheduler.common.runtime.SchedulerContext;
 import io.quarkus.scheduler.common.runtime.SkipConcurrentExecutionInvoker;
 import io.quarkus.scheduler.common.runtime.SkipPredicateInvoker;
@@ -126,7 +126,7 @@ public void run() {
         }
 
         // Create triggers and invokers for @Scheduled methods
-        for (ScheduledMethodMetadata method : context.getScheduledMethods()) {
+        for (ScheduledMethod method : context.getScheduledMethods()) {
             int nameSequence = 0;
             for (Scheduled scheduled : method.getSchedules()) {
                 nameSequence++;
diff --git a/extensions/scheduler/runtime/src/main/java/io/quarkus/scheduler/runtime/devconsole/SchedulerDevConsoleRecorder.java b/extensions/scheduler/runtime/src/main/java/io/quarkus/scheduler/runtime/devconsole/SchedulerDevConsoleRecorder.java
@@ -14,7 +14,7 @@
 import io.quarkus.scheduler.Scheduler;
 import io.quarkus.scheduler.Trigger;
 import io.quarkus.scheduler.common.runtime.ScheduledInvoker;
-import io.quarkus.scheduler.common.runtime.ScheduledMethodMetadata;
+import io.quarkus.scheduler.common.runtime.ScheduledMethod;
 import io.quarkus.scheduler.common.runtime.SchedulerContext;
 import io.quarkus.scheduler.common.runtime.util.SchedulerUtils;
 import io.quarkus.vertx.core.runtime.context.VertxContextSafetyToggle;
@@ -80,7 +80,7 @@ protected void handlePost(RoutingContext ctx, MultiMap form) throws Exception {
                 } else {
                     String name = form.get("name");
                     SchedulerContext context = Arc.container().instance(SchedulerContext.class).get();
-                    for (ScheduledMethodMetadata metadata : context.getScheduledMethods()) {
+                    for (ScheduledMethod metadata : context.getScheduledMethods()) {
                         if (metadata.getMethodDescription().equals(name)) {
                             Vertx vertx = Arc.container().instance(Vertx.class).get();
                             Context vdc = VertxContext.getOrCreateDuplicatedContext(vertx);
diff --git a/extensions/scheduler/runtime/src/main/java/io/quarkus/scheduler/runtime/devui/SchedulerJsonRPCService.java b/extensions/scheduler/runtime/src/main/java/io/quarkus/scheduler/runtime/devui/SchedulerJsonRPCService.java
@@ -15,7 +15,7 @@
 import io.quarkus.scheduler.Scheduler;
 import io.quarkus.scheduler.Trigger;
 import io.quarkus.scheduler.common.runtime.ScheduledInvoker;
-import io.quarkus.scheduler.common.runtime.ScheduledMethodMetadata;
+import io.quarkus.scheduler.common.runtime.ScheduledMethod;
 import io.quarkus.scheduler.common.runtime.SchedulerContext;
 import io.quarkus.scheduler.common.runtime.util.SchedulerUtils;
 import io.quarkus.vertx.core.runtime.context.VertxContextSafetyToggle;
@@ -47,7 +47,7 @@ public class SchedulerJsonRPCService {
     public Multi<JsonObject> streamRunningStatus() {
         SchedulerContext sc = context.get();
         Set<String> identities = new HashSet<>();
-        for (ScheduledMethodMetadata metadata : sc.getScheduledMethods()) {
+        for (ScheduledMethod metadata : sc.getScheduledMethods()) {
             for (Scheduled scheduled : metadata.getSchedules()) {
                 if (!scheduled.identity().isBlank()) {
                     identities.add(scheduled.identity());
@@ -69,7 +69,7 @@ public Multi<JsonObject> streamRunningStatus() {
     public JsonArray getScheduledMethods() {
         JsonArray methodsJson = new JsonArray();
         SchedulerContext c = context.get();
-        for (ScheduledMethodMetadata metadata : c.getScheduledMethods()) {
+        for (ScheduledMethod metadata : c.getScheduledMethods()) {
             JsonObject methodJson = new JsonObject();
             methodJson.put("declaringClassName", metadata.getDeclaringClassName());
             methodJson.put("methodName", metadata.getMethodName());
@@ -150,7 +150,7 @@ public JsonObject resumeJob(String identity) {
 
     public JsonObject executeJob(String methodDescription) {
         SchedulerContext c = context.get();
-        for (ScheduledMethodMetadata metadata : c.getScheduledMethods()) {
+        for (ScheduledMethod metadata : c.getScheduledMethods()) {
             if (metadata.getMethodDescription().equals(methodDescription)) {
                 Context vdc = VertxContext.getOrCreateDuplicatedContext(vertx.get());
                 VertxContextSafetyToggle.setContextSafe(vdc, true);
