From 440635015fbea129cbfd7b266589ea2a33dda471 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Fri, 23 Aug 2019 16:38:01 -0700
Subject: 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
---
 changelogs/unreleased/sh-fix-nplusone-issues.yml    | 5 +++++
 doc/api/issues.md                                   | 1 -
 lib/api/issues.rb                                   | 3 ++-
 spec/requests/api/issues/get_project_issues_spec.rb | 8 ++++++++
 4 files changed, 15 insertions(+), 2 deletions(-)
 create mode 100644 changelogs/unreleased/sh-fix-nplusone-issues.yml

diff --git a/changelogs/unreleased/sh-fix-nplusone-issues.yml b/changelogs/unreleased/sh-fix-nplusone-issues.yml
new file mode 100644
index 00000000000..f749b5eeb40
--- /dev/null
+++ b/changelogs/unreleased/sh-fix-nplusone-issues.yml
@@ -0,0 +1,5 @@
+---
+title: Fix N+1 Gitaly calls in /api/v4/projects/:id/issues
+merge_request: 32171
+author:
+type: performance
diff --git a/doc/api/issues.md b/doc/api/issues.md
index 8313dd2c3bd..cadc9291489 100644
--- a/doc/api/issues.md
+++ b/doc/api/issues.md
@@ -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
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index e16eeef202c..215178478d0 100644
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@@ -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
diff --git a/spec/requests/api/issues/get_project_issues_spec.rb b/spec/requests/api/issues/get_project_issues_spec.rb
index 521d6b88734..b7aa3f93451 100644
--- a/spec/requests/api/issues/get_project_issues_spec.rb
+++ b/spec/requests/api/issues/get_project_issues_spec.rb
@@ -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) { {} }
-- 
cgit v1.2.1