From 20eea70c061d9b2426fb108a8f4ac980d9bfdb81 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 25 Sep 2026 15:07:58 -0700 Subject: [PATCH] feat: Add the override marker to the model, evaluator, and reason Flag overrides, as defined by the OVERRIDE specification, need three pieces below the store: a marker on the flag and segment model that says a definition came from the override store, evaluator marking that follows every definition an evaluation reads, and an indicator on the evaluation reason that reports the marking to callers. The model classes carry the marker as an attribute that is never serialized. `as_override` returns a marked shallow copy and leaves the original unchanged. Only the SDK components that manage override entries set it. The evaluator marks an evaluation as override-affected when the evaluated flag, a prerequisite flag at any depth, or a segment read during clause matching carries the marker. A segment counts when it is read, so a segment that does not match, or that is read through a negated clause, still marks the evaluation. The marking propagates upward only: a prerequisite's own record reflects the definitions its own subtree read, and a plain prerequisite inside a marked evaluation is not marked. An error result is marked when a marked definition was read before the failure, including when the failure was raised while a prerequisite was being evaluated. An evaluation that reads no marked definition keeps the shared precomputed detail instances. `EvaluationReason#override_affected` carries the indicator. It is written to JSON as `overrideAffected` only when true, so the wire format of ordinary evaluations is unchanged. `with_override_affected` follows the `with_big_segments_status` precedent and returns the same instance when nothing changes. Flag overrides are currently experimental and subject to change. --- lib/ldclient-rb/evaluation_detail.rb | 41 ++- lib/ldclient-rb/impl/evaluator.rb | 80 ++++-- lib/ldclient-rb/impl/model/feature_flag.rb | 34 +++ lib/ldclient-rb/impl/model/segment.rb | 34 +++ spec/evaluation_detail_spec.rb | 75 ++++++ spec/impl/evaluator_override_spec.rb | 298 +++++++++++++++++++++ spec/impl/evaluator_prereq_spec.rb | 10 +- spec/impl/model/override_marker_spec.rb | 70 +++++ 8 files changed, 615 insertions(+), 27 deletions(-) create mode 100644 spec/impl/evaluator_override_spec.rb create mode 100644 spec/impl/model/override_marker_spec.rb diff --git a/lib/ldclient-rb/evaluation_detail.rb b/lib/ldclient-rb/evaluation_detail.rb index d30ebb32..30098ac7 100644 --- a/lib/ldclient-rb/evaluation_detail.rb +++ b/lib/ldclient-rb/evaluation_detail.rb @@ -150,6 +150,15 @@ class EvaluationReason # @return [Symbol] attr_reader :big_segments_status + # True if an override affected this evaluation, directly or transitively. It is true when the + # evaluated flag came from the SDK's override store. It is also true when a prerequisite flag at + # any depth, or a segment read during the evaluation, came from the override store. In the JSON + # representation, the `overrideAffected` property appears only when this is true. + # + # Flag overrides are currently experimental and subject to change. + # @return [Boolean] + attr_reader :override_affected + # Returns an instance whose {#kind} is {#OFF}. # @return [EvaluationReason] def self.off @@ -216,12 +225,14 @@ def ==(other) if other.is_a? EvaluationReason @kind == other.kind && @rule_index == other.rule_index && @rule_id == other.rule_id && @prerequisite_key == other.prerequisite_key && @error_kind == other.error_kind && - @big_segments_status == other.big_segments_status + @big_segments_status == other.big_segments_status && + @override_affected == other.override_affected elsif other.is_a? Hash @kind.to_s == other[:kind] && @rule_index == other[:ruleIndex] && @rule_id == other[:ruleId] && @prerequisite_key == other[:prerequisiteKey] && (other[:errorKind] == @error_kind.nil? ? nil : @error_kind.to_s) && - (other[:bigSegmentsStatus] == @big_segments_status.nil? ? nil : @big_segments_status.to_s) + (other[:bigSegmentsStatus] == @big_segments_status.nil? ? nil : @big_segments_status.to_s) && + !!other[:overrideAffected] == @override_affected end end @@ -286,6 +297,8 @@ def as_json(*) # parameter is unused, but may be passed if we're using the json unless @big_segments_status.nil? ret[:bigSegmentsStatus] = @big_segments_status end + # The property is written only when true, so the wire format of an ordinary evaluation is unchanged. + ret[:overrideAffected] = true if @override_affected ret end @@ -312,6 +325,8 @@ def [](key) @error_kind.nil? ? nil : @error_kind.to_s when :bigSegmentsStatus @big_segments_status.nil? ? nil : @big_segments_status.to_s + when :overrideAffected + @override_affected else nil end @@ -319,7 +334,24 @@ def [](key) def with_big_segments_status(big_segments_status) return self if @big_segments_status == big_segments_status - EvaluationReason.new(@kind, @rule_index, @rule_id, @prerequisite_key, @error_kind, @in_experiment, big_segments_status) + EvaluationReason.new(@kind, @rule_index, @rule_id, @prerequisite_key, @error_kind, @in_experiment, + big_segments_status, @override_affected) + end + + # + # Returns a reason that is the same as this one apart from the {#override_affected} indicator. + # Returns this instance when the indicator already has the given value. + # + # Flag overrides are currently experimental and subject to change. + # + # @param override_affected [Boolean] + # @return [EvaluationReason] + # + def with_override_affected(override_affected) + override_affected = !!override_affected + return self if @override_affected == override_affected + EvaluationReason.new(@kind, @rule_index, @rule_id, @prerequisite_key, @error_kind, @in_experiment, + @big_segments_status, override_affected) end # @@ -327,7 +359,7 @@ def with_big_segments_status(big_segments_status) # but should use class methods like {#off} to avoid creating unnecessary instances. # def initialize(kind, rule_index, rule_id, prerequisite_key, error_kind, in_experiment=nil, - big_segments_status = nil) + big_segments_status = nil, override_affected = false) @kind = kind.to_sym @rule_index = rule_index @rule_id = rule_id @@ -337,6 +369,7 @@ def initialize(kind, rule_index, rule_id, prerequisite_key, error_kind, in_exper @error_kind = error_kind @in_experiment = in_experiment @big_segments_status = big_segments_status + @override_affected = !!override_affected end private_class_method def self.make_error(error_kind) diff --git a/lib/ldclient-rb/impl/evaluator.rb b/lib/ldclient-rb/impl/evaluator.rb index 932d9102..6739ab69 100644 --- a/lib/ldclient-rb/impl/evaluator.rb +++ b/lib/ldclient-rb/impl/evaluator.rb @@ -12,7 +12,8 @@ module Impl PrerequisiteEvalRecord = Struct.new( :prereq_flag, # the prerequisite flag that we evaluated :prereq_of_flag, # the flag that it was a prerequisite of - :detail # the EvaluationDetail representing the evaluation result + :detail, # the EvaluationDetail representing the evaluation result + :override_affected # true if a definition read by the prerequisite's own evaluation came from the override store ) class EvaluationException < StandardError @@ -35,6 +36,9 @@ def initialize(original_flag) @segment_stack = EvaluatorStack.new(nil) @prerequisites = [] @depth = 0 + # Reading the flag's own definition is the first read of the evaluation, so the marking + # starts from the flag's override marker. + @override_affected = original_flag.override? end def record_evaluated_prereq_key(key) @@ -45,6 +49,10 @@ def record_evaluated_prereq_key(key) attr_reader :prerequisites attr_reader :prereq_stack attr_reader :segment_stack + # True if the evaluation in progress has read a definition that carries the override marker. + # While a prerequisite is evaluated, this holds the marking of the prerequisite's own subtree. + # The marking propagates upward only. + attr_accessor :override_affected end # @@ -130,7 +138,8 @@ def initialize(get_flag, get_segment, get_big_segments_membership, logger) :detail, # the EvaluationDetail representing the evaluation result :prereq_evals, # an array of PrerequisiteEvalRecord instances, or nil :big_segments_status, - :big_segments_membership + :big_segments_membership, + :override_affected # true if any definition read during the evaluation came from the override store ) # Helper function used internally to construct an EvaluationDetail for an error result. @@ -152,23 +161,25 @@ def evaluate(flag, context) result = EvalResult.new begin detail = eval_internal(flag, context, result, state) + + unless result.big_segments_status.nil? + # If big_segments_status is non-nil at the end of the evaluation, it means a query was done at + # some point and we will want to include the status in the evaluation reason. + detail = EvaluationDetail.new(detail.value, detail.variation_index, + detail.reason.with_big_segments_status(result.big_segments_status)) + end rescue EvaluationException => exn Impl::Util.log_exception(@logger, "Unexpected error when evaluating flag #{flag.key}", exn) - result.detail = EvaluationDetail.new(nil, nil, EvaluationReason::error(exn.error_kind)) - return result, state + detail = EvaluationDetail.new(nil, nil, EvaluationReason::error(exn.error_kind)) rescue => exn Impl::Util.log_exception(@logger, "Unexpected error when evaluating flag #{flag.key}", exn) - result.detail = EvaluationDetail.new(nil, nil, EvaluationReason::error(EvaluationReason::ERROR_EXCEPTION)) - return result, state + detail = EvaluationDetail.new(nil, nil, EvaluationReason::error(EvaluationReason::ERROR_EXCEPTION)) end - unless result.big_segments_status.nil? - # If big_segments_status is non-nil at the end of the evaluation, it means a query was done at - # some point and we will want to include the status in the evaluation reason. - detail = EvaluationDetail.new(detail.value, detail.variation_index, - detail.reason.with_big_segments_status(result.big_segments_status)) - end - result.detail = detail + # Error results are marked too. A malformed override definition yields an error reason, and an + # override still affected that result. + result.override_affected = state.override_affected + result.detail = mark_override_affected(detail, state.override_affected) [result, state] end @@ -240,15 +251,27 @@ def self.make_big_segment_ref(segment) # method is visible for testing @logger.error { "[LDClient] Could not retrieve prerequisite flag \"#{prereq_key}\" when evaluating \"#{flag.key}\"" } prereq_ok = false else - state.depth += 1 - prereq_res = eval_internal(prereq_flag, context, eval_result, state) - state.depth -= 1 + # The prerequisite's own record reflects only the definitions that its own subtree read. + # Its marking starts from its own definition. When it is done, the marking propagates + # upward into this flag's marking, also when the evaluation ends with an error. + parent_affected = state.override_affected + state.override_affected = prereq_flag.override? + prereq_affected = state.override_affected + begin + state.depth += 1 + prereq_res = eval_internal(prereq_flag, context, eval_result, state) + ensure + state.depth -= 1 + prereq_affected = state.override_affected + state.override_affected = parent_affected || prereq_affected + end # 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 !prereq_flag.on || prereq_res.variation_index != prerequisite.variation prereq_ok = false end - prereq_eval = PrerequisiteEvalRecord.new(prereq_flag, flag, prereq_res) + prereq_res = mark_override_affected(prereq_res, prereq_affected) + prereq_eval = PrerequisiteEvalRecord.new(prereq_flag, flag, prereq_res, prereq_affected) eval_result.prereq_evals = [] if eval_result.prereq_evals.nil? eval_result.prereq_evals.push(prereq_eval) end @@ -294,7 +317,15 @@ def self.make_big_segment_ref(segment) # method is visible for testing end segment = @get_segment.call(v) - !segment.nil? && segment_match_context(segment, context, eval_result, state) + if segment.nil? + false + else + # The segment definition was read, so an override segment marks the evaluation here. A + # match is not required: a negated clause turns a non-match into a match, so the + # definition shapes the result either way. + state.override_affected = true if segment.override? + segment_match_context(segment, context, eval_result, state) + end } clause.negate ? !result : result else @@ -482,6 +513,19 @@ def self.make_big_segment_ref(segment) # method is visible for testing bucket.nil? || bucket < weight end + # Returns the detail with its reason marked as override-affected when the evaluation read a + # definition from the override store. Returns the same detail otherwise, so that the precomputed + # detail instances stay shared. + # + # @param detail [LaunchDarkly::EvaluationDetail] + # @param override_affected [Boolean] + # @return [LaunchDarkly::EvaluationDetail] + private def mark_override_affected(detail, override_affected) + return detail unless override_affected + + EvaluationDetail.new(detail.value, detail.variation_index, detail.reason.with_override_affected(true)) + end + private def get_value_for_variation_or_rollout(flag, vr, context, precomputed_results) index, in_experiment = EvaluatorBucketing.variation_index_for_context(flag, vr, context) diff --git a/lib/ldclient-rb/impl/model/feature_flag.rb b/lib/ldclient-rb/impl/model/feature_flag.rb index b53b13e3..b394b759 100644 --- a/lib/ldclient-rb/impl/model/feature_flag.rb +++ b/lib/ldclient-rb/impl/model/feature_flag.rb @@ -25,6 +25,7 @@ def initialize(data, logger = nil) @key = data[:key] @version = data[:version] @deleted = !!data[:deleted] + @override = false return if @deleted migration_settings = data[:migration] || {} @migration_settings = MigrationSettings.new(migration_settings[:checkRatio]) @@ -96,6 +97,35 @@ def initialize(data, logger = nil) # @return [String] attr_reader :salt + # + # True if this definition came from the SDK's override store rather than from LaunchDarkly + # data. The marker is not part of the flag data and is never serialized. Only the SDK + # components that manage override entries set it, through {#as_override}. Evaluation reads + # it to mark the evaluations it affects. Every other reader treats a marked definition the + # same as any other. + # + # Flag overrides are currently experimental and subject to change. + # + # @return [Boolean] + # + def override? + @override + end + + # + # Returns a shallow copy of this flag that carries the override marker. The copy shares its + # data with this flag. Nothing writes to that data. This flag is not changed. + # + # Flag overrides are currently experimental and subject to change. + # + # @return [FeatureFlag] + # + def as_override + copy = dup + copy.override = true + copy + end + # This method allows us to read properties of the object as if it's just a hash. Currently this is # necessary because some data store logic is still written to expect hashes; we can remove it once # we migrate entirely to using attributes of the class. @@ -115,6 +145,10 @@ def as_json(*) # parameter is unused, but may be passed if we're using the json def to_json(*a) as_json.to_json(*a) end + + protected + + attr_writer :override end class Prerequisite diff --git a/lib/ldclient-rb/impl/model/segment.rb b/lib/ldclient-rb/impl/model/segment.rb index 83c1dc90..ff7fd9d7 100644 --- a/lib/ldclient-rb/impl/model/segment.rb +++ b/lib/ldclient-rb/impl/model/segment.rb @@ -17,6 +17,7 @@ def initialize(data, logger = nil) @key = data[:key] @version = data[:version] @deleted = !!data[:deleted] + @override = false return if @deleted @included = data[:included] || [] @excluded = data[:excluded] || [] @@ -67,6 +68,35 @@ def initialize(data, logger = nil) # @return [String] attr_reader :salt + # + # True if this definition came from the SDK's override store rather than from LaunchDarkly + # data. The marker is not part of the segment data and is never serialized. Only the SDK + # components that manage override entries set it, through {#as_override}. Evaluation reads + # it to mark the evaluations it affects. Every other reader treats a marked definition the + # same as any other. + # + # Flag overrides are currently experimental and subject to change. + # + # @return [Boolean] + # + def override? + @override + end + + # + # Returns a shallow copy of this segment that carries the override marker. The copy shares + # its data with this segment. Nothing writes to that data. This segment is not changed. + # + # Flag overrides are currently experimental and subject to change. + # + # @return [Segment] + # + def as_override + copy = dup + copy.override = true + copy + end + # This method allows us to read properties of the object as if it's just a hash. Currently this is # necessary because some data store logic is still written to expect hashes; we can remove it once # we migrate entirely to using attributes of the class. @@ -86,6 +116,10 @@ def as_json(*) # parameter is unused, but may be passed if we're using the json def to_json(*a) as_json.to_json(*a) end + + protected + + attr_writer :override end class SegmentTarget diff --git a/spec/evaluation_detail_spec.rb b/spec/evaluation_detail_spec.rb index df880447..522c43d3 100644 --- a/spec/evaluation_detail_spec.rb +++ b/spec/evaluation_detail_spec.rb @@ -45,6 +45,17 @@ module LaunchDarkly [ EvaluationReason::fallthrough().with_big_segments_status(BigSegmentsStatus::HEALTHY), EvaluationReason::FALLTHROUGH, { "kind" => "FALLTHROUGH", "bigSegmentsStatus" => "HEALTHY" }, "FALLTHROUGH", [ EvaluationReason::fallthrough ] ], + [ EvaluationReason::off.with_override_affected(true), EvaluationReason::OFF, + { "kind" => "OFF", "overrideAffected" => true }, "OFF", + [ EvaluationReason::off ] ], + [ EvaluationReason::rule_match(1, "x").with_big_segments_status(BigSegmentsStatus::STALE).with_override_affected(true), + EvaluationReason::RULE_MATCH, + { "kind" => "RULE_MATCH", "ruleIndex" => 1, "ruleId" => "x", "bigSegmentsStatus" => "STALE", "overrideAffected" => true }, + "RULE_MATCH(1,x)", + [ EvaluationReason::rule_match(1, "x"), EvaluationReason::rule_match(1, "x").with_override_affected(true) ] ], + [ EvaluationReason::error(EvaluationReason::ERROR_MALFORMED_FLAG).with_override_affected(true), EvaluationReason::ERROR, + { "kind" => "ERROR", "errorKind" => "MALFORMED_FLAG", "overrideAffected" => true }, "ERROR(MALFORMED_FLAG)", + [ EvaluationReason::error(EvaluationReason::ERROR_MALFORMED_FLAG) ] ], ] values.each_index do |i| params = values[i] @@ -101,6 +112,70 @@ module LaunchDarkly end end + describe "override affected indicator" do + it "is false by default" do + expect(EvaluationReason::off.override_affected).to be false + expect(EvaluationReason::rule_match(0, "x").override_affected).to be false + expect(EvaluationReason::error(EvaluationReason::ERROR_FLAG_NOT_FOUND).override_affected).to be false + end + + it "returns the same instance when the indicator does not change" do + expect(EvaluationReason::off.with_override_affected(false)).to be EvaluationReason::off + marked = EvaluationReason::off.with_override_affected(true) + expect(marked.with_override_affected(true)).to be marked + end + + it "returns a new instance when the indicator changes and keeps the other properties" do + base = EvaluationReason::rule_match(2, "y", true).with_big_segments_status(BigSegmentsStatus::HEALTHY) + marked = base.with_override_affected(true) + + expect(marked).not_to be base + expect(marked.override_affected).to be true + expect(marked.kind).to eq EvaluationReason::RULE_MATCH + expect(marked.rule_index).to eq 2 + expect(marked.rule_id).to eq "y" + expect(marked.in_experiment).to be true + expect(marked.big_segments_status).to eq BigSegmentsStatus::HEALTHY + expect(marked).not_to eq base + expect(marked.with_override_affected(false)).to eq base + end + + it "coerces the indicator to a boolean" do + expect(EvaluationReason::off.with_override_affected(nil)).to be EvaluationReason::off + expect(EvaluationReason::off.with_override_affected("yes").override_affected).to be true + end + + it "is omitted from the JSON representation when false" do + expect(EvaluationReason::off.as_json).not_to have_key(:overrideAffected) + expect(EvaluationReason::fallthrough(true).as_json).not_to have_key(:overrideAffected) + expect(JSON.parse(EvaluationReason::off.to_json)).to eq({ "kind" => "OFF" }) + end + + it "is written as true in the JSON representation when set" do + expect(EvaluationReason::fallthrough(true).with_override_affected(true).as_json).to eq( + { kind: :FALLTHROUGH, inExperiment: true, overrideAffected: true }) + end + + it "is exposed through []" do + expect(EvaluationReason::off[:overrideAffected]).to be false + expect(EvaluationReason::off.with_override_affected(true)[:overrideAffected]).to be true + end + + it "takes part in equality with a hash" do + marked = EvaluationReason::off.with_override_affected(true) + expect(marked == { kind: "OFF", overrideAffected: true }).to be true + expect(marked == { kind: "OFF" }).to be false + expect(EvaluationReason::off == { kind: "OFF" }).to be true + expect(EvaluationReason::off == { kind: "OFF", overrideAffected: true }).to be false + end + + it "keeps the indicator when the big segments status changes" do + marked = EvaluationReason::off.with_override_affected(true).with_big_segments_status(BigSegmentsStatus::STALE) + expect(marked.override_affected).to be true + expect(marked.big_segments_status).to eq BigSegmentsStatus::STALE + end + end + it "supports [] with JSON property names" do expect(EvaluationReason::off[:kind]).to eq "OFF" expect(EvaluationReason::off[:ruleIndex]).to be nil diff --git a/spec/impl/evaluator_override_spec.rb b/spec/impl/evaluator_override_spec.rb new file mode 100644 index 00000000..672841e4 --- /dev/null +++ b/spec/impl/evaluator_override_spec.rb @@ -0,0 +1,298 @@ +require "spec_helper" +require "impl/evaluator_spec_base" + +module LaunchDarkly + module Impl + describe "evaluate with override definitions", :evaluator_spec_base => true do + let(:context) { LDContext.create({ key: "user-key", kind: "user" }) } + let(:other_context) { LDContext.create({ key: "other-key", kind: "user" }) } + + # A flag that is off and serves one value, which is how a value-only override is expanded. + def value_flag(key, value) + Flags.from_hash({ key: key, version: 1, on: false, offVariation: 0, variations: [value] }) + end + + # A flag that is on and serves variation 1 unless a prerequisite fails. + def flag_with_prerequisites(key, *prereq_keys) + Flags.from_hash({ + key: key, + version: 1, + on: true, + prerequisites: prereq_keys.map { |k| { key: k, variation: 1 } }, + fallthrough: { variation: 1 }, + offVariation: 0, + variations: ["prereq-failed-#{key}", "value-#{key}"], + }) + end + + # A flag that is on and serves variation 1. + def on_flag(key) + flag_with_prerequisites(key) + end + + def segment_flag(key, segment_key, negate: false) + Flags.from_hash({ + key: key, + version: 1, + on: true, + rules: [{ id: "segment-rule", variation: 1, + clauses: [{ attribute: "", op: "segmentMatch", values: [segment_key], negate: negate }] }], + fallthrough: { variation: 0 }, + offVariation: 0, + variations: ["not-included", "included"], + }) + end + + def segment_including(key, *user_keys) + Segments.from_hash({ key: key, version: 1, included: user_keys }) + end + + def record_for(result, key) + result.prereq_evals.detect { |r| r.prereq_flag.key == key } + end + + it "marks an evaluation of a flag from the override store" do + flag = value_flag("flag", "override-value").as_override + e = EvaluatorBuilder.new(logger).build + + (result, state) = e.evaluate(flag, context) + + expect(result.override_affected).to be true + expect(state.override_affected).to be true + expect(result.detail.value).to eq "override-value" + expect(result.detail.variation_index).to eq 0 + expect(result.detail.reason.kind).to eq EvaluationReason::OFF + expect(result.detail.reason.override_affected).to be true + expect(result.detail.reason.as_json).to eq({ kind: :OFF, overrideAffected: true }) + end + + it "does not mark an evaluation that read no override definition and keeps the shared detail instance" do + flag = value_flag("flag", "ld-value") + e = EvaluatorBuilder.new(logger).build + + (result, _) = e.evaluate(flag, context) + + expect(result.override_affected).to be false + expect(result.detail).to be flag.off_result + expect(result.detail.reason.override_affected).to be false + expect(result.detail.reason.as_json).to eq({ kind: :OFF }) + end + + it "marks an evaluation whose prerequisite came from the override store, and marks the prerequisite's record" do + flag = flag_with_prerequisites("flag", "prereq") + prereq = on_flag("prereq").as_override + e = EvaluatorBuilder.new(logger).with_flag(prereq).build + + (result, _) = e.evaluate(flag, context) + + expect(result.override_affected).to be true + expect(result.detail.value).to eq "value-flag" + expect(result.detail.reason).to eq EvaluationReason::fallthrough.with_override_affected(true) + record = record_for(result, "prereq") + expect(record.override_affected).to be true + expect(record.detail.reason).to eq EvaluationReason::fallthrough.with_override_affected(true) + end + + it "does not mark the record of an unaffected prerequisite inside a marked evaluation" do + flag = flag_with_prerequisites("flag", "overridden-prereq", "plain-prereq") + e = EvaluatorBuilder.new(logger) + .with_flag(on_flag("overridden-prereq").as_override) + .with_flag(on_flag("plain-prereq")) + .build + + (result, _) = e.evaluate(flag, context) + + expect(result.override_affected).to be true + expect(result.detail.reason.override_affected).to be true + expect(record_for(result, "overridden-prereq").override_affected).to be true + expect(record_for(result, "overridden-prereq").detail.reason.override_affected).to be true + plain = record_for(result, "plain-prereq") + expect(plain.override_affected).to be false + expect(plain.detail.reason.override_affected).to be false + expect(plain.detail.reason).to eq EvaluationReason::fallthrough + end + + it "does not mark a prerequisite's record because the evaluated flag is an override" do + flag = flag_with_prerequisites("flag", "prereq").as_override + e = EvaluatorBuilder.new(logger).with_flag(on_flag("prereq")).build + + (result, _) = e.evaluate(flag, context) + + expect(result.override_affected).to be true + record = record_for(result, "prereq") + expect(record.override_affected).to be false + expect(record.detail.reason.override_affected).to be false + end + + it "propagates the marking upward through every level of prerequisites and not sideways" do + # A depends on B and C. B depends on D, which is the only override. C uses segment S1. + a = flag_with_prerequisites("a", "b", "c") + b = flag_with_prerequisites("b", "d") + c = segment_flag("c", "s1") + d = on_flag("d").as_override + s1 = segment_including("s1", context.key) + e = EvaluatorBuilder.new(logger).with_flag(b).with_flag(c).with_flag(d).with_segment(s1).build + + (result, _) = e.evaluate(a, context) + + expect(result.override_affected).to be true + expect(result.detail.reason.override_affected).to be true + expect(record_for(result, "b").override_affected).to be true + expect(record_for(result, "d").override_affected).to be true + expect(record_for(result, "c").override_affected).to be false + expect(record_for(result, "c").detail.reason.override_affected).to be false + end + + it "marks an evaluation when a rule read a segment from the override store" do + flag = segment_flag("flag", "seg") + e = EvaluatorBuilder.new(logger).with_segment(segment_including("seg", context.key).as_override).build + + (result, _) = e.evaluate(flag, context) + + expect(result.override_affected).to be true + expect(result.detail.value).to eq "included" + expect(result.detail.reason).to eq EvaluationReason::rule_match(0, "segment-rule").with_override_affected(true) + end + + it "marks an evaluation when an override segment was read but did not match" do + flag = segment_flag("flag", "seg") + e = EvaluatorBuilder.new(logger).with_segment(segment_including("seg", context.key).as_override).build + + (result, _) = e.evaluate(flag, other_context) + + expect(result.override_affected).to be true + expect(result.detail.value).to eq "not-included" + expect(result.detail.reason).to eq EvaluationReason::fallthrough.with_override_affected(true) + end + + it "marks an evaluation when a negated clause read an override segment" do + flag = segment_flag("flag", "seg", negate: true) + e = EvaluatorBuilder.new(logger).with_segment(segment_including("seg", context.key).as_override).build + + (result, _) = e.evaluate(flag, other_context) + + expect(result.override_affected).to be true + expect(result.detail.value).to eq "included" + end + + it "marks an evaluation when a segment rule read another segment from the override store" do + flag = segment_flag("flag", "outer") + outer = Segments.from_hash({ + key: "outer", + version: 1, + rules: [{ clauses: [{ attribute: "", op: "segmentMatch", values: ["inner"] }] }], + }) + inner = segment_including("inner", context.key).as_override + e = EvaluatorBuilder.new(logger).with_segment(outer).with_segment(inner).build + + (result, _) = e.evaluate(flag, context) + + expect(result.override_affected).to be true + expect(result.detail.value).to eq "included" + end + + it "does not mark an evaluation that read only LaunchDarkly segments" do + flag = segment_flag("flag", "seg") + e = EvaluatorBuilder.new(logger).with_segment(segment_including("seg", context.key)).build + + (result, _) = e.evaluate(flag, context) + + expect(result.override_affected).to be false + expect(result.detail.reason).to eq EvaluationReason::rule_match(0, "segment-rule") + end + + it "does not mark an evaluation for a definition that could not be resolved" do + flag = flag_with_prerequisites("flag", "missing") + e = EvaluatorBuilder.new(logger).with_unknown_flag("missing").build + + (result, _) = e.evaluate(flag, context) + + expect(result.override_affected).to be false + expect(result.detail.reason).to eq EvaluationReason::prerequisite_failed("missing") + end + + it "marks a prerequisite-failed result when the failed prerequisite came from the override store" do + flag = flag_with_prerequisites("flag", "prereq") + prereq = value_flag("prereq", "off-value").as_override + e = EvaluatorBuilder.new(logger).with_flag(prereq).build + + (result, _) = e.evaluate(flag, context) + + expect(result.override_affected).to be true + expect(result.detail.value).to eq "prereq-failed-flag" + expect(result.detail.reason).to eq EvaluationReason::prerequisite_failed("prereq").with_override_affected(true) + expect(record_for(result, "prereq").override_affected).to be true + end + + it "marks an error result for a malformed override definition" do + flag = Flags.from_hash({ key: "flag", version: 1, on: true, variations: ["only"], fallthrough: { variation: 5 }, offVariation: 0 }).as_override + e = EvaluatorBuilder.new(logger).build + + (result, _) = e.evaluate(flag, context) + + expect(result.override_affected).to be true + expect(result.detail.value).to be_nil + expect(result.detail.variation_index).to be_nil + expect(result.detail.reason).to eq EvaluationReason::error(EvaluationReason::ERROR_MALFORMED_FLAG).with_override_affected(true) + expect(result.detail.reason.as_json).to eq({ kind: :ERROR, errorKind: :MALFORMED_FLAG, overrideAffected: true }) + end + + it "marks an error result when an override definition was read before the evaluation failed" do + # The evaluated flag is not an override. Its override prerequisite refers back to it, which + # ends the evaluation with an error. The prerequisite definition was read, so the result is marked. + flag = flag_with_prerequisites("flag", "prereq") + prereq = flag_with_prerequisites("prereq", "flag").as_override + e = EvaluatorBuilder.new(logger).with_flag(prereq).with_flag(flag).build + + (result, _) = e.evaluate(flag, context) + + expect(result.detail.reason.kind).to eq EvaluationReason::ERROR + expect(result.detail.reason.error_kind).to eq EvaluationReason::ERROR_MALFORMED_FLAG + expect(result.override_affected).to be true + expect(result.detail.reason.override_affected).to be true + end + + it "keeps the marking of an override flag when its prerequisite evaluation fails" do + # The evaluated flag is an override. Its plain prerequisite refers back to it, which ends the + # evaluation with an error while the prerequisite is being evaluated. The flag's own marking + # must survive that. + flag = flag_with_prerequisites("flag", "prereq").as_override + prereq = flag_with_prerequisites("prereq", "flag") + e = EvaluatorBuilder.new(logger).with_flag(prereq).with_flag(flag).build + + (result, _) = e.evaluate(flag, context) + + expect(result.detail.reason.error_kind).to eq EvaluationReason::ERROR_MALFORMED_FLAG + expect(result.override_affected).to be true + expect(result.detail.reason.override_affected).to be true + end + + it "keeps the marking from an override prerequisite when a deeper prerequisite evaluation fails" do + # A depends on B, which is an override. B depends on C, which refers back to A. The marking + # that B contributed must survive the error raised while C is being evaluated. + a = flag_with_prerequisites("a", "b") + b = flag_with_prerequisites("b", "c").as_override + c = flag_with_prerequisites("c", "a") + e = EvaluatorBuilder.new(logger).with_flag(a).with_flag(b).with_flag(c).build + + (result, _) = e.evaluate(a, context) + + expect(result.detail.reason.error_kind).to eq EvaluationReason::ERROR_MALFORMED_FLAG + expect(result.override_affected).to be true + end + + it "keeps the big segments status on a marked reason" do + segment = Segments.from_hash({ key: "seg", version: 1, unbounded: true, generation: 1 }).as_override + flag = segment_flag("flag", "seg") + e = EvaluatorBuilder.new(logger).with_segment(segment).with_big_segment_for_context(context, segment, true).build + + (result, _) = e.evaluate(flag, context) + + expect(result.override_affected).to be true + expect(result.detail.value).to eq "included" + expect(result.detail.reason.big_segments_status).to eq BigSegmentsStatus::HEALTHY + expect(result.detail.reason.override_affected).to be true + end + end + end +end diff --git a/spec/impl/evaluator_prereq_spec.rb b/spec/impl/evaluator_prereq_spec.rb index fc674dd2..59675d83 100644 --- a/spec/impl/evaluator_prereq_spec.rb +++ b/spec/impl/evaluator_prereq_spec.rb @@ -69,7 +69,7 @@ module Impl context = LDContext.create({ key: 'x' }) detail = EvaluationDetail.new('b', 1, EvaluationReason::prerequisite_failed('feature1')) expected_prereqs = [ - PrerequisiteEvalRecord.new(flag1, flag, EvaluationDetail.new('d', 0, EvaluationReason::fallthrough())), + PrerequisiteEvalRecord.new(flag1, flag, EvaluationDetail.new('d', 0, EvaluationReason::fallthrough()), false), ] e = EvaluatorBuilder.new(logger).with_flag(flag1).with_unknown_flag('feature2').build (result, state) = e.evaluate(flag, context) @@ -103,7 +103,7 @@ module Impl context = LDContext.create({ key: 'x' }) detail = EvaluationDetail.new('b', 1, EvaluationReason::prerequisite_failed('feature1')) expected_prereqs = [ - PrerequisiteEvalRecord.new(flag1, flag, EvaluationDetail.new(nil, nil, EvaluationReason::prerequisite_failed('feature2'))), + PrerequisiteEvalRecord.new(flag1, flag, EvaluationDetail.new(nil, nil, EvaluationReason::prerequisite_failed('feature2')), false), ] e = EvaluatorBuilder.new(logger).with_flag(flag1).with_unknown_flag('feature2').build (result, state) = e.evaluate(flag, context) @@ -138,7 +138,7 @@ module Impl context = LDContext.create({ key: 'x' }) detail = EvaluationDetail.new('b', 1, EvaluationReason::prerequisite_failed('feature1')) expected_prereqs = [ - PrerequisiteEvalRecord.new(flag1, flag, EvaluationDetail.new('e', 1, EvaluationReason::off)), + PrerequisiteEvalRecord.new(flag1, flag, EvaluationDetail.new('e', 1, EvaluationReason::off), false), ] e = EvaluatorBuilder.new(logger).with_flag(flag1).build (result, state) = e.evaluate(flag, context) @@ -171,7 +171,7 @@ module Impl context = LDContext.create({ key: 'x' }) detail = EvaluationDetail.new('b', 1, EvaluationReason::prerequisite_failed('feature1')) expected_prereqs = [ - PrerequisiteEvalRecord.new(flag1, flag, EvaluationDetail.new('d', 0, EvaluationReason::fallthrough)), + PrerequisiteEvalRecord.new(flag1, flag, EvaluationDetail.new('d', 0, EvaluationReason::fallthrough), false), ] e = EvaluatorBuilder.new(logger).with_flag(flag1).build (result, state) = e.evaluate(flag, context) @@ -204,7 +204,7 @@ module Impl context = LDContext.create({ key: 'x' }) detail = EvaluationDetail.new('a', 0, EvaluationReason::fallthrough) expected_prereqs = [ - PrerequisiteEvalRecord.new(flag1, flag, EvaluationDetail.new('e', 1, EvaluationReason::fallthrough)), + PrerequisiteEvalRecord.new(flag1, flag, EvaluationDetail.new('e', 1, EvaluationReason::fallthrough), false), ] e = EvaluatorBuilder.new(logger).with_flag(flag1).build (result, state) = e.evaluate(flag, context) diff --git a/spec/impl/model/override_marker_spec.rb b/spec/impl/model/override_marker_spec.rb new file mode 100644 index 00000000..3facd9e1 --- /dev/null +++ b/spec/impl/model/override_marker_spec.rb @@ -0,0 +1,70 @@ +require "spec_helper" +require "model_builders" + +module LaunchDarkly + module Impl + module Model + describe "override marker" do + let(:flag_data) { { key: "flag1", version: 3, on: false, offVariation: 0, variations: ["a"] } } + let(:segment_data) { { key: "seg1", version: 4, included: ["user1"] } } + + it "is not set on a flag or segment built from data" do + expect(Flags.from_hash(flag_data).override?).to be false + expect(Segments.from_hash(segment_data).override?).to be false + end + + it "is not set on a deleted item" do + expect(Flags.from_hash({ key: "flag1", version: 3, deleted: true }).override?).to be false + expect(Segments.from_hash({ key: "seg1", version: 4, deleted: true }).override?).to be false + end + + it "is set on the copy returned by as_override and not on the original" do + flag = Flags.from_hash(flag_data) + segment = Segments.from_hash(segment_data) + + marked_flag = flag.as_override + marked_segment = segment.as_override + + expect(marked_flag.override?).to be true + expect(marked_segment.override?).to be true + expect(flag.override?).to be false + expect(segment.override?).to be false + expect(marked_flag).not_to be flag + expect(marked_segment).not_to be segment + end + + it "keeps the copy equal to the original and keeps its properties" do + flag = Flags.from_hash(flag_data) + marked = flag.as_override + + expect(marked).to eq flag + expect(marked.key).to eq "flag1" + expect(marked.version).to eq 3 + expect(marked.off_result).to eq flag.off_result + expect(marked[:variations]).to eq ["a"] + end + + it "does not serialize the marker" do + flag = Flags.from_hash(flag_data) + segment = Segments.from_hash(segment_data) + + expect(flag.as_override.to_json).to eq flag.to_json + expect(segment.as_override.to_json).to eq segment.to_json + expect(flag.as_override.as_json).to eq flag_data + expect(segment.as_override.as_json).to eq segment_data + expect(Model.serialize(DataStore::FEATURES, flag.as_override)).to eq Model.serialize(DataStore::FEATURES, flag) + end + + it "keeps the marker on a copy of a marked item" do + expect(Flags.from_hash(flag_data).as_override.as_override.override?).to be true + end + + it "returns a marked item unchanged from deserialization" do + marked = Flags.from_hash(flag_data).as_override + + expect(Model.deserialize(DataStore::FEATURES, marked)).to be marked + end + end + end + end +end