diff --git a/lib/ldclient-rb/impl/integrations/consul_impl.rb b/lib/ldclient-rb/impl/integrations/consul_impl.rb index 105f88de..143868ed 100644 --- a/lib/ldclient-rb/impl/integrations/consul_impl.rb +++ b/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 diff --git a/lib/ldclient-rb/impl/integrations/dynamodb_impl.rb b/lib/ldclient-rb/impl/integrations/dynamodb_impl.rb index 07b8944e..688e671a 100644 --- a/lib/ldclient-rb/impl/integrations/dynamodb_impl.rb +++ b/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 diff --git a/spec/feature_store_spec_base.rb b/spec/feature_store_spec_base.rb index 82d34e11..7ef7d3c4 100644 --- a/spec/feature_store_spec_base.rb +++ b/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 diff --git a/spec/impl/model/serialization_spec.rb b/spec/impl/model/serialization_spec.rb index 360af825..ca4ed8dd 100644 --- a/spec/impl/model/serialization_spec.rb +++ b/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 diff --git a/spec/integrations/consul_feature_store_spec.rb b/spec/integrations/consul_feature_store_spec.rb index fdd06833..5b9308f3 100644 --- a/spec/integrations/consul_feature_store_spec.rb +++ b/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 diff --git a/spec/integrations/dynamodb_stores_spec.rb b/spec/integrations/dynamodb_stores_spec.rb index 53f611a3..f767da0d 100644 --- a/spec/integrations/dynamodb_stores_spec.rb +++ b/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 diff --git a/spec/integrations/redis_stores_spec.rb b/spec/integrations/redis_stores_spec.rb index 791ed64e..dd38198b 100644 --- a/spec/integrations/redis_stores_spec.rb +++ b/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