PERF: Cache serialized voters at topic view level (#28894)

This commit introduces a way to fetch the "serialized voters" for
multiple polls.

* Use a single query to fetch voters for all types of polls

* Refactor to introduce all_serialized_voters

* Cache serialized voters

plugins/poll/app/models/poll.rb

@@ -25,6 +25,7 @@ class Poll < ActiveRecord::Base
 
   attr_writer :voters_count
   attr_accessor :has_voted
+  attr_accessor :serialized_voters_cache
 
   after_initialize { @has_voted = {} }
 

plugins/poll/lib/poll.rb

@@ -203,142 +203,83 @@ class DiscoursePoll::Poll
   end
 
   def self.serialized_voters(poll, opts = {})
-    limit = (opts["limit"] || 25).to_i
+    preload_serialized_voters!([poll], opts)[poll.id]
-    limit = 0 if limit < 0
+  end
-    limit = 50 if limit > 50
 
-    page = (opts["page"] || 1).to_i
+  def self.preload_serialized_voters!(polls, opts = {})
-    page = 1 if page < 1
+    # This method is used in order to avoid N+1s and preloads serialized voters
+    # for multiple polls from a topic view. After the first call, the serialized
+    # voters are cached in the Poll object and returned from there for future
+    # calls.
 
+    page = [1, (opts["page"] || 1).to_i].max
+    limit = (opts["limit"] || 25).to_i.clamp(1, 50)
     offset = (page - 1) * limit
 
-    option_digest = opts["option_id"].to_s
+    params = {
+      offset: offset,
+      offset_plus_limit: offset + limit,
+      option_digest: opts[:option_id].presence,
+    }
 
-    if poll.number?
+    result = {}
-      user_ids =
-        PollVote
-          .where(poll: poll)
-          .group(:user_id)
-          .order("MIN(created_at)")
-          .offset(offset)
-          .limit(limit)
-          .pluck(:user_id)
 
-      result = User.where(id: user_ids).map { |u| UserNameSerializer.new(u).serializable_hash }
+    uncached_poll_ids = []
-    elsif option_digest.present?
+    polls.each do |p|
-      poll_option = PollOption.find_by(poll: poll, digest: option_digest)
+      if p.serialized_voters_cache&.key?(params)
-
+        result[p.id] = p.serialized_voters_cache[params]
-      raise Discourse::InvalidParameters.new(:option_id) unless poll_option
-
-      if poll.ranked_choice?
-        params = {
-          poll_id: poll.id,
-          option_digest: option_digest,
-          offset: offset,
-          offset_plus_limit: offset + limit,
-        }
-
-        votes = DB.query(<<~SQL, params)
-        SELECT digest, rank, user_id
-          FROM (
-            SELECT digest
-                  , CASE rank WHEN 0 THEN 'Abstain' ELSE CAST(rank AS text) END AS rank
-                  , user_id
-                  , username
-                  , ROW_NUMBER() OVER (PARTITION BY poll_option_id ORDER BY pv.created_at) AS row
-              FROM poll_votes pv
-              JOIN poll_options po ON pv.poll_option_id = po.id
-              JOIN users u ON pv.user_id = u.id
-              WHERE pv.poll_id = :poll_id
-                AND po.poll_id = :poll_id
-                AND po.digest = :option_digest
-          ) v
-          WHERE row BETWEEN :offset AND :offset_plus_limit
-          ORDER BY digest, CASE WHEN rank = 'Abstain' THEN 1 ELSE CAST(rank AS integer) END, username
-        SQL
-
-        user_ids = votes.map(&:user_id).uniq
-
-        user_hashes =
-          User
-            .where(id: user_ids)
-            .map { |u| [u.id, UserNameSerializer.new(u).serializable_hash] }
-            .to_h
-
-        ranked_choice_users = []
-        votes.each do |v|
-          ranked_choice_users ||= []
-          ranked_choice_users << { rank: v.rank, user: user_hashes[v.user_id] }
-        end
-        user_hashes = ranked_choice_users
       else
-        user_ids =
+        uncached_poll_ids << p.id
-          PollVote
-            .where(poll: poll, poll_option: poll_option)
-            .group(:user_id)
-            .order("MIN(created_at)")
-            .offset(offset)
-            .limit(limit)
-            .pluck(:user_id)
-
-        user_hashes =
-          User.where(id: user_ids).map { |u| UserNameSerializer.new(u).serializable_hash }
       end
-      result = { option_digest => user_hashes }
+    end
-    else
+
-      params = { poll_id: poll.id, offset: offset, offset_plus_limit: offset + limit }
+    return result if uncached_poll_ids.empty?
-      if poll.ranked_choice?
+
-        votes = DB.query(<<~SQL, params)
+    where_clause = params[:option_digest] ? "AND po.digest = :option_digest" : ""
-        SELECT digest, rank, user_id
+    query = <<~SQL.gsub("/* where */", where_clause)
-          FROM (
+      SELECT poll_id, digest, rank, user_id
-            SELECT digest
+        FROM (
-                  , CASE rank WHEN 0 THEN 'Abstain' ELSE CAST(rank AS text) END AS rank
+          SELECT pv.poll_id
-                  , user_id
+               , digest
-                  , username
+               , CASE rank WHEN 0 THEN 'Abstain' ELSE CAST(rank AS text) END AS rank
-                  , ROW_NUMBER() OVER (PARTITION BY poll_option_id ORDER BY pv.created_at) AS row
+               , user_id
-              FROM poll_votes pv
+               , username
-              JOIN poll_options po ON pv.poll_option_id = po.id
+               , ROW_NUMBER() OVER (PARTITION BY poll_option_id ORDER BY pv.created_at) AS row
-              JOIN users u ON pv.user_id = u.id
+          FROM poll_votes pv
-              WHERE pv.poll_id = :poll_id
+          JOIN poll_options po ON pv.poll_id = po.poll_id AND pv.poll_option_id = po.id
-                AND po.poll_id = :poll_id
+          JOIN users u ON pv.user_id = u.id
-          ) v
+          WHERE pv.poll_id IN (:poll_ids)
-          WHERE row BETWEEN :offset AND :offset_plus_limit
+                /* where */
-          ORDER BY digest, CASE WHEN rank = 'Abstain' THEN 1 ELSE CAST(rank AS integer) END, username
+        ) v
-        SQL
+        WHERE row BETWEEN :offset AND :offset_plus_limit
+        ORDER BY digest, CASE WHEN rank = 'Abstain' THEN 1 ELSE CAST(rank AS integer) END, username
+      SQL
+
+    votes = DB.query(query, params.merge(poll_ids: uncached_poll_ids))
+
+    users =
+      User
+        .where(id: votes.map(&:user_id).uniq)
+        .map { |u| [u.id, UserNameSerializer.new(u).serializable_hash] }
+        .to_h
+
+    polls_by_id = polls.index_by(&:id)
+    votes.each do |v|
+      if polls_by_id[v.poll_id].number?
+        result[v.poll_id] ||= []
+        result[v.poll_id] << users[v.user_id]
+      elsif polls_by_id[v.poll_id].ranked_choice?
+        result[v.poll_id] ||= Hash.new { |h, k| h[k] = [] }
+        result[v.poll_id][v.digest] << { rank: v.rank, user: users[v.user_id] }
       else
-        votes = DB.query(<<~SQL, params)
+        result[v.poll_id] ||= Hash.new { |h, k| h[k] = [] }
-          SELECT digest, user_id
+        result[v.poll_id][v.digest] << users[v.user_id]
-            FROM (
-              SELECT digest
-                    , user_id
-                    , ROW_NUMBER() OVER (PARTITION BY poll_option_id ORDER BY pv.created_at) AS row
-                FROM poll_votes pv
-                JOIN poll_options po ON pv.poll_option_id = po.id
-                WHERE pv.poll_id = :poll_id
-                  AND po.poll_id = :poll_id
-            ) v
-            WHERE row BETWEEN :offset AND :offset_plus_limit
-        SQL
       end
+    end
 
-      user_ids = votes.map(&:user_id).uniq
+    polls.each do |p|
-
+      p.serialized_voters_cache ||= {}
-      user_hashes =
+      p.serialized_voters_cache[params] = result[p.id]
-        User
-          .where(id: user_ids)
-          .map { |u| [u.id, UserNameSerializer.new(u).serializable_hash] }
-          .to_h
-
-      result = {}
-      votes.each do |v|
-        if poll.ranked_choice?
-          result[v.digest] ||= []
-          result[v.digest] << { rank: v.rank, user: user_hashes[v.user_id] }
-        else
-          result[v.digest] ||= []
-          result[v.digest] << user_hashes[v.user_id]
-        end
-      end
     end
 
     result

plugins/poll/plugin.rb

@@ -184,6 +184,7 @@ after_initialize do
         if post_with_polls.present?
           all_polls = Poll.includes(:poll_options).where(post_id: post_with_polls)
           Poll.preload!(all_polls, user_id: @user&.id)
+          DiscoursePoll::Poll.preload_serialized_voters!(all_polls)
           all_polls.each do |p|
             polls[p.post_id] ||= []
             polls[p.post_id] << p

plugins/poll/spec/requests/topics_controller_spec.rb

@@ -45,13 +45,9 @@ RSpec.describe PostsController do
 
         # Expected queries:
         #
-        # - all queries listed for "when not logged in"
+        # - all queries listed for "when not logged in" (except it loads voters for polls in post5, post6 and post7)
         # - query to find out if the user has voted in each poll
-        # - queries to get "serialized voters" (NOT TRACKED)
+        expect(poll_queries.size).to eq(6)
-        # - voters for poll in post5
-        # - voters for poll in post6
-        # - voters for poll in post7
-        expect(poll_queries.size).to eq(8)
       end
     end
   end