From ea35fd05bb175cf921e94d8b1c5e6bc5f3e217c9 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 22 May 2018 13:55:05 +0200
Subject: [PATCH] Refactor pipeline preloader to split reponsibilities better

---
 .../projects/pipelines_controller.rb          |  2 +-
 lib/gitlab/ci/pipeline/preloader.rb           | 40 ++++++++++++++-----
 .../projects/pipelines_controller_spec.rb     |  2 -
 spec/lib/gitlab/ci/pipeline/preloader_spec.rb | 22 +++++-----
 4 files changed, 43 insertions(+), 23 deletions(-)

diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb
index 7c18c0c4fcb..c8a4709fe98 100644
--- a/app/controllers/projects/pipelines_controller.rb
+++ b/app/controllers/projects/pipelines_controller.rb
@@ -24,7 +24,7 @@ class Projects::PipelinesController < Projects::ApplicationController
     @finished_count = limited_pipelines_count(project, 'finished')
     @pipelines_count = limited_pipelines_count(project)
 
-    Gitlab::Ci::Pipeline::Preloader.preload(@pipelines)
+    Gitlab::Ci::Pipeline::Preloader.new(@pipelines).preload!
 
     respond_to do |format|
       format.html
diff --git a/lib/gitlab/ci/pipeline/preloader.rb b/lib/gitlab/ci/pipeline/preloader.rb
index 2f792062124..668c1b7189c 100644
--- a/lib/gitlab/ci/pipeline/preloader.rb
+++ b/lib/gitlab/ci/pipeline/preloader.rb
@@ -5,26 +5,46 @@ module Gitlab
     module Pipeline
       # Class for preloading data associated with pipelines such as commit
       # authors.
-      module Preloader
-        def self.preload(pipelines)
-          # This ensures that all the pipeline commits are eager loaded before we
-          # start using them.
-          pipelines.each(&:commit)
+      class Preloader
+        def initialize(pipelines)
+          @pipelines = pipelines
+        end
 
-          pipelines.each do |pipeline|
-            # This preloads the author of every commit. We're using "lazy_author"
+        def preload!
+          @pipelines.each do |pipeline|
+            Pipeline::Preloader::Instance.new(pipeline)
+              .preload_commits
+              .preload_pipeline_warnings
+              .preload_stages_warnings
+          end
+        end
+
+        class Instance
+          def initialize(pipeline)
+            @pipeline = pipeline
+          end
+
+          def preload_commits
+            # This ensures that all the pipeline commits are eager loaded before we
+            # start using them.
+            #
+            # This also preloads the author of every commit. We're using "lazy_author"
             # here since "author" immediately loads the data on the first call.
-            pipeline.commit.try(:lazy_author)
+            tap { @pipeline.commit.try(:lazy_author) }
+          end
 
+          def preload_pipeline_warnings
             # This preloads the number of warnings for every pipeline, ensuring
             # that Ci::Pipeline#has_warnings? doesn't execute any additional
             # queries.
-            pipeline.number_of_warnings
+            tap { @pipeline.number_of_warnings }
+          end
 
+          def preload_stages_warnings
             # This preloads the number of warnings for every stage, ensuring
             # that Ci::Stage#has_warnings? doesn't execute any additional
             # queries.
-            pipeline.stages.each { |stage| stage.number_of_warnings }
+            tap { @pipeline.stages.each { |stage| stage.number_of_warnings } }
           end
         end
       end
diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb
index c8080056f69..536ef9b2d79 100644
--- a/spec/controllers/projects/pipelines_controller_spec.rb
+++ b/spec/controllers/projects/pipelines_controller_spec.rb
@@ -36,8 +36,6 @@ describe Projects::PipelinesController do
       expect(json_response['count']['running']).to eq '1'
       expect(json_response['count']['pending']).to eq '1'
       expect(json_response['count']['finished']).to eq '2'
-      puts queries.log
-      puts "Queries count: #{queries.count}"
       expect(queries.count).to be < 32
     end
 
diff --git a/spec/lib/gitlab/ci/pipeline/preloader_spec.rb b/spec/lib/gitlab/ci/pipeline/preloader_spec.rb
index 477c7477df0..c8cfe2c696d 100644
--- a/spec/lib/gitlab/ci/pipeline/preloader_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/preloader_spec.rb
@@ -1,20 +1,22 @@
 # frozen_string_literal: true
 
-require 'spec_helper'
+require 'fast_spec_helper'
 
 describe Gitlab::Ci::Pipeline::Preloader do
-  describe '.preload' do
-    it 'preloads the author of every pipeline commit' do
-      commit = double(:commit)
-      pipeline = double(:pipeline, commit: commit)
+  let(:stage) { double(:stage) }
+  let(:commit) { double(:commit) }
 
-      expect(commit)
-        .to receive(:lazy_author)
+  let(:pipeline) do
+    double(:pipeline, commit: commit, stages: [stage])
+  end
 
-      expect(pipeline)
-        .to receive(:number_of_warnings)
+  describe '#preload!' do
+    it 'preloads commit authors and number of warnings' do
+      expect(commit).to receive(:lazy_author)
+      expect(pipeline).to receive(:number_of_warnings)
+      expect(stage).to receive(:number_of_warnings)
 
-      described_class.preload([pipeline])
+      described_class.new([pipeline]).preload!
     end
   end
 end