From 2dc59ab22c62d9cda2adf0321f6f3b9be4d593a7 Mon Sep 17 00:00:00 2001 From: Doug Keller Date: Tue, 26 Mar 2019 16:09:27 -0400 Subject: [PATCH 01/20] Properly escape filter parts for parent queries --- sunspot/lib/sunspot/query/block_join.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sunspot/lib/sunspot/query/block_join.rb b/sunspot/lib/sunspot/query/block_join.rb index 4a661903b..af190420e 100644 --- a/sunspot/lib/sunspot/query/block_join.rb +++ b/sunspot/lib/sunspot/query/block_join.rb @@ -152,7 +152,7 @@ def all_parents_filter # to select which parents are used in the query. fq = filter_query.to_params[:fq] raise 'allParents filter must be non-empty!' if fq.nil? - fq[0] # Type filter used by Sunspot + Util.escape(fq[0]) # Type filter used by Sunspot end def secondary_filter @@ -178,7 +178,7 @@ class ParentWhich < Abstract def all_parents_filter # Use top-level scope (on parent type) as allParents filter. - scope.to_params[:fq].flatten.join(' AND ') + scope.to_params[:fq].flatten.map { |v| Util.escape(v) }.join(' AND ') end def secondary_filter @@ -196,4 +196,4 @@ def to_params end end end -end \ No newline at end of file +end From e0b37eb1c6bc2f86b15ef9548c25deee68b1b4be Mon Sep 17 00:00:00 2001 From: Doug Keller Date: Tue, 26 Mar 2019 17:27:35 -0400 Subject: [PATCH 02/20] Only escape when necessary --- sunspot/lib/sunspot/query/block_join.rb | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/sunspot/lib/sunspot/query/block_join.rb b/sunspot/lib/sunspot/query/block_join.rb index af190420e..407640311 100644 --- a/sunspot/lib/sunspot/query/block_join.rb +++ b/sunspot/lib/sunspot/query/block_join.rb @@ -129,7 +129,7 @@ def scope?(scope) # String:: the BlockJoin query string to be added to the +:q+ filter. # def render_query_string(query_type, all_parents_key) - all_parents = all_parents_filter + all_parents = all_parents_filter(escape: true) filter = secondary_filter query_string = "{!#{query_type} #{all_parents_key}=\"#{all_parents}\"" @@ -147,12 +147,12 @@ def render_query_string(query_type, all_parents_key) class ChildOf < Abstract alias some_parents_filter secondary_filter - def all_parents_filter + def all_parents_filter(escape: false) # The scope of the initial query should give the 'allParents' filter, # to select which parents are used in the query. fq = filter_query.to_params[:fq] raise 'allParents filter must be non-empty!' if fq.nil? - Util.escape(fq[0]) # Type filter used by Sunspot + escape ? Util.escape(fq[0]) : fq[0] # Type filter used by Sunspot end def secondary_filter @@ -176,9 +176,11 @@ def to_params class ParentWhich < Abstract alias some_children_filter secondary_filter - def all_parents_filter + def all_parents_filter(escape: false) # Use top-level scope (on parent type) as allParents filter. - scope.to_params[:fq].flatten.map { |v| Util.escape(v) }.join(' AND ') + parts = scope.to_params[:fq].flatten + parts = parts.map { |v| Util.escape(v) } if escape + parts.join(' AND ') end def secondary_filter From a3cb8f1919e4fc5169ac487f77dd1aff059dd574 Mon Sep 17 00:00:00 2001 From: Doug Keller Date: Wed, 27 Mar 2019 13:56:57 -0400 Subject: [PATCH 03/20] Check for collection proxy as an acceptable child doc type --- sunspot/lib/sunspot/field_factory.rb | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/sunspot/lib/sunspot/field_factory.rb b/sunspot/lib/sunspot/field_factory.rb index a01c84324..6b728c091 100644 --- a/sunspot/lib/sunspot/field_factory.rb +++ b/sunspot/lib/sunspot/field_factory.rb @@ -1,5 +1,5 @@ module Sunspot - # + # # The FieldFactory module contains classes for generating fields. FieldFactory # implementation classes should implement a #build method, although the arity # of the method depends on the type of factory. They also must implement a @@ -41,7 +41,7 @@ def extract_value(model, options = {}) end end - # + # # A StaticFieldFactory generates normal static fields. Each factory instance # contains an eager-initialized field instance, which is returned by the # #build method. @@ -60,14 +60,14 @@ def initialize(name, type, options = {}, &block) end end - # + # # Return the field instance built by this factory # def build @field end - # + # # Extract the encapsulated field's data from the given model and add it # into the Solr document for indexing. # @@ -90,7 +90,7 @@ def populate_document(document, model, options = {}) #:nodoc: end end - # + # # A unique signature identifying this field by name and type. # def signature @@ -107,7 +107,7 @@ def initialize(name, type, options = {}, &block) @field = JoinField.new(self.name, type, options) end - # + # # Return the field instance built by this factory # def build @@ -122,7 +122,7 @@ def signature end end - # + # # DynamicFieldFactories create dynamic field instances based on dynamic # configuration. # @@ -141,13 +141,13 @@ def initialize(name, type, options = {}, &block) def build(dynamic_name) AttributeField.new([@name, dynamic_name].join(separator), @type, @options.dup) end - # + # # This alias allows a DynamicFieldFactory to be used in place of a Setup # or CompositeSetup instance by query components. # alias_method :field, :build - # + # # Generate dynamic fields based on hash returned by data accessor and # add the field data to the document. # @@ -167,7 +167,7 @@ def populate_document(document, model, options = {}) end end - # + # # Unique signature identifying this dynamic field based on name and type # def signature @@ -193,7 +193,7 @@ def extract_value(model, options = {}) # TODO(ar3s3ru): how to handle incorrect field values? values = @extractor.value_for(model) adapter = options[:adapter] - unless values.is_a? Array + unless values.is_a?(Array) || values.is_a?(ActiveRecord::Associations::CollectionProxy) raise 'Child documents field must be an Array of indexable documents' end if adapter.nil? || !adapter.respond_to?(:call) From 27f4fb34d607ad3987362a33c5145498066da992 Mon Sep 17 00:00:00 2001 From: Doug Keller Date: Wed, 27 Mar 2019 14:42:32 -0400 Subject: [PATCH 04/20] Only specify type filter for faceting --- sunspot/lib/sunspot/query/block_join.rb | 16 ++++++++++++++-- .../lib/sunspot/query/block_join_json_facet.rb | 4 ++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/sunspot/lib/sunspot/query/block_join.rb b/sunspot/lib/sunspot/query/block_join.rb index 407640311..28f282778 100644 --- a/sunspot/lib/sunspot/query/block_join.rb +++ b/sunspot/lib/sunspot/query/block_join.rb @@ -155,6 +155,10 @@ def all_parents_filter(escape: false) escape ? Util.escape(fq[0]) : fq[0] # Type filter used by Sunspot end + def type_filter + all_parents_filter + end + def secondary_filter fq = filter_query.to_params[:fq] q = super @@ -176,11 +180,19 @@ def to_params class ParentWhich < Abstract alias some_children_filter secondary_filter - def all_parents_filter(escape: false) + def all_parents_parts(escape: false) # Use top-level scope (on parent type) as allParents filter. parts = scope.to_params[:fq].flatten parts = parts.map { |v| Util.escape(v) } if escape - parts.join(' AND ') + parts + end + + def all_parents_filter(*args) + all_parents_parts(*args).join(' AND ') + end + + def type_filter + all_parents_parts.find { |part| part.starts_with?('type:') } end def secondary_filter diff --git a/sunspot/lib/sunspot/query/block_join_json_facet.rb b/sunspot/lib/sunspot/query/block_join_json_facet.rb index eeaf0b80c..35ebe7f3e 100644 --- a/sunspot/lib/sunspot/query/block_join_json_facet.rb +++ b/sunspot/lib/sunspot/query/block_join_json_facet.rb @@ -62,7 +62,7 @@ def field_name_with_local_params type: 'terms', field: @field.indexed_name, domain: { - @operator => @query.all_parents_filter, + @operator => @query.type_filter, FILTER_OP => generate_filter } }.merge!(init_params) @@ -78,4 +78,4 @@ def generate_filter end end end -end \ No newline at end of file +end From ea2e2c0953b0874e9dfa56c784d35792744fea55 Mon Sep 17 00:00:00 2001 From: Doug Keller Date: Wed, 27 Mar 2019 14:52:59 -0400 Subject: [PATCH 05/20] Escape by default --- sunspot/lib/sunspot/query/block_join.rb | 17 ++++++++--------- .../lib/sunspot/query/block_join_json_facet.rb | 2 +- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/sunspot/lib/sunspot/query/block_join.rb b/sunspot/lib/sunspot/query/block_join.rb index 28f282778..0ca078dcc 100644 --- a/sunspot/lib/sunspot/query/block_join.rb +++ b/sunspot/lib/sunspot/query/block_join.rb @@ -129,7 +129,7 @@ def scope?(scope) # String:: the BlockJoin query string to be added to the +:q+ filter. # def render_query_string(query_type, all_parents_key) - all_parents = all_parents_filter(escape: true) + all_parents = all_parents_filter filter = secondary_filter query_string = "{!#{query_type} #{all_parents_key}=\"#{all_parents}\"" @@ -147,16 +147,16 @@ def render_query_string(query_type, all_parents_key) class ChildOf < Abstract alias some_parents_filter secondary_filter - def all_parents_filter(escape: false) + def all_parents_filter # The scope of the initial query should give the 'allParents' filter, # to select which parents are used in the query. fq = filter_query.to_params[:fq] raise 'allParents filter must be non-empty!' if fq.nil? - escape ? Util.escape(fq[0]) : fq[0] # Type filter used by Sunspot + Util.escape(fq[0]) # Type filter used by Sunspot end - def type_filter - all_parents_filter + def facet_type_filter + filter_query.to_params[:fq][0] end def secondary_filter @@ -183,16 +183,15 @@ class ParentWhich < Abstract def all_parents_parts(escape: false) # Use top-level scope (on parent type) as allParents filter. parts = scope.to_params[:fq].flatten - parts = parts.map { |v| Util.escape(v) } if escape - parts + parts.map(&Util.escape) end def all_parents_filter(*args) all_parents_parts(*args).join(' AND ') end - def type_filter - all_parents_parts.find { |part| part.starts_with?('type:') } + def facet_type_filter + scope.to_params[:fq].flatten[0] end def secondary_filter diff --git a/sunspot/lib/sunspot/query/block_join_json_facet.rb b/sunspot/lib/sunspot/query/block_join_json_facet.rb index 35ebe7f3e..2adb9d532 100644 --- a/sunspot/lib/sunspot/query/block_join_json_facet.rb +++ b/sunspot/lib/sunspot/query/block_join_json_facet.rb @@ -62,7 +62,7 @@ def field_name_with_local_params type: 'terms', field: @field.indexed_name, domain: { - @operator => @query.type_filter, + @operator => @query.facet_type_filter, FILTER_OP => generate_filter } }.merge!(init_params) From 29a542b3bd5794cb600357c562dd9ac80d3abe70 Mon Sep 17 00:00:00 2001 From: Doug Keller Date: Wed, 27 Mar 2019 14:54:44 -0400 Subject: [PATCH 06/20] to_proc doesnt work for Util.escape --- sunspot/lib/sunspot/query/block_join.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sunspot/lib/sunspot/query/block_join.rb b/sunspot/lib/sunspot/query/block_join.rb index 0ca078dcc..672ff0511 100644 --- a/sunspot/lib/sunspot/query/block_join.rb +++ b/sunspot/lib/sunspot/query/block_join.rb @@ -183,7 +183,7 @@ class ParentWhich < Abstract def all_parents_parts(escape: false) # Use top-level scope (on parent type) as allParents filter. parts = scope.to_params[:fq].flatten - parts.map(&Util.escape) + parts.map { |v| Util.escape(v) } end def all_parents_filter(*args) From d16a4898097ea4ffc3873e5a6f9ce34fb6cddafc Mon Sep 17 00:00:00 2001 From: Doug Keller Date: Wed, 27 Mar 2019 15:12:08 -0400 Subject: [PATCH 07/20] Define facet name using options or the field --- sunspot/lib/sunspot/search/field_json_facet.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sunspot/lib/sunspot/search/field_json_facet.rb b/sunspot/lib/sunspot/search/field_json_facet.rb index 139e09a31..2dfa161c5 100644 --- a/sunspot/lib/sunspot/search/field_json_facet.rb +++ b/sunspot/lib/sunspot/search/field_json_facet.rb @@ -5,7 +5,8 @@ class FieldJsonFacet attr_reader :name def initialize(field, search, options) - @name, @search, @options = name, search, options + @search, @options = search, options + @name = (options[:name] || field.name) @field = field end From b102ca5bb988888d813ee9170baef4f4cbd9e5da Mon Sep 17 00:00:00 2001 From: Doug Keller Date: Wed, 27 Mar 2019 16:05:34 -0400 Subject: [PATCH 08/20] Prevent indexing of child documents without their parent --- sunspot/lib/sunspot/indexer.rb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/sunspot/lib/sunspot/indexer.rb b/sunspot/lib/sunspot/indexer.rb index 8cb2419dc..b8f7a59ba 100644 --- a/sunspot/lib/sunspot/indexer.rb +++ b/sunspot/lib/sunspot/indexer.rb @@ -1,7 +1,7 @@ require 'sunspot/batcher' module Sunspot - # + # # This class presents a service for adding, updating, and removing data # from the Solr index. An Indexer instance is associated with a particular # setup, and thus is capable of indexing instances of a certain class (and its @@ -12,7 +12,7 @@ def initialize(connection) @connection = connection end - # + # # Construct a representation of the model for indexing and send it to the # connection for indexing # @@ -100,9 +100,13 @@ def batcher # # Convert documents into hash of indexed properties # - def prepare_full_update(model) + def prepare_full_update(model, indexing_parent: false) document = document_for_full_update(model) setup = setup_for_object(model) + if setup.is_child && !indexing_parent + raise 'Child documents must be indexed with their parent' + end + if boost = setup.document_boost_for(model) document.attrs[:boost] = boost end @@ -113,7 +117,7 @@ def prepare_full_update(model) setup.child_field_factory.populate_document( document, model, - adapter: ->(child_model) { prepare_full_update(child_model) } + adapter: ->(child_model) { prepare_full_update(child_model, indexing_parent: true) } ) end document From ba6d3510bea8228d602976b29fe180bd19032972 Mon Sep 17 00:00:00 2001 From: Doug Keller Date: Wed, 27 Mar 2019 16:16:48 -0400 Subject: [PATCH 09/20] Allow multiple child field factories for a setup --- sunspot/lib/sunspot/indexer.rb | 6 +++--- sunspot/lib/sunspot/setup.rb | 5 +++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/sunspot/lib/sunspot/indexer.rb b/sunspot/lib/sunspot/indexer.rb index b8f7a59ba..76b40fc6f 100644 --- a/sunspot/lib/sunspot/indexer.rb +++ b/sunspot/lib/sunspot/indexer.rb @@ -113,8 +113,8 @@ def prepare_full_update(model, indexing_parent: false) setup.all_field_factories.each do |field_factory| field_factory.populate_document(document, model) end - unless setup.child_field_factory.nil? - setup.child_field_factory.populate_document( + setup.child_field_factories.each do |child_field_factory| + child_field_factory.populate_document( document, model, adapter: ->(child_model) { prepare_full_update(child_model, indexing_parent: true) } @@ -128,7 +128,7 @@ def prepare_atomic_update(clazz, id, updates = {}) setup = setup_for_class(clazz) # Child documents must be re-indexed with parent at each update, # otherwise Solr would discard them. - unless setup.child_field_factory.nil? + unless setup.child_field_factories.empty? raise 'Objects with child documents can\'t perform atomic updates' end setup.all_field_factories.each do |field_factory| diff --git a/sunspot/lib/sunspot/setup.rb b/sunspot/lib/sunspot/setup.rb index 83ded7f44..aff1111bf 100644 --- a/sunspot/lib/sunspot/setup.rb +++ b/sunspot/lib/sunspot/setup.rb @@ -5,7 +5,7 @@ module Sunspot # class Setup #:nodoc: attr_reader :class_object_id, - :child_field_factory + :child_field_factories attr_accessor :is_child @@ -19,6 +19,7 @@ def initialize(clazz) @more_like_this_field_factories_cache = Hash.new { |h, k| h[k] = [] } @dsl = DSL::Fields.new(self) @document_boost_extractor = nil + @child_field_factories = [] add_field_factory(:class, Type::ClassType.instance) end @@ -98,7 +99,7 @@ def add_dynamic_field_factory(name, type, options = {}, &block) # field:: The document field that contains the child documents. # def add_child_field_factory(field) - @child_field_factory = FieldFactory::Child.new(field) + @child_field_factories << FieldFactory::Child.new(field) end # From 3b5b3984f155c7b85eafadd156e26d5e6dc225de Mon Sep 17 00:00:00 2001 From: Doug Keller Date: Wed, 27 Mar 2019 16:22:51 -0400 Subject: [PATCH 10/20] Add more validation checks and compatibility for non-rails environments --- sunspot/lib/sunspot/dsl/fields.rb | 6 +++++- sunspot/lib/sunspot/field_factory.rb | 9 ++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/sunspot/lib/sunspot/dsl/fields.rb b/sunspot/lib/sunspot/dsl/fields.rb index 7b70ceb3f..81e75f963 100644 --- a/sunspot/lib/sunspot/dsl/fields.rb +++ b/sunspot/lib/sunspot/dsl/fields.rb @@ -73,8 +73,12 @@ def text(*names, &block) # # field:: Field name at object level that contains child documents # - def child_documents(field) + def child_documents(field, type:) Sunspot::Util.ensure_child_documents_support + setup = Sunspot::Setup.for(type) + unless setup.is_child + raise 'Child documents must be flagged as is_child' + end @setup.add_child_field_factory(field) end diff --git a/sunspot/lib/sunspot/field_factory.rb b/sunspot/lib/sunspot/field_factory.rb index 6b728c091..c4f4e941f 100644 --- a/sunspot/lib/sunspot/field_factory.rb +++ b/sunspot/lib/sunspot/field_factory.rb @@ -193,7 +193,7 @@ def extract_value(model, options = {}) # TODO(ar3s3ru): how to handle incorrect field values? values = @extractor.value_for(model) adapter = options[:adapter] - unless values.is_a?(Array) || values.is_a?(ActiveRecord::Associations::CollectionProxy) + unless values.is_a?(Array) || rails_association?(values) raise 'Child documents field must be an Array of indexable documents' end if adapter.nil? || !adapter.respond_to?(:call) @@ -206,6 +206,13 @@ def extract_value(model, options = {}) def signature [field, ::RSolr::Document::CHILD_DOCUMENT_KEY] end + + private + + def rails_association?(values) + return false unless defined?(ActiveRecord::Associations::CollectionProxy) + values.is_a?(ActiveRecord::Associations::CollectionProxy) + end end end end From 5204690da269484cb94cd4d5d567b168f4695450 Mon Sep 17 00:00:00 2001 From: Doug Keller Date: Wed, 27 Mar 2019 17:06:45 -0400 Subject: [PATCH 11/20] Return child documents as hits --- sunspot/lib/sunspot/search/hit.rb | 17 ++++++++++------- sunspot/lib/sunspot/search/hit_enumerable.rb | 9 ++++++--- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/sunspot/lib/sunspot/search/hit.rb b/sunspot/lib/sunspot/search/hit.rb index f7a698b27..968f5df47 100644 --- a/sunspot/lib/sunspot/search/hit.rb +++ b/sunspot/lib/sunspot/search/hit.rb @@ -1,6 +1,6 @@ module Sunspot module Search - # + # # Hit objects represent the raw information returned by Solr for a single # document. As well as the primary key and class name, hit objects give # access to stored field values, keyword relevance score, and keyword @@ -9,21 +9,23 @@ module Search class Hit SPECIAL_KEYS = Set.new(%w(id type score)) #:nodoc: - # + # # Primary key of object associated with this hit, as string. # attr_reader :primary_key - # + # # Class name of object associated with this hit, as string. # attr_reader :class_name - # + # # Keyword relevance score associated with this result. Nil if this hit # is not from a keyword search. # attr_reader :score # + attr_reader :child_documents + attr_writer :result #:nodoc: def initialize(raw_hit, highlights, search) #:nodoc: @@ -33,8 +35,9 @@ def initialize(raw_hit, highlights, search) #:nodoc: @stored_values = raw_hit @stored_cache = {} @highlights = highlights + @child_documents = (raw_hit['_childDocuments_'] || []).map { |h| Hit.new(h, highlights, search) } end - + # # Returns all highlights for this hit when called without parameters. # When a field_name is provided, returns only the highlight for this field. @@ -55,7 +58,7 @@ def highlight(field_name) highlights(field_name).first end - # + # # Retrieve stored field value. For any attribute field configured with # :stored => true, the Hit object will contain the stored value for # that field. The value of this field will be typecast according to the @@ -80,7 +83,7 @@ def stored(field_name, dynamic_field_name = nil) @stored_cache[field_key] = stored_value(field_name, dynamic_field_name) end - # + # # Retrieve the instance associated with this hit. This is lazy-loaded, but # the first time it is called on any hit, all the hits for the search will # load their instances using the adapter's #load_all method. diff --git a/sunspot/lib/sunspot/search/hit_enumerable.rb b/sunspot/lib/sunspot/search/hit_enumerable.rb index 6e2b58aa9..091c98a07 100644 --- a/sunspot/lib/sunspot/search/hit_enumerable.rb +++ b/sunspot/lib/sunspot/search/hit_enumerable.rb @@ -18,7 +18,7 @@ def verified_hits hits.select { |h| h.result } end - # + # # Populate the Hit objects with their instances. This is invoked the first # time any hit has its instance requested, and all hits are loaded as a # batch. @@ -27,10 +27,13 @@ def populate_hits #:nodoc: id_hit_hash = Hash.new { |h, k| h[k] = {} } hits.each do |hit| id_hit_hash[hit.class_name][hit.primary_key] = hit + hit.child_documents.each do |child_hit| + id_hit_hash[child_hit.class_name][child_hit.primary_key] = child_hit + end end id_hit_hash.each_pair do |class_name, hits| ids = hits.map { |id, hit| hit.primary_key } - data_accessor = data_accessor_for(Util.full_const_get(class_name)) + data_accessor = data_accessor_for(Util.full_const_get(class_name)) hits_for_class = id_hit_hash[class_name] data_accessor.load_all(ids).each do |result| hit = hits_for_class.delete(Adapters::InstanceAdapter.adapt(result).id.to_s) @@ -53,7 +56,7 @@ def each_hit_with_result verified_hits.each { |hit| yield hit, hit.result } end - # + # # Get the data accessor that will be used to load a particular class out of # persistent storage. Data accessors can implement any methods that may be # useful for refining how data is loaded out of storage. When building a From 70ea5f7b4db40d79d444055d9365198d68e015d8 Mon Sep 17 00:00:00 2001 From: Doug Keller Date: Thu, 28 Mar 2019 11:08:46 -0400 Subject: [PATCH 12/20] Add with_child_documents to field list --- sunspot/lib/sunspot/dsl/scope.rb | 6 ++++++ sunspot/lib/sunspot/query/block_join.rb | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/sunspot/lib/sunspot/dsl/scope.rb b/sunspot/lib/sunspot/dsl/scope.rb index a1830988c..ecc77951d 100644 --- a/sunspot/lib/sunspot/dsl/scope.rb +++ b/sunspot/lib/sunspot/dsl/scope.rb @@ -22,6 +22,12 @@ def without_stored_fields @query.add_field_list(Sunspot::Query::FieldList.new([:id])) end + def with_child_documents + type_filter = Util.escape(@query.to_params[:fq][0]) + list = ["[child parentFilter=#{type_filter}]"] + @query.add_field_list(Sunspot::Query::FieldList.new([:*] + list) + end + # # Build a positive restriction. This method can take three forms: equality # restriction, restriction by another restriction, or identity diff --git a/sunspot/lib/sunspot/query/block_join.rb b/sunspot/lib/sunspot/query/block_join.rb index 672ff0511..2ed554693 100644 --- a/sunspot/lib/sunspot/query/block_join.rb +++ b/sunspot/lib/sunspot/query/block_join.rb @@ -180,7 +180,7 @@ def to_params class ParentWhich < Abstract alias some_children_filter secondary_filter - def all_parents_parts(escape: false) + def all_parents_parts # Use top-level scope (on parent type) as allParents filter. parts = scope.to_params[:fq].flatten parts.map { |v| Util.escape(v) } From 28234c56dbe52b0272a3d0f24e0a96cc3cf036b4 Mon Sep 17 00:00:00 2001 From: Doug Keller Date: Thu, 28 Mar 2019 11:34:12 -0400 Subject: [PATCH 13/20] Missing paren --- sunspot/lib/sunspot/dsl/scope.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sunspot/lib/sunspot/dsl/scope.rb b/sunspot/lib/sunspot/dsl/scope.rb index ecc77951d..de7d91c11 100644 --- a/sunspot/lib/sunspot/dsl/scope.rb +++ b/sunspot/lib/sunspot/dsl/scope.rb @@ -25,7 +25,7 @@ def without_stored_fields def with_child_documents type_filter = Util.escape(@query.to_params[:fq][0]) list = ["[child parentFilter=#{type_filter}]"] - @query.add_field_list(Sunspot::Query::FieldList.new([:*] + list) + @query.add_field_list(Sunspot::Query::FieldList.new([:*] + list)) end # From b50d3692b314946d5b696f54cb70e87fb40aa5b2 Mon Sep 17 00:00:00 2001 From: Doug Keller Date: Thu, 28 Mar 2019 11:51:00 -0400 Subject: [PATCH 14/20] Do not escape parent filter --- sunspot/lib/sunspot/dsl/scope.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sunspot/lib/sunspot/dsl/scope.rb b/sunspot/lib/sunspot/dsl/scope.rb index de7d91c11..1aab64348 100644 --- a/sunspot/lib/sunspot/dsl/scope.rb +++ b/sunspot/lib/sunspot/dsl/scope.rb @@ -23,8 +23,8 @@ def without_stored_fields end def with_child_documents - type_filter = Util.escape(@query.to_params[:fq][0]) - list = ["[child parentFilter=#{type_filter}]"] + parentFilter = @query.to_params[:fq][0] + list = ["[child parentFilter=#{parentFilter}]"] @query.add_field_list(Sunspot::Query::FieldList.new([:*] + list)) end From ddb7f243a065037da5ebe4d3cc7309004d4c5e7b Mon Sep 17 00:00:00 2001 From: Doug Keller Date: Fri, 29 Mar 2019 11:17:20 -0400 Subject: [PATCH 15/20] Add method for filtering child documents --- sunspot/lib/sunspot/search/hit.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/sunspot/lib/sunspot/search/hit.rb b/sunspot/lib/sunspot/search/hit.rb index 968f5df47..e70e22fcf 100644 --- a/sunspot/lib/sunspot/search/hit.rb +++ b/sunspot/lib/sunspot/search/hit.rb @@ -24,8 +24,6 @@ class Hit attr_reader :score # - attr_reader :child_documents - attr_writer :result #:nodoc: def initialize(raw_hit, highlights, search) #:nodoc: @@ -113,6 +111,11 @@ def to_param self.primary_key end + def children(type = nil) + return @child_documents if type.nil? + @child_documents.select { |d| d.class_name == type.name } + end + private def setup From dcb82522bc7b58612bb662b81cf2b6c331e3d189 Mon Sep 17 00:00:00 2001 From: Doug Keller Date: Fri, 29 Mar 2019 14:15:34 -0400 Subject: [PATCH 16/20] Verify child docs are not being indexed in Session#index --- sunspot/lib/sunspot/indexer.rb | 9 +-- sunspot/lib/sunspot/session.rb | 10 +++ sunspot_rails/lib/sunspot/rails/searchable.rb | 66 +++++++++---------- 3 files changed, 45 insertions(+), 40 deletions(-) diff --git a/sunspot/lib/sunspot/indexer.rb b/sunspot/lib/sunspot/indexer.rb index 76b40fc6f..c6dd4e706 100644 --- a/sunspot/lib/sunspot/indexer.rb +++ b/sunspot/lib/sunspot/indexer.rb @@ -100,13 +100,8 @@ def batcher # # Convert documents into hash of indexed properties # - def prepare_full_update(model, indexing_parent: false) + def prepare_full_update(model) document = document_for_full_update(model) - setup = setup_for_object(model) - if setup.is_child && !indexing_parent - raise 'Child documents must be indexed with their parent' - end - if boost = setup.document_boost_for(model) document.attrs[:boost] = boost end @@ -117,7 +112,7 @@ def prepare_full_update(model, indexing_parent: false) child_field_factory.populate_document( document, model, - adapter: ->(child_model) { prepare_full_update(child_model, indexing_parent: true) } + adapter: ->(child_model) { prepare_full_update(child_model) } ) end document diff --git a/sunspot/lib/sunspot/session.rb b/sunspot/lib/sunspot/session.rb index 15f455c6e..252ddbe71 100644 --- a/sunspot/lib/sunspot/session.rb +++ b/sunspot/lib/sunspot/session.rb @@ -87,6 +87,7 @@ def more_like_this(object, *types, &block) # def index(*objects) objects.flatten! + verify_indexing_parents!(objects) @adds += objects.length indexer.add(objects) end @@ -279,5 +280,14 @@ def setup_for_types(types) CompositeSetup.for(types) end end + + def verify_indexing_parents!(objects) + # http://yonik.com/solr-nested-objects#Limitations + # Child and parent documents must be indexed in the same block + objects.each do |object| + next unless Setup.for(object.class).is_child + raise 'Child documents must be indexed with their parent' + end + end end end diff --git a/sunspot_rails/lib/sunspot/rails/searchable.rb b/sunspot_rails/lib/sunspot/rails/searchable.rb index f308c5212..24bfff1be 100644 --- a/sunspot_rails/lib/sunspot/rails/searchable.rb +++ b/sunspot_rails/lib/sunspot/rails/searchable.rb @@ -1,6 +1,6 @@ module Sunspot #:nodoc: module Rails #:nodoc: - # + # # This module adds Sunspot functionality to ActiveRecord models. As well as # providing class and instance methods, it optionally adds lifecycle hooks # to automatically add and remove models from the Solr index as they are @@ -16,7 +16,7 @@ def included(base) #:nodoc: end module ActsAsMethods - # + # # Makes a class searchable if it is not already, or adds search # configuration if it is. Note that the options passed in are only used # the first time this method is called for a particular class; so, @@ -28,7 +28,7 @@ module ActsAsMethods # complete information on the functionality provided by that method. # # ==== Options (+options+) - # + # # :auto_index:: # Automatically index models in Solr when they are saved. # Default: true @@ -53,9 +53,9 @@ module ActsAsMethods # to ignore. # :include:: # Define default ActiveRecord includes, set this to allow ActiveRecord - # to load required associations when indexing. See ActiveRecord's + # to load required associations when indexing. See ActiveRecord's # documentation on eager-loading for examples on how to set this - # Default: [] + # Default: [] # :unless:: # Only index models in Solr if the method, proc or string evaluates # to false (e.g. :unless => :should_not_index? or @@ -109,7 +109,7 @@ def searchable(options = {}, &block) end end - # + # # This method is defined on all ActiveRecord::Base subclasses. It # is false for classes on which #searchable has not been called, and # true for classes on which #searchable has been called. @@ -138,7 +138,7 @@ class < nil) + # Post.index(:batch_size => nil) # # # index in batches of 50, commit when all batches complete - # Post.index(:batch_commit => false) + # Post.index(:batch_commit => false) # # # include the associated +author+ object when loading to index - # Post.index(:include => :author) + # Post.index(:include => :author) # def solr_index(opts={}) options = { @@ -315,22 +315,22 @@ def solr_atomic_update!(updates = {}) Sunspot.atomic_update!(self, updates) end - # + # # Return the IDs of records of this class that are indexed in Solr but # do not exist in the database. Under normal circumstances, this should # never happen, but this method is provided in case something goes # wrong. Usually you will want to rectify the situation by calling # #clean_index_orphans or #reindex - # + # # ==== Options (passed as a hash) # # batch_size:: Override default batch size with which to load records. - # + # # ==== Returns # # Array:: Collection of IDs that exist in Solr but not in the database def solr_index_orphans(opts={}) - batch_size = opts[:batch_size] || Sunspot.config.indexing.default_batch_size + batch_size = opts[:batch_size] || Sunspot.config.indexing.default_batch_size solr_page = 0 solr_ids = [] @@ -343,7 +343,7 @@ def solr_index_orphans(opts={}) return solr_ids - self.connection.select_values("SELECT id FROM #{quoted_table_name}").collect(&:to_i) end - # + # # Find IDs of records of this class that are indexed in Solr but do not # exist in the database, and remove them from Solr. Under normal # circumstances, this should not be necessary; this method is provided @@ -352,7 +352,7 @@ def solr_index_orphans(opts={}) # ==== Options (passed as a hash) # # batch_size:: Override default batch size with which to load records - # + # def solr_clean_index_orphans(opts={}) solr_index_orphans(opts).each do |id| new do |fake_instance| @@ -361,7 +361,7 @@ def solr_clean_index_orphans(opts={}) end end - # + # # Classes that have been defined as searchable return +true+ for this # method. # @@ -372,7 +372,7 @@ def solr_clean_index_orphans(opts={}) def searchable? true end - + def solr_execute_search(options = {}) inherited_attributes = [:include, :select, :scopes] options.assert_valid_keys(*inherited_attributes) @@ -393,10 +393,10 @@ def solr_execute_search_ids(options = {}) search = yield search.raw_results.map { |raw_result| raw_result.primary_key.to_i } end - + protected - - # + + # # Does some logging for benchmarking indexing performance # def solr_benchmark(batch_size, counter, &block) @@ -422,7 +422,7 @@ def self.included(base) #:nodoc: alias_method :atomic_update!, :solr_atomic_update! unless method_defined? :atomic_update! end end - # + # # Index the model in Solr. If the model is already indexed, it will be # updated. Using the defaults, you will usually not need to call this # method, as models are indexed automatically when they are created or @@ -434,7 +434,7 @@ def solr_index Sunspot.index(self) end - # + # # Index the model in Solr and immediately commit. See #index # def solr_index! @@ -457,8 +457,8 @@ def solr_atomic_update(updates = {}) def solr_atomic_update!(updates = {}) Sunspot.atomic_update!(self.class, self.id => updates) end - - # + + # # Remove the model from the Solr index. Using the defaults, this should # not be necessary, as models will automatically be removed from the # index when they are destroyed. If you disable automatic removal @@ -469,7 +469,7 @@ def solr_remove_from_index Sunspot.remove(self) end - # + # # Remove the model from the Solr index and commit immediately. See # #remove_from_index # From a0bee426845792e75e8b57a16d84fc32fc632b79 Mon Sep 17 00:00:00 2001 From: Doug Keller Date: Mon, 1 Apr 2019 13:23:35 -0400 Subject: [PATCH 17/20] Add support for sorting on child docs --- sunspot/lib/sunspot/dsl/field_query.rb | 6 +++++ sunspot/lib/sunspot/indexer.rb | 1 + sunspot/lib/sunspot/query/sort.rb | 35 ++++++++++++++++++-------- 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/sunspot/lib/sunspot/dsl/field_query.rb b/sunspot/lib/sunspot/dsl/field_query.rb index b66eeacac..e61aada49 100644 --- a/sunspot/lib/sunspot/dsl/field_query.rb +++ b/sunspot/lib/sunspot/dsl/field_query.rb @@ -52,6 +52,12 @@ def order_by_geodist(field_name, lat, lon, direction = nil) ) end + def order_by_child_document(field_name, direction = nil, block_join:) + @query.add_sort( + Sunspot::Query::Sort::ChildDocumentSort.new(field_name, block_join, direction) + ) + end + # # DEPRECATED Use order_by(:random) # diff --git a/sunspot/lib/sunspot/indexer.rb b/sunspot/lib/sunspot/indexer.rb index c6dd4e706..a53278219 100644 --- a/sunspot/lib/sunspot/indexer.rb +++ b/sunspot/lib/sunspot/indexer.rb @@ -102,6 +102,7 @@ def batcher # def prepare_full_update(model) document = document_for_full_update(model) + setup = setup_for_object(model) if boost = setup.document_boost_for(model) document.attrs[:boost] = boost end diff --git a/sunspot/lib/sunspot/query/sort.rb b/sunspot/lib/sunspot/query/sort.rb index dcc1bb7aa..31ff99198 100644 --- a/sunspot/lib/sunspot/query/sort.rb +++ b/sunspot/lib/sunspot/query/sort.rb @@ -1,11 +1,11 @@ module Sunspot module Query - # + # # The classes in this module implement query components that build sort # parameters for Solr. As well as regular sort on fields, there are several # "special" sorts that allow ordering for metrics calculated during the # search. - # + # module Sort #:nodoc: all DIRECTIONS = { :asc => 'asc', @@ -15,7 +15,7 @@ module Sort #:nodoc: all } class < Date: Mon, 1 Apr 2019 16:18:21 -0400 Subject: [PATCH 18/20] Change method name --- sunspot/lib/sunspot/search/hit_enumerable.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sunspot/lib/sunspot/search/hit_enumerable.rb b/sunspot/lib/sunspot/search/hit_enumerable.rb index 091c98a07..1806bfd16 100644 --- a/sunspot/lib/sunspot/search/hit_enumerable.rb +++ b/sunspot/lib/sunspot/search/hit_enumerable.rb @@ -27,7 +27,7 @@ def populate_hits #:nodoc: id_hit_hash = Hash.new { |h, k| h[k] = {} } hits.each do |hit| id_hit_hash[hit.class_name][hit.primary_key] = hit - hit.child_documents.each do |child_hit| + hit.children.each do |child_hit| id_hit_hash[child_hit.class_name][child_hit.primary_key] = child_hit end end From 51260612727d12f5bad6022beb0ddd5e2fd42a40 Mon Sep 17 00:00:00 2001 From: Doug Keller Date: Tue, 2 Apr 2019 13:20:46 -0400 Subject: [PATCH 19/20] Implicitly include child documents in field list when performing bjq --- sunspot/lib/sunspot/dsl/scope.rb | 6 ------ sunspot/lib/sunspot/query/block_join.rb | 13 ++++++++++++- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/sunspot/lib/sunspot/dsl/scope.rb b/sunspot/lib/sunspot/dsl/scope.rb index 1aab64348..a1830988c 100644 --- a/sunspot/lib/sunspot/dsl/scope.rb +++ b/sunspot/lib/sunspot/dsl/scope.rb @@ -22,12 +22,6 @@ def without_stored_fields @query.add_field_list(Sunspot::Query::FieldList.new([:id])) end - def with_child_documents - parentFilter = @query.to_params[:fq][0] - list = ["[child parentFilter=#{parentFilter}]"] - @query.add_field_list(Sunspot::Query::FieldList.new([:*] + list)) - end - # # Build a positive restriction. This method can take three forms: equality # restriction, restriction by another restriction, or identity diff --git a/sunspot/lib/sunspot/query/block_join.rb b/sunspot/lib/sunspot/query/block_join.rb index 2ed554693..fe54c4eed 100644 --- a/sunspot/lib/sunspot/query/block_join.rb +++ b/sunspot/lib/sunspot/query/block_join.rb @@ -203,8 +203,19 @@ def secondary_filter q end + def field_list_string + parts = [] + parts << '[child' + parts << 'parentFilter="' + all_parents_parts[0] + '"' + parts << 'childFilter="' + secondary_filter.map { |f| Util.escape(f) }.join(' AND ') + '"]' + parts.join(' ') + end + def to_params - { q: render_query_string('parent', 'which') } + { + q: render_query_string('parent', 'which'), + fl: [:id] + [field_list_string] + } end end end From 8dac0c9ad49001485a8919d22128cd324b06cd0b Mon Sep 17 00:00:00 2001 From: Doug Keller Date: Tue, 2 Apr 2019 16:19:19 -0400 Subject: [PATCH 20/20] Revert to single child field factory --- sunspot/lib/sunspot/dsl/fields.rb | 6 +----- sunspot/lib/sunspot/indexer.rb | 6 +++--- sunspot/lib/sunspot/setup.rb | 5 ++--- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/sunspot/lib/sunspot/dsl/fields.rb b/sunspot/lib/sunspot/dsl/fields.rb index 81e75f963..7b70ceb3f 100644 --- a/sunspot/lib/sunspot/dsl/fields.rb +++ b/sunspot/lib/sunspot/dsl/fields.rb @@ -73,12 +73,8 @@ def text(*names, &block) # # field:: Field name at object level that contains child documents # - def child_documents(field, type:) + def child_documents(field) Sunspot::Util.ensure_child_documents_support - setup = Sunspot::Setup.for(type) - unless setup.is_child - raise 'Child documents must be flagged as is_child' - end @setup.add_child_field_factory(field) end diff --git a/sunspot/lib/sunspot/indexer.rb b/sunspot/lib/sunspot/indexer.rb index a53278219..70702bded 100644 --- a/sunspot/lib/sunspot/indexer.rb +++ b/sunspot/lib/sunspot/indexer.rb @@ -109,8 +109,8 @@ def prepare_full_update(model) setup.all_field_factories.each do |field_factory| field_factory.populate_document(document, model) end - setup.child_field_factories.each do |child_field_factory| - child_field_factory.populate_document( + unless setup.child_field_factory.nil? + setup.child_field_factory.populate_document( document, model, adapter: ->(child_model) { prepare_full_update(child_model) } @@ -124,7 +124,7 @@ def prepare_atomic_update(clazz, id, updates = {}) setup = setup_for_class(clazz) # Child documents must be re-indexed with parent at each update, # otherwise Solr would discard them. - unless setup.child_field_factories.empty? + unless setup.child_field_factory.nil? raise 'Objects with child documents can\'t perform atomic updates' end setup.all_field_factories.each do |field_factory| diff --git a/sunspot/lib/sunspot/setup.rb b/sunspot/lib/sunspot/setup.rb index aff1111bf..83ded7f44 100644 --- a/sunspot/lib/sunspot/setup.rb +++ b/sunspot/lib/sunspot/setup.rb @@ -5,7 +5,7 @@ module Sunspot # class Setup #:nodoc: attr_reader :class_object_id, - :child_field_factories + :child_field_factory attr_accessor :is_child @@ -19,7 +19,6 @@ def initialize(clazz) @more_like_this_field_factories_cache = Hash.new { |h, k| h[k] = [] } @dsl = DSL::Fields.new(self) @document_boost_extractor = nil - @child_field_factories = [] add_field_factory(:class, Type::ClassType.instance) end @@ -99,7 +98,7 @@ def add_dynamic_field_factory(name, type, options = {}, &block) # field:: The document field that contains the child documents. # def add_child_field_factory(field) - @child_field_factories << FieldFactory::Child.new(field) + @child_field_factory = FieldFactory::Child.new(field) end #