Commit 739d6a5a authored by Andreas Brandl's avatar Andreas Brandl

Perform two-step Routable lookup by path

In order to lookup a Project or Namespace by path, we prefer an exact
match (case-sensitive) but in absence of that, we'd also take a
case-insensitive match.

The case-insensitive matching with preference for the exact match is a
bit more involved in SQL as the exact lookup. Yet, the majority of cases
will be an exact match. The thinking here is that we can optimize the
lookup by performing an exact match first and only if there is no
result, we perform the case-insensitive lookup.

Data for GitLab.com:
* We have about 15M records in routes table
* About 2,500 routes exist where there's more than one record
  with the same `lower(path)`

It is possible for a user to craft requests that would always trigger
the 2-step search (e.g. we have a route for `/foo/bar`, the request is
always for `/FOO/bar`). In this case, the change at hand is not
beneficial as it would run an additional query.

However, based on the data, it is highly likely that the vast majority
of requests can be satisfied with an exact match only.

The context for this change is
https://gitlab.com/gitlab-org/gitlab-ce/issues/64590#note_208156463.
parent 770a15b0
...@@ -33,8 +33,15 @@ module Routable ...@@ -33,8 +33,15 @@ module Routable
# #
# Returns a single object, or nil. # Returns a single object, or nil.
def find_by_full_path(path, follow_redirects: false) def find_by_full_path(path, follow_redirects: false)
if Feature.enabled?(:routable_two_step_lookup)
# Case sensitive match first (it's cheaper and the usual case)
# If we didn't have an exact match, we perform a case insensitive search
found = joins(:route).find_by(routes: { path: path }) || where_full_path_in([path]).take
else
order_sql = Arel.sql("(CASE WHEN routes.path = #{connection.quote(path)} THEN 0 ELSE 1 END)") order_sql = Arel.sql("(CASE WHEN routes.path = #{connection.quote(path)} THEN 0 ELSE 1 END)")
found = where_full_path_in([path]).reorder(order_sql).take found = where_full_path_in([path]).reorder(order_sql).take
end
return found if found return found if found
if follow_redirects if follow_redirects
......
...@@ -58,7 +58,7 @@ describe Group, 'Routable' do ...@@ -58,7 +58,7 @@ describe Group, 'Routable' do
end end
end end
describe '.find_by_full_path' do shared_examples_for '.find_by_full_path' do
let!(:nested_group) { create(:group, parent: group) } let!(:nested_group) { create(:group, parent: group) }
context 'without any redirect routes' do context 'without any redirect routes' do
...@@ -110,6 +110,24 @@ describe Group, 'Routable' do ...@@ -110,6 +110,24 @@ describe Group, 'Routable' do
end end
end end
describe '.find_by_full_path' do
context 'with routable_two_step_lookup feature' do
before do
stub_feature_flags(routable_two_step_lookup: true)
end
it_behaves_like '.find_by_full_path'
end
context 'without routable_two_step_lookup feature' do
before do
stub_feature_flags(routable_two_step_lookup: false)
end
it_behaves_like '.find_by_full_path'
end
end
describe '.where_full_path_in' do describe '.where_full_path_in' do
context 'without any paths' do context 'without any paths' do
it 'returns an empty relation' do it 'returns an empty relation' 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