From 3f9f2b606bd9b8f9a7a0dc71883faa88b942e1cc Mon Sep 17 00:00:00 2001 From: LaunchDarklyCI Date: Fri, 2 Jul 2021 13:01:29 -0700 Subject: [PATCH] prepare 5.6.0 release (#241) --- .../sdk/server/FeatureFlagsState.java | 139 +++++++++++++----- .../com/launchdarkly/sdk/server/LDClient.java | 28 +++- .../com/launchdarkly/sdk/server/Util.java | 19 +++ .../sdk/server/integrations/FileData.java | 8 +- .../sdk/server/FeatureFlagsStateTest.java | 108 +++++++------- .../launchdarkly/sdk/server/LDClientTest.java | 29 ++++ 6 files changed, 236 insertions(+), 95 deletions(-) diff --git a/src/main/java/com/launchdarkly/sdk/server/FeatureFlagsState.java b/src/main/java/com/launchdarkly/sdk/server/FeatureFlagsState.java index 948d83a72..b7befe5f6 100644 --- a/src/main/java/com/launchdarkly/sdk/server/FeatureFlagsState.java +++ b/src/main/java/com/launchdarkly/sdk/server/FeatureFlagsState.java @@ -1,5 +1,7 @@ package com.launchdarkly.sdk.server; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Maps; import com.google.gson.TypeAdapter; import com.google.gson.annotations.JsonAdapter; import com.google.gson.stream.JsonReader; @@ -10,7 +12,6 @@ import com.launchdarkly.sdk.server.interfaces.LDClientInterface; import java.io.IOException; -import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.Objects; @@ -36,19 +37,20 @@ */ @JsonAdapter(FeatureFlagsState.JsonSerialization.class) public final class FeatureFlagsState implements JsonSerializable { - private final Map flagValues; - private final Map flagMetadata; + private final ImmutableMap flagMetadata; private final boolean valid; static class FlagMetadata { + final LDValue value; final Integer variation; final EvaluationReason reason; final Integer version; final Boolean trackEvents; final Long debugEventsUntilDate; - FlagMetadata(Integer variation, EvaluationReason reason, Integer version, boolean trackEvents, - Long debugEventsUntilDate) { + FlagMetadata(LDValue value, Integer variation, EvaluationReason reason, Integer version, + boolean trackEvents, Long debugEventsUntilDate) { + this.value = LDValue.normalize(value); this.variation = variation; this.reason = reason; this.version = version; @@ -60,7 +62,8 @@ static class FlagMetadata { public boolean equals(Object other) { if (other instanceof FlagMetadata) { FlagMetadata o = (FlagMetadata)other; - return Objects.equals(variation, o.variation) && + return value.equals(o.value) && + Objects.equals(variation, o.variation) && Objects.equals(reason, o.reason) && Objects.equals(version, o.version) && Objects.equals(trackEvents, o.trackEvents) && @@ -75,13 +78,27 @@ public int hashCode() { } } - private FeatureFlagsState(Map flagValues, - Map flagMetadata, boolean valid) { - this.flagValues = Collections.unmodifiableMap(flagValues); - this.flagMetadata = Collections.unmodifiableMap(flagMetadata); + private FeatureFlagsState(ImmutableMap flagMetadata, boolean valid) { + this.flagMetadata = flagMetadata; this.valid = valid; } + /** + * Returns a {@link Builder} for creating instances. + *

+ * Application code will not normally use this builder, since the SDK creates its own instances. + * However, it may be useful in testing, to simulate values that might be returned by + * {@link LDClient#allFlagsState(com.launchdarkly.sdk.LDUser, FlagsStateOption...)}. + * + * @param options the same {@link FlagsStateOption}s, if any, that would be passed to + * {@link LDClient#allFlagsState(com.launchdarkly.sdk.LDUser, FlagsStateOption...)} + * @return a builder object + * @since 5.6.0 + */ + public static Builder builder(FlagsStateOption... options) { + return new Builder(options); + } + /** * Returns true if this object contains a valid snapshot of feature flag state, or false if the * state could not be computed (for instance, because the client was offline or there was no user). @@ -98,7 +115,8 @@ public boolean isValid() { * {@code null} if there was no such flag */ public LDValue getFlagValue(String key) { - return flagValues.get(key); + FlagMetadata data = flagMetadata.get(key); + return data == null ? null : data.value; } /** @@ -115,20 +133,21 @@ public EvaluationReason getFlagReason(String key) { * Returns a map of flag keys to flag values. If a flag would have evaluated to the default value, * its value will be null. *

+ * The returned map is unmodifiable. + *

* Do not use this method if you are passing data to the front end to "bootstrap" the JavaScript client. * Instead, serialize the FeatureFlagsState object to JSON using {@code Gson.toJson()} or {@code Gson.toJsonTree()}. * @return an immutable map of flag keys to JSON values */ public Map toValuesMap() { - return flagValues; + return Maps.transformValues(flagMetadata, v -> v.value); } @Override public boolean equals(Object other) { if (other instanceof FeatureFlagsState) { FeatureFlagsState o = (FeatureFlagsState)other; - return flagValues.equals(o.flagValues) && - flagMetadata.equals(o.flagMetadata) && + return flagMetadata.equals(o.flagMetadata) && valid == o.valid; } return false; @@ -136,43 +155,78 @@ public boolean equals(Object other) { @Override public int hashCode() { - return Objects.hash(flagValues, flagMetadata, valid); + return Objects.hash(flagMetadata, valid); } - static class Builder { - private Map flagValues = new HashMap<>(); - private Map flagMetadata = new HashMap<>(); + /** + * A builder for a {@link FeatureFlagsState} instance. + *

+ * Application code will not normally use this builder, since the SDK creates its own instances. + * However, it may be useful in testing, to simulate values that might be returned by + * {@link LDClient#allFlagsState(com.launchdarkly.sdk.LDUser, FlagsStateOption...)}. + * + * @since 5.6.0 + */ + public static class Builder { + private ImmutableMap.Builder flagMetadata = ImmutableMap.builder(); private final boolean saveReasons; private final boolean detailsOnlyForTrackedFlags; private boolean valid = true; - Builder(FlagsStateOption... options) { + private Builder(FlagsStateOption... options) { saveReasons = FlagsStateOption.hasOption(options, FlagsStateOption.WITH_REASONS); detailsOnlyForTrackedFlags = FlagsStateOption.hasOption(options, FlagsStateOption.DETAILS_ONLY_FOR_TRACKED_FLAGS); } - Builder valid(boolean valid) { + /** + * Sets the {@link FeatureFlagsState#isValid()} property. This is true by default. + * + * @param valid the new property value + * @return the builder + */ + public Builder valid(boolean valid) { this.valid = valid; return this; } - Builder addFlag(DataModel.FeatureFlag flag, Evaluator.EvalResult eval) { - flagValues.put(flag.getKey(), eval.getValue()); - final boolean flagIsTracked = flag.isTrackEvents() || - (flag.getDebugEventsUntilDate() != null && flag.getDebugEventsUntilDate() > System.currentTimeMillis()); + public Builder add( + String flagKey, + LDValue value, + Integer variationIndex, + EvaluationReason reason, + int flagVersion, + boolean trackEvents, + Long debugEventsUntilDate + ) { + final boolean flagIsTracked = trackEvents || + (debugEventsUntilDate != null && debugEventsUntilDate > System.currentTimeMillis()); final boolean wantDetails = !detailsOnlyForTrackedFlags || flagIsTracked; FlagMetadata data = new FlagMetadata( + value, + variationIndex, + (saveReasons && wantDetails) ? reason : null, + wantDetails ? Integer.valueOf(flagVersion) : null, + trackEvents, + debugEventsUntilDate + ); + flagMetadata.put(flagKey, data); + return this; + } + + Builder addFlag(DataModel.FeatureFlag flag, Evaluator.EvalResult eval) { + return add( + flag.getKey(), + eval.getValue(), eval.isDefault() ? null : eval.getVariationIndex(), - (saveReasons && wantDetails) ? eval.getReason() : null, - wantDetails ? flag.getVersion() : null, + eval.getReason(), + flag.getVersion(), flag.isTrackEvents(), - flag.getDebugEventsUntilDate()); - flagMetadata.put(flag.getKey(), data); - return this; + flag.getDebugEventsUntilDate() + ); } FeatureFlagsState build() { - return new FeatureFlagsState(flagValues, flagMetadata, valid); + return new FeatureFlagsState(flagMetadata.build(), valid); } } @@ -181,9 +235,9 @@ static class JsonSerialization extends TypeAdapter { public void write(JsonWriter out, FeatureFlagsState state) throws IOException { out.beginObject(); - for (Map.Entry entry: state.flagValues.entrySet()) { + for (Map.Entry entry: state.flagMetadata.entrySet()) { out.name(entry.getKey()); - gsonInstance().toJson(entry.getValue(), LDValue.class, out); + gsonInstance().toJson(entry.getValue().value, LDValue.class, out); } out.name("$flagsState"); @@ -229,7 +283,7 @@ public void write(JsonWriter out, FeatureFlagsState state) throws IOException { @Override public FeatureFlagsState read(JsonReader in) throws IOException { Map flagValues = new HashMap<>(); - Map flagMetadata = new HashMap<>(); + Map flagMetadataWithoutValues = new HashMap<>(); boolean valid = true; in.beginObject(); while (in.hasNext()) { @@ -239,7 +293,7 @@ public FeatureFlagsState read(JsonReader in) throws IOException { while (in.hasNext()) { String metaName = in.nextName(); FlagMetadata meta = gsonInstance().fromJson(in, FlagMetadata.class); - flagMetadata.put(metaName, meta); + flagMetadataWithoutValues.put(metaName, meta); } in.endObject(); } else if (name.equals("$valid")) { @@ -250,7 +304,22 @@ public FeatureFlagsState read(JsonReader in) throws IOException { } } in.endObject(); - return new FeatureFlagsState(flagValues, flagMetadata, valid); + ImmutableMap.Builder allFlagMetadata = ImmutableMap.builder(); + for (Map.Entry e: flagValues.entrySet()) { + FlagMetadata m0 = flagMetadataWithoutValues.get(e.getKey()); + if (m0 != null) { + FlagMetadata m1 = new FlagMetadata( + e.getValue(), + m0.variation, + m0.reason, + m0.version, + m0.trackEvents != null && m0.trackEvents.booleanValue(), + m0.debugEventsUntilDate + ); + allFlagMetadata.put(e.getKey(), m1); + } + } + return new FeatureFlagsState(allFlagMetadata.build(), valid); } } } diff --git a/src/main/java/com/launchdarkly/sdk/server/LDClient.java b/src/main/java/com/launchdarkly/sdk/server/LDClient.java index edef1b31e..578f789a5 100644 --- a/src/main/java/com/launchdarkly/sdk/server/LDClient.java +++ b/src/main/java/com/launchdarkly/sdk/server/LDClient.java @@ -41,6 +41,7 @@ import static com.launchdarkly.sdk.EvaluationDetail.NO_VARIATION; import static com.launchdarkly.sdk.server.DataModel.FEATURES; import static com.launchdarkly.sdk.server.DataModel.SEGMENTS; +import static com.launchdarkly.sdk.server.Util.isAsciiHeaderValue; /** * A client for the LaunchDarkly API. Client instances are thread-safe. Applications should instantiate @@ -82,8 +83,15 @@ public final class LDClient implements LDClientInterface { * values; it will still continue trying to connect in the background. You can detect whether * initialization has succeeded by calling {@link #isInitialized()}. If you prefer to customize * this behavior, use {@link LDClient#LDClient(String, LDConfig)} instead. + *

+ * For rules regarding the throwing of unchecked exceptions for error conditions, see + * {@link LDClient#LDClient(String, LDConfig)}. * * @param sdkKey the SDK key for your LaunchDarkly environment + * @throws IllegalArgumentException if a parameter contained a grossly malformed value; + * for security reasons, in case of an illegal SDK key, the exception message does + * not include the key + * @throws NullPointerException if a non-nullable parameter was null * @see LDClient#LDClient(String, LDConfig) */ public LDClient(String sdkKey) { @@ -136,14 +144,32 @@ private static final DataModel.Segment getSegment(DataStore store, String key) { * // do whatever is appropriate if initialization has timed out * } * + *

+ * This constructor can throw unchecked exceptions if it is immediately apparent that + * the SDK cannot work with these parameters. For instance, if the SDK key contains a + * non-printable character that cannot be used in an HTTP header, it will throw an + * {@link IllegalArgumentException} since the SDK key is normally sent to LaunchDarkly + * in an HTTP header and no such value could possibly be valid. Similarly, a null + * value for a non-nullable parameter may throw a {@link NullPointerException}. The + * constructor will not throw an exception for any error condition that could only be + * detected after making a request to LaunchDarkly (such as an SDK key that is simply + * wrong despite being valid ASCII, so it is invalid but not illegal); those are logged + * and treated as an unsuccessful initialization, as described above. * * @param sdkKey the SDK key for your LaunchDarkly environment * @param config a client configuration object + * @throws IllegalArgumentException if a parameter contained a grossly malformed value; + * for security reasons, in case of an illegal SDK key, the exception message does + * not include the key + * @throws NullPointerException if a non-nullable parameter was null * @see LDClient#LDClient(String, LDConfig) */ public LDClient(String sdkKey, LDConfig config) { checkNotNull(config, "config must not be null"); this.sdkKey = checkNotNull(sdkKey, "sdkKey must not be null"); + if (!isAsciiHeaderValue(sdkKey) ) { + throw new IllegalArgumentException("SDK key contained an invalid character"); + } this.offline = config.offline; this.sharedExecutor = createSharedExecutor(config); @@ -268,7 +294,7 @@ private void sendFlagRequestEvent(Event.FeatureRequest event) { @Override public FeatureFlagsState allFlagsState(LDUser user, FlagsStateOption... options) { - FeatureFlagsState.Builder builder = new FeatureFlagsState.Builder(options); + FeatureFlagsState.Builder builder = FeatureFlagsState.builder(options); if (isOffline()) { Loggers.EVALUATION.debug("allFlagsState() was called when client is in offline mode."); diff --git a/src/main/java/com/launchdarkly/sdk/server/Util.java b/src/main/java/com/launchdarkly/sdk/server/Util.java index 1827080e5..fc6a4cc02 100644 --- a/src/main/java/com/launchdarkly/sdk/server/Util.java +++ b/src/main/java/com/launchdarkly/sdk/server/Util.java @@ -37,6 +37,25 @@ static Headers.Builder getHeadersBuilderFor(HttpConfiguration config) { return builder; } + // This is specifically testing whether the string would be considered a valid HTTP header value + // *by the OkHttp client*. The actual HTTP spec does not prohibit characters >= 127; OkHttp's + // check is overly strict, as was pointed out in https://github.com/square/okhttp/issues/2016. + // But all OkHttp 3.x and 4.x versions so far have continued to enforce that check. Control + // characters other than a tab are always illegal. + // + // The value we're mainly concerned with is the SDK key (Authorization header). If an SDK key + // accidentally has (for instance) a newline added to it, we don't want to end up having OkHttp + // throw an exception mentioning the value, which might get logged (https://github.com/square/okhttp/issues/6738). + static boolean isAsciiHeaderValue(String value) { + for (int i = 0; i < value.length(); i++) { + char ch = value.charAt(i); + if ((ch < 0x20 || ch > 0x7e) && ch != '\t') { + return false; + } + } + return true; + } + static void configureHttpClientBuilder(HttpConfiguration config, OkHttpClient.Builder builder) { builder.connectionPool(new ConnectionPool(5, 5, TimeUnit.SECONDS)) .connectTimeout(config.getConnectTimeout()) diff --git a/src/main/java/com/launchdarkly/sdk/server/integrations/FileData.java b/src/main/java/com/launchdarkly/sdk/server/integrations/FileData.java index 57f4357dc..630b53c26 100644 --- a/src/main/java/com/launchdarkly/sdk/server/integrations/FileData.java +++ b/src/main/java/com/launchdarkly/sdk/server/integrations/FileData.java @@ -1,5 +1,7 @@ package com.launchdarkly.sdk.server.integrations; +import com.launchdarkly.sdk.server.LDConfig; + /** * Integration between the LaunchDarkly SDK and file data. *

@@ -56,8 +58,10 @@ public enum DuplicateKeysHandling { *

* This will cause the client not to connect to LaunchDarkly to get feature flags. The * client may still make network connections to send analytics events, unless you have disabled - * this with {@link com.launchdarkly.sdk.server.Components#noEvents()} or - * {@link com.launchdarkly.sdk.server.LDConfig.Builder#offline(boolean)}. + * this with {@link com.launchdarkly.sdk.server.Components#noEvents()}. IMPORTANT: Do not + * set {@link LDConfig.Builder#offline(boolean)} to {@code true}; doing so would not just put the + * SDK "offline" with regard to LaunchDarkly, but will completely turn off all flag data sources + * to the SDK including the file data source. *

* Flag data files can be either JSON or YAML. They contain an object with three possible * properties: diff --git a/src/test/java/com/launchdarkly/sdk/server/FeatureFlagsStateTest.java b/src/test/java/com/launchdarkly/sdk/server/FeatureFlagsStateTest.java index 88657ebc7..2a0828884 100644 --- a/src/test/java/com/launchdarkly/sdk/server/FeatureFlagsStateTest.java +++ b/src/test/java/com/launchdarkly/sdk/server/FeatureFlagsStateTest.java @@ -29,85 +29,78 @@ public class FeatureFlagsStateTest { @Test public void canGetFlagValue() { - Evaluator.EvalResult eval = new Evaluator.EvalResult(LDValue.of("value"), 1, EvaluationReason.off()); - DataModel.FeatureFlag flag = flagBuilder("key").build(); - FeatureFlagsState state = new FeatureFlagsState.Builder().addFlag(flag, eval).build(); + FeatureFlagsState state = FeatureFlagsState.builder() + .add("key", LDValue.of("value"), 1, null, 10, false, null) + .build(); assertEquals(LDValue.of("value"), state.getFlagValue("key")); } @Test public void unknownFlagReturnsNullValue() { - FeatureFlagsState state = new FeatureFlagsState.Builder().build(); + FeatureFlagsState state = FeatureFlagsState.builder().build(); assertNull(state.getFlagValue("key")); } @Test public void canGetFlagReason() { - Evaluator.EvalResult eval = new Evaluator.EvalResult(LDValue.of("value1"), 1, EvaluationReason.off()); - DataModel.FeatureFlag flag = flagBuilder("key").build(); - FeatureFlagsState state = new FeatureFlagsState.Builder(FlagsStateOption.WITH_REASONS) - .addFlag(flag, eval).build(); + FeatureFlagsState state = FeatureFlagsState.builder(WITH_REASONS) + .add("key", LDValue.of("value"), 1, EvaluationReason.off(), 10, false, null) + .build(); assertEquals(EvaluationReason.off(), state.getFlagReason("key")); } @Test public void unknownFlagReturnsNullReason() { - FeatureFlagsState state = new FeatureFlagsState.Builder().build(); + FeatureFlagsState state = FeatureFlagsState.builder().build(); assertNull(state.getFlagReason("key")); } @Test public void reasonIsNullIfReasonsWereNotRecorded() { - Evaluator.EvalResult eval = new Evaluator.EvalResult(LDValue.of("value1"), 1, EvaluationReason.off()); - DataModel.FeatureFlag flag = flagBuilder("key").build(); - FeatureFlagsState state = new FeatureFlagsState.Builder().addFlag(flag, eval).build(); + FeatureFlagsState state = FeatureFlagsState.builder() + .add("key", LDValue.of("value"), 1, EvaluationReason.off(), 10, false, null) + .build(); assertNull(state.getFlagReason("key")); } @Test public void flagIsTreatedAsTrackedIfDebugEventsUntilDateIsInFuture() { - Evaluator.EvalResult eval = new Evaluator.EvalResult(LDValue.of("value1"), 1, EvaluationReason.off()); - DataModel.FeatureFlag flag = flagBuilder("key").debugEventsUntilDate(System.currentTimeMillis() + 1000000).build(); - FeatureFlagsState state = new FeatureFlagsState.Builder( - FlagsStateOption.WITH_REASONS, DETAILS_ONLY_FOR_TRACKED_FLAGS - ).addFlag(flag, eval).build(); + FeatureFlagsState state = FeatureFlagsState.builder(WITH_REASONS, DETAILS_ONLY_FOR_TRACKED_FLAGS) + .add("key", LDValue.of("value"), 1, EvaluationReason.off(), 10, false, System.currentTimeMillis() + 1000000) + .build(); assertNotNull(state.getFlagReason("key")); } @Test public void flagIsNotTreatedAsTrackedIfDebugEventsUntilDateIsInPast() { - Evaluator.EvalResult eval = new Evaluator.EvalResult(LDValue.of("value1"), 1, EvaluationReason.off()); - DataModel.FeatureFlag flag = flagBuilder("key").debugEventsUntilDate(System.currentTimeMillis() - 1000000).build(); - FeatureFlagsState state = new FeatureFlagsState.Builder( - FlagsStateOption.WITH_REASONS, DETAILS_ONLY_FOR_TRACKED_FLAGS - ).addFlag(flag, eval).build(); + FeatureFlagsState state = FeatureFlagsState.builder(WITH_REASONS, DETAILS_ONLY_FOR_TRACKED_FLAGS) + .add("key", LDValue.of("value"), 1, EvaluationReason.off(), 10, false, System.currentTimeMillis() - 1000000) + .build(); assertNull(state.getFlagReason("key")); } @Test public void flagCanHaveNullValue() { - Evaluator.EvalResult eval = new Evaluator.EvalResult(LDValue.ofNull(), 1, null); - DataModel.FeatureFlag flag = flagBuilder("key").build(); - FeatureFlagsState state = new FeatureFlagsState.Builder().addFlag(flag, eval).build(); + FeatureFlagsState state = FeatureFlagsState.builder() + .add("key", LDValue.ofNull(), 1, null, 10, false, null) + .build(); assertEquals(LDValue.ofNull(), state.getFlagValue("key")); } @Test public void canConvertToValuesMap() { - Evaluator.EvalResult eval1 = new Evaluator.EvalResult(LDValue.of("value1"), 0, EvaluationReason.off()); - DataModel.FeatureFlag flag1 = flagBuilder("key1").build(); - Evaluator.EvalResult eval2 = new Evaluator.EvalResult(LDValue.of("value2"), 1, EvaluationReason.fallthrough()); - DataModel.FeatureFlag flag2 = flagBuilder("key2").build(); - FeatureFlagsState state = new FeatureFlagsState.Builder() - .addFlag(flag1, eval1).addFlag(flag2, eval2).build(); + FeatureFlagsState state = FeatureFlagsState.builder() + .add("key1", LDValue.of("value1"), 0, null, 10, false, null) + .add("key2", LDValue.of("value2"), 1, null, 10, false, null) + .build(); ImmutableMap expected = ImmutableMap.of("key1", LDValue.of("value1"), "key2", LDValue.of("value2")); assertEquals(expected, state.toValuesMap()); @@ -115,24 +108,23 @@ public void canConvertToValuesMap() { @Test public void equalInstancesAreEqual() { - DataModel.FeatureFlag flag1 = flagBuilder("key1").build(); - DataModel.FeatureFlag flag2 = flagBuilder("key2").build(); - Evaluator.EvalResult eval1a = new Evaluator.EvalResult(LDValue.of("value1"), 0, EvaluationReason.off()); - Evaluator.EvalResult eval1b = new Evaluator.EvalResult(LDValue.of("value1"), 0, EvaluationReason.off()); - Evaluator.EvalResult eval2a = new Evaluator.EvalResult(LDValue.of("value2"), 1, EvaluationReason.fallthrough()); - Evaluator.EvalResult eval2b = new Evaluator.EvalResult(LDValue.of("value2"), 1, EvaluationReason.fallthrough()); - Evaluator.EvalResult eval3a = new Evaluator.EvalResult(LDValue.of("value1"), 1, EvaluationReason.off()); - - FeatureFlagsState justOneFlag = new FeatureFlagsState.Builder() - .addFlag(flag1, eval1a).build(); - FeatureFlagsState sameFlagsDifferentInstances1 = new FeatureFlagsState.Builder(WITH_REASONS) - .addFlag(flag1, eval1a).addFlag(flag2, eval2a).build(); - FeatureFlagsState sameFlagsDifferentInstances2 = new FeatureFlagsState.Builder(WITH_REASONS) - .addFlag(flag2, eval2b).addFlag(flag1, eval1b).build(); - FeatureFlagsState sameFlagsDifferentMetadata = new FeatureFlagsState.Builder(WITH_REASONS) - .addFlag(flag1, eval3a).addFlag(flag2, eval2a).build(); - FeatureFlagsState noFlagsButValid = new FeatureFlagsState.Builder(WITH_REASONS).build(); - FeatureFlagsState noFlagsAndNotValid = new FeatureFlagsState.Builder(WITH_REASONS).valid(false).build(); + FeatureFlagsState justOneFlag = FeatureFlagsState.builder(WITH_REASONS) + .add("key1", LDValue.of("value1"), 0, EvaluationReason.off(), 10, false, null) + .build(); + FeatureFlagsState sameFlagsDifferentInstances1 = FeatureFlagsState.builder(WITH_REASONS) + .add("key1", LDValue.of("value1"), 0, EvaluationReason.off(), 10, false, null) + .add("key2", LDValue.of("value2"), 1, EvaluationReason.fallthrough(), 10, false, null) + .build(); + FeatureFlagsState sameFlagsDifferentInstances2 = FeatureFlagsState.builder(WITH_REASONS) + .add("key1", LDValue.of("value1"), 0, EvaluationReason.off(), 10, false, null) + .add("key2", LDValue.of("value2"), 1, EvaluationReason.fallthrough(), 10, false, null) + .build(); + FeatureFlagsState sameFlagsDifferentMetadata = FeatureFlagsState.builder(WITH_REASONS) + .add("key1", LDValue.of("value1"), 1, EvaluationReason.off(), 10, false, null) + .add("key2", LDValue.of("value2"), 1, EvaluationReason.fallthrough(), 10, false, null) + .build(); + FeatureFlagsState noFlagsButValid = FeatureFlagsState.builder(WITH_REASONS).build(); + FeatureFlagsState noFlagsAndNotValid = FeatureFlagsState.builder(WITH_REASONS).valid(false).build(); assertEquals(sameFlagsDifferentInstances1, sameFlagsDifferentInstances2); assertEquals(sameFlagsDifferentInstances1.hashCode(), sameFlagsDifferentInstances2.hashCode()); @@ -148,13 +140,15 @@ public void equalMetadataInstancesAreEqual() { // Testing this various cases is easier at a low level - equalInstancesAreEqual() above already // verifies that we test for metadata equality in general List> allPermutations = new ArrayList<>(); - for (Integer variation: new Integer[] { null, 0, 1 }) { - for (EvaluationReason reason: new EvaluationReason[] { null, EvaluationReason.off(), EvaluationReason.fallthrough() }) { - for (Integer version: new Integer[] { null, 10, 11 }) { - for (boolean trackEvents: new boolean[] { false, true }) { - for (Long debugEventsUntilDate: new Long[] { null, 1000L, 1001L }) { - allPermutations.add(() -> new FeatureFlagsState.FlagMetadata( - variation, reason, version, trackEvents, debugEventsUntilDate)); + for (LDValue value: new LDValue[] { LDValue.of(1), LDValue.of(2) }) { + for (Integer variation: new Integer[] { null, 0, 1 }) { + for (EvaluationReason reason: new EvaluationReason[] { null, EvaluationReason.off(), EvaluationReason.fallthrough() }) { + for (Integer version: new Integer[] { null, 10, 11 }) { + for (boolean trackEvents: new boolean[] { false, true }) { + for (Long debugEventsUntilDate: new Long[] { null, 1000L, 1001L }) { + allPermutations.add(() -> new FeatureFlagsState.FlagMetadata( + value, variation, reason, version, trackEvents, debugEventsUntilDate)); + } } } } @@ -189,7 +183,7 @@ private static FeatureFlagsState makeInstanceForSerialization() { DataModel.FeatureFlag flag2 = flagBuilder("key2").version(200).trackEvents(true).debugEventsUntilDate(1000L).build(); Evaluator.EvalResult eval3 = new Evaluator.EvalResult(LDValue.of("default"), NO_VARIATION, EvaluationReason.error(MALFORMED_FLAG)); DataModel.FeatureFlag flag3 = flagBuilder("key3").version(300).build(); - return new FeatureFlagsState.Builder(FlagsStateOption.WITH_REASONS) + return FeatureFlagsState.builder(FlagsStateOption.WITH_REASONS) .addFlag(flag1, eval1).addFlag(flag2, eval2).addFlag(flag3, eval3).build(); } diff --git a/src/test/java/com/launchdarkly/sdk/server/LDClientTest.java b/src/test/java/com/launchdarkly/sdk/server/LDClientTest.java index 85efc339a..d16083f9f 100644 --- a/src/test/java/com/launchdarkly/sdk/server/LDClientTest.java +++ b/src/test/java/com/launchdarkly/sdk/server/LDClientTest.java @@ -37,6 +37,9 @@ import static org.easymock.EasyMock.capture; import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.isA; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -83,6 +86,32 @@ public void constructorWithConfigThrowsExceptionForNullSdkKey() throws Exception } } + @Test + public void constructorThrowsExceptionForSdkKeyWithControlCharacter() throws Exception { + try (LDClient client = new LDClient(SDK_KEY + "\n")) { + fail("expected exception"); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage(), not(containsString(SDK_KEY))); + } + } + + @Test + public void constructorWithConfigThrowsExceptionForSdkKeyWithControlCharacter() throws Exception { + try (LDClient client = new LDClient(SDK_KEY + "\n", LDConfig.DEFAULT)) { + fail("expected exception"); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage(), not(containsString(SDK_KEY))); + } + } + + @Test + public void constructorAllowsSdkKeyToBeEmpty() throws Exception { + // It may seem counter-intuitive to allow this, but if someone is using the SDK in offline + // mode, or with a file data source or a test fixture, they may reasonably assume that it's + // OK to pass an empty string since the key won't actually be used. + try (LDClient client = new LDClient(SDK_KEY + "")) {} + } + @Test public void constructorThrowsExceptionForNullConfig() throws Exception { try (LDClient client = new LDClient(SDK_KEY, null)) {