Preloading of Ci::Pipeline#has_warnings?

This changes Ci::Pipeline#has_warnings? so it supports preloading of the
number of warnings per pipeline. This removes the need for executing a
COUNT(*) query for every pipeline just to see if it has any warnings or
not.

4 files changed, 47 insertions, 1 deletions

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 0b90834d415..60972c4fc69 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -402,7 +402,18 @@ module Ci
     end
 
     def has_warnings?
-      builds.latest.failed_but_allowed.any?
+      number_of_warnings.positive?
+    end
+
+    def number_of_warnings
+      BatchLoader.for(id).batch(default_value: 0) do |pipeline_ids, loader|
+        Build.where(commit_id: pipeline_ids)
+          .latest
+          .failed_but_allowed
+          .group(:commit_id)
+          .count
+          .each { |id, amount| loader.call(id, amount) }
+      end
     end
 
     def set_config_source
diff --git a/lib/gitlab/ci/pipeline_data_preloader.rb b/lib/gitlab/ci/pipeline_data_preloader.rb
index 98aaa86d1b8..f065d12ad4e 100644
--- a/lib/gitlab/ci/pipeline_data_preloader.rb
+++ b/lib/gitlab/ci/pipeline_data_preloader.rb
@@ -14,6 +14,11 @@ module Gitlab
           # This 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)
+
+          # 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
         end
       end
     end
diff --git a/spec/lib/gitlab/ci/pipeline_data_preloader_spec.rb b/spec/lib/gitlab/ci/pipeline_data_preloader_spec.rb
index 1aa6fa91edc..11488ff1d32 100644
--- a/spec/lib/gitlab/ci/pipeline_data_preloader_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline_data_preloader_spec.rb
@@ -11,6 +11,9 @@ describe Gitlab::Ci::PipelineDataPreloader do
       expect(commit)
         .to receive(:lazy_author)
 
+      expect(pipeline)
+        .to receive(:number_of_warnings)
+
       described_class.preload([pipeline])
     end
   end
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index ddd66a6be87..e7845b693a1 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -774,6 +774,33 @@ describe Ci::Pipeline, :mailer do
     end
   end
 
+  describe '#number_of_warnings' do
+    it 'returns the number of warnings' do
+      create(:ci_build, :allowed_to_fail, :failed, pipeline: pipeline, name: 'rubocop')
+
+      expect(pipeline.number_of_warnings).to eq(1)
+    end
+
+    it 'supports eager loading of the number of warnings' do
+      pipeline2 = create(:ci_empty_pipeline, status: :created, project: project)
+
+      create(:ci_build, :allowed_to_fail, :failed, pipeline: pipeline, name: 'rubocop')
+      create(:ci_build, :allowed_to_fail, :failed, pipeline: pipeline2, name: 'rubocop')
+
+      pipelines = project.pipelines.to_a
+
+      pipelines.each(&:number_of_warnings)
+
+      # To run the queries we need to actually use the lazy objects, which we do
+      # by just sending "to_i" to them.
+      amount = ActiveRecord::QueryRecorder
+        .new { pipelines.each { |p| p.number_of_warnings.to_i } }
+        .count
+
+      expect(amount).to eq(1)
+    end
+  end
+
   shared_context 'with some outdated pipelines' do
     before do
       create_pipeline(:canceled, 'ref', 'A', project)