diff options
author | Sean McGivern <sean@gitlab.com> | 2019-08-06 17:27:46 +0100 |
---|---|---|
committer | Sean McGivern <sean@gitlab.com> | 2019-08-08 17:10:41 +0100 |
commit | e6dc5168b86d613e3334fa55618e394308bf55bf (patch) | |
tree | 6ad67848dfbe81949474e20f5224edf055c273cb | |
parent | 26087322713e2949f2bf207798512374757a484c (diff) | |
download | gitlab-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.yml | 5 | ||||
-rw-r--r-- | doc/api/group_labels.md | 9 | ||||
-rw-r--r-- | doc/api/labels.md | 9 | ||||
-rw-r--r-- | lib/api/entities.rb | 18 | ||||
-rw-r--r-- | lib/api/group_labels.rb | 2 | ||||
-rw-r--r-- | lib/api/helpers/label_helpers.rb | 6 | ||||
-rw-r--r-- | lib/api/labels.rb | 2 | ||||
-rw-r--r-- | spec/fixtures/api/schemas/public_api/v4/group_labels.json | 19 | ||||
-rw-r--r-- | spec/fixtures/api/schemas/public_api/v4/labels/label.json | 11 | ||||
-rw-r--r-- | spec/fixtures/api/schemas/public_api/v4/labels/label_with_counts.json | 16 | ||||
-rw-r--r-- | spec/fixtures/api/schemas/public_api/v4/labels/project_label.json | 15 | ||||
-rw-r--r-- | spec/fixtures/api/schemas/public_api/v4/labels/project_label_with_counts.json | 9 | ||||
-rw-r--r-- | spec/requests/api/group_labels_spec.rb | 15 | ||||
-rw-r--r-- | spec/requests/api/labels_spec.rb | 111 |
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 |