summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSean McGivern <sean@gitlab.com>2018-04-24 12:06:05 +0100
committerSean McGivern <sean@gitlab.com>2018-04-24 12:06:05 +0100
commit943fc87d9f5c817970d268e1a70ab43ed74cd6b1 (patch)
tree77856a0bce2aabf3803ca0ed062ffd2df866f71b
parent26147b730f0f41650c2b7ff6ab21d645b36273c9 (diff)
downloadgitlab-ce-943fc87d9f5c817970d268e1a70ab43ed74cd6b1.tar.gz
Fix an N+1 for MRs from forks on the MR index page
-rw-r--r--app/controllers/concerns/issuable_collections.rb4
-rw-r--r--changelogs/unreleased/43111-controller-projects-mergerequestscontroller-index-executes-more-than-100-sql-queries.yml5
-rw-r--r--spec/support/shared_examples/issuables_list_metadata_shared_examples.rb50
3 files changed, 40 insertions, 19 deletions
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