Fix the causes of the self link mismatch console warnings - #6083
Open
milanmajchrak wants to merge 2 commits into
Open
Fix the causes of the self link mismatch console warnings#6083milanmajchrak wants to merge 2 commits into
milanmajchrak 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
Contributor
|
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? |
Merged
7 tasks
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.
Contributor
Author
|
Thanks @lgeggleston!
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.