From 327546536f986030f756463ef5a4e22556141105 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Wed, 13 Sep 2017 18:16:56 +0200 Subject: Rename Gitlab::Git::Committer to User --- app/models/repository.rb | 8 ++++---- lib/gitlab/git/committer.rb | 21 --------------------- lib/gitlab/git/operation_service.rb | 12 +++++++----- lib/gitlab/git/repository.rb | 20 ++++++++++---------- lib/gitlab/git/user.rb | 25 +++++++++++++++++++++++++ spec/lib/gitlab/git/committer_spec.rb | 22 ---------------------- spec/lib/gitlab/git/hooks_service_spec.rb | 8 ++++---- spec/lib/gitlab/git/user_spec.rb | 22 ++++++++++++++++++++++ spec/models/repository_spec.rb | 2 +- 9 files changed, 73 insertions(+), 67 deletions(-) delete mode 100644 lib/gitlab/git/committer.rb create mode 100644 lib/gitlab/git/user.rb delete mode 100644 spec/lib/gitlab/git/committer_spec.rb create mode 100644 spec/lib/gitlab/git/user_spec.rb diff --git a/app/models/repository.rb b/app/models/repository.rb index 035f85a0b46..cb479777fa3 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -166,7 +166,7 @@ class Repository end def add_branch(user, branch_name, ref) - branch = raw_repository.add_branch(branch_name, committer: user, target: ref) + branch = raw_repository.add_branch(branch_name, user: user, target: ref) after_create_branch @@ -176,7 +176,7 @@ class Repository end def add_tag(user, tag_name, target, message = nil) - raw_repository.add_tag(tag_name, committer: user, target: target, message: message) + raw_repository.add_tag(tag_name, user: user, target: target, message: message) rescue Gitlab::Git::Repository::InvalidRef false end @@ -184,7 +184,7 @@ class Repository def rm_branch(user, branch_name) before_remove_branch - raw_repository.rm_branch(branch_name, committer: user) + raw_repository.rm_branch(branch_name, user: user) after_remove_branch true @@ -193,7 +193,7 @@ class Repository def rm_tag(user, tag_name) before_remove_tag - raw_repository.rm_tag(tag_name, committer: user) + raw_repository.rm_tag(tag_name, user: user) after_remove_tag true diff --git a/lib/gitlab/git/committer.rb b/lib/gitlab/git/committer.rb deleted file mode 100644 index 1f4bcf7a3a0..00000000000 --- a/lib/gitlab/git/committer.rb +++ /dev/null @@ -1,21 +0,0 @@ -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 diff --git a/lib/gitlab/git/operation_service.rb b/lib/gitlab/git/operation_service.rb index 9e6fca8c80c..347e3b5165e 100644 --- a/lib/gitlab/git/operation_service.rb +++ b/lib/gitlab/git/operation_service.rb @@ -1,11 +1,13 @@ module Gitlab module Git class OperationService - attr_reader :committer, :repository + attr_reader :user, :repository - def initialize(committer, new_repository) - committer = Gitlab::Git::Committer.from_user(committer) if committer.is_a?(User) - @committer = committer + def initialize(user, new_repository) + if user + user = Gitlab::Git::User.from_gitlab(user) unless user.respond_to?(:gl_id) + @user = user + end # Refactoring aid unless new_repository.is_a?(Gitlab::Git::Repository) @@ -128,7 +130,7 @@ module Gitlab def with_hooks(ref, newrev, oldrev) Gitlab::Git::HooksService.new.execute( - committer, + user, repository, oldrev, newrev, diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index efa13590a2c..32a265b15f2 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -610,43 +610,43 @@ module Gitlab # TODO: implement this method end - def add_branch(branch_name, committer:, target:) + def add_branch(branch_name, user:, target:) target_object = Ref.dereference_object(lookup(target)) raise InvalidRef.new("target not found: #{target}") unless target_object - OperationService.new(committer, self).add_branch(branch_name, target_object.oid) + OperationService.new(user, self).add_branch(branch_name, target_object.oid) find_branch(branch_name) rescue Rugged::ReferenceError => ex raise InvalidRef, ex end - def add_tag(tag_name, committer:, target:, message: nil) + def add_tag(tag_name, user:, target:, message: nil) target_object = Ref.dereference_object(lookup(target)) raise InvalidRef.new("target not found: #{target}") unless target_object - committer = Committer.from_user(committer) if committer.is_a?(User) + user = Gitlab::Git::User.from_gitlab(user) unless user.respond_to?(:gl_id) options = nil # Use nil, not the empty hash. Rugged cares about this. if message options = { message: message, - tagger: Gitlab::Git.committer_hash(email: committer.email, name: committer.name) + tagger: Gitlab::Git.committer_hash(email: user.email, name: user.name) } end - OperationService.new(committer, self).add_tag(tag_name, target_object.oid, options) + OperationService.new(user, self).add_tag(tag_name, target_object.oid, options) find_tag(tag_name) rescue Rugged::ReferenceError => ex raise InvalidRef, ex end - def rm_branch(branch_name, committer:) - OperationService.new(committer, self).rm_branch(find_branch(branch_name)) + def rm_branch(branch_name, user:) + OperationService.new(user, self).rm_branch(find_branch(branch_name)) end - def rm_tag(tag_name, committer:) - OperationService.new(committer, self).rm_tag(find_tag(tag_name)) + def rm_tag(tag_name, user:) + OperationService.new(user, self).rm_tag(find_tag(tag_name)) end def find_tag(name) diff --git a/lib/gitlab/git/user.rb b/lib/gitlab/git/user.rb new file mode 100644 index 00000000000..ea634d39668 --- /dev/null +++ b/lib/gitlab/git/user.rb @@ -0,0 +1,25 @@ +module Gitlab + module Git + class User + attr_reader :name, :email, :gl_id + + def self.from_gitlab(gitlab_user) + new(gitlab_user.name, gitlab_user.email, Gitlab::GlId.gl_id(gitlab_user)) + end + + def self.from_gitaly(gitaly_user) + new(gitaly_user.name, gitaly_user.email, gitaly_user.gl_id) + 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 diff --git a/spec/lib/gitlab/git/committer_spec.rb b/spec/lib/gitlab/git/committer_spec.rb deleted file mode 100644 index b0ddbb51449..00000000000 --- a/spec/lib/gitlab/git/committer_spec.rb +++ /dev/null @@ -1,22 +0,0 @@ -require 'spec_helper' - -describe Gitlab::Git::Committer do - let(:name) { 'Jane Doe' } - let(:email) { 'janedoe@example.com' } - let(:gl_id) { 'user-123' } - - subject { described_class.new(name, email, gl_id) } - - describe '#==' do - def eq_other(name, email, gl_id) - eq(described_class.new(name, email, gl_id)) - end - - it { expect(subject).to eq_other(name, email, gl_id) } - - it { expect(subject).not_to eq_other(nil, nil, nil) } - it { expect(subject).not_to eq_other(name + 'x', email, gl_id) } - it { expect(subject).not_to eq_other(name, email + 'x', gl_id) } - it { expect(subject).not_to eq_other(name, email, gl_id + 'x') } - end -end diff --git a/spec/lib/gitlab/git/hooks_service_spec.rb b/spec/lib/gitlab/git/hooks_service_spec.rb index e9c0209fe3b..d4d75b66659 100644 --- a/spec/lib/gitlab/git/hooks_service_spec.rb +++ b/spec/lib/gitlab/git/hooks_service_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::Git::HooksService, seed_helper: true do - let(:committer) { Gitlab::Git::Committer.new('Jane Doe', 'janedoe@example.com', 'user-456') } + let(:user) { Gitlab::Git::User.new('Jane Doe', 'janedoe@example.com', 'user-456') } let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, 'project-123') } let(:service) { described_class.new } @@ -18,7 +18,7 @@ describe Gitlab::Git::HooksService, seed_helper: true do hook = double(trigger: [true, nil]) expect(Gitlab::Git::Hook).to receive(:new).exactly(3).times.and_return(hook) - service.execute(committer, repository, @blankrev, @newrev, @ref) { } + service.execute(user, repository, @blankrev, @newrev, @ref) { } end end @@ -28,7 +28,7 @@ describe Gitlab::Git::HooksService, seed_helper: true do expect(service).not_to receive(:run_hook).with('post-receive') expect do - service.execute(committer, repository, @blankrev, @newrev, @ref) + service.execute(user, repository, @blankrev, @newrev, @ref) end.to raise_error(Gitlab::Git::HooksService::PreReceiveError) end end @@ -40,7 +40,7 @@ describe Gitlab::Git::HooksService, seed_helper: true do expect(service).not_to receive(:run_hook).with('post-receive') expect do - service.execute(committer, repository, @blankrev, @newrev, @ref) + service.execute(user, repository, @blankrev, @newrev, @ref) end.to raise_error(Gitlab::Git::HooksService::PreReceiveError) end end diff --git a/spec/lib/gitlab/git/user_spec.rb b/spec/lib/gitlab/git/user_spec.rb new file mode 100644 index 00000000000..0ebcecb26c0 --- /dev/null +++ b/spec/lib/gitlab/git/user_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe Gitlab::Git::User do + let(:name) { 'Jane Doe' } + let(:email) { 'janedoe@example.com' } + let(:gl_id) { 'user-123' } + + subject { described_class.new(name, email, gl_id) } + + describe '#==' do + def eq_other(name, email, gl_id) + eq(described_class.new(name, email, gl_id)) + end + + it { expect(subject).to eq_other(name, email, gl_id) } + + it { expect(subject).not_to eq_other(nil, nil, nil) } + it { expect(subject).not_to eq_other(name + 'x', email, gl_id) } + it { expect(subject).not_to eq_other(name, email + 'x', gl_id) } + it { expect(subject).not_to eq_other(name, email, gl_id + 'x') } + end +end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 7065d467ec0..5df0f75c49d 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -8,7 +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(:committer) { Gitlab::Git::User.from_gitlab(user) } let(:commit_options) do author = repository.user_to_committer(user) -- cgit v1.2.1