From a724f7e35f9f8ed9692b0f3f4d6c8a62632cdec4 Mon Sep 17 00:00:00 2001 From: Robert Schilling Date: Tue, 13 Feb 2018 20:22:37 +0100 Subject: [PATCH] Refactor commits/refs API to use hash and add pagination headers --- changelogs/unreleased/api-refs-for-commit.yml | 2 +- doc/api/commits.md | 15 ++++---- lib/api/commits.rb | 19 ++++------ lib/api/entities.rb | 8 +---- spec/requests/api/commits_spec.rb | 36 ++++++++++--------- 5 files changed, 38 insertions(+), 42 deletions(-) diff --git a/changelogs/unreleased/api-refs-for-commit.yml b/changelogs/unreleased/api-refs-for-commit.yml index a5ed3992ef3..df8a2b0eccc 100644 --- a/changelogs/unreleased/api-refs-for-commit.yml +++ b/changelogs/unreleased/api-refs-for-commit.yml @@ -1,5 +1,5 @@ --- -title: API: Get references a commit is pushed to +title: 'API: Get references a commit is pushed to' merge_request: 15026 author: Robert Schilling type: added diff --git a/doc/api/commits.md b/doc/api/commits.md index 18d31ffe24d..2c745d00887 100644 --- a/doc/api/commits.md +++ b/doc/api/commits.md @@ -203,6 +203,7 @@ Example response: > [Introduced][ce-15026] in GitLab 10.6 Get all references (from branches or tags) a commit is pushed to. +The pagination parameters `page` and `per_page` can be used to restrict the list of references. ``` GET /projects/:id/repository/commits/:sha/refs @@ -214,20 +215,22 @@ Parameters: | --------- | ---- | -------- | ----------- | | `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | `sha` | string | yes | The commit hash | -| `type` | string | no | The scope of commits. Possible values `branches`, `tags`, `all`. Default is `all`. | +| `type` | string | no | The scope of commits. Possible values `branch`, `tag`, `all`. Default is `all`. | ```bash -curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" --form "type=all" "https://gitlab.example.com/api/v4/projects/5/repository/commits/5937ac0a7beb003549fc5fd26fc247adbce4a52e/refs" +curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/projects/5/repository/commits/5937ac0a7beb003549fc5fd26fc247adbce4a52e/refs?type=all" ``` Example response: ```json [ - {"branch_name": "'test'"}, - {"branch_name": "master"}, - {"tag_name": "v1.1.0"} -] + {"type": "branch", "name": "'test'"}, + {"type": "branch", "name": "add-balsamiq-file"}, + {"type": "branch", "name": "wip"}, + {"type": "tag", "name": "v1.1.0"} + ] + ``` ## Cherry pick a commit diff --git a/lib/api/commits.rb b/lib/api/commits.rb index afaf68114e8..d83c43ee49b 100644 --- a/lib/api/commits.rb +++ b/lib/api/commits.rb @@ -162,24 +162,19 @@ module API end params do requires :sha, type: String, desc: 'A commit sha' - optional :type, type: String, values: %w[branches tags all], default: 'all', desc: 'Scope' + optional :type, type: String, values: %w[branch tag all], default: 'all', desc: 'Scope' + use :pagination end get ':id/repository/commits/:sha/refs', requirements: API::COMMIT_ENDPOINT_REQUIREMENTS do commit = user_project.commit(params[:sha]) not_found!('Commit') unless commit - refs = - case params[:type] - when 'branches' - user_project.repository.branch_names_contains(commit.id).map {|branch_name| [branch_name, true]} - when 'tags' - user_project.repository.tag_names_contains(commit.id).map {|tag_name| [tag_name, false]} - else - refs = user_project.repository.branch_names_contains(commit.id).map {|branch_name| [branch_name, true]} - refs.concat(user_project.repository.tag_names_contains(commit.id).map {|tag_name| [tag_name, false]}) - end + refs = [] + refs.concat(user_project.repository.branch_names_contains(commit.id).map {|name| { type: 'branch', name: name }}) unless params[:type] == 'tag' + refs.concat(user_project.repository.tag_names_contains(commit.id).map {|name| { type: 'tag', name: name }}) unless params[:type] == 'branch' + refs = Kaminari.paginate_array(refs) - present refs, with: Entities::BasicRef + present paginate(refs), with: Entities::BasicRef end desc 'Post comment to commit' do diff --git a/lib/api/entities.rb b/lib/api/entities.rb index c7a817877f0..c8cda85b170 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -277,13 +277,7 @@ module API end class BasicRef < Grape::Entity - expose :branch_name, if: lambda { |ref, options| ref[1] } do |ref, options| - ref[0] - end - - expose :tag_name, if: lambda { |ref, options| !ref[1] } do |ref, options| - ref[0] - end + expose :type, :name end class Branch < Grape::Entity diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index ac25f134697..31959d28fee 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -490,39 +490,43 @@ describe API::Commits do context 'for a valid commit' do it 'returns all refs with no scope' do - get api(route, current_user) + get api(route, current_user), per_page: 100 - branch_refs = project.repository.branch_names_contains(commit_id) - tag_refs = project.repository.tag_names_contains(commit_id) + refs = project.repository.branch_names_contains(commit_id).map {|name| ['branch', name]} + refs.concat(project.repository.tag_names_contains(commit_id).map {|name| ['tag', name]}) - expect(json_response.map { |refs| refs['branch_name'] }.compact).to eq(branch_refs) - expect(json_response.map { |refs| refs['tag_name'] }.compact).to eq(tag_refs) + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.map { |r| [r['type'], r['name']] }.compact).to eq(refs) end it 'returns all refs' do - get api(route, current_user), type: 'all' + get api(route, current_user), type: 'all', per_page: 100 - branch_refs = project.repository.branch_names_contains(commit_id) - tag_refs = project.repository.tag_names_contains(commit_id) + refs = project.repository.branch_names_contains(commit_id).map {|name| ['branch', name]} + refs.concat(project.repository.tag_names_contains(commit_id).map {|name| ['tag', name]}) - expect(json_response.map { |refs| refs['branch_name'] }.compact).to eq(branch_refs) - expect(json_response.map { |refs| refs['tag_name'] }.compact).to eq(tag_refs) + expect(response).to have_gitlab_http_status(200) + expect(json_response.map { |r| [r['type'], r['name']] }.compact).to eq(refs) end it 'returns the branch refs' do - get api(route, current_user), type: 'branches' + get api(route, current_user), type: 'branch', per_page: 100 - branch_refs = project.repository.branch_names_contains(commit_id) + refs = project.repository.branch_names_contains(commit_id).map {|name| ['branch', name]} - expect(json_response.map { |refs| refs['branch_name'] }.compact).to eq(branch_refs) + expect(response).to have_gitlab_http_status(200) + expect(json_response.map { |r| [r['type'], r['name']] }.compact).to eq(refs) end it 'returns the tag refs' do - get api(route, current_user), type: 'tags' + get api(route, current_user), type: 'tag', per_page: 100 - tag_refs = project.repository.tag_names_contains(commit_id) + refs = project.repository.tag_names_contains(commit_id).map {|name| ['tag', name]} - expect(json_response.map { |refs| refs['tag_name'] }.compact).to eq(tag_refs) + expect(response).to have_gitlab_http_status(200) + expect(json_response.map { |r| [r['type'], r['name']] }.compact).to eq(refs) end end end