From 8c2efbf0dcd6368093def873ac06e820308814ba Mon Sep 17 00:00:00 2001 From: Tim Dettrick Date: Thu, 16 Aug 2012 17:42:06 +1000 Subject: [PATCH] Simpler set handling through reflection. Sets can be configured by having a :set attribute, or by having proper set objects. Overriding Provider.find should no longer be necessary. --- .../provider/model/activerecord_wrapper.rb | 61 ++++++++++++---- .../database/0001_oaipmh_tables.rb | 9 ++- .../helpers/set_provider.rb | 32 +++----- .../helpers/transactional_test_case.rb | 2 +- test/activerecord_provider/models/dc_set.rb | 5 +- .../models/exclusive_set_dc_field.rb | 11 +++ .../tc_ar_sets_provider.rb | 73 +++++++++++++++++++ 7 files changed, 152 insertions(+), 41 deletions(-) create mode 100644 test/activerecord_provider/models/exclusive_set_dc_field.rb diff --git a/lib/oai/provider/model/activerecord_wrapper.rb b/lib/oai/provider/model/activerecord_wrapper.rb index 606898a..508d97b 100755 --- a/lib/oai/provider/model/activerecord_wrapper.rb +++ b/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,17 +87,49 @@ 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 @@ -102,8 +137,8 @@ def next_set(token_string) # 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 diff --git a/test/activerecord_provider/database/0001_oaipmh_tables.rb b/test/activerecord_provider/database/0001_oaipmh_tables.rb index 5cdd0d4..de9bde1 100755 --- a/test/activerecord_provider/database/0001_oaipmh_tables.rb +++ b/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 diff --git a/test/activerecord_provider/helpers/set_provider.rb b/test/activerecord_provider/helpers/set_provider.rb index 24fd8a5..105b156 100755 --- a/test/activerecord_provider/helpers/set_provider.rb +++ b/test/activerecord_provider/helpers/set_provider.rb @@ -1,31 +1,11 @@ # 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 @@ -33,4 +13,12 @@ class ARSetProvider < OAI::Provider::Base 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 \ No newline at end of file diff --git a/test/activerecord_provider/helpers/transactional_test_case.rb b/test/activerecord_provider/helpers/transactional_test_case.rb index 61ddc92..e949ade 100644 --- a/test/activerecord_provider/helpers/transactional_test_case.rb +++ b/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( diff --git a/test/activerecord_provider/models/dc_set.rb b/test/activerecord_provider/models/dc_set.rb index 1a68c22..28d2153 100755 --- a/test/activerecord_provider/models/dc_set.rb +++ b/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 \ No newline at end of file diff --git a/test/activerecord_provider/models/exclusive_set_dc_field.rb b/test/activerecord_provider/models/exclusive_set_dc_field.rb new file mode 100644 index 0000000..b1112c0 --- /dev/null +++ b/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 diff --git a/test/activerecord_provider/tc_ar_sets_provider.rb b/test/activerecord_provider/tc_ar_sets_provider.rb index 8c7833d..2a10d11 100755 --- a/test/activerecord_provider/tc_ar_sets_provider.rb +++ b/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 \ No newline at end of file