Simplify new ci config entry class interface

7 files changed, 44 insertions, 62 deletions

diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index c8c917f229f..120457690d8 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -19,7 +19,7 @@ module Gitlab
           def initialize(*)
             super
 
-            unless leaf? || has_config?
+            unless @value.is_a?(Hash)
               @errors << 'should be a configuration entry with hash value'
             end
           end
@@ -39,9 +39,9 @@ module Gitlab
 
           def create_entry(key, entry_class)
             if @value.has_key?(key)
-              entry_class.new(@value[key], @root, self)
+              entry_class.new(@value[key])
             else
-              Node::Null.new(nil, @root, self)
+              Node::Null.new(nil)
             end
           end
 
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 19fc997297a..ed1cdd6f15d 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -10,16 +10,15 @@ module Gitlab
 
           attr_accessor :description
 
-          def initialize(value, root = nil, parent = nil)
+          def initialize(value)
             @value = value
-            @root = root
-            @parent = parent
             @nodes = {}
             @errors = []
           end
 
           def process!
-            return if leaf? || invalid?
+            return if leaf?
+            return unless valid?
 
             compose!
 
@@ -41,18 +40,10 @@ module Gitlab
             errors.none?
           end
 
-          def invalid?
-            !valid?
-          end
-
           def leaf?
             allowed_nodes.none?
           end
 
-          def has_config?
-            @value.is_a?(Hash)
-          end
-
           def errors
             @errors + nodes.map(&:errors).flatten
           end
diff --git a/lib/gitlab/ci/config/node/null.rb b/lib/gitlab/ci/config/node/null.rb
index ab7b0abaf23..4f590f6bec8 100644
--- a/lib/gitlab/ci/config/node/null.rb
+++ b/lib/gitlab/ci/config/node/null.rb
@@ -1,13 +1,13 @@
 module Gitlab
   module Ci
     class Config
-      ##
-      # This class represents a configuration entry that is not being used
-      # in configuration file.
-      #
-      # This implements Null Object pattern.
-      #
       module Node
+        ##
+        # This class represents a configuration entry that is not being used
+        # in configuration file.
+        #
+        # This implements Null Object pattern.
+        #
         class Null < Entry
           def value
             nil
diff --git a/lib/gitlab/ci/config/node/script.rb b/lib/gitlab/ci/config/node/script.rb
index 84f9ec0eb04..5072bf0db7d 100644
--- a/lib/gitlab/ci/config/node/script.rb
+++ b/lib/gitlab/ci/config/node/script.rb
@@ -6,8 +6,8 @@ module Gitlab
         # Entry that represents a script.
         #
         # Each element in the value array is a command that will be executed
-        # by GitLab Runner. Currently we concatenate this commands with
-        # new line character as a separator what is compatbile with
+        # by GitLab Runner. Currently we concatenate these commands with
+        # new line character as a separator, what is compatible with
         # implementation in Runner.
         #
         class Script < Entry
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index 1a51528336b..2227fcec638 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -49,12 +49,6 @@ describe Gitlab::Ci::Config::Node::Global do
       end
     end
 
-    describe '#has_config?' do
-      it 'has config' do
-        expect(global).to have_config
-      end
-    end
-
     describe '#leaf?' do
       it 'is not leaf' do
         expect(global).not_to be_leaf
@@ -91,12 +85,6 @@ describe Gitlab::Ci::Config::Node::Global do
       end
     end
 
-    describe '#invalid?' do
-      it 'is not valid' do
-        expect(global).to be_invalid
-      end
-    end
-
     describe '#errors' do
       it 'reports errors from child nodes' do
         expect(global.errors)
diff --git a/spec/lib/gitlab/ci/config/node/null_spec.rb b/spec/lib/gitlab/ci/config/node/null_spec.rb
index fa75bdcaa6f..fb6c3b5cbc0 100644
--- a/spec/lib/gitlab/ci/config/node/null_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/null_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::Config::Node::Null do
-  let(:entry) { described_class.new(double, double) }
+  let(:entry) { described_class.new(nil) }
 
   describe '#leaf?' do
     it 'is leaf node' do
diff --git a/spec/lib/gitlab/ci/config/node/script_spec.rb b/spec/lib/gitlab/ci/config/node/script_spec.rb
index 0af97bab164..e4d6481f8a5 100644
--- a/spec/lib/gitlab/ci/config/node/script_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/script_spec.rb
@@ -1,44 +1,47 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::Config::Node::Script do
-  let(:entry) { described_class.new(value, double)}
-  before { entry.validate! }
+  let(:entry) { described_class.new(value) }
 
-  context 'when entry value is correct' do
-    let(:value) { ['ls', 'pwd'] }
+  describe '#validate!' do
+    before { entry.validate! }
 
-    describe '#value' do
-      it 'returns concatenated command' do
-        expect(entry.value).to eq "ls\npwd"
+    context 'when entry value is correct' do
+      let(:value) { ['ls', 'pwd'] }
+
+      describe '#value' do
+        it 'returns concatenated command' do
+          expect(entry.value).to eq "ls\npwd"
+        end
       end
-    end
 
-    describe '#errors' do
-      it 'does not append errors' do
-        expect(entry.errors).to be_empty
+      describe '#errors' do
+        it 'does not append errors' do
+          expect(entry.errors).to be_empty
+        end
       end
-    end
 
-    describe '#has_config?' do
-      it 'does not have config' do
-        expect(entry).not_to have_config
+      describe '#valid?' do
+        it 'is valid' do
+          expect(entry).to be_valid
+        end
       end
     end
-  end
 
-  context 'when entry value is not correct' do
-    let(:value) { 'ls' }
+    context 'when entry value is not correct' do
+      let(:value) { 'ls' }
 
-    describe '#errors' do
-      it 'saves errors' do
-        expect(entry.errors)
-          .to include /should be an array of strings/
+      describe '#errors' do
+        it 'saves errors' do
+          expect(entry.errors)
+            .to include /should be an array of strings/
+        end
       end
-    end
 
-    describe '#invalid?' do
-      it 'is not valid' do
-        expect(entry).to be_invalid
+      describe '#valid?' do
+        it 'is not valid' do
+          expect(entry).not_to be_valid
+        end
       end
     end
   end