Commit 202d6d4a authored by Robert Speicher's avatar Robert Speicher

Reduce duplication in GitAccess spec around error messages

- Adds a new `ProjectMovedError` class to encapsulate that error
  condition. Inherits from `NotFoundError` so existing rescues should
  continue to work.
- Separating that condition out of `NotFoundError` allowed us to
  simplify the `raise_not_found` helper and avoid repeating the literal
  string.
- Spec makes use of `ERROR_MESSAGES` hash to avoid repeating literal
  error message strings.
parent 4219002f
...@@ -6,6 +6,7 @@ module Gitlab ...@@ -6,6 +6,7 @@ module Gitlab
include PathLocksHelper include PathLocksHelper
UnauthorizedError = Class.new(StandardError) UnauthorizedError = Class.new(StandardError)
NotFoundError = Class.new(StandardError) NotFoundError = Class.new(StandardError)
ProjectMovedError = Class.new(NotFoundError)
ERROR_MESSAGES = { ERROR_MESSAGES = {
upload: 'You are not allowed to upload code for this project.', upload: 'You are not allowed to upload code for this project.',
...@@ -95,7 +96,8 @@ module Gitlab ...@@ -95,7 +96,8 @@ module Gitlab
end end
def check_project_moved! def check_project_moved!
if redirected_path return unless redirected_path
url = protocol == 'ssh' ? project.ssh_url_to_repo : project.http_url_to_repo url = protocol == 'ssh' ? project.ssh_url_to_repo : project.http_url_to_repo
message = <<-MESSAGE.strip_heredoc message = <<-MESSAGE.strip_heredoc
Project '#{redirected_path}' was moved to '#{project.full_path}'. Project '#{redirected_path}' was moved to '#{project.full_path}'.
...@@ -105,8 +107,7 @@ module Gitlab ...@@ -105,8 +107,7 @@ module Gitlab
git remote set-url origin #{url} git remote set-url origin #{url}
MESSAGE MESSAGE
raise NotFoundError, message raise ProjectMovedError, message
end
end end
def check_command_disabled!(cmd) def check_command_disabled!(cmd)
......
...@@ -70,8 +70,8 @@ describe Gitlab::GitAccess do ...@@ -70,8 +70,8 @@ describe Gitlab::GitAccess do
context 'when the Deploykey does not have access to the project' do context 'when the Deploykey does not have access to the project' do
it 'blocks push and pull with "not found"' do it 'blocks push and pull with "not found"' do
aggregate_failures do aggregate_failures do
expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.') expect { push_access_check }.to raise_not_found
expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.') expect { pull_access_check }.to raise_not_found
end end
end end
end end
...@@ -94,8 +94,8 @@ describe Gitlab::GitAccess do ...@@ -94,8 +94,8 @@ describe Gitlab::GitAccess do
context 'when the User cannot read the project' do context 'when the User cannot read the project' do
it 'blocks push and pull with "not found"' do it 'blocks push and pull with "not found"' do
aggregate_failures do aggregate_failures do
expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.') expect { push_access_check }.to raise_not_found
expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.') expect { pull_access_check }.to raise_not_found
end end
end end
end end
...@@ -111,7 +111,7 @@ describe Gitlab::GitAccess do ...@@ -111,7 +111,7 @@ describe Gitlab::GitAccess do
end end
it 'does not block pushes with "not found"' do it 'does not block pushes with "not found"' do
expect { push_access_check }.to raise_unauthorized('You are not allowed to upload code for this project.') expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:upload])
end end
end end
end end
...@@ -127,17 +127,17 @@ describe Gitlab::GitAccess do ...@@ -127,17 +127,17 @@ describe Gitlab::GitAccess do
end end
it 'does not block pushes with "not found"' do it 'does not block pushes with "not found"' do
expect { push_access_check }.to raise_unauthorized('You are not allowed to upload code for this project.') expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:upload])
end end
end end
context 'when guests cannot read the project' do context 'when guests cannot read the project' do
it 'blocks pulls with "not found"' do it 'blocks pulls with "not found"' do
expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.') expect { pull_access_check }.to raise_not_found
end end
it 'blocks pushes with "not found"' do it 'blocks pushes with "not found"' do
expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.') expect { push_access_check }.to raise_not_found
end end
end end
end end
...@@ -148,8 +148,8 @@ describe Gitlab::GitAccess do ...@@ -148,8 +148,8 @@ describe Gitlab::GitAccess do
it 'blocks push and pull with "not found"' do it 'blocks push and pull with "not found"' do
aggregate_failures do aggregate_failures do
expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.') expect { pull_access_check }.to raise_not_found
expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.') expect { push_access_check }.to raise_not_found
end end
end end
end end
...@@ -174,11 +174,11 @@ describe Gitlab::GitAccess do ...@@ -174,11 +174,11 @@ describe Gitlab::GitAccess do
it 'blocks push and pull access' do it 'blocks push and pull access' do
aggregate_failures do aggregate_failures do
expect { push_access_check }.to raise_not_found(/Project '#{redirected_path}' was moved to '#{project.full_path}'/) expect { push_access_check }.to raise_error(described_class::ProjectMovedError, /Project '#{redirected_path}' was moved to '#{project.full_path}'/)
expect { push_access_check }.to raise_not_found(/git remote set-url origin #{project.ssh_url_to_repo}/) expect { push_access_check }.to raise_error(described_class::ProjectMovedError, /git remote set-url origin #{project.ssh_url_to_repo}/)
expect { pull_access_check }.to raise_not_found(/Project '#{redirected_path}' was moved to '#{project.full_path}'/) expect { pull_access_check }.to raise_error(described_class::ProjectMovedError, /Project '#{redirected_path}' was moved to '#{project.full_path}'/)
expect { pull_access_check }.to raise_not_found(/git remote set-url origin #{project.ssh_url_to_repo}/) expect { pull_access_check }.to raise_error(described_class::ProjectMovedError, /git remote set-url origin #{project.ssh_url_to_repo}/)
end end
end end
...@@ -187,8 +187,8 @@ describe Gitlab::GitAccess do ...@@ -187,8 +187,8 @@ describe Gitlab::GitAccess do
it 'includes the path to the project using HTTP' do it 'includes the path to the project using HTTP' do
aggregate_failures do aggregate_failures do
expect { push_access_check }.to raise_not_found(/git remote set-url origin #{project.http_url_to_repo}/) expect { push_access_check }.to raise_error(described_class::ProjectMovedError, /git remote set-url origin #{project.http_url_to_repo}/)
expect { pull_access_check }.to raise_not_found(/git remote set-url origin #{project.http_url_to_repo}/) expect { pull_access_check }.to raise_error(described_class::ProjectMovedError, /git remote set-url origin #{project.http_url_to_repo}/)
end end
end end
end end
...@@ -243,7 +243,7 @@ describe Gitlab::GitAccess do ...@@ -243,7 +243,7 @@ describe Gitlab::GitAccess do
it 'disallows guests to pull' do it 'disallows guests to pull' do
project.add_guest(user) project.add_guest(user)
expect { pull_access_check }.to raise_unauthorized('You are not allowed to download code from this project.') expect { pull_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:download])
end end
it 'disallows blocked users to pull' do it 'disallows blocked users to pull' do
...@@ -255,7 +255,7 @@ describe Gitlab::GitAccess do ...@@ -255,7 +255,7 @@ describe Gitlab::GitAccess do
describe 'without access to project' do describe 'without access to project' do
context 'pull code' do context 'pull code' do
it { expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.') } it { expect { pull_access_check }.to raise_not_found }
end end
context 'when project is public' do context 'when project is public' do
...@@ -272,7 +272,7 @@ describe Gitlab::GitAccess do ...@@ -272,7 +272,7 @@ describe Gitlab::GitAccess do
it 'does not give access to download code' do it 'does not give access to download code' do
public_project.project_feature.update_attribute(:repository_access_level, ProjectFeature::DISABLED) public_project.project_feature.update_attribute(:repository_access_level, ProjectFeature::DISABLED)
expect { pull_access_check }.to raise_unauthorized('You are not allowed to download code from this project.') expect { pull_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:download])
end end
end end
end end
...@@ -301,13 +301,13 @@ describe Gitlab::GitAccess do ...@@ -301,13 +301,13 @@ describe Gitlab::GitAccess do
context 'from internal project' do context 'from internal project' do
let(:project) { create(:project, :internal, :repository) } let(:project) { create(:project, :internal, :repository) }
it { expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.') } it { expect { pull_access_check }.to raise_not_found }
end end
context 'from private project' do context 'from private project' do
let(:project) { create(:project, :private, :repository) } let(:project) { create(:project, :private, :repository) }
it { expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.') } it { expect { pull_access_check }.to raise_not_found }
end end
end end
end end
...@@ -360,7 +360,7 @@ describe Gitlab::GitAccess do ...@@ -360,7 +360,7 @@ describe Gitlab::GitAccess do
context 'when is not member of the project' do context 'when is not member of the project' do
context 'pull code' do context 'pull code' do
it { expect { pull_access_check }.to raise_unauthorized('You are not allowed to download code from this project.') } it { expect { pull_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:download]) }
end end
end end
end end
...@@ -885,26 +885,26 @@ describe Gitlab::GitAccess do ...@@ -885,26 +885,26 @@ describe Gitlab::GitAccess do
project.team << [user, :reporter] project.team << [user, :reporter]
end end
it { expect { push_access_check }.to raise_unauthorized('You are not allowed to upload code for this project.') } it { expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:upload]) }
end end
context 'when unauthorized' do context 'when unauthorized' do
context 'to public project' do context 'to public project' do
let(:project) { create(:project, :public, :repository) } let(:project) { create(:project, :public, :repository) }
it { expect { push_access_check }.to raise_unauthorized('You are not allowed to upload code for this project.') } it { expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:upload]) }
end end
context 'to internal project' do context 'to internal project' do
let(:project) { create(:project, :internal, :repository) } let(:project) { create(:project, :internal, :repository) }
it { expect { push_access_check }.to raise_unauthorized('You are not allowed to upload code for this project.') } it { expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:upload]) }
end end
context 'to private project' do context 'to private project' do
let(:project) { create(:project, :private, :repository) } let(:project) { create(:project, :private, :repository) }
it { expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.') } it { expect { push_access_check }.to raise_not_found }
end end
end end
end end
...@@ -938,19 +938,19 @@ describe Gitlab::GitAccess do ...@@ -938,19 +938,19 @@ describe Gitlab::GitAccess do
context 'to public project' do context 'to public project' do
let(:project) { create(:project, :public, :repository) } let(:project) { create(:project, :public, :repository) }
it { expect { push_access_check }.to raise_unauthorized('This deploy key does not have write access to this project.') } it { expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:deploy_key_upload]) }
end end
context 'to internal project' do context 'to internal project' do
let(:project) { create(:project, :internal, :repository) } let(:project) { create(:project, :internal, :repository) }
it { expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.') } it { expect { push_access_check }.to raise_not_found }
end end
context 'to private project' do context 'to private project' do
let(:project) { create(:project, :private, :repository) } let(:project) { create(:project, :private, :repository) }
it { expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.') } it { expect { push_access_check }.to raise_not_found }
end end
end end
end end
...@@ -963,26 +963,26 @@ describe Gitlab::GitAccess do ...@@ -963,26 +963,26 @@ describe Gitlab::GitAccess do
key.projects << project key.projects << project
end end
it { expect { push_access_check }.to raise_unauthorized('This deploy key does not have write access to this project.') } it { expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:deploy_key_upload]) }
end end
context 'when unauthorized' do context 'when unauthorized' do
context 'to public project' do context 'to public project' do
let(:project) { create(:project, :public, :repository) } let(:project) { create(:project, :public, :repository) }
it { expect { push_access_check }.to raise_unauthorized('This deploy key does not have write access to this project.') } it { expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:deploy_key_upload]) }
end end
context 'to internal project' do context 'to internal project' do
let(:project) { create(:project, :internal, :repository) } let(:project) { create(:project, :internal, :repository) }
it { expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.') } it { expect { push_access_check }.to raise_not_found }
end end
context 'to private project' do context 'to private project' do
let(:project) { create(:project, :private, :repository) } let(:project) { create(:project, :private, :repository) }
it { expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.') } it { expect { push_access_check }.to raise_not_found }
end end
end end
end end
...@@ -994,8 +994,9 @@ describe Gitlab::GitAccess do ...@@ -994,8 +994,9 @@ describe Gitlab::GitAccess do
raise_error(Gitlab::GitAccess::UnauthorizedError, message) raise_error(Gitlab::GitAccess::UnauthorizedError, message)
end end
def raise_not_found(message) def raise_not_found
raise_error(Gitlab::GitAccess::NotFoundError, message) raise_error(Gitlab::GitAccess::NotFoundError,
Gitlab::GitAccess::ERROR_MESSAGES[:project_not_found])
end end
def build_authentication_abilities def build_authentication_abilities
......
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