From a42cce1b966046c21ec48b18435d38e68a20f7fa Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 29 Jul 2016 12:30:38 +0200 Subject: Improve code, remove unused validator, improve names --- lib/gitlab/ci/config/node/cache.rb | 4 +- lib/gitlab/ci/config/node/commands.rb | 12 +++-- lib/gitlab/ci/config/node/configurable.rb | 7 ++- lib/gitlab/ci/config/node/global.rb | 4 +- lib/gitlab/ci/config/node/job.rb | 4 +- lib/gitlab/ci/config/node/null.rb | 2 +- lib/gitlab/ci/config/node/trigger.rb | 26 +++++++++++ lib/gitlab/ci/config/node/undefined.rb | 6 +-- lib/gitlab/ci/config/node/validators.rb | 9 ---- lib/gitlab/ci/config/node/while.rb | 26 ----------- spec/lib/gitlab/ci/config/node/artifacts_spec.rb | 2 +- spec/lib/gitlab/ci/config/node/trigger_spec.rb | 56 ++++++++++++++++++++++++ spec/lib/gitlab/ci/config/node/while_spec.rb | 56 ------------------------ 13 files changed, 100 insertions(+), 114 deletions(-) create mode 100644 lib/gitlab/ci/config/node/trigger.rb delete mode 100644 lib/gitlab/ci/config/node/while.rb create mode 100644 spec/lib/gitlab/ci/config/node/trigger_spec.rb delete mode 100644 spec/lib/gitlab/ci/config/node/while_spec.rb diff --git a/lib/gitlab/ci/config/node/cache.rb b/lib/gitlab/ci/config/node/cache.rb index 21d96b220b8..b4bda2841ac 100644 --- a/lib/gitlab/ci/config/node/cache.rb +++ b/lib/gitlab/ci/config/node/cache.rb @@ -8,8 +8,10 @@ module Gitlab class Cache < Entry include Configurable + ALLOWED_KEYS = %i[key untracked paths] + validations do - validates :config, allowed_keys: %i[key untracked paths] + validates :config, allowed_keys: ALLOWED_KEYS end node :key, Node::Key, diff --git a/lib/gitlab/ci/config/node/commands.rb b/lib/gitlab/ci/config/node/commands.rb index f7e6950001e..d7657ae314b 100644 --- a/lib/gitlab/ci/config/node/commands.rb +++ b/lib/gitlab/ci/config/node/commands.rb @@ -11,22 +11,20 @@ module Gitlab validations do include LegacyValidationHelpers - validate :string_or_array_of_strings - - def string_or_array_of_strings - unless config_valid? + validate do + unless string_or_array_of_strings?(config) errors.add(:config, 'should be a string or an array of strings') end end - def config_valid? - validate_string(config) || validate_array_of_strings(config) + def string_or_array_of_strings?(field) + validate_string(field) || validate_array_of_strings(field) end end def value - [@config].flatten + Array(@config) end end end diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb index 93a9a253322..aedc28fe1d0 100644 --- a/lib/gitlab/ci/config/node/configurable.rb +++ b/lib/gitlab/ci/config/node/configurable.rb @@ -56,10 +56,9 @@ module Gitlab end define_method("#{symbol}_value") do - if @entries[symbol] - return unless @entries[symbol].valid? - @entries[symbol].value - end + return unless @entries[symbol] && @entries[symbol].valid? + + @entries[symbol].value end alias_method symbol.to_sym, "#{symbol}_value".to_sym diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb index b545b78a940..ccd539fb003 100644 --- a/lib/gitlab/ci/config/node/global.rb +++ b/lib/gitlab/ci/config/node/global.rb @@ -42,7 +42,7 @@ module Gitlab super compose_jobs! - compose_stages! + compose_deprecated_entries! end def compose_jobs! @@ -54,7 +54,7 @@ module Gitlab @entries[:jobs] = factory.create! end - def compose_stages! + def compose_deprecated_entries! ## # Deprecated `:types` key workaround - if types are defined and # stages are not defined we use types definition as stages. diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb index ace79d829f2..e84737acbb9 100644 --- a/lib/gitlab/ci/config/node/job.rb +++ b/lib/gitlab/ci/config/node/job.rb @@ -66,10 +66,10 @@ module Gitlab node :services, Services, description: 'Services that will be used to execute this job.' - node :only, While, + node :only, Trigger, description: 'Refs policy this job will be executed for.' - node :except, While, + node :except, Trigger, description: 'Refs policy this job will be executed for.' node :variables, Variables, diff --git a/lib/gitlab/ci/config/node/null.rb b/lib/gitlab/ci/config/node/null.rb index 880d29f663d..88a5f53f13c 100644 --- a/lib/gitlab/ci/config/node/null.rb +++ b/lib/gitlab/ci/config/node/null.rb @@ -3,7 +3,7 @@ module Gitlab class Config module Node ## - # This class represents an undefined and unspecified node. + # This class represents an undefined node. # # Implements the Null Object pattern. # diff --git a/lib/gitlab/ci/config/node/trigger.rb b/lib/gitlab/ci/config/node/trigger.rb new file mode 100644 index 00000000000..d8b31975088 --- /dev/null +++ b/lib/gitlab/ci/config/node/trigger.rb @@ -0,0 +1,26 @@ +module Gitlab + module Ci + class Config + module Node + ## + # Entry that represents a trigger policy for the job. + # + class Trigger < Entry + include Validatable + + validations do + include LegacyValidationHelpers + + validate :array_of_strings_or_regexps + + def array_of_strings_or_regexps + unless validate_array_of_strings_or_regexps(config) + errors.add(:config, 'should be an array of strings or regexps') + end + end + end + end + end + end + end +end diff --git a/lib/gitlab/ci/config/node/undefined.rb b/lib/gitlab/ci/config/node/undefined.rb index 84dab61e7e9..45fef8c3ae5 100644 --- a/lib/gitlab/ci/config/node/undefined.rb +++ b/lib/gitlab/ci/config/node/undefined.rb @@ -3,16 +3,12 @@ module Gitlab class Config module Node ## - # This class represents an undefined and unspecified entry node. + # This class represents an unspecified entry node. # # It decorates original entry adding method that indicates it is # unspecified. # class Undefined < SimpleDelegator - def initialize(entry) - super - end - def specified? false end diff --git a/lib/gitlab/ci/config/node/validators.rb b/lib/gitlab/ci/config/node/validators.rb index 23d5faf6f07..e20908ad3cb 100644 --- a/lib/gitlab/ci/config/node/validators.rb +++ b/lib/gitlab/ci/config/node/validators.rb @@ -44,15 +44,6 @@ module Gitlab end end - class RequiredValidator < ActiveModel::EachValidator - def validate_each(record, attribute, value) - if value.nil? - raise Entry::InvalidError, - "Entry needs #{attribute} attribute set internally." - end - end - end - class KeyValidator < ActiveModel::EachValidator include LegacyValidationHelpers diff --git a/lib/gitlab/ci/config/node/while.rb b/lib/gitlab/ci/config/node/while.rb deleted file mode 100644 index 84d4352624d..00000000000 --- a/lib/gitlab/ci/config/node/while.rb +++ /dev/null @@ -1,26 +0,0 @@ -module Gitlab - module Ci - class Config - module Node - ## - # Entry that represents a ref and trigger policy for the job. - # - class While < Entry - include Validatable - - validations do - include LegacyValidationHelpers - - validate :array_of_strings_or_regexps - - def array_of_strings_or_regexps - unless validate_array_of_strings_or_regexps(config) - errors.add(:config, 'should be an array of strings or regexps') - end - end - end - end - end - end - end -end diff --git a/spec/lib/gitlab/ci/config/node/artifacts_spec.rb b/spec/lib/gitlab/ci/config/node/artifacts_spec.rb index beed29b18ae..c09a0a9c793 100644 --- a/spec/lib/gitlab/ci/config/node/artifacts_spec.rb +++ b/spec/lib/gitlab/ci/config/node/artifacts_spec.rb @@ -8,7 +8,7 @@ describe Gitlab::Ci::Config::Node::Artifacts do let(:config) { { paths: %w[public/] } } describe '#value' do - it 'returns image string' do + it 'returns artifacs configuration' do expect(entry.value).to eq config end end diff --git a/spec/lib/gitlab/ci/config/node/trigger_spec.rb b/spec/lib/gitlab/ci/config/node/trigger_spec.rb new file mode 100644 index 00000000000..a4a3e36754e --- /dev/null +++ b/spec/lib/gitlab/ci/config/node/trigger_spec.rb @@ -0,0 +1,56 @@ +require 'spec_helper' + +describe Gitlab::Ci::Config::Node::Trigger do + let(:entry) { described_class.new(config) } + + describe 'validations' do + context 'when entry config value is valid' do + context 'when config is a branch or tag name' do + let(:config) { %w[master feature/branch] } + + describe '#valid?' do + it 'is valid' do + expect(entry).to be_valid + end + end + + describe '#value' do + it 'returns key value' do + expect(entry.value).to eq config + end + end + end + + context 'when config is a regexp' do + let(:config) { ['/^issue-.*$/'] } + + describe '#valid?' do + it 'is valid' do + expect(entry).to be_valid + end + end + end + + context 'when config is a special keyword' do + let(:config) { %w[tags triggers branches] } + + describe '#valid?' do + it 'is valid' do + expect(entry).to be_valid + end + end + end + end + + context 'when entry value is not valid' do + let(:config) { [1] } + + describe '#errors' do + it 'saves errors' do + expect(entry.errors) + .to include 'trigger config should be an array of strings or regexps' + end + end + end + end +end diff --git a/spec/lib/gitlab/ci/config/node/while_spec.rb b/spec/lib/gitlab/ci/config/node/while_spec.rb deleted file mode 100644 index aac2ed7b3db..00000000000 --- a/spec/lib/gitlab/ci/config/node/while_spec.rb +++ /dev/null @@ -1,56 +0,0 @@ -require 'spec_helper' - -describe Gitlab::Ci::Config::Node::While do - let(:entry) { described_class.new(config) } - - describe 'validations' do - context 'when entry config value is valid' do - context 'when config is a branch or tag name' do - let(:config) { %w[master feature/branch] } - - describe '#valid?' do - it 'is valid' do - expect(entry).to be_valid - end - end - - describe '#value' do - it 'returns key value' do - expect(entry.value).to eq config - end - end - end - - context 'when config is a regexp' do - let(:config) { ['/^issue-.*$/'] } - - describe '#valid?' do - it 'is valid' do - expect(entry).to be_valid - end - end - end - - context 'when config is a special keyword' do - let(:config) { %w[tags triggers branches] } - - describe '#valid?' do - it 'is valid' do - expect(entry).to be_valid - end - end - end - end - - context 'when entry value is not valid' do - let(:config) { [1] } - - describe '#errors' do - it 'saves errors' do - expect(entry.errors) - .to include 'while config should be an array of strings or regexps' - end - end - end - end -end -- cgit v1.2.1