Skip to content

Instantly share code, notes, and snippets.

@kierenj
Created December 19, 2017 14:57
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save kierenj/26d9862b2a7415c0c973c98031e2d0d5 to your computer and use it in GitHub Desktop.
Save kierenj/26d9862b2a7415c0c973c98031e2d0d5 to your computer and use it in GitHub Desktop.
Paging defaults

Paging by default

problems

  • easy to forget default limit is there - get dropdowns with just a few options, etc
  • have to specify arbitrary-ish big number for page_size to remove limit. how to choose?

benefits

  • not having to remember to implement paging
  • query performance by default

possible solutions

  1. don't used PagedView<> for lookups or queries where consumer probably won't want paging
  2. don't make page_number optional for queries with paging: throw error if missing. can't mistake that it's a paged view then
  3. only use paging if page_number/page_size specified

KJ - prefer 1&2, 3 could mean getting in the habit of not paging, having slow queries + big results pages?

@dylmye
Copy link

dylmye commented Dec 19, 2017

+1 for solution 1. Makes the most sense to me

@sedders123
Copy link

sedders123 commented Dec 19, 2017

I agree that 1,2 are good ideas. I have seen a couple of cases where PagedView has been used when it probably shouldn't have been (unfortuantley can't remember where exactly). I think making page_number required for paged queries would have helped in these cases and possibly would have made the developer question whether PagedView was the best option. Could it be a part of the code review to check that limits and defaults are sensible?

@adamsandle
Copy link

1+2 make sense. The only issue I can see but it is a very edge case and can't think of where I've had to do it (something to do with SST user dropdown possibly) is where there are cases for a paged view and a non paged view. This would need a second endpoint. Not a huge issue since its usually a workaround for something and not done regularly

@sam-rr
Copy link

sam-rr commented Jan 3, 2018

I've had a look around at paged endpoints in other public APIs and the general approach seems to be as follows:

  • It's common for pageSize and pageNumber to have default values. A lot of the time this seems to be in the region of 20 items per page.
  • It's also very common that pageSize has a maximum number that you can supply to it. This mostly seems to be 100 items (this could vary depending upon the complexity of the query).

Based upon this I suggest the following approaches:

  • We should carefully consider whether a paged view is necessary. If a collection of entities is always going to contain less than 20-30 items then paging probably shouldn't be used (e.g. Diamond Carriers)
  • We should stick to setting pageSize and pageNumber defaults to whatever is the typical view on the frontend. So our current typical values of 10 and 1 are probably fine.
  • We should add a maximum limit to pageSize to prevent large queries being executed. This could easily be implemented as part of the validators (there are already requirePageSize and requirePageNumber validators that should be being used). Probably a default of 100 but can be overridden as a parameter on the requirePageSize function.

@kierenj
Copy link
Author

kierenj commented Jan 17, 2018

Would there be much of a downside to mandating a pageNumber query parameter for paged queries? So the caller definitely knows it's paged then? I know from the sounds of it other APIs just default it, and the biggest positive change is carefully considering which endpoints should be paged, but this seems almost free/easy-ish?

@sam-rr
Copy link

sam-rr commented Jan 18, 2018

Yes I think that is reasonable, that in combination with the pageSize max limit should enforce good responsible usage of paged endpoints.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment