diff --git a/feature_ramp/Feature.py b/feature_ramp/Feature.py index 9f6eb19..5bf79fb 100644 --- a/feature_ramp/Feature.py +++ b/feature_ramp/Feature.py @@ -36,8 +36,8 @@ def __init__(self, feature_name, feature_group_name=None, default_percentage=0): redis_raw = redis.get(key) redis_data = self._deserialize(redis_raw) - self.whitelist = redis_data.get('whitelist', []) - self.blacklist = redis_data.get('blacklist', []) + self.whitelist = set(redis_data.get('whitelist', [])) + self.blacklist = set(redis_data.get('blacklist', [])) self.percentage = redis_data.get('percentage', default_percentage) def is_visible(self, identifier): @@ -119,8 +119,8 @@ def reset_settings(self): """ self.percentage = 0 - self.whitelist = [] - self.blacklist = [] + self.whitelist = set([]) + self.blacklist = set([]) self._save() def delete(self): @@ -148,7 +148,7 @@ def set_percentage(self, percentage): def add_to_whitelist(self, identifier): """ Whitelist the given identifier to always see the feature regardless of ramp. """ - self.whitelist.append(identifier) + self.whitelist.add(identifier) self._save() def remove_from_whitelist(self, identifier): @@ -160,7 +160,7 @@ def remove_from_whitelist(self, identifier): def add_to_blacklist(self, identifier): """ Blacklist the given identifier to never see the feature regardless of ramp. """ - self.blacklist.append(identifier) + self.blacklist.add(identifier) self._save() def remove_from_blacklist(self, identifier): @@ -235,8 +235,8 @@ def _get_redis_data(self): """ Returns the dictionary representation of this object for storage in Redis. """ return { - 'whitelist': self.whitelist, - 'blacklist': self.blacklist, + 'whitelist': list(self.whitelist), + 'blacklist': list(self.blacklist), 'percentage': self.percentage } @@ -253,4 +253,8 @@ def _deserialize(self, redis_obj): def __str__(self): """ Pretty print the feature and some stats """ stats = self._get_redis_data() - return "Feature: {0}\nwhitelisted: {1}\nblacklisted: {2}\npercentage: {3}\n".format(self.feature_name, stats['whitelist'], stats['blacklist'], stats['percentage']) + return "Feature: {0}\nwhitelisted: {1}\nblacklisted: {2}\npercentage: {3}\n".format( + self.feature_name, + stats['whitelist'], + stats['blacklist'], + stats['percentage']) diff --git a/tests/test_feature.py b/tests/test_feature.py index 164d2d6..64e8f79 100644 --- a/tests/test_feature.py +++ b/tests/test_feature.py @@ -20,7 +20,7 @@ def tearDown(self): def test_initialize_feature_with_default_percentage(self): feature_test = Feature("testing", default_percentage=100) - self.assertEquals(feature_test.percentage, 100) + self.assertEqual(feature_test.percentage, 100) def test_reset_settings(self): """ Tests calling reset_settings resets the correct @@ -82,7 +82,7 @@ def test_all_features_with_data(self): all_features = Feature.all_features(include_data=True) self.assertEqual(len(all_features), 4) - for key in ['looktest1','looktest2','looktest3','looktest4']: + for key in ['looktest1', 'looktest2', 'looktest3', 'looktest4']: self.assertTrue(key in all_features) if not key == 'looktest1': self.assertEqual(all_features[key]['percentage'], 100) @@ -92,17 +92,17 @@ def test_all_features_with_data(self): self.assertFalse('blacklist' in all_features['looktest1']) self.assertTrue('whitelist' in all_features['looktest2']) - self.assertEqual(all_features['looktest2']['whitelist'], [3]) + self.assertEqual(all_features['looktest2']['whitelist'], set([3])) self.assertFalse('blacklist' in all_features['looktest2']) self.assertFalse('whitelist' in all_features['looktest3']) self.assertTrue('blacklist' in all_features['looktest3']) - self.assertEqual(all_features['looktest3']['blacklist'], [4,5]) + self.assertEqual(all_features['looktest3']['blacklist'], set([4, 5])) self.assertTrue('whitelist' in all_features['looktest4']) - self.assertEqual(all_features['looktest4']['whitelist'], [3, 5]) + self.assertEqual(all_features['looktest4']['whitelist'], set([3, 5])) self.assertTrue('blacklist' in all_features['looktest4']) - self.assertEqual(all_features['looktest4']['blacklist'], [4]) + self.assertEqual(all_features['looktest4']['blacklist'], set([4])) def test_set_add(self): """ Tests that creating a feature stores its key in a Redis set. """ @@ -152,6 +152,16 @@ def test_add_to_whitelist_with_string(self): self.feature_test.add_to_whitelist(email) self.assertTrue(email in Feature("testing").whitelist) + def test_add_to_whitelist_with_duplicate(self): + """ Tests calling add_to_whitelist doesn't set duplicate + data. """ + + self.feature_test.add_to_whitelist(3) + self.feature_test.add_to_whitelist(3) + self.assertEqual( + len([id for id in Feature("testing").whitelist if id == 3]), + 1) + def test_remove_from_whitelist(self): """ Tests calling remove_from_whitelist sets the correct data in Redis. """ @@ -182,6 +192,16 @@ def test_add_to_blacklist_with_string(self): self.feature_test.add_to_blacklist(email) self.assertTrue(email in Feature("testing").blacklist) + def test_add_to_blacklist_with_duplicate(self): + """ Tests calling add_to_blacklist doesn't set duplicate + data. """ + + self.feature_test.add_to_blacklist(3) + self.feature_test.add_to_blacklist(3) + self.assertEqual( + len([id for id in Feature("testing").blacklist if id == 3]), + 1) + def test_remove_from_blacklist(self): """ Tests calling remove_from_blacklist sets the correct data in Redis. """ @@ -296,7 +316,6 @@ def test_is_ramped_using_int(self): self.feature_test.set_percentage(100) self.assertTrue(self.feature_test._is_ramped(5)) - def test_is_ramped_using_string(self): """Tests that _is_ramped accepts strings as identifier.""" self.feature_test.set_percentage(100) @@ -324,9 +343,9 @@ def test_group_names(self): identifiers = range(1, 10001) visibility_test_one = [id for id in identifiers - if feature_one.is_visible(id)] + if feature_one.is_visible(id)] visibility_test_two = [id for id in identifiers - if feature_two.is_visible(id)] + if feature_two.is_visible(id)] self.assertEqual(visibility_test_one, visibility_test_two)