Merge pull request #141 from splitio/gather/input-validation-v2

Gather/input validation v2

Author: mmelograno

splitio/client/client.py

@@ -85,6 +85,25 @@ def _send_impression_to_listener(self, impression, attributes):
                 )
                 self._logger.debug('Error', exc_info=True)
 
+    def _evaluate_if_ready(self, matching_key, bucketing_key, feature, attributes=None):
+        if not self.ready:
+            return {
+                'treatment': CONTROL,
+                'configurations': None,
+                'impression': {
+                    'label': Label.NOT_READY,
+                    'change_number': None
+                }
+            }
+
+        return self._evaluator.evaluate_treatment(
+            feature,
+            matching_key,
+            bucketing_key,
+            attributes
+        )
+
+
     def get_treatment_with_config(self, key, feature, attributes=None):
         """
         Get the treatment and config for a feature and key, with optional dictionary of attributes.
@@ -109,19 +128,18 @@ def get_treatment_with_config(self, key, feature, attributes=None):
             start = int(round(time.time() * 1000))
 
             matching_key, bucketing_key = input_validator.validate_key(key)
-            feature = input_validator.validate_feature_name(feature)
+            feature = input_validator.validate_feature_name(
+                feature,
+                self.ready,
+                self._factory._get_storage('splits')  #pylint: disable=protected-access
+            )
 
             if (matching_key is None and bucketing_key is None) \
                     or feature is None \
                     or not input_validator.validate_attributes(attributes):
                 return CONTROL, None
 
-            result = self._evaluator.evaluate_treatment(
-                feature,
-                matching_key,
-                bucketing_key,
-                attributes
-            )
+            result = self._evaluate_if_ready(matching_key, bucketing_key, feature, attributes)
 
             impression = self._build_impression(
                 matching_key,
@@ -204,32 +222,30 @@ def get_treatments_with_config(self, key, features, attributes=None):
         if input_validator.validate_attributes(attributes) is False:
             return input_validator.generate_control_treatments(features)
 
-        features = input_validator.validate_features_get_treatments(features)
+        features, missing = input_validator.validate_features_get_treatments(
+            features,
+            self.ready,
+            self._factory._get_storage('splits')  # pylint: disable=protected-access
+        )
         if features is None:
             return {}
 
         bulk_impressions = []
-        treatments = {}
+        treatments = {name: (CONTROL, None) for name in missing}
 
         for feature in features:
             try:
-                treatment = self._evaluator.evaluate_treatment(
-                    feature,
-                    matching_key,
-                    bucketing_key,
-                    attributes
-                )
-
+                result = self._evaluate_if_ready(matching_key, bucketing_key, feature, attributes)
                 impression = self._build_impression(matching_key,
                                                     feature,
-                                                    treatment['treatment'],
-                                                    treatment['impression']['label'],
-                                                    treatment['impression']['change_number'],
+                                                    result['treatment'],
+                                                    result['impression']['label'],
+                                                    result['impression']['change_number'],
                                                     bucketing_key,
                                                     start)
 
                 bulk_impressions.append(impression)
-                treatments[feature] = (treatment['treatment'], treatment['configurations'])
+                treatments[feature] = (result['treatment'], result['configurations'])
 
             except Exception:  # pylint: disable=broad-except
                 self._logger.error('get_treatments: An exception occured when evaluating '
@@ -338,7 +354,13 @@ def track(self, key, traffic_type, event_type, value=None, properties=None):
 
         key = input_validator.validate_track_key(key)
         event_type = input_validator.validate_event_type(event_type)
-        traffic_type = input_validator.validate_traffic_type(traffic_type)
+        should_validate_existance = self.ready and self._factory._apikey != 'localhost' #pylint: disable=protected-access
+        traffic_type = input_validator.validate_traffic_type(
+            traffic_type,
+            should_validate_existance,
+            self._factory._get_storage('splits'), #pylint: disable=protected-access
+        )
+
         value = input_validator.validate_value(value)
         valid, properties, size = input_validator.valid_properties(properties)
 

splitio/client/factory.py

@@ -4,8 +4,9 @@
 
 import logging
 import threading
-from enum import Enum
+from collections import Counter
 
+from enum import Enum
 import six
 
 from splitio.client.client import Client
@@ -45,6 +46,11 @@
     LocalhostSplitSynchronizationTask, LocalhostTelemetryStorage
 
 
+_LOGGER = logging.getLogger(__name__)
+_INSTANTIATED_FACTORIES = Counter()
+_INSTANTIATED_FACTORIES_LOCK = threading.RLock()
+
+
 class Status(Enum):
     """Factory Status."""
 
@@ -64,6 +70,7 @@ class SplitFactory(object):  # pylint: disable=too-many-instance-attributes
 
     def __init__(  # pylint: disable=too-many-arguments
             self,
+            apikey,
             storages,
             labels_enabled,
             apis=None,
@@ -87,6 +94,7 @@ def __init__(  # pylint: disable=too-many-arguments
         :param impression_listener: User custom listener to handle impressions locally.
         :type impression_listener: splitio.client.listener.ImpressionListener
         """
+        self._apikey = apikey
         self._logger = logging.getLogger(self.__class__.__name__)
         self._storages = storages
         self._labels_enabled = labels_enabled
@@ -197,6 +205,8 @@ def _wait_for_tasks_to_stop():
                     task.stop()
         finally:
             self._status = Status.DESTROYED
+            with _INSTANTIATED_FACTORIES_LOCK:
+                _INSTANTIATED_FACTORIES.subtract([self._apikey])
 
     @property
     def destroyed(self):
@@ -324,6 +334,7 @@ def segment_ready_task():
     segment_completion_thread.setDaemon(True)
     segment_completion_thread.start()
     return SplitFactory(
+        api_key,
         storages,
         cfg['labelsEnabled'],
         apis,
@@ -333,7 +344,7 @@ def segment_ready_task():
     )
 
 
-def _build_redis_factory(config):
+def _build_redis_factory(api_key, config):
     """Build and return a split factory with redis-based storage."""
     cfg = DEFAULT_CONFIG.copy()
     cfg.update(config)
@@ -347,13 +358,14 @@ def _build_redis_factory(config):
         'telemetry': RedisTelemetryStorage(redis_adapter, sdk_metadata)
     }
     return SplitFactory(
+        api_key,
         storages,
         cfg['labelsEnabled'],
         impression_listener=_wrap_impression_listener(cfg['impressionListener'], sdk_metadata)
     )
 
 
-def _build_uwsgi_factory(config):
+def _build_uwsgi_factory(api_key, config):
     """Build and return a split factory with redis-based storage."""
     cfg = DEFAULT_CONFIG.copy()
     cfg.update(config)
@@ -367,6 +379,7 @@ def _build_uwsgi_factory(config):
         'telemetry': UWSGITelemetryStorage(uwsgi_adapter)
     }
     return SplitFactory(
+        api_key,
         storages,
         cfg['labelsEnabled'],
         impression_listener=_wrap_impression_listener(cfg['impressionListener'], sdk_metadata)
@@ -393,25 +406,47 @@ def _build_localhost_factory(config):
         ready_event
     )}
     tasks['splits'].start()
-    return SplitFactory(storages, False, None, tasks, ready_event)
+    return SplitFactory('localhost', storages, False, None, tasks, ready_event)
 
 
 def get_factory(api_key, **kwargs):
     """Build and return the appropriate factory."""
-    config = kwargs.get('config', {})
+    try:
+        _INSTANTIATED_FACTORIES_LOCK.acquire()
+        if _INSTANTIATED_FACTORIES:
+            if api_key in _INSTANTIATED_FACTORIES:
+                _LOGGER.warning(
+                    "factory instantiation: You already have %d %s with this API Key. "
+                    "We recommend keeping only one instance of the factory at all times "
+                    "(Singleton pattern) and reusing it throughout your application.",
+                    _INSTANTIATED_FACTORIES[api_key],
+                    'factory' if _INSTANTIATED_FACTORIES[api_key] == 1 else 'factories'
+                )
+            else:
+                _LOGGER.warning(
+                    "factory instantiation: You already have an instance of the Split factory. "
+                    "Make sure you definitely want this additional instance. "
+                    "We recommend keeping only one instance of the factory at all times "
+                    "(Singleton pattern) and reusing it throughout your application."
+                )
 
-    if api_key == 'localhost':
-        return _build_localhost_factory(config)
+        config = kwargs.get('config', {})
 
-    if 'redisHost' in config or 'redisSentinels' in config:
-        return _build_redis_factory(config)
+        if api_key == 'localhost':
+            return _build_localhost_factory(config)
 
-    if 'uwsgiClient' in config:
-        return _build_uwsgi_factory(config)
+        if 'redisHost' in config or 'redisSentinels' in config:
+            return _build_redis_factory(api_key, config)
 
-    return _build_in_memory_factory(
-        api_key,
-        config,
-        kwargs.get('sdk_api_base_url'),
-        kwargs.get('events_api_base_url')
-    )
+        if 'uwsgiClient' in config:
+            return _build_uwsgi_factory(api_key, config)
+
+        return _build_in_memory_factory(
+            api_key,
+            config,
+            kwargs.get('sdk_api_base_url'),
+            kwargs.get('events_api_base_url')
+        )
+    finally:
+        _INSTANTIATED_FACTORIES.update([api_key])
+        _INSTANTIATED_FACTORIES_LOCK.release()