diff --git a/ldclient/flag.py b/ldclient/flag.py index 4c279f93..ed2583ce 100644 --- a/ldclient/flag.py +++ b/ldclient/flag.py @@ -172,9 +172,11 @@ def _get_off_value(flag, reason): def _get_value_for_variation_or_rollout(flag, vr, user, reason): - index = _variation_index_for_user(flag, vr, user) + index, inExperiment = _variation_index_for_user(flag, vr, user) if index is None: return EvaluationDetail(None, None, error_reason('MALFORMED_FLAG')) + if inExperiment: + reason['inExperiment'] = inExperiment return _get_variation(flag, index, reason) @@ -191,34 +193,38 @@ def _get_user_attribute(user, attr): def _variation_index_for_user(feature, rule, user): if rule.get('variation') is not None: - return rule['variation'] + return (rule['variation'], False) rollout = rule.get('rollout') if rollout is None: - return None + return (None, False) variations = rollout.get('variations') + seed = rollout.get('seed') if variations is not None and len(variations) > 0: bucket_by = 'key' if rollout.get('bucketBy') is not None: bucket_by = rollout['bucketBy'] - bucket = _bucket_user(user, feature['key'], feature['salt'], bucket_by) + bucket = _bucket_user(seed, user, feature['key'], feature['salt'], bucket_by) + is_experiment = rollout.get('kind') == 'experiment' sum = 0.0 for wv in variations: sum += wv.get('weight', 0.0) / 100000.0 if bucket < sum: - return wv.get('variation') + is_experiment_partition = is_experiment and not wv.get('untracked') + return (wv.get('variation'), is_experiment_partition) # 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') + is_experiment_partition = is_experiment and not variations[-1].get('untracked') + return (variations[-1].get('variation'), is_experiment_partition) - return None + return (None, False) -def _bucket_user(user, key, salt, bucket_by): +def _bucket_user(seed, user, key, salt, bucket_by): u_value, should_pass = _get_user_attribute(user, bucket_by) bucket_by_value = _bucketable_string_value(u_value) @@ -228,7 +234,13 @@ def _bucket_user(user, key, salt, bucket_by): id_hash = u_value if user.get('secondary') is not None: id_hash = id_hash + '.' + user['secondary'] - hash_key = '%s.%s.%s' % (key, salt, id_hash) + + if seed is not None: + prefix = str(seed) + else: + prefix = '%s.%s' % (key, salt) + + hash_key = '%s.%s' % (prefix, id_hash) hash_val = int(hashlib.sha1(hash_key.encode('utf-8')).hexdigest()[:15], 16) result = hash_val / __LONG_SCALE__ return result @@ -294,7 +306,7 @@ def _segment_rule_matches_user(rule, user, segment_key, salt): # All of the clauses are met. See if the user buckets in bucket_by = 'key' if rule.get('bucketBy') is None else rule['bucketBy'] - bucket = _bucket_user(user, segment_key, salt, bucket_by) + bucket = _bucket_user(None, user, segment_key, salt, bucket_by) weight = rule['weight'] / 100000.0 return bucket < weight diff --git a/ldclient/impl/event_factory.py b/ldclient/impl/event_factory.py index 16f81ac7..062c9d02 100644 --- a/ldclient/impl/event_factory.py +++ b/ldclient/impl/event_factory.py @@ -106,6 +106,8 @@ def _user_to_context_kind(self, user): def _is_experiment(self, flag, reason): if reason is not None: + if reason.get('inExperiment'): + return True kind = reason['kind'] if kind == 'RULE_MATCH': index = reason['ruleIndex'] diff --git a/testing/test_event_factory.py b/testing/test_event_factory.py new file mode 100644 index 00000000..6b763e84 --- /dev/null +++ b/testing/test_event_factory.py @@ -0,0 +1,72 @@ +import pytest +from ldclient.flag import EvaluationDetail +from ldclient.impl.event_factory import _EventFactory + +_event_factory_default = _EventFactory(False) +_user = { 'key': 'x' } + +def make_basic_flag_with_rules(kind, should_track_events): + rule = { + 'rollout': { + 'variations': [ + { 'variation': 0, 'weight': 50000 }, + { 'variation': 1, 'weight': 50000 } + ] + } + } + if kind == 'rulematch': + rule.update({'trackEvents': should_track_events}) + + flag = { + 'key': 'feature', + 'on': True, + 'rules': [rule], + 'fallthrough': { 'variation': 0 }, + 'variations': [ False, True ], + 'salt': '' + } + if kind == 'fallthrough': + flag.update({'trackEventsFallthrough': should_track_events}) + return flag + +def test_fallthrough_track_event_false(): + flag = make_basic_flag_with_rules('fallthrough', False) + detail = EvaluationDetail('b', 1, {'kind': 'FALLTHROUGH'}) + + eval = _event_factory_default.new_eval_event(flag, _user, detail, 'b', None) + assert eval.get('trackEvents') is None + +def test_fallthrough_track_event_true(): + flag = make_basic_flag_with_rules('fallthrough', True) + detail = EvaluationDetail('b', 1, {'kind': 'FALLTHROUGH'}) + + eval = _event_factory_default.new_eval_event(flag, _user, detail, 'b', None) + assert eval['trackEvents'] == True + +def test_fallthrough_track_event_false_with_experiment(): + flag = make_basic_flag_with_rules('fallthrough', False) + detail = EvaluationDetail('b', 1, {'kind': 'FALLTHROUGH', 'inExperiment': True}) + + eval = _event_factory_default.new_eval_event(flag, _user, detail, 'b', None) + assert eval['trackEvents'] == True + +def test_rulematch_track_event_false(): + flag = make_basic_flag_with_rules('rulematch', False) + detail = EvaluationDetail('b', 1, {'kind': 'RULE_MATCH', 'ruleIndex': 0}) + + eval = _event_factory_default.new_eval_event(flag, _user, detail, 'b', None) + assert eval.get('trackEvents') is None + +def test_rulematch_track_event_true(): + flag = make_basic_flag_with_rules('rulematch', True) + detail = EvaluationDetail('b', 1, {'kind': 'RULE_MATCH', 'ruleIndex': 0}) + + eval = _event_factory_default.new_eval_event(flag, _user, detail, 'b', None) + assert eval['trackEvents'] == True + +def test_rulematch_track_event_false_with_experiment(): + flag = make_basic_flag_with_rules('rulematch', False) + detail = EvaluationDetail('b', 1, {'kind': 'RULE_MATCH', 'ruleIndex': 0, 'inExperiment': True}) + + eval = _event_factory_default.new_eval_event(flag, _user, detail, 'b', None) + assert eval['trackEvents'] == True diff --git a/testing/test_flag.py b/testing/test_flag.py index 6b50b55a..c0d61707 100644 --- a/testing/test_flag.py +++ b/testing/test_flag.py @@ -391,7 +391,7 @@ def test_variation_index_is_returned_for_bucket(): # 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) + bucket_value = math.trunc(_bucket_user(None, user, flag['key'], flag['salt'], 'key') * 100000) assert bucket_value > 0 and bucket_value < 100000 bad_variation_a = 0 @@ -407,14 +407,14 @@ def test_variation_index_is_returned_for_bucket(): } } result_variation = _variation_index_for_user(flag, rule, user) - assert result_variation == matched_variation + assert result_variation == (matched_variation, False) 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) + bucket_value = math.trunc(_bucket_user(None, user, flag['key'], flag['salt'], 'key') * 100000) rule = { 'rollout': { @@ -424,21 +424,35 @@ def test_last_bucket_is_used_if_bucket_value_equals_total_weight(): } } result_variation = _variation_index_for_user(flag, rule, user) - assert result_variation == 0 + assert result_variation == (0, False) def test_bucket_by_user_key(): user = { u'key': u'userKeyA' } - bucket = _bucket_user(user, 'hashKey', 'saltyA', 'key') + bucket = _bucket_user(None, user, 'hashKey', 'saltyA', 'key') assert bucket == pytest.approx(0.42157587) user = { u'key': u'userKeyB' } - bucket = _bucket_user(user, 'hashKey', 'saltyA', 'key') + bucket = _bucket_user(None, user, 'hashKey', 'saltyA', 'key') assert bucket == pytest.approx(0.6708485) user = { u'key': u'userKeyC' } - bucket = _bucket_user(user, 'hashKey', 'saltyA', 'key') + bucket = _bucket_user(None, user, 'hashKey', 'saltyA', 'key') assert bucket == pytest.approx(0.10343106) +def test_bucket_by_user_key_with_seed(): + seed = 61 + user = { u'key': u'userKeyA' } + point = _bucket_user(seed, user, 'hashKey', 'saltyA', 'key') + assert point == pytest.approx(0.09801207) + + user = { u'key': u'userKeyB' } + point = _bucket_user(seed, user, 'hashKey', 'saltyA', 'key') + assert point == pytest.approx(0.14483777) + + user = { u'key': u'userKeyC' } + point = _bucket_user(seed, user, 'hashKey', 'saltyA', 'key') + assert point == pytest.approx(0.9242641) + def test_bucket_by_int_attr(): user = { u'key': u'userKey', @@ -447,9 +461,9 @@ def test_bucket_by_int_attr(): u'stringAttr': u'33333' } } - bucket = _bucket_user(user, 'hashKey', 'saltyA', 'intAttr') + bucket = _bucket_user(None, user, 'hashKey', 'saltyA', 'intAttr') assert bucket == pytest.approx(0.54771423) - bucket2 = _bucket_user(user, 'hashKey', 'saltyA', 'stringAttr') + bucket2 = _bucket_user(None, user, 'hashKey', 'saltyA', 'stringAttr') assert bucket2 == bucket def test_bucket_by_float_attr_not_allowed(): @@ -459,5 +473,24 @@ def test_bucket_by_float_attr_not_allowed(): u'floatAttr': 33.5 } } - bucket = _bucket_user(user, 'hashKey', 'saltyA', 'floatAttr') + bucket = _bucket_user(None, user, 'hashKey', 'saltyA', 'floatAttr') assert bucket == 0.0 + +def test_seed_independent_of_salt_and_hashKey(): + seed = 61 + user = { u'key': u'userKeyA' } + point1 = _bucket_user(seed, user, 'hashKey', 'saltyA', 'key') + point2 = _bucket_user(seed, user, 'hashKey', 'saltyB', 'key') + point3 = _bucket_user(seed, user, 'hashKey2', 'saltyA', 'key') + + assert point1 == point2 + assert point2 == point3 + +def test_seed_changes_hash_evaluation(): + seed1 = 61 + user = { u'key': u'userKeyA' } + point1 = _bucket_user(seed1, user, 'hashKey', 'saltyA', 'key') + seed2 = 62 + point2 = _bucket_user(seed2, user, 'hashKey', 'saltyB', 'key') + + assert point1 != point2 \ No newline at end of file