From d82beda535ab45961cb285333042d8b04a6f2cfc Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 25 Sep 2026 15:17:28 -0700 Subject: [PATCH] feat: Add the override marker to the flag and segment models and mark evaluations The OVERRIDE specification requires an evaluation to be marked as override-affected when any definition it read carried the override marker: the evaluated flag, a prerequisite at any depth, or a segment consulted during matching, whether or not the segment matched. The marking propagates upward only. A prerequisite's own record reflects only the definitions that its own subtree read. An evaluation that fails is still marked when it read a marked definition. DataModel.FeatureFlag and DataModel.Segment carry a transient isOverride marker that is never serialized and never read from JSON, with a markedAsOverride() shallow copy that the override layer will use without modifying the source's entity. The evaluator tracks the marking per evaluation scope: the scope starts from its own flag's marker, each segment read can set it, and the value is saved, reset to the prerequisite's marker, and restored around each prerequisite evaluation. The result and every prerequisite record carry the marking on their reason. EvalResult exposes isOverrideAffected() and withOverrideAffected(), which copies rather than mutating the shared precomputed results. A requested-type mismatch keeps the marking on its error reason. The server SDK now depends on launchdarkly-java-sdk-common 2.6.0, which adds the overrideAffected indicator to EvaluationReason. CI for this change cannot pass until that version is released. Flag overrides are currently experimental and subject to change. --- lib/sdk/server/build.gradle | 2 +- .../launchdarkly/sdk/server/DataModel.java | 60 +++ .../launchdarkly/sdk/server/EvalResult.java | 19 + .../launchdarkly/sdk/server/Evaluator.java | 35 +- .../sdk/server/InputValidatingEvaluator.java | 6 +- .../sdk/server/DataModelTest.java | 89 +++++ .../sdk/server/EvalResultTest.java | 47 +++ .../server/EvaluatorOverrideMarkingTest.java | 346 ++++++++++++++++++ .../InputValidatingEvaluatorOverrideTest.java | 103 ++++++ 9 files changed, 702 insertions(+), 5 deletions(-) create mode 100644 lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/EvaluatorOverrideMarkingTest.java create mode 100644 lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/InputValidatingEvaluatorOverrideTest.java 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")); + } +}