Optimize /api/v4/projects endpoint for visibility level
Previously when a user requested a list of projects, `Project#public_or_visible_to_user` would search all authorized projects and public/internal projects as well. However, when a user requests a specific `visibility_level` (e.g. private), that should reduce the search space, and we shouldn't need to load public/internal projects. Improves https://gitlab.com/gitlab-org/gitlab-ce/issues/59329
This commit is contained in:
parent
0610bb0917
commit
c408be48ca
4 changed files with 71 additions and 7 deletions
|
@ -81,7 +81,7 @@ class ProjectsFinder < UnionFinder
|
||||||
if private_only?
|
if private_only?
|
||||||
current_user.authorized_projects
|
current_user.authorized_projects
|
||||||
else
|
else
|
||||||
Project.public_or_visible_to_user(current_user)
|
Project.public_or_visible_to_user(current_user, params[:visibility_level])
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -459,13 +459,25 @@ class Project < ActiveRecord::Base
|
||||||
|
|
||||||
# Returns a collection of projects that is either public or visible to the
|
# Returns a collection of projects that is either public or visible to the
|
||||||
# logged in user.
|
# logged in user.
|
||||||
def self.public_or_visible_to_user(user = nil)
|
#
|
||||||
if user
|
# 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)
|
||||||
|
visible_levels &= Array(requested_visibility_levels) if requested_visibility_levels.present?
|
||||||
|
|
||||||
|
if visible_levels.present?
|
||||||
where('EXISTS (?) OR projects.visibility_level IN (?)',
|
where('EXISTS (?) OR projects.visibility_level IN (?)',
|
||||||
user.authorizations_for_projects,
|
user.authorizations_for_projects, visible_levels)
|
||||||
Gitlab::VisibilityLevel.levels_for_user(user))
|
|
||||||
else
|
else
|
||||||
public_to_user
|
where('EXISTS (?)', user.authorizations_for_projects)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
5
changelogs/unreleased/sh-optimize-projects-api.yml
Normal file
5
changelogs/unreleased/sh-optimize-projects-api.yml
Normal file
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
title: Optimize /api/v4/projects endpoint for visibility level
|
||||||
|
merge_request: 26481
|
||||||
|
author:
|
||||||
|
type: performance
|
|
@ -2710,7 +2710,7 @@ describe Project do
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#any_lfs_file_locks?', :request_store do
|
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
|
it 'returns false when there are no LFS file locks' do
|
||||||
expect(project.any_lfs_file_locks?).to be_falsey
|
expect(project.any_lfs_file_locks?).to be_falsey
|
||||||
|
@ -3148,6 +3148,53 @@ describe Project do
|
||||||
expect(projects).to eq([public_project])
|
expect(projects).to eq([public_project])
|
||||||
end
|
end
|
||||||
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
|
end
|
||||||
|
|
||||||
describe '.with_feature_available_for_user' do
|
describe '.with_feature_available_for_user' do
|
||||||
|
|
Loading…
Reference in a new issue