Commit 95520dfc authored by Grzegorz Bizon's avatar Grzegorz Bizon

Add prototype of CI config node validator

This makes use of `ActiveModel::Validations` encapsulated in a separate
class that is accessible from a node object.
parent 76aea978
...@@ -17,11 +17,11 @@ module Gitlab ...@@ -17,11 +17,11 @@ module Gitlab
end end
def valid? def valid?
errors.none? @global.valid?
end end
def errors def errors
@global.errors.map(&:to_s) @global.errors
end end
def to_hash def to_hash
......
...@@ -16,21 +16,27 @@ module Gitlab ...@@ -16,21 +16,27 @@ module Gitlab
module Configurable module Configurable
extend ActiveSupport::Concern extend ActiveSupport::Concern
included do
validations do
validate :hash_config_value
def hash_config_value
unless self.config.is_a?(Hash)
errors.add(:config, 'should be a configuration entry hash')
end
end
end
end
def allowed_nodes def allowed_nodes
self.class.allowed_nodes || {} self.class.allowed_nodes || {}
end end
private private
def prevalidate!
unless @value.is_a?(Hash)
add_error('should be a configuration entry with hash value')
end
end
def create_node(key, factory) def create_node(key, factory)
factory.with(value: @value[key], key: key) factory.with(value: @config[key], key: key)
factory.nullify! unless @value.has_key?(key) factory.nullify! unless @config.has_key?(key)
factory.create! factory.create!
end end
...@@ -47,7 +53,6 @@ module Gitlab ...@@ -47,7 +53,6 @@ module Gitlab
define_method(symbol) do define_method(symbol) do
raise Entry::InvalidError unless valid? raise Entry::InvalidError unless valid?
@nodes[symbol].try(:value) @nodes[symbol].try(:value)
end end
......
...@@ -7,16 +7,15 @@ module Gitlab ...@@ -7,16 +7,15 @@ module Gitlab
# #
class Entry class Entry
class InvalidError < StandardError; end class InvalidError < StandardError; end
include Validatable
attr_writer :key attr_reader :config
attr_accessor :description attr_accessor :key, :description
def initialize(value) def initialize(config)
@value = value @config = config
@nodes = {} @nodes = {}
@errors = [] @validator = self.class.validator.new(self)
prevalidate!
end end
def process! def process!
...@@ -24,19 +23,13 @@ module Gitlab ...@@ -24,19 +23,13 @@ module Gitlab
return unless valid? return unless valid?
compose! compose!
process_nodes!
nodes.each(&:process!)
nodes.each(&:validate!)
end end
def nodes def nodes
@nodes.values @nodes.values
end end
def valid?
errors.none?
end
def leaf? def leaf?
allowed_nodes.none? allowed_nodes.none?
end end
...@@ -45,37 +38,35 @@ module Gitlab ...@@ -45,37 +38,35 @@ module Gitlab
@key || self.class.name.demodulize.underscore @key || self.class.name.demodulize.underscore
end end
def errors def valid?
@errors + nodes.map(&:errors).flatten errors.none?
end end
def add_error(message) def errors
@errors << Error.new(message, self) @validator.full_errors +
nodes.map(&:errors).flatten
end end
def allowed_nodes def allowed_nodes
{} {}
end end
def validate!
raise NotImplementedError
end
def value def value
raise NotImplementedError raise NotImplementedError
end end
private private
def prevalidate!
end
def compose! def compose!
allowed_nodes.each do |key, essence| allowed_nodes.each do |key, essence|
@nodes[key] = create_node(key, essence) @nodes[key] = create_node(key, essence)
end end
end end
def process_nodes!
nodes.each(&:process!)
end
def create_node(key, essence) def create_node(key, essence)
raise NotImplementedError raise NotImplementedError
end end
......
...@@ -11,17 +11,21 @@ module Gitlab ...@@ -11,17 +11,21 @@ module Gitlab
# implementation in Runner. # implementation in Runner.
# #
class Script < Entry class Script < Entry
validations do
include ValidationHelpers include ValidationHelpers
def value validate :array_of_strings
@value.join("\n")
end
def validate! def array_of_strings
unless validate_array_of_strings(@value) unless validate_array_of_strings(self.config)
add_error('should be an array of strings') errors.add(:config, 'should be an array of strings')
end
end end
end end
def value
@config.join("\n")
end
end end
end end
end end
......
module Gitlab
module Ci
class Config
module Node
module Validatable
extend ActiveSupport::Concern
class_methods do
def validator
validator = Class.new(Node::Validator)
validator.include(ActiveModel::Validations)
if defined?(@validations)
@validations.each { |rules| validator.class_eval(&rules) }
end
validator
end
private
def validations(&block)
(@validations ||= []).append(block)
end
end
end
end
end
end
end
...@@ -2,22 +2,21 @@ module Gitlab ...@@ -2,22 +2,21 @@ module Gitlab
module Ci module Ci
class Config class Config
module Node module Node
class Error class Validator < SimpleDelegator
def initialize(message, parent) def initialize(node)
@message = message @node = node
@parent = parent super(node)
validate
end end
def key def full_errors
@parent.key errors.full_messages.map do |error|
"#{@node.key} #{error}".humanize
end end
def to_s
"#{key}: #{@message}"
end end
def ==(other) def self.name
other.to_s == to_s 'Validator'
end end
end end
end end
......
...@@ -820,7 +820,7 @@ EOT ...@@ -820,7 +820,7 @@ EOT
config = YAML.dump({ before_script: "bundle update", rspec: { script: "test" } }) config = YAML.dump({ before_script: "bundle update", rspec: { script: "test" } })
expect do expect do
GitlabCiYamlProcessor.new(config, path) GitlabCiYamlProcessor.new(config, path)
end.to raise_error(GitlabCiYamlProcessor::ValidationError, "before_script: should be an array of strings") end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Before script config should be an array of strings")
end end
it "returns errors if job before_script parameter is not an array of strings" do it "returns errors if job before_script parameter is not an array of strings" do
......
...@@ -4,6 +4,7 @@ describe Gitlab::Ci::Config::Node::Configurable do ...@@ -4,6 +4,7 @@ describe Gitlab::Ci::Config::Node::Configurable do
let(:node) { Class.new } let(:node) { Class.new }
before do before do
node.include(Gitlab::Ci::Config::Node::Validatable)
node.include(described_class) node.include(described_class)
end end
......
require 'spec_helper'
describe Gitlab::Ci::Config::Node::Error do
let(:error) { described_class.new(message, parent) }
let(:message) { 'some error' }
let(:parent) { spy('parent') }
before do
allow(parent).to receive(:key).and_return('parent_key')
end
describe '#key' do
it 'returns underscored class name' do
expect(error.key).to eq 'parent_key'
end
end
describe '#to_s' do
it 'returns valid error message' do
expect(error.to_s).to eq 'parent_key: some error'
end
end
end
...@@ -85,7 +85,7 @@ describe Gitlab::Ci::Config::Node::Global do ...@@ -85,7 +85,7 @@ describe Gitlab::Ci::Config::Node::Global do
describe '#errors' do describe '#errors' do
it 'reports errors from child nodes' do it 'reports errors from child nodes' do
expect(global.errors) expect(global.errors)
.to include 'before_script: should be an array of strings' .to include 'Before script config should be an array of strings'
end end
end end
......
require 'spec_helper' require 'spec_helper'
describe Gitlab::Ci::Config::Node::Script do describe Gitlab::Ci::Config::Node::Script do
let(:entry) { described_class.new(value) } let(:entry) { described_class.new(config) }
describe '#validate!' do describe '#process!' do
before { entry.validate! } before { entry.process! }
context 'when entry value is correct' do context 'when entry config value is correct' do
let(:value) { ['ls', 'pwd'] } let(:config) { ['ls', 'pwd'] }
describe '#value' do describe '#value' do
it 'returns concatenated command' do it 'returns concatenated command' do
...@@ -29,12 +29,12 @@ describe Gitlab::Ci::Config::Node::Script do ...@@ -29,12 +29,12 @@ describe Gitlab::Ci::Config::Node::Script do
end end
context 'when entry value is not correct' do context 'when entry value is not correct' do
let(:value) { 'ls' } let(:config) { 'ls' }
describe '#errors' do describe '#errors' do
it 'saves errors' do it 'saves errors' do
expect(entry.errors) expect(entry.errors)
.to include 'script: should be an array of strings' .to include 'Script config should be an array of strings'
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment