diff options
author | Alejandro RodrÃguez <alejorro70@gmail.com> | 2018-01-03 22:47:25 -0300 |
---|---|---|
committer | Alejandro RodrÃguez <alejorro70@gmail.com> | 2018-01-04 14:08:02 -0300 |
commit | bc4ad7ed01b144bbea4bb32db9dce3abac22f32e (patch) | |
tree | 7d4a36adb944a60723d5b16b6cfd53aee58e6d4e | |
parent | d83c4049de46b44c2ac8140345d74fb8f0487370 (diff) | |
download | gitlab-ce-gitaly-multi-action-prep.tar.gz |
Move git operations for multi_action into Gitlab::Gitgitaly-multi-action-prep
-rw-r--r-- | app/models/repository.rb | 67 | ||||
-rw-r--r-- | app/services/files/multi_service.rb | 15 | ||||
-rw-r--r-- | lib/gitlab/git/index.rb | 12 | ||||
-rw-r--r-- | lib/gitlab/git/repository.rb | 36 | ||||
-rw-r--r-- | spec/models/repository_spec.rb | 46 | ||||
-rw-r--r-- | spec/services/files/multi_service_spec.rb | 4 |
6 files changed, 70 insertions, 110 deletions
diff --git a/app/models/repository.rb b/app/models/repository.rb index b1fd981965c..c2e5a51aa7f 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -781,34 +781,30 @@ class Repository end def create_dir(user, path, **options) - options[:user] = user options[:actions] = [{ action: :create_dir, file_path: path }] - multi_action(**options) + multi_action(user, **options) end def create_file(user, path, content, **options) - options[:user] = user options[:actions] = [{ action: :create, file_path: path, content: content }] - multi_action(**options) + multi_action(user, **options) end def update_file(user, path, content, **options) previous_path = options.delete(:previous_path) action = previous_path && previous_path != path ? :move : :update - options[:user] = user options[:actions] = [{ action: action, file_path: path, previous_path: previous_path, content: content }] - multi_action(**options) + multi_action(user, **options) end def delete_file(user, path, **options) - options[:user] = user options[:actions] = [{ action: :delete, file_path: path }] - multi_action(**options) + multi_action(user, **options) end def with_cache_hooks @@ -822,59 +818,14 @@ class Repository result.newrev end - def with_branch(user, *args) - with_cache_hooks do - Gitlab::Git::OperationService.new(user, raw_repository).with_branch(*args) do |start_commit| - yield start_commit - end - end - end - - # rubocop:disable Metrics/ParameterLists - def multi_action( - user:, branch_name:, message:, actions:, - author_email: nil, author_name: nil, - start_branch_name: nil, start_project: project) - - with_branch( - user, - branch_name, - start_branch_name: start_branch_name, - start_repository: start_project.repository.raw_repository) do |start_commit| - - index = Gitlab::Git::Index.new(raw_repository) - - if start_commit - index.read_tree(start_commit.rugged_commit.tree) - parents = [start_commit.sha] - else - parents = [] - end + def multi_action(user, **options) + start_project = options.delete(:start_project) - actions.each do |options| - index.public_send(options.delete(:action), options) # rubocop:disable GitlabSecurity/PublicSend - end - - options = { - tree: index.write_tree, - message: message, - parents: parents - } - options.merge!(get_committer_and_author(user, email: author_email, name: author_name)) - - create_commit(options) + if start_project + options[:start_repository] = start_project.repository.raw_repository end - end - # rubocop:enable Metrics/ParameterLists - def get_committer_and_author(user, email: nil, name: nil) - committer = user_to_committer(user) - author = Gitlab::Git.committer_hash(email: email, name: name) || committer - - { - author: author, - committer: committer - } + with_cache_hooks { raw.multi_action(user, **options) } end def can_be_merged?(source_sha, target_branch) diff --git a/app/services/files/multi_service.rb b/app/services/files/multi_service.rb index 98a3e83c130..a03c59f569d 100644 --- a/app/services/files/multi_service.rb +++ b/app/services/files/multi_service.rb @@ -4,7 +4,7 @@ module Files def create_commit! repository.multi_action( - user: current_user, + current_user, message: @commit_message, branch_name: @branch_name, actions: params[:actions], @@ -13,6 +13,8 @@ module Files start_project: @start_project, start_branch_name: @start_branch ) + rescue ArgumentError => e + raise_error(e) end private @@ -20,16 +22,7 @@ module Files def validate! super - params[:actions].each do |action| - validate_action!(action) - validate_file_status!(action) - end - end - - def validate_action!(action) - unless Gitlab::Git::Index::ACTIONS.include?(action[:action].to_s) - raise_error("Unknown action '#{action[:action]}'") - end + params[:actions].each { |action| validate_file_status!(action) } end def validate_file_status!(action) diff --git a/lib/gitlab/git/index.rb b/lib/gitlab/git/index.rb index db532600d1b..d94082a3e30 100644 --- a/lib/gitlab/git/index.rb +++ b/lib/gitlab/git/index.rb @@ -10,6 +10,7 @@ module Gitlab DEFAULT_MODE = 0o100644 ACTIONS = %w(create create_dir update move delete).freeze + ACTION_OPTIONS = %i(file_path previous_path content encoding).freeze attr_reader :repository, :raw_index @@ -20,6 +21,11 @@ module Gitlab delegate :read_tree, :get, to: :raw_index + def apply(action, options) + validate_action!(action) + public_send(action, options.slice(*ACTION_OPTIONS)) # rubocop:disable GitlabSecurity/PublicSend + end + def write_tree raw_index.write_tree(repository.rugged) end @@ -140,6 +146,12 @@ module Gitlab rescue Rugged::IndexError => e raise IndexError, e.message end + + def validate_action!(action) + unless ACTIONS.include?(action.to_s) + raise ArgumentError, "Unknown action '#{action}'" + end + end end end end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index aec85f971ca..e0e8d6a52db 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -1291,6 +1291,42 @@ module Gitlab success || gitlab_projects_error end + # rubocop:disable Metrics/ParameterLists + def multi_action( + user, branch_name:, message:, actions:, + author_email: nil, author_name: nil, + start_branch_name: nil, start_repository: self) + + OperationService.new(user, self).with_branch(branch_name, + start_branch_name: start_branch_name, + start_repository: start_repository) do |start_commit| + + index = Gitlab::Git::Index.new(self) + + if start_commit + index.read_tree(start_commit.rugged_commit.tree) + parents = [start_commit.sha] + else + parents = [] + end + + actions.each { |opts| index.apply(opts.delete(:action), opts) } + + committer = user_to_committer(user) + author = Gitlab::Git.committer_hash(email: author_email, name: author_name) || committer + options = { + tree: index.write_tree, + message: message, + parents: parents, + author: author, + committer: committer + } + + create_commit(options) + end + end + # rubocop:enable Metrics/ParameterLists + def gitaly_repository Gitlab::GitalyClient::Util.repository(@storage, @relative_path, @gl_repository) end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 9a68ae086ea..c74846eb5c5 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -582,38 +582,6 @@ describe Repository do end end - describe '#get_committer_and_author' do - it 'returns the committer and author data' do - options = repository.get_committer_and_author(user) - expect(options[:committer][:email]).to eq(user.email) - expect(options[:author][:email]).to eq(user.email) - end - - context 'when the email/name are given' do - it 'returns an object containing the email/name' do - options = repository.get_committer_and_author(user, email: author_email, name: author_name) - expect(options[:author][:email]).to eq(author_email) - expect(options[:author][:name]).to eq(author_name) - end - end - - context 'when the email is given but the name is not' do - it 'returns the committer as the author' do - options = repository.get_committer_and_author(user, email: author_email) - expect(options[:author][:email]).to eq(user.email) - expect(options[:author][:name]).to eq(user.name) - end - end - - context 'when the name is given but the email is not' do - it 'returns nil' do - options = repository.get_committer_and_author(user, name: author_name) - expect(options[:author][:email]).to eq(user.email) - expect(options[:author][:name]).to eq(user.name) - end - end - end - describe "search_files_by_content" do let(:results) { repository.search_files_by_content('feature', 'master') } subject { results } @@ -1112,16 +1080,16 @@ describe Repository do allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, '']) end - it 'expires branch cache' do - expect(repository).not_to receive(:expire_exists_cache) - expect(repository).not_to receive(:expire_root_ref_cache) - expect(repository).not_to receive(:expire_emptiness_caches) - expect(repository).to receive(:expire_branches_cache) - - repository.with_branch(user, 'new-feature') do + subject do + Gitlab::Git::OperationService.new(git_user, repository.raw_repository).with_branch('new-feature') do new_rev end end + + it 'returns branch_created as true' do + expect(subject).not_to be_repo_created + expect(subject).to be_branch_created + end end context 'when repository is empty' do diff --git a/spec/services/files/multi_service_spec.rb b/spec/services/files/multi_service_spec.rb index 2b79609930c..b9971776b33 100644 --- a/spec/services/files/multi_service_spec.rb +++ b/spec/services/files/multi_service_spec.rb @@ -41,7 +41,7 @@ describe Files::MultiService do describe '#execute' do context 'with a valid action' do - it 'returns a hash with the :success status ' do + it 'returns a hash with the :success status' do results = subject.execute expect(results[:status]).to eq(:success) @@ -51,7 +51,7 @@ describe Files::MultiService do context 'with an invalid action' do let(:action) { 'rename' } - it 'returns a hash with the :error status ' do + it 'returns a hash with the :error status' do results = subject.execute expect(results[:status]).to eq(:error) |