Merge pull request #1018 from projecthydra/solr-query-builder

Simplify ActiveFedora::SolrQueryBuilder

lib/active_fedora/indexing_service.rb

@@ -16,15 +16,15 @@ def initialize(obj)
     end
 
     def self.profile_solr_name
-      @profile_solr_name ||= ActiveFedora::SolrQueryBuilder.solr_name("object_profile", :displayable)
+      @profile_solr_name ||= ActiveFedora.index_field_mapper.solr_name("object_profile", :displayable)
     end
 
     def self.create_time_solr_name
-      @create_time_solr_name ||= ActiveFedora::SolrQueryBuilder.solr_name('system_create', :stored_sortable, type: :date)
+      @create_time_solr_name ||= ActiveFedora.index_field_mapper.solr_name('system_create', :stored_sortable, type: :date)
     end
 
     def self.modified_time_solr_name
-      @modified_time_solr_name ||= ActiveFedora::SolrQueryBuilder.solr_name('system_modified', :stored_sortable, type: :date)
+      @modified_time_solr_name ||= ActiveFedora.index_field_mapper.solr_name('system_modified', :stored_sortable, type: :date)
     end
 
     def rdf_service

lib/active_fedora/qualified_dublin_core_datastream.rb

@@ -147,7 +147,7 @@ def to_solr(solr_doc = {}, _opts = {}) # :nodoc:
       @fields.each do |field_key, field_info|
         things = send(field_key)
         next unless things
-        field_symbol = ActiveFedora::SolrQueryBuilder.solr_name(field_key, type: field_info[:type])
+        field_symbol = ActiveFedora.index_field_mapper.solr_name(field_key, type: field_info[:type])
         things.val.each do |val|
           ::Solrizer::Extractor.insert_solr_field_value(solr_doc, field_symbol, val)
         end

lib/active_fedora/query_result_builder.rb

@@ -29,6 +29,6 @@ def self.class_from_solr_document(hit, opts = {})
       ActiveFedora::SolrHit.for(hit).model(opts)
     end
 
-    HAS_MODEL_SOLR_FIELD = SolrQueryBuilder.solr_name("has_model", :symbol).freeze
+    HAS_MODEL_SOLR_FIELD = ActiveFedora.index_field_mapper.solr_name("has_model", :symbol).freeze
   end
 end

lib/active_fedora/rdf/datastream_indexing.rb

@@ -13,7 +13,7 @@ def primary_solr_name(field, file_path)
       config = self.class.config_for_term_or_uri(field)
       return nil unless config && config.behaviors # punt on index names for deep nodes!
       config.behaviors.each do |behavior|
-        result = ActiveFedora::SolrQueryBuilder.solr_name(apply_prefix(field, file_path), behavior, type: config.type)
+        result = ActiveFedora.index_field_mapper.solr_name(apply_prefix(field, file_path), behavior, type: config.type)
         return result if Solrizer::DefaultDescriptors.send(behavior).evaluate_suffix(:text).stored?
       end
       raise RuntimeError "no stored fields were found"

lib/active_fedora/reflection.rb

@@ -198,7 +198,7 @@ def predicate_for_solr
 
       def solr_key
         @solr_key ||= begin
-          ActiveFedora::SolrQueryBuilder.solr_name(predicate_for_solr, :symbol)
+          ActiveFedora.index_field_mapper.solr_name(predicate_for_solr, :symbol)
         end
       end
 

lib/active_fedora/relation/finder_methods.rb

@@ -199,7 +199,7 @@ def find_one(id, cast = nil)
       if where_values.empty?
         load_from_fedora(id, cast)
       else
-        conditions = where_values + [ActiveFedora::SolrQueryBuilder.raw_query(ActiveFedora.id_field, id)]
+        conditions = where_values + [ActiveFedora::SolrQueryBuilder.construct_query(ActiveFedora.id_field => id)]
         query = conditions.join(" AND ".freeze)
         to_enum(:find_each, query, {}).to_a.first
       end
@@ -289,7 +289,7 @@ def condition_to_clauses(key, value)
       def field_name_for(key)
         if @klass.delegated_attributes.key?(key)
           # TODO: Check to see if `key' is a possible solr field for this class, if it isn't try :searchable instead
-          ActiveFedora::SolrQueryBuilder.solr_name(key, :stored_searchable, type: :string)
+          ActiveFedora.index_field_mapper.solr_name(key, :stored_searchable, type: :string)
         elsif key == :id
           ActiveFedora.id_field
         else

lib/active_fedora/simple_datastream.rb

@@ -79,7 +79,7 @@ def to_solr(solr_doc = {}, _opts = {}) # :nodoc:
         next if field_key == :location ## FIXME HYDRA-825
         things = send(field_key)
         next unless things
-        field_symbol = ActiveFedora::SolrQueryBuilder.solr_name(field_key, type: field_info[:type])
+        field_symbol = ActiveFedora.index_field_mapper.solr_name(field_key, type: field_info[:type])
         things.val.each do |val|
           ::Solrizer::Extractor.insert_solr_field_value(solr_doc, field_symbol, val.to_s)
         end

lib/active_fedora/solr_query_builder.rb

@@ -1,7 +1,5 @@
 module ActiveFedora
   module SolrQueryBuilder
-    PARSED_SUFFIX = '_tesim'.freeze
-
     class << self
       # Construct a solr query for a list of ids
       # This is used to get a solr response based on the list of ids in an object's RELS-EXT relationhsips
@@ -17,10 +15,13 @@ def construct_query_for_ids(id_array)
       # @param [String] key
       # @param [String] value
       def raw_query(key, value)
+        Deprecation.warn(ActiveFedora::Base, 'ActiveFedora::SolrQueryBuilder.raw_query is deprecated and will be removed in ActiveFedora 10.0. Use .construct_query instead.')
         "_query_:\"{!raw f=#{key}}#{value.gsub('"', '\"')}\""
       end
 
+      # @deprecated
       def solr_name(*args)
+        Deprecation.warn(ActiveFedora::Base, 'ActiveFedora::SolrQueryBuilder.solr_name is deprecated and will be removed in ActiveFedora 10.0. Use ActiveFedora.index_field_mapper.solr_name instead.')
         ActiveFedora.index_field_mapper.solr_name(*args)
       end
 
@@ -29,7 +30,7 @@ def solr_name(*args)
       # @param [String] join_with ('AND') the value we're joining the clauses with
       # @example
       #   construct_query_for_rel [[:has_model, "info:fedora/afmodel:ComplexCollection"], [:has_model, "info:fedora/afmodel:ActiveFedora_Base"]], 'OR'
-      #   # => _query_:"{!raw f=has_model_ssim}info:fedora/afmodel:ComplexCollection" OR _query_:"{!raw f=has_model_ssim}info:fedora/afmodel:ActiveFedora_Base"
+      #   # => _query_:"{!field f=has_model_ssim}info:fedora/afmodel:ComplexCollection" OR _query_:"{!field f=has_model_ssim}info:fedora/afmodel:ActiveFedora_Base"
       #
       #   construct_query_for_rel [[Book.reflect_on_association(:library), "foo/bar/baz"]]
       def construct_query_for_rel(field_pairs, join_with = ' AND ')
@@ -43,7 +44,7 @@ def construct_query_for_rel(field_pairs, join_with = ' AND ')
       # @return [String] a solr query
       # @example
       #   construct_query([['library_id_ssim', '123'], ['owner_ssim', 'Fred']])
-      #   # => "_query_:\"{!raw f=library_id_ssim}123\" AND _query_:\"{!raw f=owner_ssim}Fred\""
+      #   # => "_query_:\"{!field f=library_id_ssim}123\" AND _query_:\"{!field f=owner_ssim}Fred\""
       def construct_query(field_pairs, join_with = ' AND ')
         pairs_to_clauses(field_pairs).join(join_with)
       end
@@ -66,29 +67,14 @@ def condition_to_clauses(field, values)
           values << nil if values.empty?
           values.map do |value|
             if value.present?
-              if parsed?(field)
-                # If you do a raw query on a parsed field you won't get the matches you expect.
-                "#{field}:#{solr_escape(value)}"
-              else
-                raw_query(field, value)
-              end
+              field_query(field, value)
             else
               # Check that the field is not present. In SQL: "WHERE field IS NULL"
               "-#{field}:[* TO *]"
             end
           end
         end
 
-        def parsed?(field)
-          field.end_with?(PARSED_SUFFIX)
-        end
-
-        # Adds esaping for spaces which are not handled by RSolr.solr_escape
-        # See rsolr/rsolr#101
-        def solr_escape(terms)
-          RSolr.solr_escape(terms).gsub(/\s+/, "\\ ")
-        end
-
         # Given a list of pairs (e.g. [field name, values]), convert the field names
         # to solr names
         # @param [Array<Array>] pairs a list of pairs of property name and values
@@ -97,8 +83,8 @@ def solr_escape(terms)
         #   property_values_to_solr([['library_id', '123'], ['owner', 'Fred']])
         #   # => [['library_id_ssim', '123'], ['owner_ssim', 'Fred']]
         def property_values_to_solr(pairs)
-          pairs.each_with_object([]) do |(property, value), list|
-            list << [solr_field(property), value]
+          pairs.map do |(property, value)|
+            [solr_field(property), value]
           end
         end
 
@@ -109,9 +95,16 @@ def solr_field(field)
           when ActiveFedora::Reflection::AssociationReflection
             field.solr_key
           else
-            solr_name(field, :symbol)
+            ActiveFedora.index_field_mapper.solr_name(field, :symbol)
           end
         end
+
+        # Create a raw query clause suitable for sending to solr as an fq element
+        # @param [String] key
+        # @param [String] value
+        def field_query(key, value)
+          "_query_:\"{!field f=#{key}}#{value.gsub('"', '\"')}\""
+        end
     end
   end
 end

lib/active_fedora/solr_service.rb

@@ -83,8 +83,8 @@ def construct_query_for_pids(id_array)
       # @param [String] key
       # @param [String] value
       def raw_query(key, value)
-        Deprecation.warn SolrService, "SolrService.raw_query is deprecated. Use SolrQueryBuilder.raw_query instead. This will be removed in active-fedora 10.0"
-        SolrQueryBuilder.raw_query(key, value)
+        Deprecation.warn SolrService, "SolrService.raw_query is deprecated. Use SolrQueryBuilder.construct_query instead. This will be removed in active-fedora 10.0"
+        SolrQueryBuilder.construct_query(key, value)
       end
 
       def solr_name(*args)
@@ -97,7 +97,7 @@ def solr_name(*args)
       # @param [String] join_with ('AND') the value we're joining the clauses with
       # @example
       #   construct_query_for_rel [[:has_model, "info:fedora/afmodel:ComplexCollection"], [:has_model, "info:fedora/afmodel:ActiveFedora_Base"]], 'OR'
-      #   # => _query_:"{!raw f=has_model_ssim}info:fedora/afmodel:ComplexCollection" OR _query_:"{!raw f=has_model_ssim}info:fedora/afmodel:ActiveFedora_Base"
+      #   # => _query_:"{!field f=has_model_ssim}info:fedora/afmodel:ComplexCollection" OR _query_:"{!field f=has_model_ssim}info:fedora/afmodel:ActiveFedora_Base"
       #
       #   construct_query_for_rel [[Book.reflect_on_association(:library), "foo/bar/baz"]]
       def construct_query_for_rel(field_pairs, join_with = 'AND')

spec/integration/base_spec.rb

@@ -22,8 +22,8 @@ class MockAFBaseRelationship < ActiveFedora::Base
         obj = described_class.find(@obj.id)
         expect(obj.foo).to_not be_new_record
         expect(obj.foo.person).to eq ['bob']
-        person_field = ActiveFedora::SolrQueryBuilder.solr_name('foo__person', type: :string)
-        solr_result = ActiveFedora::SolrService.query("{!raw f=id}#{@obj.id}", fl: "id #{person_field}").first
+        person_field = ActiveFedora.index_field_mapper.solr_name('foo__person', type: :string)
+        solr_result = ActiveFedora::SolrService.query("{!field f=id}#{@obj.id}", fl: "id #{person_field}").first
         expect(solr_result).to eq("id" => @obj.id, person_field => ['bob'])
       end
     end
@@ -41,7 +41,7 @@ class MockAFBaseRelationship < ActiveFedora::Base
         end
         it "saves the datastream." do
           expect(MockAFBaseRelationship.find(@release.id).foo.person).to eq ['frank']
-          person_field = ActiveFedora::SolrQueryBuilder.solr_name('foo__person', type: :string)
+          person_field = ActiveFedora.index_field_mapper.solr_name('foo__person', type: :string)
           expect(ActiveFedora::SolrService.query("id:\"#{@release.id}\"", fl: "id #{person_field}").first).to eq("id" => @release.id, person_field => ['frank'])
         end
       end
@@ -164,7 +164,7 @@ class ExampleBase < ActiveFedora::Base
     let(:obj) { ExampleBase.new }
     it "configures properties and solrize them" do
       obj.title = ["Test"]
-      expect(obj.to_solr[ActiveFedora::SolrQueryBuilder.solr_name("title", :symbol)]).to eq ["Test"]
+      expect(obj.to_solr[ActiveFedora.index_field_mapper.solr_name("title", :symbol)]).to eq ["Test"]
     end
   end
 

spec/integration/full_featured_model_spec.rb

@@ -135,7 +135,7 @@ class OralHistory < ActiveFedora::Base
     @properties_sample_values.each_pair do |field, value|
       next if field == :hard_copy_availability # FIXME: HYDRA-824
       next if field == :location # FIXME HYDRA-825
-      expect((@solr_result[ActiveFedora::SolrQueryBuilder.solr_name(field, type: :string)] || @solr_result[ActiveFedora::SolrQueryBuilder.solr_name(field, type: :date)])).to eq [::Solrizer::Extractor.format_node_value(value)]
+      expect((@solr_result[ActiveFedora.index_field_mapper.solr_name(field, type: :string)] || @solr_result[ActiveFedora.index_field_mapper.solr_name(field, type: :date)])).to eq [::Solrizer::Extractor.format_node_value(value)]
     end
 
     @dublin_core_sample_values.each_pair do |field, value|

spec/integration/ntriples_datastream_spec.rb

@@ -92,21 +92,21 @@ class RdfTest < ActiveFedora::Base
       subject.date_uploaded = [Date.parse('2012-11-02')]
       expect(subject.date_uploaded.first).to be_kind_of Date
       solr_document = subject.to_solr
-      expect(solr_document[ActiveFedora::SolrQueryBuilder.solr_name('rdf__date_uploaded', type: :date)]).to eq ['2012-11-02T00:00:00Z']
+      expect(solr_document[ActiveFedora.index_field_mapper.solr_name('rdf__date_uploaded', type: :date)]).to eq ['2012-11-02T00:00:00Z']
     end
     it "handles integers" do
       subject.filesize = 12_345
       expect(subject.filesize).to be_kind_of Fixnum
       solr_document = subject.to_solr
-      expect(solr_document[ActiveFedora::SolrQueryBuilder.solr_name('rdf__filesize', :stored_sortable, type: :integer)]).to eq '12345'
+      expect(solr_document[ActiveFedora.index_field_mapper.solr_name('rdf__filesize', :stored_sortable, type: :integer)]).to eq '12345'
     end
   end
 
   it "produces a solr document" do
     @subject = RdfTest.new(title: "War and Peace")
     solr_document = @subject.to_solr
-    expect(solr_document[ActiveFedora::SolrQueryBuilder.solr_name('rdf__title', :facetable)]).to eq ["War and Peace"]
-    expect(solr_document[ActiveFedora::SolrQueryBuilder.solr_name('rdf__title', type: :string)]).to eq ["War and Peace"]
+    expect(solr_document[ActiveFedora.index_field_mapper.solr_name('rdf__title', :facetable)]).to eq ["War and Peace"]
+    expect(solr_document[ActiveFedora.index_field_mapper.solr_name('rdf__title', type: :string)]).to eq ["War and Peace"]
   end
 
   it "sets and recall values" do

spec/integration/om_datastream_spec.rb

@@ -72,7 +72,7 @@ class ModsArticle2 < ActiveFedora::Base
       obj.reload
     end
     it "solrizes terms with :type=>'date' to *_dt solr terms" do
-      expect(obj.to_solr[ActiveFedora::SolrQueryBuilder.solr_name('desc_metadata__journal_issue_publication_date', type: :date)]).to eq ['2012-11-02T00:00:00Z']
+      expect(obj.to_solr[ActiveFedora.index_field_mapper.solr_name('desc_metadata__journal_issue_publication_date', type: :date)]).to eq ['2012-11-02T00:00:00Z']
     end
   end
 end

spec/integration/relation_delegation_spec.rb

@@ -16,7 +16,7 @@ class Basic < ActiveFedora::Base
 
         def to_solr(doc = {})
           doc = super
-          doc[ActiveFedora::SolrQueryBuilder.solr_name('foo', :sortable)] = doc[ActiveFedora::SolrQueryBuilder.solr_name('foo', type: :string)]
+          doc[ActiveFedora.index_field_mapper.solr_name('foo', :sortable)] = doc[ActiveFedora.index_field_mapper.solr_name('foo', type: :string)]
           doc
         end
       end

spec/integration/scoped_query_spec.rb

@@ -16,7 +16,7 @@ class Basic < ActiveFedora::Base
 
         def to_solr(doc = {})
           doc = super
-          doc[ActiveFedora::SolrQueryBuilder.solr_name('foo', :sortable)] = doc[ActiveFedora::SolrQueryBuilder.solr_name('foo', type: :string)]
+          doc[ActiveFedora.index_field_mapper.solr_name('foo', :sortable)] = doc[ActiveFedora.index_field_mapper.solr_name('foo', type: :string)]
           doc
         end
       end
@@ -64,12 +64,12 @@ def to_solr(doc = {})
       end
 
       it "queries" do
-        field = ActiveFedora::SolrQueryBuilder.solr_name('foo', type: :string)
+        field = ActiveFedora.index_field_mapper.solr_name('foo', type: :string)
         expect(ModelIntegrationSpec::Basic.where(field => 'Beta')).to eq [test_instance1]
         expect(ModelIntegrationSpec::Basic.where('foo' => 'Beta')).to eq [test_instance1]
       end
       it "orders" do
-        expect(ModelIntegrationSpec::Basic.order(ActiveFedora::SolrQueryBuilder.solr_name('foo', :sortable) + ' asc')).to eq [test_instance2, test_instance1, test_instance3]
+        expect(ModelIntegrationSpec::Basic.order(ActiveFedora.index_field_mapper.solr_name('foo', :sortable) + ' asc')).to eq [test_instance2, test_instance1, test_instance3]
       end
       it "limits" do
         expect(ModelIntegrationSpec::Basic.limit(1)).to eq [test_instance1]
@@ -79,39 +79,39 @@ def to_solr(doc = {})
       end
 
       it "chains queries" do
-        expect(ModelIntegrationSpec::Basic.where(ActiveFedora::SolrQueryBuilder.solr_name('bar', type: :string) => 'Peanuts').order(ActiveFedora::SolrQueryBuilder.solr_name('foo', :sortable) + ' asc').limit(1)).to eq [test_instance2]
+        expect(ModelIntegrationSpec::Basic.where(ActiveFedora.index_field_mapper.solr_name('bar', type: :string) => 'Peanuts').order(ActiveFedora.index_field_mapper.solr_name('foo', :sortable) + ' asc').limit(1)).to eq [test_instance2]
       end
 
       it "wraps string conditions with parentheses" do
         expect(ModelIntegrationSpec::Basic.where("foo:bar OR bar:baz").where_values).to eq ["(foo:bar OR bar:baz)"]
       end
 
       it "chains where queries" do
-        first_condition = { ActiveFedora::SolrQueryBuilder.solr_name('bar', type: :string) => 'Peanuts' }
+        first_condition = { ActiveFedora.index_field_mapper.solr_name('bar', type: :string) => 'Peanuts' }
         second_condition = "foo_tesim:bar"
         where_values = ModelIntegrationSpec::Basic.where(first_condition)
                                                   .where(second_condition).where_values
-        expect(where_values).to eq ["bar_tesim:Peanuts",
+        expect(where_values).to eq ["_query_:\"{!field f=bar_tesim}Peanuts\"",
                                     "(foo_tesim:bar)"]
       end
 
       it "chains count" do
-        expect(ModelIntegrationSpec::Basic.where(ActiveFedora::SolrQueryBuilder.solr_name('bar', type: :string) => 'Peanuts').count).to eq 2
+        expect(ModelIntegrationSpec::Basic.where(ActiveFedora.index_field_mapper.solr_name('bar', type: :string) => 'Peanuts').count).to eq 2
       end
 
       it "calling first should not affect the relation's ability to get all results later" do
-        relation = ModelIntegrationSpec::Basic.where(ActiveFedora::SolrQueryBuilder.solr_name('bar', type: :string) => 'Peanuts')
+        relation = ModelIntegrationSpec::Basic.where(ActiveFedora.index_field_mapper.solr_name('bar', type: :string) => 'Peanuts')
         expect { relation.first }.not_to change { relation.to_a.size }
       end
 
       it "calling where should not affect the relation's ability to get all results later" do
-        relation = ModelIntegrationSpec::Basic.where(ActiveFedora::SolrQueryBuilder.solr_name('bar', type: :string) => 'Peanuts')
-        expect { relation.where(ActiveFedora::SolrQueryBuilder.solr_name('foo', type: :string) => 'bar') }.not_to change { relation.to_a.size }
+        relation = ModelIntegrationSpec::Basic.where(ActiveFedora.index_field_mapper.solr_name('bar', type: :string) => 'Peanuts')
+        expect { relation.where(ActiveFedora.index_field_mapper.solr_name('foo', type: :string) => 'bar') }.not_to change { relation.to_a.size }
       end
 
       it "calling order should not affect the original order of the relation later" do
-        relation = ModelIntegrationSpec::Basic.order(ActiveFedora::SolrQueryBuilder.solr_name('foo', :sortable) + ' asc')
-        expect { relation.order(ActiveFedora::SolrQueryBuilder.solr_name('foo', :sortable) + ' desc') }.not_to change { relation.to_a }
+        relation = ModelIntegrationSpec::Basic.order(ActiveFedora.index_field_mapper.solr_name('foo', :sortable) + ' asc')
+        expect { relation.order(ActiveFedora.index_field_mapper.solr_name('foo', :sortable) + ' desc') }.not_to change { relation.to_a }
       end
     end