From 87fe50f2a0facd5bfdf287195a21932ff2340e1b Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 8 Jun 2016 12:32:56 +0200
Subject: [PATCH] Delegate Ci config entry value to single method

---
 lib/gitlab/ci/config/node/before_script.rb    |  4 +--
 lib/gitlab/ci/config/node/entry.rb            | 34 +++++++++++++------
 lib/gitlab/ci/config/node/global.rb           |  4 ---
 lib/gitlab/ci/config/node/null.rb             |  7 ++++
 .../ci/config/node/before_script_spec.rb      | 10 ++----
 spec/lib/gitlab/ci/config/node/global_spec.rb |  8 +++++
 spec/lib/gitlab/ci/config/node/null_spec.rb   |  6 ++++
 7 files changed, 48 insertions(+), 25 deletions(-)

diff --git a/lib/gitlab/ci/config/node/before_script.rb b/lib/gitlab/ci/config/node/before_script.rb
index 271cb7b5da1..be2ceebf3f9 100644
--- a/lib/gitlab/ci/config/node/before_script.rb
+++ b/lib/gitlab/ci/config/node/before_script.rb
@@ -9,9 +9,7 @@ module Gitlab
             'Script that is executed before the one defined in a job.'
           end
 
-          def script
-            raise unless valid?
-
+          def value
             @value.join("\n")
           end
 
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index f8f2d0be23a..0767fadcb9a 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -3,17 +3,14 @@ module Gitlab
     class Config
       module Node
         class Entry
-          attr_reader :value, :nodes, :parent
+          class InvalidError < StandardError; end
 
           def initialize(value, root = nil, parent = nil)
             @value = value
             @root = root
             @parent = parent
-            @nodes, @errors = [], []
-
-            keys.each_key do |key|
-              instance_variable_set("@#{key}", Null.new(nil, root, self))
-            end
+            @nodes = {}
+            @errors = []
 
             unless leaf? || value.is_a?(Hash)
               @errors << 'should be a configuration entry with hash value'
@@ -24,17 +21,23 @@ module Gitlab
             return if leaf? || !valid?
 
             keys.each do |key, entry_class|
-              next unless @value.has_key?(key)
+              if @value.has_key?(key)
+                entry = entry_class.new(@value[key], @root, self)
+              else
+                entry = Node::Null.new(nil, @root, self)
+              end
 
-              entry = entry_class.new(@value[key], @root, self)
-              instance_variable_set("@#{key}", entry)
-              @nodes.append(entry)
+              @nodes[key] = entry
             end
 
             nodes.each(&:process!)
             nodes.each(&:validate!)
           end
 
+          def nodes
+            @nodes.values
+          end
+
           def errors
             @errors + nodes.map(&:errors).flatten
           end
@@ -51,6 +54,17 @@ module Gitlab
             self.class.nodes || {}
           end
 
+          def method_missing(name, *args)
+            super unless keys.has_key?(name)
+            raise InvalidError unless valid?
+
+            @nodes[name].value
+          end
+
+          def value
+            raise NotImplementedError
+          end
+
           def validate!
             raise NotImplementedError
           end
diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index 5912ead21c6..cfa506c28b7 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -4,10 +4,6 @@ module Gitlab
       module Node
         class Global < Entry
           add_node :before_script, BeforeScript
-
-          def before_script
-            @before_script.script
-          end
         end
       end
     end
diff --git a/lib/gitlab/ci/config/node/null.rb b/lib/gitlab/ci/config/node/null.rb
index fc240e16f55..db3fa05c328 100644
--- a/lib/gitlab/ci/config/node/null.rb
+++ b/lib/gitlab/ci/config/node/null.rb
@@ -3,6 +3,13 @@ module Gitlab
     class Config
       module Node
         class Null < Entry
+          def value
+            nil
+          end
+
+          def validate!
+          end
+
           def method_missing(*)
             nil
           end
diff --git a/spec/lib/gitlab/ci/config/node/before_script_spec.rb b/spec/lib/gitlab/ci/config/node/before_script_spec.rb
index 8ccefb9b9b9..bc34b9c9b56 100644
--- a/spec/lib/gitlab/ci/config/node/before_script_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/before_script_spec.rb
@@ -7,9 +7,9 @@ describe Gitlab::Ci::Config::Node::BeforeScript do
   context 'when entry value is correct' do
     let(:value) { ['ls', 'pwd'] }
 
-    describe '#script' do
+    describe '#value' do
       it 'returns concatenated command' do
-        expect(entry.script).to eq "ls\npwd"
+        expect(entry.value).to eq "ls\npwd"
       end
     end
 
@@ -29,11 +29,5 @@ describe Gitlab::Ci::Config::Node::BeforeScript do
           .to include /should be an array of strings/
       end
     end
-
-    describe '#script' do
-      it 'raises error' do
-        expect { entry.script }.to raise_error
-      end
-    end
   end
 end
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index 7f49b89f6d6..66d40be6e6e 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -65,6 +65,14 @@ describe Gitlab::Ci::Config::Node::Global do
           .to include 'before_script should be an array of strings'
       end
     end
+
+    describe '#before_script' do
+      it 'raises error' do
+        expect { global.before_script }.to raise_error(
+          Gitlab::Ci::Config::Node::Entry::InvalidError
+        )
+      end
+    end
   end
 
   context 'when value is not a hash' do
diff --git a/spec/lib/gitlab/ci/config/node/null_spec.rb b/spec/lib/gitlab/ci/config/node/null_spec.rb
index 42a67892966..fa75bdcaa6f 100644
--- a/spec/lib/gitlab/ci/config/node/null_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/null_spec.rb
@@ -14,4 +14,10 @@ describe Gitlab::Ci::Config::Node::Null do
       expect(entry.any_method).to be nil
     end
   end
+
+  describe '#value' do
+    it 'returns nill' do
+      expect(entry.value).to be nil
+    end
+  end
 end
-- 
GitLab