Refactor ProjectsFinder#init_collection

This changes ProjectsFinder#init_collection so it no longer relies on a
UNION. For example, to get starred projects of a user we used to run:

    SELECT projects.*
    FROM projects
    WHERE projects.pending_delete = 'f'
    AND (
        projects.id IN (
            SELECT projects.id
            FROM projects
            INNER JOIN users_star_projects
                ON users_star_projects.project_id = projects.id
            INNER JOIN project_authorizations
                ON projects.id = project_authorizations.project_id
            WHERE projects.pending_delete = 'f'
            AND project_authorizations.user_id = 1
            AND users_star_projects.user_id = 1

            UNION

            SELECT projects.id
            FROM projects
            INNER JOIN users_star_projects
                ON users_star_projects.project_id = projects.id
            WHERE projects.visibility_level IN (20, 10)
            AND users_star_projects.user_id = 1
        )
    )
    ORDER BY projects.id DESC;

With these changes the above query is turned into the following instead:

    SELECT projects.*
    FROM projects
    INNER JOIN users_star_projects
        ON users_star_projects.project_id = projects.id
    WHERE projects.pending_delete = 'f'
    AND (
        EXISTS (
            SELECT 1
            FROM project_authorizations
            WHERE project_authorizations.user_id = 1
            AND (project_id = projects.id)
        )
        OR projects.visibility_level IN (20,10)
    )
    AND users_star_projects.user_id = 1
    ORDER BY projects.id DESC;

This query in turn produces a better execution plan and takes less time,
though the difference is only a few milliseconds (this however depends
on the amount of data involved and additional conditions that may be
added).

app/finders/projects_finder.rb

@@ -28,34 +28,72 @@ class ProjectsFinder < UnionFinder
   end
 
   def execute
-    items = init_collection
-    items = items.map do |item|
-      item = by_ids(item)
-      item = by_personal(item)
-      item = by_starred(item)
-      item = by_trending(item)
-      item = by_visibilty_level(item)
-      item = by_tags(item)
-      item = by_search(item)
-      by_archived(item)
-    end
-    items = union(items)
-    sort(items)
+    collection = init_collection
+    collection = by_ids(collection)
+    collection = by_personal(collection)
+    collection = by_starred(collection)
+    collection = by_trending(collection)
+    collection = by_visibilty_level(collection)
+    collection = by_tags(collection)
+    collection = by_search(collection)
+    collection = by_archived(collection)
+
+    sort(collection)
   end
 
   private
 
   def init_collection
-    projects = []
-
-    if params[:owned].present?
-      projects << current_user.owned_projects if current_user
+    if current_user
+      collection_with_user
     else
-      projects << current_user.authorized_projects if current_user
-      projects << Project.unscoped.public_to_user(current_user) unless params[:non_public].present?
+      collection_without_user
     end
+  end
 
-    projects
+  def collection_with_user
+    if owned_projects?
+      current_user.owned_projects
+    else
+      if private_only?
+        current_user.authorized_projects
+      else
+        collection_with_user_and_public_projects
+      end
+    end
+  end
+
+  # Builds a collection for a signed in user that includes additional projects
+  # such as public and internal ones.
+  #
+  # This method manually constructs some WHERE conditions in order to ensure the
+  # produced query is as efficient as possible.
+  def collection_with_user_and_public_projects
+    levels = Gitlab::VisibilityLevel.levels_for_user(current_user)
+    authorized = current_user.project_authorizations.
+      select(1).
+      where('project_id = projects.id')
+
+    Project.where('EXISTS (?) OR projects.visibility_level IN (?)',
+                  authorized,
+                  levels)
+  end
+
+  # Builds a collection for an anonymous user.
+  def collection_without_user
+    if private_only? || owned_projects?
+      Project.none
+    else
+      Project.public_to_user
+    end
+  end
+
+  def owned_projects?
+    params[:owned].present?
+  end
+
+  def private_only?
+    params[:non_public].present?
   end
 
   def by_ids(items)

changelogs/unreleased/refactor-projects-finder-init-collection.yml

@@ -0,0 +1,5 @@
+---
+title: Refactor ProjectsFinder#init_collection to produce more efficient queries for
+  retrieving projects
+merge_request:
+author:

lib/gitlab/visibility_level.rb

@@ -13,18 +13,8 @@ module Gitlab
       scope :public_and_internal_only,  -> { where(visibility_level: [PUBLIC, INTERNAL] ) }
       scope :non_public_only,           -> { where.not(visibility_level: PUBLIC) }
 
-      scope :public_to_user, -> (user) do
-        if user
-          if user.admin?
-            all
-          elsif !user.external?
-            public_and_internal_only
-          else
-            public_only
-          end
-        else
-          public_only
-        end
+      scope :public_to_user, -> (user = nil) do
+        where(visibility_level: VisibilityLevel.levels_for_user(user))
       end
     end
 
@@ -35,6 +25,18 @@ module Gitlab
     class << self
       delegate :values, to: :options
 
+      def levels_for_user(user = nil)
+        return [PUBLIC] unless user
+
+        if user.admin?
+          [PRIVATE, INTERNAL, PUBLIC]
+        elsif user.external?
+          [PUBLIC]
+        else
+          [INTERNAL, PUBLIC]
+        end
+      end
+
       def string_values
         string_options.keys
       end

spec/lib/gitlab/visibility_level_spec.rb

@@ -18,4 +18,35 @@ describe Gitlab::VisibilityLevel, lib: true do
       expect(described_class.level_value(100)).to eq(Gitlab::VisibilityLevel::PRIVATE)
     end
   end
+
+  describe '.levels_for_user' do
+    it 'returns all levels for an admin' do
+      user = double(:user, admin?: true)
+
+      expect(described_class.levels_for_user(user)).
+        to eq([Gitlab::VisibilityLevel::PRIVATE,
+               Gitlab::VisibilityLevel::INTERNAL,
+               Gitlab::VisibilityLevel::PUBLIC])
+    end
+
+    it 'returns INTERNAL and PUBLIC for internal users' do
+      user = double(:user, admin?: false, external?: false)
+
+      expect(described_class.levels_for_user(user)).
+        to eq([Gitlab::VisibilityLevel::INTERNAL,
+               Gitlab::VisibilityLevel::PUBLIC])
+    end
+
+    it 'returns PUBLIC for external users' do
+      user = double(:user, admin?: false, external?: true)
+
+      expect(described_class.levels_for_user(user)).
+        to eq([Gitlab::VisibilityLevel::PUBLIC])
+    end
+
+    it 'returns PUBLIC when no user is given' do
+      expect(described_class.levels_for_user).
+        to eq([Gitlab::VisibilityLevel::PUBLIC])
+    end
+  end
 end