Commit 26ec874f authored by Jacopo's avatar Jacopo

Get Project Branch API shows an helpful error message on invalid refname

In API v4 when requesting a branch with an invalid refname shows an
helpful error message: 'The branch refname is invalid'.
parent 9ac5338b
---
title: Get Project Branch API shows an helpful error message on invalid refname
merge_request: 14884
author: Jacopo Beschi @jacopo-beschi
type: added
...@@ -8,6 +8,16 @@ module API ...@@ -8,6 +8,16 @@ module API
before { authorize! :download_code, user_project } before { authorize! :download_code, user_project }
helpers do
def find_branch!(branch_name)
begin
user_project.repository.find_branch(branch_name) || not_found!('Branch')
rescue Gitlab::Git::CommandError
render_api_error!('The branch refname is invalid', 400)
end
end
end
params do params do
requires :id, type: String, desc: 'The ID of a project' requires :id, type: String, desc: 'The ID of a project'
end end
...@@ -38,8 +48,7 @@ module API ...@@ -38,8 +48,7 @@ module API
user_project.repository.branch_exists?(params[:branch]) ? status(204) : status(404) user_project.repository.branch_exists?(params[:branch]) ? status(204) : status(404)
end end
get do get do
branch = user_project.repository.find_branch(params[:branch]) branch = find_branch!(params[:branch])
not_found!('Branch') unless branch
present branch, with: Entities::Branch, project: user_project present branch, with: Entities::Branch, project: user_project
end end
...@@ -60,8 +69,7 @@ module API ...@@ -60,8 +69,7 @@ module API
put ':id/repository/branches/:branch/protect', requirements: BRANCH_ENDPOINT_REQUIREMENTS do put ':id/repository/branches/:branch/protect', requirements: BRANCH_ENDPOINT_REQUIREMENTS do
authorize_admin_project authorize_admin_project
branch = user_project.repository.find_branch(params[:branch]) branch = find_branch!(params[:branch])
not_found!('Branch') unless branch
protected_branch = user_project.protected_branches.find_by(name: branch.name) protected_branch = user_project.protected_branches.find_by(name: branch.name)
...@@ -96,8 +104,7 @@ module API ...@@ -96,8 +104,7 @@ module API
put ':id/repository/branches/:branch/unprotect', requirements: BRANCH_ENDPOINT_REQUIREMENTS do put ':id/repository/branches/:branch/unprotect', requirements: BRANCH_ENDPOINT_REQUIREMENTS do
authorize_admin_project authorize_admin_project
branch = user_project.repository.find_branch(params[:branch]) branch = find_branch!(params[:branch])
not_found!("Branch") unless branch
protected_branch = user_project.protected_branches.find_by(name: branch.name) protected_branch = user_project.protected_branches.find_by(name: branch.name)
protected_branch&.destroy protected_branch&.destroy
...@@ -133,8 +140,7 @@ module API ...@@ -133,8 +140,7 @@ module API
delete ':id/repository/branches/:branch', requirements: BRANCH_ENDPOINT_REQUIREMENTS do delete ':id/repository/branches/:branch', requirements: BRANCH_ENDPOINT_REQUIREMENTS do
authorize_push_project authorize_push_project
branch = user_project.repository.find_branch(params[:branch]) branch = find_branch!(params[:branch])
not_found!('Branch') unless branch
commit = user_project.repository.commit(branch.dereferenced_target) commit = user_project.repository.commit(branch.dereferenced_target)
......
...@@ -110,6 +110,15 @@ describe API::Branches do ...@@ -110,6 +110,15 @@ describe API::Branches do
end end
end end
context 'when the branch refname is invalid' do
let(:branch_name) { 'branch*' }
let(:message) { 'The branch refname is invalid' }
it_behaves_like '400 response' do
let(:request) { get api(route, current_user) }
end
end
context 'when repository is disabled' do context 'when repository is disabled' do
include_context 'disabled repository' include_context 'disabled repository'
...@@ -234,6 +243,15 @@ describe API::Branches do ...@@ -234,6 +243,15 @@ describe API::Branches do
end end
end end
context 'when the branch refname is invalid' do
let(:branch_name) { 'branch*' }
let(:message) { 'The branch refname is invalid' }
it_behaves_like '400 response' do
let(:request) { put api(route, current_user) }
end
end
context 'when repository is disabled' do context 'when repository is disabled' do
include_context 'disabled repository' include_context 'disabled repository'
...@@ -359,6 +377,15 @@ describe API::Branches do ...@@ -359,6 +377,15 @@ describe API::Branches do
end end
end end
context 'when the branch refname is invalid' do
let(:branch_name) { 'branch*' }
let(:message) { 'The branch refname is invalid' }
it_behaves_like '400 response' do
let(:request) { put api(route, current_user) }
end
end
context 'when repository is disabled' do context 'when repository is disabled' do
include_context 'disabled repository' include_context 'disabled repository'
...@@ -520,6 +547,15 @@ describe API::Branches do ...@@ -520,6 +547,15 @@ describe API::Branches do
expect(response).to have_gitlab_http_status(404) expect(response).to have_gitlab_http_status(404)
end end
context 'when the branch refname is invalid' do
let(:branch_name) { 'branch*' }
let(:message) { 'The branch refname is invalid' }
it_behaves_like '400 response' do
let(:request) { delete api("/projects/#{project.id}/repository/branches/#{branch_name}", user) }
end
end
it_behaves_like '412 response' do it_behaves_like '412 response' do
let(:request) { api("/projects/#{project.id}/repository/branches/#{branch_name}", user) } let(:request) { api("/projects/#{project.id}/repository/branches/#{branch_name}", user) }
end end
......
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
# Requires an API request: # Requires an API request:
# let(:request) { get api("/projects/#{project.id}/repository/branches", user) } # let(:request) { get api("/projects/#{project.id}/repository/branches", user) }
shared_examples_for '400 response' do shared_examples_for '400 response' do
let(:message) { nil }
before do before do
# Fires the request # Fires the request
request request
...@@ -10,6 +12,10 @@ shared_examples_for '400 response' do ...@@ -10,6 +12,10 @@ shared_examples_for '400 response' do
it 'returns 400' do it 'returns 400' do
expect(response).to have_gitlab_http_status(400) expect(response).to have_gitlab_http_status(400)
if message.present?
expect(json_response['message']).to eq(message)
end
end end
end end
...@@ -26,6 +32,7 @@ end ...@@ -26,6 +32,7 @@ end
shared_examples_for '404 response' do shared_examples_for '404 response' do
let(:message) { nil } let(:message) { nil }
before do before do
# Fires the request # Fires the request
request request
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment