Commit 9b930932 authored by Jacob Vosmaer's avatar Jacob Vosmaer

Decouple GitOperationService from User

parent 0f74ba96
......@@ -164,7 +164,8 @@ class Repository
return false unless newrev
GitOperationService.new(user, self).add_branch(branch_name, newrev)
committer = Gitlab::Git::Committer.from_user(user)
GitOperationService.new(committer, self).add_branch(branch_name, newrev)
after_create_branch
find_branch(branch_name)
......@@ -176,7 +177,8 @@ class Repository
return false unless newrev
GitOperationService.new(user, self).add_tag(tag_name, newrev, options)
committer = Gitlab::Git::Committer.from_user(user)
GitOperationService.new(committer, self).add_tag(tag_name, newrev, options)
find_tag(tag_name)
end
......@@ -185,7 +187,8 @@ class Repository
before_remove_branch
branch = find_branch(branch_name)
GitOperationService.new(user, self).rm_branch(branch)
committer = Gitlab::Git::Committer.from_user(user)
GitOperationService.new(committer, self).rm_branch(branch)
after_remove_branch
true
......@@ -195,7 +198,8 @@ class Repository
before_remove_tag
tag = find_tag(tag_name)
GitOperationService.new(user, self).rm_tag(tag)
committer = Gitlab::Git::Committer.from_user(user)
GitOperationService.new(committer, self).rm_tag(tag)
after_remove_tag
true
......@@ -763,7 +767,8 @@ class Repository
author_email: nil, author_name: nil,
start_branch_name: nil, start_project: project)
GitOperationService.new(user, self).with_branch(
committer = Gitlab::Git::Committer.from_user(user)
GitOperationService.new(committer, self).with_branch(
branch_name,
start_branch_name: start_branch_name,
start_project: start_project) do |start_commit|
......@@ -819,7 +824,8 @@ class Repository
end
def merge(user, source, merge_request, options = {})
GitOperationService.new(user, self).with_branch(
committer = Gitlab::Git::Committer.from_user(user)
GitOperationService.new(committer, self).with_branch(
merge_request.target_branch) do |start_commit|
our_commit = start_commit.sha
their_commit = source
......@@ -846,7 +852,8 @@ class Repository
def revert(
user, commit, branch_name,
start_branch_name: nil, start_project: project)
GitOperationService.new(user, self).with_branch(
committer = Gitlab::Git::Committer.from_user(user)
GitOperationService.new(committer, self).with_branch(
branch_name,
start_branch_name: start_branch_name,
start_project: start_project) do |start_commit|
......@@ -869,7 +876,8 @@ class Repository
def cherry_pick(
user, commit, branch_name,
start_branch_name: nil, start_project: project)
GitOperationService.new(user, self).with_branch(
committer = Gitlab::Git::Committer.from_user(user)
GitOperationService.new(committer, self).with_branch(
branch_name,
start_branch_name: start_branch_name,
start_project: start_project) do |start_commit|
......@@ -894,7 +902,8 @@ class Repository
end
def resolve_conflicts(user, branch_name, params)
GitOperationService.new(user, self).with_branch(branch_name) do
committer = Gitlab::Git::Committer.from_user(user)
GitOperationService.new(committer, self).with_branch(branch_name) do
committer = user_to_committer(user)
create_commit(params.merge(author: committer, committer: committer))
......
......@@ -3,9 +3,9 @@ class GitHooksService
attr_accessor :oldrev, :newrev, :ref
def execute(user, project, oldrev, newrev, ref)
def execute(committer, project, oldrev, newrev, ref)
@project = project
@user = Gitlab::GlId.gl_id(user)
@gl_id = committer.gl_id
@oldrev = oldrev
@newrev = newrev
@ref = ref
......@@ -27,6 +27,6 @@ class GitHooksService
def run_hook(name)
hook = Gitlab::Git::Hook.new(name, @project)
hook.trigger(@user, oldrev, newrev, ref)
hook.trigger(@gl_id, oldrev, newrev, ref)
end
end
class GitOperationService
attr_reader :user, :repository
attr_reader :committer, :repository
def initialize(new_user, new_repository)
@user = new_user
def initialize(committer, new_repository)
if committer && !committer.is_a?(Gitlab::Git::Committer)
raise "expected Gitlab::Git::Committer, got #{committer.inspect}"
end
@committer = committer
@repository = new_repository
end
......@@ -119,7 +122,7 @@ class GitOperationService
def with_hooks(ref, newrev, oldrev)
GitHooksService.new.execute(
user,
committer,
repository.project,
oldrev,
newrev,
......
module Gitlab
module Git
class Committer
attr_reader :name, :email, :gl_id
def self.from_user(user)
new(user.name, user.email, Gitlab::GlId.gl_id(user))
end
def initialize(name, email, gl_id)
@name = name
@email = email
@gl_id = gl_id
end
def ==(other)
[name, email, gl_id] == [other.name, other.email, other.gl_id]
end
end
end
end
......@@ -8,6 +8,7 @@ describe Repository, models: true do
let(:repository) { project.repository }
let(:broken_repository) { create(:project, :broken_storage).repository }
let(:user) { create(:user) }
let(:committer) { Gitlab::Git::Committer.from_user(user) }
let(:commit_options) do
author = repository.user_to_committer(user)
......@@ -885,7 +886,7 @@ describe Repository, models: true do
context 'when pre hooks were successful' do
it 'runs without errors' do
expect_any_instance_of(GitHooksService).to receive(:execute)
.with(user, project, old_rev, blank_sha, 'refs/heads/feature')
.with(committer, project, old_rev, blank_sha, 'refs/heads/feature')
expect { repository.rm_branch(user, 'feature') }.not_to raise_error
end
......@@ -928,20 +929,20 @@ describe Repository, models: true do
service = GitHooksService.new
expect(GitHooksService).to receive(:new).and_return(service)
expect(service).to receive(:execute)
.with(user, project, old_rev, new_rev, 'refs/heads/feature')
.with(committer, project, old_rev, new_rev, 'refs/heads/feature')
.and_yield(service).and_return(true)
end
it 'runs without errors' do
expect do
GitOperationService.new(user, repository).with_branch('feature') do
GitOperationService.new(committer, repository).with_branch('feature') do
new_rev
end
end.not_to raise_error
end
it 'ensures the autocrlf Git option is set to :input' do
service = GitOperationService.new(user, repository)
service = GitOperationService.new(committer, repository)
expect(service).to receive(:update_autocrlf_option)
......@@ -952,7 +953,7 @@ describe Repository, models: true do
it 'updates the head' do
expect(repository.find_branch('feature').dereferenced_target.id).to eq(old_rev)
GitOperationService.new(user, repository).with_branch('feature') do
GitOperationService.new(committer, repository).with_branch('feature') do
new_rev
end
......@@ -974,7 +975,7 @@ describe Repository, models: true do
end
expect do
GitOperationService.new(user, target_project.repository)
GitOperationService.new(committer, target_project.repository)
.with_branch('feature',
start_project: project,
&:itself)
......@@ -996,7 +997,7 @@ describe Repository, models: true do
repository.add_branch(user, branch, old_rev)
expect do
GitOperationService.new(user, repository).with_branch(branch) do
GitOperationService.new(committer, repository).with_branch(branch) do
new_rev
end
end.not_to raise_error
......@@ -1014,7 +1015,7 @@ describe Repository, models: true do
# Updating 'master' to new_rev would lose the commits on 'master' that
# are not contained in new_rev. This should not be allowed.
expect do
GitOperationService.new(user, repository).with_branch(branch) do
GitOperationService.new(committer, repository).with_branch(branch) do
new_rev
end
end.to raise_error(Repository::CommitError)
......@@ -1026,7 +1027,7 @@ describe Repository, models: true do
allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, ''])
expect do
GitOperationService.new(user, repository).with_branch('feature') do
GitOperationService.new(committer, repository).with_branch('feature') do
new_rev
end
end.to raise_error(GitHooksService::PreReceiveError)
......@@ -1044,7 +1045,7 @@ describe Repository, models: true do
expect(repository).not_to receive(:expire_emptiness_caches)
expect(repository).to receive(:expire_branches_cache)
GitOperationService.new(user, repository)
GitOperationService.new(committer, repository)
.with_branch('new-feature') do
new_rev
end
......
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