Refactor pipeline preloader to split reponsibilities better
This commit is contained in:
		
							parent
							
								
									76a7157c76
								
							
						
					
					
						commit
						ea35fd05bb
					
				|  | @ -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 | ||||
|  |  | |||
|  | @ -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 | ||||
|  |  | |||
|  | @ -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 | ||||
| 
 | ||||
|  |  | |||
|  | @ -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 | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue