Merge pull request #52 from launchdarkly/eb/events-for-bad-users

send as much of a feature event as possible even if user is invalid

Author: eli-darkly

ldclient/client.py

@@ -139,7 +139,8 @@ def toggle(self, key, user, default):
 
     def variation(self, key, user, default):
         default = self._config.get_default(key, default)
-        self._sanitize_user(user)
+        if user is not None:
+            self._sanitize_user(user)
 
         if self._config.offline:
             return default
@@ -158,12 +159,7 @@ def send_event(value, version=None):
                 send_event(default)
                 return default
 
-        if user is None or user.get('key') is None:
-            log.warn("Missing user or user key when evaluating Feature Flag key: " + key + ". Returning default.")
-            send_event(default)
-            return default
-
-        if user.get('key', "") == "":
+        if user is not None and user.get('key', "") == "":
             log.warn("User key is blank. Flag evaluation will proceed, but the user will not be stored in LaunchDarkly.")
 
         def cb(flag):
@@ -177,6 +173,7 @@ def cb(flag):
 
             except Exception as e:
                 log.error("Exception caught in variation: " + e.message + " for flag key: " + key + " and user: " + str(user))
+                send_event(default)
 
             return default
 
@@ -186,13 +183,19 @@ def _evaluate(self, flag, user):
         return evaluate(flag, user, self._store)
 
     def _evaluate_and_send_events(self, flag, user, default):
-        result = evaluate(flag, user, self._store)
-        for event in result.events or []:
-            self._send_event(event)
-
-        value = default if result.value is None else result.value
+        if user is None or user.get('key') is None:
+            log.warn("Missing user or user key when evaluating Feature Flag key: " + flag.get('key') + ". Returning default.")
+            value = default
+            variation = None
+        else:
+            result = evaluate(flag, user, self._store)
+            for event in result.events or []:
+                self._send_event(event)
+            value = default if result.value is None else result.value
+            variation = result.variation
+        
         self._send_event({'kind': 'feature', 'key': flag.get('key'),
-                          'user': user, 'variation': result.variation, 'value': value,
+                          'user': user, 'variation': variation, 'value': value,
                           'default': default, 'version': flag.get('version'),
                           'trackEvents': flag.get('trackEvents'),
                           'debugEventsUntilDate': flag.get('debugEventsUntilDate')})

testing/test_ldclient.py

@@ -88,6 +88,14 @@ def setup_function(function):
     }
 
 
+def make_client(store):
+    return LDClient(config=Config(sdk_key = 'SDK_KEY',
+                                  base_uri="http://localhost:3000",
+                                  event_processor_class=MockEventProcessor,
+                                  update_processor_class=MockUpdateProcessor,
+                                  feature_store=store))
+
+
 def get_first_event(c):
     return c._event_processor._events.pop(0)
 
@@ -174,25 +182,6 @@ def test_defaults_and_online_no_default():
     assert e['kind'] == 'feature' and e['key'] == u'baz' and e['user'] == user
 
 
-def test_exception_in_retrieval():
-    class ExceptionFeatureRequester(FeatureRequester):
-        def __init__(self, *_):
-            pass
-
-        def get_all(self):
-            raise Exception("blah")
-
-    client = LDClient(config=Config(base_uri="http://localhost:3000",
-                                    defaults={"foo": "bar"},
-                                    feature_store=InMemoryFeatureStore(),
-                                    feature_requester_class=ExceptionFeatureRequester,
-                                    event_processor_class=MockEventProcessor,
-                                    update_processor_class=MockUpdateProcessor))
-    assert "bar" == client.variation('foo', user, default="jim")
-    e = get_first_event(client)
-    assert e['kind'] == 'feature' and e['key'] == u'foo' and e['user'] == user
-
-
 def test_no_defaults():
     assert "bar" == offline_client.variation('foo', user, default="bar")
 
@@ -205,25 +194,88 @@ def test_event_for_existing_feature():
         u'variations': ['a', 'b'],
         u'fallthrough': {
             u'variation': 1
-        }
+        },
+        u'trackEvents': True
     }
     store = InMemoryFeatureStore()
     store.init({FEATURES: {'feature.key': feature}})
-    client = LDClient(config=Config(sdk_key = 'SDK_KEY',
-                                    base_uri="http://localhost:3000",
-                                    event_processor_class=MockEventProcessor,
-                                    update_processor_class=MockUpdateProcessor,
-                                    feature_store=store))
+    client = make_client(store)
     assert 'b' == client.variation('feature.key', user, default='c')
     e = get_first_event(client)
     assert (e['kind'] == 'feature' and
         e['key'] == 'feature.key' and
         e['user'] == user and
         e['value'] == 'b' and
         e['variation'] == 1 and
+        e['default'] == 'c' and
+        e['trackEvents'] == True)
+
+
+def test_event_for_unknown_feature():
+    store = InMemoryFeatureStore()
+    store.init({FEATURES: {}})
+    client = make_client(store)
+    assert 'c' == client.variation('feature.key', user, default='c')
+    e = get_first_event(client)
+    assert (e['kind'] == 'feature' and
+        e['key'] == 'feature.key' and
+        e['user'] == user and
+        e['value'] == 'c' and
+        e['variation'] == None and
         e['default'] == 'c')
 
 
+def test_event_for_existing_feature_with_no_user():
+    feature = {
+        u'key': u'feature.key',
+        u'salt': u'abc',
+        u'on': True,
+        u'variations': ['a', 'b'],
+        u'fallthrough': {
+            u'variation': 1
+        },
+        u'trackEvents': True
+    }
+    store = InMemoryFeatureStore()
+    store.init({FEATURES: {'feature.key': feature}})
+    client = make_client(store)
+    assert 'c' == client.variation('feature.key', None, default='c')
+    e = get_first_event(client)
+    assert (e['kind'] == 'feature' and
+        e['key'] == 'feature.key' and
+        e['user'] == None and
+        e['value'] == 'c' and
+        e['variation'] == None and
+        e['default'] == 'c' and
+        e['trackEvents'] == True)
+
+
+def test_event_for_existing_feature_with_no_user_key():
+    feature = {
+        u'key': u'feature.key',
+        u'salt': u'abc',
+        u'on': True,
+        u'variations': ['a', 'b'],
+        u'fallthrough': {
+            u'variation': 1
+        },
+        u'trackEvents': True
+    }
+    store = InMemoryFeatureStore()
+    store.init({FEATURES: {'feature.key': feature}})
+    client = make_client(store)
+    bad_user = { u'name': u'Bob' }
+    assert 'c' == client.variation('feature.key', bad_user, default='c')
+    e = get_first_event(client)
+    assert (e['kind'] == 'feature' and
+        e['key'] == 'feature.key' and
+        e['user'] == bad_user and
+        e['value'] == 'c' and
+        e['variation'] == None and
+        e['default'] == 'c' and
+        e['trackEvents'] == True)
+
+
 def test_all_flags():
     feature = {
         u'key': u'feature.key',