Optimized n+1 queries in accounts Atom and HTML views
Added stack trace for SQL queries in development Removed badly thought out accounts/lookup API
This commit is contained in:
		
							
								
								
									
										1
									
								
								Gemfile
									
									
									
									
									
								
							
							
						
						
									
										1
									
								
								Gemfile
									
									
									
									
									
								
							@@ -60,6 +60,7 @@ group :development do
 | 
				
			|||||||
  gem 'binding_of_caller'
 | 
					  gem 'binding_of_caller'
 | 
				
			||||||
  gem 'letter_opener'
 | 
					  gem 'letter_opener'
 | 
				
			||||||
  gem 'bullet'
 | 
					  gem 'bullet'
 | 
				
			||||||
 | 
					  gem 'active_record_query_trace'
 | 
				
			||||||
end
 | 
					end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
group :production do
 | 
					group :production do
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -36,6 +36,7 @@ GEM
 | 
				
			|||||||
      erubis (~> 2.7.0)
 | 
					      erubis (~> 2.7.0)
 | 
				
			||||||
      rails-dom-testing (~> 2.0)
 | 
					      rails-dom-testing (~> 2.0)
 | 
				
			||||||
      rails-html-sanitizer (~> 1.0, >= 1.0.2)
 | 
					      rails-html-sanitizer (~> 1.0, >= 1.0.2)
 | 
				
			||||||
 | 
					    active_record_query_trace (1.5.3)
 | 
				
			||||||
    activejob (5.0.0.1)
 | 
					    activejob (5.0.0.1)
 | 
				
			||||||
      activesupport (= 5.0.0.1)
 | 
					      activesupport (= 5.0.0.1)
 | 
				
			||||||
      globalid (>= 0.3.6)
 | 
					      globalid (>= 0.3.6)
 | 
				
			||||||
@@ -360,6 +361,7 @@ PLATFORMS
 | 
				
			|||||||
  ruby
 | 
					  ruby
 | 
				
			||||||
 | 
					
 | 
				
			||||||
DEPENDENCIES
 | 
					DEPENDENCIES
 | 
				
			||||||
 | 
					  active_record_query_trace
 | 
				
			||||||
  addressable
 | 
					  addressable
 | 
				
			||||||
  better_errors
 | 
					  better_errors
 | 
				
			||||||
  binding_of_caller
 | 
					  binding_of_caller
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -6,14 +6,21 @@ class AccountsController < ApplicationController
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
  def show
 | 
					  def show
 | 
				
			||||||
    respond_to do |format|
 | 
					    respond_to do |format|
 | 
				
			||||||
      format.html { @statuses = @account.statuses.order('id desc').with_includes.with_counters.paginate(page: params[:page], per_page: 10)}
 | 
					      format.html do
 | 
				
			||||||
 | 
					        @statuses   = @account.statuses.order('id desc').with_includes.with_counters.paginate(page: params[:page], per_page: 10)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        if user_signed_in?
 | 
				
			||||||
 | 
					          status_ids  = @statuses.collect { |s| [s.id, s.reblog_of_id] }.flatten.uniq
 | 
				
			||||||
 | 
					          @favourited = Favourite.where(status_id: status_ids).where(account_id: current_user.account_id).map { |f| [f.status_id, true] }.to_h
 | 
				
			||||||
 | 
					          @reblogged  = Status.where(reblog_of_id: status_ids).where(account_id: current_user.account_id).map { |s| [s.reblog_of_id, true] }.to_h
 | 
				
			||||||
 | 
					        else
 | 
				
			||||||
 | 
					          @favourited = {}
 | 
				
			||||||
 | 
					          @reblogged  = {}
 | 
				
			||||||
 | 
					        end
 | 
				
			||||||
 | 
					      end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
      format.atom do
 | 
					      format.atom do
 | 
				
			||||||
        @entries = @account.stream_entries.order('id desc').with_includes.paginate_by_max_id(20, params[:max_id] || nil)
 | 
					        @entries = @account.stream_entries.order('id desc').with_includes.paginate_by_max_id(20, params[:max_id] || nil)
 | 
				
			||||||
 | 
					 | 
				
			||||||
        ActiveRecord::Associations::Preloader.new.preload(@entries.select { |a| a.activity_type == 'Status' }, activity: [:mentions, :media_attachments, reblog: :account, thread: :account])
 | 
					 | 
				
			||||||
        ActiveRecord::Associations::Preloader.new.preload(@entries.select { |a| a.activity_type == 'Favourite' }, activity: [:account, :status])
 | 
					 | 
				
			||||||
        ActiveRecord::Associations::Preloader.new.preload(@entries.select { |a| a.activity_type == 'Follow' }, activity: :target_account)
 | 
					 | 
				
			||||||
      end
 | 
					      end
 | 
				
			||||||
    end
 | 
					    end
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -1,14 +0,0 @@
 | 
				
			|||||||
class Api::Accounts::LookupController < ApiController
 | 
					 | 
				
			||||||
  before_action :doorkeeper_authorize!
 | 
					 | 
				
			||||||
  respond_to    :json
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
  def index
 | 
					 | 
				
			||||||
    @accounts = Account.where(domain: nil).where(username: lookup_params)
 | 
					 | 
				
			||||||
  end
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
  private
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
  def lookup_params
 | 
					 | 
				
			||||||
    (params[:usernames] || '').split(',').map(&:strip)
 | 
					 | 
				
			||||||
  end
 | 
					 | 
				
			||||||
end
 | 
					 | 
				
			||||||
@@ -1,2 +0,0 @@
 | 
				
			|||||||
module Api::Accounts::LookupHelper
 | 
					 | 
				
			||||||
end
 | 
					 | 
				
			||||||
@@ -22,23 +22,26 @@ module ApplicationHelper
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
  def account_from_mentions(search_string, mentions)
 | 
					  def account_from_mentions(search_string, mentions)
 | 
				
			||||||
    mentions.each { |x| return x.account if x.account.acct.eql?(search_string) }
 | 
					    mentions.each { |x| return x.account if x.account.acct.eql?(search_string) }
 | 
				
			||||||
 | 
					    nil
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    # If that was unsuccessful, try fetching user from db separately
 | 
					    # If that was unsuccessful, try fetching user from db separately
 | 
				
			||||||
    # But this shouldn't ever happen if the mentions were created correctly!
 | 
					    # But this shouldn't ever happen if the mentions were created correctly!
 | 
				
			||||||
    username, domain = search_string.split('@')
 | 
					    # username, domain = search_string.split('@')
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    if domain == Rails.configuration.x.local_domain
 | 
					    # if domain == Rails.configuration.x.local_domain
 | 
				
			||||||
      account = Account.find_local(username)
 | 
					    #   account = Account.find_local(username)
 | 
				
			||||||
    else
 | 
					    # else
 | 
				
			||||||
      account = Account.find_remote(username, domain)
 | 
					    #   account = Account.find_remote(username, domain)
 | 
				
			||||||
    end
 | 
					    # end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    account
 | 
					    # account
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def linkify(status)
 | 
					  def linkify(status)
 | 
				
			||||||
    auto_link(HTMLEntities.new.encode(status.text), link: :urls, html: { rel: 'nofollow noopener' }).gsub(Account::MENTION_RE) do |m|
 | 
					    auto_link(HTMLEntities.new.encode(status.text), link: :urls, html: { rel: 'nofollow noopener' }).gsub(Account::MENTION_RE) do |m|
 | 
				
			||||||
      if account = account_from_mentions(Account::MENTION_RE.match(m)[1], status.mentions)
 | 
					      account = account_from_mentions(Account::MENTION_RE.match(m)[1], status.mentions)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					      unless account.nil?
 | 
				
			||||||
        "#{m.split('@').first}<a href=\"#{url_for_target(account)}\" class=\"mention\">@<span>#{account.acct}</span></a>"
 | 
					        "#{m.split('@').first}<a href=\"#{url_for_target(account)}\" class=\"mention\">@<span>#{account.acct}</span></a>"
 | 
				
			||||||
      else
 | 
					      else
 | 
				
			||||||
        m
 | 
					        m
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -21,11 +21,11 @@ module StreamEntriesHelper
 | 
				
			|||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def reblogged_by_me_class(status)
 | 
					  def reblogged_by_me_class(status)
 | 
				
			||||||
    user_signed_in? && current_user.account.reblogged?(status) ? 'reblogged' : ''
 | 
					    user_signed_in? && @reblogged.has_key?(status.id) ? 'reblogged' : ''
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def favourited_by_me_class(status)
 | 
					  def favourited_by_me_class(status)
 | 
				
			||||||
    user_signed_in? && current_user.account.favourited?(status) ? 'favourited' : ''
 | 
					    user_signed_in? && @favourited.has_key?(status.id) ? 'favourited' : ''
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def proper_status(status)
 | 
					  def proper_status(status)
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -18,7 +18,7 @@ class Status < ApplicationRecord
 | 
				
			|||||||
  validates :text, presence: true, if: Proc.new { |s| s.local? && !s.reblog? }
 | 
					  validates :text, presence: true, if: Proc.new { |s| s.local? && !s.reblog? }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  scope :with_counters, -> { select('statuses.*, (select count(r.id) from statuses as r where r.reblog_of_id = statuses.id) as reblogs_count, (select count(f.id) from favourites as f where f.status_id = statuses.id) as favourites_count') }
 | 
					  scope :with_counters, -> { select('statuses.*, (select count(r.id) from statuses as r where r.reblog_of_id = statuses.id) as reblogs_count, (select count(f.id) from favourites as f where f.status_id = statuses.id) as favourites_count') }
 | 
				
			||||||
  scope :with_includes, -> { includes(:account, :mentions, :media_attachments, :stream_entry, reblog: [:account, :mentions], thread: :account) }
 | 
					  scope :with_includes, -> { includes(:account, :media_attachments, :stream_entry, mentions: :account, reblog: [:account, mentions: :account], thread: :account) }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def local?
 | 
					  def local?
 | 
				
			||||||
    self.uri.nil?
 | 
					    self.uri.nil?
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -4,9 +4,15 @@ class StreamEntry < ApplicationRecord
 | 
				
			|||||||
  belongs_to :account, inverse_of: :stream_entries
 | 
					  belongs_to :account, inverse_of: :stream_entries
 | 
				
			||||||
  belongs_to :activity, polymorphic: true
 | 
					  belongs_to :activity, polymorphic: true
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  belongs_to :status,    foreign_type: 'Status',    foreign_key: 'activity_id'
 | 
				
			||||||
 | 
					  belongs_to :follow,    foreign_type: 'Follow',    foreign_key: 'activity_id'
 | 
				
			||||||
 | 
					  belongs_to :favourite, foreign_type: 'Favourite', foreign_key: 'activity_id'
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  validates :account, :activity, presence: true
 | 
					  validates :account, :activity, presence: true
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  scope :with_includes, -> { includes(:activity) }
 | 
					  STATUS_INCLUDES = [:account, :stream_entry, :media_attachments, mentions: :account, reblog: [:stream_entry, :account, mentions: :account], thread: [:stream_entry, :account]]
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  scope :with_includes, -> { includes(:account, status: STATUS_INCLUDES, favourite: [:account, :stream_entry, status: STATUS_INCLUDES], follow: [:target_account, :stream_entry]) }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def object_type
 | 
					  def object_type
 | 
				
			||||||
    orphaned? ? :activity : (targeted? ? :activity : self.activity.object_type)
 | 
					    orphaned? ? :activity : (targeted? ? :activity : self.activity.object_type)
 | 
				
			||||||
@@ -44,6 +50,10 @@ class StreamEntry < ApplicationRecord
 | 
				
			|||||||
    self.activity.respond_to?(:mentions) ? self.activity.mentions.map { |x| x.account } : []
 | 
					    self.activity.respond_to?(:mentions) ? self.activity.mentions.map { |x| x.account } : []
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  def activity
 | 
				
			||||||
 | 
					    self.send(self.activity_type.downcase.to_sym)
 | 
				
			||||||
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  private
 | 
					  private
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def orphaned?
 | 
					  def orphaned?
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -59,9 +59,9 @@ Rails.application.configure do
 | 
				
			|||||||
  config.action_mailer.delivery_method = :letter_opener
 | 
					  config.action_mailer.delivery_method = :letter_opener
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  config.after_initialize do
 | 
					  config.after_initialize do
 | 
				
			||||||
    Bullet.enable = true
 | 
					    Bullet.enable        = true
 | 
				
			||||||
    Bullet.bullet_logger = true
 | 
					    Bullet.bullet_logger = true
 | 
				
			||||||
    Bullet.rails_logger = true
 | 
					    Bullet.rails_logger  = false
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    Bullet.add_whitelist type: :n_plus_one_query, class_name: 'User', association: :account
 | 
					    Bullet.add_whitelist type: :n_plus_one_query, class_name: 'User', association: :account
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
@@ -71,3 +71,5 @@ end
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
require 'sidekiq/testing'
 | 
					require 'sidekiq/testing'
 | 
				
			||||||
Sidekiq::Testing.inline!
 | 
					Sidekiq::Testing.inline!
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					ActiveRecordQueryTrace.enabled = true
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -56,10 +56,6 @@ Rails.application.routes.draw do
 | 
				
			|||||||
    resources :media,    only: [:create]
 | 
					    resources :media,    only: [:create]
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    resources :accounts, only: [:show] do
 | 
					    resources :accounts, only: [:show] do
 | 
				
			||||||
      collection do
 | 
					 | 
				
			||||||
        get :lookup, to: 'accounts/lookup#index', as: :lookup
 | 
					 | 
				
			||||||
      end
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
      member do
 | 
					      member do
 | 
				
			||||||
        get :statuses
 | 
					        get :statuses
 | 
				
			||||||
        get :followers
 | 
					        get :followers
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -1,24 +0,0 @@
 | 
				
			|||||||
require 'rails_helper'
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
RSpec.describe Api::Accounts::LookupController, type: :controller do
 | 
					 | 
				
			||||||
  render_views
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
  let(:user)  { Fabricate(:user, account: Fabricate(:account, username: 'alice')) }
 | 
					 | 
				
			||||||
  let(:token) { double acceptable?: true, resource_owner_id: user.id }
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
  before do
 | 
					 | 
				
			||||||
    allow(controller).to receive(:doorkeeper_token) { token }
 | 
					 | 
				
			||||||
  end
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
  describe 'GET #index' do
 | 
					 | 
				
			||||||
    before do
 | 
					 | 
				
			||||||
      Fabricate(:account, username: 'bob')
 | 
					 | 
				
			||||||
      Fabricate(:account, username: 'mcbeth')
 | 
					 | 
				
			||||||
      get :index, params: { usernames: 'alice,bob,mcbeth' }
 | 
					 | 
				
			||||||
    end
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
    it 'returns http success' do
 | 
					 | 
				
			||||||
      expect(response).to have_http_status(:success)
 | 
					 | 
				
			||||||
    end
 | 
					 | 
				
			||||||
  end
 | 
					 | 
				
			||||||
end
 | 
					 | 
				
			||||||
@@ -1,5 +0,0 @@
 | 
				
			|||||||
require 'rails_helper'
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
RSpec.describe Api::Accounts::LookupHelper, type: :helper do
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
end
 | 
					 | 
				
			||||||
		Reference in New Issue
	
	Block a user