prepare 6.11.3 release (#138)

ldclient/flag.py

@@ -198,17 +198,28 @@ def _variation_index_for_user(feature, rule, user):
     if rule.get('variation') is not None:
         return rule['variation']
 
-    if rule.get('rollout') is not None:
+    rollout = rule.get('rollout')
+    if rollout is None:
+        return None
+    variations = rollout.get('variations')
+    if variations is not None and len(variations) > 0:
         bucket_by = 'key'
-        if rule['rollout'].get('bucketBy') is not None:
-            bucket_by = rule['rollout']['bucketBy']
+        if rollout.get('bucketBy') is not None:
+            bucket_by = rollout['bucketBy']
         bucket = _bucket_user(user, feature['key'], feature['salt'], bucket_by)
         sum = 0.0
-        for wv in rule['rollout'].get('variations') or []:
+        for wv in variations:
             sum += wv.get('weight', 0.0) / 100000.0
             if bucket < sum:
                 return wv.get('variation')
 
+        # The user's bucket value was greater than or equal to the end of the last bucket. This could happen due
+        # to a rounding error, or due to the fact that we are scaling to 100000 rather than 99999, or the flag
+        # data could contain buckets that don't actually add up to 100000. Rather than returning an error in
+        # this case (or changing the scaling, which would potentially change the results for *all* users), we
+        # will simply put the user in the last bucket.
+        return variations[-1].get('variation')
+
     return None
 
 

testing/test_flag.py

@@ -1,6 +1,7 @@
+import math
 import pytest
 from ldclient.feature_store import InMemoryFeatureStore
-from ldclient.flag import EvaluationDetail, EvalResult, _bucket_user, evaluate
+from ldclient.flag import EvaluationDetail, EvalResult, _bucket_user, _variation_index_for_user, evaluate
 from ldclient.impl.event_factory import _EventFactory
 from ldclient.versioned_data_kind import FEATURES, SEGMENTS
 
@@ -384,7 +385,47 @@ def _make_bool_flag_from_clause(clause):
         'variations': [ False, True ]
     }
 
+def test_variation_index_is_returned_for_bucket():
+    user = { 'key': 'userkey' }
+    flag = { 'key': 'flagkey', 'salt': 'salt' }
+
+    # First verify that with our test inputs, the bucket value will be greater than zero and less than 100000,
+    # so we can construct a rollout whose second bucket just barely contains that value
+    bucket_value = math.trunc(_bucket_user(user, flag['key'], flag['salt'], 'key') * 100000)
+    assert bucket_value > 0 and bucket_value < 100000
+
+    bad_variation_a = 0
+    matched_variation = 1
+    bad_variation_b = 2
+    rule = {
+        'rollout': {
+            'variations': [
+                { 'variation': bad_variation_a, 'weight': bucket_value }, # end of bucket range is not inclusive, so it will *not* match the target value
+                { 'variation': matched_variation, 'weight': 1 }, # size of this bucket is 1, so it only matches that specific value
+                { 'variation': bad_variation_b, 'weight': 100000 - (bucket_value + 1) }
+            ]
+        }
+    }
+    result_variation = _variation_index_for_user(flag, rule, user)
+    assert result_variation == matched_variation
 
+def test_last_bucket_is_used_if_bucket_value_equals_total_weight():
+    user = { 'key': 'userkey' }
+    flag = { 'key': 'flagkey', 'salt': 'salt' }
+
+    # We'll construct a list of variations that stops right at the target bucket value
+    bucket_value = math.trunc(_bucket_user(user, flag['key'], flag['salt'], 'key') * 100000)
+
+    rule = {
+        'rollout': {
+            'variations': [
+                { 'variation': 0, 'weight': bucket_value }
+            ]
+        }
+    }
+    result_variation = _variation_index_for_user(flag, rule, user)
+    assert result_variation == 0
+
 def test_bucket_by_user_key():
     user = { u'key': u'userKeyA' }
     bucket = _bucket_user(user, 'hashKey', 'saltyA', 'key')