Commit 4ad85b22 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'tc-improve-project-api-perf' into 'master'

Improve /project API performance

Closes #31855 and #31937

See merge request !11666
parents 2c288ee1 1e5506d0
......@@ -24,7 +24,7 @@ def activity
def load_events
projects =
if params[:filter] == "starred"
current_user.viewable_starred_projects
ProjectsFinder.new(current_user: current_user, params: { starred: true }).execute
else
current_user.authorized_projects
end
......
......@@ -7,6 +7,7 @@
# project_ids_relation: int[] - project ids to use
# params:
# trending: boolean
# owned: boolean
# non_public: boolean
# starred: boolean
# sort: string
......@@ -28,13 +29,17 @@ def initialize(params: {}, current_user: nil, project_ids_relation: nil)
def execute
items = init_collection
items = by_ids(items)
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)
items = by_personal(items)
items = by_visibilty_level(items)
items = by_tags(items)
items = by_search(items)
items = by_archived(items)
sort(items)
end
......@@ -43,10 +48,8 @@ def execute
def init_collection
projects = []
if params[:trending].present?
projects << Project.trending
elsif params[:starred].present? && current_user
projects << current_user.viewable_starred_projects
if params[:owned].present?
projects << current_user.owned_projects if current_user
else
projects << current_user.authorized_projects if current_user
projects << Project.unscoped.public_to_user(current_user) unless params[:non_public].present?
......@@ -56,7 +59,7 @@ def init_collection
end
def by_ids(items)
project_ids_relation ? items.map { |item| item.where(id: project_ids_relation) } : items
project_ids_relation ? items.where(id: project_ids_relation) : items
end
def union(items)
......@@ -67,6 +70,14 @@ def by_personal(items)
(params[:personal].present? && current_user) ? items.personal(current_user) : items
end
def by_starred(items)
(params[:starred].present? && current_user) ? items.starred_by(current_user) : items
end
def by_trending(items)
params[:trending].present? ? items.trending : items
end
def by_visibilty_level(items)
params[:visibility_level].present? ? items.where(visibility_level: params[:visibility_level]) : items
end
......
......@@ -242,6 +242,7 @@ def update_forks_visibility_level
scope :in_namespace, ->(namespace_ids) { where(namespace_id: namespace_ids) }
scope :personal, ->(user) { where(namespace_id: user.namespace_id) }
scope :joined, ->(user) { where('namespace_id != ?', user.namespace_id) }
scope :starred_by, ->(user) { joins(:users_star_projects).where('users_star_projects.user_id': user.id) }
scope :visible_to_user, ->(user) { where(id: user.authorized_projects.select(:id).reorder(nil)) }
scope :non_archived, -> { where(archived: false) }
scope :for_milestones, ->(ids) { joins(:milestones).where('milestones.id' => ids).distinct }
......@@ -350,10 +351,6 @@ def search(query)
where("projects.id IN (#{union.to_sql})")
end
def search_by_visibility(level)
where(visibility_level: Gitlab::VisibilityLevel.string_options[level])
end
def search_by_title(query)
pattern = "%#{query}%"
table = Project.arel_table
......
......@@ -557,12 +557,6 @@ def projects_with_reporter_access_limited_to(projects)
authorized_projects(Gitlab::Access::REPORTER).where(id: projects)
end
def viewable_starred_projects
starred_projects.where("projects.visibility_level IN (?) OR projects.id IN (?)",
[Project::PUBLIC, Project::INTERNAL],
authorized_projects.select(:project_id))
end
def owned_projects
@owned_projects ||=
Project.where('namespace_id IN (?) OR namespace_id = ?',
......
---
title: Improve performance of ProjectFinder used in /projects API endpoint
merge_request: 11666
author:
......@@ -151,8 +151,8 @@ def present_groups(groups, options = {})
end
get ":id/projects" do
group = find_group!(params[:id])
projects = GroupProjectsFinder.new(group: group, current_user: current_user).execute
projects = filter_projects(projects)
projects = GroupProjectsFinder.new(group: group, current_user: current_user, params: project_finder_params).execute
projects = reorder_projects(projects)
entity = params[:simple] ? Entities::BasicProjectDetails : Entities::Project
present paginate(projects), with: entity, current_user: current_user
end
......
......@@ -256,31 +256,21 @@ def handle_api_exception(exception)
# project helpers
def filter_projects(projects)
if params[:membership]
projects = projects.merge(current_user.authorized_projects)
end
if params[:owned]
projects = projects.merge(current_user.owned_projects)
end
if params[:starred]
projects = projects.merge(current_user.starred_projects)
end
if params[:search].present?
projects = projects.search(params[:search])
end
if params[:visibility].present?
projects = projects.search_by_visibility(params[:visibility])
end
projects = projects.where(archived: params[:archived])
def reorder_projects(projects)
projects.reorder(params[:order_by] => params[:sort])
end
def project_finder_params
finder_params = {}
finder_params[:owned] = true if params[:owned].present?
finder_params[:non_public] = true if params[:membership].present?
finder_params[:starred] = true if params[:starred].present?
finder_params[:visibility_level] = Gitlab::VisibilityLevel.level_value(params[:visibility]) if params[:visibility]
finder_params[:archived] = params[:archived]
finder_params[:search] = params[:search] if params[:search]
finder_params
end
# file helpers
def uploaded_file(field, uploads_path)
......
......@@ -68,20 +68,19 @@ class Projects < Grape::API
optional :import_url, type: String, desc: 'URL from which the project is imported'
end
def present_projects(projects, options = {})
def present_projects(options = {})
projects = ProjectsFinder.new(current_user: current_user, params: project_finder_params).execute
projects = reorder_projects(projects)
projects = projects.with_statistics if params[:statistics]
projects = projects.with_issues_enabled if params[:with_issues_enabled]
projects = projects.with_merge_requests_enabled if params[:with_merge_requests_enabled]
options = options.reverse_merge(
with: Entities::Project,
current_user: current_user,
simple: params[:simple],
with_issues_enabled: params[:with_issues_enabled],
with_merge_requests_enabled: params[:with_merge_requests_enabled]
with: current_user ? Entities::ProjectWithAccess : Entities::BasicProjectDetails,
statistics: params[:statistics],
current_user: current_user
)
projects = filter_projects(projects)
projects = projects.with_statistics if options[:statistics]
projects = projects.with_issues_enabled if options[:with_issues_enabled]
projects = projects.with_merge_requests_enabled if options[:with_merge_requests_enabled]
options[:with] = Entities::BasicProjectDetails if options[:simple]
options[:with] = Entities::BasicProjectDetails if params[:simple]
present paginate(projects), options
end
......@@ -95,8 +94,7 @@ def present_projects(projects, options = {})
use :statistics_params
end
get do
entity = current_user ? Entities::ProjectWithAccess : Entities::BasicProjectDetails
present_projects ProjectsFinder.new(current_user: current_user).execute, with: entity, statistics: params[:statistics]
present_projects
end
desc 'Create new project' do
......
......@@ -14,6 +14,33 @@ def find_merge_request_with_access(id, access_level = :read_merge_request)
authorize! access_level, merge_request
merge_request
end
# project helpers
def filter_projects(projects)
if params[:membership]
projects = projects.merge(current_user.authorized_projects)
end
if params[:owned]
projects = projects.merge(current_user.owned_projects)
end
if params[:starred]
projects = projects.merge(current_user.starred_projects)
end
if params[:search].present?
projects = projects.search(params[:search])
end
if params[:visibility].present?
projects = projects.where(visibility_level: Gitlab::VisibilityLevel.level_value(params[:visibility]))
end
projects = projects.where(archived: params[:archived])
projects.reorder(params[:order_by] => params[:sort])
end
end
end
end
......@@ -147,7 +147,7 @@ def present_projects(projects, options = {})
get '/starred' do
authenticate!
present_projects current_user.viewable_starred_projects
present_projects ProjectsFinder.new(current_user: current_user, params: { starred: true }).execute
end
desc 'Get all projects for admin user' do
......
......@@ -137,6 +137,13 @@
it { is_expected.to eq([public_project]) }
end
describe 'filter by owned' do
let(:params) { { owned: true } }
let!(:owned_project) { create(:empty_project, :private, namespace: current_user.namespace) }
it { is_expected.to eq([owned_project]) }
end
describe 'filter by non_public' do
let(:params) { { non_public: true } }
before do
......@@ -146,13 +153,19 @@
it { is_expected.to eq([private_project]) }
end
describe 'filter by viewable_starred_projects' do
describe 'filter by starred' do
let(:params) { { starred: true } }
before do
current_user.toggle_star(public_project)
end
it { is_expected.to eq([public_project]) }
it 'returns only projects the user has access to' do
current_user.toggle_star(private_project)
is_expected.to eq([public_project])
end
end
describe 'sorting' do
......
......@@ -948,6 +948,20 @@ def create_pipeline
end
end
describe '.starred_by' do
it 'returns only projects starred by the given user' do
user1 = create(:user)
user2 = create(:user)
project1 = create(:empty_project)
project2 = create(:empty_project)
create(:empty_project)
user1.toggle_star(project1)
user2.toggle_star(project2)
expect(Project.starred_by(user1)).to contain_exactly(project1)
end
end
describe '.visible_to_user' do
let!(:project) { create(:empty_project, :private) }
let!(:user) { create(:user) }
......
......@@ -1496,25 +1496,6 @@ def add_user(access)
end
end
describe '#viewable_starred_projects' do
let(:user) { create(:user) }
let(:public_project) { create(:empty_project, :public) }
let(:private_project) { create(:empty_project, :private) }
let(:private_viewable_project) { create(:empty_project, :private) }
before do
private_viewable_project.team << [user, Gitlab::Access::MASTER]
[public_project, private_project, private_viewable_project].each do |project|
user.toggle_star(project)
end
end
it 'returns only starred projects the user can view' do
expect(user.viewable_starred_projects).not_to include(private_project)
end
end
describe '#projects_with_reporter_access_limited_to' do
let(:project1) { create(:empty_project) }
let(:project2) { create(:empty_project) }
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment