Unify parameters and callback after hooks
Feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7237#note_19747856
This commit is contained in:
parent
46d752ce21
commit
d03c605bd4
|
@ -11,7 +11,7 @@ class GitOperationService
|
|||
ref = Gitlab::Git::BRANCH_REF_PREFIX + branch_name
|
||||
oldrev = Gitlab::Git::BLANK_SHA
|
||||
|
||||
with_hooks_and_update_ref(ref, oldrev, newrev)
|
||||
update_ref_in_hooks(ref, newrev, oldrev)
|
||||
end
|
||||
|
||||
def rm_branch(branch)
|
||||
|
@ -19,14 +19,14 @@ class GitOperationService
|
|||
oldrev = branch.dereferenced_target.id
|
||||
newrev = Gitlab::Git::BLANK_SHA
|
||||
|
||||
with_hooks_and_update_ref(ref, oldrev, newrev)
|
||||
update_ref_in_hooks(ref, newrev, oldrev)
|
||||
end
|
||||
|
||||
def add_tag(tag_name, newrev, options = {})
|
||||
ref = Gitlab::Git::TAG_REF_PREFIX + tag_name
|
||||
oldrev = Gitlab::Git::BLANK_SHA
|
||||
|
||||
with_hooks(ref, oldrev, newrev) do |service|
|
||||
with_hooks(ref, newrev, oldrev) do |service|
|
||||
raw_tag = repository.rugged.tags.create(tag_name, newrev, options)
|
||||
service.newrev = raw_tag.target_id
|
||||
end
|
||||
|
@ -82,25 +82,23 @@ class GitOperationService
|
|||
end
|
||||
|
||||
ref = Gitlab::Git::BRANCH_REF_PREFIX + branch_name
|
||||
with_hooks_and_update_ref(ref, oldrev, newrev) do
|
||||
# If repo was empty expire cache
|
||||
repository.after_create if was_empty
|
||||
repository.after_create_branch if was_empty ||
|
||||
Gitlab::Git.blank_ref?(oldrev)
|
||||
end
|
||||
update_ref_in_hooks(ref, newrev, oldrev)
|
||||
|
||||
# If repo was empty expire cache
|
||||
repository.after_create if was_empty
|
||||
repository.after_create_branch if was_empty ||
|
||||
Gitlab::Git.blank_ref?(oldrev)
|
||||
|
||||
newrev
|
||||
end
|
||||
|
||||
def with_hooks_and_update_ref(ref, oldrev, newrev)
|
||||
with_hooks(ref, oldrev, newrev) do |service|
|
||||
update_ref!(ref, newrev, oldrev)
|
||||
|
||||
yield(service) if block_given?
|
||||
def update_ref_in_hooks(ref, newrev, oldrev)
|
||||
with_hooks(ref, newrev, oldrev) do
|
||||
update_ref(ref, newrev, oldrev)
|
||||
end
|
||||
end
|
||||
|
||||
def with_hooks(ref, oldrev, newrev)
|
||||
def with_hooks(ref, newrev, oldrev)
|
||||
result = nil
|
||||
|
||||
GitHooksService.new.execute(
|
||||
|
@ -116,7 +114,7 @@ class GitOperationService
|
|||
result
|
||||
end
|
||||
|
||||
def update_ref!(name, newrev, oldrev)
|
||||
def update_ref(ref, newrev, oldrev)
|
||||
# We use 'git update-ref' because libgit2/rugged currently does not
|
||||
# offer 'compare and swap' ref updates. Without compare-and-swap we can
|
||||
# (and have!) accidentally reset the ref to an earlier state, clobbering
|
||||
|
@ -125,12 +123,12 @@ class GitOperationService
|
|||
_, status = Gitlab::Popen.popen(
|
||||
command,
|
||||
repository.path_to_repo) do |stdin|
|
||||
stdin.write("update #{name}\x00#{newrev}\x00#{oldrev}\x00")
|
||||
stdin.write("update #{ref}\x00#{newrev}\x00#{oldrev}\x00")
|
||||
end
|
||||
|
||||
unless status.zero?
|
||||
raise Repository::CommitError.new(
|
||||
"Could not update branch #{name.sub('refs/heads/', '')}." \
|
||||
"Could not update branch #{Gitlab::Git.branch_name(ref)}." \
|
||||
" Please refresh and try again.")
|
||||
end
|
||||
end
|
||||
|
|
|
@ -6,7 +6,7 @@ module Gitlab
|
|||
|
||||
class << self
|
||||
def ref_name(ref)
|
||||
ref.gsub(/\Arefs\/(tags|heads)\//, '')
|
||||
ref.sub(/\Arefs\/(tags|heads)\//, '')
|
||||
end
|
||||
|
||||
def branch_name(ref)
|
||||
|
|
|
@ -829,7 +829,6 @@ describe Repository, models: true do
|
|||
context 'when target branch is different from source branch' do
|
||||
before do
|
||||
allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, ''])
|
||||
allow(repository).to receive(:update_ref!)
|
||||
end
|
||||
|
||||
it 'expires branch cache' do
|
||||
|
@ -1474,16 +1473,16 @@ describe Repository, models: true do
|
|||
end
|
||||
end
|
||||
|
||||
describe '#update_ref!' do
|
||||
describe '#update_ref' do
|
||||
it 'can create a ref' do
|
||||
GitOperationService.new(nil, repository).send(:update_ref!, 'refs/heads/foobar', 'refs/heads/master', Gitlab::Git::BLANK_SHA)
|
||||
GitOperationService.new(nil, repository).send(:update_ref, 'refs/heads/foobar', 'refs/heads/master', Gitlab::Git::BLANK_SHA)
|
||||
|
||||
expect(repository.find_branch('foobar')).not_to be_nil
|
||||
end
|
||||
|
||||
it 'raises CommitError when the ref update fails' do
|
||||
expect do
|
||||
GitOperationService.new(nil, repository).send(:update_ref!, 'refs/heads/master', 'refs/heads/master', Gitlab::Git::BLANK_SHA)
|
||||
GitOperationService.new(nil, repository).send(:update_ref, 'refs/heads/master', 'refs/heads/master', Gitlab::Git::BLANK_SHA)
|
||||
end.to raise_error(Repository::CommitError)
|
||||
end
|
||||
end
|
||||
|
|
|
@ -108,7 +108,7 @@ describe GitGarbageCollectWorker do
|
|||
parents: [old_commit],
|
||||
)
|
||||
GitOperationService.new(nil, project.repository).send(
|
||||
:update_ref!,
|
||||
:update_ref,
|
||||
"refs/heads/#{SecureRandom.hex(6)}",
|
||||
new_commit_sha,
|
||||
Gitlab::Git::BLANK_SHA
|
||||
|
|
Loading…
Reference in New Issue