Merge pull request #97 from code4lib/provider-validation-updates

Merge changes to be more responsive to provider's configured granularity

Author: barmintor

examples/models/file_model.rb

@@ -45,8 +45,8 @@ def find(selector, opts={})
     case selector
     when :all
       records = Dir["#{@directory}/*.xml"].sort.collect do |file|
-        File.new(file) unless File.stat(file).mtime.utc < opts[:from] or
-          File.stat(file).mtime.utc > opts[:until]
+        File.new(file) unless File.stat(file).mtime.utc < opts[:from].to_time or
+          File.stat(file).mtime.utc > opts[:until].to_time
       end
       records
     else

lib/oai/client/response.rb

@@ -37,14 +37,14 @@ def initialize(doc, &resumption_block)
           message = error.content
           code = ""
           if defined?(error.property) == nil
-              code = error.attributes['code']
-           else
-	      begin
-	 	code = error["code"]
-	      rescue
-                code = error.property('code')
-	      end
+            code = error.attributes['code']
+         else
+           begin
+             code = error["code"]
+           rescue
+             code = error.property('code')
            end
+         end
       end
       raise OAI::Exception.new(message, code)
     end

lib/oai/harvester.rb

@@ -9,6 +9,10 @@
 require 'readline'
 require 'socket'
 
+if not defined?(OAI::Const::VERBS)
+  require 'oai/constants'
+end
+
 require 'oai/client'
 require 'oai/harvester/config'
 require 'oai/harvester/harvest'

lib/oai/harvester/config.rb

@@ -5,7 +5,7 @@
 module OAI
   module Harvester
 
-    LOW_RESOLUTION = "YYYY-MM-DD"
+    LOW_RESOLUTION = OAI::Const::Granularity::LOW
 
     class Config < OpenStruct
 

lib/oai/harvester/harvest.rb

@@ -38,7 +38,7 @@ def harvest(site)
           harvest_time = Time.now.utc
         end
 
-        if "YYYY-MM-DD" == granularity(opts[:url])
+        if OAI::Const::Granularity::LOW == granularity(opts[:url])
           opts[:until] = harvest_time.strftime("%Y-%m-%d")
           opts[:from] = @from.strftime("%Y-%m-%d") if @from
         else
@@ -139,7 +139,7 @@ def filename(from_time, until_time)
         "#{from_time.strftime(format)}_til_#{until_time.strftime(format)}"\
         "_at_#{until_time.strftime('%H-%M-%S')}"
       end
-    
+
       def granularity(url)
         client = OAI::Client.new url
         client.identify.granularity
@@ -149,7 +149,7 @@ def granularity(url)
       def earliest(url)
         client = OAI::Client.new url
         identify = client.identify
-        if "YYYY-MM-DD" == identify.granularity
+        if OAI::Const::Granularity::LOW == identify.granularity
           Time.parse(identify.earliest_datestamp).strftime("%Y-%m-%d")
         else
           Time.parse(identify.earliest_datestamp).xmlschema

lib/oai/provider/model/activerecord_wrapper.rb

@@ -61,7 +61,7 @@ def find(selector, options={})
           find_scope.where(conditions)
         end
       else
-        find_scope.where(conditions).find_by!(identifier_field => selector)
+        find_scope.where(conditions).where(identifier_field => selector).first
       end
     end
 
@@ -177,22 +177,40 @@ def sql_conditions(opts)
         sql << "#{timestamp_field} < :until"
         esc_values[:until] = parse_to_local(opts[:until]) { |t| t + 1 }
       end
+      
       return [sql.join(" AND "), esc_values]
     end
 
     private
 
     def parse_to_local(time)
-      time_obj = Time.parse(time.to_s)
+      if time.respond_to?(:strftime)
+        time_obj = time
+      else
+        begin
+          if time[-1] == "Z"
+            time_obj = Time.strptime(time, "%Y-%m-%dT%H:%M:%S%Z")
+          else
+            time_obj = Date.strptime(time, "%Y-%m-%d")
+          end
+        rescue
+          raise OAI::ArgumentException.new, "unparsable date: '#{time}'"
+        end
+      end
+        
       time_obj = yield(time_obj) if block_given?
-      # Convert to same as DB - :local => :getlocal, :utc => :getutc
 
-      if ActiveRecord::VERSION::MAJOR >= 7
-        tzconv = "get#{ActiveRecord.default_timezone.to_s}".to_sym
+      if time_obj.kind_of?(Date)
+        time_obj.strftime("%Y-%m-%d")
       else
-        tzconv = "get#{model.default_timezone.to_s}".to_sym
+        # Convert to same as DB - :local => :getlocal, :utc => :getutc
+        if ActiveRecord::VERSION::MAJOR >= 7
+          tzconv = "get#{ActiveRecord.default_timezone.to_s}".to_sym
+        else
+          tzconv = "get#{model.default_timezone.to_s}".to_sym
+        end
+        time_obj.send(tzconv).strftime("%Y-%m-%d %H:%M:%S")
       end
-      time_obj.send(tzconv).strftime("%Y-%m-%d %H:%M:%S")
     end
 
   end

lib/oai/provider/response.rb

@@ -90,10 +90,13 @@ def externalize(value)
 
     def parse_date(value)
       return value if value.respond_to?(:strftime)
-
-      Date.parse(value) # This will raise an exception for badly formatted dates
-      Time.parse(value).utc #  -- UTC Bug fix hack 8/08 not in core
-    rescue
+      
+      if value[-1] == "Z"
+        Time.strptime(value, "%Y-%m-%dT%H:%M:%S%Z").utc
+      else
+        Date.strptime(value, "%Y-%m-%d")
+      end
+    rescue ArgumentError => e
       raise OAI::ArgumentException.new, "unparsable date: '#{value}'"
     end
 

lib/oai/provider/response/list_records.rb

@@ -2,6 +2,18 @@ module OAI::Provider::Response
 
   class ListRecords < RecordResponse
     required_parameters :metadata_prefix
+    
+    def valid?
+      super && matching_granularity?
+    end
+    
+    def matching_granularity?
+      if options[:from].nil? == false && options[:until].nil? == false && options[:from].class.name != options[:until].class.name
+        raise OAI::ArgumentException.new, "The 'from' and 'until' options specified must have the same granularity"
+      else
+        true
+      end
+    end
 
     def to_xml
       result = provider.model.find(:all, options)

lib/oai/provider/resumption_token.rb

@@ -126,8 +126,20 @@ def encode_conditions
 
       encoded_token = @prefix.to_s.dup
       encoded_token << ".s(#{set})" if set
-      encoded_token << ".f(#{self.from.utc.xmlschema})" if self.from
-      encoded_token << ".u(#{self.until.utc.xmlschema})" if self.until
+      if self.from
+        if self.from.respond_to?(:utc)
+          encoded_token << ".f(#{self.from.utc.xmlschema})"
+        else
+          encoded_token << ".f(#{self.from.xmlschema})"
+        end
+      end
+      if self.until
+        if self.until.respond_to?(:utc)
+          encoded_token << ".u(#{self.until.utc.xmlschema})" 
+        else
+          encoded_token << ".u(#{self.until.xmlschema})" 
+        end
+      end
       encoded_token << ":#{last_str}"
     end
 

test/activerecord_provider/tc_ar_provider.rb

@@ -86,7 +86,7 @@ def test_from
     DCField.where("id < #{first_id + 10}").update_all(updated_at: Time.parse("June 1 2005"))
 
 
-    from_param = Time.parse("January 1 2006")
+    from_param = Time.parse("January 1 2006").getutc.iso8601
 
     doc = REXML::Document.new(
       @provider.list_records(
@@ -97,7 +97,7 @@ def test_from
 
     doc = REXML::Document.new(
       @provider.list_records(
-        :metadata_prefix => 'oai_dc', :from => Time.parse("May 30 2005"))
+        :metadata_prefix => 'oai_dc', :from => Time.parse("May 30 2005").getutc.iso8601)
     )
     assert_equal 20, doc.elements['OAI-PMH/ListRecords'].to_a.size
   end
@@ -122,11 +122,38 @@ def test_from_and_until
     doc = REXML::Document.new(
       @provider.list_records(
         :metadata_prefix => 'oai_dc',
-        :from => Time.parse("June 3 2005"),
-        :until => Time.parse("June 16 2005"))
+        :from => Time.parse("June 3 2005").getutc.iso8601,
+        :until => Time.parse("June 16 2005").getutc.iso8601)
       )
     assert_equal 40, doc.elements['OAI-PMH/ListRecords'].to_a.size
   end
+  
+  def test_bad_until_raises_exception
+    DCField.order('id asc').limit(10).update_all(updated_at: 1.year.ago)
+    DCField.order('id desc').limit(10).update_all(updated_at: 1.year.from_now)
+    badTimes = [
+      'junk',
+      'February 92nd, 2015']
+    badTimes.each do |time|
+      assert_raise(OAI::ArgumentException) do
+        @provider.list_records(:metadata_prefix => 'oai_dc', :until => time)
+      end
+    end
+  end
+  
+  def test_bad_from_raises_exception
+    DCField.order('id asc').limit(10).update_all(updated_at: 1.year.ago)
+    DCField.order('id desc').limit(10).update_all(updated_at: 1.year.from_now)
+    
+    badTimes = [
+      'junk',
+      'February 92nd, 2015']
+    badTimes.each do |time|
+      assert_raise(OAI::ArgumentException) do
+        @provider.list_records(:metadata_prefix => 'oai_dc', :from => time)
+      end
+    end
+  end
 
   def test_handles_empty_collections
     DCField.delete_all
@@ -142,6 +169,21 @@ def test_handles_empty_collections
       REXML::Document.new(@provider.list_records(:metadata_prefix => 'oai_dc'))
     end
   end
+  
+  def test_bad_id_raises_exception
+    badIdentifiers = [
+      'invalid"id',
+      'oai:test/5000',
+      'oai:test/-1',
+      'oai:test/one',
+      'oai:test/\\$1\1!']
+    badIdentifiers.each do |id|
+      assert_raise(OAI::IdException) do
+        @provider.get_record(:identifier => id, :metadata_prefix => 'oai_dc')
+      end
+    end
+  end
+  
 
   def setup
     @provider = ARProvider.new