diff --git a/lib/sdk/server/build.gradle b/lib/sdk/server/build.gradle index 1dadb4ed..399c78df 100644 --- a/lib/sdk/server/build.gradle +++ b/lib/sdk/server/build.gradle @@ -70,7 +70,7 @@ ext.versions = [ "gson": "2.13.1", "guava": "32.0.1-jre", "jackson": "2.11.2", - "launchdarklyJavaSdkCommon": "2.3.0", + "launchdarklyJavaSdkCommon": "2.6.0", "launchdarklyJavaSdkInternal": "1.11.1", "launchdarklyLogging": "1.1.0", "okhttp": "4.12.0", // specify this for the SDK build instead of relying on the transitive dependency from okhttp-eventsource diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModel.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModel.java index 1e919cfe..0390abd2 100644 --- a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModel.java +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DataModel.java @@ -145,6 +145,15 @@ static final class FeatureFlag implements VersionedData, JsonHelpers.PostProcess private Migration migration; private boolean excludeFromSummaries; + // True if this definition came from the SDK's override store rather than from LaunchDarkly. The + // field is transient so that it never appears in the JSON form of the flag and never takes part + // in deserialization. Only the override layer sets it, on a copy that it owns. Evaluation reads + // it to mark the evaluation as override-affected. Other readers treat a marked flag the same as + // any other flag. + // + // Flag overrides are currently experimental and subject to change. + private transient boolean isOverride; + /** * Container for migration specific flag data. */ @@ -267,6 +276,31 @@ boolean isExcludeFromSummaries() { return excludeFromSummaries; } + /** + * Returns true if this definition came from the override store. + * + * @return true for an override entry + */ + boolean isOverride() { + return isOverride; + } + + /** + * Returns a shallow copy of this flag that carries the override marker. The copy shares its + * nested lists and its preprocessing data with this flag and never writes to them. This flag is + * not modified. + * + * @return a marked copy + */ + FeatureFlag markedAsOverride() { + FeatureFlag copy = new FeatureFlag(key, version, on, prerequisites, salt, targets, contextTargets, rules, + fallthrough, offVariation, variations, clientSide, trackEvents, trackEventsFallthrough, + debugEventsUntilDate, deleted, samplingRatio, migration, excludeFromSummaries); + copy.preprocessed = preprocessed; + copy.isOverride = true; + return copy; + } + public void afterDeserialized() { DataModelPreprocessing.preprocessFlag(this); } @@ -507,6 +541,10 @@ static final class Segment implements VersionedData, JsonHelpers.PostProcessingD private ContextKind unboundedContextKind; private Integer generation; + // True if this definition came from the SDK's override store rather than from LaunchDarkly. See + // the note on the same field in FeatureFlag. + private transient boolean isOverride; + Segment() {} Segment(String key, @@ -588,6 +626,28 @@ public Integer getGeneration() { return generation; } + /** + * Returns true if this definition came from the override store. + * + * @return true for an override entry + */ + boolean isOverride() { + return isOverride; + } + + /** + * Returns a shallow copy of this segment that carries the override marker. The copy shares its + * nested collections with this segment and never writes to them. This segment is not modified. + * + * @return a marked copy + */ + Segment markedAsOverride() { + Segment copy = new Segment(key, included, excluded, includedContexts, excludedContexts, salt, rules, + version, deleted, unbounded, unboundedContextKind, generation); + copy.isOverride = true; + return copy; + } + public void afterDeserialized() { DataModelPreprocessing.preprocessSegment(this); } diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/EvalResult.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/EvalResult.java index 8611afbb..ac7a2b89 100644 --- a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/EvalResult.java +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/EvalResult.java @@ -228,6 +228,14 @@ public EvaluationDetail getAsString() { */ public boolean isForceReasonTracking() { return forceReasonTracking; } + /** + * Returns true if an override affected this evaluation, directly or transitively. The value is + * the reason's indicator, so the result and the reason it returns to the caller always agree. + * Flag overrides are currently experimental and subject to change. + * @return true if an override affected the evaluation + */ + public boolean isOverrideAffected() { return anyType.getReason().isOverrideAffected(); } + public List getPrerequisiteEvalRecords() { return prerequisiteEvalRecords; } /** @@ -251,6 +259,17 @@ public EvalResult withForceReasonTracking(boolean newValue) { public EvalResult withPrerequisiteEvalRecords(List newValue) { return this.prerequisiteEvalRecords == newValue ? this : new EvalResult(this, newValue); } + + /** + * Returns a transformed copy of this EvalResult whose reason carries the given override-affected + * indicator, or this same instance if the indicator is unchanged. Precomputed results are shared + * between evaluations, so a marked result is always a new instance. + * @param newValue the new value for the indicator + * @return a transformed copy + */ + public EvalResult withOverrideAffected(boolean newValue) { + return withReason(anyType.getReason().withOverrideAffected(newValue)); + } @Override public boolean equals(Object other) { diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/Evaluator.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/Evaluator.java index 845d4aaf..fefd1add 100644 --- a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/Evaluator.java +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/Evaluator.java @@ -124,6 +124,12 @@ private static class EvaluatorState { private List prerequisiteStack = null; private List prerequisiteEvalRecords = new ArrayList<>(0); // 0 initial capacity uses a static instance for performance private List segmentStack = null; + // True if the current evaluation scope has read a definition that carries the override marker. + // The scope starts from its own flag's marker. Each segment read can set it. Around a + // prerequisite evaluation the value is saved and reset, so the prerequisite's record reflects + // only the definitions that its own subtree read, and the parent scope accumulates that result + // afterwards. The marking therefore propagates upward only. + private boolean overrideAffected = false; } Evaluator(Getters getters, LDLogger logger) { @@ -146,6 +152,8 @@ EvalResult evaluate(FeatureFlag flag, LDContext context, @Nonnull EvaluationReco EvaluatorState state = new EvaluatorState(); state.originalFlag = flag; + // Reading the flag's own definition is the first read of this scope. + state.overrideAffected = flag.isOverride(); try { EvalResult result = evaluateInternal(flag, context, recorder, state); @@ -160,10 +168,12 @@ EvalResult evaluate(FeatureFlag flag, LDContext context, @Nonnull EvaluationReco result = result.withPrerequisiteEvalRecords(state.prerequisiteEvalRecords); } - return result; + return result.withOverrideAffected(state.overrideAffected); } catch (EvaluationException e) { logger.error("Could not evaluate flag \"{}\": {}", flag.getKey(), e.getMessage()); - return EvalResult.error(e.errorKind); + // An error result is marked too. A malformed override definition yields the caller's default + // value with an error reason, and an override still affected that result. + return EvalResult.error(e.errorKind).withOverrideAffected(state.overrideAffected); } } @@ -246,7 +256,20 @@ private EvalResult checkPrerequisites(FeatureFlag flag, LDContext context, @Nonn logger.error("Could not retrieve prerequisite flag \"{}\" when evaluating \"{}\"", prereq.getKey(), flag.getKey()); prereqOk = false; } else { - EvalResult prereqEvalResult = evaluateInternal(prereqFeatureFlag, context, recorder, state); + // The prerequisite evaluation is a scope of its own. Its marking starts from its own flag's + // marker, so its record reflects only the definitions that its subtree read. This scope + // accumulates that result afterwards, whether the nested evaluation returns or throws. + boolean parentOverrideAffected = state.overrideAffected; + state.overrideAffected = prereqFeatureFlag.isOverride(); + EvalResult prereqEvalResult; + boolean prereqOverrideAffected; + try { + prereqEvalResult = evaluateInternal(prereqFeatureFlag, context, recorder, state); + } finally { + prereqOverrideAffected = state.overrideAffected; + state.overrideAffected = parentOverrideAffected || prereqOverrideAffected; + } + prereqEvalResult = prereqEvalResult.withOverrideAffected(prereqOverrideAffected); // Note that if the prerequisite flag is off, we don't consider it a match no matter what its // off variation was. But we still need to evaluate it in order to generate an event. if (!prereqFeatureFlag.isOn() || prereqEvalResult.getVariationIndex() != prereq.getVariation()) { @@ -461,6 +484,12 @@ private boolean matchAnySegment(List values, LDContext context, Evaluat } Segment segment = getters.getSegment(segmentKey); if (segment != null) { + // The segment definition is read at this point, so an override segment marks the scope here. + // A match is not required. A negated clause turns a non-match into a match, so the definition + // shapes the result either way. + if (segment.isOverride()) { + state.overrideAffected = true; + } if (segmentMatchesContext(segment, context, state)) { return true; } diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/InputValidatingEvaluator.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/InputValidatingEvaluator.java index 8e7b61fd..013a7b5d 100644 --- a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/InputValidatingEvaluator.java +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/InputValidatingEvaluator.java @@ -135,7 +135,11 @@ EvalResultAndFlag evaluate(String flagKey, LDContext context, LDValue defaultVal value.getType() != requireType) { logger.error("Feature flag \"{}\"; evaluation expected result as {}, but got {}", flagKey, defaultValue.getType(), value.getType()); recorder.recordEvaluationError(featureFlag, context, defaultValue, ErrorKind.WRONG_TYPE); - return new EvalResultAndFlag(EvalResult.error(ErrorKind.WRONG_TYPE, defaultValue), featureFlag); + // The type mismatch replaces the reason. The evaluation read the same definitions, so the + // new reason keeps the override-affected marking. + return new EvalResultAndFlag( + EvalResult.error(ErrorKind.WRONG_TYPE, defaultValue).withOverrideAffected(result.isOverrideAffected()), + featureFlag); } } diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/DataModelTest.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/DataModelTest.java index 9cbbe9cd..eab1684c 100644 --- a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/DataModelTest.java +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/DataModelTest.java @@ -11,9 +11,20 @@ import com.launchdarkly.sdk.server.DataModel.SegmentRule; import com.launchdarkly.sdk.server.DataModel.Target; +import com.launchdarkly.sdk.LDValue; +import com.launchdarkly.sdk.server.subsystems.DataStoreTypes.ItemDescriptor; + import org.junit.Test; +import static com.launchdarkly.sdk.server.ModelBuilders.flagBuilder; +import static com.launchdarkly.sdk.server.ModelBuilders.prerequisite; +import static com.launchdarkly.sdk.server.ModelBuilders.segmentBuilder; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; @SuppressWarnings("javadoc") public class DataModelTest { @@ -108,4 +119,82 @@ private Segment segmentWithAllZeroValuedFields() { // and no preprocessing has happened. return new Segment(); } + + @Test + public void flagAndSegmentAreNotOverridesByDefault() { + assertFalse(flagBuilder("f").build().isOverride()); + assertFalse(segmentBuilder("s").build().isOverride()); + assertFalse(((FeatureFlag) DataModel.FEATURES.deserialize("{\"key\":\"f\",\"version\":1}").getItem()).isOverride()); + assertFalse(((Segment) DataModel.SEGMENTS.deserialize("{\"key\":\"s\",\"version\":1}").getItem()).isOverride()); + } + + @Test + public void markedFlagCopyCarriesMarkerAndSharesDataWithoutMutatingSource() { + FeatureFlag source = flagBuilder("f").version(7).on(true).variations(LDValue.of("a"), LDValue.of("b")) + .fallthroughVariation(1).offVariation(0).prerequisites(prerequisite("p", 1)).trackEvents(true) + .debugEventsUntilDate(1000L).build(); + + FeatureFlag marked = source.markedAsOverride(); + + assertTrue(marked.isOverride()); + assertFalse(source.isOverride()); + assertNotSame(source, marked); + assertEquals(source.getKey(), marked.getKey()); + assertEquals(source.getVersion(), marked.getVersion()); + assertEquals(source.isOn(), marked.isOn()); + assertSame(source.getVariations(), marked.getVariations()); + assertSame(source.getPrerequisites(), marked.getPrerequisites()); + assertSame(source.getFallthrough(), marked.getFallthrough()); + assertEquals(source.getOffVariation(), marked.getOffVariation()); + assertEquals(source.isTrackEvents(), marked.isTrackEvents()); + assertEquals(source.getDebugEventsUntilDate(), marked.getDebugEventsUntilDate()); + assertSame(source.preprocessed, marked.preprocessed); + } + + @Test + public void markedFlagCopyOfUnpreprocessedFlagHasNoPreprocessing() { + FeatureFlag source = flagBuilder("f").disablePreprocessing(true).build(); + FeatureFlag marked = source.markedAsOverride(); + assertNull(marked.preprocessed); + assertNull(source.preprocessed); + assertTrue(marked.isOverride()); + } + + @Test + public void markedSegmentCopyCarriesMarkerAndSharesDataWithoutMutatingSource() { + Segment source = segmentBuilder("s").version(3).included("u1").excluded("u2").unbounded(false).build(); + + Segment marked = source.markedAsOverride(); + + assertTrue(marked.isOverride()); + assertFalse(source.isOverride()); + assertNotSame(source, marked); + assertEquals(source.getKey(), marked.getKey()); + assertEquals(source.getVersion(), marked.getVersion()); + assertSame(source.getIncluded(), marked.getIncluded()); + assertSame(source.getExcluded(), marked.getExcluded()); + assertSame(source.getRules(), marked.getRules()); + } + + @Test + public void overrideMarkerIsNeverSerialized() { + FeatureFlag flag = flagBuilder("f").version(7).build().markedAsOverride(); + String flagJson = DataModel.FEATURES.serialize(new ItemDescriptor(flag.getVersion(), flag)); + assertFalse(flagJson.toLowerCase().contains("override")); + assertEquals(LDValue.of("f"), LDValue.parse(flagJson).get("key")); + + Segment segment = segmentBuilder("s").version(3).build().markedAsOverride(); + String segmentJson = DataModel.SEGMENTS.serialize(new ItemDescriptor(segment.getVersion(), segment)); + assertFalse(segmentJson.toLowerCase().contains("override")); + } + + @Test + public void overrideMarkerInJsonIsIgnoredWhenDeserializing() { + FeatureFlag flag = (FeatureFlag) DataModel.FEATURES.deserialize( + "{\"key\":\"f\",\"version\":1,\"isOverride\":true}").getItem(); + assertFalse(flag.isOverride()); + Segment segment = (Segment) DataModel.SEGMENTS.deserialize( + "{\"key\":\"s\",\"version\":1,\"isOverride\":true}").getItem(); + assertFalse(segment.isOverride()); + } } diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/EvalResultTest.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/EvalResultTest.java index 71ef5dec..59f1dfd0 100644 --- a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/EvalResultTest.java +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/EvalResultTest.java @@ -13,6 +13,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.sameInstance; @SuppressWarnings("javadoc") @@ -134,6 +135,52 @@ public void withForceReasonTracking() { assertThat(r1.getAnyType(), sameInstance(r.getAnyType())); } + @Test + public void overrideAffectedFollowsTheReason() { + EvalResult r = EvalResult.of(SOME_VALUE, SOME_VARIATION, SOME_REASON); + assertThat(r.isOverrideAffected(), is(false)); + assertThat(EvalResult.of(SOME_VALUE, SOME_VARIATION, SOME_REASON.withOverrideAffected(true)).isOverrideAffected(), + is(true)); + assertThat(EvalResult.error(EvaluationReason.ErrorKind.MALFORMED_FLAG).isOverrideAffected(), is(false)); + } + + @Test + public void withOverrideAffected() { + EvalResult r = EvalResult.of(SOME_VALUE, SOME_VARIATION, SOME_REASON); + + // Unchanged value keeps the same instance, so shared precomputed results stay shared. + assertThat(r.withOverrideAffected(false), sameInstance(r)); + + EvalResult marked = r.withOverrideAffected(true); + assertThat(marked, not(sameInstance(r))); + assertThat(marked.isOverrideAffected(), is(true)); + assertThat(marked.getReason(), equalTo(SOME_REASON.withOverrideAffected(true))); + assertThat(marked.getValue(), equalTo(r.getValue())); + assertThat(marked.getVariationIndex(), equalTo(r.getVariationIndex())); + assertThat(marked.withOverrideAffected(true), sameInstance(marked)); + assertThat(marked.withOverrideAffected(false), equalTo(r)); + + // Every typed view carries the marked reason. + assertThat(marked.getAsBoolean().getReason().isOverrideAffected(), is(true)); + assertThat(marked.getAsInteger().getReason().isOverrideAffected(), is(true)); + assertThat(marked.getAsDouble().getReason().isOverrideAffected(), is(true)); + assertThat(marked.getAsString().getReason().isOverrideAffected(), is(true)); + assertThat(marked.getAnyType().getReason().isOverrideAffected(), is(true)); + + // The original is untouched. + assertThat(r.isOverrideAffected(), is(false)); + } + + @Test + public void withOverrideAffectedKeepsPrerequisiteRecordsAndForceTracking() { + EvalResult r = EvalResult.of(SOME_VALUE, SOME_VARIATION, EvaluationReason.fallthrough(true)) + .withPrerequisiteEvalRecords(java.util.Collections.singletonList( + new PrerequisiteEvalRecord(null, null, EvalResult.of(SOME_VALUE, SOME_VARIATION, SOME_REASON)))); + EvalResult marked = r.withOverrideAffected(true); + assertThat(marked.isForceReasonTracking(), is(true)); + assertThat(marked.getPrerequisiteEvalRecords(), sameInstance(r.getPrerequisiteEvalRecords())); + } + private void testForType(T value, LDValue ldValue, Function getter) { assertThat( getter.apply(EvalResult.of(EvaluationDetail.fromValue(ldValue, SOME_VARIATION, SOME_REASON))), diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/EvaluatorOverrideMarkingTest.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/EvaluatorOverrideMarkingTest.java new file mode 100644 index 00000000..98affee2 --- /dev/null +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/EvaluatorOverrideMarkingTest.java @@ -0,0 +1,346 @@ +package com.launchdarkly.sdk.server; + +import com.launchdarkly.sdk.EvaluationReason; +import com.launchdarkly.sdk.EvaluationReason.ErrorKind; +import com.launchdarkly.sdk.LDContext; +import com.launchdarkly.sdk.LDValue; +import com.launchdarkly.sdk.server.DataModel.FeatureFlag; +import com.launchdarkly.sdk.server.DataModel.Segment; +import com.launchdarkly.sdk.server.EvaluatorTestUtil.EvaluatorBuilder; + +import org.junit.Test; + +import java.util.ArrayList; +import java.util.List; + +import static com.launchdarkly.sdk.server.EvaluatorTestUtil.BASE_USER; +import static com.launchdarkly.sdk.server.EvaluatorTestUtil.FALLTHROUGH_VALUE; +import static com.launchdarkly.sdk.server.EvaluatorTestUtil.FALLTHROUGH_VARIATION; +import static com.launchdarkly.sdk.server.EvaluatorTestUtil.GREEN_VARIATION; +import static com.launchdarkly.sdk.server.EvaluatorTestUtil.MATCH_VALUE; +import static com.launchdarkly.sdk.server.EvaluatorTestUtil.MATCH_VARIATION; +import static com.launchdarkly.sdk.server.EvaluatorTestUtil.OFF_VALUE; +import static com.launchdarkly.sdk.server.EvaluatorTestUtil.OFF_VARIATION; +import static com.launchdarkly.sdk.server.EvaluatorTestUtil.buildRedGreenFlag; +import static com.launchdarkly.sdk.server.EvaluatorTestUtil.buildThreeWayFlag; +import static com.launchdarkly.sdk.server.EvaluatorTestUtil.evaluatorBuilder; +import static com.launchdarkly.sdk.server.ModelBuilders.clause; +import static com.launchdarkly.sdk.server.ModelBuilders.clauseMatchingSegment; +import static com.launchdarkly.sdk.server.ModelBuilders.flagBuilder; +import static com.launchdarkly.sdk.server.ModelBuilders.negateClause; +import static com.launchdarkly.sdk.server.ModelBuilders.prerequisite; +import static com.launchdarkly.sdk.server.ModelBuilders.ruleBuilder; +import static com.launchdarkly.sdk.server.ModelBuilders.segmentBuilder; +import static com.launchdarkly.sdk.server.ModelBuilders.segmentRuleBuilder; +import static com.launchdarkly.sdk.server.ModelBuilders.target; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; + +/** + * An evaluation is override-affected when any definition it read carries the override marker: the + * evaluated flag, a prerequisite at any depth, or a segment consulted during matching. The marking + * propagates upward only. A prerequisite's own record reflects only its own subtree. + */ +@SuppressWarnings("javadoc") +public class EvaluatorOverrideMarkingTest { + private static final LDContext OTHER_USER = LDContext.create("other"); + + private static final class RecordingRecorder implements EvaluationRecorder { + final List prerequisites = new ArrayList<>(); + + @Override + public void recordPrerequisiteEvaluation(FeatureFlag flag, FeatureFlag prereqOfFlag, LDContext context, EvalResult result) { + prerequisites.add(new PrerequisiteEvalRecord(flag, prereqOfFlag, result)); + } + } + + private static PrerequisiteEvalRecord recordFor(List records, String flagKey) { + for (PrerequisiteEvalRecord r : records) { + if (r.flag.getKey().equals(flagKey)) { + return r; + } + } + throw new AssertionError("no prerequisite record for " + flagKey); + } + + @Test + public void evaluationOfUnmarkedFlagIsNotOverrideAffected() { + FeatureFlag f = buildThreeWayFlag("feature").on(true).build(); + EvalResult result = evaluatorBuilder().build().evaluate(f, BASE_USER, new EvaluationRecorder() {}); + + assertFalse(result.isOverrideAffected()); + assertFalse(result.getReason().isOverrideAffected()); + assertEquals(EvaluationReason.fallthrough(), result.getReason()); + } + + @Test + public void evaluationOfOverrideFlagIsMarkedForEveryReasonKind() { + Evaluator e = evaluatorBuilder().build(); + + FeatureFlag off = buildThreeWayFlag("off").on(false).build().markedAsOverride(); + EvalResult offResult = e.evaluate(off, BASE_USER, new EvaluationRecorder() {}); + assertEquals(EvalResult.of(OFF_VALUE, OFF_VARIATION, EvaluationReason.off().withOverrideAffected(true)), offResult); + assertTrue(offResult.isOverrideAffected()); + + FeatureFlag fallthrough = buildThreeWayFlag("fallthrough").on(true).build().markedAsOverride(); + assertEquals(EvaluationReason.fallthrough().withOverrideAffected(true), + e.evaluate(fallthrough, BASE_USER, new EvaluationRecorder() {}).getReason()); + + FeatureFlag targeted = buildThreeWayFlag("target").on(true) + .targets(target(MATCH_VARIATION, BASE_USER.getKey())).build().markedAsOverride(); + assertEquals(EvaluationReason.targetMatch().withOverrideAffected(true), + e.evaluate(targeted, BASE_USER, new EvaluationRecorder() {}).getReason()); + + FeatureFlag ruled = buildThreeWayFlag("rule").on(true) + .rules(ruleBuilder().id("r").variation(MATCH_VARIATION) + .clauses(clause("key", DataModel.Operator.in, LDValue.of(BASE_USER.getKey()))).build()) + .build().markedAsOverride(); + EvalResult ruleResult = e.evaluate(ruled, BASE_USER, new EvaluationRecorder() {}); + assertEquals(EvaluationReason.ruleMatch(0, "r").withOverrideAffected(true), ruleResult.getReason()); + assertEquals(MATCH_VALUE, ruleResult.getValue()); + } + + @Test + public void evaluationErrorOfOverrideFlagIsMarked() { + // The fallthrough points at a variation that does not exist, which is a malformed flag. + FeatureFlag malformed = flagBuilder("malformed").on(true).variations(LDValue.of("only")) + .fallthroughVariation(5).offVariation(0).build().markedAsOverride(); + EvalResult result = evaluatorBuilder().build().evaluate(malformed, BASE_USER, new EvaluationRecorder() {}); + + assertEquals(EvaluationReason.error(ErrorKind.MALFORMED_FLAG).withOverrideAffected(true), result.getReason()); + assertTrue(result.isNoVariation()); + assertTrue(result.isOverrideAffected()); + } + + @Test + public void thrownEvaluationErrorOfOverrideFlagIsMarked() { + // A clause without an attribute makes the evaluator throw, which becomes an error result. + FeatureFlag broken = buildThreeWayFlag("broken").on(true) + .rules(ruleBuilder().id("r").variation(MATCH_VARIATION) + .clauses(clause(null, DataModel.Operator.in, LDValue.of("x"))).build()) + .build().markedAsOverride(); + EvalResult result = evaluatorBuilder().build().evaluate(broken, BASE_USER, new EvaluationRecorder() {}); + + assertEquals(EvaluationReason.error(ErrorKind.MALFORMED_FLAG).withOverrideAffected(true), result.getReason()); + } + + @Test + public void overridePrerequisiteMarksParentAndItsOwnRecord() { + FeatureFlag parent = buildThreeWayFlag("parent").on(true) + .prerequisites(prerequisite("prereq", GREEN_VARIATION)).build(); + FeatureFlag prereq = buildRedGreenFlag("prereq").on(true).build().markedAsOverride(); + RecordingRecorder recorder = new RecordingRecorder(); + + EvalResult result = evaluatorBuilder().withStoredFlags(prereq).build().evaluate(parent, BASE_USER, recorder); + + assertEquals(FALLTHROUGH_VALUE, result.getValue()); + assertEquals(EvaluationReason.fallthrough().withOverrideAffected(true), result.getReason()); + assertTrue(result.isOverrideAffected()); + + PrerequisiteEvalRecord record = recordFor(recorder.prerequisites, "prereq"); + assertTrue(record.result.isOverrideAffected()); + assertEquals(EvaluationReason.fallthrough().withOverrideAffected(true), record.result.getReason()); + // The same record is on the result. + assertTrue(recordFor(result.getPrerequisiteEvalRecords(), "prereq").result.isOverrideAffected()); + } + + @Test + public void failedOverridePrerequisiteStillMarksParent() { + FeatureFlag parent = buildThreeWayFlag("parent").on(true) + .prerequisites(prerequisite("prereq", GREEN_VARIATION)).build(); + FeatureFlag prereq = buildRedGreenFlag("prereq").on(false).build().markedAsOverride(); + + EvalResult result = evaluatorBuilder().withStoredFlags(prereq).build() + .evaluate(parent, BASE_USER, new EvaluationRecorder() {}); + + assertEquals(OFF_VALUE, result.getValue()); + assertEquals(EvaluationReason.prerequisiteFailed("prereq").withOverrideAffected(true), result.getReason()); + } + + @Test + public void unaffectedPrerequisiteRecordIsNotMarkedInsideMarkedEvaluation() { + FeatureFlag parent = buildThreeWayFlag("parent").on(true) + .prerequisites(prerequisite("overridden", GREEN_VARIATION), prerequisite("plain", GREEN_VARIATION)).build(); + FeatureFlag overridden = buildRedGreenFlag("overridden").on(true).build().markedAsOverride(); + FeatureFlag plain = buildRedGreenFlag("plain").on(true).build(); + RecordingRecorder recorder = new RecordingRecorder(); + + EvalResult result = evaluatorBuilder().withStoredFlags(overridden, plain).build().evaluate(parent, BASE_USER, recorder); + + assertTrue(result.isOverrideAffected()); + assertTrue(recordFor(recorder.prerequisites, "overridden").result.isOverrideAffected()); + assertFalse(recordFor(recorder.prerequisites, "plain").result.isOverrideAffected()); + assertEquals(EvaluationReason.fallthrough(), recordFor(recorder.prerequisites, "plain").result.getReason()); + } + + @Test + public void markingPropagatesUpwardThroughEveryDepthButNotSideways() { + // A depends on B and C. B depends on D, which is the only override. A, B, and D are marked. + // C is not. + FeatureFlag a = buildThreeWayFlag("a").on(true) + .prerequisites(prerequisite("b", GREEN_VARIATION), prerequisite("c", GREEN_VARIATION)).build(); + FeatureFlag b = buildRedGreenFlag("b").on(true).prerequisites(prerequisite("d", GREEN_VARIATION)).build(); + FeatureFlag c = buildRedGreenFlag("c").on(true).build(); + FeatureFlag d = buildRedGreenFlag("d").on(true).build().markedAsOverride(); + RecordingRecorder recorder = new RecordingRecorder(); + + EvalResult result = evaluatorBuilder().withStoredFlags(b, c, d).build().evaluate(a, BASE_USER, recorder); + + assertTrue(result.isOverrideAffected()); + assertTrue(recordFor(recorder.prerequisites, "b").result.isOverrideAffected()); + assertTrue(recordFor(recorder.prerequisites, "d").result.isOverrideAffected()); + assertFalse(recordFor(recorder.prerequisites, "c").result.isOverrideAffected()); + } + + @Test + public void overrideFlagWithUnmarkedPrerequisiteMarksOnlyItself() { + FeatureFlag parent = buildThreeWayFlag("parent").on(true) + .prerequisites(prerequisite("prereq", GREEN_VARIATION)).build().markedAsOverride(); + FeatureFlag prereq = buildRedGreenFlag("prereq").on(true).build(); + RecordingRecorder recorder = new RecordingRecorder(); + + EvalResult result = evaluatorBuilder().withStoredFlags(prereq).build().evaluate(parent, BASE_USER, recorder); + + assertTrue(result.isOverrideAffected()); + // The parent's marking does not flow down into the prerequisite's record. + assertFalse(recordFor(recorder.prerequisites, "prereq").result.isOverrideAffected()); + } + + @Test + public void circularReferenceThroughOverridePrerequisiteIsMarkedError() { + FeatureFlag a = buildThreeWayFlag("a").on(true).prerequisites(prerequisite("b", GREEN_VARIATION)).build(); + FeatureFlag b = buildRedGreenFlag("b").on(true).prerequisites(prerequisite("a", GREEN_VARIATION)).build() + .markedAsOverride(); + + EvalResult result = evaluatorBuilder().withStoredFlags(b).build().evaluate(a, BASE_USER, new EvaluationRecorder() {}); + + assertEquals(EvaluationReason.error(ErrorKind.MALFORMED_FLAG).withOverrideAffected(true), result.getReason()); + } + + @Test + public void matchingOverrideSegmentMarksEvaluation() { + Segment segment = segmentBuilder("seg").included(BASE_USER.getKey()).build().markedAsOverride(); + FeatureFlag f = buildThreeWayFlag("feature").on(true) + .rules(ruleBuilder().id("r").variation(MATCH_VARIATION).clauses(clauseMatchingSegment("seg")).build()) + .build(); + + EvalResult result = evaluatorBuilder().withStoredSegments(segment).build() + .evaluate(f, BASE_USER, new EvaluationRecorder() {}); + + assertEquals(MATCH_VALUE, result.getValue()); + assertEquals(EvaluationReason.ruleMatch(0, "r").withOverrideAffected(true), result.getReason()); + } + + @Test + public void nonMatchingOverrideSegmentStillMarksEvaluation() { + Segment segment = segmentBuilder("seg").included(BASE_USER.getKey()).build().markedAsOverride(); + FeatureFlag f = buildThreeWayFlag("feature").on(true) + .rules(ruleBuilder().id("r").variation(MATCH_VARIATION).clauses(clauseMatchingSegment("seg")).build()) + .build(); + + EvalResult result = evaluatorBuilder().withStoredSegments(segment).build() + .evaluate(f, OTHER_USER, new EvaluationRecorder() {}); + + assertEquals(FALLTHROUGH_VALUE, result.getValue()); + assertEquals(EvaluationReason.fallthrough().withOverrideAffected(true), result.getReason()); + } + + @Test + public void negatedClauseOnOverrideSegmentMarksEvaluation() { + Segment segment = segmentBuilder("seg").included(BASE_USER.getKey()).build().markedAsOverride(); + FeatureFlag f = buildThreeWayFlag("feature").on(true) + .rules(ruleBuilder().id("r").variation(MATCH_VARIATION) + .clauses(negateClause(clauseMatchingSegment("seg"))).build()) + .build(); + + EvalResult result = evaluatorBuilder().withStoredSegments(segment).build() + .evaluate(f, OTHER_USER, new EvaluationRecorder() {}); + + assertEquals(MATCH_VALUE, result.getValue()); + assertTrue(result.isOverrideAffected()); + } + + @Test + public void unmarkedSegmentDoesNotMarkEvaluation() { + Segment segment = segmentBuilder("seg").included(BASE_USER.getKey()).build(); + FeatureFlag f = buildThreeWayFlag("feature").on(true) + .rules(ruleBuilder().id("r").variation(MATCH_VARIATION).clauses(clauseMatchingSegment("seg")).build()) + .build(); + + EvalResult result = evaluatorBuilder().withStoredSegments(segment).build() + .evaluate(f, BASE_USER, new EvaluationRecorder() {}); + + assertEquals(MATCH_VALUE, result.getValue()); + assertFalse(result.isOverrideAffected()); + } + + @Test + public void overrideSegmentReferencedByAnotherSegmentMarksEvaluation() { + Segment inner = segmentBuilder("inner").included(BASE_USER.getKey()).build().markedAsOverride(); + Segment outer = segmentBuilder("outer") + .rules(segmentRuleBuilder().clauses(clauseMatchingSegment("inner")).build()).build(); + FeatureFlag f = buildThreeWayFlag("feature").on(true) + .rules(ruleBuilder().id("r").variation(MATCH_VARIATION).clauses(clauseMatchingSegment("outer")).build()) + .build(); + + EvalResult result = evaluatorBuilder().withStoredSegments(inner, outer).build() + .evaluate(f, BASE_USER, new EvaluationRecorder() {}); + + assertEquals(MATCH_VALUE, result.getValue()); + assertTrue(result.isOverrideAffected()); + } + + @Test + public void segmentReadByPrerequisiteMarksPrerequisiteAndParent() { + Segment segment = segmentBuilder("seg").included(BASE_USER.getKey()).build().markedAsOverride(); + FeatureFlag parent = buildThreeWayFlag("parent").on(true) + .prerequisites(prerequisite("prereq", GREEN_VARIATION)).build(); + FeatureFlag prereq = buildRedGreenFlag("prereq").on(true) + .rules(ruleBuilder().id("r").variation(GREEN_VARIATION).clauses(clauseMatchingSegment("seg")).build()) + .build(); + RecordingRecorder recorder = new RecordingRecorder(); + + EvalResult result = evaluatorBuilder().withStoredFlags(prereq).withStoredSegments(segment).build() + .evaluate(parent, BASE_USER, recorder); + + assertTrue(result.isOverrideAffected()); + assertTrue(recordFor(recorder.prerequisites, "prereq").result.isOverrideAffected()); + } + + @Test + public void markedEvaluationDoesNotAlterSharedPrecomputedResults() { + FeatureFlag source = buildThreeWayFlag("feature").on(false).build(); + FeatureFlag marked = source.markedAsOverride(); + Evaluator e = evaluatorBuilder().build(); + + EvalResult markedResult = e.evaluate(marked, BASE_USER, new EvaluationRecorder() {}); + EvalResult sourceResult = e.evaluate(source, BASE_USER, new EvaluationRecorder() {}); + + assertTrue(markedResult.isOverrideAffected()); + assertFalse(sourceResult.isOverrideAffected()); + assertFalse(source.isOverride()); + assertNotSame(markedResult, sourceResult); + // The unmarked evaluation still returns the shared precomputed instance. + assertSame(sourceResult, e.evaluate(source, BASE_USER, new EvaluationRecorder() {})); + } + + @Test + public void bigSegmentsStatusAndOverrideMarkingAreBothKept() { + Segment bigSegment = segmentBuilder("big").unbounded(true).generation(1).build().markedAsOverride(); + FeatureFlag f = buildThreeWayFlag("feature").on(true) + .rules(ruleBuilder().id("r").variation(MATCH_VARIATION).clauses(clauseMatchingSegment("big")).build()) + .build(); + // No big segment store is configured, so the status is NOT_CONFIGURED. + EvaluatorBuilder builder = evaluatorBuilder().withStoredSegments(bigSegment) + .withBigSegmentQueryResult(BASE_USER.getKey(), null); + + EvalResult result = builder.build().evaluate(f, BASE_USER, new EvaluationRecorder() {}); + + assertEquals(FALLTHROUGH_VARIATION, result.getVariationIndex()); + assertEquals(EvaluationReason.BigSegmentsStatus.NOT_CONFIGURED, result.getReason().getBigSegmentsStatus()); + assertTrue(result.getReason().isOverrideAffected()); + } +} diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/InputValidatingEvaluatorOverrideTest.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/InputValidatingEvaluatorOverrideTest.java new file mode 100644 index 00000000..cf92d61c --- /dev/null +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/InputValidatingEvaluatorOverrideTest.java @@ -0,0 +1,103 @@ +package com.launchdarkly.sdk.server; + +import com.launchdarkly.sdk.EvaluationReason; +import com.launchdarkly.sdk.EvaluationReason.ErrorKind; +import com.launchdarkly.sdk.LDValue; +import com.launchdarkly.sdk.LDValueType; +import com.launchdarkly.sdk.server.DataModel.FeatureFlag; +import com.launchdarkly.sdk.server.subsystems.DataStoreTypes.DataKind; +import com.launchdarkly.sdk.server.subsystems.DataStoreTypes.ItemDescriptor; +import com.launchdarkly.sdk.server.subsystems.DataStoreTypes.KeyedItems; + +import org.junit.Test; + +import java.util.AbstractMap; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +import static com.launchdarkly.sdk.server.DataModel.FEATURES; +import static com.launchdarkly.sdk.server.EvaluatorTestUtil.BASE_USER; +import static com.launchdarkly.sdk.server.ModelBuilders.flagBuilder; +import static com.launchdarkly.sdk.server.TestComponents.nullLogger; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +@SuppressWarnings("javadoc") +public class InputValidatingEvaluatorOverrideTest { + private static ReadOnlyStore storeWith(FeatureFlag... flags) { + Map items = new HashMap<>(); + for (FeatureFlag f : flags) { + items.put(f.getKey(), new ItemDescriptor(f.getVersion(), f)); + } + return new ReadOnlyStore() { + @Override + public ItemDescriptor get(DataKind kind, String key) { + return kind == FEATURES ? items.get(key) : null; + } + + @Override + public KeyedItems getAll(DataKind kind) { + return kind == FEATURES ? new KeyedItems<>(items.entrySet()) : new KeyedItems<>(Collections.emptyList()); + } + + @Override + public boolean isInitialized() { + return true; + } + }; + } + + private static InputValidatingEvaluator evaluatorOver(ReadOnlyStore store) { + return new InputValidatingEvaluator(store, null, new NoOpEventProcessor(), nullLogger); + } + + @Test + public void typeMismatchOnOverrideFlagKeepsMarking() { + FeatureFlag flag = flagBuilder("flag").on(false).offVariation(0).variations(LDValue.of("a string")) + .build().markedAsOverride(); + EvalResultAndFlag result = evaluatorOver(storeWith(flag)).evaluate("flag", BASE_USER, LDValue.of(true), + LDValueType.BOOLEAN, InputValidatingEvaluator.NO_OP_EVALUATION_EVENT_RECORDER); + + assertEquals(LDValue.of(true), result.getResult().getValue()); + assertEquals(EvaluationReason.error(ErrorKind.WRONG_TYPE).withOverrideAffected(true), result.getResult().getReason()); + assertTrue(result.getResult().isOverrideAffected()); + } + + @Test + public void typeMismatchOnOrdinaryFlagIsNotMarked() { + FeatureFlag flag = flagBuilder("flag").on(false).offVariation(0).variations(LDValue.of("a string")).build(); + EvalResultAndFlag result = evaluatorOver(storeWith(flag)).evaluate("flag", BASE_USER, LDValue.of(true), + LDValueType.BOOLEAN, InputValidatingEvaluator.NO_OP_EVALUATION_EVENT_RECORDER); + + assertEquals(EvaluationReason.error(ErrorKind.WRONG_TYPE), result.getResult().getReason()); + assertFalse(result.getResult().isOverrideAffected()); + } + + @Test + public void errorResultOfOverrideFlagKeepsMarkingWithCallerDefault() { + FeatureFlag malformed = flagBuilder("flag").on(true).variations(LDValue.of("only")) + .fallthroughVariation(5).offVariation(0).build().markedAsOverride(); + EvalResultAndFlag result = evaluatorOver(storeWith(malformed)).evaluate("flag", BASE_USER, LDValue.of("fallback"), + null, InputValidatingEvaluator.NO_OP_EVALUATION_EVENT_RECORDER); + + assertEquals(LDValue.of("fallback"), result.getResult().getValue()); + assertTrue(result.getResult().isNoVariation()); + assertEquals(EvaluationReason.error(ErrorKind.MALFORMED_FLAG).withOverrideAffected(true), result.getResult().getReason()); + } + + @Test + public void allFlagsStateKeepsMarkedReason() { + FeatureFlag marked = flagBuilder("marked").on(false).offVariation(0).variations(LDValue.of("x")).build() + .markedAsOverride(); + FeatureFlag plain = flagBuilder("plain").on(false).offVariation(0).variations(LDValue.of("y")).build(); + FeatureFlagsState state = evaluatorOver(storeWith(marked, plain)).allFlagsState(BASE_USER, FlagsStateOption.WITH_REASONS); + + assertTrue(state.isValid()); + assertTrue(state.getFlagReason("marked").isOverrideAffected()); + assertFalse(state.getFlagReason("plain").isOverrideAffected()); + Map.Entry expected = new AbstractMap.SimpleEntry<>("marked", LDValue.of("x")); + assertEquals(expected.getValue(), state.getFlagValue("marked")); + } +}