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