feat: link pagination with query parameters and improve wording (#445) #454
Conversation
tkrop
force-pushed
the
445-link-pagination-with-standard-query-parameters
branch
from
0e8f342
to
fcbc941
Compare
chapters/naming.adoc
Outdated
| "expand" correctly is difficult, so do it with care. See <<158>> below. | ||
| * `cursor` — key-based page start. See <<pagination>> section below. | ||
| * `offset` — numeric offset page start. See <<pagination>> section below. | ||
| Hint: In combination with limit, you can use page as an alternative to |
There was a problem hiding this comment.
page with code ticks and doesn't page have a different semantic then offset?
My understanding:
- offset is the absolute number of records
- page is offset/limit
- limit can be understood as a page size
There was a problem hiding this comment.
Damned, I missed this. I wanted to remove this too.
|
@whiskeysierra thanks for the comment. Do you think we should keep |
No, but we may add better descriptions to each parameter to make their semantic clear. |
|
@whiskeysierra good idea. |
chapters/naming.adoc
Outdated
| should have an entity specific alias, like sku | ||
| * `sort` — comma-separated list of fields to define the sort order. To | ||
| indicate sorting direction, fields may be prefixed with + (ascending) or | ||
| - (descending, default), e.g. /sales-orders?sort=+id |
There was a problem hiding this comment.
Why is descending the default?
There was a problem hiding this comment.
(I realize that you didn't introduce that, just asking.)
There was a problem hiding this comment.
@whiskeysierra I also only guess, but I think order is often based on date. Thus descending might make sense to see the most resent results first. But I would like to question this as you. Lets remove this!
chapters/naming.adoc
Outdated
| `embed` correctly is difficult, so do it with care. See <<158>> below. | ||
| * `offset` — numeric offset of the first element on a page. See <<pagination>> | ||
| section below. | ||
| * `cursor` — encoded pointer to the first or last element on a page, the |
There was a problem hiding this comment.
Isn't that an implementation detail? From an abstract perspective the cursor just addresses a single page, doesn't it?
There was a problem hiding this comment.
@whiskeysierra Yes, it is an implementation detail, that describes best practice. It is based on my experience in the workshops that many developers have no idea how to start. Thus, I think it might be helpful to have a description about what a cursor is about. When I start to explain this, I often see people starting to understand this concept. What do you think.
There was a problem hiding this comment.
I'm skipping over my belief that we shouldn't document cursor at all and I might open up that discussion separately.
Let's try to split apart the API and implementation aspects more clearly then. Clients should only care about the contract that a cursor provides (opaque value, never to be inspected/constructed, addresses a single page) while providers care about implementation hints (cursor encodes direction although I'm still not a big fan of that, forward and backward cursors address the first/last item of the page respectively).
Mixing both in a sentence creates some confusion that we can try to avoid.
tkrop
force-pushed
the
445-link-pagination-with-standard-query-parameters
branch
from
0fb5d38
to
d0156ea
Compare
chapters/naming.adoc
Outdated
| `embed` correctly is difficult, so do it with care. See <<158>> below. | ||
| * `offset` — numeric offset of the first element on a page. See <<pagination>> | ||
| section below. | ||
| paque value, never to be inspected/constructed, addresses a single page |
There was a problem hiding this comment.
Sorry, intended to remove this fragment. Not supposed to be part of offset definition.
| * `embed` — to expand or embedded sub-entities (ie.: inside of an article | ||
| entity, expand silhouette code into the silhouette object). Implementing | ||
| `embed` correctly is difficult, so do it with care. See <<158>> below. | ||
| * `offset` — numeric offset of the first element on a page. See <<pagination>> |
There was a problem hiding this comment.
Why do we define offset and cursor independently? Wouldn't it be beneficial for both to be indistinguishable to clients so that servers can migrate from one to the other?
There was a problem hiding this comment.
In case of offset, there may be a use case for setting the value manually. Unless we propose a more advanced scheme for pagination links with, the offset is not opaque.
| paque value, never to be inspected/constructed, addresses a single page | ||
| * `cursor` — an opaque pointer to a page, never to be inspected/constructed by | ||
| clients. It usually (encrypted) encodes the identifier of the first or last | ||
| page element, the pagination direction, and the applied query filters to |
There was a problem hiding this comment.
I wouldn't encrypt the query filters in the cursor. Those are query parameters that clients should in fact modify (at least on the very first request when they start their pagination).
There was a problem hiding this comment.
The behavior of the pagination gets absolutely unpredictable, if you allow to change the query filters, e.g. the selector page element might even not belong to the page making the cursor fail. If you want to change the query filters, you have to paginate from begin. Thus, it is best practice to include the filters into the cursor, and neglect all query filters given in addition.
There was a problem hiding this comment.
If you want to change the query filters, you have to paginate from begin.
Agreed, but that requirement in itself doesn't dictate that all other query parameters need to be encoded/encrypted inside the cursor. It just needs to be part of the contract that clients are not allowed/meant to modify the pagination links in any which way.
There was a problem hiding this comment.
You can only check correctness of filters with the cursor, if you put them into the cursor. There is no other state information in pagination.
|
👍 |
|
👍 |
|
|
||
| * `q` — default query parameter (e.g. used by browser tab completion); should | ||
| have an entity specific alias, like sku | ||
| * `sort` — comma-separated list of fields to define the sort order. To indicate |
There was a problem hiding this comment.
Does it have to be comma-separated? We specify the format for collection query parameters in 154. A link to the rule would be nice.
|
👍 |
Closes #445.
This change tries to better communicate a consistent look and feel of APIs.
While it replaces a
MAYby aMUST, I think this was the intended semantic for providing conventional query parameters supporting query parameters for searching, sorting, filtering, and paginating. Thus I'm not sure whether this is a guideline change.I also remove the alternate
sizeforlimit, andpageforoffsetto enforce a stricter look and feel. If voted for I will reintroduce them again.