From 1912e39e75a56e71f08fa9053709c32a22ba1b73 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 25 Sep 2026 15:23:12 -0700 Subject: [PATCH 1/9] feat: Add the override store, overlay, and data system wiring Adds the override layer that the OVERRIDE specification describes: a runtime-mutable collection of flag and segment definitions that takes precedence over LaunchDarkly data at the store read boundary. - `LaunchDarkly::Interfaces::Overrides::OverrideSource` and `OverrideSink` define how a source supplies complete snapshots to the SDK. - `Impl::Overrides::Layer` holds marked entries in an immutable hash that is swapped on update, so the layer holds exactly one snapshot at any instant. - `Impl::Overrides::Overlay` sits at the store read boundary. A read for a key returns the override entry when one exists and the LaunchDarkly entry otherwise. An enumeration is the union with the override entry winning. Initialization status is the base store's alone. - `Impl::Overrides::Sink` applies snapshots and notifies flag change listeners of every flag whose merged-view evaluation may have changed, including flags that depend on an overridden prerequisite or segment. - `DataSystem::ConfigBuilder#overrides` accepts one override source builder. The FDv2 data system builds it at construction, so an invalid configuration raises from `LDClient.new`, starts it before its run loop so the initial load is in effect before the constructor returns, serves reads through the overlay, and stops it when the client closes. Offline clients build no source. - The client consults the override store before the not-initialized short-circuit: an overridden flag is served before LaunchDarkly data arrives, and a flag that is not overridden still returns the client-not-ready default. `all_flags_state` reads through the overlay and, before initialization, returns only the overridden flags. A wrong-type migration result keeps the marking of the evaluation it replaces. - `FileData.make_flag_with_value` gains an off form, which the override source uses so that a value-only override reports the OFF reason kind. The file data sources keep their existing form. The OVERRIDE specification's test vectors run as a spec through the full client stack. The per-evaluation summary marker in the vectors is asserted once the event processor carries it. Flag overrides are currently experimental and subject to change. --- lib/ldclient-rb/config.rb | 12 +- lib/ldclient-rb/data_system/config_builder.rb | 33 +- lib/ldclient-rb/impl/data_store/store.rb | 21 ++ lib/ldclient-rb/impl/data_system.rb | 10 + lib/ldclient-rb/impl/data_system/fdv1.rb | 5 + lib/ldclient-rb/impl/data_system/fdv2.rb | 38 +++ lib/ldclient-rb/impl/overrides.rb | 5 + lib/ldclient-rb/impl/overrides/layer.rb | 98 ++++++ lib/ldclient-rb/impl/overrides/overlay.rb | 80 +++++ lib/ldclient-rb/impl/overrides/sink.rb | 178 ++++++++++ lib/ldclient-rb/interfaces.rb | 1 + lib/ldclient-rb/interfaces/overrides.rb | 82 +++++ lib/ldclient-rb/ldclient.rb | 39 ++- spec/fixtures/override-vectors/vectors.json | 316 ++++++++++++++++++ spec/impl/data_system/fdv2_overrides_spec.rb | 160 +++++++++ spec/impl/overrides/layer_spec.rb | 117 +++++++ spec/impl/overrides/overlay_spec.rb | 145 ++++++++ spec/impl/overrides/sink_spec.rb | 265 +++++++++++++++ spec/ldclient_override_vectors_spec.rb | 93 ++++++ spec/ldclient_overrides_spec.rb | 283 ++++++++++++++++ spec/override_test_components.rb | 153 +++++++++ 21 files changed, 2131 insertions(+), 3 deletions(-) create mode 100644 lib/ldclient-rb/impl/overrides.rb create mode 100644 lib/ldclient-rb/impl/overrides/layer.rb create mode 100644 lib/ldclient-rb/impl/overrides/overlay.rb create mode 100644 lib/ldclient-rb/impl/overrides/sink.rb create mode 100644 lib/ldclient-rb/interfaces/overrides.rb create mode 100644 spec/fixtures/override-vectors/vectors.json create mode 100644 spec/impl/data_system/fdv2_overrides_spec.rb create mode 100644 spec/impl/overrides/layer_spec.rb create mode 100644 spec/impl/overrides/overlay_spec.rb create mode 100644 spec/impl/overrides/sink_spec.rb create mode 100644 spec/ldclient_override_vectors_spec.rb create mode 100644 spec/ldclient_overrides_spec.rb create mode 100644 spec/override_test_components.rb 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..c5a8c8c1 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,35 @@ 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. + # + # @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 +126,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/impl/data_store/store.rb b/lib/ldclient-rb/impl/data_store/store.rb index f374b9d4..5dd74113 100644 --- a/lib/ldclient-rb/impl/data_store/store.rb +++ b/lib/ldclient-rb/impl/data_store/store.rb @@ -179,6 +179,27 @@ 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 + # # Get the data store status provider for the persistent store, if configured. # diff --git a/lib/ldclient-rb/impl/data_system.rb b/lib/ldclient-rb/impl/data_system.rb index d137d460..e98b9415 100644 --- a/lib/ldclient-rb/impl/data_system.rb +++ b/lib/ldclient-rb/impl/data_system.rb @@ -108,6 +108,16 @@ def store raise NotImplementedError, "#{self.class} must implement #store" end + # + # Returns whether the data system was built with an override source. The value is fixed at + # construction. Flag overrides are currently experimental and subject to change. + # + # @return [Boolean] + # + def override_source_configured? + raise NotImplementedError, "#{self.class} must implement #override_source_configured?" + 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..72b7617e 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_source_configured?) + def override_source_configured? + false + 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..abe3c2ae 100644 --- a/lib/ldclient-rb/impl/data_system/fdv2.rb +++ b/lib/ldclient-rb/impl/data_system/fdv2.rb @@ -8,6 +8,7 @@ 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/interfaces/data_system" @@ -118,6 +119,21 @@ 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 + if @data_system_config.overrides && !@disabled + # 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) + end end # (see DataSystem#start) @@ -131,6 +147,13 @@ 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. + if @override_source + sink = LaunchDarkly::Impl::Overrides::Sink.new(@override_layer, @store, @flag_change_broadcaster, @logger) + @override_source.start(sink) + end + # Start the main coordination thread main_thread = Thread.new { run_main_loop } main_thread.name = "FDv2-main" @@ -141,6 +164,14 @@ def start # (see DataSystem#stop) def stop + if @override_source + begin + @override_source.stop + rescue => e + @logger.error { "[LDClient] Error stopping override source: #{e.message}" } + end + end + @stop_event.set @lock.synchronize do @@ -175,9 +206,16 @@ def set_diagnostic_accumulator(diagnostic_accumulator) # (see DataSystem#store) def store + return @overlay if @overlay + @store.get_active_store end + # (see DataSystem#override_source_configured?) + def override_source_configured? + !@override_source.nil? + end + # (see DataSystem#data_source_status_provider) def data_source_status_provider @data_source_status_provider 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..ea64c977 --- /dev/null +++ b/lib/ldclient-rb/impl/overrides/layer.rb @@ -0,0 +1,98 @@ +# frozen_string_literal: true + +require "ldclient-rb/impl/data_store" + +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. + # + class Layer + EMPTY_CONTENTS = { + DataStore::FEATURES => {}.freeze, + DataStore::SEGMENTS => {}.freeze, + }.freeze + private_constant :EMPTY_CONTENTS + + def initialize + @contents = Concurrent::AtomicReference.new(EMPTY_CONTENTS) + 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 + previous = @contents.get_and_set(replacement) + [previous, replacement] + 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 = @contents.get[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) + @contents.get[kind] || {} + end + + # + # Returns the current contents, keyed by data kind. The returned hash must not be modified. + # + # @return [Hash] + # + def contents + @contents.get + end + + # + # Returns true if the layer contains no entries. + # + # @return [Boolean] + # + def empty? + @contents.get.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..91d6f71b --- /dev/null +++ b/lib/ldclient-rb/impl/overrides/overlay.rb @@ -0,0 +1,80 @@ +# 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. + # + # When the base store fails and the layer holds entries, the result is the layer's entries + # alone, with no error. A per-key read serves those entries whatever the state of the base, + # so an all-flags read does the same. When the layer is empty, the base error is raised. + # + # (see LaunchDarkly::Interfaces::DataSystem::ReadOnlyStore#all) + # + def all(kind) + overrides = @layer.all(kind) + begin + base_items = @base.all(kind) + rescue + raise if overrides.empty? + + base_items = {} + end + 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..e3c2f943 --- /dev/null +++ b/lib/ldclient-rb/impl/overrides/sink.rb @@ -0,0 +1,178 @@ +# 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 + 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) + flag_items = index_items(DataStore::FEATURES, flags) + segment_items = index_items(DataStore::SEGMENTS, segments) + + @lock.synchronize do + # 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 + 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 + + # + # 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/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..ebac58b5 --- /dev/null +++ b/lib/ldclient-rb/interfaces/overrides.rb @@ -0,0 +1,82 @@ +# 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`. + # + # @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. + # + # @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..1aa0b799 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 @@ -177,6 +179,10 @@ def postfork(wait_for_sec = 5) @data_system = Impl::DataSystem::FDv2.new(@sdk_key, @config, data_system_config) end + # True when the data system was built with an override source. The override store is then + # consulted before the not-initialized short-circuit. + @overrides_configured = @data_system.override_source_configured? + # Components not managed by data system @big_segment_store_manager = Impl::BigSegmentStoreManager.new(@config.big_segments, @config.logger) @big_segment_store_status_provider = @big_segment_store_manager.status_provider @@ -511,7 +517,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,11 +641,16 @@ def all_flags_state(context, options={}) check_forked + overrides_only = false 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 + elsif @overrides_configured + # No data from LaunchDarkly is available. The store read below returns only the entries + # that the override store holds. The result decides the state. + overrides_only = true else @config.logger.warn { "Called all_flags_state before client initialization. Data store not available; returning empty state" } return FeatureFlagsState.new(false) @@ -655,6 +670,16 @@ def all_flags_state(context, options={}) return FeatureFlagsState.new(false) end + if overrides_only + if features.empty? + @config.logger.warn { "Called all_flags_state before client initialization. Data store not available; returning empty state" } + return FeatureFlagsState.new(false) + end + 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 + end + state = FeatureFlagsState.new(true) client_only = options[:client_side_only] || false with_reasons = options[:with_reasons] || false @@ -769,11 +794,16 @@ def flag_tracker return detail, nil, context.error end + no_launchdarkly_data = false 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 + elsif @overrides_configured + # No data from LaunchDarkly is available. The store read below still finds an entry that + # the override store holds, and the SDK serves it. A miss returns the not-ready default. + no_launchdarkly_data = true 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) @@ -789,6 +819,13 @@ def flag_tracker end if feature.nil? + if no_launchdarkly_data + @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 + @config.logger.info { "[LDClient] Unknown feature flag \"#{key}\". Returning default value" } detail = Evaluator.error_result(EvaluationReason::ERROR_FLAG_NOT_FOUND, default) record_unknown_flag_eval(key, context, default, detail.reason, with_reasons) diff --git a/spec/fixtures/override-vectors/vectors.json b/spec/fixtures/override-vectors/vectors.json new file mode 100644 index 00000000..48c3347c --- /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": "OFF", "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": "OFF", "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_system/fdv2_overrides_spec.rb b/spec/impl/data_system/fdv2_overrides_spec.rb new file mode 100644 index 00000000..419bac45 --- /dev/null +++ b/spec/impl/data_system/fdv2_overrides_spec.rb @@ -0,0 +1,160 @@ +# frozen_string_literal: true + +require "spec_helper" +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_source_configured?).to be false + 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.override_source_configured?).to be true + 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 "does not build the override source when the SDK is offline" do + source = TestOverrideSource.new([override_flag]) + offline = LaunchDarkly::Config.new(logger: $null_log, offline: true) + with_fdv2(config_with(overrides: source), config: offline) do |fdv2| + fdv2.start + + expect(source.build_args).to be_nil + expect(fdv2.override_source_configured?).to be false + expect(fdv2.store.get(DataStore::FEATURES, "flag")).to be_nil + end + 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..a1dc66a7 --- /dev/null +++ b/spec/impl/overrides/layer_spec.rb @@ -0,0 +1,117 @@ +# 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"] }) } + + subject(:layer) { Layer.new } + + 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 "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..93f061f6 --- /dev/null +++ b/spec/impl/overrides/overlay_spec.rb @@ -0,0 +1,145 @@ +# 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 "returns the override entries alone when the base fails and the layer has entries" do + failing = double("base") + allow(failing).to receive(:all).and_raise(RuntimeError, "store down") + overlay = Overlay.new(failing, layer) + layer.set_all({ c: override_c }, {}) + + expect(overlay.all(DataStore::FEATURES).keys).to eq [:c] + 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..403146a4 --- /dev/null +++ b/spec/impl/overrides/sink_spec.rb @@ -0,0 +1,265 @@ +# 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 "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/ldclient_override_vectors_spec.rb b/spec/ldclient_override_vectors_spec.rb new file mode 100644 index 00000000..54b13fec --- /dev/null +++ b/spec/ldclient_override_vectors_spec.rb @@ -0,0 +1,93 @@ +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) + LDClient.new("sdk-key", config, wait) + 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 = 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) + 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..d0c70c18 --- /dev/null +++ b/spec/ldclient_overrides_spec.rb @@ -0,0 +1,283 @@ +require "spec_helper" +require "capturing_logger" +require "model_builders" +require "override_test_components" + +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 + + 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 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 "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 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 "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 = changes.pop(timeout: 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" do + source = TestOverrideSource.new([override_flag]) + config = Config.new(data_system_config: data_system(overrides: source), offline: true, logger: $null_log) + client = LDClient.new("sdk-key", config, 0) + begin + expect(source.started?).to be false + expect(client.variation("flag", context, "default")).to eq "default" + ensure + client.close + 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..bcfb0e62 --- /dev/null +++ b/spec/override_test_components.rb @@ -0,0 +1,153 @@ +require "ldclient-rb/interfaces" +require "ldclient-rb/impl/data_store" +require "ldclient-rb/impl/file_data" + +require "concurrent" + +module LaunchDarkly + # + # 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 + + # + # 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) + @changes.pop(timeout: 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 From 45c6daac41c9c6e401d3d98de1c94173c25f69b7 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 1 Oct 2026 23:07:40 +0000 Subject: [PATCH 2/9] test: Vendor the specification vectors with the fallthrough expansion --- spec/fixtures/override-vectors/vectors.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/fixtures/override-vectors/vectors.json b/spec/fixtures/override-vectors/vectors.json index 48c3347c..82dc62da 100644 --- a/spec/fixtures/override-vectors/vectors.json +++ b/spec/fixtures/override-vectors/vectors.json @@ -30,7 +30,7 @@ "expect": { "value": "override-value", "variationIndex": 0, - "reason": { "kind": "OFF", "overrideAffected": true }, + "reason": { "kind": "FALLTHROUGH", "overrideAffected": true }, "summaryOverrideAffected": true } }, @@ -53,7 +53,7 @@ "expect": { "value": "override-value", "variationIndex": 0, - "reason": { "kind": "OFF", "overrideAffected": true }, + "reason": { "kind": "FALLTHROUGH", "overrideAffected": true }, "summaryOverrideAffected": true } }, From 54fd4702a5aa2a4b1df2283f862e5658c14249ff Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 16 Oct 2026 21:39:16 +0000 Subject: [PATCH 3/9] feat: Notify override flags when the LaunchDarkly data they depend on changes The override layer keeps a dependency graph of its entries in each snapshot. The store expands every changed LaunchDarkly item through both its own graph and the layer's, to a fixpoint, so a change to a prerequisite flag or a segment that an override entry references notifies that entry's flag as well. This matches how dependent flags behave for LaunchDarkly data. The store's own dependency graph now keys items by string, the form in which the data model names prerequisites and segments. Changes keyed by symbol, which is what the change set carries, previously reached no dependents at all. --- lib/ldclient-rb/impl/data_store/store.rb | 62 ++++++++++-- lib/ldclient-rb/impl/data_system/fdv2.rb | 4 + lib/ldclient-rb/impl/overrides/layer.rb | 49 +++++++-- spec/impl/data_store/store_spec.rb | 123 +++++++++++++++++++++++ spec/impl/overrides/layer_spec.rb | 34 +++++++ spec/ldclient_overrides_spec.rb | 19 ++++ 6 files changed, 278 insertions(+), 13 deletions(-) diff --git a/lib/ldclient-rb/impl/data_store/store.rb b/lib/ldclient-rb/impl/data_store/store.rb index 5dd74113..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 @@ -200,6 +205,20 @@ 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. # @@ -275,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 @@ -359,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 @@ -401,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/fdv2.rb b/lib/ldclient-rb/impl/data_system/fdv2.rb index abe3c2ae..9066eed8 100644 --- a/lib/ldclient-rb/impl/data_system/fdv2.rb +++ b/lib/ldclient-rb/impl/data_system/fdv2.rb @@ -133,6 +133,10 @@ def initialize(sdk_key, config, data_system_config) @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 diff --git a/lib/ldclient-rb/impl/overrides/layer.rb b/lib/ldclient-rb/impl/overrides/layer.rb index ea64c977..8604b1bd 100644 --- a/lib/ldclient-rb/impl/overrides/layer.rb +++ b/lib/ldclient-rb/impl/overrides/layer.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "ldclient-rb/impl/data_store" +require "ldclient-rb/impl/dependency_tracker" require "concurrent/atomics" @@ -20,6 +21,10 @@ module Overrides # 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, @@ -27,8 +32,12 @@ class Layer }.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 - @contents = Concurrent::AtomicReference.new(EMPTY_CONTENTS) + @snapshot = Concurrent::AtomicReference.new(Snapshot.new(EMPTY_CONTENTS, DependencyTracker.new).freeze) end # @@ -47,8 +56,34 @@ def set_all(flags, segments) DataStore::FEATURES => flags.transform_values(&:as_override).freeze, DataStore::SEGMENTS => segments.transform_values(&:as_override).freeze, }.freeze - previous = @contents.get_and_set(replacement) - [previous, replacement] + 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 # @@ -59,7 +94,7 @@ def set_all(flags, segments) # @return [Object, nil] # def get(kind, key) - items = @contents.get[kind] + items = @snapshot.get.contents[kind] return nil if items.nil? items[key.to_sym] @@ -72,7 +107,7 @@ def get(kind, key) # @return [Hash{Symbol => Object}] # def all(kind) - @contents.get[kind] || {} + @snapshot.get.contents[kind] || {} end # @@ -81,7 +116,7 @@ def all(kind) # @return [Hash] # def contents - @contents.get + @snapshot.get.contents end # @@ -90,7 +125,7 @@ def contents # @return [Boolean] # def empty? - @contents.get.each_value.all?(&:empty?) + @snapshot.get.contents.each_value.all?(&:empty?) end end end diff --git a/spec/impl/data_store/store_spec.rb b/spec/impl/data_store/store_spec.rb index c4aa6df2..b7216b0f 100644 --- a/spec/impl/data_store/store_spec.rb +++ b/spec/impl/data_store/store_spec.rb @@ -4,7 +4,9 @@ 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" module LaunchDarkly module Impl @@ -288,6 +290,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 = @keys.pop(timeout: timeout) + return keys if first.nil? + + keys << first + loop do + key = @keys.pop(timeout: 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/overrides/layer_spec.rb b/spec/impl/overrides/layer_spec.rb index a1dc66a7..724f6660 100644 --- a/spec/impl/overrides/layer_spec.rb +++ b/spec/impl/overrides/layer_spec.rb @@ -11,9 +11,23 @@ module Overrides 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 @@ -78,6 +92,26 @@ module Overrides 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 }, {}) diff --git a/spec/ldclient_overrides_spec.rb b/spec/ldclient_overrides_spec.rb index d0c70c18..6e1d1c2e 100644 --- a/spec/ldclient_overrides_spec.rb +++ b/spec/ldclient_overrides_spec.rb @@ -2,6 +2,7 @@ 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 @@ -238,6 +239,24 @@ def with_client(data_system_config, wait: nil, logger: $null_log) 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| From 3ab4bfca0bea32edb339b34aaa41c4565a20f6be Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 28 Oct 2026 22:08:17 +0000 Subject: [PATCH 4/9] fix: Serve overrides-only results from the override store and keep the source from blocking the client Before initialization, all_flags_state returns a state of overrides alone only when the store is not initialized and the override store holds flags, and it reads those flags from the override store itself. An evaluation does the same for the flag it evaluates. Before, a configured source was enough: an empty override store over a store that held LaunchDarkly data without an init marker produced a valid state of that data, and an evaluation served it. The overlay no longer returns the override entries alone when the base store fails to list its items, so a client whose store fails reports an invalid state and logs the failure, as it does without a source. An exception from the source's start no longer prevents the client from starting: it is logged, the source is stopped, a snapshot it supplied is removed, and the client runs without overrides. Stopping the data system closes the sink before it stops the source, so a snapshot supplied during shutdown is ignored. In offline mode the SDK logs that the configured source is not started. --- lib/ldclient-rb/data_system/config_builder.rb | 3 + lib/ldclient-rb/impl/data_system.rb | 13 ++- lib/ldclient-rb/impl/data_system/fdv1.rb | 6 +- lib/ldclient-rb/impl/data_system/fdv2.rb | 66 ++++++++++--- lib/ldclient-rb/impl/overrides/overlay.rb | 15 +-- lib/ldclient-rb/impl/overrides/sink.rb | 70 +++++++++++--- lib/ldclient-rb/interfaces/overrides.rb | 10 +- lib/ldclient-rb/ldclient.rb | 75 ++++++++------- spec/impl/data_system/fdv2_overrides_spec.rb | 44 ++++++++- spec/impl/overrides/overlay_spec.rb | 7 +- spec/impl/overrides/sink_spec.rb | 25 +++++ spec/ldclient_overrides_spec.rb | 95 ++++++++++++++++++- 12 files changed, 333 insertions(+), 96 deletions(-) diff --git a/lib/ldclient-rb/data_system/config_builder.rb b/lib/ldclient-rb/data_system/config_builder.rb index c5a8c8c1..7a4176cc 100644 --- a/lib/ldclient-rb/data_system/config_builder.rb +++ b/lib/ldclient-rb/data_system/config_builder.rb @@ -102,6 +102,9 @@ def data_store(data_store, store_mode) # # 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)) diff --git a/lib/ldclient-rb/impl/data_system.rb b/lib/ldclient-rb/impl/data_system.rb index e98b9415..4bdb7668 100644 --- a/lib/ldclient-rb/impl/data_system.rb +++ b/lib/ldclient-rb/impl/data_system.rb @@ -109,13 +109,16 @@ def store end # - # Returns whether the data system was built with an override source. The value is fixed at - # construction. Flag overrides are currently experimental and subject to change. + # 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. # - # @return [Boolean] + # @param kind [LaunchDarkly::Impl::DataStore::DataKind] + # @return [Hash{Symbol => Object}] # - def override_source_configured? - raise NotImplementedError, "#{self.class} must implement #override_source_configured?" + def override_items(kind) + raise NotImplementedError, "#{self.class} must implement #override_items" end # diff --git a/lib/ldclient-rb/impl/data_system/fdv1.rb b/lib/ldclient-rb/impl/data_system/fdv1.rb index 72b7617e..a646156f 100644 --- a/lib/ldclient-rb/impl/data_system/fdv1.rb +++ b/lib/ldclient-rb/impl/data_system/fdv1.rb @@ -101,9 +101,9 @@ def store @store_wrapper end - # (see DataSystem#override_source_configured?) - def override_source_configured? - false + # (see DataSystem#override_items) + def override_items(_kind) + {} end # (see DataSystem#set_diagnostic_accumulator) diff --git a/lib/ldclient-rb/impl/data_system/fdv2.rb b/lib/ldclient-rb/impl/data_system/fdv2.rb index 9066eed8..e4c0c07a 100644 --- a/lib/ldclient-rb/impl/data_system/fdv2.rb +++ b/lib/ldclient-rb/impl/data_system/fdv2.rb @@ -10,6 +10,7 @@ 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 @@ -127,7 +128,10 @@ def initialize(sdk_key, config, data_system_config) @override_layer = nil @overlay = nil @override_source = nil - if @data_system_config.overrides && !@disabled + @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) @@ -153,10 +157,7 @@ def start # The override source starts before the run loop, so a source that loads synchronously has # its overrides in place before the client begins evaluating. - if @override_source - sink = LaunchDarkly::Impl::Overrides::Sink.new(@override_layer, @store, @flag_change_broadcaster, @logger) - @override_source.start(sink) - end + start_override_source if @override_source # Start the main coordination thread main_thread = Thread.new { run_main_loop } @@ -169,11 +170,11 @@ def start # (see DataSystem#stop) def stop if @override_source - begin - @override_source.stop - rescue => e - @logger.error { "[LDClient] Error stopping override source: #{e.message}" } - end + # 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 @@ -215,9 +216,11 @@ def store @store.get_active_store end - # (see DataSystem#override_source_configured?) - def override_source_configured? - !@override_source.nil? + # (see DataSystem#override_items) + def override_items(kind) + return {} if @override_layer.nil? + + @override_layer.all(kind) end # (see DataSystem#data_source_status_provider) @@ -257,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/overrides/overlay.rb b/lib/ldclient-rb/impl/overrides/overlay.rb index 91d6f71b..4f012704 100644 --- a/lib/ldclient-rb/impl/overrides/overlay.rb +++ b/lib/ldclient-rb/impl/overrides/overlay.rb @@ -43,23 +43,14 @@ def get(kind, key) # # 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. - # - # When the base store fails and the layer holds entries, the result is the layer's entries - # alone, with no error. A per-key read serves those entries whatever the state of the base, - # so an all-flags read does the same. When the layer is empty, the base error is raised. + # 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) - begin - base_items = @base.all(kind) - rescue - raise if overrides.empty? - - base_items = {} - end return base_items if overrides.empty? base_items.merge(overrides) diff --git a/lib/ldclient-rb/impl/overrides/sink.rb b/lib/ldclient-rb/impl/overrides/sink.rb index e3c2f943..9c94dd7c 100644 --- a/lib/ldclient-rb/impl/overrides/sink.rb +++ b/lib/ldclient-rb/impl/overrides/sink.rb @@ -37,6 +37,7 @@ def initialize(layer, base, flag_change_broadcaster, logger) @flag_change_broadcaster = flag_change_broadcaster @logger = logger @lock = Mutex.new + @closed = false end # @@ -47,26 +48,41 @@ def initialize(layer, base, flag_change_broadcaster, logger) # (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 - # 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 + return log_ignored_snapshot if @closed - previous, current = @layer.set_all(flag_items, segment_items) - old_merged = merged_view(previous) - new_merged = merged_view(current) + apply(flag_items, segment_items) + end + end - 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 + # + # 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 @@ -137,6 +153,32 @@ def self.affected_flag_keys(previous, current, old_merged, new_merged) 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. diff --git a/lib/ldclient-rb/interfaces/overrides.rb b/lib/ldclient-rb/interfaces/overrides.rb index ebac58b5..5487e72f 100644 --- a/lib/ldclient-rb/interfaces/overrides.rb +++ b/lib/ldclient-rb/interfaces/overrides.rb @@ -59,7 +59,11 @@ module OverrideSource # 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`. + # The SDK calls this method at most once, before any call to `stop`. An exception raised + # from it does not prevent the client from starting: the SDK logs the exception, stops the + # source, removes any snapshot the source supplied, and the client runs without overrides. + # A source that cannot load its data should log that and retry rather than raise, as the + # SDK's file-based source does. # # @param sink [OverrideSink] # @return [void] @@ -69,7 +73,9 @@ def start(sink) end # - # Stops the source and releases any resources it holds. + # 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] # diff --git a/lib/ldclient-rb/ldclient.rb b/lib/ldclient-rb/ldclient.rb index 1aa0b799..58abd260 100644 --- a/lib/ldclient-rb/ldclient.rb +++ b/lib/ldclient-rb/ldclient.rb @@ -179,10 +179,6 @@ def postfork(wait_for_sec = 5) @data_system = Impl::DataSystem::FDv2.new(@sdk_key, @config, data_system_config) end - # True when the data system was built with an override source. The override store is then - # consulted before the not-initialized short-circuit. - @overrides_configured = @data_system.override_source_configured? - # Components not managed by data system @big_segment_store_manager = Impl::BigSegmentStoreManager.new(@config.big_segments, @config.logger) @big_segment_store_status_provider = @big_segment_store_manager.status_provider @@ -641,19 +637,22 @@ def all_flags_state(context, options={}) check_forked - overrides_only = false + 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 - elsif @overrides_configured - # No data from LaunchDarkly is available. The store read below returns only the entries - # that the override store holds. The result decides the state. - overrides_only = true 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 @@ -663,21 +662,18 @@ 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) - end - - if overrides_only - if features.empty? - @config.logger.warn { "Called all_flags_state before client initialization. Data store not available; returning empty state" } - return FeatureFlagsState.new(false) - end + 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) + return FeatureFlagsState.new(false) + end end state = FeatureFlagsState.new(true) @@ -794,38 +790,34 @@ def flag_tracker return detail, nil, context.error end - no_launchdarkly_data = false + 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 - elsif @overrides_configured - # No data from LaunchDarkly is available. The store read below still finds an entry that - # the override store holds, and the SDK serves it. A miss returns the not-ready default. - no_launchdarkly_data = true 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 if feature.nil? - if no_launchdarkly_data - @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 - @config.logger.info { "[LDClient] Unknown feature flag \"#{key}\". Returning default value" } detail = Evaluator.error_result(EvaluationReason::ERROR_FLAG_NOT_FOUND, default) record_unknown_flag_eval(key, context, default, detail.reason, with_reasons) @@ -853,6 +845,13 @@ def flag_tracker end end + # 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) add_experiment_data = experiment?(flag, detail.reason) @event_processor.record_eval_event( diff --git a/spec/impl/data_system/fdv2_overrides_spec.rb b/spec/impl/data_system/fdv2_overrides_spec.rb index 419bac45..4b73da0e 100644 --- a/spec/impl/data_system/fdv2_overrides_spec.rb +++ b/spec/impl/data_system/fdv2_overrides_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "spec_helper" +require "capturing_logger" require "model_builders" require "override_test_components" require "ldclient-rb/impl/data_system/fdv2" @@ -34,7 +35,7 @@ def with_fdv2(data_system_config, config: self.config) it "reports that no override source is configured by default" do with_fdv2(config_with) do |fdv2| - expect(fdv2.override_source_configured?).to be false + expect(fdv2.override_items(DataStore::FEATURES)).to eq({}) expect(fdv2.store).not_to be_a(Overrides::Overlay) end end @@ -42,7 +43,7 @@ def with_fdv2(data_system_config, config: self.config) 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.override_source_configured?).to be true + expect(fdv2.store).to be_a(Overrides::Overlay) expect(source.build_args).to eq [sdk_key, config] expect(source.started?).to be false end @@ -143,15 +144,48 @@ def with_fdv2(data_system_config, config: self.config) expect(Thread.list.map(&:name)).not_to include("FDv2-main") end - it "does not build the override source when the SDK is offline" do + 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]) - offline = LaunchDarkly::Config.new(logger: $null_log, offline: true) + 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_source_configured?).to be false + 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 diff --git a/spec/impl/overrides/overlay_spec.rb b/spec/impl/overrides/overlay_spec.rb index 93f061f6..ac7e5560 100644 --- a/spec/impl/overrides/overlay_spec.rb +++ b/spec/impl/overrides/overlay_spec.rb @@ -112,13 +112,16 @@ def load_base expect(overlay.all(DataStore::SEGMENTS)).to eq({}) end - it "returns the override entries alone when the base fails and the layer has entries" do + 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).keys).to eq [: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 diff --git a/spec/impl/overrides/sink_spec.rb b/spec/impl/overrides/sink_spec.rb index 403146a4..cc989e29 100644 --- a/spec/impl/overrides/sink_spec.rb +++ b/spec/impl/overrides/sink_spec.rb @@ -77,6 +77,31 @@ def changed_keys(listener) 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) diff --git a/spec/ldclient_overrides_spec.rb b/spec/ldclient_overrides_spec.rb index 6e1d1c2e..2c491122 100644 --- a/spec/ldclient_overrides_spec.rb +++ b/spec/ldclient_overrides_spec.rb @@ -22,6 +22,23 @@ def data_system(overrides: nil, initialized: true, flags: {}, segments: {}) 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? @@ -143,6 +160,20 @@ def with_client(data_system_config, wait: nil, logger: $null_log) 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") @@ -196,6 +227,45 @@ def with_client(data_system_config, wait: nil, logger: $null_log) 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 and logs the failure when the store cannot list its flags, even with overrides" do + # A daemon-mode client reads its store directly. A state of the overrides alone would look + # complete to a consumer that bootstraps from it. + 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) + + 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") + expect(logger.output).to include("store failed") + 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 @@ -286,17 +356,38 @@ def with_client(data_system_config, wait: nil, logger: $null_log) expect(source.stopped?).to be true end - it "has no effect when the client is offline" do + it "has no effect when the client is offline, and logs that" do source = TestOverrideSource.new([override_flag]) - config = Config.new(data_system_config: data_system(overrides: source), offline: true, logger: $null_log) + 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 From 2b7dd38b5be446c10b92caba0a5c3bb3f93ed386 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 28 Oct 2026 22:15:18 +0000 Subject: [PATCH 5/9] test: Wait on queues without Queue#pop's timeout, which Ruby 3.1 and JRuby 9.4 lack --- spec/impl/data_store/store_spec.rb | 5 +++-- spec/ldclient_overrides_spec.rb | 2 +- spec/override_test_components.rb | 19 ++++++++++++++++++- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/spec/impl/data_store/store_spec.rb b/spec/impl/data_store/store_spec.rb index b7216b0f..7d111ef1 100644 --- a/spec/impl/data_store/store_spec.rb +++ b/spec/impl/data_store/store_spec.rb @@ -7,6 +7,7 @@ require "ldclient-rb/impl/overrides" require "ldclient-rb/interfaces/data_system" require "model_builders" +require "override_test_components" module LaunchDarkly module Impl @@ -304,12 +305,12 @@ def update(flag_change) # Every key that arrives within the settle time after the first one, sorted. def collect(timeout = 2, settle = 0.3) keys = [] - first = @keys.pop(timeout: timeout) + first = TestQueues.pop_within(@keys, timeout) return keys if first.nil? keys << first loop do - key = @keys.pop(timeout: settle) + key = TestQueues.pop_within(@keys, settle) break if key.nil? keys << key diff --git a/spec/ldclient_overrides_spec.rb b/spec/ldclient_overrides_spec.rb index 2c491122..c593f7bb 100644 --- a/spec/ldclient_overrides_spec.rb +++ b/spec/ldclient_overrides_spec.rb @@ -337,7 +337,7 @@ def with_client(data_system_config, wait: nil, logger: $null_log) source.update([override_flag]) - change = changes.pop(timeout: 2) + 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" diff --git a/spec/override_test_components.rb b/spec/override_test_components.rb index bcfb0e62..d84f69d1 100644 --- a/spec/override_test_components.rb +++ b/spec/override_test_components.rb @@ -5,6 +5,23 @@ 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 @@ -131,7 +148,7 @@ def update(flag_change) # Waits for the next change and returns its key, or nil after the timeout. def next_key(timeout = 2) - @changes.pop(timeout: timeout) + TestQueues.pop_within(@changes, timeout) end # Collects every key that arrives within the settle time after the first one. From f11d735c3cb17850c50359879e1ada8e3c57af49 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 28 Oct 2026 22:40:50 +0000 Subject: [PATCH 6/9] docs: State that an override source's start is not intended to fail A source logs and retries a failed load, and raises a configuration error from its builder's build. If start raises anyway, the SDK logs it, stops the source, removes its snapshot, and the client runs without overrides. --- lib/ldclient-rb/interfaces/overrides.rb | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/ldclient-rb/interfaces/overrides.rb b/lib/ldclient-rb/interfaces/overrides.rb index 5487e72f..431d9658 100644 --- a/lib/ldclient-rb/interfaces/overrides.rb +++ b/lib/ldclient-rb/interfaces/overrides.rb @@ -59,11 +59,12 @@ module OverrideSource # 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`. An exception raised - # from it does not prevent the client from starting: the SDK logs the exception, stops the - # source, removes any snapshot the source supplied, and the client runs without overrides. - # A source that cannot load its data should log that and retry rather than raise, as the - # SDK's file-based source does. + # 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] From b0505dcd4eb95dab80fe6f55fba068663714d3d9 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 28 Oct 2026 22:48:54 +0000 Subject: [PATCH 7/9] fix: Mark the error reason of an overridden flag whose evaluation raises When the evaluator raises for a flag that came from the override store, variation_detail and all_flags_state now report the EXCEPTION error reason with override_affected set, from the flag's own marker, as the events already do. all_flags_state also no longer raises NoMethodError when an evaluation raises: the failed flag has no evaluation state, so it reports no prerequisites. That crash predates overrides; it made the exception path of all_flags_state unreachable. --- lib/ldclient-rb/ldclient.rb | 11 ++++++++--- spec/ldclient_overrides_spec.rb | 30 ++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/lib/ldclient-rb/ldclient.rb b/lib/ldclient-rb/ldclient.rb index 58abd260..062b7bdb 100644 --- a/lib/ldclient-rb/ldclient.rb +++ b/lib/ldclient-rb/ldclient.rb @@ -688,7 +688,9 @@ 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(f.override?) + detail = EvaluationDetail.new(nil, nil, reason) Impl::Util.log_exception(@config.logger, "Error evaluating flag \"#{k}\" in all_flags_state", exn) end @@ -698,7 +700,8 @@ def all_flags_state(context, options={}) 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, @@ -839,7 +842,9 @@ def flag_tracker [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) + # The flag definition was read before the failure, so its own marker decides the marking. + reason = EvaluationReason.error(EvaluationReason::ERROR_EXCEPTION).with_override_affected(feature.override?) + detail = EvaluationDetail.new(default, nil, reason) record_flag_eval_error(feature, context, default, detail.reason, with_reasons) [detail, feature, exn.to_s] end diff --git a/spec/ldclient_overrides_spec.rb b/spec/ldclient_overrides_spec.rb index c593f7bb..a23cd1fd 100644 --- a/spec/ldclient_overrides_spec.rb +++ b/spec/ldclient_overrides_spec.rb @@ -122,6 +122,20 @@ def with_client(data_system_config, wait: nil, logger: $null_log) 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 "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| @@ -266,6 +280,22 @@ def with_client(data_system_config, wait: nil, logger: $null_log) 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 "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 From 8ba99282eea48f27923388915dbbba0339352000 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 28 Oct 2026 23:24:54 +0000 Subject: [PATCH 8/9] fix: Read the override marker only from model objects on the error paths The FeatureStore interface documents its entities as hashes, and a custom store can return a plain hash. The evaluator raises on one, and the rescue then called override? on the hash, so variation_detail and all_flags_state raised NoMethodError instead of returning the EXCEPTION result they returned before overrides. A helper now reads the marker only from an object that has it. A spec also shows that a flag whose evaluation raises after a flag with prerequisites reports no prerequisites: block locals start fresh on each iteration. --- lib/ldclient-rb/ldclient.rb | 16 +++++++++-- spec/ldclient_overrides_spec.rb | 48 +++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/lib/ldclient-rb/ldclient.rb b/lib/ldclient-rb/ldclient.rb index 062b7bdb..e41f3da1 100644 --- a/lib/ldclient-rb/ldclient.rb +++ b/lib/ldclient-rb/ldclient.rb @@ -689,7 +689,7 @@ def all_flags_state(context, options={}) detail = eval_result.detail rescue => exn # The flag definition was read before the failure, so its own marker decides the marking. - reason = EvaluationReason::error(EvaluationReason::ERROR_EXCEPTION).with_override_affected(f.override?) + 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 @@ -843,7 +843,7 @@ def flag_tracker rescue => exn Impl::Util.log_exception(@config.logger, "Error evaluating feature flag \"#{key}\"", exn) # The flag definition was read before the failure, so its own marker decides the marking. - reason = EvaluationReason.error(EvaluationReason::ERROR_EXCEPTION).with_override_affected(feature.override?) + 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) [detail, feature, exn.to_s] @@ -910,6 +910,18 @@ def flag_tracker false, nil, nil, 1, 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) return false unless reason diff --git a/spec/ldclient_overrides_spec.rb b/spec/ldclient_overrides_spec.rb index a23cd1fd..36af5bf5 100644 --- a/spec/ldclient_overrides_spec.rb +++ b/spec/ldclient_overrides_spec.rb @@ -136,6 +136,29 @@ def with_client(data_system_config, wait: nil, logger: $null_log) 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| @@ -296,6 +319,31 @@ def with_client(data_system_config, wait: nil, logger: $null_log) 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 From 0a4c8b70dd40ca10505093cb30c0199f19568ff9 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 29 Oct 2026 16:45:09 +0000 Subject: [PATCH 9/9] fix: Report the overridden flags in an invalid all flags state when the store fails Overrides only add to the all flags state. When the store cannot list its flags, the state without overrides is invalid and empty, so with overrides it now holds the override store's flags and stays invalid, and the failure is logged as before. A client-side SDK bootstrapped from the state uses its flags whatever $valid says, so the overridden values still reach it. --- lib/ldclient-rb/ldclient.rb | 11 +++++++++-- spec/ldclient_overrides_spec.rb | 29 ++++++++++++++++++++++++----- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/lib/ldclient-rb/ldclient.rb b/lib/ldclient-rb/ldclient.rb index e41f3da1..66fcdbc7 100644 --- a/lib/ldclient-rb/ldclient.rb +++ b/lib/ldclient-rb/ldclient.rb @@ -662,6 +662,7 @@ def all_flags_state(context, options={}) return FeatureFlagsState.new(false) end + 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." } @@ -672,11 +673,17 @@ def all_flags_state(context, options={}) 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) + # 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 diff --git a/spec/ldclient_overrides_spec.rb b/spec/ldclient_overrides_spec.rb index 36af5bf5..47fa6d18 100644 --- a/spec/ldclient_overrides_spec.rb +++ b/spec/ldclient_overrides_spec.rb @@ -285,24 +285,43 @@ def with_client(data_system_config, wait: nil, logger: $null_log) end end - it "returns an invalid state and logs the failure when the store cannot list its flags, even with overrides" do - # A daemon-mode client reads its store directly. A state of the overrides alone would look - # complete to a consumer that bootstraps from it. + 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) + state = client.all_flags_state(context, with_reasons: true) expect(state.valid?).to be false - expect(state.values_map).to eq({}) + 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|