Merge branch 'sh-optimize-projects-api' into 'master'

Optimize /api/v4/projects endpoint for visibility level

See merge request gitlab-org/gitlab-ce!26481

4 files changed, 89 insertions, 10 deletions

diff --git a/app/finders/projects_finder.rb b/app/finders/projects_finder.rb
index 93d3c991846..0319e95d439 100644
--- a/app/finders/projects_finder.rb
+++ b/app/finders/projects_finder.rb
@@ -81,7 +81,7 @@ class ProjectsFinder < UnionFinder
       if private_only?
         current_user.authorized_projects
       else
-        Project.public_or_visible_to_user(current_user)
+        Project.public_or_visible_to_user(current_user, params[:visibility_level])
       end
     end
   end
diff --git a/app/models/project.rb b/app/models/project.rb
index 97a17d120c6..06010409574 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -459,14 +459,41 @@ 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
-      where('EXISTS (?) OR projects.visibility_level IN (?)',
-            user.authorizations_for_projects,
-            Gitlab::VisibilityLevel.levels_for_user(user))
-    else
-      public_to_user
-    end
+  #
+  # requested_visiblity_levels: Normally all projects that are visible
+  # to the user (e.g. internal and public) are queried, but this
+  # parameter allows the caller to narrow the search space to optimize
+  # database queries. For instance, a caller may only want to see
+  # internal projects. Instead of querying for internal and public
+  # projects and throwing away public projects, this parameter allows
+  # the query to be targeted for only internal projects.
+  def self.public_or_visible_to_user(user = nil, requested_visibility_levels = [])
+    return public_to_user unless user
+
+    visible_levels = Gitlab::VisibilityLevel.levels_for_user(user)
+    include_private = true
+    requested_visibility_levels = Array(requested_visibility_levels)
+
+    if requested_visibility_levels.present?
+      visible_levels &= requested_visibility_levels
+      include_private = requested_visibility_levels.include?(Gitlab::VisibilityLevel::PRIVATE)
+    end
+
+    public_or_internal_rel =
+      if visible_levels.present?
+        where('projects.visibility_level IN (?)', visible_levels)
+      else
+        Project.none
+      end
+
+    private_rel =
+      if include_private
+        where('EXISTS (?)', user.authorizations_for_projects)
+      else
+        Project.none
+      end
+
+    public_or_internal_rel.or(private_rel)
   end
 
   # project features may be "disabled", "internal", "enabled" or "public". If "internal",
diff --git a/changelogs/unreleased/sh-optimize-projects-api.yml b/changelogs/unreleased/sh-optimize-projects-api.yml
new file mode 100644
index 00000000000..2f2459be77f
--- /dev/null
+++ b/changelogs/unreleased/sh-optimize-projects-api.yml
@@ -0,0 +1,5 @@
+---
+title: Optimize /api/v4/projects endpoint for visibility level
+merge_request: 26481
+author:
+type: performance
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index fc9d9a28b9a..90dcf861849 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -2710,7 +2710,7 @@ describe Project do
   end
 
   describe '#any_lfs_file_locks?', :request_store do
-    set(:project) { create(:project) }
+    let!(:project) { create(:project) }
 
     it 'returns false when there are no LFS file locks' do
       expect(project.any_lfs_file_locks?).to be_falsey
@@ -3148,6 +3148,53 @@ describe Project do
         expect(projects).to eq([public_project])
       end
     end
+
+    context 'with requested visibility levels' do
+      set(:internal_project) { create(:project, :internal, :repository) }
+      set(:private_project_2) { create(:project, :private) }
+
+      context 'with admin user' do
+        set(:admin) { create(:admin) }
+
+        it 'returns all projects' do
+          projects = described_class.all.public_or_visible_to_user(admin, [])
+
+          expect(projects).to match_array([public_project, private_project, private_project_2, internal_project])
+        end
+
+        it 'returns all public and private projects' do
+          projects = described_class.all.public_or_visible_to_user(admin, [Gitlab::VisibilityLevel::PUBLIC, Gitlab::VisibilityLevel::PRIVATE])
+
+          expect(projects).to match_array([public_project, private_project, private_project_2])
+        end
+
+        it 'returns all private projects' do
+          projects = described_class.all.public_or_visible_to_user(admin, [Gitlab::VisibilityLevel::PRIVATE])
+
+          expect(projects).to match_array([private_project, private_project_2])
+        end
+      end
+
+      context 'with regular user' do
+        it 'returns authorized projects' do
+          projects = described_class.all.public_or_visible_to_user(user, [])
+
+          expect(projects).to match_array([public_project, private_project, internal_project])
+        end
+
+        it "returns user's public and private projects" do
+          projects = described_class.all.public_or_visible_to_user(user, [Gitlab::VisibilityLevel::PUBLIC, Gitlab::VisibilityLevel::PRIVATE])
+
+          expect(projects).to match_array([public_project, private_project])
+        end
+
+        it 'returns one private project' do
+          projects = described_class.all.public_or_visible_to_user(user, [Gitlab::VisibilityLevel::PRIVATE])
+
+          expect(projects).to eq([private_project])
+        end
+      end
+    end
   end
 
   describe '.with_feature_available_for_user' do