Merge branch 'ab-routable-two-step-search' into 'master'

Perform two-step Routable lookup by path

See merge request gitlab-org/gitlab-ce!32272

2 files changed, 41 insertions, 3 deletions

diff --git a/app/models/concerns/routable.rb b/app/models/concerns/routable.rb
index 116e8967651..3a486632800 100644
--- a/app/models/concerns/routable.rb
+++ b/app/models/concerns/routable.rb
@@ -33,8 +33,17 @@ module Routable
     #
     # Returns a single object, or nil.
     def find_by_full_path(path, follow_redirects: false)
-      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
+      increment_counter(:routable_find_by_full_path, 'Number of calls to Routable.find_by_full_path')
+
+      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)")
+        found = where_full_path_in([path]).reorder(order_sql).take
+      end
+
       return found if found
 
       if follow_redirects
@@ -52,12 +61,23 @@ module Routable
     def where_full_path_in(paths)
       return none if paths.empty?
 
+      increment_counter(:routable_where_full_path_in, 'Number of calls to Routable.where_full_path_in')
+
       wheres = paths.map do |path|
         "(LOWER(routes.path) = LOWER(#{connection.quote(path)}))"
       end
 
       joins(:route).where(wheres.join(' OR '))
     end
+
+    # Temporary instrumentation of method calls
+    def increment_counter(counter, description)
+      @counters[counter] ||= Gitlab::Metrics.counter(counter, description)
+
+      @counters[counter].increment
+    rescue
+      # ignore the error
+    end
   end
 
   def full_name
diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb
index 31163a5bb5c..cff86afe768 100644
--- a/spec/models/concerns/routable_spec.rb
+++ b/spec/models/concerns/routable_spec.rb
@@ -58,7 +58,7 @@ describe Group, 'Routable' do
     end
   end
 
-  describe '.find_by_full_path' do
+  shared_examples_for '.find_by_full_path' do
     let!(:nested_group) { create(:group, parent: group) }
 
     context 'without any redirect routes' do
@@ -110,6 +110,24 @@ describe Group, 'Routable' do
     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
     context 'without any paths' do
       it 'returns an empty relation' do