From 7b712d359880cb15bad4c0f01308bf12b1518e60 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 14 Jan 2019 17:27:13 +0100
Subject: [PATCH] Make default config entry value configurable

Introduce `default:` configuration entry setting that makes it possible
to configure a default value of an entry, what overrides class-level
`def self.default` value.
---
 lib/gitlab/ci/config/entry/job.rb             |  3 +-
 lib/gitlab/ci/config/entry/key.rb             |  2 +-
 lib/gitlab/ci/config/entry/policy.rb          |  8 +----
 lib/gitlab/ci/config/entry/stage.rb           |  2 +-
 lib/gitlab/ci/config/entry/stages.rb          |  2 +-
 lib/gitlab/config/entry/configurable.rb       |  1 +
 lib/gitlab/config/entry/factory.rb            | 23 +++++++------
 lib/gitlab/config/entry/node.rb               |  2 +-
 lib/gitlab/config/entry/simplifiable.rb       |  2 +-
 .../lib/gitlab/ci/config/entry/policy_spec.rb | 33 +++----------------
 .../gitlab/config/entry/configurable_spec.rb  | 10 ++++--
 11 files changed, 33 insertions(+), 55 deletions(-)

diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb
index 326f2df1ea7..1d8904f7b29 100644
--- a/lib/gitlab/ci/config/entry/job.rb
+++ b/lib/gitlab/ci/config/entry/job.rb
@@ -66,7 +66,8 @@ module Gitlab
             description: 'Services that will be used to execute this job.'
 
           entry :only, Entry::Policy,
-            description: 'Refs policy this job will be executed for.'
+            description: 'Refs policy this job will be executed for.',
+            default: { refs: %w[branches tags] }
 
           entry :except, Entry::Policy,
             description: 'Refs policy this job will be executed for.'
diff --git a/lib/gitlab/ci/config/entry/key.rb b/lib/gitlab/ci/config/entry/key.rb
index 23d0dea0eb3..0c10967e629 100644
--- a/lib/gitlab/ci/config/entry/key.rb
+++ b/lib/gitlab/ci/config/entry/key.rb
@@ -14,7 +14,7 @@ module Gitlab
             validates :config, key: true
           end
 
-          def self.default(**)
+          def self.default
             'default'
           end
         end
diff --git a/lib/gitlab/ci/config/entry/policy.rb b/lib/gitlab/ci/config/entry/policy.rb
index bd571b8ea83..5195c4d87fe 100644
--- a/lib/gitlab/ci/config/entry/policy.rb
+++ b/lib/gitlab/ci/config/entry/policy.rb
@@ -65,13 +65,7 @@ module Gitlab
           end
 
           def value
-            self.class.default(key: @subject.key).yield_self do |default|
-              default.to_h.deep_merge(@subject.value.to_h)
-            end
-          end
-
-          def self.default(**attributes)
-            { refs: %w(branches tags) } if attributes[:key] == :only
+            default.to_h.deep_merge(@subject.value.to_h)
           end
         end
       end
diff --git a/lib/gitlab/ci/config/entry/stage.rb b/lib/gitlab/ci/config/entry/stage.rb
index 8c8494829d3..d6d576a3139 100644
--- a/lib/gitlab/ci/config/entry/stage.rb
+++ b/lib/gitlab/ci/config/entry/stage.rb
@@ -14,7 +14,7 @@ module Gitlab
             validates :config, type: String
           end
 
-          def self.default(**)
+          def self.default
             'test'
           end
         end
diff --git a/lib/gitlab/ci/config/entry/stages.rb b/lib/gitlab/ci/config/entry/stages.rb
index da91d57c2e7..2d715cbc6bb 100644
--- a/lib/gitlab/ci/config/entry/stages.rb
+++ b/lib/gitlab/ci/config/entry/stages.rb
@@ -14,7 +14,7 @@ module Gitlab
             validates :config, array_of_strings: true
           end
 
-          def self.default(**)
+          def self.default
             %w[build test deploy]
           end
         end
diff --git a/lib/gitlab/config/entry/configurable.rb b/lib/gitlab/config/entry/configurable.rb
index afdb60b2cd5..37ba16dba25 100644
--- a/lib/gitlab/config/entry/configurable.rb
+++ b/lib/gitlab/config/entry/configurable.rb
@@ -56,6 +56,7 @@ module Gitlab
           def entry(key, entry, metadata)
             factory = ::Gitlab::Config::Entry::Factory.new(entry)
               .with(description: metadata[:description])
+              .with(default: metadata[:default])
 
             (@nodes ||= {}).merge!(key.to_sym => factory)
           end
diff --git a/lib/gitlab/config/entry/factory.rb b/lib/gitlab/config/entry/factory.rb
index 089a20dd324..79f9ff32514 100644
--- a/lib/gitlab/config/entry/factory.rb
+++ b/lib/gitlab/config/entry/factory.rb
@@ -12,7 +12,7 @@ module Gitlab
         def initialize(entry)
           @entry = entry
           @metadata = {}
-          @attributes = {}
+          @attributes = { default: entry.default }
         end
 
         def value(value)
@@ -21,12 +21,12 @@ module Gitlab
         end
 
         def metadata(metadata)
-          @metadata.merge!(metadata)
+          @metadata.merge!(metadata.compact)
           self
         end
 
         def with(attributes)
-          @attributes.merge!(attributes)
+          @attributes.merge!(attributes.compact)
           self
         end
 
@@ -38,9 +38,7 @@ module Gitlab
           # See issue #18775.
           #
           if @value.nil?
-            Entry::Unspecified.new(
-              fabricate_unspecified
-            )
+            Entry::Unspecified.new(fabricate_unspecified)
           else
             fabricate(@entry, @value)
           end
@@ -53,12 +51,12 @@ module Gitlab
           # If entry has a default value we fabricate concrete node
           # with default value.
           #
-          @entry.default(@attributes).yield_self do |default|
-            if default.nil?
-              fabricate(Entry::Undefined)
-            else
-              fabricate(@entry, default)
-            end
+          default = @attributes.fetch(:default)
+
+          if default.nil?
+            fabricate(Entry::Undefined)
+          else
+            fabricate(@entry, default)
           end
         end
 
@@ -66,6 +64,7 @@ module Gitlab
           entry.new(value, @metadata).tap do |node|
             node.key = @attributes[:key]
             node.parent = @attributes[:parent]
+            node.default = @attributes[:default]
             node.description = @attributes[:description]
           end
         end
diff --git a/lib/gitlab/config/entry/node.rb b/lib/gitlab/config/entry/node.rb
index fe05158a73c..9999ab4ff95 100644
--- a/lib/gitlab/config/entry/node.rb
+++ b/lib/gitlab/config/entry/node.rb
@@ -10,7 +10,7 @@ module Gitlab
         InvalidError = Class.new(StandardError)
 
         attr_reader :config, :metadata
-        attr_accessor :key, :parent, :description
+        attr_accessor :key, :parent, :default, :description
 
         def initialize(config, **metadata)
           @config = config
diff --git a/lib/gitlab/config/entry/simplifiable.rb b/lib/gitlab/config/entry/simplifiable.rb
index 213a7c35f4b..22a0b448230 100644
--- a/lib/gitlab/config/entry/simplifiable.rb
+++ b/lib/gitlab/config/entry/simplifiable.rb
@@ -38,7 +38,7 @@ module Gitlab
           end
         end
 
-        def self.default(**)
+        def self.default
         end
       end
     end
diff --git a/spec/lib/gitlab/ci/config/entry/policy_spec.rb b/spec/lib/gitlab/ci/config/entry/policy_spec.rb
index d3e84298f1e..1c987e13a9a 100644
--- a/spec/lib/gitlab/ci/config/entry/policy_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/policy_spec.rb
@@ -169,9 +169,9 @@ describe Gitlab::Ci::Config::Entry::Policy do
   end
 
   describe '#value' do
-    context 'when it is `only` policy' do
+    context 'when default value has been provided' do
       before do
-        entry.key = :only
+        entry.default = { refs: %w[branches tags] }
       end
 
       context 'when user overrides default values' do
@@ -182,7 +182,7 @@ describe Gitlab::Ci::Config::Entry::Policy do
         end
       end
 
-      context 'when user does not override default values' do
+      context 'when default value has not been defined' do
         let(:config) { { variables: %w[$VARIABLE] } }
 
         it 'includes default values' do
@@ -191,34 +191,11 @@ describe Gitlab::Ci::Config::Entry::Policy do
         end
       end
     end
-
-    context 'when it is `except` policy' do
-      before do
-        entry.key = :except
-      end
-
-      context 'when user does not override default values' do
-        let(:config) { { variables: %w[$VARIABLE] } }
-
-        it 'does not include default values' do
-          expect(entry.value).to eq config
-        end
-      end
-    end
   end
 
   describe '.default' do
-    context 'when `only` policy is about to be fabricated' do
-      it 'has a default value' do
-        expect(described_class.default(key: :only))
-          .to eq(refs: %w[branches tags])
-      end
-    end
-
-    context 'when `except` policy is about to be fabricated' do
-      it 'does not have a default value' do
-        expect(described_class.default(key: :except)).to be_nil
-      end
+    it 'does not have default policy' do
+      expect(described_class.default).to be_nil
     end
   end
 end
diff --git a/spec/lib/gitlab/config/entry/configurable_spec.rb b/spec/lib/gitlab/config/entry/configurable_spec.rb
index 85a7cf1d241..09aa7f91b83 100644
--- a/spec/lib/gitlab/config/entry/configurable_spec.rb
+++ b/spec/lib/gitlab/config/entry/configurable_spec.rb
@@ -7,6 +7,10 @@ describe Gitlab::Config::Entry::Configurable do
     end
   end
 
+  before do
+    allow(entry).to receive(:default)
+  end
+
   describe 'validations' do
     context 'when entry is a hash' do
       let(:instance) { entry.new(key: 'value') }
@@ -26,9 +30,11 @@ describe Gitlab::Config::Entry::Configurable do
   end
 
   describe 'configured entries' do
+    let(:another) { double('another', default: nil) }
+
     before do
-      entry.class_eval do
-        entry :object, Object, description: 'test object'
+      entry.class_exec(another) do |another|
+        entry :object, another, description: 'test object'
       end
     end
 
-- 
2.30.9