diff options
author | Bjørn Christian Seime <bjorncs@verizonmedia.com> | 2019-12-03 13:48:27 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-12-03 13:48:27 +0100 |
commit | fec2553bc3dc96382e4cb4cd9e7da9e567a1023f (patch) | |
tree | baecfb08aeb140cb31445b60a8fb6c712a7f59d5 | |
parent | 919ff19010a0510aff29db5e142095ef05d4b72b (diff) | |
parent | a537998c3b43d1ad36e07ec3baeb0252b0d6263f (diff) |
Merge pull request #11483 from vespa-engine/bjorncs/system-flags-improvements
Bjorncs/system flags improvements
11 files changed, 97 insertions, 30 deletions
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java index 83e3c03ffaa..7e9da53b1c9 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java @@ -9,6 +9,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.io.UncheckedIOException; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -48,10 +49,8 @@ public class SystemFlagsDataArchive { String name = entry.getName(); if (!entry.isDirectory() && name.startsWith("flags/") && name.endsWith(".json")) { Path filePath = Paths.get(name); - String filename = filePath.getFileName().toString(); - FlagData flagData = FlagData.deserializeUtf8Json(zipIn.readAllBytes()); - verifyFlagDataMatchesDirectoryName(filePath, flagData); - builder.addFile(filename, flagData); + String rawData = new String(zipIn.readAllBytes(), StandardCharsets.UTF_8); + addFile(builder, rawData, filePath); } } return builder.build(); @@ -70,13 +69,10 @@ public class SystemFlagsDataArchive { Builder builder = new Builder(); directoryStream.forEach(absolutePath -> { Path relativePath = root.relativize(absolutePath); - if (!Files.isDirectory(absolutePath) && relativePath.startsWith("flags")) { - String filename = relativePath.getFileName().toString(); - if (filename.endsWith(".json")) { - FlagData flagData = FlagData.deserializeUtf8Json(uncheck(() -> Files.readAllBytes(absolutePath))); - verifyFlagDataMatchesDirectoryName(relativePath, flagData); - builder.addFile(filename, flagData); - } + if (!Files.isDirectory(absolutePath) && + relativePath.startsWith("flags") && relativePath.toString().endsWith(".json")) { + String rawData = uncheck(() -> Files.readString(absolutePath, StandardCharsets.UTF_8)); + addFile(builder, rawData, relativePath); } }); return builder.build(); @@ -106,20 +102,31 @@ public class SystemFlagsDataArchive { for (String filename : filenames) { FlagData data = fileMap.get(filename); if (data != null) { - targetData.add(data); - break; + if (!data.isEmpty()) { + targetData.add(data); + } + return; } } }); return targetData; } - private static void verifyFlagDataMatchesDirectoryName(Path filePath, FlagData flagData) { - String flagDirectoryName = filePath.getName(1).toString(); - if (!flagDirectoryName.equals(flagData.id().toString())) { - throw new IllegalArgumentException( - String.format("Flag data file with flag id '%s' in directory for '%s'", flagData.id(), flagDirectoryName)); + private static void addFile(Builder builder, String rawData, Path filePath) { + String filename = filePath.getFileName().toString(); + FlagId directoryDeducedFlagId = new FlagId(filePath.getName(1).toString()); + FlagData flagData; + if (rawData.isBlank()) { + flagData = new FlagData(directoryDeducedFlagId); + } else { + flagData = FlagData.deserialize(rawData); + if (!directoryDeducedFlagId.equals(flagData.id())) { + throw new IllegalArgumentException( + String.format("Flag data file with flag id '%s' in directory for '%s'", + flagData.id(), directoryDeducedFlagId.toString())); + } } + builder.addFile(filename, flagData); } public static class Builder { diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java index 35fec04e4c5..14584650d3b 100644 --- a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java +++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java @@ -8,6 +8,7 @@ import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.athenz.api.AthenzService; import com.yahoo.vespa.flags.FetchVector; +import com.yahoo.vespa.flags.FlagId; import com.yahoo.vespa.flags.RawFlag; import com.yahoo.vespa.flags.json.FlagData; import org.junit.Rule; @@ -24,9 +25,10 @@ import java.io.InputStream; import java.io.OutputStream; import java.net.URI; import java.nio.file.Paths; +import java.util.List; import java.util.Map; -import java.util.Set; +import static java.util.stream.Collectors.toList; import static org.assertj.core.api.Assertions.assertThat; /** @@ -35,6 +37,8 @@ import static org.assertj.core.api.Assertions.assertThat; public class SystemFlagsDataArchiveTest { private static final SystemName SYSTEM = SystemName.main; + private static final FlagId MY_TEST_FLAG = new FlagId("my-test-flag"); + private static final FlagId FLAG_WITH_EMPTY_DATA = new FlagId("flag-with-empty-data"); @Rule public final TemporaryFolder temporaryFolder = new TemporaryFolder(); @@ -61,29 +65,49 @@ public class SystemFlagsDataArchiveTest { } try (InputStream in = new BufferedInputStream(new FileInputStream(tempFile))) { SystemFlagsDataArchive archive = SystemFlagsDataArchive.fromZip(in); - assertArchiveReturnsCorrectDataForTarget(archive); + assertArchiveReturnsCorrectTestFlagDataForTarget(archive); } } @Test public void retrieves_correct_flag_data_for_target() { var archive = SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags/")); - assertArchiveReturnsCorrectDataForTarget(archive); + assertArchiveReturnsCorrectTestFlagDataForTarget(archive); } - private static void assertArchiveReturnsCorrectDataForTarget(SystemFlagsDataArchive archive) { - assertFlagDataHasValue(archive, mainControllerTarget, "main.controller"); - assertFlagDataHasValue(archive, prodUsWestCfgTarget, "main.prod.us-west-1.json"); - assertFlagDataHasValue(archive, prodUsEast3CfgTarget, "main.prod"); - assertFlagDataHasValue(archive, devUsEast1CfgTarget, "main"); + @Test + public void empty_files_are_handled_as_no_flag_data_for_target() { + var archive = SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags/")); + assertNoFlagData(archive, FLAG_WITH_EMPTY_DATA, mainControllerTarget); + assertFlagDataHasValue(archive, FLAG_WITH_EMPTY_DATA, prodUsWestCfgTarget, "main.prod.us-west-1"); + assertNoFlagData(archive, FLAG_WITH_EMPTY_DATA, prodUsEast3CfgTarget); + assertFlagDataHasValue(archive, FLAG_WITH_EMPTY_DATA, devUsEast1CfgTarget, "main"); + } + + private static void assertArchiveReturnsCorrectTestFlagDataForTarget(SystemFlagsDataArchive archive) { + assertFlagDataHasValue(archive, MY_TEST_FLAG, mainControllerTarget, "main.controller"); + assertFlagDataHasValue(archive, MY_TEST_FLAG, prodUsWestCfgTarget, "main.prod.us-west-1"); + assertFlagDataHasValue(archive, MY_TEST_FLAG, prodUsEast3CfgTarget, "main.prod"); + assertFlagDataHasValue(archive, MY_TEST_FLAG, devUsEast1CfgTarget, "main"); } - private static void assertFlagDataHasValue(SystemFlagsDataArchive archive, FlagsTarget target, String value) { - Set<FlagData> data = archive.flagData(target); + private static void assertFlagDataHasValue(SystemFlagsDataArchive archive, FlagId flagId, FlagsTarget target, String value) { + List<FlagData> data = getData(archive, flagId, target); assertThat(data).hasSize(1); - FlagData flagData = data.iterator().next(); + FlagData flagData = data.get(0); RawFlag rawFlag = flagData.resolve(FetchVector.fromMap(Map.of())).get(); assertThat(rawFlag.asJson()).isEqualTo(String.format("\"%s\"", value)); } + private static void assertNoFlagData(SystemFlagsDataArchive archive, FlagId flagId, FlagsTarget target) { + List<FlagData> data = getData(archive, flagId, target); + assertThat(data).isEmpty(); + } + + private static List<FlagData> getData(SystemFlagsDataArchive archive, FlagId flagId, FlagsTarget target) { + return archive.flagData(target).stream() + .filter(d -> d.id().equals(flagId)) + .collect(toList()); + } + }
\ No newline at end of file diff --git a/controller-api/src/test/resources/system-flags/flags/flag-with-empty-data/default.json b/controller-api/src/test/resources/system-flags/flags/flag-with-empty-data/default.json new file mode 100644 index 00000000000..54aba0d9923 --- /dev/null +++ b/controller-api/src/test/resources/system-flags/flags/flag-with-empty-data/default.json @@ -0,0 +1,8 @@ +{ + "id" : "flag-with-empty-data", + "rules" : [ + { + "value" : "default" + } + ] +}
\ No newline at end of file diff --git a/controller-api/src/test/resources/system-flags/flags/flag-with-empty-data/main.controller.json b/controller-api/src/test/resources/system-flags/flags/flag-with-empty-data/main.controller.json new file mode 100644 index 00000000000..c9b46d68ace --- /dev/null +++ b/controller-api/src/test/resources/system-flags/flags/flag-with-empty-data/main.controller.json @@ -0,0 +1,4 @@ +{ + "id" : "flag-with-empty-data", + "comment": "empty data using only id field" +}
\ No newline at end of file diff --git a/controller-api/src/test/resources/system-flags/flags/flag-with-empty-data/main.json b/controller-api/src/test/resources/system-flags/flags/flag-with-empty-data/main.json new file mode 100644 index 00000000000..cef75be02b7 --- /dev/null +++ b/controller-api/src/test/resources/system-flags/flags/flag-with-empty-data/main.json @@ -0,0 +1,8 @@ +{ + "id" : "flag-with-empty-data", + "rules" : [ + { + "value" : "main" + } + ] +}
\ No newline at end of file diff --git a/controller-api/src/test/resources/system-flags/flags/flag-with-empty-data/main.prod.json b/controller-api/src/test/resources/system-flags/flags/flag-with-empty-data/main.prod.json new file mode 100644 index 00000000000..e69de29bb2d --- /dev/null +++ b/controller-api/src/test/resources/system-flags/flags/flag-with-empty-data/main.prod.json diff --git a/controller-api/src/test/resources/system-flags/flags/flag-with-empty-data/main.prod.us-west-1.json b/controller-api/src/test/resources/system-flags/flags/flag-with-empty-data/main.prod.us-west-1.json new file mode 100644 index 00000000000..fc2690c9c04 --- /dev/null +++ b/controller-api/src/test/resources/system-flags/flags/flag-with-empty-data/main.prod.us-west-1.json @@ -0,0 +1,8 @@ +{ + "id" : "flag-with-empty-data", + "rules" : [ + { + "value" : "main.prod.us-west-1" + } + ] +}
\ No newline at end of file diff --git a/controller-api/src/test/resources/system-flags/flags/my-test-flag/main.prod.us-west-1.json b/controller-api/src/test/resources/system-flags/flags/my-test-flag/main.prod.us-west-1.json index 87b435cdab1..45989773df8 100644 --- a/controller-api/src/test/resources/system-flags/flags/my-test-flag/main.prod.us-west-1.json +++ b/controller-api/src/test/resources/system-flags/flags/my-test-flag/main.prod.us-west-1.json @@ -2,7 +2,7 @@ "id" : "my-test-flag", "rules" : [ { - "value" : "main.prod.us-west-1.json" + "value" : "main.prod.us-west-1" } ] }
\ No newline at end of file diff --git a/flags/pom.xml b/flags/pom.xml index 6afa920e261..87fbc30d92d 100644 --- a/flags/pom.xml +++ b/flags/pom.xml @@ -96,6 +96,10 @@ <artifactId>bundle-plugin</artifactId> <extensions>true</extensions> </plugin> + <plugin> + <groupId>org.apache.maven.plugins</groupId> + <artifactId>maven-compiler-plugin</artifactId> + </plugin> </plugins> </build> </project> 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 4cce7a9bc2a..866d4782a2a 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/FetchVector.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/FetchVector.java @@ -82,6 +82,8 @@ public class FetchVector { return map; } + public boolean isEmpty() { return map.isEmpty(); } + /** Returns a new FetchVector, identical to {@code this} except for its value in {@code dimension}. */ public FetchVector with(Dimension dimension, String value) { return makeFetchVector(merged -> merged.put(dimension, value)); 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 64c4bbe7616..c4079380a8c 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 @@ -52,6 +52,8 @@ public class FlagData { return id; } + public boolean isEmpty() { return rules.isEmpty() && defaultFetchVector.isEmpty(); } + public Optional<RawFlag> resolve(FetchVector fetchVector) { return rules.stream() .filter(rule -> rule.match(defaultFetchVector.with(fetchVector))) |