From cc18f7bbe4e3c78381146ac155b0d75dc3430bef Mon Sep 17 00:00:00 2001 From: Sampo Kuokkanen Date: Wed, 9 Sep 2026 21:53:20 +0900 Subject: [PATCH 1/4] Pin json below 3 for the sunspot_rails suite ActiveSupport passes quirks_mode to JSON.generate and JSON.parse. json 3 made unknown options an ArgumentError, so Sunspot.remove_all! in spec_helper blows up and every sunspot_rails spec fails with it. Rails 8 dropped quirks_mode, but the Appraisals file stops at Rails 7.0, so nothing in the matrix can run on json 3. Pinning it as a development dependency, next to the psych pin that is already there for the same kind of reason. --- sunspot_rails/sunspot_rails.gemspec | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sunspot_rails/sunspot_rails.gemspec b/sunspot_rails/sunspot_rails.gemspec index 42ecec647..677ac8813 100644 --- a/sunspot_rails/sunspot_rails.gemspec +++ b/sunspot_rails/sunspot_rails.gemspec @@ -39,6 +39,9 @@ Gem::Specification.new do |s| s.add_development_dependency 'bundler', '>= 1.3.0', '< 2.0' if RUBY_VERSION <= '2.0.0' s.add_development_dependency 'nokogiri', '< 1.7' if RUBY_VERSION <= '2.0.0' s.add_development_dependency 'rake', '~> 13.2' + # ActiveSupport passes quirks_mode to JSON.generate/parse and json 3 rejects it. + # Rails 8 dropped the option; nothing in the Rails matrix is that new. + s.add_development_dependency 'json', '< 3' # Ruby 2.5 compatibility if RUBY_VERSION < '2.6' From 282d758510fe2567f089b878ca2d06e4c83f63cc Mon Sep 17 00:00:00 2001 From: Sampo Kuokkanen Date: Wed, 21 Oct 2026 09:44:48 +0900 Subject: [PATCH 2/4] Move the json pin into the per-Rails appraisals The quirks_mode incompatibility depends on the Rails version, so pin json next to the other version-specific gems instead of in the gemspec, and correct the comment: 8.0.x still passes quirks_mode; only 7.2.4+ and 8.1+ don't. --- sunspot_rails/Appraisals | 4 ++++ sunspot_rails/sunspot_rails.gemspec | 3 --- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/sunspot_rails/Appraisals b/sunspot_rails/Appraisals index ca3d08520..66f1f291a 100644 --- a/sunspot_rails/Appraisals +++ b/sunspot_rails/Appraisals @@ -11,6 +11,8 @@ if ruby_version < Gem::Version.new('3.0.0') gem 'sqlite3', '~> 1.3.0' gem 'rspec', '~> 3.7' gem 'rspec-rails', '~> 3.7' + # ActiveSupport < 7.2.4 and 8.0.x pass quirks_mode to JSON.generate/parse, which json 3 rejects + gem 'json', '< 3' end end end @@ -26,6 +28,8 @@ if Gem::Version.new('3.0.0') <= ruby_version && ruby_version < Gem::Version.new( gem 'sqlite3', '~> 1.4.0' gem 'rspec', '~> 3' gem 'rspec-rails', '~> 6' + # ActiveSupport < 7.2.4 and 8.0.x pass quirks_mode to JSON.generate/parse, which json 3 rejects + gem 'json', '< 3' end end end diff --git a/sunspot_rails/sunspot_rails.gemspec b/sunspot_rails/sunspot_rails.gemspec index 677ac8813..42ecec647 100644 --- a/sunspot_rails/sunspot_rails.gemspec +++ b/sunspot_rails/sunspot_rails.gemspec @@ -39,9 +39,6 @@ Gem::Specification.new do |s| s.add_development_dependency 'bundler', '>= 1.3.0', '< 2.0' if RUBY_VERSION <= '2.0.0' s.add_development_dependency 'nokogiri', '< 1.7' if RUBY_VERSION <= '2.0.0' s.add_development_dependency 'rake', '~> 13.2' - # ActiveSupport passes quirks_mode to JSON.generate/parse and json 3 rejects it. - # Rails 8 dropped the option; nothing in the Rails matrix is that new. - s.add_development_dependency 'json', '< 3' # Ruby 2.5 compatibility if RUBY_VERSION < '2.6' From 4fd7ce8a258790e64b4628cbad71ccdab26fd6fa Mon Sep 17 00:00:00 2001 From: Sampo Kuokkanen Date: Wed, 9 Sep 2026 20:36:06 +0900 Subject: [PATCH 3/4] Avoid NoMethodError when resolving DSL calls ContextBoundDelegate tries the receiver first and falls back to the calling context in a rescue, so every DSL call that belongs to the calling context raises a NoMethodError whose backtrace is then thrown away. Raising captures the whole stack at roughly 3 objects per frame, and a Rails request is already ~182 frames deep inside a search block, so each raise costs ~600 objects. In our app a block calling a few model helpers raised 70 times per request; dropping those took the request from 288,801 to 246,807 objects allocated, 14.6% less. The fast path uses respond_to? to dispatch straight to the calling context. Where respond_to? can't answer, method_missing without respond_to_missing? being the obvious case, it falls through to the old path. --- sunspot/lib/sunspot/util.rb | 33 +++++++++----- sunspot/spec/api/binding_spec.rb | 77 ++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 11 deletions(-) diff --git a/sunspot/lib/sunspot/util.rb b/sunspot/lib/sunspot/util.rb index 85ea7d245..e64790f30 100644 --- a/sunspot/lib/sunspot/util.rb +++ b/sunspot/lib/sunspot/util.rb @@ -298,20 +298,31 @@ def respond_to_missing?(method, _) end def __proxy_method__(method, *args, **kwargs, &block) - if kwargs.empty? - @__receiver__.__send__(method.to_sym, *args, &block) - else - @__receiver__.__send__(method.to_sym, *args, **kwargs, &block) + method = method.to_sym + + # Fast path: skip the rescue below when respond_to? can say where the + # method lives, avoiding a raise on every call into the calling context. + if !@__receiver__.respond_to?(method, true) && + @__calling_context__.respond_to?(method, true) + return __dispatch__(@__calling_context__, method, args, kwargs, &block) end - rescue ::NoMethodError => e + begin - if kwargs.empty? - @__calling_context__.__send__(method.to_sym, *args, &block) - else - @__calling_context__.__send__(method.to_sym, *args, **kwargs, &block) + __dispatch__(@__receiver__, method, args, kwargs, &block) + rescue ::NoMethodError => e + begin + __dispatch__(@__calling_context__, method, args, kwargs, &block) + rescue ::NoMethodError + raise(e) end - rescue ::NoMethodError - raise(e) + end + end + + def __dispatch__(target, method, args, kwargs, &block) + if kwargs.empty? + target.__send__(method, *args, &block) + else + target.__send__(method, *args, **kwargs, &block) end end end diff --git a/sunspot/spec/api/binding_spec.rb b/sunspot/spec/api/binding_spec.rb index 14008252b..e7f6ba751 100644 --- a/sunspot/spec/api/binding_spec.rb +++ b/sunspot/spec/api/binding_spec.rb @@ -63,3 +63,80 @@ def kwargs_method(a:, b:) { a: a, b: b } end end + +describe Sunspot::Util::ContextBoundDelegate do + # Resolution must not depend on raising; the discarded backtraces are costly. + it 'does not raise NoMethodError when resolving a calling context method' do + raised = 0 + tracer = TracePoint.new(:raise) do |tp| + raised += 1 if tp.raised_exception.is_a?(NoMethodError) + end + + context = Class.new do + def resolve(receiver) + Sunspot::Util::ContextBoundDelegate + .instance_eval_with_context(receiver) { context_only_method } + end + + def context_only_method + 'from context' + end + end.new + + result = nil + tracer.enable { result = context.resolve(Object.new) } + + expect(result).to eq('from context') + expect(raised).to eq(0) + end + + # respond_to? cannot see method_missing, so this must still resolve via the + # rescue-based fallback. + it 'resolves a calling context method that only exists via method_missing' do + context = Class.new do + def resolve(receiver) + Sunspot::Util::ContextBoundDelegate + .instance_eval_with_context(receiver) { ghost_method } + end + + def method_missing(name, *args, &block) + name == :ghost_method ? 'from method_missing' : super + end + # respond_to_missing? is deliberately not defined. + end.new + + expect(context.resolve(Object.new)).to eq('from method_missing') + end + + it 'raises NoMethodError when neither the receiver nor the context responds' do + context = Class.new do + def resolve(receiver) + Sunspot::Util::ContextBoundDelegate + .instance_eval_with_context(receiver) { totally_unknown_method } + end + end.new + + expect { context.resolve(Object.new) }.to raise_error(NoMethodError) + end + + it 'prefers the receiver when both define the method' do + receiver = Class.new do + def shared_method + 'from receiver' + end + end.new + + context = Class.new do + def resolve(receiver) + Sunspot::Util::ContextBoundDelegate + .instance_eval_with_context(receiver) { shared_method } + end + + def shared_method + 'from context' + end + end.new + + expect(context.resolve(receiver)).to eq('from receiver') + end +end From 0f1accca1f3f978a81c621bdbc4904494698fe8c Mon Sep 17 00:00:00 2001 From: Sampo Kuokkanen Date: Wed, 21 Oct 2026 09:54:17 +0900 Subject: [PATCH 4/4] Keep receiver-first dispatch for receivers with method_missing The respond_to? fast path let the calling context win over receivers that handle calls in method_missing without respond_to_missing?, such as DSL::Function (function { sum(...) } in a model class method) and DSL::Fields (double inside an RSpec example). It also called respond_to? on BasicObject receivers that don't have it. Only take the fast path when the receiver has Kernel, doesn't respond to the method and uses the default method_missing, so it could only have raised. Add specs for each case, and only count raises on the test thread. --- sunspot/lib/sunspot/util.rb | 18 +++++-- sunspot/spec/api/binding_spec.rb | 86 +++++++++++++++++++++++++++++++- 2 files changed, 100 insertions(+), 4 deletions(-) diff --git a/sunspot/lib/sunspot/util.rb b/sunspot/lib/sunspot/util.rb index e64790f30..e83be0378 100644 --- a/sunspot/lib/sunspot/util.rb +++ b/sunspot/lib/sunspot/util.rb @@ -300,9 +300,10 @@ def respond_to_missing?(method, _) def __proxy_method__(method, *args, **kwargs, &block) method = method.to_sym - # Fast path: skip the rescue below when respond_to? can say where the - # method lives, avoiding a raise on every call into the calling context. - if !@__receiver__.respond_to?(method, true) && + # Fast path: skip the rescue below when the receiver can't handle the + # method, avoiding a raise on every call into the calling context. + if !__may_handle__?(@__receiver__, method) && + ::Kernel === @__calling_context__ && @__calling_context__.respond_to?(method, true) return __dispatch__(@__calling_context__, method, args, kwargs, &block) end @@ -318,6 +319,17 @@ def __proxy_method__(method, *args, **kwargs, &block) end end + KERNEL_METHOD = ::Kernel.instance_method(:method) + + # respond_to? only settles it when method_missing is the default one; + # receivers like DSL::Function answer everything through method_missing. + # Objects without Kernel (BasicObject proxies) may not have respond_to?. + def __may_handle__?(target, method) + !(::Kernel === target) || + target.respond_to?(method, true) || + KERNEL_METHOD.bind(target).call(:method_missing).owner != ::BasicObject + end + def __dispatch__(target, method, args, kwargs, &block) if kwargs.empty? target.__send__(method, *args, &block) diff --git a/sunspot/spec/api/binding_spec.rb b/sunspot/spec/api/binding_spec.rb index e7f6ba751..5c4467548 100644 --- a/sunspot/spec/api/binding_spec.rb +++ b/sunspot/spec/api/binding_spec.rb @@ -49,6 +49,50 @@ expect(value).to eq({ a: 10, b: 20 }) end + it 'should build function queries whose names the calling context also defines' do + # Like a model class method calling solr_search, where the model responds + # to ActiveRecord's #sum. + searcher = Class.new do + def search(session) + session.search(Post) do + keywords('pizza') do + boost(function { sum(:average_rating, 10) }) + end + end + end + + def sum(*) + 'from context' + end + end.new + + searcher.search(session) + expect(connection).to have_last_search_including(:bf, 'sum(average_rating_ft,10)') + end + + it 'should define typed fields whose names the calling context also defines' do + stub_const('BindingSpecDocument', Class.new) + # The example group responds to rspec-mocks' #double. + Sunspot::Setup.setup(BindingSpecDocument) { double :rating } + expect(Sunspot::Setup.for(BindingSpecDocument).field(:rating).type).to be_a(Sunspot::Type::DoubleType) + end + + it 'should prefer the record\'s method_missing in index-time blocks' do + record_class = Class.new do + def method_missing(name, *args, &block) + name == :name ? 'from record' : super + end + # respond_to_missing? is deliberately not defined. + end + # Inside a searchable block the calling context is the model class, which + # responds to Class#name. + extractor = record_class.instance_eval do + Sunspot::DataExtractor::BlockExtractor.new { name } + end + + expect(extractor.value_for(record_class.new)).to eq('from record') + end + private def test_method @@ -68,8 +112,9 @@ def kwargs_method(a:, b:) # Resolution must not depend on raising; the discarded backtraces are costly. it 'does not raise NoMethodError when resolving a calling context method' do raised = 0 + test_thread = Thread.current tracer = TracePoint.new(:raise) do |tp| - raised += 1 if tp.raised_exception.is_a?(NoMethodError) + raised += 1 if Thread.current == test_thread && tp.raised_exception.is_a?(NoMethodError) end context = Class.new do @@ -139,4 +184,43 @@ def shared_method expect(context.resolve(receiver)).to eq('from receiver') end + + it 'prefers a receiver method_missing over a context method of the same name' do + receiver = Class.new do + def method_missing(name, *args, &block) + name == :shared_method ? 'from receiver' : super + end + # respond_to_missing? is deliberately not defined. + end.new + + context = Class.new do + def resolve(receiver) + Sunspot::Util::ContextBoundDelegate + .instance_eval_with_context(receiver) { shared_method } + end + + def shared_method + 'from context' + end + end.new + + expect(context.resolve(receiver)).to eq('from receiver') + end + + it 'dispatches to a BasicObject receiver that does not implement respond_to?' do + receiver = Class.new(BasicObject) do + def method_missing(name, *args, &block) + name == :proxied_method ? 'from proxy' : super + end + end.new + + context = Class.new do + def resolve(receiver) + Sunspot::Util::ContextBoundDelegate + .instance_eval_with_context(receiver) { proxied_method } + end + end.new + + expect(context.resolve(receiver)).to eq('from proxy') + end end