From 580d250166d97bd5c2b0526be737d02806e577c2 Mon Sep 17 00:00:00 2001 From: Yorick Peterse 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 From 021d3810c300d1e0514f21ccb6f1439f59e20565 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 2 Jun 2016 16:19:18 +0200 Subject: Rename Ci::Commit to Ci::Pipeline and rename some of the ci_commit to pipeline --- app/models/commit.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'app/models/commit.rb') diff --git a/app/models/commit.rb b/app/models/commit.rb index f96c7cb34d0..b5637bc4fbc 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -214,13 +214,13 @@ class Commit @raw.short_id(7) end - def ci_commits - @ci_commits ||= project.ci_commits.where(sha: sha) + def pipelines + @pipeline ||= project.pipelines.where(sha: sha) end def status return @status if defined?(@status) - @status ||= ci_commits.status + @status ||= pipelines.status end def revert_branch_name -- cgit v1.2.1 From ad83c3085513dd248b979d445e545e88a17c6ebc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Fri, 3 Jun 2016 14:47:09 -0400 Subject: Remove `projects` inclusion in `notes_with_associations` to skip some unnecessary queries `notes_with_associations` are used for `participant` declarations, but `Participable` only really cares about the target entity project, and not the participants projects. `notes_with_associations` are also used in `Commit::has_been_reverted?` which employs the reference extractor of the commit, so no references to the notes projects are made there (`Mentionable::all_references` cares only about the `author` and other `attr_mentionable`). A paralel situation occurs on `Issue::referenced_merge_requests`. --- app/models/commit.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/models/commit.rb') diff --git a/app/models/commit.rb b/app/models/commit.rb index b5637bc4fbc..d69d518fadd 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -198,7 +198,7 @@ class Commit end def notes_with_associations - notes.includes(:author, :project) + notes.includes(:author) end def method_missing(m, *args, &block) -- cgit v1.2.1