diff --git a/optimizely/helpers/validator.py b/optimizely/helpers/validator.py index 9c27418f..48887089 100644 --- a/optimizely/helpers/validator.py +++ b/optimizely/helpers/validator.py @@ -1,4 +1,4 @@ -# Copyright 2016-2017, Optimizely +# Copyright 2016-2018, Optimizely # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. # You may obtain a copy of the License at @@ -13,6 +13,7 @@ import json import jsonschema +from six import string_types from optimizely.user_profile import UserProfile from . import constants @@ -151,3 +152,18 @@ def is_user_profile_valid(user_profile): return False return True + + +def is_non_empty_string(input_id_key): + """ Determine if provided input_id_key is a non-empty string or not. + + Args: + input_id_key: Variable which needs to be validated. + + Returns: + Boolean depending upon whether input is valid or not. + """ + if input_id_key and isinstance(input_id_key, string_types): + return True + + return False diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index 6b775146..28b86dd6 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -179,6 +179,7 @@ def _send_impression_event(self, experiment, variation, user_id, attributes): self.event_dispatcher.dispatch_event(impression_event) except: self.logger.exception('Unable to dispatch impression event!') + self.notification_center.send_notifications(enums.NotificationTypes.ACTIVATE, experiment, user_id, attributes, variation, impression_event) @@ -262,6 +263,14 @@ def activate(self, experiment_key, user_id, attributes=None): self.logger.error(enums.Errors.INVALID_DATAFILE.format('activate')) return None + if not validator.is_non_empty_string(experiment_key): + self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('experiment_key')) + return None + + if not validator.is_non_empty_string(user_id): + self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('user_id')) + return None + variation_key = self.get_variation(experiment_key, user_id, attributes) if not variation_key: @@ -291,6 +300,14 @@ def track(self, event_key, user_id, attributes=None, event_tags=None): self.logger.error(enums.Errors.INVALID_DATAFILE.format('track')) return + if not validator.is_non_empty_string(event_key): + self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('event_key')) + return + + if not validator.is_non_empty_string(user_id): + self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('user_id')) + return + if not self._validate_user_inputs(attributes, event_tags): return @@ -339,6 +356,14 @@ def get_variation(self, experiment_key, user_id, attributes=None): self.logger.error(enums.Errors.INVALID_DATAFILE.format('get_variation')) return None + if not validator.is_non_empty_string(experiment_key): + self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('experiment_key')) + return None + + if not validator.is_non_empty_string(user_id): + self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('user_id')) + return None + experiment = self.config.get_experiment_from_key(experiment_key) if not experiment: @@ -373,12 +398,12 @@ def is_feature_enabled(self, feature_key, user_id, attributes=None): self.logger.error(enums.Errors.INVALID_DATAFILE.format('is_feature_enabled')) return False - if feature_key is None: - self.logger.error(enums.Errors.NONE_FEATURE_KEY_PARAMETER) + if not validator.is_non_empty_string(feature_key): + self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('feature_key')) return False - if user_id is None: - self.logger.error(enums.Errors.NONE_USER_ID_PARAMETER) + if not validator.is_non_empty_string(user_id): + self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('user_id')) return False feature = self.config.get_feature_from_key(feature_key) @@ -417,6 +442,10 @@ def get_enabled_features(self, user_id, attributes=None): self.logger.error(enums.Errors.INVALID_DATAFILE.format('get_enabled_features')) return enabled_features + if not validator.is_non_empty_string(user_id): + self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('user_id')) + return enabled_features + for feature in self.config.feature_key_map.values(): if self.is_feature_enabled(feature.key, user_id, attributes): enabled_features.append(feature.key) diff --git a/tests/helpers_tests/test_validator.py b/tests/helpers_tests/test_validator.py index 4c833d95..33d7f7be 100644 --- a/tests/helpers_tests/test_validator.py +++ b/tests/helpers_tests/test_validator.py @@ -1,4 +1,4 @@ -# Copyright 2016-2017, Optimizely +# Copyright 2016-2018, Optimizely # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. # You may obtain a copy of the License at @@ -130,6 +130,22 @@ def test_is_user_profile_valid__returns_false(self): 'experiment_bucket_map': {'1234': {'variation_id': '5678'}, '1235': {'some_key': 'some_value'}}})) + def test_is_non_empty_string(self): + """ Test that the method returns True only for a non-empty string. """ + + self.assertFalse(validator.is_non_empty_string(None)) + self.assertFalse(validator.is_non_empty_string([])) + self.assertFalse(validator.is_non_empty_string({})) + self.assertFalse(validator.is_non_empty_string(0)) + self.assertFalse(validator.is_non_empty_string(99)) + self.assertFalse(validator.is_non_empty_string(1.2)) + self.assertFalse(validator.is_non_empty_string(True)) + self.assertFalse(validator.is_non_empty_string(False)) + self.assertFalse(validator.is_non_empty_string('')) + + self.assertTrue(validator.is_non_empty_string('0')) + self.assertTrue(validator.is_non_empty_string('test_user')) + class DatafileValidationTests(base.BaseTest): diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index f065b051..8db46616 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -1143,6 +1143,30 @@ def test_track__invalid_object(self): mock_client_logging.error.assert_called_once_with('Datafile has invalid format. Failing "track".') + def test_track__invalid_experiment_key(self): + """ Test that None is returned and expected log messages are logged during track \ + when exp_key is in invalid format. """ + + with mock.patch.object(self.optimizely, 'logger') as mock_client_logging, \ + mock.patch('optimizely.helpers.validator.is_non_empty_string', return_value=False) as mock_validator: + self.assertIsNone(self.optimizely.track(99, 'test_user')) + + mock_validator.assert_any_call(99) + + mock_client_logging.error.assert_called_once_with('Provided "event_key" is in an invalid format.') + + def test_track__invalid_user_id(self): + """ Test that None is returned and expected log messages are logged during track \ + when user_id is in invalid format. """ + + with mock.patch.object(self.optimizely, 'logger') as mock_client_logging, \ + mock.patch('optimizely.helpers.validator.is_non_empty_string', side_effect=[True, False]) as mock_validator: + self.assertIsNone(self.optimizely.track('test_event', 99)) + + mock_validator.assert_any_call(99) + + mock_client_logging.error.assert_called_once_with('Provided "user_id" is in an invalid format.') + def test_get_variation__invalid_object(self): """ Test that get_variation logs error if Optimizely object is not created correctly. """ @@ -1153,7 +1177,7 @@ def test_get_variation__invalid_object(self): mock_client_logging.error.assert_called_once_with('Datafile has invalid format. Failing "get_variation".') - def test_get_variation_invalid_experiment_key(self): + def test_get_variation_unknown_experiment_key(self): """ Test that get_variation retuns None when invalid experiment key is given. """ with mock.patch.object(self.optimizely, 'logger') as mock_client_logging: self.optimizely.get_variation('aabbccdd', 'test_user', None) @@ -1162,25 +1186,29 @@ def test_get_variation_invalid_experiment_key(self): 'Experiment key "aabbccdd" is invalid. Not activating user "test_user".' ) - 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. """ + def test_is_feature_enabled__returns_false_for_invalid_feature_key(self): + """ Test that is_feature_enabled returns false if the provided feature key is invalid. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - with mock.patch.object(opt_obj, 'logger') as mock_client_logging: + with mock.patch.object(opt_obj, 'logger') as mock_client_logging,\ + mock.patch('optimizely.helpers.validator.is_non_empty_string', return_value=False) as mock_validator: self.assertFalse(opt_obj.is_feature_enabled(None, 'test_user')) - mock_client_logging.error.assert_called_once_with(enums.Errors.NONE_FEATURE_KEY_PARAMETER) + mock_validator.assert_any_call(None) + mock_client_logging.error.assert_called_with('Provided "feature_key" is in an invalid format.') - 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. """ + def test_is_feature_enabled__returns_false_for_invalid_user_id(self): + """ Test that is_feature_enabled returns false if the provided user ID is invalid. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - with mock.patch.object(opt_obj, 'logger') as mock_client_logging: - self.assertFalse(opt_obj.is_feature_enabled('feature_key', None)) + with mock.patch.object(opt_obj, 'logger') as mock_client_logging,\ + mock.patch('optimizely.helpers.validator.is_non_empty_string', side_effect=[True, False]) as mock_validator: + self.assertFalse(opt_obj.is_feature_enabled('feature_key', 1.2)) - mock_client_logging.error.assert_called_once_with(enums.Errors.NONE_USER_ID_PARAMETER) + mock_validator.assert_any_call(1.2) + mock_client_logging.error.assert_called_with('Provided "user_id" is in an invalid format.') 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. """ @@ -1188,7 +1216,7 @@ def test_is_feature_enabled__returns_false_for_invalid_feature(self): opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature') as mock_decision, \ - mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event') as mock_dispatch_event: + mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event') as mock_dispatch_event: self.assertFalse(opt_obj.is_feature_enabled('invalid_feature', 'user1')) self.assertFalse(mock_decision.called) @@ -1462,6 +1490,14 @@ def side_effect(*args, **kwargs): mock_is_feature_enabled.assert_any_call('test_feature_in_group', 'user_1', None) mock_is_feature_enabled.assert_any_call('test_feature_in_experiment_and_rollout', 'user_1', None) + def test_get_enabled_features_invalid_user_id(self): + with mock.patch.object(self.optimizely, 'logger') as mock_client_logging, \ + mock.patch('optimizely.helpers.validator.is_non_empty_string', return_value=False) as mock_validator: + self.optimizely.get_enabled_features(1.2) + + mock_validator.assert_any_call(1.2) + mock_client_logging.error.assert_called_once_with('Provided "user_id" is in an invalid format.') + def test_get_enabled_features__invalid_object(self): """ Test that get_enabled_features returns empty list if Optimizely object is not valid. """ @@ -2003,6 +2039,52 @@ def test_get_variation__invalid_attributes(self): mock_client_logging.error.assert_called_once_with('Provided attributes are in an invalid format.') + def test_get_variation__invalid_experiment_key(self): + """ Test that None is returned and expected log messages are logged during get_variation \ + when exp_key is in invalid format. """ + + with mock.patch.object(self.optimizely, 'logger') as mock_client_logging,\ + mock.patch('optimizely.helpers.validator.is_non_empty_string', return_value=False) as mock_validator: + self.assertIsNone(self.optimizely.get_variation(99, 'test_user')) + + mock_validator.assert_any_call(99) + mock_client_logging.error.assert_called_once_with('Provided "experiment_key" is in an invalid format.') + + def test_get_variation__invalid_user_id(self): + """ Test that None is returned and expected log messages are logged during get_variation \ + when user_id is in invalid format. """ + + with mock.patch.object(self.optimizely, 'logger') as mock_client_logging,\ + mock.patch('optimizely.helpers.validator.is_non_empty_string', side_effect=[True, False]) as mock_validator: + self.assertIsNone(self.optimizely.get_variation('test_experiment', 99)) + + mock_validator.assert_any_call(99) + mock_client_logging.error.assert_called_once_with('Provided "user_id" is in an invalid format.') + + def test_activate__invalid_experiment_key(self): + """ Test that None is returned and expected log messages are logged during activate \ + when exp_key is in invalid format. """ + + with mock.patch.object(self.optimizely, 'logger') as mock_client_logging,\ + mock.patch('optimizely.helpers.validator.is_non_empty_string', return_value=False) as mock_validator: + self.assertIsNone(self.optimizely.activate(99, 'test_user')) + + mock_validator.assert_any_call(99) + + mock_client_logging.error.assert_called_once_with('Provided "experiment_key" is in an invalid format.') + + def test_activate__invalid_user_id(self): + """ Test that None is returned and expected log messages are logged during activate \ + when user_id is in invalid format. """ + + with mock.patch.object(self.optimizely, 'logger') as mock_client_logging,\ + mock.patch('optimizely.helpers.validator.is_non_empty_string', side_effect=[True, False]) as mock_validator: + self.assertIsNone(self.optimizely.activate('test_experiment', 99)) + + mock_validator.assert_any_call(99) + + mock_client_logging.error.assert_called_once_with('Provided "user_id" is in an invalid format.') + def test_activate__invalid_attributes(self): """ Test that expected log messages are logged during activate when attributes are in invalid format. """ with mock.patch.object(self.optimizely, 'logger') as mock_client_logging: