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

Add pageNumber #76

Merged
merged 5 commits into from Feb 1, 2019

Conversation

Projects
None yet
4 participants
@pgh70
Copy link
Contributor

pgh70 commented Nov 27, 2018

The prev and next links in _links describe to provide a link to the next resultset of a list. No parameter has been specified to accomplish this. To standardize this a pageNumber is added.

Also fixes some issue's with the type:number and format:int32 combination. This combination isn't valid according to OpenAPI v3 spec.

pgh70 added some commits Nov 27, 2018

Add pageNumber to ge a specific page of results
Also fix pageSize type: number -> integer, because int32 isn't a valid format for number
@joostfarla

This comment has been minimized.

Copy link
Member

joostfarla commented Nov 28, 2018

I now remember why we chose to leave the pageNumber parameter out of the specification. Most databases are compatible with a simple page number, because you can calculate an "offset" value (e.g. pageSize * (pageNumber - 1)) from it. But this is not the case for every database. If you look at DynamoDB (AWS) for example, you have to pass the identifier of the next document (which is a set of 1 or two key-value pairs with strings/numbers) as offset. It cannot be calculated by using the pageNumber. Therefore, to not restrict the underlying database type used, I think we should leave it this way. The next and prev links should suffice to do pagination.

- _embedded
- _links
properties:
pageSize:
type: number
type: integer

This comment has been minimized.

@joostfarla

joostfarla Nov 28, 2018

Member

Can you fix this for the whole spec in the oas3 branch?

This comment has been minimized.

@pgh70

pgh70 Dec 5, 2018

Author Contributor

I did in commit: 0c4f6bb. If we decide not to merge this pull-request i will create a seperate pull-request for it.

@usapieter

This comment has been minimized.

Copy link

usapieter commented Nov 28, 2018

@joostfarla

This comment has been minimized.

Copy link
Member

joostfarla commented Nov 28, 2018

based on this reasoning, we have actually chosen to include it our implementation: it would not work without it :) We used it to make the next and prev links to work, as if we would not paginate before doing so, the next and prev links would not work on our side .

Every institution is free to add implementation-specific parameters to make this work. It's just not part of the OOAPI spec. Clients should blindly follow next/prev links without awareness of the parameters used.

@pgh70

This comment has been minimized.

Copy link
Contributor Author

pgh70 commented Dec 5, 2018

I made the pageNumber completly optional. It can be seen as a suggestion how to implement paging to create next and prev links. Of course every institute is free to implement it another way, but by suggesting a pageNumber we prevent reinvention of the wheel by each institute.

@fransward

This comment has been minimized.

Copy link
Contributor

fransward commented Feb 1, 2019

Merged to prepare for V3.0 voting

@fransward fransward merged commit bd3bc15 into open-education-api:oas3 Feb 1, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.