From 1ac62de2c12a26e6f5158cdb4f008a71729b39fc Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 8 Jul 2016 12:51:47 +0200
Subject: [PATCH] Extract CI entry node validator and improve naming

---
 lib/gitlab/ci/config.rb                     |  1 +
 lib/gitlab/ci/config/node/configurable.rb   |  6 ++--
 lib/gitlab/ci/config/node/entry.rb          | 36 ++++++++++-----------
 lib/gitlab/ci/config/node/jobs.rb           |  6 ++--
 spec/lib/gitlab/ci/config/node/jobs_spec.rb |  5 ++-
 5 files changed, 29 insertions(+), 25 deletions(-)

diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb
index ae82c0db3f1..20f5f8e2ff8 100644
--- a/lib/gitlab/ci/config.rb
+++ b/lib/gitlab/ci/config.rb
@@ -15,6 +15,7 @@ module Gitlab
 
         @global = Node::Global.new(@config)
         @global.process!
+        @global.validate!
       end
 
       def valid?
diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index 88403a9de1e..e5780c60e70 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -25,7 +25,7 @@ module Gitlab
 
           private
 
-          def create_node(key, factory)
+          def create(key, factory)
             factory
               .value(config[key])
               .with(key: key, parent: self, global: global)
@@ -50,12 +50,12 @@ module Gitlab
             def helpers(*nodes)
               nodes.each do |symbol|
                 define_method("#{symbol}_defined?") do
-                  @nodes[symbol].try(:defined?)
+                  @entries[symbol].try(:defined?)
                 end
 
                 define_method("#{symbol}_value") do
                   raise Entry::InvalidError unless valid?
-                  @nodes[symbol].try(:value)
+                  @entries[symbol].try(:value)
                 end
 
                 alias_method symbol.to_sym, "#{symbol}_value".to_sym
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index e8b0160edc1..67e59ffb86e 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -13,7 +13,7 @@ module Gitlab
 
           def initialize(config, **attributes)
             @config = config
-            @nodes = {}
+            @entries = {}
 
             (@attributes = attributes).each do |attribute, value|
               public_send("#{attribute}=", value)
@@ -24,8 +24,18 @@ module Gitlab
           end
 
           def process!
-            compose! unless leaf?
-            @validator.validate(:processed) if valid?
+            return unless valid?
+
+            nodes.each do |key, essence|
+              @entries[key] = create(key, essence)
+            end
+
+            @entries.each_value(&:process!)
+          end
+
+          def validate!
+            @validator.validate(:after)
+            @entries.each_value(&:validate!)
           end
 
           def leaf?
@@ -37,7 +47,7 @@ module Gitlab
           end
 
           def descendants
-            @nodes.values
+            @entries.values
           end
 
           def ancestors
@@ -49,18 +59,18 @@ module Gitlab
           end
 
           def errors
-            @validator.messages + @nodes.values.flat_map(&:errors)
+            @validator.messages + @entries.values.flat_map(&:errors)
           end
 
           def value
             if leaf?
               @config
             else
-              meaningful = @nodes.select do |_key, value|
+              meaningful = @entries.select do |_key, value|
                 value.defined? && value.relevant?
               end
 
-              Hash[meaningful.map { |key, node| [key, node.value] }]
+              Hash[meaningful.map { |key, entry| [key, entry.value] }]
             end
           end
 
@@ -85,17 +95,7 @@ module Gitlab
 
           private
 
-          def compose!
-            return unless valid?
-
-            nodes.each do |key, essence|
-              @nodes[key] = create_node(key, essence)
-            end
-
-            @nodes.each_value(&:process!)
-          end
-
-          def create_node(key, essence)
+          def create(entry, essence)
             raise NotImplementedError
           end
         end
diff --git a/lib/gitlab/ci/config/node/jobs.rb b/lib/gitlab/ci/config/node/jobs.rb
index cba1fce4a4c..6199749a508 100644
--- a/lib/gitlab/ci/config/node/jobs.rb
+++ b/lib/gitlab/ci/config/node/jobs.rb
@@ -10,7 +10,7 @@ module Gitlab
 
           validations do
             validates :config, type: Hash
-            validate :jobs_presence, on: :processed
+            validate :jobs_presence, on: :after
 
             def jobs_presence
               unless relevant?
@@ -24,12 +24,12 @@ module Gitlab
           end
 
           def relevant?
-            @nodes.values.any?(&:relevant?)
+            @entries.values.any?(&:relevant?)
           end
 
           private
 
-          def create_node(name, config)
+          def create(name, config)
             job_node(name).new(config, job_attributes(name))
           end
 
diff --git a/spec/lib/gitlab/ci/config/node/jobs_spec.rb b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
index 7ec28b642b4..1ecc3e18d4e 100644
--- a/spec/lib/gitlab/ci/config/node/jobs_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
@@ -35,7 +35,10 @@ describe Gitlab::Ci::Config::Node::Jobs do
           end
 
           context 'when processed' do
-            before { entry.process! }
+            before do
+              entry.process!
+              entry.validate!
+            end
 
             it 'returns error about no visible jobs defined' do
               expect(entry.errors)
-- 
GitLab