Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Components should never send requests with a page size of 9999 as this may bypass pagination #2513

Open
tdonohue opened this issue Sep 21, 2023 · 0 comments
Labels
bug code task performance / caching Related to performance, caching or embedded objects

Comments

@tdonohue
Copy link
Member

tdonohue commented Sep 21, 2023

Describe the bug

In the dspace-angular codebase, some older components have a hardcoded pagination parameter of:

elementsPerPage: 9999

or

pageSize: 9999

or

elementsPerPage: [SOME OTHER LARGE NUMBER]
# See registry.service.ts
# See item-detail-preview.component.ts

This was an old model used as a way to get "everything" related to a specific REST endpoint, so that the UI could perform some detailed action on them.

This behavior of passing a size of 9999 has two side effects:

  1. It can result in the User Interface logs reporting warnings because the REST API has a maximum page size of 1,000
    The response for '[some-endpoint]?size=9999' has the self link '[some-endpoint]?size=1000'. These don't match. 
    This could mean there's an issue with the REST endpoint
    
    • These warnings are safe to ignore, but they can be confusing because they imply the REST API may have an issue. Instead, the REST API has just changed the size=9999 into size=1000 (it's max value).
  2. It can essentially "bypass" pagination built into the REST API, by attempting to request all objects on one page (assuming there are <1,000 objects). This may result in slower performance on the backend if the REST API has to load many large objects into memory in order to respond to the request.

In many cases, this behavior in dspace-angular implies that a REST API endpoint is missing. The UI is attempting to grab a large amount of data in order to do detailed processing which doesn't belong in the UI layer.

This ticket describes a general "code problem". It may be necessary to create additional tickets to analyze a solution for each scenario where this code problem occurs. Once such example is #2512

Expected behavior
When these hardcoded large page sizes are encountered, they should be removed in favor of a new REST API endpoint. Or, if that is not possible, the dspace-angular component should be redesigned to use pagination in the way it is intended.

Related work
One example in #2512

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug code task performance / caching Related to performance, caching or embedded objects
Projects
Development

No branches or pull requests

1 participant