Use aspect-oriented design in CI/CD config entries

6 files changed, 36 insertions, 37 deletions

diff --git a/lib/gitlab/ci/config/entry/configurable.rb b/lib/gitlab/ci/config/entry/configurable.rb
index e05aca9881b..68b6742385a 100644
--- a/lib/gitlab/ci/config/entry/configurable.rb
+++ b/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
diff --git a/lib/gitlab/ci/config/entry/node.rb b/lib/gitlab/ci/config/entry/node.rb
index a6a914d79c1..2474684e07f 100644
--- a/lib/gitlab/ci/config/entry/node.rb
+++ b/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
@@ -79,8 +80,8 @@ module Gitlab
           def self.default
           end
 
-          def self.validator
-            Validator
+          def self.aspects
+            @aspects ||= []
           end
         end
       end
diff --git a/lib/gitlab/ci/config/entry/validatable.rb b/lib/gitlab/ci/config/entry/validatable.rb
index f7f1b111571..5ced778d311 100644
--- a/lib/gitlab/ci/config/entry/validatable.rb
+++ b/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|
diff --git a/lib/gitlab/ci/config/entry/validator.rb b/lib/gitlab/ci/config/entry/validator.rb
index 55343005fe3..5ab54d7e218 100644
--- a/lib/gitlab/ci/config/entry/validator.rb
+++ b/lib/gitlab/ci/config/entry/validator.rb
@@ -30,7 +30,7 @@ module Gitlab
 
           def key_name
             if key.blank?
-              @entry.class.name.demodulize.underscore.humanize
+              @entry.class.name.to_s.demodulize.underscore.humanize
             else
               key
             end
diff --git a/spec/lib/gitlab/ci/config/entry/configurable_spec.rb b/spec/lib/gitlab/ci/config/entry/configurable_spec.rb
index ae7e628b5b5..088d4b472da 100644
--- a/spec/lib/gitlab/ci/config/entry/configurable_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/configurable_spec.rb
@@ -1,40 +1,26 @@
 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
-        end
+      it 'correctly validates an instance' do
+        expect(instance).to be_valid
       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
diff --git a/spec/lib/gitlab/ci/config/entry/validatable_spec.rb b/spec/lib/gitlab/ci/config/entry/validatable_spec.rb
index d1856801827..ae2a7a51ba6 100644
--- a/spec/lib/gitlab/ci/config/entry/validatable_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/validatable_spec.rb
@@ -1,10 +1,10 @@
 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