Merge 55a721b into f3e395a

optimizely/exceptions.py

@@ -1,52 +1,57 @@
-# Copyright 2016-2017, 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
-#
-# http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-
-
-class InvalidAttributeException(Exception):
-  """ Raised when provided attribute is invalid. """
-  pass
-
-
-class InvalidAudienceException(Exception):
-  """ Raised when provided audience is invalid. """
-  pass
-
-
-class InvalidEventTagException(Exception):
-  """ Raised when provided event tag is invalid. """
-  pass
-
-
-class InvalidExperimentException(Exception):
-  """ Raised when provided experiment key is invalid. """
-  pass
-
-
-class InvalidEventException(Exception):
-  """ Raised when provided event key is invalid. """
-  pass
-
-
-class InvalidGroupException(Exception):
-  """ Raised when provided group ID is invalid. """
-  pass
-
-
-class InvalidInputException(Exception):
-  """ Raised when provided datafile, event dispatcher, logger or error handler is invalid. """
-  pass
-
-
-class InvalidVariationException(Exception):
-  """ Raised when provided variation is invalid. """
-  pass
+# 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
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+
+class InvalidAttributeException(Exception):
+  """ Raised when provided attribute is invalid. """
+  pass
+
+
+class InvalidAudienceException(Exception):
+  """ Raised when provided audience is invalid. """
+  pass
+
+
+class InvalidEventTagException(Exception):
+  """ Raised when provided event tag is invalid. """
+  pass
+
+
+class InvalidExperimentException(Exception):
+  """ Raised when provided experiment key is invalid. """
+  pass
+
+
+class InvalidEventException(Exception):
+  """ Raised when provided event key is invalid. """
+  pass
+
+
+class InvalidGroupException(Exception):
+  """ Raised when provided group ID is invalid. """
+  pass
+
+
+class InvalidInputException(Exception):
+  """ Raised when provided datafile, event dispatcher, logger or error handler is invalid. """
+  pass
+
+
+class InvalidVariationException(Exception):
+  """ Raised when provided variation is invalid. """
+  pass
+
+
+class UnsupportedDatafileVersionException(Exception):
+  """ Raised when provided version in datafile is not supported. """
+  pass

optimizely/helpers/enums.py

@@ -44,8 +44,7 @@ class Errors(object):
   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.'
+  UNSUPPORTED_DATAFILE_VERSION = 'This version of the Python SDK does not support the given datafile version: "{}".'
 
 
 class NotificationTypes(object):
@@ -65,3 +64,9 @@ class ControlAttributes(object):
   BOT_FILTERING = '$opt_bot_filtering'
   BUCKETING_ID = '$opt_bucketing_id'
   USER_AGENT = '$opt_user_agent'
+
+
+class DatafileVersions(object):
+  V2 = '2'
+  V3 = '3'
+  V4 = '4'

optimizely/optimizely.py

@@ -63,20 +63,22 @@ def __init__(self,
 
     try:
       self.config = project_config.ProjectConfig(datafile, self.logger, self.error_handler)
-    except:
+
+    except Exception as error:
       self.is_valid = False
-      self.config = None
       # We actually want to log this error to stderr, so make sure the logger
-      #   has a handler capable of doing that.
+      # has a handler capable of doing that.
       self.logger = _logging.reset_logger(self.logger_name)
-      self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('datafile'))
-      return
 
-    if not self.config.was_parsing_successful():
-      self.is_valid = False
-      # We actually want to log this error to stderr, so make sure the logger
-      #   has a handler capable of doing that.
-      self.logger.error(enums.Errors.UNSUPPORTED_DATAFILE_VERSION)
+      if type(error) is exceptions.UnsupportedDatafileVersionException:
+        error_msg = error.args[0]
+        error_to_handle = error
+      else:
+        error_msg = enums.Errors.INVALID_INPUT_ERROR.format('datafile')
+        error_to_handle = exceptions.InvalidInputException(error_msg)
+
+      self.logger.exception(error_msg)
+      self.error_handler.handle_error(error_to_handle)
       return
 
     self.event_builder = event_builder.EventBuilder(self.config)

optimizely/project_config.py

@@ -18,12 +18,7 @@
 from . import entities
 from . import exceptions
 
-REVENUE_GOAL_KEY = 'Total Revenue'
-V1_CONFIG_VERSION = '1'
-V2_CONFIG_VERSION = '2'
-
-SUPPORTED_VERSIONS = [V2_CONFIG_VERSION]
-UNSUPPORTED_VERSIONS = [V1_CONFIG_VERSION]
+SUPPORTED_VERSIONS = [enums.DatafileVersions.V2, enums.DatafileVersions.V3, enums.DatafileVersions.V4]
 
 RESERVED_ATTRIBUTE_PREFIX = '$opt_'
 
@@ -41,12 +36,14 @@ def __init__(self, datafile, logger, error_handler):
     """
 
     config = json.loads(datafile)
-    self.parsing_succeeded = False
     self.logger = logger
     self.error_handler = error_handler
     self.version = config.get('version')
-    if self.version in UNSUPPORTED_VERSIONS:
-      return
+    if self.version not in SUPPORTED_VERSIONS:
+      raise exceptions.UnsupportedDatafileVersionException(
+        enums.Errors.UNSUPPORTED_DATAFILE_VERSION.format(self.version)
+      )
+
     self.account_id = config.get('accountId')
     self.project_id = config.get('projectId')
     self.revision = config.get('revision')
@@ -109,8 +106,6 @@ def __init__(self, datafile, logger, error_handler):
           # Experiments in feature can only belong to one mutex group
           break
 
-    self.parsing_succeeded = True
-
     # Map of user IDs to another map of experiments to variations.
     # This contains all the forced variations set by the user
     # by calling set_forced_variation (it is not the same as the
@@ -176,15 +171,6 @@ def get_typecast_value(self, value, type):
     else:
       return value
 
-  def was_parsing_successful(self):
-    """ Helper method to determine if parsing the datafile was successful.
-
-    Returns:
-      Boolean depending on whether parsing the datafile succeeded or not.
-    """
-
-    return self.parsing_succeeded
-
   def get_version(self):
     """ Get version of the datafile.
 

tests/base.py

@@ -521,6 +521,56 @@ def setUp(self, config_dict='config_dict'):
       'projectId': '111001'
     }
 
+    self.config_dict_with_unsupported_version = {
+      'version': '5',
+      'rollouts': [],
+      'projectId': '10431130345',
+      'variables': [],
+      'featureFlags': [],
+      'experiments': [
+        {
+          'status': 'Running',
+          'key': 'ab_running_exp_untargeted',
+          'layerId': '10417730432',
+          'trafficAllocation': [
+            {
+              'entityId': '10418551353',
+              'endOfRange': 10000
+            }
+          ],
+          'audienceIds': [],
+          'variations': [
+            {
+              'variables': [],
+              'id': '10418551353',
+              'key': 'all_traffic_variation'
+            },
+            {
+              'variables': [],
+              'id': '10418510624',
+              'key': 'no_traffic_variation'
+            }
+          ],
+          'forcedVariations': {},
+          'id': '10420810910'
+        }
+      ],
+      'audiences': [],
+      'groups': [],
+      'attributes': [],
+      'accountId': '10367498574',
+      'events': [
+        {
+          'experimentIds': [
+            '10420810910'
+          ],
+          'id': '10404198134',
+          'key': 'winning'
+        }
+      ],
+      'revision': '1337'
+    }
+
     config = getattr(self, config_dict)
     self.optimizely = optimizely.Optimizely(json.dumps(config))
     self.project_config = self.optimizely.config

tests/test_optimizely.py

@@ -83,6 +83,26 @@ def test_init__invalid_datafile__logs_error(self):
     mock_client_logger.exception.assert_called_once_with('Provided "datafile" is in an invalid format.')
     self.assertFalse(opt_obj.is_valid)
 
+  def test_init__null_datafile__logs_error(self):
+    """ Test that null datafile logs error on init. """
+
+    mock_client_logger = mock.MagicMock()
+    with mock.patch('optimizely.logger.reset_logger', return_value=mock_client_logger):
+      opt_obj = optimizely.Optimizely(None)
+
+    mock_client_logger.exception.assert_called_once_with('Provided "datafile" is in an invalid format.')
+    self.assertFalse(opt_obj.is_valid)
+
+  def test_init__empty_datafile__logs_error(self):
+    """ Test that empty datafile logs error on init. """
+
+    mock_client_logger = mock.MagicMock()
+    with mock.patch('optimizely.logger.reset_logger', return_value=mock_client_logger):
+      opt_obj = optimizely.Optimizely("")
+
+    mock_client_logger.exception.assert_called_once_with('Provided "datafile" is in an invalid format.')
+    self.assertFalse(opt_obj.is_valid)
+
   def test_init__invalid_event_dispatcher__logs_error(self):
     """ Test that invalid event_dispatcher logs error on init. """
 
@@ -122,19 +142,37 @@ class InvalidErrorHandler(object):
     mock_client_logger.exception.assert_called_once_with('Provided "error_handler" is in an invalid format.')
     self.assertFalse(opt_obj.is_valid)
 
-  def test_init__v1_datafile__logs_error(self):
-    """ Test that v1 datafile logs error on init. """
+  def test_init__unsupported_datafile_version__logs_error(self):
+    """ Test that datafile with unsupported version logs error on init. """
 
-    self.config_dict['version'] = project_config.V1_CONFIG_VERSION
     mock_client_logger = mock.MagicMock()
-    with mock.patch('optimizely.logger.reset_logger', return_value=mock_client_logger):
-      opt_obj = optimizely.Optimizely(json.dumps(self.config_dict))
+    with mock.patch('optimizely.logger.reset_logger', return_value=mock_client_logger),\
+      mock.patch('optimizely.error_handler.NoOpErrorHandler.handle_error') as mock_error_handler:
+      opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_unsupported_version))
 
-    mock_client_logger.error.assert_called_once_with(
-      'Provided datafile has unsupported version. Please use SDK version 1.1.0 or earlier for datafile version 1.'
+    mock_client_logger.exception.assert_called_once_with(
+      'This version of the Python SDK does not support the given datafile version: "5".'
     )
+
+    args, kwargs = mock_error_handler.call_args
+    self.assertIsInstance(args[0], exceptions.UnsupportedDatafileVersionException)
+    self.assertEqual(args[0].args[0],
+                     'This version of the Python SDK does not support the given datafile version: "5".')
+
     self.assertFalse(opt_obj.is_valid)
 
+  def test_init_with_supported_datafile_version(self):
+    """ Test that datafile with supported version works as expected. """
+
+    self.assertTrue(self.config_dict['version'] in project_config.SUPPORTED_VERSIONS)
+
+    mock_client_logger = mock.MagicMock()
+    with mock.patch('optimizely.logger.reset_logger', return_value=mock_client_logger):
+      opt_obj = optimizely.Optimizely(json.dumps(self.config_dict))
+
+    mock_client_logger.exception.assert_not_called()
+    self.assertTrue(opt_obj.is_valid)
+
   def test_skip_json_validation_true(self):
     """ Test that on setting skip_json_validation to true, JSON schema validation is not performed. """
 
@@ -148,18 +186,32 @@ def test_invalid_json_raises_schema_validation_off(self):
 
     # Not  JSON
     mock_client_logger = mock.MagicMock()
-    with mock.patch('optimizely.logger.reset_logger', return_value=mock_client_logger):
-      optimizely.Optimizely('invalid_json', skip_json_validation=True)
+    with mock.patch('optimizely.logger.reset_logger', return_value=mock_client_logger),\
+      mock.patch('optimizely.error_handler.NoOpErrorHandler.handle_error') as mock_error_handler:
+      opt_obj = optimizely.Optimizely('invalid_json', skip_json_validation=True)
+
+    mock_client_logger.exception.assert_called_once_with('Provided "datafile" is in an invalid format.')
+    args, kwargs = mock_error_handler.call_args
+    self.assertIsInstance(args[0], exceptions.InvalidInputException)
+    self.assertEqual(args[0].args[0],
+                     'Provided "datafile" is in an invalid format.')
+    self.assertFalse(opt_obj.is_valid)
 
-    mock_client_logger.error.assert_called_once_with('Provided "datafile" is in an invalid format.')
     mock_client_logger.reset_mock()
+    mock_error_handler.reset_mock()
 
     # JSON having valid version, but entities have invalid format
-    with mock.patch('optimizely.logger.reset_logger', return_value=mock_client_logger):
-      optimizely.Optimizely({'version': '2', 'events': 'invalid_value', 'experiments': 'invalid_value'},
-                            skip_json_validation=True)
+    with mock.patch('optimizely.logger.reset_logger', return_value=mock_client_logger),\
+      mock.patch('optimizely.error_handler.NoOpErrorHandler.handle_error') as mock_error_handler:
+      opt_obj = optimizely.Optimizely({'version': '2', 'events': 'invalid_value', 'experiments': 'invalid_value'},
+                                      skip_json_validation=True)
 
-    mock_client_logger.error.assert_called_once_with('Provided "datafile" is in an invalid format.')
+    mock_client_logger.exception.assert_called_once_with('Provided "datafile" is in an invalid format.')
+    args, kwargs = mock_error_handler.call_args
+    self.assertIsInstance(args[0], exceptions.InvalidInputException)
+    self.assertEqual(args[0].args[0],
+                     'Provided "datafile" is in an invalid format.')
+    self.assertFalse(opt_obj.is_valid)
 
   def test_activate(self):
     """ Test that activate calls dispatch_event with right params and returns expected variation. """