Use field queries instead of raw queries

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
@@ -32,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 ')
@@ -46,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
@@ -69,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
@@ -100,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
 
@@ -115,6 +98,13 @@ def solr_field(field)
             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

@@ -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

@@ -23,7 +23,7 @@ class MockAFBaseRelationship < ActiveFedora::Base
         expect(obj.foo).to_not be_new_record
         expect(obj.foo.person).to eq ['bob']
         person_field = ActiveFedora.index_field_mapper.solr_name('foo__person', type: :string)
-        solr_result = ActiveFedora::SolrService.query("{!raw f=id}#{@obj.id}", fl: "id #{person_field}").first
+        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

spec/integration/scoped_query_spec.rb

@@ -91,7 +91,7 @@ def to_solr(doc = {})
         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
 

spec/unit/finder_methods_spec.rb

@@ -46,13 +46,13 @@ def initialize
 
     context "when value is an id" do
       let(:value) { 'one/two/three' }
-      it { is_expected.to eq "_query_:\"{!raw f=library_id}one/two/three\"" }
+      it { is_expected.to eq "_query_:\"{!field f=library_id}one/two/three\"" }
     end
 
     context "when value is an array" do
       let(:value) { ['one', 'four'] }
-      it { is_expected.to eq "_query_:\"{!raw f=library_id}one\" AND " \
-                             "_query_:\"{!raw f=library_id}four\"" }
+      it { is_expected.to eq "_query_:\"{!field f=library_id}one\" AND " \
+                             "_query_:\"{!field f=library_id}four\"" }
     end
   end
 

spec/unit/query_spec.rb

@@ -8,7 +8,7 @@ class Basic < ActiveFedora::Base
     end
   end
   let(:sort_query) { ActiveFedora.index_field_mapper.solr_name("system_create", :stored_sortable, type: :date) + ' asc' }
-  let(:model_query) { "_query_:\"{!raw f=has_model_ssim}SpecModel::Basic\"" }
+  let(:model_query) { "_query_:\"{!field f=has_model_ssim}SpecModel::Basic\"" }
 
   after(:all) do
     Object.send(:remove_const, :SpecModel)
@@ -77,9 +77,9 @@ class Basic < ActiveFedora::Base
     let(:relation) { ActiveFedora::Relation.new(SpecModel::Basic) }
     let(:solr) { ActiveFedora::SolrService.instance.conn }
     let(:expected_query) { "#{model_query} AND " \
-                           "_query_:\"{!raw f=foo}bar\" AND " \
-                           "_query_:\"{!raw f=baz}quix\" AND " \
-                           "_query_:\"{!raw f=baz}quack\"" }
+                           "_query_:\"{!field f=foo}bar\" AND " \
+                           "_query_:\"{!field f=baz}quix\" AND " \
+                           "_query_:\"{!field f=baz}quack\"" }
     let(:expected_params) { { params: { sort: [sort_query], fl: 'id', q: expected_query, qt: 'standard' } } }
     let(:expected_sort_params) { { params: { sort: ["title_t desc"], fl: 'id', q: expected_query, qt: 'standard' } } }
     let(:mock_docs) { [{ "id" => "changeme:30" }, { "id" => "changeme:22" }] }
@@ -136,9 +136,9 @@ class Basic < ActiveFedora::Base
     describe "with conditions" do
       let(:solr) { ActiveFedora::SolrService.instance.conn }
       let(:expected_query) { "#{model_query} AND " \
-                             "_query_:\"{!raw f=foo}bar\" AND " \
-                             "_query_:\"{!raw f=baz}quix\" AND " \
-                             "_query_:\"{!raw f=baz}quack\"" }
+                             "_query_:\"{!field f=foo}bar\" AND " \
+                             "_query_:\"{!field f=baz}quix\" AND " \
+                             "_query_:\"{!field f=baz}quack\"" }
       let(:expected_params) { { params: { sort: [sort_query], fl: 'id', q: expected_query, qt: 'standard' } } }
       let(:mock_docs) { [{ "id" => "changeme-30" }, { "id" => "changeme-22" }] }
 
@@ -157,9 +157,9 @@ class Basic < ActiveFedora::Base
     describe "with conditions hash" do
       let(:solr) { ActiveFedora::SolrService.instance.conn }
       let(:expected_query) { "#{model_query} AND " \
-                             "_query_:\"{!raw f=foo}bar\" AND " \
-                             "_query_:\"{!raw f=baz}quix\" AND " \
-                             "_query_:\"{!raw f=baz}quack\"" }
+                             "_query_:\"{!field f=foo}bar\" AND " \
+                             "_query_:\"{!field f=baz}quix\" AND " \
+                             "_query_:\"{!field f=baz}quack\"" }
       let(:expected_params) { { params: { sort: [sort_query], fl: 'id', q: expected_query, qt: 'standard' } } }
       let(:mock_docs) { double('docs') }
 
@@ -252,9 +252,9 @@ class Basic < ActiveFedora::Base
 
     context "with a hash of conditions" do
       let(:expected_query) { "#{model_query} AND " \
-                             "_query_:\"{!raw f=foo}bar\" AND " \
-                             "_query_:\"{!raw f=baz}quix\" AND " \
-                             "_query_:\"{!raw f=baz}quack\"" }
+                             "_query_:\"{!field f=foo}bar\" AND " \
+                             "_query_:\"{!field f=baz}quix\" AND " \
+                             "_query_:\"{!field f=baz}quack\"" }
       let(:conditions) { { foo: 'bar', baz: ['quix', 'quack'] } }
 
       it "makes a query to solr and returns the results" do
@@ -264,9 +264,9 @@ class Basic < ActiveFedora::Base
 
     context "with quotes in the params" do
       let(:expected_query) { "#{model_query} AND " \
-                             "_query_:\"{!raw f=foo}9\\\" Nails\" AND " \
-                             "_query_:\"{!raw f=baz}7\\\" version\" AND " \
-                             "_query_:\"{!raw f=baz}quack\"" }
+                             "_query_:\"{!field f=foo}9\\\" Nails\" AND " \
+                             "_query_:\"{!field f=baz}7\\\" version\" AND " \
+                             "_query_:\"{!field f=baz}quack\"" }
       let(:conditions) { { foo: '9" Nails', baz: ['7" version', 'quack'] } }
 
       it "escapes quotes" do
@@ -279,7 +279,7 @@ class Basic < ActiveFedora::Base
 
       context "with a hash" do
         let(:conditions) { { baz: 'quack' } }
-        let(:expected_query) { "_query_:\"{!raw f=baz}quack\"" }
+        let(:expected_query) { "_query_:\"{!field f=baz}quack\"" }
         it "doesn't use the class if it's called on AF:Base " do
           expect(subject).to eq mock_result
         end

spec/unit/solr_config_options_spec.rb

@@ -27,9 +27,9 @@ class Basic < ActiveFedora::Base
     it "is used by ActiveFedora::Base#search_with_conditions" do
       mock_response = double("SolrResponse")
       expect(ActiveFedora::SolrService).to receive(:query)
-        .with("_query_:\"{!raw f=has_model_ssim}SolrSpecModel::Basic\" AND " \
-              "_query_:\"{!raw f=#{field}}changeme:30\"",
-              sort: ["#{ActiveFedora.index_field_mapper.solr_name('system_create', :stored_sortable, type: :date)} asc"])
+        .with("_query_:\"{!field f=has_model_ssim}SolrSpecModel::Basic\" AND " \
+              "_query_:\"{!field f=#{field}}changeme:30\"",
+              sort: ["#{described_class.index_field_mapper.solr_name('system_create', :stored_sortable, type: :date)} asc"])
         .and_return(mock_response)
 
       expect(SolrSpecModel::Basic.search_with_conditions(id: "changeme:30")).to equal(mock_response)

spec/unit/solr_query_builder_spec.rb

@@ -9,6 +9,12 @@
     end
   end
 
+  describe "construct_query" do
+    it "generates a query clause" do
+      expect(described_class.construct_query('id' => "my:_ID1_")).to eq '_query_:"{!field f=id}my:_ID1_"'
+    end
+  end
+
   describe '#construct_query_for_ids' do
     it "generates a useable solr query from an array of Fedora ids" do
       expect(described_class.construct_query_for_ids(["my:_ID1_", "my:_ID2_", "my:_ID3_"])).to eq '{!terms f=id}my:_ID1_,my:_ID2_,my:_ID3_'