diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb index 7a43d494d3d..36d7d20c110 100644 --- a/lib/gitlab/ci/config/node/configurable.rb +++ b/lib/gitlab/ci/config/node/configurable.rb @@ -28,7 +28,8 @@ module Gitlab def create(key, factory) factory .value(@config[key]) - .with(key: key, parent: self, global: @global) + .parent(self) + .with(key: key) factory.create! end @@ -40,11 +41,11 @@ module Gitlab private - def node(symbol, entry_class, metadata) - factory = Node::Factory.new(entry_class) + def node(key, node, metadata) + factory = Node::Factory.new(node) .with(description: metadata[:description]) - (@nodes ||= {}).merge!(symbol.to_sym => factory) + (@nodes ||= {}).merge!(key.to_sym => factory) end def helpers(*nodes) diff --git a/lib/gitlab/ci/config/node/factory.rb b/lib/gitlab/ci/config/node/factory.rb index 3f2cdf436e3..3488aec4a22 100644 --- a/lib/gitlab/ci/config/node/factory.rb +++ b/lib/gitlab/ci/config/node/factory.rb @@ -18,6 +18,11 @@ module Gitlab self end + def parent(parent) + @parent = parent + self + end + def with(attributes) @attributes.merge!(attributes) self @@ -25,15 +30,19 @@ module Gitlab def create! raise InvalidFactory unless defined?(@value) + raise InvalidFactory unless defined?(@parent) + + attributes = { parent: @parent, global: @parent.global } + attributes.merge!(@attributes) ## # We assume that unspecified entry is undefined. # See issue #18775. # if @value.nil? - Node::Undefined.new(@node, @attributes) + Node::Undefined.new(@node, attributes) else - @node.new(@value, @attributes) + @node.new(@value, attributes) end end end diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb index f5500e27439..4a958735599 100644 --- a/lib/gitlab/ci/config/node/global.rb +++ b/lib/gitlab/ci/config/node/global.rb @@ -53,9 +53,10 @@ module Gitlab def compose_jobs! factory = Node::Factory.new(Node::Jobs) - factory.value(@config.except(*nodes.keys)) - factory.with(key: :jobs, parent: self, global: self) - factory.with(description: 'Jobs definition for this pipeline') + .value(@config.except(*nodes.keys)) + .parent(self) + .with(key: :jobs, global: self) + .with(description: 'Jobs definition for this pipeline') @entries[:jobs] = factory.create! end diff --git a/lib/gitlab/ci/config/node/jobs.rb b/lib/gitlab/ci/config/node/jobs.rb index 7a164b69aff..548441df37c 100644 --- a/lib/gitlab/ci/config/node/jobs.rb +++ b/lib/gitlab/ci/config/node/jobs.rb @@ -30,14 +30,14 @@ module Gitlab private def create(name, config) - Node::Factory.new(job_node(name)) + Node::Factory.new(node(name)) .value(config || {}) - .with(key: name, parent: self, global: @global) - .with(description: "#{name} job definition.") + .parent(self) + .with(key: name, description: "#{name} job definition.") .create! end - def job_node(name) + def node(name) if name.to_s.start_with?('.') Node::HiddenJob else diff --git a/spec/lib/gitlab/ci/config/node/factory_spec.rb b/spec/lib/gitlab/ci/config/node/factory_spec.rb index c912b1b2044..bc6bf32ffbf 100644 --- a/spec/lib/gitlab/ci/config/node/factory_spec.rb +++ b/spec/lib/gitlab/ci/config/node/factory_spec.rb @@ -2,22 +2,40 @@ require 'spec_helper' describe Gitlab::Ci::Config::Node::Factory do describe '#create!' do - let(:factory) { described_class.new(entry_class) } - let(:entry_class) { Gitlab::Ci::Config::Node::Script } + let(:factory) { described_class.new(node) } + let(:node) { Gitlab::Ci::Config::Node::Script } + let(:parent) { double('parent') } + let(:global) { double('global') } - context 'when setting up a value' do + before do + allow(parent).to receive(:global).and_return(global) + end + + context 'when setting a concrete value' do it 'creates entry with valid value' do entry = factory .value(['ls', 'pwd']) + .parent(parent) .create! expect(entry.value).to eq ['ls', 'pwd'] end + it 'sets parent and global attributes' do + entry = factory + .value('ls') + .parent(parent) + .create! + + expect(entry.global).to eq global + expect(entry.parent).to eq parent + end + context 'when setting description' do it 'creates entry with description' do entry = factory .value(['ls', 'pwd']) + .parent(parent) .with(description: 'test description') .create! @@ -30,6 +48,7 @@ describe Gitlab::Ci::Config::Node::Factory do it 'creates entry with custom key' do entry = factory .value(['ls', 'pwd']) + .parent(parent) .with(key: 'test key') .create! @@ -38,20 +57,21 @@ describe Gitlab::Ci::Config::Node::Factory do end context 'when setting a parent' do - let(:parent) { Object.new } + let(:object) { Object.new } it 'creates entry with valid parent' do entry = factory .value('ls') - .with(parent: parent) + .parent(parent) + .with(parent: object) .create! - expect(entry.parent).to eq parent + expect(entry.parent).to eq object end end end - context 'when not setting up a value' do + context 'when not setting a value' do it 'raises error' do expect { factory.create! }.to raise_error( Gitlab::Ci::Config::Node::Factory::InvalidFactory @@ -59,10 +79,19 @@ describe Gitlab::Ci::Config::Node::Factory do end end + context 'when not setting parent object' do + it 'raises error' do + expect { factory.value('ls').create! }.to raise_error( + Gitlab::Ci::Config::Node::Factory::InvalidFactory + ) + end + end + context 'when creating entry with nil value' do it 'creates an undefined entry' do entry = factory .value(nil) + .parent(parent) .create! expect(entry).to be_an_instance_of Gitlab::Ci::Config::Node::Undefined