Include flag id in flag data and other review fixes

6 files changed, 31 insertions, 20 deletions

diff --git a/flags/pom.xml b/flags/pom.xml
index ade598556de..8a60a8c27ae 100644
--- a/flags/pom.xml
+++ b/flags/pom.xml
@@ -56,14 +56,6 @@
                 <artifactId>bundle-plugin</artifactId>
                 <extensions>true</extensions>
             </plugin>
-            <plugin>
-                <groupId>org.apache.maven.plugins</groupId>
-                <artifactId>maven-compiler-plugin</artifactId>
-                <configuration>
-                    <source>8</source>
-                    <target>8</target>
-                </configuration>
-            </plugin>
         </plugins>
     </build>
 </project>
diff --git a/flags/src/main/java/com/yahoo/vespa/flags/FileFlagSource.java b/flags/src/main/java/com/yahoo/vespa/flags/FileFlagSource.java
index 3403a15d7ff..f4e23144449 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/FileFlagSource.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/FileFlagSource.java
@@ -57,11 +57,11 @@ public class FileFlagSource implements FlagSource {
             // version 2: File contains FileResolver as a JSON (which may contain many values, one for each rule)
             // version 1 files should probably be discontinued
             Rule rule = new Rule(Optional.of(JsonNodeRawFlag.fromJson(v1String.get())), Collections.emptyList());
-            return new FlagData(new FetchVector(), Collections.singletonList(rule));
+            return new FlagData(flagId, new FetchVector(), Collections.singletonList(rule));
         }
 
         // Will eventually resolve to empty RawFlag
-        return new FlagData();
+        return new FlagData(flagId);
     }
 
     private Optional<String> getString(FlagId id, String suffix) {
diff --git a/flags/src/main/java/com/yahoo/vespa/flags/json/FlagData.java b/flags/src/main/java/com/yahoo/vespa/flags/json/FlagData.java
index a73033a9314..bfd959e82f9 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/json/FlagData.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/json/FlagData.java
@@ -3,6 +3,7 @@ package com.yahoo.vespa.flags.json;
 
 import com.fasterxml.jackson.databind.JsonNode;
 import com.yahoo.vespa.flags.FetchVector;
+import com.yahoo.vespa.flags.FlagId;
 import com.yahoo.vespa.flags.FlagSource;
 import com.yahoo.vespa.flags.RawFlag;
 import com.yahoo.vespa.flags.json.wire.WireFlagData;
@@ -15,6 +16,7 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.stream.Collectors;
 
@@ -26,22 +28,28 @@ import java.util.stream.Collectors;
  */
 @Immutable
 public class FlagData {
+    private final FlagId id;
     private final List<Rule> rules;
     private final FetchVector defaultFetchVector;
 
-    public FlagData() {
-        this(new FetchVector(), Collections.emptyList());
+    public FlagData(FlagId id) {
+        this(id, new FetchVector(), Collections.emptyList());
     }
 
-    public FlagData(FetchVector defaultFetchVector, Rule... rules) {
-        this(defaultFetchVector, Arrays.asList(rules));
+    public FlagData(FlagId id, FetchVector defaultFetchVector, Rule... rules) {
+        this(id, defaultFetchVector, Arrays.asList(rules));
     }
 
-    public FlagData(FetchVector defaultFetchVector, List<Rule> rules) {
+    public FlagData(FlagId id, FetchVector defaultFetchVector, List<Rule> rules) {
+        this.id = id;
         this.rules = Collections.unmodifiableList(new ArrayList<>(rules));
         this.defaultFetchVector = defaultFetchVector;
     }
 
+    public FlagId id() {
+        return id;
+    }
+
     public Optional<RawFlag> resolve(FetchVector fetchVector) {
         return rules.stream()
                 .filter(rule -> rule.match(defaultFetchVector.with(fetchVector)))
@@ -68,6 +76,8 @@ public class FlagData {
     private WireFlagData toWire() {
         WireFlagData wireFlagData = new WireFlagData();
 
+        wireFlagData.id = id.toString();
+
         if (!rules.isEmpty()) {
             wireFlagData.rules = rules.stream().map(Rule::toWire).collect(Collectors.toList());
         }
@@ -91,7 +101,9 @@ public class FlagData {
 
     private static FlagData fromWire(WireFlagData wireFlagData) {
         return new FlagData(
-                FetchVectorHelper.fromWire(wireFlagData.defaultFetchVector), rulesFromWire(wireFlagData.rules)
+                new FlagId(Objects.requireNonNull(wireFlagData.id)),
+                FetchVectorHelper.fromWire(wireFlagData.defaultFetchVector),
+                rulesFromWire(wireFlagData.rules)
         );
     }
 
diff --git a/flags/src/main/java/com/yahoo/vespa/flags/json/wire/WireFlagData.java b/flags/src/main/java/com/yahoo/vespa/flags/json/wire/WireFlagData.java
index b4a000d7b70..8e21de3e5ad 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/json/wire/WireFlagData.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/json/wire/WireFlagData.java
@@ -20,6 +20,7 @@ import static com.yahoo.yolean.Exceptions.uncheck;
 @JsonIgnoreProperties(ignoreUnknown = true)
 @JsonInclude(JsonInclude.Include.NON_NULL)
 public class WireFlagData {
+    @JsonProperty("id") public String id;
     @JsonProperty("rules") public List<WireRule> rules;
     @JsonProperty("attributes") public Map<String, String> defaultFetchVector;
 
diff --git a/flags/src/test/java/com/yahoo/vespa/flags/json/FlagDataTest.java b/flags/src/test/java/com/yahoo/vespa/flags/json/FlagDataTest.java
index 2eb12e53ddc..9eaa4ae4504 100644
--- a/flags/src/test/java/com/yahoo/vespa/flags/json/FlagDataTest.java
+++ b/flags/src/test/java/com/yahoo/vespa/flags/json/FlagDataTest.java
@@ -16,6 +16,7 @@ import static org.junit.Assert.assertTrue;
  */
 public class FlagDataTest {
     private final String json = "{\n" +
+            "    \"id\": \"id1\",\n" +
             "    \"rules\": [\n" +
             "        {\n" +
             "            \"conditions\": [\n" +
@@ -69,6 +70,7 @@ public class FlagDataTest {
 
     private void verify(Optional<String> expectedValue, FetchVector vector) {
         FlagData data = FlagData.deserialize(json);
+        assertEquals("id1", data.id().toString());
         Optional<RawFlag> rawFlag = data.resolve(vector);
 
         if (expectedValue.isPresent()) {
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 f3f8c147212..7836eb702b1 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
@@ -23,18 +23,20 @@ import static org.hamcrest.Matchers.nullValue;
 public class SerializationTest {
     @Test
     public void emptyJson() throws IOException {
-        String json = "{}";
+        String json = "{\"id\":\"id1\"}";
         WireFlagData wireData = WireFlagData.deserialize(json);
+        assertThat(wireData.id, equalTo("id1"));
         assertThat(wireData.defaultFetchVector, nullValue());
         assertThat(wireData.rules, nullValue());
         assertThat(wireData.serializeToJson(), equalTo(json));
 
-        assertThat(FlagData.deserialize(json).serializeToJson(), equalTo("{}"));
+        assertThat(FlagData.deserialize(json).serializeToJson(), equalTo(json));
     }
 
     @Test
     public void deserialization() throws IOException {
         String json = "{\n" +
+                "    \"id\": \"id2\",\n" +
                 "    \"rules\": [\n" +
                 "        {\n" +
                 "            \"conditions\": [\n" +
@@ -61,6 +63,7 @@ public class SerializationTest {
 
         WireFlagData wireData = WireFlagData.deserialize(json);
 
+        assertThat(wireData.id, equalTo("id2"));
         // rule
         assertThat(wireData.rules.size(), equalTo(1));
         assertThat(wireData.rules.get(0).andConditions.size(), equalTo(2));
@@ -95,6 +98,7 @@ public class SerializationTest {
     @Test
     public void jsonWithStrayFields() {
         String json = "{\n" +
+                "    \"id\": \"id3\",\n" +
                 "    \"foo\": true,\n" +
                 "    \"rules\": [\n" +
                 "        {\n" +
@@ -123,8 +127,8 @@ public class SerializationTest {
         assertThat(wireData.rules.get(0).value, nullValue());
         assertThat(wireData.defaultFetchVector, anEmptyMap());
 
-        assertThat(wireData.serializeToJson(), equalTo("{\"rules\":[{\"conditions\":[{\"type\":\"whitelist\",\"dimension\":\"zone\"}]}],\"attributes\":{}}"));
+        assertThat(wireData.serializeToJson(), equalTo("{\"id\":\"id3\",\"rules\":[{\"conditions\":[{\"type\":\"whitelist\",\"dimension\":\"zone\"}]}],\"attributes\":{}}"));
 
-        assertThat(FlagData.deserialize(json).serializeToJson(), equalTo("{\"rules\":[{\"conditions\":[{\"type\":\"whitelist\",\"dimension\":\"zone\"}]}]}"));
+        assertThat(FlagData.deserialize(json).serializeToJson(), equalTo("{\"id\":\"id3\",\"rules\":[{\"conditions\":[{\"type\":\"whitelist\",\"dimension\":\"zone\"}]}]}"));
     }
 }