PERF: Preload voters_count and has_voted (#28808)
These fields are often used when serializing topics which may contain multiple polls. On average, serializing a poll took 2+N queries where N is the number of options. This change reduces the number of queries to 3, one for each field (Poll#voters_count, PollOption#voters_count and Poll#has_voted?).
This commit is contained in:
parent
f2059bf15f
commit
9b0300a647
|
@ -23,6 +23,16 @@ class Poll < ActiveRecord::Base
|
|||
validates :max, numericality: { allow_nil: true, only_integer: true, greater_than: 0 }
|
||||
validates :step, numericality: { allow_nil: true, only_integer: true, greater_than: 0 }
|
||||
|
||||
attr_writer :voters_count
|
||||
attr_accessor :has_voted
|
||||
|
||||
after_initialize { @has_voted = {} }
|
||||
|
||||
def reload
|
||||
@has_voted = {}
|
||||
super
|
||||
end
|
||||
|
||||
def is_closed?
|
||||
closed? || (close_at && close_at <= Time.zone.now)
|
||||
end
|
||||
|
@ -37,7 +47,17 @@ class Poll < ActiveRecord::Base
|
|||
end
|
||||
|
||||
def has_voted?(user)
|
||||
user&.id && poll_votes.where(user_id: user.id).exists?
|
||||
if user&.id
|
||||
return @has_voted[user.id] if @has_voted.key?(user.id)
|
||||
|
||||
@has_voted[user.id] = poll_votes.where(user_id: user.id).exists?
|
||||
end
|
||||
end
|
||||
|
||||
def voters_count
|
||||
return @voters_count if defined?(@voters_count)
|
||||
|
||||
@voters_count = poll_votes.count("DISTINCT user_id")
|
||||
end
|
||||
|
||||
def can_see_voters?(user)
|
||||
|
@ -47,6 +67,36 @@ class Poll < ActiveRecord::Base
|
|||
def ranked_choice?
|
||||
type == "ranked_choice"
|
||||
end
|
||||
|
||||
def self.preload!(polls, user_id: nil)
|
||||
poll_ids = polls.map(&:id)
|
||||
|
||||
voters_count =
|
||||
PollVote
|
||||
.where(poll_id: poll_ids)
|
||||
.group(:poll_id)
|
||||
.pluck(:poll_id, "COUNT(DISTINCT user_id)")
|
||||
.to_h
|
||||
|
||||
option_voters_count =
|
||||
PollVote
|
||||
.where(poll_option_id: PollOption.where(poll_id: poll_ids).select(:id))
|
||||
.group(:poll_option_id)
|
||||
.pluck(:poll_option_id, "COUNT(*)")
|
||||
.to_h
|
||||
|
||||
polls.each do |poll|
|
||||
poll.voters_count = voters_count[poll.id] || 0
|
||||
poll.poll_options.each do |poll_option|
|
||||
poll_option.voters_count = option_voters_count[poll_option.id] || 0
|
||||
end
|
||||
end
|
||||
|
||||
if user_id
|
||||
has_voted = PollVote.where(poll_id: poll_ids, user_id: user_id).pluck(:poll_id).to_set
|
||||
polls.each { |poll| poll.has_voted[user_id] = has_voted.include?(poll.id) }
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
# == Schema Information
|
||||
|
|
|
@ -3,6 +3,14 @@
|
|||
class PollOption < ActiveRecord::Base
|
||||
belongs_to :poll
|
||||
has_many :poll_votes, dependent: :delete_all
|
||||
|
||||
attr_writer :voters_count
|
||||
|
||||
def voters_count
|
||||
return @voters_count if defined?(@voters_count)
|
||||
|
||||
@voters_count = poll_votes.count
|
||||
end
|
||||
end
|
||||
|
||||
# == Schema Information
|
||||
|
|
|
@ -8,8 +8,7 @@ class PollOptionSerializer < ApplicationSerializer
|
|||
end
|
||||
|
||||
def votes
|
||||
# `size` instead of `count` to prevent N+1
|
||||
object.poll_votes.size + object.anonymous_votes.to_i
|
||||
object.voters_count + object.anonymous_votes.to_i
|
||||
end
|
||||
|
||||
def include_votes?
|
||||
|
|
|
@ -58,7 +58,7 @@ class PollSerializer < ApplicationSerializer
|
|||
end
|
||||
|
||||
def voters
|
||||
object.poll_votes.count("DISTINCT user_id") + object.anonymous_voters.to_i
|
||||
object.voters_count + object.anonymous_voters.to_i
|
||||
end
|
||||
|
||||
def close
|
||||
|
|
|
@ -182,12 +182,12 @@ after_initialize do
|
|||
end
|
||||
|
||||
if post_with_polls.present?
|
||||
Poll
|
||||
.where(post_id: post_with_polls)
|
||||
.each do |p|
|
||||
polls[p.post_id] ||= []
|
||||
polls[p.post_id] << p
|
||||
end
|
||||
all_polls = Poll.includes(:poll_options).where(post_id: post_with_polls)
|
||||
Poll.preload!(all_polls, user_id: @user&.id)
|
||||
all_polls.each do |p|
|
||||
polls[p.post_id] ||= []
|
||||
polls[p.post_id] << p
|
||||
end
|
||||
end
|
||||
|
||||
polls
|
||||
|
|
|
@ -34,7 +34,7 @@ RSpec.describe ::DiscoursePoll::Poll do
|
|||
|
||||
expect(poll.can_see_results?(user)).to eq(false)
|
||||
poll.poll_votes.create!(poll_option_id: option.id, user_id: user.id)
|
||||
expect(poll.can_see_results?(user)).to eq(true)
|
||||
expect(poll.reload.can_see_results?(user)).to eq(true)
|
||||
end
|
||||
|
||||
it "author can see results when results setting is on_vote" do
|
||||
|
|
|
@ -0,0 +1,50 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
RSpec.describe PostsController do
|
||||
fab!(:admin) { Fabricate(:admin) }
|
||||
fab!(:topic) { Fabricate(:topic, user: admin) }
|
||||
|
||||
fab!(:post1) { Fabricate(:post, topic:, raw: "[poll]\n- A\n- B\n[/poll]") }
|
||||
fab!(:post2) { Fabricate(:post, topic:, raw: "[poll results=on_vote]\n- A\n- B\n[/poll]") }
|
||||
fab!(:post3) { Fabricate(:post, topic:, raw: "[poll results=on_vote]\n- A\n- B\n[/poll]") }
|
||||
fab!(:post4) { Fabricate(:post, topic:, raw: "[poll results=on_vote]\n- A\n- B\n[/poll]") }
|
||||
fab!(:post5) { Fabricate(:post, topic:, raw: "[poll results=staff_only]\n- A\n- B\n[/poll]") }
|
||||
fab!(:post6) { Fabricate(:post, topic:, raw: "[poll results=staff_only]\n- A\n- B\n[/poll]") }
|
||||
fab!(:post7) { Fabricate(:post, topic:, raw: "[poll visibility=]\n- A\n- B\n[/poll]") }
|
||||
|
||||
describe "#show" do
|
||||
context "when not logged in" do
|
||||
it "does not create N+1 queries to load polls" do
|
||||
queries = track_sql_queries { get "/t/#{topic.id}.json" }
|
||||
|
||||
expect(response.status).to eq(200)
|
||||
|
||||
poll_queries = queries.filter { |q| q =~ /FROM "?poll/ }
|
||||
# Expected queries:
|
||||
#
|
||||
# - load all polls
|
||||
# - load all options
|
||||
# - count votes for each poll
|
||||
# - count votes for each option
|
||||
expect(poll_queries.size).to eq(4)
|
||||
end
|
||||
end
|
||||
|
||||
context "when logged in" do
|
||||
before { sign_in(admin) }
|
||||
|
||||
it "does not create N+1 queries to load polls" do
|
||||
queries = track_sql_queries { get "/t/#{topic.id}.json" }
|
||||
|
||||
poll_queries = queries.filter { |q| q =~ /FROM "?poll/ }
|
||||
|
||||
# Expected queries:
|
||||
#
|
||||
# - all queries listed for "when not logged in"
|
||||
# - 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(5)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
Loading…
Reference in New Issue