From f75cd317acb131e9de3b220e6c621def1d97a8cb Mon Sep 17 00:00:00 2001 From: Jonathan Rochkind Date: Mon, 26 Aug 2019 11:56:25 -0400 Subject: [PATCH 1/3] whitespace --- lib/oai/provider/model.rb | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/lib/oai/provider/model.rb b/lib/oai/provider/model.rb index 2923473..80edcbe 100755 --- a/lib/oai/provider/model.rb +++ b/lib/oai/provider/model.rb @@ -1,12 +1,12 @@ module OAI::Provider # = OAI::Provider::Model # - # Model implementers should subclass OAI::Provider::Model and override + # Model implementers should subclass OAI::Provider::Model and override # Model#earliest, Model#latest, and Model#find. Optionally Model#sets and - # Model#deleted? can be used to support sets and record deletions. It - # is also the responsibility of the model implementer to account for - # resumption tokens if support is required. Models that don't support - # resumption tokens should raise an exception if a limit is requested + # Model#deleted? can be used to support sets and record deletions. It + # is also the responsibility of the model implementer to account for + # resumption tokens if support is required. Models that don't support + # resumption tokens should raise an exception if a limit is requested # during initialization. # # earliest - should return the earliest update time in the repository. @@ -14,8 +14,8 @@ module OAI::Provider # sets - should return an array of sets supported by the repository. # deleted? - individual records returned should respond true or false # when sent the deleted? message. - # available_formats - if overridden, individual records should return an - # array of prefixes for all formats in which that record is available, + # available_formats - if overridden, individual records should return an + # array of prefixes for all formats in which that record is available, # if other than ["oai_dc"] # about - if overridden, should return a String or Array of XML Strings to # insert into the OAI Record chunks. @@ -28,10 +28,9 @@ module OAI::Provider # There are several helper models for dealing with resumption tokens please # see the ResumptionToken class for more details. # - class Model attr_reader :timestamp_field - + def initialize(limit = nil, timestamp_field = 'updated_at') @limit = limit @timestamp_field = timestamp_field @@ -41,16 +40,16 @@ def initialize(limit = nil, timestamp_field = 'updated_at') def earliest raise NotImplementedError.new end - + # should return the latest timestamp available from this model. def latest raise NotImplementedError.new end - + def sets nil end - + # find is the core method of a model, it returns records from the model # bases on the parameters passed in. # @@ -61,12 +60,12 @@ def sets # * :from => earliest timestamp to be included in the results # * :until => latest timestamp to be included in the results # * :set => the set from which to retrieve the results - # * :metadata_prefix => type of metadata requested (this may be useful if + # * :metadata_prefix => type of metadata requested (this may be useful if # not all records are available in all formats) def find(selector, options={}) raise NotImplementedError.new end - + def deleted? false end @@ -76,5 +75,5 @@ def about record nil end end - + end From 795ade236e794ba211b3b043b3f66ad34644911b Mon Sep 17 00:00:00 2001 From: Jonathan Rochkind Date: Mon, 26 Aug 2019 12:44:59 -0400 Subject: [PATCH 2/3] Let identifier_field be customizable, analgous to timestamp_field To be used for a feature I actually need in the ActiveRecordWrapper, and will implement there next. Sorry, haven't figured out how to test this at present. The customizable timestamp_field does not seem to be tested either. --- lib/oai/provider.rb | 4 +++- lib/oai/provider/model.rb | 5 +++-- lib/oai/provider/response/record_response.rb | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/oai/provider.rb b/lib/oai/provider.rb index a938fd9..b218ed9 100755 --- a/lib/oai/provider.rb +++ b/lib/oai/provider.rb @@ -155,8 +155,10 @@ # # Valid options include: # -# * `timestamp_field` - Specifies the model field to use as the update +# * `timestamp_field` - Specifies the model field/method to use as the update # filter. Defaults to `updated_at`. +# * `identifier_field` -- specifies the model field/method to use to get value to use +# as oai identifier (method return value should not include prefix) # * `limit` - Maximum number of records to return in each page/set. # Defaults to 100, set to `nil` for all records in one page. Otherwise # the wrapper will paginate the result via resumption tokens. diff --git a/lib/oai/provider/model.rb b/lib/oai/provider/model.rb index 80edcbe..cfc106e 100755 --- a/lib/oai/provider/model.rb +++ b/lib/oai/provider/model.rb @@ -29,10 +29,11 @@ module OAI::Provider # see the ResumptionToken class for more details. # class Model - attr_reader :timestamp_field + attr_reader :timestamp_field, :identifier_field - def initialize(limit = nil, timestamp_field = 'updated_at') + def initialize(limit = nil, timestamp_field = 'updated_at', identifier_field = 'id') @limit = limit + @identifier_field = identifier_field @timestamp_field = timestamp_field end diff --git a/lib/oai/provider/response/record_response.rb b/lib/oai/provider/response/record_response.rb index a49dc57..736c640 100755 --- a/lib/oai/provider/response/record_response.rb +++ b/lib/oai/provider/response/record_response.rb @@ -50,7 +50,7 @@ def about_for(record) # Namespace syntax suggested in http://www.openarchives.org/OAI/2.0/guidelines-oai-identifier.htm def identifier_for(record) - "#{provider.prefix}:#{record.id}" + "#{provider.prefix}:#{record.send( provider.model.identifier_field )}" end def timestamp_for(record) From f99ef8e40b6aed55fb7c18d90f1afcda83861d20 Mon Sep 17 00:00:00 2001 From: Jonathan Rochkind Date: Mon, 26 Aug 2019 13:11:53 -0400 Subject: [PATCH 3/3] ActiveRecordWrapper works with custom identifier_field With limited test. --- .../provider/model/activerecord_wrapper.rb | 21 ++++++++++++------- .../helpers/providers.rb | 7 +++++++ test/activerecord_provider/tc_ar_provider.rb | 12 +++++++++++ 3 files changed, 33 insertions(+), 7 deletions(-) diff --git a/lib/oai/provider/model/activerecord_wrapper.rb b/lib/oai/provider/model/activerecord_wrapper.rb index 8ed5d09..2bcced4 100755 --- a/lib/oai/provider/model/activerecord_wrapper.rb +++ b/lib/oai/provider/model/activerecord_wrapper.rb @@ -10,11 +10,18 @@ module OAI::Provider # class ActiveRecordWrapper < Model - attr_reader :model, :timestamp_field - + attr_reader :model, :timestamp_field, :identifier_field + + # If custom 'timestamp_field' is used, be aware this will be an ActiveRecord + # attribute that we will limit on, so perhaps should be indexe appropriately. + # + # If custom `identifier_field` is used, be aware this will be an ActiveRecord + # attribute that we will sort on, and use in WHERE clauses with `=` as well as + # greater than/less than, so should be indexed appropriately. def initialize(model, options={}) @model = model @timestamp_field = options.delete(:timestamp_field) || 'updated_at' + @identifier_field = options.delete(:identifier_field) || model.primary_key || "id" @limit = options.delete(:limit) || 100 unless options.empty? @@ -54,7 +61,7 @@ def find(selector, options={}) find_scope.where(conditions) end else - find_scope.where(conditions).find(selector) + find_scope.where(conditions).find_by!(identifier_field => selector) end end @@ -129,7 +136,7 @@ def next_set(find_scope, token_string) else # end of result set find_scope.where(token_conditions(token)) .limit(@limit) - .order("#{model.primary_key} asc") + .order("#{identifier_field} asc") end end @@ -138,9 +145,9 @@ def next_set(find_scope, token_string) def select_partial(find_scope, token) records = find_scope.where(token_conditions(token)) .limit(@limit) - .order("#{model.primary_key} asc") + .order("#{identifier_field} asc") raise OAI::ResumptionTokenException.new unless records - offset = records.last.send(model.primary_key.to_sym) + offset = records.last.send(identifier_field) PartialResult.new(records, token.next(offset)) end @@ -157,7 +164,7 @@ def token_conditions(token) return sql if 0 == last # Now add last id constraint - sql.first << " AND #{model.primary_key} > :id" + sql.first << " AND #{identifier_field} > :id" sql.last[:id] = last return sql diff --git a/test/activerecord_provider/helpers/providers.rb b/test/activerecord_provider/helpers/providers.rb index b110263..4facde7 100755 --- a/test/activerecord_provider/helpers/providers.rb +++ b/test/activerecord_provider/helpers/providers.rb @@ -12,6 +12,13 @@ class ARProvider < OAI::Provider::Base source_model ActiveRecordWrapper.new(DCField) end +class ARProviderCustomIdentifierField < OAI::Provider::Base + repository_name 'ActiveRecord Based Provider' + repository_url 'http://localhost' + record_prefix 'oai:test' + source_model ActiveRecordWrapper.new(DCField, identifier_field: "source") +end + class ARProviderWithScope < OAI::Provider::Base DATE_LESS_THAN_RESTRICTION = Time.parse("2007-03-12 19:30:22 UTC") diff --git a/test/activerecord_provider/tc_ar_provider.rb b/test/activerecord_provider/tc_ar_provider.rb index 8c2e16f..16ab5cd 100755 --- a/test/activerecord_provider/tc_ar_provider.rb +++ b/test/activerecord_provider/tc_ar_provider.rb @@ -40,6 +40,18 @@ def test_list_records_scope assert_equal expected_count, doc.elements['OAI-PMH/ListRecords'].to_a.size end + + def test_get_record_alternate_identifier_column + @provider = ARProviderCustomIdentifierField.new + + record_id = DCField.first.send(@provider.class.model.identifier_field) + + doc = REXML::Document.new(@provider.get_record( + :identifier => "oai:test:#{record_id}", :metadata_prefix => 'oai_dc')) + + assert_equal "oai:test:#{record_id}", doc.elements['OAI-PMH/GetRecord/record/header/identifier'].text + end + def test_list_identifiers assert_nothing_raised { REXML::Document.new(@provider.list_identifiers) } doc = REXML::Document.new(@provider.list_identifiers)