Improve code, remove unused validator, improve names

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,

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

lib/gitlab/ci/config/node/configurable.rb

@@ -56,11 +56,10 @@ module Gitlab
                 end
 
                 define_method("#{symbol}_value") do
-                  if @entries[symbol]
-                    return unless @entries[symbol].valid?
+                  return unless @entries[symbol] && @entries[symbol].valid?
+
                   @entries[symbol].value
                 end
-                end
 
                 alias_method symbol.to_sym, "#{symbol}_value".to_sym
               end

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.

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,

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.
         #

lib/gitlab/ci/config/node/while.rb → lib/gitlab/ci/config/node/trigger.rb

@@ -3,9 +3,9 @@ module Gitlab
     class Config
       module Node
         ##
-        # Entry that represents a ref and trigger policy for the job.
+        # Entry that represents a trigger policy for the job.
         #
-        class While < Entry
+        class Trigger < Entry
           include Validatable
 
           validations do

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

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
 

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

spec/lib/gitlab/ci/config/node/while_spec.rb → spec/lib/gitlab/ci/config/node/trigger_spec.rb

 require 'spec_helper'
 
-describe Gitlab::Ci::Config::Node::While do
+describe Gitlab::Ci::Config::Node::Trigger do
   let(:entry) { described_class.new(config) }
 
   describe 'validations' do
@@ -48,7 +48,7 @@ describe Gitlab::Ci::Config::Node::While do
       describe '#errors' do
         it 'saves errors' do
           expect(entry.errors)
-            .to include 'while config should be an array of strings or regexps'
+            .to include 'trigger config should be an array of strings or regexps'
         end
       end
     end