From 0ef07dfd4b368799387d49df17ce91819fb34b45 Mon Sep 17 00:00:00 2001 From: Jonathan Rochkind Date: Thu, 22 Aug 2019 12:01:39 -0400 Subject: [PATCH] oai-pmh identifiers use colon to separate namespace and id, not slash This is in conformance with oai-pmh non-mandatory guidelines: http://www.openarchives.org/OAI/2.0/guidelines-oai-identifier.htm It also matches how sample identifier in 'Identify' was being generated before. https://github.com/code4lib/ruby-oai/blob/23e3ac83c43231fbf55f8dfb70175dbe0975b914/lib/oai/provider/response/identify.rb#L24 It was a bug that sample identifier didn't match actual identifiers. Hypothetically, it would have been less backwards-incompat to make sample identifier use slash. But let's use 1.0 to make this conform to oai-pmh guidance. Especially becuase the majority of current users of this gem are probably using it via blacklight_oai_provider, and it was already providing runtime patches to the oai gem to get colon behavior, and had a commit message suggesting it believed it was patching an "identifier bug in oai gem." So let's fix the thing our main user thinks was a bug! https://github.com/projectblacklight/blacklight_oai_provider/blob/f4b25ee43846cb9efc9d5124a8639a496153aa18/config/initializers/oai_patches.rb Closes #38 Ref #61 --- lib/oai/provider/response.rb | 2 +- lib/oai/provider/response/record_response.rb | 3 ++- test/activerecord_provider/tc_ar_provider.rb | 10 +++++----- test/client/tc_get_record.rb | 7 +++---- test/client/tc_libxml.rb | 2 +- test/provider/tc_exceptions.rb | 12 +++++++----- test/provider/tc_simple_provider.rb | 14 +++++++------- 7 files changed, 26 insertions(+), 24 deletions(-) diff --git a/lib/oai/provider/response.rb b/lib/oai/provider/response.rb index 916f45e..9f42f02 100755 --- a/lib/oai/provider/response.rb +++ b/lib/oai/provider/response.rb @@ -51,7 +51,7 @@ def header } end def extract_identifier(id) - id.sub("#{provider.prefix}/", '') + id.sub("#{provider.prefix}:", '') end def valid? diff --git a/lib/oai/provider/response/record_response.rb b/lib/oai/provider/response/record_response.rb index 2a798e8..a49dc57 100755 --- a/lib/oai/provider/response/record_response.rb +++ b/lib/oai/provider/response/record_response.rb @@ -48,8 +48,9 @@ def about_for(record) private + # 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.id}" end def timestamp_for(record) diff --git a/test/activerecord_provider/tc_ar_provider.rb b/test/activerecord_provider/tc_ar_provider.rb index 3c8db15..40983b8 100755 --- a/test/activerecord_provider/tc_ar_provider.rb +++ b/test/activerecord_provider/tc_ar_provider.rb @@ -14,7 +14,7 @@ def test_metadata_formats def test_metadata_formats_for_record record_id = DCField.first.id - assert_nothing_raised { REXML::Document.new(@provider.list_metadata_formats(:identifier => "oai:test/#{record_id}")) } + assert_nothing_raised { REXML::Document.new(@provider.list_metadata_formats(:identifier => "oai:test:#{record_id}")) } doc = REXML::Document.new(@provider.list_metadata_formats) assert doc.elements['/OAI-PMH/ListMetadataFormats/metadataFormat/metadataPrefix'].text == 'oai_dc' end @@ -38,11 +38,11 @@ def test_get_record record_id = DCField.first.id assert_nothing_raised do REXML::Document.new(@provider.get_record( - :identifier => "oai:test/#{record_id}", :metadata_prefix => 'oai_dc')) + :identifier => "oai:test:#{record_id}", :metadata_prefix => 'oai_dc')) end doc = REXML::Document.new(@provider.get_record( :identifier => "#{record_id}", :metadata_prefix => 'oai_dc')) - assert_equal "oai:test/#{record_id}", doc.elements['OAI-PMH/GetRecord/record/header/identifier'].text + assert_equal "oai:test:#{record_id}", doc.elements['OAI-PMH/GetRecord/record/header/identifier'].text end def test_deleted @@ -50,8 +50,8 @@ def test_deleted record.deleted = true; record.save 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 + :identifier => "oai:test:#{record.id}", :metadata_prefix => 'oai_dc')) + assert_equal "oai:test:#{record.id}", doc.elements['OAI-PMH/GetRecord/record/header/identifier'].text assert_equal 'deleted', doc.elements['OAI-PMH/GetRecord/record/header'].attributes["status"] end diff --git a/test/client/tc_get_record.rb b/test/client/tc_get_record.rb index 8696303..6b6b376 100644 --- a/test/client/tc_get_record.rb +++ b/test/client/tc_get_record.rb @@ -4,7 +4,7 @@ class GetRecordTest < Test::Unit::TestCase def test_get_one client = OAI::Client.new 'http://localhost:3333/oai' - response = client.get_record :identifier => 'oai:test/3' + response = client.get_record :identifier => 'oai:test:3' assert_kind_of OAI::GetRecordResponse, response assert_kind_of OAI::Record, response.record assert_kind_of REXML::Element, response.record._source @@ -13,8 +13,7 @@ def test_get_one assert_kind_of REXML::Element, response.record.about # minimal check that the header is working - assert_equal 'oai:test/3', - response.record.header.identifier + assert_equal 'oai:test:3', response.record.header.identifier # minimal check that the metadata is working #assert 'en', response.record.metadata.elements['.//dc:language'].text @@ -33,7 +32,7 @@ def test_missing_identifier def test_deleted_record client = OAI::Client.new 'http://localhost:3333/oai' - record = client.get_record :identifier => 'oai:test/275' + record = client.get_record :identifier => 'oai:test:275' assert record.deleted? end diff --git a/test/client/tc_libxml.rb b/test/client/tc_libxml.rb index 4f2d0e5..f87380c 100644 --- a/test/client/tc_libxml.rb +++ b/test/client/tc_libxml.rb @@ -43,7 +43,7 @@ def test_deleted_record uri = 'http://localhost:3333/oai' client = OAI::Client.new(uri, :parser => 'libxml') - response = client.get_record :identifier => 'oai:test/275' + response = client.get_record :identifier => 'oai:test:275' assert response.record.deleted? end diff --git a/test/provider/tc_exceptions.rb b/test/provider/tc_exceptions.rb index 7ed759d..b7c8310 100755 --- a/test/provider/tc_exceptions.rb +++ b/test/provider/tc_exceptions.rb @@ -36,7 +36,7 @@ def test_bad_verb_raises_exception def test_bad_format_raises_exception assert_raise(OAI::FormatException) do - @provider.get_record(:identifier => 'oai:test/1', :metadata_prefix => 'html') + @provider.get_record(:identifier => 'oai:test:1', :metadata_prefix => 'html') end end @@ -45,16 +45,18 @@ def test_missing_format_raises_exception @provider.list_records() end assert_raise(OAI::ArgumentException) do - @provider.get_record(:identifier => 'oai:test/1') + @provider.get_record(:identifier => 'oai:test:1') end end def test_bad_id_raises_exception badIdentifiers = [ - 'oai:test/5000', - 'oai:test/-1', + 'oai:test:5000', + 'oai:test:-1', + 'oai:test:one', 'oai:test/one', - 'oai:test/\\$1\1!'] + 'oai:test/1', + 'oai:test:\\$1\1!'] badIdentifiers.each do |id| assert_raise(OAI::IdException) do @provider.get_record(:identifier => id, :metadata_prefix => 'oai_dc') diff --git a/test/provider/tc_simple_provider.rb b/test/provider/tc_simple_provider.rb index 996a082..d650721 100755 --- a/test/provider/tc_simple_provider.rb +++ b/test/provider/tc_simple_provider.rb @@ -35,7 +35,7 @@ def test_metadata_formats end def test_metadata_formats_for_document - assert_nothing_raised { REXML::Document.new(@simple_provider.list_metadata_formats(:identifier => "oai:test/1")) } + assert_nothing_raised { REXML::Document.new(@simple_provider.list_metadata_formats(:identifier => "oai:test:1")) } doc = REXML::Document.new(@simple_provider.list_metadata_formats) assert_equal "oai_dc", doc.elements['/OAI-PMH/ListMetadataFormats/metadataFormat/metadataPrefix'].text @@ -85,18 +85,18 @@ def test_get_record assert_nothing_raised do REXML::Document.new( @simple_provider.get_record( - :identifier => 'oai:test/1', + :identifier => 'oai:test:1', :metadataPrefix => 'oai_dc' ) ) end doc = REXML::Document.new( @simple_provider.get_record( - :identifier => 'oai:test/1', + :identifier => 'oai:test:1', :metadataPrefix => 'oai_dc' ) ) - assert_equal 'oai:test/1', + assert_equal 'oai:test:1', doc.elements['OAI-PMH/GetRecord/record/header/identifier'].text end @@ -104,18 +104,18 @@ def test_deleted_record assert_nothing_raised do REXML::Document.new( @simple_provider.get_record( - :identifier => 'oai:test/6', + :identifier => 'oai:test:6', :metadataPrefix => 'oai_dc' ) ) end doc = REXML::Document.new( @simple_provider.get_record( - :identifier => 'oai:test/5', + :identifier => 'oai:test:5', :metadataPrefix => 'oai_dc' ) ) - assert_equal 'oai:test/5', doc.elements['OAI-PMH/GetRecord/record/header/identifier'].text + assert_equal 'oai:test:5', doc.elements['OAI-PMH/GetRecord/record/header/identifier'].text assert_equal 'deleted', doc.elements['OAI-PMH/GetRecord/record/header'].attributes["status"] end