Refactor Gitlab::Ci::Config::Normalizer

Use Hash#each_with_object to manipulate job hashes.

3 files changed, 56 insertions, 44 deletions

diff --git a/lib/gitlab/ci/config/normalizer.rb b/lib/gitlab/ci/config/normalizer.rb
index 0eae6b9770b..7bd0e08e817 100644
--- a/lib/gitlab/ci/config/normalizer.rb
+++ b/lib/gitlab/ci/config/normalizer.rb
@@ -4,54 +4,56 @@ module Gitlab
   module Ci
     class Config
       class Normalizer
-        def initialize(jobs_config)
-          @jobs_config = jobs_config
-        end
-
-        def normalize_jobs
-          parallelized_jobs = parallelize_jobs
-          parallelize_dependencies(parallelized_jobs)
-        end
+        class << self
+          def normalize_jobs(jobs_config)
+            parallelized_jobs = extract_parallelized_jobs(jobs_config)
+            parallelized_config = parallelize_jobs(jobs_config, parallelized_jobs)
+            parallelize_dependencies(parallelized_config, parallelized_jobs)
+          end
+
+          private
+
+          def extract_parallelized_jobs(jobs_config)
+            parallelized_jobs = {}
+
+            jobs_config.each do |job_name, config|
+              if config[:parallel]
+                parallelized_jobs[job_name] = parallelize_job_names(job_name, config[:parallel])
+              end
+            end
 
-        private
+            parallelized_jobs
+          end
 
-        def parallelize_jobs
-          parallelized_jobs = {}
+          def parallelize_jobs(jobs_config, parallelized_jobs)
+            jobs_config.each_with_object({}) do |(job_name, config), hash|
+              if parallelized_jobs.keys.include?(job_name)
+                parallelized_jobs[job_name].each { |name, index| hash[name.to_sym] = config.merge(name: name, instance: index) }
+              else
+                hash[job_name] = config
+              end
 
-          @jobs_config = @jobs_config.map do |name, config|
-            if config[:parallel]
-              total = config[:parallel]
-              names = self.class.parallelize_job_names(name, total)
-              parallelized_jobs[name] = names.map(&:first)
-              Hash[names.collect { |job_name, index| [job_name.to_sym, config.merge(name: job_name, instance: index)] }]
-            else
-              { name => config }
+              hash
             end
-          end.reduce(:merge)
-
-          parallelized_jobs
-        end
-
-        def parallelize_dependencies(parallelized_jobs)
-          @jobs_config.map do |name, config|
-            if config[:dependencies]
-              deps = config[:dependencies].map do |dep|
-                if parallelized_jobs.keys.include?(dep.to_sym)
-                  parallelized_jobs[dep.to_sym]
-                else
-                  dep
-                end
-              end.flatten
-
-              { name => config.merge(dependencies: deps) }
-            else
-              { name => config }
+          end
+
+          def parallelize_dependencies(parallelized_config, parallelized_jobs)
+            parallelized_config.each_with_object({}) do |(job_name, config), hash|
+              intersection = config[:dependencies] & parallelized_jobs.keys.map(&:to_s)
+              if intersection && intersection.any?
+                deps = intersection.map { |dep| parallelized_jobs[dep.to_sym].map(&:first) }.flatten
+                hash[job_name] = config.merge(dependencies: deps)
+              else
+                hash[job_name] = config
+              end
+
+              hash
             end
-          end.reduce(:merge)
-        end
+          end
 
-        def self.parallelize_job_names(name, total)
-          Array.new(total) { |index| ["#{name} #{index + 1}/#{total}", index + 1] }
+          def parallelize_job_names(name, total)
+            Array.new(total) { |index| ["#{name} #{index + 1}/#{total}", index + 1] }
+          end
         end
       end
     end
diff --git a/lib/gitlab/ci/yaml_processor.rb b/lib/gitlab/ci/yaml_processor.rb
index f65a4e198cc..63b55c57913 100644
--- a/lib/gitlab/ci/yaml_processor.rb
+++ b/lib/gitlab/ci/yaml_processor.rb
@@ -104,7 +104,7 @@ module Gitlab
         ##
         # Jobs
         #
-        @jobs = Ci::Config::Normalizer.new(@ci_config.jobs).normalize_jobs
+        @jobs = Ci::Config::Normalizer.normalize_jobs(@ci_config.jobs)
 
         @jobs.each do |name, job|
           # logical validation for job
diff --git a/spec/lib/gitlab/ci/config/normalizer_spec.rb b/spec/lib/gitlab/ci/config/normalizer_spec.rb
index 84f8ea3e861..2c8396199d0 100644
--- a/spec/lib/gitlab/ci/config/normalizer_spec.rb
+++ b/spec/lib/gitlab/ci/config/normalizer_spec.rb
@@ -8,7 +8,7 @@ describe Gitlab::Ci::Config::Normalizer do
   let(:config) { { job_name => job_config } }
 
   describe '.normalize_jobs' do
-    subject { described_class.new(config).normalize_jobs }
+    subject { described_class.normalize_jobs(config) }
 
     it 'does not have original job' do
       is_expected.not_to include(job_name)
@@ -30,6 +30,16 @@ describe Gitlab::Ci::Config::Normalizer do
 
       expect(configs).to all(eq(original_config))
     end
+
+    context 'when jobs depend on parallelized jobs' do
+      let(:config) { { job_name => job_config, other_job: { script: 'echo 1', dependencies: [job_name.to_s] } } }
+
+      it 'parallelizes dependencies' do
+        job_names = described_class.send(:parallelize_job_names, job_name, 5).map(&:first)
+
+        expect(subject[:other_job][:dependencies]).to include(*job_names)
+      end
+    end
   end
 
   describe '.parallelize_job_names' do