Refactor commits/refs API to use hash and add pagination headers

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