diff options
12 files changed, 71 insertions, 47 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 1fe61130348..0523090c18b 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 @@ -31,15 +31,14 @@ public class FlagsDbImplTest { Condition condition1 = new Condition(Condition.Type.WHITELIST, FetchVector.Dimension.HOSTNAME, "host1"); Rule rule1 = new Rule(Optional.of(JsonNodeRawFlag.fromJson("13")), condition1); - FlagData data = new FlagData(new FetchVector().with(FetchVector.Dimension.ZONE_ID, "zone-a"), rule1); FlagId flagId = new FlagId("id"); + FlagData data = new FlagData(flagId, new FetchVector().with(FetchVector.Dimension.ZONE_ID, "zone-a"), rule1); db.setValue(flagId, data); - assertTrue(db.getValue(flagId).isPresent()); Optional<FlagData> dataCopy = db.getValue(flagId); assertTrue(dataCopy.isPresent()); - assertEquals("{\"rules\":[{\"conditions\":[{\"type\":\"whitelist\",\"dimension\":\"hostname\"," + + assertEquals("{\"id\":\"id\",\"rules\":[{\"conditions\":[{\"type\":\"whitelist\",\"dimension\":\"hostname\"," + "\"values\":[\"host1\"]}],\"value\":13}],\"attributes\":{\"zone\":\"zone-a\"}}", dataCopy.get().serializeToJson()); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/DefinedFlags.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/DefinedFlags.java index 6d1f9f452d5..9f0e39b7d43 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/DefinedFlags.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/DefinedFlags.java @@ -6,6 +6,7 @@ import com.fasterxml.jackson.databind.node.ArrayNode; import com.fasterxml.jackson.databind.node.ObjectNode; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.jdisc.Response; +import com.yahoo.vespa.config.server.http.HttpConfigResponse; import com.yahoo.vespa.flags.FlagDefinition; import com.yahoo.vespa.flags.json.DimensionHelper; @@ -44,6 +45,6 @@ public class DefinedFlags extends HttpResponse { @Override public String getContentType() { - return "application/json"; + return HttpConfigResponse.JSON_CONTENT_TYPE; } } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagDataListResponse.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagDataListResponse.java index 4d9ba96b791..ae72660ce9f 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagDataListResponse.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagDataListResponse.java @@ -2,9 +2,11 @@ package com.yahoo.vespa.config.server.http.flags; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ArrayNode; import com.fasterxml.jackson.databind.node.ObjectNode; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.jdisc.Response; +import com.yahoo.vespa.config.server.http.HttpConfigResponse; import com.yahoo.vespa.flags.FlagId; import com.yahoo.vespa.flags.json.FlagData; @@ -22,24 +24,27 @@ public class FlagDataListResponse extends HttpResponse { private final String flagsV1Uri; private final Map<FlagId, FlagData> flags; - private final boolean showDataInsteadOfUrl; + private final boolean recursive; - public FlagDataListResponse(String flagsV1Uri, Map<FlagId, FlagData> flags, boolean showDataInsteadOfUrl) { + public FlagDataListResponse(String flagsV1Uri, Map<FlagId, FlagData> flags, boolean recursive) { super(Response.Status.OK); this.flagsV1Uri = flagsV1Uri; this.flags = flags; - this.showDataInsteadOfUrl = showDataInsteadOfUrl; + this.recursive = recursive; } @Override public void render(OutputStream outputStream) { ObjectNode rootNode = mapper.createObjectNode(); + ArrayNode flagsArray = rootNode.putArray("flags"); // Order flags by ID - new TreeMap<>(flags).forEach((flagId, flagData) -> { - if (showDataInsteadOfUrl) { - rootNode.set(flagId.toString(), flagData.toJsonNode()); + new TreeMap<>(this.flags).forEach((flagId, flagData) -> { + if (recursive) { + flagsArray.add(flagData.toJsonNode()); } else { - rootNode.putObject(flagId.toString()).put("url", flagsV1Uri + "/data/" + flagId.toString()); + ObjectNode object = flagsArray.addObject(); + object.put("id", flagId.toString()); + object.put("url", flagsV1Uri + "/data/" + flagId.toString()); } }); uncheck(() -> mapper.writeValue(outputStream, rootNode)); @@ -47,6 +52,6 @@ public class FlagDataListResponse extends HttpResponse { @Override public String getContentType() { - return "application/json"; + return HttpConfigResponse.JSON_CONTENT_TYPE; } } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagsHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagsHandler.java index 5a59c4665b7..e64c356aa96 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagsHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagsHandler.java @@ -75,9 +75,7 @@ public class FlagsHandler extends HttpHandler { private HttpResponse putFlagData(HttpRequest request, FlagId flagId) { flagsDb.setValue(flagId, FlagData.deserialize(request.getData())); - - // The set & get is not atomic, but no harm is done in showing an outdated flag value in the response. - return getFlagData(flagId); + return new OKResponse(); } private HttpResponse deleteFlagData(FlagId flagId) { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/OKResponse.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/OKResponse.java index 19363fbdadc..87c02ae56f1 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/OKResponse.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/OKResponse.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.config.server.http.flags; import com.yahoo.container.jdisc.EmptyResponse; import com.yahoo.jdisc.Response; +import com.yahoo.vespa.config.server.http.HttpConfigResponse; /** * @author hakonhall @@ -14,6 +15,6 @@ public class OKResponse extends EmptyResponse { @Override public String getContentType() { - return "application/json"; + return HttpConfigResponse.JSON_CONTENT_TYPE; } } 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 e7bf36e1778..8781ec413d2 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 @@ -67,30 +67,31 @@ public class FlagsHandlerTest { // PUT flag with ID id1 verifySuccessfulRequest(Method.PUT, "/data/" + FLAG1.id(), "{\n" + + " \"id\": \"id1\",\n" + " \"rules\": [\n" + " {\n" + " \"value\": true\n" + " }\n" + " ]\n" + "}", - "{\"rules\":[{\"value\":true}]}"); + ""); - // GET on ID id1 should return the same as the put (this will also issue a payload for the get, - // which we assume will be ignored). + // GET on ID id1 should return the same as the put. verifySuccessfulRequest(Method.GET, "/data/" + FLAG1.id(), - "", "{\"rules\":[{\"value\":true}]}"); + "", "{\"id\":\"id1\",\"rules\":[{\"value\":true}]}"); // List all flags should list only id1 verifySuccessfulRequest(Method.GET, "/data", - "", "{\"id1\":{\"url\":\"https://foo.com:4443/flags/v1/data/id1\"}}"); + "", "{\"flags\":[{\"id\":\"id1\",\"url\":\"https://foo.com:4443/flags/v1/data/id1\"}]}"); // Should be identical to above: suffix / on path should be ignored verifySuccessfulRequest(Method.GET, "/data/", - "", "{\"id1\":{\"url\":\"https://foo.com:4443/flags/v1/data/id1\"}}"); + "", "{\"flags\":[{\"id\":\"id1\",\"url\":\"https://foo.com:4443/flags/v1/data/id1\"}]}"); // PUT id2 verifySuccessfulRequest(Method.PUT, "/data/" + FLAG2.id(), "{\n" + + " \"id\": \"id2\",\n" + " \"rules\": [\n" + " {\n" + " \"conditions\": [\n" + @@ -112,34 +113,42 @@ public class FlagsHandlerTest { " \"zone\": \"zone1\"\n" + " }\n" + "}\n", - "{\"rules\":[{\"conditions\":[{\"type\":\"whitelist\",\"dimension\":\"hostname\",\"values\":[\"host1\",\"host2\"]},{\"type\":\"blacklist\",\"dimension\":\"application\",\"values\":[\"app2\",\"app1\"]}],\"value\":true}],\"attributes\":{\"zone\":\"zone1\"}}"); + ""); + + // 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\"}}"); // The list of flag data should return id1 and id2 verifySuccessfulRequest(Method.GET, "/data", "", - "{\"id1\":{\"url\":\"https://foo.com:4443/flags/v1/data/id1\"},\"id2\":{\"url\":\"https://foo.com:4443/flags/v1/data/id2\"}}"); + "{\"flags\":[{\"id\":\"id1\",\"url\":\"https://foo.com:4443/flags/v1/data/id1\"},{\"id\":\"id2\",\"url\":\"https://foo.com:4443/flags/v1/data/id2\"}]}"); // Putting (overriding) id1 should work silently verifySuccessfulRequest(Method.PUT, "/data/" + FLAG1.id(), "{\n" + + " \"id\": \"id1\",\n" + " \"rules\": [\n" + " {\n" + " \"value\": false\n" + " }\n" + " ]\n" + "}\n", - "{\"rules\":[{\"value\":false}]}"); + ""); + + // Verify PUT + verifySuccessfulRequest(Method.GET, "/data/" + FLAG1.id(), "", "{\"id\":\"id1\",\"rules\":[{\"value\":false}]}"); // Get all recursivelly displays all flag data verifySuccessfulRequest(Method.GET, "/data?recursive=true", "", - "{\"id1\":{\"rules\":[{\"value\":false}]},\"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\":[\"app2\",\"app1\"]}],\"value\":true}],\"attributes\":{\"zone\":\"zone1\"}}]}"); // Deleting both flags verifySuccessfulRequest(Method.DELETE, "/data/" + FLAG1.id(), "", ""); verifySuccessfulRequest(Method.DELETE, "/data/" + FLAG2.id(), "", ""); // And the list of data flags should now be empty - verifySuccessfulRequest(Method.GET, "/data", "", "{}"); + verifySuccessfulRequest(Method.GET, "/data", "", "{\"flags\":[]}"); } @Test 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\"}]}]}")); } } |