From 943fc87d9f5c817970d268e1a70ab43ed74cd6b1 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Tue, 24 Apr 2018 12:06:05 +0100 Subject: Fix an N+1 for MRs from forks on the MR index page --- app/controllers/concerns/issuable_collections.rb | 4 +- ...er-index-executes-more-than-100-sql-queries.yml | 5 +++ .../issuables_list_metadata_shared_examples.rb | 50 ++++++++++++++-------- 3 files changed, 40 insertions(+), 19 deletions(-) create mode 100644 changelogs/unreleased/43111-controller-projects-mergerequestscontroller-index-executes-more-than-100-sql-queries.yml diff --git a/app/controllers/concerns/issuable_collections.rb b/app/controllers/concerns/issuable_collections.rb index 4114ca6bf7c..34228cf0b82 100644 --- a/app/controllers/concerns/issuable_collections.rb +++ b/app/controllers/concerns/issuable_collections.rb @@ -165,8 +165,8 @@ module IssuableCollections [:project, :author, :assignees, :labels, :milestone, project: :namespace] when 'MergeRequest' [ - :source_project, :target_project, :author, :assignee, :labels, :milestone, - head_pipeline: :project, target_project: :namespace, latest_merge_request_diff: :merge_request_diff_commits + :target_project, :author, :assignee, :labels, :milestone, + source_project: :route, head_pipeline: :project, target_project: :namespace, latest_merge_request_diff: :merge_request_diff_commits ] end end diff --git a/changelogs/unreleased/43111-controller-projects-mergerequestscontroller-index-executes-more-than-100-sql-queries.yml b/changelogs/unreleased/43111-controller-projects-mergerequestscontroller-index-executes-more-than-100-sql-queries.yml new file mode 100644 index 00000000000..120e319acfb --- /dev/null +++ b/changelogs/unreleased/43111-controller-projects-mergerequestscontroller-index-executes-more-than-100-sql-queries.yml @@ -0,0 +1,5 @@ +--- +title: Reduce queries on merge requests list page for merge requests from forks +merge_request: 18561 +author: +type: performance diff --git a/spec/support/shared_examples/issuables_list_metadata_shared_examples.rb b/spec/support/shared_examples/issuables_list_metadata_shared_examples.rb index e61983c60b4..f4bc6f8efa5 100644 --- a/spec/support/shared_examples/issuables_list_metadata_shared_examples.rb +++ b/spec/support/shared_examples/issuables_list_metadata_shared_examples.rb @@ -1,25 +1,30 @@ shared_examples 'issuables list meta-data' do |issuable_type, action = nil| - before do - @issuable_ids = [] - - %w[fix improve/awesome].each do |source_branch| - issuable = - if issuable_type == :issue - create(issuable_type, project: project, author: project.creator) - else - create(issuable_type, source_project: project, source_branch: source_branch, author: project.creator) - end - - @issuable_ids << issuable.id - end - end + include ProjectForksHelper - it "creates indexed meta-data object for issuable notes and votes count" do + def get_action(action, project) if action get action, author_id: project.creator.id else get :index, namespace_id: project.namespace, project_id: project end + end + + def create_issuable(issuable_type, project, source_branch:) + if issuable_type == :issue + create(issuable_type, project: project, author: project.creator) + else + create(issuable_type, source_project: project, source_branch: source_branch, author: project.creator) + end + end + + before do + @issuable_ids = %w[fix improve/awesome].map do |source_branch| + create_issuable(issuable_type, project, source_branch: source_branch).id + end + end + + it "creates indexed meta-data object for issuable notes and votes count" do + get_action(action, project) meta_data = assigns(:issuable_meta_data) @@ -29,18 +34,29 @@ shared_examples 'issuables list meta-data' do |issuable_type, action = nil| end end + it "avoids N+1 queries" do + control = ActiveRecord::QueryRecorder.new { get_action(action, project) } + issuable = create_issuable(issuable_type, project, source_branch: 'csv') + + if issuable_type == :merge_request + issuable.update!(source_project: fork_project(project)) + end + + expect { get_action(action, project) }.not_to exceed_query_limit(control.count) + end + describe "when given empty collection" do let(:project2) { create(:project, :public) } it "doesn't execute any queries with false conditions" do - get_action = + get_empty = if action proc { get action, author_id: project.creator.id } else proc { get :index, namespace_id: project2.namespace, project_id: project2 } end - expect(&get_action).not_to make_queries_matching(/WHERE (?:1=0|0=1)/) + expect(&get_empty).not_to make_queries_matching(/WHERE (?:1=0|0=1)/) end end end -- cgit v1.2.1