Merge pull request #8676 from vespa-engine/hakonhall/use-list-instead-of-set-for-flags-rulesconditionsvalues

Use List instead of Set for flags rules.conditions.values

5 files changed, 14 insertions, 17 deletions

diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/flags/FlagsHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/flags/FlagsHandlerTest.java
index 78bc664d050..5ae6ce9820b 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/flags/FlagsHandlerTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/flags/FlagsHandlerTest.java
@@ -125,7 +125,7 @@ public class FlagsHandlerTest {
 
         // GET on id2 should now return what was put
         verifySuccessfulRequest(Method.GET, "/data/" + FLAG2.id(), "",
-                "{\"id\":\"id2\",\"rules\":[{\"conditions\":[{\"type\":\"whitelist\",\"dimension\":\"hostname\",\"values\":[\"host1\",\"host2\"]},{\"type\":\"blacklist\",\"dimension\":\"application\",\"values\":[\"app2\",\"app1\"]}],\"value\":true}],\"attributes\":{\"zone\":\"zone1\"}}");
+                "{\"id\":\"id2\",\"rules\":[{\"conditions\":[{\"type\":\"whitelist\",\"dimension\":\"hostname\",\"values\":[\"host1\",\"host2\"]},{\"type\":\"blacklist\",\"dimension\":\"application\",\"values\":[\"app1\",\"app2\"]}],\"value\":true}],\"attributes\":{\"zone\":\"zone1\"}}");
 
         // The list of flag data should return id1 and id2
         verifySuccessfulRequest(Method.GET, "/data",
@@ -149,7 +149,7 @@ public class FlagsHandlerTest {
 
         // Get all recursivelly displays all flag data
         verifySuccessfulRequest(Method.GET, "/data?recursive=true", "",
-                "{\"flags\":[{\"id\":\"id1\",\"rules\":[{\"value\":false}]},{\"id\":\"id2\",\"rules\":[{\"conditions\":[{\"type\":\"whitelist\",\"dimension\":\"hostname\",\"values\":[\"host1\",\"host2\"]},{\"type\":\"blacklist\",\"dimension\":\"application\",\"values\":[\"app2\",\"app1\"]}],\"value\":true}],\"attributes\":{\"zone\":\"zone1\"}}]}");
+                "{\"flags\":[{\"id\":\"id1\",\"rules\":[{\"value\":false}]},{\"id\":\"id2\",\"rules\":[{\"conditions\":[{\"type\":\"whitelist\",\"dimension\":\"hostname\",\"values\":[\"host1\",\"host2\"]},{\"type\":\"blacklist\",\"dimension\":\"application\",\"values\":[\"app1\",\"app2\"]}],\"value\":true}],\"attributes\":{\"zone\":\"zone1\"}}]}");
 
         // Deleting both flags
         verifySuccessfulRequest(Method.DELETE, "/data/" + FLAG1.id(), "", "");
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 4bc86fd96f5..a0ad08fb0b3 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
@@ -5,10 +5,8 @@ import com.yahoo.vespa.flags.FetchVector;
 import com.yahoo.vespa.flags.json.wire.WireCondition;
 
 import java.util.Arrays;
-import java.util.Collections;
-import java.util.HashSet;
+import java.util.List;
 import java.util.Objects;
-import java.util.Set;
 import java.util.function.Predicate;
 
 /**
@@ -19,13 +17,13 @@ public class Condition implements Predicate<FetchVector> {
 
     private final Type type;
     private final FetchVector.Dimension dimension;
-    private final Set<String> values;
+    private final List<String> values;
 
     public Condition(Type type, FetchVector.Dimension dimension, String... values) {
-        this(type, dimension, new HashSet<>(Arrays.asList(values)));
+        this(type, dimension, Arrays.asList(values));
     }
 
-    public Condition(Type type, FetchVector.Dimension dimension, Set<String> values) {
+    public Condition(Type type, FetchVector.Dimension dimension, List<String> values) {
         this.type = type;
         this.dimension = dimension;
         this.values = values;
@@ -49,7 +47,7 @@ public class Condition implements Predicate<FetchVector> {
         Objects.requireNonNull(wireCondition.dimension);
         FetchVector.Dimension dimension = DimensionHelper.fromWire(wireCondition.dimension);
 
-        Set<String> values = wireCondition.values == null ? Collections.emptySet() : wireCondition.values;
+        List<String> values = wireCondition.values == null ? List.of() : wireCondition.values;
 
         return new Condition(type, dimension, values);
     }
diff --git a/flags/src/main/java/com/yahoo/vespa/flags/json/wire/WireCondition.java b/flags/src/main/java/com/yahoo/vespa/flags/json/wire/WireCondition.java
index 31aa6200fdb..2020ce1e49f 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/json/wire/WireCondition.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/json/wire/WireCondition.java
@@ -5,7 +5,7 @@ import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
 import com.fasterxml.jackson.annotation.JsonInclude;
 import com.fasterxml.jackson.annotation.JsonProperty;
 
-import java.util.Set;
+import java.util.List;
 
 /**
  * @author hakonhall
@@ -15,5 +15,5 @@ import java.util.Set;
 public class WireCondition {
     @JsonProperty("type") public String type;
     @JsonProperty("dimension") public String dimension;
-    @JsonProperty("values") public Set<String> values;
+    @JsonProperty("values") public List<String> values;
 }
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 96cbce71fa8..d19442ae0f0 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
@@ -18,7 +18,7 @@ public class ConditionTest {
     public void testWhitelist() {
         String hostname1 = "host1";
         Condition condition = new Condition(Condition.Type.WHITELIST, FetchVector.Dimension.HOSTNAME,
-                Stream.of(hostname1).collect(Collectors.toSet()));
+                Stream.of(hostname1).collect(Collectors.toList()));
         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")));
@@ -29,7 +29,7 @@ public class ConditionTest {
     public void testBlacklist() {
         String hostname1 = "host1";
         Condition condition = new Condition(Condition.Type.BLACKLIST, FetchVector.Dimension.HOSTNAME,
-                Stream.of(hostname1).collect(Collectors.toSet()));
+                Stream.of(hostname1).collect(Collectors.toList()));
         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")));
diff --git a/flags/src/test/java/com/yahoo/vespa/flags/json/SerializationTest.java b/flags/src/test/java/com/yahoo/vespa/flags/json/SerializationTest.java
index b3ff2a92b07..b0e4cd0f682 100644
--- a/flags/src/test/java/com/yahoo/vespa/flags/json/SerializationTest.java
+++ b/flags/src/test/java/com/yahoo/vespa/flags/json/SerializationTest.java
@@ -8,8 +8,7 @@ import com.yahoo.vespa.flags.json.wire.WireFlagData;
 import org.junit.Test;
 
 import java.io.IOException;
-import java.util.Arrays;
-import java.util.HashSet;
+import java.util.List;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.anEmptyMap;
@@ -74,12 +73,12 @@ public class SerializationTest {
         WireCondition whitelistCondition = wireData.rules.get(0).andConditions.get(0);
         assertThat(whitelistCondition.type, equalTo("whitelist"));
         assertThat(whitelistCondition.dimension, equalTo("application"));
-        assertThat(whitelistCondition.values, equalTo(new HashSet<>(Arrays.asList("a1", "a2"))));
+        assertThat(whitelistCondition.values, equalTo(List.of("a1", "a2")));
         // second condition
         WireCondition blacklistCondition = wireData.rules.get(0).andConditions.get(1);
         assertThat(blacklistCondition.type, equalTo("blacklist"));
         assertThat(blacklistCondition.dimension, equalTo("hostname"));
-        assertThat(blacklistCondition.values, equalTo(new HashSet<>(Arrays.asList("h1"))));
+        assertThat(blacklistCondition.values, equalTo(List.of("h1")));
 
         // attributes
         assertThat(wireData.defaultFetchVector, notNullValue());