Commit eb5f293e authored by Igor Drozdov's avatar Igor Drozdov

Fix N + 1 for MilestonesController#merge_requests

Preload merge requests associations to avoid N + 1
on milestones#show page
parent 7b72698a
...@@ -20,7 +20,7 @@ module MilestoneActions ...@@ -20,7 +20,7 @@ module MilestoneActions
format.html { redirect_to milestone_redirect_path } format.html { redirect_to milestone_redirect_path }
format.json do format.json do
render json: tabs_json("shared/milestones/_merge_requests_tab", { render json: tabs_json("shared/milestones/_merge_requests_tab", {
merge_requests: @milestone.sorted_merge_requests(current_user), # rubocop:disable Gitlab/ModuleWithInstanceVariables merge_requests: @milestone.sorted_merge_requests(current_user).preload_milestoneish_associations, # rubocop:disable Gitlab/ModuleWithInstanceVariables
show_project_name: Gitlab::Utils.to_boolean(params[:show_project_name]) show_project_name: Gitlab::Utils.to_boolean(params[:show_project_name])
}) })
end end
......
...@@ -359,6 +359,8 @@ class MergeRequest < ApplicationRecord ...@@ -359,6 +359,8 @@ class MergeRequest < ApplicationRecord
scope :preload_metrics, -> (relation) { preload(metrics: relation) } scope :preload_metrics, -> (relation) { preload(metrics: relation) }
scope :preload_project_and_latest_diff, -> { preload(:source_project, :latest_merge_request_diff) } scope :preload_project_and_latest_diff, -> { preload(:source_project, :latest_merge_request_diff) }
scope :preload_latest_diff_commit, -> { preload(latest_merge_request_diff: :merge_request_diff_commits) } scope :preload_latest_diff_commit, -> { preload(latest_merge_request_diff: :merge_request_diff_commits) }
scope :preload_milestoneish_associations, -> { preload_routables.preload(:assignees, :labels) }
scope :with_web_entity_associations, -> { preload(:author, target_project: [:project_feature, group: [:route, :parent], namespace: :route]) } scope :with_web_entity_associations, -> { preload(:author, target_project: [:project_feature, group: [:route, :parent], namespace: :route]) }
scope :with_auto_merge_enabled, -> do scope :with_auto_merge_enabled, -> do
......
---
title: 'Fix N + 1 for MilestonesController#merge_requests'
merge_request: 57980
author:
type: performance
...@@ -4,8 +4,8 @@ require 'spec_helper' ...@@ -4,8 +4,8 @@ require 'spec_helper'
RSpec.describe Groups::MilestonesController do RSpec.describe Groups::MilestonesController do
context 'N+1 DB queries' do context 'N+1 DB queries' do
let(:user) { create(:user) } let_it_be(:user) { create(:user) }
let!(:public_group) { create(:group, :public) } let_it_be(:public_group) { create(:group, :public) }
let!(:public_project_with_private_issues_and_mrs) do let!(:public_project_with_private_issues_and_mrs) do
create(:project, :public, :issues_private, :merge_requests_private, group: public_group) create(:project, :public, :issues_private, :merge_requests_private, group: public_group)
...@@ -53,5 +53,25 @@ RSpec.describe Groups::MilestonesController do ...@@ -53,5 +53,25 @@ RSpec.describe Groups::MilestonesController do
expect { get show_path }.not_to exceed_all_query_limit(control) expect { get show_path }.not_to exceed_all_query_limit(control)
end end
end end
describe 'GET #merge_requests' do
let(:milestone) { create(:milestone, group: public_group) }
let(:project) { create(:project, :public, :merge_requests_enabled, :issues_enabled, group: public_group) }
let!(:merge_request) { create(:merge_request, milestone: milestone, source_project: project) }
def perform_request
get merge_requests_group_milestone_path(public_group, milestone, format: :json)
end
it 'avoids N+1 database queries' do
perform_request # warm up the cache
control_count = ActiveRecord::QueryRecorder.new { perform_request }.count
create(:merge_request, milestone: milestone, source_project: project, source_branch: 'fix')
expect { perform_request }.not_to exceed_query_limit(control_count)
end
end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment