code4lib / ruby-oai

a Ruby library for building OAI-PMH clients and servers
MIT License
62 stars 43 forks source link

Discrepancy between sample identifier format and real identifier #38

Closed simonlamb closed 5 years ago

simonlamb commented 10 years ago

Within OAI::Provider::Response::Identify, the sample identifier is set as...

r.sampleIdentifier "#{provider.prefix}:#{provider.identifier}"

Which will result in a sample in the form of "oai:test.edu:123"

Within OAI::Provider::Response::RecordResponse the identifier is formatted with the following code:-

def identifier_for(record)
  "#{provider.prefix}/#{record.id}"
end

This results in the format "oai:test.edu/123"

The easiest (and the one less likely to cause issues for existing users) is to change the Identify class to use:-

r.sampleIdentifier "#{provider.prefix}/#{provider.identifier}"

This will accurately reflect a real identifier format returned by the service.

simonlamb commented 10 years ago

Please ignore the above issue. For information, and perhaps interest to others...

Contradictory to what I said in the initial post. In our usage of Ruby OAI I have overridden OAI::Provider::Response::RecordResponse#identifier_for and changed to:-

"#{provider.prefix}:#{record.id}"

In our testing we found the original code that uses "/" (and thus gave us IDs like 'oai:hull.ac.uk/hull:123') was failing identifier validations using the Open Archives Repository Explorer - http://re.cs.uct.ac.za/. Therefore I implemented the above, which results in IDs like "oai:hull.ac.uk:hull:123".

jrochkind commented 5 years ago

Closing as obsolete/non-applicable.

jrochkind commented 5 years ago

Actually re-opening, I think this is a real problem, at least with the ActiveRecordWrapper.

Identifiers are coming back as prefix/id, which I think is wrong, looking at the oai-pmh spec, I think it should be prefix:id, matching the sample.

That is, I believe : is right, and / is wrong. But it's unclear if changing this will be a backwards breaking change.

jrochkind commented 5 years ago

Summary of issue:

The gem is constructing identifiers as [namespace]/[prefix]. But is construting a sample record in Identify as [namespace]:[prefix]. (colon instead of slash).

OAI guidance suggests the colon, but it is not a requirement just a suggestion.

But they should both match in this gem. I am inclined to switch it to colon to be in conformance with Guidance, even though that's technically a backwards-breaking change.

I think the change may not affect users of blacklight_oai_provider, as it overrides the relevant methods anyway (to use colon). In fact, it overrides them in a monkey-patch file whose commit says "Patch identifier bug in OAI gem", so apparently they considered it a bug -- yep, we should just fix it here.