Merge branch 'speed-up-labels-api' into 'master'

Remove label issue and MR counts from default API responses

See merge request gitlab-org/gitlab-ce!31543

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

+---
+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

@@ -1090,16 +1090,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

-{
-  "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

+{
+  "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

+{
+  "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

+{
+  "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

+{
+  "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
-    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 )
+    let(:group) { create(:group) }
+    let!(:group_label) { create(:group_label, title: 'feature', group: group) }
 
-      expected_keys = %w(
-        id name color text_color description
-        open_issues_count closed_issues_count open_merge_requests_count
-        subscribed priority is_project_label
-      )
+    before do
+      project.update!(group: group)
+    end
 
+    it 'returns all available labels to the project' do
       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 }
-
-      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
-
-      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(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
+    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
+
+      it 'includes counts in the response' do
+        get api("/projects/#{project.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 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])
+
+        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