summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/models/concerns/routable.rb15
-rw-r--r--changelogs/unreleased/sh-optimize-duplicate-routable-full-path.yml4
-rw-r--r--spec/models/concerns/routable_spec.rb18
3 files changed, 35 insertions, 2 deletions
diff --git a/app/models/concerns/routable.rb b/app/models/concerns/routable.rb
index aca99feee53..b28e05d0c28 100644
--- a/app/models/concerns/routable.rb
+++ b/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
diff --git a/changelogs/unreleased/sh-optimize-duplicate-routable-full-path.yml b/changelogs/unreleased/sh-optimize-duplicate-routable-full-path.yml
new file mode 100644
index 00000000000..b1ef00f09b2
--- /dev/null
+++ b/changelogs/unreleased/sh-optimize-duplicate-routable-full-path.yml
@@ -0,0 +1,4 @@
+---
+title: Cache Routable#full_path in RequestStore to reduce duplicate route loads
+merge_request:
+author:
diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb
index f191605dbdb..221647d7a48 100644
--- a/spec/models/concerns/routable_spec.rb
+++ b/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