Merge branch 'sh-optimize-duplicate-routable-full-path' into 'master'

app/models/concerns/routable.rb

@@ -163,7 +163,20 @@ module Routable
     end
   end
 
+  # Every time `project.namespace.becomes(Namespace)` is called for polymorphic_path,
+  # a new instance is instantiated, and we end up duplicating the same query to retrieve
+  # the route. Caching this per request ensures that even if we have multiple instances,
+  # we will not have to duplicate work, avoiding N+1 queries in some cases.
   def full_path
+    return uncached_full_path unless RequestStore.active?
+
+    key = "routable/full_path/#{self.class.name}/#{self.id}"
+    RequestStore[key] ||= uncached_full_path
+  end
+
+  private
+
+  def uncached_full_path
     if route && route.path.present?
       @full_path ||= route.path
     else
@@ -173,8 +186,6 @@ module Routable
     end
   end
 
-  private
-
   def full_name_changed?
     name_changed? || parent_changed?
   end

changelogs/unreleased/sh-optimize-duplicate-routable-full-path.yml

+---
+title: Cache Routable#full_path in RequestStore to reduce duplicate route loads
+merge_request:
+author:

spec/models/concerns/routable_spec.rb

@@ -194,6 +194,24 @@ describe Group, 'Routable' do
 
     it { expect(group.full_path).to eq(group.path) }
     it { expect(nested_group.full_path).to eq("#{group.full_path}/#{nested_group.path}") }
+
+    context 'with RequestStore active' do
+      before do
+        RequestStore.begin!
+      end
+
+      after do
+        RequestStore.end!
+        RequestStore.clear!
+      end
+
+      it 'does not load the route table more than once' do
+        expect(group).to receive(:uncached_full_path).once.and_call_original
+
+        3.times { group.full_path }
+        expect(group.full_path).to eq(group.path)
+      end
+    end
   end
 
   describe '#full_name' do