From 1af2274b4126b3e910604ef321b58d327ea21e63 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 25 May 2018 13:43:11 +0200 Subject: [PATCH] Restore lazy loading of pipeline commits in preloader --- lib/gitlab/ci/pipeline/preloader.rb | 14 +++++++++----- spec/lib/gitlab/ci/pipeline/preloader_spec.rb | 19 ++++++++++++++++++- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/lib/gitlab/ci/pipeline/preloader.rb b/lib/gitlab/ci/pipeline/preloader.rb index 6db6554a606..db0a1ea4dab 100644 --- a/lib/gitlab/ci/pipeline/preloader.rb +++ b/lib/gitlab/ci/pipeline/preloader.rb @@ -7,9 +7,16 @@ module Gitlab # authors. class Preloader def self.preload!(pipelines) + ## + # This preloads all commits at once, because `Ci::Pipeline#commit` is + # using a lazy batch loading, what results in only one batched Gitaly + # call. + # + pipelines.each(&:commit) + pipelines.each do |pipeline| self.new(pipeline).tap do |preloader| - preloader.preload_commits + preloader.preload_commit_authors preloader.preload_pipeline_warnings preloader.preload_stages_warnings end @@ -20,10 +27,7 @@ module Gitlab @pipeline = pipeline end - def preload_commits - # This ensures that all the pipeline commits are eager loaded before we - # start using them. - # + def preload_commit_authors # 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) diff --git a/spec/lib/gitlab/ci/pipeline/preloader_spec.rb b/spec/lib/gitlab/ci/pipeline/preloader_spec.rb index 7821d28ab06..40dfd893465 100644 --- a/spec/lib/gitlab/ci/pipeline/preloader_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/preloader_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'fast_spec_helper' +require 'spec_helper' describe Gitlab::Ci::Pipeline::Preloader do let(:stage) { double(:stage) } @@ -11,6 +11,23 @@ describe Gitlab::Ci::Pipeline::Preloader do end describe '.preload!' do + context 'when preloading multiple commits' do + let(:project) { create(:project, :repository) } + + it 'preloads all commits once' do + expect(Commit).to receive(:decorate).once.and_call_original + + pipelines = [build_pipeline(ref: 'HEAD'), + build_pipeline(ref: 'HEAD~1')] + + described_class.preload!(pipelines) + end + + def build_pipeline(ref:) + build_stubbed(:ci_pipeline, project: project, sha: project.commit(ref).id) + end + end + it 'preloads commit authors and number of warnings' do expect(commit).to receive(:lazy_author) expect(pipeline).to receive(:number_of_warnings)