fix: Read all items when a tombstone has no key

lib/ldclient-rb/impl/integrations/consul_impl.rb

@@ -68,13 +68,16 @@ def get_internal(kind, key)
 
           def get_all_internal(kind)
             items_out = {}
-            results = Diplomat::Kv.get(kind_key(kind), { recurse: true }, :return)
+            prefix = kind_key(kind)
+            results = Diplomat::Kv.get(prefix, { recurse: true }, :return)
             (results == "" ? [] : results).each do |result|
               value = result[:value]
-              unless value.nil?
-                item = Model.deserialize(kind, value)
-                items_out[item[:key].to_sym] = item
-              end
+              next if value.nil?
+              db_key = result[:key].to_s
+              next unless db_key.start_with?(prefix)
+              # Use the key that the item is stored under, not the key inside the item. A deleted
+              # item (a "tombstone") is not guaranteed to carry a key of its own.
+              items_out[db_key[prefix.length..].to_sym] = Model.deserialize(kind, value)
             end
             items_out
           end

lib/ldclient-rb/impl/integrations/dynamodb_impl.rb

@@ -113,7 +113,10 @@ def get_all_internal(kind)
               resp = @client.query(req)
               resp.items.each do |item|
                 item_out = unmarshal_item(kind, item)
-                items_out[item_out[:key].to_sym] = item_out
+                next if item_out.nil?
+                # Use the sort key that the item is stored under, not the key inside the item. A
+                # deleted item (a "tombstone") is not guaranteed to carry a key of its own.
+                items_out[item[SORT_KEY].to_sym] = item_out
               end
               break if resp.last_evaluated_key.nil? || resp.last_evaluated_key.length == 0
               req.exclusive_start_key = resp.last_evaluated_key

spec/feature_store_spec_base.rb

@@ -15,6 +15,10 @@
 #   def clear_data
 #     # clear any existing data from the database, taking @options[:prefix] into account if any
 #   end
+#   def write_raw_item(kind, key, item)
+#     # write the item straight to the database as JSON, under the given key, taking
+#     # @options[:prefix] into account if any
+#   end
 # end
 #
 # describe "my persistent feature store" do
@@ -244,6 +248,22 @@ def new_version_plus(f, delta_version, attrs = {})
               end
             end
           end
+
+          it "can read all items when one of them is a tombstone with no key" do
+            # Other LaunchDarkly SDKs write a deleted item with only a version, and no key of its
+            # own. The store must read such an item back under the key it is stored under, and must
+            # not let it spoil the rest of the collection.
+            ensure_stop(store_tester.create_feature_store) do |store1|
+              store1.init({ $things_kind => { $key1.to_sym => $thing1 } })
+              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({ $key1.to_sym => $thing1 })
+                expect(store2.get($things_kind, "deleted-thing")).to be_nil
+              end
+            end
+          end
         end
       end
 

spec/impl/model/serialization_spec.rb

@@ -30,6 +30,33 @@ module Model
           segment_out = Model.deserialize(Impl::DataStore::SEGMENTS, json, nil)
           expect(segment_out.data).to eq segment_in
         end
+
+        # Other LaunchDarkly SDKs write a deleted item to a persistent store with only a version,
+        # and no key of its own. The store knows the key, because it is the key the item is stored
+        # under, so a tombstone must deserialize without one.
+        [ Impl::DataStore::FEATURES, Impl::DataStore::SEGMENTS ].each do |kind|
+          it "deserializes a tombstone with no key for #{kind[:namespace]}" do
+            item_in = { version: 99, deleted: true }
+            item_out = Model.deserialize(kind, item_in.to_json, nil)
+
+            expect(item_out.key).to be_nil
+            expect(item_out.version).to eq 99
+            expect(item_out.deleted).to be true
+            # The store re-serializes what it read, so the original data must survive unchanged.
+            expect(item_out.data).to eq item_in
+          end
+
+          it "deserializes a tombstone with a placeholder key for #{kind[:namespace]}" do
+            # The Go SDK and the Relay Proxy write a deleted item as a full object whose key is
+            # the placeholder "$deleted".
+            item_in = { key: "$deleted", version: 99, deleted: true }
+            item_out = Model.deserialize(kind, item_in.to_json, nil)
+
+            expect(item_out.key).to eq "$deleted"
+            expect(item_out.deleted).to be true
+            expect(item_out.data).to eq item_in
+          end
+        end
       end
     end
   end

spec/integrations/consul_feature_store_spec.rb

@@ -17,6 +17,10 @@ def clear_data
         Diplomat::Kv.delete(@actual_prefix + '/', recurse: true)
       end
 
+      def write_raw_item(kind, key, item)
+        Diplomat::Kv.put("#{@actual_prefix}/#{kind[:namespace]}/#{key}", item.to_json)
+      end
+
       def create_feature_store
         Consul.new_feature_store(@options)
       end

spec/integrations/dynamodb_stores_spec.rb

@@ -93,6 +93,18 @@ def create_feature_store
         LaunchDarkly::Integrations::DynamoDB::new_feature_store(TABLE_NAME, @options)
       end
 
+      def write_raw_item(kind, key, item)
+        self.class.create_test_client.put_item(
+          table_name: TABLE_NAME,
+          item: {
+            "namespace" => @actual_prefix + kind[:namespace],
+            "key" => key,
+            LaunchDarkly::Impl::Integrations::DynamoDB::DynamoDBFeatureStoreCore::VERSION_ATTRIBUTE => item[:version],
+            LaunchDarkly::Impl::Integrations::DynamoDB::DynamoDBFeatureStoreCore::ITEM_JSON_ATTRIBUTE => item.to_json,
+          }
+        )
+      end
+
       def create_big_segment_store
         LaunchDarkly::Integrations::DynamoDB::new_big_segment_store(TABLE_NAME, @options)
       end

spec/integrations/redis_stores_spec.rb

@@ -37,6 +37,12 @@ def create_feature_store
         Redis::new_feature_store(@options)
       end
 
+      def write_raw_item(kind, key, item)
+        with_redis_test_client do |client|
+          client.hset("#{@actual_prefix}:#{kind[:namespace]}", key, item.to_json)
+        end
+      end
+
       def create_big_segment_store
         Redis.new_big_segment_store(@options)
       end