Fix the causes of the self link mismatch console warnings - #6083
Fix the causes of the self link mismatch console warnings#6083milanmajchrak wants to merge 2 commits into
Conversation
ensureSelfLink compares the requested url against the self link in the response. Two of the differences it reported were not mismatches: - embed params are removed from the requested url before the comparison, but not from the self link, which echoes them back. - encoding is compared literally. RequestParam encodes with encodeURIComponent while the REST API escapes only what it has to, so uri=http%3A%2F%2Fx returns as uri=http://x. Both sides are now brought to the same form before being compared. Decoding is done per url part after the split, so a decoded '&' cannot merge two params, and is wrapped in try/catch because a malformed escape makes decodeURIComponent throw. The code that rewrites _links.self is unchanged, so caching is unaffected. The third cause is on the frontend side. Spring Data REST caps a page at spring.data.rest.max-page-size, left at its default of 1000, so a request for 9999 returns a self link saying 1000. MAX_PAGE_SIZE replaces the oversized values in the six call sites that used them: bundle-data.service.ts 9999 browse.service.ts 9999 relationship-type-data.service.ts 9999 registry.service.ts 10000 item-bitstreams.service.ts 9999 filtered-items.component.ts 4 x 10000 The API already capped each of these, so the same rows are returned. A reduced page size still warns, so an oversized request added later is still reported. browse-by-geospatial-data.component.ts is left unchanged: its 99999 is a Discovery facet limit, not a page size. Adds a spec for dspace-rest-response-parsing.service.ts, which had none.
lgeggleston
added
bug
error handling
lgeggleston
added
port to dspace-8_x
|
Thank you @milanmajchrak! It looks like it may make sense for this to fully link to/close out DSpace/DSpace#8577 - what do you think? Or, that question could also be left to reviewers. Second, in the case of the remaining legitimate cause you mention of requests for page sizes the REST API cannot serve, would there be a way for the warning message to be more specific than the existing self link one in that case? |
The generic wording ends with "This could mean there's an issue with the REST endpoint", which points at the backend. For a reduced page size that is the wrong place to look: the API did nothing wrong, the caller asked for a bigger page than it will serve. The request for '.../bundles?size=9999' asked for a page of 9999 elements, but the REST API served 1000. Ask for at most MAX_PAGE_SIZE elements Anything else keeps the generic message, including a page that came back larger than requested.
|
Thanks @lgeggleston!
|
lgeggleston
left a comment
lgeggleston
left a comment
There was a problem hiding this comment.
Hi @milanmajchrak! I gave this a quick test, and confirmed I did not see the error anymore on a default item page or browse/title! Didn't see it on homepage before the fix either, though I'm wondering if that's just a difference in the number of items or setup I have locally.
However, I am still seeing the self link error on 2 pages tested: a Person item that includes a small Search section on the item page (the self link error is on the /discover endpoint), and the home page of a community or collection (also the /discover endpoint). I haven't dug into it in detail, but do you think the cause behind those remaining errors might fall into one of the categories you described?
|
Thanks for testing this @lgeggleston! Those two have a different cause, and the warning there is actually correct — the self link really doesn't match the request. That's a backend bug, DSpace/DSpace#8576, so this PR leaves the warning firing on purpose. I've opened a fix for it: DSpace/DSpace#12984. |
lgeggleston
left a comment
lgeggleston
left a comment
There was a problem hiding this comment.
@milanmajchrak Got it, thanks for clarifying and adding that separate fix on the backend! I'll aim to take a look at that as well.
✅ Otherwise, looks good to me! (Note: test, not a full code review - although at a high-level look through the code changes made sense as well.)
github-project-automation
Bot
moved this from 🙋 Needs Reviewers Assigned
to 👍 Reviewer Approved
in DSpace 11.0 Release
2 participants
References
9999as this may bypass pagination #2513RequestParamvalues increase the number of "self link...don't match" logs #3045Description
ensureSelfLink()logsThe response for '…' has the self link '…'. These don't matchon healthy responses. Most of those are not mismatches at all; the remainder are caused by requests for page sizes the REST API cannot serve.Instructions for Reviewers
The check compares the requested url against the self link in the response. Two of the differences it reported were not mismatches:
embedparams are removed from the requested url before the comparison, but not from the self link, which echoes them back.RequestParamencodes withencodeURIComponentwhile the REST API escapes only what it has to, souri=http%3A%2F%2Fxreturns asuri=http://x.Both sides are now brought to the same form before being compared. The code that rewrites
_links.selfis unchanged, so caching is unaffected.The third cause is on the frontend side. Spring Data REST caps a page at 1000, so a request for
9999returns a self link saying1000.MAX_PAGE_SIZEreplaces the oversized values in the six call sites that used them:bundle-data,browse,relationship-type-data,registry,item-bitstreamsandfiltered-items. The API already capped each of these, so the same rows are returned. A reduced page size still warns, so an oversized request added later is still reported.How to test: open an item page, the home page and
/browse/titlewith the browser console open. Before the change each logs the warning, after it none do.Checklist
mainbranch of code.npm run lint.npm run check-circ-deps).Written with some help from Claude Code.