Improve CI config entries validations prototype

9 files changed, 144 insertions, 25 deletions

diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index 23d3f9f3277..7b428c1362c 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -15,6 +15,7 @@ module Gitlab
         #
         module Configurable
           extend ActiveSupport::Concern
+          include Validatable
 
           included do
             validations do
@@ -28,10 +29,6 @@ module Gitlab
             end
           end
 
-          def allowed_nodes
-            self.class.allowed_nodes || {}
-          end
-
           private
 
           def create_node(key, factory)
@@ -41,7 +38,7 @@ module Gitlab
           end
 
           class_methods do
-            def allowed_nodes
+            def nodes
               Hash[@allowed_nodes.map { |key, factory| [key, factory.dup] }]
             end
 
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 823c8bb5d13..f044ef965e9 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -7,7 +7,6 @@ module Gitlab
         #
         class Entry
           class InvalidError < StandardError; end
-          include Validatable
 
           attr_reader :config
           attr_accessor :key, :description
@@ -16,6 +15,7 @@ module Gitlab
             @config = config
             @nodes = {}
             @validator = self.class.validator.new(self)
+            @validator.validate
           end
 
           def process!
@@ -31,7 +31,7 @@ module Gitlab
           end
 
           def leaf?
-            allowed_nodes.none?
+            self.class.nodes.none?
           end
 
           def key
@@ -47,18 +47,22 @@ module Gitlab
               nodes.map(&:errors).flatten
           end
 
-          def allowed_nodes
+          def value
+            raise NotImplementedError
+          end
+
+          def self.nodes
             {}
           end
 
-          def value
-            raise NotImplementedError
+          def self.validator
+            Validator
           end
 
           private
 
           def compose!
-            allowed_nodes.each do |key, essence|
+            self.class.nodes.each do |key, essence|
               @nodes[key] = create_node(key, essence)
             end
           end
diff --git a/lib/gitlab/ci/config/node/script.rb b/lib/gitlab/ci/config/node/script.rb
index 18592acdac6..059650d8a52 100644
--- a/lib/gitlab/ci/config/node/script.rb
+++ b/lib/gitlab/ci/config/node/script.rb
@@ -11,6 +11,8 @@ module Gitlab
         # implementation in Runner.
         #
         class Script < Entry
+          include Validatable
+
           validations do
             include ValidationHelpers
 
diff --git a/lib/gitlab/ci/config/node/validatable.rb b/lib/gitlab/ci/config/node/validatable.rb
index a0617d21ad8..f6e2896dfb2 100644
--- a/lib/gitlab/ci/config/node/validatable.rb
+++ b/lib/gitlab/ci/config/node/validatable.rb
@@ -8,7 +8,6 @@ module Gitlab
           class_methods do
             def validator
               validator = Class.new(Node::Validator)
-              validator.include(ActiveModel::Validations)
 
               if defined?(@validations)
                 @validations.each { |rules| validator.class_eval(&rules) }
diff --git a/lib/gitlab/ci/config/node/validator.rb b/lib/gitlab/ci/config/node/validator.rb
index 891b19c9743..2454cc6d957 100644
--- a/lib/gitlab/ci/config/node/validator.rb
+++ b/lib/gitlab/ci/config/node/validator.rb
@@ -3,10 +3,11 @@ module Gitlab
     class Config
       module Node
         class Validator < SimpleDelegator
+          include ActiveModel::Validations
+
           def initialize(node)
-            @node = node
             super(node)
-            validate
+            @node = node
           end
 
           def full_errors
diff --git a/spec/lib/gitlab/ci/config/node/configurable_spec.rb b/spec/lib/gitlab/ci/config/node/configurable_spec.rb
index 8ec7919f4d4..9bbda6e7396 100644
--- a/spec/lib/gitlab/ci/config/node/configurable_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/configurable_spec.rb
@@ -4,30 +4,29 @@ describe Gitlab::Ci::Config::Node::Configurable do
   let(:node) { Class.new }
 
   before do
-    node.include(Gitlab::Ci::Config::Node::Validatable)
     node.include(described_class)
   end
 
-  describe 'allowed nodes' do
+  describe 'configured nodes' do
     before do
       node.class_eval do
         allow_node :object, Object, description: 'test object'
       end
     end
 
-    describe '#allowed_nodes' do
-      it 'has valid allowed nodes' do
-        expect(node.allowed_nodes).to include :object
+    describe '.nodes' do
+      it 'has valid nodes' do
+        expect(node.nodes).to include :object
       end
 
       it 'creates a node factory' do
-        expect(node.allowed_nodes[:object])
+        expect(node.nodes[:object])
           .to be_an_instance_of Gitlab::Ci::Config::Node::Factory
       end
 
       it 'returns a duplicated factory object' do
-        first_factory = node.allowed_nodes[:object]
-        second_factory = node.allowed_nodes[:object]
+        first_factory = node.nodes[:object]
+        second_factory = node.nodes[:object]
 
         expect(first_factory).not_to be_equal(second_factory)
       end
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index 8bb76cf920a..fddd53a2b57 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -3,13 +3,13 @@ require 'spec_helper'
 describe Gitlab::Ci::Config::Node::Global do
   let(:global) { described_class.new(hash) }
 
-  describe '#allowed_nodes' do
+  describe '.nodes' do
     it 'can contain global config keys' do
-      expect(global.allowed_nodes).to include :before_script
+      expect(described_class.nodes).to include :before_script
     end
 
     it 'returns a hash' do
-      expect(global.allowed_nodes).to be_a Hash
+      expect(described_class.nodes).to be_a Hash
     end
   end
 
diff --git a/spec/lib/gitlab/ci/config/node/validatable_spec.rb b/spec/lib/gitlab/ci/config/node/validatable_spec.rb
new file mode 100644
index 00000000000..10cd01afcd1
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/validatable_spec.rb
@@ -0,0 +1,50 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Validatable do
+  let(:node) { Class.new }
+
+  before do
+    node.include(described_class)
+  end
+
+  describe '.validator' do
+    before do
+      node.class_eval do
+        attr_accessor :test_attribute
+
+        validations do
+          validates :test_attribute, presence: true
+        end
+      end
+    end
+
+    it 'returns validator' do
+      expect(node.validator.superclass)
+        .to be Gitlab::Ci::Config::Node::Validator
+    end
+
+    context 'when validating node instance' do
+      let(:node_instance) { node.new }
+
+      context 'when attribute is valid' do
+        before do
+          node_instance.test_attribute = 'valid'
+        end
+
+        it 'instance of validator is valid' do
+          expect(node.validator.new(node_instance)).to be_valid
+        end
+      end
+
+      context 'when attribute is not valid' do
+        before do
+          node_instance.test_attribute = nil
+        end
+
+        it 'instance of validator is invalid' do
+          expect(node.validator.new(node_instance)).to be_invalid
+        end
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/ci/config/node/validator_spec.rb b/spec/lib/gitlab/ci/config/node/validator_spec.rb
new file mode 100644
index 00000000000..ad875d55384
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/validator_spec.rb
@@ -0,0 +1,67 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Validator do
+  let(:validator) { Class.new(described_class) }
+  let(:validator_instance) { validator.new(node) }
+  let(:node) { spy('node') }
+
+  shared_examples 'delegated validator' do
+    context 'when node is valid' do
+      before do
+        allow(node).to receive(:test_attribute).and_return('valid value')
+      end
+
+      it 'validates attribute in node' do
+        expect(node).to receive(:test_attribute)
+        expect(validator_instance).to be_valid
+      end
+
+      it 'returns no errors' do
+        validator_instance.validate
+
+        expect(validator_instance.full_errors).to be_empty
+      end
+    end
+
+    context 'when node is invalid' do
+      before do
+        allow(node).to receive(:test_attribute).and_return(nil)
+      end
+
+      it 'validates attribute in node' do
+        expect(node).to receive(:test_attribute)
+        expect(validator_instance).to be_invalid
+      end
+
+      it 'returns errors' do
+        validator_instance.validate
+
+        expect(validator_instance.full_errors).not_to be_empty
+      end
+    end
+  end
+
+  describe 'attributes validations' do
+    before do
+      validator.class_eval do
+        validates :test_attribute, presence: true
+      end
+    end
+
+    it_behaves_like 'delegated validator'
+  end
+
+  describe 'interface validations' do
+    before do
+      validator.class_eval do
+        validate do
+          unless @node.test_attribute == 'valid value'
+            errors.add(:test_attribute, 'invalid value')
+          end
+        end
+      end
+    end
+
+    it_behaves_like 'delegated validator'
+  end
+end