diff options
author | Håkon Hallingstad <hakon@oath.com> | 2019-01-02 17:22:56 +0100 |
---|---|---|
committer | Håkon Hallingstad <hakon@oath.com> | 2019-01-02 17:22:56 +0100 |
commit | 475b59bafc01b15e97903019b0f7d3cc97ed97cd (patch) | |
tree | 6328d4af7ebbde7c55cb235817084d726e6aa6ca /configserver | |
parent | a6703b62b6c2b2c2f8e1633e78d4ea50d564dc71 (diff) |
Include flag id in flag data and other review fixes
Diffstat (limited to 'configserver')
5 files changed, 38 insertions, 24 deletions
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 |