summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSean McGivern <sean@gitlab.com>2019-08-06 17:27:46 +0100
committerSean McGivern <sean@gitlab.com>2019-08-08 17:10:41 +0100
commite6dc5168b86d613e3334fa55618e394308bf55bf (patch)
tree6ad67848dfbe81949474e20f5224edf055c273cb
parent26087322713e2949f2bf207798512374757a484c (diff)
downloadgitlab-ce-speed-up-labels-api.tar.gz
Remove label issue and MR counts from default API responsesspeed-up-labels-api
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.
-rw-r--r--changelogs/unreleased/speed-up-labels-api.yml5
-rw-r--r--doc/api/group_labels.md9
-rw-r--r--doc/api/labels.md9
-rw-r--r--lib/api/entities.rb18
-rw-r--r--lib/api/group_labels.rb2
-rw-r--r--lib/api/helpers/label_helpers.rb6
-rw-r--r--lib/api/labels.rb2
-rw-r--r--spec/fixtures/api/schemas/public_api/v4/group_labels.json19
-rw-r--r--spec/fixtures/api/schemas/public_api/v4/labels/label.json11
-rw-r--r--spec/fixtures/api/schemas/public_api/v4/labels/label_with_counts.json16
-rw-r--r--spec/fixtures/api/schemas/public_api/v4/labels/project_label.json15
-rw-r--r--spec/fixtures/api/schemas/public_api/v4/labels/project_label_with_counts.json9
-rw-r--r--spec/requests/api/group_labels_spec.rb15
-rw-r--r--spec/requests/api/labels_spec.rb111
14 files changed, 160 insertions, 87 deletions
diff --git a/changelogs/unreleased/speed-up-labels-api.yml b/changelogs/unreleased/speed-up-labels-api.yml
new file mode 100644
index 00000000000..d5ac4313414
--- /dev/null
+++ b/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
diff --git a/doc/api/group_labels.md b/doc/api/group_labels.md
index 3d4b099b49e..e2ba0dea642 100644
--- a/doc/api/group_labels.md
+++ b/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:
diff --git a/doc/api/labels.md b/doc/api/labels.md
index 9d10d383bf9..5db0edcf14d 100644
--- a/doc/api/labels.md
+++ b/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:
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 2f5ce3d4003..70201502b57 100644
--- a/lib/api/entities.rb
+++ b/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|
diff --git a/lib/api/group_labels.rb b/lib/api/group_labels.rb
index 0dbc5f45a68..79a44941c81 100644
--- a/lib/api/group_labels.rb
+++ b/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
diff --git a/lib/api/helpers/label_helpers.rb b/lib/api/helpers/label_helpers.rb
index c11e7d614ab..896b0aba52b 100644
--- a/lib/api/helpers/label_helpers.rb
+++ b/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)
diff --git a/lib/api/labels.rb b/lib/api/labels.rb
index d729d3ee625..c183198d3c6 100644
--- a/lib/api/labels.rb
+++ b/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
diff --git a/spec/fixtures/api/schemas/public_api/v4/group_labels.json b/spec/fixtures/api/schemas/public_api/v4/group_labels.json
deleted file mode 100644
index fbde45f2904..00000000000
--- a/spec/fixtures/api/schemas/public_api/v4/group_labels.json
+++ /dev/null
@@ -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
- }
-}
diff --git a/spec/fixtures/api/schemas/public_api/v4/labels/label.json b/spec/fixtures/api/schemas/public_api/v4/labels/label.json
new file mode 100644
index 00000000000..a116b33572d
--- /dev/null
+++ b/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" }
+ }
+}
diff --git a/spec/fixtures/api/schemas/public_api/v4/labels/label_with_counts.json b/spec/fixtures/api/schemas/public_api/v4/labels/label_with_counts.json
new file mode 100644
index 00000000000..2331932e07d
--- /dev/null
+++ b/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" }
+ }
+ }
+ ]
+ }
+}
diff --git a/spec/fixtures/api/schemas/public_api/v4/labels/project_label.json b/spec/fixtures/api/schemas/public_api/v4/labels/project_label.json
new file mode 100644
index 00000000000..a9a065ee71f
--- /dev/null
+++ b/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" }
+ }
+ }
+ ]
+ }
+}
diff --git a/spec/fixtures/api/schemas/public_api/v4/labels/project_label_with_counts.json b/spec/fixtures/api/schemas/public_api/v4/labels/project_label_with_counts.json
new file mode 100644
index 00000000000..87b90b2b3b5
--- /dev/null
+++ b/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" }
+ ]
+ }
+}
diff --git a/spec/requests/api/group_labels_spec.rb b/spec/requests/api/group_labels_spec.rb
index fcea57d9df7..0be4e2e9121 100644
--- a/spec/requests/api/group_labels_spec.rb
+++ b/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
diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb
index 518181e4d93..ad0974f55a3 100644
--- a/spec/requests/api/labels_spec.rb
+++ b/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