Merge remote branch 'upstream/3.2.x' into HYDRA-661

lib/active_fedora/delegating.rb

@@ -31,11 +31,11 @@ def delegate(field, args ={})
         def create_delegate_accessor(field, args)
             define_method field do
               ds = self.send(args[:to])
-              val = if ds.kind_of? ActiveFedora::NokogiriDatastream 
+              val = if ds.kind_of? ActiveFedora::MetadataDatastream 
+                ds.send(:get_values, field)
+              else 
                 terminology = args[:at] || [field]
                 ds.send(:term_values, *terminology)
-              else 
-                ds.send(:get_values, field)
               end 
               args[:unique] ? val.first : val
 
@@ -45,11 +45,11 @@ def create_delegate_accessor(field, args)
         def create_delegate_setter(field, args)
             define_method "#{field}=".to_sym do |v|
               ds = self.send(args[:to])
-              if ds.kind_of? ActiveFedora::NokogiriDatastream 
+              if ds.kind_of? ActiveFedora::MetadataDatastream 
+                ds.send(:set_value, field, v)
+              else 
                 terminology = args[:at] || [field]
                 ds.send(:update_indexed_attributes, {terminology => v})
-              else 
-                ds.send(:set_value, field, v)
               end
             end
         end

lib/active_fedora/metadata_datastream.rb

@@ -6,11 +6,56 @@ module ActiveFedora
   #     <title>Foo</title>
   #     <author>Bar</author>
   #   </fields>
-  class MetadataDatastream < Datastream
+  class MetadataDatastream < NokogiriDatastream 
 
     # .to_solr (among other things) is provided by ActiveFedora::MetadataDatastreamHelper
     include ActiveFedora::MetadataDatastreamHelper
 
+    # def initialize(digital_object, dsid)
+    #   ActiveSupport::Deprecation.warn("MetadataDatastream is deprecated and will be removed in a future release. Create a NokogiriDatastream to structure your data")
+    #   super
+    # end
+    def to_solr(solr_doc = Hash.new) # :nodoc:
+      fields.each do |field_key, field_info|
+        if field_info.has_key?(:values) && !field_info[:values].nil?
+          field_symbol = ActiveFedora::SolrService.solr_name(field_key, field_info[:type])
+          values = field_info[:values]
+          values = [values] unless values.respond_to? :each
+          values.each do |val|    
+            ::Solrizer::Extractor.insert_solr_field_value(solr_doc, field_symbol, val )         
+          end
+        end
+      end
+
+      return solr_doc
+    end
+
+    # ** EXPERIMENTAL **
+    #
+    # This is utilized by ActiveFedora::Base.load_instance_from_solr to set 
+    # metadata values in this object using the Solr document passed in.
+    # Any keys in the solr document that map to a metadata field key within a MetadataDatastream object
+    # are set to the corresponding value.  Any others are ignored. ActiveFedora::SolrService.solr_name
+    # is used to map solr key to field key name.
+    #
+    # ====Warning
+    #  Solr must be synchronized with data in Fedora.
+    def from_solr(solr_doc)
+      fields.each do |field_key, field_info|
+        field_symbol = ActiveFedora::SolrService.solr_name(field_key, field_info[:type])
+        value = (solr_doc[field_symbol].nil? ? solr_doc[field_symbol.to_s]: solr_doc[field_symbol]) 
+        unless value.nil?
+          if value.is_a? Array
+            update_attributes({field_key=>value})
+          else
+            update_indexed_attributes({field_key=>{0=>value}})
+          end
+        end
+      end
+    end
+
+
+
 
     def update_attributes(params={},opts={})
       result = params.dup

lib/active_fedora/metadata_datastream_helper.rb

@@ -36,44 +36,4 @@ def serialize! # :nodoc:
     end
   end
 
-  def to_solr(solr_doc = Hash.new) # :nodoc:
-    fields.each do |field_key, field_info|
-      if field_info.has_key?(:values) && !field_info[:values].nil?
-        field_symbol = ActiveFedora::SolrService.solr_name(field_key, field_info[:type])
-        values = field_info[:values]
-        values = [values] unless values.respond_to? :each
-        values.each do |val|    
-          ::Solrizer::Extractor.insert_solr_field_value(solr_doc, field_symbol, val )         
-        end
-      end
-    end
-
-    return solr_doc
-  end
-
-  # ** EXPERIMENTAL **
-  #
-  # This is utilized by ActiveFedora::Base.load_instance_from_solr to set 
-  # metadata values in this object using the Solr document passed in.
-  # Any keys in the solr document that map to a metadata field key within a MetadataDatastream object
-  # are set to the corresponding value.  Any others are ignored. ActiveFedora::SolrService.solr_name
-  # is used to map solr key to field key name.
-  #
-  # ====Warning
-  #  Solr must be synchronized with data in Fedora.
-  def from_solr(solr_doc)
-    fields.each do |field_key, field_info|
-      field_symbol = ActiveFedora::SolrService.solr_name(field_key, field_info[:type])
-      value = (solr_doc[field_symbol].nil? ? solr_doc[field_symbol.to_s]: solr_doc[field_symbol]) 
-      unless value.nil?
-        if value.is_a? Array
-          update_attributes({field_key=>value})
-        else
-          update_indexed_attributes({field_key=>{0=>value}})
-        end
-      end
-    end
-  end
-
-
 end

lib/active_fedora/model.rb

@@ -164,7 +164,7 @@ def find_by_solr(query, args={})
       # Find all ActiveFedora objects for this model that match arguments
       # passed in by querying Solr.  Like find_by_solr this returns a solr result.
       #
-      # query_fields   a hash of object field names and values to filter on
+      # query_fields   a hash of object field names and values to filter on (query_fields must be the solr_field_name for non-MetadataDatastream derived datastreams)
       # opts           specifies options for the solr query
       #
       #   options may include:

spec/integration/base_find_by_fields_spec.rb

@@ -1,4 +1,5 @@
 require 'spec_helper'
+require "active_fedora/samples"
 
 class MockAFBaseQuerySolr < ActiveFedora::Base
   has_metadata :name => "properties", :type => ActiveFedora::MetadataDatastream do |m|
@@ -13,15 +14,45 @@ class MockAFBaseQuerySolr < ActiveFedora::Base
     m.field "geography", :string, :xml_node => "geography"
     m.field "title", :string, :xml_node => "title"
   end
+
+  has_metadata :name=>'ng_metadata', :type=> Hydra::ModsArticleDatastream
 end
 
 describe ActiveFedora::Base do
 
-  before(:each) do
+  before(:all) do
     @test_object = ActiveFedora::Base.new
+    #get objects into fedora and solr
+    @test_object2 = MockAFBaseQuerySolr.new
+    attributes = {"holding_id"=>{0=>"Holding 1"},
+                  "language"=>{0=>"Italian"},
+                  "creator"=>{0=>"Linguist, A."},
+                  "geography"=>{0=>"Italy"},
+                  "title"=>{0=>"Italian and Spanish: A Comparison of Common Phrases"}}
+    @test_object2.update_indexed_attributes(attributes)
+    @test_object2.save
+
+    @test_object3 = MockAFBaseQuerySolr.new
+    attributes = {"holding_id"=>{0=>"Holding 2"},
+                  "language"=>{0=>"Spanish;Latin"},
+                  "creator"=>{0=>"Linguist, A."},
+                  "geography"=>{0=>"Spain"},
+                  "title"=>{0=>"A study of the evolution of Spanish from Latin"}}
+    @test_object3.update_indexed_attributes(attributes)
+    @test_object3.save      
+
+    @test_object4 = MockAFBaseQuerySolr.new
+    attributes = {"holding_id"=>{0=>"Holding 3"},
+                  "language"=>{0=>"Spanish;Latin"},
+                  "creator"=>{0=>"Linguist, A."},
+                  "geography"=>{0=>"Spain"},
+                  "title"=>{0=>"An obscure look into early nomadic tribes of Spain"}}
+    @test_object4.update_indexed_attributes(attributes)
+    @test_object4.save
+
   end
 
-  after(:each) do
+  after(:all) do
     begin
     @test_object.delete
     rescue
@@ -38,42 +69,10 @@ class MockAFBaseQuerySolr < ActiveFedora::Base
     @test_object4.delete
     rescue
     end
-    begin
-    @test_object5.delete
-    rescue
-    end
   end
 
   describe '#find_by_fields_by_solr' do
-    it 'should return fedora objects of the model of self that match the given solr query, queries the active_fedora solr instance' do
-      #get objects into fedora and solr
-      @test_object2 = MockAFBaseQuerySolr.new
-      attributes = {"holding_id"=>{0=>"Holding 1"},
-                    "language"=>{0=>"Italian"},
-                    "creator"=>{0=>"Linguist, A."},
-                    "geography"=>{0=>"Italy"},
-                    "title"=>{0=>"Italian and Spanish: A Comparison of Common Phrases"}}
-      @test_object2.update_indexed_attributes(attributes)
-      @test_object2.save
-
-      @test_object3 = MockAFBaseQuerySolr.new
-      attributes = {"holding_id"=>{0=>"Holding 2"},
-                    "language"=>{0=>"Spanish;Latin"},
-                    "creator"=>{0=>"Linguist, A."},
-                    "geography"=>{0=>"Spain"},
-                    "title"=>{0=>"A study of the evolution of Spanish from Latin"}}
-      @test_object3.update_indexed_attributes(attributes)
-      @test_object3.save      
-
-      @test_object4 = MockAFBaseQuerySolr.new
-      attributes = {"holding_id"=>{0=>"Holding 3"},
-                    "language"=>{0=>"Spanish;Latin"},
-                    "creator"=>{0=>"Linguist, A."},
-                    "geography"=>{0=>"Spain"},
-                    "title"=>{0=>"An obscure look into early nomadic tribes of Spain"}}
-      @test_object4.update_indexed_attributes(attributes)
-      @test_object4.save
-
+    it 'should return fedora objects of the correct class' do
       #query based on just model
       results = MockAFBaseQuerySolr.find_by_fields_by_solr({})
       found_pids = []
@@ -82,7 +81,9 @@ class MockAFBaseQuerySolr < ActiveFedora::Base
       end
 
       found_pids.should == [@test_object2.pid,@test_object3.pid,@test_object4.pid]
+    end
 
+    it 'should match a query against multiple result fields' do
       #query on certain fields
       results = MockAFBaseQuerySolr.find_by_fields_by_solr({"language"=>"Latin"})
       found_pids = []
@@ -107,23 +108,10 @@ class MockAFBaseQuerySolr < ActiveFedora::Base
       end
 
       found_pids.should == [@test_object3.pid,@test_object4.pid]
+    end
 
+    it 'should  query against many fields' do
       #assume spaces removed at index time so query by 'Linguist,A.' instead of 'Linguist, A.'
-      results = MockAFBaseQuerySolr.find_by_fields_by_solr({"creator"=>"Linguist,A."})
-      found_pids = []
-      results.hits.each do |hit|
-        found_pids.push(hit[SOLR_DOCUMENT_ID])
-      end
-
-      found_pids.should == [@test_object2.pid,@test_object3.pid,@test_object4.pid]
-
-      results = MockAFBaseQuerySolr.find_by_fields_by_solr({"geography"=>"Italy"})
-      found_pids = []
-      results.hits.each do |hit|
-        found_pids.push(hit[SOLR_DOCUMENT_ID])
-      end
-
-      found_pids.should == [@test_object2.pid]
 
       results = MockAFBaseQuerySolr.find_by_fields_by_solr({"creator"=>"Linguist,A.","title"=>"latin"})
       found_pids = []
@@ -132,6 +120,9 @@ class MockAFBaseQuerySolr < ActiveFedora::Base
       end
 
       found_pids.should == [@test_object3.pid]
+    end
+
+    it 'should query by id' do
 
       #query with value with embedded ':' (pid)
       results = MockAFBaseQuerySolr.find_by_fields_by_solr({"id"=>@test_object3.pid})
@@ -141,44 +132,51 @@ class MockAFBaseQuerySolr < ActiveFedora::Base
       end
 
       found_pids.should == [@test_object3.pid]
-
-      #query with different options
-      #sort defaults to system_create_date
+    end
+
+
+    it "should sort by default by system_create_date" do
       results = MockAFBaseQuerySolr.find_by_fields_by_solr({"creator"=>"Linguist,A.","language"=>"Spanish"})
       found_pids = []
       results.hits.each do |hit|
         found_pids.push(hit[SOLR_DOCUMENT_ID])
       end
 
       found_pids.should == [@test_object3.pid,@test_object4.pid]
-
-      #change sort direction
+    end
+
+    it "should be able to change the sort direction" do 
       results = MockAFBaseQuerySolr.find_by_fields_by_solr({"creator"=>"Linguist,A."},{:sort=>[{"system_create"=>"desc"}]})
       found_pids = []
       results.hits.each do |hit|
         found_pids.push(hit[SOLR_DOCUMENT_ID])
       end
 
       found_pids.should == [@test_object4.pid,@test_object3.pid,@test_object2.pid]
-
-      #pass in sort without direction defined and make ascending by default
+    end
+
+    it "should default the sort direction to ascending" do
       results = MockAFBaseQuerySolr.find_by_fields_by_solr({"creator"=>"Linguist,A."},{:sort=>["system_create"]})
       found_pids = []
       results.hits.each do |hit|
         found_pids.push(hit[SOLR_DOCUMENT_ID])
       end
 
       found_pids.should == [@test_object2.pid,@test_object3.pid,@test_object4.pid]
+    end
 
-      #sort on multiple fields
+
+    it "should sort by multiple fields" do
       results = MockAFBaseQuerySolr.find_by_fields_by_solr({"creator"=>"Linguist,A."},{:sort=>["geography",{"system_create"=>"desc"}]})
       found_pids = []
       results.hits.each do |hit|
         found_pids.push(hit[SOLR_DOCUMENT_ID])
       end
 
       found_pids.should == [@test_object2.pid,@test_object4.pid,@test_object3.pid]
+    end
 
+    it "should transform system_modified_date" do
       #check appropriate logic for system_modified_date field name transformation
       results = MockAFBaseQuerySolr.find_by_fields_by_solr({"creator"=>"Linguist,A."},{:sort=>["geography",{"system_mod"=>"desc"}]})
       found_pids = []
@@ -187,7 +185,9 @@ class MockAFBaseQuerySolr < ActiveFedora::Base
       end
 
       found_pids.should == [@test_object2.pid,@test_object4.pid,@test_object3.pid]
-
+    end
+
+    it "should accept rows as a parameter" do 
       #check pass in rows values
       results = MockAFBaseQuerySolr.find_by_fields_by_solr({"creator"=>"Linguist,A."},{:rows=>2})
       found_pids = []
@@ -196,16 +196,26 @@ class MockAFBaseQuerySolr < ActiveFedora::Base
       end
 
       found_pids.should == [@test_object2.pid,@test_object3.pid]
+    end
 
+    it "should accept a solr field (geography_t) that is not an object field name(e.g. geography)" do
       #check query with field mapping to solr field and with solr field that is not a field in object
       #should be able to query by either active fedora model field name or solr key name
       results = MockAFBaseQuerySolr.find_by_fields_by_solr({"geography_t"=>"Italy"})
-      found_pids = []
-      results.hits.each do |hit|
-        found_pids.push(hit[SOLR_DOCUMENT_ID])
-      end
-
+      found_pids = results.hits.map{|h| h[SOLR_DOCUMENT_ID]}
       found_pids.should == [@test_object2.pid]
     end
+
+    describe "with Nokogiri based datastreams" do
+      before do
+        @test_object2.ng_metadata.journal_title = "foo"
+        @test_object2.save
+      end
+      it "should query Nokogiri based datastreams if you use the solr field names (doesn't do mapping)" do
+        results = MockAFBaseQuerySolr.find_by_fields_by_solr({"journal_title_t" => "foo"})
+        found_pids = results.hits.map{|h| h[SOLR_DOCUMENT_ID]}
+        found_pids.should == [@test_object2.pid]
+      end
+    end
   end
 end

spec/unit/metadata_datastream_spec.rb

@@ -21,6 +21,7 @@
     mock_inner = mock('inner object')
     @test_object = ActiveFedora::Base.new
     @mock_repo = mock('repository')
+    @mock_repo.stubs(:datastream)
     @mock_repo.stubs(:datastream_dissemination=>'My Content')
     mock_inner.stubs(:repository).returns(@mock_repo)
     mock_inner.stubs(:pid)