diff options
author | Luke Duncalfe <lduncalfe@eml.cc> | 2019-04-05 07:19:30 +0300 |
---|---|---|
committer | Luke Duncalfe <lduncalfe@eml.cc> | 2019-04-09 01:03:26 +0300 |
commit | 1883e320eafa02b332a16eec658f65c4a28def83 (patch) | |
tree | 6e231cba20ae8fda69f328e0ede27452fca9cf14 | |
parent | 867ac4d1f733254de1b4c44f314e585e6b2720e8 (diff) |
Use Gitlab::PushOptions for `ci.skip` push option
Previously the raw push option Array was sent to Pipeline::Chain::Skip.
This commit updates this class (and the chain of classes that pass the
push option parameters from the API internal `post_receive` endpoint to
that class) to treat push options as a Hash of options parsed by
GitLab::PushOptions.
The GitLab::PushOptions class takes options like this:
-o ci.skip -o merge_request.create -o merge_request.target=branch
and turns them into a Hash like this:
{
ci: {
skip: true
},
merge_request: {
create: true,
target: 'branch'
}
}
This now how Pipeline::Chain::Skip is determining if the `ci.skip` push
option was used.
-rw-r--r-- | app/services/ci/create_pipeline_service.rb | 2 | ||||
-rw-r--r-- | app/services/git/base_hooks_service.rb | 2 | ||||
-rw-r--r-- | app/workers/post_receive.rb | 2 | ||||
-rw-r--r-- | lib/api/internal.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/ci/pipeline/chain/skip.rb | 4 | ||||
-rw-r--r-- | lib/gitlab/data_builder/push.rb | 9 | ||||
-rw-r--r-- | lib/gitlab/git_post_receive.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/push_options.rb | 5 | ||||
-rw-r--r-- | spec/lib/gitlab/push_options_spec.rb | 12 | ||||
-rw-r--r-- | spec/requests/api/internal_spec.rb | 2 | ||||
-rw-r--r-- | spec/services/ci/create_pipeline_service_spec.rb | 3 |
11 files changed, 29 insertions, 16 deletions
diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 8973c5ffc9e..41dee4e5641 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -37,7 +37,7 @@ module Ci variables_attributes: params[:variables_attributes], project: project, current_user: current_user, - push_options: params[:push_options], + push_options: params[:push_options] || {}, chat_data: params[:chat_data], **extra_options(options)) diff --git a/app/services/git/base_hooks_service.rb b/app/services/git/base_hooks_service.rb index e0dccb8716a..fce4040e390 100644 --- a/app/services/git/base_hooks_service.rb +++ b/app/services/git/base_hooks_service.rb @@ -79,7 +79,7 @@ module Git limited_commits, event_message, commits_count: commits_count, - push_options: params[:push_options] || [] + push_options: params[:push_options] || {} ) # Dependent code may modify the push data, so return a duplicate each time diff --git a/app/workers/post_receive.rb b/app/workers/post_receive.rb index 396f44396a3..a5554f07699 100644 --- a/app/workers/post_receive.rb +++ b/app/workers/post_receive.rb @@ -3,7 +3,7 @@ class PostReceive include ApplicationWorker - def perform(gl_repository, identifier, changes, push_options = []) + def perform(gl_repository, identifier, changes, push_options = {}) project, repo_type = Gitlab::GlRepository.parse(gl_repository) if project.nil? diff --git a/lib/api/internal.rb b/lib/api/internal.rb index 75202fa953c..ee1fb465608 100644 --- a/lib/api/internal.rb +++ b/lib/api/internal.rb @@ -262,7 +262,7 @@ module API push_options = Gitlab::PushOptions.new(params[:push_options]) PostReceive.perform_async(params[:gl_repository], params[:identifier], - params[:changes], params[:push_options].to_a) + params[:changes], push_options.as_json) if (mr_options = push_options.get(:merge_request)) begin diff --git a/lib/gitlab/ci/pipeline/chain/skip.rb b/lib/gitlab/ci/pipeline/chain/skip.rb index 79bbcc1ed1e..7d6e0704d4a 100644 --- a/lib/gitlab/ci/pipeline/chain/skip.rb +++ b/lib/gitlab/ci/pipeline/chain/skip.rb @@ -8,7 +8,6 @@ module Gitlab include ::Gitlab::Utils::StrongMemoize SKIP_PATTERN = /\[(ci[ _-]skip|skip[ _-]ci)\]/i - SKIP_PUSH_OPTION = 'ci.skip' def perform! if skipped? @@ -35,7 +34,8 @@ module Gitlab end def push_option_skips_ci? - !!(@command.push_options&.include?(SKIP_PUSH_OPTION)) + @command.push_options.present? && + @command.push_options.deep_symbolize_keys.dig(:ci, :skip).present? end end end diff --git a/lib/gitlab/data_builder/push.rb b/lib/gitlab/data_builder/push.rb index ea08b5f7eae..af385d7d4ca 100644 --- a/lib/gitlab/data_builder/push.rb +++ b/lib/gitlab/data_builder/push.rb @@ -32,10 +32,7 @@ module Gitlab } ], total_commits_count: 1, - push_options: [ - "ci.skip", - "custom option" - ] + push_options: { ci: { skip: true } } }.freeze # Produce a hash of post-receive data @@ -57,11 +54,11 @@ module Gitlab # }, # commits: Array, # total_commits_count: Fixnum, - # push_options: Array + # push_options: Hash # } # # rubocop:disable Metrics/ParameterLists - def build(project, user, oldrev, newrev, ref, commits = [], message = nil, commits_count: nil, push_options: []) + def build(project, user, oldrev, newrev, ref, commits = [], message = nil, commits_count: nil, push_options: {}) commits = Array(commits) # Total commits count diff --git a/lib/gitlab/git_post_receive.rb b/lib/gitlab/git_post_receive.rb index 426436c2164..d98b85fecc4 100644 --- a/lib/gitlab/git_post_receive.rb +++ b/lib/gitlab/git_post_receive.rb @@ -5,7 +5,7 @@ module Gitlab include Gitlab::Identifier attr_reader :project, :identifier, :changes, :push_options - def initialize(project, identifier, changes, push_options) + def initialize(project, identifier, changes, push_options = {}) @project = project @identifier = identifier @changes = deserialize_changes(changes) diff --git a/lib/gitlab/push_options.rb b/lib/gitlab/push_options.rb index 923aa09527d..7c655c12995 100644 --- a/lib/gitlab/push_options.rb +++ b/lib/gitlab/push_options.rb @@ -27,6 +27,11 @@ module Gitlab options.dig(*args) end + # Allow #to_json serialization + def as_json(*_args) + options + end + private def parse_options(raw_options) diff --git a/spec/lib/gitlab/push_options_spec.rb b/spec/lib/gitlab/push_options_spec.rb index 68b64863c21..fc9e421bea6 100644 --- a/spec/lib/gitlab/push_options_spec.rb +++ b/spec/lib/gitlab/push_options_spec.rb @@ -39,6 +39,18 @@ describe Gitlab::PushOptions do end end + describe '#as_json' do + it 'returns all options' do + options = described_class.new(['merge_request.target=value']) + + expect(options.as_json).to include( + merge_request: { + target: 'value' + } + ) + end + end + it 'can parse multiple push options' do options = described_class.new([ 'merge_request.create', diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb index 3cc0cceeab1..a5a6e08e73b 100644 --- a/spec/requests/api/internal_spec.rb +++ b/spec/requests/api/internal_spec.rb @@ -905,7 +905,7 @@ describe API::Internal do it 'enqueues a PostReceive worker job' do expect(PostReceive).to receive(:perform_async) - .with(gl_repository, identifier, changes, push_options) + .with(gl_repository, identifier, changes, { ci: { skip: true } }) post api('/internal/post_receive'), params: valid_params end diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 866d709d446..101b91e9cd8 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -418,8 +418,7 @@ describe Ci::CreatePipelineService do context 'when push options contain ci.skip' do let(:push_options) do - ['ci.skip', - 'another push option'] + { 'ci' => { 'skip' => true } } end it 'creates a pipline in the skipped state' do |