Merge branch 'backstage/gb/refactor-only-except-policies-config' into 'master'

Refactor only/except configuration policies

See merge request gitlab-org/gitlab-ce!24359

Author: ayufan

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

@@ -16,13 +16,6 @@ class Job < ::Gitlab::Config::Entry::Node
                             dependencies before_script after_script variables
                             environment coverage retry parallel extends].freeze
 
-          DEFAULT_ONLY_POLICY = {
-            refs: %w(branches tags)
-          }.freeze
-
-          DEFAULT_EXCEPT_POLICY = {
-          }.freeze
-
           validations do
             validates :config, allowed_keys: ALLOWED_KEYS
             validates :config, presence: true
@@ -73,7 +66,8 @@ class Job < ::Gitlab::Config::Entry::Node
             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.'
@@ -156,8 +150,8 @@ def to_hash
               services: services_value,
               stage: stage_value,
               cache: cache_value,
-              only: DEFAULT_ONLY_POLICY.deep_merge(only_value.to_h),
-              except: DEFAULT_EXCEPT_POLICY.deep_merge(except_value.to_h),
+              only: only_value,
+              except: except_value,
               variables: variables_defined? ? variables_value : nil,
               environment: environment_defined? ? environment_value : nil,
               environment_name: environment_defined? ? environment_value[:name] : nil,

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

@@ -64,7 +64,8 @@ def errors
             end
           end
 
-          def self.default
+          def value
+            default.to_h.deep_merge(subject.value.to_h)
           end
         end
       end

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

@@ -82,9 +82,6 @@ def location
               'retry config'
             end
           end
-
-          def self.default
-          end
         end
       end
     end

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

@@ -14,7 +14,7 @@ class Variables < ::Gitlab::Config::Entry::Node
             validates :config, variables: true
           end
 
-          def self.default
+          def self.default(**)
             {}
           end
 

lib/gitlab/config/entry/configurable.rb

@@ -56,6 +56,7 @@ def nodes
           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

lib/gitlab/config/entry/factory.rb

@@ -12,7 +12,7 @@ class Factory
         def initialize(entry)
           @entry = entry
           @metadata = {}
-          @attributes = {}
+          @attributes = { default: entry.default }
         end
 
         def value(value)
@@ -21,12 +21,12 @@ def value(value)
         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 @@ def create!
           # See issue #18775.
           #
           if @value.nil?
-            Entry::Unspecified.new(
-              fabricate_unspecified
-            )
+            Entry::Unspecified.new(fabricate_unspecified)
           else
             fabricate(@entry, @value)
           end
@@ -53,17 +51,20 @@ def fabricate_unspecified
           # If entry has a default value we fabricate concrete node
           # with default value.
           #
-          if @entry.default.nil?
+          default = @attributes.fetch(:default)
+
+          if default.nil?
             fabricate(Entry::Undefined)
           else
-            fabricate(@entry, @entry.default)
+            fabricate(@entry, default)
           end
         end
 
         def fabricate(entry, value = nil)
           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

lib/gitlab/config/entry/node.rb

@@ -10,7 +10,7 @@ class Node
         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
@@ -85,7 +85,7 @@ def inspect
           "#<#{self.class.name} #{unspecified}{#{key}: #{val.inspect}}>"
         end
 
-        def self.default
+        def self.default(**)
         end
 
         def self.aspects

lib/gitlab/config/entry/simplifiable.rb

@@ -6,6 +6,8 @@ module Entry
       class Simplifiable < SimpleDelegator
         EntryStrategy = Struct.new(:name, :condition)
 
+        attr_reader :subject
+
         def initialize(config, **metadata)
           unless self.class.const_defined?(:UnknownStrategy)
             raise ArgumentError, 'UndefinedStrategy not available!'
@@ -17,7 +19,7 @@ def initialize(config, **metadata)
 
           entry = self.class.entry_class(strategy)
 
-          super(entry.new(config, metadata))
+          super(@subject = entry.new(config, metadata))
         end
 
         def self.strategy(name, **opts)
@@ -37,6 +39,9 @@ def self.entry_class(strategy)
             self::UnknownStrategy
           end
         end
+
+        def self.default
+        end
       end
     end
   end

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

@@ -160,8 +160,7 @@
                        variables: { 'VAR' => 'value' },
                        ignore: false,
                        after_script: ['make clean'],
-                       only: { refs: %w[branches tags] },
-                       except: {} },
+                       only: { refs: %w[branches tags] } },
               spinach: { name: :spinach,
                          before_script: [],
                          script: %w[spinach],
@@ -172,8 +171,7 @@
                          variables: {},
                          ignore: false,
                          after_script: ['make clean'],
-                         only: { refs: %w[branches tags] },
-                         except: {} }
+                         only: { refs: %w[branches tags] } }
             )
           end
         end

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

@@ -258,8 +258,7 @@
                    stage: 'test',
                    ignore: false,
                    after_script: %w[cleanup],
-                   only: { refs: %w[branches tags] },
-                   except: {})
+                   only: { refs: %w[branches tags] })
         end
       end
     end