From de1f3aab868f8d18198438c405d0852c58f34e10 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sun, 16 Oct 2016 18:57:54 +0200 Subject: [PATCH] Fix #16 - Optimize n+1 queries when checking reblogged/favourited values for status lists in API --- app/controllers/api/v1/accounts_controller.rb | 3 ++- app/controllers/api/v1/statuses_controller.rb | 4 ++++ app/controllers/api_controller.rb | 6 ++++++ app/models/account.rb | 2 ++ app/models/follow_suggestion.rb | 2 +- app/models/status.rb | 6 +++--- app/views/api/v1/accounts/show.rabl | 6 +++--- app/views/api/v1/statuses/show.rabl | 4 ++-- 8 files changed, 23 insertions(+), 10 deletions(-) diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index 7757fd7f8a..2669315e20 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -25,6 +25,7 @@ class Api::V1::AccountsController < ApiController def statuses @statuses = @account.statuses.with_includes.with_counters.paginate_by_max_id(20, params[:max_id], params[:since_id]).to_a + set_maps(@statuses) end def follow @@ -53,7 +54,7 @@ class Api::V1::AccountsController < ApiController def relationships ids = params[:id].is_a?(Enumerable) ? params[:id].map(&:to_i) : [params[:id].to_i] - @accounts = Account.find(ids) + @accounts = Account.where(id: ids).select('id') @following = Account.following_map(ids, current_user.account_id) @followed_by = Account.followed_by_map(ids, current_user.account_id) @blocking = Account.blocking_map(ids, current_user.account_id) diff --git a/app/controllers/api/v1/statuses_controller.rb b/app/controllers/api/v1/statuses_controller.rb index 64dc3f84eb..a7305233ea 100644 --- a/app/controllers/api/v1/statuses_controller.rb +++ b/app/controllers/api/v1/statuses_controller.rb @@ -10,6 +10,7 @@ class Api::V1::StatusesController < ApiController @status = Status.find(params[:id]) @ancestors = @status.ancestors @descendants = @status.descendants + set_maps([@status] + @ancestors + @descendants) end def create @@ -46,16 +47,19 @@ class Api::V1::StatusesController < ApiController def home @statuses = Feed.new(:home, current_user.account).get(20, params[:max_id], params[:since_id]).to_a + set_maps(@statuses) render action: :index end def mentions @statuses = Feed.new(:mentions, current_user.account).get(20, params[:max_id], params[:since_id]).to_a + set_maps(@statuses) render action: :index end def public @statuses = Status.as_public_timeline(current_user.account).paginate_by_max_id(20, params[:max_id], params[:since_id]).to_a + set_maps(@statuses) render action: :index end end diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index 4ccf20bc97..c4ad450cf9 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -35,4 +35,10 @@ class ApiController < ApplicationController def render_empty render json: {}, status: 200 end + + def set_maps(statuses) + status_ids = statuses.flat_map { |s| [s.id, s.reblog_of_id] }.compact + @reblogs_map = Status.reblogs_map(status_ids, current_user.account) + @favourites_map = Status.favourites_map(status_ids, current_user.account) + end end diff --git a/app/models/account.rb b/app/models/account.rb index c1b52ea1ba..8eba4da79f 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -48,6 +48,8 @@ class Account < ApplicationRecord scope :with_followers, -> { where('(select count(f.id) from follows as f where f.target_account_id = accounts.id) > 0') } scope :expiring, -> (time) { where(subscription_expires_at: nil).or(where('subscription_expires_at < ?', time)).remote.with_followers } + scope :with_counters, -> { select('accounts.*, (select count(f.id) from follows as f where f.target_account_id = accounts.id) as followers_count, (select count(f.id) from follows as f where f.account_id = accounts.id) as following_count, (select count(s.id) from statuses as s where s.account_id = accounts.id) as statuses_count') } + def follow!(other_account) active_relationships.where(target_account: other_account).first_or_create!(target_account: other_account) end diff --git a/app/models/follow_suggestion.rb b/app/models/follow_suggestion.rb index 25d28f5ac3..dbe86a0e43 100644 --- a/app/models/follow_suggestion.rb +++ b/app/models/follow_suggestion.rb @@ -22,7 +22,7 @@ END account_ids = results['data'].map(&:first) blocked_ids = Block.where(account_id: for_account_id).pluck(:target_account_id) - accounts_map = Account.where(id: account_ids - blocked_ids).map { |a| [a.id, a] }.to_h + accounts_map = Account.where(id: account_ids - blocked_ids).with_counters.map { |a| [a.id, a] }.to_h account_ids.map { |id| accounts_map[id] }.compact rescue Neography::NeographyError, Excon::Error::Socket => e diff --git a/app/models/status.rb b/app/models/status.rb index 3f150c5dea..58b2cb1f35 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -2,7 +2,7 @@ class Status < ApplicationRecord include Paginable include Streamable - belongs_to :account, inverse_of: :statuses + belongs_to :account, -> { with_counters }, inverse_of: :statuses belongs_to :thread, foreign_key: 'in_reply_to_id', class_name: 'Status', inverse_of: :replies belongs_to :reblog, foreign_key: 'reblog_of_id', class_name: 'Status', inverse_of: :reblogs @@ -90,11 +90,11 @@ class Status < ApplicationRecord end def self.favourites_map(status_ids, account_id) - Favourite.where(status_id: status_ids).where(account_id: account_id).map { |f| [f.status_id, true] }.to_h + Favourite.select('status_id').where(status_id: status_ids).where(account_id: account_id).map { |f| [f.status_id, true] }.to_h end def self.reblogs_map(status_ids, account_id) - where(reblog_of_id: status_ids).where(account_id: account_id).map { |s| [s.reblog_of_id, true] }.to_h + select('reblog_of_id').where(reblog_of_id: status_ids).where(account_id: account_id).map { |s| [s.reblog_of_id, true] }.to_h end before_validation do diff --git a/app/views/api/v1/accounts/show.rabl b/app/views/api/v1/accounts/show.rabl index 4f6a3ff998..0d9e268a19 100644 --- a/app/views/api/v1/accounts/show.rabl +++ b/app/views/api/v1/accounts/show.rabl @@ -5,6 +5,6 @@ attributes :id, :username, :acct, :display_name, :note node(:url) { |account| TagManager.instance.url_for(account) } node(:avatar) { |account| full_asset_url(account.avatar.url(:large, false)) } node(:header) { |account| full_asset_url(account.header.url(:medium, false)) } -node(:followers_count) { |account| account.followers.count } -node(:following_count) { |account| account.following.count } -node(:statuses_count) { |account| account.statuses.count } +node(:followers_count) { |account| account.try(:followers_count) || account.followers.count } +node(:following_count) { |account| account.try(:following_count) || account.following.count } +node(:statuses_count) { |account| account.try(:statuses_count) || account.statuses.count } diff --git a/app/views/api/v1/statuses/show.rabl b/app/views/api/v1/statuses/show.rabl index 20cb65e298..69dcd10584 100644 --- a/app/views/api/v1/statuses/show.rabl +++ b/app/views/api/v1/statuses/show.rabl @@ -6,8 +6,8 @@ node(:content) { |status| Formatter.instance.format(status) } node(:url) { |status| TagManager.instance.url_for(status) } node(:reblogs_count) { |status| status.reblogs_count } node(:favourites_count) { |status| status.favourites_count } -node(:favourited, if: proc { !current_account.nil? }) { |status| current_account.favourited?(status) } -node(:reblogged, if: proc { !current_account.nil? }) { |status| current_account.reblogged?(status) } +node(:favourited, if: proc { !current_account.nil? }) { |status| defined?(@favourites_map) ? !!@favourites_map[status.id] : current_account.favourited?(status) } +node(:reblogged, if: proc { !current_account.nil? }) { |status| defined?(@reblogs_map) ? !!@reblogs_map[status.id] : current_account.reblogged?(status) } child :reblog => :reblog do extends('api/v1/statuses/show')