Commit 1b33c7dd authored by Yorick Peterse's avatar Yorick Peterse
Browse files

Merge branch 'ab-45247-project-lookups-validation' into 'master'

Validate project path prior to hitting the database.

Closes #45247

See merge request gitlab-org/gitlab-ce!18322
---
title: Validate project path prior to hitting the database.
merge_request: 18322
author:
type: performance
......@@ -103,9 +103,9 @@ def find_user(id)
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
......
......@@ -3,6 +3,48 @@
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) }
......
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