fix: Fix an issue with rollouts with a missing attribute. (#153)

When a rollout had a seed, and the context being evaluated was missing
the attribute referenced by `bucketBy`, then the `null` atom would
attempt to be used as a binary.

When the attribute is `null`, then the bucket value should always be
`0`.

This doesn't affect the experimentation logic related to presence of the
context itself.

Fixes: #152

Author: kinyoklion

src/ldclient_rollout.erl

@@ -147,7 +147,8 @@ match_weighted_variations(Bucket, [#{weight := Weight}|Rest], Sum) ->
     FlagKey :: ldclient_flag:key(),
     Salt :: binary(),
     ContextValue :: binary() | null) -> float().
-bucket_context_value(null, _FlagKey, _Salt, null) -> 0.0;
+
+bucket_context_value(_Seed, _FlagKey, _Salt, null) -> 0.0;
 %% when no seed is present hash with `key.salt.attribute`
 bucket_context_value(null, FlagKey, Salt, ContextValue) ->
     bucket_hash(<<FlagKey/binary, $., Salt/binary, $., ContextValue/binary>>);

test/ldclient_rollout_SUITE.erl

@@ -15,7 +15,12 @@
     can_rollout_experiment/1,
     untracked_variation_for_experiment_rollout_is_not_in_experiment/1,
     missing_kind_is_not_in_experiment/1,
-    experiment_rollouts_do_not_use_bucket_by/1
+    experiment_rollouts_do_not_use_bucket_by/1,
+    missing_bucket_bucket_by_does_not_error_with_seed/1,
+    bucket_by_null_attribute_does_not_error_with_seed/1,
+    missing_bucket_bucket_by_does_not_error_without_seed/1,
+    bucket_by_null_attribute_does_not_error_without_seed/1,
+    missing_kind_is_not_in_experiment_with_seed/1
 ]).
 
 %%====================================================================
@@ -28,7 +33,12 @@ all() ->
         can_rollout_experiment,
         untracked_variation_for_experiment_rollout_is_not_in_experiment,
         missing_kind_is_not_in_experiment,
-        experiment_rollouts_do_not_use_bucket_by
+        experiment_rollouts_do_not_use_bucket_by,
+        missing_bucket_bucket_by_does_not_error_with_seed,
+        bucket_by_null_attribute_does_not_error_with_seed,
+        missing_bucket_bucket_by_does_not_error_without_seed,
+        bucket_by_null_attribute_does_not_error_without_seed,
+        missing_kind_is_not_in_experiment_with_seed
     ].
 
 init_per_suite(Config) ->
@@ -87,6 +97,17 @@ missing_kind_is_not_in_experiment(_) ->
         #{key => <<"flagKey">>, salt => <<"flagSalt">>},
         ldclient_context:new(<<"user-key">>)).
 
+missing_kind_is_not_in_experiment_with_seed(_) ->
+    {0, false} = ldclient_rollout:rollout_context(
+        ldclient_rollout:new(#{
+            <<"kind">> => <<"experiment">>,
+            <<"contextKind">> => <<"org">>,
+            <<"seed">> => 1234567890,
+            <<"variations">> => [#{<<"variation">> => 0, <<"weight">> => 100000, <<"untracked">> => false}]
+        }),
+        #{key => <<"flagKey">>, salt => <<"flagSalt">>},
+        ldclient_context:new(<<"user-key">>)).
+
 experiment_rollouts_do_not_use_bucket_by(_) ->
     {1, true} = ldclient_rollout:rollout_context(
         ldclient_rollout:new(#{
@@ -116,3 +137,61 @@ experiment_rollouts_do_not_use_bucket_by(_) ->
         }),
         #{key => <<"flagKey">>, salt => <<"flagSalt">>},
         ldclient_context:set(<<"decoy">>, <<"valueZZZ">>, ldclient_context:new(<<"org-key">>, <<"org">>))).
+
+missing_bucket_bucket_by_does_not_error_with_seed(_) ->
+    {0, false} = ldclient_rollout:rollout_context(
+        ldclient_rollout:new(#{
+            <<"kind">> => <<"rollout">>,
+            <<"contextKind">> => <<"user">>,
+            <<"bucketBy">> => <<"missing">>,
+            <<"seed">> => 1234567890,
+            <<"variations">> => [
+                #{<<"variation">> => 0, <<"weight">> => 50000, <<"untracked">> => false},
+                #{<<"variation">> => 1, <<"weight">> => 50000, <<"untracked">> => false}
+            ]
+        }),
+        #{key => <<"flagKey">>, salt => <<"flagSalt">>},
+        ldclient_context:new(<<"user-key">>)).
+
+bucket_by_null_attribute_does_not_error_with_seed(_) ->
+    {0, false} = ldclient_rollout:rollout_context(
+        ldclient_rollout:new(#{
+            <<"kind">> => <<"rollout">>,
+            <<"contextKind">> => <<"user">>,
+            <<"bucketBy">> => <<"null">>,
+            <<"seed">> => 1234567890,
+            <<"variations">> => [
+                #{<<"variation">> => 0, <<"weight">> => 50000, <<"untracked">> => false},
+                #{<<"variation">> => 1, <<"weight">> => 50000, <<"untracked">> => false}
+            ]
+        }),
+        #{key => <<"flagKey">>, salt => <<"flagSalt">>},
+        ldclient_context:set(<<"null">>, null, ldclient_context:new(<<"user-key">>))).
+
+missing_bucket_bucket_by_does_not_error_without_seed(_) ->
+    {0, false} = ldclient_rollout:rollout_context(
+        ldclient_rollout:new(#{
+            <<"kind">> => <<"rollout">>,
+            <<"contextKind">> => <<"user">>,
+            <<"bucketBy">> => <<"missing">>,
+            <<"variations">> => [
+                #{<<"variation">> => 0, <<"weight">> => 50000, <<"untracked">> => false},
+                #{<<"variation">> => 1, <<"weight">> => 50000, <<"untracked">> => false}
+            ]
+        }),
+        #{key => <<"flagKey">>, salt => <<"flagSalt">>},
+        ldclient_context:new(<<"user-key">>)).
+
+bucket_by_null_attribute_does_not_error_without_seed(_) ->
+    {0, false} = ldclient_rollout:rollout_context(
+        ldclient_rollout:new(#{
+            <<"kind">> => <<"rollout">>,
+            <<"contextKind">> => <<"user">>,
+            <<"bucketBy">> => <<"null">>,
+            <<"variations">> => [
+                #{<<"variation">> => 0, <<"weight">> => 50000, <<"untracked">> => false},
+                #{<<"variation">> => 1, <<"weight">> => 50000, <<"untracked">> => false}
+            ]
+        }),
+        #{key => <<"flagKey">>, salt => <<"flagSalt">>},
+        ldclient_context:set(<<"null">>, null, ldclient_context:new(<<"user-key">>))).