Remove label issue and MR counts from default API responses

These counts significantly increase the load time for these
requests. Users can now opt in to receiving the counts by setting
`with_counts=true` in requests. This is a breaking change, but hopefully
a fairly minor one.

changelogs/unreleased/speed-up-labels-api.yml

@@ -0,0 +1,5 @@
+---
+title: Remove counts from default labels API responses
+merge_request: 31543
+author:
+type: changed

doc/api/group_labels.md

@@ -12,12 +12,13 @@ Get all labels for a given group.
 GET /groups/:id/labels
 ```
 
-| Attribute | Type | Required | Description |
-| --------- | ---- | -------- | ----------- |
-| `id` | integer/string | yes | The ID or [URL-encoded path of the group](README.md#namespaced-path-encoding) owned by the authenticated user. |
+| Attribute     | Type           | Required | Description                                                                                                                                                                  |
+| ---------     | ----           | -------- | -----------                                                                                                                                                                  |
+| `id`          | integer/string | yes      | The ID or [URL-encoded path of the group](README.md#namespaced-path-encoding) owned by the authenticated user.                                                               |
+| `with_counts` | boolean        | no       | Whether or not to include issue and merge request counts. Defaults to `false`. _([Introduced in GitLab 12.2](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/31543))_ |
 
 ```bash
-curl --header "PRIVATE-TOKEN: <your_access_token>" https://gitlab.example.com/api/v4/groups/5/labels
+curl --header "PRIVATE-TOKEN: <your_access_token>" https://gitlab.example.com/api/v4/groups/5/labels?with_counts=true
 ```
 
 Example response:

doc/api/labels.md

@@ -8,12 +8,13 @@ Get all labels for a given project.
 GET /projects/:id/labels
 ```
 
-| Attribute | Type    | Required | Description           |
-| --------- | ------- | -------- | --------------------- |
-| `id`      | integer/string    | yes      | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user |
+| Attribute     | Type           | Required | Description                                                                                                                                                                  |
+| ---------     | -------        | -------- | ---------------------                                                                                                                                                        |
+| `id`          | integer/string | yes      | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user                                                              |
+| `with_counts` | boolean        | no       | Whether or not to include issue and merge request counts. Defaults to `false`. _([Introduced in GitLab 12.2](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/31543))_ |
 
 ```bash
-curl --header "PRIVATE-TOKEN: <your_access_token>" https://gitlab.example.com/api/v4/projects/1/labels
+curl --header "PRIVATE-TOKEN: <your_access_token>" https://gitlab.example.com/api/v4/projects/1/labels?with_counts=true
 ```
 
 Example response:

lib/api/entities.rb

@@ -1085,16 +1085,18 @@ module API
     end
 
     class Label < LabelBasic
-      expose :open_issues_count do |label, options|
-        label.open_issues_count(options[:current_user])
-      end
+      with_options if: lambda { |_, options| options[:with_counts] } do
+        expose :open_issues_count do |label, options|
+          label.open_issues_count(options[:current_user])
+        end
 
-      expose :closed_issues_count do |label, options|
-        label.closed_issues_count(options[:current_user])
-      end
+        expose :closed_issues_count do |label, options|
+          label.closed_issues_count(options[:current_user])
+        end
 
-      expose :open_merge_requests_count do |label, options|
-        label.open_merge_requests_count(options[:current_user])
+        expose :open_merge_requests_count do |label, options|
+          label.open_merge_requests_count(options[:current_user])
+        end
       end
 
       expose :subscribed do |label, options|

lib/api/group_labels.rb

@@ -16,6 +16,8 @@ module API
         success Entities::GroupLabel
       end
       params do
+        optional :with_counts, type: Boolean, default: false,
+                 desc: 'Include issue and merge request counts'
         use :pagination
       end
       get ':id/labels' do

lib/api/helpers/label_helpers.rb

@@ -19,7 +19,11 @@ module API
       end
 
       def get_labels(parent, entity)
-        present paginate(available_labels_for(parent)), with: entity, current_user: current_user, parent: parent
+        present paginate(available_labels_for(parent)),
+                with: entity,
+                current_user: current_user,
+                parent: parent,
+                with_counts: params[:with_counts]
       end
 
       def create_label(parent, entity)

lib/api/labels.rb

@@ -15,6 +15,8 @@ module API
         success Entities::ProjectLabel
       end
       params do
+        optional :with_counts, type: Boolean, default: false,
+                 desc: 'Include issue and merge request counts'
         use :pagination
       end
       get ':id/labels' do

spec/fixtures/api/schemas/public_api/v4/group_labels.json

@@ -1,19 +0,0 @@
-{
-  "type": "array",
-  "items": {
-    "type": "object",
-    "properties" : {
-      "id" : { "type": "integer" },
-      "name" : { "type": "string "},
-      "color" : { "type": "string "},
-      "text_color" : { "type": "string "},
-      "description" : { "type": "string "},
-      "open_issues_count" : { "type": "integer "},
-      "closed_issues_count" : { "type": "integer "},
-      "open_merge_requests_count" : { "type": "integer "},
-      "subscribed" : { "type": "boolean" },
-      "priority" : { "type": "null" }
-    },
-    "additionalProperties": false
-  }
-}

spec/fixtures/api/schemas/public_api/v4/labels/label.json

@@ -0,0 +1,11 @@
+{
+  "type": "object",
+  "properties": {
+    "id": { "type": "integer" },
+    "name": { "type": "string" },
+    "color": { "type": "string" },
+    "text_color": { "type": "string" },
+    "description": { "type": ["string", "null"] },
+    "subscribed": { "type": "boolean" }
+  }
+}

spec/fixtures/api/schemas/public_api/v4/labels/label_with_counts.json

@@ -0,0 +1,16 @@
+{
+  "type": "object",
+  "properties": {
+    "allOf": [
+      { "$ref": "label.json" },
+      {
+        "type": "object",
+        "properties": {
+          "open_issues_count": { "type": "integer" },
+          "closed_issues_count": { "type": "integer" },
+          "open_merge_requests_count": { "type": "integer" }
+        }
+      }
+    ]
+  }
+}

spec/fixtures/api/schemas/public_api/v4/labels/project_label.json

@@ -0,0 +1,15 @@
+{
+  "type": "object",
+  "properties": {
+    "allOf": [
+      { "$ref": "label.json" },
+      {
+        "type": "object",
+        "properties": {
+          "priority": { "type": ["integer", "null"] },
+          "is_project_label": { "type": "boolean" }
+        }
+      }
+    ]
+  }
+}

spec/fixtures/api/schemas/public_api/v4/labels/project_label_with_counts.json

@@ -0,0 +1,9 @@
+{
+  "type": "object",
+  "properties": {
+    "allOf": [
+      { "$ref": "project_label.json" },
+      { "$ref": "label_with_counts.json" }
+    ]
+  }
+}

spec/requests/api/group_labels_spec.rb

@@ -14,12 +14,25 @@ describe API::GroupLabels do
       get api("/groups/#{group.id}/labels", user)
 
       expect(response).to have_gitlab_http_status(200)
-      expect(response).to match_response_schema('public_api/v4/group_labels')
       expect(response).to include_pagination_headers
       expect(json_response).to be_an Array
+      expect(json_response).to all(match_schema('public_api/v4/labels/label'))
       expect(json_response.size).to eq(2)
       expect(json_response.map {|r| r['name'] }).to contain_exactly('feature', 'bug')
     end
+
+    context 'when the with_counts parameter is set' do
+      it 'includes counts in the response' do
+        get api("/groups/#{group.id}/labels", user), params: { with_counts: true }
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(response).to include_pagination_headers
+        expect(json_response).to be_an Array
+        expect(json_response).to all(match_schema('public_api/v4/labels/label_with_counts'))
+        expect(json_response.size).to eq(2)
+        expect(json_response.map { |r| r['open_issues_count'] }).to contain_exactly(0, 0)
+      end
+    end
   end
 
   describe 'POST /groups/:id/labels' do

spec/requests/api/labels_spec.rb

@@ -11,65 +11,76 @@ describe API::Labels do
   end
 
   describe 'GET /projects/:id/labels' do
+    let(:group) { create(:group) }
+    let!(:group_label) { create(:group_label, title: 'feature', group: group) }
+
+    before do
+      project.update!(group: group)
+    end
+
     it 'returns all available labels to the project' do
-      group = create(:group)
-      group_label = create(:group_label, title: 'feature', group: group)
-      project.update(group: group)
-      create(:labeled_issue, project: project, labels: [group_label], author: user)
-      create(:labeled_issue, project: project, labels: [label1], author: user, state: :closed)
-      create(:labeled_merge_request, labels: [priority_label], author: user, source_project: project )
-
-      expected_keys = %w(
-        id name color text_color description
-        open_issues_count closed_issues_count open_merge_requests_count
-        subscribed priority is_project_label
-      )
-
       get api("/projects/#{project.id}/labels", user)
 
       expect(response).to have_gitlab_http_status(200)
       expect(response).to include_pagination_headers
-      expect(json_response).to be_an Array
+      expect(json_response).to all(match_schema('public_api/v4/labels/project_label'))
       expect(json_response.size).to eq(3)
-      expect(json_response.first.keys).to match_array expected_keys
       expect(json_response.map { |l| l['name'] }).to match_array([group_label.name, priority_label.name, label1.name])
+    end
 
-      label1_response = json_response.find { |l| l['name'] == label1.title }
-      group_label_response = json_response.find { |l| l['name'] == group_label.title }
-      priority_label_response = json_response.find { |l| l['name'] == priority_label.title }
+    context 'when the with_counts parameter is set' do
+      before do
+        create(:labeled_issue, project: project, labels: [group_label], author: user)
+        create(:labeled_issue, project: project, labels: [label1], author: user, state: :closed)
+        create(:labeled_merge_request, labels: [priority_label], author: user, source_project: project )
+      end
 
-      expect(label1_response['open_issues_count']).to eq(0)
-      expect(label1_response['closed_issues_count']).to eq(1)
-      expect(label1_response['open_merge_requests_count']).to eq(0)
-      expect(label1_response['name']).to eq(label1.name)
-      expect(label1_response['color']).to be_present
-      expect(label1_response['text_color']).to be_present
-      expect(label1_response['description']).to be_nil
-      expect(label1_response['priority']).to be_nil
-      expect(label1_response['subscribed']).to be_falsey
-      expect(label1_response['is_project_label']).to be_truthy
+      it 'includes counts in the response' do
+        get api("/projects/#{project.id}/labels", user), params: { with_counts: true }
 
-      expect(group_label_response['open_issues_count']).to eq(1)
-      expect(group_label_response['closed_issues_count']).to eq(0)
-      expect(group_label_response['open_merge_requests_count']).to eq(0)
-      expect(group_label_response['name']).to eq(group_label.name)
-      expect(group_label_response['color']).to be_present
-      expect(group_label_response['text_color']).to be_present
-      expect(group_label_response['description']).to be_nil
-      expect(group_label_response['priority']).to be_nil
-      expect(group_label_response['subscribed']).to be_falsey
-      expect(group_label_response['is_project_label']).to be_falsey
+        expect(response).to have_gitlab_http_status(200)
+        expect(response).to include_pagination_headers
+        expect(json_response).to all(match_schema('public_api/v4/labels/project_label_with_counts'))
+        expect(json_response.size).to eq(3)
+        expect(json_response.map { |l| l['name'] }).to match_array([group_label.name, priority_label.name, label1.name])
 
-      expect(priority_label_response['open_issues_count']).to eq(0)
-      expect(priority_label_response['closed_issues_count']).to eq(0)
-      expect(priority_label_response['open_merge_requests_count']).to eq(1)
-      expect(priority_label_response['name']).to eq(priority_label.name)
-      expect(priority_label_response['color']).to be_present
-      expect(priority_label_response['text_color']).to be_present
-      expect(priority_label_response['description']).to be_nil
-      expect(priority_label_response['priority']).to eq(3)
-      expect(priority_label_response['subscribed']).to be_falsey
-      expect(priority_label_response['is_project_label']).to be_truthy
+        label1_response = json_response.find { |l| l['name'] == label1.title }
+        group_label_response = json_response.find { |l| l['name'] == group_label.title }
+        priority_label_response = json_response.find { |l| l['name'] == priority_label.title }
+
+        expect(label1_response).to include('open_issues_count' => 0,
+                                           'closed_issues_count' => 1,
+                                           'open_merge_requests_count' => 0,
+                                           'name' => label1.name,
+                                           'description' => nil,
+                                           'color' => a_string_matching(/^#\h{6}$/),
+                                           'text_color' => a_string_matching(/^#\h{6}$/),
+                                           'priority' => nil,
+                                           'subscribed' => false,
+                                           'is_project_label' => true)
+
+        expect(group_label_response).to include('open_issues_count' => 1,
+                                                'closed_issues_count' => 0,
+                                                'open_merge_requests_count' => 0,
+                                                'name' => group_label.name,
+                                                'description' => nil,
+                                                'color' => a_string_matching(/^#\h{6}$/),
+                                                'text_color' => a_string_matching(/^#\h{6}$/),
+                                                'priority' => nil,
+                                                'subscribed' => false,
+                                                'is_project_label' => false)
+
+        expect(priority_label_response).to include('open_issues_count' => 0,
+                                                   'closed_issues_count' => 0,
+                                                   'open_merge_requests_count' => 1,
+                                                   'name' => priority_label.name,
+                                                   'description' => nil,
+                                                   'color' => a_string_matching(/^#\h{6}$/),
+                                                   'text_color' => a_string_matching(/^#\h{6}$/),
+                                                   'priority' => 3,
+                                                   'subscribed' => false,
+                                                   'is_project_label' => true)
+      end
     end
   end