diff --git a/contract-tests/Gemfile b/contract-tests/Gemfile index 2cf6ac62..af66f0bc 100644 --- a/contract-tests/Gemfile +++ b/contract-tests/Gemfile @@ -8,6 +8,9 @@ gem "puma", "~> 8.0" gem "rackup", "~> 2.2" gem 'sinatra', '>= 4.1' +# The file-based override source uses the listen gem for its watching change detection mode. +gem 'listen', '~> 3.3' + gem 'rubocop', '~> 1.37', group: 'development' gem 'rubocop-performance', '~> 1.15', group: 'development' diff --git a/contract-tests/client_entity.rb b/contract-tests/client_entity.rb index 5df1f56a..8fe1ce60 100644 --- a/contract-tests/client_entity.rb +++ b/contract-tests/client_entity.rb @@ -56,7 +56,12 @@ def initialize(log, config) opts[:payload_filter_key] = data_system_config[:payloadFilter] end + # Flag overrides are an option of the FDv2 data system configuration. + data_system.overrides(build_override_source(config[:overrides])) if config[:overrides] + opts[:data_system_config] = data_system.build + elsif config[:overrides] + raise ArgumentError, "flag overrides require the data system to be configured" elsif config[:streaming] streaming = config[:streaming] opts[:stream_uri] = streaming[:baseUri] unless streaming[:baseUri].nil? @@ -365,6 +370,23 @@ def close builder end + # + # Builds the file-based override source from the `overrides` block of the contract test + # configuration. + # + # @param overrides_config [Hash] the overrides configuration + # @return [Object] a builder for LaunchDarkly::DataSystem::ConfigBuilder#overrides + # + private def build_override_source(overrides_config) + options = { paths: overrides_config[:filePaths] } + if overrides_config[:duplicateKeysHandling] + options[:duplicate_keys_handling] = overrides_config[:duplicateKeysHandling].to_sym + end + options[:change_detection] = overrides_config[:changeDetection].to_sym if overrides_config[:changeDetection] + set_optional_time_prop(overrides_config, :pollIntervalMs, options, :poll_interval) + LaunchDarkly::Integrations::FileData.override_source(options) + end + # # Builds a persistent data store from the contract test configuration. # diff --git a/contract-tests/service.rb b/contract-tests/service.rb index 779ed37d..246220a6 100644 --- a/contract-tests/service.rb +++ b/contract-tests/service.rb @@ -17,7 +17,7 @@ } set :bind, '0.0.0.0' -set :port, 9000 +set :port, (ENV['PORT'] || 9000).to_i set :logging, false clients = {} @@ -55,6 +55,7 @@ 'flag-change-listeners', 'flag-value-change-listeners', 'fdv1-fallback', + 'flag-overrides', ], }.to_json end diff --git a/lib/ldclient-rb/config.rb b/lib/ldclient-rb/config.rb index db0f5e6c..354b317e 100644 --- a/lib/ldclient-rb/config.rb +++ b/lib/ldclient-rb/config.rb @@ -736,14 +736,18 @@ class DataSystemConfig # @param data_store [LaunchDarkly::Interfaces::FeatureStore, nil] The (optional) data store # @param fdv1_fallback_synchronizer [#build(String, Config), nil] # The (optional) builder for FDv1-compatible fallback synchronizer + # @param overrides [#build(String, Config), nil] The (optional) builder for an override source. + # Flag overrides are currently experimental and subject to change. # def initialize(initializers: nil, synchronizers: nil, - data_store_mode: LaunchDarkly::Interfaces::DataSystem::DataStoreMode::READ_ONLY, data_store: nil, fdv1_fallback_synchronizer: nil) + data_store_mode: LaunchDarkly::Interfaces::DataSystem::DataStoreMode::READ_ONLY, data_store: nil, fdv1_fallback_synchronizer: nil, + overrides: nil) @initializers = initializers @synchronizers = synchronizers @data_store_mode = data_store_mode @data_store = data_store @fdv1_fallback_synchronizer = fdv1_fallback_synchronizer + @overrides = overrides end # The initializer builders for the data system. Each builder responds to build(sdk_key, config) and returns an Initializer. @@ -765,5 +769,11 @@ def initialize(initializers: nil, synchronizers: nil, # The FDv1-compatible fallback synchronizer builder. Responds to build(sdk_key, config) and returns a Synchronizer. # @return [#build(String, Config), nil] attr_reader :fdv1_fallback_synchronizer + + # The override source builder. Responds to build(sdk_key, config) and returns an + # {LaunchDarkly::Interfaces::Overrides::OverrideSource}. Flag overrides are currently + # experimental and subject to change. + # @return [#build(String, Config), nil] + attr_reader :overrides end end diff --git a/lib/ldclient-rb/data_system/config_builder.rb b/lib/ldclient-rb/data_system/config_builder.rb index 43dced05..7a4176cc 100644 --- a/lib/ldclient-rb/data_system/config_builder.rb +++ b/lib/ldclient-rb/data_system/config_builder.rb @@ -23,6 +23,7 @@ def initialize @fdv1_fallback_synchronizer = nil @data_store_mode = LaunchDarkly::Interfaces::DataSystem::DataStoreMode::READ_ONLY @data_store = nil + @overrides = nil end # @@ -85,6 +86,38 @@ def data_store(data_store, store_mode) self end + # + # Configures the SDK with an override source. Flag overrides are currently experimental and + # subject to change. + # + # Overrides are flag and segment definitions that take precedence over data received from + # LaunchDarkly at evaluation time, on a per-key basis. They exist for resilience during an + # incident. Overrides let an operator force one or more flags to a known state on a running + # client, whether or not the client can reach LaunchDarkly. Flags not present in the override + # data are completely unaffected. + # + # The override source is not a data source. It has no effect on the client's initialization + # status, data availability, or data source status. Without a configured source, or while + # the source supplies no overrides, the SDK behaves as it would without this feature. + # + # At most one override source can be configured. A later call replaces an earlier one. + # + # When the client is offline, the source is not built or started: no overrides are served, + # and a configuration error in the source is not reported. The SDK logs a warning instead. + # + # @example + # overrides = LaunchDarkly::Integrations::FileData.override_source(paths: ["/etc/launchdarkly/overrides.json"]) + # config = LaunchDarkly::Config.new(data_system: LaunchDarkly::DataSystem.default.overrides(overrides)) + # + # @param source [#build(String, Config), nil] a builder that responds to build(sdk_key, config) + # and returns a {LaunchDarkly::Interfaces::Overrides::OverrideSource}, or nil to remove one + # @return [ConfigBuilder] self for chaining + # + def overrides(source) + @overrides = source + self + end + # # Builds the data system configuration. # @@ -96,7 +129,8 @@ def build synchronizers: @synchronizers, data_store_mode: @data_store_mode, data_store: @data_store, - fdv1_fallback_synchronizer: @fdv1_fallback_synchronizer + fdv1_fallback_synchronizer: @fdv1_fallback_synchronizer, + overrides: @overrides ) end end 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/events.rb b/lib/ldclient-rb/events.rb index f056a9aa..9e0aec48 100644 --- a/lib/ldclient-rb/events.rb +++ b/lib/ldclient-rb/events.rb @@ -44,7 +44,8 @@ def record_eval_event( debug_until = nil, prereq_of = nil, sampling_ratio = nil, - exclude_from_summaries = false + exclude_from_summaries = false, + override_affected = false ) end @@ -176,10 +177,11 @@ def record_eval_event( debug_until = nil, prereq_of = nil, sampling_ratio = nil, - exclude_from_summaries = false + exclude_from_summaries = false, + override_affected = false ) post_to_inbox(LaunchDarkly::Impl::EvalEvent.new(timestamp, context, key, version, variation, value, reason, - default, track_events, debug_until, prereq_of, sampling_ratio, exclude_from_summaries)) + default, track_events, debug_until, prereq_of, sampling_ratio, exclude_from_summaries, override_affected)) end def record_identify_event(context) @@ -328,8 +330,10 @@ def dispatch_event(event, outbox) will_add_full_event = false debug_event = nil if event.is_a?(LaunchDarkly::Impl::EvalEvent) - will_add_full_event = event.track_events - if should_debug_event(event) + # An override-affected evaluation appears only in the summary counters. It produces no + # individual feature event and no debug event, whatever the flag's configuration requests. + will_add_full_event = event.track_events && !event.override_affected + if !event.override_affected && should_debug_event(event) debug_event = LaunchDarkly::Impl::DebugEvent.new(event) end else @@ -630,18 +634,22 @@ def make_output_events(events, summary) summary.counters.each do |flagKey, flagInfo| counters = [] flagInfo.versions.each do |version, variations| - variations.each do |variation, counter| - c = { - value: counter.value, - count: counter.count, - } - c[:variation] = variation unless variation.nil? - if version.nil? - c[:unknown] = true - else - c[:version] = version + variations.each do |variation, counters_by_marker| + counters_by_marker.each do |override_affected, counter| + c = { + value: counter.value, + count: counter.count, + } + c[:variation] = variation unless variation.nil? + if version.nil? + c[:unknown] = true + else + c[:version] = version + end + # The marker is present only for override-affected counters, like the unknown marker. + c[:overrideAffected] = true if override_affected + counters.push(c) end - counters.push(c) end end flags[flagKey] = { default: flagInfo.default, counters: counters, contextKinds: flagInfo.context_kinds.to_a } diff --git a/lib/ldclient-rb/impl/data_store/store.rb b/lib/ldclient-rb/impl/data_store/store.rb index f374b9d4..0bca8ba0 100644 --- a/lib/ldclient-rb/impl/data_store/store.rb +++ b/lib/ldclient-rb/impl/data_store/store.rb @@ -38,9 +38,14 @@ def initialize(flag_change_broadcaster, change_set_broadcaster, logger) # Source of truth for flag evaluations once initialized @memory_store = InMemoryFeatureStoreV2.new(logger) - # Used to track dependencies between items in the store + # Used to track dependencies between items in the store. Items are keyed by string, + # the form in which the data model names prerequisites and segments. @dependency_tracker = LaunchDarkly::Impl::DependencyTracker.new(logger) + # A second dependency graph, such as the override layer's, whose items may depend on + # items in this store. Set once when the data system is built, before any data. + @external_dependencies = nil + # Broadcasters for events @flag_change_broadcaster = flag_change_broadcaster @change_set_broadcaster = change_set_broadcaster @@ -179,6 +184,41 @@ def initialized? get_active_store.initialized? end + # + # Retrieves an item from the active store. + # + # @param kind [LaunchDarkly::Impl::DataStore::DataKind] + # @param key [String, Symbol] + # @return [Object, nil] the item, or nil if it is unknown or deleted + # + def get(kind, key) + get_active_store.get(kind, key) + end + + # + # Retrieves all items of a kind from the active store. + # + # @param kind [LaunchDarkly::Impl::DataStore::DataKind] + # @return [Hash{Symbol => Object}] + # + def all(kind) + get_active_store.all(kind) + end + + # + # Registers a second dependency graph whose items may depend on items in this store. A + # change to an item in this store then also notifies the external items that depend on + # it, directly or through any chain of dependencies in either graph. + # + # @param dependencies [#add_affected_items, nil] a graph answering `add_affected_items(items_out, { kind:, key: })` + # @return [void] + # + def set_external_dependencies(dependencies) + @lock.synchronize do + @external_dependencies = dependencies + end + end + # # Get the data store status provider for the persistent store, if configured. # @@ -254,9 +294,9 @@ def get_data_store_status_provider collections.each do |kind, collection| collection.each do |key, item| - @dependency_tracker.update_dependencies_from(kind, key, item) + @dependency_tracker.update_dependencies_from(kind, key.to_s, item) if has_listeners - @dependency_tracker.add_affected_items(affected_items, { kind: kind, key: key }) + add_affected_items(affected_items, { kind: kind, key: key }) end end end @@ -338,7 +378,38 @@ def get_data_store_status_provider @dependency_tracker.reset all_data.each do |kind, items| items.each do |key, item| - @dependency_tracker.update_dependencies_from(kind, key, item) + @dependency_tracker.update_dependencies_from(kind, key.to_s, item) + end + end + end + + # + # Adds the item and everything that depends on it to the set, following this store's own + # dependency graph and the external one. Every item reached through either graph is + # expanded through both, so a dependency chain may cross between them any number of + # times. Keys are compared as strings, so a change keyed by symbol reaches the same + # dependents as one keyed by string. + # + # @param items_out [Set] Set of {kind:, key:} hashes + # @param initial_modified_item [Hash] {kind:, key:} hash + # @return [void] + # + private def add_affected_items(items_out, initial_modified_item) + pending = [{ kind: initial_modified_item[:kind], key: initial_modified_item[:key].to_s }] + until pending.empty? + current = pending.pop + # Each graph gets its own working set: a tracker does not expand an item that is + # already present, so a shared set would stop the second graph at the start. + via_store = Set.new + @dependency_tracker.add_affected_items(via_store, current) + via_external = Set.new + @external_dependencies&.add_affected_items(via_external, current) + (via_store | via_external).each do |reached| + item = { kind: reached[:kind], key: reached[:key].to_s } + next if items_out.include?(item) + + items_out.add(item) + pending << item end end end @@ -380,9 +451,9 @@ def get_data_store_status_provider # If either is missing or versions differ, it's a change if old_item.nil? || new_item.nil? - @dependency_tracker.add_affected_items(affected_items, { kind: kind, key: key }) + add_affected_items(affected_items, { kind: kind, key: key }) elsif old_item[:version] != new_item[:version] - @dependency_tracker.add_affected_items(affected_items, { kind: kind, key: key }) + add_affected_items(affected_items, { kind: kind, key: key }) end end end diff --git a/lib/ldclient-rb/impl/data_system.rb b/lib/ldclient-rb/impl/data_system.rb index d137d460..4bdb7668 100644 --- a/lib/ldclient-rb/impl/data_system.rb +++ b/lib/ldclient-rb/impl/data_system.rb @@ -108,6 +108,19 @@ def store raise NotImplementedError, "#{self.class} must implement #store" end + # + # Returns the override store's entries of the given kind, without LaunchDarkly data. The + # result is empty when no override source is configured or the store holds none. The + # returned hash must not be modified. Flag overrides are currently experimental and subject + # to change. + # + # @param kind [LaunchDarkly::Impl::DataStore::DataKind] + # @return [Hash{Symbol => Object}] + # + def override_items(kind) + raise NotImplementedError, "#{self.class} must implement #override_items" + end + # # Sets the diagnostic accumulator for streaming initialization metrics. # This should be called before start() to ensure metrics are collected. diff --git a/lib/ldclient-rb/impl/data_system/fdv1.rb b/lib/ldclient-rb/impl/data_system/fdv1.rb index b6ba6cbc..a646156f 100644 --- a/lib/ldclient-rb/impl/data_system/fdv1.rb +++ b/lib/ldclient-rb/impl/data_system/fdv1.rb @@ -101,6 +101,11 @@ def store @store_wrapper end + # (see DataSystem#override_items) + def override_items(_kind) + {} + end + # (see DataSystem#set_diagnostic_accumulator) def set_diagnostic_accumulator(diagnostic_accumulator) @diagnostic_accumulator = diagnostic_accumulator diff --git a/lib/ldclient-rb/impl/data_system/fdv2.rb b/lib/ldclient-rb/impl/data_system/fdv2.rb index 15994d7e..e4c0c07a 100644 --- a/lib/ldclient-rb/impl/data_system/fdv2.rb +++ b/lib/ldclient-rb/impl/data_system/fdv2.rb @@ -8,7 +8,9 @@ require "ldclient-rb/impl/data_source/status_provider" require "ldclient-rb/impl/data_store/status_provider" require "ldclient-rb/impl/broadcaster" +require "ldclient-rb/impl/overrides" require "ldclient-rb/impl/repeating_task" +require "ldclient-rb/impl/util" require "ldclient-rb/interfaces/data_system" module LaunchDarkly @@ -118,6 +120,28 @@ def initialize(sdk_key, config, data_system_config) # Track configuration @configured_with_data_sources = (@data_system_config.initializers && !@data_system_config.initializers.empty?) || !@synchronizer_builders.empty? + + # The following are nil unless an override source is configured. The layer holds override + # entries. The overlay serves them in preference to the store's data at the store read + # boundary. The source populates the layer at runtime. The override system is separate + # from the initializer and synchronizer pipeline and never affects data availability. + @override_layer = nil + @overlay = nil + @override_source = nil + @override_sink = nil + if @data_system_config.overrides && @disabled + @logger.warn { "[LDClient] Offline mode is enabled; the override source is not started and no overrides are served" } + elsif @data_system_config.overrides + # An invalid override source configuration raises here, so LDClient.new reports it + # the same way as other invalid component configuration. + @override_source = @data_system_config.overrides.build(@sdk_key, @config) + @override_layer = LaunchDarkly::Impl::Overrides::Layer.new + @overlay = LaunchDarkly::Impl::Overrides::Overlay.new(@store, @override_layer) + # A LaunchDarkly change to a flag or segment that an override entry depends on must + # notify that entry's flag as well. The store reaches those entries through the + # layer's dependency graph. + @store.set_external_dependencies(@override_layer) + end end # (see DataSystem#start) @@ -131,6 +155,10 @@ def start @stop_event.reset @ready_event.reset + # The override source starts before the run loop, so a source that loads synchronously has + # its overrides in place before the client begins evaluating. + start_override_source if @override_source + # Start the main coordination thread main_thread = Thread.new { run_main_loop } main_thread.name = "FDv2-main" @@ -141,6 +169,14 @@ def start # (see DataSystem#stop) def stop + if @override_source + # The sink stops accepting snapshots first, so a snapshot that the source supplies while + # it stops cannot update the layer or notify listeners while the system shuts down. The + # sink does not exist if the system was never started. + @override_sink&.close + stop_override_source("Error stopping override source") + end + @stop_event.set @lock.synchronize do @@ -175,9 +211,18 @@ def set_diagnostic_accumulator(diagnostic_accumulator) # (see DataSystem#store) def store + return @overlay if @overlay + @store.get_active_store end + # (see DataSystem#override_items) + def override_items(kind) + return {} if @override_layer.nil? + + @override_layer.all(kind) + end + # (see DataSystem#data_source_status_provider) def data_source_status_provider @data_source_status_provider @@ -215,6 +260,43 @@ def target_availability private + # + # Starts the override source. The source has no effect on initialization, and a source that + # fails to start does not prevent the client from operating: the failure is logged, the + # source is stopped, and the client runs without overrides. The SDK's own file-based source + # does not raise from start; it logs a failed load and retries it. A custom source could + # raise, and an application in an incident is better served by a client that runs without + # its overrides than by one that does not start. + # + def start_override_source + @override_sink = LaunchDarkly::Impl::Overrides::Sink.new(@override_layer, @store, @flag_change_broadcaster, @logger) + begin + @override_source.start(@override_sink) + rescue => e + LaunchDarkly::Impl::Util.log_exception(@logger, "Unable to start the override source; the client runs without overrides", e) + stop_override_source("Error stopping override source after it failed to start") + # The source may have supplied a snapshot before it failed. Clearing the layer removes + # it, so the client runs without overrides as logged, and a late snapshot from the + # stopped source is ignored. + @override_sink.close_and_clear + end + end + + # + # Stops the override source and forgets it. A custom source can raise from stop; that is + # logged and not raised, so the rest of the system still stops, or still starts after a + # failed start. + # + def stop_override_source(failure_message) + source = @override_source + @override_source = nil + begin + source.stop + rescue => e + @logger.error { "[LDClient] #{failure_message}: #{e.message}" } + end + end + # # Main coordination loop that manages initializers and synchronizers. # 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/event_summarizer.rb b/lib/ldclient-rb/impl/event_summarizer.rb index 4109f80a..a16da506 100644 --- a/lib/ldclient-rb/impl/event_summarizer.rb +++ b/lib/ldclient-rb/impl/event_summarizer.rb @@ -5,6 +5,10 @@ module LaunchDarkly module Impl EventSummary = Struct.new(:start_date, :end_date, :counters) + # The `versions` member is a hash of hashes: flag version, then variation index, then the + # override-affected marker (a boolean), to an EventSummaryFlagVariationCounter. The marker + # takes part in the key, so override-affected and other evaluations of the same flag, + # variation, and version accumulate into separate counters. EventSummaryFlagInfo = Struct.new(:default, :versions, :context_kinds) EventSummaryFlagVariationCounter = Struct.new(:value, :count) @@ -39,9 +43,16 @@ def summarize_event(event) counters_for_flag.context_kinds.merge(event.context.kinds) - variation_counter = counters_for_flag_version[event.variation] + counters_for_variation = counters_for_flag_version[event.variation] + if counters_for_variation.nil? + counters_for_variation = Hash.new + counters_for_flag_version[event.variation] = counters_for_variation + end + + override_affected = event.override_affected + variation_counter = counters_for_variation[override_affected] if variation_counter.nil? - counters_for_flag_version[event.variation] = EventSummaryFlagVariationCounter.new(event.value, 1) + counters_for_variation[override_affected] = EventSummaryFlagVariationCounter.new(event.value, 1) else variation_counter.count = variation_counter.count + 1 end diff --git a/lib/ldclient-rb/impl/event_types.rb b/lib/ldclient-rb/impl/event_types.rb index 9745373b..172c21d8 100644 --- a/lib/ldclient-rb/impl/event_types.rb +++ b/lib/ldclient-rb/impl/event_types.rb @@ -26,7 +26,8 @@ def initialize(timestamp, context, sampling_ratio = nil, exclude_from_summaries class EvalEvent < Event def initialize(timestamp, context, key, version = nil, variation = nil, value = nil, reason = nil, default = nil, - track_events = false, debug_until = nil, prereq_of = nil, sampling_ratio = nil, exclude_from_summaries = false) + track_events = false, debug_until = nil, prereq_of = nil, sampling_ratio = nil, exclude_from_summaries = false, + override_affected = false) super(timestamp, context, sampling_ratio, exclude_from_summaries) @key = key @version = version @@ -38,6 +39,7 @@ def initialize(timestamp, context, key, version = nil, variation = nil, value = @track_events = track_events if track_events @debug_until = debug_until if debug_until @prereq_of = prereq_of if prereq_of + @override_affected = true if override_affected end attr_reader :key @@ -49,6 +51,14 @@ def initialize(timestamp, context, key, version = nil, variation = nil, value = attr_reader :track_events attr_reader :debug_until attr_reader :prereq_of + + # True if an override affected the evaluation, directly or through a prerequisite or segment. + # Such an evaluation appears only in the summary counters, under a counter that carries the + # override-affected marker. It produces no individual feature event and no debug event. + # @return [Boolean] + def override_affected + @override_affected ? true : false + end end class MigrationOpEvent < Event diff --git a/lib/ldclient-rb/impl/file_data/document.rb b/lib/ldclient-rb/impl/file_data/document.rb index 53e820b9..fa02cfba 100644 --- a/lib/ldclient-rb/impl/file_data/document.rb +++ b/lib/ldclient-rb/impl/file_data/document.rb @@ -2,6 +2,7 @@ require "ldclient-rb/impl/model/serialization" +require "json" require "yaml" module LaunchDarkly @@ -61,21 +62,45 @@ def initialize(flags: {}, flag_values: {}, segments: {}) @segments = segments end + # The document of YAML content that is only a null, such as an empty file, a comment, or + # "---": a document with no entries. + NONE = new(flags: {}.freeze, flag_values: {}.freeze, segments: {}.freeze).freeze + # - # Parses the content of a data file. The content may be JSON or YAML. JSON is a subset of - # YAML, and the Ruby YAML parser handles it, so one parser serves both formats. + # Parses the content of a data file in the given format. {FileData.format_of} chooses the + # format of a file from its name. # - # An empty document is a document with no entries. A document that is not a mapping, or - # whose "flags", "flagValues", or "segments" member is not a mapping, is an error. + # Whether content with no entries is valid depends on the format, like the rest of the + # content. Content that is empty or only white space is not a JSON document, so it fails to + # parse. In YAML it is a document with no entries, {NONE}, and so is a YAML document that is + # only a null, such as one with only a comment. A document that is not a mapping, or whose + # "flags", "flagValues", or "segments" member is not a mapping, is an error, and so is + # content that is not valid UTF-8, in either format. # - # @param content [String] + # @param content [String] UTF-8 content + # @param format [Symbol] `:json` or `:yaml` # @return [Document] # @raise [ArgumentError] if the content is not a valid document - # @raise [Psych::SyntaxError] if the content cannot be parsed + # @raise [JSON::ParserError, Psych::SyntaxError] if the content cannot be parsed # - def self.parse(content) - raw = YAML.safe_load(content) - raw = {} if raw.nil? + def self.parse(content, format:) + FileData.check_encoding(content) + case format + when :json + # A file that is being rewritten in place can be empty for a moment. Failing to parse + # keeps its last good data until it is complete. + raise ArgumentError, "the file has no content, which is not a JSON document" if FileData.blank?(content) + + raw = JSON.parse(content) + when :yaml + # The YAML parser rejects some white space, such as a tab, that is still no content. + return NONE if FileData.blank?(content) + + raw = YAML.safe_load(content) + return NONE if raw.nil? + else + raise ArgumentError, "unknown file format #{format.inspect}" + end raise ArgumentError, "file content must be an object" unless raw.is_a?(Hash) data = FileData.symbolize_keys(raw) @@ -107,15 +132,69 @@ def self.read(path) end end + # The endings of the names of the files that are read as YAML, compared without regard to + # case. Every other file is read as JSON. + YAML_FILE_NAME_ENDINGS = [".yaml", ".yml"].freeze + + # The bytes that count as white space when deciding whether content is empty. + WHITE_SPACE_BYTES = [0x20, 0x09, 0x0a, 0x0d, 0x0b, 0x0c].freeze + private_constant :WHITE_SPACE_BYTES + + # + # Returns the format of a data file, which its name decides: a name that ends in ".yaml" or + # ".yml", in any case, is YAML, and any other name is JSON. + # + # @param path [String] + # @return [Symbol] `:json` or `:yaml` + # + def self.format_of(path) + path.downcase.end_with?(*YAML_FILE_NAME_ENDINGS) ? :yaml : :json + end + + # + # Raises an error if the content is not valid UTF-8. + # + # @param content [String] + # @raise [ArgumentError] if the content is not valid UTF-8 + # + def self.check_encoding(content) + return if content.valid_encoding? + + if content.b.start_with?(*UTF16_AND_UTF32_BOMS) + raise ArgumentError, "the file is encoded in UTF-16 or UTF-32; data files must be UTF-8" + end + raise ArgumentError, "the file is not valid UTF-8" + end + + # + # Returns true if the content is empty or only white space. + # + # @param content [String] + # @return [Boolean] + # + def self.blank?(content) + content.each_byte.all? { |byte| WHITE_SPACE_BYTES.include?(byte) } + end + + # The byte order marks of UTF-8 and of the encodings that files must not use. + UTF8_BOM = "\xEF\xBB\xBF".b.freeze + UTF16_AND_UTF32_BOMS = ["\xFF\xFE".b, "\xFE\xFF".b, "\x00\x00\xFE\xFF".b].freeze + private_constant :UTF8_BOM, :UTF16_AND_UTF32_BOMS + # - # Reads the raw content of one file. + # Reads the raw content of one file, as UTF-8 and without a UTF-8 byte order mark, whatever + # the process's default encoding. The bytes are not checked here: {Document.parse} rejects + # content that is not valid UTF-8, so that such a file fails to load like any other content + # that cannot be parsed. # # @param path [String] # @return [String] # @raise [ReadError] if the file cannot be read # def self.read_file(path) - File.read(path) + content = File.binread(path) + content = content.byteslice(UTF8_BOM.bytesize..) if content.start_with?(UTF8_BOM) + content.force_encoding(Encoding::UTF_8) rescue Errno::ENOENT => e raise ReadError.new(path, "unable to read file: #{e.message}", missing: true) rescue SystemCallError, IOError => e @@ -131,7 +210,7 @@ def self.read_file(path) # @raise [ReadError] if the content cannot be parsed # def self.parse_file(path, content) - Document.parse(content) + Document.parse(content, format: format_of(path)) rescue StandardError => e raise ReadError.new(path, "error parsing file: #{e.message}") end diff --git a/lib/ldclient-rb/impl/file_data/poller.rb b/lib/ldclient-rb/impl/file_data/poller.rb index b0ce08fc..73d9db5e 100644 --- a/lib/ldclient-rb/impl/file_data/poller.rb +++ b/lib/ldclient-rb/impl/file_data/poller.rb @@ -59,14 +59,20 @@ def stop # @return [Array] # def self.observe_all(paths) - paths.map do |path| - begin - stat = File.stat(path) - FileState.new(true, stat.mtime, stat.size) - rescue SystemCallError - ABSENT - end - end + paths.map { |path| observe(path) } + end + + # + # Examines one file. The state is that of the file a symbolic link leads to. + # + # @param path [String] + # @return [FileState] + # + def self.observe(path) + stat = File.stat(path) + FileState.new(true, stat.mtime, stat.size) + rescue SystemCallError + ABSENT end private def examine diff --git a/lib/ldclient-rb/impl/file_data/reloader.rb b/lib/ldclient-rb/impl/file_data/reloader.rb index a4d5c19a..132d0d33 100644 --- a/lib/ldclient-rb/impl/file_data/reloader.rb +++ b/lib/ldclient-rb/impl/file_data/reloader.rb @@ -2,7 +2,7 @@ require "ldclient-rb/impl/file_data/document" require "ldclient-rb/impl/file_data/merge" -require "ldclient-rb/impl/util" +require "ldclient-rb/impl/file_data/poller" require "digest" @@ -12,7 +12,37 @@ module FileData # # Owns the reload cycle for a set of data files. It serializes reloads, debounces change # signals, retains the last good result on failure (by not calling `apply`), retries after - # failures, and skips no-op applications. + # failures, holds back a file that was emptied before applying it, and skips no-op + # applications. + # + # Applications and editors rewrite files in place: they truncate the file, then write the new + # content. A load in between finds the file empty, or, for YAML, can find only a comment or + # "---", which the YAML parser reads as nothing. Whether a file with no entries is valid + # depends on its format (see {Document.parse}), so a parser cannot tell an emptied file from + # one in the middle of a rewrite. So a load that finds a file with no content (empty, white + # space, or YAML that is only a null), while the data currently applied has content from that + # file, holds the file back: it reads the other files, so that files emptied together are + # held back together, then keeps the current data and arms the retry, without reporting a + # failure. A file that cannot be read or parsed still fails the load. A YAML document such as + # `{}` is content, and is applied at once. + # + # A file is held back until it has stayed so, without being written, for the retry delay, + # counted from the load that read it. A change of its state (its modification time and size, + # read through any symbolic link) means it was written since a load last found it so, for + # example rewritten and truncated again between two loads, so the delay starts over. Each file + # is held back for at most five retry delays from the first load that held it back. The bound + # goes on across reads that find the file with content, and starts over only at a load that + # holds nothing back, whether that load applies, is skipped as unchanged, or fails, for each + # file it read. A load reads every file, even after one fails to read or parse, and fails with + # the first failure after them. So a file that keeps being rewritten, or files rewritten in + # turn, cannot hold the data back without end. A read that finds a file with content, even content + # that fails to parse, or finds it missing, ends its empty state, so its next emptying starts + # a new delay. The retry that a hold arms comes when the first held file's + # delay or bound is over, plus a tenth of the retry delay. A file that contributed no content + # to the data currently applied, such as a placeholder that has been empty from the start, a + # file that was missing, or a file whose emptying has been applied, is parsed at once. So is every file when there are no + # automatic retries, because nothing would read it again. After the delay, an empty JSON file + # fails to parse and the data stays; an empty YAML file is a document with no entries. # # The worker thread starts on the first {#reload_now} or {#trigger} call rather than in the # constructor. A reloader can be constructed by a component whose lifecycle never uses it, @@ -30,6 +60,16 @@ class Reloader # file is cheap, so this can be short. In seconds. DEFAULT_RETRY_DELAY = 1.0 + # A file is held back for at most this many retry delays from the first load that held it + # back. The bound starts over at a load that holds nothing back. + MAX_HOLD_RETRY_DELAYS = 5 + + # A file whose content is in effect and that loads have found with no content: since when it + # has been so without being written, or nil once a load read it with content or found it + # missing; its {Poller::FileState} then; and when a load first held it back, or nil. + EmptyFile = Struct.new(:since, :state, :first_held) + private_constant :EmptyFile + # # @param paths [Array] the files to load, already resolved to absolute paths. The # order is significant: it determines which file wins under the duplicate keys handling. @@ -45,24 +85,32 @@ class Reloader # a {MergeError} when the documents could not be combined, or the exception that `apply` # raised. The reloader logs failures itself. # @param duplicate_keys_handling [Symbol] one of the {DuplicateKeysHandling} values - # @param skip_missing_paths [Boolean] when true, a configured file that does not exist is a - # file with no content, and the reload succeeds with the data of the files that exist. + # @param skip_missing_paths [Boolean] when true, a configured file that does not exist + # contributes no data, and the reload succeeds with the data of the files that exist. # When false, a missing file fails the reload like any other read error. # @param debounce_delay [Numeric] seconds to wait after a {#trigger} call for further calls # to settle before reloading. If zero or negative, each trigger reloads at once. # @param retry_delay [Numeric] seconds to wait after a failed reload before retrying - # automatically. If zero or negative, there is no automatic retry. + # automatically. A file whose content is in effect and that is found empty is held back + # for the same delay. If zero or negative, there is no automatic retry, and an empty file + # is parsed as it is. # @param skip_unchanged [Boolean] if true, `apply` is not invoked when the files' raw # contents are byte-identical to the last successfully applied contents. + # @param log_prefix [String] the prefix of every log line this reloader writes + # @param clock [#call, nil] returns the seconds of a monotonic clock, for the time a file has + # been held back; nil for the system's. Scheduling always uses the system's clock. # def initialize(paths:, logger:, apply:, on_error: nil, duplicate_keys_handling: DuplicateKeysHandling::FAIL, skip_missing_paths: false, debounce_delay: DEFAULT_DEBOUNCE_DELAY, retry_delay: DEFAULT_RETRY_DELAY, - skip_unchanged: false) + skip_unchanged: false, + log_prefix: "[LDClient]", + clock: nil) @paths = paths @logger = logger + @log_prefix = log_prefix @apply = apply @on_error = on_error @duplicate_keys_handling = duplicate_keys_handling @@ -70,6 +118,7 @@ def initialize(paths:, logger:, apply:, on_error: nil, @debounce_delay = debounce_delay @retry_delay = retry_delay @skip_unchanged = skip_unchanged + @clock = clock || method(:monotonic_now) # Guards the scheduling state below and wakes the worker. @mutex = Mutex.new @@ -77,7 +126,7 @@ def initialize(paths:, logger:, apply:, on_error: nil, @worker = nil @stopped = false @trigger_pending = false - @retry_requested = false + @requested_retry_delay = nil @debounce_deadline = nil @retry_deadline = nil @@ -85,20 +134,41 @@ def initialize(paths:, logger:, apply:, on_error: nil, @reload_mutex = Mutex.new @last_good_digest = nil @last_error_message = nil + # Whether each configured path contributed content to the data currently applied; the + # EmptyFile of each path whose content is in effect and that loads have found with no + # content since; whether the last load held a file back; and whether the load in progress + # holds one back. Only touched while holding @reload_mutex. + @applied_had_content = {} + @empty_files = {} + @read_again_pending = false + @holding_this_load = false + # The delay of the last retry that was armed. Only tests read it. Guarded by @mutex. + @last_armed_retry_delay = nil end # # Synchronously loads the files and applies the result, or reports the failure. Use it for # the initial load. A failure here schedules the same automatic retry as a failed - # triggered reload. + # triggered reload, and a load that holds back a file it finds empty arms the retry for when + # the file may be read as it is. # - # @return [Boolean] true if the load succeeded + # @return [Boolean] true if the load succeeded, false if it failed or held back an empty file # def reload_now ensure_started - ok = reload(retrying: false) - request_retry unless ok - ok + retry_in = reload(retrying: false) + request_retry(retry_in) unless retry_in.nil? + retry_in.nil? + end + + # + # The delay in seconds of the last retry that was armed, from the time it was armed. Only + # tests use it. + # + # @return [Numeric, nil] + # + def last_armed_retry_delay + @mutex.synchronize { @last_armed_retry_delay } end # @@ -137,11 +207,11 @@ def stop end end - private def request_retry + private def request_retry(delay) return unless @retry_delay > 0 @mutex.synchronize do - @retry_requested = true + @requested_retry_delay = [@requested_retry_delay, delay].compact.min @cond.signal end end @@ -151,20 +221,29 @@ def stop action = next_action break if action == :stop - if action == :reload - @logger.info { "[LDClient] Reloading flag data after detecting a change" } - else - @logger.debug { "[LDClient] Retrying flag data load after earlier failure" } - end - ok = reload(retrying: action == :retry) - @mutex.synchronize do - # A pending retry is superseded by this reload. The reload either succeeded, or it - # failed and arms a fresh retry here. - @retry_deadline = ok || @retry_delay <= 0 ? nil : monotonic_now + @retry_delay + # An unexpected error must not end the worker: no reload would happen after it. + begin + @logger.info { "#{@log_prefix} Reloading flag data after detecting a change" } if action == :reload + retry_in = reload(retrying: action == :retry) + @mutex.synchronize do + # A pending retry is superseded by this reload. The reload either succeeded, or it + # failed or held back a file and arms a fresh retry here. + if retry_in.nil? || @retry_delay <= 0 + @retry_deadline = nil + else + @retry_deadline = monotonic_now + retry_in + @last_armed_retry_delay = retry_in + end + end + rescue => e + log_unexpected_error(e) end end - rescue => e - Util.log_exception(@logger, "Unexpected error in file data reloader", e) + end + + private def log_unexpected_error(error) + @logger.error { "#{@log_prefix} Unexpected error in file data reloader: #{error.inspect}" } + @logger.debug { "#{@log_prefix} Exception trace: #{error.backtrace}" } end # @@ -183,11 +262,13 @@ def stop @debounce_deadline = now + @debounce_delay end - if @retry_requested - # A synchronous reload_now failed. Arm the retry without reloading again at once. - # An already armed retry keeps its earlier deadline. - @retry_requested = false - @retry_deadline ||= now + @retry_delay + unless @requested_retry_delay.nil? + # A synchronous reload_now failed or held back a file. Arm the retry without + # reloading again at once. An already armed retry that comes sooner is kept. + deadline = now + @requested_retry_delay + @requested_retry_delay = nil + @retry_deadline = [@retry_deadline, deadline].compact.min + @last_armed_retry_delay = @retry_deadline - now end if !@debounce_deadline.nil? && now >= @debounce_deadline @debounce_deadline = nil @@ -206,103 +287,269 @@ def stop end # - # Performs one full load of all configured files and returns whether it succeeded. That - # decides whether a retry is armed, so a skipped no-op application counts as success. The - # whole set is re-read on every reload: entries are combined across files in order, so a - # change to one file can alter which file wins for a key. + # Performs one full load of all configured files. Returns nil if it succeeded, or the + # seconds after which to retry if it failed or held back a file. A skipped no-op application + # counts as success. The whole set is re-read on every reload: entries are combined across + # files in order, so a change to one file can alter which file wins for a key. # private def reload(retrying: false) @reload_mutex.synchronize do # A trigger already queued when stop was called can still reach here. - return true if stopped? + return nil if stopped? - documents = [] - files = [] - digest = Digest::SHA256.new - @paths.each do |path| + if retrying + if @read_again_pending + @logger.debug { "#{@log_prefix} Reading flag data again after a file was found empty" } + else + @logger.debug { "#{@log_prefix} Retrying flag data load after earlier failure" } + end + end + @read_again_pending = false + @holding_this_load = false + # The files that the load read and did not hold back, including those whose read or parse + # failed. A load reads every file unless an error it does not expect stops it. + reached = [] + retry_in = begin - # One read feeds both the digest and the parse, so the skip-unchanged digest can - # never disagree with the content that was applied. - content = FileData.read_file(path) - rescue ReadError => e - if e.missing && @skip_missing_paths - @logger.debug { "[LDClient] File #{path} does not exist; it contributes no data" } - files << FileSummary.new(path, false, 0, 0) - next - end - return record_failure(e) + load_files(reached) + rescue => e + # A failure that the load does not expect is retried like any other. + log_unexpected_error(e) + stopped? ? nil : @retry_delay + end + unless @holding_this_load + # A load that holds nothing back, whether it applies, is skipped or fails, starts the + # bound over for every file that it read: such a file is no longer held back, so its + # next rewrite is a new one. + reached.each do |path| + empty = @empty_files[path] + empty.first_held = nil unless empty.nil? end - digest << content << "\0" + end + retry_in + end + end + + # The body of {#reload}, while holding @reload_mutex. + private def load_files(reached) + documents = [] + files = [] + had_content = {} + held = [] + # The first read or parse failure. The other files are still read, so that each of them is + # classified, and the load fails with it after them. + failure = nil + digest = Digest::SHA256.new + @paths.each do |path| + begin + # One read feeds both the digest and the parse, so the skip-unchanged digest can never + # disagree with the content that was applied. + content = FileData.read_file(path) + rescue ReadError => e + if e.missing + not_empty(path, reached) + else + reached << path + end + if e.missing && @skip_missing_paths + @logger.debug { "#{@log_prefix} File #{path} does not exist; it contributes no data" } + had_content[path] = false + append_to_digest(digest, path, nil) + files << FileSummary.new(path, false, 0, 0) + next + end + failure ||= e + next + end + # A file with no content, or YAML that the parser reads as nothing, such as only a + # comment, has no entries. A read can see that much of a file that is being written. + # Content with no JSON document is only parsed when the file is not held back, and then + # fails to parse. + document = nil + has_content = !FileData.blank?(content) + if has_content begin - documents << FileData.parse_file(path, content) + document = FileData.parse_file(path, content) rescue ReadError => e - return record_failure(e) + # Content that fails to parse is content: the file's empty state is over. + not_empty(path, reached) + failure ||= e + next end - files << FileSummary.new(path, true, 0, 0) + has_content = !document.equal?(Document::NONE) end + if has_content + not_empty(path, reached) + elsif hold_back?(path, @clock.call) + # The clock is read after this file's own read, so a slow read of another file does + # not shorten its delay. + # The other files are still read, so that files emptied together start their delays + # in the same load. + @holding_this_load = true + held << path + next + else + reached << path + end + had_content[path] = has_content - begin - merged = FileData.merge(documents, - duplicate_keys_handling: @duplicate_keys_handling, - logger: @logger) - rescue => e - return record_failure(e) + append_to_digest(digest, path, content) + if document.nil? + begin + document = FileData.parse_file(path, content) + rescue ReadError => e + failure ||= e + next + end end + documents << document + files << FileSummary.new(path, true, 0, 0) + end + return record_failure(failure) unless failure.nil? + return hold_back(held, @clock.call) unless held.empty? + + begin + merged = FileData.merge(documents, + duplicate_keys_handling: @duplicate_keys_handling, + logger: @logger) + rescue => e + return record_failure(e) + end - # Documents are the present files in order. Copy their counts onto the file summaries. - document_index = 0 - files.each do |file| - next unless file.present + # Documents are the present files in order. Copy their counts onto the file summaries. + document_index = 0 + files.each do |file| + next unless file.present - summary = merged.documents[document_index] - file.flags = summary.flags - file.segments = summary.segments - document_index += 1 - end - merged.files = files + summary = merged.documents[document_index] + file.flags = summary.flags + file.segments = summary.segments + document_index += 1 + end + merged.files = files + + # stop may have been called while the files were being read. Deliver nothing then. + return nil if stopped? + + # A success right after a failure must apply even when the content is unchanged since the + # last success. The consumer heard about the failure through on_error and may have moved + # to an interrupted state. Only apply tells it that things are good again. + recovering = !@last_error_message.nil? + hexdigest = digest.hexdigest + if @skip_unchanged && !recovering && hexdigest == @last_good_digest + remember_applied(had_content) + return nil + end + + # Nothing is remembered until the consumer has accepted the result. A result that apply + # rejects must not become the baseline that skip-unchanged compares against, and must not + # count as a recovery. + begin + @apply.call(merged) + rescue => e + return record_failure(e) + end + @last_error_message = nil + @last_good_digest = hexdigest + remember_applied(had_content) + nil + end - # stop may have been called while the files were being read. Deliver nothing then. - return true if stopped? + # Adds a file to the skip-unchanged digest: its path, whether it is present, and its content. + # The same content in another file can parse differently, since the format comes from the + # name, and does not come from the same file. + private def append_to_digest(digest, path, content) + digest << path.b << "\0" + if content.nil? + digest << "-\0" + else + digest << "+" << content.bytesize.to_s << "\0" << content.b + end + end - # A success right after a failure must apply even when the content is unchanged since - # the last success. The consumer heard about the failure through on_error and may - # have moved to an interrupted state. Only apply tells it that things are good again. - recovering = !@last_error_message.nil? - hexdigest = digest.hexdigest - return true if @skip_unchanged && !recovering && hexdigest == @last_good_digest + # Records that a load found the file with content, even content that fails to parse, or + # missing: its empty state is over, so its next emptying starts a new delay even where a + # coarse modification time makes it look the same. Its bound goes on, so that files + # rewritten in turn stay bounded. + private def not_empty(path, reached) + reached << path + empty = @empty_files[path] + empty.since = nil unless empty.nil? + end - # Nothing is remembered until the consumer has accepted the result. A result that - # apply rejects must not become the baseline that skip-unchanged compares against, - # and must not count as a recovery. - begin - @apply.call(merged) - rescue => e - return record_failure(e) + # Records what each file contributed to the data now in effect. The holds of the files start + # over. + private def remember_applied(had_content) + @applied_had_content = had_content + @empty_files.clear + end + + # + # Decides whether a load that found a file with no content holds it back, because it may be + # in the middle of an in-place rewrite, and records since when it has been so. `now` is read + # after the file's own read. + # + private def hold_back?(path, now) + return false if @retry_delay <= 0 || !@applied_had_content[path] + + state = Poller.observe(path) + empty = (@empty_files[path] ||= EmptyFile.new(nil, nil, nil)) + if empty.since.nil? || empty.state != state + # Found empty for the first time since it had content, or written since a load last + # found it so: a new rewrite. + empty.since = now + empty.state = state + end + # After the delay the file is parsed. The entry stays, so later loads keep parsing it until + # it has content. Its bound starts over only at a load that holds nothing back. + return false if now - empty.since >= @retry_delay + return false if !empty.first_held.nil? && now - empty.first_held >= MAX_HOLD_RETRY_DELAYS * @retry_delay + + empty.first_held ||= now + true + end + + # + # Holds back a load that found files with no content. The current data stays and no failure + # is reported. Returns when to retry: when the delay or the bound of the first of the files + # is over, plus a tenth of the retry delay, so that a retry that comes a little early still + # finds it over. + # + private def hold_back(paths, now) + # stop may have been called while the files were being read. Arm no retry. + return nil if stopped? + + @read_again_pending = true + retry_at = nil + paths.each do |path| + @logger.debug do + "#{@log_prefix} File #{path} is empty, possibly while it is being rewritten; reading it again before applying it" end - @last_error_message = nil - @last_good_digest = hexdigest - true + empty = @empty_files[path] + over = [empty.since + @retry_delay, empty.first_held + MAX_HOLD_RETRY_DELAYS * @retry_delay].min + retry_at = over if retry_at.nil? || over < retry_at end + [retry_at - now, 0].max + @retry_delay / 10.0 end private def record_failure(error) # stop may have been called while the files were being read. Deliver nothing then, and - # report success so that no retry is armed. - return true if stopped? + # arm no retry. + return nil if stopped? # With automatic retries, a persistent failure would repeat the same log entry and the # same callback on every attempt. Repeats of an identical failure are logged at debug # level and do not re-invoke on_error. message = error.message if message == @last_error_message - @logger.debug { "[LDClient] Unable to load flags: #{message}" } - return false + @logger.debug { "#{@log_prefix} Unable to load flags: #{message}" } + return @retry_delay end @last_error_message = message - @logger.error { "[LDClient] Unable to load flags: #{message}" } + @logger.error { "#{@log_prefix} Unable to load flags: #{message}" } @on_error&.call(error) - false + @retry_delay end private def stopped? diff --git a/lib/ldclient-rb/impl/integrations/file_override_source.rb b/lib/ldclient-rb/impl/integrations/file_override_source.rb new file mode 100644 index 00000000..6320d619 --- /dev/null +++ b/lib/ldclient-rb/impl/integrations/file_override_source.rb @@ -0,0 +1,151 @@ +# frozen_string_literal: true + +require "ldclient-rb/impl/file_data" +require "ldclient-rb/interfaces/overrides" + +module LaunchDarkly + module Impl + module Integrations + # + # The file-based override source. It reads flag and segment overrides from one or more local + # files and reloads them as the files change. See + # {LaunchDarkly::Integrations::FileData.override_source} for the public API and its options. + # + # Flag overrides are currently experimental and subject to change. + # + # @private + # + class FileOverrideSource + include LaunchDarkly::Interfaces::Overrides::OverrideSource + + LOG_PREFIX = "[LDClient] FileOverrideSource:" + + # + # @param paths [Array] absolute file paths, in precedence order + # @param duplicate_keys_handling [Symbol] `:fail` or `:ignore` + # @param change_detection [Symbol] `:polling` or `:watching` + # @param poll_interval [Numeric] seconds between examinations in polling mode + # @param logger [Logger] + # + def initialize(paths:, duplicate_keys_handling:, change_detection:, poll_interval:, logger:) + @paths = paths + @duplicate_keys_handling = duplicate_keys_handling + @change_detection = change_detection + @poll_interval = poll_interval + @logger = logger + @lock = Mutex.new + @reloader = nil + @change_detector = nil + @stopped = false + end + + # @return [Array] + attr_reader :paths + + # @return [Symbol] + attr_reader :duplicate_keys_handling + + # @return [Symbol] + attr_reader :change_detection + + # @return [Numeric] + attr_reader :poll_interval + + # + # Performs the initial load synchronously, so overrides present in the files are in effect + # when this method returns, then starts change detection. A file that does not exist yet + # contributes no overrides. A file that cannot be read or parsed is not fatal: the client + # runs with the last good overrides, the failure is logged, and the retry plus the change + # signal recover once the file is readable. + # + # (see LaunchDarkly::Interfaces::Overrides::OverrideSource#start) + # + def start(sink) + @lock.synchronize do + return if @stopped + + @reloader = FileData::Reloader.new( + paths: @paths, + logger: @logger, + log_prefix: LOG_PREFIX, + duplicate_keys_handling: @duplicate_keys_handling, + skip_missing_paths: true, + skip_unchanged: true, + apply: lambda do |merged| + sink.set_overrides(merged.flags.values, merged.segments.values) + log_overrides_in_effect(merged) + end + ) + end + + @reloader.reload_now + + @lock.synchronize do + return if @stopped + + trigger = @reloader.method(:trigger) + @change_detector = + if @change_detection == :watching + FileData::Watcher.new(@paths, trigger, @logger) + else + FileData::Poller.new(@paths, @poll_interval, trigger, @logger) + end + end + end + + # (see LaunchDarkly::Interfaces::Overrides::OverrideSource#stop) + def stop + reloader = nil + change_detector = nil + @lock.synchronize do + @stopped = true + reloader = @reloader + change_detector = @change_detector + @reloader = nil + @change_detector = nil + end + change_detector&.stop + reloader&.stop + end + + # + # Reports the overrides now in effect and the file each came from. The reloader applies a + # snapshot only when the content changed, so this logs each change once. + # + # @param merged [LaunchDarkly::Impl::FileData::MergeResult] + # + private def log_overrides_in_effect(merged) + details = merged.files.map do |file| + if !file.present + "#{file.path}: absent" + elsif file.flags.zero? && file.segments.zero? + "#{file.path}: no entries" + else + "#{file.path}: #{counts_text(file.flags, file.segments)}" + end + end.join("; ") + + if merged.empty? + @logger.info { "#{LOG_PREFIX} Flag overrides: none in effect (#{details})" } + else + @logger.info do + "#{LOG_PREFIX} Flag overrides in effect: #{counts_text(merged.flags.length, merged.segments.length)} (#{details})" + end + end + end + + # Formats flag and segment counts, for example "2 flags, 1 segment". + private def counts_text(flags, segments) + parts = [] + parts << pluralize(flags, "flag") if flags > 0 + parts << pluralize(segments, "segment") if segments > 0 + parts.join(", ") + end + + private def pluralize(count, noun) + count == 1 ? "1 #{noun}" : "#{count} #{noun}s" + end + end + end + end +end 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/lib/ldclient-rb/impl/overrides.rb b/lib/ldclient-rb/impl/overrides.rb new file mode 100644 index 00000000..d9a59941 --- /dev/null +++ b/lib/ldclient-rb/impl/overrides.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +require "ldclient-rb/impl/overrides/layer" +require "ldclient-rb/impl/overrides/overlay" +require "ldclient-rb/impl/overrides/sink" diff --git a/lib/ldclient-rb/impl/overrides/layer.rb b/lib/ldclient-rb/impl/overrides/layer.rb new file mode 100644 index 00000000..8604b1bd --- /dev/null +++ b/lib/ldclient-rb/impl/overrides/layer.rb @@ -0,0 +1,133 @@ +# frozen_string_literal: true + +require "ldclient-rb/impl/data_store" +require "ldclient-rb/impl/dependency_tracker" + +require "concurrent/atomics" + +module LaunchDarkly + module Impl + # + # The flag and segment override layer. The layer is a runtime-mutable collection of flag and + # segment definitions, supplied by an override source. Those definitions take precedence over + # LaunchDarkly data at evaluation time. + # + # @private + # + module Overrides + # + # A thread-safe store of override entries, replaced as a whole on each update from an + # override source. Reads are lock-free: the contents are an immutable hash held in an + # atomic reference and swapped on update, so the layer holds exactly one snapshot at any + # instant. + # + # Each snapshot also carries the dependency graph of its entries. The store consults it so + # that a LaunchDarkly change to a prerequisite or a segment also notifies the override + # entries that depend on it. + # + class Layer + EMPTY_CONTENTS = { + DataStore::FEATURES => {}.freeze, + DataStore::SEGMENTS => {}.freeze, + }.freeze + private_constant :EMPTY_CONTENTS + + # The layer contents keyed by data kind, with the dependency graph of those entries. + Snapshot = Struct.new(:contents, :dependencies) + private_constant :Snapshot + + def initialize + @snapshot = Concurrent::AtomicReference.new(Snapshot.new(EMPTY_CONTENTS, DependencyTracker.new).freeze) + end + + # + # Atomically replaces the entire layer contents. Empty hashes clear the layer. + # + # Each flag or segment is stored as a marked shallow copy. The copy shares its data with the + # caller's object, and the layer never writes to it. The caller's object is never marked. + # + # @param flags [Hash{Symbol => LaunchDarkly::Impl::Model::FeatureFlag}] + # @param segments [Hash{Symbol => LaunchDarkly::Impl::Model::Segment}] + # @return [Array(Hash, Hash)] the previous and the new contents, keyed by data kind. The + # returned hashes must not be modified. + # + def set_all(flags, segments) + replacement = { + DataStore::FEATURES => flags.transform_values(&:as_override).freeze, + DataStore::SEGMENTS => segments.transform_values(&:as_override).freeze, + }.freeze + dependencies = DependencyTracker.new + replacement.each do |kind, items| + items.each do |key, item| + # The graph keys items by string, the form in which the data model names + # prerequisites and segments. + dependencies.update_dependencies_from(kind, key.to_s, item) + rescue StandardError + # An entry whose dependency fields cannot be read contributes no edges. It is still + # served, and the remaining entries are still tracked. + end + end + previous = @snapshot.get_and_set(Snapshot.new(replacement, dependencies).freeze) + [previous.contents, replacement] + end + + # + # Adds the given item and every override entry that directly or indirectly depends on it + # to the set. The item itself need not be an override entry: a LaunchDarkly flag or + # segment that an override entry references is a valid starting point. Items are + # `{ kind:, key: }` hashes. The key given may be a string or a symbol; the items added + # carry string keys. + # + # @param items_out [Set] + # @param item [Hash] `{ kind:, key: }` + # @return [void] + # + def add_affected_items(items_out, item) + @snapshot.get.dependencies.add_affected_items(items_out, { kind: item[:kind], key: item[:key].to_s }) + end + + # + # Returns the override entry for a key, or nil. + # + # @param kind [LaunchDarkly::Impl::DataStore::DataKind] + # @param key [String, Symbol] + # @return [Object, nil] + # + def get(kind, key) + items = @snapshot.get.contents[kind] + return nil if items.nil? + + items[key.to_sym] + end + + # + # Returns the entries of the given kind. The returned hash must not be modified. + # + # @param kind [LaunchDarkly::Impl::DataStore::DataKind] + # @return [Hash{Symbol => Object}] + # + def all(kind) + @snapshot.get.contents[kind] || {} + end + + # + # Returns the current contents, keyed by data kind. The returned hash must not be modified. + # + # @return [Hash] + # + def contents + @snapshot.get.contents + end + + # + # Returns true if the layer contains no entries. + # + # @return [Boolean] + # + def empty? + @snapshot.get.contents.each_value.all?(&:empty?) + end + end + end + end +end diff --git a/lib/ldclient-rb/impl/overrides/overlay.rb b/lib/ldclient-rb/impl/overrides/overlay.rb new file mode 100644 index 00000000..4f012704 --- /dev/null +++ b/lib/ldclient-rb/impl/overrides/overlay.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +require "ldclient-rb/impl/overrides/layer" +require "ldclient-rb/interfaces/data_system" + +module LaunchDarkly + module Impl + module Overrides + # + # Merges an override {Layer} over a base store. A read for a key returns the override entry + # when one exists, and the base entry otherwise. The overlay sits at the store read boundary. + # That placement makes targeting rules, prerequisites, and segment matches behave identically + # for overridden and ordinary data. They are the same reads through the same boundary. + # + # @private + # + class Overlay + include LaunchDarkly::Interfaces::DataSystem::ReadOnlyStore + + # + # @param base [#get, #all, #initialized?] the store that holds LaunchDarkly data + # @param layer [Layer] + # + def initialize(base, layer) + @base = base + @layer = layer + end + + # + # Returns the override entry for the key if one exists, and otherwise delegates to the base + # store. This works even when the base store is uninitialized, because an uninitialized + # base reports not-found rather than failing. + # + # (see LaunchDarkly::Interfaces::DataSystem::ReadOnlyStore#get) + # + def get(kind, key) + item = @layer.get(kind, key) + return item unless item.nil? + + @base.get(kind, key) + end + + # + # Returns the union of the base store's items and the layer's items. The override entry + # wins for any key present in both. This includes keys the base holds only as deleted + # items. A failure of the base store is raised, as it is without the overlay: the layer's + # entries alone would be an incomplete result. + # + # (see LaunchDarkly::Interfaces::DataSystem::ReadOnlyStore#all) + # + def all(kind) + base_items = @base.all(kind) + overrides = @layer.all(kind) + return base_items if overrides.empty? + + base_items.merge(overrides) + end + + # + # Delegates to the base store: the override layer never affects initialization status or + # data availability. + # + # (see LaunchDarkly::Interfaces::DataSystem::ReadOnlyStore#initialized?) + # + def initialized? + @base.initialized? + end + end + end + end +end diff --git a/lib/ldclient-rb/impl/overrides/sink.rb b/lib/ldclient-rb/impl/overrides/sink.rb new file mode 100644 index 00000000..9c94dd7c --- /dev/null +++ b/lib/ldclient-rb/impl/overrides/sink.rb @@ -0,0 +1,220 @@ +# frozen_string_literal: true + +require "ldclient-rb/impl/data_store" +require "ldclient-rb/impl/dependency_tracker" +require "ldclient-rb/impl/model/serialization" +require "ldclient-rb/impl/overrides/layer" +require "ldclient-rb/interfaces/flag_tracker" +require "ldclient-rb/interfaces/overrides" + +require "set" + +module LaunchDarkly + module Impl + module Overrides + # + # Applies override layer replacements supplied by an override source, and notifies flag + # change listeners of the flags affected by each replacement. + # + # @private + # + class Sink + include LaunchDarkly::Interfaces::Overrides::OverrideSink + + KINDS = [DataStore::FEATURES, DataStore::SEGMENTS].freeze + private_constant :KINDS + + # + # @param layer [Layer] the layer to write to + # @param base [#all] the store holding LaunchDarkly data, without the overlay. Merged-view + # snapshots for change computation are built from it plus the layer. + # @param flag_change_broadcaster [LaunchDarkly::Impl::Broadcaster] + # @param logger [Logger] + # + def initialize(layer, base, flag_change_broadcaster, logger) + @layer = layer + @base = base + @flag_change_broadcaster = flag_change_broadcaster + @logger = logger + @lock = Mutex.new + @closed = false + end + + # + # Atomically replaces the entire override layer, then notifies listeners of every flag + # whose merged-view evaluation may have changed. Calls are serialized, so overlapping + # updates from a source cannot interleave. + # + # (see LaunchDarkly::Interfaces::Overrides::OverrideSink#set_overrides) + # + def set_overrides(flags, segments) + return log_ignored_snapshot if @closed + + flag_items = index_items(DataStore::FEATURES, flags) + segment_items = index_items(DataStore::SEGMENTS, segments) + + @lock.synchronize do + return log_ignored_snapshot if @closed + + apply(flag_items, segment_items) + end + end + + # + # Stops accepting snapshots. Used when the client closes: a snapshot that the source + # supplies while it stops is ignored. The layer keeps its contents. + # + # @return [void] + # + def close + @lock.synchronize do + @closed = true + end + end + + # + # Stops accepting snapshots and clears the layer. Used when the source failed to start: a + # snapshot the source supplied before failing is removed, so the client runs without + # overrides, and a late snapshot from the stopped source is ignored. + # + # @return [void] + # + def close_and_clear + @lock.synchronize do + @closed = true + apply({}, {}) + end + end + + # + # Returns the keys of all flags whose merged-view evaluation may have changed when the + # override layer was replaced. The result includes the flags whose override entries were + # added, removed, or changed. Dependency fan-out adds every flag that depends, directly or + # transitively, on any added, removed, or changed entry of either kind. + # + # Dependency edges are computed over both the old and the new merged views, because a + # replacement can rewire dependencies. For example, removing a flag override restores the + # prerequisite edges of the LaunchDarkly definition. + # + # @param previous [Hash] layer contents before the replacement + # @param current [Hash] layer contents after the replacement + # @param old_merged [Hash] merged view before the replacement + # @param new_merged [Hash] merged view after the replacement + # @return [Array] + # + def self.affected_flag_keys(previous, current, old_merged, new_merged) + seeds = changed_entries(previous, current) + return [] if seeds.empty? + + old_tracker = tracker_for(old_merged) + new_tracker = tracker_for(new_merged) + affected = Set.new + seeds.each do |seed| + old_tracker.add_affected_items(affected, seed) + new_tracker.add_affected_items(affected, seed) + end + + affected.select { |item| item[:kind] == DataStore::FEATURES }.map { |item| item[:key] } + end + + # + # Returns an item reference for each key whose override entry differs between the two + # layer snapshots. An added or removed entry is always a change, even when its content + # matches the underlying LaunchDarkly data. The override marker alone changes the served + # entry. Entries present in both snapshots are compared by their data. + # + private_class_method def self.changed_entries(previous, current) + seeds = [] + KINDS.each do |kind| + old_items = previous[kind] || {} + new_items = current[kind] || {} + old_items.each do |key, old_item| + new_item = new_items[key] + seeds << { kind: kind, key: key.to_s } if new_item.nil? || old_item != new_item || old_item.version != new_item.version + end + new_items.each_key do |key| + seeds << { kind: kind, key: key.to_s } unless old_items.key?(key) + end + end + seeds + end + + # + # Builds a dependency tracker over a merged view. The tracker keys items by string, the + # same form in which the data model names prerequisites and segments. + # + private_class_method def self.tracker_for(view) + tracker = DependencyTracker.new + KINDS.each do |kind| + view[kind].each do |key, item| + tracker.update_dependencies_from(kind, key.to_s, item) + end + end + tracker + end + + # + # Replaces the layer and notifies listeners of the affected flags. Called inside @lock. + # + private def apply(flag_items, segment_items) + # Computing affected flags requires snapshots of the merged view before and after the + # replacement. Skip all of that work when nothing is listening. + unless @flag_change_broadcaster.has_listeners? + @layer.set_all(flag_items, segment_items) + return + end + + previous, current = @layer.set_all(flag_items, segment_items) + old_merged = merged_view(previous) + new_merged = merged_view(current) + + affected = Sink.affected_flag_keys(previous, current, old_merged, new_merged) + @logger.debug { "[LDClient] Override update affected #{affected.length} flag(s)" } unless affected.empty? + affected.each do |key| + @flag_change_broadcaster.broadcast(LaunchDarkly::Interfaces::FlagChange.new(key)) + end + end + + private def log_ignored_snapshot + @logger.debug { "[LDClient] Ignoring an override snapshot supplied after the override source was closed" } + end + + # + # Converts the definitions passed to the sink into a hash keyed by symbol. A hash entry is + # deserialized into the data model. A model object is used as is. + # + private def index_items(kind, items) + result = {} + (items || []).each do |item| + model = Model.deserialize(kind, item, @logger) + key = model.respond_to?(:key) ? model.key : nil + raise ArgumentError, "an override #{kind.namespace} entry has no key" if key.nil? || key.to_s.empty? + + result[key.to_sym] = model + end + result + end + + # + # Captures the merged view of the base store and a layer snapshot: base data with override + # entries overlaid. A base read failure for a kind yields just the overrides for that kind. + # This degrades the dependency fan-out but never loses the directly changed keys. + # + private def merged_view(layer_contents) + view = {} + KINDS.each do |kind| + items = {} + begin + @base.all(kind).each { |key, item| items[key.to_sym] = item } + rescue => e + @logger.warn { "[LDClient] Unable to read #{kind.namespace} for override change detection: #{e.message}" } + end + (layer_contents[kind] || {}).each { |key, item| items[key.to_sym] = item } + view[kind] = items + end + view + end + end + end + end +end diff --git a/lib/ldclient-rb/integrations/file_data.rb b/lib/ldclient-rb/integrations/file_data.rb index 1f4662e1..1064c8b8 100644 --- a/lib/ldclient-rb/integrations/file_data.rb +++ b/lib/ldclient-rb/integrations/file_data.rb @@ -1,5 +1,7 @@ +require 'ldclient-rb/impl/file_data' require 'ldclient-rb/impl/integrations/file_data_source' require 'ldclient-rb/impl/integrations/file_data_source_v2' +require 'ldclient-rb/impl/integrations/file_override_source' module LaunchDarkly module Integrations @@ -81,6 +83,11 @@ module Integrations # If the data source encounters any error in any file-- malformed content, a missing file, or a # duplicate key-- it will not load flags from any of the files. # + # The same file format serves the file-based override source, {FileData.override_source}, which + # supplies flag and segment overrides that take precedence over LaunchDarkly data instead of + # replacing the connection to LaunchDarkly. Flag overrides are currently experimental and subject + # to change. + # module FileData # # Returns a factory for the file data source component. @@ -149,6 +156,170 @@ def self.data_source_v2(options = {}) FileDataSourceV2Builder.new(paths, poll_interval) end + + # + # Returns a builder for a file-based override source. Flag overrides are currently experimental + # and subject to change. + # + # Overrides are flag and segment definitions that take precedence over data received from + # LaunchDarkly at evaluation time, on a per-key basis. They exist for resilience during an + # incident. An operator can force one or more flags to a known state on a running application, + # whether or not the application can reach LaunchDarkly, by writing them to a file. The override + # stays in effect until it is removed from the file. Flags not present in the files are + # completely unaffected. + # + # The files use the same document format as the file data source: a JSON or YAML document with + # optional `flags`, `flagValues`, and `segments` members. A `flagValues` entry expands into a + # flag that is on and serves the given value as its fallthrough for every context. Such a flag is + # not available to client-side SDKs, so `all_flags_state` with `client_side_only: true` leaves it + # out, even when the flag it overrides is available to them. To override such a flag, define it + # in `flags` with `"clientSide": true`. When several files are configured, their + # entries are combined in the configured order, and the duplicate keys handling decides what + # happens when the same key appears in more than one file. + # + # A file whose name ends in `.yaml` or `.yml`, in any case, is read as YAML, and any other file is + # read as JSON. Files must be UTF-8. Whether a JSON file may contain comments or repeated keys + # depends on the application's version of the `json` gem. + # + # A reload replaces the entire set of overrides, so removing an entry from a file removes the + # override. A configured file that does not exist contributes no overrides. It can be created + # later, and deleting a file removes its overrides. A file that exists but cannot be read or + # parsed fails that whole reload. The previously loaded overrides stay in effect, the failure is + # logged, and the source retries after a short delay and recovers on its own once the file is + # readable. Whenever the set of overrides in effect changes, including at startup, the source + # logs the overrides in effect and what each file supplied, at Info level. + # + # Files can be rewritten in place: if the source finds a file empty while it is being rewritten, + # it keeps the overrides in effect until the new content is written. To remove a file's + # overrides, delete the file or write a document with no entries, such as `{}`. An empty JSON + # file is not a valid document. For a YAML file, replacing its contents in one operation, such + # as by renaming a fully written temporary file over it, avoids briefly applying a partly + # written file. + # + # An evaluation that an override affects, directly or through a prerequisite or segment, is + # marked: its reason reports {EvaluationReason#override_affected}, it produces no individual + # analytics event, and it is counted under a separate summary counter. + # + # Pass the returned builder to {LaunchDarkly::DataSystem::ConfigBuilder#overrides}. The + # configuration is validated when the client is created, and an invalid configuration raises + # `ArgumentError` from `LDClient.new`. + # + # @example + # overrides = LaunchDarkly::Integrations::FileData.override_source(paths: ["/etc/launchdarkly/overrides.json"]) + # config = LaunchDarkly::Config.new(data_system: LaunchDarkly::DataSystem.default.overrides(overrides)) + # client = LaunchDarkly::LDClient.new(sdk_key, config) + # + # @param options [Hash] the configuration options + # @option options [Array, String] :paths One or more files, in precedence order. Required. + # Each path must be a non-empty String, absolute or relative to the current working directory. + # @option options [Symbol] :duplicate_keys_handling What to do when the same key appears in more + # than one file. `:fail` (the default) treats the reload as failed and keeps the previous + # overrides. `:ignore` keeps the entry from the first configured file that defines the key. + # @option options [Symbol] :change_detection How the source detects file changes. `:polling` (the + # default) examines the files on an interval and works on every file system, including network + # mounts and directories whose contents are swapped through symbolic links. `:watching` uses file + # system change notifications through the `listen` gem, which the host application must provide. + # @option options [Numeric] :poll_interval Seconds between examinations of the files in polling + # mode. The default is 1. An interval below 1 is raised to 1 with a warning. + # @return [FileOverrideSourceBuilder] a builder for {LaunchDarkly::DataSystem::ConfigBuilder#overrides} + # + def self.override_source(options = {}) + FileOverrideSourceBuilder.new(options) + end + end + + # + # Builder for the file-based override source. Create it with {FileData.override_source}. + # + # Flag overrides are currently experimental and subject to change. + # + class FileOverrideSourceBuilder + # The default interval, in seconds, between examinations of the files in polling mode. + DEFAULT_POLL_INTERVAL = 1 + + # The shortest allowed polling interval, in seconds. A configured interval below this is raised + # to it. The minimum exists only to prevent a tight loop over the file system. + MINIMUM_POLL_INTERVAL = 1 + + DUPLICATE_KEYS_HANDLING_VALUES = [:fail, :ignore].freeze + CHANGE_DETECTION_VALUES = [:polling, :watching].freeze + OPTION_KEYS = [:paths, :duplicate_keys_handling, :change_detection, :poll_interval].freeze + private_constant :DUPLICATE_KEYS_HANDLING_VALUES, :CHANGE_DETECTION_VALUES, :OPTION_KEYS + + # + # @param options [Hash] see {FileData.override_source} + # + def initialize(options) + raise ArgumentError, "options for the file-based override source must be a Hash" unless options.is_a?(Hash) + + @options = options + end + + # + # Builds the override source. Called by the SDK when the client is created. + # + # @param sdk_key [String] unused + # @param config [LaunchDarkly::Config] the client configuration, for its logger + # @return [LaunchDarkly::Interfaces::Overrides::OverrideSource] + # @raise [ArgumentError] if the options are invalid + # + def build(sdk_key, config) + unknown = @options.keys - OPTION_KEYS + raise ArgumentError, "unknown options for the file-based override source: #{unknown.join(', ')}" unless unknown.empty? + + paths = Array(@options[:paths]) + raise ArgumentError, "no file paths were specified for the file-based override source" if paths.empty? + + invalid = paths.reject { |path| path.is_a?(String) && !path.empty? } + unless invalid.empty? + raise ArgumentError, + "file paths for the file-based override source must be non-empty strings; got #{invalid.map(&:inspect).join(', ')}" + end + paths = Impl::FileData.absolute_paths(paths) + # A path listed more than once would make every entry of that file a duplicate of itself, so + # the repeats are dropped and the first occurrence keeps its place in the order. File names + # on Windows ignore case, so there paths that differ only in case name the same file. + unique_paths = Gem.win_platform? ? paths.uniq(&:downcase) : paths.uniq + if unique_paths.size != paths.size + config.logger.warn do + "#{Impl::Integrations::FileOverrideSource::LOG_PREFIX} File paths list the same file more than once; the repeats are ignored" + end + paths = unique_paths + end + + duplicate_keys_handling = @options.fetch(:duplicate_keys_handling, :fail) + unless DUPLICATE_KEYS_HANDLING_VALUES.include?(duplicate_keys_handling) + raise ArgumentError, + "unrecognized duplicate keys handling #{duplicate_keys_handling.inspect} for the file-based override source" + end + + change_detection = @options.fetch(:change_detection, :polling) + unless CHANGE_DETECTION_VALUES.include?(change_detection) + raise ArgumentError, "unrecognized change detection mode #{change_detection.inspect} for the file-based override source" + end + if change_detection == :watching && !Impl::FileData::Watcher.available? + raise ArgumentError, "change detection mode :watching for the file-based override source requires the listen gem" + end + + poll_interval = @options.fetch(:poll_interval, DEFAULT_POLL_INTERVAL) + unless poll_interval.is_a?(Numeric) + raise ArgumentError, "poll interval #{poll_interval.inspect} for the file-based override source must be a number" + end + if change_detection == :polling && poll_interval < MINIMUM_POLL_INTERVAL + config.logger.warn do + "#{Impl::Integrations::FileOverrideSource::LOG_PREFIX} Poll interval #{poll_interval}s is below the minimum; using #{MINIMUM_POLL_INTERVAL}s" + end + poll_interval = MINIMUM_POLL_INTERVAL + end + + Impl::Integrations::FileOverrideSource.new( + paths: paths, + duplicate_keys_handling: duplicate_keys_handling, + change_detection: change_detection, + poll_interval: poll_interval, + logger: config.logger + ) + end end # diff --git a/lib/ldclient-rb/interfaces.rb b/lib/ldclient-rb/interfaces.rb index 95d1e9b9..d0700a92 100644 --- a/lib/ldclient-rb/interfaces.rb +++ b/lib/ldclient-rb/interfaces.rb @@ -7,3 +7,4 @@ require "ldclient-rb/interfaces/migrations" require "ldclient-rb/interfaces/hooks" require "ldclient-rb/interfaces/plugins" +require "ldclient-rb/interfaces/overrides" diff --git a/lib/ldclient-rb/interfaces/overrides.rb b/lib/ldclient-rb/interfaces/overrides.rb new file mode 100644 index 00000000..431d9658 --- /dev/null +++ b/lib/ldclient-rb/interfaces/overrides.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +module LaunchDarkly + module Interfaces + # + # Interfaces for flag overrides. Overrides are flag and segment definitions that take + # precedence over data received from LaunchDarkly at evaluation time, on a per-key basis. + # They exist for resilience during an incident. An operator can force one or more flags to + # a known state on a running client, whether or not the client can reach LaunchDarkly. + # + # Flag overrides are currently experimental and subject to change. + # + module Overrides + # + # Receives the contents of the SDK's override store. The SDK implements it and passes it + # to an {OverrideSource}'s `start` method. Override sources call it. They do not implement it. + # + # Flag overrides are currently experimental and subject to change. + # + module OverrideSink + # + # Replaces the entire override store with the given flag and segment definitions. Each + # call is a full snapshot. Entries absent from the call are removed. Empty collections + # clear the store. + # + # Each definition is either a data model object, as produced by + # `LaunchDarkly::Impl::Model.deserialize`, or a hash in the flag or segment data model. + # The SDK itself marks the entries as overrides. + # + # This method is safe to call from any thread. Calls are serialized by the SDK, and the + # new contents are visible to evaluations when the call returns. + # + # @param flags [Enumerable] full flag definitions + # @param segments [Enumerable] full segment definitions + # @return [void] + # + def set_overrides(flags, segments) + raise NotImplementedError, "#{self.class} must implement #set_overrides" + end + end + + # + # Supplies flag and segment overrides that take precedence over LaunchDarkly data at + # evaluation time, on a per-key basis. + # + # An override source is not a data source. It does not take part in the data system's + # initializer and synchronizer pipeline. The override store it populates has no effect on + # the client's initialization status, data availability, or data source status. + # + # To configure an override source, use {LaunchDarkly::DataSystem::ConfigBuilder#overrides}. + # + # Flag overrides are currently experimental and subject to change. + # + module OverrideSource + # + # Begins supplying overrides to the sink and returns without blocking on long-running + # work. An implementation performs an initial load synchronously, then pushes a full + # replacement snapshot to the sink whenever its backing data changes, until `stop` is + # called. A failed load leaves the previously supplied store untouched by not calling + # the sink. + # + # The SDK calls this method at most once, before any call to `stop`. This method is not + # intended to fail. A source that cannot load its data logs the failure and keeps retrying, + # as the SDK's file-based source does. A configuration error belongs in the builder's + # `build`, where it is raised when the client is created. If this method does raise, the + # client still starts: the SDK logs the exception, stops the source, removes any snapshot + # the source supplied, and the client runs without overrides. + # + # @param sink [OverrideSink] + # @return [void] + # + def start(sink) + raise NotImplementedError, "#{self.class} must implement #start" + end + + # + # Stops the source and releases any resources it holds. An exception raised from it is + # logged and does not stop the client from shutting down. A snapshot that the source + # supplies after the SDK begins to stop it is ignored. + # + # @return [void] + # + def stop + raise NotImplementedError, "#{self.class} must implement #stop" + end + end + end + end +end diff --git a/lib/ldclient-rb/ldclient.rb b/lib/ldclient-rb/ldclient.rb index bbe65e48..b72a1d29 100644 --- a/lib/ldclient-rb/ldclient.rb +++ b/lib/ldclient-rb/ldclient.rb @@ -84,6 +84,8 @@ def initialize(sdk_key, config = Config.default, wait_for_sec = 5) # Each flag lets the matching cached-data warning log once per client. @cached_data_evaluation_warned = Concurrent::AtomicBoolean.new(false) @cached_data_all_flags_warned = Concurrent::AtomicBoolean.new(false) + # Set after the first warning that all_flags_state returned only override entries before initialization. + @all_flags_overrides_only_warned = Concurrent::AtomicBoolean.new(false) start_up(wait_for_sec) end @@ -511,7 +513,11 @@ def migration_variation(key, context, default_stage) next LaunchDarkly::Impl::EvaluationWithHookResult.new(detail, {stage: stage, tracker: tracker}) end - detail = LaunchDarkly::Impl::Evaluator.error_result(LaunchDarkly::EvaluationReason::ERROR_WRONG_TYPE, default_stage.to_s) + # The type mismatch replaces the reason. The evaluation read the same definitions, so the new + # reason keeps the override-affected marking. + reason = LaunchDarkly::EvaluationReason.error(LaunchDarkly::EvaluationReason::ERROR_WRONG_TYPE) + .with_override_affected(detail.reason.override_affected) + detail = EvaluationDetail.new(default_stage.to_s, nil, reason) tracker = Impl::Migrations::OpTracker.new(@config.logger, key, flag, context, detail, default_stage) LaunchDarkly::Impl::EvaluationWithHookResult.new(detail, {stage: default_stage, tracker: tracker}) @@ -631,14 +637,22 @@ def all_flags_state(context, options={}) check_forked + override_features = nil unless initialized? if @data_system.store.initialized? if @cached_data_all_flags_warned.make_true @config.logger.warn { "Called all_flags_state before client initialization; using last known values from data store. This message is logged once." } end else + # No data from LaunchDarkly is available. When the override store holds flags, the state + # contains only those, read from the override store itself: a store that is not + # initialized can still hold LaunchDarkly data, which the state must not include. + # Without override flags the state is invalid, as it is without an override source. + override_features = @data_system.override_items(Impl::DataStore::FEATURES) + if override_features.empty? @config.logger.warn { "Called all_flags_state before client initialization. Data store not available; returning empty state" } return FeatureFlagsState.new(false) + end end end @@ -648,14 +662,28 @@ def all_flags_state(context, options={}) return FeatureFlagsState.new(false) end - begin - features = @data_system.store.all(Impl::DataStore::FEATURES) - rescue => exn - Impl::Util.log_exception(@config.logger, "Unable to read flags for all_flags_state", exn) - return FeatureFlagsState.new(false) + valid = true + if override_features + if @all_flags_overrides_only_warned.make_true + @config.logger.warn { "Called all_flags_state before client initialization; returning only flags from the override store. This message is logged once." } + end + features = override_features + else + begin + features = @data_system.store.all(Impl::DataStore::FEATURES) + rescue => exn + Impl::Util.log_exception(@config.logger, "Unable to read flags for all_flags_state", exn) + # Overrides only add to the state. Without them the state is invalid and empty here, so + # with them it holds the override flags and stays invalid. A client-side SDK bootstrapped + # from the state uses its flags whatever $valid says (js-client-sdk 3.x and the 4.x browser + # SDK only log a warning), so the overridden values still reach it. + features = @data_system.override_items(Impl::DataStore::FEATURES) + return FeatureFlagsState.new(false) if features.empty? + valid = false + end end - state = FeatureFlagsState.new(true) + state = FeatureFlagsState.new(valid) client_only = options[:client_side_only] || false with_reasons = options[:with_reasons] || false details_only_if_tracked = options[:details_only_for_tracked_flags] || false @@ -667,21 +695,38 @@ def all_flags_state(context, options={}) (eval_result, eval_state) = @evaluator.evaluate(f, context) detail = eval_result.detail rescue => exn - detail = EvaluationDetail.new(nil, nil, EvaluationReason::error(EvaluationReason::ERROR_EXCEPTION)) + # The flag definition was read before the failure, so its own marker decides the marking. + reason = EvaluationReason::error(EvaluationReason::ERROR_EXCEPTION).with_override_affected(override_item?(f)) + detail = EvaluationDetail.new(nil, nil, reason) Impl::Util.log_exception(@config.logger, "Error evaluating flag \"#{k}\" in all_flags_state", exn) end requires_experiment_data = experiment?(f, detail.reason) + track_events = f[:trackEvents] || requires_experiment_data + track_reason = requires_experiment_data + debug_events_until_date = f[:debugEventsUntilDate] + # A failed evaluation has no result, so the flag's own marker decides, as it does for the + # reason. + override_affected = eval_result.nil? ? override_item?(f) : eval_result.override_affected + if override_affected + # A consumer of this state sends individual events according to these fields. An + # override-affected evaluation produces no individual events, so the state turns them off + # for this flag. The flag, its value, and its reason stay. + track_events = false + track_reason = false + debug_events_until_date = nil + end flag_state = { key: f[:key], value: detail.value, variation: detail.variation_index, reason: detail.reason, - prerequisites: eval_state.prerequisites, + # A failed evaluation has no evaluation state, so it reports no prerequisites. + prerequisites: eval_state&.prerequisites, version: f[:version], - trackEvents: f[:trackEvents] || requires_experiment_data, - trackReason: requires_experiment_data, - debugEventsUntilDate: f[:debugEventsUntilDate], + trackEvents: track_events, + trackReason: track_reason, + debugEventsUntilDate: debug_events_until_date, } state.add_flag(flag_state, with_reasons, details_only_if_tracked) @@ -769,21 +814,29 @@ def flag_tracker return detail, nil, context.error end + override_feature = nil if @data_system.data_availability != Impl::DataSystem::DataAvailability::REFRESHED if @data_system.data_availability == Impl::DataSystem::DataAvailability::CACHED if @cached_data_evaluation_warned.make_true @config.logger.warn { "[LDClient] Client has not finished initializing; using last known values from feature store. This message is logged once." } end else - @config.logger.error { "[LDClient] Client has not finished initializing; feature store unavailable, returning default value" } - detail = Evaluator.error_result(EvaluationReason::ERROR_CLIENT_NOT_READY, default) - record_unknown_flag_eval(key, context, default, detail.reason, with_reasons) - return detail, nil, "client not initialized" + # No data from LaunchDarkly is available. A flag that the override store holds is served, + # read from the override store itself. Every other flag takes the not-ready path, as + # without an override source: a store that is not initialized can still hold + # LaunchDarkly data, which the SDK must not serve. + override_feature = override_flag(key) + if override_feature.nil? + @config.logger.error { "[LDClient] Client has not finished initializing; feature store unavailable, returning default value" } + detail = Evaluator.error_result(EvaluationReason::ERROR_CLIENT_NOT_READY, default) + record_unknown_flag_eval(key, context, default, detail.reason, with_reasons) + return detail, nil, "client not initialized" + end end end begin - feature = @data_system.store.get(Impl::DataStore::FEATURES, key) + feature = override_feature || @data_system.store.get(Impl::DataStore::FEATURES, key) rescue # Ignored end @@ -798,25 +851,37 @@ def flag_tracker begin (res, _) = @evaluator.evaluate(feature, context) unless res.prereq_evals.nil? + # Each prerequisite record carries the marking of the prerequisite's own evaluation, not + # the marking of the evaluation that requested it. res.prereq_evals.each do |prereq_eval| - record_prereq_flag_eval(prereq_eval.prereq_flag, prereq_eval.prereq_of_flag, context, prereq_eval.detail, with_reasons) + record_prereq_flag_eval(prereq_eval.prereq_flag, prereq_eval.prereq_of_flag, context, prereq_eval.detail, + with_reasons, prereq_eval.override_affected) end end detail = res.detail if detail.default_value? detail = EvaluationDetail.new(default, nil, detail.reason) end - record_flag_eval(feature, context, detail, default, with_reasons) + record_flag_eval(feature, context, detail, default, with_reasons, res.override_affected) [detail, feature, nil] rescue => exn Impl::Util.log_exception(@config.logger, "Error evaluating feature flag \"#{key}\"", exn) - detail = Evaluator.error_result(EvaluationReason::ERROR_EXCEPTION, default) - record_flag_eval_error(feature, context, default, detail.reason, with_reasons) + # The flag definition was read before the failure, so its own marker decides the marking. + reason = EvaluationReason.error(EvaluationReason::ERROR_EXCEPTION).with_override_affected(override_item?(feature)) + detail = EvaluationDetail.new(default, nil, reason) + record_flag_eval_error(feature, context, default, detail.reason, with_reasons, override_item?(feature)) [detail, feature, exn.to_s] end end - private def record_flag_eval(flag, context, detail, default, with_reasons) + # Returns the override store's entry for the flag key, or nil. + private def override_flag(key) + return nil unless key.respond_to?(:to_sym) + + @data_system.override_items(Impl::DataStore::FEATURES)[key.to_sym] + end + + private def record_flag_eval(flag, context, detail, default, with_reasons, override_affected) add_experiment_data = experiment?(flag, detail.reason) @event_processor.record_eval_event( context, @@ -830,11 +895,12 @@ def flag_tracker flag[:debugEventsUntilDate], nil, flag[:samplingRatio], - !!flag[:excludeFromSummaries] + !!flag[:excludeFromSummaries], + override_affected ) end - private def record_prereq_flag_eval(prereq_flag, prereq_of_flag, context, detail, with_reasons) + private def record_prereq_flag_eval(prereq_flag, prereq_of_flag, context, detail, with_reasons, override_affected) add_experiment_data = experiment?(prereq_flag, detail.reason) @event_processor.record_eval_event( context, @@ -848,13 +914,15 @@ def flag_tracker prereq_flag[:debugEventsUntilDate], prereq_of_flag[:key], prereq_flag[:samplingRatio], - !!prereq_flag[:excludeFromSummaries] + !!prereq_flag[:excludeFromSummaries], + override_affected ) end - private def record_flag_eval_error(flag, context, default, reason, with_reasons) + private def record_flag_eval_error(flag, context, default, reason, with_reasons, override_affected) @event_processor.record_eval_event(context, flag[:key], flag[:version], nil, default, with_reasons ? reason : nil, default, - flag[:trackEvents], flag[:debugEventsUntilDate], nil, flag[:samplingRatio], !!flag[:excludeFromSummaries]) + flag[:trackEvents], flag[:debugEventsUntilDate], nil, flag[:samplingRatio], !!flag[:excludeFromSummaries], + override_affected) end # @@ -865,8 +933,21 @@ def flag_tracker # @param with_reasons [Boolean] # private def record_unknown_flag_eval(flag_key, context, default, reason, with_reasons) + # Nothing was read from the override store, so the evaluation is not marked. @event_processor.record_eval_event(context, flag_key, nil, nil, default, with_reasons ? reason : nil, default, - false, nil, nil, 1, false) + false, nil, nil, 1, false, false) + end + + # + # Whether a stored flag carries the override marker. The FeatureStore interface documents its + # entities as hashes, and a custom store can return one. Only the SDK's model objects carry the + # marker; a hash never came from the override store. + # + # @param item [Object] a flag as returned by the store + # @return [Boolean] + # + private def override_item?(item) + item.respond_to?(:override?) && item.override? end private def experiment?(flag, reason) 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/events_spec.rb b/spec/events_spec.rb index d5c50661..5a92627f 100644 --- a/spec/events_spec.rb +++ b/spec/events_spec.rb @@ -384,6 +384,83 @@ module LaunchDarkly end end + describe "override-affected evaluations" do + let(:future_time) { (Time.now.to_f * 1000).to_i + 1000000 } + + it "does not produce a feature event even when the flag tracks events" do + with_processor_and_sender(default_config, starting_timestamp) do |ep, sender| + ep.record_eval_event(context, 'flagkey', 11, 1, 'value', nil, 'default', true, nil, nil, nil, false, true) + + output = flush_and_get_events(ep, sender) + expect(output).to contain_exactly( + eq(index_event(default_config, context)), + include(:kind => "summary") + ) + end + end + + it "does not produce a debug event even when the flag is in debug mode" do + with_processor_and_sender(default_config, starting_timestamp) do |ep, sender| + ep.record_eval_event(context, 'flagkey', 11, 1, 'value', nil, 'default', true, future_time, nil, nil, false, true) + + output = flush_and_get_events(ep, sender) + expect(output).to contain_exactly( + eq(index_event(default_config, context)), + include(:kind => "summary") + ) + end + end + + it "is counted in a summary counter that carries the override-affected marker" do + with_processor_and_sender(default_config, starting_timestamp) do |ep, sender| + ep.record_eval_event(context, 'flagkey', 11, 1, 'value', nil, 'default', true, nil, nil, nil, false, true) + ep.record_eval_event(context, 'flagkey', 11, 1, 'value', nil, 'default', true, nil, nil, nil, false, true) + + output = flush_and_get_events(ep, sender) + summary = output.detect { |e| e[:kind] == "summary" } + expect(summary[:features][:flagkey][:counters]).to contain_exactly( + { version: 11, variation: 1, value: "value", count: 2, overrideAffected: true } + ) + end + end + + it "is counted separately from other evaluations of the same flag, version, and variation" do + with_processor_and_sender(default_config, starting_timestamp) do |ep, sender| + ep.record_eval_event(context, 'flagkey', 11, 1, 'value', nil, 'default', false, nil, nil, nil, false, true) + ep.record_eval_event(context, 'flagkey', 11, 1, 'value', nil, 'default', false) + + output = flush_and_get_events(ep, sender) + summary = output.detect { |e| e[:kind] == "summary" } + expect(summary[:features][:flagkey][:counters]).to contain_exactly( + { version: 11, variation: 1, value: "value", count: 1, overrideAffected: true }, + { version: 11, variation: 1, value: "value", count: 1 } + ) + end + end + + it "does not add the marker to counters for other evaluations" do + with_processor_and_sender(default_config, starting_timestamp) do |ep, sender| + ep.record_eval_event(context, 'flagkey', 11, 1, 'value', nil, 'default', false) + + output = flush_and_get_events(ep, sender) + summary = output.detect { |e| e[:kind] == "summary" } + expect(summary[:features][:flagkey][:counters]).to eq([{ version: 11, variation: 1, value: "value", count: 1 }]) + end + end + + it "keeps a marked prerequisite record out of the individual events" do + with_processor_and_sender(default_config, starting_timestamp) do |ep, sender| + ep.record_eval_event(context, 'prereq', 11, 1, 'value', nil, nil, true, nil, 'top', nil, false, true) + + output = flush_and_get_events(ep, sender) + expect(output).to contain_exactly( + eq(index_event(default_config, context)), + include(:kind => "summary") + ) + end + end + end + it "queues custom event with context" do with_processor_and_sender(default_config, starting_timestamp) do |ep, sender| ep.record_custom_event(context, 'eventkey', { thing: 'stuff' }, 1.5) diff --git a/spec/fixtures/override-vectors/vectors.json b/spec/fixtures/override-vectors/vectors.json new file mode 100644 index 00000000..82dc62da --- /dev/null +++ b/spec/fixtures/override-vectors/vectors.json @@ -0,0 +1,316 @@ +{ + "schemaVersion": "0.4.0", + "vectors": [ + { + "description": "An override takes precedence over LaunchDarkly data for the same flag key", + "group": "precedence", + "launchDarklyData": { + "initialized": true, + "flags": { + "flag-a": { + "key": "flag-a", + "version": 7, + "on": true, + "variations": ["ld-value"], + "fallthrough": { "variation": 0 }, + "offVariation": 0, + "salt": "salt-a" + } + }, + "segments": {} + }, + "overrides": { + "flagValues": { "flag-a": "override-value" } + }, + "evaluate": { + "flagKey": "flag-a", + "context": { "kind": "user", "key": "user-1" }, + "defaultValue": "fallback" + }, + "expect": { + "value": "override-value", + "variationIndex": 0, + "reason": { "kind": "FALLTHROUGH", "overrideAffected": true }, + "summaryOverrideAffected": true + } + }, + { + "description": "An overridden flag is served when the SDK has not initialized from LaunchDarkly", + "group": "uninitialized", + "launchDarklyData": { + "initialized": false, + "flags": {}, + "segments": {} + }, + "overrides": { + "flagValues": { "flag-a": "override-value" } + }, + "evaluate": { + "flagKey": "flag-a", + "context": { "kind": "user", "key": "user-1" }, + "defaultValue": "fallback" + }, + "expect": { + "value": "override-value", + "variationIndex": 0, + "reason": { "kind": "FALLTHROUGH", "overrideAffected": true }, + "summaryOverrideAffected": true + } + }, + { + "description": "A flag absent from the override layer is evaluated from LaunchDarkly data and is not marked", + "group": "passthrough", + "launchDarklyData": { + "initialized": true, + "flags": { + "flag-b": { + "key": "flag-b", + "version": 3, + "on": true, + "variations": ["off-value", "ld-value"], + "fallthrough": { "variation": 1 }, + "offVariation": 0, + "salt": "salt-b" + } + }, + "segments": {} + }, + "overrides": { + "flagValues": { "flag-a": "override-value" } + }, + "evaluate": { + "flagKey": "flag-b", + "context": { "kind": "user", "key": "user-1" }, + "defaultValue": "fallback" + }, + "expect": { + "value": "ld-value", + "variationIndex": 1, + "reason": { "kind": "FALLTHROUGH" }, + "summaryOverrideAffected": false + } + }, + { + "description": "A flag absent from the override layer still short-circuits when the SDK has not initialized", + "group": "passthrough", + "launchDarklyData": { + "initialized": false, + "flags": {}, + "segments": {} + }, + "overrides": { + "flagValues": { "flag-a": "override-value" } + }, + "evaluate": { + "flagKey": "flag-b", + "context": { "kind": "user", "key": "user-1" }, + "defaultValue": "fallback" + }, + "expect": { + "value": "fallback", + "variationIndex": null, + "reason": { "kind": "ERROR", "errorKind": "CLIENT_NOT_READY" } + } + }, + { + "description": "A full flag override with a matching targeting rule serves the rule's variation and is marked", + "group": "full-evaluation", + "launchDarklyData": { + "initialized": true, + "flags": {}, + "segments": {} + }, + "overrides": { + "flags": { + "flag-c": { + "key": "flag-c", + "version": 1, + "on": true, + "variations": ["default-value", "eu-value"], + "fallthrough": { "variation": 0 }, + "offVariation": 0, + "rules": [ + { + "id": "rule-eu", + "variation": 1, + "clauses": [ + { + "contextKind": "user", + "attribute": "region", + "op": "in", + "values": ["eu"], + "negate": false + } + ] + } + ], + "salt": "salt-c" + } + } + }, + "evaluate": { + "flagKey": "flag-c", + "context": { "kind": "user", "key": "user-1", "region": "eu" }, + "defaultValue": "fallback" + }, + "expect": { + "value": "eu-value", + "variationIndex": 1, + "reason": { "kind": "RULE_MATCH", "ruleIndex": 0, "ruleId": "rule-eu", "overrideAffected": true }, + "summaryOverrideAffected": true + } + }, + { + "description": "An override flag resolves a referenced segment through the merged view while the SDK is uninitialized", + "group": "merged-view", + "launchDarklyData": { + "initialized": false, + "flags": {}, + "segments": {} + }, + "overrides": { + "flags": { + "flag-d": { + "key": "flag-d", + "version": 1, + "on": true, + "variations": ["default-value", "member-value"], + "fallthrough": { "variation": 0 }, + "offVariation": 0, + "rules": [ + { + "id": "rule-seg", + "variation": 1, + "clauses": [ + { + "attribute": "", + "op": "segmentMatch", + "values": ["seg-1"], + "negate": false + } + ] + } + ], + "salt": "salt-d" + } + }, + "segments": { + "seg-1": { + "key": "seg-1", + "version": 1, + "included": ["user-1"], + "excluded": [], + "salt": "seg-salt-1" + } + } + }, + "evaluate": { + "flagKey": "flag-d", + "context": { "kind": "user", "key": "user-1" }, + "defaultValue": "fallback" + }, + "expect": { + "value": "member-value", + "variationIndex": 1, + "reason": { "kind": "RULE_MATCH", "ruleIndex": 0, "ruleId": "rule-seg", "overrideAffected": true }, + "summaryOverrideAffected": true + } + }, + { + "description": "A flag is marked overrideAffected when it matches via an overridden segment", + "group": "marking", + "launchDarklyData": { + "initialized": true, + "flags": { + "flag-f": { + "key": "flag-f", + "version": 4, + "on": true, + "variations": ["default-value", "member-value"], + "fallthrough": { "variation": 0 }, + "offVariation": 0, + "rules": [ + { + "id": "rule-seg", + "variation": 1, + "clauses": [ + { + "attribute": "", + "op": "segmentMatch", + "values": ["seg-2"], + "negate": false + } + ] + } + ], + "salt": "salt-f" + } + }, + "segments": { + "seg-2": { + "key": "seg-2", + "version": 1, + "included": [], + "excluded": [], + "salt": "seg-salt-2" + } + } + }, + "overrides": { + "segments": { + "seg-2": { + "key": "seg-2", + "version": 2, + "included": ["user-1"], + "excluded": [], + "salt": "seg-salt-2" + } + } + }, + "evaluate": { + "flagKey": "flag-f", + "context": { "kind": "user", "key": "user-1" }, + "defaultValue": "fallback" + }, + "expect": { + "value": "member-value", + "variationIndex": 1, + "reason": { "kind": "RULE_MATCH", "ruleIndex": 0, "ruleId": "rule-seg", "overrideAffected": true }, + "summaryOverrideAffected": true + } + }, + { + "description": "A malformed override flag returns the caller default with an error reason and is marked overrideAffected", + "group": "error-handling", + "launchDarklyData": { + "initialized": true, + "flags": {}, + "segments": {} + }, + "overrides": { + "flags": { + "flag-e": { + "key": "flag-e", + "version": 1, + "on": true, + "variations": ["only-value"], + "fallthrough": { "variation": 5 }, + "offVariation": 0, + "salt": "salt-e" + } + } + }, + "evaluate": { + "flagKey": "flag-e", + "context": { "kind": "user", "key": "user-1" }, + "defaultValue": "fallback" + }, + "expect": { + "value": "fallback", + "variationIndex": null, + "reason": { "kind": "ERROR", "errorKind": "MALFORMED_FLAG", "overrideAffected": true }, + "summaryOverrideAffected": true + } + } + ] +} diff --git a/spec/impl/data_store/store_spec.rb b/spec/impl/data_store/store_spec.rb index c4aa6df2..7d111ef1 100644 --- a/spec/impl/data_store/store_spec.rb +++ b/spec/impl/data_store/store_spec.rb @@ -4,7 +4,10 @@ require "ldclient-rb/impl/data_store/store" require "ldclient-rb/impl/broadcaster" require "ldclient-rb/impl/data_store" +require "ldclient-rb/impl/overrides" require "ldclient-rb/interfaces/data_system" +require "model_builders" +require "override_test_components" module LaunchDarkly module Impl @@ -288,6 +291,127 @@ def disable_cache end end + describe "flag change notifications" do + # Collects the flag keys the asynchronous broadcaster delivers. + class CollectingListener + def initialize + @keys = Queue.new + end + + def update(flag_change) + @keys << flag_change.key + end + + # Every key that arrives within the settle time after the first one, sorted. + def collect(timeout = 2, settle = 0.3) + keys = [] + first = TestQueues.pop_within(@keys, timeout) + return keys if first.nil? + + keys << first + loop do + key = TestQueues.pop_within(@keys, settle) + break if key.nil? + + keys << key + end + keys.sort + end + end + + let(:listener) { CollectingListener.new } + let(:layer) { LaunchDarkly::Impl::Overrides::Layer.new } + + def flag_hash(key, version, prerequisite: nil, segment: nil) + data = { key: key, version: version, on: true, fallthrough: { variation: 0 }, offVariation: 0, variations: ["v"] } + data[:prerequisites] = [{ key: prerequisite, variation: 0 }] if prerequisite + data[:rules] = [{ id: "r", variation: 0, clauses: [Clauses.match_segment(segment)] }] if segment + data + end + + def segment_hash(key, version) + { key: key, version: version, included: ["user"] } + end + + def put(kind, item) + LaunchDarkly::Interfaces::DataSystem::Change.new( + action: LaunchDarkly::Interfaces::DataSystem::ChangeType::PUT, kind: kind, key: item[:key].to_sym, + version: item[:version], object: item + ) + end + + def apply(intent, *changes) + subject.apply(LaunchDarkly::Interfaces::DataSystem::ChangeSet.new( + intent_code: intent, changes: changes, selector: LaunchDarkly::Interfaces::DataSystem::Selector.no_selector + ), false) + end + + def full(*changes) + apply(LaunchDarkly::Interfaces::DataSystem::IntentCode::TRANSFER_FULL, *changes) + end + + def delta(*changes) + apply(LaunchDarkly::Interfaces::DataSystem::IntentCode::TRANSFER_CHANGES, *changes) + end + + FLAG = LaunchDarkly::Interfaces::DataSystem::ObjectKind::FLAG + SEGMENT = LaunchDarkly::Interfaces::DataSystem::ObjectKind::SEGMENT + + before { flag_change_broadcaster.add_listener(listener) } + + it "notifies the flags that depend on a changed item when changes are keyed by symbol" do + full(put(FLAG, flag_hash("prereq", 1)), put(FLAG, flag_hash("dependent", 1, prerequisite: "prereq"))) + listener.collect + + delta(put(FLAG, flag_hash("prereq", 2))) + expect(listener.collect).to eq %w[dependent prereq] + end + + context "with the override layer as external dependencies" do + before { subject.set_external_dependencies(layer) } + + it "notifies an override flag whose prerequisite changed in a delta" do + layer.set_all({ override: Flags.from_hash(flag_hash("override", 1, prerequisite: "prereq")) }, {}) + full(put(FLAG, flag_hash("prereq", 1))) + listener.collect + + delta(put(FLAG, flag_hash("prereq", 2))) + expect(listener.collect).to eq %w[override prereq] + end + + it "notifies override flags that depend on items changed by a full data set" do + layer.set_all({ override: Flags.from_hash(flag_hash("override", 1, segment: "seg")) }, {}) + full(put(SEGMENT, segment_hash("seg", 1)), put(FLAG, flag_hash("unrelated", 1))) + listener.collect + + full(put(SEGMENT, segment_hash("seg", 2)), put(FLAG, flag_hash("unrelated", 1))) + # The segment is not a flag, so only the dependent override flag is notified. + expect(listener.collect).to eq %w[override] + end + + it "follows a dependency chain that crosses between the store and the layer" do + # LaunchDarkly flag -> override flag -> LaunchDarkly segment: a segment change must reach + # the LaunchDarkly flag at the top of the chain. + layer.set_all({ override: Flags.from_hash(flag_hash("override", 1, segment: "seg")) }, {}) + full(put(SEGMENT, segment_hash("seg", 1)), put(FLAG, flag_hash("ld", 1, prerequisite: "override"))) + listener.collect + + delta(put(SEGMENT, segment_hash("seg", 2))) + expect(listener.collect).to eq %w[ld override] + end + + it "follows the layer's current graph after a replacement" do + layer.set_all({ override: Flags.from_hash(flag_hash("override", 1, prerequisite: "prereq")) }, {}) + full(put(FLAG, flag_hash("prereq", 1)), put(FLAG, flag_hash("top", 1, prerequisite: "prereq"))) + listener.collect + + layer.set_all({}, {}) + delta(put(FLAG, flag_hash("prereq", 2))) + expect(listener.collect).to eq %w[prereq top] + end + end + end + describe "#commit" do context "without persistent store" do it "returns nil and does nothing" do diff --git a/spec/impl/data_system/fdv2_overrides_spec.rb b/spec/impl/data_system/fdv2_overrides_spec.rb new file mode 100644 index 00000000..4b73da0e --- /dev/null +++ b/spec/impl/data_system/fdv2_overrides_spec.rb @@ -0,0 +1,194 @@ +# frozen_string_literal: true + +require "spec_helper" +require "capturing_logger" +require "model_builders" +require "override_test_components" +require "ldclient-rb/impl/data_system/fdv2" +require "ldclient-rb/data_system" + +module LaunchDarkly + module Impl + module DataSystem + describe FDv2, "with an override source" do + let(:sdk_key) { "test-sdk-key" } + let(:config) { LaunchDarkly::Config.new(logger: $null_log) } + let(:override_flag) { { key: "flag", version: 1, on: false, offVariation: 0, variations: ["override"] } } + let(:ld_flag) { { key: "flag", version: 1, on: false, offVariation: 0, variations: ["ld"] } } + + def config_with(overrides: nil, initializers: nil, synchronizers: nil) + builder = LaunchDarkly::DataSystem.custom + builder.initializers(initializers) if initializers + builder.synchronizers(synchronizers) if synchronizers + builder.overrides(overrides) if overrides + builder.build + end + + def with_fdv2(data_system_config, config: self.config) + fdv2 = FDv2.new(sdk_key, config, data_system_config) + begin + yield fdv2 + ensure + fdv2.stop + end + end + + it "reports that no override source is configured by default" do + with_fdv2(config_with) do |fdv2| + expect(fdv2.override_items(DataStore::FEATURES)).to eq({}) + expect(fdv2.store).not_to be_a(Overrides::Overlay) + end + end + + it "builds the override source at construction with the SDK key and configuration" do + source = TestOverrideSource.new + with_fdv2(config_with(overrides: source)) do |fdv2| + expect(fdv2.store).to be_a(Overrides::Overlay) + expect(source.build_args).to eq [sdk_key, config] + expect(source.started?).to be false + end + end + + it "raises from construction when the override source cannot be built" do + builder = Object.new + builder.define_singleton_method(:build) { |_sdk_key, _config| raise ArgumentError, "no file paths" } + + expect { FDv2.new(sdk_key, config, config_with(overrides: builder)) }.to raise_error(ArgumentError, "no file paths") + end + + it "serves reads through the overlay when an override source is configured" do + source = TestOverrideSource.new([override_flag]) + with_fdv2(config_with(overrides: source)) do |fdv2| + expect(fdv2.store).to be_a(Overrides::Overlay) + end + end + + it "starts the source with a sink before start returns, so the initial load is in effect at once" do + source = TestOverrideSource.new([override_flag]) + with_fdv2(config_with(overrides: source, synchronizers: [HangingSynchronizer.new])) do |fdv2| + fdv2.start + + expect(source.started?).to be true + expect(source.sink).to be_a(Overrides::Sink) + flag = fdv2.store.get(DataStore::FEATURES, "flag") + expect(flag.variations).to eq ["override"] + expect(flag.override?).to be true + end + end + + it "serves the override entry in preference to LaunchDarkly data" do + source = TestOverrideSource.new([override_flag]) + initializer = TestDataInitializer.new(flags: { flag: ld_flag, other: ld_flag.merge(key: "other") }) + with_fdv2(config_with(overrides: source, initializers: [initializer])) do |fdv2| + expect(fdv2.start.wait(2)).to be true + + expect(fdv2.store.get(DataStore::FEATURES, "flag").variations).to eq ["override"] + expect(fdv2.store.get(DataStore::FEATURES, "other").variations).to eq ["ld"] + expect(fdv2.store.all(DataStore::FEATURES).keys.sort).to eq [:flag, :other] + end + end + + it "applies a later snapshot from the source to the running system" do + source = TestOverrideSource.new([override_flag]) + with_fdv2(config_with(overrides: source, synchronizers: [HangingSynchronizer.new])) do |fdv2| + fdv2.start + source.update([override_flag.merge(variations: ["changed"])]) + expect(fdv2.store.get(DataStore::FEATURES, "flag").variations).to eq ["changed"] + + source.update([]) + expect(fdv2.store.get(DataStore::FEATURES, "flag")).to be_nil + end + end + + it "does not let overrides affect data availability or initialization" do + source = TestOverrideSource.new([override_flag]) + with_fdv2(config_with(overrides: source, synchronizers: [HangingSynchronizer.new])) do |fdv2| + fdv2.start + + expect(fdv2.data_availability).to eq DataAvailability::DEFAULTS + expect(fdv2.store.initialized?).to be false + expect(fdv2.store.get(DataStore::FEATURES, "flag")).not_to be_nil + end + end + + it "notifies flag change listeners for an override change" do + source = TestOverrideSource.new([]) + with_fdv2(config_with(overrides: source, synchronizers: [HangingSynchronizer.new])) do |fdv2| + listener = CollectingFlagChangeListener.new + fdv2.flag_change_broadcaster.add_listener(listener) + fdv2.start + + source.update([override_flag]) + + expect(listener.next_key).to eq "flag" + end + end + + it "stops the source when the data system stops" do + source = TestOverrideSource.new([override_flag]) + fdv2 = FDv2.new(sdk_key, config, config_with(overrides: source, synchronizers: [HangingSynchronizer.new])) + fdv2.start + fdv2.stop + + expect(source.stopped?).to be true + end + + it "stops the rest of the system when the source raises on stop" do + source = TestOverrideSource.new([override_flag]) + source.define_singleton_method(:stop) { raise "boom" } + synchronizer = HangingSynchronizer.new + fdv2 = FDv2.new(sdk_key, config, config_with(overrides: source, synchronizers: [synchronizer])) + fdv2.start + + expect { fdv2.stop }.not_to raise_error + expect(Thread.list.map(&:name)).not_to include("FDv2-main") + end + + it "ignores a snapshot that the source supplies while it stops" do + source = TestOverrideSource.new([override_flag]) + late = override_flag.merge(key: "late") + source.define_singleton_method(:stop) { update([late]) } + fdv2 = FDv2.new(sdk_key, config, config_with(overrides: source, synchronizers: [HangingSynchronizer.new])) + fdv2.start + fdv2.stop + + expect(fdv2.override_items(DataStore::FEATURES).keys).to eq [:flag] + end + + it "starts without overrides when the source raises from start" do + source = TestOverrideSource.new([override_flag]) + source.define_singleton_method(:start) do |sink| + sink.set_overrides([{ key: "flag", version: 1, on: false, offVariation: 0, variations: ["override"] }], []) + raise "source failed" + end + logger = CapturingLogger.new + logged = LaunchDarkly::Config.new(logger: logger) + fdv2 = FDv2.new(sdk_key, logged, config_with(overrides: source, synchronizers: [HangingSynchronizer.new])) + begin + expect { fdv2.start }.not_to raise_error + expect(source.stopped?).to be true + expect(fdv2.override_items(DataStore::FEATURES)).to eq({}) + expect(fdv2.store.get(DataStore::FEATURES, "flag")).to be_nil + expect(logger.output).to include("Unable to start the override source; the client runs without overrides") + ensure + fdv2.stop + end + end + + it "does not build the override source when the SDK is offline, and logs that" do + source = TestOverrideSource.new([override_flag]) + logger = CapturingLogger.new + offline = LaunchDarkly::Config.new(logger: logger, offline: true) + with_fdv2(config_with(overrides: source), config: offline) do |fdv2| + fdv2.start + + expect(source.build_args).to be_nil + expect(fdv2.override_items(DataStore::FEATURES)).to eq({}) + expect(fdv2.store.get(DataStore::FEATURES, "flag")).to be_nil + expect(logger.output).to include("Offline mode is enabled; the override source is not started and no overrides are served") + end + end + end + end + end +end 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/event_summarizer_spec.rb b/spec/impl/event_summarizer_spec.rb index 1aedb757..3ff82e09 100644 --- a/spec/impl/event_summarizer_spec.rb +++ b/spec/impl/event_summarizer_spec.rb @@ -55,8 +55,8 @@ module Impl 'key1' => EventSummaryFlagInfo.new( 'default1', { 11 => { - 1 => EventSummaryFlagVariationCounter.new('value1', 2), - 2 => EventSummaryFlagVariationCounter.new('value2', 1), + 1 => { false => EventSummaryFlagVariationCounter.new('value1', 2) }, + 2 => { false => EventSummaryFlagVariationCounter.new('value2', 1) }, }, }, Set.new(["user"]) @@ -64,7 +64,7 @@ module Impl 'key2' => EventSummaryFlagInfo.new( 'default2', { 22 => { - 1 => EventSummaryFlagVariationCounter.new('value99', 1), + 1 => { false => EventSummaryFlagVariationCounter.new('value99', 1) }, }, }, Set.new(["user"]) @@ -72,7 +72,31 @@ module Impl 'badkey' => EventSummaryFlagInfo.new( 'default3', { nil => { - nil => EventSummaryFlagVariationCounter.new('default3', 1), + nil => { false => EventSummaryFlagVariationCounter.new('default3', 1) }, + }, + }, + Set.new(["user"]) + ), + } + expect(data.counters).to eq expected_counters + end + + it "counts override-affected evaluations separately from other evaluations of the same flag, version, and variation" do + es = subject.new + plain = make_eval_event(0, context, 'key1', 11, 1, 'value1', nil, 'default1') + marked = LaunchDarkly::Impl::EvalEvent.new(0, context, 'key1', 11, 1, 'value1', nil, 'default1', + false, nil, nil, nil, false, true) + [plain, marked, marked].each { |e| es.summarize_event(e) } + data = es.snapshot + + expected_counters = { + 'key1' => EventSummaryFlagInfo.new( + 'default1', { + 11 => { + 1 => { + false => EventSummaryFlagVariationCounter.new('value1', 1), + true => EventSummaryFlagVariationCounter.new('value1', 2), + }, }, }, Set.new(["user"]) diff --git a/spec/impl/file_data/document_spec.rb b/spec/impl/file_data/document_spec.rb index b5a20703..544e88b2 100644 --- a/spec/impl/file_data/document_spec.rb +++ b/spec/impl/file_data/document_spec.rb @@ -10,7 +10,7 @@ module FileData describe Document do describe "parse" do it "parses a JSON document with every section and symbolizes nested keys" do - document = Document.parse(<<~JSON) + document = Document.parse(<<~JSON, format: :json) { "flags": { "flag1": { "key": "flag1", "on": true, "rules": [ { "clauses": [ { "op": "in" } ] } ] } }, "flagValues": { "flag2": "value2" }, @@ -25,7 +25,7 @@ module FileData end it "parses a YAML document" do - document = Document.parse(<<~YAML) + document = Document.parse(<<~YAML, format: :yaml) --- flags: flag1: @@ -43,41 +43,97 @@ module FileData expect(document.segments.keys).to eq([:seg1]) end - it "treats an empty document as a document with no entries" do - document = Document.parse("") + it "treats empty YAML content as a document with no entries" do + ["", " \n\r\n", "# a comment\n", "---\n", "~\n"].each do |content| + document = Document.parse(content, format: :yaml) - expect(document.flags).to eq({}) - expect(document.flag_values).to eq({}) - expect(document.segments).to eq({}) + expect(document.flags).to eq({}) + expect(document.flag_values).to eq({}) + expect(document.segments).to eq({}) + end + end + + it "returns NONE for YAML that is only a null, and a document for {} or a member that is null" do + ["", "# a comment\n", "---\n", "~\n"].each do |content| + expect(Document.parse(content, format: :yaml)).to be(Document::NONE) + end + ["{}\n", "flagValues:\n"].each do |content| + document = Document.parse(content, format: :yaml) + + expect(document).not_to be(Document::NONE) + expect(document.flag_values).to eq({}) + end + expect(Document.parse("{}", format: :json)).not_to be(Document::NONE) + end + + it "treats YAML with only white space, including a tab, vertical tab or form feed, as no entries" do + expect(Document.parse("\t\n", format: :yaml)).to be(Document::NONE) + expect(Document.parse(" \v\f\t\r\n", format: :yaml)).to be(Document::NONE) + end + + it "rejects content that is not valid UTF-8, in both formats" do + latin1 = (+"{\"flagValues\": {\"flag1\": \"caf\xE9\"}}").force_encoding(Encoding::UTF_8) + [:json, :yaml].each do |format| + expect { Document.parse(latin1, format: format) }.to raise_error(ArgumentError, "the file is not valid UTF-8") + end + end + + it "rejects empty JSON content, which is not a JSON document" do + ["", " \n\r\n\t"].each do |content| + expect { Document.parse(content, format: :json) } + .to raise_error(ArgumentError, "the file has no content, which is not a JSON document") + end end it "treats a document with no known sections as a document with no entries" do - document = Document.parse("{}") + [[:json, "{}"], [:yaml, "{}"], [:yaml, "other: 1\n"]].each do |format, content| + document = Document.parse(content, format: format) - expect(document.flags).to eq({}) - expect(document.flag_values).to eq({}) - expect(document.segments).to eq({}) + expect(document.flags).to eq({}) + expect(document.flag_values).to eq({}) + expect(document.segments).to eq({}) + end end it "rejects a document that is not an object" do - expect { Document.parse("\"hello\"") }.to raise_error(ArgumentError, /must be an object/) - expect { Document.parse("[1, 2]") }.to raise_error(ArgumentError, /must be an object/) + [:json, :yaml].each do |format| + expect { Document.parse("\"hello\"", format: format) }.to raise_error(ArgumentError, /must be an object/) + expect { Document.parse("[1, 2]", format: format) }.to raise_error(ArgumentError, /must be an object/) + end + expect { Document.parse("null", format: :json) }.to raise_error(ArgumentError, /must be an object/) + expect { Document.parse("false", format: :yaml) }.to raise_error(ArgumentError, /must be an object/) end it "rejects a section that is not an object" do - expect { Document.parse('{"flags": []}') }.to raise_error(ArgumentError, /"flags" must be an object/) - expect { Document.parse('{"flagValues": 3}') }.to raise_error(ArgumentError, /"flagValues" must be an object/) - expect { Document.parse('{"segments": "x"}') }.to raise_error(ArgumentError, /"segments" must be an object/) + expect { Document.parse('{"flags": []}', format: :json) }.to raise_error(ArgumentError, /"flags" must be an object/) + expect { Document.parse('{"flagValues": 3}', format: :json) }.to raise_error(ArgumentError, /"flagValues" must be an object/) + expect { Document.parse('{"segments": "x"}', format: :json) }.to raise_error(ArgumentError, /"segments" must be an object/) end it "symbolizes keys that YAML parsed as numbers" do - document = Document.parse("flagValues:\n 123: true\n") + document = Document.parse("flagValues:\n 123: true\n", format: :yaml) expect(document.flag_values).to eq({ "123": true }) end + it "reads JSON numbers as JSON does" do + document = Document.parse('{"flagValues": {"flag1": 1e3, "flag2": 12}}', format: :json) + + expect(document.flag_values).to eq({ flag1: 1000.0, flag2: 12 }) + end + it "raises a syntax error for content that cannot be parsed" do - expect { Document.parse('{"flagValues"') }.to raise_error(Psych::SyntaxError) + expect { Document.parse('{"flagValues"', format: :json) }.to raise_error(JSON::ParserError) + expect { Document.parse('{"flagValues": {}} extra', format: :json) }.to raise_error(JSON::ParserError) + expect { Document.parse("flagValues: [", format: :yaml) }.to raise_error(Psych::SyntaxError) + end + + it "rejects a YAML document as JSON" do + expect { Document.parse("flagValues:\n flag1: true\n", format: :json) }.to raise_error(JSON::ParserError) + end + + it "rejects an unknown format" do + expect { Document.parse("{}", format: :toml) }.to raise_error(ArgumentError, /unknown file format/) end end @@ -98,6 +154,74 @@ module FileData expect(document.flag_values).to eq({ flag1: 1 }) end + it "reads a file as YAML when its name ends in .yaml or .yml, in any case" do + ["flags.yaml", "flags.yml", "flags.YAML", "flags.Yml"].each do |name| + path = File.join(@dir, name) + File.write(path, "flagValues:\n flag1: true\n") + + expect(Document.read(path).flag_values).to eq({ flag1: true }) + end + end + + it "reads any other file as JSON" do + ["flags.json", "flags", "flags.txt", "flags.yaml.json"].each do |name| + path = File.join(@dir, name) + File.write(path, "flagValues:\n flag1: true\n") + + expect { Document.read(path) }.to raise_error(ReadError, /error parsing file/) + end + end + + it "reports an empty file as a read error when it is JSON, and as no entries when it is YAML" do + ["flags.json", "flags"].each do |name| + path = File.join(@dir, name) + File.write(path, "") + + expect { Document.read(path) }.to raise_error(ReadError, /the file has no content, which is not a JSON document/) + end + + path = File.join(@dir, "flags.yaml") + File.write(path, "") + expect(Document.read(path).flag_values).to eq({}) + end + + it "reads a file that starts with a byte order mark" do + path = File.join(@dir, "flags.json") + File.binwrite(path, "\xEF\xBB\xBF{\"flagValues\": {\"flag1\": \"caf\xC3\xA9\"}}") + + expect(Document.read(path).flag_values).to eq({ flag1: "caf\u00e9" }) + end + + it "reports a file encoded in UTF-16 or UTF-32 as a read error" do + path = File.join(@dir, "flags.json") + content = '{"flagValues": {"flag1": true}}' + ["\xFF\xFE".b + content.encode("UTF-16LE").b, "\xFE\xFF".b + content.encode("UTF-16BE").b, + "\xFF\xFE\x00\x00".b + content.encode("UTF-32LE").b].each do |bytes| + File.binwrite(path, bytes) + + expect { Document.read(path) }.to raise_error(ReadError, /encoded in UTF-16 or UTF-32; data files must be UTF-8/) + end + end + + it "reads a file as UTF-8 whatever the default external encoding" do + path = File.join(@dir, "flags.yaml") + File.binwrite(path, "flagValues:\n flag1: caf\xC3\xA9\n") + previous = Encoding.default_external + begin + Encoding.default_external = Encoding::US_ASCII + expect(Document.read(path).flag_values).to eq({ flag1: "caf\u00e9" }) + ensure + Encoding.default_external = previous + end + end + + it "reads a file that holds only a byte order mark as empty" do + path = File.join(@dir, "flags.json") + File.binwrite(path, "\xEF\xBB\xBF") + + expect(FileData.read_file(path)).to eq("") + end + it "reports a missing file as a read error that is marked missing" do path = File.join(@dir, "no-such-file.json") @@ -135,6 +259,28 @@ module FileData end end + describe "format_of" do + it "chooses YAML for a name that ends in .yaml or .yml, in any case, and JSON otherwise" do + expect(FileData.format_of("/a/flags.yaml")).to eq(:yaml) + expect(FileData.format_of("/a/.yaml")).to eq(:yaml) + expect(FileData.format_of("/a/.yml")).to eq(:yaml) + expect(FileData.format_of("/a/flags.YML")).to eq(:yaml) + expect(FileData.format_of("/a/flags.json")).to eq(:json) + expect(FileData.format_of("/a.yaml/flags")).to eq(:json) + expect(FileData.format_of("/a/flags.yaml.bak")).to eq(:json) + end + end + + describe "blank?" do + it "is true for content that is empty or only white space" do + expect(FileData.blank?("")).to be true + expect(FileData.blank?(" \t\r\n\f\v")).to be true + expect(FileData.blank?(" {}")).to be false + expect(FileData.blank?("# comment")).to be false + expect(FileData.blank?("\xFF".b)).to be false + end + end + describe "make_flag_with_value" do it "builds a flag that is on and serves the value as its fallthrough" do flag = FileData.make_flag_with_value("flag1", "value1") diff --git a/spec/impl/file_data/poller_spec.rb b/spec/impl/file_data/poller_spec.rb index 14eb503e..88e820f2 100644 --- a/spec/impl/file_data/poller_spec.rb +++ b/spec/impl/file_data/poller_spec.rb @@ -40,6 +40,18 @@ def with_poller(paths) end end + it "observes the state of the file that a symbolic link leads to" do + skip "symbolic links need extra privileges on Windows" if Gem.win_platform? + File.write(path("target.json"), "{}") + File.symlink(path("target.json"), path("link.json")) + File.utime(Time.at(1_000_000), Time.at(1_000_000), path("target.json")) + + state = Poller.observe(path("link.json")) + + expect(state).to eq(Poller::FileState.new(true, Time.at(1_000_000), 2)) + expect(Poller.observe(path("missing.json")).exists).to be false + end + it "invokes the callback when a file's modification time changes" do File.write(path("a.json"), "{}") with_poller([path("a.json")]) do |_poller, calls| diff --git a/spec/impl/file_data/reloader_spec.rb b/spec/impl/file_data/reloader_spec.rb index 5f8b8df5..df3807c6 100644 --- a/spec/impl/file_data/reloader_spec.rb +++ b/spec/impl/file_data/reloader_spec.rb @@ -2,6 +2,7 @@ require "spec_helper" require "capturing_logger" +require "in_place_writes" require "tmpdir" require "ldclient-rb/impl/file_data" @@ -20,8 +21,15 @@ def path(name) File.join(@dir, name) end + # Applications rewrite data files in place, which is not atomic. The tests do too, deliberately + # slowly: a file that exists is truncated, and written 30 ms later, so a load can find it + # empty, as it would with an application's writes, instead of only by chance. def write(name, content) - File.write(path(name), content) + if File.exist?(path(name)) + InPlaceWrites.rewrite(path(name), content, 0.03) + else + File.write(path(name), content) + end path(name) end @@ -197,11 +205,11 @@ def with_reloader(paths, logger: $null_log, reject: 0, **options) expect(reloader.reload_now).to be false write("a.json", values_doc({ flag1: "fixed" })) - # The retry recovers. The write is ordinary, not atomic, so the retry may read the - # file while it is still empty: an empty document is valid and is applied as the - # recovery, and the fixed content is then loaded on demand. + # The retry recovers with the fixed content. The write is in place, so a retry can + # find the file empty while it is being written. It reads the file again later + # instead of applying it. expect(wait_for { recorder.applied.length >= 1 }).to be true - expect(reloader.reload_now).to be true if recorder.flag_values != { flag1: "fixed" } + expect(recorder.applied.map { |merged| merged.flags.keys }).to all(eq([:flag1])) expect(recorder.flag_values).to eq({ flag1: "fixed" }) end end @@ -439,6 +447,1459 @@ def with_reloader(paths, logger: $null_log, reject: 0, **options) expect(logger.output).to include("Reloading flag data after detecting a change") end end + + it "writes the configured prefix once on the log line for an unexpected error" do + a = write("a.json", values_doc({ flag1: "a" })) + logger = CapturingLogger.new + allow(FileData).to receive(:read_file).and_raise(RuntimeError, "unexpected") + + with_reloader([a], logger: logger, debounce_delay: 0, log_prefix: "[LDClient] Prefix:") do |reloader, _recorder| + reloader.trigger + + expect(wait_for { logger.output.include?("Unexpected error") }).to be true + expect(logger.output).to match(/ERROR -- : \[LDClient\] Prefix: Unexpected error in file data reloader/) + end + end + + describe "a file that is found empty" do + # The retry comes long enough after the change signal for a test to finish writing the + # file before it, even on a slow machine. + let(:retry_delay) { 1.5 } + + # Shorter than the retry delay, so a load that happens within it after a change signal + # is not the retry. + let(:before_the_retry) { 1.0 } + + let(:logger) { CapturingLogger.new } + + let(:read_again_log) { "reading it again before applying it" } + + def yaml_doc(values) + "flagValues:\n" + values.map { |key, value| " #{key}: #{value}\n" }.join + end + + def read_again_logged? + logger.output.include?(read_again_log) + end + + def hold_count + logger.output.scan(read_again_log).length + end + + def with_empty_file_reloader(paths, **options, &block) + with_reloader(paths, logger: logger, debounce_delay: 0.01, retry_delay: retry_delay, **options, &block) + end + + it "is held back before it is applied" do + # A writer that rewrites the file in place truncates it and then writes the new content. + # In YAML an empty file is valid, so applying it would briefly remove every entry. + y = write("a.yaml", yaml_doc(flag1: true)) + + with_empty_file_reloader([y]) do |reloader, recorder| + reloader.reload_now + File.write(y, "") + reloader.trigger + expect(wait_for { read_again_logged? }).to be true + expect(logger.output).to include("DEBUG -- : [LDClient] File #{y} is empty, possibly while it is being rewritten") + + # The retry reads the rewritten file. + File.write(y, yaml_doc(flag2: true)) + expect(wait_for(retry_delay + 2) { recorder.applied.length == 2 }).to be true + expect(recorder.flag_values).to eq({ flag2: true }) + expect(recorder.errors).to be_empty + expect(logger.output).not_to include("ERROR") + end + end + + it "counts a file with only white space as empty" do + y = write("a.yaml", yaml_doc(flag1: true)) + + with_empty_file_reloader([y]) do |reloader, recorder| + reloader.reload_now + File.write(y, " \n\r\n \n") + reloader.trigger + + expect(wait_for { read_again_logged? }).to be true + expect(recorder.applied.length).to eq(1) + end + end + + it "is applied by the retry when a YAML file stays empty" do + y = write("a.yaml", yaml_doc(flag1: true)) + + with_empty_file_reloader([y]) do |reloader, recorder| + reloader.reload_now + File.write(y, "") + reloader.trigger + expect(wait_for { read_again_logged? }).to be true + expect(wait_for(before_the_retry) { recorder.applied.length > 1 }).to be false + + # The file is still empty when the retry reads it, so it is a document with no entries. + expect(wait_for(retry_delay + 2) { recorder.applied.length == 2 }).to be true + expect(recorder.applied[-1].empty?).to be true + expect(recorder.errors).to be_empty + end + end + + it "is not held back again once its empty state is applied" do + y = write("a.yaml", yaml_doc(flag1: true)) + + with_empty_file_reloader([y]) do |reloader, recorder| + reloader.reload_now + File.write(y, "") + reloader.trigger + expect(wait_for(retry_delay + 2) { recorder.applied.length == 2 }).to be true + + # The applied data has no content from the file, so a new truncation is applied at once. + File.write(y, "") + reloader.trigger + expect(wait_for(before_the_retry) { recorder.applied.length == 3 }).to be true + expect(recorder.applied[-1].empty?).to be true + expect(logger.output.scan(read_again_log).length).to eq(1) + end + end + + it "is not reported by the load that finds an emptied JSON file, and the retry reports it" do + a = write("a.json", values_doc({ flag1: "a" })) + + with_empty_file_reloader([a]) do |reloader, recorder| + reloader.reload_now + File.write(a, "") + reloader.trigger + expect(wait_for { read_again_logged? }).to be true + expect(recorder.errors).to be_empty + + # The retry finds the file still empty, which is not a JSON document. The data stays. + expect(wait_for(retry_delay + 2) { recorder.errors.length == 1 }).to be true + expect(recorder.errors[0]).to be_a(ReadError) + expect(recorder.errors[0].message).to include("the file has no content, which is not a JSON document") + expect(recorder.applied.length).to eq(1) + end + end + + it "is not applied by a further change signal within the retry delay" do + y = write("a.yaml", yaml_doc(flag1: true)) + + with_empty_file_reloader([y]) do |reloader, recorder| + reloader.reload_now + File.write(y, "") + reloader.trigger + expect(wait_for { read_again_logged? }).to be true + + # The file has not been written since, so the delay goes on from the first load that + # found it empty, and a stream of change signals cannot hold it back longer. + reloader.trigger + expect(wait_for { logger.output.scan(read_again_log).length == 2 }).to be true + expect(recorder.applied.length).to eq(1) + expect(wait_for(retry_delay + 2) { recorder.applied.length == 2 }).to be true + expect(recorder.applied[-1].empty?).to be true + end + end + + it "is held back again when a second rewrite truncates it before a load reads its content" do + # Two quick in-place rewrites: the write of the first and the truncation of the second + # come before the next load, which finds the file empty again. It is a new rewrite, so + # the delay starts again. + y = write("a.yaml", yaml_doc(flag1: true)) + + with_empty_file_reloader([y]) do |reloader, recorder| + reloader.reload_now + File.write(y, "") + reloader.trigger + expect(wait_for { read_again_logged? }).to be true + + sleep 0.02 + File.write(y, yaml_doc(flag2: true)) + File.write(y, "") + reloader.trigger + expect(wait_for { logger.output.scan(read_again_log).length == 2 }).to be true + + File.write(y, yaml_doc(flag3: true)) + reloader.trigger + expect(wait_for(before_the_retry) { recorder.applied.length == 2 }).to be true + expect(recorder.flag_values).to eq({ flag3: true }) + end + end + + it "is held back again when the retry finds a second truncation" do + # A retry, or the next poll, can find the file empty again after a second rewrite started. + y = write("a.yaml", yaml_doc(flag1: true)) + + with_empty_file_reloader([y]) do |reloader, recorder| + reloader.reload_now + File.write(y, "") + reloader.trigger + expect(wait_for { read_again_logged? }).to be true + + # No change signal from here on: only the retries read the file. + sleep 0.02 + File.write(y, yaml_doc(flag2: true)) + File.write(y, "") + expect(wait_for(retry_delay + 2) { logger.output.scan(read_again_log).length == 2 }).to be true + expect(recorder.applied.length).to eq(1) + + File.write(y, yaml_doc(flag3: true)) + expect(wait_for(retry_delay + 2) { recorder.applied.length == 2 }).to be true + expect(recorder.flag_values).to eq({ flag3: true }) + end + end + + it "is held back again when a change to another file starts a load within the delay" do + a = write("a.yaml", yaml_doc(flag1: true)) + b = write("b.yaml", yaml_doc(flag2: true)) + + with_empty_file_reloader([a, b]) do |reloader, recorder| + reloader.reload_now + File.write(a, "") + reloader.trigger + expect(wait_for { read_again_logged? }).to be true + + # The load for the change to b finds a still empty, within the delay. + File.write(b, yaml_doc(flag2: false)) + reloader.trigger + expect(wait_for { logger.output.scan(read_again_log).length == 2 }).to be true + expect(recorder.applied.length).to eq(1) + + File.write(a, yaml_doc(flag1: false)) + reloader.trigger + expect(wait_for(before_the_retry) { recorder.applied.length == 2 }).to be true + expect(recorder.flag_values).to eq({ flag1: false, flag2: false }) + end + end + + it "is held back after the same content moved to it from another file" do + # The same content moves from one file to another. The data in effect now comes from the + # second file, so the move is applied rather than skipped as unchanged, and emptying the + # second file is held back. + a = write("a.yaml", yaml_doc(flag1: true)) + b = path("b.yaml") + + with_empty_file_reloader([a, b], skip_missing_paths: true, skip_unchanged: true) do |reloader, recorder| + expect(reloader.reload_now).to be true + File.write(b, yaml_doc(flag1: true)) + File.delete(a) + expect(reloader.reload_now).to be true + expect(recorder.applied.length).to eq(2) + expect(recorder.applied[-1].files.map(&:present)).to eq([false, true]) + + File.write(b, "") + expect(reloader.reload_now).to be false + expect(recorder.applied.length).to eq(2) + end + end + + it "applies the same bytes moved to a file of another format" do + # 1e3 is a number in JSON and a string in YAML, so the move is not skipped as unchanged. + a = write("a.json", '{"flagValues": {"f": 1e3}}') + b = path("b.yaml") + + with_reloader([a, b], skip_missing_paths: true, skip_unchanged: true) do |reloader, recorder| + reloader.reload_now + expect(recorder.flag_values).to eq({ f: 1000.0 }) + + File.delete(a) + File.write(b, '{"flagValues": {"f": 1e3}}') + expect(reloader.reload_now).to be true + expect(recorder.flag_values).to eq({ f: "1e3" }) + end + end + + it "does not keep a load from failing on another file that cannot be parsed" do + a = write("a.yaml", yaml_doc(flag1: true)) + b = write("b.json", values_doc({ flag2: "b" })) + + with_empty_file_reloader([a, b]) do |reloader, recorder| + reloader.reload_now + File.write(a, "") + File.write(b, '{"flagValues"') + reloader.trigger + + # The load that puts a off reports b's failure, before the retry. + expect(wait_for(before_the_retry) { recorder.errors.length == 1 }).to be true + expect(recorder.errors[0].path).to eq(b) + expect(recorder.applied.length).to eq(1) + end + end + + it "does not delay the other files' changes when it is an empty placeholder from the start" do + # An empty YAML file next to the data, such as a placeholder for later entries, has never + # had content, so it is never read again. + a = write("a.json", values_doc({ flag1: "a" })) + placeholder = write("placeholder.yaml", "") + + with_empty_file_reloader([a, placeholder]) do |reloader, recorder| + reloader.reload_now + expect(recorder.flag_values).to eq({ flag1: "a" }) + + File.write(a, values_doc({ flag1: "b" })) + reloader.trigger + expect(wait_for(before_the_retry) { recorder.applied.length == 2 }).to be true + expect(recorder.flag_values).to eq({ flag1: "b" }) + expect(read_again_logged?).to be false + end + end + + it "is applied at once when it was missing at its last read" do + y = write("a.yaml", yaml_doc(flag1: true)) + + with_empty_file_reloader([y], skip_missing_paths: true) do |reloader, recorder| + reloader.reload_now + File.delete(y) + reloader.trigger + expect(wait_for { recorder.applied.length == 2 }).to be true + expect(recorder.applied[-1].empty?).to be true + + # A missing file has no content, so the empty file that replaces it is not read again. + File.write(y, "") + reloader.trigger + expect(wait_for(before_the_retry) { recorder.applied.length == 3 }).to be true + expect(recorder.applied[-1].empty?).to be true + expect(read_again_logged?).to be false + end + end + + it "is held back by a retry left over from an earlier failure instead of applied empty" do + y = write("a.yaml", yaml_doc(flag1: true)) + + with_empty_file_reloader([y]) do |reloader, recorder| + reloader.reload_now + File.write(y, "flagValues: [") + reloader.trigger + expect(wait_for { recorder.errors.length == 1 }).to be true + + # No change signal from here on: only the retries read the file. + File.write(y, "") + expect(wait_for(retry_delay + 2) { read_again_logged? }).to be true + File.write(y, yaml_doc(flag2: true)) + expect(wait_for(retry_delay + 2) { recorder.applied.length == 2 }).to be true + expect(recorder.flag_values).to eq({ flag2: true }) + end + end + + it "makes reload_now return false and arms the retry" do + y = write("a.yaml", yaml_doc(flag1: true)) + + with_empty_file_reloader([y]) do |reloader, recorder| + expect(reloader.reload_now).to be true + File.write(y, "") + + expect(reloader.reload_now).to be false + expect(read_again_logged?).to be true + expect(recorder.errors).to be_empty + + # The retry applies the file, which is still empty. + expect(wait_for(retry_delay + 2) { recorder.applied.length == 2 }).to be true + expect(recorder.applied[-1].empty?).to be true + end + end + + it "neither logs nor arms a retry when the reloader is stopped while the files are read" do + y = write("a.yaml", yaml_doc(flag1: true)) + + with_empty_file_reloader([y]) do |reloader, recorder| + reloader.reload_now + allow(FileData).to receive(:read_file).and_wrap_original do |_original, _path| + reloader.stop + "" + end + + expect(reloader.reload_now).to be true + expect(read_again_logged?).to be false + expect(recorder.applied.length).to eq(1) + end + end + + it "is applied at once when there are no automatic retries" do + y = write("a.yaml", yaml_doc(flag1: true)) + + with_reloader([y], logger: logger, retry_delay: 0) do |reloader, recorder| + reloader.reload_now + + # Without a retry nothing would read the file again, so the load applies it as it is. + File.write(y, "") + expect(reloader.reload_now).to be true + expect(recorder.applied[-1].empty?).to be true + expect(read_again_logged?).to be false + end + end + + it "is a document with no entries at the initial load when it is YAML" do + y = write("a.yaml", "") + + with_empty_file_reloader([y]) do |reloader, recorder| + expect(reloader.reload_now).to be true + expect(recorder.applied[0].empty?).to be true + end + end + + it "fails to parse at the initial load when it is JSON" do + a = write("a.json", "") + + with_empty_file_reloader([a]) do |reloader, recorder| + expect(reloader.reload_now).to be false + expect(recorder.applied).to be_empty + expect(recorder.errors[0].message).to include("the file has no content, which is not a JSON document") + end + end + + it "says that the retry after a hold reads the files again because a file was found empty" do + y = write("a.yaml", yaml_doc(flag1: true)) + + with_reloader([y], logger: logger, debounce_delay: 0.01, retry_delay: 0.2) do |reloader, recorder| + reloader.reload_now + File.write(y, "") + reloader.trigger + expect(wait_for { recorder.applied.length == 2 }).to be true + expect(logger.output).to include("DEBUG -- : [LDClient] Reading flag data again after a file was found empty") + expect(logger.output).not_to include("Retrying flag data load after earlier failure") + + File.write(y, "flagValues: [") + reloader.trigger + expect(wait_for { logger.output.include?("Retrying flag data load after earlier failure") }).to be true + end + end + + it "keeps reloading after an error that a load does not expect" do + y = write("a.yaml", yaml_doc(flag1: true)) + failures = 1 + allow(FileData).to receive(:read_file).and_wrap_original do |original, path| + raise "unexpected" if (failures -= 1) >= 0 + + original.call(path) + end + + with_reloader([y], logger: logger, debounce_delay: 0.01, retry_delay: 0.2) do |reloader, recorder| + reloader.trigger + expect(wait_for { logger.output.include?("Unexpected error in file data reloader") }).to be true + + # The retry loads the file, and so does a later change signal. + expect(wait_for { recorder.applied.length == 1 }).to be true + File.write(y, yaml_doc(flag2: true)) + reloader.trigger + expect(wait_for { recorder.applied.length == 2 }).to be true + expect(recorder.flag_values).to eq({ flag2: true }) + end + end + + it "keeps the worker running after an error outside a load" do + y = write("a.yaml", yaml_doc(flag1: true)) + + with_reloader([y], logger: logger, debounce_delay: 0.01, retry_delay: 0.2) do |reloader, recorder| + failures = 1 + allow(reloader).to receive(:reload).and_wrap_original do |original, **options| + raise "unexpected" if (failures -= 1) >= 0 + + original.call(**options) + end + reloader.trigger + expect(wait_for { logger.output.include?("Unexpected error in file data reloader") }).to be true + + reloader.trigger + expect(wait_for { recorder.applied.length == 1 }).to be true + end + end + + it "replaces an earlier retry with the retry of a later hold" do + # A failure arms the retry for the retry delay. A load soon after holds a file back and + # arms its own retry for the end of that file's delay, which replaces the earlier one, + # so the file is not read again before its delay is over. + a = write("a.yaml", yaml_doc(flag1: true)) + b = write("b.json", values_doc({ flag2: "b" })) + + with_reloader([a, b], logger: logger, debounce_delay: 0.01, retry_delay: 1.0) do |reloader, recorder| + reloader.reload_now + File.write(b, '{"flagValues"') + reloader.trigger + expect(wait_for { recorder.errors.length == 1 }).to be true + + sleep 0.3 + File.write(a, "") + File.write(b, values_doc({ flag2: "b" })) + reloader.trigger + expect(wait_for { hold_count == 1 }).to be true + expect(wait_for(3) { recorder.applied.length == 2 }).to be true + expect(hold_count).to eq(1) + end + end + + it "arms the retry of a hold for the rest of the delay, plus a tenth of it" do + y = write("a.yaml", yaml_doc(flag1: true)) + + with_reloader([y], logger: logger, debounce_delay: 0.01, retry_delay: 2.0) do |reloader, recorder| + reloader.reload_now + File.write(y, "") + reloader.trigger + expect(wait_for { hold_count == 1 }).to be true + first_hold = Time.now + + # A load halfway through the delay holds the file back again, and arms the retry for + # the rest of the delay, not for a whole delay. + sleep 1.0 + reloader.trigger + expect(wait_for { hold_count == 2 }).to be true + expect(wait_for(4) { recorder.applied.length == 2 }).to be true + expect(Time.now - first_hold).to be_between(1.9, 2.7) + end + end + end + + describe "holding back a file, on a controlled clock" do + # A monotonic clock that only moves when a test moves it. + class FakeClock + attr_reader :now + + def initialize + @now = 1000.0 + end + + def advance(seconds) + @now += seconds + end + end + + # A retry delay of an hour on the controlled clock, so that no real retry fires during a + # test. Each load is a synchronous reload_now. + let(:hour) { 3600.0 } + let(:clock) { FakeClock.new } + let(:logger) { CapturingLogger.new } + + def yaml_doc(values) + "flagValues:\n" + values.map { |key, value| " #{key}: #{value}\n" }.join + end + + def hold_count + logger.output.scan("reading it again before applying it").length + end + + # Lets the next write get a later modification time on a file system whose timestamps only + # move once per clock tick of the kernel. + def next_tick + sleep 0.02 + end + + # Waits for the worker to arm the retry that a load asked for, and expects its delay. A + # retry that reload_now asks for is armed by the worker. + def expect_armed_retry_delay(reloader, seconds) + armed = wait_for do + delay = reloader.last_armed_retry_delay + !delay.nil? && (delay - seconds).abs < 0.001 + end + expect(armed).to be(true), "expected a retry armed for #{seconds}s, got #{reloader.last_armed_retry_delay.inspect}" + end + + # Gives the file a new modification time, as a write while it is empty does. + def mark_written(path) + @marks = (@marks || 0) + 1 + File.utime(Time.at(@marks), Time.at(@marks), path) + end + + def with_clocked_reloader(paths, **options, &block) + with_reloader(paths, logger: logger, retry_delay: hour, clock: clock.method(:now), **options, &block) + end + + it "holds the file back until it has stayed empty for exactly the retry delay" do + y = write("a.yaml", yaml_doc(flag1: true)) + + with_clocked_reloader([y]) do |reloader, recorder| + reloader.reload_now + File.write(y, "") + expect(reloader.reload_now).to be false + + clock.advance(0.95 * hour) + expect(reloader.reload_now).to be false + expect(recorder.applied.length).to eq(1) + + clock.advance(0.05 * hour) + expect(reloader.reload_now).to be true + expect(recorder.applied[-1].empty?).to be true + end + end + + it "starts the delay over when the file is written while it is empty" do + y = write("a.yaml", yaml_doc(flag1: true)) + + with_clocked_reloader([y]) do |reloader, recorder| + reloader.reload_now + File.write(y, "") + expect(reloader.reload_now).to be false + + # Rewritten and truncated again between two loads. + clock.advance(0.6 * hour) + next_tick + File.write(y, yaml_doc(flag2: true)) + File.write(y, "") + expect(reloader.reload_now).to be false + + clock.advance(0.6 * hour) + expect(reloader.reload_now).to be false + + clock.advance(0.4 * hour) + expect(reloader.reload_now).to be true + expect(recorder.applied[-1].empty?).to be true + end + end + + it "reads the state of the file a symbolic link leads to" do + skip "symbolic links need extra privileges on Windows" if Gem.win_platform? + target = write("target.yaml", yaml_doc(flag1: true)) + link = path("link.yaml") + File.symlink(target, link) + + with_clocked_reloader([link]) do |reloader, recorder| + reloader.reload_now + File.write(target, "") + expect(reloader.reload_now).to be false + + clock.advance(0.6 * hour) + next_tick + File.write(target, yaml_doc(flag2: true)) + File.write(target, "") + expect(reloader.reload_now).to be false + + # The target was written, so its delay started again. + clock.advance(0.6 * hour) + expect(reloader.reload_now).to be false + expect(recorder.applied.length).to eq(1) + end + end + + it "stops holding back after five retry delays while one file keeps being rewritten" do + y = write("a.yaml", yaml_doc(flag1: true)) + + with_clocked_reloader([y]) do |reloader, recorder| + reloader.reload_now + File.write(y, "") + expect(reloader.reload_now).to be false + + # Truncating the file again changes its modification time, so every load finds a new + # rewrite. + 9.times do + clock.advance(0.5 * hour) + next_tick + File.write(y, "") + expect(reloader.reload_now).to be false + end + + clock.advance(0.5 * hour) + next_tick + File.write(y, "") + expect(reloader.reload_now).to be true + expect(recorder.applied[-1].empty?).to be true + end + end + + it "stops holding back after five retry delays while two files are rewritten in turn" do + a = write("a.yaml", yaml_doc(flag1: true)) + b = write("b.yaml", yaml_doc(flag2: true)) + + with_clocked_reloader([a, b]) do |reloader, recorder| + reloader.reload_now + File.write(a, "") + expect(reloader.reload_now).to be false + + # Each load finds one of the files in the middle of a rewrite and the other one whole. + 9.times do |i| + clock.advance(0.5 * hour) + next_tick + emptied, rewritten = i.even? ? [b, a] : [a, b] + File.write(rewritten, yaml_doc(rewritten == a ? { flag1: true } : { flag2: true })) + File.write(emptied, "") + expect(reloader.reload_now).to be false + end + expect(recorder.applied.length).to eq(1) + + clock.advance(0.5 * hour) + File.write(b, yaml_doc(flag2: true)) + File.write(a, "") + expect(reloader.reload_now).to be true + expect(recorder.flag_values).to eq({ flag2: true }) + end + end + + it "applies files that are emptied together after one retry delay" do + a = write("a.yaml", yaml_doc(flag1: true)) + b = write("b.yaml", yaml_doc(flag2: true)) + + with_clocked_reloader([a, b]) do |reloader, recorder| + reloader.reload_now + File.write(a, "") + File.write(b, "") + expect(reloader.reload_now).to be false + expect(hold_count).to eq(2) + + clock.advance(hour) + expect(reloader.reload_now).to be true + expect(recorder.applied[-1].empty?).to be true + end + end + + it "holds back YAML that is only a comment or a document marker, and applies {} at once" do + y = write("a.yaml", yaml_doc(flag1: true)) + + with_clocked_reloader([y]) do |reloader, recorder| + reloader.reload_now + File.write(y, "# overrides for the incident\n") + expect(reloader.reload_now).to be false + File.write(y, "---\n") + expect(reloader.reload_now).to be false + expect(recorder.applied.length).to eq(1) + + File.write(y, "{}\n") + expect(reloader.reload_now).to be true + expect(recorder.applied[-1].empty?).to be true + end + end + + it "applies a JSON file that is {} at once" do + a = write("a.json", values_doc({ flag1: "a" })) + + with_clocked_reloader([a]) do |reloader, recorder| + reloader.reload_now + File.write(a, "{}") + expect(reloader.reload_now).to be true + expect(recorder.applied[-1].empty?).to be true + expect(hold_count).to eq(0) + end + end + + it "keeps parsing a JSON file that stays empty, without holding it back again" do + a = write("a.json", values_doc({ flag1: "a" })) + + with_clocked_reloader([a]) do |reloader, recorder| + reloader.reload_now + File.write(a, "") + expect(reloader.reload_now).to be false + + clock.advance(hour) + expect(reloader.reload_now).to be false + expect(recorder.errors.length).to eq(1) + clock.advance(hour) + expect(reloader.reload_now).to be false + expect(hold_count).to eq(1) + expect(recorder.applied.length).to eq(1) + end + end + + it "holds back a JSON file that stayed empty for long when it is rewritten" do + # The failing loads held nothing back, so they started the file's bound over, and the + # rewrite is held back although the file was first found empty more than five retry + # delays ago. + a = write("a.json", values_doc({ flag1: "a" })) + + with_clocked_reloader([a]) do |reloader, recorder| + reloader.reload_now + File.write(a, "") + expect(reloader.reload_now).to be false + clock.advance(hour) + expect(reloader.reload_now).to be false + clock.advance(5 * hour) + expect(reloader.reload_now).to be false + expect(recorder.errors.length).to eq(1) + + next_tick + File.write(a, "") + expect(reloader.reload_now).to be false + expect(hold_count).to eq(2) + File.write(a, values_doc({ flag1: "b" })) + expect(reloader.reload_now).to be true + expect(recorder.flag_values).to eq({ flag1: "b" }) + expect(recorder.errors.length).to eq(1) + end + end + + it "holds back a file emptied again after its content was read, even if its state looks the same" do + # On a file system with coarse modification times, a rewrite can leave the state of an + # empty file unchanged. + y = write("a.yaml", yaml_doc(flag1: true)) + allow(Poller).to receive(:observe).and_return(Poller::FileState.new(true, Time.at(0), 0)) + + with_clocked_reloader([y]) do |reloader, _recorder| + reloader.reload_now + File.write(y, "") + expect(reloader.reload_now).to be false + File.write(y, yaml_doc(flag2: true)) + expect(reloader.reload_now).to be true + + clock.advance(2 * hour) + File.write(y, "") + expect(reloader.reload_now).to be false + end + end + + it "starts a file's delay again after a load reads it with content, even if its state looks the same" do + # The load that reads a.yaml with content holds b.yaml back, so nothing is applied in between. + a = write("a.yaml", yaml_doc(flag1: true)) + b = write("b.yaml", yaml_doc(flag2: true)) + allow(Poller).to receive(:observe).and_return(Poller::FileState.new(true, Time.at(0), 0)) + + with_clocked_reloader([a, b]) do |reloader, recorder| + reloader.reload_now + File.write(a, "") + expect(reloader.reload_now).to be false + + clock.advance(0.5 * hour) + File.write(a, yaml_doc(flag1: true)) + File.write(b, "") + expect(reloader.reload_now).to be false + + clock.advance(0.1 * hour) + File.write(a, "") + File.write(b, yaml_doc(flag2: true)) + expect(reloader.reload_now).to be false + + # a.yaml's delay started again at 0.6 hours, not at the start. + clock.advance(0.6 * hour) + expect(reloader.reload_now).to be false + expect(recorder.applied.length).to eq(1) + end + end + + it "holds a file emptied again after its content was read back again when its state looks the same" do + # On a file system that records modification times coarsely, a file emptied a second time + # can look exactly as it did the first time. The read of its content in between still + # starts its delay over. b.yaml is kept written while empty, so that no load applies. + a = write("a.yaml", yaml_doc(flag1: true)) + b = write("b.yaml", yaml_doc(flag2: true)) + + with_clocked_reloader([a, b]) do |reloader, recorder| + reloader.reload_now + File.write(a, "") + first_empty_write_time = File.mtime(a) + File.write(b, "") + expect(reloader.reload_now).to be false + + clock.advance(50.0 / 60 * hour) + File.write(a, yaml_doc(flag1: true)) + mark_written(b) + expect(reloader.reload_now).to be false + + clock.advance(20.0 / 60 * hour) + File.write(a, "") + File.utime(first_empty_write_time, first_empty_write_time, a) + mark_written(b) + expect(reloader.reload_now).to be false + + # b.yaml gets its content back while a.yaml is still within its new delay. + clock.advance(10.0 / 60 * hour) + File.write(b, yaml_doc(flag2: true)) + expect(reloader.reload_now).to be false + expect(recorder.applied.length).to eq(1) + end + end + + it "holds back a file created empty after it was missing, even if its state looks the same" do + y = write("a.yaml", yaml_doc(flag1: true)) + allow(Poller).to receive(:observe).and_return(Poller::FileState.new(true, Time.at(0), 0)) + + with_clocked_reloader([y]) do |reloader, recorder| + reloader.reload_now + File.write(y, "") + expect(reloader.reload_now).to be false + + # Without skipping missing paths, a missing file fails the load and the data stays. + File.delete(y) + expect(reloader.reload_now).to be false + expect(recorder.errors.length).to eq(1) + + clock.advance(2 * hour) + File.write(y, "") + expect(reloader.reload_now).to be false + expect(recorder.applied.length).to eq(1) + end + end + + it "does not let a slow read of an earlier file shorten a later file's delay" do + # The read of b.yaml, which comes first, takes most of a retry delay. a.yaml's delay is + # counted from when it was read. + a = write("a.yaml", yaml_doc(flag1: true)) + b = write("b.yaml", yaml_doc(flag2: true)) + slow = nil + allow(FileData).to receive(:read_file).and_wrap_original do |original, path| + content = original.call(path) + if path == slow + slow = nil + clock.advance(0.9 * hour) + end + content + end + + with_clocked_reloader([b, a]) do |reloader, recorder| + reloader.reload_now + File.write(a, "") + slow = b + expect(reloader.reload_now).to be false + + clock.advance(0.2 * hour) + expect(reloader.reload_now).to be false + expect(recorder.applied.length).to eq(1) + end + end + + it "does not start the bound of a held file over when another file fails to load" do + a = write("a.yaml", yaml_doc(flag1: true)) + c = write("c.json", values_doc({ flag3: "c" })) + + with_clocked_reloader([a, c]) do |reloader, recorder| + reloader.reload_now + File.write(a, "") + expect(reloader.reload_now).to be false + + # a.yaml keeps being written while empty, and every other load fails on c.json. + 9.times do |i| + clock.advance(0.5 * hour) + mark_written(a) + File.write(c, i.even? ? '{"flagValues"' : values_doc({ flag3: "c" })) + expect(reloader.reload_now).to be false + end + expect(recorder.applied.length).to eq(1) + + clock.advance(0.5 * hour) + mark_written(a) + File.write(c, values_doc({ flag3: "c" })) + expect(reloader.reload_now).to be true + expect(recorder.flag_values).to eq({ flag3: "c" }) + end + end + + it "does not cut a file's rewrite short when another file reaches its bound" do + # b.yaml keeps being written while empty, and reaches its bound while a.yaml is in the + # middle of an ordinary rewrite. a.yaml is still held back. + a = write("a.yaml", yaml_doc(flag1: true)) + b = write("b.yaml", yaml_doc(flag2: true)) + + with_clocked_reloader([a, b]) do |reloader, recorder| + reloader.reload_now + File.write(b, "") + expect(reloader.reload_now).to be false + 9.times do |i| + clock.advance(0.5 * hour) + mark_written(b) + File.write(a, "") if i == 8 + expect(reloader.reload_now).to be false + end + + clock.advance(0.5 * hour) + mark_written(b) + expect(reloader.reload_now).to be false + + clock.advance(0.25 * hour) + File.write(a, yaml_doc(flag3: true)) + expect(reloader.reload_now).to be true + expect(recorder.flag_values).to eq({ flag3: true }) + end + end + + it "starts the bound over once data is applied" do + y = write("a.yaml", yaml_doc(flag1: true)) + + with_clocked_reloader([y]) do |reloader, recorder| + reloader.reload_now + File.write(y, "") + expect(reloader.reload_now).to be false + 8.times do + clock.advance(0.5 * hour) + mark_written(y) + expect(reloader.reload_now).to be false + end + File.write(y, yaml_doc(flag2: true)) + expect(reloader.reload_now).to be true + + # Emptied again: a new bound of five retry delays, not the rest of the old one, which + # would be over at 5 hours. + clock.advance(0.1 * hour) + File.write(y, "") + expect(reloader.reload_now).to be false + 2.times do + clock.advance(0.5 * hour) + mark_written(y) + expect(reloader.reload_now).to be false + end + expect(recorder.flag_values).to eq({ flag2: true }) + end + end + + it "holds back a file again later after a rewrite with the same content was skipped as unchanged" do + # A tool rewrites the file in place now and then with the same content. The load that + # finds the content again is skipped as unchanged, and starts the file's bound over, as + # an application does. + y = write("a.yaml", yaml_doc(flag1: true)) + + with_clocked_reloader([y], skip_unchanged: true) do |reloader, recorder| + reloader.reload_now + File.write(y, "") + expect(reloader.reload_now).to be false + File.write(y, yaml_doc(flag1: true)) + expect(reloader.reload_now).to be true + expect(recorder.applied.length).to eq(1) + + clock.advance(6 * hour) + File.write(y, "") + expect(reloader.reload_now).to be false + expect(recorder.applied.length).to eq(1) + end + end + + it "applies files emptied in staggered turns after the bound" do + # Two files stay empty and are truncated again in turn, so that whenever one of them has + # stayed empty for the whole delay, the other is still within its delay and holds the + # load back. A file that settles so does not get a fresh bound while the other is held. + a = write("a.yaml", yaml_doc(flag1: true)) + b = write("b.yaml", yaml_doc(flag2: true)) + + with_clocked_reloader([a, b]) do |reloader, recorder| + reloader.reload_now + File.write(a, "") + expect(reloader.reload_now).to be false + + # One file is truncated again every 0.9 retry delays, the files in turn, with a load at + # each truncation and one 0.2 retry delays later, which stands for the retry that the + # hold arms a little after the other file's delay is over. + applied = false + (1..14).each do |i| + clock.advance(i == 1 ? 0.9 * hour : 0.7 * hour) + mark_written(i.odd? ? b : a) + File.write(b, "") if i == 1 + reloader.reload_now + clock.advance(0.2 * hour) + reloader.reload_now + applied = recorder.applied.length > 1 + break if applied + end + expect(applied).to be true + end + end + + it "starts the bound of a file held back during a failure over once a load reads it again" do + # While b.json fails to load, a.yaml is rewritten once and its gap is held back. Later + # b.json is fixed while a.yaml is in the middle of another rewrite, which is held back + # with a bound of its own. + a = write("a.yaml", yaml_doc(flag1: true)) + b = write("b.json", values_doc({ flag3: "c" })) + + with_clocked_reloader([a, b]) do |reloader, recorder| + reloader.reload_now + File.write(b, '{"flagValues"') + File.write(a, "") + expect(reloader.reload_now).to be false + File.write(a, yaml_doc(flag2: true)) + expect(reloader.reload_now).to be false + clock.advance(6 * hour) + expect(reloader.reload_now).to be false + + # The load that held a.yaml back first failed on b.json, so that hold was not logged. + File.write(b, values_doc({ flag3: "c" })) + File.write(a, "") + expect(reloader.reload_now).to be false + expect(hold_count).to eq(1) + expect(recorder.applied.length).to eq(1) + end + end + + it "asks for the retry from the end of the load's reads" do + # The read of the file after the held one takes longer than what is left of the held + # file's delay, so the retry is due right away, after the margin. + a = write("a.yaml", yaml_doc(flag1: true)) + b = write("b.yaml", yaml_doc(flag2: true)) + slow = nil + allow(FileData).to receive(:read_file).and_wrap_original do |original, path| + content = original.call(path) + if path == slow + slow = nil + clock.advance(0.1 * hour) + end + content + end + + with_clocked_reloader([a, b]) do |reloader, _recorder| + reloader.reload_now + File.write(a, "") + expect(reloader.reload_now).to be false + + clock.advance(0.95 * hour) + slow = b + expect(reloader.reload_now).to be false + expect_armed_retry_delay(reloader, 0.1 * hour) + end + end + + [ + ["YAML", "a.yaml", "flagValues:\n flag1: true\n", "flagValues: [\n", "flagValues:\n flag2: true\n"], + ["JSON", "a.json", '{"flagValues": {"flag1": true}}', '{"flagValues": ', '{"flagValues": {"flag2": true}}'], + ].each do |format, name, good, broken, fixed| + it "holds back the in-place save that fixes a #{format} file saved with an error" do + # An operator saves the file in place with a mistake, and saves the fix in place long + # after. The loads in between fail on the file, and the fix's own gap is held back. + y = write(name, good) + + with_clocked_reloader([y]) do |reloader, recorder| + reloader.reload_now + File.write(y, "") + expect(reloader.reload_now).to be false + File.write(y, broken) + expect(reloader.reload_now).to be false + expect(recorder.errors.length).to eq(1) + 6.times do + clock.advance(hour) + reloader.reload_now + end + errors = logger.output.scan("ERROR").length + + File.write(y, "") + expect(reloader.reload_now).to be false + expect(hold_count).to eq(2) + expect(logger.output.scan("ERROR").length).to eq(errors) + File.write(y, fixed) + expect(reloader.reload_now).to be true + expect(recorder.flag_values).to eq({ flag2: true }) + end + end + end + + it "holds back the fix of a file saved with an error even when its emptying looks like the first" do + # The broken content ended the first empty state, so the later truncation starts a new + # delay although, on a file system with coarse modification times, it looks the same. + y = write("a.yaml", yaml_doc(flag1: true)) + + with_clocked_reloader([y]) do |reloader, _recorder| + reloader.reload_now + File.write(y, "") + first_empty_write_time = File.mtime(y) + expect(reloader.reload_now).to be false + File.write(y, "flagValues: [\n") + expect(reloader.reload_now).to be false + + clock.advance(2 * hour) + File.write(y, "") + File.utime(first_empty_write_time, first_empty_write_time, y) + expect(reloader.reload_now).to be false + expect(hold_count).to eq(2) + end + end + + it "starts the bound over for a file whose read fails at a load that holds nothing back" do + y = write("a.yaml", yaml_doc(flag1: true)) + + with_clocked_reloader([y]) do |reloader, recorder| + reloader.reload_now + File.write(y, "") + expect(reloader.reload_now).to be false + + allow(FileData).to receive(:read_file).and_raise(ReadError.new(y, "unable to read file: denied")) + expect(reloader.reload_now).to be false + expect(recorder.errors.length).to eq(1) + allow(FileData).to receive(:read_file).and_call_original + + clock.advance(6 * hour) + mark_written(y) + expect(reloader.reload_now).to be false + expect(recorder.applied.length).to eq(1) + end + end + + it "starts the bound over for a file found missing at a load that holds nothing back" do + a = write("a.yaml", yaml_doc(flag1: true)) + b = write("b.json", values_doc({ flag3: "c" })) + + with_clocked_reloader([a, b], skip_missing_paths: true) do |reloader, recorder| + reloader.reload_now + File.write(a, "") + expect(reloader.reload_now).to be false + + File.delete(a) + File.write(b, '{"flagValues"') + expect(reloader.reload_now).to be false + + # b.json is fixed while a.yaml is created empty again, in the middle of a rewrite. + clock.advance(6 * hour) + File.write(a, "") + File.write(b, values_doc({ flag3: "c" })) + expect(reloader.reload_now).to be false + expect(hold_count).to eq(2) + expect(recorder.applied.length).to eq(1) + end + end + + it "applies a file kept written while empty after its bound while an earlier file fails" do + # c.json comes first and fails now and then, while a.yaml keeps being written while empty. + # Those loads hold a.yaml back, so they do not give it a fresh bound. + c = write("c.json", values_doc({ flag3: "c" })) + a = write("a.yaml", yaml_doc(flag1: true)) + + with_clocked_reloader([c, a]) do |reloader, recorder| + reloader.reload_now + File.write(a, "") + expect(reloader.reload_now).to be false + (1..9).each do |i| + clock.advance(0.5 * hour) + mark_written(a) + File.write(c, i.odd? ? '{"flagValues"' : values_doc({ flag3: "c" })) + reloader.reload_now + end + + clock.advance(0.5 * hour) + mark_written(a) + File.write(c, values_doc({ flag3: "c" })) + expect(reloader.reload_now).to be true + expect(recorder.flag_values).to eq({ flag3: "c" }) + end + end + + it "starts the bound of a file after a failing file over once a load reads it again" do + # c.json comes first and is saved with an error while a.yaml is being rewritten. The loads + # that fail on c.json still read a.yaml, with content, so its bound starts over, and its + # next rewrite, when c.json is fixed, is held back. + c = write("c.json", values_doc({ flag3: "c" })) + a = write("a.yaml", yaml_doc(flag1: true)) + + with_clocked_reloader([c, a]) do |reloader, recorder| + reloader.reload_now + File.write(a, "") + expect(reloader.reload_now).to be false + File.write(c, '{"flagValues"') + File.write(a, yaml_doc(flag1: false)) + expect(reloader.reload_now).to be false + 6.times do + clock.advance(hour) + reloader.reload_now + end + + File.write(c, values_doc({ flag3: "c" })) + File.write(a, "") + expect(reloader.reload_now).to be false + expect(recorder.applied.length).to eq(1) + end + end + + it "fails a load with the first failure when several files fail" do + a = write("a.json", values_doc({ flag1: "a" })) + b = write("b.json", values_doc({ flag2: "b" })) + + with_clocked_reloader([a, b]) do |reloader, recorder| + reloader.reload_now + File.write(a, '{"flagValues"') + File.write(b, "[1") + expect(reloader.reload_now).to be false + expect(recorder.errors.length).to eq(1) + expect(recorder.errors[0].path).to eq(a) + end + end + + it "does not start bounds over at a load that holds a file back and then fails" do + # a.yaml and b.yaml are rewritten in turn, so that every load holds one of them back, and + # c.json fails at every other load. Those loads hold a file back, so they start no bound + # over. + a = write("a.yaml", yaml_doc(flag1: true)) + b = write("b.yaml", yaml_doc(flag2: true)) + c = write("c.json", values_doc({ flag3: "c" })) + + with_clocked_reloader([a, b, c]) do |reloader, recorder| + reloader.reload_now + File.write(a, "") + expect(reloader.reload_now).to be false + applied = false + (1..12).each do |i| + clock.advance(0.5 * hour) + rewritten, emptied = i.odd? ? [a, b] : [b, a] + File.write(rewritten, yaml_doc(rewritten == a ? { flag1: true } : { flag2: true })) + File.write(emptied, "") + File.write(c, i.odd? ? '{"flagValues"' : values_doc({ flag3: "c" })) + reloader.reload_now + applied = recorder.applied.length > 1 + break if applied + end + expect(applied).to be true + end + end + + it "starts a new delay for a file that is created empty again after it was missing" do + # While b.json fails, a.yaml is emptied, deleted, and created empty again with the state it + # had when it was first found empty, as within one tick of a coarse clock. Being missing + # ended its empty state, so it is held back with a new delay. + a = write("a.yaml", yaml_doc(flag1: true)) + b = write("b.json", values_doc({ flag3: "c" })) + + with_clocked_reloader([a, b], skip_missing_paths: true) do |reloader, recorder| + reloader.reload_now + File.write(b, '{"flagValues"') + File.write(a, "") + first_empty_write_time = File.mtime(a) + reloader.reload_now + clock.advance(0.5 * hour) + File.delete(a) + reloader.reload_now + clock.advance(40.0 / 60 * hour) + File.write(a, "") + File.utime(first_empty_write_time, first_empty_write_time, a) + reloader.reload_now + + File.write(b, values_doc({ flag3: "c" })) + expect(reloader.reload_now).to be false + expect(recorder.applied.length).to eq(1) + end + end + + it "holds back YAML with only a tab and then applies it as no entries" do + y = write("a.yaml", yaml_doc(flag1: true)) + + with_clocked_reloader([y]) do |reloader, recorder| + reloader.reload_now + File.write(y, "\t\n") + expect(reloader.reload_now).to be false + + clock.advance(hour) + expect(reloader.reload_now).to be true + expect(recorder.applied[-1].empty?).to be true + expect(recorder.errors).to be_empty + end + end + + it "starts the delay over when a YAML file with no document changes size but not modification time" do + # Partial reads of a file being written can find only a comment, of different lengths, on + # a file system with coarse modification times. + y = write("a.yaml", yaml_doc(flag1: true)) + pinned = Time.at(1_000_000) + + with_clocked_reloader([y]) do |reloader, recorder| + reloader.reload_now + File.write(y, "# one\n") + File.utime(pinned, pinned, y) + expect(reloader.reload_now).to be false + + clock.advance(0.6 * hour) + File.write(y, "# three\n") + File.utime(pinned, pinned, y) + expect(reloader.reload_now).to be false + + clock.advance(0.6 * hour) + expect(reloader.reload_now).to be false + expect(recorder.applied.length).to eq(1) + end + end + + it "does not start the bound of a file that it does not hold back" do + # a.json stays empty past its delay while b.yaml holds every load back. Those loads do not + # hold a.json back, so they give it no bound, and its later rewrite gets a full delay. + b = write("b.yaml", yaml_doc(flag2: true)) + a = write("a.json", values_doc({ flag1: "a" })) + + with_clocked_reloader([b, a]) do |reloader, _recorder| + reloader.reload_now + File.write(a, "") + expect(reloader.reload_now).to be false + clock.advance(hour) + expect(reloader.reload_now).to be false + + clock.advance(0.5 * hour) + File.write(b, "") + expect(reloader.reload_now).to be false + 9.times do + clock.advance(0.5 * hour) + mark_written(b) + expect(reloader.reload_now).to be false + end + + # b.yaml's bound is over, and a.json is truncated again. + clock.advance(0.5 * hour) + mark_written(b) + mark_written(a) + holds = hold_count + expect(reloader.reload_now).to be false + expect(hold_count).to eq(holds + 1) + end + end + + it "asks for the retry when the delay is over, plus a tenth of it" do + y = write("a.yaml", yaml_doc(flag1: true)) + + with_clocked_reloader([y]) do |reloader, _recorder| + reloader.reload_now + File.write(y, "") + expect(reloader.reload_now).to be false + expect_armed_retry_delay(reloader, 1.1 * hour) + + # A later load asks for the rest of the delay, not a whole one. + clock.advance(0.5 * hour) + expect(reloader.reload_now).to be false + expect_armed_retry_delay(reloader, 0.6 * hour) + end + end + + it "asks for the retry when the bound is over, if that comes first" do + y = write("a.yaml", yaml_doc(flag1: true)) + + with_clocked_reloader([y]) do |reloader, _recorder| + reloader.reload_now + File.write(y, "") + expect(reloader.reload_now).to be false + 4.times do + clock.advance(hour) + mark_written(y) + expect(reloader.reload_now).to be false + end + clock.advance(0.8 * hour) + mark_written(y) + expect(reloader.reload_now).to be false + + # The file's delay would be over in an hour, but its bound is over in 0.2 hours. + expect_armed_retry_delay(reloader, 0.3 * hour) + end + end + + it "asks for the retry when the first of several held files is due" do + a = write("a.yaml", yaml_doc(flag1: true)) + b = write("b.yaml", yaml_doc(flag2: true)) + + with_clocked_reloader([a, b]) do |reloader, _recorder| + reloader.reload_now + File.write(a, "") + expect(reloader.reload_now).to be false + clock.advance(0.25 * hour) + File.write(b, "") + expect(reloader.reload_now).to be false + + expect_armed_retry_delay(reloader, 0.85 * hour) + end + end + + it "asks for the same retry when a change signal starts the load that holds a file back" do + y = write("a.yaml", yaml_doc(flag1: true)) + + with_clocked_reloader([y], debounce_delay: 0.01) do |reloader, _recorder| + reloader.reload_now + File.write(y, "") + reloader.trigger + + expect(wait_for { hold_count == 1 }).to be true + expect_armed_retry_delay(reloader, 1.1 * hour) + end + end + + it "asks for the retry delay after a failure" do + a = write("a.json", values_doc({ flag1: "a" })) + + with_clocked_reloader([a]) do |reloader, recorder| + reloader.reload_now + File.write(a, '{"flagValues"') + expect(reloader.reload_now).to be false + expect(recorder.errors.length).to eq(1) + expect_armed_retry_delay(reloader, hour) + end + end + + it "does not hold back a placeholder or a file that was missing" do + a = write("a.yaml", yaml_doc(flag1: true)) + placeholder = write("placeholder.yaml", "") + later = path("later.yaml") + + with_clocked_reloader([a, placeholder, later], skip_missing_paths: true) do |reloader, recorder| + reloader.reload_now + File.write(later, "") + expect(reloader.reload_now).to be true + expect(recorder.flag_values).to eq({ flag1: true }) + expect(hold_count).to eq(0) + end + end + end end end end 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 diff --git a/spec/impl/overrides/layer_spec.rb b/spec/impl/overrides/layer_spec.rb new file mode 100644 index 00000000..724f6660 --- /dev/null +++ b/spec/impl/overrides/layer_spec.rb @@ -0,0 +1,151 @@ +# frozen_string_literal: true + +require "spec_helper" +require "model_builders" +require "ldclient-rb/impl/overrides" + +module LaunchDarkly + module Impl + module Overrides + describe Layer do + let(:flag_a) { Flags.from_hash({ key: "a", version: 1, on: false, offVariation: 0, variations: ["a"] }) } + let(:flag_b) { Flags.from_hash({ key: "b", version: 2, on: false, offVariation: 0, variations: ["b"] }) } + let(:segment_s) { Segments.from_hash({ key: "s", version: 3, included: ["user"] }) } + let(:top) do + Flags.from_hash({ key: "top", version: 1, on: true, fallthrough: { variation: 0 }, offVariation: 0, variations: ["t"], + prerequisites: [{ key: "mid", variation: 0 }] }) + end + let(:mid) do + Flags.from_hash({ key: "mid", version: 1, on: true, fallthrough: { variation: 0 }, offVariation: 0, variations: ["m"], + rules: [{ id: "r", variation: 0, clauses: [Clauses.match_segment("seg")] }] }) + end + + subject(:layer) { Layer.new } + + def affected_by(kind, key) + affected = Set.new + layer.add_affected_items(affected, { kind: kind, key: key }) + affected + end + + it "is empty when created" do + expect(layer.empty?).to be true + expect(layer.get(DataStore::FEATURES, "a")).to be_nil + expect(layer.all(DataStore::FEATURES)).to eq({}) + expect(layer.all(DataStore::SEGMENTS)).to eq({}) + end + + it "stores marked copies and leaves the caller's objects unmarked" do + layer.set_all({ a: flag_a }, { s: segment_s }) + + stored_flag = layer.get(DataStore::FEATURES, "a") + stored_segment = layer.get(DataStore::SEGMENTS, :s) + expect(stored_flag.override?).to be true + expect(stored_segment.override?).to be true + expect(stored_flag).to eq flag_a + expect(stored_flag).not_to be flag_a + expect(flag_a.override?).to be false + expect(segment_s.override?).to be false + expect(layer.empty?).to be false + end + + it "reads keys given as strings or symbols" do + layer.set_all({ a: flag_a }, {}) + + expect(layer.get(DataStore::FEATURES, "a")).to eq flag_a + expect(layer.get(DataStore::FEATURES, :a)).to eq flag_a + end + + it "returns nil for a key that is not overridden and for an unknown kind" do + layer.set_all({ a: flag_a }, {}) + + expect(layer.get(DataStore::FEATURES, "b")).to be_nil + expect(layer.get(DataStore::SEGMENTS, "a")).to be_nil + expect(layer.get(DataStore::DataKind.new(namespace: "other", priority: 9), "a")).to be_nil + end + + it "replaces the whole contents on each update" do + layer.set_all({ a: flag_a }, { s: segment_s }) + layer.set_all({ b: flag_b }, {}) + + expect(layer.get(DataStore::FEATURES, "a")).to be_nil + expect(layer.get(DataStore::FEATURES, "b")).to eq flag_b + expect(layer.all(DataStore::SEGMENTS)).to eq({}) + end + + it "is cleared by an empty snapshot" do + layer.set_all({ a: flag_a }, { s: segment_s }) + layer.set_all({}, {}) + + expect(layer.empty?).to be true + expect(layer.get(DataStore::FEATURES, "a")).to be_nil + end + + it "returns the previous and the new contents from an update" do + previous, current = layer.set_all({ a: flag_a }, {}) + expect(previous[DataStore::FEATURES]).to eq({}) + expect(current[DataStore::FEATURES].keys).to eq([:a]) + + previous, current = layer.set_all({ b: flag_b }, { s: segment_s }) + expect(previous[DataStore::FEATURES].keys).to eq([:a]) + expect(current[DataStore::FEATURES].keys).to eq([:b]) + expect(current[DataStore::SEGMENTS].keys).to eq([:s]) + end + + it "adds the entries that depend on an item, starting from a key given as a string or a symbol" do + layer.set_all({ top: top, mid: mid, a: flag_a }, {}) + + # The segment is not an override entry; the chain still starts from it. + expected = Set[{ kind: DataStore::SEGMENTS, key: "seg" }, { kind: DataStore::FEATURES, key: "mid" }, + { kind: DataStore::FEATURES, key: "top" }] + expect(affected_by(DataStore::SEGMENTS, "seg")).to eq expected + expect(affected_by(DataStore::SEGMENTS, :seg)).to eq expected + expect(affected_by(DataStore::FEATURES, :a)).to eq Set[{ kind: DataStore::FEATURES, key: "a" }] + end + + it "rebuilds the dependency graph on each replacement" do + layer.set_all({ top: top }, {}) + expect(affected_by(DataStore::FEATURES, "mid")).to eq Set[{ kind: DataStore::FEATURES, key: "mid" }, + { kind: DataStore::FEATURES, key: "top" }] + + layer.set_all({}, {}) + expect(affected_by(DataStore::FEATURES, "mid")).to eq Set[{ kind: DataStore::FEATURES, key: "mid" }] + end + + it "returns frozen contents that cannot be modified" do + layer.set_all({ a: flag_a }, {}) + + expect(layer.contents).to be_frozen + expect(layer.all(DataStore::FEATURES)).to be_frozen + end + + it "always shows a reader exactly one snapshot" do + snapshot_one = { a: flag_a } + snapshot_two = { b: flag_b } + stop = Concurrent::AtomicBoolean.new(false) + mixed = Concurrent::AtomicBoolean.new(false) + + writer = Thread.new do + i = 0 + until stop.value + layer.set_all(i.even? ? snapshot_one : snapshot_two, {}) + i += 1 + end + end + reader = Thread.new do + until stop.value + keys = layer.all(DataStore::FEATURES).keys + mixed.make_true unless [[], [:a], [:b]].include?(keys) + end + end + + sleep 0.3 + stop.make_true + [writer, reader].each(&:join) + + expect(mixed.value).to be false + end + end + end + end +end diff --git a/spec/impl/overrides/overlay_spec.rb b/spec/impl/overrides/overlay_spec.rb new file mode 100644 index 00000000..ac7e5560 --- /dev/null +++ b/spec/impl/overrides/overlay_spec.rb @@ -0,0 +1,148 @@ +# frozen_string_literal: true + +require "spec_helper" +require "model_builders" +require "ldclient-rb/impl/overrides" +require "ldclient-rb/impl/data_store/in_memory_feature_store" + +module LaunchDarkly + module Impl + module Overrides + describe Overlay do + let(:ld_a) { { key: "a", version: 1, on: false, offVariation: 0, variations: ["ld-a"] } } + let(:ld_b) { { key: "b", version: 1, on: false, offVariation: 0, variations: ["ld-b"] } } + let(:ld_deleted) { { key: "gone", version: 5, deleted: true } } + let(:ld_segment) { { key: "s", version: 1, included: ["ld-user"] } } + let(:override_a) { Flags.from_hash({ key: "a", version: 9, on: false, offVariation: 0, variations: ["override-a"] }) } + let(:override_c) { Flags.from_hash({ key: "c", version: 1, on: false, offVariation: 0, variations: ["override-c"] }) } + let(:override_gone) { Flags.from_hash({ key: "gone", version: 1, on: false, offVariation: 0, variations: ["back"] }) } + let(:override_segment) { Segments.from_hash({ key: "s", version: 2, included: ["override-user"] }) } + + let(:base) { DataStore::InMemoryFeatureStoreV2.new($null_log) } + let(:layer) { Layer.new } + subject(:overlay) { Overlay.new(base, layer) } + + def load_base + base.set_basis({ + DataStore::FEATURES => { a: ld_a, b: ld_b, gone: ld_deleted }, + DataStore::SEGMENTS => { s: ld_segment }, + }) + end + + it "is a read-only store" do + expect(overlay).to be_a(Interfaces::DataSystem::ReadOnlyStore) + end + + describe "get" do + it "returns the override entry when the layer has the key" do + load_base + layer.set_all({ a: override_a }, { s: override_segment }) + + flag = overlay.get(DataStore::FEATURES, "a") + expect(flag.variations).to eq ["override-a"] + expect(flag.override?).to be true + segment = overlay.get(DataStore::SEGMENTS, :s) + expect(segment.included).to eq ["override-user"] + expect(segment.override?).to be true + end + + it "returns the base entry when the layer does not have the key" do + load_base + layer.set_all({ a: override_a }, {}) + + flag = overlay.get(DataStore::FEATURES, "b") + expect(flag.variations).to eq ["ld-b"] + expect(flag.override?).to be false + end + + it "returns an override entry for a key the base holds as deleted" do + load_base + layer.set_all({ gone: override_gone }, {}) + + expect(overlay.get(DataStore::FEATURES, "gone").variations).to eq ["back"] + end + + it "returns nil for a key that neither side has" do + load_base + + expect(overlay.get(DataStore::FEATURES, "missing")).to be_nil + end + + it "serves an override entry when the base is not initialized" do + layer.set_all({ c: override_c }, {}) + + expect(base.initialized?).to be false + expect(overlay.get(DataStore::FEATURES, "c").variations).to eq ["override-c"] + expect(overlay.get(DataStore::FEATURES, "a")).to be_nil + end + end + + describe "all" do + it "returns the base items when the layer is empty" do + load_base + + items = overlay.all(DataStore::FEATURES) + expect(items.keys.sort).to eq [:a, :b] + expect(items[:a].variations).to eq ["ld-a"] + end + + it "returns the union with the override entry winning for a shared key" do + load_base + layer.set_all({ a: override_a, c: override_c }, {}) + + items = overlay.all(DataStore::FEATURES) + expect(items.keys.sort).to eq [:a, :b, :c] + expect(items[:a].variations).to eq ["override-a"] + expect(items[:a].override?).to be true + expect(items[:b].override?).to be false + expect(items[:c].variations).to eq ["override-c"] + end + + it "includes an override entry for a key the base holds as deleted" do + load_base + layer.set_all({ gone: override_gone }, {}) + + expect(overlay.all(DataStore::FEATURES).keys.sort).to eq [:a, :b, :gone] + end + + it "returns only the override entries when the base is not initialized" do + layer.set_all({ c: override_c }, {}) + + expect(overlay.all(DataStore::FEATURES).keys).to eq [:c] + expect(overlay.all(DataStore::SEGMENTS)).to eq({}) + end + + it "raises the base error when the base fails and the layer has entries" do + # The layer's entries alone would look complete to a caller such as all_flags_state. + failing = double("base") + allow(failing).to receive(:all).and_raise(RuntimeError, "store down") + allow(failing).to receive(:get).and_return(nil) + overlay = Overlay.new(failing, layer) + layer.set_all({ c: override_c }, {}) + + expect { overlay.all(DataStore::FEATURES) }.to raise_error(RuntimeError, "store down") + expect(overlay.get(DataStore::FEATURES, "c")).not_to be_nil + end + + it "raises the base error when the base fails and the layer is empty" do + failing = double("base") + allow(failing).to receive(:all).and_raise(RuntimeError, "store down") + overlay = Overlay.new(failing, layer) + + expect { overlay.all(DataStore::FEATURES) }.to raise_error(RuntimeError, "store down") + end + end + + describe "initialized?" do + it "delegates to the base and ignores the layer" do + layer.set_all({ c: override_c }, {}) + expect(overlay.initialized?).to be false + + load_base + expect(overlay.initialized?).to be true + end + end + end + end + end +end diff --git a/spec/impl/overrides/sink_spec.rb b/spec/impl/overrides/sink_spec.rb new file mode 100644 index 00000000..cc989e29 --- /dev/null +++ b/spec/impl/overrides/sink_spec.rb @@ -0,0 +1,290 @@ +# frozen_string_literal: true + +require "spec_helper" +require "model_builders" +require "ldclient-rb/impl/overrides" +require "ldclient-rb/impl/broadcaster" +require "ldclient-rb/impl/data_store/in_memory_feature_store" + +module LaunchDarkly + module Impl + module Overrides + describe Sink do + # The synchronous executor delivers notifications inside broadcast, so the tests can read + # them right after set_overrides returns. + let(:broadcaster) { Broadcaster.new(SynchronousExecutor.new, $null_log) } + let(:base) { DataStore::InMemoryFeatureStoreV2.new($null_log) } + let(:layer) { Layer.new } + subject(:sink) { Sink.new(layer, base, broadcaster, $null_log) } + + def flag(key, value, version: 1, prereqs: [], segment: nil) + data = { + key: key, version: version, on: true, offVariation: 0, fallthrough: { variation: 0 }, variations: [value], + prerequisites: prereqs.map { |p| { key: p, variation: 0 } } + } + if segment + data[:rules] = [{ id: "r", variation: 0, clauses: [{ attribute: "", op: "segmentMatch", values: [segment] }] }] + end + Flags.from_hash(data) + end + + def segment(key, version: 1, nested: nil) + data = { key: key, version: version, included: ["user"] } + data[:rules] = [{ clauses: [{ attribute: "", op: "segmentMatch", values: [nested] }] }] if nested + Segments.from_hash(data) + end + + def listen + listener = ListenerSpy.new + broadcaster.add_listener(listener) + listener + end + + def changed_keys(listener) + listener.statuses.map(&:key).sort + end + + it "is an override sink" do + expect(sink).to be_a(Interfaces::Overrides::OverrideSink) + end + + it "replaces the layer with marked entries" do + sink.set_overrides([flag("a", "x")], [segment("s")]) + + expect(layer.get(DataStore::FEATURES, "a").override?).to be true + expect(layer.get(DataStore::FEATURES, "a").variations).to eq ["x"] + expect(layer.get(DataStore::SEGMENTS, "s").override?).to be true + end + + it "accepts definitions given as hashes" do + sink.set_overrides([{ key: "a", version: 1, on: false, offVariation: 0, variations: ["x"] }], + [{ key: "s", version: 1, included: ["user"] }]) + + expect(layer.get(DataStore::FEATURES, "a")).to be_a(Model::FeatureFlag) + expect(layer.get(DataStore::FEATURES, "a").variations).to eq ["x"] + expect(layer.get(DataStore::SEGMENTS, "s")).to be_a(Model::Segment) + end + + it "accepts nil and empty collections and clears the layer" do + sink.set_overrides([flag("a", "x")], [segment("s")]) + sink.set_overrides(nil, nil) + + expect(layer.empty?).to be true + end + + it "rejects an entry without a key" do + expect { sink.set_overrides([{ version: 1, on: false }], []) }.to raise_error(ArgumentError, /has no key/) + expect { sink.set_overrides([], [{ key: "", version: 1 }]) }.to raise_error(ArgumentError, /has no key/) + end + + it "keeps the layer and ignores later snapshots after close" do + sink.set_overrides([flag("a", "x")], []) + listener = listen + + sink.close + sink.set_overrides([flag("b", "y")], []) + + expect(layer.get(DataStore::FEATURES, "a")).not_to be_nil + expect(layer.get(DataStore::FEATURES, "b")).to be_nil + expect(changed_keys(listener)).to eq [] + end + + it "clears the layer, notifies the removal, and ignores later snapshots after close_and_clear" do + sink.set_overrides([flag("a", "x")], []) + listener = listen + + sink.close_and_clear + expect(layer.empty?).to be true + expect(changed_keys(listener)).to eq %w[a] + + sink.set_overrides([flag("b", "y")], []) + expect(layer.empty?).to be true + expect(changed_keys(listener)).to eq %w[a] + end + + it "does not notify when nothing is listening but still replaces the layer" do + expect(broadcaster).not_to receive(:broadcast) + + sink.set_overrides([flag("a", "x")], []) + + expect(layer.get(DataStore::FEATURES, "a")).not_to be_nil + end + + it "notifies the flags whose override entries were added" do + listener = listen + + sink.set_overrides([flag("a", "x"), flag("b", "y")], []) + + expect(changed_keys(listener)).to eq %w[a b] + end + + it "notifies an added override even when it matches the LaunchDarkly data" do + base.set_basis({ DataStore::FEATURES => { a: flag("a", "x").data }, DataStore::SEGMENTS => {} }) + listener = listen + + sink.set_overrides([flag("a", "x")], []) + + expect(changed_keys(listener)).to eq %w[a] + end + + it "notifies the flags whose override entries were removed" do + sink.set_overrides([flag("a", "x"), flag("b", "y")], []) + listener = listen + + sink.set_overrides([flag("b", "y")], []) + + expect(changed_keys(listener)).to eq %w[a] + end + + it "notifies a flag whose override entry changed and not one that stayed the same" do + sink.set_overrides([flag("a", "x"), flag("b", "y")], []) + listener = listen + + sink.set_overrides([flag("a", "x2"), flag("b", "y")], []) + + expect(changed_keys(listener)).to eq %w[a] + end + + it "treats a version change alone as a change" do + sink.set_overrides([flag("a", "x", version: 1)], []) + listener = listen + + sink.set_overrides([flag("a", "x", version: 2)], []) + + expect(changed_keys(listener)).to eq %w[a] + end + + it "does not notify when the snapshot is identical" do + sink.set_overrides([flag("a", "x")], [segment("s")]) + listener = listen + + sink.set_overrides([flag("a", "x")], [segment("s")]) + + expect(changed_keys(listener)).to eq [] + end + + it "notifies every override that an empty snapshot removes" do + sink.set_overrides([flag("a", "x"), flag("b", "y")], []) + listener = listen + + sink.set_overrides([], []) + + expect(changed_keys(listener)).to eq %w[a b] + end + + it "notifies LaunchDarkly flags that depend on an overridden prerequisite" do + base.set_basis({ + DataStore::FEATURES => { + prereq: flag("prereq", "p").data, + top: flag("top", "t", prereqs: ["prereq"]).data, + higher: flag("higher", "h", prereqs: ["top"]).data, + unrelated: flag("unrelated", "u").data, + }, + DataStore::SEGMENTS => {}, + }) + listener = listen + + sink.set_overrides([flag("prereq", "p2")], []) + + expect(changed_keys(listener)).to eq %w[higher prereq top] + end + + it "notifies flags whose rules reference an overridden segment, including through a nested segment" do + base.set_basis({ + DataStore::FEATURES => { + direct: flag("direct", "d", segment: "s").data, + nested: flag("nested", "n", segment: "outer").data, + unrelated: flag("unrelated", "u").data, + }, + DataStore::SEGMENTS => { s: segment("s").data, outer: segment("outer", nested: "s").data }, + }) + listener = listen + + sink.set_overrides([], [segment("s", version: 2)]) + + expect(changed_keys(listener)).to eq %w[direct nested] + end + + it "uses the old merged view as well as the new one to find dependents" do + # The base can change between the two snapshots. A flag that depended on a removed override + # only in the old view is still notified. + y = flag("y", "1") + previous = { DataStore::FEATURES => { y: y.as_override }, DataStore::SEGMENTS => {} } + current = { DataStore::FEATURES => {}, DataStore::SEGMENTS => {} } + old_view = { DataStore::FEATURES => { y: y, x: flag("x", "a", prereqs: ["y"]) }, DataStore::SEGMENTS => {} } + new_view = { DataStore::FEATURES => { x: flag("x", "a") }, DataStore::SEGMENTS => {} } + + expect(Sink.affected_flag_keys(previous, current, old_view, new_view).sort).to eq %w[x y] + end + + it "notifies dependents after an override that rewired prerequisites is removed" do + # The override of "top" replaced its prerequisite edge. Removing the override restores the + # LaunchDarkly definition, whose prerequisite is "ld-prereq". "top" changes either way, and + # the flags that depend on "top" are notified with it. + base.set_basis({ + DataStore::FEATURES => { + top: flag("top", "t", prereqs: ["ld-prereq"]).data, + "ld-prereq": flag("ld-prereq", "l").data, + "override-prereq": flag("override-prereq", "o").data, + dependent: flag("dependent", "d", prereqs: ["top"]).data, + }, + DataStore::SEGMENTS => {}, + }) + sink.set_overrides([flag("top", "t2", prereqs: ["override-prereq"])], []) + listener = listen + + sink.set_overrides([], []) + + expect(changed_keys(listener)).to eq %w[dependent top] + end + + it "notifies dependents through prerequisite edges that only the override introduces" do + base.set_basis({ + DataStore::FEATURES => { + top: flag("top", "t").data, + other: flag("other", "o").data, + dependent: flag("dependent", "d", prereqs: ["top"]).data, + }, + DataStore::SEGMENTS => {}, + }) + # The override makes "top" depend on "other". A later change to "other" then affects "top" + # and "dependent" through the new view. + sink.set_overrides([flag("top", "t2", prereqs: ["other"])], []) + listener = listen + + sink.set_overrides([flag("top", "t2", prereqs: ["other"]), flag("other", "o2")], []) + + expect(changed_keys(listener)).to eq %w[dependent other top] + end + + it "still notifies the directly changed keys when the base store cannot be read" do + failing = double("base") + allow(failing).to receive(:all).and_raise(RuntimeError, "store down") + sink = Sink.new(layer, failing, broadcaster, $null_log) + listener = listen + + sink.set_overrides([flag("a", "x")], []) + + expect(changed_keys(listener)).to eq %w[a] + expect(layer.get(DataStore::FEATURES, "a")).not_to be_nil + end + + it "serializes overlapping updates so the layer always holds one whole snapshot" do + listener = listen + threads = Array.new(4) do |i| + Thread.new do + 20.times do |n| + sink.set_overrides([flag("f#{i}", "v#{n}")], []) + end + end + end + threads.each(&:join) + + contents = layer.all(DataStore::FEATURES) + expect(contents.length).to eq 1 + expect(listener.statuses).not_to be_empty + end + end + end + end +end diff --git a/spec/in_place_writes.rb b/spec/in_place_writes.rb new file mode 100644 index 00000000..bfba8546 --- /dev/null +++ b/spec/in_place_writes.rb @@ -0,0 +1,29 @@ +# +# Rewrites files the way applications and editors often do: in place, which is not atomic. The file +# is truncated first, and the new content is written after a pause, so that a reader in between +# finds the file empty, as it can with an application's writes, instead of only by chance. +# +module InPlaceWrites + # + # Truncates the file, writes the first part of the content if one is given, waits for the pause, + # then writes the rest. A file that does not exist is created first. + # + # @param path [String] + # @param content [String] + # @param pause [Numeric] seconds between truncating the file and writing the rest of the content + # @param first [String] the beginning of the content, which a reader can see during the pause + # + def self.rewrite(path, content, pause, first: "") + raise ArgumentError, "the content must start with the first part" unless content.start_with?(first) + + # Opening with "w" truncates the file at once. + File.open(path, "w") do |file| + unless first.empty? + file.write(first) + file.flush + end + sleep pause + file.write(content[first.length..]) + end + end +end diff --git a/spec/integrations/file_override_source_spec.rb b/spec/integrations/file_override_source_spec.rb new file mode 100644 index 00000000..4df47d21 --- /dev/null +++ b/spec/integrations/file_override_source_spec.rb @@ -0,0 +1,705 @@ +# frozen_string_literal: true + +require "spec_helper" +require "capturing_logger" +require "in_place_writes" +require "model_builders" +require "override_test_components" +require "tmpdir" +require "ldclient-rb/integrations/file_data" + +module LaunchDarkly + module Integrations + describe "FileData.override_source" do + # Records every snapshot the source supplies. + class RecordingSink + include Interfaces::Overrides::OverrideSink + + def initialize + @lock = Mutex.new + @snapshots = [] + end + + def set_overrides(flags, segments) + @lock.synchronize { @snapshots << [flags, segments] } + end + + def snapshots + @lock.synchronize { @snapshots.dup } + end + + def flag_values(index = -1) + snapshots[index][0].to_h { |flag| [flag.key, flag.variations[0]] } + end + + def segment_keys(index = -1) + snapshots[index][1].map(&:key) + end + end + + around do |example| + Dir.mktmpdir do |dir| + @dir = dir + example.run + end + end + + let(:logger) { CapturingLogger.new } + let(:config) { Config.new(logger: logger) } + + def path(name) + File.join(@dir, name) + end + + # Applications rewrite override files in place, which is not atomic. The tests do too, + # deliberately slowly: a file that exists is truncated, and written 30 ms later, so a load can + # find it empty, as it would with an application's writes, instead of only by chance. + def write(name, content) + if File.exist?(path(name)) + InPlaceWrites.rewrite(path(name), content, 0.03) + else + File.write(path(name), content) + end + path(name) + end + + def yaml_values_doc(values) + "flagValues:\n" + values.map { |key, value| " #{key}: #{value}\n" }.join + end + + def values_doc(values) + { flagValues: values }.to_json + end + + def wait_for(timeout = 5) + deadline = Time.now + timeout + until yield + return false if Time.now > deadline + sleep 0.02 + end + true + end + + def build(options) + FileData.override_source(options).build("sdk-key", config) + end + + # Builds the source directly so that tests can use a short polling interval. + def make_source(paths, duplicate_keys_handling: :fail, change_detection: :polling, poll_interval: 0.05) + Impl::Integrations::FileOverrideSource.new( + paths: Impl::FileData.absolute_paths(paths), + duplicate_keys_handling: duplicate_keys_handling, + change_detection: change_detection, + poll_interval: poll_interval, + logger: logger + ) + end + + def with_source(paths, **options) + source = make_source(paths, **options) + sink = RecordingSink.new + source.start(sink) + begin + yield source, sink + ensure + source.stop + end + end + + describe "builder" do + it "builds a polling source with the default options and absolute paths" do + source = build(paths: ["relative.json"]) + + expect(source).to be_a(Interfaces::Overrides::OverrideSource) + expect(source.paths).to eq [File.absolute_path("relative.json")] + expect(source.duplicate_keys_handling).to eq :fail + expect(source.change_detection).to eq :polling + expect(source.poll_interval).to eq 1 + end + + it "accepts a single path string and explicit options" do + source = build(paths: "one.json", duplicate_keys_handling: :ignore, poll_interval: 2.5) + + expect(source.paths).to eq [File.absolute_path("one.json")] + expect(source.duplicate_keys_handling).to eq :ignore + expect(source.poll_interval).to eq 2.5 + end + + it "drops a path listed more than once with a warning" do + # Under the default fail handling the same file listed twice would make every one of its + # keys a duplicate of itself. The repeat is ignored and the first occurrence keeps its place. + source = build(paths: ["a.json", "b.json", "a.json"]) + + expect(source.paths).to eq [File.absolute_path("a.json"), File.absolute_path("b.json")] + expect(logger.output).to include("File paths list the same file more than once") + end + + it "treats a path repeated in different case as a repeat on Windows, where file names ignore case" do + allow(Gem).to receive(:win_platform?).and_return(true) + source = build(paths: ["a.json", "A.json"]) + + expect(source.paths).to eq [File.absolute_path("a.json")] + expect(logger.output).to include("File paths list the same file more than once") + end + + it "keeps paths that differ only in case elsewhere, where they name different files" do + allow(Gem).to receive(:win_platform?).and_return(false) + source = build(paths: ["a.json", "A.json"]) + + expect(source.paths).to eq [File.absolute_path("a.json"), File.absolute_path("A.json")] + expect(logger.output).not_to include("File paths list the same file more than once") + end + + it "rejects a configuration with no file paths" do + expect { build({}) }.to raise_error(ArgumentError, /no file paths/) + expect { build(paths: []) }.to raise_error(ArgumentError, /no file paths/) + expect { build(paths: nil) }.to raise_error(ArgumentError, /no file paths/) + end + + it "rejects a file path that is not a non-empty string" do + expect { build(paths: [""]) }.to raise_error(ArgumentError, /must be non-empty strings/) + expect { build(paths: ["a.json", nil]) }.to raise_error(ArgumentError, /must be non-empty strings/) + expect { build(paths: [:a]) }.to raise_error(ArgumentError, /must be non-empty strings/) + end + + it "rejects options that are not a hash and unknown option keys" do + expect { FileData.override_source("x") }.to raise_error(ArgumentError, /must be a Hash/) + expect { build(paths: ["a.json"], path: "b.json") }.to raise_error(ArgumentError, /unknown options.*path/) + end + + it "rejects an unrecognized duplicate keys handling" do + expect { build(paths: ["a.json"], duplicate_keys_handling: :first) }.to raise_error(ArgumentError, /duplicate keys handling/) + expect { build(paths: ["a.json"], duplicate_keys_handling: "fail") }.to raise_error(ArgumentError, /duplicate keys handling/) + end + + it "rejects an unrecognized change detection mode" do + expect { build(paths: ["a.json"], change_detection: :notify) }.to raise_error(ArgumentError, /change detection/) + end + + it "rejects watching when the listen gem is not available" do + allow(Impl::FileData::Watcher).to receive(:available?).and_return(false) + + expect { build(paths: ["a.json"], change_detection: :watching) }.to raise_error(ArgumentError, /listen gem/) + end + + it "builds a watching source when the listen gem is available" do + allow(Impl::FileData::Watcher).to receive(:available?).and_return(true) + + expect(build(paths: ["a.json"], change_detection: :watching).change_detection).to eq :watching + end + + it "rejects a poll interval that is not a number" do + expect { build(paths: ["a.json"], poll_interval: "1") }.to raise_error(ArgumentError, /poll interval/) + end + + it "raises a poll interval below the minimum to the minimum with a warning" do + source = build(paths: ["a.json"], poll_interval: 0.1) + + expect(source.poll_interval).to eq 1 + expect(logger.output).to include("Poll interval 0.1s is below the minimum") + end + + it "does not warn about the poll interval in watching mode" do + allow(Impl::FileData::Watcher).to receive(:available?).and_return(true) + + build(paths: ["a.json"], change_detection: :watching, poll_interval: 0.1) + + expect(logger.output).not_to include("below the minimum") + end + end + + describe "initial load" do + it "supplies the merged files to the sink before start returns" do + a = write("a.json", values_doc({ flag1: "a" })) + b = write("b.json", { flags: { flag2: { key: "flag2", on: false, offVariation: 0, variations: ["b"] } }, + segments: { seg1: { key: "seg1", included: ["user"] } } }.to_json) + + with_source([a, b]) do |_source, sink| + expect(sink.snapshots.length).to eq 1 + expect(sink.flag_values).to eq({ "flag1" => "a", "flag2" => "b" }) + expect(sink.segment_keys).to eq ["seg1"] + end + end + + it "expands a flag value into a flag that is on and serves the value by fallthrough" do + a = write("a.json", values_doc({ flag1: "a" })) + + with_source([a]) do |_source, sink| + flag = sink.snapshots[0][0][0] + expect(flag.on).to be true + expect(flag.off_variation).to be_nil + expect(flag.fallthrough.variation).to eq 0 + expect(flag.variations).to eq ["a"] + end + end + + it "reads YAML files" do + a = write("a.yaml", "flagValues:\n yaml-flag: \"override-value\"\n") + + with_source([a]) do |_source, sink| + expect(sink.flag_values).to eq({ "yaml-flag" => "override-value" }) + end + end + + it "treats a configured file that does not exist as contributing no overrides" do + a = write("a.json", values_doc({ flag1: "a" })) + + with_source([a, path("missing.json")]) do |_source, sink| + expect(sink.snapshots.length).to eq 1 + expect(sink.flag_values).to eq({ "flag1" => "a" }) + expect(logger.output).not_to include("ERROR") + end + end + + it "supplies an empty snapshot when no configured file exists" do + with_source([path("missing.json")]) do |_source, sink| + expect(sink.snapshots).to eq [[[], []]] + end + end + + it "fails the load and logs when a file cannot be parsed, leaving the sink untouched" do + a = write("a.json", '{"flagValues"') + + with_source([a]) do |_source, sink| + expect(sink.snapshots).to be_empty + expect(logger.output).to include("ERROR") + expect(logger.output).to include("FileOverrideSource: Unable to load flags") + end + end + + it "fails the load when a key appears in two files with the default handling" do + a = write("a.json", values_doc({ flag1: "first" })) + b = write("b.json", values_doc({ flag1: "second" })) + + with_source([a, b]) do |_source, sink| + expect(sink.snapshots).to be_empty + expect(logger.output).to include("was used more than once") + end + end + + it "keeps the first file's entry with ignore handling" do + a = write("a.json", values_doc({ flag1: "first" })) + b = write("b.json", values_doc({ flag1: "second", flag2: "other" })) + + with_source([a, b], duplicate_keys_handling: :ignore) do |_source, sink| + expect(sink.flag_values).to eq({ "flag1" => "first", "flag2" => "other" }) + end + end + end + + describe "logging" do + it "logs the overrides in effect and what each file supplied" do + a = write("a.json", { flagValues: { flag1: "a", flag2: "b" }, segments: { seg1: { key: "seg1" } } }.to_json) + b = write("b.json", "{}") + missing = path("missing.json") + + with_source([a, b, missing]) do |_source, _sink| + expect(logger.output).to include( + "INFO -- : [LDClient] FileOverrideSource: Flag overrides in effect: 2 flags, 1 segment " \ + "(#{a}: 2 flags, 1 segment; #{b}: no entries; #{missing}: absent)" + ) + end + end + + it "logs when no overrides are in effect" do + a = write("a.json", "{}") + + with_source([a]) do |_source, _sink| + expect(logger.output).to include("Flag overrides: none in effect (#{a}: no entries)") + end + end + end + + describe "polling change detection" do + it "applies a changed file" do + a = write("a.json", values_doc({ flag1: "a" })) + + with_source([a]) do |_source, sink| + sleep 0.1 + write("a.json", values_doc({ flag1: "b" })) + + expect(wait_for { sink.snapshots.length == 2 }).to be true + expect(sink.flag_values).to eq({ "flag1" => "b" }) + end + end + + it "applies a file that appears after start" do + with_source([path("later.json")]) do |_source, sink| + expect(sink.snapshots.length).to eq 1 + write("later.json", values_doc({ flag1: "a" })) + + expect(wait_for { sink.snapshots.length == 2 }).to be true + expect(sink.flag_values).to eq({ "flag1" => "a" }) + end + end + + it "removes the overrides of a deleted file" do + a = write("a.json", values_doc({ flag1: "a" })) + + with_source([a]) do |_source, sink| + File.delete(a) + + expect(wait_for { sink.snapshots.length == 2 }).to be true + expect(sink.snapshots[-1]).to eq [[], []] + end + end + + it "keeps the last good overrides through a malformed edit and recovers" do + a = write("a.json", values_doc({ flag1: "a" })) + + with_source([a]) do |_source, sink| + write("a.json", '{"flagValues"') + sleep 0.5 + expect(sink.snapshots.length).to eq 1 + expect(logger.output).to include("Unable to load flags") + + write("a.json", values_doc({ flag1: "c" })) + expect(wait_for { sink.snapshots.length == 2 }).to be true + expect(sink.flag_values).to eq({ "flag1" => "c" }) + end + end + + it "does not supply a snapshot for a rewrite with identical content" do + a = write("a.json", values_doc({ flag1: "a" })) + + with_source([a]) do |_source, sink| + sleep 0.1 + File.utime(Time.now + 5, Time.now + 5, a) + sleep 0.5 + + expect(sink.snapshots.length).to eq 1 + end + end + + it "stops detecting changes when stopped" do + a = write("a.json", values_doc({ flag1: "a" })) + source = make_source([a]) + sink = RecordingSink.new + source.start(sink) + poller_threads = Thread.list.select { |t| t.name == "LD/FileDataPoller" } + expect(poller_threads.length).to eq 1 + + source.stop + + expect(poller_threads[0].alive?).to be false + write("a.json", values_doc({ flag1: "b" })) + sleep 0.5 + expect(sink.snapshots.length).to eq 1 + end + + it "does not start when stopped before start" do + a = write("a.json", values_doc({ flag1: "a" })) + source = make_source([a]) + sink = RecordingSink.new + source.stop + source.start(sink) + + expect(sink.snapshots).to be_empty + end + end + + describe "watching change detection" do + before do + skip "the listen gem is not installed" unless Impl::FileData::Watcher.available? + end + + it "applies a changed file" do + a = write("a.json", values_doc({ flag1: "a" })) + + with_source([a], change_detection: :watching) do |_source, sink| + sleep 0.3 + write("a.json", values_doc({ flag1: "b" })) + + expect(wait_for { sink.snapshots.length == 2 }).to be true + expect(sink.flag_values).to eq({ "flag1" => "b" }) + end + end + + it "applies a file that appears after start" do + with_source([path("later.json")], change_detection: :watching) do |_source, sink| + sleep 0.3 + write("later.json", values_doc({ flag1: "a" })) + + expect(wait_for { sink.snapshots.length == 2 }).to be true + expect(sink.flag_values).to eq({ "flag1" => "a" }) + end + end + + it "removes the overrides of a deleted file" do + a = write("a.json", values_doc({ flag1: "a" })) + + with_source([a], change_detection: :watching) do |_source, sink| + sleep 0.3 + File.delete(a) + + expect(wait_for { sink.snapshots.length == 2 }).to be true + expect(sink.snapshots[-1]).to eq [[], []] + end + end + end + + describe "in-place rewrites" do + # A writer that rewrites a file in place truncates it, and writes the new content after this + # pause. A load in between finds the file empty. + let(:pause) { 0.3 } + + before do + # The retry comes long enough after a load that finds the file empty for the rewrite to + # finish before it, even on a slow machine. + stub_const("LaunchDarkly::Impl::FileData::Reloader::DEFAULT_RETRY_DELAY", 1.5) + end + + [:polling, :watching].each do |mode| + context "with #{mode} change detection" do + before do + skip "the listen gem is not installed" if mode == :watching && !Impl::FileData::Watcher.available? + end + + it "never supplies a snapshot without the override while a JSON file is rewritten" do + a = write("a.json", values_doc({ flag1: "a" })) + + with_source([a], change_detection: mode) do |_source, sink| + sleep 0.3 if mode == :watching + ["b", "c"].each do |value| + InPlaceWrites.rewrite(a, values_doc({ flag1: value }), pause) + expect(wait_for { sink.flag_values == { "flag1" => value } }).to be true + end + + expect(sink.snapshots.map { |flags, _segments| flags.map(&:key) }).to all(eq(["flag1"])) + expect(logger.output).not_to include("ERROR") + end + end + + it "never supplies a snapshot without the override while a YAML file is rewritten" do + a = write("a.yaml", yaml_values_doc({ flag1: "a" })) + + with_source([a], change_detection: mode) do |_source, sink| + sleep 0.3 if mode == :watching + ["b", "c"].each do |value| + InPlaceWrites.rewrite(a, yaml_values_doc({ flag1: value }), pause) + expect(wait_for { sink.flag_values == { "flag1" => value } }).to be true + end + + expect(sink.snapshots.map { |flags, _segments| flags.map(&:key) }).to all(eq(["flag1"])) + end + end + + it "never supplies a snapshot without the override while a YAML file that starts with a comment is rewritten" do + # A load during the pause finds only the comment, which the YAML parser reads as nothing. + comment = "# overrides for the incident\n" + a = write("a.yaml", comment + yaml_values_doc({ flag1: "a" })) + + with_source([a], change_detection: mode) do |_source, sink| + sleep 0.3 if mode == :watching + ["b", "c"].each do |value| + InPlaceWrites.rewrite(a, comment + yaml_values_doc({ flag1: value }), pause, first: comment) + expect(wait_for { sink.flag_values == { "flag1" => value } }).to be true + end + + expect(sink.snapshots.map { |flags, _segments| flags.map(&:key) }).to all(eq(["flag1"])) + end + end + end + end + + it "keeps the overrides in effect when a JSON file stays empty" do + a = write("a.json", values_doc({ flag1: "a" })) + + with_source([a]) do |_source, sink| + File.write(a, "") + + expect(wait_for { logger.output.include?("the file has no content, which is not a JSON document") }).to be true + expect(sink.snapshots.length).to eq 1 + expect(sink.flag_values).to eq({ "flag1" => "a" }) + end + end + + it "removes the overrides of a YAML file that stays empty after the retry delay" do + a = write("a.yaml", yaml_values_doc({ flag1: "a" })) + + with_source([a]) do |_source, sink| + File.write(a, "") + + expect(wait_for { sink.snapshots.length == 2 }).to be true + expect(sink.snapshots[-1]).to eq [[], []] + expect(logger.output).to include("reading it again before applying it") + end + end + end + + describe "files that are not UTF-8" do + # Windows PowerShell 5.1 writes a file with `>` in UTF-16 with a byte order mark. + def utf16(text) + "\xFF\xFE".b + text.encode("UTF-16LE").b + end + + it "keeps the overrides in effect when a file is rewritten as UTF-16, and applies the fix" do + a = write("a.json", values_doc({ flag1: "a" })) + + with_source([a]) do |_source, sink| + File.binwrite(a, utf16(values_doc({ flag1: "b" }))) + expect(wait_for { logger.output.include?("encoded in UTF-16 or UTF-32") }).to be true + expect(sink.snapshots.length).to eq 1 + + File.write(a, values_doc({ flag1: "c" })) + expect(wait_for { sink.flag_values == { "flag1" => "c" } }).to be true + expect(logger.output.scan("ERROR").length).to eq 1 + end + end + + it "starts with a UTF-16 file without its overrides, and applies the file once it is fixed" do + a = path("a.json") + File.binwrite(a, utf16(values_doc({ flag1: "a" }))) + + with_source([a]) do |_source, sink| + expect(sink.snapshots).to be_empty + expect(logger.output).to include("encoded in UTF-16 or UTF-32") + + File.write(a, values_doc({ flag1: "b" })) + expect(wait_for { sink.snapshots.length == 1 }).to be true + expect(sink.flag_values).to eq({ "flag1" => "b" }) + end + end + + it "keeps the overrides in effect when a file is rewritten with content that is not UTF-8" do + a = write("a.json", values_doc({ flag1: "a" })) + + with_source([a]) do |_source, sink| + File.binwrite(a, "{\"flagValues\": {\"flag1\": \"caf\xE9\"}}".b) + expect(wait_for { logger.output.include?("the file is not valid UTF-8") }).to be true + expect(sink.snapshots.length).to eq 1 + end + end + end + + describe "in-place rewrites with the default timing" do + it "never supplies a snapshot without the override while a YAML file is rewritten about once a second" do + # Polling once a second, a poll, or the retry, can find the second rewrite's truncation + # before any load has read the first rewrite's content. + a = write("a.yaml", yaml_values_doc({ flag1: "v0" })) + source = build(paths: [a]) + sink = RecordingSink.new + source.start(sink) + begin + (1..5).each do |i| + InPlaceWrites.rewrite(a, yaml_values_doc({ flag1: "v#{i}" }), 0.3) + sleep 0.7 + end + expect(wait_for { sink.flag_values == { "flag1" => "v5" } }).to be true + expect(sink.snapshots.map { |flags, _segments| flags.map(&:key) }).to all(eq(["flag1"])) + ensure + source.stop + end + end + end + + describe "with a client" do + let(:context) { LDContext.create({ key: "user-key", kind: "user" }) } + let(:ld_flag) { { key: "flag", version: 100, on: false, offVariation: 0, variations: ["ld-value"] } } + + def with_client(overrides) + data_system = DataSystem.custom + .initializers([TestDataInitializer.new(flags: { flag: ld_flag })]) + .overrides(overrides) + .build + client = LDClient.new("sdk-key", Config.new(data_system_config: data_system, send_events: false, logger: logger), 5) + begin + yield client + ensure + client.close + end + end + + it "serves overrides from the files and reloads them on a running client" do + a = write("a.json", values_doc({ flag: "override-value", other: true })) + overrides = FileData.override_source(paths: [a]) + + with_client(overrides) do |client| + detail = client.variation_detail("flag", context, "default") + expect(detail.value).to eq "override-value" + expect(detail.reason).to eq EvaluationReason.fallthrough.with_override_affected(true) + expect(client.variation("other", context, false)).to be true + + write("a.json", "{}") + expect(wait_for { client.variation("flag", context, "default") == "ld-value" }).to be true + expect(client.variation("other", context, false)).to be false + end + end + + it "leaves a flagValues entry out of a client-side state and keeps a definition with clientSide" do + # A flagValues entry is not available to client-side SDKs. A full definition that sets + # clientSide is, so that is how an operator overrides a flag that client-side SDKs use. + a = write("a.json", JSON.generate({ + flagValues: { "value-flag": true }, + flags: { + "full-flag": { key: "full-flag", version: 1, on: true, variations: [true], + fallthrough: { variation: 0 }, clientSide: true }, + }, + })) + + with_client(FileData.override_source(paths: [a])) do |client| + expect(client.all_flags_state(context).values_map.keys).to include("value-flag", "full-flag") + state = client.all_flags_state(context, client_side_only: true) + expect(state.valid?).to be true + expect(state.values_map).to eq({ "full-flag" => true }) + end + end + + [ + ["JSON", "a.json", ->(value) { { flagValues: { flag: value } }.to_json }], + ["YAML", "a.yaml", ->(value) { "flagValues:\n flag: #{value}\n" }], + ].each do |format, name, doc| + it "serves the override throughout an in-place rewrite of a #{format} file" do + stub_const("LaunchDarkly::Impl::FileData::Reloader::DEFAULT_RETRY_DELAY", 1.5) + a = write(name, doc.call("override-a")) + change_detection = Impl::FileData::Watcher.available? ? :watching : :polling + source = make_source([a], change_detection: change_detection) + builder = Object.new + builder.define_singleton_method(:build) { |_sdk_key, _config| source } + + with_client(builder) do |client| + sleep 0.3 if change_detection == :watching + served = [] + sampling = true + sampler = Thread.new do + while sampling + value = client.variation("flag", context, "default") + served << value if served.last != value + sleep 0.002 + end + end + + InPlaceWrites.rewrite(a, doc.call("override-b"), 0.3) + expect(wait_for { served.last == "override-b" }).to be true + sampling = false + sampler.join + + expect(served).to eq ["override-a", "override-b"] + end + end + end + + it "serves a UTF-16 override file once it is fixed, after starting without it" do + a = path("a.json") + File.binwrite(a, "\xFF\xFE".b + { flagValues: { flag: "override-a" } }.to_json.encode("UTF-16LE").b) + source = make_source([a]) + builder = Object.new + builder.define_singleton_method(:build) { |_sdk_key, _config| source } + + with_client(builder) do |client| + expect(client.variation("flag", context, "default")).to eq "ld-value" + File.write(a, { flagValues: { flag: "override-b" } }.to_json) + expect(wait_for { client.variation("flag", context, "default") == "override-b" }).to be true + end + end + + it "reports invalid options from client construction" do + data_system = DataSystem.custom.overrides(FileData.override_source(paths: [])).build + + expect { LDClient.new("sdk-key", Config.new(data_system_config: data_system, send_events: false, logger: logger), 0) } + .to raise_error(ArgumentError, /no file paths/) + end + end + end + end +end diff --git a/spec/ldclient_events_spec.rb b/spec/ldclient_events_spec.rb index 9c1d7f07..1a78fb1f 100644 --- a/spec/ldclient_events_spec.rb +++ b/spec/ldclient_events_spec.rb @@ -22,7 +22,7 @@ def event_processor(client) with_client(test_config) do |client| context = basic_context expect(event_processor(client)).to receive(:record_eval_event).with( - context, 'badkey', nil, nil, 'default', nil, 'default', false, nil, nil, 1, false + context, 'badkey', nil, nil, 'default', nil, 'default', false, nil, nil, 1, false, false ) client.variation("badkey", context, "default") end @@ -35,7 +35,7 @@ def event_processor(client) context = basic_context with_client(test_config(data_source: td)) do |client| expect(event_processor(client)).to receive(:record_eval_event).with( - context, 'flagkey', 1, 0, 'value', nil, 'default', false, nil, nil, nil, false + context, 'flagkey', 1, 0, 'value', nil, 'default', false, nil, nil, nil, false, false ) client.variation("flagkey", context, "default") end @@ -81,7 +81,7 @@ def event_processor(client) with_client(test_config(data_source: td)) do |client| expect(event_processor(client)).to receive(:record_eval_event).with( context, 'flagkey', 100, 0, 'value', LaunchDarkly::EvaluationReason::rule_match(0, 'id'), - 'default', true, nil, nil, nil, false + 'default', true, nil, nil, nil, false, false ) client.variation("flagkey", context, "default") end @@ -98,7 +98,7 @@ def event_processor(client) with_client(test_config(data_source: td)) do |client| expect(event_processor(client)).to receive(:record_eval_event).with( context, 'flagkey', 100, 0, 'value', LaunchDarkly::EvaluationReason::fallthrough, - 'default', true, nil, nil, nil, false + 'default', true, nil, nil, nil, false, false ) client.variation("flagkey", context, "default") end @@ -112,7 +112,7 @@ def event_processor(client) expect(event_processor(client)).to receive(:record_eval_event).with( context, 'badkey', nil, nil, 'default', LaunchDarkly::EvaluationReason::error(LaunchDarkly::EvaluationReason::ERROR_FLAG_NOT_FOUND), - 'default', false, nil, nil, 1, false + 'default', false, nil, nil, 1, false, false ) client.variation_detail("badkey", context, "default") end @@ -126,7 +126,7 @@ def event_processor(client) with_client(test_config(data_source: td)) do |client| expect(event_processor(client)).to receive(:record_eval_event).with( context, 'flagkey', 1, 0, 'value', LaunchDarkly::EvaluationReason::off, - 'default', false, nil, nil, nil, false + 'default', false, nil, nil, nil, false, false ) client.variation_detail("flagkey", context, "default") end diff --git a/spec/ldclient_override_events_spec.rb b/spec/ldclient_override_events_spec.rb new file mode 100644 index 00000000..9078bfeb --- /dev/null +++ b/spec/ldclient_override_events_spec.rb @@ -0,0 +1,291 @@ +require "spec_helper" +require "events_test_util" +require "model_builders" +require "override_test_components" + +module LaunchDarkly + describe LDClient, "events for override-affected evaluations" do + let(:context) { LDContext.create({ key: "user-key", kind: "user" }) } + let(:debug_until) { (Time.now.to_f * 1000).to_i + 1000000 } + + def flag(key, value, version: 100, prereqs: [], segment: nil, track: true) + data = { + key: key, version: version, on: true, offVariation: 0, fallthrough: { variation: 1 }, + variations: ["#{value}-off", value], + prerequisites: prereqs.map { |p| { key: p, variation: 1 } }, + trackEvents: track + } + data[:debugEventsUntilDate] = debug_until if track + if segment + data[:rules] = [{ id: "segment-rule", variation: 1, clauses: [{ attribute: "", op: "segmentMatch", values: [segment] }] }] + data[:fallthrough] = { variation: 0 } + end + data + end + + def segment(key, *included) + { key: key, version: 100, included: included } + end + + def data_system(overrides:, flags: {}, segments: {}) + DataSystem.custom + .initializers([TestDataInitializer.new(flags: flags, segments: segments)]) + .overrides(overrides) + .build + end + + def with_recording_client(data_system_config) + config = Config.new(data_system_config: data_system_config, send_events: false, logger: $null_log) + client = LDClient.new("sdk-key", config, 5) + events = RecordingEventProcessor.new + client.instance_variable_set(:@event_processor, events) + begin + yield client, events + ensure + client.close + end + end + + describe "records handed to the event processor" do + it "carries the marking of a directly overridden flag" do + source = TestOverrideSource.new([flag("flag", "override")]) + with_recording_client(data_system(overrides: source, flags: { flag: flag("flag", "ld") })) do |client, events| + client.variation("flag", context, "default") + + record = events.records_for("flag")[0] + expect(record.override_affected).to be true + expect(record.track_events).to be true + expect(record.debug_until).to eq debug_until + expect(record.value).to eq "override" + end + end + + it "does not mark an unaffected flag" do + source = TestOverrideSource.new([flag("flag", "override")]) + with_recording_client(data_system(overrides: source, flags: { normal: flag("normal", "n") })) do |client, events| + client.variation("normal", context, "default") + + expect(events.records_for("normal")[0].override_affected).to be false + end + end + + it "marks a flag affected through an overridden segment" do + source = TestOverrideSource.new([], [segment("seg", context.key)]) + with_recording_client(data_system(overrides: source, flags: { flag: flag("flag", "included", segment: "seg") }, + segments: { seg: segment("seg") })) do |client, events| + expect(client.variation("flag", context, "default")).to eq "included" + + expect(events.records_for("flag")[0].override_affected).to be true + end + end + + it "marks the prerequisite record by the prerequisite's own reads" do + source = TestOverrideSource.new([flag("overridden-prereq", "op")]) + flags = { + "overridden-prereq": flag("overridden-prereq", "ld-op", version: 50), + "plain-prereq": flag("plain-prereq", "pp"), + top: flag("top", "t", prereqs: ["overridden-prereq", "plain-prereq"]), + } + with_recording_client(data_system(overrides: source, flags: flags)) do |client, events| + expect(client.variation("top", context, "default")).to eq "t" + + top = events.records_for("top")[0] + expect(top.override_affected).to be true + overridden = events.records_for("overridden-prereq")[0] + expect(overridden.override_affected).to be true + expect(overridden.version).to eq 100 + expect(overridden.prereq_of).to eq "top" + plain = events.records_for("plain-prereq")[0] + expect(plain.override_affected).to be false + expect(plain.prereq_of).to eq "top" + end + end + + it "does not mark an unknown flag" do + source = TestOverrideSource.new([flag("flag", "override")]) + with_recording_client(data_system(overrides: source)) do |client, events| + client.variation("unknown", context, "default") + + expect(events.records_for("unknown")[0].override_affected).to be false + end + end + + it "marks an error record by the flag's own marker when the evaluation raises" do + source = TestOverrideSource.new([flag("flag", "override")]) + with_recording_client(data_system(overrides: source)) do |client, events| + evaluator = client.instance_variable_get(:@evaluator) + allow(evaluator).to receive(:evaluate).and_raise(RuntimeError, "boom") + + expect(client.variation("flag", context, "default")).to eq "default" + + record = events.records_for("flag")[0] + expect(record.override_affected).to be true + expect(record.variation).to be_nil + end + end + end + + describe "a custom store that returns plain hashes" do + it "records an unmarked error event and keeps tracking when evaluating a plain hash flag raises" do + # The FeatureStore interface documents its entities as hashes. The evaluator raises on one, and + # a hash never came from the override store, so the failure is unmarked and the state keeps + # the flag's tracking, as before overrides. + flag_hash = flag("plain", "value") + store = InMemoryFeatureStore.new + store.define_singleton_method(:initialized?) { true } + store.define_singleton_method(:get) { |kind, key| kind == Impl::DataStore::FEATURES && key == "plain" ? flag_hash : nil } + store.define_singleton_method(:all) { |kind| kind == Impl::DataStore::FEATURES ? { plain: flag_hash } : {} } + source = TestOverrideSource.new([flag("other", "override")]) + with_recording_client(DataSystem.custom.data_store(store, :read_only).overrides(source).build) do |client, events| + expect(client.variation("plain", context, "default")).to eq "default" + expect(events.records_for("plain")[0].override_affected).to be false + + meta = client.all_flags_state(context).as_json["$flagsState"] + expect(meta["plain"][:trackEvents]).to be true + expect(meta["plain"][:debugEventsUntilDate]).to eq debug_until + end + end + end + + describe "all_flags_state" do + it "turns off event tracking for override-affected flags and keeps it for others" do + source = TestOverrideSource.new([flag("direct", "override")], [segment("seg", context.key)]) + flags = { + direct: flag("direct", "ld"), + transitive: flag("transitive", "included", segment: "seg"), + normal: flag("normal", "n"), + } + with_recording_client(data_system(overrides: source, flags: flags, segments: { seg: segment("seg") })) do |client, _| + state = client.all_flags_state(context, with_reasons: true) + meta = state.as_json["$flagsState"] + + expect(state.values_map).to eq({ "direct" => "override", "transitive" => "included", "normal" => "n" }) + %w[direct transitive].each do |key| + expect(meta[key]).not_to have_key(:trackEvents) + expect(meta[key]).not_to have_key(:trackReason) + expect(meta[key]).not_to have_key(:debugEventsUntilDate) + expect(meta[key][:version]).to eq 100 + expect(meta[key][:reason].override_affected).to be true + end + expect(meta["normal"][:trackEvents]).to be true + expect(meta["normal"][:debugEventsUntilDate]).to eq debug_until + expect(meta["normal"][:reason].override_affected).to be false + end + end + + it "turns off event tracking for an overridden flag whose evaluation raises" do + # A failed evaluation has no result, so the flag's own marker decides, as it does for the + # reason and for the event of a single evaluation. + source = TestOverrideSource.new([flag("direct", "override")]) + with_recording_client(data_system(overrides: source, flags: { normal: flag("normal", "n") })) do |client, _| + allow(client.instance_variable_get(:@evaluator)).to receive(:evaluate).and_raise("evaluation failed") + + meta = client.all_flags_state(context, with_reasons: true).as_json["$flagsState"] + + expect(meta["direct"]).not_to have_key(:trackEvents) + expect(meta["direct"]).not_to have_key(:trackReason) + expect(meta["direct"]).not_to have_key(:debugEventsUntilDate) + expect(meta["direct"][:reason].override_affected).to be true + expect(meta["normal"][:trackEvents]).to be true + expect(meta["normal"][:debugEventsUntilDate]).to eq debug_until + expect(meta["normal"][:reason].override_affected).to be false + end + end + + it "turns off tracking that an experiment would otherwise require" do + experiment = flag("experiment", "e", track: false).merge(trackEventsFallthrough: true) + source = TestOverrideSource.new([experiment]) + with_recording_client(data_system(overrides: source, flags: { experiment: experiment })) do |client, _| + meta = client.all_flags_state(context).as_json["$flagsState"] + + expect(meta["experiment"]).not_to have_key(:trackEvents) + expect(meta["experiment"]).not_to have_key(:trackReason) + end + end + + it "omits details for an override-affected flag when details are only wanted for tracked flags" do + source = TestOverrideSource.new([flag("direct", "override")]) + with_recording_client(data_system(overrides: source, flags: { normal: flag("normal", "n") })) do |client, _| + meta = client.all_flags_state(context, details_only_for_tracked_flags: true).as_json["$flagsState"] + + expect(meta["direct"]).not_to have_key(:version) + expect(meta["normal"][:version]).to eq 100 + end + end + end + + describe "delivered payload" do + def with_sending_client(data_system_config) + config = Config.new(data_system_config: data_system_config, logger: $null_log, diagnostic_opt_out: true) + client = LDClient.new("sdk-key", config, 5) + sender = FakeEventSender.new + ep = EventProcessor.new("sdk-key", config, nil, nil, { event_sender: sender }) + client.instance_variable_set(:@event_processor, ep) + begin + yield client, ep, sender + ensure + ep.stop + client.close + end + end + + def payload(ep, sender) + ep.flush + ep.wait_until_inactive + TestQueues.pop_within(sender.analytics_payloads, 5) + end + + it "sends only an index event and a marked summary counter for an overridden flag that tracks events" do + source = TestOverrideSource.new([flag("flag", "override", version: 300)]) + with_sending_client(data_system(overrides: source, flags: { normal: flag("normal", "n", track: false) })) do |client, ep, sender| + 2.times { client.variation("flag", context, "default1") } + client.variation("normal", context, "default2") + + events = payload(ep, sender) + expect(events.map { |e| e[:kind] }.sort).to eq %w[index summary] + summary = events.detect { |e| e[:kind] == "summary" } + expect(summary[:features][:flag][:default]).to eq "default1" + expected_counters = [{ version: 300, variation: 1, value: "override", count: 2, overrideAffected: true }] + expect(summary[:features][:flag][:counters]).to eq(expected_counters) + expect(summary[:features][:normal][:counters]).to eq([{ version: 100, variation: 1, value: "n", count: 1 }]) + end + end + + it "sends the individual event for an unaffected prerequisite inside a marked evaluation and none for the marked ones" do + source = TestOverrideSource.new([flag("overridden-prereq", "op", version: 200)]) + flags = { + "overridden-prereq": flag("overridden-prereq", "ld-op"), + "plain-prereq": flag("plain-prereq", "pp"), + mixed: flag("mixed", "m", prereqs: ["overridden-prereq", "plain-prereq"]), + } + with_sending_client(data_system(overrides: source, flags: flags)) do |client, ep, sender| + expect(client.variation("mixed", context, "default")).to eq "m" + + events = payload(ep, sender) + feature_events = events.select { |e| e[:kind] == "feature" } + expect(feature_events.map { |e| e[:key] }).to eq ["plain-prereq"] + expect(feature_events[0][:prereqOf]).to eq "mixed" + # The unaffected prerequisite is in debug mode too, so it alone produces a debug event. + expect(events.select { |e| e[:kind] == "debug" }.map { |e| e[:key] }).to eq ["plain-prereq"] + + summary = events.detect { |e| e[:kind] == "summary" } + expect(summary[:features][:mixed][:counters]).to eq([{ version: 100, variation: 1, value: "m", count: 1, overrideAffected: true }]) + expect(summary[:features][:"overridden-prereq"][:counters]).to eq([{ version: 200, variation: 1, value: "op", count: 1, overrideAffected: true }]) + expect(summary[:features][:"plain-prereq"][:counters]).to eq([{ version: 100, variation: 1, value: "pp", count: 1 }]) + end + end + + it "sends the usual feature and debug events when nothing is overridden" do + source = TestOverrideSource.new([]) + with_sending_client(data_system(overrides: source, flags: { flag: flag("flag", "ld") })) do |client, ep, sender| + client.variation("flag", context, "default") + + events = payload(ep, sender) + expect(events.map { |e| e[:kind] }.sort).to eq %w[debug feature index summary] + summary = events.detect { |e| e[:kind] == "summary" } + expect(summary[:features][:flag][:counters]).to eq([{ version: 100, variation: 1, value: "ld", count: 1 }]) + end + end + end + end +end diff --git a/spec/ldclient_override_vectors_spec.rb b/spec/ldclient_override_vectors_spec.rb new file mode 100644 index 00000000..c7da3817 --- /dev/null +++ b/spec/ldclient_override_vectors_spec.rb @@ -0,0 +1,104 @@ +require "spec_helper" +require "override_test_components" +require "ldclient-rb/impl/file_data" + +require "json" + +# +# Runs the OVERRIDE specification's test vectors in spec/fixtures/override-vectors/vectors.json. +# Each vector sets up LaunchDarkly data, an override layer, and an initialization state. The test +# evaluates one flag through the full client stack and checks the value, the variation index, and +# the reason against the vector's expectations, using the comparison rules from the vectors' README. +# +module LaunchDarkly + describe LDClient, "OVERRIDE specification vectors" do + VECTORS_PATH = File.expand_path("fixtures/override-vectors/vectors.json", __dir__) + + # The vectors' semantics are versioned. A schema change means this runner needs review. + SUPPORTED_VECTOR_SCHEMA = "0.4.0" + + vector_file = JSON.parse(File.read(VECTORS_PATH), symbolize_names: true) + + it "uses vectors with the supported schema version" do + expect(vector_file[:schemaVersion]).to eq SUPPORTED_VECTOR_SCHEMA + expect(vector_file[:vectors]).not_to be_empty + end + + def override_definitions(overrides) + flags = (overrides[:flags] || {}).values + (overrides[:flagValues] || {}).each do |key, value| + flags << Impl::FileData.make_flag_with_value(key.to_s, value) + end + [flags, (overrides[:segments] || {}).values] + end + + def build_client(vector) + flags, segments = override_definitions(vector[:overrides] || {}) + source = TestOverrideSource.new(flags, segments) + ld_data = vector[:launchDarklyData] + + builder = DataSystem.custom.overrides(source) + if ld_data[:initialized] + builder.initializers([TestDataInitializer.new(flags: ld_data[:flags] || {}, segments: ld_data[:segments] || {})]) + wait = 5 + else + # With no data sources at all, the client would consider cached data available rather than + # applying its not-initialized handling. A synchronizer that never delivers anything avoids that. + builder.synchronizers([HangingSynchronizer.new]) + wait = 0 + end + + config = Config.new(data_system_config: builder.build, send_events: false, logger: $null_log) + client = LDClient.new("sdk-key", config, wait) + # The summary marker is the marking the client hands to the event processor for this + # evaluation. Event handling keys on that scalar, not on the reason. + events = RecordingEventProcessor.new + client.instance_variable_set(:@event_processor, events) + [client, events] + end + + # Compares the actual reason against only the fields present in the expected reason. An expected + # reason that omits overrideAffected requires the actual reason to report false, which is never + # serialized, or to omit it. + def expect_reason(expected, actual) + actual_json = JSON.parse(actual.to_json) + expected.each do |field, value| + expect(actual_json[field.to_s]).to eq(value), "reason field #{field}" + end + unless expected.key?(:overrideAffected) + expect(actual_json["overrideAffected"]).to be_nil + expect(actual.override_affected).to be false + end + end + + vector_file[:vectors].each do |vector| + it "#{vector[:group]}: #{vector[:description]}" do + client, events = build_client(vector) + begin + expect(client.initialized?).to eq(vector[:launchDarklyData][:initialized]) + + evaluate = vector[:evaluate] + context = LDContext.create(evaluate[:context]) + detail = client.variation_detail(evaluate[:flagKey], context, evaluate[:defaultValue]) + expected = vector[:expect] + + expect(detail.value).to eq(expected[:value]) + if expected[:variationIndex].nil? + expect(detail.variation_index).to be_nil + else + expect(detail.variation_index).to eq(expected[:variationIndex]) + end + expect_reason(expected[:reason], detail.reason) + + unless expected[:summaryOverrideAffected].nil? + records = events.records_for(evaluate[:flagKey]) + expect(records.length).to eq(1), "expected exactly one evaluation record for the flag" + expect(records[0].override_affected).to eq(expected[:summaryOverrideAffected]), "summaryOverrideAffected" + end + ensure + client.close + end + end + end + end +end diff --git a/spec/ldclient_overrides_spec.rb b/spec/ldclient_overrides_spec.rb new file mode 100644 index 00000000..47fa6d18 --- /dev/null +++ b/spec/ldclient_overrides_spec.rb @@ -0,0 +1,490 @@ +require "spec_helper" +require "capturing_logger" +require "model_builders" +require "override_test_components" +require "ldclient-rb/integrations/test_data_v2" + +module LaunchDarkly + describe LDClient, "with flag overrides" do + let(:context) { LDContext.create({ key: "user-key", kind: "user" }) } + let(:ld_flag) { { key: "flag", version: 100, on: false, offVariation: 0, variations: ["ld-value"] } } + let(:ld_normal) { { key: "normal", version: 100, on: false, offVariation: 0, variations: ["normal-value"] } } + let(:override_flag) { { key: "flag", version: 1, on: false, offVariation: 0, variations: ["override-value"] } } + + def data_system(overrides: nil, initialized: true, flags: {}, segments: {}) + builder = DataSystem.custom + if initialized + builder.initializers([TestDataInitializer.new(flags: flags, segments: segments)]) + else + builder.synchronizers([HangingSynchronizer.new]) + end + builder.overrides(overrides) if overrides + builder.build + end + + # A store that holds the given LaunchDarkly flags but no init marker, like a persistent store + # that LaunchDarkly data was written to before an interrupted initialization. + def store_without_init_marker(*flags) + store = InMemoryFeatureStore.new + flags.each { |f| store.upsert(Impl::DataStore::FEATURES, Impl::Model.deserialize(Impl::DataStore::FEATURES, f)) } + expect(store.initialized?).to be false + store + end + + # A client with a data source that never delivers data, reading LaunchDarkly data from the given + # store. + def uninitialized_data_system_with_store(store, overrides: nil) + builder = DataSystem.custom.synchronizers([HangingSynchronizer.new]).data_store(store, :read_write) + builder.overrides(overrides) if overrides + builder.build + end + + def with_client(data_system_config, wait: nil, logger: $null_log) + config = Config.new(data_system_config: data_system_config, send_events: false, logger: logger) + wait = data_system_config.synchronizers.nil? ? 5 : 0 if wait.nil? + client = LDClient.new("sdk-key", config, wait) + begin + yield client + ensure + client.close + end + end + + describe "configuration" do + it "accepts an override source on the data system builder" do + source = TestOverrideSource.new + config = DataSystem.default.overrides(source).build + + expect(config.overrides).to be source + end + + it "leaves the override source unset by default and allows it to be removed" do + expect(DataSystem.default.build.overrides).to be_nil + expect(DataSystem.custom.overrides(TestOverrideSource.new).overrides(nil).build.overrides).to be_nil + end + + it "reports an override source that cannot be built as a construction error" do + builder = Object.new + builder.define_singleton_method(:build) { |_sdk_key, _config| raise ArgumentError, "no file paths" } + config = Config.new(data_system_config: data_system(overrides: builder), send_events: false, logger: $null_log) + + expect { LDClient.new("sdk-key", config, 0) }.to raise_error(ArgumentError, "no file paths") + end + end + + describe "evaluation when the client is initialized" do + it "serves the override entry in preference to LaunchDarkly data and marks the reason" do + source = TestOverrideSource.new([override_flag]) + with_client(data_system(overrides: source, flags: { flag: ld_flag, normal: ld_normal })) do |client| + expect(client.initialized?).to be true + + detail = client.variation_detail("flag", context, "default") + expect(detail.value).to eq "override-value" + expect(detail.variation_index).to eq 0 + expect(detail.reason).to eq EvaluationReason.off.with_override_affected(true) + expect(client.variation("flag", context, "default")).to eq "override-value" + end + end + + it "leaves a flag without an override unaffected" do + source = TestOverrideSource.new([override_flag]) + with_client(data_system(overrides: source, flags: { flag: ld_flag, normal: ld_normal })) do |client| + detail = client.variation_detail("normal", context, "default") + + expect(detail.value).to eq "normal-value" + expect(detail.reason).to eq EvaluationReason.off + expect(detail.reason.override_affected).to be false + end + end + + it "behaves as without the feature when the source supplies no overrides" do + source = TestOverrideSource.new([]) + with_client(data_system(overrides: source, flags: { flag: ld_flag })) do |client| + detail = client.variation_detail("flag", context, "default") + + expect(detail.value).to eq "ld-value" + expect(detail.reason.override_affected).to be false + expect(client.variation("unknown", context, "default")).to eq "default" + end + end + + it "returns to LaunchDarkly data when the override is removed, and to the default when there is none" do + source = TestOverrideSource.new([override_flag, override_flag.merge(key: "only-override")]) + with_client(data_system(overrides: source, flags: { flag: ld_flag })) do |client| + expect(client.variation("only-override", context, "default")).to eq "override-value" + + source.update([]) + + expect(client.variation("flag", context, "default")).to eq "ld-value" + detail = client.variation_detail("only-override", context, "default") + expect(detail.value).to eq "default" + expect(detail.reason).to eq EvaluationReason.error(EvaluationReason::ERROR_FLAG_NOT_FOUND) + end + end + + it "keeps the marking when evaluating an overridden flag raises" do + source = TestOverrideSource.new([override_flag]) + with_client(data_system(overrides: source, flags: { flag: ld_flag, normal: ld_normal })) do |client| + allow(client.instance_variable_get(:@evaluator)).to receive(:evaluate).and_raise("evaluation failed") + + detail = client.variation_detail("flag", context, "default") + expect(detail.value).to eq "default" + expect(detail.reason).to eq EvaluationReason.error(EvaluationReason::ERROR_EXCEPTION).with_override_affected(true) + + normal = client.variation_detail("normal", context, "default") + expect(normal.reason).to eq EvaluationReason.error(EvaluationReason::ERROR_EXCEPTION) + end + end + + it "returns the EXCEPTION result for a flag that a custom store returns as a plain hash" do + # The FeatureStore interface documents its entities as hashes, while the SDK's own stores + # return model objects. The evaluator raises on a plain hash, and that failure must still give + # the EXCEPTION result, with or without an override source, as it did before overrides. + flag_hash = { key: "plain", version: 1, on: false, offVariation: 0, variations: ["value"] } + store = InMemoryFeatureStore.new + store.define_singleton_method(:initialized?) { true } + store.define_singleton_method(:get) { |kind, key| kind == Impl::DataStore::FEATURES && key == "plain" ? flag_hash : nil } + store.define_singleton_method(:all) { |kind| kind == Impl::DataStore::FEATURES ? { plain: flag_hash } : {} } + [nil, TestOverrideSource.new([override_flag.merge(key: "other")])].each do |source| + builder = DataSystem.custom.data_store(store, :read_only) + builder.overrides(source) if source + with_client(builder.build) do |client| + detail = client.variation_detail("plain", context, "default") + expect(detail.value).to eq "default" + expect(detail.reason).to eq EvaluationReason.error(EvaluationReason::ERROR_EXCEPTION) + + meta = client.all_flags_state(context, with_reasons: true).as_json["$flagsState"] + expect(meta["plain"][:reason]).to eq EvaluationReason.error(EvaluationReason::ERROR_EXCEPTION) + end + end + end + + it "keeps the marking when a migration stage is not a valid stage" do + source = TestOverrideSource.new([override_flag.merge(variations: ["not-a-stage"])]) + with_client(data_system(overrides: source, flags: { flag: ld_flag })) do |client| + stage, tracker = client.migration_variation("flag", context, Migrations::STAGE_OFF) + + expect(stage).to eq Migrations::STAGE_OFF + detail = tracker.instance_variable_get(:@detail) + expect(detail.reason.error_kind).to eq EvaluationReason::ERROR_WRONG_TYPE + expect(detail.reason.override_affected).to be true + end + end + end + + describe "evaluation before the client is initialized" do + it "serves an overridden flag" do + source = TestOverrideSource.new([override_flag]) + with_client(data_system(overrides: source, initialized: false)) do |client| + expect(client.initialized?).to be false + + detail = client.variation_detail("flag", context, "default") + expect(detail.value).to eq "override-value" + expect(detail.reason).to eq EvaluationReason.off.with_override_affected(true) + end + end + + it "returns the default with a client-not-ready reason for a flag that is not overridden" do + source = TestOverrideSource.new([override_flag]) + logger = CapturingLogger.new + with_client(data_system(overrides: source, initialized: false), logger: logger) do |client| + detail = client.variation_detail("other", context, "default") + + expect(detail.value).to eq "default" + expect(detail.variation_index).to be_nil + expect(detail.reason).to eq EvaluationReason.error(EvaluationReason::ERROR_CLIENT_NOT_READY) + expect(logger.output).to include("Client has not finished initializing") + end + end + + it "does not serve LaunchDarkly data from a store without an init marker for a flag that is not overridden" do + # Configuring a source changes nothing for a flag that it does not override: the result is + # the not-ready default, as it is without a source. + [nil, TestOverrideSource.new([override_flag.merge(key: "other")])].each do |source| + store = store_without_init_marker(ld_flag) + with_client(uninitialized_data_system_with_store(store, overrides: source)) do |client| + detail = client.variation_detail("flag", context, "default") + + expect(detail.value).to eq "default" + expect(detail.reason).to eq EvaluationReason.error(EvaluationReason::ERROR_CLIENT_NOT_READY) + end + end + end + + it "still short-circuits when no override source is configured" do + with_client(data_system(initialized: false)) do |client| + detail = client.variation_detail("flag", context, "default") + + expect(detail.reason).to eq EvaluationReason.error(EvaluationReason::ERROR_CLIENT_NOT_READY) + end + end + + it "does not report the client as initialized because of overrides" do + source = TestOverrideSource.new([override_flag]) + with_client(data_system(overrides: source, initialized: false)) do |client| + expect(client.initialized?).to be false + expect(client.data_source_status_provider.status.state).not_to eq Interfaces::DataSource::Status::VALID + end + end + end + + describe "all_flags_state" do + it "reflects overrides and includes flags that exist only in the override store" do + source = TestOverrideSource.new([override_flag, override_flag.merge(key: "only-override", variations: ["extra"])]) + with_client(data_system(overrides: source, flags: { flag: ld_flag, normal: ld_normal })) do |client| + state = client.all_flags_state(context, with_reasons: true) + + expect(state.valid?).to be true + expect(state.values_map).to eq({ "flag" => "override-value", "normal" => "normal-value", "only-override" => "extra" }) + json = state.as_json + expect(json["$flagsState"]["flag"][:reason]).to eq EvaluationReason.off.with_override_affected(true) + expect(json["$flagsState"]["normal"][:reason]).to eq EvaluationReason.off + end + end + + it "returns only the overridden flags before initialization and logs that once" do + source = TestOverrideSource.new([override_flag]) + logger = CapturingLogger.new + with_client(data_system(overrides: source, initialized: false), logger: logger) do |client| + state = client.all_flags_state(context) + client.all_flags_state(context) + + expect(state.valid?).to be true + expect(state.values_map).to eq({ "flag" => "override-value" }) + expect(logger.output.scan("returning only flags from the override store").length).to eq 1 + end + end + + it "returns an invalid state before initialization when the override store is empty" do + source = TestOverrideSource.new([]) + with_client(data_system(overrides: source, initialized: false)) do |client| + state = client.all_flags_state(context) + + expect(state.valid?).to be false + end + end + + it "returns an invalid state before initialization when the override store is empty and the store holds LaunchDarkly data without an init marker" do + [nil, TestOverrideSource.new([])].each do |source| + with_client(uninitialized_data_system_with_store(store_without_init_marker(ld_flag), overrides: source)) do |client| + state = client.all_flags_state(context) + + expect(state.valid?).to be false + expect(state.values_map).to eq({}) + end + end + end + + it "returns only the overridden flags before initialization when the store holds LaunchDarkly data without an init marker" do + source = TestOverrideSource.new([override_flag.merge(key: "other")]) + with_client(uninitialized_data_system_with_store(store_without_init_marker(ld_flag), overrides: source)) do |client| + state = client.all_flags_state(context) + + expect(state.valid?).to be true + expect(state.values_map).to eq({ "other" => "override-value" }) + end + end + + it "returns an invalid state of the overridden flags and logs the failure when the store cannot list its flags" do + # A daemon-mode client reads its store directly. Overrides only add to the state: without + # them it is invalid and empty, so with them it holds the overridden flags and stays invalid. + store = InMemoryFeatureStore.new + store.define_singleton_method(:all) { |_kind| raise "store failed" } + source = TestOverrideSource.new([override_flag]) + logger = CapturingLogger.new + with_client(DataSystem.custom.data_store(store, :read_only).overrides(source).build, logger: logger) do |client| + expect(client.initialized?).to be true + state = client.all_flags_state(context, with_reasons: true) + + expect(state.valid?).to be false + expect(state.values_map).to eq({ "flag" => "override-value" }) + json = state.as_json + expect(json["$valid"]).to be false + expect(json["flag"]).to eq "override-value" + expect(json["$flagsState"]["flag"][:reason]).to eq EvaluationReason.off.with_override_affected(true) + expect(logger.output).to include("Unable to read flags for all_flags_state") + expect(logger.output).to include("store failed") + end + end + + it "returns an invalid, empty state and logs the failure when the store cannot list its flags and no flags are overridden" do + store = InMemoryFeatureStore.new + store.define_singleton_method(:all) { |_kind| raise "store failed" } + [nil, TestOverrideSource.new([])].each do |source| + logger = CapturingLogger.new + with_client(DataSystem.custom.data_store(store, :read_only).overrides(source).build, logger: logger) do |client| + state = client.all_flags_state(context) + + expect(state.valid?).to be false + expect(state.values_map).to eq({}) + expect(logger.output).to include("Unable to read flags for all_flags_state") + end + end + end + + it "reports a flag whose evaluation raises with an error reason that keeps the marking" do + source = TestOverrideSource.new([override_flag]) + with_client(data_system(overrides: source, flags: { flag: ld_flag, normal: ld_normal })) do |client| + allow(client.instance_variable_get(:@evaluator)).to receive(:evaluate).and_raise("evaluation failed") + + state = client.all_flags_state(context, with_reasons: true) + + expect(state.valid?).to be true + expect(state.values_map).to eq({ "flag" => nil, "normal" => nil }) + json = state.as_json + expect(json["$flagsState"]["flag"][:reason]).to eq( + EvaluationReason.error(EvaluationReason::ERROR_EXCEPTION).with_override_affected(true)) + expect(json["$flagsState"]["normal"][:reason]).to eq EvaluationReason.error(EvaluationReason::ERROR_EXCEPTION) + end + end + + it "reports no prerequisites for a flag whose evaluation raises after a flag with prerequisites" do + # Each invocation of the block gets its own local variables, so a failed flag cannot pick up + # the evaluation state of the flag evaluated before it. + prereq = { key: "prereq", version: 1, on: false, offVariation: 0, variations: ["p"] } + holder = { key: "holder", version: 1, on: true, fallthrough: { variation: 0 }, variations: ["h"], + prerequisites: [{ key: "prereq", variation: 0 }] } + failing = { key: "failing", version: 1, on: false, offVariation: 0, variations: ["f"] } + with_client(data_system(flags: { prereq: prereq, holder: holder, failing: failing })) do |client| + order = [] + allow(client.instance_variable_get(:@evaluator)).to receive(:evaluate).and_wrap_original do |original, flag, ctx| + order << flag[:key] + raise "evaluation failed" if flag[:key] == "failing" + original.call(flag, ctx) + end + + meta = client.all_flags_state(context, with_reasons: true).as_json["$flagsState"] + + # The flag evaluated just before the failing one has a prerequisite. + expect(order[order.index("failing") - 1]).to eq "holder" + expect(meta["holder"][:prerequisites]).to eq ["prereq"] + expect(meta["failing"]).not_to have_key(:prerequisites) + expect(meta["failing"][:reason]).to eq EvaluationReason.error(EvaluationReason::ERROR_EXCEPTION) + end + end + + it "returns an invalid state before initialization when no override source is configured" do + with_client(data_system(initialized: false)) do |client| + expect(client.all_flags_state(context).valid?).to be false + end + end + end + + describe "flag change notifications" do + let(:prereq_ld) { { key: "prereq", version: 100, on: true, fallthrough: { variation: 0 }, offVariation: 0, variations: ["p"] } } + let(:dependent_ld) do + { key: "dependent", version: 100, on: true, prerequisites: [{ key: "prereq", variation: 0 }], + fallthrough: { variation: 0 }, offVariation: 0, variations: ["d"] } + end + + it "fires when an override is added, changed, and removed" do + source = TestOverrideSource.new([]) + with_client(data_system(overrides: source, flags: { flag: ld_flag })) do |client| + listener = CollectingFlagChangeListener.new + client.flag_tracker.add_listener(listener) + + source.update([override_flag]) + expect(listener.collect).to eq %w[flag] + + source.update([override_flag.merge(variations: ["changed"])]) + expect(listener.collect).to eq %w[flag] + + source.update([]) + expect(listener.collect).to eq %w[flag] + end + end + + it "fires for flags that depend on an overridden prerequisite" do + source = TestOverrideSource.new([]) + with_client(data_system(overrides: source, flags: { prereq: prereq_ld, dependent: dependent_ld, flag: ld_flag })) do |client| + listener = CollectingFlagChangeListener.new + client.flag_tracker.add_listener(listener) + + source.update([prereq_ld.merge(version: 200, variations: ["p2"])]) + + expect(listener.collect).to eq %w[dependent prereq] + end + end + + it "fires for an override flag when the LaunchDarkly prerequisite it depends on changes" do + td = Integrations::TestDataV2.data_source + td.update(td.flag("prereq").on(true)) + override_dependent = { key: "override-dependent", version: 1, on: true, prerequisites: [{ key: "prereq", variation: 0 }], + fallthrough: { variation: 0 }, offVariation: 0, variations: ["d"] } + source = TestOverrideSource.new([override_dependent]) + config = DataSystem.custom.synchronizers([td.test_data_ds_builder]).overrides(source).build + with_client(config, wait: 5) do |client| + expect(client.initialized?).to be true + listener = CollectingFlagChangeListener.new + client.flag_tracker.add_listener(listener) + + td.update(td.flag("prereq").on(false)) + + expect(listener.collect).to eq %w[override-dependent prereq] + end + end + + it "reports the new value to a flag value change listener" do + source = TestOverrideSource.new([]) + with_client(data_system(overrides: source, flags: { flag: ld_flag })) do |client| + changes = Queue.new + listener = Object.new + listener.define_singleton_method(:update) { |change| changes << change } + client.flag_tracker.add_flag_value_change_listener("flag", context, listener) + + source.update([override_flag]) + + change = TestQueues.pop_within(changes, 2) + expect(change).not_to be_nil + expect(change.old_value).to eq "ld-value" + expect(change.new_value).to eq "override-value" + end + end + end + + describe "lifecycle" do + it "starts the source during construction and stops it when the client closes" do + source = TestOverrideSource.new([override_flag]) + config = Config.new(data_system_config: data_system(overrides: source, flags: { flag: ld_flag }), send_events: false, logger: $null_log) + client = LDClient.new("sdk-key", config, 5) + + expect(source.started?).to be true + client.close + expect(source.stopped?).to be true + end + + it "has no effect when the client is offline, and logs that" do + source = TestOverrideSource.new([override_flag]) + logger = CapturingLogger.new + config = Config.new(data_system_config: data_system(overrides: source), offline: true, logger: logger) + client = LDClient.new("sdk-key", config, 0) + begin + expect(source.started?).to be false + expect(client.variation("flag", context, "default")).to eq "default" + expect(logger.output).to include("Offline mode is enabled; the override source is not started and no overrides are served") + ensure + client.close + end + end + + it "starts and runs without overrides when the source raises from start" do + # The SDK's own source never raises from start. A custom source could, and that must not + # keep the client from starting. The snapshot it supplied before failing is removed. + source = TestOverrideSource.new([override_flag]) + source.define_singleton_method(:start) do |sink| + sink.set_overrides([{ key: "flag", version: 1, on: false, offVariation: 0, variations: ["override-value"] }], []) + raise "source failed" + end + logger = CapturingLogger.new + with_client(data_system(overrides: source, flags: { flag: ld_flag }), logger: logger) do |client| + expect(source.stopped?).to be true + expect(logger.output).to include("Unable to start the override source; the client runs without overrides") + + detail = client.variation_detail("flag", context, "default") + expect(detail.value).to eq "ld-value" + expect(detail.reason.override_affected).to be false + end + end + end + end +end diff --git a/spec/override_test_components.rb b/spec/override_test_components.rb new file mode 100644 index 00000000..5b021390 --- /dev/null +++ b/spec/override_test_components.rb @@ -0,0 +1,196 @@ +require "ldclient-rb/interfaces" +require "ldclient-rb/impl/data_store" +require "ldclient-rb/impl/file_data" + +require "concurrent" + +module LaunchDarkly + # + # Queue helpers for tests. Queue#pop accepts a timeout only from Ruby 3.2, and the SDK also runs + # on Ruby 3.1 and JRuby 9.4, where that call returns at once. + # + module TestQueues + # Waits up to the given number of seconds for an item from the queue, and returns it or nil. + def self.pop_within(queue, seconds) + deadline = Time.now + seconds + loop do + return queue.pop(true) unless queue.empty? + return nil if Time.now >= deadline + + sleep 0.01 + end + end + end + + # + # An override source for tests. It supplies the given definitions when started, keeps the sink + # so that a test can supply further snapshots, and records its lifecycle. It acts as its own + # builder so that it can be passed to ConfigBuilder#overrides directly. + # + class TestOverrideSource + include Interfaces::Overrides::OverrideSource + + attr_reader :sink, :build_args + + def initialize(flags = [], segments = []) + @flags = flags + @segments = segments + @sink = nil + @started = Concurrent::AtomicBoolean.new(false) + @stopped = Concurrent::AtomicBoolean.new(false) + @build_args = nil + end + + def build(sdk_key, config) + @build_args = [sdk_key, config] + self + end + + def start(sink) + @started.make_true + @sink = sink + sink.set_overrides(@flags, @segments) + end + + def stop + @stopped.make_true + end + + def started? + @started.value + end + + def stopped? + @stopped.value + end + + # Supplies a new snapshot through the sink the SDK passed to start. + def update(flags, segments = []) + @sink.set_overrides(flags, segments) + end + end + + # + # An initializer for tests that delivers the given flag and segment data as a full transfer with a + # defined selector, which is what makes the client report that it is initialized. + # + class TestDataInitializer + include Interfaces::DataSystem::Initializer + + def initialize(flags: {}, segments: {}, selector: Interfaces::DataSystem::Selector.new(state: "test", version: 1)) + @flags = flags + @segments = segments + @selector = selector + end + + def build(_sdk_key, _config) + self + end + + def name + "TestDataInitializer" + end + + def fetch(_selector_store) + builder = Interfaces::DataSystem::ChangeSetBuilder.new + builder.start(Interfaces::DataSystem::IntentCode::TRANSFER_FULL) + @flags.each do |key, flag| + builder.add_put(Interfaces::DataSystem::ObjectKind::FLAG, key.to_sym, flag[:version] || 1, flag) + end + @segments.each do |key, segment| + builder.add_put(Interfaces::DataSystem::ObjectKind::SEGMENT, key.to_sym, segment[:version] || 1, segment) + end + basis = Interfaces::DataSystem::Basis.new(change_set: builder.finish(@selector), persist: false) + Interfaces::DataSystem::FetchResult.new(result: Result.success(basis)) + end + end + + # + # A synchronizer for tests that never delivers anything. With it configured, the client has a data + # source but no data, so it applies its not-initialized handling instead of treating the empty + # store as cached data. + # + class HangingSynchronizer + include Interfaces::DataSystem::Synchronizer + + def initialize + @stop_event = Concurrent::Event.new + end + + def build(_sdk_key, _config) + self + end + + def name + "HangingSynchronizer" + end + + def sync(_selector_store) + @stop_event.wait + end + + def stop + @stop_event.set + end + end + + # + # An event processor for tests that records the evaluation records the client hands to it. + # + class RecordingEventProcessor + include EventProcessorMethods + + EvalRecord = Struct.new(:context, :key, :version, :variation, :value, :reason, :default, :track_events, + :debug_until, :prereq_of, :sampling_ratio, :exclude_from_summaries, :override_affected) + + attr_reader :records + + def initialize + @records = [] + @lock = Mutex.new + end + + def record_eval_event(*args) + @lock.synchronize { @records << EvalRecord.new(*args) } + end + + # Returns the records for a flag key, in order. + def records_for(key) + @lock.synchronize { @records.select { |r| r.key == key } } + end + end + + # + # A flag change listener for tests that collects the changed keys. + # + class CollectingFlagChangeListener + def initialize + @changes = Queue.new + end + + def update(flag_change) + @changes << flag_change.key + end + + # Waits for the next change and returns its key, or nil after the timeout. + def next_key(timeout = 2) + TestQueues.pop_within(@changes, timeout) + end + + # Collects every key that arrives within the settle time after the first one. + def collect(timeout = 2, settle = 0.3) + keys = [] + first = next_key(timeout) + return keys if first.nil? + + keys << first + loop do + key = next_key(settle) + break if key.nil? + + keys << key + end + keys.sort + end + end +end