Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

If AR model provides no "sets", then default should be [] instead of nil to avoid uncaught exception #109

Merged
merged 1 commit into from
Mar 4, 2025

Conversation

jrochkind
Copy link
Collaborator

@jrochkind jrochkind commented Mar 4, 2025

Previously, if the model defined no sets, and a client asked for them, an uncaught exception would be raised, NoMethodError: undefined method detect' for nil`

With a pretty darn confusing stack trace

/Users/jrochkind/code/ruby-oai/lib/oai/provider/model/activerecord_wrapper.rb:123:in `find_set_by_spec'
/Users/jrochkind/code/ruby-oai/lib/oai/provider/model/activerecord_wrapper.rb:99:in `find_scope'
/Users/jrochkind/code/ruby-oai/lib/oai/provider/model/activerecord_wrapper.rb:51:in `find'
/Users/jrochkind/code/ruby-oai/lib/oai/provider/response/list_records.rb:19:in `to_xml'
/Users/jrochkind/code/ruby-oai/lib/oai/provider.rb:444:in `list_records'
/Users/jrochkind/code/ruby-oai/lib/oai/provider.rb:470:in `process_request'
app/controllers/oai_pmh_controller.rb:27:in `index'

By making the default return [] instead of nil, it changes to an appropriate OAI::NoMatchException which is rescued properly by controller to return this response:

<OAI-PMH xmlns="http://www.openarchives.org/OAI/2.0/" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.openarchives.org/OAI/2.0/ http://www.openarchives.org/OAI/2.0/OAI-PMH.xsd">
<responseDate>2025-03-04T17:56:12Z</responseDate>
<request>http://localhost:3000/oai</request>
<error code="noRecordsMatch">The combination of the values of the from, until, set and metadataPrefix arguments results in an empty list.</error>
</OAI-PMH>

I don't know if it's ideal, but seems to be what was intended.

I do not use sets, and am not totally sure functionality is working if you do, haven't tracked down a test and the code isn't looking great to me -- But pretty sure this change won't break anything, a sets method that was defined, as modelled, seems like [] would be a legal return value. But maybe not nil.

Got to the bottom of this after local error: sciencehistory/scihist_digicoll#2911

…nil to avoid uncaught exception

Previously, if the model defined no sets, and a client asked for them, an uncaught exception would be raised, `NoMethodError: undefined method `detect' for nil`

By making the default return `[]` instead of `nil`, it changes to an appropriate `OAI::NoMatchException` which is rescued properly by controller etc.

I do not use sets, and am not totally sure functionality is working if you do, haven't tracked down a list. But pretty sure this wont' break anything.
@jrochkind jrochkind force-pushed the fix_request_for_set_when_none_defined branch from c76b4f9 to 4fe86b7 Compare March 4, 2025 18:13
@jrochkind jrochkind merged commit 6f597e7 into master Mar 4, 2025
8 checks passed
@jrochkind jrochkind deleted the fix_request_for_set_when_none_defined branch March 4, 2025 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant