Improve code, remove unused validator, improve names

9 files changed, 19 insertions, 33 deletions

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/while.rb b/lib/gitlab/ci/config/node/trigger.rb
index 84d4352624d..d8b31975088 100644
--- a/lib/gitlab/ci/config/node/while.rb
+++ b/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
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