Skip to content

validate parameters in feature APIs #98

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Jan 19, 2018
Merged

validate parameters in feature APIs #98

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
f0422dd
add additional error messages around invalid inputs to feature APIs
wangjoshuah Jan 10, 2018
9c48cdd
add parameter validation to isFeatureEnabled
wangjoshuah Jan 10, 2018
c984a2e
add parameter validation and logs to getFeatureVariable
wangjoshuah Jan 10, 2018
86983e0
return None if user ID is None
wangjoshuah Jan 11, 2018
7656fc7
remove duplicate logs
wangjoshuah Jan 11, 2018
fa2bcfe
add null parameter tests for get_feature_variable_*
wangjoshuah Jan 11, 2018
e095395
add null parameter tests for is_feature_enabled
wangjoshuah Jan 11, 2018
53562bc
fix feature enabled logger checks
wangjoshuah Jan 17, 2018
77d9bf3
check each feature variable API log after the method is called
wangjoshuah Jan 17, 2018
5a84bb9
use double quotes for None
wangjoshuah Jan 18, 2018
c19c18e
remove extraneous parameter in logs
wangjoshuah Jan 18, 2018
ef0c0ee
explicitly check for None
wangjoshuah Jan 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions optimizely/helpers/enums.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,21 @@ class LogLevels(object):


class Errors(object):
INVALID_INPUT_ERROR = 'Provided "{}" is in an invalid format.'
INVALID_ATTRIBUTE_ERROR = 'Provided attribute is not in datafile.'
INVALID_ATTRIBUTE_FORMAT = 'Attributes provided are in an invalid format.'
INVALID_EVENT_TAG_FORMAT = 'Event tags provided are in an invalid format.'
INVALID_AUDIENCE_ERROR = 'Provided audience is not in datafile.'
INVALID_DATAFILE = 'Datafile has invalid format. Failing "{}".'
INVALID_EVENT_TAG_FORMAT = 'Event tags provided are in an invalid format.'
INVALID_EXPERIMENT_KEY_ERROR = 'Provided experiment is not in datafile.'
INVALID_EVENT_KEY_ERROR = 'Provided event is not in datafile.'
INVALID_DATAFILE = 'Datafile has invalid format. Failing "{}".'
INVALID_FEATURE_KEY_ERROR = 'Provided feature key is not in the datafile.'
INVALID_GROUP_ID_ERROR = 'Provided group is not in datafile.'
INVALID_INPUT_ERROR = 'Provided "{}" is in an invalid format.'
INVALID_VARIATION_ERROR = 'Provided variation is not in datafile.'
INVALID_VARIABLE_KEY_ERROR = 'Provided variable key is not in the feature flag.'
NONE_FEATURE_KEY_PARAMETER = '"None" is an invalid value for feature key.'
NONE_USER_ID_PARAMETER = '"None" is an invalid value for user ID.'
NONE_VARIABLE_KEY_PARAMETER = '"None" is an invalid value for variable key.'
UNSUPPORTED_DATAFILE_VERSION = 'Provided datafile has unsupported version. ' \
'Please use SDK version 1.1.0 or earlier for datafile version 1.'

Expand Down
19 changes: 19 additions & 0 deletions optimizely/optimizely.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,17 @@ def _get_feature_variable_for_type(self, feature_key, variable_key, variable_typ
- Variable key is invalid.
- Mismatch with type of variable.
"""
if feature_key is None:
self.logger.log(enums.LogLevels.ERROR, enums.Errors.NONE_FEATURE_KEY_PARAMETER)
return None

if variable_key is None:
self.logger.log(enums.LogLevels.ERROR, enums.Errors.NONE_VARIABLE_KEY_PARAMETER)
return None

if user_id is None:
self.logger.log(enums.LogLevels.ERROR, enums.Errors.NONE_USER_ID_PARAMETER)
return None

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

if feature_key is None:
self.logger.log(enums.LogLevels.ERROR, enums.Errors.NONE_FEATURE_KEY_PARAMETER)
return False

if user_id is None:
self.logger.log(enums.LogLevels.ERROR, enums.Errors.NONE_USER_ID_PARAMETER)
return False

feature = self.config.get_feature_from_key(feature_key)
if not feature:
return False
Expand Down
68 changes: 68 additions & 0 deletions tests/test_optimizely.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1173,6 +1173,26 @@ def test_get_variation__invalid_object(self):

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

def test_is_feature_enabled__returns_false_for_none_feature_key(self):
""" Test that is_feature_enabled returns false if the provided feature key is None. """

opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))

with mock.patch('optimizely.logger.NoOpLogger.log') as mock_logger:
self.assertFalse(opt_obj.is_feature_enabled(None, 'test_user'))

mock_logger.assert_called_once_with(enums.LogLevels.ERROR, enums.Errors.NONE_FEATURE_KEY_PARAMETER)

def test_is_feature_enabled__returns_false_for_none_user_id(self):
""" Test that is_feature_enabled returns false if the provided user ID is None. """

opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))

with mock.patch('optimizely.logger.NoOpLogger.log') as mock_logger:
self.assertFalse(opt_obj.is_feature_enabled('feature_key', None))

mock_logger.assert_called_once_with(enums.LogLevels.ERROR, enums.Errors.NONE_USER_ID_PARAMETER)

def test_is_feature_enabled__returns_false_for_invalid_feature(self):
""" Test that the feature is not enabled for the user if the provided feature key is invalid. """

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

def test_get_feature_variable__returns_none_if_none_feature_key(self):
""" Test that get_feature_variable_* returns None for None feature key. """

opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
with mock.patch('optimizely.logger.NoOpLogger.log') as mock_logger:
self.assertIsNone(opt_obj.get_feature_variable_boolean(None, 'variable_key', 'test_user'))
mock_logger.assert_called_with(enums.LogLevels.ERROR, enums.Errors.NONE_FEATURE_KEY_PARAMETER)
self.assertIsNone(opt_obj.get_feature_variable_double(None, 'variable_key', 'test_user'))
mock_logger.assert_called_with(enums.LogLevels.ERROR, enums.Errors.NONE_FEATURE_KEY_PARAMETER)
self.assertIsNone(opt_obj.get_feature_variable_integer(None, 'variable_key', 'test_user'))
mock_logger.assert_called_with(enums.LogLevels.ERROR, enums.Errors.NONE_FEATURE_KEY_PARAMETER)
self.assertIsNone(opt_obj.get_feature_variable_string(None, 'variable_key', 'test_user'))
mock_logger.assert_called_with(enums.LogLevels.ERROR, enums.Errors.NONE_FEATURE_KEY_PARAMETER)

self.assertEqual(4, mock_logger.call_count)

def test_get_feature_variable__returns_none_if_none_variable_key(self):
""" Test that get_feature_variable_* returns None for None variable key. """

opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
with mock.patch('optimizely.logger.NoOpLogger.log') as mock_logger:
self.assertIsNone(opt_obj.get_feature_variable_boolean('feature_key', None, 'test_user'))
mock_logger.assert_called_with(enums.LogLevels.ERROR, enums.Errors.NONE_VARIABLE_KEY_PARAMETER)
self.assertIsNone(opt_obj.get_feature_variable_double('feature_key', None, 'test_user'))
mock_logger.assert_called_with(enums.LogLevels.ERROR, enums.Errors.NONE_VARIABLE_KEY_PARAMETER)
self.assertIsNone(opt_obj.get_feature_variable_integer('feature_key', None, 'test_user'))
mock_logger.assert_called_with(enums.LogLevels.ERROR, enums.Errors.NONE_VARIABLE_KEY_PARAMETER)
self.assertIsNone(opt_obj.get_feature_variable_string('feature_key', None, 'test-User'))
mock_logger.assert_called_with(enums.LogLevels.ERROR, enums.Errors.NONE_VARIABLE_KEY_PARAMETER)

self.assertEqual(4, mock_logger.call_count)

def test_get_feature_variable__returns_none_if_none_user_id(self):
""" Test that get_feature_variable_* returns None for None user ID. """

opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
with mock.patch('optimizely.logger.NoOpLogger.log') as mock_logger:
self.assertIsNone(opt_obj.get_feature_variable_boolean('feature_key', 'variable_key', None))
mock_logger.assert_called_with(enums.LogLevels.ERROR, enums.Errors.NONE_USER_ID_PARAMETER)
self.assertIsNone(opt_obj.get_feature_variable_double('feature_key', 'variable_key', None))
mock_logger.assert_called_with(enums.LogLevels.ERROR, enums.Errors.NONE_USER_ID_PARAMETER)
self.assertIsNone(opt_obj.get_feature_variable_integer('feature_key', 'variable_key', None))
mock_logger.assert_called_with(enums.LogLevels.ERROR, enums.Errors.NONE_USER_ID_PARAMETER)
self.assertIsNone(opt_obj.get_feature_variable_string('feature_key', 'variable_key', None))
mock_logger.assert_called_with(enums.LogLevels.ERROR, enums.Errors.NONE_USER_ID_PARAMETER)

self.assertEqual(4, mock_logger.call_count)

def test_get_feature_variable__returns_none_if_invalid_feature_key(self):
""" Test that get_feature_variable_* returns None for invalid feature key. """

Expand Down