Fix Retry Scheduler discrepancies

- Be clear in the Javadoc about *all* defaulted and required fields, not
 just the fields defined in the implementations
- The validate() method only has to assert fields which are not
defaulted. Thus, the implementations can entirely drop this call. Added,
 this ensure that we do not miss the ScheduledRetryExecutor validation,
 which at the moment is a bug.
- Add builder tests
- Make other minor clean-up efforts where applicable

messaging/src/main/java/org/axonframework/commandhandling/gateway/AbstractRetryScheduler.java

@@ -54,7 +54,6 @@ public abstract class AbstractRetryScheduler implements RetryScheduler {
      */
     protected AbstractRetryScheduler(Builder builder) {
         builder.validate();
-
         this.retryExecutor = builder.retryExecutor;
         this.maxRetryCount = builder.maxRetryCount;
     }
@@ -142,25 +141,16 @@ public boolean scheduleRetry(CommandMessage commandMessage,
     }
 
     /**
-     * A builder class for {@link RetryScheduler}s. Values for the {@link Builder#retryExecutor} and
-     * {@link Builder#maxRetryCount} fields are both required, with the latter having a default value of
-     * {@link #DEFAULT_MAX_RETRIES}.
+     * A builder class for the {@link RetryScheduler} implementations.
+     * <p>
+     * The default for {@code maxRetryCount} is set to a single retry.
+     * The {@link ScheduledExecutorService} is a <b>hard requirement</b> and as such should be provided.
      */
     public abstract static class Builder<B extends Builder> {
 
         private ScheduledExecutorService retryExecutor;
         private int maxRetryCount = DEFAULT_MAX_RETRIES;
 
-        /**
-         * Validate the fields. This method is called in the {@link AbstractRetryScheduler}'s constructor.
-         *
-         * @throws AxonConfigurationException if validation fails.
-         */
-        protected void validate() throws AxonConfigurationException {
-            assertNonNull(retryExecutor, "The ScheduledExecutorService is a hard requirement and should be provided");
-            assertStrictPositive(maxRetryCount, "The maxRetryCount is a hard requirement and must be at least 1");
-        }
-
         /**
          * Sets the {@link ScheduledExecutorService} used to schedule a command retry.
          *
@@ -175,7 +165,7 @@ public B retryExecutor(ScheduledExecutorService retryExecutor) {
         }
 
         /**
-         * Sets the maximum number of retries allowed for a single command, defaulted to 1.
+         * Sets the maximum number of retries allowed for a single command. Defaults to 1.
          *
          * @param maxRetryCount an {@code int} specifying the maximum number of retries allowed for a single command
          * @return the current Builder instance, for fluent interfacing
@@ -186,5 +176,14 @@ public B maxRetryCount(int maxRetryCount) {
             // noinspection unchecked
             return (B) this;
         }
+
+        /**
+         * Validate the fields. This method is called in the {@link AbstractRetryScheduler}'s constructor.
+         *
+         * @throws AxonConfigurationException if validation fails.
+         */
+        protected void validate() throws AxonConfigurationException {
+            assertNonNull(retryExecutor, "The ScheduledExecutorService is a hard requirement and should be provided");
+        }
     }
 }

messaging/src/main/java/org/axonframework/commandhandling/gateway/ExponentialBackOffIntervalRetryScheduler.java

@@ -19,6 +19,7 @@
 import org.axonframework.commandhandling.CommandMessage;
 
 import java.util.List;
+import java.util.concurrent.ScheduledExecutorService;
 
 import static org.axonframework.common.BuilderUtils.assertStrictPositive;
 
@@ -51,7 +52,8 @@ protected ExponentialBackOffIntervalRetryScheduler(Builder builder) {
     /**
      * Instantiate a Builder to be able to create a {@link ExponentialBackOffIntervalRetryScheduler}.
      * <p>
-     * The default for {@link Builder#backoffFactor} is set to 100ms, and must be at least 1.
+     * The default for {@code maxRetryCount} is set to a single retry and the {@code backoffFactor} defaults to 100ms.
+     * The {@link ScheduledExecutorService} is a <b>hard requirement</b> and as such should be provided.
      *
      * @return a Builder to be able to create a {@link ExponentialBackOffIntervalRetryScheduler}
      */
@@ -68,21 +70,24 @@ protected long computeRetryInterval(CommandMessage commandMessage,
     }
 
     /**
-     * A builder class for an exponential backoff retry scheduler.
+     * Builder class to instantiate an {@link ExponentialBackOffIntervalRetryScheduler}.
      * <p>
-     * The default for {@link Builder#backoffFactor} is set to 100ms, and must be at least 1.
+     * The default for the {@code backoffFactor} is set to 100ms, and must be at least 1.
+     * The {@link ScheduledExecutorService} is a <b>hard requirement</b> and as such should be provided.
      */
     public static class Builder extends AbstractRetryScheduler.Builder<Builder> {
 
         private long backoffFactor = DEFAULT_BACKOFF_FACTOR;
 
         /**
-         * Sets the backoff factor in milliseconds at which to schedule a retry, defaulted to 100ms.
+         * Sets the backoff factor in milliseconds at which to schedule a retry. This field defaults to 100ms and is
+         * required to be a positive number.
          *
-         * @param backoffFactor an {@code int} specifying the interval in milliseconds at which to schedule a retry.
-         * @return the current Builder instance, for fluent interfacing.
+         * @param backoffFactor an {@code int} specifying the interval in milliseconds at which to schedule a retry
+         * @return the current Builder instance, for fluent interfacing
          */
         public Builder backoffFactor(long backoffFactor) {
+            assertStrictPositive(backoffFactor, "The backoffFactor must be a positive number");
             this.backoffFactor = backoffFactor;
             return this;
         }
@@ -95,12 +100,5 @@ public Builder backoffFactor(long backoffFactor) {
         public ExponentialBackOffIntervalRetryScheduler build() {
             return new ExponentialBackOffIntervalRetryScheduler(this);
         }
-
-        /**
-         * Validate the input, in this case asserting that the backoff factor is strictly positive (>= 1).
-         */
-        protected void validate() {
-            assertStrictPositive(backoffFactor, "The backoff factor is a hard requirement and must be at least 1.");
-        }
     }
 }

messaging/src/main/java/org/axonframework/commandhandling/gateway/IntervalRetryScheduler.java

@@ -18,8 +18,6 @@
 
 import org.axonframework.commandhandling.CommandMessage;
 import org.axonframework.common.AxonConfigurationException;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import java.util.List;
 import java.util.concurrent.ScheduledExecutorService;
@@ -35,8 +33,6 @@
  */
 public class IntervalRetryScheduler extends AbstractRetryScheduler {
 
-    private static final Logger logger = LoggerFactory.getLogger(IntervalRetryScheduler.class);
-
     private static final long DEFAULT_RETRY_INTERVAL = 100L;
 
     private final long retryInterval;
@@ -67,8 +63,7 @@ protected long computeRetryInterval(CommandMessage commandMessage,
      * Instantiate a Builder to be able to create a {@link IntervalRetryScheduler}.
      * <p>
      * The default for {@code retryInterval} is set to 100ms, while {@code maxRetryCount} gets a single retry.
-     * The {@code retryInterval}, {@code maxRetryCount} and {@link ScheduledExecutorService} are
-     * <b>hard requirements</b> and as such should be provided.
+     * The {@link ScheduledExecutorService} is a <b>hard requirement</b> and as such should be provided.
      *
      * @return a Builder to be able to create a {@link IntervalRetryScheduler}
      */
@@ -80,15 +75,15 @@ public static Builder builder() {
      * Builder class to instantiate a {@link IntervalRetryScheduler}.
      * <p>
      * The default for {@code retryInterval} is set to 100ms, while {@code maxRetryCount} gets a single retry.
-     * The {@code retryInterval}, {@code maxRetryCount} and {@link ScheduledExecutorService} are
-     * <b>hard requirements</b> and as such should be provided.
+     * The {@link ScheduledExecutorService} is a <b>hard requirement</b> and as such should be provided.
      */
     public static class Builder extends AbstractRetryScheduler.Builder<Builder> {
 
         private long retryInterval = DEFAULT_RETRY_INTERVAL;
 
         /**
-         * Sets the retry interval in milliseconds at which to schedule a retry, defaulted to 100ms.
+         * Sets the retry interval in milliseconds at which to schedule a retry. This field defaults to 100ms and is
+         * required to be a positive number.
          *
          * @param retryInterval an {@code int} specifying the retry interval in milliseconds at which to schedule a
          *                      retry
@@ -108,15 +103,5 @@ public Builder retryInterval(int retryInterval) {
         public IntervalRetryScheduler build() {
             return new IntervalRetryScheduler(this);
         }
-
-        /**
-         * Validates whether the fields contained in this Builder are set accordingly.
-         *
-         * @throws AxonConfigurationException if one field is asserted to be incorrect according to the Builder's
-         *                                    specifications
-         */
-        protected void validate() throws AxonConfigurationException {
-            assertPositive(retryInterval, "The retryInterval is a hard requirement and should be provided");
-        }
     }
 }

messaging/src/main/java/org/axonframework/eventhandling/ReplayStatus.java

@@ -31,7 +31,7 @@
 public enum ReplayStatus {
 
     /**
-     * Indicats the message is delivered as part of a replay (and may have been delivered before)
+     * Indicates the message is delivered as part of a replay (and may have been delivered before)
      */
     REPLAY(true),
     /**

messaging/src/test/java/org/axonframework/commandhandling/gateway/ExponentialBackOffIntervalRetrySchedulerTest.java

@@ -1,5 +1,6 @@
 package org.axonframework.commandhandling.gateway;
 
+import org.axonframework.common.AxonConfigurationException;
 import org.junit.*;
 
 import java.util.concurrent.ScheduledThreadPoolExecutor;
@@ -36,4 +37,9 @@ public void scheduleRetry() {
         assertEquals("Scheduling a retry when past maxRetryCount should have failed.",
                      0, doScheduleRetry(retryScheduler, MAX_RETRIES + 1));
     }
+
+    @Test(expected = AxonConfigurationException.class)
+    public void testBuildingWhilstMissingScheduledExecutorServiceThrowsConfigurationException() {
+        ExponentialBackOffIntervalRetryScheduler.builder().build();
+    }
 }

messaging/src/test/java/org/axonframework/commandhandling/gateway/IntervalRetrySchedulerTest.java

@@ -2,6 +2,7 @@
 
 import org.axonframework.commandhandling.CommandMessage;
 import org.axonframework.commandhandling.GenericCommandMessage;
+import org.axonframework.common.AxonConfigurationException;
 import org.axonframework.common.jdbc.JdbcException;
 import org.junit.*;
 import org.slf4j.Logger;
@@ -59,6 +60,7 @@ static long doScheduleRetry(RetryScheduler retryScheduler, int nrOfFailures) {
             Class<?>[] arr = new Class[2];
             arr[0] = JdbcException.class;
             arr[1] = NullPointerException.class;
+            //noinspection unchecked
             failures.add((Class<? extends Throwable>[]) arr);
         }
         if (retryScheduler.scheduleRetry(msg, exc, failures, after)) {
@@ -88,4 +90,9 @@ public void scheduleRetry() {
         assertEquals("Scheduling a retry when past maxRetryCount should have failed.",
                      0, doScheduleRetry(retryScheduler, MAX_RETRIES + 1));
     }
+
+    @Test(expected = AxonConfigurationException.class)
+    public void testBuildingWhilstMissingScheduledExecutorServiceThrowsConfigurationException() {
+        IntervalRetryScheduler.builder().build();
+    }
 }