From dc280dc3e422bde81ab2b483e99cc813c43ba245 Mon Sep 17 00:00:00 2001
From: Matthew Keeler <mkeeler@launchdarkly.com>
Date: Mon, 2 Nov 2026 11:39:20 -0500
Subject: [PATCH] feat!: Require status and close methods on feature stores

The SDK no longer checks with respond_to? for the optional feature store
methods. Custom stores must now implement them.

A custom LaunchDarkly::Interfaces::FeatureStore must implement:

- monitoring_enabled? -- return true only if the store publishes status
  updates when it becomes unavailable and when it recovers. In-memory
  stores return false.
- available? -- return true if the underlying data store is reachable.
  The SDK polls this method after a store operation fails, while
  monitoring is enabled. Stores that cannot become unavailable return
  true.
- stop -- release resources. The SDK calls it when the client closes.

A custom FeatureStoreCore used with CachingStoreWrapper must implement
available? and stop. CachingStoreWrapper now always reports
monitoring_enabled? as true and delegates available? to the core.

Before this change, a store without these methods silently lost status
monitoring, and a core without available? turned monitoring off. The
interface docs also disagreed on whether available? would stay. Making
the methods required gives one clear contract and removes the probing.

The lifecycle method stays named stop. All built-in stores, the big
segment store, data sources and event processors already use stop, so
renaming only the feature store to close would make the SDK less
consistent.
---
 .../feature_store_client_wrapper.rb           | 29 ++-------
 .../impl/data_store/status_provider.rb        |  1 -
 lib/ldclient-rb/impl/data_store/store.rb      |  2 +-
 lib/ldclient-rb/impl/store_client_wrapper.rb  | 28 ++-------
 lib/ldclient-rb/in_memory_store.rb            |  4 ++
 .../integrations/util/store_wrapper.rb        | 17 ++++--
 lib/ldclient-rb/interfaces/feature_store.rb   | 61 ++++++++-----------
 .../feature_store_client_wrapper_spec.rb      | 27 +++++++-
 spec/impl/data_store/store_spec.rb            | 29 +++++++++
 spec/impl/store_client_wrapper_spec.rb        | 31 ++++------
 spec/in_memory_feature_store_spec.rb          |  4 ++
 spec/integrations/util/store_wrapper_spec.rb  | 17 ++++--
 spec/mock_components.rb                       |  8 +++
 13 files changed, 145 insertions(+), 113 deletions(-)

diff --git a/lib/ldclient-rb/impl/data_store/feature_store_client_wrapper.rb b/lib/ldclient-rb/impl/data_store/feature_store_client_wrapper.rb
index 5c86147f..0a0e43af 100644
--- a/lib/ldclient-rb/impl/data_store/feature_store_client_wrapper.rb
+++ b/lib/ldclient-rb/impl/data_store/feature_store_client_wrapper.rb
@@ -26,7 +26,7 @@ def initialize(store, store_update_sink, logger)
           @store = store
           @store_update_sink = store_update_sink
           @logger = logger
-          @monitoring_enabled = store_supports_monitoring?
+          @monitoring_enabled = @store.monitoring_enabled?
 
           # Thread synchronization
           @mutex = Mutex.new
@@ -103,6 +103,11 @@ def monitoring_enabled?
           @monitoring_enabled
         end
 
+        # (see LaunchDarkly::Interfaces::FeatureStore#available?)
+        def available?
+          @store.available?
+        end
+
         #
         # Wraps store operations with exception handling and availability tracking.
         #
@@ -171,28 +176,6 @@ def monitoring_enabled?
             @logger.error { "[LDClient] Unexpected error from data store status function: #{e.message}" }
           end
         end
-
-        #
-        # Determines whether the wrapped store can support enabling monitoring.
-        #
-        # The wrapped store must provide a monitoring_enabled? method, which must
-        # be true. But this alone is not sufficient.
-        #
-        # Because this class wraps all interactions with a provided store, it can
-        # technically "monitor" any store. However, monitoring also requires that
-        # we notify listeners when the store is available again.
-        #
-        # We determine this by checking the store's available? method, so this
-        # is also a requirement for monitoring support.
-        #
-        # @return [Boolean]
-        #
-        private def store_supports_monitoring?
-          return false unless @store.respond_to?(:monitoring_enabled?)
-          return false unless @store.respond_to?(:available?)
-
-          @store.monitoring_enabled?
-        end
       end
     end
   end
diff --git a/lib/ldclient-rb/impl/data_store/status_provider.rb b/lib/ldclient-rb/impl/data_store/status_provider.rb
index deae9ee0..cdb73969 100644
--- a/lib/ldclient-rb/impl/data_store/status_provider.rb
+++ b/lib/ldclient-rb/impl/data_store/status_provider.rb
@@ -63,7 +63,6 @@ def monitoring_enabled?
         #
         private def store_supports_monitoring?
           return false if @store.nil?
-          return false unless @store.respond_to?(:monitoring_enabled?)
 
           @store.monitoring_enabled?
         end
diff --git a/lib/ldclient-rb/impl/data_store/store.rb b/lib/ldclient-rb/impl/data_store/store.rb
index 704c82c5..becf7671 100644
--- a/lib/ldclient-rb/impl/data_store/store.rb
+++ b/lib/ldclient-rb/impl/data_store/store.rb
@@ -97,7 +97,7 @@ def close
             return nil if @persistent_store.nil?
 
             begin
-              @persistent_store.stop if @persistent_store.respond_to?(:stop)
+              @persistent_store.stop
             rescue => e
               return e
             end
diff --git a/lib/ldclient-rb/impl/store_client_wrapper.rb b/lib/ldclient-rb/impl/store_client_wrapper.rb
index 7698b43f..73fd836a 100644
--- a/lib/ldclient-rb/impl/store_client_wrapper.rb
+++ b/lib/ldclient-rb/impl/store_client_wrapper.rb
@@ -15,7 +15,7 @@ def initialize(store, store_update_sink, logger)
         # @type [LaunchDarkly::Interfaces::FeatureStore]
         @store = store
 
-        @monitoring_enabled = does_store_support_monitoring?
+        @monitoring_enabled = @store.monitoring_enabled?
 
         # @type [LaunchDarkly::Impl::DataStore::UpdateSink]
         @store_update_sink = store_update_sink
@@ -68,6 +68,10 @@ def monitoring_enabled?
         @monitoring_enabled
       end
 
+      def available?
+        @store.available?
+      end
+
       private def wrapper()
         begin
           yield
@@ -120,28 +124,6 @@ def monitoring_enabled?
           @logger.error("Unexpected error from data store status function: #{e}")
         end
       end
-
-      # This methods determines whether the wrapped store can support enabling monitoring.
-      #
-      # The wrapped store must provide a monitoring_enabled method, which must
-      # be true. But this alone is not sufficient.
-      #
-      # Because this class wraps all interactions with a provided store, it can
-      # technically "monitor" any store. However, monitoring also requires that
-      # we notify listeners when the store is available again.
-      #
-      # We determine this by checking the store's `available?` method, so this
-      # is also a requirement for monitoring support.
-      #
-      # These extra checks won't be necessary once `available` becomes a part
-      # of the core interface requirements and this class no longer wraps every
-      # feature store.
-      private def does_store_support_monitoring?
-        return false unless @store.respond_to? :monitoring_enabled?
-        return false unless @store.respond_to? :available?
-
-        @store.monitoring_enabled?
-      end
     end
   end
 end
diff --git a/lib/ldclient-rb/in_memory_store.rb b/lib/ldclient-rb/in_memory_store.rb
index 200b629c..0c6cb2d7 100644
--- a/lib/ldclient-rb/in_memory_store.rb
+++ b/lib/ldclient-rb/in_memory_store.rb
@@ -20,6 +20,10 @@ def monitoring_enabled?
       false
     end
 
+    def available?
+      true
+    end
+
     def get(kind, key)
       @lock.with_read_lock do
         coll = @items[kind]
diff --git a/lib/ldclient-rb/integrations/util/store_wrapper.rb b/lib/ldclient-rb/integrations/util/store_wrapper.rb
index 402e59ec..bbb600aa 100644
--- a/lib/ldclient-rb/integrations/util/store_wrapper.rb
+++ b/lib/ldclient-rb/integrations/util/store_wrapper.rb
@@ -44,16 +44,13 @@ def initialize(core, opts)
           end
 
           @inited = Concurrent::AtomicBoolean.new(false)
-          @has_available_method = @core.respond_to? :available?
         end
 
         def monitoring_enabled?
-          @has_available_method
+          true
         end
 
         def available?
-          return false unless @has_available_method
-
           @core.available?
         end
 
@@ -254,6 +251,18 @@ def upsert_internal(kind, item)
         def initialized_internal?
         end
 
+        #
+        # Tests whether the data store operates normally. This is the same as
+        # {LaunchDarkly::Interfaces::FeatureStore#available?}.
+        #
+        # The wrapper reports status monitoring as enabled for all cores. When an operation
+        # fails, the SDK calls this method at intervals until it returns true.
+        #
+        # @return [Boolean] true if the underlying data store is reachable
+        #
+        def available?
+        end
+
         #
         # Performs any necessary cleanup to shut down the store when the client is being shut down.
         #
diff --git a/lib/ldclient-rb/interfaces/feature_store.rb b/lib/ldclient-rb/interfaces/feature_store.rb
index f3527b0b..65f6e7b3 100644
--- a/lib/ldclient-rb/interfaces/feature_store.rb
+++ b/lib/ldclient-rb/interfaces/feature_store.rb
@@ -112,54 +112,47 @@ def stop
       end
 
       #
-      # WARN: This isn't a required method on a FeatureStore yet. The SDK will
-      # currently check if the provided store responds to this method, and if
-      # it does, will take appropriate action based on the documented behavior
-      # below. This will become required in a future major version release of
-      # the SDK.
+      # Returns true if this store supports status monitoring.
       #
-      # Returns true if this data store implementation supports status
-      # monitoring.
+      # This is normally true only for persistent data stores. A custom
+      # {FeatureStore} implementation can also return true.
       #
-      # This is normally only true for persistent data stores but it could also
-      # be true for any custom {FeatureStore} implementation.
+      # If this method returns true, the store must publish a status update
+      # when it enters an invalid state. An invalid state is a state where an
+      # operation has failed, or where the store knows that operations cannot
+      # succeed. The store must then publish another status update when it
+      # returns to a valid state. The SDK uses {#available?} to detect this
+      # return.
       #
-      # Returning true means that the store guarantees that if it ever enters
-      # an invalid state (that is, an operation has failed or it knows that
-      # operations cannot succeed at the moment), it will publish a status
-      # update, and will then publish another status update once it has
-      # returned to a valid state.
+      # In-memory stores must return false.
       #
-      # Custom implementations must implement `def available?` which
-      # synchronously checks if the store is available. Without this method,
-      # the SDK cannot ensure status updates will occur once the store has gone
-      # offline.
+      # The SDK returns the same value from
+      # {LaunchDarkly::Interfaces::DataStore::StatusProvider#monitoring_enabled?}.
       #
-      # The same value will be returned from
-      # {StatusProvider::monitoring_enabled?}.
+      # @return [Boolean] true if the store supports status monitoring
       #
-      # def monitoring_enabled? end
+      def monitoring_enabled?
+      end
 
       #
-      # WARN: This isn't a required method on a FeatureStore. The SDK will
-      # check if the provided store responds to this method, and if it does,
-      # will take appropriate action based on the documented behavior below.
-      # Usage of this method will be dropped in a future version of the SDK.
+      # Tests whether the data store operates normally.
       #
-      # Tests whether the data store seems to be functioning normally.
+      # This must not be a detailed test of different kinds of operations. Do
+      # the smallest possible operation that shows that the store works. For
+      # example, make sure that the database is reachable.
       #
-      # This should not be a detailed test of different kinds of operations,
-      # but just the smallest possible operation to determine whether (for
-      # instance) we can reach the database.
+      # When one of the other methods of the store raises an exception, the SDK
+      # assumes that the store can be unavailable. If {#monitoring_enabled?}
+      # returns true, the SDK then calls this method at intervals until it
+      # returns true.
       #
-      # Whenever one of the store's other methods throws an exception, the SDK
-      # will assume that it may have become unavailable (e.g. the database
-      # connection was lost). The SDK will then call {#available?} at intervals
-      # until it returns true.
+      # Stores that cannot become unavailable, such as in-memory stores, must
+      # return true.
       #
       # @return [Boolean] true if the underlying data store is reachable
       #
-      # def available? end
+      def available?
+      end
     end
   end
 end
diff --git a/spec/impl/data_store/feature_store_client_wrapper_spec.rb b/spec/impl/data_store/feature_store_client_wrapper_spec.rb
index 0a390922..94d6cf4a 100644
--- a/spec/impl/data_store/feature_store_client_wrapper_spec.rb
+++ b/spec/impl/data_store/feature_store_client_wrapper_spec.rb
@@ -10,9 +10,32 @@ module DataStore
         let(:logger) { double.as_null_object }
         let(:status_sink) { double(update_status: nil) }
 
+        describe "required store methods" do
+          [true, false].each do |enabled|
+            it "reports monitoring_enabled? #{enabled} from the store" do
+              inner = double("store", monitoring_enabled?: enabled)
+              expect(described_class.new(inner, status_sink, logger).monitoring_enabled?).to be enabled
+            end
+          end
+
+          [true, false].each do |available|
+            it "gets available? #{available} from the store" do
+              inner = double("store", monitoring_enabled?: true, available?: available)
+              expect(described_class.new(inner, status_sink, logger).available?).to be available
+            end
+          end
+
+          it "stops the store" do
+            inner = double("store", monitoring_enabled?: false)
+            expect(inner).to receive(:stop).once
+            described_class.new(inner, status_sink, logger).stop
+          end
+        end
+
         describe "#disable_cache" do
           it "forwards to the underlying store when supported" do
-            inner = double("store", disable_cache: nil, init: nil, get: nil, all: nil, delete: nil, upsert: nil, initialized?: false, stop: nil)
+            inner = double("store", disable_cache: nil, init: nil, get: nil, all: nil, delete: nil, upsert: nil, initialized?: false,
+                                    stop: nil, monitoring_enabled?: false, available?: true)
             expect(inner).to receive(:disable_cache).once
 
             wrapper = described_class.new(inner, status_sink, logger)
@@ -27,6 +50,8 @@ def all(_); end
               def delete(_, _, _); end
               def upsert(_, _); end
               def initialized?; false; end
+              def monitoring_enabled?; false; end
+              def available?; true; end
               def stop; end
             end.new
 
diff --git a/spec/impl/data_store/store_spec.rb b/spec/impl/data_store/store_spec.rb
index 546371d3..75cf6ff4 100644
--- a/spec/impl/data_store/store_spec.rb
+++ b/spec/impl/data_store/store_spec.rb
@@ -60,6 +60,14 @@ def disable_cache
             @disable_cache_called_count += 1
           end
 
+          def monitoring_enabled?
+            true
+          end
+
+          def available?
+            true
+          end
+
           def init(all_data)
             @init_called_count += 1
             if @should_fail
@@ -130,6 +138,8 @@ def all(kind); @data[kind] || {}; end
           def upsert(_kind, _item); end
           def delete(_kind, _key, _version); end
           def initialized?; @init_called_count > 0; end
+          def monitoring_enabled?; true; end
+          def available?; true; end
           def stop; end
         end
 
@@ -701,6 +711,25 @@ def changed_flags
           end
         end
 
+        describe "#close" do
+          it "stops the persistent store" do
+            persistent = StubPersistentStore.new
+            subject.with_persistence(persistent, true, nil)
+            expect(persistent).to receive(:stop).once
+
+            expect(subject.close).to be_nil
+          end
+
+          it "returns the error when the persistent store fails to stop" do
+            persistent = StubPersistentStore.new
+            subject.with_persistence(persistent, true, nil)
+            error = RuntimeError.new("stop failed")
+            allow(persistent).to receive(:stop).and_raise(error)
+
+            expect(subject.close).to be error
+          end
+        end
+
         describe "FDv2 cache lifecycle" do
           let(:flag_change) do
             LaunchDarkly::Interfaces::DataSystem::Change.new(
diff --git a/spec/impl/store_client_wrapper_spec.rb b/spec/impl/store_client_wrapper_spec.rb
index 43ba7745..3f5d2ace 100644
--- a/spec/impl/store_client_wrapper_spec.rb
+++ b/spec/impl/store_client_wrapper_spec.rb
@@ -25,26 +25,6 @@ module Impl
           end
         end
 
-        it "will not notify sink if wrapped store cannot come back online" do
-          store = double
-          sink = double
-
-          allow(store).to receive(:stop)
-          allow(store).to receive(:monitoring_enabled?).and_return(true)
-          allow(store).to receive(:init).and_raise(StandardError.new('init error'))
-
-          ensure_stop(FeatureStoreClientWrapper.new(store, sink, $null_log)) do |wrapper|
-            begin
-              wrapper.init({})
-              raise "init should have raised exception"
-            rescue StandardError
-              # Ignored
-            end
-
-            expect(sink).not_to receive(:update_status)
-          end
-        end
-
         it "sink will be notified when store is back online" do
           event = Concurrent::Event.new
           statuses = []
@@ -142,6 +122,17 @@ module Impl
           expect(checks.value).to eq 0
         end
       end
+
+      describe "required store methods" do
+        [true, false].each do |available|
+          it "gets available? #{available} from the store" do
+            store = double(monitoring_enabled?: true, available?: available, stop: nil)
+            ensure_stop(FeatureStoreClientWrapper.new(store, double, $null_log)) do |wrapper|
+              expect(wrapper.available?).to be available
+            end
+          end
+        end
+      end
     end
   end
 end
diff --git a/spec/in_memory_feature_store_spec.rb b/spec/in_memory_feature_store_spec.rb
index 6216c2fd..d3c663b6 100644
--- a/spec/in_memory_feature_store_spec.rb
+++ b/spec/in_memory_feature_store_spec.rb
@@ -18,5 +18,9 @@ def create_feature_store
 
       expect(store.monitoring_enabled?).to be false
     end
+
+    it "is always available" do
+      expect(subject.new.available?).to be true
+    end
   end
 end
diff --git a/spec/integrations/util/store_wrapper_spec.rb b/spec/integrations/util/store_wrapper_spec.rb
index a696745e..b1e0c487 100644
--- a/spec/integrations/util/store_wrapper_spec.rb
+++ b/spec/integrations/util/store_wrapper_spec.rb
@@ -7,7 +7,7 @@ module Integrations
 
       THINGS = { namespace: "things" }
 
-      it "monitoring enabled if available is defined" do
+      it "reports monitoring enabled and gets availability from the core" do
         [true, false].each do |expected|
           core = double
           allow(core).to receive(:available?).and_return(expected)
@@ -18,12 +18,10 @@ module Integrations
         end
       end
 
-      it "available is false if core doesn't support monitoring" do
+      it "stops the core" do
         core = double
-        wrapper = subject.new(core, {})
-
-        expect(wrapper.monitoring_enabled?).to be false
-        expect(wrapper.available?).to be false
+        expect(core).to receive(:stop).once
+        subject.new(core, {}).stop
       end
 
       shared_examples "tests" do |cached|
@@ -355,6 +353,13 @@ def initialized_internal?
           @inited_query_count = @inited_query_count + 1
           @inited
         end
+
+        def available?
+          true
+        end
+
+        def stop
+        end
       end
     end
   end
diff --git a/spec/mock_components.rb b/spec/mock_components.rb
index c7dc8b9b..fc11d010 100644
--- a/spec/mock_components.rb
+++ b/spec/mock_components.rb
@@ -78,6 +78,14 @@ def init(all_data)
       @received_data = all_data
     end
 
+    def monitoring_enabled?
+      false
+    end
+
+    def available?
+      true
+    end
+
     def stop
     end
   end