diff options
author | Sean McGivern <sean@mcgivern.me.uk> | 2017-08-24 14:32:17 +0000 |
---|---|---|
committer | Sean McGivern <sean@mcgivern.me.uk> | 2017-08-24 14:32:17 +0000 |
commit | e46a3d2fda47f96885b38c6c6caf7dc3e0df19bd (patch) | |
tree | 05d362e372d1d84371977fcb8868beffa5fc612f | |
parent | 88892e43adecd68de7940fdc9856a4557098cb6c (diff) | |
parent | a1a914994f6376b03d95e9d4b05dea422e8610f9 (diff) | |
download | gitlab-ce-e46a3d2fda47f96885b38c6c6caf7dc3e0df19bd.tar.gz |
Merge branch 'git-operation-user' into 'master'
Move GitHooksService into Gitlab::Git
See merge request !13739
37 files changed, 180 insertions, 127 deletions
diff --git a/app/models/repository.rb b/app/models/repository.rb index cb2cf658084..9fb2e2aa306 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -1192,7 +1192,7 @@ class Repository end def initialize_raw_repository - Gitlab::Git::Repository.new(project.repository_storage, disk_path + '.git') + Gitlab::Git::Repository.new(project.repository_storage, disk_path + '.git', Gitlab::GlRepository.gl_repository(project, false)) end def circuit_breaker diff --git a/app/services/commits/create_service.rb b/app/services/commits/create_service.rb index c58f04a252b..dbd0b9ef43a 100644 --- a/app/services/commits/create_service.rb +++ b/app/services/commits/create_service.rb @@ -17,7 +17,7 @@ module Commits new_commit = create_commit! success(result: new_commit) - rescue ValidationError, ChangeError, Gitlab::Git::Index::IndexError, Repository::CommitError, GitHooksService::PreReceiveError => ex + rescue ValidationError, ChangeError, Gitlab::Git::Index::IndexError, Repository::CommitError, Gitlab::Git::HooksService::PreReceiveError => ex error(ex.message) end diff --git a/app/services/create_branch_service.rb b/app/services/create_branch_service.rb index 673ed02f952..0ba6a0ac6b5 100644 --- a/app/services/create_branch_service.rb +++ b/app/services/create_branch_service.rb @@ -14,7 +14,7 @@ class CreateBranchService < BaseService else error('Invalid reference name') end - rescue GitHooksService::PreReceiveError => ex + rescue Gitlab::Git::HooksService::PreReceiveError => ex error(ex.message) end diff --git a/app/services/delete_branch_service.rb b/app/services/delete_branch_service.rb index 64b3c0118fb..1f059c31944 100644 --- a/app/services/delete_branch_service.rb +++ b/app/services/delete_branch_service.rb @@ -16,7 +16,7 @@ class DeleteBranchService < BaseService else error('Failed to remove branch') end - rescue GitHooksService::PreReceiveError => ex + rescue Gitlab::Git::HooksService::PreReceiveError => ex error(ex.message) end diff --git a/app/services/git_hooks_service.rb b/app/services/git_hooks_service.rb deleted file mode 100644 index eab65d09299..00000000000 --- a/app/services/git_hooks_service.rb +++ /dev/null @@ -1,32 +0,0 @@ -class GitHooksService - PreReceiveError = Class.new(StandardError) - - attr_accessor :oldrev, :newrev, :ref - - def execute(user, project, oldrev, newrev, ref) - @project = project - @user = Gitlab::GlId.gl_id(user) - @oldrev = oldrev - @newrev = newrev - @ref = ref - - %w(pre-receive update).each do |hook_name| - status, message = run_hook(hook_name) - - unless status - raise PreReceiveError, message - end - end - - yield(self).tap do - run_hook('post-receive') - end - end - - private - - def run_hook(name) - hook = Gitlab::Git::Hook.new(name, @project) - hook.trigger(@user, oldrev, newrev, ref) - end -end diff --git a/app/services/git_operation_service.rb b/app/services/git_operation_service.rb index 545ca0742e4..6b7a56e6922 100644 --- a/app/services/git_operation_service.rb +++ b/app/services/git_operation_service.rb @@ -1,8 +1,10 @@ class GitOperationService - attr_reader :user, :repository + attr_reader :committer, :repository + + def initialize(committer, new_repository) + committer = Gitlab::Git::Committer.from_user(committer) if committer.is_a?(User) + @committer = committer - def initialize(new_user, new_repository) - @user = new_user @repository = new_repository end @@ -118,9 +120,9 @@ class GitOperationService end def with_hooks(ref, newrev, oldrev) - GitHooksService.new.execute( - user, - repository.project, + Gitlab::Git::HooksService.new.execute( + committer, + repository, oldrev, newrev, ref) do |service| diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index bc846e07f24..5be749cd6a0 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -49,7 +49,7 @@ module MergeRequests raise MergeError, 'Conflicts detected during merge' unless commit_id merge_request.update(merge_commit_sha: commit_id) - rescue GitHooksService::PreReceiveError => e + rescue Gitlab::Git::HooksService::PreReceiveError => e raise MergeError, e.message rescue StandardError => e raise MergeError, "Something went wrong during merge: #{e.message}" diff --git a/app/services/tags/create_service.rb b/app/services/tags/create_service.rb index 674792f6138..b3f4a72d6fe 100644 --- a/app/services/tags/create_service.rb +++ b/app/services/tags/create_service.rb @@ -13,7 +13,7 @@ module Tags new_tag = repository.add_tag(current_user, tag_name, target, message) rescue Rugged::TagError return error("Tag #{tag_name} already exists") - rescue GitHooksService::PreReceiveError => ex + rescue Gitlab::Git::HooksService::PreReceiveError => ex return error(ex.message) end diff --git a/app/services/tags/destroy_service.rb b/app/services/tags/destroy_service.rb index a368f4f5b61..d3d46064729 100644 --- a/app/services/tags/destroy_service.rb +++ b/app/services/tags/destroy_service.rb @@ -21,7 +21,7 @@ module Tags else error('Failed to remove tag') end - rescue GitHooksService::PreReceiveError => ex + rescue Gitlab::Git::HooksService::PreReceiveError => ex error(ex.message) end diff --git a/app/services/validate_new_branch_service.rb b/app/services/validate_new_branch_service.rb index d232e85cd33..7d1ed768ee8 100644 --- a/app/services/validate_new_branch_service.rb +++ b/app/services/validate_new_branch_service.rb @@ -13,7 +13,7 @@ class ValidateNewBranchService < BaseService end success - rescue GitHooksService::PreReceiveError => ex + rescue Gitlab::Git::HooksService::PreReceiveError => ex error(ex.message) end end diff --git a/db/fixtures/development/17_cycle_analytics.rb b/db/fixtures/development/17_cycle_analytics.rb index 7c1d758dada..383782112a8 100644 --- a/db/fixtures/development/17_cycle_analytics.rb +++ b/db/fixtures/development/17_cycle_analytics.rb @@ -15,7 +15,7 @@ class Gitlab::Seeder::CycleAnalytics # to disable the `pre_receive` hook in order to remove this # dependency on the GitLab API. def stub_git_pre_receive! - GitHooksService.class_eval do + Gitlab::Git::HooksService.class_eval do def run_hook(name) [true, ''] end diff --git a/db/migrate/20140502125220_migrate_repo_size.rb b/db/migrate/20140502125220_migrate_repo_size.rb index f5d5d834307..ca1b054600c 100644 --- a/db/migrate/20140502125220_migrate_repo_size.rb +++ b/db/migrate/20140502125220_migrate_repo_size.rb @@ -11,7 +11,7 @@ class MigrateRepoSize < ActiveRecord::Migration path = File.join(namespace_path, project['project_path'] + '.git') begin - repo = Gitlab::Git::Repository.new('default', path) + repo = Gitlab::Git::Repository.new('default', path, '') if repo.empty? print '-' else diff --git a/lib/gitlab/git/committer.rb b/lib/gitlab/git/committer.rb new file mode 100644 index 00000000000..1f4bcf7a3a0 --- /dev/null +++ b/lib/gitlab/git/committer.rb @@ -0,0 +1,21 @@ +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/hook.rb b/lib/gitlab/git/hook.rb index 8f0c377ef4f..cc35d77c6e4 100644 --- a/lib/gitlab/git/hook.rb +++ b/lib/gitlab/git/hook.rb @@ -1,20 +1,23 @@ -# Gitaly note: JV: looks like this is only used by GitHooksService in +# Gitaly note: JV: looks like this is only used by Gitlab::Git::HooksService in # app/services. We shouldn't bother migrating this until we know how -# GitHooksService will be migrated. +# Gitlab::Git::HooksService will be migrated. module Gitlab module Git class Hook GL_PROTOCOL = 'web'.freeze - attr_reader :name, :repo_path, :path + attr_reader :name, :path, :repository - def initialize(name, project) + def initialize(name, repository) @name = name - @project = project - @repo_path = project.repository.path + @repository = repository @path = File.join(repo_path.strip, 'hooks', name) end + def repo_path + repository.path + end + def exists? File.exist?(path) end @@ -44,7 +47,7 @@ module Gitlab 'GL_ID' => gl_id, 'PWD' => repo_path, 'GL_PROTOCOL' => GL_PROTOCOL, - 'GL_REPOSITORY' => Gitlab::GlRepository.gl_repository(@project, false) + 'GL_REPOSITORY' => repository.gl_repository } options = { diff --git a/lib/gitlab/git/hooks_service.rb b/lib/gitlab/git/hooks_service.rb new file mode 100644 index 00000000000..ea8a87a1290 --- /dev/null +++ b/lib/gitlab/git/hooks_service.rb @@ -0,0 +1,36 @@ +module Gitlab + module Git + class HooksService + PreReceiveError = Class.new(StandardError) + + attr_accessor :oldrev, :newrev, :ref + + def execute(committer, repository, oldrev, newrev, ref) + @repository = repository + @gl_id = committer.gl_id + @oldrev = oldrev + @newrev = newrev + @ref = ref + + %w(pre-receive update).each do |hook_name| + status, message = run_hook(hook_name) + + unless status + raise PreReceiveError, message + end + end + + yield(self).tap do + run_hook('post-receive') + end + end + + private + + def run_hook(name) + hook = Gitlab::Git::Hook.new(name, @repository) + hook.trigger(@gl_id, oldrev, newrev, ref) + end + end + end +end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 860ed01c05d..b835dec24eb 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -49,13 +49,14 @@ module Gitlab # Rugged repo object attr_reader :rugged - attr_reader :storage + attr_reader :storage, :gl_repository, :relative_path # 'path' must be the path to a _bare_ git repository, e.g. # /path/to/my-repo.git - def initialize(storage, relative_path) + def initialize(storage, relative_path, gl_repository) @storage = storage @relative_path = relative_path + @gl_repository = gl_repository storage_path = Gitlab.config.repositories.storages[@storage]['path'] @path = File.join(storage_path, @relative_path) diff --git a/spec/features/projects/import_export/import_file_spec.rb b/spec/features/projects/import_export/import_file_spec.rb index 9f2c86923b7..2eb6fab129d 100644 --- a/spec/features/projects/import_export/import_file_spec.rb +++ b/spec/features/projects/import_export/import_file_spec.rb @@ -99,6 +99,6 @@ feature 'Import/Export - project import integration test', js: true do end def project_hook_exists?(project) - Gitlab::Git::Hook.new('post-receive', project).exists? + Gitlab::Git::Hook.new('post-receive', project.repository.raw_repository).exists? end end diff --git a/spec/features/tags/master_deletes_tag_spec.rb b/spec/features/tags/master_deletes_tag_spec.rb index 4d6fc13557f..d6a6b8fc7d5 100644 --- a/spec/features/tags/master_deletes_tag_spec.rb +++ b/spec/features/tags/master_deletes_tag_spec.rb @@ -36,8 +36,8 @@ feature 'Master deletes tag' do context 'when pre-receive hook fails', js: true do before do - allow_any_instance_of(GitHooksService).to receive(:execute) - .and_raise(GitHooksService::PreReceiveError, 'Do not delete tags') + allow_any_instance_of(Gitlab::Git::HooksService).to receive(:execute) + .and_raise(Gitlab::Git::HooksService::PreReceiveError, 'Do not delete tags') end scenario 'shows the error message' do diff --git a/spec/lib/gitlab/git/blame_spec.rb b/spec/lib/gitlab/git/blame_spec.rb index 800c245b130..465c2012b05 100644 --- a/spec/lib/gitlab/git/blame_spec.rb +++ b/spec/lib/gitlab/git/blame_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" describe Gitlab::Git::Blame, seed_helper: true do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } let(:blame) do Gitlab::Git::Blame.new(repository, SeedRepo::Commit::ID, "CONTRIBUTING.md") end diff --git a/spec/lib/gitlab/git/blob_spec.rb b/spec/lib/gitlab/git/blob_spec.rb index dfab0c2fe85..66ba00acb7d 100644 --- a/spec/lib/gitlab/git/blob_spec.rb +++ b/spec/lib/gitlab/git/blob_spec.rb @@ -3,7 +3,7 @@ require "spec_helper" describe Gitlab::Git::Blob, seed_helper: true do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } describe 'initialize' do let(:blob) { Gitlab::Git::Blob.new(name: 'test') } diff --git a/spec/lib/gitlab/git/branch_spec.rb b/spec/lib/gitlab/git/branch_spec.rb index cdf1b8beee3..318a7b7a332 100644 --- a/spec/lib/gitlab/git/branch_spec.rb +++ b/spec/lib/gitlab/git/branch_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Branch, seed_helper: true do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } subject { repository.branches } diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index ac33cd8a2c9..14d64d8c4da 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Commit, seed_helper: true do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } let(:commit) { described_class.find(repository, SeedRepo::Commit::ID) } let(:rugged_commit) do repository.rugged.lookup(SeedRepo::Commit::ID) @@ -9,7 +9,7 @@ describe Gitlab::Git::Commit, seed_helper: true do describe "Commit info" do before do - repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH).rugged + repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '').rugged @committer = { email: 'mike@smith.com', @@ -59,7 +59,7 @@ describe Gitlab::Git::Commit, seed_helper: true do after do # Erase the new commit so other tests get the original repo - repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH).rugged + repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '').rugged repo.references.update("refs/heads/master", SeedRepo::LastCommit::ID) end end @@ -144,7 +144,7 @@ describe Gitlab::Git::Commit, seed_helper: true do end context 'with broken repo' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_BROKEN_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_BROKEN_REPO_PATH, '') } it 'returns nil' do expect(described_class.find(repository, SeedRepo::Commit::ID)).to be_nil diff --git a/spec/lib/gitlab/git/committer_spec.rb b/spec/lib/gitlab/git/committer_spec.rb new file mode 100644 index 00000000000..b0ddbb51449 --- /dev/null +++ b/spec/lib/gitlab/git/committer_spec.rb @@ -0,0 +1,22 @@ +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/compare_spec.rb b/spec/lib/gitlab/git/compare_spec.rb index 4c9f4a28f32..b6a42e422b5 100644 --- a/spec/lib/gitlab/git/compare_spec.rb +++ b/spec/lib/gitlab/git/compare_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Compare, seed_helper: true do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } let(:compare) { Gitlab::Git::Compare.new(repository, SeedRepo::BigCommit::ID, SeedRepo::Commit::ID, straight: false) } let(:compare_straight) { Gitlab::Git::Compare.new(repository, SeedRepo::BigCommit::ID, SeedRepo::Commit::ID, straight: true) } diff --git a/spec/lib/gitlab/git/diff_spec.rb b/spec/lib/gitlab/git/diff_spec.rb index 7ea3386ac2a..dfbdbee48f7 100644 --- a/spec/lib/gitlab/git/diff_spec.rb +++ b/spec/lib/gitlab/git/diff_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Diff, seed_helper: true do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } before do @raw_diff_hash = { diff --git a/spec/lib/gitlab/git/hook_spec.rb b/spec/lib/gitlab/git/hook_spec.rb index 19391a70cf6..ea3e4680b1d 100644 --- a/spec/lib/gitlab/git/hook_spec.rb +++ b/spec/lib/gitlab/git/hook_spec.rb @@ -10,7 +10,8 @@ describe Gitlab::Git::Hook do describe "#trigger" do let(:project) { create(:project, :repository) } - let(:repo_path) { project.repository.path } + let(:repository) { project.repository.raw_repository } + let(:repo_path) { repository.path } let(:user) { create(:user) } let(:gl_id) { Gitlab::GlId.gl_id(user) } @@ -48,7 +49,7 @@ describe Gitlab::Git::Hook do it "returns success with no errors" do create_hook(hook_name) - hook = described_class.new(hook_name, project) + hook = described_class.new(hook_name, repository) blank = Gitlab::Git::BLANK_SHA ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch' @@ -66,7 +67,7 @@ describe Gitlab::Git::Hook do context "when the hook is unsuccessful" do it "returns failure with errors" do create_failing_hook(hook_name) - hook = described_class.new(hook_name, project) + hook = described_class.new(hook_name, repository) blank = Gitlab::Git::BLANK_SHA ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch' @@ -80,7 +81,7 @@ describe Gitlab::Git::Hook do context "when the hook doesn't exist" do it "returns success with no errors" do - hook = described_class.new('unknown_hook', project) + hook = described_class.new('unknown_hook', repository) blank = Gitlab::Git::BLANK_SHA ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch' diff --git a/spec/services/git_hooks_service_spec.rb b/spec/lib/gitlab/git/hooks_service_spec.rb index 3ce01a995b4..e9c0209fe3b 100644 --- a/spec/services/git_hooks_service_spec.rb +++ b/spec/lib/gitlab/git/hooks_service_spec.rb @@ -1,16 +1,14 @@ require 'spec_helper' -describe GitHooksService do - include RepoHelpers - - let(:user) { create(:user) } - let(:project) { create(:project, :repository) } +describe Gitlab::Git::HooksService, seed_helper: true do + let(:committer) { Gitlab::Git::Committer.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 } before do @blankrev = Gitlab::Git::BLANK_SHA - @oldrev = sample_commit.parent_id - @newrev = sample_commit.id + @oldrev = SeedRepo::Commit::PARENT_ID + @newrev = SeedRepo::Commit::ID @ref = 'refs/heads/feature' end @@ -20,7 +18,7 @@ describe GitHooksService do hook = double(trigger: [true, nil]) expect(Gitlab::Git::Hook).to receive(:new).exactly(3).times.and_return(hook) - service.execute(user, project, @blankrev, @newrev, @ref) { } + service.execute(committer, repository, @blankrev, @newrev, @ref) { } end end @@ -30,8 +28,8 @@ describe GitHooksService do expect(service).not_to receive(:run_hook).with('post-receive') expect do - service.execute(user, project, @blankrev, @newrev, @ref) - end.to raise_error(GitHooksService::PreReceiveError) + service.execute(committer, repository, @blankrev, @newrev, @ref) + end.to raise_error(Gitlab::Git::HooksService::PreReceiveError) end end @@ -42,8 +40,8 @@ describe GitHooksService do expect(service).not_to receive(:run_hook).with('post-receive') expect do - service.execute(user, project, @blankrev, @newrev, @ref) - end.to raise_error(GitHooksService::PreReceiveError) + service.execute(committer, repository, @blankrev, @newrev, @ref) + end.to raise_error(Gitlab::Git::HooksService::PreReceiveError) end end end diff --git a/spec/lib/gitlab/git/index_spec.rb b/spec/lib/gitlab/git/index_spec.rb index 21b71654251..73fbc6a6afa 100644 --- a/spec/lib/gitlab/git/index_spec.rb +++ b/spec/lib/gitlab/git/index_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::Git::Index, seed_helper: true do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } let(:index) { described_class.new(repository) } before do diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 62da733170c..b84f54c87fc 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -17,7 +17,7 @@ describe Gitlab::Git::Repository, seed_helper: true do end end - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } describe "Respond to" do subject { repository } @@ -56,14 +56,14 @@ describe Gitlab::Git::Repository, seed_helper: true do describe "#rugged" do describe 'when storage is broken', broken_storage: true do it 'raises a storage exception when storage is not available' do - broken_repo = described_class.new('broken', 'a/path.git') + broken_repo = described_class.new('broken', 'a/path.git', '') expect { broken_repo.rugged }.to raise_error(Gitlab::Git::Storage::Inaccessible) end end it 'raises a no repository exception when there is no repo' do - broken_repo = described_class.new('default', 'a/path.git') + broken_repo = described_class.new('default', 'a/path.git', '') expect { broken_repo.rugged }.to raise_error(Gitlab::Git::Repository::NoRepository) end @@ -257,7 +257,7 @@ describe Gitlab::Git::Repository, seed_helper: true do end describe '#submodule_url_for' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } let(:ref) { 'master' } def submodule_url(path) @@ -295,7 +295,7 @@ describe Gitlab::Git::Repository, seed_helper: true do end context '#submodules' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } context 'where repo has submodules' do let(:submodules) { repository.send(:submodules, 'master') } @@ -391,7 +391,7 @@ describe Gitlab::Git::Repository, seed_helper: true do describe "#delete_branch" do before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH) + @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') @repo.delete_branch("feature") end @@ -407,7 +407,7 @@ describe Gitlab::Git::Repository, seed_helper: true do describe "#create_branch" do before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH) + @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') end it "should create a new branch" do @@ -445,7 +445,7 @@ describe Gitlab::Git::Repository, seed_helper: true do describe "#remote_delete" do before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH) + @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') @repo.remote_delete("expendable") end @@ -461,7 +461,7 @@ describe Gitlab::Git::Repository, seed_helper: true do describe "#remote_add" do before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH) + @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') @repo.remote_add("new_remote", SeedHelper::GITLAB_GIT_TEST_REPO_URL) end @@ -477,7 +477,7 @@ describe Gitlab::Git::Repository, seed_helper: true do describe "#remote_update" do before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH) + @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') @repo.remote_update("expendable", url: TEST_NORMAL_REPO_PATH) end @@ -506,7 +506,7 @@ describe Gitlab::Git::Repository, seed_helper: true do before(:context) do # Add new commits so that there's a renamed file in the commit history - repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH).rugged + repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '').rugged @commit_with_old_name_id = new_commit_edit_old_file(repo) @rename_commit_id = new_commit_move_file(repo) @commit_with_new_name_id = new_commit_edit_new_file(repo) @@ -514,7 +514,7 @@ describe Gitlab::Git::Repository, seed_helper: true do after(:context) do # Erase our commits so other tests get the original repo - repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH).rugged + repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '').rugged repo.references.update("refs/heads/master", SeedRepo::LastCommit::ID) end @@ -849,7 +849,7 @@ describe Gitlab::Git::Repository, seed_helper: true do describe '#autocrlf' do before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH) + @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') @repo.rugged.config['core.autocrlf'] = true end @@ -864,7 +864,7 @@ describe Gitlab::Git::Repository, seed_helper: true do describe '#autocrlf=' do before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH) + @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') @repo.rugged.config['core.autocrlf'] = false end @@ -933,7 +933,7 @@ describe Gitlab::Git::Repository, seed_helper: true do context 'with local and remote branches' do let(:repository) do - Gitlab::Git::Repository.new('default', File.join(TEST_MUTABLE_REPO_PATH, '.git')) + Gitlab::Git::Repository.new('default', File.join(TEST_MUTABLE_REPO_PATH, '.git'), '') end before do @@ -1186,7 +1186,7 @@ describe Gitlab::Git::Repository, seed_helper: true do describe '#local_branches' do before(:all) do - @repo = Gitlab::Git::Repository.new('default', File.join(TEST_MUTABLE_REPO_PATH, '.git')) + @repo = Gitlab::Git::Repository.new('default', File.join(TEST_MUTABLE_REPO_PATH, '.git'), '') end after(:all) do diff --git a/spec/lib/gitlab/git/tag_spec.rb b/spec/lib/gitlab/git/tag_spec.rb index 78d1e120013..cc10679ef1e 100644 --- a/spec/lib/gitlab/git/tag_spec.rb +++ b/spec/lib/gitlab/git/tag_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Tag, seed_helper: true do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } shared_examples 'Gitlab::Git::Repository#tags' do describe 'first tag' do diff --git a/spec/lib/gitlab/git/tree_spec.rb b/spec/lib/gitlab/git/tree_spec.rb index 98ddd3c3664..c07a2d91768 100644 --- a/spec/lib/gitlab/git/tree_spec.rb +++ b/spec/lib/gitlab/git/tree_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Tree, seed_helper: true do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } context :repo do let(:tree) { Gitlab::Git::Tree.where(repository, SeedRepo::Commit::ID) } diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index 80dc49e99cb..295a979da76 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -384,7 +384,7 @@ describe Gitlab::GitAccess do def stub_git_hooks # Running the `pre-receive` hook is expensive, and not necessary for this test. - allow_any_instance_of(GitHooksService).to receive(:execute) do |service, &block| + allow_any_instance_of(Gitlab::Git::HooksService).to receive(:execute) do |service, &block| block.call(service) end end diff --git a/spec/lib/gitlab/import_export/repo_restorer_spec.rb b/spec/lib/gitlab/import_export/repo_restorer_spec.rb index 2786bc92fe5..c49af602a01 100644 --- a/spec/lib/gitlab/import_export/repo_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/repo_restorer_spec.rb @@ -34,7 +34,7 @@ describe Gitlab::ImportExport::RepoRestorer do it 'has the webhooks' do restorer.restore - expect(Gitlab::Git::Hook.new('post-receive', project)).to exist + expect(Gitlab::Git::Hook.new('post-receive', project.repository.raw_repository)).to exist end end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 4926d5d6c49..462e92b8b62 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -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) @@ -846,7 +847,7 @@ describe Repository, models: true do expect do repository.add_branch(user, 'new_feature', 'master') - end.to raise_error(GitHooksService::PreReceiveError) + end.to raise_error(Gitlab::Git::HooksService::PreReceiveError) end it 'does not create the branch' do @@ -854,7 +855,7 @@ describe Repository, models: true do expect do repository.add_branch(user, 'new_feature', 'master') - end.to raise_error(GitHooksService::PreReceiveError) + end.to raise_error(Gitlab::Git::HooksService::PreReceiveError) expect(repository.find_branch('new_feature')).to be_nil end end @@ -884,8 +885,8 @@ 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') + expect_any_instance_of(Gitlab::Git::HooksService).to receive(:execute) + .with(committer, repository, old_rev, blank_sha, 'refs/heads/feature') expect { repository.rm_branch(user, 'feature') }.not_to raise_error end @@ -905,7 +906,7 @@ describe Repository, models: true do expect do repository.rm_branch(user, 'feature') - end.to raise_error(GitHooksService::PreReceiveError) + end.to raise_error(Gitlab::Git::HooksService::PreReceiveError) end it 'does not delete the branch' do @@ -913,7 +914,7 @@ describe Repository, models: true do expect do repository.rm_branch(user, 'feature') - end.to raise_error(GitHooksService::PreReceiveError) + end.to raise_error(Gitlab::Git::HooksService::PreReceiveError) expect(repository.find_branch('feature')).not_to be_nil end end @@ -925,23 +926,23 @@ describe Repository, models: true do context 'when pre hooks were successful' do before do - service = GitHooksService.new - expect(GitHooksService).to receive(:new).and_return(service) + service = Gitlab::Git::HooksService.new + expect(Gitlab::Git::HooksService).to receive(:new).and_return(service) expect(service).to receive(:execute) - .with(user, project, old_rev, new_rev, 'refs/heads/feature') + .with(committer, repository, 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,10 +1027,10 @@ 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) + end.to raise_error(Gitlab::Git::HooksService::PreReceiveError) end end @@ -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 diff --git a/spec/services/files/update_service_spec.rb b/spec/services/files/update_service_spec.rb index cc950ae6bb3..2b4f8cd42ee 100644 --- a/spec/services/files/update_service_spec.rb +++ b/spec/services/files/update_service_spec.rb @@ -76,7 +76,7 @@ describe Files::UpdateService do let(:branch_name) { "#{project.default_branch}-new" } it 'fires hooks only once' do - expect(GitHooksService).to receive(:new).once.and_call_original + expect(Gitlab::Git::HooksService).to receive(:new).once.and_call_original subject.execute end diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index e593bfeeaf7..5cfdb5372f3 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -217,7 +217,7 @@ describe MergeRequests::MergeService do it 'logs and saves error if there is an PreReceiveError exception' do error_message = 'error message' - allow(service).to receive(:repository).and_raise(GitHooksService::PreReceiveError, error_message) + allow(service).to receive(:repository).and_raise(Gitlab::Git::HooksService::PreReceiveError, error_message) allow(service).to receive(:execute_hooks) service.execute(merge_request) diff --git a/spec/services/tags/create_service_spec.rb b/spec/services/tags/create_service_spec.rb index 1b31ce29f7a..57013b54560 100644 --- a/spec/services/tags/create_service_spec.rb +++ b/spec/services/tags/create_service_spec.rb @@ -41,7 +41,7 @@ describe Tags::CreateService do it 'returns an error' do expect(repository).to receive(:add_tag) .with(user, 'v1.1.0', 'master', 'Foo') - .and_raise(GitHooksService::PreReceiveError, 'something went wrong') + .and_raise(Gitlab::Git::HooksService::PreReceiveError, 'something went wrong') response = service.execute('v1.1.0', 'master', 'Foo') |