Skip to content

Commit

Permalink
Fix multiple N+1s in ConversationsController (mastodon#25134)
Browse files Browse the repository at this point in the history
  • Loading branch information
ClearlyClaire authored and skerit committed Jul 7, 2023
1 parent d8e0ea2 commit 5f1f198
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 12 deletions.
17 changes: 15 additions & 2 deletions app/controllers/api/v1/conversations_controller.rb
Expand Up @@ -11,7 +11,7 @@ class Api::V1::ConversationsController < Api::BaseController

def index
@conversations = paginated_conversations
render json: @conversations, each_serializer: REST::ConversationSerializer
render json: @conversations, each_serializer: REST::ConversationSerializer, relationships: StatusRelationshipsPresenter.new(@conversations.map(&:last_status), current_user&.account_id)
end

def read
Expand All @@ -32,7 +32,20 @@ def set_conversation

def paginated_conversations
AccountConversation.where(account: current_account)
.to_a_paginated_by_id(limit_param(LIMIT), params_slice(:max_id, :since_id, :min_id))
.includes(
account: :account_stat,
last_status: [
:media_attachments,
:preview_cards,
:status_stat,
:tags,
{
active_mentions: [account: :account_stat],
account: :account_stat,
},
]
)
.to_a_paginated_by_id(limit_param(LIMIT), **params_slice(:max_id, :since_id, :min_id))
end

def insert_pagination_headers
Expand Down
38 changes: 28 additions & 10 deletions app/models/account_conversation.rb
Expand Up @@ -17,6 +17,8 @@
class AccountConversation < ApplicationRecord
include Redisable

attr_writer :participant_accounts

before_validation :set_last_status
after_commit :push_to_streaming_api

Expand All @@ -26,24 +28,40 @@ class AccountConversation < ApplicationRecord

def participant_account_ids=(arr)
self[:participant_account_ids] = arr.sort
@participant_accounts = nil
end

def participant_accounts
if participant_account_ids.empty?
[account]
else
participants = Account.where(id: participant_account_ids)
participants.empty? ? [account] : participants
@participant_accounts ||= begin
if participant_account_ids.empty?
[account]
else
participants = Account.where(id: participant_account_ids).to_a
participants.empty? ? [account] : participants
end
end
end

class << self
def to_a_paginated_by_id(limit, options = {})
if options[:min_id]
paginate_by_min_id(limit, options[:min_id], options[:max_id]).reverse
else
paginate_by_max_id(limit, options[:max_id], options[:since_id]).to_a
def to_a_paginated_by_id(limit, min_id: nil, max_id: nil, since_id: nil, preload_participants: true)
array = begin
if min_id
paginate_by_min_id(limit, min_id, max_id).reverse
else
paginate_by_max_id(limit, max_id, since_id).to_a
end
end

if preload_participants
participant_ids = array.flat_map(&:participant_account_ids)
accounts_by_id = Account.where(id: participant_ids).index_by(&:id)

array.each do |conversation|
conversation.participant_accounts = conversation.participant_account_ids.filter_map { |id| accounts_by_id[id] }
end
end

array
end

def paginate_by_min_id(limit, min_id = nil, max_id = nil)
Expand Down

0 comments on commit 5f1f198

Please sign in to comment.