Merge pull request #18 from tjdett/smarter-set-membership

Simpler provider set handling through reflection

Author: tjdett

lib/oai/provider/model/activerecord_wrapper.rb

@@ -43,17 +43,20 @@ def sets
     end
 
     def find(selector, options={})
-      return next_set(options[:resumption_token]) if options[:resumption_token]
+      find_scope = find_scope(options)
+      return next_set(find_scope,
+        options[:resumption_token]) if options[:resumption_token]
       conditions = sql_conditions(options)
       if :all == selector
-        total = model.count(:id, :conditions => conditions)
+        total = find_scope.count(:id, :conditions => conditions)
         if @limit && total > @limit
-          select_partial(ResumptionToken.new(options.merge({:last => 0})))
+          select_partial(find_scope,
+            ResumptionToken.new(options.merge({:last => 0})))
         else
-          model.find(:all, :conditions => conditions)
+          find_scope.find(:all, :conditions => conditions)
         end
       else
-        model.find(selector, :conditions => conditions)
+        find_scope.find(selector, :conditions => conditions)
       end
     end
 
@@ -84,26 +87,58 @@ def method_missing(m, *args, &block)
 
     protected
 
+    def find_scope(options)
+      return model unless options.key?(:set)
+
+      # Find the set or return an empty scope
+      set = find_set_by_spec(options[:set])
+      return model.scoped(:limit => 0) if set.nil?
+
+      # If the set has a backward relationship, we'll use it
+      if set.class.respond_to?(:reflect_on_all_associations)
+        set.class.reflect_on_all_associations.each do |assoc|
+          return set.send(assoc.name).scoped if assoc.klass == model
+        end
+      end
+
+      # Search the attributes for 'set'
+      if model.column_names.include?('set')
+        # Scope using the set attribute as the spec
+        model.scoped(:conditions => {:set => options[:set]})
+      else
+        # Default to empty set, as we've tried everything else
+        model.scoped(:limit => 0)
+      end
+    end
+
+    def find_set_by_spec(spec)
+      if sets.class == ActiveRecord::Relation
+        sets.find_by_spec(spec)
+      else
+        sets.detect {|set| set.spec == spec}
+      end
+    end
+
     # Request the next set in this sequence.
-    def next_set(token_string)
+    def next_set(find_scope, token_string)
       raise OAI::ResumptionTokenException.new unless @limit
 
       token = ResumptionToken.parse(token_string)
-      total = model.count(:id, :conditions => token_conditions(token))
+      total = find_scope.count(:id, :conditions => token_conditions(token))
 
       if @limit < total
-        select_partial(token)
+        select_partial(find_scope, token)
       else # end of result set
-        model.find(:all,
+        find_scope.find(:all,
           :conditions => token_conditions(token),
           :limit => @limit, :order => "#{model.primary_key} asc")
       end
     end
 
     # select a subset of the result set, and return it with a
     # resumption token to get the next subset
-    def select_partial(token)
-      records = model.find(:all,
+    def select_partial(find_scope, token)
+      records = find_scope.find(:all,
         :conditions => token_conditions(token),
         :limit => @limit,
         :order => "#{model.primary_key} asc")
@@ -144,10 +179,6 @@ def sql_conditions(opts)
         sql << "#{timestamp_field} < :until"
         esc_values[:until] = parse_to_local(opts[:until]) { |t| t + 1 }
       end
-      if opts.has_key?(:set)
-        sql << "set = :set"
-        esc_values[:set] = opts[:set]
-      end
       return [sql.join(" AND "), esc_values]
     end
 

test/activerecord_provider/database/0001_oaipmh_tables.rb

@@ -10,7 +10,7 @@ def self.up
       t.column :oai_token_id, :integer, :null => false
     end
 
-    create_table :dc_fields do |t|
+    dc_fields = proc do |t|
       t.column  :title,         :string
       t.column  :creator,       :string
       t.column  :subject,       :string
@@ -30,6 +30,13 @@ def self.up
       t.column  :deleted,       :boolean,   :default => false
     end
 
+    create_table :exclusive_set_dc_fields do |t|
+      dc_fields.call(t)
+      t.column  :set,           :string
+    end
+
+    create_table :dc_fields, &dc_fields
+
     create_table :dc_fields_dc_sets, :id => false do |t|
       t.column :dc_field_id,    :integer
       t.column :dc_set_id,      :integer

test/activerecord_provider/helpers/set_provider.rb

@@ -1,36 +1,24 @@
 # Extend ActiveRecordModel to support sets
 class SetModel < OAI::Provider::ActiveRecordWrapper
-  
+
   # Return all available sets
   def sets
-    DCSet.find(:all)
+    DCSet.scoped
   end
 
-  # Scope the find to a set relation if we get a set in the options    
-  def find(selector, opts={})
-    if opts[:set]
-      set = DCSet.find_by_spec(opts.delete(:set))
-      conditions = sql_conditions(opts)
-
-      if :all == selector
-        set.dc_fields.find(selector, :conditions => conditions)
-      else
-        set.dc_fields.find(selector, :conditions => conditions)
-      end
-    else
-      if :all == selector
-        model.find(selector, :conditions => sql_conditions(opts))
-      else
-        model.find(selector, :conditions => sql_conditions(opts))
-      end
-    end
-  end
-        
 end
 
 class ARSetProvider < OAI::Provider::Base
   repository_name 'ActiveRecord Set Based Provider'
   repository_url 'http://localhost'
   record_prefix = 'oai:test'
   source_model SetModel.new(DCField, :timestamp_field => 'date')
+end
+
+class ARExclusiveSetProvider < OAI::Provider::Base
+  repository_name 'ActiveRecord Set Based Provider'
+  repository_url 'http://localhost'
+  record_prefix = 'oai:test'
+  source_model OAI::Provider::ActiveRecordWrapper.new(
+    ExclusiveSetDCField, :timestamp_field => 'date')
 end

test/activerecord_provider/helpers/transactional_test_case.rb

@@ -11,7 +11,7 @@ def run(result, &block)
     result
   end
 
-  private
+  protected
 
   def load_fixtures
     fixtures = YAML.load_file(

test/activerecord_provider/models/dc_set.rb

@@ -1,6 +1,7 @@
 class DCSet < ActiveRecord::Base
-  has_and_belongs_to_many :dc_fields, 
-    :join_table => "dc_fields_dc_sets", 
+  has_and_belongs_to_many :dc_fields,
+    :join_table => "dc_fields_dc_sets",
     :foreign_key => "dc_set_id",
     :class_name => "DCField"
+
 end

test/activerecord_provider/models/exclusive_set_dc_field.rb

@@ -0,0 +1,11 @@
+class ExclusiveSetDCField < ActiveRecord::Base
+  inheritance_column = 'DONOTINHERIT'
+
+  def self.sets
+    klass = Struct.new(:name, :spec)
+    self.uniq.pluck('`set`').compact.map do |spec|
+      klass.new("Set #{spec}", spec)
+    end
+  end
+
+end

test/activerecord_provider/tc_ar_sets_provider.rb

@@ -62,4 +62,77 @@ def define_sets
       next_id = record.id
     end
   end
+end
+
+
+class ActiveRecordExclusiveSetsProviderTest < TransactionalTestCase
+
+  def test_list_sets
+    doc = REXML::Document.new(@provider.list_sets)
+    sets = doc.elements["/OAI-PMH/ListSets"]
+    assert_equal 3, sets.size
+    assert_equal "Set A", sets[0].elements["//setName"].text
+  end
+
+  def test_set_a
+    doc = REXML::Document.new(@provider.list_records(:set => "A"))
+    assert_equal 20, doc.elements['OAI-PMH/ListRecords'].to_a.size
+  end
+
+  def test_set_b
+    doc = REXML::Document.new(@provider.list_records(:set => "B"))
+    assert_equal 10, doc.elements['OAI-PMH/ListRecords'].to_a.size
+  end
+
+  def test_set_ab
+    doc = REXML::Document.new(@provider.list_records(:set => "A:B"))
+    assert_equal 10, doc.elements['OAI-PMH/ListRecords'].to_a.size
+  end
+
+  def setup
+    @provider = ARExclusiveSetProvider.new
+    define_sets
+  end
+
+  def define_sets
+    next_id = 0
+
+    ExclusiveSetDCField.find(:all, :limit => 10, :order => "id asc").each do |record|
+      record.set = "A"
+      record.save!
+      next_id = record.id
+    end
+
+    ExclusiveSetDCField.find(:all, :limit => 10, :order => "id asc", :conditions => "id > #{next_id}").each do |record|
+      record.set = "B"
+      record.save!
+      next_id = record.id
+    end
+
+    ExclusiveSetDCField.find(:all, :limit => 10, :order => "id asc", :conditions => "id > #{next_id}").each do |record|
+      record.set = "A:B"
+      record.save!
+      next_id = record.id
+    end
+
+    ExclusiveSetDCField.find(:all, :limit => 10, :order => "id asc", :conditions => "id > #{next_id}").each do |record|
+      record.set = "A"
+      record.save!
+      next_id = record.id
+    end
+  end
+
+  protected
+
+  def load_fixtures
+    fixtures = YAML.load_file(
+      File.join(File.dirname(__FILE__), 'fixtures', 'dc.yml')
+    )
+    disable_logging do
+      fixtures.keys.sort.each do |key|
+        ExclusiveSetDCField.create(fixtures[key])
+      end
+    end
+  end
+
 end