Skip to content

Commit 3b45166

Browse files
maxkadelclaude
andcommitted
Add failing test for Site.instance memoization
Site.instance (upstream Hyku) uses first_or_create with no memoization, so every call is a fresh DB round-trip. Profiling a single /catalog page load found it called ~650-680 times via the `delegate :account, ..., to: :instance` line alone (see pals-stress-tests repo for the full profiling writeup). This test asserts the intended fix's behavior -- currently fails against unmemoized Site.instance. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 0d1c3a4 commit 3b45166

1 file changed

Lines changed: 50 additions & 0 deletions

File tree

spec/models/site_decorator_spec.rb

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
# frozen_string_literal: true
2+
3+
# Site.instance (upstream Hyku, hyrax-webapp/app/models/site.rb) is defined
4+
# as `first_or_create`, with no memoization -- every call is a fresh
5+
# database round-trip. Profiling a single /catalog page load found
6+
# Site.instance called ~650-680 times, almost entirely through the
7+
# `delegate :account, ..., to: :instance` line, i.e. via the ubiquitous
8+
# `Site.account` convenience accessor -- not through any one obvious N+1
9+
# loop. This decorator memoizes it per-request via RequestStore (already a
10+
# transitive dependency here), the same way `current_account` already is
11+
# in ApplicationController/HykuHelper.
12+
RSpec.describe Site, type: :model do
13+
before { RequestStore.clear! }
14+
after { RequestStore.clear! }
15+
16+
describe '.instance' do
17+
context 'on a specific tenant' do
18+
it 'only queries the database once across multiple calls in the same request' do
19+
expect(Site).to receive(:first_or_create).once.and_call_original
20+
21+
3.times { Site.instance }
22+
end
23+
24+
it 'returns the same object across multiple calls' do
25+
first = Site.instance
26+
second = Site.instance
27+
28+
expect(first).to equal(second)
29+
end
30+
31+
it 'queries again after RequestStore is cleared (simulating a new request)' do
32+
Site.instance
33+
RequestStore.clear!
34+
35+
expect(Site).to receive(:first_or_create).once.and_call_original
36+
Site.instance
37+
end
38+
end
39+
40+
context 'on global tenant' do
41+
before do
42+
allow(Account).to receive(:global_tenant?).and_return true
43+
end
44+
45+
it 'is still a NilSite (memoization does not change the existing global-tenant branch)' do
46+
expect(Site.instance).to eq(NilSite.instance)
47+
end
48+
end
49+
end
50+
end

0 commit comments

Comments
 (0)