Batch active_experiments winner lookups into one roundtrip

Author: andrehjr

lib/split/user.rb

@@ -55,10 +55,17 @@ def cleanup_old_versions!(experiment)
     end
 
     def active_experiments
+      experiments_by_key = keys_without_finished(user.keys).each_with_object({}) do |key, memo|
+        memo[key] = Metric.possible_experiments(key_without_version(key))
+      end
+
+      names = experiments_by_key.values.flatten.map(&:name).uniq
+      winners = names.empty? ? {} : names.zip(Split.redis.hmget(:experiment_winner, *names)).to_h
+
       experiment_pairs = {}
-      keys_without_finished(user.keys).each do |key|
-        Metric.possible_experiments(key_without_version(key)).each do |experiment|
-          if !experiment.has_winner?
+      experiments_by_key.each do |key, experiments|
+        experiments.each do |experiment|
+          unless winners[experiment.name]
             experiment_pairs[key_without_version(key)] = user[key]
           end
         end

spec/user_spec.rb

@@ -118,6 +118,27 @@
     end
   end
 
+  context "#active_experiments" do
+    let(:user_keys) { { "with_winner" => "red", "active" => "red" } }
+
+    before do
+      with_winner = Split::ExperimentCatalog.find_or_create("with_winner", "red", "blue")
+      with_winner.start
+      with_winner.winner = "red"
+
+      Split::ExperimentCatalog.find_or_create("active", "red", "blue").start
+    end
+
+    it "excludes experiments that already have a winner" do
+      expect(@subject.active_experiments).to eq("active" => "red")
+    end
+
+    it "fetches every experiment's winner in a single call" do
+      expect(Split.redis).to receive(:hmget).with(:experiment_winner, any_args).once.and_call_original
+      @subject.active_experiments
+    end
+  end
+
   context "allows user to be loaded from adapter" do
     it "loads user from adapter (RedisAdapter)" do
       user = Split::Persistence::RedisAdapter.new(nil, 112233)