Commit 42598786 authored by Yorick Peterse's avatar Yorick Peterse

Fixed ordering in Project.find_with_namespace

This ensures that Project.find_with_namespace returns a row matching
literally as the first value, instead of returning a random value.

The ordering here is _only_ applied to Project.find_with_namespace and
_not_ Project.where_paths_in as currently there's no code that requires
Project.where_paths_in to return rows in a certain order. Since this
method also returns all rows that match there's no real harm in not
setting a specific order either. Another reason is that generating all
the "WHEN" arms for multiple values in Project.where_paths_in becomes
really messy.

On MySQL we have to use the "BINARY" operator to turn a "WHERE" into a
case-sensitive WHERE as otherwise MySQL may still end up returning rows
in an unpredictable order.

Fixes gitlab-org/gitlab-ce#18603
parent 33b68f87
......@@ -262,7 +262,23 @@ class Project < ActiveRecord::Base
#
# Returns a Project, or nil if no project could be found.
def find_with_namespace(path)
where_paths_in([path]).reorder(nil).take
namespace_path, project_path = path.split('/', 2)
return unless namespace_path && project_path
namespace_path = connection.quote(namespace_path)
project_path = connection.quote(project_path)
# On MySQL we want to ensure the ORDER BY uses a case-sensitive match so
# any literal matches come first, for this we have to use "BINARY".
# Without this there's still no guarantee in what order MySQL will return
# rows.
binary = Gitlab::Database.mysql? ? 'BINARY' : ''
order_sql = "(CASE WHEN #{binary} namespaces.path = #{namespace_path} " \
"AND #{binary} projects.path = #{project_path} THEN 0 ELSE 1 END)"
where_paths_in([path]).reorder(order_sql).take
end
# Builds a relation to find multiple projects by their full paths.
......
......@@ -220,7 +220,7 @@ describe Project, models: true do
end
end
describe :find_with_namespace do
describe '.find_with_namespace' do
context 'with namespace' do
before do
@group = create :group, name: 'gitlab'
......@@ -231,6 +231,22 @@ describe Project, models: true do
it { expect(Project.find_with_namespace('GitLab/GitlabHQ')).to eq(@project) }
it { expect(Project.find_with_namespace('gitlab-ci')).to be_nil }
end
context 'when multiple projects using a similar name exist' do
let(:group) { create(:group, name: 'gitlab') }
let!(:project1) do
create(:empty_project, name: 'gitlab1', path: 'gitlab', namespace: group)
end
let!(:project2) do
create(:empty_project, name: 'gitlab2', path: 'GITLAB', namespace: group)
end
it 'returns the row where the path matches literally' do
expect(Project.find_with_namespace('gitlab/GITLAB')).to eq(project2)
end
end
end
describe :to_param do
......
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