From 434855cfe6487e106a4cda8b1313b28a5c6d5f1e Mon Sep 17 00:00:00 2001 From: jsonbailey Date: Fri, 18 Sep 2026 10:47:39 -0500 Subject: [PATCH 1/3] fix: Read all items when a kind holds a single key A recursive Consul get returns the bare value string when only one key matches the prefix, instead of a list of key/value pairs. Reading all items of that kind then raised NoMethodError and failed the whole read, so every flag fell back to its default. Read with get_all, which always returns key/value pairs and gives back an empty list when no key matches. --- .../impl/integrations/consul_impl.rb | 6 ++-- spec/feature_store_spec_base.rb | 28 +++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/lib/ldclient-rb/impl/integrations/consul_impl.rb b/lib/ldclient-rb/impl/integrations/consul_impl.rb index 143868ed..1b15eeca 100644 --- a/lib/ldclient-rb/impl/integrations/consul_impl.rb +++ b/lib/ldclient-rb/impl/integrations/consul_impl.rb @@ -69,8 +69,10 @@ def get_internal(kind, key) def get_all_internal(kind) items_out = {} prefix = kind_key(kind) - results = Diplomat::Kv.get(prefix, { recurse: true }, :return) - (results == "" ? [] : results).each do |result| + # Use get_all, not a recursive get. A recursive get returns the bare value string + # when only one key matches the prefix; get_all always returns key/value pairs. + # :return means "give back an empty list if no key matches, don't throw an error". + Diplomat::Kv.get_all(prefix, {}, :return).each do |result| value = result[:value] next if value.nil? db_key = result[:key].to_s diff --git a/spec/feature_store_spec_base.rb b/spec/feature_store_spec_base.rb index 7ef7d3c4..bfd0d333 100644 --- a/spec/feature_store_spec_base.rb +++ b/spec/feature_store_spec_base.rb @@ -264,6 +264,34 @@ def new_version_plus(f, delta_version, attrs = {}) end end end + + it "can read all items when a kind holds a single item" do + # A store must treat a collection of one as a collection. Some database clients + # return a single matching row on its own, rather than in a list. + ensure_stop(store_tester.create_feature_store) do |store1| + store1.init({ $things_kind => { $key1.to_sym => $thing1 } }) + + # A second instance reads through to the database instead of its own cache. + ensure_stop(store_tester.create_feature_store) do |store2| + expect(store2.all($things_kind)).to eq({ $key1.to_sym => $thing1 }) + end + end + end + + it "can read all items when the single item is a tombstone with no key" do + # This is the single-item case where the one item is also a deleted item with no + # key of its own, as happens for a one-flag project or after every flag is deleted. + ensure_stop(store_tester.create_feature_store) do |store1| + store1.init({ $things_kind => {} }) + store_tester.write_raw_item($things_kind, "deleted-thing", { version: 99, deleted: true }) + + # A second instance reads through to the database instead of its own cache. + ensure_stop(store_tester.create_feature_store) do |store2| + expect(store2.all($things_kind)).to eq({}) + expect(store2.get($things_kind, "deleted-thing")).to be_nil + end + end + end end end From 53acb1191bf4fa5629f02f10478b928f0c32833d Mon Sep 17 00:00:00 2001 From: jsonbailey Date: Fri, 18 Sep 2026 17:25:54 -0500 Subject: [PATCH 2/3] chore: Drop the redundant comment on the get_all call --- lib/ldclient-rb/impl/integrations/consul_impl.rb | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/ldclient-rb/impl/integrations/consul_impl.rb b/lib/ldclient-rb/impl/integrations/consul_impl.rb index 1b15eeca..d3e8a1a2 100644 --- a/lib/ldclient-rb/impl/integrations/consul_impl.rb +++ b/lib/ldclient-rb/impl/integrations/consul_impl.rb @@ -69,9 +69,6 @@ def get_internal(kind, key) def get_all_internal(kind) items_out = {} prefix = kind_key(kind) - # Use get_all, not a recursive get. A recursive get returns the bare value string - # when only one key matches the prefix; get_all always returns key/value pairs. - # :return means "give back an empty list if no key matches, don't throw an error". Diplomat::Kv.get_all(prefix, {}, :return).each do |result| value = result[:value] next if value.nil? From 2734305435de550a2e4b83a924151a09df0c2494 Mon Sep 17 00:00:00 2001 From: jsonbailey Date: Tue, 22 Sep 2026 12:07:45 -0500 Subject: [PATCH 3/3] test: Cover an all-items read of an empty kind A brand-new store holds no items for a kind. get_all needs not_found = :return for that case; without it Diplomat raises KeyNotFound and the first read of a fresh store fails. --- spec/feature_store_spec_base.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/spec/feature_store_spec_base.rb b/spec/feature_store_spec_base.rb index bfd0d333..ea24bc84 100644 --- a/spec/feature_store_spec_base.rb +++ b/spec/feature_store_spec_base.rb @@ -278,6 +278,19 @@ def new_version_plus(f, delta_version, attrs = {}) end end + it "can read all items when a kind holds no items" do + # A brand-new store holds no items. Some database clients report this as an error + # or a sentinel value rather than an empty list. + ensure_stop(store_tester.create_feature_store) do |store1| + store1.init({ $things_kind => {} }) + + # A second instance reads through to the database instead of its own cache. + ensure_stop(store_tester.create_feature_store) do |store2| + expect(store2.all($things_kind)).to eq({}) + end + end + end + it "can read all items when the single item is a tombstone with no key" do # This is the single-item case where the one item is also a deleted item with no # key of its own, as happens for a one-flag project or after every flag is deleted.