fixes for tests and code improvement on input_validator

Author: Matias Melograno

splitio/clients.py

@@ -48,10 +48,15 @@ def destroy(self):
         self._destroyed = True
         self._broker.destroy()
 
-    def _handle_custom_impression(self, impression, attributes):
+    def _send_impression_to_listener(self, impression, attributes):
         '''
-        Handles custom impression if is present. Basically, sends the data
-        to client if some logic is wanted to do.
+        Sends impression result to custom listener.
+
+        :param impression: Generated impression
+        :type impression: Impression
+
+        :param attributes: An optional dictionary of attributes
+        :type attributes: dict
         '''
         if self._impression_listener is not None:
             try:
@@ -124,7 +129,7 @@ def get_treatment(self, key, feature, attributes=None):
                                                 _change_number, bucketing_key, start)
             self._record_stats(impression, start, SDK_GET_TREATMENT)
 
-            self._handle_custom_impression(impression, attributes)
+            self._send_impression_to_listener(impression, attributes)
 
             return _treatment
         except Exception:  # pylint: disable=broad-except
@@ -140,7 +145,7 @@ def get_treatment(self, key, feature, attributes=None):
                 )
                 self._record_stats(impression, start, SDK_GET_TREATMENT)
 
-                self._handle_custom_impression(impression, attributes)
+                self._send_impression_to_listener(impression, attributes)
             except Exception:  # pylint: disable=broad-except
                 self._logger.exception(
                     'Exception reporting impression into get_treatment exception block'
@@ -153,8 +158,6 @@ def _build_impression(
     ):
         """
         Build an impression.
-
-        TODO: REFACTOR THIS!
         """
         if not self._labels_enabled:
             label = None
@@ -176,7 +179,7 @@ def _record_stats(self, impression, start, operation):
         :type start: int
 
         :param operation: operation performed.
-        :type operation: string
+        :type operation: str
         """
         try:
             end = int(round(time.time() * 1000))
@@ -243,13 +246,13 @@ def track(self, key, traffic_type, event_type, value=None):
         Track an event.
 
         :param key: user key associated to the event
-        :type key: string
+        :type key: str
 
         :param traffic_type: traffic type name
-        :type traffic_type: string
+        :type traffic_type: str
 
         :param event_type: event type name
-        :type event_type: string
+        :type event_type: str
 
         :param value: (Optional) value associated to the event
         :type value: Number

splitio/input_validator.py

@@ -62,7 +62,7 @@ def _check_not_empty(value, name, operation):
     :return: The result of validation
     :rtype: True|False
     """
-    if not value:
+    if value.strip() == "":
         _LOGGER.error('{}: {} must not be empty.'.format(operation, name))
         return False
     return True
@@ -108,7 +108,7 @@ def _check_can_convert(value, name, operation, message):
     if isinstance(value, six.string_types):
         return True
     else:
-        if isinstance(value, bool) or isinstance(value, Number) is False:
+        if isinstance(value, bool) or (not isinstance(value, Number)):
             _LOGGER.error('{}: {} {} {}'.format(operation, name, value, message))
             return False
     _LOGGER.warning('{}: {} {} is not of type string, converting.'
@@ -131,11 +131,11 @@ def _check_valid_matching_key(matching_key):
                       'matchingKey with valid string properties.')
         return False
     if isinstance(matching_key, six.string_types):
-        if _check_not_empty(matching_key, 'matching_key', 'get_treatment') is False:
+        if not _check_not_empty(matching_key, 'matching_key', 'get_treatment'):
             return False
     else:
-        if _check_can_convert(matching_key, 'matching_key', 'get_treatment',
-                              'has to be of type string.') is False:
+        if not _check_can_convert(matching_key, 'matching_key', 'get_treatment',
+                                  'has to be of type string.'):
             return False
     return True
 
@@ -153,8 +153,8 @@ def _check_valid_bucketing_key(bucketing_key):
     if bucketing_key is None:
         _LOGGER.warning('get_treatment: Key object should have bucketingKey set.')
         return None
-    if _check_can_convert(bucketing_key, 'bucketing_key', 'get_treatment',
-                          'has to be of type string.') is False:
+    if not _check_can_convert(bucketing_key, 'bucketing_key', 'get_treatment',
+                              'has to be of type string.'):
         return False
     return str(bucketing_key)
 
@@ -171,7 +171,7 @@ def validate_key(key):
     """
     matching_key_result = None
     bucketing_key_result = None
-    if _check_not_null(key, 'key', 'get_treatment') is False:
+    if not _check_not_null(key, 'key', 'get_treatment'):
         return None, None
     if isinstance(key, Key):
         if _check_valid_matching_key(key.matching_key):
@@ -198,8 +198,8 @@ def validate_feature_name(feature_name):
     :return: feature_name
     :rtype: str|None
     """
-    if _check_not_null(feature_name, 'feature_name', 'get_treatment') is False or \
-       _check_is_string(feature_name, 'feature_name', 'get_treatment') is False:
+    if (not _check_not_null(feature_name, 'feature_name', 'get_treatment')) or \
+       (not _check_is_string(feature_name, 'feature_name', 'get_treatment')):
         return None
     return feature_name
 
@@ -213,8 +213,8 @@ def validate_track_key(key):
     :return: key
     :rtype: str|None
     """
-    if _check_not_null(key, 'key', 'track') is False or \
-       _check_can_convert(key, 'key', 'track', 'has to be of type string.') is False:
+    if (not _check_not_null(key, 'key', 'track')) or \
+       (not _check_can_convert(key, 'key', 'track', 'has to be of type string.')):
         return None
     return str(key)
 
@@ -228,9 +228,9 @@ def validate_traffic_type(traffic_type):
     :return: traffic_type
     :rtype: str|None
     """
-    if _check_not_null(traffic_type, 'traffic_type', 'track') is False or \
-       _check_is_string(traffic_type, 'traffic_type', 'track') is False or \
-       _check_not_empty(traffic_type, 'traffic_type', 'track') is False:
+    if (not _check_not_null(traffic_type, 'traffic_type', 'track')) or \
+       (not _check_is_string(traffic_type, 'traffic_type', 'track')) or \
+       (not _check_not_empty(traffic_type, 'traffic_type', 'track')):
         return None
     return traffic_type
 
@@ -244,10 +244,10 @@ def validate_event_type(event_type):
     :return: event_type
     :rtype: str|None
     """
-    if _check_not_null(event_type, 'event_type', 'track') is False or \
-       _check_is_string(event_type, 'event_type', 'track') is False or \
-       _check_pattern_match(event_type, 'event_type', 'track',
-                            r'[a-zA-Z0-9][-_\.a-zA-Z0-9]{0,62}') is False:
+    if (not _check_not_null(event_type, 'event_type', 'track')) or \
+       (not _check_is_string(event_type, 'event_type', 'track')) or \
+       (not _check_pattern_match(event_type, 'event_type', 'track',
+                                 r'[a-zA-Z0-9][-_\.a-zA-Z0-9]{0,62}')):
         return None
     return event_type
 
@@ -263,7 +263,7 @@ def validate_value(value):
     """
     if value is None:
         return None
-    if not isinstance(value, Number) or isinstance(value, bool):
+    if (not isinstance(value, Number)) or isinstance(value, bool):
         _LOGGER.error('track: value must be a number.')
         return False
     return value
@@ -278,7 +278,7 @@ def validate_manager_feature_name(feature_name):
     :return: feature_name
     :rtype: str|None
     """
-    if _check_not_null(feature_name, 'feature_name', 'split') is False or \
-       _check_is_string(feature_name, 'feature_name', 'split') is False:
+    if (not _check_not_null(feature_name, 'feature_name', 'split')) or \
+       (not _check_is_string(feature_name, 'feature_name', 'split')):
         return None
     return feature_name

splitio/tests/test_clients.py

@@ -8,13 +8,16 @@
     # Python 2
     import mock
 
+import tempfile
 import arrow
 import os.path
 
 from unittest import TestCase
+from time import sleep
 
+from splitio import get_factory
 from splitio.clients import Client
-from splitio.brokers import JSONFileBroker, RedisBroker, \
+from splitio.brokers import JSONFileBroker, RedisBroker, LocalhostBroker, \
     UWSGIBroker, randomize_interval, SelfRefreshingBroker
 from splitio.exceptions import TimeoutException
 from splitio.config import DEFAULT_CONFIG, MAX_INTERVAL, SDK_API_BASE_URL, \
@@ -1149,7 +1152,6 @@ def test_test_killed_not_in_segment_key(self):
             self.fake_id_not_in_segment, 'test_killed'))
 
 
-'''
 class LocalhostEnvironmentClientParseSplitFileTests(TestCase, MockUtilsMixin):
     def setUp(self):
         self.some_file_name = mock.MagicMock()
@@ -1161,9 +1163,8 @@ def setUp(self):
 
         self.open_mock = self.patch_builtin('open')
         self.some_config = mock.MagicMock()
-        self.broker = LocalhostBroker(self.some_config)
         self.threading_mock = self.patch('threading.Thread')
-        self.broker = LocalhostBroker()
+        self.broker = LocalhostBroker(self.some_config)
 
     def test_skips_comment_lines(self):
         """Test that _parse_split_file skips comment lines"""
@@ -1202,6 +1203,7 @@ def test_raises_value_error_if_ioerror_is_raised(self):
         with self.assertRaises(ValueError):
             self.broker._parse_split_file(self.some_file_name)
 
+
 class LocalhostBrokerOffTheGrid(TestCase):
     """
     Tests for LocalhostEnvironmentClient. Auto update config behaviour
@@ -1226,7 +1228,6 @@ def test_auto_update_splits(self):
 
             self.assertEqual(client.get_treatment('x', 'a_test_split'), 'on')
             client.destroy()
-'''
 
 
 class TestClientDestroy(TestCase):