Commit 6d75b935 authored by Kerri Miller's avatar Kerri Miller

Add fallback path lookup

parent 79f605fb
...@@ -30,9 +30,25 @@ module Gitlab ...@@ -30,9 +30,25 @@ module Gitlab
Loader.new( Loader.new(
merge_request.target_project, merge_request.target_project,
merge_request.target_branch, merge_request.target_branch,
merge_request.modified_paths(past_merge_request_diff: merge_request_diff) paths_for_merge_request(merge_request, merge_request_diff)
) )
end end
private_class_method :loader_for_merge_request private_class_method :loader_for_merge_request
def self.paths_for_merge_request(merge_request, merge_request_diff)
# Because MergeRequest#modified_paths is limited to only returning 1_000
# records, if the diff_size is more than 1_000, we need to fall back to
# the MUCH slower method of using Repository#diff_stats, which isn't
# subject to the same limit.
#
if merge_request.diff_size == "1000+"
merge_request.project.repository.diff_stats(
merge_request.commits.last.sha,
merge_request.commits.first.sha
).paths
else
merge_request.modified_paths(past_merge_request_diff: merge_request_diff)
end
end
end end
end end
...@@ -47,7 +47,7 @@ describe Gitlab::CodeOwners do ...@@ -47,7 +47,7 @@ describe Gitlab::CodeOwners do
describe '.entries_for_merge_request' do describe '.entries_for_merge_request' do
let(:codeowner_lookup_ref) { merge_request.target_branch } let(:codeowner_lookup_ref) { merge_request.target_branch }
let(:merge_request) do let(:merge_request) do
build( create(
:merge_request, :merge_request,
source_project: project, source_project: project,
source_branch: 'feature', source_branch: 'feature',
...@@ -71,16 +71,27 @@ describe Gitlab::CodeOwners do ...@@ -71,16 +71,27 @@ describe Gitlab::CodeOwners do
end end
context 'when merge_request_diff is specified' do context 'when merge_request_diff is specified' do
let(:merge_request_diff) { double(:merge_request_diff) }
it 'returns owners at the specified ref' do it 'returns owners at the specified ref' do
expect(merge_request).to receive(:modified_paths).with(past_merge_request_diff: merge_request_diff).and_return(['docs/CODEOWNERS']) expect(merge_request).to receive(:modified_paths).with(past_merge_request_diff: merge_request.merge_request_diff).and_return(['docs/CODEOWNERS'])
entry = described_class.entries_for_merge_request(merge_request, merge_request_diff: merge_request_diff).first entry = described_class.entries_for_merge_request(merge_request, merge_request_diff: merge_request.merge_request_diff).first
expect(entry.users).to eq([code_owner]) expect(entry.users).to eq([code_owner])
end end
end end
context 'when the merge request is large (>1_000 files)' do
before do
expect(merge_request).to receive(:diff_size).and_return("1000+")
end
it 'generates paths via Repository#diff_stats' do
expect(merge_request).not_to receive(:modified_paths)
expect(merge_request.project.repository).to receive(:diff_stats).and_call_original
described_class.entries_for_merge_request(merge_request)
end
end
end end
context 'when the feature is not available' do context 'when the feature is not available' 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