samvera / ldp

Linked Data Platform.rb client
Other
16 stars 15 forks source link

.new? and ActiveFedora .exist? are broken with misplaced 400 errors. #33

Closed flyingzumwalt closed 9 years ago

flyingzumwalt commented 9 years ago

Ldp::Resource.new? and ActiveFedora::FinderMethods.exists? both raise this error when they should be returning false:

Ldp::HttpError:
       STATUS: 400 ... 

The underlying issue might be that Fedora is returning 400 when it should return 404, but either way, the methods are failing on the simple cases where an object doesn't exist and the code is checking whether it exists.

flyingzumwalt commented 9 years ago

@jcoyne / @cbeer I need your help deciding how to handle this. Haven't touched the ldp code before.

jcoyne commented 9 years ago

Do you have a test case? it works for me:

ActiveFedora::Base.create(id: 'foo')
=> #<ActiveFedora::Base id: "foo">
ActiveFedora::Base.exists?('foo')
=> true
ActiveFedora::Base.exists?('bar')
=> false
flyingzumwalt commented 9 years ago

It's happening in the worthwhile code base. I was stumped on how to create a test case. Hit this at the end of the work day so logged the ticket and plan to look again in the morning.

flyingzumwalt commented 9 years ago

Here's what was triggering it:

ActiveFedora::Base.exists?('foo:123')
=> Ldp::HttpError Exception: STATUS: 400 ...
=> nil

My test has fedora3 -style pids. That's what was triggering it. Could we make the error message more informative? Even if it had said something like "the request contains an illegal character ':'" I would have figured it out.

See: "PID Mapping and Retention" section of https://wiki.duraspace.org/display/FF/Fedora+3+Object+representation+in+Fedora+4

jcoyne commented 9 years ago

That's a perfectly legal character, provided you've configured Fedora correctly. I'm not sure we can do anything better unless fcrepo is updated to give a better response