Remove duplication in Project methods.

2 files changed, 21 insertions, 38 deletions

diff --git a/app/finders/snippets_finder.rb b/app/finders/snippets_finder.rb
index 7a343faecdd..c4a84d88dbd 100644
--- a/app/finders/snippets_finder.rb
+++ b/app/finders/snippets_finder.rb
@@ -56,7 +56,10 @@ class SnippetsFinder < UnionFinder
   end
 
   def feature_available_projects
-    projects = Project.public_or_visible_to_user_with_feature_available(current_user, :snippets).select(:id)
+    projects = Project.public_or_visible_to_user(current_user) do |part|
+      part.with_feature_available_for_user(:snippets, current_user)
+    end.select(:id)
+
     arel_query = Arel::Nodes::SqlLiteral.new(projects.to_sql)
     table[:project_id].in(arel_query)
   end
diff --git a/app/models/project.rb b/app/models/project.rb
index e575ab9a728..14bc2185d1b 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -316,59 +316,39 @@ class Project < ActiveRecord::Base
 
   # Returns a collection of projects that is either public or visible to the
   # logged in user.
-  def self.public_or_visible_to_user(user = nil)
-    if user
-      authorized = user
-        .project_authorizations
-        .select(1)
-        .where('project_authorizations.project_id = projects.id')
+  #
+  # A caller may pass in a block to modify individual parts of
+  # the query, e.g. to apply .with_feature_available_for_user on top of it.
+  # This is useful for performance as we can stick those additional filters
+  # at the bottom of e.g. the UNION.
+  def self.public_or_visible_to_user(user = nil, &block)
+    # If we don't get a block passed, use identity to avoid if/else repetitions
+    block = ->(part) { part } unless block_given?
 
+    if user
       levels = Gitlab::VisibilityLevel.levels_for_user(user)
 
       if Gitlab::VisibilityLevel.all_levels?(levels)
         # If the user is allowed to see all projects,
         # we can shortcut and just return.
-        return all
+        return block.call(all)
       end
 
-      authorized_projects = where('EXISTS (?)', authorized).select(:id)
-      visible_projects = where('visibility_level IN (?)', levels).select(:id)
-
-      # We use a UNION here instead of OR clauses since this results in better
-      # performance.
-      union = Gitlab::SQL::Union.new([authorized_projects, visible_projects])
-      where("projects.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection
-    else
-      public_to_user
-    end
-  end
-
-  # Combination of .public_or_visible_to_user AND .with_feature_available_for_user
-  # We duplicated this for (database) performance reasons to optimize the query.
-  def self.public_or_visible_to_user_with_feature_available(user, feature)
-    if user
       authorized = user
         .project_authorizations
         .select(1)
-        .where('project_authorizations.project_id = projects.id')
-
-      levels = Gitlab::VisibilityLevel.levels_for_user(user)
+        .where('project_authorizations.project_id = p1.id')
+      authorized_projects = block.call(from("#{table_name} AS p1").where('EXISTS (?)', authorized))
 
-      if Gitlab::VisibilityLevel.all_levels?(levels)
-        # If the user is allowed to see all projects,
-        # we can shortcut and just return.
-        return all.with_feature_available_for_user(feature, user)
-      end
-
-      authorized_projects = where('EXISTS (?)', authorized).with_feature_available_for_user(feature, user).select(:id)
-      visible_projects = where('visibility_level IN (?)', levels).with_feature_available_for_user(feature, user).select(:id)
+      visible_projects = block.call(from("#{table_name} AS p2").where('visibility_level IN (?)', levels))
 
       # We use a UNION here instead of OR clauses since this results in better
       # performance.
-      union = Gitlab::SQL::Union.new([authorized_projects, visible_projects])
-      from("(#{union.to_sql}) projects")
+      union = Gitlab::SQL::Union.new([authorized_projects.select('p1.id'), visible_projects.select('p2.id')])
+      # TODO: from("(#{union.to_sql}) AS #{table_name}")
+      where("projects.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection
     else
-      public_to_user.with_feature_available_for_user(feature, user)
+      block.call(public_to_user)
     end
   end