Lookup and redirect by uuid and handle

[mspalti]

#256

This initial work on routing by id works with uuid requests. In separate work I was able to test handle routing using cached objects. However, I think we need a real REST endpoint for handle lookups in the actual implementation.

I'm currently using the dso endpoint to look up communities, collections and items. Wondering if that endpoint is the correct place to add support for handle (and other id type) lookups?

UPDATE: Discovered that the pid endpoint is designed for this task. Updated.

[mspalti]

@artlowel, I have a question about the motivation for this work.

The obvious motivation is resolving external handle (or other identifier) requests. Each time one of these external requests happens the application state gets re-initialized, previously cached data is lost, and the REST api is queried by handle for dso data, etc.

Following the recommendation in #256, handle ids are added to the object cache. We also query the cache by handle/uuid in RequestService.isCachedOrPending(). That suggests that later on RequestService.configure() should (in some cases that I'm not sure about) use objectCache.getObjectByID(handle) when configuring the request.

I can see a way to do this for findById(). It involves additional changes to data.service and request.service. But obviously I should first verify the need since the handle index may exist for other reasons and I might be on the wrong track.

[mspalti]

Integration test failed for one of the node versions.

[mspalti]

After looking at the planning spreadsheet I realized that the REST API task was completed.

I looked again and saw that the pid endpoint uses IdentifierService providers. Updating code to use that endpoint. I presume it will work for handles when that's done. We'll see.

[mspalti]

As expected, routing now works for handles. Examples of working routes:

id/uuid/9b4f22f4-164a-49db-8817-3316b6ee5746
id/handle/123456789%2F22
handle/123456789/22

[mspalti]

This should be ready for review now.

[mspalti]

I updated the object-not-found page with i18n messages. Should be reviewed.

(This isn't necessarily a 404 so I think the message can't just mimic the existing page-not-found.)

[mspalti]

I'm getting back this after some time away from the office. I noticed that I had introduced a bug into the legacy handle lookup -- some unnecessary url encoding. Redirects seem to be working again. Here's a lookup by handle that succeeds (using the demo REST api):

http://localhost:3000/handle/10673/703
http://localhost:3000/id/handle/10673%2F703

[mspalti]

The pid endpoint supports a single GET method that takes an id parameter. The DSpace IdentifierService.resolve(context,id) is called directly from the REST controller and the dso is converted to a response.

Well actually ... the REST controller just sends a redirect to the endpoint for the actual object. The client code receives the result of that redirect.

[mspalti]

The pid should support DOIs as well as handles. That obviously hasn't been tested in this PR.

[artlowel]

Thanks @mspalti!

It works for handles. But it's a shame you need to escape the handle in the second case. I think you could write a custom UrlMatcher to use in the routing module instead of the path. That could cover the case where if the idType is handle, you need to include the slash in the id. Here's an article about it: https://medium.com/@brandontroberts/custom-route-matching-with-the-angular-router-fbdd48665483

[mspalti]

Thanks for that suggestion, @artlowel . I added a matcher function so a url like this now works:

http://localhost:3000/id/handle/10673/703

[mspalti]

There is a separate REST endpoint dso that can be used to lookup by uuid. I know it works because I tried it previously.

If we want both uuid and handle lookups in this PR, it would certainly be possible to switch the endpoint based on the idType. Or the pid REST endpoint could be modified to support uuids.

[mspalti]

This PR creates a object/uuid-to-self-link/handle index that is not used for handle lookups. I'm mentioning that again just to make sure that this is ok. It doesn't seem necessary for what I think of as the typical use case: redirecting external handle (doi, etc) requests to the object. If there are other use cases in which redirection should make use of cached objects then that would need to be added now, or later.

[mspalti]

I added uuid lookup and redirect using the dso endpoint. So links like this now work:

http://localhost:3000/id/uuid/98b3e297-dd7e-4da7-a93e-67e5622e37ed

I notice that the dso endpoint is currently used by search and browse pages via the DSpaceObjectDataService.

[mspalti]

@artlowel , I decided to add uuid lookups by switching between endpoints.

I don't know if there was more discussion about the REST endpoints at the last WG meeting. For now, I just assumed the status quo. I'm using the dso endpoint when the identifier is a uuid; otherwise using the pid endpoint. Both uuid and handle lookups are working.

[artlowel]

This PR creates a object/uuid-to-self-link/handle index that is not used for handle lookups. I'm mentioning that again just to make sure that this is ok.

You're right, I don't think we need that index, the uuid should be the identifier we use internally, support for others should only be available at the /id and /handle routes.

@artlowel , I decided to add uuid lookups by switching between endpoints.

I don't know if there was more discussion about the REST endpoints at the last WG meeting. For now, I just assumed the status quo. I'm using the dso endpoint when the identifier is a uuid; otherwise using the pid endpoint. Both uuid and handle lookups are working.

That sounds fine.

If I enter an identifier that doesn't exist I keep seeing the loading component instead of a 404. The console shows an "EmptyError: no elements in sequence", so it's likely due to a first() somewhere. If you replace it by take(1) that should solve it. (first requires the observable to always emit something, take(1) doesn't)

[mspalti]

The 404 response is working. I needed to remove the successful response filter in DsoRedirectDataService.findById() so that the observable emits the response regardless of whether it succeeds.

[mspalti]

Removed the separate handle index. Back to using the single uuid-to-self-link cache (and the IndexName enum). The IdentifierType is still useful to distinguish between when to use dso and pid endpoints.

[mspalti]

One problem. When I got back to this today, and before making any code changes, I saw only 404 responses from the demo server. Testing against my local dspace instance (which is synced to master) works fine. So I'm assuming that something has changed in the REST demo

[mspalti]

Actually, the uuid lookup works fine on the demo server. Handle lookups do not seem to work.

[artlowel]

I see the same thing. I'll bring it up in the meeting today.

[mspalti]

I think it might be a good idea to add a new message to the i18n files (rather than repurposing existing messages) but I wasn't sure how the translation process worked. Looking at Marie's i18n PR and Bram's comment there, I see translation work is ongoing. Let me know if I should propose language and to add it to the english i18n. (For display in the objectnotfound component.)

[artlowel]

Feel free to add new messages.

We ask that you always add new messages in English to en.json5. If you happen to know any of the other languages, it is appreciated if you add it to those as well. But the English file should always be kept up to date.

[artlowel]

I still get the same issue against the demo rest backend. But as it seems this is a rest issue, this PR can be merged

[atarix83]

@mspalti Thanks for the work

the code looks good, and it works properly on my local rest server.

I don't know why most of the ids are not found on rest demo server, but I found one that works

http://localhost:3000/handle/10673/790