prepare 5.6.1 release (#145)

Author: LaunchDarklyCI

lib/ldclient-rb/evaluation.rb

@@ -323,20 +323,28 @@ def clause_match_user_no_segments(clause, user)
     end
 
     def variation_index_for_user(flag, rule, user)
-      if !rule[:variation].nil? # fixed variation
-        return rule[:variation]
-      elsif !rule[:rollout].nil? # percentage rollout
+      variation = rule[:variation]
+      return variation if !variation.nil? # fixed variation
+      rollout = rule[:rollout]
+      return nil if rollout.nil?
+      variations = rollout[:variations]
+      if !variations.nil? && variations.length > 0 # percentage rollout
         rollout = rule[:rollout]
         bucket_by = rollout[:bucketBy].nil? ? "key" : rollout[:bucketBy]
         bucket = bucket_user(user, flag[:key], bucket_by, flag[:salt])
         sum = 0;
-        rollout[:variations].each do |variate|
+        variations.each do |variate|
           sum += variate[:weight].to_f / 100000.0
           if bucket < sum
             return variate[:variation]
           end
         end
-        nil
+        # 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.
+        variations[-1][:variation]
       else # the rule isn't well-formed
         nil
       end

spec/evaluation_spec.rb

@@ -560,6 +560,58 @@ def boolean_flag_with_clauses(clauses)
     end
   end
 
+  describe "variation_index_for_user" do
+    it "matches bucket" do
+      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 = (bucket_user(user, flag_key, "key", salt) * 100000).truncate()
+      expect(bucket_value).to be > 0
+      expect(bucket_value).to be < 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) }
+          ]
+        }
+      }
+      flag = { key: flag_key, salt: salt }
+
+      result_variation = variation_index_for_user(flag, rule, user)
+      expect(result_variation).to be matched_variation
+    end
+
+    it "uses last bucket if bucket value is equal to total weight" do
+      user = { key: "userkey" }
+      flag_key = "flagkey"
+      salt = "salt"
+
+      bucket_value = (bucket_user(user, flag_key, "key", salt) * 100000).truncate()
+
+      # We'll construct a list of variations that stops right at the target bucket value
+      rule = {
+        rollout: {
+          variations: [
+            { variation: 0, weight: bucket_value }
+          ]
+        }
+      }
+      flag = { key: flag_key, salt: salt }
+
+      result_variation = variation_index_for_user(flag, rule, user)
+      expect(result_variation).to be 0
+    end
+  end
+
   describe "bucket_user" do
     it "gets expected bucket values for specific keys" do
       user = { key: "userKeyA" }