Commit 44063501 authored by Stan Hu's avatar Stan Hu

Fix N+1 Gitaly calls in /api/v4/projects/:id/issues

This is a follow-up from
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/31938.

In GitLab 9.0,
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9661 removed the
`subscribed` flag from the API when the user requested a list of issues
or merge requests since calculating this value triggers extensive
Markdown processing.

In GitLab 12.0 via a4fbf39e, we accidentally reintroduced this
performance regression by changing `IssueBasic` to `Issue` in
`entities.rb`. This showed up as a Gitaly N+1 issue since the Markdown
processing would attempt to extract a commit if it detected a regex that
matched a commit.

We restore the prior behavior by once again removing the `subscribed`
flag for the bulk list of issues and merge requests and add a test to
ensure they aren't reintroduced.

Relates to https://gitlab.com/gitlab-org/gitlab-ce/issues/66202
parent 20d38fed
---
title: Fix N+1 Gitaly calls in /api/v4/projects/:id/issues
merge_request: 32171
author:
type: performance
......@@ -284,7 +284,6 @@ Example response:
"award_emoji":"http://example.com/api/v4/projects/4/issues/41/award_emoji",
"project":"http://example.com/api/v4/projects/4"
},
"subscribed": false,
"task_completion_status":{
"count":0,
"completed_count":0
......
......@@ -163,7 +163,8 @@ module API
with_labels_details: declared_params[:with_labels_details],
current_user: current_user,
project: user_project,
issuable_metadata: issuable_meta_data(issues, 'Issue', current_user)
issuable_metadata: issuable_meta_data(issues, 'Issue', current_user),
include_subscribed: false
}
present issues, options
......
......@@ -446,6 +446,14 @@ describe API::Issues do
expect_paginated_array_response([closed_issue.id, confidential_issue.id, issue.id])
end
it 'exposes known attributes' do
get api("#{base_url}/issues", user)
expect(response).to have_gitlab_http_status(200)
expect(json_response.last.keys).to include(*%w(id iid project_id title description))
expect(json_response.last).not_to have_key('subscribed')
end
context 'issues_statistics' do
context 'no state is treated as all state' do
let(:params) { {} }
......
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