diff --git a/src/main/java/com/launchdarkly/client/EvaluationException.java b/src/main/java/com/launchdarkly/client/EvaluationException.java new file mode 100644 index 000000000..f7adf563f --- /dev/null +++ b/src/main/java/com/launchdarkly/client/EvaluationException.java @@ -0,0 +1,10 @@ +package com.launchdarkly.client; + +/** + * An error indicating an abnormal result from evaluating a feature + */ +class EvaluationException extends Exception { + public EvaluationException(String message) { + super(message); + } +} diff --git a/src/main/java/com/launchdarkly/client/FeatureFlag.java b/src/main/java/com/launchdarkly/client/FeatureFlag.java index f4f116c77..c839aa046 100644 --- a/src/main/java/com/launchdarkly/client/FeatureFlag.java +++ b/src/main/java/com/launchdarkly/client/FeatureFlag.java @@ -50,44 +50,33 @@ static Map fromJsonMap(String json) { this.deleted = deleted; } - Integer getOffVariation() { - return this.offVariation; - } - - JsonElement getOffVariationValue() { - if (offVariation != null && offVariation < variations.size()) { - return variations.get(offVariation); - } - return null; - } - - EvalResult evaluate(LDUser user, FeatureStore featureStore) { + EvalResult evaluate(LDUser user, FeatureStore featureStore) throws EvaluationException { if (user == null || user.getKey() == null) { - return null; + throw new EvaluationException("User or user key is null"); } List prereqEvents = new ArrayList<>(); - Set visited = new HashSet<>(); - JsonElement value = evaluate(user, featureStore, prereqEvents, visited); + JsonElement value = evaluate(user, featureStore, prereqEvents); return new EvalResult(value, prereqEvents); } // Returning either a JsonElement or null indicating prereq failure/error. - private JsonElement evaluate(LDUser user, FeatureStore featureStore, List events, Set visited) { + private JsonElement evaluate(LDUser user, FeatureStore featureStore, List events) throws EvaluationException { boolean prereqOk = true; - visited.add(key); for (Prerequisite prereq : prerequisites) { - if (visited.contains(prereq.getKey())) { - logger.error("Prerequisite cycle detected when evaluating feature flag: " + key); - return null; - } FeatureFlag prereqFeatureFlag = featureStore.get(prereq.getKey()); JsonElement prereqEvalResult = null; if (prereqFeatureFlag == null) { logger.error("Could not retrieve prerequisite flag: " + prereq.getKey() + " when evaluating: " + key); return null; } else if (prereqFeatureFlag.isOn()) { - prereqEvalResult = prereqFeatureFlag.evaluate(user, featureStore, events, visited); - if (prereqEvalResult == null || !prereqEvalResult.equals(prereqFeatureFlag.getVariation(prereq.getVariation()))) { + prereqEvalResult = prereqFeatureFlag.evaluate(user, featureStore, events); + try { + JsonElement variation = prereqFeatureFlag.getVariation(prereq.getVariation()); + if (prereqEvalResult == null || variation == null || !prereqEvalResult.equals(variation)) { + prereqOk = false; + } + } catch (EvaluationException err) { + logger.warn("Error evaluating prerequisites: " + err.getMessage()); prereqOk = false; } } else { @@ -123,10 +112,30 @@ private Integer evaluateIndex(LDUser user) { return fallthrough.variationIndexForUser(user, key, salt); } - private JsonElement getVariation(Integer index) { - if (index == null || index >= variations.size()) { + JsonElement getOffVariationValue() throws EvaluationException { + if (offVariation == null) { return null; - } else { + } + + if (offVariation >= variations.size()) { + throw new EvaluationException("Invalid off variation index"); + } + + return variations.get(offVariation); + } + + private JsonElement getVariation(Integer index) throws EvaluationException { + // If the supplied index is null, then rules didn't match, and we want to return + // the off variation + if (index == null) { + return null; + } + // If the index doesn't refer to a valid variation, that's an unexpected exception and we will + // return the default variation + else if (index >= variations.size()) { + throw new EvaluationException("Invalid index"); + } + else { return variations.get(index); } } @@ -171,6 +180,8 @@ List getVariations() { return variations; } + Integer getOffVariation() { return offVariation; } + static class EvalResult { private final JsonElement value; private final List prerequisiteEvents; diff --git a/src/main/java/com/launchdarkly/client/LDClient.java b/src/main/java/com/launchdarkly/client/LDClient.java index 940b53093..506450685 100644 --- a/src/main/java/com/launchdarkly/client/LDClient.java +++ b/src/main/java/com/launchdarkly/client/LDClient.java @@ -294,47 +294,36 @@ public JsonElement jsonVariation(String featureKey, LDUser user, JsonElement def return value; } - private JsonElement evaluate(String featureKey, LDUser user, JsonElement defaultValue) { if (!initialized()) { return defaultValue; } try { FeatureFlag featureFlag = config.featureStore.get(featureKey); - if (featureFlag != null) { - if (config.stream && config.debugStreaming) { - FeatureFlag pollingResult = requestor.makeRequest(featureKey, true); - if (!featureFlag.equals(pollingResult)) { - logger.warn("Mismatch between streaming and polling feature! Streaming: {} Polling: {}", featureFlag, pollingResult); - } - } - } else { + if (featureFlag == null) { logger.warn("Unknown feature flag " + featureKey + "; returning default value: "); return defaultValue; } if (featureFlag.isOn()) { FeatureFlag.EvalResult evalResult = featureFlag.evaluate(user, config.featureStore); - if (evalResult != null) { if (!isOffline()) { for (FeatureRequestEvent event : evalResult.getPrerequisiteEvents()) { eventProcessor.sendEvent(event); } } - if (evalResult.getValue() == null) { - return defaultValue; - } else { + if (evalResult.getValue() != null) { return evalResult.getValue(); } - } - } else { - JsonElement offVariation = featureFlag.getOffVariationValue(); - if (offVariation != null) { - return offVariation; - } + } + JsonElement offVariation = featureFlag.getOffVariationValue(); + if (offVariation != null) { + return offVariation; } } catch (Exception e) { logger.error("Encountered exception in LaunchDarkly client", e); } + + return defaultValue; } diff --git a/src/main/java/com/launchdarkly/client/LDConfig.java b/src/main/java/com/launchdarkly/client/LDConfig.java index 3407677cb..1dc0a959b 100644 --- a/src/main/java/com/launchdarkly/client/LDConfig.java +++ b/src/main/java/com/launchdarkly/client/LDConfig.java @@ -37,7 +37,6 @@ public final class LDConfig { final int flushInterval; final HttpHost proxyHost; final boolean stream; - final boolean debugStreaming; final FeatureStore featureStore; final boolean useLdd; final boolean offline; @@ -55,7 +54,6 @@ protected LDConfig(Builder builder) { this.proxyHost = builder.proxyHost(); this.streamURI = builder.streamURI; this.stream = builder.stream; - this.debugStreaming = builder.debugStreaming; this.featureStore = builder.featureStore; this.useLdd = builder.useLdd; this.offline = builder.offline; @@ -91,7 +89,6 @@ public static class Builder { private int proxyPort = -1; private String proxyScheme; private boolean stream = true; - private boolean debugStreaming = false; private boolean useLdd = false; private boolean offline = false; private long pollingIntervalMillis = DEFAULT_POLLING_INTERVAL_MILLIS; @@ -140,18 +137,6 @@ public Builder featureStore(FeatureStore store) { return this; } - /** - * Set whether we should debug streaming mode. If set, the client will fetch features via polling and compare the - * retrieved feature with the value in the feature store. There is a performance cost to this, so it is not - * recommended for production use. - * @param debugStreaming whether streaming mode should be debugged - * @return the builder - */ - public Builder debugStreaming(boolean debugStreaming) { - this.debugStreaming = debugStreaming; - return this; - } - /** * Set whether streaming mode should be enabled. By default, streaming is enabled. * @param stream whether streaming mode should be enabled diff --git a/src/main/java/com/launchdarkly/client/Operator.java b/src/main/java/com/launchdarkly/client/Operator.java index 980e32a84..b63c2c7e2 100644 --- a/src/main/java/com/launchdarkly/client/Operator.java +++ b/src/main/java/com/launchdarkly/client/Operator.java @@ -25,13 +25,6 @@ public boolean apply(JsonPrimitive uValue, JsonPrimitive cValue) { if (uValue.isNumber() && cValue.isNumber()) { return uValue.getAsDouble() == cValue.getAsDouble(); } - DateTime uDateTime = Util.jsonPrimitiveToDateTime(uValue); - if (uDateTime != null) { - DateTime cDateTime = Util.jsonPrimitiveToDateTime(cValue); - if (cDateTime != null) { - return uDateTime.getMillis() == cDateTime.getMillis(); - } - } return false; } }, diff --git a/src/test/java/com/launchdarkly/client/FeatureFlagTest.java b/src/test/java/com/launchdarkly/client/FeatureFlagTest.java index bcb87da1e..372591ddd 100644 --- a/src/test/java/com/launchdarkly/client/FeatureFlagTest.java +++ b/src/test/java/com/launchdarkly/client/FeatureFlagTest.java @@ -21,54 +21,7 @@ public void before() { } @Test - public void testPrereqSelfCycle() { - String keyA = "keyA"; - FeatureFlag f = newFlagWithPrereq(keyA, keyA); - - featureStore.upsert(keyA, f); - LDUser user = new LDUser.Builder("userKey").build(); - - FeatureFlag.EvalResult actual = f.evaluate(user, featureStore); - - Assert.assertNull(actual.getValue()); - Assert.assertNotNull(actual.getPrerequisiteEvents()); - Assert.assertEquals(0, actual.getPrerequisiteEvents().size()); - } - - @Test - public void testPrereqSimpleCycle() { - String keyA = "keyA"; - String keyB = "keyB"; - FeatureFlag f1 = newFlagWithPrereq(keyA, keyB); - FeatureFlag f2 = newFlagWithPrereq(keyB, keyA); - - featureStore.upsert(f1.getKey(), f1); - featureStore.upsert(f2.getKey(), f2); - LDUser user = new LDUser.Builder("userKey").build(); - Assert.assertNull(f1.evaluate(user, featureStore).getValue()); - Assert.assertNull(f2.evaluate(user, featureStore).getValue()); - } - - @Test - public void testPrereqCycle() { - String keyA = "keyA"; - String keyB = "keyB"; - String keyC = "keyC"; - FeatureFlag f1 = newFlagWithPrereq(keyA, keyB); - FeatureFlag f2 = newFlagWithPrereq(keyB, keyC); - FeatureFlag f3 = newFlagWithPrereq(keyC, keyA); - - featureStore.upsert(f1.getKey(), f1); - featureStore.upsert(f2.getKey(), f2); - featureStore.upsert(f3.getKey(), f3); - LDUser user = new LDUser.Builder("userKey").build(); - Assert.assertNull(f1.evaluate(user, featureStore).getValue()); - Assert.assertNull(f2.evaluate(user, featureStore).getValue()); - Assert.assertNull(f3.evaluate(user, featureStore).getValue()); - } - - @Test - public void testPrereqDoesNotExist() { + public void testPrereqDoesNotExist() throws EvaluationException { String keyA = "keyA"; String keyB = "keyB"; FeatureFlag f1 = newFlagWithPrereq(keyA, keyB); @@ -83,7 +36,7 @@ public void testPrereqDoesNotExist() { } @Test - public void testPrereqCollectsEventsForPrereqs() { + public void testPrereqCollectsEventsForPrereqs() throws EvaluationException { String keyA = "keyA"; String keyB = "keyB"; String keyC = "keyC";