From 2bf2fd4a5cea64ac5e4123840de8ccf7714ed80b Mon Sep 17 00:00:00 2001 From: Nicholas Jakobsen Date: Mon, 5 Oct 2026 01:21:28 -0700 Subject: [PATCH 01/11] feat: Index associations as nested child documents and search them with block joins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue: Sunspot indexes one flat document per record, so a search can't require that a single associated record meet several conditions together. Indexing an association's fields as multivalued fields on the parent loses which value came from which record: a parent with one milestone named "design" and another started in Q1 matches "a design milestone started in Q1". `nested :milestones do … end` in a setup block now indexes each record of the association as a Solr child document inside its parent's block, with the fields declared in the block. Each child gets an id derived from its parent's (`"//"`) and a `_sunspot_nested_path_s` marker naming the association, and no `type` or `class_name`. A nested block rejects boosts, id prefixes, joins and further nesting. `with_child :milestones do … end` and `without_child` scope a search to parents with, or without, a child meeting every restriction in the block. They render as a `{!parent}` block join embedded with `_query_`, so they work in filter queries, `any_of`/`all_of`, query facet rows and delete-by-query. The block mask is every document without the marker, so documents of other classes indexed between blocks are never taken for children, and the child query always requires the marker, so a negated restriction can't match documents outside the association. Each restriction in the block is joined directly to that marker condition, so a block of only negated restrictions still matches. Removing a nested class's records also deletes their children: by `_root_` after a delete by id, since Solr 6 leaves children behind on a delete by id where Solr 9 removes them, and through a `{!child}` query before `remove_all` and `remove_by_scope`. Atomic updates raise `ArgumentError` for a class with nested associations. The bundled configset gains the `_root_` field, which Solr requires before it indexes a document with children. Specs: 46 examples in `spec/api/nested_documents_spec.rb` and `spec/integration/nested_documents_spec.rb`, passing on Solr 6.6.6 and 9.10.1 with XML and JSON updates. The full suite's other failures on Solr 6 match master's. Co-Authored-By: Claude Opus 5.5 --- sunspot/lib/sunspot.rb | 3 +- sunspot/lib/sunspot/composite_setup.rb | 19 ++ sunspot/lib/sunspot/dsl/fields.rb | 58 +++++ sunspot/lib/sunspot/dsl/scope.rb | 46 ++++ sunspot/lib/sunspot/indexer.rb | 99 +++++++- sunspot/lib/sunspot/nested_setup.rb | 55 +++++ sunspot/lib/sunspot/query.rb | 2 +- sunspot/lib/sunspot/query/block_join.rb | 83 +++++++ sunspot/lib/sunspot/session.rb | 2 +- sunspot/lib/sunspot/setup.rb | 28 +++ sunspot/spec/api/nested_documents_spec.rb | 214 ++++++++++++++++++ .../spec/integration/nested_documents_spec.rb | 157 +++++++++++++ sunspot/spec/mocks/project.rb | 44 ++++ .../solr/configsets/sunspot/conf/schema.xml | 3 + 14 files changed, 803 insertions(+), 10 deletions(-) create mode 100644 sunspot/lib/sunspot/nested_setup.rb create mode 100644 sunspot/lib/sunspot/query/block_join.rb create mode 100644 sunspot/spec/api/nested_documents_spec.rb create mode 100644 sunspot/spec/integration/nested_documents_spec.rb create mode 100644 sunspot/spec/mocks/project.rb diff --git a/sunspot/lib/sunspot.rb b/sunspot/lib/sunspot.rb index 72798f82a..294653086 100644 --- a/sunspot/lib/sunspot.rb +++ b/sunspot/lib/sunspot.rb @@ -12,7 +12,7 @@ require File.join(File.dirname(__FILE__), 'light_config') -%w(util adapters configuration setup composite_setup text_field_setup field +%w(util adapters configuration setup nested_setup composite_setup text_field_setup field field_factory data_extractor indexer query search session session_proxy type dsl class_set).each do |filename| require File.join(File.dirname(__FILE__), 'sunspot', filename) @@ -38,6 +38,7 @@ module Sunspot UnrecognizedRestrictionError = Class.new(StandardError) NoAdapterError = Class.new(StandardError) NoSetupError = Class.new(StandardError) + NestedDocumentsNotSupportedError = Class.new(StandardError) IllegalSearchError = Class.new(StandardError) NotImplementedError = Class.new(StandardError) AtomicUpdateRequireInstanceForCompositeIdMessage = lambda do |class_name| diff --git a/sunspot/lib/sunspot/composite_setup.rb b/sunspot/lib/sunspot/composite_setup.rb index 4090f1427..5fb753402 100644 --- a/sunspot/lib/sunspot/composite_setup.rb +++ b/sunspot/lib/sunspot/composite_setup.rb @@ -101,6 +101,25 @@ def dynamic_field_factory(field_name) ) end + # + # Returns the NestedSetup for the given association from the first of the + # searched types that declares it. Raises UnrecognizedFieldError when none + # does. + # + def nested_setup(name) + setups.each do |setup| + begin + return setup.nested_setup(name) + rescue UnrecognizedFieldError + next + end + end + raise( + UnrecognizedFieldError, + "No nested association configured for #{@types * ', '} with name '#{name}'" + ) + end + # # Collection of all text fields configured for any of the enclosed types. # diff --git a/sunspot/lib/sunspot/dsl/fields.rb b/sunspot/lib/sunspot/dsl/fields.rb index dd107fff5..4b67c8823 100644 --- a/sunspot/lib/sunspot/dsl/fields.rb +++ b/sunspot/lib/sunspot/dsl/fields.rb @@ -71,6 +71,41 @@ def id_prefix(attr_name = nil, &block) @setup.add_id_prefix(attr_name, &block) end + # + # Indexes the records of an association as child documents of this + # class's documents, in the same Solr block. Fields declared in the block + # are indexed on each child, and a field's block is evaluated against the + # child record. Use DSL::Scope#with_child to find parents by conditions + # that a single child must meet together. + # + # Children are indexed only as part of their parent, so the parent must + # be reindexed whenever its children change. A class with nested + # associations cannot be atomically updated. + # + # The block cannot declare a document boost, an id prefix, a join, or a + # nested association of its own. Each raises ArgumentError. + # + # ==== Parameters + # + # name:: The association, called on the parent to get the children + # + # ==== Options + # + # :using:: Method to call on the parent instead of +name+ + # + # ==== Example + # + # Sunspot.setup(Project) do + # nested :milestones do + # string :name + # time :started_at + # end + # end + # + def nested(name, options = {}, &block) + @setup.add_nested(name, options, &block) + end + # method_missing is used to provide access to typed fields, because # developers should be able to add new Sunspot::Type implementations # dynamically and have them recognized inside the Fields DSL. Like #text, @@ -119,5 +154,28 @@ def method_missing(method, *args, &block) end end end + + # + # The fields DSL inside a DSL::Fields#nested block. Rejects document + # boosts, id prefixes, joins, and nested associations with ArgumentError. + # + class NestedFields < Fields #:nodoc: + def boost(*) + raise ArgumentError, "Document boosts are not supported on nested documents" + end + + def id_prefix(*) + raise ArgumentError, "ID prefixes are not supported on nested documents, which take their parent's" + end + + def nested(*) + raise ArgumentError, "Nested documents cannot have nested documents of their own" + end + + def method_missing(method, *args, &block) + raise ArgumentError, "Joins are not supported on nested documents" if method.to_s == 'join' + super + end + end end end diff --git a/sunspot/lib/sunspot/dsl/scope.rb b/sunspot/lib/sunspot/dsl/scope.rb index a1830988c..f52f3c868 100644 --- a/sunspot/lib/sunspot/dsl/scope.rb +++ b/sunspot/lib/sunspot/dsl/scope.rb @@ -97,6 +97,45 @@ def without(*args) add_restriction(true, *args) end + # + # Scope the results to documents with at least one child in the given + # association that satisfies every restriction in the block. Without a + # block, any child in the association matches. The association is one + # declared with DSL::Fields#nested. + # + # The block takes the same restrictions as a scope, with field names + # referring to the child's fields. + # + # ==== Example + # + # Sunspot.search(Project) do + # with_child :milestones do + # with :name, 'design' + # with(:started_at).between(Time.utc(2026, 1, 1)...Time.utc(2026, 4, 1)) + # end + # end + # + def with_child(name, &block) + add_block_join(false, name, &block) + end + + # + # Scope the results to documents with no child in the given association + # that satisfies every restriction in the block. Documents with no + # children in the association match. Without a block, only those match. + # + # ==== Example + # + # Sunspot.search(Project) do + # without_child :milestones do + # with :name, 'launch' + # end + # end + # + def without_child(name, &block) + add_block_join(true, name, &block) + end + # # Create a disjunction, scoping the results to documents that match any # of the enclosed restrictions. @@ -197,6 +236,13 @@ def text_fields(&block) private + def add_block_join(negated, name, &block) + nested_setup = @setup.nested_setup(name) + block_join = Sunspot::Query::BlockJoin.new(nested_setup, negated) + Util.instance_eval_or_call(Scope.new(block_join.scope, nested_setup), &block) if block + @scope.add_component(block_join) + end + def add_restriction(negated, *args) case args.first when String, Symbol diff --git a/sunspot/lib/sunspot/indexer.rb b/sunspot/lib/sunspot/indexer.rb index 0703e3e7b..069fcd542 100644 --- a/sunspot/lib/sunspot/indexer.rb +++ b/sunspot/lib/sunspot/indexer.rb @@ -45,9 +45,9 @@ def add_atomic_update(clazz, updates={}) # Remove the given model from the Solr index # def remove(*models) - @connection.delete_by_id( - models.map { |model| Adapters::InstanceAdapter.adapt(model).index_id } - ) + ids = models.map { |model| Adapters::InstanceAdapter.adapt(model).index_id } + @connection.delete_by_id(ids) + remove_blocks(ids.select.with_index { |_, i| nested?(models[i].class) }) end # @@ -70,9 +70,9 @@ def remove_by_id(class_name, *ids) end ids.flatten! - @connection.delete_by_id( - ids.map { |id| Adapters::InstanceAdapter.index_id_for("#{id_prefix}#{class_name}", id) } - ) + index_ids = ids.map { |id| Adapters::InstanceAdapter.index_id_for("#{id_prefix}#{class_name}", id) } + @connection.delete_by_id(index_ids) + remove_blocks(index_ids) if nested?(Util.full_const_get(class_name)) end # @@ -80,6 +80,11 @@ def remove_by_id(class_name, *ids) # def remove_all(clazz = nil) if clazz + # Children are deleted first, because the query that finds them goes + # through their parents. + if nested?(clazz) + @connection.delete_by_query(children_of("type:#{Util.escape(clazz.name)}")) + end @connection.delete_by_query("type:#{Util.escape(clazz.name)}") else @connection.delete_by_query("*:*") @@ -89,7 +94,12 @@ def remove_all(clazz = nil) # # Remove all documents that match the scope given in the Query # - def remove_by_scope(scope) + def remove_by_scope(scope, types = []) + # Children are deleted first, because the query that finds them goes + # through their parents. + if types.any? { |type| nested?(type) } + @connection.delete_by_query(children_of(scope.to_boolean_phrase)) + end @connection.delete_by_query(scope.to_boolean_phrase) end @@ -125,10 +135,85 @@ def prepare_full_update(model) setup.all_field_factories.each do |field_factory| field_factory.populate_document(document, model) end + setup.nested_setups.each do |nested_setup| + add_child_documents(document, nested_setup, model) + end document end + # + # Adds a child document to +document+ for each of the model's children in + # the association, so Solr indexes the parent and its children as one + # block. Raises NestedDocumentsNotSupportedError when there are children + # and RSolr cannot send child documents. + # + # A child's id is the parent's id followed by the association name and + # #child_key, so it is unique within the index and carries the parent's + # id prefix. + # + def add_child_documents(document, nested_setup, model) + children = nested_setup.children_for(model) + return if children.empty? + ensure_child_documents_supported! + + parent_id = document.field_by_name(:id).value + children.each_with_index do |child, position| + child_document = RSolr::Xml::Document.new( + id: "#{parent_id}/#{nested_setup.name}/#{child_key(child, position)}", + NestedSetup::PATH_FIELD.to_sym => nested_setup.path + ) + nested_setup.all_field_factories.each do |field_factory| + field_factory.populate_document(child_document, child) + end + document.add_field(RSolr::Document::CHILD_DOCUMENT_KEY, child_document) + end + end + + # Returns the child's index id when it has an adapter, so a child document + # can be traced back to its record. Returns its position in the + # association otherwise. + def child_key(child, position) + Adapters::InstanceAdapter.adapt(child).index_id + rescue NoAdapterError + position + end + + def ensure_child_documents_supported! + return if defined?(RSolr::Document::CHILD_DOCUMENT_KEY) + raise NestedDocumentsNotSupportedError, "Nested documents require an RSolr version that defines RSolr::Document::CHILD_DOCUMENT_KEY" + end + + def nested?(clazz) + setup = Setup.for(clazz) + !setup.nil? && setup.nested_setups.any? + end + + # + # Deletes every document in the blocks rooted at the given ids. Solr 9 + # deletes a parent's children along with it on a delete by id, and Solr 6 + # leaves them in the index. A child left behind is joined to whichever + # parent follows it in the index. + # + def remove_blocks(ids) + return if ids.empty? + @connection.delete_by_query("_root_:(#{ids.map { |id| %Q("#{Util.escape(id)}") }.join(' OR ')})") + end + + # + # Returns a query matching the children of the parents that match + # +parent_query+. The block mask is every document that is not a child, so + # documents of other classes indexed between blocks are never matched as + # children. + # + def children_of(parent_query) + %Q({!child of="*:* -#{NestedSetup::PATH_FIELD}:[* TO *]"}#{parent_query}) + end + def prepare_atomic_update(clazz, id, updates = {}) + if nested?(clazz) + raise ArgumentError, "Atomic updates are not supported for #{clazz.name}, which has nested documents. " \ + "Index the whole record instead, which reindexes its children with it" + end document = document_for_atomic_update(clazz, id) setup_for_class(clazz).all_field_factories.each do |field_factory| if updates.has_key?(field_factory.name) diff --git a/sunspot/lib/sunspot/nested_setup.rb b/sunspot/lib/sunspot/nested_setup.rb new file mode 100644 index 000000000..8d5ecb47e --- /dev/null +++ b/sunspot/lib/sunspot/nested_setup.rb @@ -0,0 +1,55 @@ +module Sunspot + # + # The field setup for the children of one association declared with + # DSL::Fields#nested. Children are indexed only as part of their parent's + # block, so a nested setup has fields but no class of its own. Child + # documents carry no +type+ or +class_name+ field. PATH_FIELD names their + # association instead. + # + class NestedSetup < Setup #:nodoc: + # PATH_FIELD marks each child document with the association it belongs + # to. A document without it is the root of a block. Child queries are + # scoped on it, which keeps children of other associations and documents + # of other classes out of the block join. + PATH_FIELD = '_sunspot_nested_path_s'.freeze + + attr_reader :name, :parent_setup + + def initialize(parent_setup, name, options = {}) + @parent_setup = parent_setup + @name = name.to_sym + @class_name = "#{parent_setup.type_names.first}.#{@name}" + @field_factories, @text_field_factories, @dynamic_field_factories, + @field_factories_cache, @text_field_factories_cache, + @dynamic_field_factories_cache = *Array.new(6) { Hash.new } + @stored_field_factories_cache = Hash.new { |h, k| h[k] = [] } + @more_like_this_field_factories_cache = Hash.new { |h, k| h[k] = [] } + @nested_setups = {} + @dsl = DSL::NestedFields.new(self) + @children_extractor = DataExtractor::AttributeExtractor.new(options[:using] || @name) + end + + # + # Returns the value stored in PATH_FIELD on every child of this + # association: the declaring class and the association name, such as + # "Project.milestones". + # + def path + @class_name + end + + def children_for(model) + Util.Array(@children_extractor.value_for(model)).compact + end + + def clazz + raise NotImplementedError, "Nested association #{path} has no class of its own" + end + + protected + + def parent + nil + end + end +end diff --git a/sunspot/lib/sunspot/query.rb b/sunspot/lib/sunspot/query.rb index c3248bcee..6cdae57d7 100644 --- a/sunspot/lib/sunspot/query.rb +++ b/sunspot/lib/sunspot/query.rb @@ -1,5 +1,5 @@ %w(filter abstract_field_facet abstract_json_field_facet connective boost_query date_field_facet field_json_facet - range_facet range_json_facet date_field_json_facet abstract_fulltext dismax join + range_facet range_json_facet date_field_json_facet abstract_fulltext dismax join block_join field_list field_facet highlighting pagination restriction common_query spellcheck standard_query more_like_this more_like_this_query geo geofilt bbox query_facet scope sort sort_composite text_field_boost function_query field_stats diff --git a/sunspot/lib/sunspot/query/block_join.rb b/sunspot/lib/sunspot/query/block_join.rb new file mode 100644 index 000000000..295b80f90 --- /dev/null +++ b/sunspot/lib/sunspot/query/block_join.rb @@ -0,0 +1,83 @@ +module Sunspot + module Query + # + # A restriction matching parents with at least one child in a nested + # association that satisfies every restriction in #scope. Negated, it + # matches parents with no such child. + # + # Renders as a Solr block join embedded with +_query_+, so it can sit in a + # filter query, a connective, a query facet row or a delete-by-query like + # any other restriction. + # + class BlockJoin #:nodoc: + include Filter + + # ROOTS is Solr's block mask: every document without + # NestedSetup::PATH_FIELD, whatever its class. A document of another class + # indexed between blocks is therefore a root to Solr, never a child. + ROOTS = "*:* -#{NestedSetup::PATH_FIELD}:[* TO *]".freeze + + # The restrictions a child must satisfy. Its first component is the + # condition on NestedSetup::PATH_FIELD, so every restriction added to it, + # negated ones included, is joined to that condition. + attr_reader :scope + + def initialize(nested_setup, negated = false, scope = nil) + @nested_setup, @negated = nested_setup, negated + @scope = scope || Connective::Conjunction.new.tap { |conjunction| conjunction.add_component(PathRestriction.new(nested_setup)) } + end + + def to_boolean_phrase + phrase = %Q(_query_:"#{escape(%Q({!parent which="#{escape(ROOTS)}" v="#{escape(child_query)}"}))}") + negated? ? "-#{phrase}" : phrase + end + + def negated? + @negated + end + + def negate + self.class.new(@nested_setup, !negated?, @scope) + end + + private + + # + # Returns the query for this association's children: #scope, whose first + # component requires NestedSetup::PATH_FIELD to match the association. + # Solr joins every document the child query matches to the next parent + # in the index, including documents of other classes and children of + # other associations, so that condition is always required. + # + def child_query + @scope.to_boolean_phrase + end + + def escape(value) + self.class.escape(value) + end + + # Backslash-escapes quotes and backslashes for a quoted Solr string. + # #to_boolean_phrase applies it at both levels of nesting: to the local + # param values, then to the whole +_query_+ phrase. + def self.escape(value) + value.gsub(/(["\\])/, '\\\\\1') + end + + # The condition matching the children of one nested association. + class PathRestriction #:nodoc: + def initialize(nested_setup) + @nested_setup = nested_setup + end + + def to_boolean_phrase + %Q(#{NestedSetup::PATH_FIELD}:"#{BlockJoin.escape(@nested_setup.path)}") + end + + def negated? + false + end + end + end + end +end diff --git a/sunspot/lib/sunspot/session.rb b/sunspot/lib/sunspot/session.rb index 947a858c1..846691574 100644 --- a/sunspot/lib/sunspot/session.rb +++ b/sunspot/lib/sunspot/session.rb @@ -145,7 +145,7 @@ def remove(*objects, &block) end dsl = DSL::Scope.new(conjunction, setup_for_types(types)) Util.instance_eval_or_call(dsl, &block) - indexer.remove_by_scope(conjunction) + indexer.remove_by_scope(conjunction, types) else objects.flatten! @deletes += objects.length diff --git a/sunspot/lib/sunspot/setup.rb b/sunspot/lib/sunspot/setup.rb index 1cba5f071..7709e7ffc 100644 --- a/sunspot/lib/sunspot/setup.rb +++ b/sunspot/lib/sunspot/setup.rb @@ -13,6 +13,7 @@ def initialize(clazz) @dynamic_field_factories_cache = *Array.new(6) { Hash.new } @stored_field_factories_cache = Hash.new { |h, k| h[k] = [] } @more_like_this_field_factories_cache = Hash.new { |h, k| h[k] = [] } + @nested_setups = {} @dsl = DSL::Fields.new(self) @document_boost_extractor = nil @id_prefix_extractor = nil @@ -50,6 +51,33 @@ def add_join_field_factory(name, type, options = {}, &block) end end + # + # Declares an association whose records are indexed as child documents of + # this class's documents. Returns the NestedSetup the block's fields are + # added to. + # + def add_nested(name, options = {}, &block) + nested_setup = NestedSetup.new(self, name, options) + nested_setup.setup(&block) if block + @nested_setups[nested_setup.name] = nested_setup + end + + # + # Returns the NestedSetup for the given association, including + # associations declared on a superclass. Raises UnrecognizedFieldError when + # no such association is declared. + # + def nested_setup(name) + get_inheritable_hash(:nested_setups)[name.to_sym] || raise( + UnrecognizedFieldError, + "No nested association configured for #{@class_name} with name '#{name}'" + ) + end + + def nested_setups + collection_from_inheritable_hash(:nested_setups) + end + # # Add field_factories for fulltext search # diff --git a/sunspot/spec/api/nested_documents_spec.rb b/sunspot/spec/api/nested_documents_spec.rb new file mode 100644 index 000000000..96257b094 --- /dev/null +++ b/sunspot/spec/api/nested_documents_spec.rb @@ -0,0 +1,214 @@ +require File.expand_path('spec_helper', File.dirname(__FILE__)) + +describe 'nested documents' do + let(:connection) { Mock::Connection.new } + let(:session) { Sunspot::Session.new(Sunspot::Configuration.build, connection) } + let(:roots) { '*:* -_sunspot_nested_path_s:[* TO *]' } + + def last_fq + connection.searches.last[:fq] + end + + describe 'setup' do + it 'resolves child fields against the association, not the parent' do + nested_setup = Sunspot::Setup.for(Project).nested_setup(:milestones) + expect(nested_setup.field(:started_at).indexed_name).to eq('started_at_d') + expect { nested_setup.field(:status) }.to raise_error(Sunspot::UnrecognizedFieldError) + end + + it "keeps each association's fields separate" do + setup = Sunspot::Setup.for(Project) + expect(setup.nested_setups.map(&:name)).to eq([:milestones, :reviews]) + expect { setup.nested_setup(:milestones).field(:verdict) }.to raise_error(Sunspot::UnrecognizedFieldError) + end + + it 'names each association by its declaring class' do + expect(Sunspot::Setup.for(Project).nested_setup(:milestones).path).to eq('Project.milestones') + end + + it 'raises for an association that was never declared' do + expect { Sunspot::Setup.for(Project).nested_setup(:tasks) }.to raise_error(Sunspot::UnrecognizedFieldError) + end + + it 'rejects boosts, nested associations and joins in a nested block' do + expect { Sunspot::Setup.for(Project).add_nested(:x) { boost 2 } }.to raise_error(ArgumentError) + expect { Sunspot::Setup.for(Project).add_nested(:x) { nested(:y) {} } }.to raise_error(ArgumentError) + expect { Sunspot::Setup.for(Project).add_nested(:x) { join(:y, :target => Post, :type => :string, :join => { :from => :a, :to => :b }) } }.to raise_error(ArgumentError) + end + end + + describe 'indexing' do + let(:milestone) { Milestone.new(:name => 'design', :started_at => Time.utc(2026, 2, 1), :owner_names => %w(ana ben)) } + let(:project) { Project.new(:name => 'Bridge', :milestones => [milestone], :reviews => [OpenStruct.new(:verdict => 'approved')]) } + + def indexed_project + session.index(project) + connection.adds.last.first + end + + def children(document) + document.fields_by_name(RSolr::Document::CHILD_DOCUMENT_KEY).map(&:value) + end + + it 'sends children inside the parent document' do + expect(children(indexed_project).length).to eq(2) + end + + it 'gives each child an id derived from its parent, and its own index id when it has an adapter' do + milestone_doc = children(indexed_project).first + expect(milestone_doc.field_by_name(:id).value).to eq("Project #{project.id}/milestones/Milestone #{milestone.id}") + end + + it 'falls back to the position in the association for children without an adapter' do + review_doc = children(indexed_project).last + expect(review_doc.field_by_name(:id).value).to eq("Project #{project.id}/reviews/0") + end + + it 'marks each child with its association and gives it no type of its own' do + milestone_doc = children(indexed_project).first + expect(milestone_doc.field_by_name(:_sunspot_nested_path_s).value).to eq('Project.milestones') + expect(milestone_doc.field_by_name(:type)).to be_nil + expect(milestone_doc.field_by_name(:class_name)).to be_nil + end + + it 'indexes the child fields, evaluating blocks against the child' do + milestone_doc = children(indexed_project).first + expect(milestone_doc.field_by_name(:name_s).value).to eq('design') + expect(milestone_doc.field_by_name(:started_at_d).value).to eq('2026-02-01T00:00:00Z') + expect(milestone_doc.fields_by_name(:owner_names_sm).map(&:value)).to eq(%w(ana ben)) + expect(milestone_doc.field_by_name(:days_late_i).value).to eq('0') + end + + it 'keeps the parent fields on the parent' do + expect(indexed_project.field_by_name(:name_s).value).to eq('Bridge') + end + + it 'sends no children for an empty association' do + session.index(Project.new(:name => 'Empty')) + expect(children(connection.adds.last.first)).to be_empty + end + + it 'refuses atomic updates to a class with nested associations' do + expect { session.atomic_update(Project, project.id => { :name => 'x' }) }.to raise_error(ArgumentError, /nested documents/) + end + end + + describe 'querying' do + it 'finds parents by conditions a single child meets together' do + session.search(Project) do + with_child :milestones do + with :name, 'design' + with(:started_at).greater_than(Time.utc(2026, 1, 1)) + end + end + expect(last_fq).to include( + %q(_query_:"{!parent which=\\"*:* -_sunspot_nested_path_s:[* TO *]\\" v=\\"(_sunspot_nested_path_s:\\\\\\"Project.milestones\\\\\\" AND name_s:design AND started_at_d:{2026\\\\\\\\-01\\\\\\\\-01T00\\\\\\\\:00\\\\\\\\:00Z TO *})\\"}") + ) + end + + it 'scopes a block with no restrictions to the association alone' do + session.search(Project) { with_child(:milestones) } + expect(last_fq.last).to include('v=\\"_sunspot_nested_path_s:\\\\\\"Project.milestones\\\\\\"\\"') + end + + it 'negates with without_child' do + session.search(Project) { without_child(:milestones) { with :name, 'launch' } } + expect(last_fq.last).to start_with('-_query_:"{!parent') + end + + it 'joins each negated restriction directly to the association condition' do + session.search(Project) { with_child(:milestones) { without :name, 'launch'; without :name, 'design' } } + expect(last_fq.last).to include('Project.milestones\\\\\\" AND -name_s:launch AND -name_s:design)') + end + + it 'combines with parent restrictions as separate filters' do + session.search(Project) do + with :status, 'active' + with_child(:milestones) { with :name, 'design' } + end + expect(last_fq).to include('status_s:active') + expect(last_fq.last).to start_with('_query_:"{!parent') + end + + it 'works inside any_of, including negated alternatives' do + session.search(Project) do + any_of do + with_child(:milestones) { with :name, 'design' } + without_child(:milestones) { with :name, 'launch' } + end + end + expect(last_fq.last).to match(/\A-\(-_query_:"\{!parent .*name_s:design.* AND _query_:"\{!parent .*name_s:launch.*\)\z/) + end + + it 'supports connectives inside the block' do + session.search(Project) do + with_child :milestones do + any_of do + with :name, 'design' + with :name, 'build' + end + end + end + expect(last_fq.last).to include('(name_s:design OR name_s:build)') + end + + it 'escapes quotes and backslashes through both levels of nesting' do + value = %q(say "hi" \ bye) + session.search(Project) { with_child(:milestones) { with :name, value } } + + # Undo the escaping the way Solr's parsers will: the _query_ phrase, then the local param + unescape = ->(string) { string.gsub(/\\(.)/, '\1') } + local_params = unescape.(last_fq.last[/\A_query_:"(.*)"\z/, 1]) + child_query = unescape.(local_params[/ v="(.*)"\}\z/, 1]) + + expect(child_query).to eq(%Q((_sunspot_nested_path_s:"Project.milestones" AND name_s:#{Sunspot::Util.escape(value)}))) + end + + it 'raises for an association that was never declared' do + expect { session.search(Project) { with_child(:tasks) {} } }.to raise_error(Sunspot::UnrecognizedFieldError) + end + + it 'resolves the association across a multi-class search' do + session.search(Project, Post) { with_child(:milestones) { with :name, 'design' } } + expect(last_fq.last).to include('Project.milestones') + end + end + + describe 'removal' do + it 'removes the whole block when removing a parent' do + project = Project.new + session.remove(project) + expect(connection).to have_delete("Project #{project.id}") + expect(connection).to have_delete_by_query(%Q(_root_:("Project\\ #{project.id}"))) + end + + it 'removes the whole block when removing by id' do + session.remove_by_id(Project, 1, 2) + expect(connection).to have_delete('Project 1', 'Project 2') + expect(connection).to have_delete_by_query('_root_:("Project\\ 1" OR "Project\\ 2")') + end + + it 'removes children before parents when removing a class' do + session.remove_all(Project) + expect(connection.deletes_by_query).to eq([ + %Q({!child of="#{roots}"}type:Project), + 'type:Project' + ]) + end + + it 'removes children before parents when removing by scope' do + session.remove(Project) { with :status, 'archived' } + expect(connection.deletes_by_query).to eq([ + %Q({!child of="#{roots}"}(type:Project AND status_s:archived)), + '(type:Project AND status_s:archived)' + ]) + end + + it 'leaves classes without nested documents alone' do + post = Post.new + session.remove(post) + session.remove_all(Post) + expect(connection.deletes_by_query).to eq(['type:Post']) + end + end +end diff --git a/sunspot/spec/integration/nested_documents_spec.rb b/sunspot/spec/integration/nested_documents_spec.rb new file mode 100644 index 000000000..fd9ea5825 --- /dev/null +++ b/sunspot/spec/integration/nested_documents_spec.rb @@ -0,0 +1,157 @@ +require File.expand_path('../spec_helper', File.dirname(__FILE__)) + +describe 'nested documents' do + def milestone(name, started_at, attrs = {}) + Milestone.new({ :name => name, :started_at => started_at }.merge(attrs)) + end + + def search(&block) + Sunspot.search(Project, &block).results + end + + # Raw count, straight from Solr, so leftover children can't hide behind Sunspot's type filter + def solr_count(query) + Sunspot.session.send(:connection).get('select', :params => { :q => query, :rows => 0 })['response']['numFound'] + end + + let(:q1) { Time.utc(2026, 1, 1)...Time.utc(2026, 4, 1) } + + before :each do + Sunspot.remove_all! + + @designed_in_q1 = Project.new(:name => 'designed in q1', :status => 'active', :milestones => [ + milestone('design', Time.utc(2026, 2, 1), :due_at => Time.utc(2026, 1, 15), :finished_at => Time.utc(2026, 2, 20), :owner_names => %w(ana)) + ]) + # A design milestone and a Q1 date, but on different milestones + @designed_earlier = Project.new(:name => 'designed earlier', :status => 'active', :milestones => [ + milestone('design', Time.utc(2025, 6, 1), :due_at => Time.utc(2025, 7, 1), :finished_at => Time.utc(2025, 6, 20), :owner_names => %w(ben)), + milestone('build', Time.utc(2026, 2, 1), :owner_names => %w(ana)) + ]) + @without_milestones = Project.new(:name => 'no milestones', :status => 'archived') + + # A document of another class between the blocks, sharing a field name with the children + @memo = Memo.new(:name => 'design') + + Sunspot.index!(@designed_in_q1, @memo, @designed_earlier, @without_milestones) + end + + describe 'with_child' do + it 'requires one child to meet every condition' do + q1 = self.q1 + expect(search { with_child(:milestones) { with :name, 'design'; with(:started_at, q1) } }).to eq([@designed_in_q1]) + end + + it 'matches any child with no conditions' do + expect(search { with_child(:milestones) }).to match_array([@designed_in_q1, @designed_earlier]) + end + + it 'filters on a field computed from the child' do + expect(search { with_child(:milestones) { with(:days_late).greater_than(0) } }).to eq([@designed_in_q1]) + end + + it 'matches a value of a multivalued child field' do + expect(search { with_child(:milestones) { with :owner_names, 'ben' } }).to eq([@designed_earlier]) + end + + it 'matches a child by a negated condition' do + expect(search { with_child(:milestones) { without :name, 'design' } }).to eq([@designed_earlier]) + end + + it 'matches a child by several negated conditions' do + expect(search { with_child(:milestones) { without :name, 'design'; without :name, 'launch' } }).to eq([@designed_earlier]) + end + + it 'supports alternatives inside one child' do + q1 = self.q1 + results = search do + with_child :milestones do + any_of do + all_of { with :name, 'design'; with(:started_at, q1) } + with :owner_names, 'ben' + end + end + end + expect(results).to match_array([@designed_in_q1, @designed_earlier]) + end + + it 'combines a child condition with a parent condition in any_of' do + results = search do + any_of do + with_child(:milestones) { with :owner_names, 'ben' } + with :status, 'archived' + end + end + expect(results).to match_array([@designed_earlier, @without_milestones]) + end + + it 'combines with parent restrictions' do + expect(search { with :status, 'active'; with_child(:milestones) { with :owner_names, 'ana' } }).to match_array([@designed_in_q1, @designed_earlier]) + expect(search { with :status, 'archived'; with_child(:milestones) }).to eq([]) + end + + it 'finds values that need escaping' do + tricky = %q(say "hi" \ bye) + odd = Project.new(:milestones => [milestone(tricky, Time.utc(2026, 1, 1))]) + Sunspot.index!(odd) + expect(search { with_child(:milestones) { with :name, tricky } }).to eq([odd]) + end + end + + describe 'without_child' do + it 'matches parents with no child meeting the conditions, including parents with no children' do + q1 = self.q1 + expect(search { without_child(:milestones) { with :name, 'design'; with(:started_at, q1) } }).to match_array([@designed_earlier, @without_milestones]) + end + end + + describe 'searching the parents alone' do + it 'returns parents, never children' do + expect(search {}).to match_array([@designed_in_q1, @designed_earlier, @without_milestones]) + end + + it 'leaves other classes alone' do + expect(Sunspot.search(Memo).results).to eq([@memo]) + end + end + + describe 'reindexing' do + it 'replaces the whole block' do + @designed_earlier.milestones = [@designed_earlier.milestones.last] + Sunspot.index!(@designed_earlier) + + expect(solr_count(%Q(_root_:"Project #{@designed_earlier.id}"))).to eq(2) + expect(solr_count('_sunspot_nested_path_s:[* TO *]')).to eq(2) + expect(search { with_child(:milestones) { with :owner_names, 'ben' } }).to eq([]) + end + end + + describe 'removal' do + def children + solr_count('_sunspot_nested_path_s:[* TO *]') + end + + it 'removes the children with their parent' do + Sunspot.remove!(@designed_earlier) + expect(children).to eq(1) + expect(search { with_child(:milestones) }).to eq([@designed_in_q1]) + end + + it 'removes the children when removing by id' do + Sunspot.remove_by_id!(Project, @designed_earlier.id) + expect(children).to eq(1) + end + + it 'removes the children when removing by scope' do + Sunspot.remove!(Project) { with :name, 'designed earlier' } + expect(children).to eq(1) + expect(search {}).to match_array([@designed_in_q1, @without_milestones]) + end + + it 'removes every child when removing the class, and nothing else' do + Sunspot.remove_all!(Project) + expect(children).to eq(0) + expect(solr_count('*:*')).to eq(1) + expect(Sunspot.search(Memo).results).to eq([@memo]) + end + end +end diff --git a/sunspot/spec/mocks/project.rb b/sunspot/spec/mocks/project.rb new file mode 100644 index 000000000..66c4ccffb --- /dev/null +++ b/sunspot/spec/mocks/project.rb @@ -0,0 +1,44 @@ +class Milestone < MockRecord + attr_accessor :name, :started_at, :finished_at, :due_at, :owner_names +end + +class Project < MockRecord + attr_accessor :name, :status + attr_writer :milestones, :reviews + + def milestones + @milestones ||= [] + end + + # Children that have no adapter of their own + def reviews + @reviews ||= [] + end +end + +Sunspot.setup(Project) do + string :name + string :status + + nested :milestones do + string :name + time :started_at + time :finished_at + time :due_at + string :owner_names, :multiple => true + integer(:days_late) { finished_at && due_at && finished_at > due_at ? ((finished_at - due_at) / 86_400).to_i : 0 } + end + + nested :reviews do + string :verdict + end +end + +# Another class sharing a field name with the children +class Memo < MockRecord + attr_accessor :name +end + +Sunspot.setup(Memo) do + string :name +end diff --git a/sunspot_solr/solr/solr/configsets/sunspot/conf/schema.xml b/sunspot_solr/solr/solr/configsets/sunspot/conf/schema.xml index 9777c1146..5f80559e3 100644 --- a/sunspot_solr/solr/solr/configsets/sunspot/conf/schema.xml +++ b/sunspot_solr/solr/solr/configsets/sunspot/conf/schema.xml @@ -129,6 +129,9 @@ + + + From 1052e25c464a20c2bd0e77eee9e1767d5b2c1151 Mon Sep 17 00:00:00 2001 From: Nicholas Jakobsen Date: Mon, 5 Oct 2026 03:13:45 -0700 Subject: [PATCH 02/11] test: Run the child-document specs only where RSolr can send child documents CI runs every spec under the `rsolr-1.x` and `rsolr-2.x` appraisals, and RSolr 1.x has no `RSolr::Document`, so the nested-document specs that index children raised `NestedDocumentsNotSupportedError` there: 25 failures, all in `spec/api/nested_documents_spec.rb`'s indexing block and `spec/integration/nested_documents_spec.rb`. Those two blocks now run only when `RSolr::Document::CHILD_DOCUMENT_KEY` is defined. Under RSolr 1.x, new examples check that indexing a parent with children raises `NestedDocumentsNotSupportedError` and that a parent with no children still indexes. The atomic-update example moves out of the indexing block, since it needs no children and runs under both. Co-Authored-By: Claude Opus 5.5 --- sunspot/spec/api/nested_documents_spec.rb | 22 +++++++++++++++++-- .../spec/integration/nested_documents_spec.rb | 3 ++- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/sunspot/spec/api/nested_documents_spec.rb b/sunspot/spec/api/nested_documents_spec.rb index 96257b094..b1a68d540 100644 --- a/sunspot/spec/api/nested_documents_spec.rb +++ b/sunspot/spec/api/nested_documents_spec.rb @@ -1,5 +1,8 @@ require File.expand_path('spec_helper', File.dirname(__FILE__)) +# RSolr 1.x has no RSolr::Document, so it can't send child documents +child_documents = defined?(RSolr::Document::CHILD_DOCUMENT_KEY) + describe 'nested documents' do let(:connection) { Mock::Connection.new } let(:session) { Sunspot::Session.new(Sunspot::Configuration.build, connection) } @@ -37,7 +40,7 @@ def last_fq end end - describe 'indexing' do + describe 'indexing', :if => child_documents do let(:milestone) { Milestone.new(:name => 'design', :started_at => Time.utc(2026, 2, 1), :owner_names => %w(ana ben)) } let(:project) { Project.new(:name => 'Bridge', :milestones => [milestone], :reviews => [OpenStruct.new(:verdict => 'approved')]) } @@ -88,8 +91,23 @@ def children(document) expect(children(connection.adds.last.first)).to be_empty end + end + + describe 'indexing with an RSolr that has no child documents', :unless => child_documents do + it 'raises for a parent with children' do + project = Project.new(:name => 'Bridge', :milestones => [Milestone.new(:name => 'design')]) + expect { session.index(project) }.to raise_error(Sunspot::NestedDocumentsNotSupportedError) + end + + it 'indexes a parent with no children' do + session.index(Project.new(:name => 'Empty')) + expect(connection.adds.last.first.field_by_name(:name_s).value).to eq('Empty') + end + end + + describe 'atomic updates' do it 'refuses atomic updates to a class with nested associations' do - expect { session.atomic_update(Project, project.id => { :name => 'x' }) }.to raise_error(ArgumentError, /nested documents/) + expect { session.atomic_update(Project, 1 => { :name => 'x' }) }.to raise_error(ArgumentError, /nested documents/) end end diff --git a/sunspot/spec/integration/nested_documents_spec.rb b/sunspot/spec/integration/nested_documents_spec.rb index fd9ea5825..4f5d4434b 100644 --- a/sunspot/spec/integration/nested_documents_spec.rb +++ b/sunspot/spec/integration/nested_documents_spec.rb @@ -1,6 +1,7 @@ require File.expand_path('../spec_helper', File.dirname(__FILE__)) -describe 'nested documents' do +# RSolr 1.x has no RSolr::Document, so it can't send child documents +describe 'nested documents', :if => defined?(RSolr::Document::CHILD_DOCUMENT_KEY) do def milestone(name, started_at, attrs = {}) Milestone.new({ :name => name, :started_at => started_at }.merge(attrs)) end From c4c17184b1a31066ac64e5efe4673d49cc0b4ccd Mon Sep 17 00:00:00 2001 From: Nicholas Jakobsen Date: Tue, 6 Oct 2026 01:41:51 -0700 Subject: [PATCH 03/11] fix: Remove and reindex nested blocks correctly, and match an association across classes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five problems a review of the nested documents found, each with a spec that failed before this change: - `Sunspot.remove!(Project) { with_child … }` deleted the children and left the parents on Solr 9. The children were deleted first, by a separate query, so by the time the parent query ran its child condition matched nothing. `remove_all(Class)` and `remove_by_scope` now send one delete query, `(query) OR _query_:"{!child …}"`, which Solr evaluates once, so parents and their children go together. - On Solr 6, reindexing a parent whose children went from some to none left the old children behind, and from none to some left two copies of the parent, doubling it in search results. Solr before 8 replaces a document with children by `_root_` and one without by `id`. `Indexer#add` now deletes each nested parent's previous block first: by id when it has children, by `_root_` when it has none. - Removing through a class with no nested associations, such as `remove_all(Asset)`, left the children of a nested subclass like `Vehicle < Asset`. The children clause is now added whenever the removed class, or a subclass of it, declares a nested association (`Setup.nested_under?`), and left off otherwise, so other classes' removal queries are unchanged. - A subclass that declared an association again marked its children with its own name, `SubProject.milestones`, so a `with_child` search on the superclass never found them. A redeclared association now keeps the superclass's `_sunspot_nested_path_s` value. - A search of several classes that each declare the same association matched only the first class's children. The child query now accepts the path of every searched class that declares it (`Setup#nested_paths`). `with(record)` and `without(record)` inside a `with_child` block restricted on the child document's id, which has the parent's id in front of it, so they silently matched nothing or everything. They now raise `ArgumentError`. `remove_by_id` for a class name that no longer resolves no longer raises `NameError`. The nested specs pass on Solr 6.6.6 and 9.10.1 with XML and JSON updates (57 examples), and under the `rsolr-1.x` appraisal (26). Outside them, the full suite fails only where upstream `master` does. Co-Authored-By: Claude Opus 5.5 --- sunspot/lib/sunspot/composite_setup.rb | 11 +++ sunspot/lib/sunspot/dsl/scope.rb | 5 +- sunspot/lib/sunspot/indexer.rb | 72 +++++++++++++------ sunspot/lib/sunspot/nested_setup.rb | 5 +- sunspot/lib/sunspot/query/block_join.rb | 18 ++--- sunspot/lib/sunspot/setup.rb | 31 +++++++- sunspot/spec/api/nested_documents_spec.rb | 46 +++++++++--- .../spec/integration/nested_documents_spec.rb | 56 +++++++++++++++ sunspot/spec/mocks/project.rb | 51 +++++++++++++ 9 files changed, 252 insertions(+), 43 deletions(-) diff --git a/sunspot/lib/sunspot/composite_setup.rb b/sunspot/lib/sunspot/composite_setup.rb index 5fb753402..0ba23ffa8 100644 --- a/sunspot/lib/sunspot/composite_setup.rb +++ b/sunspot/lib/sunspot/composite_setup.rb @@ -106,6 +106,17 @@ def dynamic_field_factory(field_name) # searched types that declares it. Raises UnrecognizedFieldError when none # does. # + def nested_paths(name) + paths = setups.map do |setup| + begin + setup.nested_setup(name).path + rescue UnrecognizedFieldError + nil + end + end.compact.uniq + paths.empty? ? [nested_setup(name).path] : paths + end + def nested_setup(name) setups.each do |setup| begin diff --git a/sunspot/lib/sunspot/dsl/scope.rb b/sunspot/lib/sunspot/dsl/scope.rb index f52f3c868..511ab7aaf 100644 --- a/sunspot/lib/sunspot/dsl/scope.rb +++ b/sunspot/lib/sunspot/dsl/scope.rb @@ -238,7 +238,7 @@ def text_fields(&block) def add_block_join(negated, name, &block) nested_setup = @setup.nested_setup(name) - block_join = Sunspot::Query::BlockJoin.new(nested_setup, negated) + block_join = Sunspot::Query::BlockJoin.new(nested_setup, negated, nil, @setup.nested_paths(name)) Util.instance_eval_or_call(Scope.new(block_join.scope, nested_setup), &block) if block @scope.add_component(block_join) end @@ -255,6 +255,9 @@ def add_restriction(negated, *args) DSL::Restriction.new(field, @scope, negated) end else # args are instances + if @setup.is_a?(NestedSetup) + raise ArgumentError, "Instance restrictions are not supported inside with_child or without_child; restrict on the child's fields instead" + end @scope.add_restriction( negated, IdField.instance, diff --git a/sunspot/lib/sunspot/indexer.rb b/sunspot/lib/sunspot/indexer.rb index 069fcd542..86f7dfc3f 100644 --- a/sunspot/lib/sunspot/indexer.rb +++ b/sunspot/lib/sunspot/indexer.rb @@ -22,7 +22,9 @@ def initialize(connection) # model:: the model to index # def add(model) - documents = Util.Array(model).map { |m| prepare_full_update(m) } + models = Util.Array(model) + documents = models.map { |m| prepare_full_update(m) } + remove_replaced_blocks(models, documents) add_batch_documents(documents) end @@ -72,7 +74,7 @@ def remove_by_id(class_name, *ids) ids.flatten! index_ids = ids.map { |id| Adapters::InstanceAdapter.index_id_for("#{id_prefix}#{class_name}", id) } @connection.delete_by_id(index_ids) - remove_blocks(index_ids) if nested?(Util.full_const_get(class_name)) + remove_blocks(index_ids) if nested_class_name?(class_name) end # @@ -80,12 +82,7 @@ def remove_by_id(class_name, *ids) # def remove_all(clazz = nil) if clazz - # Children are deleted first, because the query that finds them goes - # through their parents. - if nested?(clazz) - @connection.delete_by_query(children_of("type:#{Util.escape(clazz.name)}")) - end - @connection.delete_by_query("type:#{Util.escape(clazz.name)}") + @connection.delete_by_query(with_children("type:#{Util.escape(clazz.name)}", [clazz])) else @connection.delete_by_query("*:*") end @@ -95,12 +92,7 @@ def remove_all(clazz = nil) # Remove all documents that match the scope given in the Query # def remove_by_scope(scope, types = []) - # Children are deleted first, because the query that finds them goes - # through their parents. - if types.any? { |type| nested?(type) } - @connection.delete_by_query(children_of(scope.to_boolean_phrase)) - end - @connection.delete_by_query(scope.to_boolean_phrase) + @connection.delete_by_query(with_children(scope.to_boolean_phrase, types)) end # @@ -188,6 +180,38 @@ def nested?(clazz) !setup.nil? && setup.nested_setups.any? end + # Returns true when the named class has nested associations, or when it + # no longer exists and any class does, since its documents may have + # children. + def nested_class_name?(class_name) + nested?(Util.full_const_get(class_name)) + rescue NameError + Setup.nested_anywhere? + end + + # + # Deletes the blocks the given models' documents replace. Solr before 8 + # replaces a document with children by +_root_+ and one without by +id+, + # so reindexing a parent whose children went from some to none, or from + # none to some, would leave the old children or the old parent behind. + # Deleting a parent with children by id, and one without by +_root_+, + # clears either case. + # + def remove_replaced_blocks(models, documents) + with_children, without_children = [], [] + models.each_with_index do |model, i| + next unless nested?(model.class) + id = documents[i].field_by_name(:id).value + (child_documents?(documents[i]) ? with_children : without_children) << id + end + @connection.delete_by_id(with_children) if with_children.any? + remove_blocks(without_children) + end + + def child_documents?(document) + defined?(RSolr::Document::CHILD_DOCUMENT_KEY) && document.fields_by_name(RSolr::Document::CHILD_DOCUMENT_KEY).any? + end + # # Deletes every document in the blocks rooted at the given ids. Solr 9 # deletes a parent's children along with it on a delete by id, and Solr 6 @@ -200,13 +224,19 @@ def remove_blocks(ids) end # - # Returns a query matching the children of the parents that match - # +parent_query+. The block mask is every document that is not a child, so - # documents of other classes indexed between blocks are never matched as - # children. - # - def children_of(parent_query) - %Q({!child of="*:* -#{NestedSetup::PATH_FIELD}:[* TO *]"}#{parent_query}) + # Returns a query matching the documents +query+ matches and, when one of + # +classes+ or a subclass of one has nested associations, their children + # too. Solr evaluates it once, so a delete by it removes the parents and + # children together, and a parent condition that refers to children still + # matches the parents. The block mask is every document that is not a + # child, so documents of other classes indexed between blocks are never + # matched as children. + # + def with_children(query, classes) + return query unless Setup.nested_under?(classes) + escape = Query::BlockJoin.method(:escape) + children = %Q({!child of="#{escape.(Query::BlockJoin::ROOTS)}" v="#{escape.(query)}"}) + %Q((#{query}) OR _query_:"#{escape.(children)}") end def prepare_atomic_update(clazz, id, updates = {}) diff --git a/sunspot/lib/sunspot/nested_setup.rb b/sunspot/lib/sunspot/nested_setup.rb index 8d5ecb47e..136fe67f3 100644 --- a/sunspot/lib/sunspot/nested_setup.rb +++ b/sunspot/lib/sunspot/nested_setup.rb @@ -15,10 +15,11 @@ class NestedSetup < Setup #:nodoc: attr_reader :name, :parent_setup - def initialize(parent_setup, name, options = {}) + def initialize(parent_setup, name, options = {}, path = nil) @parent_setup = parent_setup @name = name.to_sym @class_name = "#{parent_setup.type_names.first}.#{@name}" + @path = path || @class_name @field_factories, @text_field_factories, @dynamic_field_factories, @field_factories_cache, @text_field_factories_cache, @dynamic_field_factories_cache = *Array.new(6) { Hash.new } @@ -35,7 +36,7 @@ def initialize(parent_setup, name, options = {}) # "Project.milestones". # def path - @class_name + @path end def children_for(model) diff --git a/sunspot/lib/sunspot/query/block_join.rb b/sunspot/lib/sunspot/query/block_join.rb index 295b80f90..6188e0c53 100644 --- a/sunspot/lib/sunspot/query/block_join.rb +++ b/sunspot/lib/sunspot/query/block_join.rb @@ -22,9 +22,9 @@ class BlockJoin #:nodoc: # negated ones included, is joined to that condition. attr_reader :scope - def initialize(nested_setup, negated = false, scope = nil) - @nested_setup, @negated = nested_setup, negated - @scope = scope || Connective::Conjunction.new.tap { |conjunction| conjunction.add_component(PathRestriction.new(nested_setup)) } + def initialize(nested_setup, negated = false, scope = nil, paths = [nested_setup.path]) + @nested_setup, @negated, @paths = nested_setup, negated, paths + @scope = scope || Connective::Conjunction.new.tap { |conjunction| conjunction.add_component(PathRestriction.new(paths)) } end def to_boolean_phrase @@ -37,7 +37,7 @@ def negated? end def negate - self.class.new(@nested_setup, !negated?, @scope) + self.class.new(@nested_setup, !negated?, @scope, @paths) end private @@ -64,14 +64,16 @@ def self.escape(value) value.gsub(/(["\\])/, '\\\\\1') end - # The condition matching the children of one nested association. + # The condition matching the children of a nested association, on any + # of the given PATH_FIELD values. class PathRestriction #:nodoc: - def initialize(nested_setup) - @nested_setup = nested_setup + def initialize(paths) + @paths = paths end def to_boolean_phrase - %Q(#{NestedSetup::PATH_FIELD}:"#{BlockJoin.escape(@nested_setup.path)}") + phrases = @paths.map { |path| %Q(#{NestedSetup::PATH_FIELD}:"#{BlockJoin.escape(path)}") } + phrases.one? ? phrases.first : "(#{phrases.join(' OR ')})" end def negated? diff --git a/sunspot/lib/sunspot/setup.rb b/sunspot/lib/sunspot/setup.rb index 7709e7ffc..baef9e45e 100644 --- a/sunspot/lib/sunspot/setup.rb +++ b/sunspot/lib/sunspot/setup.rb @@ -57,7 +57,8 @@ def add_join_field_factory(name, type, options = {}, &block) # added to. # def add_nested(name, options = {}, &block) - nested_setup = NestedSetup.new(self, name, options) + inherited = parent && parent.get_inheritable_hash(:nested_setups)[name.to_sym] + nested_setup = NestedSetup.new(self, name, options, inherited && inherited.path) nested_setup.setup(&block) if block @nested_setups[nested_setup.name] = nested_setup end @@ -78,6 +79,14 @@ def nested_setups collection_from_inheritable_hash(:nested_setups) end + # + # Returns the PATH_FIELD values of the given association's children: one + # here, one per searched type that declares it on a CompositeSetup. + # + def nested_paths(name) + [nested_setup(name).path] + end + # # Add field_factories for fulltext search # @@ -421,6 +430,25 @@ def for(clazz) #:nodoc: setups[clazz.name.to_sym] || self.for(clazz.superclass) if clazz end + # Returns true when any class's setup declares a nested association. + def nested_anywhere? #:nodoc: + setups.values.any? { |setup| setup.nested_setups.any? } + end + + # Returns true when one of the given classes, or a subclass of one, + # declares a nested association. + def nested_under?(classes) #:nodoc: + setups.values.any? do |setup| + next false if setup.nested_setups.empty? + clazz = begin + setup.clazz + rescue NameError + next false + end + classes.any? { |type| clazz <= type } + end + end + protected # @@ -454,6 +482,7 @@ def for!(clazz) #:nodoc: def setups @setups ||= {} end + end end end diff --git a/sunspot/spec/api/nested_documents_spec.rb b/sunspot/spec/api/nested_documents_spec.rb index b1a68d540..f25e5b540 100644 --- a/sunspot/spec/api/nested_documents_spec.rb +++ b/sunspot/spec/api/nested_documents_spec.rb @@ -86,6 +86,17 @@ def children(document) expect(indexed_project.field_by_name(:name_s).value).to eq('Bridge') end + it 'deletes the previous block by id before indexing a parent with children' do + indexed_project + expect(connection).to have_delete("Project #{project.id}") + end + + it 'deletes the previous block by _root_ before indexing a parent without children' do + empty = Project.new(:name => 'Empty') + session.index(empty) + expect(connection).to have_delete_by_query(%Q(_root_:("Project\\ #{empty.id}"))) + end + it 'sends no children for an empty association' do session.index(Project.new(:name => 'Empty')) expect(children(connection.adds.last.first)).to be_empty @@ -139,6 +150,12 @@ def children(document) expect(last_fq.last).to include('Project.milestones\\\\\\" AND -name_s:launch AND -name_s:design)') end + it 'rejects an instance restriction inside a child block' do + milestone = Milestone.new(:name => 'design') + expect { session.search(Project) { with_child(:milestones) { with(milestone) } } }.to raise_error(ArgumentError, /Instance restrictions/) + expect { session.search(Project) { with_child(:milestones) { without(milestone) } } }.to raise_error(ArgumentError, /Instance restrictions/) + end + it 'combines with parent restrictions as separate filters' do session.search(Project) do with :status, 'active' @@ -206,27 +223,36 @@ def children(document) expect(connection).to have_delete_by_query('_root_:("Project\\ 1" OR "Project\\ 2")') end - it 'removes children before parents when removing a class' do + it 'removes the parents and their children in one query when removing a class' do session.remove_all(Project) expect(connection.deletes_by_query).to eq([ - %Q({!child of="#{roots}"}type:Project), - 'type:Project' + %q{(type:Project) OR _query_:"{!child of=\"*:* -_sunspot_nested_path_s:[* TO *]\" v=\"type:Project\"}"} ]) end - it 'removes children before parents when removing by scope' do + it 'removes the parents and their children in one query when removing by scope' do session.remove(Project) { with :status, 'archived' } expect(connection.deletes_by_query).to eq([ - %Q({!child of="#{roots}"}(type:Project AND status_s:archived)), - '(type:Project AND status_s:archived)' + %q{((type:Project AND status_s:archived)) OR _query_:"{!child of=\"*:* -_sunspot_nested_path_s:[* TO *]\" v=\"(type:Project AND status_s:archived)\"}"} ]) end - it 'leaves classes without nested documents alone' do - post = Post.new - session.remove(post) + it "removes a nested subclass's children when removing a class without nested associations" do + session.remove_all(Asset) + expect(connection.deletes_by_query).to eq([ + %q{(type:Asset) OR _query_:"{!child of=\"*:* -_sunspot_nested_path_s:[* TO *]\" v=\"type:Asset\"}"} + ]) + end + + it 'removes a class with no nested associations anywhere beneath it by its plain query' do session.remove_all(Post) - expect(connection.deletes_by_query).to eq(['type:Post']) + session.remove(Post) { with :title, 'monkeys' } + expect(connection.deletes_by_query).to eq(['type:Post', '(type:Post AND title_ss:monkeys)']) + end + + it 'sends no block delete when removing a record of a class without nested associations' do + session.remove(Post.new) + expect(connection.deletes_by_query).to be_empty end end end diff --git a/sunspot/spec/integration/nested_documents_spec.rb b/sunspot/spec/integration/nested_documents_spec.rb index 4f5d4434b..3dbcab48b 100644 --- a/sunspot/spec/integration/nested_documents_spec.rb +++ b/sunspot/spec/integration/nested_documents_spec.rb @@ -124,6 +124,23 @@ def solr_count(query) expect(solr_count('_sunspot_nested_path_s:[* TO *]')).to eq(2) expect(search { with_child(:milestones) { with :owner_names, 'ben' } }).to eq([]) end + + it 'removes every child when a parent is reindexed with none' do + @designed_earlier.milestones = [] + Sunspot.index!(@designed_earlier) + + expect(solr_count(%Q(_root_:"Project #{@designed_earlier.id}" AND _sunspot_nested_path_s:[* TO *]))).to eq(0) + expect(solr_count(%Q(id:"Project #{@designed_earlier.id}"))).to eq(1) + end + + it 'replaces a parent that had no children when it is reindexed with some' do + @without_milestones.milestones = [milestone('launch', Time.utc(2026, 3, 1))] + Sunspot.index!(@without_milestones) + + expect(solr_count(%Q(id:"Project #{@without_milestones.id}"))).to eq(1) + expect(Sunspot.search(Project).hits.length).to eq(3) + expect(search { with_child(:milestones) { with :name, 'launch' } }).to eq([@without_milestones]) + end end describe 'removal' do @@ -148,6 +165,14 @@ def children expect(search {}).to match_array([@designed_in_q1, @without_milestones]) end + it 'removes the parents a child condition selects, with their children' do + Sunspot.remove!(Project) { with_child(:milestones) { with :owner_names, 'ben' } } + + expect(solr_count(%Q(_root_:"Project #{@designed_earlier.id}"))).to eq(0) + expect(children).to eq(1) + expect(search {}).to match_array([@designed_in_q1, @without_milestones]) + end + it 'removes every child when removing the class, and nothing else' do Sunspot.remove_all!(Project) expect(children).to eq(0) @@ -155,4 +180,35 @@ def children expect(Sunspot.search(Memo).results).to eq([@memo]) end end + + describe 'other classes with the same association' do + let(:t) { Time.utc(2026, 2, 1) } + + it 'finds a subclass that declares the association again through a search of its superclass' do + sub_project = SubProject.new(:name => 'sub', :milestones => [milestone('review', t)]) + Sunspot.index!(sub_project) + + expect(search { with_child(:milestones) { with :name, 'review' } }).to eq([sub_project]) + end + + it 'matches children of every searched class that declares the association' do + program = Program.new(:name => 'program', :milestones => [milestone('design', t)]) + Sunspot.index!(program) + + [[Project, Program], [Program, Project]].each do |types| + results = Sunspot.search(*types) { with_child(:milestones) { with :name, 'design' } }.results + expect(results).to match_array([@designed_in_q1, @designed_earlier, program]) + end + end + + it 'removes the children of a nested subclass when removing through its superclass' do + Sunspot.index!(Vehicle.new(:name => 'van', :parts => [milestone('wheel', t)])) + Sunspot.remove_all!(Asset) + expect(solr_count('_sunspot_nested_path_s:"Vehicle.parts"')).to eq(0) + + Sunspot.index!(Vehicle.new(:name => 'van', :parts => [milestone('wheel', t)])) + Sunspot.remove!(Asset) { with :name, 'van' } + expect(solr_count('_sunspot_nested_path_s:"Vehicle.parts"')).to eq(0) + end + end end diff --git a/sunspot/spec/mocks/project.rb b/sunspot/spec/mocks/project.rb index 66c4ccffb..1b3c6ee5a 100644 --- a/sunspot/spec/mocks/project.rb +++ b/sunspot/spec/mocks/project.rb @@ -42,3 +42,54 @@ class Memo < MockRecord Sunspot.setup(Memo) do string :name end + +# A subclass that declares the same association again +class SubProject < Project +end + +Sunspot.setup(SubProject) do + nested :milestones do + string :name + end +end + +# An unrelated class with an association of the same name +class Program < MockRecord + attr_accessor :name + attr_writer :milestones + + def milestones + @milestones ||= [] + end +end + +Sunspot.setup(Program) do + string :name + + nested :milestones do + string :name + end +end + +# A superclass with no nested associations, and a subclass that has one +class Asset < MockRecord + attr_accessor :name +end + +Sunspot.setup(Asset) do + string :name +end + +class Vehicle < Asset + attr_writer :parts + + def parts + @parts ||= [] + end +end + +Sunspot.setup(Vehicle) do + nested :parts do + string :name + end +end From 267683e830c9602e007c1697e429e1c87216f1f5 Mon Sep 17 00:00:00 2001 From: Nicholas Jakobsen Date: Tue, 6 Oct 2026 01:53:28 -0700 Subject: [PATCH 04/11] fix: Drop the pre-index block delete, and resolve child fields across every declaring class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A second review of the nested documents found four problems in the previous commit: - `Setup.nested_under?` only scanned registered setups for subclasses of the removed class, and never asked whether the class itself inherits a nested association. Removing a subclass with no setup of its own, the usual single-table-inheritance shape (`Spinoff < Project`), left its children behind on both `remove_all` and `remove_by_scope`. It now checks the classes themselves first. - `Indexer#add` deleted each nested parent's previous block in a separate request before adding it. Inside `Sunspot.batch` the delete went out at once and the add waited for the flush, so an exception in the batch, or any commit before the flush, dropped the record from the index. The deletes also cost an extra update request on every index of a nested class, and on Solr 8 and later, where Solr replaces the block itself, they bought nothing. The pre-delete is gone. The `nested` documentation now says that replacing a block whose children went from some to none or none to some needs Solr 8, and the two specs for it skip on earlier versions. - `remove_blocks` built one `_root_:(…)` query for every id, which fails past Solr's limit of 1024 boolean clauses. It now sends batches of 500. - Fields inside `with_child` resolved against the first searched class that declared the association, so a field only a later class declared raised `UnrecognizedFieldError`, a field two classes declared with different types silently used the first's Solr field, and results depended on the order of the classes. A child block now resolves fields through `CompositeNestedSetup`, built from every `NestedSetup` the search covers, `Setup#nested_setups_named`: each searched class's own and those of registered subclasses that declare the association again. A field resolves when every setup that declares it agrees on its Solr field, as `CompositeSetup` does for parent fields, and raises otherwise. `nested_class_name?` no longer swallows a `NoMethodError`. Nested specs: 59 on Solr 9.10.1 and 6.6.6 (2 skipped on 6.6.6), xml and json, 27 under `rsolr-1.x`. Co-Authored-By: Claude Opus 5.5 --- sunspot/lib/sunspot.rb | 2 +- sunspot/lib/sunspot/composite_nested_setup.rb | 50 +++++++++++++++++++ sunspot/lib/sunspot/composite_setup.rb | 22 +++++--- sunspot/lib/sunspot/dsl/fields.rb | 6 +++ sunspot/lib/sunspot/dsl/scope.rb | 9 ++-- sunspot/lib/sunspot/indexer.rb | 39 ++++----------- sunspot/lib/sunspot/setup.rb | 34 ++++++++++--- sunspot/spec/api/nested_documents_spec.rb | 16 ++---- .../spec/integration/nested_documents_spec.rb | 32 ++++++++++++ sunspot/spec/mocks/project.rb | 22 ++++++++ 10 files changed, 173 insertions(+), 59 deletions(-) create mode 100644 sunspot/lib/sunspot/composite_nested_setup.rb diff --git a/sunspot/lib/sunspot.rb b/sunspot/lib/sunspot.rb index 294653086..6581112bb 100644 --- a/sunspot/lib/sunspot.rb +++ b/sunspot/lib/sunspot.rb @@ -12,7 +12,7 @@ require File.join(File.dirname(__FILE__), 'light_config') -%w(util adapters configuration setup nested_setup composite_setup text_field_setup field +%w(util adapters configuration setup nested_setup composite_setup composite_nested_setup text_field_setup field field_factory data_extractor indexer query search session session_proxy type dsl class_set).each do |filename| require File.join(File.dirname(__FILE__), 'sunspot', filename) diff --git a/sunspot/lib/sunspot/composite_nested_setup.rb b/sunspot/lib/sunspot/composite_nested_setup.rb new file mode 100644 index 000000000..ddacda89e --- /dev/null +++ b/sunspot/lib/sunspot/composite_nested_setup.rb @@ -0,0 +1,50 @@ +module Sunspot + # + # The fields of one association's children across several NestedSetups: + # one per searched class, and one per subclass that declares the association + # again. A field resolves when every setup that declares it agrees on its + # Solr field, as with CompositeSetup, and raises UnrecognizedFieldError when + # none declares it or they declare it differently. + # + class CompositeNestedSetup #:nodoc: + def initialize(nested_setups) + @nested_setups = nested_setups + end + + def field(field_name) + fields = @nested_setups.map do |setup| + begin + setup.field(field_name) + rescue UnrecognizedFieldError + nil + end + end.compact.uniq(&:indexed_name) + return fields.first if fields.one? + + raise( + UnrecognizedFieldError, + fields.empty? ? "No field configured for #{paths} with name '#{field_name}'" : + "Field '#{field_name}' is configured differently for #{paths}" + ) + end + + def dynamic_field_factory(field_name) + factories = @nested_setups.map { |setup| setup.dynamic_field_factory(field_name) } + return factories.first if factories.map { |factory| factory.build('x').indexed_name }.uniq.one? + + raise UnrecognizedFieldError, "Dynamic field '#{field_name}' is configured differently for #{paths}" + end + + def nested_setups_named(name) + raise UnrecognizedFieldError, "No nested association configured for #{paths} with name '#{name}'" + end + + alias_method :nested_setup, :nested_setups_named + + private + + def paths + @nested_setups.map(&:path).uniq * ', ' + end + end +end diff --git a/sunspot/lib/sunspot/composite_setup.rb b/sunspot/lib/sunspot/composite_setup.rb index 0ba23ffa8..d8aa4bd81 100644 --- a/sunspot/lib/sunspot/composite_setup.rb +++ b/sunspot/lib/sunspot/composite_setup.rb @@ -102,21 +102,27 @@ def dynamic_field_factory(field_name) end # - # Returns the NestedSetup for the given association from the first of the - # searched types that declares it. Raises UnrecognizedFieldError when none + # Returns every NestedSetup a search of the enclosed types has to cover for + # the given association, from each type that declares it and from their + # subclasses that declare it again. Raises UnrecognizedFieldError when none # does. # - def nested_paths(name) - paths = setups.map do |setup| + def nested_setups_named(name) + nested_setups = setups.flat_map do |setup| begin - setup.nested_setup(name).path + setup.nested_setups_named(name) rescue UnrecognizedFieldError - nil + [] end - end.compact.uniq - paths.empty? ? [nested_setup(name).path] : paths + end.uniq + nested_setups.empty? ? [nested_setup(name)] : nested_setups end + # + # Returns the NestedSetup for the given association from the first of the + # searched types that declares it. Raises UnrecognizedFieldError when none + # does. + # def nested_setup(name) setups.each do |setup| begin diff --git a/sunspot/lib/sunspot/dsl/fields.rb b/sunspot/lib/sunspot/dsl/fields.rb index 4b67c8823..dfaa3078d 100644 --- a/sunspot/lib/sunspot/dsl/fields.rb +++ b/sunspot/lib/sunspot/dsl/fields.rb @@ -82,6 +82,12 @@ def id_prefix(attr_name = nil, &block) # be reindexed whenever its children change. A class with nested # associations cannot be atomically updated. # + # Reindexing a parent replaces its old block only on Solr 8 or later. + # Solr before 8 replaces a document with children by +_root_+ and one + # without by +id+, so a parent whose children went from some to none + # keeps its old children, and one whose children went from none to some + # is indexed twice. Remove such a parent before reindexing it there. + # # The block cannot declare a document boost, an id prefix, a join, or a # nested association of its own. Each raises ArgumentError. # diff --git a/sunspot/lib/sunspot/dsl/scope.rb b/sunspot/lib/sunspot/dsl/scope.rb index 511ab7aaf..3732a51d6 100644 --- a/sunspot/lib/sunspot/dsl/scope.rb +++ b/sunspot/lib/sunspot/dsl/scope.rb @@ -237,9 +237,10 @@ def text_fields(&block) private def add_block_join(negated, name, &block) - nested_setup = @setup.nested_setup(name) - block_join = Sunspot::Query::BlockJoin.new(nested_setup, negated, nil, @setup.nested_paths(name)) - Util.instance_eval_or_call(Scope.new(block_join.scope, nested_setup), &block) if block + nested_setups = @setup.nested_setups_named(name) + child_setup = nested_setups.one? ? nested_setups.first : CompositeNestedSetup.new(nested_setups) + block_join = Sunspot::Query::BlockJoin.new(nested_setups.first, negated, nil, nested_setups.map(&:path).uniq) + Util.instance_eval_or_call(Scope.new(block_join.scope, child_setup), &block) if block @scope.add_component(block_join) end @@ -255,7 +256,7 @@ def add_restriction(negated, *args) DSL::Restriction.new(field, @scope, negated) end else # args are instances - if @setup.is_a?(NestedSetup) + if @setup.is_a?(NestedSetup) || @setup.is_a?(CompositeNestedSetup) raise ArgumentError, "Instance restrictions are not supported inside with_child or without_child; restrict on the child's fields instead" end @scope.add_restriction( diff --git a/sunspot/lib/sunspot/indexer.rb b/sunspot/lib/sunspot/indexer.rb index 86f7dfc3f..d1d5e78fb 100644 --- a/sunspot/lib/sunspot/indexer.rb +++ b/sunspot/lib/sunspot/indexer.rb @@ -22,9 +22,7 @@ def initialize(connection) # model:: the model to index # def add(model) - models = Util.Array(model) - documents = models.map { |m| prepare_full_update(m) } - remove_replaced_blocks(models, documents) + documents = Util.Array(model).map { |m| prepare_full_update(m) } add_batch_documents(documents) end @@ -46,6 +44,10 @@ def add_atomic_update(clazz, updates={}) # # Remove the given model from the Solr index # + # REMOVE_BLOCKS_BATCH_SIZE keeps each +_root_+ delete query under Solr's + # default limit of 1024 boolean clauses. + REMOVE_BLOCKS_BATCH_SIZE = 500 + def remove(*models) ids = models.map { |model| Adapters::InstanceAdapter.adapt(model).index_id } @connection.delete_by_id(ids) @@ -185,33 +187,11 @@ def nested?(clazz) # children. def nested_class_name?(class_name) nested?(Util.full_const_get(class_name)) - rescue NameError + rescue NameError => e + raise if e.is_a?(NoMethodError) Setup.nested_anywhere? end - # - # Deletes the blocks the given models' documents replace. Solr before 8 - # replaces a document with children by +_root_+ and one without by +id+, - # so reindexing a parent whose children went from some to none, or from - # none to some, would leave the old children or the old parent behind. - # Deleting a parent with children by id, and one without by +_root_+, - # clears either case. - # - def remove_replaced_blocks(models, documents) - with_children, without_children = [], [] - models.each_with_index do |model, i| - next unless nested?(model.class) - id = documents[i].field_by_name(:id).value - (child_documents?(documents[i]) ? with_children : without_children) << id - end - @connection.delete_by_id(with_children) if with_children.any? - remove_blocks(without_children) - end - - def child_documents?(document) - defined?(RSolr::Document::CHILD_DOCUMENT_KEY) && document.fields_by_name(RSolr::Document::CHILD_DOCUMENT_KEY).any? - end - # # Deletes every document in the blocks rooted at the given ids. Solr 9 # deletes a parent's children along with it on a delete by id, and Solr 6 @@ -219,8 +199,9 @@ def child_documents?(document) # parent follows it in the index. # def remove_blocks(ids) - return if ids.empty? - @connection.delete_by_query("_root_:(#{ids.map { |id| %Q("#{Util.escape(id)}") }.join(' OR ')})") + ids.each_slice(REMOVE_BLOCKS_BATCH_SIZE) do |batch| + @connection.delete_by_query("_root_:(#{batch.map { |id| %Q("#{Util.escape(id)}") }.join(' OR ')})") + end end # diff --git a/sunspot/lib/sunspot/setup.rb b/sunspot/lib/sunspot/setup.rb index baef9e45e..a73efa8f5 100644 --- a/sunspot/lib/sunspot/setup.rb +++ b/sunspot/lib/sunspot/setup.rb @@ -80,11 +80,28 @@ def nested_setups end # - # Returns the PATH_FIELD values of the given association's children: one - # here, one per searched type that declares it on a CompositeSetup. - # - def nested_paths(name) - [nested_setup(name).path] + # Returns every NestedSetup a search of this class has to cover for the + # given association: its own, inherited if need be, and those of + # registered subclasses that declare the association again. Raises + # UnrecognizedFieldError when this class has no such association. + # + def nested_setups_named(name) + own = nested_setup(name) + redeclared = Setup.all.map do |setup| + next if setup.equal?(self) + subclass = begin + setup.clazz <= clazz + rescue NameError + false + end + setup.declared_nested_setup(name) if subclass + end + [own, *redeclared.compact].uniq + end + + # Returns the NestedSetup declared on this class itself, not inherited. + def declared_nested_setup(name) + @nested_setups[name.to_sym] end # @@ -430,6 +447,11 @@ def for(clazz) #:nodoc: setups[clazz.name.to_sym] || self.for(clazz.superclass) if clazz end + # Returns every class's setup. + def all #:nodoc: + setups.values + end + # Returns true when any class's setup declares a nested association. def nested_anywhere? #:nodoc: setups.values.any? { |setup| setup.nested_setups.any? } @@ -438,6 +460,7 @@ def nested_anywhere? #:nodoc: # Returns true when one of the given classes, or a subclass of one, # declares a nested association. def nested_under?(classes) #:nodoc: + return true if classes.any? { |type| (setup = self.for(type)) && setup.nested_setups.any? } setups.values.any? do |setup| next false if setup.nested_setups.empty? clazz = begin @@ -482,7 +505,6 @@ def for!(clazz) #:nodoc: def setups @setups ||= {} end - end end end diff --git a/sunspot/spec/api/nested_documents_spec.rb b/sunspot/spec/api/nested_documents_spec.rb index f25e5b540..9731f4765 100644 --- a/sunspot/spec/api/nested_documents_spec.rb +++ b/sunspot/spec/api/nested_documents_spec.rb @@ -86,17 +86,6 @@ def children(document) expect(indexed_project.field_by_name(:name_s).value).to eq('Bridge') end - it 'deletes the previous block by id before indexing a parent with children' do - indexed_project - expect(connection).to have_delete("Project #{project.id}") - end - - it 'deletes the previous block by _root_ before indexing a parent without children' do - empty = Project.new(:name => 'Empty') - session.index(empty) - expect(connection).to have_delete_by_query(%Q(_root_:("Project\\ #{empty.id}"))) - end - it 'sends no children for an empty association' do session.index(Project.new(:name => 'Empty')) expect(children(connection.adds.last.first)).to be_empty @@ -217,6 +206,11 @@ def children(document) expect(connection).to have_delete_by_query(%Q(_root_:("Project\\ #{project.id}"))) end + it 'splits the block deletes to stay under the boolean clause limit' do + session.remove_by_id(Project, (1..1100).to_a) + expect(connection.deletes_by_query.length).to eq(3) + end + it 'removes the whole block when removing by id' do session.remove_by_id(Project, 1, 2) expect(connection).to have_delete('Project 1', 'Project 2') diff --git a/sunspot/spec/integration/nested_documents_spec.rb b/sunspot/spec/integration/nested_documents_spec.rb index 3dbcab48b..62180854f 100644 --- a/sunspot/spec/integration/nested_documents_spec.rb +++ b/sunspot/spec/integration/nested_documents_spec.rb @@ -17,6 +17,12 @@ def solr_count(query) let(:q1) { Time.utc(2026, 1, 1)...Time.utc(2026, 4, 1) } + # Solr before 8 replaces a document with children by _root_ and one without by id + def skip_before_solr_8 + version = Sunspot.session.send(:connection).get('admin/system')['lucene']['solr-spec-version'] + skip "Solr #{version} doesn't replace a block whose children went from some to none or none to some" if version.to_i < 8 + end + before :each do Sunspot.remove_all! @@ -126,6 +132,7 @@ def solr_count(query) end it 'removes every child when a parent is reindexed with none' do + skip_before_solr_8 @designed_earlier.milestones = [] Sunspot.index!(@designed_earlier) @@ -134,6 +141,7 @@ def solr_count(query) end it 'replaces a parent that had no children when it is reindexed with some' do + skip_before_solr_8 @without_milestones.milestones = [milestone('launch', Time.utc(2026, 3, 1))] Sunspot.index!(@without_milestones) @@ -173,6 +181,16 @@ def children expect(search {}).to match_array([@designed_in_q1, @without_milestones]) end + it 'removes the children of a subclass that has no setup of its own' do + Sunspot.index!(Spinoff.new(:name => 'spun off', :milestones => [milestone('design', Time.utc(2026, 2, 1))])) + Sunspot.remove_all!(Spinoff) + expect(solr_count('id:Spinoff*')).to eq(0) + + Sunspot.index!(Spinoff.new(:name => 'spun off', :milestones => [milestone('design', Time.utc(2026, 2, 1))])) + Sunspot.remove!(Spinoff) { with :name, 'spun off' } + expect(solr_count('id:Spinoff*')).to eq(0) + end + it 'removes every child when removing the class, and nothing else' do Sunspot.remove_all!(Project) expect(children).to eq(0) @@ -201,6 +219,20 @@ def children end end + it 'resolves a child field declared by any searched class, whatever their order' do + gadget = Gadget.new(:name => 'gadget', :milestones => [milestone('design', Time.utc(2026, 2, 1))]) + Sunspot.index!(gadget) + + [[Project, Gadget], [Gadget, Project]].each do |types| + expect(Sunspot.search(*types) { with_child(:milestones) { with :only_here, 'yes' } }.results).to eq([gadget]) + expect(Sunspot.search(*types) { with_child(:milestones) { with :name, 'design' } }.results).to match_array([@designed_in_q1, @designed_earlier]) + end + end + + it 'rejects a child field the searched classes declare differently' do + expect { Sunspot.search(Project, Gadget) { with_child(:milestones) { with(:started_at).greater_than(0) } } }.to raise_error(Sunspot::UnrecognizedFieldError) + end + it 'removes the children of a nested subclass when removing through its superclass' do Sunspot.index!(Vehicle.new(:name => 'van', :parts => [milestone('wheel', t)])) Sunspot.remove_all!(Asset) diff --git a/sunspot/spec/mocks/project.rb b/sunspot/spec/mocks/project.rb index 1b3c6ee5a..a7bb54b05 100644 --- a/sunspot/spec/mocks/project.rb +++ b/sunspot/spec/mocks/project.rb @@ -93,3 +93,25 @@ def parts string :name end end + +# A subclass with no setup of its own, which inherits Project's +class Spinoff < Project +end + +# An unrelated class whose milestones declare a field Project's don't, and +# one Project's declare with another type +class Gadget < MockRecord + attr_accessor :name + attr_writer :milestones + + def milestones + @milestones ||= [] + end +end + +Sunspot.setup(Gadget) do + nested :milestones do + string(:only_here) { 'yes' } + integer(:started_at) { 1 } + end +end From 4ddf770118dacf5891b8c63d3bff7edf858893eb Mon Sep 17 00:00:00 2001 From: Nicholas Jakobsen Date: Tue, 6 Oct 2026 02:00:29 -0700 Subject: [PATCH 05/11] fix: Resolve a class's child fields from its own nested setup, as parent fields resolve MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Setup#nested_setups_named` added the redeclared nested setup of every registered subclass, so as soon as one subclass declared an association again, a search of the superclass alone resolved child fields through `CompositeNestedSetup`. With `Crate` declaring `nested :items { time :at; text :body; dynamic_string :attrs }` and `SubCrate < Crate` redeclaring `at` as a string, `with(:at)` on a `Crate` search raised "configured differently", `text_fields { … }` raised `NoMethodError` because the composite had no `text_fields`, and `dynamic(:attrs)` raised because the subclass didn't declare it. Which subclasses were registered depended on what had been loaded, so the same query could work in one process and raise in another. A class's search now uses only its own nested setup, inherited if need be, the way a superclass search never consults subclass setups for parent fields. A redeclared association already keeps its superclass's path, so the subclass's children still match. `CompositeNestedSetup` is now used only for a search of several classes, and gains `text_fields` and `type_names`, which `TextFieldSetup` calls. Its field rule now compares the Solr field name and type, so a `time` and a `date` field with the same name are no longer taken for one. `Setup.all` and `Setup#declared_nested_setup` go with the scan, which also cost a pass over every registered setup on each `with_child`. Co-Authored-By: Claude Opus 5.5 --- sunspot/lib/sunspot/composite_nested_setup.rb | 33 +++++++++++++++---- sunspot/lib/sunspot/composite_setup.rb | 6 ++-- sunspot/lib/sunspot/indexer.rb | 6 ++-- sunspot/lib/sunspot/setup.rb | 28 +++------------- .../spec/integration/nested_documents_spec.rb | 16 +++++++++ sunspot/spec/mocks/project.rb | 31 +++++++++++++++++ 6 files changed, 82 insertions(+), 38 deletions(-) diff --git a/sunspot/lib/sunspot/composite_nested_setup.rb b/sunspot/lib/sunspot/composite_nested_setup.rb index ddacda89e..605154c46 100644 --- a/sunspot/lib/sunspot/composite_nested_setup.rb +++ b/sunspot/lib/sunspot/composite_nested_setup.rb @@ -1,10 +1,10 @@ module Sunspot # - # The fields of one association's children across several NestedSetups: - # one per searched class, and one per subclass that declares the association - # again. A field resolves when every setup that declares it agrees on its - # Solr field, as with CompositeSetup, and raises UnrecognizedFieldError when - # none declares it or they declare it differently. + # The fields of one association's children across the NestedSetups of the + # classes in a multi-class search. A field resolves when every setup that + # declares it agrees on its Solr field name and type, as with CompositeSetup, + # and raises UnrecognizedFieldError when none declares it or they declare it + # differently. # class CompositeNestedSetup #:nodoc: def initialize(nested_setups) @@ -18,7 +18,7 @@ def field(field_name) rescue UnrecognizedFieldError nil end - end.compact.uniq(&:indexed_name) + end.compact.uniq { |field| [field.indexed_name, field.type.class] } return fields.first if fields.one? raise( @@ -28,6 +28,25 @@ def field(field_name) ) end + # Returns the text fields with the given name, one per distinct Solr field. + # TextFieldSetup raises when there is more than one. + def text_fields(field_name) + fields = @nested_setups.flat_map do |setup| + begin + setup.text_fields(field_name) + rescue UnrecognizedFieldError + [] + end + end.uniq(&:indexed_name) + return fields if fields.any? + + raise UnrecognizedFieldError, "No text field configured for #{paths} with name '#{field_name}'" + end + + def type_names + @nested_setups.map(&:path).uniq + end + def dynamic_field_factory(field_name) factories = @nested_setups.map { |setup| setup.dynamic_field_factory(field_name) } return factories.first if factories.map { |factory| factory.build('x').indexed_name }.uniq.one? @@ -44,7 +63,7 @@ def nested_setups_named(name) private def paths - @nested_setups.map(&:path).uniq * ', ' + type_names * ', ' end end end diff --git a/sunspot/lib/sunspot/composite_setup.rb b/sunspot/lib/sunspot/composite_setup.rb index d8aa4bd81..8dd8a8f03 100644 --- a/sunspot/lib/sunspot/composite_setup.rb +++ b/sunspot/lib/sunspot/composite_setup.rb @@ -102,10 +102,8 @@ def dynamic_field_factory(field_name) end # - # Returns every NestedSetup a search of the enclosed types has to cover for - # the given association, from each type that declares it and from their - # subclasses that declare it again. Raises UnrecognizedFieldError when none - # does. + # Returns the NestedSetup of each enclosed type that declares the given + # association. Raises UnrecognizedFieldError when none does. # def nested_setups_named(name) nested_setups = setups.flat_map do |setup| diff --git a/sunspot/lib/sunspot/indexer.rb b/sunspot/lib/sunspot/indexer.rb index d1d5e78fb..c388f011b 100644 --- a/sunspot/lib/sunspot/indexer.rb +++ b/sunspot/lib/sunspot/indexer.rb @@ -41,13 +41,13 @@ def add_atomic_update(clazz, updates={}) add_batch_documents(documents) end - # - # Remove the given model from the Solr index - # # REMOVE_BLOCKS_BATCH_SIZE keeps each +_root_+ delete query under Solr's # default limit of 1024 boolean clauses. REMOVE_BLOCKS_BATCH_SIZE = 500 + # + # Remove the given model from the Solr index + # def remove(*models) ids = models.map { |model| Adapters::InstanceAdapter.adapt(model).index_id } @connection.delete_by_id(ids) diff --git a/sunspot/lib/sunspot/setup.rb b/sunspot/lib/sunspot/setup.rb index a73efa8f5..860b9cd44 100644 --- a/sunspot/lib/sunspot/setup.rb +++ b/sunspot/lib/sunspot/setup.rb @@ -80,28 +80,13 @@ def nested_setups end # - # Returns every NestedSetup a search of this class has to cover for the - # given association: its own, inherited if need be, and those of - # registered subclasses that declare the association again. Raises + # Returns the NestedSetups a search of this class covers for the given + # association: its own, inherited if need be. A subclass that declares the + # association again keeps its path, so its children match too. Raises # UnrecognizedFieldError when this class has no such association. # def nested_setups_named(name) - own = nested_setup(name) - redeclared = Setup.all.map do |setup| - next if setup.equal?(self) - subclass = begin - setup.clazz <= clazz - rescue NameError - false - end - setup.declared_nested_setup(name) if subclass - end - [own, *redeclared.compact].uniq - end - - # Returns the NestedSetup declared on this class itself, not inherited. - def declared_nested_setup(name) - @nested_setups[name.to_sym] + [nested_setup(name)] end # @@ -447,11 +432,6 @@ def for(clazz) #:nodoc: setups[clazz.name.to_sym] || self.for(clazz.superclass) if clazz end - # Returns every class's setup. - def all #:nodoc: - setups.values - end - # Returns true when any class's setup declares a nested association. def nested_anywhere? #:nodoc: setups.values.any? { |setup| setup.nested_setups.any? } diff --git a/sunspot/spec/integration/nested_documents_spec.rb b/sunspot/spec/integration/nested_documents_spec.rb index 62180854f..29c87a58b 100644 --- a/sunspot/spec/integration/nested_documents_spec.rb +++ b/sunspot/spec/integration/nested_documents_spec.rb @@ -229,6 +229,22 @@ def children end end + it 'searches text fields on the children of several classes' do + gadget = Gadget.new(:name => 'gadget', :milestones => [milestone('design', Time.utc(2026, 2, 1))]) + Sunspot.index!(gadget) + + expect(Sunspot.search(Project, Gadget) { with_child(:milestones) { text_fields { with :notes, 'gadget notes' } } }.results).to eq([gadget]) + end + + it "resolves a superclass's child fields from its own setup when a subclass declares the association again" do + crate = Crate.new(:name => 'crate', :items => [OpenStruct.new(:at => Time.utc(2026, 2, 1), :body => 'design', :attrs => { :color => 'red' })]) + Sunspot.index!(crate) + + expect(Sunspot.search(Crate) { with_child(:items) { with(:at).greater_than(Time.utc(2026, 1, 1)) } }.results).to eq([crate]) + expect(Sunspot.search(Crate) { with_child(:items) { text_fields { with :body, 'design' } } }.results).to eq([crate]) + expect(Sunspot.search(Crate) { with_child(:items) { dynamic(:attrs) { with :color, 'red' } } }.results).to eq([crate]) + end + it 'rejects a child field the searched classes declare differently' do expect { Sunspot.search(Project, Gadget) { with_child(:milestones) { with(:started_at).greater_than(0) } } }.to raise_error(Sunspot::UnrecognizedFieldError) end diff --git a/sunspot/spec/mocks/project.rb b/sunspot/spec/mocks/project.rb index a7bb54b05..092b5b199 100644 --- a/sunspot/spec/mocks/project.rb +++ b/sunspot/spec/mocks/project.rb @@ -113,5 +113,36 @@ def milestones nested :milestones do string(:only_here) { 'yes' } integer(:started_at) { 1 } + text(:notes) { 'gadget notes' } + end +end + +# Child fields of every kind, and a subclass that declares the association +# again with one of them typed differently +class Crate < MockRecord + attr_accessor :name + attr_writer :items + + def items + @items ||= [] + end +end + +class SubCrate < Crate +end + +Sunspot.setup(Crate) do + string :name + + nested :items do + time :at + text :body + dynamic_string :attrs + end +end + +Sunspot.setup(SubCrate) do + nested :items do + string(:at) { 'x' } end end From a15caa257a3a744ae490e21713e6d346439edd3d Mon Sep 17 00:00:00 2001 From: Nicholas Jakobsen Date: Tue, 6 Oct 2026 02:03:35 -0700 Subject: [PATCH 06/11] docs: Say that redeclaring a nested association in a subclass replaces its fields A subclass's `nested` block replaces the association's fields for its children rather than adding to them, unlike parent-level setups, which add to what they inherit. A subclass that redeclared `nested :leaves` with only a new field indexed its children without the superclass's fields, so a superclass search on one of those fields silently skipped them. The `nested` documentation now says so. Co-Authored-By: Claude Opus 5.5 --- sunspot/lib/sunspot/dsl/fields.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sunspot/lib/sunspot/dsl/fields.rb b/sunspot/lib/sunspot/dsl/fields.rb index dfaa3078d..36231fe08 100644 --- a/sunspot/lib/sunspot/dsl/fields.rb +++ b/sunspot/lib/sunspot/dsl/fields.rb @@ -88,6 +88,12 @@ def id_prefix(attr_name = nil, &block) # keeps its old children, and one whose children went from none to some # is indexed twice. Remove such a parent before reindexing it there. # + # A subclass that declares an association again replaces its fields + # rather than adding to them: its children are indexed with only the + # fields of its own block. They keep the superclass's association + # marker, so a search of the superclass still reaches them, but only + # through fields both blocks declare the same way. + # # The block cannot declare a document boost, an id prefix, a join, or a # nested association of its own. Each raises ArgumentError. # From 85492558b6de1c2577e6ea36af4e2d8a1cec277c Mon Sep 17 00:00:00 2001 From: Nicholas Jakobsen Date: Tue, 6 Oct 2026 18:05:17 -0700 Subject: [PATCH 07/11] docs: Document nested documents and block joins in the README Adds a "Nested Documents (Block Joins)" section after "Joins": declaring an association with `nested`, searching with `with_child` and `without_child`, the `_root_` schema field and the full reindex that adding it to an existing index needs on Solr 8 and above, and the limits the code enforces or the specs record (Solr 8 for replacing a block on reindex, atomic updates, what a nested block rejects, subclass redeclaration, child ids). Co-Authored-By: Claude Opus 5.5 --- README.md | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/README.md b/README.md index f7ea3e2c8..44d7ea52d 100644 --- a/README.md +++ b/README.md @@ -1028,6 +1028,70 @@ end # qRss91753840: _query_:"{!field f=type}Rss"+_query_:"{!edismax qf='keywords_text'}keyword3" ``` +### Nested Documents (Block Joins) + +**Solr 8 and above recommended; requires RSolr 2** + +Nested documents let a search require that a *single* associated record meet several conditions together. Indexing an association's fields as multivalued fields on the parent loses which value came from which record: a project with one milestone named "design" and another started in Q1 would match "a design milestone started in Q1". `nested` indexes each record of an association as a Solr child document inside its parent's block, and `with_child` / `without_child` search them with Solr's [block join query parsers](https://solr.apache.org/guide/solr/latest/query-guide/block-join-query-parser.html). + +```ruby +class Project < ActiveRecord::Base + has_many :milestones + + searchable do + string :status + + nested :milestones do + string :name + time :started_at + string(:owner_names, :multiple => true) { owners.map(&:name) } + end + end +end + +# Projects with a design milestone started in Q1. One milestone has to match +# both conditions. +Project.search do + with_child :milestones do + with :name, 'design' + with(:started_at).between(Time.utc(2026, 1, 1)...Time.utc(2026, 4, 1)) + end +end + +# Projects with no design milestone, including projects with no milestones +Project.search do + without_child :milestones do + with :name, 'design' + end +end + +# Projects with any milestone at all +Project.search { with_child :milestones } +``` + +The block inside `with_child` / `without_child` takes the same restrictions as a search, including `without`, `any_of`, `all_of`, `dynamic` and `text_fields`, with field names referring to the children's fields. `with_child` combines with restrictions on the parent and works inside `any_of`, query facets and `remove`. + +Searching several classes covers each class that declares the association, and a child field resolves as parent fields do: when every class declaring it agrees on its type. + +#### Schema + +The schema needs a `_root_` field with the same type as `id`. The bundled configset has one. + +```xml + +``` + +On Solr 8 and above, adding `_root_` to an index that already holds documents changes how Solr replaces them: documents indexed before the field existed are not replaced when reindexed, so each one, of any class, is indexed twice. Reindex every class from scratch (`rake sunspot:reindex`) right after adding the field. + +#### Things to know + +* Children are indexed only as part of their parent, so reindex the parent whenever its children change. +* Reindexing a parent replaces its whole block on Solr 8 and above. On earlier versions, including the Solr that `sunspot_solr` bundles, a parent whose children went from some to none keeps its old children, and one whose children went from none to some is indexed twice; remove it before reindexing it there. +* Atomic updates raise `ArgumentError` for a class with nested associations. Index the whole record instead. +* A nested block can't declare a boost, an id prefix, a join or a nested association of its own, and `with(record)` / `without(record)` inside `with_child` raise `ArgumentError`. +* A subclass that declares the association again replaces its fields rather than adding to them. A search of the superclass still reaches its children, but only through fields both declare the same way. +* A child document's id is `"//"`, where the child id is its Sunspot index id, or its position in the association when its class has no Sunspot adapter. Children carry a `_sunspot_nested_path_s` field naming their association and no `type`, so searches for the parent class never return them. + ### Composite ID **SolrCloud only** From 162767d9db520d88f2047f1db9d6601f407ae3b8 Mon Sep 17 00:00:00 2001 From: Nicholas Jakobsen Date: Tue, 6 Oct 2026 18:25:07 -0700 Subject: [PATCH 08/11] docs: Explain why adding _root_ to an existing index leaves old copies behind The README said documents indexed before `_root_` existed are "indexed twice", which read as if they had never been indexed. It now says what happens: Solr 8 and above replace a document by its `_root_` value once the schema has it, documents indexed earlier have none, so a reindex adds a new copy beside the old one and later reindexes replace only the new copy, and the old copies stay until `rake sunspot:reindex` deletes each class's documents. Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 44d7ea52d..53addc8b2 100644 --- a/README.md +++ b/README.md @@ -1081,7 +1081,7 @@ The schema needs a `_root_` field with the same type as `id`. The bundled config ``` -On Solr 8 and above, adding `_root_` to an index that already holds documents changes how Solr replaces them: documents indexed before the field existed are not replaced when reindexed, so each one, of any class, is indexed twice. Reindex every class from scratch (`rake sunspot:reindex`) right after adding the field. +On Solr 8 and above, once the schema has `_root_`, Solr replaces a document by its `_root_` value. Documents indexed before the field was added have none, so reindexing one adds a new copy beside the old one, and later reindexes replace only the new copy. The old copies stay until they're deleted, so clear and reindex every class (`rake sunspot:reindex`, which deletes each class's documents first) right after adding the field. #### Things to know From 6ba0b98197bab8c87ec52a13d87a6c0b5f8864d2 Mon Sep 17 00:00:00 2001 From: Nicholas Jakobsen Date: Sat, 10 Oct 2026 01:41:10 -0700 Subject: [PATCH 09/11] fix: Keep removal free of nested checks for apps without nested documents, and merge a repeated nested block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A fourth review found: - `remove_all(Class)` and `remove(Class) { … }` ran `Setup.nested_under?`, which walked every registered setup through `get_inheritable_hash`, `parent` and `clazz`. If any registered class no longer resolved, as after a test removes a stubbed class, both raised `NameError`, even in an app with no nested associations at all, where they used to send only `type:Post`. `Setup.nested_declared?`, set by the first `nested` declaration, now returns the plain query at once when no class ever declared one, and each setup's check in the scan is wrapped so an unresolvable class counts as not nested. - Declaring `nested :kids` twice on one class, as a concern and the model body might, kept only the second block's fields, unlike parent fields, which add up across setup blocks. A repeated declaration now adds to the existing `NestedSetup`. - An association that listed the same record twice, as `has_many :through` can, sent two child documents with the same id. `NestedSetup#children_for` now returns each child once. Each has a spec that fails on 162767d9. New specs also cover `nested … :using`, `with_child` in a query facet row, and a facet excluding a `with_child` filter. The README's warning about adding `_root_` to an existing core moves out of the nested section into its own subsection under "Running Solr in production environment", since it applies to every class. It now also says, as checked on Solr 9.10.1, that removing a document indexed before the field was added leaves it in place. The nested section notes the delete-by-query each removal of a nested record sends. `examples/solr7_core` gains `_root_`. Co-Authored-By: Claude Opus 5.5 --- README.md | 7 ++- examples/solr7_core/conf/schema.xml | 3 ++ sunspot/lib/sunspot/nested_setup.rb | 4 +- sunspot/lib/sunspot/setup.rb | 41 +++++++++++----- sunspot/spec/api/nested_documents_spec.rb | 47 +++++++++++++++++++ .../spec/integration/nested_documents_spec.rb | 10 ++++ sunspot/spec/mocks/project.rb | 16 +++++++ 7 files changed, 115 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 53addc8b2..34f7b7031 100644 --- a/README.md +++ b/README.md @@ -1081,13 +1081,14 @@ The schema needs a `_root_` field with the same type as `id`. The bundled config ``` -On Solr 8 and above, once the schema has `_root_`, Solr replaces a document by its `_root_` value. Documents indexed before the field was added have none, so reindexing one adds a new copy beside the old one, and later reindexes replace only the new copy. The old copies stay until they're deleted, so clear and reindex every class (`rake sunspot:reindex`, which deletes each class's documents first) right after adding the field. +Adding `_root_` to a core that already holds documents needs a full reindex of every class on Solr 8 and above; see [Adding `_root_` to an existing core](#adding-_root_-to-an-existing-core). #### Things to know * Children are indexed only as part of their parent, so reindex the parent whenever its children change. * Reindexing a parent replaces its whole block on Solr 8 and above. On earlier versions, including the Solr that `sunspot_solr` bundles, a parent whose children went from some to none keeps its old children, and one whose children went from none to some is indexed twice; remove it before reindexing it there. * Atomic updates raise `ArgumentError` for a class with nested associations. Index the whole record instead. +* Removing a record of a nested class also sends a delete-by-query on `_root_`, because Solr before 8 leaves the children behind on a delete by id. Solr 8 and above delete them anyway, so there it's an extra, slower request per removal. * A nested block can't declare a boost, an id prefix, a join or a nested association of its own, and `with(record)` / `without(record)` inside `with_child` raise `ArgumentError`. * A subclass that declares the association again replaces its fields rather than adding to them. A search of the superclass still reaches its children, but only through fields both declare the same way. * A child document's id is `"//"`, where the child id is its Sunspot index id, or its position in the association when its class has no Sunspot adapter. Children carry a `_sunspot_nested_path_s` field naming their association and no `type`, so searches for the parent class never return them. @@ -1742,6 +1743,10 @@ solr: where the `./solr/init` directory contains a shell script that does any initial setup like downloading and unzipping your cores. In both cases, the solr images by default expects cores to be placed in `/opt/solr/server/solr/mycores`. +### Adding `_root_` to an existing core + +The bundled configset and `examples/solr7_core` define a `_root_` field, which [nested documents](#nested-documents-block-joins) need. If you bring an existing core's schema up to date with them on Solr 8 or above, the change affects every class, nested or not. Once the schema has `_root_`, Solr replaces and deletes documents by their `_root_` value, and documents indexed before the field was added have none. Reindexing one of them adds a new copy beside the old one, later reindexes replace only the new copy, and removing it by id leaves the old copy in place. The old copies stay until they're deleted, so clear and reindex every class (`rake sunspot:reindex`, which deletes each class's documents first) right after adding the field. + ## Development ### Running Tests diff --git a/examples/solr7_core/conf/schema.xml b/examples/solr7_core/conf/schema.xml index 11c16cffc..50200d8e8 100644 --- a/examples/solr7_core/conf/schema.xml +++ b/examples/solr7_core/conf/schema.xml @@ -133,6 +133,9 @@ + + + diff --git a/sunspot/lib/sunspot/nested_setup.rb b/sunspot/lib/sunspot/nested_setup.rb index 136fe67f3..8c4bc83f7 100644 --- a/sunspot/lib/sunspot/nested_setup.rb +++ b/sunspot/lib/sunspot/nested_setup.rb @@ -39,8 +39,10 @@ def path @path end + # Returns the model's children, each once, so an association that lists a + # record twice doesn't give two child documents the same id. def children_for(model) - Util.Array(@children_extractor.value_for(model)).compact + Util.Array(@children_extractor.value_for(model)).compact.uniq end def clazz diff --git a/sunspot/lib/sunspot/setup.rb b/sunspot/lib/sunspot/setup.rb index 860b9cd44..145deed9b 100644 --- a/sunspot/lib/sunspot/setup.rb +++ b/sunspot/lib/sunspot/setup.rb @@ -54,11 +54,15 @@ def add_join_field_factory(name, type, options = {}, &block) # # Declares an association whose records are indexed as child documents of # this class's documents. Returns the NestedSetup the block's fields are - # added to. + # added to. Declaring the same association again on this class adds the + # block's fields to it, as repeated setup blocks add parent fields. # def add_nested(name, options = {}, &block) - inherited = parent && parent.get_inheritable_hash(:nested_setups)[name.to_sym] - nested_setup = NestedSetup.new(self, name, options, inherited && inherited.path) + Setup.nested_declared! + nested_setup = @nested_setups[name.to_sym] || begin + inherited = parent && parent.get_inheritable_hash(:nested_setups)[name.to_sym] + NestedSetup.new(self, name, options, inherited && inherited.path) + end nested_setup.setup(&block) if block @nested_setups[nested_setup.name] = nested_setup end @@ -434,24 +438,39 @@ def for(clazz) #:nodoc: # Returns true when any class's setup declares a nested association. def nested_anywhere? #:nodoc: - setups.values.any? { |setup| setup.nested_setups.any? } + nested_declared? && setups.values.any? { |setup| ignoring_missing_constants { setup.nested_setups.any? } } + end + + # Records that some class has declared a nested association, so the + # nested checks in removal can return at once in an app that never does. + def nested_declared! #:nodoc: + @nested_declared = true + end + + def nested_declared? #:nodoc: + !!@nested_declared end # Returns true when one of the given classes, or a subclass of one, # declares a nested association. def nested_under?(classes) #:nodoc: + return false unless nested_declared? return true if classes.any? { |type| (setup = self.for(type)) && setup.nested_setups.any? } setups.values.any? do |setup| - next false if setup.nested_setups.empty? - clazz = begin - setup.clazz - rescue NameError - next false - end - classes.any? { |type| clazz <= type } + ignoring_missing_constants { setup.nested_setups.any? && classes.any? { |type| setup.clazz <= type } } end end + # Yields, returning false when a setup's class, or one of its ancestors, + # no longer resolves to a constant, as after a test removes a stubbed + # class. + def ignoring_missing_constants #:nodoc: + yield + rescue NameError => e + raise if e.is_a?(NoMethodError) + false + end + protected # diff --git a/sunspot/spec/api/nested_documents_spec.rb b/sunspot/spec/api/nested_documents_spec.rb index 9731f4765..ce49b7fb7 100644 --- a/sunspot/spec/api/nested_documents_spec.rb +++ b/sunspot/spec/api/nested_documents_spec.rb @@ -86,6 +86,11 @@ def children(document) expect(indexed_project.field_by_name(:name_s).value).to eq('Bridge') end + it 'sends a record the association lists twice as one child' do + session.index(Project.new(:name => 'Twice', :milestones => [milestone, milestone])) + expect(children(connection.adds.last.first).length).to eq(1) + end + it 'sends no children for an empty association' do session.index(Project.new(:name => 'Empty')) expect(children(connection.adds.last.first)).to be_empty @@ -111,6 +116,18 @@ def children(document) end end + describe 'declaring the same association twice on one class' do + it 'keeps the fields of both blocks' do + Object.const_set(:TwiceDeclared, Class.new(MockRecord)) + Sunspot.setup(TwiceDeclared) { nested(:kids) { string :a } } + Sunspot.setup(TwiceDeclared) { nested(:kids) { string :b } } + + expect(Sunspot::Setup.for(TwiceDeclared).nested_setup(:kids).fields.map(&:name)).to match_array([:a, :b]) + ensure + Object.send(:remove_const, :TwiceDeclared) + end + end + describe 'querying' do it 'finds parents by conditions a single child meets together' do session.search(Project) do @@ -145,6 +162,26 @@ def children(document) expect { session.search(Project) { with_child(:milestones) { without(milestone) } } }.to raise_error(ArgumentError, /Instance restrictions/) end + it 'works in a query facet row' do + session.search(Project) do + facet :designed do + row(:yes) { with_child(:milestones) { with :name, 'design' } } + end + end + expect(Array(connection.searches.last[:'facet.query'])).to include(a_string_starting_with('_query_:"{!parent')) + end + + it 'can be excluded from a facet' do + session.search(Project) do + designed = with_child(:milestones) { with :name, 'design' } + facet :status, :exclude => designed + end + tag = last_fq.last[/\A\{!tag=([^}]+)\}/, 1] + expect(tag).not_to be_nil + expect(last_fq.last).to include('_query_:"{!parent') + expect(connection.searches.last[:'facet.field']).to include("{!ex=#{tag}}status_s") + end + it 'combines with parent restrictions as separate filters' do session.search(Project) do with :status, 'active' @@ -244,6 +281,16 @@ def children(document) expect(connection.deletes_by_query).to eq(['type:Post', '(type:Post AND title_ss:monkeys)']) end + it 'removes a class without nested associations when another registered class no longer resolves' do + Object.const_set(:GoneThing, Class.new(MockRecord)) + Sunspot.setup(GoneThing) { string :name } + Object.send(:remove_const, :GoneThing) + + session.remove_all(Post) + session.remove(Post) { with :title, 'monkeys' } + expect(connection.deletes_by_query).to eq(['type:Post', '(type:Post AND title_ss:monkeys)']) + end + it 'sends no block delete when removing a record of a class without nested associations' do session.remove(Post.new) expect(connection.deletes_by_query).to be_empty diff --git a/sunspot/spec/integration/nested_documents_spec.rb b/sunspot/spec/integration/nested_documents_spec.rb index 29c87a58b..cbd6f2044 100644 --- a/sunspot/spec/integration/nested_documents_spec.rb +++ b/sunspot/spec/integration/nested_documents_spec.rb @@ -199,6 +199,16 @@ def children end end + describe 'an association read through another method' do + it 'indexes and finds the children :using names' do + plan = Plan.new(:name => 'plan', :milestones => [milestone('design', Time.utc(2026, 2, 1))]) + Sunspot.index!(plan) + + expect(Sunspot.search(Plan) { with_child(:steps) { with :name, 'design' } }.results).to eq([plan]) + expect(solr_count('_sunspot_nested_path_s:"Plan.steps"')).to eq(1) + end + end + describe 'other classes with the same association' do let(:t) { Time.utc(2026, 2, 1) } diff --git a/sunspot/spec/mocks/project.rb b/sunspot/spec/mocks/project.rb index 092b5b199..5d2c4e4c6 100644 --- a/sunspot/spec/mocks/project.rb +++ b/sunspot/spec/mocks/project.rb @@ -146,3 +146,19 @@ class SubCrate < Crate string(:at) { 'x' } end end + +# Children read through a method named differently from the association +class Plan < MockRecord + attr_accessor :name + attr_writer :milestones + + def milestones + @milestones ||= [] + end +end + +Sunspot.setup(Plan) do + nested :steps, :using => :milestones do + string :name + end +end From 995c844169e2dc608957bb207ea5fd04e9ed0dd5 Mon Sep 17 00:00:00 2001 From: Nicholas Jakobsen Date: Sat, 10 Oct 2026 02:15:54 -0700 Subject: [PATCH 10/11] fix: Add a repeated nested block only to the class's own setup, never an inherited one 6ba0b981 made a second `nested :kids` on a class add its fields to the existing `NestedSetup`. It found that setup in `@nested_setups`, but reading a class's nested setups copies its superclass's entries into that table, so once a subclass had been indexed, searched or scanned during removal, a subclass `nested :kids` found the superclass's `NestedSetup` and added its fields to it. Every superclass child was then indexed with the subclass's fields, the reverse of the documented replacement. A repeated block is now added to the existing `NestedSetup` only when this class declared it; otherwise the class gets a new one, as before. The spec reads the subclass's setup before it declares the association again, and fails on 6ba0b981. Co-Authored-By: Claude Opus 5.5 --- sunspot/lib/sunspot/nested_setup.rb | 4 +++- sunspot/lib/sunspot/setup.rb | 5 ++++- sunspot/spec/api/nested_documents_spec.rb | 17 +++++++++++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/sunspot/lib/sunspot/nested_setup.rb b/sunspot/lib/sunspot/nested_setup.rb index 8c4bc83f7..05e24590c 100644 --- a/sunspot/lib/sunspot/nested_setup.rb +++ b/sunspot/lib/sunspot/nested_setup.rb @@ -40,7 +40,9 @@ def path end # Returns the model's children, each once, so an association that lists a - # record twice doesn't give two child documents the same id. + # record twice doesn't give two child documents the same id. Children + # that compare equal, such as two Structs with the same attributes, count + # as one. def children_for(model) Util.Array(@children_extractor.value_for(model)).compact.uniq end diff --git a/sunspot/lib/sunspot/setup.rb b/sunspot/lib/sunspot/setup.rb index 145deed9b..148486d92 100644 --- a/sunspot/lib/sunspot/setup.rb +++ b/sunspot/lib/sunspot/setup.rb @@ -59,7 +59,10 @@ def add_join_field_factory(name, type, options = {}, &block) # def add_nested(name, options = {}, &block) Setup.nested_declared! - nested_setup = @nested_setups[name.to_sym] || begin + # Reading this class's nested setups copies inherited entries into + # @nested_setups, so only one this class declared itself is added to. + existing = @nested_setups[name.to_sym] + nested_setup = existing && existing.parent_setup.equal?(self) ? existing : begin inherited = parent && parent.get_inheritable_hash(:nested_setups)[name.to_sym] NestedSetup.new(self, name, options, inherited && inherited.path) end diff --git a/sunspot/spec/api/nested_documents_spec.rb b/sunspot/spec/api/nested_documents_spec.rb index ce49b7fb7..d06f03adb 100644 --- a/sunspot/spec/api/nested_documents_spec.rb +++ b/sunspot/spec/api/nested_documents_spec.rb @@ -128,6 +128,23 @@ def children(document) end end + describe 'a subclass declaring an association again after its setup has been read' do + it "keeps the subclass's fields off the superclass's children" do + Object.const_set(:ReadBase, Class.new(MockRecord)) + Object.const_set(:ReadSub, Class.new(ReadBase)) + Sunspot.setup(ReadBase) { nested(:kids) { string :name } } + Sunspot.setup(ReadSub) { string :title } + Sunspot::Setup.for(ReadSub).nested_setups # copies the inherited entry into ReadSub's own table + Sunspot.setup(ReadSub) { nested(:kids) { string :only_sub } } + + expect(Sunspot::Setup.for(ReadBase).nested_setup(:kids).fields.map(&:name)).to eq([:name]) + expect(Sunspot::Setup.for(ReadSub).nested_setup(:kids).fields.map(&:name)).to eq([:only_sub]) + ensure + Object.send(:remove_const, :ReadSub) + Object.send(:remove_const, :ReadBase) + end + end + describe 'querying' do it 'finds parents by conditions a single child meets together' do session.search(Project) do From 4bd2a36a5a894b916d1979c949cedd9d42917ad7 Mon Sep 17 00:00:00 2001 From: Nicholas Jakobsen Date: Sat, 10 Oct 2026 03:51:24 -0700 Subject: [PATCH 11/11] docs: Tighten the nested documents comments after a comment pass A comment pass, checking each comment the PR adds against the code, found: - `BlockJoin#child_query`'s comment said Solr would join documents of other classes to the next parent, which they can't be, since every document without the path marker is a root. It's gone, and `#scope`'s comment says what the path condition guarantees. - Several comments described only one branch: `Setup#add_nested` (same-class merge versus a subclass's own setup, and that a repeated declaration keeps the first one's options), `nested_anywhere?` / `nested_under?` / `ignoring_missing_constants` (unresolvable classes, the early return, the re-raised `NoMethodError`), `NestedSetup#path` (a redeclaring subclass), `CompositeNestedSetup#text_fields` and `#dynamic_field_factory`, `DSL::Scope#with_child` (the raises and multi-class coverage), and `DSL::Fields#nested` (same-class merge). - `Indexer#with_children` and `#add_child_documents` gain examples. `#remove_blocks` names the `_root_` delete and its batches, and the Solr versions it was checked against (8.11 and 9 delete children with their parent by id, 6 doesn't). - The README's nested section covers the same-class merge, and no longer calls the removal query "slower", which nothing measured. The instance-restriction error splits its semicolon into two sentences. Co-Authored-By: Claude Opus 5.5 --- README.md | 6 +-- sunspot/lib/sunspot/composite_nested_setup.rb | 7 +++- sunspot/lib/sunspot/dsl/fields.rb | 15 ++++---- sunspot/lib/sunspot/dsl/scope.rb | 9 ++++- sunspot/lib/sunspot/indexer.rb | 33 +++++++++------- sunspot/lib/sunspot/nested_setup.rb | 13 ++++--- sunspot/lib/sunspot/query/block_join.rb | 19 ++++------ sunspot/lib/sunspot/setup.rb | 38 +++++++++++-------- .../spec/integration/nested_documents_spec.rb | 2 +- 9 files changed, 81 insertions(+), 61 deletions(-) diff --git a/README.md b/README.md index 34f7b7031..010ed5057 100644 --- a/README.md +++ b/README.md @@ -1086,11 +1086,11 @@ Adding `_root_` to a core that already holds documents needs a full reindex of e #### Things to know * Children are indexed only as part of their parent, so reindex the parent whenever its children change. -* Reindexing a parent replaces its whole block on Solr 8 and above. On earlier versions, including the Solr that `sunspot_solr` bundles, a parent whose children went from some to none keeps its old children, and one whose children went from none to some is indexed twice; remove it before reindexing it there. +* Reindexing a parent replaces its whole block on Solr 8 and above. On earlier versions, including the Solr that `sunspot_solr` bundles, a parent whose children went from some to none keeps its old children, and one whose children went from none to some is indexed twice. Remove such a parent before reindexing it there. * Atomic updates raise `ArgumentError` for a class with nested associations. Index the whole record instead. -* Removing a record of a nested class also sends a delete-by-query on `_root_`, because Solr before 8 leaves the children behind on a delete by id. Solr 8 and above delete them anyway, so there it's an extra, slower request per removal. +* Removing a record of a nested class also sends a delete-by-query on `_root_`, because Solr before 8 leaves the children behind on a delete by id. Solr 8 and above delete them anyway, so there it's an extra request per removal. * A nested block can't declare a boost, an id prefix, a join or a nested association of its own, and `with(record)` / `without(record)` inside `with_child` raise `ArgumentError`. -* A subclass that declares the association again replaces its fields rather than adding to them. A search of the superclass still reaches its children, but only through fields both declare the same way. +* Declaring an association again in the same class adds to its fields. A subclass that declares an inherited association again replaces its fields. A search of the superclass still reaches the subclass's children, but only through fields both declare the same way. * A child document's id is `"//"`, where the child id is its Sunspot index id, or its position in the association when its class has no Sunspot adapter. Children carry a `_sunspot_nested_path_s` field naming their association and no `type`, so searches for the parent class never return them. ### Composite ID diff --git a/sunspot/lib/sunspot/composite_nested_setup.rb b/sunspot/lib/sunspot/composite_nested_setup.rb index 605154c46..0759104b1 100644 --- a/sunspot/lib/sunspot/composite_nested_setup.rb +++ b/sunspot/lib/sunspot/composite_nested_setup.rb @@ -29,7 +29,8 @@ def field(field_name) end # Returns the text fields with the given name, one per distinct Solr field. - # TextFieldSetup raises when there is more than one. + # Raises UnrecognizedFieldError when no setup declares it. TextFieldSetup + # raises when there is more than one. def text_fields(field_name) fields = @nested_setups.flat_map do |setup| begin @@ -47,6 +48,10 @@ def type_names @nested_setups.map(&:path).uniq end + # Returns the dynamic field factory with the given base name. Unlike + # #field, it requires every setup to declare it, as CompositeSetup does + # for parents' dynamic fields. Raises UnrecognizedFieldError when one + # doesn't, or when they build different Solr fields. def dynamic_field_factory(field_name) factories = @nested_setups.map { |setup| setup.dynamic_field_factory(field_name) } return factories.first if factories.map { |factory| factory.build('x').indexed_name }.uniq.one? diff --git a/sunspot/lib/sunspot/dsl/fields.rb b/sunspot/lib/sunspot/dsl/fields.rb index 36231fe08..a153f8d60 100644 --- a/sunspot/lib/sunspot/dsl/fields.rb +++ b/sunspot/lib/sunspot/dsl/fields.rb @@ -79,8 +79,8 @@ def id_prefix(attr_name = nil, &block) # that a single child must meet together. # # Children are indexed only as part of their parent, so the parent must - # be reindexed whenever its children change. A class with nested - # associations cannot be atomically updated. + # be reindexed whenever its children change. Atomic updates to a class + # with nested associations raise ArgumentError. # # Reindexing a parent replaces its old block only on Solr 8 or later. # Solr before 8 replaces a document with children by +_root_+ and one @@ -88,11 +88,12 @@ def id_prefix(attr_name = nil, &block) # keeps its old children, and one whose children went from none to some # is indexed twice. Remove such a parent before reindexing it there. # - # A subclass that declares an association again replaces its fields - # rather than adding to them: its children are indexed with only the - # fields of its own block. They keep the superclass's association - # marker, so a search of the superclass still reaches them, but only - # through fields both blocks declare the same way. + # Declaring an association again in the same class adds the block's + # fields to it. A subclass that declares an inherited association again + # replaces its fields: its children are indexed with only the fields of + # its own block. They keep the superclass's association marker, so a + # search of the superclass still reaches them, but only through fields + # both blocks declare the same way. # # The block cannot declare a document boost, an id prefix, a join, or a # nested association of its own. Each raises ArgumentError. diff --git a/sunspot/lib/sunspot/dsl/scope.rb b/sunspot/lib/sunspot/dsl/scope.rb index 3732a51d6..14af387a5 100644 --- a/sunspot/lib/sunspot/dsl/scope.rb +++ b/sunspot/lib/sunspot/dsl/scope.rb @@ -104,7 +104,12 @@ def without(*args) # declared with DSL::Fields#nested. # # The block takes the same restrictions as a scope, with field names - # referring to the child's fields. + # referring to the child's fields. Restricting by instance, as in + # with(record), raises ArgumentError, and a nested #with_child + # raises UnrecognizedFieldError. + # + # In a search of several classes, the block matches the children of + # every searched class that declares the association. # # ==== Example # @@ -257,7 +262,7 @@ def add_restriction(negated, *args) end else # args are instances if @setup.is_a?(NestedSetup) || @setup.is_a?(CompositeNestedSetup) - raise ArgumentError, "Instance restrictions are not supported inside with_child or without_child; restrict on the child's fields instead" + raise ArgumentError, "Instance restrictions are not supported inside with_child or without_child. Restrict on the child's fields instead" end @scope.add_restriction( negated, diff --git a/sunspot/lib/sunspot/indexer.rb b/sunspot/lib/sunspot/indexer.rb index c388f011b..e0952ab1e 100644 --- a/sunspot/lib/sunspot/indexer.rb +++ b/sunspot/lib/sunspot/indexer.rb @@ -137,13 +137,13 @@ def prepare_full_update(model) # # Adds a child document to +document+ for each of the model's children in - # the association, so Solr indexes the parent and its children as one - # block. Raises NestedDocumentsNotSupportedError when there are children - # and RSolr cannot send child documents. + # +nested_setup+'s association, so Solr indexes the parent and its + # children as one block. Raises NestedDocumentsNotSupportedError when there + # are children and RSolr cannot send child documents. # # A child's id is the parent's id followed by the association name and - # #child_key, so it is unique within the index and carries the parent's - # id prefix. + # #child_key, such as "Project 1/milestones/Milestone 7", so it is + # unique within the index and carries the parent's id prefix. # def add_child_documents(document, nested_setup, model) children = nested_setup.children_for(model) @@ -193,10 +193,11 @@ def nested_class_name?(class_name) end # - # Deletes every document in the blocks rooted at the given ids. Solr 9 - # deletes a parent's children along with it on a delete by id, and Solr 6 - # leaves them in the index. A child left behind is joined to whichever - # parent follows it in the index. + # Deletes every document in the blocks rooted at the given ids, by + # +_root_+, in batches of REMOVE_BLOCKS_BATCH_SIZE. Solr 8.11 and 9 delete a + # parent's children along with it on a delete by id, and Solr 6 leaves + # them in the index. A child left behind is joined to whichever parent + # follows it in the index. # def remove_blocks(ids) ids.each_slice(REMOVE_BLOCKS_BATCH_SIZE) do |batch| @@ -207,11 +208,15 @@ def remove_blocks(ids) # # Returns a query matching the documents +query+ matches and, when one of # +classes+ or a subclass of one has nested associations, their children - # too. Solr evaluates it once, so a delete by it removes the parents and - # children together, and a parent condition that refers to children still - # matches the parents. The block mask is every document that is not a - # child, so documents of other classes indexed between blocks are never - # matched as children. + # too. Returns +query+ unchanged otherwise. + # + # with_children("type:Project", [Project]) + # # => (type:Project) OR _query_:"{!child of=\"*:* -_sunspot_nested_path_s:[* TO *]\" v=\"type:Project\"}" + # + # It is one query, so a delete by it removes the parents and their + # children together. A +query+ with a DSL::Scope#with_child condition + # still matches its parents, since the children it refers to are deleted + # by the same query. # def with_children(query, classes) return query unless Setup.nested_under?(classes) diff --git a/sunspot/lib/sunspot/nested_setup.rb b/sunspot/lib/sunspot/nested_setup.rb index 05e24590c..3935fec60 100644 --- a/sunspot/lib/sunspot/nested_setup.rb +++ b/sunspot/lib/sunspot/nested_setup.rb @@ -32,17 +32,18 @@ def initialize(parent_setup, name, options = {}, path = nil) # # Returns the value stored in PATH_FIELD on every child of this - # association: the declaring class and the association name, such as - # "Project.milestones". + # association: the class that first declared it and the association name, + # such as "Project.milestones". A subclass that declares the association + # again keeps its superclass's path. # def path @path end - # Returns the model's children, each once, so an association that lists a - # record twice doesn't give two child documents the same id. Children - # that compare equal, such as two Structs with the same attributes, count - # as one. + # Returns the model's non-nil children, each once, so an association that + # lists a record twice doesn't give two child documents the same id. + # Children that compare equal, such as two Structs with the same + # attributes, count as one. def children_for(model) Util.Array(@children_extractor.value_for(model)).compact.uniq end diff --git a/sunspot/lib/sunspot/query/block_join.rb b/sunspot/lib/sunspot/query/block_join.rb index 6188e0c53..47f158aac 100644 --- a/sunspot/lib/sunspot/query/block_join.rb +++ b/sunspot/lib/sunspot/query/block_join.rb @@ -18,8 +18,10 @@ class BlockJoin #:nodoc: ROOTS = "*:* -#{NestedSetup::PATH_FIELD}:[* TO *]".freeze # The restrictions a child must satisfy. Its first component is the - # condition on NestedSetup::PATH_FIELD, so every restriction added to it, - # negated ones included, is joined to that condition. + # condition on NestedSetup::PATH_FIELD, and every restriction added to it + # is ANDed with that condition. The child query therefore matches only + # this association's children, and a negated restriction can't match a + # parent or a child of another association. attr_reader :scope def initialize(nested_setup, negated = false, scope = nil, paths = [nested_setup.path]) @@ -27,6 +29,7 @@ def initialize(nested_setup, negated = false, scope = nil, paths = [nested_setup @scope = scope || Connective::Conjunction.new.tap { |conjunction| conjunction.add_component(PathRestriction.new(paths)) } end + # Escapes twice: the local param values, then the whole +_query_+ phrase. def to_boolean_phrase phrase = %Q(_query_:"#{escape(%Q({!parent which="#{escape(ROOTS)}" v="#{escape(child_query)}"}))}") negated? ? "-#{phrase}" : phrase @@ -42,13 +45,6 @@ def negate private - # - # Returns the query for this association's children: #scope, whose first - # component requires NestedSetup::PATH_FIELD to match the association. - # Solr joins every document the child query matches to the next parent - # in the index, including documents of other classes and children of - # other associations, so that condition is always required. - # def child_query @scope.to_boolean_phrase end @@ -57,9 +53,8 @@ def escape(value) self.class.escape(value) end - # Backslash-escapes quotes and backslashes for a quoted Solr string. - # #to_boolean_phrase applies it at both levels of nesting: to the local - # param values, then to the whole +_query_+ phrase. + # Backslash-escapes quotes and backslashes for a quoted Solr string: + # say "hi" becomes say \"hi\". def self.escape(value) value.gsub(/(["\\])/, '\\\\\1') end diff --git a/sunspot/lib/sunspot/setup.rb b/sunspot/lib/sunspot/setup.rb index 148486d92..7ad9a6a1a 100644 --- a/sunspot/lib/sunspot/setup.rb +++ b/sunspot/lib/sunspot/setup.rb @@ -54,13 +54,18 @@ def add_join_field_factory(name, type, options = {}, &block) # # Declares an association whose records are indexed as child documents of # this class's documents. Returns the NestedSetup the block's fields are - # added to. Declaring the same association again on this class adds the - # block's fields to it, as repeated setup blocks add parent fields. + # added to. + # + # Declaring the same association again on this class adds the block's + # fields to it, as repeated setup blocks add parent fields, and keeps the + # first declaration's options. A subclass that declares an inherited + # association gets a NestedSetup of its own, with only its block's fields + # and its superclass's path. # def add_nested(name, options = {}, &block) Setup.nested_declared! - # Reading this class's nested setups copies inherited entries into - # @nested_setups, so only one this class declared itself is added to. + # @nested_setups also holds entries get_inheritable_hash copied from the + # superclass, so only a NestedSetup this class created is added to. existing = @nested_setups[name.to_sym] nested_setup = existing && existing.parent_setup.equal?(self) ? existing : begin inherited = parent && parent.get_inheritable_hash(:nested_setups)[name.to_sym] @@ -88,9 +93,9 @@ def nested_setups # # Returns the NestedSetups a search of this class covers for the given - # association: its own, inherited if need be. A subclass that declares the - # association again keeps its path, so its children match too. Raises - # UnrecognizedFieldError when this class has no such association. + # association: its own, inherited if need be, as a one-element array to + # match CompositeSetup#nested_setups_named. Raises UnrecognizedFieldError + # when this class has no such association. # def nested_setups_named(name) [nested_setup(name)] @@ -439,13 +444,15 @@ def for(clazz) #:nodoc: setups[clazz.name.to_sym] || self.for(clazz.superclass) if clazz end - # Returns true when any class's setup declares a nested association. + # Returns true when any class's setup has a nested association. A setup + # whose class no longer resolves counts as having none. def nested_anywhere? #:nodoc: nested_declared? && setups.values.any? { |setup| ignoring_missing_constants { setup.nested_setups.any? } } end - # Records that some class has declared a nested association, so the - # nested checks in removal can return at once in an app that never does. + # Records that a class has declared a nested association. Until one has, + # #nested_anywhere? and #nested_under? return false without scanning + # the registered setups. def nested_declared! #:nodoc: @nested_declared = true end @@ -454,8 +461,9 @@ def nested_declared? #:nodoc: !!@nested_declared end - # Returns true when one of the given classes, or a subclass of one, - # declares a nested association. + # Returns true when one of the given classes, or a registered subclass of + # one, has a nested association, declared or inherited. A setup whose + # class no longer resolves counts as having none. def nested_under?(classes) #:nodoc: return false unless nested_declared? return true if classes.any? { |type| (setup = self.for(type)) && setup.nested_setups.any? } @@ -464,9 +472,9 @@ def nested_under?(classes) #:nodoc: end end - # Yields, returning false when a setup's class, or one of its ancestors, - # no longer resolves to a constant, as after a test removes a stubbed - # class. + # Returns the block's result, or false when a setup's class, or one of + # its ancestors, no longer resolves to a constant, as after a test + # removes a stubbed class. A NoMethodError still raises. def ignoring_missing_constants #:nodoc: yield rescue NameError => e diff --git a/sunspot/spec/integration/nested_documents_spec.rb b/sunspot/spec/integration/nested_documents_spec.rb index cbd6f2044..f463f2683 100644 --- a/sunspot/spec/integration/nested_documents_spec.rb +++ b/sunspot/spec/integration/nested_documents_spec.rb @@ -200,7 +200,7 @@ def children end describe 'an association read through another method' do - it 'indexes and finds the children :using names' do + it 'indexes and finds the children the :using method returns' do plan = Plan.new(:name => 'plan', :milestones => [milestone('design', Time.utc(2026, 2, 1))]) Sunspot.index!(plan)