diff options
9 files changed, 126 insertions, 10 deletions
diff --git a/controller-api/pom.xml b/controller-api/pom.xml index 5c744db2d6d..5d0bbff1c62 100644 --- a/controller-api/pom.xml +++ b/controller-api/pom.xml @@ -53,6 +53,13 @@ <version>${project.version}</version> </dependency> + <dependency> + <groupId>com.yahoo.vespa</groupId> + <artifactId>vespajlib</artifactId> + <scope>provided</scope> + <version>${project.version}</version> + </dependency> + <!-- compile --> <dependency> 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 0cb4baab790..1817b3e6e50 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 @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.controller.api.systemflags.v1; import com.yahoo.config.provision.SystemName; +import com.yahoo.text.JSON; import com.yahoo.vespa.flags.FlagId; import com.yahoo.vespa.flags.json.FlagData; @@ -152,6 +153,9 @@ public class SystemFlagsDataArchive { throw new IllegalArgumentException( String.format("Flag data file with flag id '%s' in directory for '%s'", flagData.id(), directoryDeducedFlagId.toString())); + } else if (!JSON.equals(flagData.serializeToJson(), rawData)) { + throw new IllegalArgumentException("Failed to reconstruct the original JSON at " + + filePath + ", got: " + flagData.serializeToJson()); } } builder.addFile(filename, flagData); 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 6e78b8da91c..51683a4a82f 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 @@ -104,6 +104,13 @@ public class SystemFlagsDataArchiveTest { archive.validateAllFilesAreForTargets(SystemName.main, Set.of(mainControllerTarget, prodUsWestCfgTarget)); } + @Test + public void throws_on_unknown_field() { + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Failed to reconstruct the original JSON at flags/my-test-flag/main.prod.us-west-1.json, got: {\"id\":\"my-test-flag\",\"rules\":[{\"value\":\"default\"}]}"); + SystemFlagsDataArchive archive = SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags-with-unknown-field-name/")); + } + 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"); diff --git a/controller-api/src/test/resources/system-flags-with-unknown-field-name/flags/my-test-flag/main.prod.us-west-1.json b/controller-api/src/test/resources/system-flags-with-unknown-field-name/flags/my-test-flag/main.prod.us-west-1.json new file mode 100644 index 00000000000..c41083fc7ab --- /dev/null +++ b/controller-api/src/test/resources/system-flags-with-unknown-field-name/flags/my-test-flag/main.prod.us-west-1.json @@ -0,0 +1,15 @@ +{ + "id" : "my-test-flag", + "rules" : [ + { + "condition": [ + { + "type": "whitelist", + "dimension": "hostname", + "values": ["foo.com"] + } + ], + "value" : "default" + } + ] +} 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 index c9b46d68ace..7b9e785b6b4 100644 --- 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 @@ -1,4 +1,3 @@ { - "id" : "flag-with-empty-data", - "comment": "empty data using only id field" + "id" : "flag-with-empty-data" }
\ No newline at end of file diff --git a/vespajlib/abi-spec.json b/vespajlib/abi-spec.json index 04f859e2802..7c9f1f31bff 100644 --- a/vespajlib/abi-spec.json +++ b/vespajlib/abi-spec.json @@ -2859,7 +2859,8 @@ ], "methods": [ "public static java.lang.String encode(java.util.Map)", - "public static java.lang.String escape(java.lang.String)" + "public static java.lang.String escape(java.lang.String)", + "public static boolean equals(java.lang.String, java.lang.String)" ], "fields": [] }, diff --git a/vespajlib/pom.xml b/vespajlib/pom.xml index 7631a2af0fb..1b23d024d29 100644 --- a/vespajlib/pom.xml +++ b/vespajlib/pom.xml @@ -39,6 +39,11 @@ <scope>provided</scope> </dependency> <dependency> + <groupId>com.fasterxml.jackson.core</groupId> + <artifactId>jackson-databind</artifactId> + <scope>provided</scope> + </dependency> + <dependency> <groupId>com.yahoo.vespa</groupId> <artifactId>annotations</artifactId> <version>${project.version}</version> @@ -73,11 +78,6 @@ <version>${project.version}</version> <scope>test</scope> </dependency> - <dependency> - <groupId>com.fasterxml.jackson.core</groupId> - <artifactId>jackson-databind</artifactId> - <scope>test</scope> - </dependency> </dependencies> <build> diff --git a/vespajlib/src/main/java/com/yahoo/text/JSON.java b/vespajlib/src/main/java/com/yahoo/text/JSON.java index cfff16c9aba..4091930ed0e 100644 --- a/vespajlib/src/main/java/com/yahoo/text/JSON.java +++ b/vespajlib/src/main/java/com/yahoo/text/JSON.java @@ -1,15 +1,23 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.text; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; + import java.util.Map; +import java.util.Objects; + +import static com.yahoo.yolean.Exceptions.uncheck; /** - * Static methods for working with the map textual format which is parsed by {@link MapParser} + * Static mthods for working with JSON. * * @author bratseth */ public final class JSON { + private static final ObjectMapper mapper = new ObjectMapper(); + /** No instances */ private JSON() {} @@ -56,4 +64,17 @@ public final class JSON { return b != null ? b.toString() : s; } + /** + * Test whether two JSON strings are equal, e.g. the order of fields in an object is irrelevant. + * + * <p>When comparing two numbers of the two JSON strings, the result is only guaranteed to be + * correct if (a) both are integers (without fraction and exponent) and each fits in a long, or + * (b) both are non-integers, were syntactically identical, and fits in a double.</p> + */ + public static boolean equals(String left, String right) { + JsonNode leftJsonNode = uncheck(() -> mapper.readTree(left)); + JsonNode rightJsonNode = uncheck(() -> mapper.readTree(right)); + return Objects.equals(leftJsonNode, rightJsonNode); + } + } diff --git a/vespajlib/src/test/java/com/yahoo/text/JSONTest.java b/vespajlib/src/test/java/com/yahoo/text/JSONTest.java index 22174761571..bba36a7cadd 100644 --- a/vespajlib/src/test/java/com/yahoo/text/JSONTest.java +++ b/vespajlib/src/test/java/com/yahoo/text/JSONTest.java @@ -2,11 +2,15 @@ package com.yahoo.text; import org.junit.Test; -import static org.junit.Assert.assertEquals; import java.util.LinkedHashMap; import java.util.Map; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertTrue; + /** * @author bratseth */ @@ -22,4 +26,62 @@ public class JSONTest { assertEquals("{\"a \\\"key\\\"\":3,\"key2\":\"value\",\"key3\":3.3}", JSON.encode(map)); } + @Test + public void testEquals() { + assertTrue(JSON.equals("{}", "{}")); + + // Whitespace is irrelevant + assertTrue(JSON.equals("{}", "\n{ }")); + + // Order of fields in object is irrelevant + assertTrue(JSON.equals("{\"a\":0, \"c\":1}", "{\"c\":1, \"a\":0}")); + + // Order of elements of array is significant + assertFalse(JSON.equals("[\"a\",\"b\"]", "[\"b\",\"c\"]")); + + // Verify null-valued fields are not ignored + assertFalse(JSON.equals("{\"a\":null}", "{}")); + + // Current impl uses BigInteger if integer doesn't fit in a long. + assertEquals(9223372036854775807L, Long.MAX_VALUE); + assertTrue(JSON.equals("{\"a\": 9223372036854775807}", "{\"a\": 9223372036854775807}")); + + // double 1.0 and int 1 are different + assertTrue(JSON.equals( "{\"a\": 1}", "{\"a\": 1}")); + assertTrue(JSON.equals( "{\"a\": 1.0}", "{\"a\": 1.0}")); + assertFalse(JSON.equals( "{\"a\": 1.0}", "{\"a\": 1}")); + + // Double-precision on numbers. Constant from Math.E. + assertTrue(JSON.equals( "{\"e\": 2.71828182845904}", "{\"e\": 2.71828182845904}")); + + // Justification of above float values + double e1 = 2.7182818284590452354; + double e2 = 2.718281828459045; + double e3 = 2.71828182845904; + assertEquals(e1, Math.E, -1); + assertEquals(e1, e2, -1); + assertNotEquals(e1, e3, -1); + + // Impl uses BigInteger + assertTrue(JSON.equals( "{\"a\": 92233720368547758070}", + "{\"a\": 92233720368547758070}")); + assertFalse(JSON.equals("{\"a\": 92233720368547758070}", + "{\"a\": 92233720368547758071}")); + + // Impl converts to double and ignores extraneous digits + assertTrue(JSON.equals( "{\"e\": 2.7182818284590452354}", + "{\"e\": 2.7182818284590452354}")); + assertTrue(JSON.equals( "{\"e\": 2.7182818284590452354}", + "{\"e\": 2.7182818284590452355}")); + assertFalse(JSON.equals("{\"e\": 2.7182818284590452354}", + "{\"e\": 2.71828182845904}")); + + // Misc impl defined results + assertFalse(JSON.equals("{\"a\": 1.0}", "{\"a\":1}")); + assertTrue(JSON.equals("{\"a\": 1.0}", "{\"a\":1.00}")); + assertTrue(JSON.equals("{\"a\": 1.0}", "{\"a\":1.0000000000000000000000000000}")); + assertTrue(JSON.equals("{\"a\": 10.0}", "{\"a\":1e1}")); + assertTrue(JSON.equals("{\"a\": 1.2}", "{\"a\":12e-1}")); + } + } |