diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb index 36d7d20c110..29de2d7d0b5 100644 --- a/lib/gitlab/ci/config/node/configurable.rb +++ b/lib/gitlab/ci/config/node/configurable.rb @@ -51,7 +51,7 @@ module Gitlab def helpers(*nodes) nodes.each do |symbol| define_method("#{symbol}_defined?") do - @entries[symbol].try(:defined?) + @entries[symbol].specified? end define_method("#{symbol}_value") do diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb index 02e4f7693da..c95e85200d2 100644 --- a/lib/gitlab/ci/config/node/entry.rb +++ b/lib/gitlab/ci/config/node/entry.rb @@ -66,14 +66,14 @@ module Gitlab @config else meaningful = @entries.select do |_key, value| - value.defined? && value.relevant? + value.specified? && value.relevant? end Hash[meaningful.map { |key, entry| [key, entry.value] }] end end - def defined? + def specified? true end diff --git a/lib/gitlab/ci/config/node/factory.rb b/lib/gitlab/ci/config/node/factory.rb index 3488aec4a22..602660acb93 100644 --- a/lib/gitlab/ci/config/node/factory.rb +++ b/lib/gitlab/ci/config/node/factory.rb @@ -40,11 +40,21 @@ module Gitlab # See issue #18775. # if @value.nil? - Node::Undefined.new(@node, attributes) + Node::Undefined.new(fabricate_undefined(attributes)) else @node.new(@value, attributes) end end + + private + + def fabricate_undefined(attributes) + if @node.default.nil? + Node::Null.new(nil, attributes) + else + @node.new(@node.default, attributes) + end + end end end end diff --git a/lib/gitlab/ci/config/node/null.rb b/lib/gitlab/ci/config/node/null.rb index c7bbc16939a..880d29f663d 100644 --- a/lib/gitlab/ci/config/node/null.rb +++ b/lib/gitlab/ci/config/node/null.rb @@ -8,10 +8,6 @@ module Gitlab # Implements the Null Object pattern. # class Null < Entry - def initialize(config = nil, **attributes) - super - end - def value nil end @@ -23,6 +19,14 @@ module Gitlab def errors [] end + + def specified? + false + end + + def relevant? + false + end end end end diff --git a/lib/gitlab/ci/config/node/undefined.rb b/lib/gitlab/ci/config/node/undefined.rb index fedb9d020be..384774c9b69 100644 --- a/lib/gitlab/ci/config/node/undefined.rb +++ b/lib/gitlab/ci/config/node/undefined.rb @@ -3,66 +3,19 @@ module Gitlab class Config module Node ## - # This class represents an undefined entry node. + # This class represents an undefined and unspecified entry node. # - # It takes original entry class as configuration and creates an object - # if original entry has a default value. If there is default value - # some methods are delegated to it. + # It decorates original entry adding method that idicates it is + # unspecified. # - # - class Undefined < Entry - include Validatable - - delegate :valid?, :errors, :value, to: :@strategy - - validations do - validates :config, type: Class - end - - def initialize(node, **attributes) + class Undefined < SimpleDelegator + def initialize(entry) super - @strategy = create_strategy(node, node.default) end - def defined? + def specified? false end - - private - - def create_strategy(node, default) - if default.nil? - Undefined::NullStrategy.new - else - entry = node.new(default, attributes) - Undefined::DefaultStrategy.new(entry) - end - end - - class DefaultStrategy - delegate :valid?, :errors, :value, to: :@default - - def initialize(entry) - @default = entry - end - end - - class NullStrategy - def initialize(*) - end - - def value - nil - end - - def valid? - true - end - - def errors - [] - end - end end end end diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb index 786fc5bb6ce..1945b0326cc 100644 --- a/spec/lib/gitlab/ci/config/node/global_spec.rb +++ b/spec/lib/gitlab/ci/config/node/global_spec.rb @@ -233,9 +233,9 @@ describe Gitlab::Ci::Config::Node::Global do end end - describe '#defined?' do + describe '#specified?' do it 'is concrete entry that is defined' do - expect(global.defined?).to be true + expect(global.specified?).to be true end end end diff --git a/spec/lib/gitlab/ci/config/node/null_spec.rb b/spec/lib/gitlab/ci/config/node/null_spec.rb index 63b23ed0bd1..1ab5478dcfa 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(:null) { described_class.new } + let(:null) { described_class.new(nil) } describe '#leaf?' do it 'is leaf node' do @@ -26,4 +26,16 @@ describe Gitlab::Ci::Config::Node::Null do expect(null.value).to eq nil end end + + describe '#relevant?' do + it 'is not relevant' do + expect(null.relevant?).to eq false + end + end + + describe '#specified?' do + it 'is not defined' do + expect(null.specified?).to eq false + end + end end diff --git a/spec/lib/gitlab/ci/config/node/undefined_spec.rb b/spec/lib/gitlab/ci/config/node/undefined_spec.rb index 417b4a0ad6f..2d43e1c1a9d 100644 --- a/spec/lib/gitlab/ci/config/node/undefined_spec.rb +++ b/spec/lib/gitlab/ci/config/node/undefined_spec.rb @@ -4,66 +4,29 @@ describe Gitlab::Ci::Config::Node::Undefined do let(:undefined) { described_class.new(entry) } let(:entry) { spy('Entry') } - context 'when entry does not have a default value' do - before { allow(entry).to receive(:default).and_return(nil) } - - describe '#leaf?' do - it 'is leaf node' do - expect(undefined).to be_leaf - end - end - - describe '#valid?' do - it 'is always valid' do - expect(undefined).to be_valid - end - end - - describe '#errors' do - it 'is does not contain errors' do - expect(undefined.errors).to be_empty - end - end - - describe '#value' do - it 'returns nil' do - expect(undefined.value).to eq nil - end + describe '#valid?' do + it 'delegates method to entry' do + expect(undefined.valid).to eq entry end end - context 'when entry has a default value' do - before do - allow(entry).to receive(:default).and_return('some value') - allow(entry).to receive(:value).and_return('some value') - end - - describe '#value' do - it 'returns default value for entry' do - expect(undefined.value).to eq 'some value' - end - end - - describe '#errors' do - it 'delegates errors to default entry' do - expect(entry).to receive(:errors) - - undefined.errors - end - end - - describe '#valid?' do - it 'delegates valid? to default entry' do - expect(entry).to receive(:valid?) - - undefined.valid? - end + describe '#errors' do + it 'delegates method to entry' do + expect(undefined.errors).to eq entry end end - describe '#undefined?' do - it 'is not a defined entry' do - expect(undefined.defined?).to be false + describe '#value' do + it 'delegates method to entry' do + expect(undefined.value).to eq entry + end + end + + describe '#specified?' do + it 'is always false' do + allow(entry).to receive(:specified?).and_return(true) + + expect(undefined.specified?).to be false end end end