Skip to content

Commit fef6401

Browse files
wangjoshuahwangjoshuah
authored
validate parameters in feature APIs (#98)
* add additional error messages around invalid inputs to feature APIs * add parameter validation to isFeatureEnabled * add parameter validation and logs to getFeatureVariable * return None if user ID is None * add null parameter tests for get_feature_variable_* * add null parameter tests for is_feature_enabled
1 parent 1d882da commit fef6401

File tree

3 files changed

+95
-3
lines changed

3 files changed

+95
-3
lines changed

‎optimizely/helpers/enums.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,21 @@ class LogLevels(object):
2929

3030

3131
class Errors(object):
32-
INVALID_INPUT_ERROR = 'Provided "{}" is in an invalid format.'
3332
INVALID_ATTRIBUTE_ERROR = 'Provided attribute is not in datafile.'
3433
INVALID_ATTRIBUTE_FORMAT = 'Attributes provided are in an invalid format.'
35-
INVALID_EVENT_TAG_FORMAT = 'Event tags provided are in an invalid format.'
3634
INVALID_AUDIENCE_ERROR = 'Provided audience is not in datafile.'
35+
INVALID_DATAFILE = 'Datafile has invalid format. Failing "{}".'
36+
INVALID_EVENT_TAG_FORMAT = 'Event tags provided are in an invalid format.'
3737
INVALID_EXPERIMENT_KEY_ERROR = 'Provided experiment is not in datafile.'
3838
INVALID_EVENT_KEY_ERROR = 'Provided event is not in datafile.'
39-
INVALID_DATAFILE = 'Datafile has invalid format. Failing "{}".'
39+
INVALID_FEATURE_KEY_ERROR = 'Provided feature key is not in the datafile.'
4040
INVALID_GROUP_ID_ERROR = 'Provided group is not in datafile.'
41+
INVALID_INPUT_ERROR = 'Provided "{}" is in an invalid format.'
4142
INVALID_VARIATION_ERROR = 'Provided variation is not in datafile.'
43+
INVALID_VARIABLE_KEY_ERROR = 'Provided variable key is not in the feature flag.'
44+
NONE_FEATURE_KEY_PARAMETER = '"None" is an invalid value for feature key.'
45+
NONE_USER_ID_PARAMETER = '"None" is an invalid value for user ID.'
46+
NONE_VARIABLE_KEY_PARAMETER = '"None" is an invalid value for variable key.'
4247
UNSUPPORTED_DATAFILE_VERSION = 'Provided datafile has unsupported version. ' \
4348
'Please use SDK version 1.1.0 or earlier for datafile version 1.'
4449

‎optimizely/optimizely.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,17 @@ def _get_feature_variable_for_type(self, feature_key, variable_key, variable_typ
198198
- Variable key is invalid.
199199
- Mismatch with type of variable.
200200
"""
201+
if feature_key is None:
202+
self.logger.log(enums.LogLevels.ERROR, enums.Errors.NONE_FEATURE_KEY_PARAMETER)
203+
return None
204+
205+
if variable_key is None:
206+
self.logger.log(enums.LogLevels.ERROR, enums.Errors.NONE_VARIABLE_KEY_PARAMETER)
207+
return None
208+
209+
if user_id is None:
210+
self.logger.log(enums.LogLevels.ERROR, enums.Errors.NONE_USER_ID_PARAMETER)
211+
return None
201212

202213
feature_flag = self.config.get_feature_from_key(feature_key)
203214
if not feature_flag:
@@ -375,6 +386,14 @@ def is_feature_enabled(self, feature_key, user_id, attributes=None):
375386
self.logger.log(enums.LogLevels.ERROR, enums.Errors.INVALID_DATAFILE.format('is_feature_enabled'))
376387
return False
377388

389+
if feature_key is None:
390+
self.logger.log(enums.LogLevels.ERROR, enums.Errors.NONE_FEATURE_KEY_PARAMETER)
391+
return False
392+
393+
if user_id is None:
394+
self.logger.log(enums.LogLevels.ERROR, enums.Errors.NONE_USER_ID_PARAMETER)
395+
return False
396+
378397
feature = self.config.get_feature_from_key(feature_key)
379398
if not feature:
380399
return False

‎tests/test_optimizely.py

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1173,6 +1173,26 @@ def test_get_variation__invalid_object(self):
11731173

11741174
mock_logging.assert_called_once_with(enums.LogLevels.ERROR, 'Datafile has invalid format. Failing "get_variation".')
11751175

1176+
def test_is_feature_enabled__returns_false_for_none_feature_key(self):
1177+
""" Test that is_feature_enabled returns false if the provided feature key is None. """
1178+
1179+
opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
1180+
1181+
with mock.patch('optimizely.logger.NoOpLogger.log') as mock_logger:
1182+
self.assertFalse(opt_obj.is_feature_enabled(None, 'test_user'))
1183+
1184+
mock_logger.assert_called_once_with(enums.LogLevels.ERROR, enums.Errors.NONE_FEATURE_KEY_PARAMETER)
1185+
1186+
def test_is_feature_enabled__returns_false_for_none_user_id(self):
1187+
""" Test that is_feature_enabled returns false if the provided user ID is None. """
1188+
1189+
opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
1190+
1191+
with mock.patch('optimizely.logger.NoOpLogger.log') as mock_logger:
1192+
self.assertFalse(opt_obj.is_feature_enabled('feature_key', None))
1193+
1194+
mock_logger.assert_called_once_with(enums.LogLevels.ERROR, enums.Errors.NONE_USER_ID_PARAMETER)
1195+
11761196
def test_is_feature_enabled__returns_false_for_invalid_feature(self):
11771197
""" Test that the feature is not enabled for the user if the provided feature key is invalid. """
11781198

@@ -1449,6 +1469,54 @@ def test_get_feature_variable__returns_default_value(self):
14491469
'Returning default value for variable "environment" of feature flag "test_feature_in_experiment".'
14501470
)
14511471

1472+
def test_get_feature_variable__returns_none_if_none_feature_key(self):
1473+
""" Test that get_feature_variable_* returns None for None feature key. """
1474+
1475+
opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
1476+
with mock.patch('optimizely.logger.NoOpLogger.log') as mock_logger:
1477+
self.assertIsNone(opt_obj.get_feature_variable_boolean(None, 'variable_key', 'test_user'))
1478+
mock_logger.assert_called_with(enums.LogLevels.ERROR, enums.Errors.NONE_FEATURE_KEY_PARAMETER)
1479+
self.assertIsNone(opt_obj.get_feature_variable_double(None, 'variable_key', 'test_user'))
1480+
mock_logger.assert_called_with(enums.LogLevels.ERROR, enums.Errors.NONE_FEATURE_KEY_PARAMETER)
1481+
self.assertIsNone(opt_obj.get_feature_variable_integer(None, 'variable_key', 'test_user'))
1482+
mock_logger.assert_called_with(enums.LogLevels.ERROR, enums.Errors.NONE_FEATURE_KEY_PARAMETER)
1483+
self.assertIsNone(opt_obj.get_feature_variable_string(None, 'variable_key', 'test_user'))
1484+
mock_logger.assert_called_with(enums.LogLevels.ERROR, enums.Errors.NONE_FEATURE_KEY_PARAMETER)
1485+
1486+
self.assertEqual(4, mock_logger.call_count)
1487+
1488+
def test_get_feature_variable__returns_none_if_none_variable_key(self):
1489+
""" Test that get_feature_variable_* returns None for None variable key. """
1490+
1491+
opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
1492+
with mock.patch('optimizely.logger.NoOpLogger.log') as mock_logger:
1493+
self.assertIsNone(opt_obj.get_feature_variable_boolean('feature_key', None, 'test_user'))
1494+
mock_logger.assert_called_with(enums.LogLevels.ERROR, enums.Errors.NONE_VARIABLE_KEY_PARAMETER)
1495+
self.assertIsNone(opt_obj.get_feature_variable_double('feature_key', None, 'test_user'))
1496+
mock_logger.assert_called_with(enums.LogLevels.ERROR, enums.Errors.NONE_VARIABLE_KEY_PARAMETER)
1497+
self.assertIsNone(opt_obj.get_feature_variable_integer('feature_key', None, 'test_user'))
1498+
mock_logger.assert_called_with(enums.LogLevels.ERROR, enums.Errors.NONE_VARIABLE_KEY_PARAMETER)
1499+
self.assertIsNone(opt_obj.get_feature_variable_string('feature_key', None, 'test-User'))
1500+
mock_logger.assert_called_with(enums.LogLevels.ERROR, enums.Errors.NONE_VARIABLE_KEY_PARAMETER)
1501+
1502+
self.assertEqual(4, mock_logger.call_count)
1503+
1504+
def test_get_feature_variable__returns_none_if_none_user_id(self):
1505+
""" Test that get_feature_variable_* returns None for None user ID. """
1506+
1507+
opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
1508+
with mock.patch('optimizely.logger.NoOpLogger.log') as mock_logger:
1509+
self.assertIsNone(opt_obj.get_feature_variable_boolean('feature_key', 'variable_key', None))
1510+
mock_logger.assert_called_with(enums.LogLevels.ERROR, enums.Errors.NONE_USER_ID_PARAMETER)
1511+
self.assertIsNone(opt_obj.get_feature_variable_double('feature_key', 'variable_key', None))
1512+
mock_logger.assert_called_with(enums.LogLevels.ERROR, enums.Errors.NONE_USER_ID_PARAMETER)
1513+
self.assertIsNone(opt_obj.get_feature_variable_integer('feature_key', 'variable_key', None))
1514+
mock_logger.assert_called_with(enums.LogLevels.ERROR, enums.Errors.NONE_USER_ID_PARAMETER)
1515+
self.assertIsNone(opt_obj.get_feature_variable_string('feature_key', 'variable_key', None))
1516+
mock_logger.assert_called_with(enums.LogLevels.ERROR, enums.Errors.NONE_USER_ID_PARAMETER)
1517+
1518+
self.assertEqual(4, mock_logger.call_count)
1519+
14521520
def test_get_feature_variable__returns_none_if_invalid_feature_key(self):
14531521
""" Test that get_feature_variable_* returns None for invalid feature key. """
14541522

0 commit comments

Comments
 (0)