Commit 1883e320 authored by Luke Duncalfe's avatar Luke Duncalfe

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.
parent 867ac4d1
...@@ -37,7 +37,7 @@ module Ci ...@@ -37,7 +37,7 @@ module Ci
variables_attributes: params[:variables_attributes], variables_attributes: params[:variables_attributes],
project: project, project: project,
current_user: current_user, current_user: current_user,
push_options: params[:push_options], push_options: params[:push_options] || {},
chat_data: params[:chat_data], chat_data: params[:chat_data],
**extra_options(options)) **extra_options(options))
......
...@@ -79,7 +79,7 @@ module Git ...@@ -79,7 +79,7 @@ module Git
limited_commits, limited_commits,
event_message, event_message,
commits_count: commits_count, 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 # Dependent code may modify the push data, so return a duplicate each time
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
class PostReceive class PostReceive
include ApplicationWorker 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) project, repo_type = Gitlab::GlRepository.parse(gl_repository)
if project.nil? if project.nil?
......
...@@ -262,7 +262,7 @@ module API ...@@ -262,7 +262,7 @@ module API
push_options = Gitlab::PushOptions.new(params[:push_options]) push_options = Gitlab::PushOptions.new(params[:push_options])
PostReceive.perform_async(params[:gl_repository], params[:identifier], 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)) if (mr_options = push_options.get(:merge_request))
begin begin
......
...@@ -8,7 +8,6 @@ module Gitlab ...@@ -8,7 +8,6 @@ module Gitlab
include ::Gitlab::Utils::StrongMemoize include ::Gitlab::Utils::StrongMemoize
SKIP_PATTERN = /\[(ci[ _-]skip|skip[ _-]ci)\]/i SKIP_PATTERN = /\[(ci[ _-]skip|skip[ _-]ci)\]/i
SKIP_PUSH_OPTION = 'ci.skip'
def perform! def perform!
if skipped? if skipped?
...@@ -35,7 +34,8 @@ module Gitlab ...@@ -35,7 +34,8 @@ module Gitlab
end end
def push_option_skips_ci? 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 end
end end
......
...@@ -32,10 +32,7 @@ module Gitlab ...@@ -32,10 +32,7 @@ module Gitlab
} }
], ],
total_commits_count: 1, total_commits_count: 1,
push_options: [ push_options: { ci: { skip: true } }
"ci.skip",
"custom option"
]
}.freeze }.freeze
# Produce a hash of post-receive data # Produce a hash of post-receive data
...@@ -57,11 +54,11 @@ module Gitlab ...@@ -57,11 +54,11 @@ module Gitlab
# }, # },
# commits: Array, # commits: Array,
# total_commits_count: Fixnum, # total_commits_count: Fixnum,
# push_options: Array # push_options: Hash
# } # }
# #
# rubocop:disable Metrics/ParameterLists # 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) commits = Array(commits)
# Total commits count # Total commits count
......
...@@ -5,7 +5,7 @@ module Gitlab ...@@ -5,7 +5,7 @@ module Gitlab
include Gitlab::Identifier include Gitlab::Identifier
attr_reader :project, :identifier, :changes, :push_options attr_reader :project, :identifier, :changes, :push_options
def initialize(project, identifier, changes, push_options) def initialize(project, identifier, changes, push_options = {})
@project = project @project = project
@identifier = identifier @identifier = identifier
@changes = deserialize_changes(changes) @changes = deserialize_changes(changes)
......
...@@ -27,6 +27,11 @@ module Gitlab ...@@ -27,6 +27,11 @@ module Gitlab
options.dig(*args) options.dig(*args)
end end
# Allow #to_json serialization
def as_json(*_args)
options
end
private private
def parse_options(raw_options) def parse_options(raw_options)
......
...@@ -39,6 +39,18 @@ describe Gitlab::PushOptions do ...@@ -39,6 +39,18 @@ describe Gitlab::PushOptions do
end end
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 it 'can parse multiple push options' do
options = described_class.new([ options = described_class.new([
'merge_request.create', 'merge_request.create',
......
...@@ -905,7 +905,7 @@ describe API::Internal do ...@@ -905,7 +905,7 @@ describe API::Internal do
it 'enqueues a PostReceive worker job' do it 'enqueues a PostReceive worker job' do
expect(PostReceive).to receive(:perform_async) 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 post api('/internal/post_receive'), params: valid_params
end end
......
...@@ -418,8 +418,7 @@ describe Ci::CreatePipelineService do ...@@ -418,8 +418,7 @@ describe Ci::CreatePipelineService do
context 'when push options contain ci.skip' do context 'when push options contain ci.skip' do
let(:push_options) do let(:push_options) do
['ci.skip', { 'ci' => { 'skip' => true } }
'another push option']
end end
it 'creates a pipline in the skipped state' do it 'creates a pipline in the skipped state' do
......
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