Merge branch 'feature/gb/add-complex-jobs-only-except-config-policy' into 'master'

New syntax for CI/CD job triggering policy that supports variables expressions

See merge request !13769

lib/gitlab/ci/config/entry/attributable.rb

@@ -8,6 +8,10 @@ module Gitlab
           class_methods do
             def attributes(*attributes)
               attributes.flatten.each do |attribute|
+                if method_defined?(attribute)
+                  raise ArgumentError, 'Method already defined!'
+                end
+
                 define_method(attribute) do
                   return unless config.is_a?(Hash)
 

lib/gitlab/ci/config/entry/configurable.rb

@@ -15,9 +15,10 @@ module Gitlab
         #
         module Configurable
           extend ActiveSupport::Concern
-          include Validatable
 
           included do
+            include Validatable
+
             validations do
               validates :config, type: Hash
             end

lib/gitlab/ci/config/entry/job.rb

@@ -59,10 +59,10 @@ module Gitlab
           entry :services, Entry::Services,
             description: 'Services that will be used to execute this job.'
 
-          entry :only, Entry::Trigger,
+          entry :only, Entry::Policy,
             description: 'Refs policy this job will be executed for.'
 
-          entry :except, Entry::Trigger,
+          entry :except, Entry::Policy,
             description: 'Refs policy this job will be executed for.'
 
           entry :variables, Entry::Variables,

lib/gitlab/ci/config/entry/node.rb

@@ -16,8 +16,9 @@ module Gitlab
             @metadata = metadata
             @entries = {}
 
-            @validator = self.class.validator.new(self)
-            @validator.validate(:new)
+            self.class.aspects.to_a.each do |aspect|
+              instance_exec(&aspect)
+            end
           end
 
           def [](key)
@@ -47,7 +48,7 @@ module Gitlab
           end
 
           def errors
-            @validator.messages + descendants.flat_map(&:errors)
+            []
           end
 
           def value
@@ -70,6 +71,13 @@ module Gitlab
             true
           end
 
+          def location
+            name = @key.presence || self.class.name.to_s.demodulize
+                                        .underscore.humanize.downcase
+
+            ancestors.map(&:key).append(name).compact.join(':')
+          end
+
           def inspect
             val = leaf? ? config : descendants
             unspecified = specified? ? '' : '(unspecified) '
@@ -79,8 +87,8 @@ module Gitlab
           def self.default
           end
 
-          def self.validator
-            Validator
+          def self.aspects
+            @aspects ||= []
           end
         end
       end

lib/gitlab/ci/config/entry/policy.rb

+module Gitlab
+  module Ci
+    class Config
+      module Entry
+        ##
+        # Entry that represents an only/except trigger policy for the job.
+        #
+        class Policy < Simplifiable
+          strategy :RefsPolicy, if: -> (config) { config.is_a?(Array) }
+
+          class RefsPolicy < Entry::Node
+            include Entry::Validatable
+
+            validations do
+              validates :config, array_of_strings_or_regexps: true
+            end
+          end
+
+          class UnknownStrategy < Entry::Node
+            def errors
+              ["#{location} has to be either an array of conditions or a hash"]
+            end
+          end
+
+          def self.default
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/ci/config/entry/simplifiable.rb

+module Gitlab
+  module Ci
+    class Config
+      module Entry
+        class Simplifiable < SimpleDelegator
+          EntryStrategy = Struct.new(:name, :condition)
+
+          def initialize(config, **metadata)
+            unless self.class.const_defined?(:UnknownStrategy)
+              raise ArgumentError, 'UndefinedStrategy not available!'
+            end
+
+            strategy = self.class.strategies.find do |variant|
+              variant.condition.call(config)
+            end
+
+            entry = self.class.entry_class(strategy)
+
+            super(entry.new(config, metadata))
+          end
+
+          def self.strategy(name, **opts)
+            EntryStrategy.new(name, opts.fetch(:if)).tap do |strategy|
+              strategies.append(strategy)
+            end
+          end
+
+          def self.strategies
+            @strategies ||= []
+          end
+
+          def self.entry_class(strategy)
+            if strategy.present?
+              self.const_get(strategy.name)
+            else
+              self::UnknownStrategy
+            end
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/ci/config/entry/trigger.rb

-module Gitlab
-  module Ci
-    class Config
-      module Entry
-        ##
-        # Entry that represents a trigger policy for the job.
-        #
-        class Trigger < Node
-          include Validatable
-
-          validations do
-            validates :config, array_of_strings_or_regexps: true
-          end
-        end
-      end
-    end
-  end
-end

lib/gitlab/ci/config/entry/validatable.rb

@@ -5,6 +5,17 @@ module Gitlab
         module Validatable
           extend ActiveSupport::Concern
 
+          def self.included(node)
+            node.aspects.append -> do
+              @validator = self.class.validator.new(self)
+              @validator.validate(:new)
+            end
+          end
+
+          def errors
+            @validator.messages + descendants.flat_map(&:errors)
+          end
+
           class_methods do
             def validator
               @validator ||= Class.new(Entry::Validator).tap do |validator|

lib/gitlab/ci/config/entry/validator.rb

@@ -8,7 +8,6 @@ module Gitlab
 
           def initialize(entry)
             super(entry)
-            @entry = entry
           end
 
           def messages
@@ -20,21 +19,6 @@ module Gitlab
           def self.name
             'Validator'
           end
-
-          private
-
-          def location
-            predecessors = ancestors.map(&:key).compact
-            predecessors.append(key_name).join(':')
-          end
-
-          def key_name
-            if key.blank?
-              @entry.class.name.demodulize.underscore.humanize
-            else
-              key
-            end
-          end
         end
       end
     end

spec/lib/ci/gitlab_ci_yaml_processor_spec.rb

@@ -344,28 +344,31 @@ module Ci
           let(:config) { { rspec: { script: "rspec", type: "test", only: only } } }
           let(:processor) { GitlabCiYamlProcessor.new(YAML.dump(config)) }
 
-          shared_examples 'raises an error' do
-            it do
-              expect { processor }.to raise_error(GitlabCiYamlProcessor::ValidationError, 'jobs:rspec:only config should be an array of strings or regexps')
-            end
-          end
-
           context 'when it is integer' do
             let(:only) { 1 }
 
-            it_behaves_like 'raises an error'
+            it do
+              expect { processor }.to raise_error(GitlabCiYamlProcessor::ValidationError,
+                                                  'jobs:rspec:only has to be either an array of conditions or a hash')
+            end
           end
 
           context 'when it is an array of integers' do
             let(:only) { [1, 1] }
 
-            it_behaves_like 'raises an error'
+            it do
+              expect { processor }.to raise_error(GitlabCiYamlProcessor::ValidationError,
+                                                  'jobs:rspec:only config should be an array of strings or regexps')
+            end
           end
 
           context 'when it is invalid regex' do
             let(:only) { ["/*invalid/"] }
 
-            it_behaves_like 'raises an error'
+            it do
+              expect { processor }.to raise_error(GitlabCiYamlProcessor::ValidationError,
+                                                  'jobs:rspec:only config should be an array of strings or regexps')
+            end
           end
         end
       end
@@ -518,28 +521,31 @@ module Ci
           let(:config) { { rspec: { script: "rspec", except: except } } }
           let(:processor) { GitlabCiYamlProcessor.new(YAML.dump(config)) }
 
-          shared_examples 'raises an error' do
-            it do
-              expect { processor }.to raise_error(GitlabCiYamlProcessor::ValidationError, 'jobs:rspec:except config should be an array of strings or regexps')
-            end
-          end
-
           context 'when it is integer' do
             let(:except) { 1 }
 
-            it_behaves_like 'raises an error'
+            it do
+              expect { processor }.to raise_error(GitlabCiYamlProcessor::ValidationError,
+                                                  'jobs:rspec:except has to be either an array of conditions or a hash')
+            end
           end
 
           context 'when it is an array of integers' do
             let(:except) { [1, 1] }
 
-            it_behaves_like 'raises an error'
+            it do
+              expect { processor }.to raise_error(GitlabCiYamlProcessor::ValidationError,
+                                                  'jobs:rspec:except config should be an array of strings or regexps')
+            end
           end
 
           context 'when it is invalid regex' do
             let(:except) { ["/*invalid/"] }
 
-            it_behaves_like 'raises an error'
+            it do
+              expect { processor }.to raise_error(GitlabCiYamlProcessor::ValidationError,
+                                                  'jobs:rspec:except config should be an array of strings or regexps')
+            end
           end
         end
       end

spec/lib/gitlab/ci/config/entry/attributable_spec.rb

 require 'spec_helper'
 
 describe Gitlab::Ci::Config::Entry::Attributable do
-  let(:node) { Class.new }
+  let(:node) do
+    Class.new do
+      include Gitlab::Ci::Config::Entry::Attributable
+    end
+  end
+
   let(:instance) { node.new }
 
   before do
-    node.include(described_class)
-
     node.class_eval do
       attributes :name, :test
     end
   end
 
-  context 'config is a hash' do
+  context 'when config is a hash' do
     before do
       allow(instance)
         .to receive(:config)
@@ -29,7 +32,7 @@ describe Gitlab::Ci::Config::Entry::Attributable do
     end
   end
 
-  context 'config is not a hash' do
+  context 'when config is not a hash' do
     before do
       allow(instance)
         .to receive(:config)
@@ -40,4 +43,18 @@ describe Gitlab::Ci::Config::Entry::Attributable do
       expect(instance.test).to be_nil
     end
   end
+
+  context 'when method is already defined in a superclass' do
+    it 'raises an error' do
+      expectation = expect do
+        Class.new(String) do
+          include Gitlab::Ci::Config::Entry::Attributable
+
+          attributes :length
+        end
+      end
+
+      expectation.to raise_error(ArgumentError, 'Method already defined!')
+    end
+  end
 end

spec/lib/gitlab/ci/config/entry/configurable_spec.rb

 require 'spec_helper'
 
 describe Gitlab::Ci::Config::Entry::Configurable do
-  let(:entry) { Class.new }
-
-  before do
-    entry.include(described_class)
+  let(:entry) do
+    Class.new(Gitlab::Ci::Config::Entry::Node) do
+      include Gitlab::Ci::Config::Entry::Configurable
+    end
   end
 
   describe 'validations' do
-    let(:validator) { entry.validator.new(instance) }
-
-    before do
-      entry.class_eval do
-        attr_reader :config
+    context 'when entry is a hash' do
+      let(:instance) { entry.new(key: 'value') }
 
-        def initialize(config)
-          @config = config
+      it 'correctly validates an instance' do
+        expect(instance).to be_valid
       end
     end
 
-      validator.validate
-    end
-
-    context 'when entry validator is invalid' do
+    context 'when entry is not a hash' do
       let(:instance) { entry.new('ls') }
 
-      it 'returns invalid validator' do
-        expect(validator).to be_invalid
-      end
-    end
-
-    context 'when entry instance is valid' do
-      let(:instance) { entry.new(key: 'value') }
-
-      it 'returns valid validator' do
-        expect(validator).to be_valid
+      it 'invalidates the instance' do
+        expect(instance).not_to be_valid
       end
     end
   end

spec/lib/gitlab/ci/config/entry/policy_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Entry::Policy do
+  let(:entry) { described_class.new(config) }
+
+  context 'when using simplified policy' do
+    describe 'validations' do
+      context 'when entry config value is valid' do
+        context 'when config is a branch or tag name' do
+          let(:config) { %w[master feature/branch] }
+
+          describe '#valid?' do
+            it 'is valid' do
+              expect(entry).to be_valid
+            end
+          end
+
+          describe '#value' do
+            it 'returns key value' do
+              expect(entry.value).to eq config
+            end
+          end
+        end
+
+        context 'when config is a regexp' do
+          let(:config) { ['/^issue-.*$/'] }
+
+          describe '#valid?' do
+            it 'is valid' do
+              expect(entry).to be_valid
+            end
+          end
+        end
+
+        context 'when config is a special keyword' do
+          let(:config) { %w[tags triggers branches] }
+
+          describe '#valid?' do
+            it 'is valid' do
+              expect(entry).to be_valid
+            end
+          end
+        end
+      end
+
+      context 'when entry value is not valid' do
+        let(:config) { [1] }
+
+        describe '#errors' do
+          it 'saves errors' do
+            expect(entry.errors)
+              .to include /policy config should be an array of strings or regexps/
+          end
+        end
+      end
+    end
+  end
+
+  context 'when policy strategy does not match' do
+    let(:config) { 'string strategy' }
+
+    it 'returns information about errors' do
+      expect(entry.errors)
+        .to include /has to be either an array of conditions or a hash/
+    end
+  end
+
+  describe '.default' do
+    it 'does not have a default value' do
+      expect(described_class.default).to be_nil
+    end
+  end
+end

spec/lib/gitlab/ci/config/entry/simplifiable_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Entry::Simplifiable do
+  describe '.strategy' do
+    let(:entry) do
+      Class.new(described_class) do
+        strategy :Something, if: -> { 'condition' }
+        strategy :DifferentOne, if: -> { 'condition' }
+      end
+    end
+
+    it 'defines entry strategies' do
+      expect(entry.strategies.size).to eq 2
+      expect(entry.strategies.map(&:name))
+        .to eq %i[Something DifferentOne]
+    end
+  end
+
+  describe 'setting strategy by a condition' do
+    let(:first) { double('first strategy') }
+    let(:second) { double('second strategy') }
+    let(:unknown) { double('unknown strategy') }
+
+    before do
+      stub_const("#{described_class.name}::Something", first)
+      stub_const("#{described_class.name}::DifferentOne", second)
+      stub_const("#{described_class.name}::UnknownStrategy", unknown)
+    end
+
+    context 'when first strategy should be used' do
+      let(:entry) do
+        Class.new(described_class) do
+          strategy :Something, if: -> (arg) { arg == 'something' }
+          strategy :DifferentOne, if: -> (*) { false }
+        end
+      end
+
+      it 'attemps to load a first strategy' do
+        expect(first).to receive(:new).with('something', anything)
+
+        entry.new('something')
+      end
+    end
+
+    context 'when second strategy should be used' do
+      let(:entry) do
+        Class.new(described_class) do
+          strategy :Something, if: -> (arg) { arg == 'something' }
+          strategy :DifferentOne, if: -> (arg) { arg == 'test' }
+        end
+      end
+
+      it 'attemps to load a second strategy' do
+        expect(second).to receive(:new).with('test', anything)
+
+        entry.new('test')
+      end
+    end
+
+    context 'when neither one is a valid strategy' do
+      let(:entry) do
+        Class.new(described_class) do
+          strategy :Something, if: -> (*) { false }
+          strategy :DifferentOne, if: -> (*) { false }
+        end
+      end
+
+      it 'instantiates an unknown strategy' do
+        expect(unknown).to receive(:new).with('test', anything)
+
+        entry.new('test')
+      end
+    end
+  end
+
+  context 'when a unknown strategy class is not defined' do
+    let(:entry) do
+      Class.new(described_class) do
+        strategy :String, if: -> (*) { true }
+      end
+    end
+
+    it 'raises an error when being initialized' do
+      expect { entry.new('something') }
+        .to raise_error ArgumentError, /UndefinedStrategy not available!/
+    end
+  end
+end

spec/lib/gitlab/ci/config/entry/trigger_spec.rb

-require 'spec_helper'
-
-describe Gitlab::Ci::Config::Entry::Trigger do
-  let(:entry) { described_class.new(config) }
-
-  describe 'validations' do
-    context 'when entry config value is valid' do
-      context 'when config is a branch or tag name' do
-        let(:config) { %w[master feature/branch] }
-
-        describe '#valid?' do
-          it 'is valid' do
-            expect(entry).to be_valid
-          end
-        end
-
-        describe '#value' do
-          it 'returns key value' do
-            expect(entry.value).to eq config
-          end
-        end
-      end
-
-      context 'when config is a regexp' do
-        let(:config) { ['/^issue-.*$/'] }
-
-        describe '#valid?' do
-          it 'is valid' do
-            expect(entry).to be_valid
-          end
-        end
-      end
-
-      context 'when config is a special keyword' do
-        let(:config) { %w[tags triggers branches] }
-
-        describe '#valid?' do
-          it 'is valid' do
-            expect(entry).to be_valid
-          end
-        end
-      end
-    end
-
-    context 'when entry value is not valid' do
-      let(:config) { [1] }
-
-      describe '#errors' do
-        it 'saves errors' do
-          expect(entry.errors)
-            .to include 'trigger config should be an array of strings or regexps'
-        end
-      end
-    end
-  end
-end

spec/lib/gitlab/ci/config/entry/validatable_spec.rb

 require 'spec_helper'
 
 describe Gitlab::Ci::Config::Entry::Validatable do
-  let(:entry) { Class.new }
-
-  before do
-    entry.include(described_class)
+  let(:entry) do
+    Class.new(Gitlab::Ci::Config::Entry::Node) do
+      include Gitlab::Ci::Config::Entry::Validatable
+    end
   end
 
   describe '.validator' do
@@ -28,7 +28,7 @@ describe Gitlab::Ci::Config::Entry::Validatable do
     end
 
     context 'when validating entry instance' do
-      let(:entry_instance) { entry.new }
+      let(:entry_instance) { entry.new('something') }
 
       context 'when attribute is valid' do
         before do

spec/lib/gitlab/ci/config/entry/validator_spec.rb

@@ -48,7 +48,7 @@ describe Gitlab::Ci::Config::Entry::Validator do
         validator_instance.validate
 
         expect(validator_instance.messages)
-          .to include "node test attribute can't be blank"
+          .to include /test attribute can't be blank/
       end
     end
   end