Commit 887a3739 authored by Ahmad Sherif's avatar Ahmad Sherif

Migrate Gitlab::Git::Repository#revert to Gitaly

Closes gitaly#780
parent a39d6d89
......@@ -400,7 +400,7 @@ group :ed25519 do
end
# Gitaly GRPC client
gem 'gitaly-proto', '~> 0.58.0', require: 'gitaly'
gem 'gitaly-proto', '~> 0.59.0', require: 'gitaly'
gem 'toml-rb', '~> 0.3.15', require: false
......
......@@ -276,7 +276,7 @@ GEM
po_to_json (>= 1.0.0)
rails (>= 3.2.0)
gherkin-ruby (0.3.2)
gitaly-proto (0.58.0)
gitaly-proto (0.59.0)
google-protobuf (~> 3.1)
grpc (~> 1.0)
github-linguist (4.7.6)
......@@ -1037,7 +1037,7 @@ DEPENDENCIES
gettext (~> 3.2.2)
gettext_i18n_rails (~> 1.8.0)
gettext_i18n_rails_js (~> 1.2.0)
gitaly-proto (~> 0.58.0)
gitaly-proto (~> 0.59.0)
github-linguist (~> 4.7.0)
gitlab-flowdock-git-hook (~> 1.0.1)
gitlab-markup (~> 1.6.2)
......
......@@ -776,24 +776,21 @@ module Gitlab
end
def revert(user:, commit:, branch_name:, message:, start_branch_name:, start_repository:)
OperationService.new(user, self).with_branch(
branch_name,
start_branch_name: start_branch_name,
start_repository: start_repository
) do |start_commit|
Gitlab::Git.check_namespace!(commit, start_repository)
revert_tree_id = check_revert_content(commit, start_commit.sha)
raise CreateTreeError unless revert_tree_id
committer = user_to_committer(user)
gitaly_migrate(:revert) do |is_enabled|
args = {
user: user,
commit: commit,
branch_name: branch_name,
message: message,
start_branch_name: start_branch_name,
start_repository: start_repository
}
create_commit(message: message,
author: committer,
committer: committer,
tree: revert_tree_id,
parents: [start_commit.sha])
if is_enabled
gitaly_operations_client.user_revert(args)
else
rugged_revert(args)
end
end
end
......@@ -1769,6 +1766,28 @@ module Gitlab
end
end
def rugged_revert(user:, commit:, branch_name:, message:, start_branch_name:, start_repository:)
OperationService.new(user, self).with_branch(
branch_name,
start_branch_name: start_branch_name,
start_repository: start_repository
) do |start_commit|
Gitlab::Git.check_namespace!(commit, start_repository)
revert_tree_id = check_revert_content(commit, start_commit.sha)
raise CreateTreeError unless revert_tree_id
committer = user_to_committer(user)
create_commit(message: message,
author: committer,
committer: committer,
tree: revert_tree_id,
parents: [start_commit.sha])
end
end
def gitaly_add_branch(branch_name, user, target)
gitaly_operation_client.user_create_branch(branch_name, user, target)
rescue GRPC::FailedPrecondition => ex
......
......@@ -124,7 +124,31 @@ module Gitlab
end
def user_cherry_pick(user:, commit:, branch_name:, message:, start_branch_name:, start_repository:)
request = Gitaly::UserCherryPickRequest.new(
call_cherry_pick_or_revert(:cherry_pick,
user: user,
commit: commit,
branch_name: branch_name,
message: message,
start_branch_name: start_branch_name,
start_repository: start_repository)
end
def user_revert(user:, commit:, branch_name:, message:, start_branch_name:, start_repository:)
call_cherry_pick_or_revert(:revert,
user: user,
commit: commit,
branch_name: branch_name,
message: message,
start_branch_name: start_branch_name,
start_repository: start_repository)
end
private
def call_cherry_pick_or_revert(rpc, user:, commit:, branch_name:, message:, start_branch_name:, start_repository:)
request_class = "Gitaly::User#{rpc.to_s.camelcase}Request".constantize
request = request_class.new(
repository: @gitaly_repo,
user: Gitlab::Git::User.from_gitlab(user).to_gitaly,
commit: commit.to_gitaly_commit,
......@@ -137,11 +161,15 @@ module Gitlab
response = GitalyClient.call(
@repository.storage,
:operation_service,
:user_cherry_pick,
:"user_#{rpc}",
request,
remote_storage: start_repository.storage
)
handle_cherry_pick_or_revert_response(response)
end
def handle_cherry_pick_or_revert_response(response)
if response.pre_receive_error.presence
raise Gitlab::Git::HooksService::PreReceiveError, response.pre_receive_error
elsif response.commit_error.presence
......
......@@ -1372,39 +1372,49 @@ describe Repository do
end
describe '#revert' do
let(:new_image_commit) { repository.commit('33f3729a45c02fc67d00adb1b8bca394b0e761d9') }
let(:update_image_commit) { repository.commit('2f63565e7aac07bcdadb654e253078b727143ec4') }
let(:message) { 'revert message' }
shared_examples 'reverting a commit' do
let(:new_image_commit) { repository.commit('33f3729a45c02fc67d00adb1b8bca394b0e761d9') }
let(:update_image_commit) { repository.commit('2f63565e7aac07bcdadb654e253078b727143ec4') }
let(:message) { 'revert message' }
context 'when there is a conflict' do
it 'raises an error' do
expect { repository.revert(user, new_image_commit, 'master', message) }.to raise_error(Gitlab::Git::Repository::CreateTreeError)
context 'when there is a conflict' do
it 'raises an error' do
expect { repository.revert(user, new_image_commit, 'master', message) }.to raise_error(Gitlab::Git::Repository::CreateTreeError)
end
end
end
context 'when commit was already reverted' do
it 'raises an error' do
repository.revert(user, update_image_commit, 'master', message)
context 'when commit was already reverted' do
it 'raises an error' do
repository.revert(user, update_image_commit, 'master', message)
expect { repository.revert(user, update_image_commit, 'master', message) }.to raise_error(Gitlab::Git::Repository::CreateTreeError)
expect { repository.revert(user, update_image_commit, 'master', message) }.to raise_error(Gitlab::Git::Repository::CreateTreeError)
end
end
end
context 'when commit can be reverted' do
it 'reverts the changes' do
expect(repository.revert(user, update_image_commit, 'master', message)).to be_truthy
context 'when commit can be reverted' do
it 'reverts the changes' do
expect(repository.revert(user, update_image_commit, 'master', message)).to be_truthy
end
end
end
context 'reverting a merge commit' do
it 'reverts the changes' do
merge_commit
expect(repository.blob_at_branch('master', 'files/ruby/feature.rb')).to be_present
context 'reverting a merge commit' do
it 'reverts the changes' do
merge_commit
expect(repository.blob_at_branch('master', 'files/ruby/feature.rb')).to be_present
repository.revert(user, merge_commit, 'master', message)
expect(repository.blob_at_branch('master', 'files/ruby/feature.rb')).not_to be_present
repository.revert(user, merge_commit, 'master', message)
expect(repository.blob_at_branch('master', 'files/ruby/feature.rb')).not_to be_present
end
end
end
context 'when Gitaly revert feature is enabled' do
it_behaves_like 'reverting a commit'
end
context 'when Gitaly revert feature is disabled', :disable_gitaly do
it_behaves_like 'reverting a commit'
end
end
describe '#cherry_pick' 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