Commit e37b1c76 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'reduce-query-count-for-mergerequestscontroller-show' into 'master'

Fix N+1 in `MergeRequest#merge_request_diff_for`

See merge request gitlab-org/gitlab-ce!17908
parents 1e307c8b 39c9928c
......@@ -536,18 +536,25 @@ class MergeRequest < ActiveRecord::Base
merge_request_diff(true)
end
def viewable_diffs
@viewable_diffs ||= merge_request_diffs.viewable.to_a
end
def merge_request_diff_for(diff_refs_or_sha)
@merge_request_diffs_by_diff_refs_or_sha ||= Hash.new do |h, diff_refs_or_sha|
diffs = merge_request_diffs.viewable
h[diff_refs_or_sha] =
if diff_refs_or_sha.is_a?(Gitlab::Diff::DiffRefs)
diffs.find_by_diff_refs(diff_refs_or_sha)
else
diffs.find_by(head_commit_sha: diff_refs_or_sha)
end
end
matcher =
if diff_refs_or_sha.is_a?(Gitlab::Diff::DiffRefs)
{
'start_commit_sha' => diff_refs_or_sha.start_sha,
'head_commit_sha' => diff_refs_or_sha.head_sha,
'base_commit_sha' => diff_refs_or_sha.base_sha
}
else
{ 'head_commit_sha' => diff_refs_or_sha }
end
@merge_request_diffs_by_diff_refs_or_sha[diff_refs_or_sha]
viewable_diffs.find do |diff|
diff.attributes.slice(*matcher.keys) == matcher
end
end
def version_params_for(diff_refs)
......
---
title: Reduce number of queries when viewing a merge request
merge_request:
author:
type: performance
......@@ -1961,6 +1961,17 @@ describe MergeRequest do
expect(subject.merge_request_diff_for(merge_request_diff3.head_commit_sha)).to eq(merge_request_diff3)
end
end
it 'runs a single query on the initial call, and none afterwards' do
expect { subject.merge_request_diff_for(merge_request_diff1.diff_refs) }
.not_to exceed_query_limit(1)
expect { subject.merge_request_diff_for(merge_request_diff2.diff_refs) }
.not_to exceed_query_limit(0)
expect { subject.merge_request_diff_for(merge_request_diff3.head_commit_sha) }
.not_to exceed_query_limit(0)
end
end
describe '#version_params_for' do
......
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