From 580d250166d97bd5c2b0526be737d02806e577c2 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Thu, 26 May 2016 13:38:28 +0200
Subject: Refactor Participable

There are several changes to this module:

1. The use of an explicit stack in Participable#participants
2. Proc behaviour has been changed
3. Batch permissions checking

== Explicit Stack

Participable#participants no longer uses recursion to process "self" and
all child objects, instead it uses an Array and processes objects in
breadth-first order. This allows us to for example create a single
Gitlab::ReferenceExtractor instance and pass this to any Procs. Re-using
a ReferenceExtractor removes the need for running potentially many SQL
queries every time a Proc is called on a new object.

== Proc Behaviour Changed

Previously a Proc in Participable was expected to return an Array of
User instances. This has been changed and instead it's now expected that
a Proc modifies the Gitlab::ReferenceExtractor passed to it. The return
value of the Proc is ignored.

== Permissions Checking

The method Participable#participants uses
Ability.users_that_can_read_project to check if the returned users have
access to the project of "self" _without_ running multiple SQL queries
for every user.
---
 app/models/commit.rb | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

(limited to 'app/models/commit.rb')

diff --git a/app/models/commit.rb b/app/models/commit.rb
index 562c3ed15b2..f96c7cb34d0 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -8,7 +8,10 @@ class Commit
   include StaticModel
 
   attr_mentionable :safe_message, pipeline: :single_line
-  participant :author, :committer, :notes
+
+  participant :author
+  participant :committer
+  participant :notes_with_associations
 
   attr_accessor :project
 
@@ -194,6 +197,10 @@ class Commit
     project.notes.for_commit_id(self.id)
   end
 
+  def notes_with_associations
+    notes.includes(:author, :project)
+  end
+
   def method_missing(m, *args, &block)
     @raw.send(m, *args, &block)
   end
@@ -219,7 +226,7 @@ class Commit
   def revert_branch_name
     "revert-#{short_id}"
   end
-  
+
   def cherry_pick_branch_name
     project.repository.next_branch("cherry-pick-#{short_id}", mild: true)
   end
@@ -251,11 +258,13 @@ class Commit
   end
 
   def has_been_reverted?(current_user = nil, noteable = self)
-    Gitlab::ReferenceExtractor.lazily do
-      noteable.notes.system.flat_map do |note|
-        note.all_references(current_user).commits
-      end
-    end.any? { |commit_ref| commit_ref.reverts_commit?(self) }
+    ext = all_references(current_user)
+
+    noteable.notes_with_associations.system.each do |note|
+      note.all_references(current_user, extractor: ext)
+    end
+
+    ext.commits.any? { |commit_ref| commit_ref.reverts_commit?(self) }
   end
 
   def change_type_title
-- 
cgit v1.2.1