Merge branch 'ci-pipeline-status-query' into 'master'

Optimise getting the pipeline status of commits

See merge request gitlab-org/gitlab-ce!15332

5 files changed, 114 insertions, 25 deletions

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 19814864e50..c77c837ff3f 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -149,34 +149,70 @@ module Ci
       end
     end
 
-    # ref can't be HEAD or SHA, can only be branch/tag name
-    scope :latest, ->(ref = nil) do
-      max_id = unscope(:select)
-        .select("max(#{quoted_table_name}.id)")
-        .group(:ref, :sha)
-
-      if ref
-        where(ref: ref, id: max_id.where(ref: ref))
-      else
-        where(id: max_id)
-      end
-    end
     scope :internal, -> { where(source: internal_sources) }
 
+    # Returns the pipelines in descending order (= newest first), optionally
+    # limited to a number of references.
+    #
+    # ref - The name (or names) of the branch(es)/tag(s) to limit the list of
+    #       pipelines to.
+    def self.newest_first(ref = nil)
+      relation = order(id: :desc)
+
+      ref ? relation.where(ref: ref) : relation
+    end
+
     def self.latest_status(ref = nil)
-      latest(ref).status
+      newest_first(ref).pluck(:status).first
     end
 
     def self.latest_successful_for(ref)
-      success.latest(ref).order(id: :desc).first
+      newest_first(ref).success.take
     end
 
     def self.latest_successful_for_refs(refs)
-      success.latest(refs).order(id: :desc).each_with_object({}) do |pipeline, hash|
+      relation = newest_first(refs).success
+
+      relation.each_with_object({}) do |pipeline, hash|
         hash[pipeline.ref] ||= pipeline
       end
     end
 
+    # Returns a Hash containing the latest pipeline status for every given
+    # commit.
+    #
+    # The keys of this Hash are the commit SHAs, the values the statuses.
+    #
+    # commits - The list of commit SHAs to get the status for.
+    # ref - The ref to scope the data to (e.g. "master"). If the ref is not
+    #       given we simply get the latest status for the commits, regardless
+    #       of what refs their pipelines belong to.
+    def self.latest_status_per_commit(commits, ref = nil)
+      p1 = arel_table
+      p2 = arel_table.alias
+
+      # This LEFT JOIN will filter out all but the newest row for every
+      # combination of (project_id, sha) or (project_id, sha, ref) if a ref is
+      # given.
+      cond = p1[:sha].eq(p2[:sha])
+        .and(p1[:project_id].eq(p2[:project_id]))
+        .and(p1[:id].lt(p2[:id]))
+
+      cond = cond.and(p1[:ref].eq(p2[:ref])) if ref
+      join = p1.join(p2, Arel::Nodes::OuterJoin).on(cond)
+
+      relation = select(:sha, :status)
+        .where(sha: commits)
+        .where(p2[:id].eq(nil))
+        .joins(join.join_sources)
+
+      relation = relation.where(ref: ref) if ref
+
+      relation.each_with_object({}) do |row, hash|
+        hash[row[:sha]] = row[:status]
+      end
+    end
+
     def self.truncate_sha(sha)
       sha[0...8]
     end
diff --git a/app/models/commit.rb b/app/models/commit.rb
index 6dba154a6ea..a31ebe9cc87 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -80,6 +80,7 @@ class Commit
 
     @raw = raw_commit
     @project = project
+    @statuses = {}
   end
 
   def id
@@ -236,11 +237,13 @@ class Commit
   end
 
   def status(ref = nil)
-    @statuses ||= {}
-
     return @statuses[ref] if @statuses.key?(ref)
 
-    @statuses[ref] = pipelines.latest_status(ref)
+    @statuses[ref] = project.pipelines.latest_status_per_commit(id, ref)[id]
+  end
+
+  def set_status_for_ref(ref, status)
+    @statuses[ref] = status
   end
 
   def signature
diff --git a/app/models/commit_collection.rb b/app/models/commit_collection.rb
new file mode 100644
index 00000000000..dd93af9df64
--- /dev/null
+++ b/app/models/commit_collection.rb
@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+# A collection of Commit instances for a specific project and Git reference.
+class CommitCollection
+  include Enumerable
+
+  attr_reader :project, :ref, :commits
+
+  # project - The project the commits belong to.
+  # commits - The Commit instances to store.
+  # ref - The name of the ref (e.g. "master").
+  def initialize(project, commits, ref = nil)
+    @project = project
+    @commits = commits
+    @ref = ref
+  end
+
+  def each(&block)
+    commits.each(&block)
+  end
+
+  # Sets the pipeline status for every commit.
+  #
+  # Setting this status ahead of time removes the need for running a query for
+  # every commit we're displaying.
+  def with_pipeline_status
+    statuses = project.pipelines.latest_status_per_commit(map(&:id), ref)
+
+    each do |commit|
+      commit.set_status_for_ref(ref, statuses[commit.id])
+    end
+
+    self
+  end
+
+  def respond_to_missing?(message, inc_private = false)
+    commits.respond_to?(message, inc_private)
+  end
+
+  # rubocop:disable GitlabSecurity/PublicSend
+  def method_missing(message, *args, &block)
+    commits.public_send(message, *args, &block)
+  end
+end
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index 1eda0f9cbbd..5382f5cc627 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -284,8 +284,10 @@ class MergeRequestDiff < ActiveRecord::Base
 
   def load_commits
     commits = st_commits.presence || merge_request_diff_commits
+    commits = commits.map { |commit| Commit.from_hash(commit.to_hash, project) }
 
-    commits.map { |commit| Commit.from_hash(commit.to_hash, project) }
+    CommitCollection
+      .new(merge_request.source_project, commits, merge_request.source_branch)
   end
 
   def save_diffs
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 3a89fa9264b..35ee12bdfa1 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -132,7 +132,8 @@ class Repository
 
     commits = Gitlab::Git::Commit.where(options)
     commits = Commit.decorate(commits, @project) if commits.present?
-    commits
+
+    CommitCollection.new(project, commits, ref)
   end
 
   def commits_between(from, to)
@@ -148,11 +149,14 @@ class Repository
     end
 
     raw_repository.gitaly_migrate(:commits_by_message) do |is_enabled|
-      if is_enabled
-        find_commits_by_message_by_gitaly(query, ref, path, limit, offset)
-      else
-        find_commits_by_message_by_shelling_out(query, ref, path, limit, offset)
-      end
+      commits =
+        if is_enabled
+          find_commits_by_message_by_gitaly(query, ref, path, limit, offset)
+        else
+          find_commits_by_message_by_shelling_out(query, ref, path, limit, offset)
+        end
+
+      CommitCollection.new(project, commits, ref)
     end
   end