Commit 71568a69 authored by Thong Kuah's avatar Thong Kuah

Merge branch 'fix-n-plus-one-in-jira-github-api' into 'master'

Fix N+1 queries in Jira Development Panel API endpoint

See merge request gitlab-org/gitlab!18329
parents 170e1e54 6625a6d3
---
title: Fix N+1 queries in Jira Development Panel API endpoint
merge_request: 18329
author:
type: performance
...@@ -9,7 +9,9 @@ module API ...@@ -9,7 +9,9 @@ module API
class Repository < Grape::Entity class Repository < Grape::Entity
expose :id expose :id
expose :owner do |project, options| expose :owner do |project, options|
{ login: project.root_namespace.path } root_namespace = options[:root_namespace] || project.root_namespace
{ login: root_namespace.path }
end end
expose :name do |project, options| expose :name do |project, options|
::Gitlab::Jira::Dvcs.encode_project_name(project) ::Gitlab::Jira::Dvcs.encode_project_name(project)
......
...@@ -54,7 +54,7 @@ module API ...@@ -54,7 +54,7 @@ module API
project = find_project!( project = find_project!(
::Gitlab::Jira::Dvcs.restore_full_path(params.slice(:namespace, :project).symbolize_keys) ::Gitlab::Jira::Dvcs.restore_full_path(params.slice(:namespace, :project).symbolize_keys)
) )
not_found! unless licensed_project?(project) not_found! unless licensed?(project)
project project
end end
...@@ -62,7 +62,7 @@ module API ...@@ -62,7 +62,7 @@ module API
def find_merge_requests def find_merge_requests
merge_requests = authorized_merge_requests.reorder(updated_at: :desc).preload(:target_project) merge_requests = authorized_merge_requests.reorder(updated_at: :desc).preload(:target_project)
merge_requests = paginate(merge_requests) merge_requests = paginate(merge_requests)
merge_requests.select { |mr| licensed_project?(mr.target_project) } merge_requests.select { |mr| licensed?(mr.target_project) }
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
...@@ -87,8 +87,8 @@ module API ...@@ -87,8 +87,8 @@ module API
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
def licensed_project?(project) def licensed?(routable)
project.feature_available?(JIRA_DEV_PANEL_FEATURE) routable.feature_available?(JIRA_DEV_PANEL_FEATURE)
end end
end end
...@@ -111,15 +111,16 @@ module API ...@@ -111,15 +111,16 @@ module API
get ':namespace/repos' do get ':namespace/repos' do
namespace = Namespace.find_by_full_path(params[:namespace]) namespace = Namespace.find_by_full_path(params[:namespace])
not_found!('Namespace') unless namespace not_found!('Namespace') unless namespace && licensed?(namespace)
projects = Project.public_or_visible_to_user(current_user) projects = Project.public_or_visible_to_user(current_user)
.in_namespace(namespace.self_and_descendants) .in_namespace(namespace.self_and_descendants)
.to_a .eager_load_namespace_and_owner
projects.select! { |project| licensed_project?(project) } .with_route
projects = ::Kaminari.paginate_array(projects)
present paginate(projects), with: ::API::Github::Entities::Repository present paginate(projects),
with: ::API::Github::Entities::Repository,
root_namespace: namespace.root_ancestor
end end
get ':username' do get ':username' do
......
...@@ -323,6 +323,38 @@ describe API::V3::Github do ...@@ -323,6 +323,38 @@ describe API::V3::Github do
it 'returns an array of projects belonging to group with github format' do it 'returns an array of projects belonging to group with github format' do
expect_project_under_namespace([parent_group_project, child_group_project], group.parent, user) expect_project_under_namespace([parent_group_project, child_group_project], group.parent, user)
end end
context 'when namespace license checks are enabled' do
before do
enable_namespace_license_check!
end
context 'when the root group does not have the correct license' do
it 'returns not found' do
jira_get v3_api("/users/#{group.parent.path}/repos", user)
expect(response).to have_gitlab_http_status(404)
end
end
context 'when the root group has the correct license' do
before do
create(:gitlab_subscription, :gold, namespace: group.parent)
end
it 'avoids N+1 queries' do
jira_get v3_api("/users/#{group.parent.path}/repos", user)
control = ActiveRecord::QueryRecorder.new { jira_get v3_api("/users/#{group.parent.path}/repos", user) }
new_group = create(:group, parent: group.parent)
create(:project, :repository, group: new_group, creator: user)
expect { jira_get v3_api("/users/#{group.parent.path}/repos", user) }.not_to exceed_query_limit(control)
expect(response).to have_gitlab_http_status(200)
end
end
end
end end
context 'user namespace' do context 'user namespace' do
......
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