Validate input and Use default values if not set

4 files changed, 82 insertions, 60 deletions

diff --git a/flags/src/main/java/com/yahoo/vespa/flags/custom/ClusterCapacity.java b/flags/src/main/java/com/yahoo/vespa/flags/custom/ClusterCapacity.java
index fc5f2456a77..dcef85f9a0d 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/custom/ClusterCapacity.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/custom/ClusterCapacity.java
@@ -8,9 +8,14 @@ import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
 import com.fasterxml.jackson.annotation.JsonInclude;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import java.util.Objects;
-import java.util.Optional;
 import java.util.OptionalDouble;
 
+import static com.yahoo.vespa.flags.custom.Validation.requireNonNegative;
+import static com.yahoo.vespa.flags.custom.Validation.validArchitectures;
+import static com.yahoo.vespa.flags.custom.Validation.validDiskSpeeds;
+import static com.yahoo.vespa.flags.custom.Validation.validStorageTypes;
+import static com.yahoo.vespa.flags.custom.Validation.validateEnum;
+
 /**
  * @author freva
  */
@@ -23,10 +28,9 @@ public class ClusterCapacity {
     private final OptionalDouble memoryGb;
     private final OptionalDouble diskGb;
     private final OptionalDouble bandwidthGbps;
-    private final Optional<String> diskSpeed;
-    private final Optional<String> storageType;
-    private final Optional<String> architecture;
-
+    private final String diskSpeed;
+    private final String storageType;
+    private final String architecture;
 
     @JsonCreator
     public ClusterCapacity(@JsonProperty("count") int count,
@@ -42,15 +46,15 @@ public class ClusterCapacity {
         this.memoryGb = memoryGb == null ? OptionalDouble.empty() : OptionalDouble.of(requireNonNegative("memoryGb", memoryGb));
         this.diskGb = diskGb == null ? OptionalDouble.empty() : OptionalDouble.of(requireNonNegative("diskGb", diskGb));
         this.bandwidthGbps = bandwidthGbps == null ? OptionalDouble.empty() : OptionalDouble.of(bandwidthGbps);
-        this.diskSpeed = Optional.ofNullable(diskSpeed);
-        this.storageType = Optional.ofNullable(storageType);
-        this.architecture = Optional.ofNullable(architecture);
+        this.diskSpeed = validateEnum("diskSpeed", validDiskSpeeds, diskSpeed == null ? "fast" : diskSpeed);
+        this.storageType = validateEnum("storageType", validStorageTypes, storageType == null ? "any" : storageType);
+        this.architecture = validateEnum("architecture", validArchitectures, architecture == null ? "x86_64" : architecture);
     }
 
     /** Returns a new ClusterCapacity equal to {@code this}, but with the given count. */
     public ClusterCapacity withCount(int count) {
         return new ClusterCapacity(count, vcpuOrNull(), memoryGbOrNull(), diskGbOrNull(), bandwidthGbpsOrNull(),
-                                   diskSpeedOrNull(), storageTypeOrNull(), architectureOrNull());
+                                   diskSpeed, storageType, architecture);
     }
 
     @JsonGetter("count") public int count() { return count; }
@@ -66,23 +70,14 @@ public class ClusterCapacity {
     @JsonGetter("bandwidthGbps") public Double bandwidthGbpsOrNull() {
         return bandwidthGbps.isPresent() ? bandwidthGbps.getAsDouble() : null;
     }
-    @JsonGetter("diskSpeed") public String diskSpeedOrNull() {
-        return diskSpeed.orElse(null);
-    }
-    @JsonGetter("storageType") public String storageTypeOrNull() {
-        return storageType.orElse(null);
-    }
-    @JsonGetter("architecture") public String architectureOrNull() {
-        return architecture.orElse(null);
-    }
+    @JsonGetter("diskSpeed") public String diskSpeed() { return diskSpeed; }
+    @JsonGetter("storageType") public String storageType() { return storageType; }
+    @JsonGetter("architecture") public String architecture() { return architecture; }
 
     @JsonIgnore public Double vcpu() { return vcpu.orElse(0.0); }
     @JsonIgnore public Double memoryGb() { return memoryGb.orElse(0.0); }
     @JsonIgnore public Double diskGb() { return diskGb.orElse(0.0); }
     @JsonIgnore public double bandwidthGbps() { return bandwidthGbps.orElse(1.0); }
-    @JsonIgnore public String diskSpeed() { return diskSpeed.orElse("fast"); }
-    @JsonIgnore public String storageType() { return storageType.orElse("any"); }
-    @JsonIgnore public String architecture() { return architecture.orElse("x86_64"); }
 
     @Override
     public String toString() {
@@ -118,10 +113,4 @@ public class ClusterCapacity {
         return Objects.hash(count, vcpu, memoryGb, diskGb, bandwidthGbps, diskSpeed, storageType, architecture);
     }
 
-    private static double requireNonNegative(String name, double value) {
-        if (value < 0)
-            throw new IllegalArgumentException("'" + name + "' must be positive, was " + value);
-        return value;
-    }
-
 }
diff --git a/flags/src/main/java/com/yahoo/vespa/flags/custom/HostResources.java b/flags/src/main/java/com/yahoo/vespa/flags/custom/HostResources.java
index 6b2cac39065..b7a1fd0be32 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/custom/HostResources.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/custom/HostResources.java
@@ -5,10 +5,15 @@ import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
 import com.fasterxml.jackson.annotation.JsonProperty;
-
 import java.util.Objects;
 import java.util.Optional;
-import java.util.Set;
+
+import static com.yahoo.vespa.flags.custom.Validation.requirePositive;
+import static com.yahoo.vespa.flags.custom.Validation.validArchitectures;
+import static com.yahoo.vespa.flags.custom.Validation.validClusterTypes;
+import static com.yahoo.vespa.flags.custom.Validation.validDiskSpeeds;
+import static com.yahoo.vespa.flags.custom.Validation.validStorageTypes;
+import static com.yahoo.vespa.flags.custom.Validation.validateEnum;
 
 /**
  * The advertised node resources of a host, similar to config-provision's NodeResources,
@@ -18,10 +23,6 @@ import java.util.Set;
  */
 @JsonIgnoreProperties(ignoreUnknown = true)
 public class HostResources {
-    private static final Set<String> validDiskSpeeds = Set.of("slow", "fast");
-    private static final Set<String> validStorageTypes = Set.of("remote", "local");
-    private static final Set<String> validClusterTypes = Set.of("container", "content", "combined", "admin");
-    private static final Set<String> validArchitectures = Set.of("arm64", "x86_64");
 
     private final double vcpu;
     private final double memoryGb;
@@ -91,32 +92,6 @@ public class HostResources {
         return this.clusterType.map(clusterType::equalsIgnoreCase).orElse(true);
     }
 
-    private static double requirePositive(String name, Double value) {
-        requireNonNull(name, value);
-        if (value <= 0)
-            throw new IllegalArgumentException("'" + name + "' must be positive, was " + value);
-        return value;
-    }
-
-    private static int requirePositive(String name, Integer value) {
-        requireNonNull(name, value);
-        if (value <= 0)
-            throw new IllegalArgumentException("'" + name + "' must be positive, was " + value);
-        return value;
-    }
-
-    private static String validateEnum(String name, Set<String> validValues, String value) {
-        requireNonNull(name, value);
-        if (!validValues.contains(value))
-            throw new IllegalArgumentException("Invalid " + name + ", valid values are: " +
-                    validValues + ", got: " + value);
-        return value;
-    }
-
-    private static <T> T requireNonNull(String name, T value) {
-        return Objects.requireNonNull(value, () -> "'" + name + "' has not been specified");
-    }
-
     @Override
     public String toString() {
         return "HostResources{" +
diff --git a/flags/src/main/java/com/yahoo/vespa/flags/custom/Validation.java b/flags/src/main/java/com/yahoo/vespa/flags/custom/Validation.java
new file mode 100644
index 00000000000..7e831aa5046
--- /dev/null
+++ b/flags/src/main/java/com/yahoo/vespa/flags/custom/Validation.java
@@ -0,0 +1,45 @@
+// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.flags.custom;
+
+import java.util.Objects;
+import java.util.Set;
+
+public class Validation {
+    static final Set<String> validDiskSpeeds = Set.of("slow", "fast", "any");
+    static final Set<String> validStorageTypes = Set.of("remote", "local", "any");
+    static final Set<String> validClusterTypes = Set.of("container", "content", "combined", "admin");
+    static final Set<String> validArchitectures = Set.of("arm64", "x86_64", "any");
+
+    static double requirePositive(String name, Double value) {
+        requireNonNull(name, value);
+        if (value <= 0)
+            throw new IllegalArgumentException("'" + name + "' must be positive, was " + value);
+        return value;
+    }
+
+    static int requirePositive(String name, Integer value) {
+        requireNonNull(name, value);
+        if (value <= 0)
+            throw new IllegalArgumentException("'" + name + "' must be positive, was " + value);
+        return value;
+    }
+
+    static double requireNonNegative(String name, double value) {
+        if (value < 0)
+            throw new IllegalArgumentException("'" + name + "' must be positive, was " + value);
+        return value;
+    }
+
+    static String validateEnum(String name, Set<String> validValues, String value) {
+        requireNonNull(name, value);
+        if (!validValues.contains(value))
+            throw new IllegalArgumentException("Invalid " + name + ", valid values are: " +
+                                                       validValues + ", got: " + value);
+        return value;
+    }
+
+    private static <T> T requireNonNull(String name, T value) {
+        return Objects.requireNonNull(value, () -> "'" + name + "' has not been specified");
+    }
+
+}
diff --git a/flags/src/test/java/com/yahoo/vespa/flags/custom/ClusterCapacityTest.java b/flags/src/test/java/com/yahoo/vespa/flags/custom/ClusterCapacityTest.java
index 471e458846b..23ab3a48ffa 100644
--- a/flags/src/test/java/com/yahoo/vespa/flags/custom/ClusterCapacityTest.java
+++ b/flags/src/test/java/com/yahoo/vespa/flags/custom/ClusterCapacityTest.java
@@ -51,7 +51,7 @@ public class ClusterCapacityTest {
         ClusterCapacity clusterCapacity = new ClusterCapacity(7, null, null, null, null, null, null, null);
         ObjectMapper mapper = new ObjectMapper();
         String json = mapper.writeValueAsString(clusterCapacity);
-        assertEquals("{\"count\":7}", json);
+        assertEquals("{\"count\":7,\"diskSpeed\":\"fast\",\"storageType\":\"any\",\"architecture\":\"x86_64\"}", json);
 
         ClusterCapacity deserialized = mapper.readValue(json, ClusterCapacity.class);
         assertEquals(7, deserialized.count());
@@ -62,6 +62,19 @@ public class ClusterCapacityTest {
         assertEquals("fast", deserialized.diskSpeed());
         assertEquals("any", deserialized.storageType());
         assertEquals("x86_64", deserialized.architecture());
+
+
+        // Test that using no values for diskSpeed, storageType and architecture will give expected values (the default values)
+        var input = "{\"count\":7}";
+        deserialized = mapper.readValue(input, ClusterCapacity.class);
+        assertEquals(7, deserialized.count());
+        assertEquals(0.0, deserialized.vcpu(), 0.0001);
+        assertEquals(0.0, deserialized.memoryGb(), 0.0001);
+        assertEquals(0.0, deserialized.diskGb(), 0.0001);
+        assertEquals(1.0, deserialized.bandwidthGbps(), 0.0001);  // 1.0 is used as fallback
+        assertEquals("fast", deserialized.diskSpeed());
+        assertEquals("any", deserialized.storageType());
+        assertEquals("x86_64", deserialized.architecture());
     }
 
 }
\ No newline at end of file