used get_many in pluggable storage instead of individual keys

Author: chillaq

splitio/storage/pluggable.py

@@ -18,6 +18,7 @@ class PluggableSplitStorageBase(SplitStorage):
     """InMemory implementation of a feature flag storage."""
 
     _FEATURE_FLAG_NAME_LENGTH = 19
+    _TILL_LENGTH = 4
 
     def __init__(self, pluggable_adapter, prefix=None, config_flag_sets=[]):
         """
@@ -137,15 +138,6 @@ def get_split_names(self):
         """
         pass
 
-    def get_all(self):
-        """
-        Return all the feature flags.
-
-        :return: List of all the feature flags.
-        :rtype: list
-        """
-        pass
-
     def traffic_type_exists(self, traffic_type_name):
         """
         Return whether the traffic type exists in at least one feature flag in cache.
@@ -336,26 +328,16 @@ def get_split_names(self):
         :rtype: list(str)
         """
         try:
-            return [feature_flag.name for feature_flag in self.get_all()]
+            keys = []
+            for key in self._pluggable_adapter.get_keys_by_prefix(self._prefix[:-self._FEATURE_FLAG_NAME_LENGTH]):
+                if key[-self._TILL_LENGTH:] != 'till':
+                    keys.append(key[len(self._prefix[:-self._FEATURE_FLAG_NAME_LENGTH]):])
+            return keys
         except Exception:
             _LOGGER.error('Error getting feature flag names from storage')
             _LOGGER.debug('Error: ', exc_info=True)
             return None
 
-    def get_all(self):
-        """
-        Return all the feature flags.
-
-        :return: List of all the feature flags.
-        :rtype: list
-        """
-        try:
-            return [splits.from_raw(self._pluggable_adapter.get(key)) for key in self._pluggable_adapter.get_keys_by_prefix(self._prefix[:-self._FEATURE_FLAG_NAME_LENGTH])]
-        except Exception:
-            _LOGGER.error('Error getting feature flag keys from storage')
-            _LOGGER.debug('Error: ', exc_info=True)
-            return None
-
     def traffic_type_exists(self, traffic_type_name):
         """
         Return whether the traffic type exists in at least one feature flag in cache.
@@ -381,7 +363,11 @@ def get_all_splits(self):
         :rtype: list
         """
         try:
-            return self.get_all()
+            keys = []
+            for key in self._pluggable_adapter.get_keys_by_prefix(self._prefix[:-self._FEATURE_FLAG_NAME_LENGTH]):
+                if key[-self._TILL_LENGTH:] != 'till':
+                    keys.append(key)
+            return [splits.from_raw(feature_flag) for feature_flag in self._pluggable_adapter.get_many(keys)]
         except Exception:
             _LOGGER.error('Error fetching feature flags from storage')
             _LOGGER.debug('Error: ', exc_info=True)
@@ -498,26 +484,16 @@ async def get_split_names(self):
         :rtype: list(str)
         """
         try:
-            return [feature_flag.name for feature_flag in await self.get_all()]
+            keys = []
+            for key in await self._pluggable_adapter.get_keys_by_prefix(self._prefix[:-self._FEATURE_FLAG_NAME_LENGTH]):
+                if key[-self._TILL_LENGTH:] != 'till':
+                    keys.append(key[len(self._prefix[:-self._FEATURE_FLAG_NAME_LENGTH]):])
+            return keys
         except Exception:
             _LOGGER.error('Error getting feature flag names from storage')
             _LOGGER.debug('Error: ', exc_info=True)
             return None
 
-    async def get_all(self):
-        """
-        Return all the feature flags.
-
-        :return: List of all the feature flags.
-        :rtype: list
-        """
-        try:
-            return [splits.from_raw(await self._pluggable_adapter.get(key)) for key in await self._pluggable_adapter.get_keys_by_prefix(self._prefix[:-self._FEATURE_FLAG_NAME_LENGTH])]
-        except Exception:
-            _LOGGER.error('Error getting feature flag keys from storage')
-            _LOGGER.debug('Error: ', exc_info=True)
-            return None
-
     async def traffic_type_exists(self, traffic_type_name):
         """
         Return whether the traffic type exists in at least one feature flag in cache.
@@ -543,7 +519,11 @@ async def get_all_splits(self):
         :rtype: list
         """
         try:
-            return await self.get_all()
+            keys = []
+            for key in await self._pluggable_adapter.get_keys_by_prefix(self._prefix[:-self._FEATURE_FLAG_NAME_LENGTH]):
+                if key[-self._TILL_LENGTH:] != 'till':
+                    keys.append(key)
+            return [splits.from_raw(feature_flag) for feature_flag in await self._pluggable_adapter.get_many(keys)]
         except Exception:
             _LOGGER.error('Error fetching feature flags from storage')
             _LOGGER.debug('Error: ', exc_info=True)

tests/integration/test_pluggable_integration.py

@@ -24,9 +24,9 @@ def test_put_fetch(self):
             with open(split_fn, 'r') as flo:
                 data = json.loads(flo.read())
                 for split in data['splits']:
-                    adapter.set(storage._prefix.format(split_name=split['name']), split)
+                    adapter.set(storage._prefix.format(feature_flag_name=split['name']), split)
                     adapter.increment(storage._traffic_type_prefix.format(traffic_type_name=split['trafficTypeName']), 1)
-                adapter.set(storage._split_till_prefix, data['till'])
+                adapter.set(storage._feature_flag_till_prefix, data['till'])
 
             split_objects = [splits.from_raw(raw) for raw in data['splits']]
             for split_object in split_objects:
@@ -53,7 +53,7 @@ def test_put_fetch(self):
                     assert len(original_condition.matchers) == len(fetched_condition.matchers)
                     assert len(original_condition.partitions) == len(fetched_condition.partitions)
 
-            adapter.set(storage._split_till_prefix, data['till'])
+            adapter.set(storage._feature_flag_till_prefix, data['till'])
             assert storage.get_change_number() == data['till']
 
             assert storage.is_valid_traffic_type('user') is True
@@ -90,9 +90,9 @@ def test_get_all(self):
             with open(split_fn, 'r') as flo:
                 data = json.loads(flo.read())
                 for split in data['splits']:
-                    adapter.set(storage._prefix.format(split_name=split['name']), split)
+                    adapter.set(storage._prefix.format(feature_flag_name=split['name']), split)
                     adapter.increment(storage._traffic_type_prefix.format(traffic_type_name=split['trafficTypeName']), 1)
-                adapter.set(storage._split_till_prefix, data['till'])
+                adapter.set(storage._feature_flag_till_prefix, data['till'])
 
             split_objects = [splits.from_raw(raw) for raw in data['splits']]
             original_splits = {split.name: split for split in split_objects}
@@ -261,9 +261,9 @@ async def test_put_fetch(self):
             with open(split_fn, 'r') as flo:
                 data = json.loads(flo.read())
                 for split in data['splits']:
-                    await adapter.set(storage._prefix.format(split_name=split['name']), split)
+                    await adapter.set(storage._prefix.format(feature_flag_name=split['name']), split)
                     await adapter.increment(storage._traffic_type_prefix.format(traffic_type_name=split['trafficTypeName']), 1)
-                await adapter.set(storage._split_till_prefix, data['till'])
+                await adapter.set(storage._feature_flag_till_prefix, data['till'])
 
             split_objects = [splits.from_raw(raw) for raw in data['splits']]
             for split_object in split_objects:
@@ -290,7 +290,7 @@ async def test_put_fetch(self):
                     assert len(original_condition.matchers) == len(fetched_condition.matchers)
                     assert len(original_condition.partitions) == len(fetched_condition.partitions)
 
-            await adapter.set(storage._split_till_prefix, data['till'])
+            await adapter.set(storage._feature_flag_till_prefix, data['till'])
             assert await storage.get_change_number() == data['till']
 
             assert await storage.is_valid_traffic_type('user') is True
@@ -328,9 +328,9 @@ async def test_get_all(self):
             with open(split_fn, 'r') as flo:
                 data = json.loads(flo.read())
                 for split in data['splits']:
-                    await adapter.set(storage._prefix.format(split_name=split['name']), split)
+                    await adapter.set(storage._prefix.format(feature_flag_name=split['name']), split)
                     await adapter.increment(storage._traffic_type_prefix.format(traffic_type_name=split['trafficTypeName']), 1)
-                await adapter.set(storage._split_till_prefix, data['till'])
+                await adapter.set(storage._feature_flag_till_prefix, data['till'])
 
             split_objects = [splits.from_raw(raw) for raw in data['splits']]
             original_splits = {split.name: split for split in split_objects}

tests/storage/test_pluggable.py

@@ -196,11 +196,9 @@ async def get_keys_by_prefix(self, prefix):
     async def get_many(self, keys):
         async with self._lock:
             returned_keys = []
-            for key in keys:
-                if key in self._keys:
+            for key in self._keys:
+                if key in keys:
                     returned_keys.append(self._keys[key])
-                else:
-                    returned_keys.append(None)
             return returned_keys
 
     async def add_items(self, key, added_items):
@@ -336,20 +334,6 @@ def test_get_split_names(self):
             self.mock_adapter.set(pluggable_split_storage._prefix.format(feature_flag_name=split2.name), split2.to_json())
             assert(pluggable_split_storage.get_split_names() == [split1.name, split2.name])
 
-    def test_get_all(self):
-        self.mock_adapter._keys = {}
-        for sprefix in [None, 'myprefix']:
-            pluggable_split_storage = PluggableSplitStorage(self.mock_adapter, prefix=sprefix)
-            split1 = splits.from_raw(splits_json['splitChange1_2']['splits'][0])
-            split2_temp = splits_json['splitChange1_2']['splits'][0].copy()
-            split2_temp['name'] = 'another_split'
-            split2 = splits.from_raw(split2_temp)
-
-            self.mock_adapter.set(pluggable_split_storage._prefix.format(feature_flag_name=split1.name), split1.to_json())
-            self.mock_adapter.set(pluggable_split_storage._prefix.format(feature_flag_name=split2.name), split2.to_json())
-            all_splits = pluggable_split_storage.get_all()
-            assert([all_splits[0].to_json(), all_splits[1].to_json()] == [split1.to_json(), split2.to_json()])
-
     # TODO: To be added when producer mode is aupported
 #    def test_kill_locally(self):
 #        self.mock_adapter._keys = {}
@@ -474,23 +458,8 @@ async def test_get_split_names(self):
             split2 = splits.from_raw(split2_temp)
             await self.mock_adapter.set(pluggable_split_storage._prefix.format(feature_flag_name=split1.name), split1.to_json())
             await self.mock_adapter.set(pluggable_split_storage._prefix.format(feature_flag_name=split2.name), split2.to_json())
-            assert(await pluggable_split_storage.get_split_names() == [split1.name, split2.name])
-
-    @pytest.mark.asyncio
-    async def test_get_all(self):
-        self.mock_adapter._keys = {}
-        for sprefix in [None, 'myprefix']:
-            pluggable_split_storage = PluggableSplitStorageAsync(self.mock_adapter, prefix=sprefix)
-            split1 = splits.from_raw(splits_json['splitChange1_2']['splits'][0])
-            split2_temp = splits_json['splitChange1_2']['splits'][0].copy()
-            split2_temp['name'] = 'another_split'
-            split2 = splits.from_raw(split2_temp)
-
-            await self.mock_adapter.set(pluggable_split_storage._prefix.format(feature_flag_name=split1.name), split1.to_json())
-            await self.mock_adapter.set(pluggable_split_storage._prefix.format(feature_flag_name=split2.name), split2.to_json())
-            all_splits = await pluggable_split_storage.get_all()
-            assert([all_splits[0].to_json(), all_splits[1].to_json()] == [split1.to_json(), split2.to_json()])
 
+            assert(await pluggable_split_storage.get_split_names() == [split1.name, split2.name])
 
 class PluggableSegmentStorageTests(object):
     """In memory split storage test cases."""