Document HOSTNAME and make factories

9 files changed, 38 insertions, 26 deletions

diff --git a/configserver-flags/src/test/java/com/yahoo/vespa/configserver/flags/db/FlagsDbImplTest.java b/configserver-flags/src/test/java/com/yahoo/vespa/configserver/flags/db/FlagsDbImplTest.java
index e366c012a9e..4f2d5ae13d4 100644
--- a/configserver-flags/src/test/java/com/yahoo/vespa/configserver/flags/db/FlagsDbImplTest.java
+++ b/configserver-flags/src/test/java/com/yahoo/vespa/configserver/flags/db/FlagsDbImplTest.java
@@ -33,7 +33,7 @@ public class FlagsDbImplTest {
         FlagsDbImpl db = new FlagsDbImpl(curator);
 
         var params = new Condition.CreateParams(FetchVector.Dimension.HOSTNAME, List.of("host1"), Optional.empty());
-        Condition condition1 = new WhitelistCondition(params);
+        Condition condition1 = WhitelistCondition.create(params);
         Rule rule1 = new Rule(Optional.of(JsonNodeRawFlag.fromJson("13")), condition1);
         FlagId flagId = new FlagId("id");
         FlagData data = new FlagData(flagId, new FetchVector().with(FetchVector.Dimension.ZONE_ID, "zone-a"), rule1);
diff --git a/flags/src/main/java/com/yahoo/vespa/flags/FetchVector.java b/flags/src/main/java/com/yahoo/vespa/flags/FetchVector.java
index a4fbec4922f..4cce7a9bc2a 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/FetchVector.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/FetchVector.java
@@ -27,9 +27,6 @@ public class FetchVector {
         /** Value from ApplicationId::serializedForm of the form tenant:applicationName:instance. */
         APPLICATION_ID,
 
-        /** Fully qualified hostname */
-        HOSTNAME,
-
         /** Node type from com.yahoo.config.provision.NodeType::name, e.g. tenant, host, confighost, controller, etc. */
         NODE_TYPE,
 
@@ -37,24 +34,30 @@ public class FetchVector {
         CLUSTER_TYPE,
 
         /**
-         * WARNING: DO SET THIS DIMENSION FOR A FLAG
+         * Fully qualified hostname.
          *
-         * <p>ALL flags can be set differently in different zones: This dimension is ONLY useful for the controller
-         * that needs to handle multiple zones.
-         *
-         * <p>Value from ZoneId::value is of the form environment.region.
+         * <p>NOTE: There is seldom any need to set HOSTNAME, as it is always set implicitly (in {@link Flags})
+         * from {@code Defaults.getDefaults().vespaHostname()}. The hostname may e.g. be overridden when
+         * fetching flag value for a Docker container node.
          */
-        ZONE_ID,
+        HOSTNAME,
 
         /**
-         * WARNING: DO SET THIS DIMENSION FOR A FLAG
+         * Vespa version from Version::toFullString of the form Major.Minor.Micro.
          *
-         * <p>The Vespa version is always fetched implicitly from {@link com.yahoo.component.Vtag#currentVersion}.
+         * <p>NOTE: There is seldom any need to set VESPA_VERSION, as it is always set implicitly
+         * (in {@link Flags}) from {@link com.yahoo.component.Vtag#currentVersion}. The version COULD e.g.
+         * be overridden when fetching flag value for a Docker container node.
+         */
+        VESPA_VERSION,
+
+        /**
+         * Zone from ZoneId::value of the form environment.region.
          *
-         * <p>Value from Version::toFullString is of the form Major.Minor.Micro[.qualifier]. When ordering
-         * versions, note that 7.3 == 7.3.0.
+         * <p>NOTE: There is seldom any need to set ZONE_ID, as all flags are set per zone anyways. The controller
+         * could PERHAPS use this where it handles multiple zones.
          */
-        VESPA_VERSION
+        ZONE_ID
     }
 
     private final Map<Dimension, String> map;
diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
index ef5e8451650..f4b223ead82 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
@@ -237,7 +237,8 @@ public class Flags {
      *                           from the FlagSource.
      * @param <T>                The boxed type of the flag value, e.g. Boolean for flags guarding features.
      * @param <U>                The type of the unbound flag, e.g. UnboundBooleanFlag.
-     * @return An unbound flag with {@link FetchVector.Dimension#HOSTNAME HOSTNAME} environment. The ZONE environment
+     * @return An unbound flag with {@link FetchVector.Dimension#HOSTNAME HOSTNAME} and
+     *         {@link FetchVector.Dimension#VESPA_VERSION VESPA_VERSION} already set. The ZONE environment
      *         is typically implicit.
      */
     private static <T, U extends UnboundFlag<?, ?, ?>> U define(TypedUnboundFlagFactory<T, U> factory,
diff --git a/flags/src/main/java/com/yahoo/vespa/flags/json/BlacklistCondition.java b/flags/src/main/java/com/yahoo/vespa/flags/json/BlacklistCondition.java
index 22f907a7e0d..435ddbb3e19 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/json/BlacklistCondition.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/json/BlacklistCondition.java
@@ -5,7 +5,6 @@ package com.yahoo.vespa.flags.json;
  * @author hakonhall
  */
 public class BlacklistCondition extends ListCondition {
-    public BlacklistCondition(CreateParams params) {
-        super(Type.BLACKLIST, params);
-    }
+    public static  BlacklistCondition create(CreateParams params) { return new BlacklistCondition(params); }
+    private BlacklistCondition(CreateParams params) { super(Type.BLACKLIST, params); }
 }
diff --git a/flags/src/main/java/com/yahoo/vespa/flags/json/Condition.java b/flags/src/main/java/com/yahoo/vespa/flags/json/Condition.java
index e107cef8acd..96dbc8197c1 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/json/Condition.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/json/Condition.java
@@ -60,8 +60,8 @@ public interface Condition extends Predicate<FetchVector> {
         var params = new CreateParams(dimension, values, predicate);
 
         switch (type) {
-            case WHITELIST: return new WhitelistCondition(params);
-            case BLACKLIST: return new BlacklistCondition(params);
+            case WHITELIST: return WhitelistCondition.create(params);
+            case BLACKLIST: return BlacklistCondition.create(params);
             case RELATIONAL: return RelationalCondition.create(params);
         }
 
diff --git a/flags/src/main/java/com/yahoo/vespa/flags/json/ListCondition.java b/flags/src/main/java/com/yahoo/vespa/flags/json/ListCondition.java
index 948f1fc6c3e..c2c76529833 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/json/ListCondition.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/json/ListCondition.java
@@ -20,6 +20,10 @@ public abstract class ListCondition implements Condition {
         this.dimension = params.dimension();
         this.values = List.copyOf(params.values());
         this.isWhitelist = type == Type.WHITELIST;
+
+        if (params.predicate().isPresent()) {
+            throw new IllegalArgumentException(getClass().getSimpleName() + " does not support the 'predicate' field");
+        }
     }
 
     @Override
diff --git a/flags/src/main/java/com/yahoo/vespa/flags/json/RelationalCondition.java b/flags/src/main/java/com/yahoo/vespa/flags/json/RelationalCondition.java
index 996f1f925f7..afaf94b26b6 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/json/RelationalCondition.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/json/RelationalCondition.java
@@ -5,6 +5,7 @@ import com.yahoo.component.Version;
 import com.yahoo.vespa.flags.FetchVector;
 import com.yahoo.vespa.flags.json.wire.WireCondition;
 
+import java.util.List;
 import java.util.function.Predicate;
 
 /**
@@ -16,6 +17,11 @@ public class RelationalCondition implements Condition {
     private final FetchVector.Dimension dimension;
 
     public static RelationalCondition create(CreateParams params) {
+        if (!params.values().isEmpty()) {
+            throw new IllegalArgumentException(RelationalCondition.class.getSimpleName() +
+                    " does not support the 'values' field");
+        }
+
         String predicate = params.predicate().orElseThrow(() ->
                 new IllegalArgumentException(RelationalCondition.class.getSimpleName() +
                         " requires the predicate field in the condition"));
diff --git a/flags/src/main/java/com/yahoo/vespa/flags/json/WhitelistCondition.java b/flags/src/main/java/com/yahoo/vespa/flags/json/WhitelistCondition.java
index 5c3f8bd3ade..ab266fdaf1d 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/json/WhitelistCondition.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/json/WhitelistCondition.java
@@ -5,7 +5,6 @@ package com.yahoo.vespa.flags.json;
  * @author hakonhall
  */
 public class WhitelistCondition extends ListCondition {
-    public WhitelistCondition(CreateParams params) {
-        super(Type.WHITELIST, params);
-    }
+    public static WhitelistCondition create(CreateParams params) { return new WhitelistCondition(params); }
+    private WhitelistCondition(CreateParams params) { super(Type.WHITELIST, params); }
 }
diff --git a/flags/src/test/java/com/yahoo/vespa/flags/json/ConditionTest.java b/flags/src/test/java/com/yahoo/vespa/flags/json/ConditionTest.java
index cf8d06fd312..618bc86baaf 100644
--- a/flags/src/test/java/com/yahoo/vespa/flags/json/ConditionTest.java
+++ b/flags/src/test/java/com/yahoo/vespa/flags/json/ConditionTest.java
@@ -19,7 +19,7 @@ public class ConditionTest {
     public void testWhitelist() {
         String hostname1 = "host1";
         var params = new Condition.CreateParams(FetchVector.Dimension.HOSTNAME, List.of(hostname1), Optional.empty());
-        Condition condition = new WhitelistCondition(params);
+        Condition condition = WhitelistCondition.create(params);
         assertFalse(condition.test(new FetchVector()));
         assertFalse(condition.test(new FetchVector().with(FetchVector.Dimension.APPLICATION_ID, "foo")));
         assertFalse(condition.test(new FetchVector().with(FetchVector.Dimension.HOSTNAME, "bar")));
@@ -30,7 +30,7 @@ public class ConditionTest {
     public void testBlacklist() {
         String hostname1 = "host1";
         var params = new Condition.CreateParams(FetchVector.Dimension.HOSTNAME, List.of(hostname1), Optional.empty());
-        Condition condition = new BlacklistCondition(params);
+        Condition condition = BlacklistCondition.create(params);
         assertTrue(condition.test(new FetchVector()));
         assertTrue(condition.test(new FetchVector().with(FetchVector.Dimension.APPLICATION_ID, "foo")));
         assertTrue(condition.test(new FetchVector().with(FetchVector.Dimension.HOSTNAME, "bar")));