diff options
-rw-r--r-- | changelogs/unreleased/ab-45247-project-lookups-validation.yml | 5 | ||||
-rw-r--r-- | lib/api/helpers.rb | 4 | ||||
-rw-r--r-- | spec/lib/api/helpers_spec.rb | 42 |
3 files changed, 49 insertions, 2 deletions
diff --git a/changelogs/unreleased/ab-45247-project-lookups-validation.yml b/changelogs/unreleased/ab-45247-project-lookups-validation.yml new file mode 100644 index 00000000000..cd5ebdebc58 --- /dev/null +++ b/changelogs/unreleased/ab-45247-project-lookups-validation.yml @@ -0,0 +1,5 @@ +--- +title: Validate project path prior to hitting the database. +merge_request: 18322 +author: +type: performance diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 61dab1dd5cb..b8657cd7ee4 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -103,9 +103,9 @@ module API end def find_project(id) - if id =~ /^\d+$/ + if id.is_a?(Integer) || id =~ /^\d+$/ Project.find_by(id: id) - else + elsif id.include?("/") Project.find_by_full_path(id) end end diff --git a/spec/lib/api/helpers_spec.rb b/spec/lib/api/helpers_spec.rb index 3c4deba4712..58a49124ce6 100644 --- a/spec/lib/api/helpers_spec.rb +++ b/spec/lib/api/helpers_spec.rb @@ -3,6 +3,48 @@ require 'spec_helper' describe API::Helpers do subject { Class.new.include(described_class).new } + describe '#find_project' do + let(:project) { create(:project) } + + shared_examples 'project finder' do + context 'when project exists' do + it 'returns requested project' do + expect(subject.find_project(existing_id)).to eq(project) + end + + it 'returns nil' do + expect(subject.find_project(non_existing_id)).to be_nil + end + end + end + + context 'when ID is used as an argument' do + let(:existing_id) { project.id } + let(:non_existing_id) { (Project.maximum(:id) || 0) + 1 } + + it_behaves_like 'project finder' + end + + context 'when PATH is used as an argument' do + let(:existing_id) { project.full_path } + let(:non_existing_id) { 'something/else' } + + it_behaves_like 'project finder' + + context 'with an invalid PATH' do + let(:non_existing_id) { 'undefined' } # path without slash + + it_behaves_like 'project finder' + + it 'does not hit the database' do + expect(Project).not_to receive(:find_by_full_path) + + subject.find_project(non_existing_id) + end + end + end + end + describe '#find_namespace' do let(:namespace) { create(:namespace) } |