From e6dc5168b86d613e3334fa55618e394308bf55bf Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Tue, 6 Aug 2019 17:27:46 +0100 Subject: 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 | 5 + doc/api/group_labels.md | 9 +- doc/api/labels.md | 9 +- lib/api/entities.rb | 18 ++-- lib/api/group_labels.rb | 2 + lib/api/helpers/label_helpers.rb | 6 +- lib/api/labels.rb | 2 + .../api/schemas/public_api/v4/group_labels.json | 19 ---- .../api/schemas/public_api/v4/labels/label.json | 11 ++ .../public_api/v4/labels/label_with_counts.json | 16 +++ .../public_api/v4/labels/project_label.json | 15 +++ .../v4/labels/project_label_with_counts.json | 9 ++ spec/requests/api/group_labels_spec.rb | 15 ++- spec/requests/api/labels_spec.rb | 111 +++++++++++---------- 14 files changed, 160 insertions(+), 87 deletions(-) create mode 100644 changelogs/unreleased/speed-up-labels-api.yml delete mode 100644 spec/fixtures/api/schemas/public_api/v4/group_labels.json create mode 100644 spec/fixtures/api/schemas/public_api/v4/labels/label.json create mode 100644 spec/fixtures/api/schemas/public_api/v4/labels/label_with_counts.json create mode 100644 spec/fixtures/api/schemas/public_api/v4/labels/project_label.json create mode 100644 spec/fixtures/api/schemas/public_api/v4/labels/project_label_with_counts.json 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: " https://gitlab.example.com/api/v4/groups/5/labels +curl --header "PRIVATE-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: " https://gitlab.example.com/api/v4/projects/1/labels +curl --header "PRIVATE-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 -- cgit v1.2.1