Refactor GroupProjectsFinder#init_collection

This optimises how GroupProjectsFinder builds it collection, producing
simpler and faster queries in the process. It also cleans up the code a
bit to make it easier to understand.

3 files changed, 57 insertions, 38 deletions

diff --git a/app/finders/events_finder.rb b/app/finders/events_finder.rb
index b0450ddc1fd..29beb6cb224 100644
--- a/app/finders/events_finder.rb
+++ b/app/finders/events_finder.rb
@@ -33,7 +33,8 @@ class EventsFinder
   private
 
   def by_current_user_access(events)
-    events.merge(ProjectsFinder.new(current_user: current_user).execute).references(:project)
+    events.merge(ProjectsFinder.new(current_user: current_user).execute).
+      joins(:project)
   end
 
   def by_action(events)
diff --git a/app/finders/group_projects_finder.rb b/app/finders/group_projects_finder.rb
index f043c38c6f9..f2d3b90b8e2 100644
--- a/app/finders/group_projects_finder.rb
+++ b/app/finders/group_projects_finder.rb
@@ -29,35 +29,69 @@ class GroupProjectsFinder < ProjectsFinder
   private
 
   def init_collection
-    only_owned  = options.fetch(:only_owned, false)
-    only_shared = options.fetch(:only_shared, false)
+    projects = if current_user
+                 collection_with_user
+               else
+                 collection_without_user
+               end
 
-    projects = []
+    union(projects)
+  end
 
-    if current_user
-      if group.users.include?(current_user)
-        projects << group.projects unless only_shared
-        projects << group.shared_projects unless only_owned
+  def collection_with_user
+    if group.users.include?(current_user)
+      if only_shared?
+        [shared_projects]
+      elsif only_owned?
+        [owned_projects]
       else
-        unless only_shared
-          projects << group.projects.visible_to_user(current_user)
-          projects << group.projects.public_to_user(current_user)
-        end
-
-        unless only_owned
-          projects << group.shared_projects.visible_to_user(current_user)
-          projects << group.shared_projects.public_to_user(current_user)
-        end
+        [shared_projects, owned_projects]
       end
     else
-      projects << group.projects.public_only unless only_shared
-      projects << group.shared_projects.public_only unless only_owned
+      if only_shared?
+        [shared_projects.public_or_visible_to_user(current_user)]
+      elsif only_owned?
+        [owned_projects.public_or_visible_to_user(current_user)]
+      else
+        [
+          owned_projects.public_or_visible_to_user(current_user),
+          shared_projects.public_or_visible_to_user(current_user)
+        ]
+      end
     end
+  end
 
-    projects
+  def collection_without_user
+    if only_shared?
+      [shared_projects.public_only]
+    elsif only_owned?
+      [owned_projects.public_only]
+    else
+      [shared_projects.public_only, owned_projects.public_only]
+    end
   end
 
   def union(items)
-    find_union(items, Project)
+    if items.one?
+      items.first
+    else
+      find_union(items, Project)
+    end
+  end
+
+  def only_owned?
+    options.fetch(:only_owned, false)
+  end
+
+  def only_shared?
+    options.fetch(:only_shared, false)
+  end
+
+  def owned_projects
+    group.projects
+  end
+
+  def shared_projects
+    group.shared_projects
   end
 end
diff --git a/app/finders/projects_finder.rb b/app/finders/projects_finder.rb
index 72e9c7a1cd7..8bfbe37c543 100644
--- a/app/finders/projects_finder.rb
+++ b/app/finders/projects_finder.rb
@@ -58,27 +58,11 @@ class ProjectsFinder < UnionFinder
       if private_only?
         current_user.authorized_projects
       else
-        collection_with_user_and_public_projects
+        Project.public_or_visible_to_user(current_user)
       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?