Skip to content
This repository has been archived by the owner on Mar 11, 2021. It is now read-only.

Expose pagination metadata in WorkItem List API #329

Closed
aslakknutsen opened this issue Sep 30, 2016 · 10 comments
Closed

Expose pagination metadata in WorkItem List API #329

aslakknutsen opened this issue Sep 30, 2016 · 10 comments
Milestone

Comments

@aslakknutsen
Copy link
Contributor

  • next/prev links ?
  • total count ?

For reference https://developer.github.com/v3/#pagination, http://jsonapi.org/format/#fetching-pagination

@ldimaggi
Copy link

We'd probably want to see these options:

  • Prev/Next/Home
  • Page counter (page N of NN)
  • Total record/workitem count

@Mgranfie
Copy link

@ldimaggi @michaelkleinhenz @joshuawilson

See patternfly:
https://redhat.invisionapp.com/share/VW8E0F2DY

(can add totals to the PF pager if this sounds like a priority)

See mock ups for totals:

https://redhat.invisionapp.com/d/main/#/console/8575185/183714495/preview

@aslakknutsen
Copy link
Contributor Author

@tsmaeder
Copy link
Contributor

tsmaeder commented Oct 3, 2016

Why are we using jsonapi.org as a reference when we are not following their recommendations anyway? Our response documents don't look anything like jsonapi.org docs.

@aslakknutsen
Copy link
Contributor Author

aslakknutsen commented Oct 3, 2016

@tsmaeder Maybe reference was the wrong word. Example. Similarly we're not using GitHub either so...

@VineetReynolds
Copy link
Contributor

Chiming in with a short response, since I've worked with this problem before.

At a high level, supporting links like next/prev ought to be handled using Link headers, as the GitHub v3 API does. Any API that uses RFC 5988 is a good reference for your problem. Older REST APIs had next/prev links within the message envelope, so if you find references to such examples, treat them as an outdated practice. As an example, the following practice is not recommended anymore, since it forces an envelope to be created just for next/prev links:

{
 "previous": "http://api.example.com/items?page=1",
 "next": "http://api.example.com/items?page=3",
 "_items": [{a,b,c}]
}

Total count of available items in a collection is an interesting problem. Say, the client requested for a paginated result of 10 items, but there are 30 in the collection, and the API ought to respond with information to the client that a total count of 30 items are available. There are two choices:

  • Respond with the count in the envelope, like so:

    {
     "total": 30,
     "count": 10,
     "_items": [{a,b,c,d,e,f,g,h,i,j}]
    }
    
  • Or respond with the count as a HTTP header. Since this is a custom HTTP header, a header with name like X-Total-Count or something thereof, is recommended.

The overarching theme in pagination is whether you want to create an envelope, since it almost always forces a simpler response with JSON arrays to be converted into a response with an envelope.

@aslakknutsen
Copy link
Contributor Author

@VineetReynolds Got any references to why this the 'old' vs the 'new' way of doing it, besides personal opinion?

  • What about other links that are not paging related?
  • What about links within the individual items in the collection?

If there is a need for JSONP support in the future then all use of HTTP Headers fall apart.

@VineetReynolds
Copy link
Contributor

@aslakknutsen , before the rest of my comment is read, I'd like to clarify something old vs new should not be interpreted as "Do the new thing instead of the old." Instead it should be "Do the new thing if it makes more sense".

Subbu's post here https://www.subbu.org/blog/2008/10/generalized-linking is one of the earliest references to creating link representations in JSON. The RFC came later, and you can see the RFC being referenced in this Safari Book.

There's always going to be confusion over whether to use Link headers or links in JSON representations. GitHub seems to use the former for next/prev links, so they don't have to put these in an envelope. Their API however does not return a count of items in the collection (their UI, does but this is server-side rendered UI). That might explain why they do things in a certain way.

So, realistically this problem won't be solved until JSON representations for REST resources will be standardized. This RFC was in the draft stage, with no movement, so don't expect pagination in JSON resource representations to be standardized in the immediate future.
The drawback of not using what comes with a standard is backward compatibility - if you break the way pagination occurs in the REST API, you pretty much need to create a new version of it, and communicate to all clients that pagination is handled differently henceforth.

On JSONP, I'd recommend not supporting it, if you can get CORS support to function for all browser-based clients. That might mean allowing some sort of a registration facility for browser-based apps which feeds into the decision of what values (i.e. which external domains do we trust? Or is this * ?) are sent in the CORS headers by an ALMighty core deployment. Also, JSONP is only for HTTP GET.

@VineetReynolds
Copy link
Contributor

VineetReynolds commented Oct 5, 2016

@aslakknutsen To respond to the other specific queries:

On links that are not pagination related, they could go in either place - Link header or in the JSON envelope.

On links to individual items in the collection, assuming your URIs are of the form http://api.example.com/items (for the collection) and http://api.example.com/items/<item_id> (for individual items in it), then Link header examples will be like:

('firstItem' above refers to first item in collection since you could also want to put a link to first page, so the link name needs to be thought through).

I'll also post some details about how links are used. You could embed links within individual items in the collection. This really depends on how you design the resource representations. So you could end up with something like this example which is more complicated (links are embedded as a separate attribute, i.e. array, in each of the items in the collection, with each item having various different links in them - the first item has no previous link, and the last item has no next link etc., reasons for which should be obvious):

{
  "previous": "http://api.example.com/items?page=1",
  "next": "http://api.example.com/items?page=3",
  "_items": [{
     {
       "name":"a",
       "links: [
            "next" : "http://api.example.com/items/b"
        ]
     },
     {
       "name":"a",
       "links: [
            "next" : "http://api.example.com/items/c",
            "previous" : "http://api.example.com/items/b",
        ]
     },
     {
       "name":"c",
       "links: [
            "previous" : "http://api.example.com/items/b"
        ]
     }
   }]
}

Where you create the link in the resource representation depends on what the clients wants. The above case of embedding links within every item in the collection is quite rare (it's almost like a HTML page). Often links are generated only at the collection level when the collection (or a page in the collection) is queried (so you can browse to the first page, next page, previous or last page in collection), or at the individual item in colleciton level when an individual element in the collection is queried (so you can browse to next or previous item, or related items etc).

@aslakknutsen
Copy link
Contributor Author

@VineetReynolds Embedded links in the Items in the Collection are rarely used for 'Pagination', that I agree. But I don't agree, Links in Items in a Collection are rare. An Item in a Collection would at very minimum have a 'self' link to point to the Item.

Example: https://developer.github.com/v3/pulls/#list-pull-requests

The Item level 'links' is required either way, why would we not have 'links' on our Collection object in the same way as our Items? What makes a Collection of Items any different from an Item.

@aslakknutsen aslakknutsen modified the milestones: Sprint #121, Sprint #122 Oct 19, 2016
aslakknutsen pushed a commit that referenced this issue Oct 31, 2016
Moved API towards JSONAPI structure using a data.{}, links: {} type structure.

The links exposed by pagination are prev, next, first and last.

API mounted on /api/workitem.2 to not break deployed UI.

Fixes #329
aslakknutsen added a commit to aslakknutsen/fabric8-wit that referenced this issue Nov 2, 2016
Query parameter separator should be '&' not ','

Related fabric8-services#329
aslakknutsen added a commit to aslakknutsen/fabric8-wit that referenced this issue Nov 3, 2016
Query parameter separator should be '&' not ','

Upgrade to go 1.7 is required so we can add a new custom encoder to goa
that turn of & to \u0026 escaping in json strings

Related fabric8-services#329
aslakknutsen added a commit to aslakknutsen/fabric8-wit that referenced this issue Nov 3, 2016
Query parameter separator should be '&' not ','

Upgrade to go 1.7 is required so we can add a new custom encoder to goa
that turn of & to \u0026 escaping in json strings

Fixes fabric8-services#427
Related fabric8-services#329
aslakknutsen added a commit to aslakknutsen/fabric8-wit that referenced this issue Nov 4, 2016
Query parameter separator should be '&' not ','

Upgrade to go 1.7 is required so we can add a new custom encoder to goa
that turn of & to \u0026 escaping in json strings

Fixes fabric8-services#427
Related fabric8-services#329
aslakknutsen added a commit to aslakknutsen/fabric8-wit that referenced this issue Nov 4, 2016
Query parameter separator should be '&' not ','

Upgrade to go 1.7 is required so we can add a new custom encoder to goa
that turn of & to \u0026 escaping in json strings

Fixes fabric8-services#427
Related fabric8-services#329
aslakknutsen added a commit to aslakknutsen/fabric8-wit that referenced this issue Nov 4, 2016
Query parameter separator should be '&' not ','

Upgrade to go 1.7 is required so we can add a new custom encoder to goa
that turn of & to \u0026 escaping in json strings

Fixes fabric8-services#427
Related fabric8-services#329
aslakknutsen added a commit to aslakknutsen/fabric8-wit that referenced this issue Nov 4, 2016
Query parameter separator should be '&' not ','

Upgrade to go 1.7 is required so we can add a new custom encoder to goa
that turn of & to \u0026 escaping in json strings

Fixes fabric8-services#427
Related fabric8-services#329
aslakknutsen added a commit to aslakknutsen/fabric8-wit that referenced this issue Nov 4, 2016
Query parameter separator should be '&' not ','

Upgrade to go 1.7 is required so we can add a new custom encoder to goa
that turn of & to \u0026 escaping in json strings

Fixes fabric8-services#427
Related fabric8-services#329
aslakknutsen added a commit that referenced this issue Nov 4, 2016
Query parameter separator should be '&' not ','

Upgrade to go 1.7 is required so we can add a new custom encoder to goa
that turn of & to \u0026 escaping in json strings

Fixes #427
Related #329
aslakknutsen added a commit to aslakknutsen/fabric8-wit that referenced this issue Nov 4, 2016
aslakknutsen added a commit to aslakknutsen/fabric8-wit that referenced this issue Nov 4, 2016
aslakknutsen added a commit that referenced this issue Nov 5, 2016
baijum pushed a commit that referenced this issue Nov 5, 2016
aslakknutsen added a commit to aslakknutsen/fabric8-wit that referenced this issue Nov 6, 2016
aslakknutsen added a commit to aslakknutsen/fabric8-wit that referenced this issue Nov 6, 2016
aslakknutsen added a commit to aslakknutsen/fabric8-wit that referenced this issue Nov 7, 2016
aslakknutsen added a commit to aslakknutsen/fabric8-wit that referenced this issue Nov 7, 2016
aslakknutsen added a commit to aslakknutsen/fabric8-wit that referenced this issue Nov 7, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants