Fix n+1 queries in StatusThreadingConcern (#7321)

diff --git a/app/lib/status_filter.rb b/app/lib/status_filter.rb
index 41d4381e56cdde00dd59318de2ecf932c703eab2..b6c80b801c374734e6c38218078417ed546b4c53 100644 (file)
--- a/app/lib/status_filter.rb
+++ b/app/lib/status_filter.rb
@@ -3,9 +3,10 @@
 class StatusFilter
   attr_reader :status, :account
 
-  def initialize(status, account)
-    @status = status
-    @account = account
+  def initialize(status, account, preloaded_relations = {})
+    @status              = status
+    @account             = account
+    @preloaded_relations = preloaded_relations
   end
 
   def filtered?
@@ -24,15 +25,15 @@ class StatusFilter
   end
 
   def blocking_account?
-    account.blocking? status.account_id
+    @preloaded_relations[:blocking] ? @preloaded_relations[:blocking][status.account_id] : account.blocking?(status.account_id)
   end
 
   def blocking_domain?
-    account.domain_blocking? status.account_domain
+    @preloaded_relations[:domain_blocking_by_domain] ? @preloaded_relations[:domain_blocking_by_domain][status.account_domain] : account.domain_blocking?(status.account_domain)
   end
 
   def muting_account?
-    account.muting? status.account_id
+    @preloaded_relations[:muting] ? @preloaded_relations[:muting][status.account_id] : account.muting?(status.account_id)
   end
 
   def silenced_account?
@@ -44,7 +45,7 @@ class StatusFilter
   end
 
   def account_following_status_account?
-    account&.following? status.account_id
+    @preloaded_relations[:following] ? @preloaded_relations[:following][status.account_id] : account&.following?(status.account_id)
   end
 
   def blocked_by_policy?
@@ -52,6 +53,6 @@ class StatusFilter
   end
 
   def policy_allows_show?
-    StatusPolicy.new(account, status).show?
+    StatusPolicy.new(account, status, @preloaded_relations).show?
   end
 end

diff --git a/app/models/concerns/account_interactions.rb b/app/models/concerns/account_interactions.rb
index 2dbd2590df6e56f47f4a7957781a0723cd660052..4a01eed654ea37786cf86ed751fe4adab1de1136 100644 (file)
--- a/app/models/concerns/account_interactions.rb
+++ b/app/models/concerns/account_interactions.rb
@@ -20,6 +20,10 @@ module AccountInteractions
       follow_mapping(Block.where(target_account_id: target_account_ids, account_id: account_id), :target_account_id)
     end
 
+    def blocked_by_map(target_account_ids, account_id)
+      follow_mapping(Block.where(account_id: target_account_ids, target_account_id: account_id), :account_id)
+    end
+
     def muting_map(target_account_ids, account_id)
       Mute.where(target_account_id: target_account_ids, account_id: account_id).each_with_object({}) do |mute, mapping|
         mapping[mute.target_account_id] = {
@@ -38,8 +42,12 @@ module AccountInteractions
 
     def domain_blocking_map(target_account_ids, account_id)
       accounts_map    = Account.where(id: target_account_ids).select('id, domain').map { |a| [a.id, a.domain] }.to_h
-      blocked_domains = AccountDomainBlock.where(account_id: account_id, domain: accounts_map.values).pluck(:domain)
-      accounts_map.map { |id, domain| [id, blocked_domains.include?(domain)] }.to_h
+      blocked_domains = domain_blocking_map_by_domain(accounts_map.values.compact, account_id)
+      accounts_map.map { |id, domain| [id, blocked_domains[domain]] }.to_h
+    end
+
+    def domain_blocking_map_by_domain(target_domains, account_id)
+      follow_mapping(AccountDomainBlock.where(account_id: account_id, domain: target_domains), :domain)
     end
 
     private

diff --git a/app/models/concerns/status_threading_concern.rb b/app/models/concerns/status_threading_concern.rb
index a3fd34e94d75ef121b35c06b2f5e73b3d85213a2..8e817be00c4579a22f0b38c55881e0348b895b84 100644 (file)
--- a/app/models/concerns/status_threading_concern.rb
+++ b/app/models/concerns/status_threading_concern.rb
@@ -71,10 +71,21 @@ module StatusThreadingConcern
   end
 
   def find_statuses_from_tree_path(ids, account)
-    statuses = statuses_with_accounts(ids).to_a
+    statuses    = statuses_with_accounts(ids).to_a
+    account_ids = statuses.map(&:account_id).uniq
+    domains     = statuses.map(&:account_domain).compact.uniq
 
-    # FIXME: n+1 bonanza
-    statuses.reject! { |status| filter_from_context?(status, account) }
+    relations = if account.present?
+                  {
+                    blocking: Account.blocking_map(account_ids, account.id),
+                    blocked_by: Account.blocked_by_map(account_ids, account.id),
+                    muting: Account.muting_map(account_ids, account.id),
+                    following: Account.following_map(account_ids, account.id),
+                    domain_blocking_by_domain: Account.domain_blocking_map_by_domain(domains, account.id),
+                  }
+                end
+
+    statuses.reject! { |status| filter_from_context?(status, account, relations) }
 
     # Order ancestors/descendants by tree path
     statuses.sort_by! { |status| ids.index(status.id) }
@@ -84,7 +95,7 @@ module StatusThreadingConcern
     Status.where(id: ids).includes(:account)
   end
 
-  def filter_from_context?(status, account)
-    StatusFilter.new(status, account).filtered?
+  def filter_from_context?(status, account, relations)
+    StatusFilter.new(status, account, relations).filtered?
   end
 end

diff --git a/app/policies/status_policy.rb b/app/policies/status_policy.rb
index 4145d7e9c513097a3d66d16878a02be677c3a5d8..6addc8a8a8ebe770dca60cb56ab784101ec8dcdb 100644 (file)
--- a/app/policies/status_policy.rb
+++ b/app/policies/status_policy.rb
@@ -1,26 +1,32 @@
 # frozen_string_literal: true
 
 class StatusPolicy < ApplicationPolicy
+  def initialize(current_account, record, preloaded_relations = {})
+    super(current_account, record)
+
+    @preloaded_relations = preloaded_relations
+  end
+
   def index?
     staff?
   end
 
   def show?
     if direct?
-      owned? || record.mentions.where(account: current_account).exists?
+      owned? || mention_exists?
     elsif private?
-      owned? || current_account&.following?(author) || record.mentions.where(account: current_account).exists?
+      owned? || following_author? || mention_exists?
     else
-      current_account.nil? || !author.blocking?(current_account)
+      current_account.nil? || !author_blocking?
     end
   end
 
   def reblog?
-    !direct? && (!private? || owned?) && show? && !current_account&.blocking?(author)
+    !direct? && (!private? || owned?) && show? && !blocking_author?
   end
 
   def favourite?
-    show? && !current_account&.blocking?(author)
+    show? && !blocking_author?
   end
 
   def destroy?
@@ -47,6 +53,34 @@ class StatusPolicy < ApplicationPolicy
     record.private_visibility?
   end
 
+  def mention_exists?
+    return false if current_account.nil?
+
+    if record.mentions.loaded?
+      record.mentions.any? { |mention| mention.account_id == current_account.id }
+    else
+      record.mentions.where(account: current_account).exists?
+    end
+  end
+
+  def blocking_author?
+    return false if current_account.nil?
+
+    @preloaded_relations[:blocking] ? @preloaded_relations[:blocking][author.id] : current_account.blocking?(author)
+  end
+
+  def author_blocking?
+    return false if current_account.nil?
+
+    @preloaded_relations[:blocked_by] ? @preloaded_relations[:blocked_by][author.id] : author.blocking?(current_account)
+  end
+
+  def following_author?
+    return false if current_account.nil?
+
+    @preloaded_relations[:following] ? @preloaded_relations[:following][author.id] : current_account.following?(author)
+  end
+
   def author
     record.account
   end