Blank values for type Valkyrie::ID cause URI-to-ID lookups to return the Fedora base path as an ID

[randalldfloyd]

In Hyrax using Valkyrie (i.e. koppie and sirenia) there are demonstrable scenarios where a work's attributes get changed from nil to type Valkyrie::ID containing a blank string when saved. Specific examples are embargo_id and lease_id when a work is in a one-step workflow. This doesn't adversely affect koppie on postgres, because the blank string seems to be functionally equivalent to nil, AFAIK.

But this breaks ID handling in sirenia on fedora because the URI values on those attributes get stored in fedora as just http://fedora_url/base_path with no trailing ID. The result is that this call to translate the URI to an ID returns just the configured base_path part of the URI as if it were the ID. On the flip side, this call to translate an ID into a URI returns http://fedora_url/base_path/base_path, which will produce Ldp::NotFound errors.

In the context of Hyrax (i.e. sirenia), the actual breaking behavior can be baffling; those IDs now contain the configured base_path when they should have been nil or blank, triggering calls in parts of the stack that it should normally slip silently through. For example, this Hyrax issue describes errors while saving workflow state for a work, which is caused by Ldp::NotFound for the URI returned for embargo_id, even though it is not under embargo.

I'm opening an issue here and in Hyrax because it's not clear if something in Hyrax or the Valkyrie persister itself is converting those ID attributes from nil to Valkyrie::ID @id="" .

Either way, should the Valkyrie fedora adapters also anticipate and mitigate this? Should they:

• Validate the Valkyrie::ID type so that they either can't be a blank string, or at least not persist them to Fedora if they are?
• Make sure the uri_to_id method in the fedora metadata adapter removes the base_path or returns blank if it's trailing as the ID portion of the URI?

[tpendragon]

Hey @randalldfloyd - we looked at this in #605 and implemented Valkyrie::Types::Params::ID - maybe y'all should be using that instead?

[randalldfloyd]

@tpendragon Just realized I never said thanks for that help. Switching to that type for the affected attributes fixed the problem. I have an open Hyrax PR that should fix it, so closing this issue.