Move `deltas` and `diff_from_parents` logic to Gitlab::Git::Commit
This helps keep the abstraction layers simpler, and also keep the interface of those methods consistent, in case of implementation changes.
This commit is contained in:
parent
c21ae07e33
commit
e363fbf71a
|
@ -321,21 +321,11 @@ class Commit
|
|||
end
|
||||
|
||||
def raw_diffs(*args)
|
||||
if Gitlab::GitalyClient.feature_enabled?(:commit_raw_diffs)
|
||||
Gitlab::GitalyClient::CommitService.new(project.repository).diff_from_parent(self, *args)
|
||||
else
|
||||
raw.diffs(*args)
|
||||
end
|
||||
raw.diffs(*args)
|
||||
end
|
||||
|
||||
def raw_deltas
|
||||
@deltas ||= Gitlab::GitalyClient.migrate(:commit_deltas) do |is_enabled|
|
||||
if is_enabled
|
||||
Gitlab::GitalyClient::CommitService.new(project.repository).commit_deltas(self)
|
||||
else
|
||||
raw.deltas
|
||||
end
|
||||
end
|
||||
@deltas ||= raw.deltas
|
||||
end
|
||||
|
||||
def diffs(diff_options = nil)
|
||||
|
|
|
@ -187,25 +187,6 @@ module Gitlab
|
|||
Gitlab::Git::Commit.new(repository, commit, ref)
|
||||
end
|
||||
|
||||
# Returns a diff object for the changes introduced by +rugged_commit+.
|
||||
# If +rugged_commit+ doesn't have a parent, then the diff is between
|
||||
# this commit and an empty repo. See Repository#diff for the keys
|
||||
# allowed in the +options+ hash.
|
||||
def diff_from_parent(rugged_commit, options = {})
|
||||
options ||= {}
|
||||
break_rewrites = options[:break_rewrites]
|
||||
actual_options = Gitlab::Git::Diff.filter_diff_options(options)
|
||||
|
||||
diff = if rugged_commit.parents.empty?
|
||||
rugged_commit.diff(actual_options.merge(reverse: true))
|
||||
else
|
||||
rugged_commit.parents[0].diff(rugged_commit, actual_options)
|
||||
end
|
||||
|
||||
diff.find_similar!(break_rewrites: break_rewrites)
|
||||
diff
|
||||
end
|
||||
|
||||
# Returns the `Rugged` sorting type constant for one or more given
|
||||
# sort types. Valid keys are `:none`, `:topo`, and `:date`, or an array
|
||||
# containing more than one of them. `:date` uses a combination of date and
|
||||
|
@ -270,19 +251,50 @@ module Gitlab
|
|||
#
|
||||
# Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/324
|
||||
def to_diff
|
||||
diff_from_parent.patch
|
||||
rugged_diff_from_parent.patch
|
||||
end
|
||||
|
||||
# Returns a diff object for the changes from this commit's first parent.
|
||||
# If there is no parent, then the diff is between this commit and an
|
||||
# empty repo. See Repository#diff for keys allowed in the +options+
|
||||
# empty repo. See Repository#diff for keys allowed in the +options+
|
||||
# hash.
|
||||
def diff_from_parent(options = {})
|
||||
Commit.diff_from_parent(raw_commit, options)
|
||||
Gitlab::GitalyClient.migrate(:commit_raw_diffs) do |is_enabled|
|
||||
if is_enabled
|
||||
@repository.gitaly_commit_client.diff_from_parent(self, options)
|
||||
else
|
||||
rugged_diff_from_parent(options)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def rugged_diff_from_parent(options = {})
|
||||
options ||= {}
|
||||
break_rewrites = options[:break_rewrites]
|
||||
actual_options = Gitlab::Git::Diff.filter_diff_options(options)
|
||||
|
||||
diff = if raw_commit.parents.empty?
|
||||
raw_commit.diff(actual_options.merge(reverse: true))
|
||||
else
|
||||
raw_commit.parents[0].diff(raw_commit, actual_options)
|
||||
end
|
||||
|
||||
diff.find_similar!(break_rewrites: break_rewrites)
|
||||
diff
|
||||
end
|
||||
|
||||
def deltas
|
||||
@deltas ||= diff_from_parent.each_delta.map { |d| Gitlab::Git::Diff.new(d) }
|
||||
@deltas ||= begin
|
||||
deltas = Gitlab::GitalyClient.migrate(:commit_deltas) do |is_enabled|
|
||||
if is_enabled
|
||||
@repository.gitaly_commit_client.commit_deltas(self)
|
||||
else
|
||||
rugged_diff_from_parent.each_delta
|
||||
end
|
||||
end
|
||||
|
||||
deltas.map { |delta| Gitlab::Git::Diff.new(delta) }
|
||||
end
|
||||
end
|
||||
|
||||
def has_zero_stats?
|
||||
|
|
|
@ -16,7 +16,7 @@ module Gitlab
|
|||
@deletions = 0
|
||||
@total = 0
|
||||
|
||||
diff = commit.diff_from_parent
|
||||
diff = commit.rugged_diff_from_parent
|
||||
|
||||
diff.each_patch do |p|
|
||||
# TODO: Use the new Rugged convenience methods when they're released
|
||||
|
|
|
@ -29,15 +29,14 @@ module Gitlab
|
|||
|
||||
request = Gitaly::CommitDiffRequest.new(request_params)
|
||||
response = GitalyClient.call(@repository.storage, :diff_service, :commit_diff, request)
|
||||
Gitlab::Git::DiffCollection.new(GitalyClient::DiffStitcher.new(response), options.merge(from_gitaly: true))
|
||||
GitalyClient::DiffStitcher.new(response)
|
||||
end
|
||||
|
||||
def commit_deltas(commit)
|
||||
request = Gitaly::CommitDeltaRequest.new(commit_diff_request_params(commit))
|
||||
response = GitalyClient.call(@repository.storage, :diff_service, :commit_delta, request)
|
||||
response.flat_map do |msg|
|
||||
msg.deltas.map { |d| Gitlab::Git::Diff.new(d) }
|
||||
end
|
||||
|
||||
response.flat_map { |msg| msg.deltas }
|
||||
end
|
||||
|
||||
def tree_entry(ref, path, limit = nil)
|
||||
|
|
|
@ -170,7 +170,7 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits do
|
|||
context 'when the merge request diffs are Rugged::Patch instances' do
|
||||
let(:commits) { merge_request_diff.commits.map(&:to_hash) }
|
||||
let(:first_commit) { merge_request.project.repository.commit(merge_request_diff.head_commit_sha) }
|
||||
let(:diffs) { first_commit.diff_from_parent.patches }
|
||||
let(:diffs) { first_commit.rugged_diff_from_parent.patches }
|
||||
let(:expected_diffs) { [] }
|
||||
|
||||
include_examples 'updated MR diff'
|
||||
|
@ -179,7 +179,7 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits do
|
|||
context 'when the merge request diffs are Rugged::Diff::Delta instances' do
|
||||
let(:commits) { merge_request_diff.commits.map(&:to_hash) }
|
||||
let(:first_commit) { merge_request.project.repository.commit(merge_request_diff.head_commit_sha) }
|
||||
let(:diffs) { first_commit.diff_from_parent.deltas }
|
||||
let(:diffs) { first_commit.rugged_diff_from_parent.deltas }
|
||||
let(:expected_diffs) { [] }
|
||||
|
||||
include_examples 'updated MR diff'
|
||||
|
|
|
@ -759,7 +759,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
|
|||
let(:options) { { ref: 'master', path: ['PROCESS.md', 'README.md'] } }
|
||||
|
||||
def commit_files(commit)
|
||||
commit.diff_from_parent.deltas.flat_map do |delta|
|
||||
commit.rugged_diff_from_parent.deltas.flat_map do |delta|
|
||||
[delta.old_file[:path], delta.new_file[:path]].uniq.compact
|
||||
end
|
||||
end
|
||||
|
|
|
@ -46,18 +46,10 @@ describe Gitlab::GitalyClient::CommitService do
|
|||
end
|
||||
end
|
||||
|
||||
it 'returns a Gitlab::Git::DiffCollection' do
|
||||
it 'returns a Gitlab::GitalyClient::DiffStitcher' do
|
||||
ret = client.diff_from_parent(commit)
|
||||
|
||||
expect(ret).to be_kind_of(Gitlab::Git::DiffCollection)
|
||||
end
|
||||
|
||||
it 'passes options to Gitlab::Git::DiffCollection' do
|
||||
options = { max_files: 31, max_lines: 13, from_gitaly: true }
|
||||
|
||||
expect(Gitlab::Git::DiffCollection).to receive(:new).with(kind_of(Enumerable), options)
|
||||
|
||||
client.diff_from_parent(commit, options)
|
||||
expect(ret).to be_kind_of(Gitlab::GitalyClient::DiffStitcher)
|
||||
end
|
||||
end
|
||||
|
||||
|
|
Loading…
Reference in New Issue