From 002e6ed1f042852bef78c3a749e13567d7a0ee5a Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 17 Jun 2016 09:33:32 +0200 Subject: Improve CI config entries validations prototype --- lib/gitlab/ci/config/node/configurable.rb | 7 +-- lib/gitlab/ci/config/node/entry.rb | 16 ++++-- lib/gitlab/ci/config/node/script.rb | 2 + lib/gitlab/ci/config/node/validatable.rb | 1 - lib/gitlab/ci/config/node/validator.rb | 5 +- .../lib/gitlab/ci/config/node/configurable_spec.rb | 15 +++-- spec/lib/gitlab/ci/config/node/global_spec.rb | 6 +- spec/lib/gitlab/ci/config/node/validatable_spec.rb | 50 ++++++++++++++++ spec/lib/gitlab/ci/config/node/validator_spec.rb | 67 ++++++++++++++++++++++ 9 files changed, 144 insertions(+), 25 deletions(-) create mode 100644 spec/lib/gitlab/ci/config/node/validatable_spec.rb create mode 100644 spec/lib/gitlab/ci/config/node/validator_spec.rb 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 -- cgit v1.2.1