[FIX] consistent validation and version comparison fix

src/main/java/io/nats/client/api/KeyValueConfiguration.java

@@ -85,16 +85,26 @@ public Republish getRepublish() {
 
     /**
      * Creates a builder for the Key Value Configuration.
-     * @return a key value configuration builder
+     * @return a KeyValueConfiguration Builder
      */
     public static Builder builder() {
-        return new Builder();
+        return new Builder((KeyValueConfiguration)null);
+    }
+
+
+    /**
+     * Creates a builder for the Key Value Configuration.
+     * @param name the name of the key value bucket
+     * @return a KeyValueConfiguration Builder
+     */
+    public static Builder builder(String name) {
+        return new Builder(name);
     }
 
     /**
      * Creates a builder to copy the key value configuration.
      * @param kvc an existing KeyValueConfiguration
-     * @return a KeyValueConfiguration builder
+     * @return a KeyValueConfiguration Builder
      */
     public static Builder builder(KeyValueConfiguration kvc) {
         return new Builder(kvc);
@@ -108,17 +118,25 @@ public static Builder builder(KeyValueConfiguration kvc) {
      *
      */
     public static class Builder {
-
         String name;
-        StreamConfiguration.Builder scBuilder;
         Mirror mirror;
-        List<Source> sources = new ArrayList<>();
+        final List<Source> sources = new ArrayList<>();
+        final StreamConfiguration.Builder scBuilder;
 
         /**
          * Default Builder
          */
         public Builder() {
-            this(null);
+            this((KeyValueConfiguration)null);
+        }
+
+        /**
+         * Builder accepting the key value bucket name.
+         * @param name name of the key value bucket.
+         */
+        public Builder(String name) {
+            this((KeyValueConfiguration)null);
+            name(name);
         }
 
         /**
@@ -138,12 +156,12 @@ public Builder(KeyValueConfiguration kvc) {
         }
 
         /**
-         * Sets the name of the store.
-         * @param name name of the store.
+         * Sets the name of the key value bucket.
+         * @param name name of the key value bucket.
          * @return the builder
          */
         public Builder name(String name) {
-            this.name = name;
+            this.name = validateBucketName(name, true);
             return this;
         }
 
@@ -213,7 +231,7 @@ public Builder storageType(StorageType storageType) {
          * @return Builder
          */
         public Builder replicas(int replicas) {
-            scBuilder.replicas(Math.max(replicas, 1));
+            scBuilder.replicas(replicas);
             return this;
         }
 
@@ -312,7 +330,7 @@ public Builder addSources(Collection<Source> sources) {
          * @return the KeyValueConfiguration.
          */
         public KeyValueConfiguration build() {
-            name = validateBucketName(name, true);
+            name = required(name, "name");
             scBuilder.name(toStreamName(name))
                 .allowRollup(true)
                 .allowDirect(true) // by design

src/main/java/io/nats/client/api/ObjectStoreConfiguration.java

@@ -17,8 +17,7 @@
 import java.time.Duration;
 
 import static io.nats.client.support.NatsObjectStoreUtil.*;
-import static io.nats.client.support.Validator.validateBucketName;
-import static io.nats.client.support.Validator.validateMaxBucketBytes;
+import static io.nats.client.support.Validator.*;
 
 /**
  * The ObjectStoreStatus class contains information about an object store.
@@ -54,20 +53,27 @@ public String toString() {
             '}';
     }
 
+    /**
+     * Creates a builder for the Object Store Configuration.
+     * @return an ObjectStoreConfiguration Builder
+     */
+    public static ObjectStoreConfiguration.Builder builder() {
+        return new ObjectStoreConfiguration.Builder((ObjectStoreConfiguration)null);
+    }
 
     /**
      * Creates a builder for the Object Store Configuration.
      * @param name the name of the object store bucket
-     * @return an Object Store configuration builder
+     * @return an ObjectStoreConfiguration Builder
      */
     public static ObjectStoreConfiguration.Builder builder(String name) {
-        return new ObjectStoreConfiguration.Builder().name(name);
+        return new ObjectStoreConfiguration.Builder(name);
     }
 
     /**
      * Creates a builder to copy the Object Store configuration.
      * @param osc an existing ObjectStoreConfiguration
-     * @return an ObjectStoreConfiguration builder
+     * @return an ObjectStoreConfiguration Builder
      */
     public static ObjectStoreConfiguration.Builder builder(ObjectStoreConfiguration osc) {
         return new ObjectStoreConfiguration.Builder(osc);
@@ -81,15 +87,23 @@ public static ObjectStoreConfiguration.Builder builder(ObjectStoreConfiguration
      *
      */
     public static class Builder {
-
         String name;
         StreamConfiguration.Builder scBuilder;
 
         /**
          * Default Builder
          */
         public Builder() {
-            this(null);
+            this((ObjectStoreConfiguration)null);
+        }
+
+        /**
+         * Builder accepting the object store bucket name.
+         * @param name name of the store.
+         */
+        public Builder(String name) {
+            this((ObjectStoreConfiguration)null);
+            name(name);
         }
 
         /**
@@ -112,7 +126,7 @@ public Builder(ObjectStoreConfiguration osc) {
          * @param name name of the store.
          * @return the builder
          */
-        public ObjectStoreConfiguration.Builder name(String name) {
+        public Builder name(String name) {
             this.name = validateBucketName(name, true);
             return this;
         }
@@ -122,7 +136,7 @@ public ObjectStoreConfiguration.Builder name(String name) {
          * @param description description of the store.
          * @return the builder
          */
-        public ObjectStoreConfiguration.Builder description(String description) {
+        public Builder description(String description) {
             scBuilder.description(description);
             return this;
         }
@@ -132,7 +146,7 @@ public ObjectStoreConfiguration.Builder description(String description) {
          * @param maxBucketSize the maximum number of bytes
          * @return Builder
          */
-        public ObjectStoreConfiguration.Builder maxBucketSize(long maxBucketSize) {
+        public Builder maxBucketSize(long maxBucketSize) {
             scBuilder.maxBytes(validateMaxBucketBytes(maxBucketSize));
             return this;
         }
@@ -142,7 +156,7 @@ public ObjectStoreConfiguration.Builder maxBucketSize(long maxBucketSize) {
          * @param ttl the maximum age
          * @return Builder
          */
-        public ObjectStoreConfiguration.Builder ttl(Duration ttl) {
+        public Builder ttl(Duration ttl) {
             scBuilder.maxAge(ttl);
             return this;
         }
@@ -152,7 +166,7 @@ public ObjectStoreConfiguration.Builder ttl(Duration ttl) {
          * @param storageType the storage type
          * @return Builder
          */
-        public ObjectStoreConfiguration.Builder storageType(StorageType storageType) {
+        public Builder storageType(StorageType storageType) {
             scBuilder.storageType(storageType);
             return this;
         }
@@ -162,8 +176,8 @@ public ObjectStoreConfiguration.Builder storageType(StorageType storageType) {
          * @param replicas the number of replicas
          * @return Builder
          */
-        public ObjectStoreConfiguration.Builder replicas(int replicas) {
-            scBuilder.replicas(Math.max(replicas, 1));
+        public Builder replicas(int replicas) {
+            scBuilder.replicas(replicas);
             return this;
         }
 
@@ -172,7 +186,7 @@ public ObjectStoreConfiguration.Builder replicas(int replicas) {
          * @param placement the placement directive object
          * @return Builder
          */
-        public ObjectStoreConfiguration.Builder placement(Placement placement) {
+        public Builder placement(Placement placement) {
             scBuilder.placement(placement);
             return this;
         }
@@ -182,7 +196,7 @@ public ObjectStoreConfiguration.Builder placement(Placement placement) {
          * @return the ObjectStoreConfiguration.
          */
         public ObjectStoreConfiguration build() {
-            name = validateBucketName(name, true);
+            name = required(name, "name");
             scBuilder.name(toStreamName(name))
                 .subjects(toMetaStreamSubject(name), toChunkStreamSubject(name))
                 .allowRollup(true)

src/main/java/io/nats/client/api/ServerInfo.java

@@ -140,15 +140,32 @@ public String getCluster() {
     private String getComparableVersion(String vString) {
         try {
             String[] v = vString.replaceAll("v", "").replaceAll("-", ".").split("\\Q.\\E");
-            int at = vString.indexOf("-");
-            return "" + (Integer.parseInt(v[0]) * 10000) + (Integer.parseInt(v[1]) * 100) + Integer.parseInt(v[2])
-                    + (at == -1 ? "~" : vString.substring(at));
+            return padded(v[0]) + padded(v[1]) + padded(v[2]) + normalExtra(vString);
         }
         catch (NumberFormatException nfe) {
             return "";
         }
     }
 
+    private static String padded(String vcomp) {
+        int x = Integer.parseInt(vcomp);
+        if (x < 10) {
+            return "000" + x;
+        }
+        if (x < 100) {
+            return "00" + x;
+        }
+        if (x < 1000) {
+            return "0" + x;
+        }
+        return "" + x;
+    }
+
+    private static String normalExtra(String vString) {
+        int at = vString.indexOf("-");
+        return at == -1 ? "~" : vString.substring(at).toLowerCase();
+    }
+
     public boolean isNewerVersionThan(String vTarget) {
         return getComparableVersion(version).compareTo(getComparableVersion(vTarget)) > 0;
     }

src/main/java/io/nats/client/api/StreamConfiguration.java

@@ -692,6 +692,7 @@ public Builder storageType(StorageType storageType) {
 
         /**
          * Sets the number of replicas a message must be stored on in the StreamConfiguration.
+         * Must be 1 to 5 inclusive
          * @param replicas the number of replicas to store this message on
          * @return Builder
          */
@@ -732,7 +733,7 @@ public Builder discardPolicy(DiscardPolicy policy) {
         }
 
         /**
-         * Sets the duplicate checking window in the the StreamConfiguration.  A Duration.Zero
+         * Sets the duplicate checking window in the StreamConfiguration.  A Duration.Zero
          * disables duplicate checking.  Duplicate checking is disabled by default.
          * @param window duration to hold message ids for duplicate checking.
          * @return Builder
@@ -743,7 +744,7 @@ public Builder duplicateWindow(Duration window) {
         }
 
         /**
-         * Sets the duplicate checking window in the the StreamConfiguration.  A Duration.Zero
+         * Sets the duplicate checking window in the StreamConfiguration.  A Duration.Zero
          * disables duplicate checking.  Duplicate checking is disabled by default.
          * @param windowMillis duration to hold message ids for duplicate checking.
          * @return Builder

src/main/java/io/nats/client/support/Validator.java

@@ -213,11 +213,8 @@ public static long validateMaxMessagesPerSubject(long max) {
     }
 
     public static int validateMaxHistory(int max) {
-        if (max < 2) {
-            return 1;
-        }
-        if (max > MAX_HISTORY_PER_KEY) {
-            throw new IllegalArgumentException("Max History Per Key cannot be more than " + MAX_HISTORY_PER_KEY);
+        if (max < 1 || max > MAX_HISTORY_PER_KEY) {
+            throw new IllegalArgumentException("Max History must be from 1 to " + MAX_HISTORY_PER_KEY + " inclusive.");
         }
         return max;
     }

src/test/java/io/nats/client/api/KeyValueConfigurationTests.java

@@ -17,6 +17,7 @@
 
 import java.time.Duration;
 
+import static io.nats.client.support.NatsConstants.EMPTY;
 import static org.junit.jupiter.api.Assertions.*;
 
 public class KeyValueConfigurationTests extends JetStreamTestBase {
@@ -71,4 +72,33 @@ private void validate(KeyValueConfiguration kvc) {
 
         assertTrue(kvc.toString().contains("bucketName"));
     }
+
+    @Test
+    public void testConstructionInvalidsCoverage() {
+        assertThrows(IllegalArgumentException.class, () -> KeyValueConfiguration.builder().build());
+        assertThrows(IllegalArgumentException.class, () -> KeyValueConfiguration.builder().name(null));
+        assertThrows(IllegalArgumentException.class, () -> KeyValueConfiguration.builder().name(EMPTY));
+        assertThrows(IllegalArgumentException.class, () -> KeyValueConfiguration.builder().name(HAS_SPACE));
+        assertThrows(IllegalArgumentException.class, () -> KeyValueConfiguration.builder().name(HAS_PRINTABLE));
+        assertThrows(IllegalArgumentException.class, () -> KeyValueConfiguration.builder().name(HAS_DOT));
+        assertThrows(IllegalArgumentException.class, () -> KeyValueConfiguration.builder().name(HAS_STAR));
+        assertThrows(IllegalArgumentException.class, () -> KeyValueConfiguration.builder().name(HAS_GT));
+        assertThrows(IllegalArgumentException.class, () -> KeyValueConfiguration.builder().name(HAS_DOLLAR));
+        assertThrows(IllegalArgumentException.class, () -> KeyValueConfiguration.builder().name(HAS_LOW));
+        assertThrows(IllegalArgumentException.class, () -> KeyValueConfiguration.builder(HAS_127));
+        assertThrows(IllegalArgumentException.class, () -> KeyValueConfiguration.builder(HAS_FWD_SLASH));
+        assertThrows(IllegalArgumentException.class, () -> KeyValueConfiguration.builder(HAS_BACK_SLASH));
+        assertThrows(IllegalArgumentException.class, () -> KeyValueConfiguration.builder(HAS_EQUALS));
+        assertThrows(IllegalArgumentException.class, () -> KeyValueConfiguration.builder(HAS_TIC));
+        assertThrows(IllegalArgumentException.class, () -> KeyValueConfiguration.builder().maxHistoryPerKey(0));
+        assertThrows(IllegalArgumentException.class, () -> KeyValueConfiguration.builder().maxHistoryPerKey(-1));
+        assertThrows(IllegalArgumentException.class, () -> KeyValueConfiguration.builder().maxHistoryPerKey(65));
+        assertThrows(IllegalArgumentException.class, () -> KeyValueConfiguration.builder().maxBucketSize(0));
+        assertThrows(IllegalArgumentException.class, () -> KeyValueConfiguration.builder().maxBucketSize(-2));
+        assertThrows(IllegalArgumentException.class, () -> KeyValueConfiguration.builder().maxValueSize(0));
+        assertThrows(IllegalArgumentException.class, () -> KeyValueConfiguration.builder().maxValueSize(-2));
+        assertThrows(IllegalArgumentException.class, () -> KeyValueConfiguration.builder().ttl(Duration.ofNanos(-1)));
+        assertThrows(IllegalArgumentException.class, () -> KeyValueConfiguration.builder().replicas(0));
+        assertThrows(IllegalArgumentException.class, () -> KeyValueConfiguration.builder().replicas(6));
+    }
 }

src/test/java/io/nats/client/api/ObjectStoreApiTests.java

@@ -22,6 +22,7 @@
 import java.util.ArrayList;
 import java.util.List;
 
+import static io.nats.client.support.NatsConstants.EMPTY;
 import static io.nats.client.utils.ResourceUtils.dataAsString;
 import static org.junit.jupiter.api.Assertions.*;
 
@@ -316,4 +317,28 @@ private void infoCoverage(ObjectLink link1, ObjectLink link2) {
             assertTrue(info.hashCode() != 0); // coverage
         }
     }
+
+    @Test
+    public void testConstructionInvalidsCoverage() {
+        assertThrows(IllegalArgumentException.class, () -> ObjectStoreConfiguration.builder().build());
+        assertThrows(IllegalArgumentException.class, () -> ObjectStoreConfiguration.builder().name(null));
+        assertThrows(IllegalArgumentException.class, () -> ObjectStoreConfiguration.builder().name(EMPTY));
+        assertThrows(IllegalArgumentException.class, () -> ObjectStoreConfiguration.builder().name(HAS_SPACE));
+        assertThrows(IllegalArgumentException.class, () -> ObjectStoreConfiguration.builder().name(HAS_PRINTABLE));
+        assertThrows(IllegalArgumentException.class, () -> ObjectStoreConfiguration.builder().name(HAS_DOT));
+        assertThrows(IllegalArgumentException.class, () -> ObjectStoreConfiguration.builder().name(HAS_STAR));
+        assertThrows(IllegalArgumentException.class, () -> ObjectStoreConfiguration.builder().name(HAS_GT));
+        assertThrows(IllegalArgumentException.class, () -> ObjectStoreConfiguration.builder().name(HAS_DOLLAR));
+        assertThrows(IllegalArgumentException.class, () -> ObjectStoreConfiguration.builder().name(HAS_LOW));
+        assertThrows(IllegalArgumentException.class, () -> ObjectStoreConfiguration.builder(HAS_127));
+        assertThrows(IllegalArgumentException.class, () -> ObjectStoreConfiguration.builder(HAS_FWD_SLASH));
+        assertThrows(IllegalArgumentException.class, () -> ObjectStoreConfiguration.builder(HAS_BACK_SLASH));
+        assertThrows(IllegalArgumentException.class, () -> ObjectStoreConfiguration.builder(HAS_EQUALS));
+        assertThrows(IllegalArgumentException.class, () -> ObjectStoreConfiguration.builder(HAS_TIC));
+        assertThrows(IllegalArgumentException.class, () -> ObjectStoreConfiguration.builder().maxBucketSize(0));
+        assertThrows(IllegalArgumentException.class, () -> ObjectStoreConfiguration.builder().maxBucketSize(-2));
+        assertThrows(IllegalArgumentException.class, () -> ObjectStoreConfiguration.builder().ttl(Duration.ofNanos(-1)));
+        assertThrows(IllegalArgumentException.class, () -> ObjectStoreConfiguration.builder().replicas(0));
+        assertThrows(IllegalArgumentException.class, () -> ObjectStoreConfiguration.builder().replicas(6));
+    }
 }