From 840df60b53468ba621029d9c66076270eb655359 Mon Sep 17 00:00:00 2001
From: "daniel.solis" <2894221+dsolistorres@users.noreply.github.com>
Date: Thu, 22 Aug 2024 08:23:01 -0600
Subject: [PATCH] #29478 Applying feedback

---
 .../business/api/JobQueueManagerAPIImpl.java  | 53 ++++++++++++++-----
 .../dotcms/jobs/business/job/AbstractJob.java | 13 ++---
 2 files changed, 45 insertions(+), 21 deletions(-)

diff --git a/dotCMS/src/main/java/com/dotcms/jobs/business/api/JobQueueManagerAPIImpl.java b/dotCMS/src/main/java/com/dotcms/jobs/business/api/JobQueueManagerAPIImpl.java
index 69f3fae9eaf5..e6176128c93f 100644
--- a/dotCMS/src/main/java/com/dotcms/jobs/business/api/JobQueueManagerAPIImpl.java
+++ b/dotCMS/src/main/java/com/dotcms/jobs/business/api/JobQueueManagerAPIImpl.java
@@ -84,10 +84,41 @@ public class JobQueueManagerAPIImpl implements JobQueueManagerAPI {
     private final Map<String, RetryStrategy> retryStrategies;
     private final RetryStrategy defaultRetryStrategy;
 
+    // The number of threads to use for job processing.
     static final int DEFAULT_THREAD_POOL_SIZE = Config.getIntProperty(
             "JOB_QUEUE_THREAD_POOL_SIZE", 10
     );
 
+    // The number of failures that will cause the circuit to open
+    static final int DEFAULT_CIRCUIT_BREAKER_FAILURE_THRESHOLD = Config.getIntProperty(
+            "DEFAULT_CIRCUIT_BREAKER_FAILURE_THRESHOLD", 5
+    );
+
+    // The time in milliseconds after which to attempt to close the circuit
+    static final int DEFAULT_CIRCUIT_BREAKER_RESET_TIMEOUT = Config.getIntProperty(
+            "DEFAULT_CIRCUIT_BREAKER_RESET_TIMEOUT", 60000
+    );
+
+    // The initial delay between retries in milliseconds
+    static final int DEFAULT_RETRY_STRATEGY_INITIAL_DELAY = Config.getIntProperty(
+            "DEFAULT_RETRY_STRATEGY_INITIAL_DELAY", 1000
+    );
+
+    // The maximum delay between retries in milliseconds
+    static final int DEFAULT_RETRY_STRATEGY_MAX_DELAY = Config.getIntProperty(
+            "DEFAULT_RETRY_STRATEGY_MAX_DELAY", 60000
+    );
+
+    // The factor by which the delay increases with each retry
+    static final float DEFAULT_RETRY_STRATEGY_BACK0FF_FACTOR = Config.getFloatProperty(
+            "DEFAULT_RETRY_STRATEGY_BACK0FF_FACTOR", 2.0f
+    );
+
+    // The maximum number of retry attempts allowed
+    static final int DEFAULT_RETRY_STRATEGY_MAX_RETRIES = Config.getIntProperty(
+            "DEFAULT_RETRY_STRATEGY_MAX_RETRIES", 5
+    );
+
     /**
      * Constructs a new JobQueueManagerAPIImpl with the default job queue implementation and the default
      * number of threads.
@@ -106,17 +137,12 @@ public JobQueueManagerAPIImpl() {
      */
     @VisibleForTesting
     public JobQueueManagerAPIImpl(JobQueue jobQueue, int threadPoolSize) {
-        this.jobQueue = jobQueue;
-        this.threadPoolSize = threadPoolSize;
-        this.processors = new ConcurrentHashMap<>();
-        this.jobWatchers = new ConcurrentHashMap<>();
-        this.retryStrategies = new ConcurrentHashMap<>();
-        this.defaultRetryStrategy = new ExponentialBackoffRetryStrategy(
-                1000, 60000, 2.0, 5
+        this(jobQueue, threadPoolSize,
+                new CircuitBreaker(
+                        DEFAULT_CIRCUIT_BREAKER_FAILURE_THRESHOLD,
+                        DEFAULT_CIRCUIT_BREAKER_RESET_TIMEOUT
+                )
         );
-        this.circuitBreaker = new CircuitBreaker(
-                5, 60000
-        ); // 5 failures within 1 minute
     }
 
     /**
@@ -134,7 +160,10 @@ public JobQueueManagerAPIImpl(JobQueue jobQueue, int threadPoolSize, CircuitBrea
         this.jobWatchers = new ConcurrentHashMap<>();
         this.retryStrategies = new ConcurrentHashMap<>();
         this.defaultRetryStrategy = new ExponentialBackoffRetryStrategy(
-                1000, 60000, 2.0, 5
+                DEFAULT_RETRY_STRATEGY_INITIAL_DELAY,
+                DEFAULT_RETRY_STRATEGY_MAX_DELAY,
+                DEFAULT_RETRY_STRATEGY_BACK0FF_FACTOR,
+                DEFAULT_RETRY_STRATEGY_MAX_RETRIES
         );
         this.circuitBreaker = circuitBreaker;
     }
@@ -535,7 +564,7 @@ private RetryStrategy retryStrategy(final String queueName) {
      */
     private boolean canRetry(final Job job) {
         final RetryStrategy retryStrategy = retryStrategy(job.queueName());
-        return retryStrategy.shouldRetry(job, job.lastException());
+        return retryStrategy.shouldRetry(job, job.lastException().orElse(null));
     }
 
     /**
diff --git a/dotCMS/src/main/java/com/dotcms/jobs/business/job/AbstractJob.java b/dotCMS/src/main/java/com/dotcms/jobs/business/job/AbstractJob.java
index 4295586631fe..149942d5a1e1 100644
--- a/dotCMS/src/main/java/com/dotcms/jobs/business/job/AbstractJob.java
+++ b/dotCMS/src/main/java/com/dotcms/jobs/business/job/AbstractJob.java
@@ -5,7 +5,6 @@
 import java.time.LocalDateTime;
 import java.util.Map;
 import java.util.Optional;
-import javax.annotation.Nullable;
 import org.immutables.value.Value;
 import org.immutables.value.Value.Default;
 
@@ -26,11 +25,9 @@ public interface AbstractJob {
 
     JobState state();
 
-    @Nullable
-    LocalDateTime createdAt();
+    Optional<LocalDateTime> createdAt();
 
-    @Nullable
-    LocalDateTime updatedAt();
+    Optional<LocalDateTime> updatedAt();
 
     Optional<LocalDateTime> completedAt();
 
@@ -38,11 +35,9 @@ public interface AbstractJob {
 
     Map<String, Object> parameters();
 
-    @Nullable
-    Throwable lastException();
+    Optional<Throwable> lastException();
 
-    @Nullable
-    com.dotcms.jobs.business.error.ErrorDetail errorDetail();
+    Optional<com.dotcms.jobs.business.error.ErrorDetail> errorDetail();
 
     @Default
     default int retryCount() {