From bc8bc978c93f0bae571d56673850c7ac6412585c Mon Sep 17 00:00:00 2001 From: ConnorSheremeta Date: Fri, 23 Oct 2020 14:04:03 -0600 Subject: [PATCH 1/6] Fix perl validators outstanding error messages --- app/controllers/oaisys/pmh_controller.rb | 75 +++++++++++++++---- config/routes.rb | 21 ++++-- test/integration/bad_verb_test.rb | 26 ++++--- test/integration/expect_args_test.rb | 29 ++++--- test/integration/identify_test.rb | 11 +++ test/integration/list_identifiers_test.rb | 12 +++ .../integration/list_metadata_formats_test.rb | 16 ++-- 7 files changed, 142 insertions(+), 48 deletions(-) diff --git a/app/controllers/oaisys/pmh_controller.rb b/app/controllers/oaisys/pmh_controller.rb index 78e8736..10d0872 100644 --- a/app/controllers/oaisys/pmh_controller.rb +++ b/app/controllers/oaisys/pmh_controller.rb @@ -8,10 +8,7 @@ class Oaisys::PMHController < Oaisys::ApplicationController metadataNamespace: 'http://www.openarchives.org/OAI/2.0/oai_dc/' }, { metadataPrefix: 'oai_etdms', schema: 'http://www.ndltd.org/standards/metadata/etdms/1-0/etdms.xsd', - metadataNamespace: 'http://www.ndltd.org/standards/metadata/etdms/1.0/' }, - { metadataPrefix: 'ore', - schema: 'http://www.kbcafe.com/rss/atom.xsd.xml', - metadataNamespace: 'http://www.w3.org/2005/Atom' } + metadataNamespace: 'http://www.ndltd.org/standards/metadata/etdms/1.0/' } ].freeze def bad_verb @@ -89,9 +86,13 @@ def list_records query_params = query_params_from_api_params(params) items, total_count, cursor = public_items_for_metadata_format(**query_params) + + # If the model is nil there was a bad argument; Otherwise, if blank, no items were returned. + raise Oaisys::BadArgumentError.new(parameters: params.slice(:verb)) if items.nil? + params[:item_count] = total_count if params[:item_count].nil? - raise Oaisys::NoRecordsMatchError.new(parameters: params.slice(:verb, :metadataPrefix)) if items.empty? + raise Oaisys::NoRecordsMatchError.new(parameters: params.slice(:verb, :metadataPrefix)) if items.blank? check_resumption_token(items, resumption_token_provided, total_count, params) @@ -134,10 +135,14 @@ def list_identifiers query_params = query_params_from_api_params(params) identifiers_model, total_count, cursor = public_items_for_metadata_format(**query_params) + + # If the model is nil there was a bad argument; Otherwise, if blank, no items were returned. + raise Oaisys::BadArgumentError.new(parameters: params.slice(:verb)) if identifiers_model.nil? + identifiers = identifiers_model.pluck(:id, :updated_at, :member_of_paths) params[:item_count] = total_count if params[:item_count].nil? - raise Oaisys::NoRecordsMatchError.new(parameters: params.slice(:verb, :metadataPrefix)) if identifiers.empty? + raise Oaisys::NoRecordsMatchError.new(parameters: params.slice(:verb, :metadataPrefix)) if identifiers.blank? check_resumption_token(identifiers_model, resumption_token_provided, total_count, params) @@ -158,7 +163,7 @@ def expect_no_args end def expect_args(required: [], optional: [], exclusive: []) - params[:identifier]&.slice! 'oai:era.library.ualberta.ca:' + params[:identifier]&.slice! 'oai:era.library.ualberta.ca:' if params.present? # This makes the strong assumption that there's only one exclusive param per verb (which is the resumption token.) if params.key?(exclusive.first) @@ -217,17 +222,11 @@ def public_items_for_metadata_format(verb:, format:, page:, restricted_to_set: n model = model.public_items model = model.public_items.belongs_to_path(restricted_to_set.tr(':', '/')) if restricted_to_set.present? - model = model.updated_on_or_after(from_date) if from_date.present? - - if until_date.present? - just_after_until_date = (until_date.to_time + 1.second).utc.xmlschema - model = model.updated_before(just_after_until_date) - end - + model = handle_from_and_until_dates(model, from_date, until_date) items_per_request = Oaisys::Engine.config.items_per_request - model = model.page(page).per(items_per_request) + model = model&.page(page)&.per(items_per_request) cursor = (page - 1) * items_per_request - [model, model.total_count, cursor] + [model, model&.total_count, cursor] end def sets_on_page(page:) @@ -278,4 +277,48 @@ def prep_identifiers(parameters) parameters end + # Returning nil gives a bad argument error. + def handle_from_and_until_dates(model, from_date, until_date) + if from_date.present? + from_date_format = get_date_format(from_date) + + case from_date_format + when 1 + model = model.updated_on_or_after(from_date) + when 2 + model = model.updated_on_or_after(DateTime.strptime(from_date, '%Y-%m-%d')) + else + return nil + end + end + + if until_date.present? + until_date_format = get_date_format(until_date) + + case until_date_format + when 1 + just_after_until_date = (until_date.to_time + 1.second).utc.xmlschema + model = model.updated_before(just_after_until_date) + return nil if from_date.present? && from_date_format != 1 + when 2 + model = model.updated_before(DateTime.strptime(until_date, '%Y-%m-%d') + 1.day) + return nil if from_date.present? && from_date_format != 2 + else + return nil + end + end + + model + end + + def get_date_format(date) + if date.match('\b[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z\b') + 1 + elsif date.match('\b[0-9]{4}-[0-9]{2}-[0-9]{2}\b') + 2 + else + 3 + end + end + end diff --git a/config/routes.rb b/config/routes.rb index 857dcea..1e9acaa 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -3,7 +3,14 @@ class PMHConstraint PMHVERBS = %w[Identify ListSets ListMetadataFormats ListRecords GetRecord ListIdentifiers].freeze def matches?(request) - request.query_parameters.key?(:verb) && PMHVERBS.include?(request.query_parameters[:verb]) + # If post request, otherwise get request. + @parameters = if request.raw_post.present? && request.request_parameters.present? + request.request_parameters.symbolize_keys + else + request.query_parameters + end + + @parameters.key?(:verb) && PMHVERBS.include?(@parameters[:verb]) end end @@ -19,7 +26,7 @@ def matches?(request) class IdentifyConstraint < PMHConstraint def matches?(request) - super && request.query_parameters[:verb] == 'Identify' + super && @parameters[:verb] == 'Identify' end end @@ -27,7 +34,7 @@ def matches?(request) class ListSetsConstraint < PMHConstraint def matches?(request) - super && request.query_parameters[:verb] == 'ListSets' + super && @parameters[:verb] == 'ListSets' end end @@ -35,7 +42,7 @@ def matches?(request) class ListMetadataFormatsConstraint < PMHConstraint def matches?(request) - super && request.query_parameters[:verb] == 'ListMetadataFormats' + super && @parameters[:verb] == 'ListMetadataFormats' end end @@ -43,7 +50,7 @@ def matches?(request) class ListRecordsConstraint < PMHConstraint def matches?(request) - super && request.query_parameters[:verb] == 'ListRecords' + super && @parameters[:verb] == 'ListRecords' end end @@ -51,7 +58,7 @@ def matches?(request) class GetRecordConstraint < PMHConstraint def matches?(request) - super && request.query_parameters[:verb] == 'GetRecord' + super && @parameters[:verb] == 'GetRecord' end end @@ -59,7 +66,7 @@ def matches?(request) class ListIdentifiersConstraint < PMHConstraint def matches?(request) - super && request.query_parameters[:verb] == 'ListIdentifiers' + super && @parameters[:verb] == 'ListIdentifiers' end end diff --git a/test/integration/bad_verb_test.rb b/test/integration/bad_verb_test.rb index 197cb27..a64e83c 100644 --- a/test/integration/bad_verb_test.rb +++ b/test/integration/bad_verb_test.rb @@ -10,23 +10,29 @@ class BadVerbTest < ActionDispatch::IntegrationTest def test_bad_verb_xml get oaisys_path(verb: 'nastyVerb'), headers: { 'Accept' => 'application/xml' } - assert_response :success - schema = Nokogiri::XML::Schema(File.open(file_fixture('OAI-PMH.xsd'))) - document = Nokogiri::XML(@response.body) - assert_empty schema.validate(document) + assert_nasty_verb_response + end - assert_select 'OAI-PMH' do - assert_select 'responseDate' - assert_select 'request' - assert_select 'error', I18n.t('error_messages.unknown_verb', bad_verb: 'nastyVerb') - end + def test_bad_verb_xml_post + post oaisys_path(verb: 'nastyVerb'), headers: { 'Content-Type' => 'application/x-www-form-urlencoded', + 'Content-Length' => 82 } + assert_nasty_verb_response end def test_no_verb_xml get oaisys_path, headers: { 'Accept' => 'application/xml' } - assert_response :success + assert_nasty_verb_response + end + + def test_no_verb_xml_post + post oaisys_path, headers: { 'Content-Type' => 'application/x-www-form-urlencoded', 'Content-Length' => 82 } + + assert_nasty_verb_response + end + + def assert_nasty_verb_response schema = Nokogiri::XML::Schema(File.open(file_fixture('OAI-PMH.xsd'))) document = Nokogiri::XML(@response.body) assert_empty schema.validate(document) diff --git a/test/integration/expect_args_test.rb b/test/integration/expect_args_test.rb index 1273cec..fad5b5a 100644 --- a/test/integration/expect_args_test.rb +++ b/test/integration/expect_args_test.rb @@ -10,22 +10,31 @@ class ExpectArgsTest < ActionDispatch::IntegrationTest def test_unexpected_arg_xml get oaisys_path(verb: 'ListMetadataFormats', nastyParam: 'nasty'), headers: { 'Accept' => 'application/xml' } - assert_response :success - schema = Nokogiri::XML::Schema(File.open(file_fixture('OAI-PMH.xsd'))) - document = Nokogiri::XML(@response.body) - assert_empty schema.validate(document) + assert_illegal_or_missing_args_response + end - assert_select 'OAI-PMH' do - assert_select 'responseDate' - assert_select 'request' - assert_select 'error', I18n.t('error_messages.illegal_or_missing_arguments') - end + def test_unexpected_arg_xml_post + post oaisys_path(verb: 'ListMetadataFormats', nastyParam: 'nasty'), + headers: { 'Content-Type' => 'application/x-www-form-urlencoded', 'Content-Length' => 82 } + assert_illegal_or_missing_args_response end - def test_missing_required_arg_xml + def test_missing_required_arg_xml_post # Missing required metadataPrefix param get oaisys_path(verb: 'ListRecords'), headers: { 'Accept' => 'application/xml' } + + assert_illegal_or_missing_args_response + end + + def test_missing_required_arg_xml + # Missing required metadataPrefix param + post oaisys_path(verb: 'ListRecords'), headers: { 'Content-Type' => 'application/x-www-form-urlencoded', + 'Content-Length' => 82 } + assert_illegal_or_missing_args_response + end + + def assert_illegal_or_missing_args_response assert_response :success schema = Nokogiri::XML::Schema(File.open(file_fixture('OAI-PMH.xsd'))) diff --git a/test/integration/identify_test.rb b/test/integration/identify_test.rb index bacb4f2..3e30c6a 100644 --- a/test/integration/identify_test.rb +++ b/test/integration/identify_test.rb @@ -10,6 +10,17 @@ class IdentifyTest < ActionDispatch::IntegrationTest def test_identify_xml get oaisys_path(verb: 'Identify'), headers: { 'Accept' => 'application/xml' } + + assert_proper_identify_response + end + + def test_identify_xml_post + post oaisys_path(verb: 'Identify'), headers: { 'Content-Type' => 'application/x-www-form-urlencoded', + 'Content-Length' => 82 } + assert_proper_identify_response + end + + def assert_proper_identify_response assert_response :success schema = Nokogiri::XML::Schema(File.open(file_fixture('OAI-PMH.xsd'))) diff --git a/test/integration/list_identifiers_test.rb b/test/integration/list_identifiers_test.rb index a625ff1..9ef267d 100644 --- a/test/integration/list_identifiers_test.rb +++ b/test/integration/list_identifiers_test.rb @@ -10,6 +10,18 @@ class ListIdentifiersTest < ActionDispatch::IntegrationTest def test_cannot_disseminate_format_xml get oaisys_path(verb: 'ListIdentifiers', metadataPrefix: 'nasty'), headers: { 'Accept' => 'application/xml' } + + assert_unavailable_metadata_format_response + end + + def test_cannot_disseminate_format_xml_post + post oaisys_path(verb: 'ListIdentifiers', metadataPrefix: 'nasty'), + headers: { 'Content-Type' => 'application/x-www-form-urlencoded', 'Content-Length' => 82 } + + assert_unavailable_metadata_format_response + end + + def assert_unavailable_metadata_format_response assert_response :success schema = Nokogiri::XML::Schema(File.open(file_fixture('OAI-PMH.xsd'))) diff --git a/test/integration/list_metadata_formats_test.rb b/test/integration/list_metadata_formats_test.rb index abda3ff..e91cf76 100644 --- a/test/integration/list_metadata_formats_test.rb +++ b/test/integration/list_metadata_formats_test.rb @@ -10,6 +10,17 @@ class ListMetadataFormatsTest < ActionDispatch::IntegrationTest def test_list_metadata_formats_xml get oaisys_path(verb: 'ListMetadataFormats'), headers: { 'Accept' => 'application/xml' } + + assert_list_metadata_formats_response + end + + def test_list_metadata_formats_xml_post + post oaisys_path(verb: 'ListMetadataFormats'), headers: { 'Content-Type' => 'application/x-www-form-urlencoded', + 'Content-Length' => 82 } + assert_list_metadata_formats_response + end + + def assert_list_metadata_formats_response assert_response :success schema = Nokogiri::XML::Schema(File.open(file_fixture('OAI-PMH.xsd'))) @@ -30,11 +41,6 @@ def test_list_metadata_formats_xml assert_select 'schema', 'http://www.ndltd.org/standards/metadata/etdms/1-0/etdms.xsd' assert_select 'metadataNamespace', 'http://www.ndltd.org/standards/metadata/etdms/1.0/' end - assert_select 'metadataFormat' do - assert_select 'metadataPrefix', 'ore' - assert_select 'schema', 'http://www.kbcafe.com/rss/atom.xsd.xml' - assert_select 'metadataNamespace', 'http://www.w3.org/2005/Atom' - end end end end From 0b4187159865c5b1b9dcf6b8e3ebbd7883e276a6 Mon Sep 17 00:00:00 2001 From: ConnorSheremeta Date: Tue, 3 Nov 2020 13:25:44 -0700 Subject: [PATCH 2/6] Fix rubocop violation --- app/controllers/oaisys/pmh_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/oaisys/pmh_controller.rb b/app/controllers/oaisys/pmh_controller.rb index 10d0872..da9c976 100644 --- a/app/controllers/oaisys/pmh_controller.rb +++ b/app/controllers/oaisys/pmh_controller.rb @@ -312,9 +312,9 @@ def handle_from_and_until_dates(model, from_date, until_date) end def get_date_format(date) - if date.match('\b[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z\b') + if date.match?('\b[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z\b') 1 - elsif date.match('\b[0-9]{4}-[0-9]{2}-[0-9]{2}\b') + elsif date.match?('\b[0-9]{4}-[0-9]{2}-[0-9]{2}\b') 2 else 3 From ef861e3e0d00330acc48903e830d638bb8f77b57 Mon Sep 17 00:00:00 2001 From: ConnorSheremeta Date: Wed, 4 Nov 2020 12:59:30 -0700 Subject: [PATCH 3/6] fix tests --- test/integration/bad_verb_test.rb | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/test/integration/bad_verb_test.rb b/test/integration/bad_verb_test.rb index a64e83c..8c395aa 100644 --- a/test/integration/bad_verb_test.rb +++ b/test/integration/bad_verb_test.rb @@ -11,28 +11,42 @@ class BadVerbTest < ActionDispatch::IntegrationTest def test_bad_verb_xml get oaisys_path(verb: 'nastyVerb'), headers: { 'Accept' => 'application/xml' } - assert_nasty_verb_response + assert_unknown_verb_response end def test_bad_verb_xml_post - post oaisys_path(verb: 'nastyVerb'), headers: { 'Content-Type' => 'application/x-www-form-urlencoded', - 'Content-Length' => 82 } - assert_nasty_verb_response + post oaisys_path(verb: 'nastyVerb'), headers: { 'Content-Type' => 'application/x-www-form-urlencoded', 'Content-Length' => 82 } + + assert_unknown_verb_response end def test_no_verb_xml get oaisys_path, headers: { 'Accept' => 'application/xml' } - assert_nasty_verb_response + assert_no_verb_response end def test_no_verb_xml_post post oaisys_path, headers: { 'Content-Type' => 'application/x-www-form-urlencoded', 'Content-Length' => 82 } - assert_nasty_verb_response + assert_no_verb_response + end + + def assert_unknown_verb_response + assert_response :success + schema = Nokogiri::XML::Schema(File.open(file_fixture('OAI-PMH.xsd'))) + document = Nokogiri::XML(@response.body) + assert_empty schema.validate(document) + + assert_select 'OAI-PMH' do + assert_select 'responseDate' + assert_select 'request' + assert_select 'error', I18n.t('error_messages.unknown_verb') + end end - def assert_nasty_verb_response + def assert_no_verb_response + assert_response :success schema = Nokogiri::XML::Schema(File.open(file_fixture('OAI-PMH.xsd'))) document = Nokogiri::XML(@response.body) assert_empty schema.validate(document) From 3cdcb42570c976a50bb36e342cbd476fd57f8016 Mon Sep 17 00:00:00 2001 From: ConnorSheremeta Date: Wed, 4 Nov 2020 14:05:28 -0700 Subject: [PATCH 4/6] resolve failing test and ruboco nag --- test/integration/bad_verb_test.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/integration/bad_verb_test.rb b/test/integration/bad_verb_test.rb index 8c395aa..acd363f 100644 --- a/test/integration/bad_verb_test.rb +++ b/test/integration/bad_verb_test.rb @@ -15,7 +15,8 @@ def test_bad_verb_xml end def test_bad_verb_xml_post - post oaisys_path(verb: 'nastyVerb'), headers: { 'Content-Type' => 'application/x-www-form-urlencoded', 'Content-Length' => 82 } + post oaisys_path(verb: 'nastyVerb'), headers: { 'Content-Type' => 'application/x-www-form-urlencoded', + 'Content-Length' => 82 } assert_unknown_verb_response end @@ -41,7 +42,7 @@ def assert_unknown_verb_response assert_select 'OAI-PMH' do assert_select 'responseDate' assert_select 'request' - assert_select 'error', I18n.t('error_messages.unknown_verb') + assert_select 'error', I18n.t('error_messages.unknown_verb', bad_verb: 'nastyVerb') end end From 27fb322c60f1a56087a229d9c23aaab3b086d0d6 Mon Sep 17 00:00:00 2001 From: ConnorSheremeta Date: Tue, 24 Nov 2020 12:43:56 -0700 Subject: [PATCH 5/6] skip auth token verification for oaisys --- app/controllers/oaisys/pmh_controller.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/controllers/oaisys/pmh_controller.rb b/app/controllers/oaisys/pmh_controller.rb index da9c976..4774dd6 100644 --- a/app/controllers/oaisys/pmh_controller.rb +++ b/app/controllers/oaisys/pmh_controller.rb @@ -2,6 +2,8 @@ class Oaisys::PMHController < Oaisys::ApplicationController + skip_before_action :verify_authenticity_token + SUPPORTED_FORMATS = [ { metadataPrefix: 'oai_dc', schema: 'http://www.openarchives.org/OAI/2.0/oai_dc.xsd', From ad5eb59a899ebee99f2b67a60f47d333ae4e9961 Mon Sep 17 00:00:00 2001 From: Connor Sheremeta Date: Thu, 14 Jan 2021 14:17:48 -0700 Subject: [PATCH 6/6] Use symbols --- app/controllers/oaisys/pmh_controller.rb | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/app/controllers/oaisys/pmh_controller.rb b/app/controllers/oaisys/pmh_controller.rb index 4774dd6..adcb495 100644 --- a/app/controllers/oaisys/pmh_controller.rb +++ b/app/controllers/oaisys/pmh_controller.rb @@ -285,9 +285,9 @@ def handle_from_and_until_dates(model, from_date, until_date) from_date_format = get_date_format(from_date) case from_date_format - when 1 + when :full_date_with_time model = model.updated_on_or_after(from_date) - when 2 + when :full_date model = model.updated_on_or_after(DateTime.strptime(from_date, '%Y-%m-%d')) else return nil @@ -298,13 +298,13 @@ def handle_from_and_until_dates(model, from_date, until_date) until_date_format = get_date_format(until_date) case until_date_format - when 1 + when :full_date_with_time just_after_until_date = (until_date.to_time + 1.second).utc.xmlschema model = model.updated_before(just_after_until_date) - return nil if from_date.present? && from_date_format != 1 - when 2 + return nil if from_date.present? && from_date_format != :full_date_with_time + when :full_date model = model.updated_before(DateTime.strptime(until_date, '%Y-%m-%d') + 1.day) - return nil if from_date.present? && from_date_format != 2 + return nil if from_date.present? && from_date_format != :full_date else return nil end @@ -314,12 +314,14 @@ def handle_from_and_until_dates(model, from_date, until_date) end def get_date_format(date) + # YYYY-MM-DDThh:mm:ssZ if date.match?('\b[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z\b') - 1 + :full_date_with_time + # YYYY-MM-DD elsif date.match?('\b[0-9]{4}-[0-9]{2}-[0-9]{2}\b') - 2 + :full_date else - 3 + :unknown end end