The Wayback Machine - https://web.archive.org/web/20200524180913/https://github.com/ory/hydra/issues/1666
Skip to content

/ hydra

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

Jump to bottom

Document how pagination works #1666

Open
UkonnRa opened this issue Dec 18, 2019 · 11 comments
Open

Document how pagination works #1666

UkonnRa opened this issue Dec 18, 2019 · 11 comments
Labels
docs

Comments

@UkonnRa
Copy link

@UkonnRa UkonnRa commented Dec 18, 2019

Is your feature request related to a problem? Please describe.

When querying the Clients/Consent Sessions using pagination, ORY Hydra will only return results with some links, but not the total count of the items, which is useful to display in the frontend, you know, showing the total pages or something.

Describe the solution you'd like

Add the total_count parameter into Header.

Although I think that the most elegant pagination solution is following JSON:API Spec, but I know it's almost impossible to change the data structure totally. So I think adding a parameter into Header is also a nice solution.
@aeneasr
Copy link
Member

@aeneasr aeneasr commented Dec 19, 2019

I believe we're sending rel=last as part of the HTTP Link Header, similar as we do with next/prev?
@aeneasr
Copy link
Member

@aeneasr aeneasr commented Dec 19, 2019

Yes, that is indeed the case:

https://github.com/ory/hydra/blob/master/client/handler.go#L200-L206

Should probably document this somewhere :D
@aeneasr aeneasr added the docs label Dec 19, 2019
@aeneasr aeneasr changed the title How about returning the total count of items during the pagination query? Document how pagination works Dec 19, 2019
@UkonnRa
Copy link
Author

@UkonnRa UkonnRa commented Dec 20, 2019 •
edited

@aeneasr I have seen the code also, but in fact, there is no total_page param in the Link header or anywhere.

Say I have 100 clients, and calling http://127.0.0.1:4445/clients?limit=10&offset=30. I will get the following response headers:

HTTP/1.1 200 OK
Content-Type: application/json
Link: </clients?limit=10&offset=0>; rel="first",</clients?limit=10&offset=40>; rel="next",</clients?limit=10&offset=20>; rel="prev",</clients?limit=10&offset=50>; rel="last"
Vary: Origin
Date: Fri, 20 Dec 2019 09:17:30 GMT
Transfer-Encoding: chunked

I have no idea where I have to expose more Headers, now my config is just SERVE_ADMIN_CORS_EXPOSED_HEADERS=Link
@aeneasr
Copy link
Member

@aeneasr aeneasr commented Dec 23, 2019

SERVE_ADMIN_CORS_EXPOSED_HEADERS is for CORS, it doesn't have any effect here.

What I meant is that you can calculate the approximate number of items by taking last (in that case it would be about 500 items or "50 pages a 10 items"):

</clients?limit=10&offset=50>; rel="last"

Does that help?
@UkonnRa
Copy link
Author

@UkonnRa UkonnRa commented Dec 23, 2019

If Hydra can ensure that every list request can return the last rel, I think it helps
@aeneasr
Copy link
Member

@aeneasr aeneasr commented Dec 23, 2019 •
edited

I am confused, I thought you always receive the last rel in the header, that's what you said here ( #1666 (comment) ), no? Is the header missing sometimes? And if so, when exactly?
@UkonnRa
Copy link
Author

@UkonnRa UkonnRa commented Dec 24, 2019 •
edited

Like this, #1667, when the limit is larger than the total count, the Link header will only return first and prev(And the prev is wrong). So what should I expect in such situation?

Before fixing the bug, I can only assume that, if lacking of last rel, there is only one page of data. Otherwise, the page count is ${last.offset} / ${last.limit}
@nmlc
Copy link

@nmlc nmlc commented Dec 24, 2019 •
edited

We can see the current behavior of the Link header in these test cases. The case when there is only a single page (#1667) is not covered. If we could agree on the desired behavior (only rel=first or maybe no header at all) I'm willing to fix this.
@UkonnRa
Copy link
Author

@UkonnRa UkonnRa commented Dec 24, 2019

Yes, no matter what the behavior is, I hope the feature could be freezed and documented as fast as we can
@aeneasr
Copy link
Member

@aeneasr aeneasr commented Dec 30, 2019

As always, we happily accept any PRs that help with that - for implementing this within Ory it always depends on internal sprint and milestone planning.
@aeneasr
Copy link
Member

@aeneasr aeneasr commented Jan 3, 2020

@nmlc sorry I missed your comment - your contribution would be very much welcomed!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs
3 participants
@aeneasr @UkonnRa @nmlc
You can’t perform that action at this time.