diff options
author | Håkon Hallingstad <hakon@yahooinc.com> | 2023-08-02 10:59:51 +0200 |
---|---|---|
committer | Håkon Hallingstad <hakon@yahooinc.com> | 2023-08-02 10:59:51 +0200 |
commit | 0adede88d0aa7bbb7280a5c26a5279fc08246652 (patch) | |
tree | 10f42c2494edde12407bc6a3b7c9a28fb36f20e6 /flags | |
parent | ae86f6951daa412bbad509ff06998bb66b632cae (diff) |
Remove sentinel null valued rules independent of any conditions
Diffstat (limited to 'flags')
4 files changed, 80 insertions, 33 deletions
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 8a5366c65e2..bc103c2d68a 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 @@ -69,16 +69,7 @@ public class FlagData { } } } - - // Remove trailing rules that have no conditions and no value to apply. - while (newRules.size() > 0) { - Rule lastRule = newRules.get(newRules.size() - 1); - if (lastRule.conditions().isEmpty() && lastRule.getValueToApply().isEmpty()) { - newRules.remove(newRules.size() - 1); - } else { - break; - } - } + newRules = optimizeRules(newRules); FetchVector newDefaultFetchVector = defaultFetchVector.without(fetchVector.dimensions()); @@ -198,7 +189,26 @@ public class FlagData { private static List<Rule> rulesFromWire(List<WireRule> wireRules) { if (wireRules == null) return List.of(); - return wireRules.stream().map(Rule::fromWire).toList(); + return optimizeRules(wireRules.stream().map(Rule::fromWire).toList()); + } + + /** Take a raw list of rules from e.g. deserialization or partial resolution and normalize/simplify it. */ + private static List<Rule> optimizeRules(List<Rule> rules) { + // Remove trailing rules without value, as absent value implies the code default. + // Removing trailing rules may further simplify when e.g. this results in no rules, + // which is equivalent to no flag data at all, and flag data may be deleted from a zone. + if (rules.isEmpty()) return rules; + if (rules.get(rules.size() - 1).getValueToApply().isPresent()) return rules; + var newRules = new ArrayList<>(rules); + while (newRules.size() > 0) { + Rule lastRule = newRules.get(newRules.size() - 1); + if (lastRule.getValueToApply().isEmpty()) { + newRules.remove(newRules.size() - 1); + } else { + break; + } + } + return newRules; } } diff --git a/flags/src/main/java/com/yahoo/vespa/flags/json/Rule.java b/flags/src/main/java/com/yahoo/vespa/flags/json/Rule.java index 127c2b4f9da..b1b15faa938 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/json/Rule.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/json/Rule.java @@ -83,9 +83,11 @@ public class Rule { public static Rule fromWire(WireRule wireRule) { List<Condition> conditions = wireRule.andConditions == null ? - Collections.emptyList() : + List.of() : wireRule.andConditions.stream().map(Condition::fromWire).toList(); - Optional<RawFlag> value = wireRule.value == null ? Optional.empty() : Optional.of(JsonNodeRawFlag.fromJsonNode(wireRule.value)); + Optional<RawFlag> value = wireRule.value == null || wireRule.value.isNull() ? + Optional.empty() : + Optional.of(JsonNodeRawFlag.fromJsonNode(wireRule.value)); return new Rule(value, conditions); } 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 4ec0ea0652f..3ca7f59c759 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 @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.flags.json; +import com.yahoo.text.JSON; import com.yahoo.vespa.flags.FetchVector; import com.yahoo.vespa.flags.RawFlag; import org.junit.jupiter.api.Test; @@ -231,7 +232,40 @@ public class FlagDataTest { } ] }"""); - FlagData flagData = data.partialResolve(vector.with(FetchVector.Dimension.ZONE_ID, "zone3")); + assertEquals(data, new FlagData(data.id(), new FetchVector(), List.of())); + assertTrue(data.isEmpty()); + } + + @Test + void testRemovalOfSentinelRuleWithoutValue() { + String json = """ + { + "id": "id1", + "rules": [ + { + "conditions": [ + { + "type": "whitelist", + "dimension": "zone", + "values": [ "zone1", "zone2" ] + } + ] + }, + { + "conditions": [ + { + "type": "whitelist", + "dimension": "cloud", + "values": [ "aws" ] + } + ], + "value": true + } + ] + }"""; + FlagData data = FlagData.deserialize(json); + assertTrue(JSON.equals(data.serializeToJson(), json)); + FlagData flagData = data.partialResolve(vector.with(FetchVector.Dimension.CLOUD, "gcp")); assertEquals(flagData, new FlagData(data.id(), new FetchVector(), List.of())); assertTrue(flagData.isEmpty()); } 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 2cc19917793..2192739c96c 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 @@ -108,24 +108,25 @@ public class SerializationTest { @Test void jsonWithStrayFields() { - String json = "{\n" + - " \"id\": \"id3\",\n" + - " \"foo\": true,\n" + - " \"rules\": [\n" + - " {\n" + - " \"conditions\": [\n" + - " {\n" + - " \"type\": \"whitelist\",\n" + - " \"dimension\": \"zone\",\n" + - " \"bar\": \"zoo\"\n" + - " }\n" + - " ],\n" + - " \"other\": true\n" + - " }\n" + - " ],\n" + - " \"attributes\": {\n" + - " }\n" + - "}"; + String json = """ + { + "id": "id3", + "foo": true, + "rules": [ + { + "conditions": [ + { + "type": "whitelist", + "dimension": "zone", + "bar": "zoo" + } + ], + "other": true + } + ], + "attributes": { + } + }"""; WireFlagData wireData = WireFlagData.deserialize(json); @@ -140,6 +141,6 @@ public class SerializationTest { assertThat(wireData.serializeToJson(), equalTo("{\"id\":\"id3\",\"rules\":[{\"conditions\":[{\"type\":\"whitelist\",\"dimension\":\"zone\"}]}],\"attributes\":{}}")); - assertThat(FlagData.deserialize(json).serializeToJson(), equalTo("{\"id\":\"id3\",\"rules\":[{\"conditions\":[{\"type\":\"whitelist\",\"dimension\":\"zone\"}]}]}")); + assertThat(FlagData.deserialize(json).serializeToJson(), equalTo("{\"id\":\"id3\"}")); } } |