Skip to content

Conversation

rohkhann
Copy link
Contributor

@rohkhann rohkhann commented Apr 4, 2024

Why make this change?

This change allows users to use the runtime config to provide limits on the number of records that can be retrieved through paginated calls. It also gives them the ability to define a default size which will be the size returned when no pagination input is given (no first in case of graphql and no limit in case of rest calls).

What is this change?

It adds the nullable paginationoptions property to runtimeoptions of runtimeConfig.
Default page size is set to 100.
Max page size is set to 100,000 ( can be altered by the user in their runtimeconfig).

A call with -1 pagination input will result in max page size records being returned. Any call with a pagination number higher than max page size will be rejected with bad request.

In config:
default-page-size is an integer
default-page-size default is 100
default-page-size value -1 means "same as max-page-size"
default-page-size value 0 is an error
default-page-size value less than -1 is an error
default-page-size value more than than max-page-size is an error
max-page-size is an integer
max-page-size default is 100,000
max-page-size value -1 means "same as int.MaxValue"
max-page-size value 0 is an error
max-page-size value less than -1 is an error
max-page-size value more than int.MaxValue is an error

In a query (REST or GQL):
$first=-1 means "whatever the max-page-size value is"
$first=less than -1 is an error
$first=0 is an error
$first=(any value more than max-page-size) is an error

Sample configuration file:

{
  "runtime": {
    "pagination": {
      "default-page-size": -1,
      "max-page-size": 1233
    }
  }
}

How was this tested?

  1. For default page value of 100, invalid page value of 0 or <-1 the existing test cases should cover.
  2. Added tests for both GQL and REST for above conditions.

@rohkhann
Copy link
Contributor Author

rohkhann commented Apr 4, 2024

/azp run

@rohkhann
Copy link
Contributor Author

rohkhann commented Apr 4, 2024

/azp run

@rohkhann
Copy link
Contributor Author

rohkhann commented Apr 5, 2024

/azp run

@rohkhann
Copy link
Contributor Author

rohkhann commented Apr 5, 2024

/azp run

rohkhann and others added 4 commits April 15, 2024 09:01
Co-authored-by: aaronburtle <93220300+aaronburtle@users.noreply.github.com>
Co-authored-by: aaronburtle <93220300+aaronburtle@users.noreply.github.com>
@rohkhann
Copy link
Contributor Author

/azp run

Copy link
Contributor

@seantleonard seantleonard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last nits and should be good to merge once addressed. Still one last remaining question for how we want to handle user provided First:0 since before this change, that resolved to default page size.

rohkhann and others added 3 commits April 15, 2024 11:22
@rohkhann
Copy link
Contributor Author

/azp run

Copy link
Contributor

@seantleonard seantleonard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working through feedback! Pending test of first: 0 for GraphQL in main, and then closing out remainder of discussions since we discussed over a call.

@rohkhann
Copy link
Contributor Author

/azp run

@rohkhann rohkhann enabled auto-merge (squash) April 15, 2024 20:06
@rohkhann rohkhann disabled auto-merge April 15, 2024 20:11
@rohkhann
Copy link
Contributor Author

/azp run

Copy link
Contributor

@aaronburtle aaronburtle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for quickly addressing comments.

@rohkhann
Copy link
Contributor Author

/azp run

@rohkhann
Copy link
Contributor Author

/azp run

@rohkhann
Copy link
Contributor Author

/azp run

@rohkhann rohkhann enabled auto-merge (squash) April 16, 2024 20:16
@rohkhann rohkhann merged commit a999b62 into main Apr 16, 2024
@rohkhann rohkhann deleted the rohkhann/AddingPaginationLimits branch April 16, 2024 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants