diff --git a/ldclient/impl/integrations/consul/consul_feature_store.py b/ldclient/impl/integrations/consul/consul_feature_store.py index 48c31da2..07f8d842 100644 --- a/ldclient/impl/integrations/consul/consul_feature_store.py +++ b/ldclient/impl/integrations/consul/consul_feature_store.py @@ -89,8 +89,9 @@ def get_all_internal(self, kind): # Use the key that each item is stored under, not the key inside the item. A deleted # item (a "tombstone") is not guaranteed to have a key of its own. item_key_prefix = self._kind_key(kind) + '/' + # A recursive get returns None, not an empty list, when no key has this prefix. index, results = self._client.kv.get(self._kind_key(kind), recurse=True) - for result in results: + for result in results or []: db_key = result['Key'] if not db_key.startswith(item_key_prefix): continue diff --git a/ldclient/testing/integrations/persistent_feature_store_test_base.py b/ldclient/testing/integrations/persistent_feature_store_test_base.py index 4be6e069..5a57d46e 100644 --- a/ldclient/testing/integrations/persistent_feature_store_test_base.py +++ b/ldclient/testing/integrations/persistent_feature_store_test_base.py @@ -85,6 +85,17 @@ def test_all_reads_tombstone_with_no_key(self, tester): assert items == {'foo': self.make_feature('foo', 10), 'bar': self.make_feature('bar', 10)} assert store.get(FEATURES, 'deleted-flag', lambda x: x) is None + def test_all_reads_empty_collection(self, tester): + # A store that holds no items of a kind must read that kind as an empty collection. + # Some database clients report "nothing matched" with a null value rather than an + # empty list. + with self.store(tester) as store: + store.init({FEATURES: {}}) + + # A second instance reads through to the database instead of its own cache. + with self.store(tester) as other_store: + assert other_store.all(FEATURES, lambda x: x) == {} + def test_stores_with_different_prefixes_are_independent(self): # This verifies that init(), get(), all(), and upsert() are all correctly using the specified key prefix. # The delete() method isn't tested separately because it's implemented as a variant of upsert().