From cf70bfb260fa8731617a327b56be433128aa0450 Mon Sep 17 00:00:00 2001 From: Håkon Hallingstad Date: Thu, 3 Jan 2019 19:54:11 +0100 Subject: Typed flag classes This reintroduces the non-generic flag classes: - a value() returns the primitive type for flags wrapping a primitive type - easier to use in testing - Serializer is moved to internals of typed class Defines the flag backed by boolean BooleanFlag instead of FeatureFlag since not all boolean flags are necessarily guarding a feature. --- .../config/server/application/Application.java | 4 +- .../config/server/http/flags/DefinedFlags.java | 6 +- .../config/server/http/flags/FlagsHandler.java | 29 ++++- .../config/server/http/flags/FlagsHandlerTest.java | 50 +++++---- .../java/com/yahoo/vespa/flags/BooleanFlag.java | 18 +++ .../src/main/java/com/yahoo/vespa/flags/Flag.java | 47 +++----- .../java/com/yahoo/vespa/flags/FlagDefinition.java | 15 +-- .../main/java/com/yahoo/vespa/flags/FlagImpl.java | 47 ++++++++ .../src/main/java/com/yahoo/vespa/flags/Flags.java | 123 +++++++++++---------- .../main/java/com/yahoo/vespa/flags/IntFlag.java | 18 +++ .../java/com/yahoo/vespa/flags/JacksonFlag.java | 18 +++ .../main/java/com/yahoo/vespa/flags/LongFlag.java | 18 +++ .../com/yahoo/vespa/flags/OrderedFlagSource.java | 1 - .../java/com/yahoo/vespa/flags/StringFlag.java | 18 +++ .../com/yahoo/vespa/flags/UnboundBooleanFlag.java | 27 +++++ .../java/com/yahoo/vespa/flags/UnboundFlag.java | 45 +++----- .../com/yahoo/vespa/flags/UnboundFlagImpl.java | 58 ++++++++++ .../java/com/yahoo/vespa/flags/UnboundIntFlag.java | 23 ++++ .../com/yahoo/vespa/flags/UnboundJacksonFlag.java | 21 ++++ .../com/yahoo/vespa/flags/UnboundLongFlag.java | 23 ++++ .../com/yahoo/vespa/flags/UnboundStringFlag.java | 23 ++++ .../java/com/yahoo/vespa/flags/json/FlagData.java | 17 ++- .../com/yahoo/vespa/flags/FileFlagSourceTest.java | 30 ++--- .../test/java/com/yahoo/vespa/flags/FlagsTest.java | 19 ++-- .../vespa/service/health/HealthMonitorManager.java | 8 +- .../service/health/HealthMonitorManagerTest.java | 5 +- 26 files changed, 513 insertions(+), 198 deletions(-) create mode 100644 flags/src/main/java/com/yahoo/vespa/flags/BooleanFlag.java create mode 100644 flags/src/main/java/com/yahoo/vespa/flags/FlagImpl.java create mode 100644 flags/src/main/java/com/yahoo/vespa/flags/IntFlag.java create mode 100644 flags/src/main/java/com/yahoo/vespa/flags/JacksonFlag.java create mode 100644 flags/src/main/java/com/yahoo/vespa/flags/LongFlag.java create mode 100644 flags/src/main/java/com/yahoo/vespa/flags/StringFlag.java create mode 100644 flags/src/main/java/com/yahoo/vespa/flags/UnboundBooleanFlag.java create mode 100644 flags/src/main/java/com/yahoo/vespa/flags/UnboundFlagImpl.java create mode 100644 flags/src/main/java/com/yahoo/vespa/flags/UnboundIntFlag.java create mode 100644 flags/src/main/java/com/yahoo/vespa/flags/UnboundJacksonFlag.java create mode 100644 flags/src/main/java/com/yahoo/vespa/flags/UnboundLongFlag.java create mode 100644 flags/src/main/java/com/yahoo/vespa/flags/UnboundStringFlag.java diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/Application.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/Application.java index 2d5d5296762..2463cca190a 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/Application.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/Application.java @@ -23,9 +23,9 @@ import com.yahoo.vespa.config.server.rpc.ConfigResponseFactory; import com.yahoo.vespa.config.server.rpc.UncompressedConfigResponseFactory; import com.yahoo.vespa.config.server.tenant.TenantRepository; import com.yahoo.vespa.config.util.ConfigUtils; +import com.yahoo.vespa.flags.BooleanFlag; import com.yahoo.vespa.flags.FetchVector; import com.yahoo.vespa.flags.FileFlagSource; -import com.yahoo.vespa.flags.Flag; import com.yahoo.vespa.flags.FlagSource; import com.yahoo.vespa.flags.Flags; @@ -49,7 +49,7 @@ public class Application implements ModelResult { private final ServerCache cache; private final MetricUpdater metricUpdater; private final ApplicationId app; - private final Flag useConfigServerCache; + private final BooleanFlag useConfigServerCache; public Application(Model model, ServerCache cache, long appGeneration, boolean internalRedeploy, Version vespaVersion, MetricUpdater metricUpdater, ApplicationId app) { 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 9f0e39b7d43..6d5d0df8f1a 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 @@ -20,12 +20,12 @@ import java.util.List; */ public class DefinedFlags extends HttpResponse { private static ObjectMapper mapper = new ObjectMapper(); - private static final Comparator> sortByFlagId = + private static final Comparator sortByFlagId = (left, right) -> left.getUnboundFlag().id().compareTo(right.getUnboundFlag().id()); - private final List> flags; + private final List flags; - public DefinedFlags(List> flags) { + public DefinedFlags(List flags) { super(Response.Status.OK); this.flags = flags; } 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 e64c356aa96..e5efc74fb75 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 @@ -6,13 +6,17 @@ import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.container.jdisc.LoggingRequestHandler; import com.yahoo.restapi.Path; +import com.yahoo.vespa.config.server.http.HttpErrorResponse; import com.yahoo.vespa.config.server.http.HttpHandler; import com.yahoo.vespa.config.server.http.NotFoundException; import com.yahoo.vespa.configserver.flags.FlagsDb; +import com.yahoo.vespa.flags.FlagDefinition; import com.yahoo.vespa.flags.FlagId; import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.flags.json.FlagData; +import com.yahoo.yolean.Exceptions; +import java.io.UncheckedIOException; import java.net.URI; import java.util.Objects; @@ -74,7 +78,19 @@ public class FlagsHandler extends HttpHandler { } private HttpResponse putFlagData(HttpRequest request, FlagId flagId) { - flagsDb.setValue(flagId, FlagData.deserialize(request.getData())); + FlagData data; + try { + data = FlagData.deserialize(request.getData()); + } catch (UncheckedIOException e) { + return HttpErrorResponse.badRequest("Failed to deserialize request data: " + Exceptions.toMessageString(e)); + } + + if (!isForce(request)) { + FlagDefinition definition = Flags.getFlag(flagId).get(); // FlagId has been validated in findFlagId() + data.validate(definition.getUnboundFlag().serializer()); + } + + flagsDb.setValue(flagId, data); return new OKResponse(); } @@ -86,12 +102,15 @@ public class FlagsHandler extends HttpHandler { private FlagId findFlagId(HttpRequest request, Path path) { FlagId flagId = new FlagId(path.get("flagId")); - if (!Objects.equals(request.getProperty("force"), "true")) { - if (Flags.getAllFlags().stream().noneMatch(definition -> flagId.equals(definition.getUnboundFlag().id()))) { - throw new NotFoundException("There is no flag '" + flagId + "' (use ?force=true to override)"); - } + if (!isForce(request)) { + Flags.getFlag(flagId).orElseThrow(() -> + new NotFoundException("There is no flag '" + flagId + "' (use ?force=true to override)")); } return flagId; } + + private boolean isForce(HttpRequest request) { + return Objects.equals(request.getProperty("force"), "true"); + } } 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 8781ec413d2..71caed77dbe 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 @@ -11,7 +11,7 @@ import com.yahoo.vespa.curator.mock.MockCurator; import com.yahoo.vespa.flags.FetchVector; import com.yahoo.vespa.flags.FlagId; import com.yahoo.vespa.flags.Flags; -import com.yahoo.vespa.flags.UnboundFlag; +import com.yahoo.vespa.flags.UnboundBooleanFlag; import org.junit.Test; import java.io.ByteArrayInputStream; @@ -21,6 +21,7 @@ import java.util.stream.Stream; import static com.yahoo.yolean.Exceptions.uncheck; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertEquals; @@ -28,11 +29,11 @@ import static org.junit.Assert.assertEquals; * @author hakonhall */ public class FlagsHandlerTest { - private static final UnboundFlag FLAG1 = - Flags.defineBoolean("id1", false, "desc1", "mod1"); - private static final UnboundFlag FLAG2 = - Flags.defineBoolean("id2", true, "desc2", "mod2", - FetchVector.Dimension.HOSTNAME, FetchVector.Dimension.APPLICATION_ID); + private static final UnboundBooleanFlag FLAG1 = Flags.defineFeatureFlag( + "id1", false, "desc1", "mod1"); + private static final UnboundBooleanFlag FLAG2 = Flags.defineFeatureFlag( + "id2", true, "desc2", "mod2", + FetchVector.Dimension.HOSTNAME, FetchVector.Dimension.APPLICATION_ID); private static final String FLAGS_V1_URL = "https://foo.com:4443/flags/v1"; @@ -54,7 +55,7 @@ public class FlagsHandlerTest { public void testDefined() { try (Flags.Replacer replacer = Flags.clearFlagsForTesting()) { fixUnusedWarning(replacer); - Flags.defineBoolean("id", false, "desc", "mod", FetchVector.Dimension.HOSTNAME); + Flags.defineFeatureFlag("id", false, "desc", "mod", FetchVector.Dimension.HOSTNAME); verifySuccessfulRequest(Method.GET, "/defined", "", "{\"id\":{\"description\":\"desc\",\"modification-effect\":\"mod\",\"dimensions\":[\"hostname\"]}}"); } @@ -153,32 +154,33 @@ public class FlagsHandlerTest { @Test public void testForcing() { - FlagId undefinedFlagId = new FlagId("undef"); - HttpResponse response = handle(Method.PUT, "/data/" + undefinedFlagId, ""); + assertThat(handle(Method.PUT, "/data/" + new FlagId("undef"), "", 404), + containsString("There is no flag 'undef'")); - assertEquals(404, response.getStatus()); - assertEquals("application/json", response.getContentType()); + assertThat(handle(Method.PUT, "/data/" + new FlagId("undef") + "?force=true", "", 400), + containsString("No content to map due to end-of-input")); - } + assertThat(handle(Method.PUT, "/data/" + FLAG1.id(), "{}", 400), + containsString("Flag ID missing")); - private void verifySuccessfulRequest(Method method, String pathSuffix, String requestBody, String expectedResponseBody) { - HttpResponse response = handle(method, pathSuffix, requestBody); + assertThat(handle(Method.PUT, "/data/" + FLAG1.id(), "{\"id\": \"id1\",\"rules\": [{\"value\":\"string\"}]}", 400), + containsString("Wrong type of JsonNode: STRING")); - assertEquals(200, response.getStatus()); - assertEquals("application/json", response.getContentType()); - String actualResponse = uncheck(() -> SessionHandlerTest.getRenderedString(response)); + assertThat(handle(Method.PUT, "/data/" + FLAG1.id() + "?force=true", "{\"id\": \"id1\",\"rules\": [{\"value\":\"string\"}]}", 200), + is("")); + } - assertThat(actualResponse, is(expectedResponseBody)); + private void verifySuccessfulRequest(Method method, String pathSuffix, String requestBody, String expectedResponseBody) { + assertThat(handle(method, pathSuffix, requestBody, 200), is(expectedResponseBody)); } - private HttpResponse handle(Method method, String pathSuffix, String requestBody) { + private String handle(Method method, String pathSuffix, String requestBody, int expectedStatus) { String uri = FLAGS_V1_URL + pathSuffix; HttpRequest request = HttpRequest.createTestRequest(uri, method, makeInputStream(requestBody)); - return handler.handle(request); - } - - private String makeUrl(String component) { - return FLAGS_V1_URL + "/" + component; + HttpResponse response = handler.handle(request); + assertEquals(expectedStatus, response.getStatus()); + assertEquals("application/json", response.getContentType()); + return uncheck(() -> SessionHandlerTest.getRenderedString(response)); } private InputStream makeInputStream(String content) { diff --git a/flags/src/main/java/com/yahoo/vespa/flags/BooleanFlag.java b/flags/src/main/java/com/yahoo/vespa/flags/BooleanFlag.java new file mode 100644 index 00000000000..1245aca65b8 --- /dev/null +++ b/flags/src/main/java/com/yahoo/vespa/flags/BooleanFlag.java @@ -0,0 +1,18 @@ +// Copyright 2019 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.flags; + +import javax.annotation.concurrent.Immutable; + +/** + * @author hakonhall + */ +@Immutable +public class BooleanFlag extends FlagImpl { + public BooleanFlag(FlagId id, boolean defaultValue, FetchVector vector, FlagSerializer serializer, FlagSource source) { + super(id, defaultValue, vector, serializer, source, BooleanFlag::new); + } + + public boolean value() { + return boxedValue(); + } +} diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flag.java b/flags/src/main/java/com/yahoo/vespa/flags/Flag.java index 18c1db0d756..7bf491a39c7 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flag.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flag.java @@ -1,44 +1,23 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.flags; -import javax.annotation.concurrent.Immutable; - /** + * Interface for flag. + * + * @param The type of the flag value (boxed for primitives) + * @param The concrete subclass type of the flag * @author hakonhall */ -@Immutable -public class Flag { - private final FlagId id; - private final T defaultValue; - private final FlagSource source; - private final Deserializer deserializer; - private final FetchVector fetchVector; - - public Flag(String flagId, T defaultValue, FlagSource source, Deserializer deserializer) { - this(new FlagId(flagId), defaultValue, source, deserializer); - } - - public Flag(FlagId id, T defaultValue, FlagSource source, Deserializer deserializer) { - this(id, defaultValue, deserializer, new FetchVector(), source); - } - - public Flag(FlagId id, T defaultValue, Deserializer deserializer, FetchVector fetchVector, FlagSource source) { - this.id = id; - this.defaultValue = defaultValue; - this.source = source; - this.deserializer = deserializer; - this.fetchVector = fetchVector; - } +public interface Flag { + /** The flag ID. */ + FlagId id(); - public FlagId id() { - return id; - } + /** Returns the flag serializer. */ + FlagSerializer serializer(); - public Flag with(FetchVector.Dimension dimension, String value) { - return new Flag<>(id, defaultValue, deserializer, fetchVector.with(dimension, value), source); - } + /** Returns an immutable clone of the current object, except with the dimension set accordingly. */ + F with(FetchVector.Dimension dimension, String dimensionValue); - public T value() { - return source.fetch(id, fetchVector).map(deserializer::deserialize).orElse(defaultValue); - } + /** Returns the value, boxed if the flag wraps a primitive type. */ + T boxedValue(); } diff --git a/flags/src/main/java/com/yahoo/vespa/flags/FlagDefinition.java b/flags/src/main/java/com/yahoo/vespa/flags/FlagDefinition.java index a3f490e2f96..0dce61cf0bb 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/FlagDefinition.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/FlagDefinition.java @@ -1,7 +1,8 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright 2019 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.flags; import javax.annotation.concurrent.Immutable; +import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -9,21 +10,21 @@ import java.util.List; * @author hakonhall */ @Immutable -public class FlagDefinition { - private final UnboundFlag unboundFlag; +public class FlagDefinition { + private final UnboundFlag unboundFlag; private final String description; private final String modificationEffect; private final List dimensions; - public FlagDefinition(UnboundFlag unboundFlag, String description, String modificationEffect, - List dimensions) { + public FlagDefinition(UnboundFlag unboundFlag, String description, String modificationEffect, + FetchVector.Dimension... dimensions) { this.unboundFlag = unboundFlag; this.description = description; this.modificationEffect = modificationEffect; - this.dimensions = Collections.unmodifiableList(dimensions); + this.dimensions = Collections.unmodifiableList(Arrays.asList(dimensions)); } - public UnboundFlag getUnboundFlag() { + public UnboundFlag getUnboundFlag() { return unboundFlag; } diff --git a/flags/src/main/java/com/yahoo/vespa/flags/FlagImpl.java b/flags/src/main/java/com/yahoo/vespa/flags/FlagImpl.java new file mode 100644 index 00000000000..79da03e4982 --- /dev/null +++ b/flags/src/main/java/com/yahoo/vespa/flags/FlagImpl.java @@ -0,0 +1,47 @@ +// Copyright 2019 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.flags; + +/** + * @author hakonhall + */ +public abstract class FlagImpl> implements Flag { + private final FlagId id; + private final T defaultValue; + private final FlagSerializer serializer; + private final FetchVector fetchVector; + private final UnboundFlagImpl.FlagFactory factory; + private final FlagSource source; + + protected FlagImpl(FlagId id, + T defaultValue, + FetchVector fetchVector, FlagSerializer serializer, + FlagSource source, + UnboundFlagImpl.FlagFactory factory) { + this.id = id; + this.defaultValue = defaultValue; + this.serializer = serializer; + this.fetchVector = fetchVector; + this.factory = factory; + this.source = source; + } + + @Override + public FlagId id() { + return id; + } + + @Override + public F with(FetchVector.Dimension dimension, String dimensionValue) { + return factory.create(id, defaultValue, fetchVector.with(dimension, dimensionValue), serializer, source); + } + + @Override + public T boxedValue() { + return source.fetch(id, fetchVector).map(serializer::deserialize).orElse(defaultValue); + } + + @Override + public FlagSerializer serializer() { + return serializer; + } +} diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index 50cb5ffdf7c..50f65e5ce13 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -1,53 +1,33 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright 2019 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.flags; -import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.node.BooleanNode; -import com.fasterxml.jackson.databind.node.IntNode; -import com.fasterxml.jackson.databind.node.LongNode; -import com.fasterxml.jackson.databind.node.TextNode; import com.yahoo.vespa.defaults.Defaults; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; /** - * Definitions of most/all flags. - * - *

The flags are centrally defined in this module to allow 1. all code to access flags that may be used in - * quite different modules and processes, and 2. in particular allow the config server to access all flags - * so operators have a nicer UI for setting, modifying, or removing flag values. - * - *

This class should have been an enum, but unfortunately enums cannot be generic, which will eventually be - * fixed with JEP 301: Enhanced Enums. - * * @author hakonhall */ public class Flags { - public static final FlagSerializer BOOLEAN_SERIALIZER = new SimpleFlagSerializer<>(BooleanNode::valueOf, JsonNode::isBoolean, JsonNode::asBoolean); - public static final FlagSerializer STRING_SERIALIZER = new SimpleFlagSerializer<>(TextNode::new, JsonNode::isTextual, JsonNode::asText); - public static final FlagSerializer INT_SERIALIZER = new SimpleFlagSerializer<>(IntNode::new, JsonNode::isIntegralNumber, JsonNode::asInt); - public static final FlagSerializer LONG_SERIALIZER = new SimpleFlagSerializer<>(LongNode::new, JsonNode::isIntegralNumber, JsonNode::asLong); + private static volatile ConcurrentHashMap flags = new ConcurrentHashMap<>(); - private static volatile ConcurrentHashMap> flags = new ConcurrentHashMap<>(); - - public static final UnboundFlag HEALTHMONITOR_MONITOR_INFRA = defineBoolean( + public static final UnboundBooleanFlag HEALTHMONITOR_MONITOR_INFRA = defineFeatureFlag( "healthmonitor-monitorinfra", true, - "Whether the health monitor in service monitor monitors the health of infrastructure applications.", - "Affects all applications activated after the value is changed.", + "Whether the health monitor in service monitor monitors the health of infrastructure applications.", + "Affects all applications activated after the value is changed.", FetchVector.Dimension.HOSTNAME); - public static final UnboundFlag DUPERMODEL_CONTAINS_INFRA = defineBoolean( + public static final UnboundBooleanFlag DUPERMODEL_CONTAINS_INFRA = defineFeatureFlag( "dupermodel-contains-infra", true, "Whether the DuperModel in config server/controller includes active infrastructure applications " + "(except from controller/config apps).", "Requires restart of config server/controller to take effect.", FetchVector.Dimension.HOSTNAME); - public static final UnboundFlag DUPERMODEL_USE_CONFIGSERVERCONFIG = defineBoolean( + public static final UnboundBooleanFlag DUPERMODEL_USE_CONFIGSERVERCONFIG = defineFeatureFlag( "dupermodel-use-configserverconfig", true, "For historical reasons, the ApplicationInfo in the DuperModel for controllers and config servers " + "is based on the ConfigserverConfig (this flag is true). We want to transition to use the " + @@ -55,75 +35,86 @@ public class Flags { "Requires restart of config server/controller to take effect.", FetchVector.Dimension.HOSTNAME); - public static final UnboundFlag USE_CONFIG_SERVER_CACHE = defineBoolean( + public static final UnboundBooleanFlag USE_CONFIG_SERVER_CACHE = defineFeatureFlag( "use-config-server-cache", true, "Whether config server will use cache to answer config requests.", "Takes effect immediately when changed.", FetchVector.Dimension.HOSTNAME, FetchVector.Dimension.APPLICATION_ID); - public static final UnboundFlag CONFIG_SERVER_BOOTSTRAP_IN_SEPARATE_THREAD = defineBoolean( + public static final UnboundBooleanFlag CONFIG_SERVER_BOOTSTRAP_IN_SEPARATE_THREAD = defineFeatureFlag( "config-server-bootstrap-in-separate-thread", true, "Whether to run config server/controller bootstrap in a separate thread.", "Takes effect only at bootstrap of config server/controller", FetchVector.Dimension.HOSTNAME); - public static final UnboundFlag PROXYHOST_USES_REAL_ORCHESTRATOR = defineBoolean( + public static final UnboundBooleanFlag PROXYHOST_USES_REAL_ORCHESTRATOR = defineFeatureFlag( "proxyhost-uses-real-orchestrator", true, "Whether proxy hosts uses the real Orchestrator when suspending/resuming, or a synthetic.", "Takes effect immediately when changed.", FetchVector.Dimension.HOSTNAME); - public static final UnboundFlag CONFIGHOST_USES_REAL_ORCHESTRATOR = defineBoolean( + public static final UnboundBooleanFlag CONFIGHOST_USES_REAL_ORCHESTRATOR = defineFeatureFlag( "confighost-uses-real-orchestrator", false, "Whether the config server hosts uses the real Orchestrator when suspending/resuming, or a synthetic.", "Takes effect immediately when changed.", FetchVector.Dimension.HOSTNAME); - public static final UnboundFlag ENABLE_DOCKER_1_13 = defineBoolean( + public static final UnboundBooleanFlag ENABLE_DOCKER_1_13 = defineFeatureFlag( "enable-docker-1.13", true, "Whether to upgrade to Docker version 1.13.", "Takes effect on next host admin tick.", FetchVector.Dimension.HOSTNAME); - public static final UnboundFlag ENABLE_CROWDSTRIKE = defineBoolean( + public static final UnboundBooleanFlag ENABLE_CROWDSTRIKE = defineFeatureFlag( "enable-crowdstrike", true, "Whether to enable CrowdStrike.", "Takes effect on next host admin tick"); - public static final UnboundFlag ENABLE_NESSUS = defineBoolean( + public static final UnboundBooleanFlag ENABLE_NESSUS = defineFeatureFlag( "enable-nessus", true, "Whether to enable Nessus.", "Takes effect on next host admin tick"); - public static UnboundFlag defineBoolean(String flagId, boolean defaultValue, String description, + /** WARNING: public for testing: All flags should be defined in {@link Flags}. */ + public static UnboundBooleanFlag defineFeatureFlag(String flagId, boolean defaultValue, String description, + String modificationEffect, FetchVector.Dimension... dimensions) { + return define(UnboundBooleanFlag::new, flagId, defaultValue, description, modificationEffect, dimensions); + } + + /** WARNING: public for testing: All flags should be defined in {@link Flags}. */ + public static UnboundStringFlag defineStringFlag(String flagId, String defaultValue, String description, String modificationEffect, FetchVector.Dimension... dimensions) { - return define(flagId, defaultValue, BOOLEAN_SERIALIZER, description, modificationEffect, dimensions); + return define(UnboundStringFlag::new, flagId, defaultValue, description, modificationEffect, dimensions); } - public static UnboundFlag defineString(String flagId, String defaultValue, String description, - String modificationEffect, FetchVector.Dimension... dimensions) { - return define(flagId, defaultValue, STRING_SERIALIZER, description, modificationEffect, dimensions); + /** WARNING: public for testing: All flags should be defined in {@link Flags}. */ + public static UnboundIntFlag defineIntFlag(String flagId, int defaultValue, String description, + String modificationEffect, FetchVector.Dimension... dimensions) { + return define(UnboundIntFlag::new, flagId, defaultValue, description, modificationEffect, dimensions); } - public static UnboundFlag defineInt(String flagId, Integer defaultValue, String description, + /** WARNING: public for testing: All flags should be defined in {@link Flags}. */ + public static UnboundLongFlag defineLongFlag(String flagId, long defaultValue, String description, String modificationEffect, FetchVector.Dimension... dimensions) { - return define(flagId, defaultValue, INT_SERIALIZER, description, modificationEffect, dimensions); + return define(UnboundLongFlag::new, flagId, defaultValue, description, modificationEffect, dimensions); } - public static UnboundFlag defineLong(String flagId, Long defaultValue, String description, - String modificationEffect, FetchVector.Dimension... dimensions) { - return define(flagId, defaultValue, LONG_SERIALIZER, description, modificationEffect, dimensions); + /** WARNING: public for testing: All flags should be defined in {@link Flags}. */ + public static UnboundJacksonFlag defineJacksonFlag(String flagId, T defaultValue, Class jacksonClass, String description, + String modificationEffect, FetchVector.Dimension... dimensions) { + return define((id2, defaultValue2, vector2) -> new UnboundJacksonFlag<>(id2, defaultValue2, vector2, jacksonClass), + flagId, defaultValue, description, modificationEffect, dimensions); } - public static UnboundFlag defineJackson(String flagId, Class jacksonClass, T defaultValue, String description, - String modificationEffect, FetchVector.Dimension... dimensions) { - return define(flagId, defaultValue, new JacksonSerializer<>(jacksonClass), description, modificationEffect, dimensions); + @FunctionalInterface + private interface TypedUnboundFlagFactory> { + U create(FlagId id, T defaultVale, FetchVector defaultFetchVector); } /** * Defines a Flag. * + * @param factory Factory for creating unbound flag of type U * @param flagId The globally unique FlagId. * @param defaultValue The default value if none is present after resolution. - * @param deserializer Deserialize JSON to value type. * @param description Description of how the flag is used. * @param modificationEffect What is required for the flag to take effect? A restart of process? immediately? etc. * @param dimensions What dimensions will be set in the {@link FetchVector} when fetching @@ -132,34 +123,46 @@ public class Flags { * For instance, if APPLICATION is one of the dimensions here, you should make sure * APPLICATION is set to the ApplicationId in the fetch vector when fetching the RawFlag * from the FlagSource. - * @param The type of the flag value, typically Boolean for flags guarding features. + * @param The boxed type of the flag value, e.g. Boolean for flags guarding features. + * @param The type of the unbound flag, e.g. UnboundBooleanFlag. * @return An unbound flag with {@link FetchVector.Dimension#HOSTNAME HOSTNAME} environment. The ZONE environment * is typically implicit. */ - private static UnboundFlag define(String flagId, T defaultValue, Deserializer deserializer, - String description, String modificationEffect, - FetchVector.Dimension... dimensions) { - UnboundFlag flag = new UnboundFlag<>(flagId, defaultValue, deserializer) - .with(FetchVector.Dimension.HOSTNAME, Defaults.getDefaults().vespaHostname()); - FlagDefinition definition = new FlagDefinition<>(flag, description, modificationEffect, Arrays.asList(dimensions)); - flags.put(flag.id(), definition); - return flag; + private static > U define(TypedUnboundFlagFactory factory, + String flagId, + T defaultValue, + String description, + String modificationEffect, + FetchVector.Dimension[] dimensions) { + FlagId id = new FlagId(flagId); + FetchVector vector = new FetchVector().with(FetchVector.Dimension.HOSTNAME, Defaults.getDefaults().vespaHostname()); + U unboundFlag = factory.create(id, defaultValue, vector); + FlagDefinition definition = new FlagDefinition(unboundFlag, description, modificationEffect, dimensions); + flags.put(id, definition); + return unboundFlag; } - public static List> getAllFlags() { + public static List getAllFlags() { return new ArrayList<>(flags.values()); } - public static Optional> getFlag(FlagId flagId) { + public static Optional getFlag(FlagId flagId) { return Optional.ofNullable(flags.get(flagId)); } + /** + * Allows the statically defined flags to be controlled in a test. + * + *

Returns a Replacer instance to be used with e.g. a try-with-resources block. Within the block, + * the flags starts out as cleared. Flags can be defined, etc. When leaving the block, the flags from + * before the block is reinserted. + * */ public static Replacer clearFlagsForTesting() { return new Replacer(); } public static class Replacer implements AutoCloseable { - private final ConcurrentHashMap> savedFlags; + private final ConcurrentHashMap savedFlags; private Replacer() { this.savedFlags = Flags.flags; diff --git a/flags/src/main/java/com/yahoo/vespa/flags/IntFlag.java b/flags/src/main/java/com/yahoo/vespa/flags/IntFlag.java new file mode 100644 index 00000000000..4a59c34e8d0 --- /dev/null +++ b/flags/src/main/java/com/yahoo/vespa/flags/IntFlag.java @@ -0,0 +1,18 @@ +// Copyright 2019 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.flags; + +import javax.annotation.concurrent.Immutable; + +/** + * @author hakonhall + */ +@Immutable +public class IntFlag extends FlagImpl { + public IntFlag(FlagId id, int defaultValue, FetchVector vector, FlagSerializer serializer, FlagSource source) { + super(id, defaultValue, vector, serializer, source, IntFlag::new); + } + + public int value() { + return boxedValue(); + } +} diff --git a/flags/src/main/java/com/yahoo/vespa/flags/JacksonFlag.java b/flags/src/main/java/com/yahoo/vespa/flags/JacksonFlag.java new file mode 100644 index 00000000000..e4afbd231a1 --- /dev/null +++ b/flags/src/main/java/com/yahoo/vespa/flags/JacksonFlag.java @@ -0,0 +1,18 @@ +// Copyright 2019 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.flags; + +import javax.annotation.concurrent.Immutable; + +/** + * @author hakonhall + */ +@Immutable +public class JacksonFlag extends FlagImpl> { + public JacksonFlag(FlagId id, T defaultValue, FetchVector vector, FlagSerializer serializer, FlagSource source) { + super(id, defaultValue, vector, serializer, source, JacksonFlag::new); + } + + public T value() { + return boxedValue(); + } +} diff --git a/flags/src/main/java/com/yahoo/vespa/flags/LongFlag.java b/flags/src/main/java/com/yahoo/vespa/flags/LongFlag.java new file mode 100644 index 00000000000..8a1707c9e6a --- /dev/null +++ b/flags/src/main/java/com/yahoo/vespa/flags/LongFlag.java @@ -0,0 +1,18 @@ +// Copyright 2019 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.flags; + +import javax.annotation.concurrent.Immutable; + +/** + * @author hakonhall + */ +@Immutable +public class LongFlag extends FlagImpl { + public LongFlag(FlagId id, Long defaultValue, FetchVector vector, FlagSerializer serializer, FlagSource source) { + super(id, defaultValue, vector, serializer, source, LongFlag::new); + } + + public long value() { + return boxedValue(); + } +} diff --git a/flags/src/main/java/com/yahoo/vespa/flags/OrderedFlagSource.java b/flags/src/main/java/com/yahoo/vespa/flags/OrderedFlagSource.java index 6ca74715999..1c6e113f489 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/OrderedFlagSource.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/OrderedFlagSource.java @@ -15,7 +15,6 @@ public class OrderedFlagSource implements FlagSource { private final List sources; /** - * * @param sources Flag sources in descending priority order. */ public OrderedFlagSource(FlagSource... sources) { diff --git a/flags/src/main/java/com/yahoo/vespa/flags/StringFlag.java b/flags/src/main/java/com/yahoo/vespa/flags/StringFlag.java new file mode 100644 index 00000000000..925af130b95 --- /dev/null +++ b/flags/src/main/java/com/yahoo/vespa/flags/StringFlag.java @@ -0,0 +1,18 @@ +// Copyright 2019 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.flags; + +import javax.annotation.concurrent.Immutable; + +/** + * @author hakonhall + */ +@Immutable +public class StringFlag extends FlagImpl { + public StringFlag(FlagId id, String defaultValue, FetchVector vector, FlagSerializer serializer, FlagSource source) { + super(id, defaultValue, vector, serializer, source, StringFlag::new); + } + + public String value() { + return boxedValue(); + } +} diff --git a/flags/src/main/java/com/yahoo/vespa/flags/UnboundBooleanFlag.java b/flags/src/main/java/com/yahoo/vespa/flags/UnboundBooleanFlag.java new file mode 100644 index 00000000000..c228364434c --- /dev/null +++ b/flags/src/main/java/com/yahoo/vespa/flags/UnboundBooleanFlag.java @@ -0,0 +1,27 @@ +// Copyright 2019 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.flags; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.BooleanNode; + +import javax.annotation.concurrent.Immutable; + +/** + * @author hakonhall + */ +@Immutable +public class UnboundBooleanFlag extends UnboundFlagImpl { + public UnboundBooleanFlag(FlagId id) { + this(id, false); + } + + public UnboundBooleanFlag(FlagId id, boolean defaultValue) { + this(id, defaultValue, new FetchVector()); + } + + public UnboundBooleanFlag(FlagId id, boolean defaultValue, FetchVector defaultFetchVector) { + super(id, defaultValue, defaultFetchVector, + new SimpleFlagSerializer<>(BooleanNode::valueOf, JsonNode::isBoolean, JsonNode::asBoolean), + UnboundBooleanFlag::new, BooleanFlag::new); + } +} diff --git a/flags/src/main/java/com/yahoo/vespa/flags/UnboundFlag.java b/flags/src/main/java/com/yahoo/vespa/flags/UnboundFlag.java index 597e19080ab..28307536233 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/UnboundFlag.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/UnboundFlag.java @@ -1,38 +1,27 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright 2019 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.flags; -import javax.annotation.concurrent.Immutable; - /** * @author hakonhall */ -@Immutable -public class UnboundFlag { - private final FlagId id; - private final T defaultValue; - private final Deserializer deserializer; - private final FetchVector fetchVector; - - public UnboundFlag(String flagId, T defaultValue, Deserializer deserializer) { - this(new FlagId(flagId), defaultValue, deserializer, new FetchVector()); - } - public UnboundFlag(FlagId id, T defaultValue, Deserializer deserializer, FetchVector fetchVector) { - this.id = id; - this.defaultValue = defaultValue; - this.deserializer = deserializer; - this.fetchVector = fetchVector; - } +/** + * Interface of an unbound flag. + * + * @param Type of boxed value, e.g. Integer + * @param Type of flag, e.g. IntFlag + * @param Type of unbound flag, e.g. UnboundIntFlag + */ +public interface UnboundFlag, U extends UnboundFlag> { + /** The flag ID. */ + FlagId id(); - public FlagId id() { - return id; - } + /** Returns the flag serializer. */ + FlagSerializer serializer(); - public UnboundFlag with(FetchVector.Dimension dimension, String value) { - return new UnboundFlag<>(id, defaultValue, deserializer, fetchVector.with(dimension, value)); - } + /** Returns a clone of the unbound flag, but with the dimension set accordingly. */ + U with(FetchVector.Dimension dimension, String dimensionValue); - public Flag bindTo(FlagSource source) { - return new Flag<>(id, defaultValue, deserializer, fetchVector, source); - } + /** Binds to a flag source, returning a (bound) flag. */ + F bindTo(FlagSource source); } diff --git a/flags/src/main/java/com/yahoo/vespa/flags/UnboundFlagImpl.java b/flags/src/main/java/com/yahoo/vespa/flags/UnboundFlagImpl.java new file mode 100644 index 00000000000..513712d8753 --- /dev/null +++ b/flags/src/main/java/com/yahoo/vespa/flags/UnboundFlagImpl.java @@ -0,0 +1,58 @@ +// Copyright 2019 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.flags; + +/** + * @author hakonhall + */ +public abstract class UnboundFlagImpl, U extends UnboundFlag> + implements UnboundFlag { + + private final FlagId id; + private final T defaultValue; + private final FlagSerializer serializer; + private final FetchVector defaultFetchVector; + private final UnboundFlagFactory unboundFlagFactory; + private final FlagFactory flagFactory; + + public interface UnboundFlagFactory, U1 extends UnboundFlag> { + U1 create(FlagId id, T1 defaultValue, FetchVector fetchVector); + } + + public interface FlagFactory> { + F2 create(FlagId id, T2 defaultValue, FetchVector fetchVector, FlagSerializer serializer, FlagSource source); + } + + protected UnboundFlagImpl(FlagId id, + T defaultValue, + FetchVector defaultFetchVector, + FlagSerializer serializer, + UnboundFlagFactory unboundFlagFactory, + FlagFactory flagFactory) { + this.id = id; + this.defaultValue = defaultValue; + this.serializer = serializer; + this.defaultFetchVector = defaultFetchVector; + this.unboundFlagFactory = unboundFlagFactory; + this.flagFactory = flagFactory; + } + + @Override + public FlagId id() { + return id; + } + + @Override + public U with(FetchVector.Dimension dimension, String dimensionValue) { + return unboundFlagFactory.create(id, defaultValue, defaultFetchVector.with(dimension, dimensionValue)); + } + + @Override + public F bindTo(FlagSource source) { + return flagFactory.create(id, defaultValue, defaultFetchVector, serializer, source); + } + + @Override + public FlagSerializer serializer() { + return serializer; + } +} diff --git a/flags/src/main/java/com/yahoo/vespa/flags/UnboundIntFlag.java b/flags/src/main/java/com/yahoo/vespa/flags/UnboundIntFlag.java new file mode 100644 index 00000000000..01b660fe625 --- /dev/null +++ b/flags/src/main/java/com/yahoo/vespa/flags/UnboundIntFlag.java @@ -0,0 +1,23 @@ +// Copyright 2019 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.flags; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.IntNode; + +import javax.annotation.concurrent.Immutable; + +/** + * @author hakonhall + */ +@Immutable +public class UnboundIntFlag extends UnboundFlagImpl { + public UnboundIntFlag(FlagId id, int defaultValue) { + this(id, defaultValue, new FetchVector()); + } + + public UnboundIntFlag(FlagId id, int defaultValue, FetchVector defaultFetchVector) { + super(id, defaultValue, defaultFetchVector, + new SimpleFlagSerializer<>(IntNode::new, JsonNode::isIntegralNumber, JsonNode::asInt), + UnboundIntFlag::new, IntFlag::new); + } +} diff --git a/flags/src/main/java/com/yahoo/vespa/flags/UnboundJacksonFlag.java b/flags/src/main/java/com/yahoo/vespa/flags/UnboundJacksonFlag.java new file mode 100644 index 00000000000..1cd303d1ede --- /dev/null +++ b/flags/src/main/java/com/yahoo/vespa/flags/UnboundJacksonFlag.java @@ -0,0 +1,21 @@ +// Copyright 2019 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.flags; + +import javax.annotation.concurrent.Immutable; + +/** + * @author hakonhall + */ +@Immutable +public class UnboundJacksonFlag extends UnboundFlagImpl, UnboundJacksonFlag> { + public UnboundJacksonFlag(FlagId id, T defaultValue, Class jacksonClass) { + this(id, defaultValue, new FetchVector(), jacksonClass); + } + + public UnboundJacksonFlag(FlagId id, T defaultValue, FetchVector defaultFetchVector, Class jacksonClass) { + super(id, defaultValue, defaultFetchVector, + new JacksonSerializer<>(jacksonClass), + (id2, defaultValue2, vector2) -> new UnboundJacksonFlag<>(id2, defaultValue2, vector2, jacksonClass), + JacksonFlag::new); + } +} diff --git a/flags/src/main/java/com/yahoo/vespa/flags/UnboundLongFlag.java b/flags/src/main/java/com/yahoo/vespa/flags/UnboundLongFlag.java new file mode 100644 index 00000000000..732a9073fc8 --- /dev/null +++ b/flags/src/main/java/com/yahoo/vespa/flags/UnboundLongFlag.java @@ -0,0 +1,23 @@ +// Copyright 2019 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.flags; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.LongNode; + +import javax.annotation.concurrent.Immutable; + +/** + * @author hakonhall + */ +@Immutable +public class UnboundLongFlag extends UnboundFlagImpl { + public UnboundLongFlag(FlagId id, long defaultValue) { + this(id, defaultValue, new FetchVector()); + } + + public UnboundLongFlag(FlagId id, Long defaultValue, FetchVector defaultFetchVector) { + super(id, defaultValue, defaultFetchVector, + new SimpleFlagSerializer<>(LongNode::new, JsonNode::isIntegralNumber, JsonNode::asLong), + UnboundLongFlag::new, LongFlag::new); + } +} diff --git a/flags/src/main/java/com/yahoo/vespa/flags/UnboundStringFlag.java b/flags/src/main/java/com/yahoo/vespa/flags/UnboundStringFlag.java new file mode 100644 index 00000000000..67c4bceb3e9 --- /dev/null +++ b/flags/src/main/java/com/yahoo/vespa/flags/UnboundStringFlag.java @@ -0,0 +1,23 @@ +// Copyright 2019 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.flags; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.TextNode; + +import javax.annotation.concurrent.Immutable; + +/** + * @author hakonhall + */ +@Immutable +public class UnboundStringFlag extends UnboundFlagImpl { + public UnboundStringFlag(FlagId id, String defaultValue) { + this(id, defaultValue, new FetchVector()); + } + + public UnboundStringFlag(FlagId id, String defaultValue, FetchVector defaultFetchVector) { + super(id, defaultValue, defaultFetchVector, + new SimpleFlagSerializer<>(TextNode::new, JsonNode::isTextual, JsonNode::asText), + UnboundStringFlag::new, StringFlag::new); + } +} 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 bfd959e82f9..b5bc2032bae 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 @@ -2,6 +2,7 @@ package com.yahoo.vespa.flags.json; import com.fasterxml.jackson.databind.JsonNode; +import com.yahoo.vespa.flags.Deserializer; import com.yahoo.vespa.flags.FetchVector; import com.yahoo.vespa.flags.FlagId; import com.yahoo.vespa.flags.FlagSource; @@ -16,9 +17,9 @@ 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; +import java.util.stream.Stream; /** * A data structure containing all data for a single flag, that can be serialized to/from JSON, @@ -100,8 +101,12 @@ public class FlagData { } private static FlagData fromWire(WireFlagData wireFlagData) { + if (wireFlagData.id == null) { + throw new IllegalArgumentException("Flag ID missing"); + } + return new FlagData( - new FlagId(Objects.requireNonNull(wireFlagData.id)), + new FlagId(wireFlagData.id), FetchVectorHelper.fromWire(wireFlagData.defaultFetchVector), rulesFromWire(wireFlagData.rules) ); @@ -111,5 +116,13 @@ public class FlagData { if (wireRules == null) return Collections.emptyList(); return wireRules.stream().map(Rule::fromWire).collect(Collectors.toList()); } + + /** E.g. verify all RawFlag can be deserialized. */ + public void validate(Deserializer deserializer) { + rules.stream() + .flatMap(rule -> rule.getValueToApply().map(Stream::of).orElse(null)) + .forEach(deserializer::deserialize); + + } } diff --git a/flags/src/test/java/com/yahoo/vespa/flags/FileFlagSourceTest.java b/flags/src/test/java/com/yahoo/vespa/flags/FileFlagSourceTest.java index 7d9d7868308..e19436069b5 100644 --- a/flags/src/test/java/com/yahoo/vespa/flags/FileFlagSourceTest.java +++ b/flags/src/test/java/com/yahoo/vespa/flags/FileFlagSourceTest.java @@ -24,44 +24,44 @@ public class FileFlagSourceTest { @Test public void testFeatureLikeFlags() throws IOException { - Flag featureFlag = new Flag<>(id, false, source, Flags.BOOLEAN_SERIALIZER); - Flag byDefaultTrue = new Flag<>(id, true, source, Flags.BOOLEAN_SERIALIZER); + BooleanFlag booleanFlag = new UnboundBooleanFlag(id).bindTo(source); + BooleanFlag byDefaultTrue = new UnboundBooleanFlag(id, true).bindTo(source); - assertFalse(featureFlag.value()); + assertFalse(booleanFlag.value()); assertTrue(byDefaultTrue.value()); writeFlag(id.toString(), "true\n"); - assertTrue(featureFlag.value()); + assertTrue(booleanFlag.value()); assertTrue(byDefaultTrue.value()); writeFlag(id.toString(), "false\n"); - assertFalse(featureFlag.value()); + assertFalse(booleanFlag.value()); assertFalse(byDefaultTrue.value()); } @Test public void testIntegerLikeFlags() throws IOException { - Flag intFlag = new Flag<>(id, -1, source, Flags.INT_SERIALIZER); - Flag longFlag = new Flag<>(id, -2L, source, Flags.LONG_SERIALIZER); + IntFlag intFlag = new UnboundIntFlag(id, -1).bindTo(source); + LongFlag longFlag = new UnboundLongFlag(id, -2L).bindTo(source); assertFalse(fetch().isPresent()); assertFalse(fetch().isPresent()); - assertEquals(-1, (int) intFlag.value()); - assertEquals(-2L, (long) longFlag.value()); + assertEquals(-1, intFlag.value()); + assertEquals(-2L, longFlag.value()); writeFlag(id.toString(), "1\n"); assertTrue(fetch().isPresent()); assertTrue(fetch().isPresent()); - assertEquals(1, (int) intFlag.value()); - assertEquals(1L, (long) longFlag.value()); + assertEquals(1, intFlag.value()); + assertEquals(1L, longFlag.value()); } @Test public void testStringFlag() throws IOException { - Flag stringFlag = new Flag<>(id, "default", source, Flags.STRING_SERIALIZER); + StringFlag stringFlag = new UnboundStringFlag(id, "default").bindTo(source); assertFalse(fetch().isPresent()); assertEquals("default", stringFlag.value()); @@ -71,11 +71,11 @@ public class FileFlagSourceTest { @Test public void parseFailure() throws IOException { - Flag featureFlag = new Flag<>(id, false, source, Flags.BOOLEAN_SERIALIZER); - writeFlag(featureFlag.id().toString(), "garbage"); + BooleanFlag booleanFlag = new UnboundBooleanFlag(id).bindTo(source); + writeFlag(booleanFlag.id().toString(), "garbage"); try { - featureFlag.value(); + booleanFlag.value(); } catch (UncheckedIOException e) { assertThat(e.getMessage(), containsString("garbage")); } diff --git a/flags/src/test/java/com/yahoo/vespa/flags/FlagsTest.java b/flags/src/test/java/com/yahoo/vespa/flags/FlagsTest.java index 4f7d797e07d..591198f5e50 100644 --- a/flags/src/test/java/com/yahoo/vespa/flags/FlagsTest.java +++ b/flags/src/test/java/com/yahoo/vespa/flags/FlagsTest.java @@ -31,7 +31,7 @@ public class FlagsTest { public void testBoolean() { final boolean defaultValue = false; FlagSource source = mock(FlagSource.class); - Flag booleanFlag = Flags.defineBoolean("id", defaultValue, "description", + BooleanFlag booleanFlag = Flags.defineFeatureFlag("id", defaultValue, "description", "modification effect", FetchVector.Dimension.ZONE_ID, FetchVector.Dimension.HOSTNAME) .with(FetchVector.Dimension.ZONE_ID, "a-zone") .bindTo(source); @@ -65,19 +65,19 @@ public class FlagsTest { @Test public void testString() { - testGeneric(Flags.defineString("string-id", "default value", "description", + testGeneric(Flags.defineStringFlag("string-id", "default value", "description", "modification effect", FetchVector.Dimension.ZONE_ID, FetchVector.Dimension.HOSTNAME), "default value", "other value"); } @Test public void testInt() { - testGeneric(Flags.defineInt("int-id", 2, "desc", "mod"), 2, 3); + testGeneric(Flags.defineIntFlag("int-id", 2, "desc", "mod"), 2, 3); } @Test public void testLong() { - testGeneric(Flags.defineLong("long-id", 1L, "desc", "mod"), 1L, 2L); + testGeneric(Flags.defineLongFlag("long-id", 1L, "desc", "mod"), 1L, 2L); } @Test @@ -87,25 +87,24 @@ public class FlagsTest { instance.integer = -2; instance.string = "foo"; - testGeneric(Flags.defineJackson("jackson-id", ExampleJacksonClass.class, defaultInstance, + testGeneric(Flags.defineJacksonFlag("jackson-id", defaultInstance, ExampleJacksonClass.class, "description", "modification effect", FetchVector.Dimension.HOSTNAME), defaultInstance, instance); } - private void testGeneric(UnboundFlag unboundFlag, T defaultValue, T value) { + private void testGeneric(UnboundFlag unboundFlag, T defaultValue, T value) { FlagSource source = mock(FlagSource.class); - Flag flag = unboundFlag.bindTo(source); + Flag flag = unboundFlag.bindTo(source); when(source.fetch(any(), any())).thenReturn(Optional.empty()); - assertThat(flag.value(), equalTo(defaultValue)); + assertThat(flag.boxedValue(), equalTo(defaultValue)); when(source.fetch(any(), any())).thenReturn(Optional.of(JsonNodeRawFlag.fromJacksonClass(value))); - assertThat(flag.value(), equalTo(value)); + assertThat(flag.boxedValue(), equalTo(value)); assertTrue(Flags.getFlag(unboundFlag.id()).isPresent()); } - @JsonIgnoreProperties(ignoreUnknown = true) private static class ExampleJacksonClass { @JsonProperty("integer") diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/health/HealthMonitorManager.java b/service-monitor/src/main/java/com/yahoo/vespa/service/health/HealthMonitorManager.java index 340d30e9f41..682f677a626 100644 --- a/service-monitor/src/main/java/com/yahoo/vespa/service/health/HealthMonitorManager.java +++ b/service-monitor/src/main/java/com/yahoo/vespa/service/health/HealthMonitorManager.java @@ -8,7 +8,7 @@ import com.yahoo.vespa.applicationmodel.ClusterId; import com.yahoo.vespa.applicationmodel.ConfigId; import com.yahoo.vespa.applicationmodel.ServiceStatus; import com.yahoo.vespa.applicationmodel.ServiceType; -import com.yahoo.vespa.flags.Flag; +import com.yahoo.vespa.flags.BooleanFlag; import com.yahoo.vespa.flags.FlagSource; import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.service.duper.DuperModelManager; @@ -50,7 +50,7 @@ public class HealthMonitorManager implements MonitorManager { private final ConcurrentHashMap healthMonitors = new ConcurrentHashMap<>(); private final DuperModelManager duperModel; private final ApplicationHealthMonitorFactory applicationHealthMonitorFactory; - private final Flag monitorInfra; + private final BooleanFlag monitorInfra; @Inject public HealthMonitorManager(DuperModelManager duperModel, FlagSource flagSource) { @@ -60,13 +60,13 @@ public class HealthMonitorManager implements MonitorManager { } private HealthMonitorManager(DuperModelManager duperModel, - Flag monitorInfra, + BooleanFlag monitorInfra, StateV1HealthModel healthModel) { this(duperModel, monitorInfra, id -> new ApplicationHealthMonitor(id, healthModel)); } HealthMonitorManager(DuperModelManager duperModel, - Flag monitorInfra, + BooleanFlag monitorInfra, ApplicationHealthMonitorFactory applicationHealthMonitorFactory) { this.duperModel = duperModel; this.monitorInfra = monitorInfra; diff --git a/service-monitor/src/test/java/com/yahoo/vespa/service/health/HealthMonitorManagerTest.java b/service-monitor/src/test/java/com/yahoo/vespa/service/health/HealthMonitorManagerTest.java index 39147955d4c..78b0bed0e6f 100644 --- a/service-monitor/src/test/java/com/yahoo/vespa/service/health/HealthMonitorManagerTest.java +++ b/service-monitor/src/test/java/com/yahoo/vespa/service/health/HealthMonitorManagerTest.java @@ -7,7 +7,7 @@ import com.yahoo.vespa.applicationmodel.ClusterId; import com.yahoo.vespa.applicationmodel.ConfigId; import com.yahoo.vespa.applicationmodel.ServiceStatus; import com.yahoo.vespa.applicationmodel.ServiceType; -import com.yahoo.vespa.flags.Flag; +import com.yahoo.vespa.flags.BooleanFlag; import com.yahoo.vespa.service.duper.ConfigServerApplication; import com.yahoo.vespa.service.duper.ControllerHostApplication; import com.yahoo.vespa.service.duper.DuperModelManager; @@ -33,8 +33,7 @@ import static org.mockito.Mockito.when; public class HealthMonitorManagerTest { private final ConfigServerApplication configServerApplication = new ConfigServerApplication(); private final DuperModelManager duperModel = mock(DuperModelManager.class); - @SuppressWarnings("unchecked") - private final Flag monitorInfra = (Flag) mock(Flag.class); + private final BooleanFlag monitorInfra = mock(BooleanFlag.class); private final ApplicationHealthMonitor monitor = mock(ApplicationHealthMonitor.class); private final ApplicationHealthMonitorFactory monitorFactory = mock(ApplicationHealthMonitorFactory.class); private final HealthMonitorManager manager = new HealthMonitorManager(duperModel, monitorInfra, monitorFactory); -- cgit v1.2.3