On Tue, Jul 16, 2013 at 4:42 PM, Chris Geer <[email protected]> wrote:
> On Tue, Jul 16, 2013 at 12:55 PM, Erin Noe-Payne > <[email protected]>wrote: > > > In terms of implementation, I will definitely be interested to see how > > the fields selector works. > > > > With respect to response format, all responses will be wrapped in the > > same way. Will it make sense to create a custom "ApiResponse" class or > > something of the sort that represents the wrapper. Alternatively could > > cxf's outgoing interceptors be used to reformat the data response? > > Same thing for errors. > > > > I don't think we can do this with an interceptor because we would still > need to communicate to the interceptor the data (total records....) so we > might as well just wrap the responses. > > I was going to model the fields implementation after what we do for Shindig > since they already have the fileds concept. > Ideally, we would have a better way than what we did in Shindig. I haven't thought through it much, but it would be nice if we could intercept the intermediate JSON object and remove fields like you can with JSONObject form json.org. > > > > > > > On Tue, Jul 16, 2013 at 2:22 PM, Erin Noe-Payne > > <[email protected]> wrote: > > > On Tue, Jul 16, 2013 at 1:53 PM, Chris Geer <[email protected]> > > wrote: > > >> On Tue, Jul 16, 2013 at 10:32 AM, Erin Noe-Payne > > >> <[email protected]>wrote: > > >> > > >>> Any further discussion here? I would like to start implementing more > > >>> of the REST APIs, as it is foundational for the entire angular > > >>> architecture. > > >>> > > >>> My understanding from Matt is that the current apis in trunk are > > >>> mostly proof of concept - they are not tested and much of the > > >>> functionality is just stubbed. Are any of the rest api > implementations > > >>> in the code base a good working example? Is there other documentation > > >>> we can reference? > > >>> > > >> > > >> I've been working on the People resource as a "reference" of how I'd > > like > > >> to see them done but it's still a work in progress. I need to go back > > and > > >> pull out the JSONView stuff and reimplement the "fields" concept. > > Couple of > > >> notes: > > >> > > >> - Object representations should be as flat as possible > > >> and separate requests should be made to nested resources to get nested > > >> details (i.e. if you have regions and regions/1/regionwidgets, the > > regions > > >> representation should not contain an array of regionwidgets) > > >> - All methods should return standard HTTP codes. We should document > > this > > >> further on the wiki to make sure we all do the same way. > > >> - We won't accept partial updates with PUT, we will eventually add > > PATCH > > >> to support that in the future > > >> - If the "fields" query attribute isn't included in a GET then all > > fields > > >> are returned. > > > > > > +1 to all of the above > > > > > >> - What is the full meta structure we want to return? > > > > > > So this can be fluid. In the case of lists (search or list endpoints) > > > it should definitely contain pagination and count data. In the case of > > > individual resources it does not necessarily need to contain anything. > > > As long as we have the wrapped object then we have the key reserved to > > > attach metadata as the need arises. One example of what we could use > > > it for is to contain link information. I.E. a request to /page/1 > > > returns a flat resource, without the regions. Then meta could look > > > like: > > > > > > { > > > "meta": { > > > "regions": "http://localhost:8080/portal/api/page/1/regions";, > > > ... > > > } > > > } > > > > > >> > > >> > > >>> > > >>> On Thu, Jul 11, 2013 at 5:48 PM, Erin Noe-Payne > > >>> <[email protected]> wrote: > > >>> > On Thu, Jul 11, 2013 at 5:02 PM, Matt Franklin < > > [email protected]> > > >>> wrote: > > >>> >> +1 for every one of Chris' +1s, unless otherwise noted. > > >>> >> > > >>> >> On Thu, Jul 11, 2013 at 3:47 PM, Chris Geer < > [email protected]> > > >>> wrote: > > >>> >> > > >>> >>> Oh boy!! :) > > >>> >>> > > >>> >>> Comments inline > > >>> >>> > > >>> >>> > > >>> >>> On Thu, Jul 11, 2013 at 1:20 PM, Erin Noe-Payne < > > >>> [email protected] > > >>> >>> >wrote: > > >>> >>> > > >>> >>> > Hey All, > > >>> >>> > > > >>> >>> > As we are starting to look at the rest apis in more detail, I > > would > > >>> >>> > like to discuss and agree upon a consistent interface for our > > apis. > > >>> >>> > We currently have several developers interested in contributing > > to > > >>> the > > >>> >>> > apis and the angular branch, and I would like to solidify the > > >>> >>> > interface, methods, response format, etc so that we can be on > the > > >>> same > > >>> >>> > page going forward. If we can agree on an api virtualization > > layer > > >>> >>> > then we should be able to build against it on the server and on > > the > > >>> >>> > angular application in parallel. > > >>> >>> > > > >>> >>> > I'll start with a proposal and look for feedback to iterate > from > > >>> there. > > >>> >>> > > > >>> >>> > 1. API root url > > >>> >>> > > > >>> >>> > "/api". Drop support for rpc api, move from /api/rest to just > > /api. > > >>> >>> > > > >>> >>> > > >>> >>> +1 - the only downside of this is that it prohibits implementing > > over > > >>> time > > >>> >>> and requires a rip/replace approach of the whole system > > >>> > > > >>> > Well the development in trunk can continue to happen on /rest. > > Angular > > >>> > (aka the consuming client for most of these apis) is already > > happening > > >>> > in a branch, so those changes can be treated as a rip / replace > > >>> > easily. > > >>> > > > >>> >>> > > >>> >>> > > > >>> >>> > 2. Media Types > > >>> >>> > > > >>> >>> > Initially support only application/json. We can revisit > > >>> >>> > application/xml as a nice-to-have. > > >>> >>> > > > >>> >>> > > >>> >>> +1 > > >>> >>> > > >>> >>> > > > >>> >>> > 3. HTTP Methods > > >>> >>> > > > >>> >>> > GET, PUT, POST, DELETE > > >>> >>> > > > >>> >>> > > >>> >>> +1 (We also need to decide if PUT can handle partial updates) > > >>> >>> > > >>> >> > > >>> >> I say not. That is what PATCH is for, once everything supports > it: > > >>> >> http://tools.ietf.org/html/rfc5789 > > >>> > > > >>> > My understanding is that PUT should always be a full object > replace. > > A > > >>> > quick search returns the suggestion to use PATCH, or to use POST > to a > > >>> > subresource with a 303 response. > > >>> > > > >>> >> > > >>> >>> > > >>> >>> > > > >>> >>> > 4. Status Codes > > >>> >>> > > > >>> >>> > 200, 201, 400, 401, 403, 404, 500 > > >>> >>> > > > >>> >>> > > >>> >>> +1 > > >>> >>> > > >>> >>> > > > >>> >>> > 5. URL formats > > >>> >>> > > > >>> >>> > Use plural nouns (pages, people, widgets). Do not nest > > associations > > >>> >>> > beyond one level deep. For example: > > >>> >>> > /pages/1/regions (ok) > > >>> >>> > /pages/1/regions/2/regionwidgets (not ok) > > >>> >>> > /regions/2/regionwidgets (ok) > > >>> >>> > > > >>> >>> > > >>> >>> I'm not a fan of this requirement. Your example is the exact > reason > > >>> I'm not > > >>> >>> a fan actually. In all reality, regions don't mean anything > > outside a > > >>> page, > > >>> >>> and region widgets don't mean anything outside of a region. Yes, > > they > > >>> have > > >>> >>> IDs, but in reality, those IDs should be subordinate to the > parent > > (so > > >>> >>> there should be nothing wrong with having Page 1 with regions > > [1,2] and > > >>> >>> Page 2 with regions [1,2]). I understand that's not how the DB > > works > > >>> today > > >>> >>> but it's what makes the most logical sense. > > >>> >>> > > >>> >> > > >>> >> I agree with Chris. We should not limit to a single level. That is > > >>> counter > > >>> >> to a few REST web service principles. > > >>> >> > > >>> > > > >>> > Fair enough. In this case I guess I would just be looking for > > >>> > consistency - will associations be infinitely nest-able. If not, > what > > >>> > is the rule to determine where we support more or less deeply > nested > > >>> > associations. > > >>> > > > >>> >> > > >>> >>> > > > >>> >>> > 6. Response formats > > >>> >>> > > > >>> >>> > 6a. Wrap all responses in an object. All valid (200) responses > > should > > >>> >>> > be wrapped in an object that includes a "meta" object for > > metadata, > > >>> >>> > and a "data" object for the response body. This allows us to > > capture > > >>> >>> > or extend metadata associated with a response as needed. Any > > metadata > > >>> >>> > properties should be standardized. > > >>> >>> > > > >>> >>> > Example: > > >>> >>> > > > >>> >>> > GET /people > > >>> >>> > { > > >>> >>> > meta: {count: 253, limit: 10, offset: 0, ...} > > >>> >>> > data: [ {id: 1, name: 'canonical', ...}, ... ] > > >>> >>> > } > > >>> >>> > > > >>> >>> > GET /people/1 > > >>> >>> > { > > >>> >>> > meta: { ... } > > >>> >>> > data: {id:1, name: 'canonical', ...} > > >>> >>> > } > > >>> >>> > > > >>> >>> > > >>> >>> This really complicates a couple things, first, it means the GET > > != PUT > > >>> >>> since the GET will include the meta data. Can we achieve this > same > > >>> result > > >>> >>> with HTTP Headers? > > >>> >>> > > >>> > > > >>> > We could possibly achieve the same with HTTP headers. I prefer the > > >>> > object approach for clarity, since custom http headers are less > > >>> > accessible or discoverable than object structure. I get your point, > > >>> > but I see the wrapped object approach used commonly in major apis. > If > > >>> > it's clearly documented and used consistently across the entire > api I > > >>> > don't really see an issue. > > >>> > > > >>> >>> > > > >>> >>> > 6b. Error objects. In the case of an error, the correct error > > code > > >>> >>> > should be returned. In addition, an error object should be > > returned > > >>> >>> > with a standardized format. Ideally including a verbose, > > >>> >>> > human-readable error message for developers, and an > > internationalized > > >>> >>> > readable error message for display to end users. > > >>> >>> > > > >>> >>> > GET /people/25 > > >>> >>> > 401 > > >>> >>> > { > > >>> >>> > developerMessage: 'Unauthorized. Access to this resource > > requires > > >>> >>> > authentication', > > >>> >>> > userMessage: 'Please login', > > >>> >>> > stackTrace: ... > > >>> >>> > } > > >>> >>> > > > >>> >>> > > >>> >>> +1 > > >>> >>> > > >>> >>> > > > >>> >>> > 6c. Partial responses. By default all responses, whether a list > > or > > >>> >>> > individual resource, should return a full representation of the > > >>> >>> > resources (not including security constraints). All endpoints > > should > > >>> >>> > support the query string parameter "fields", which accepts a > > comma > > >>> >>> > delimited list of fields to build a partial response. > > >>> >>> > > > >>> >>> > > >>> >>> Hmmm.....what's funny (except for the wasted work) is this is > how I > > >>> >>> originally built the people resource. I changed it because the > > >>> "fields" > > >>> >>> approach gets almost impossible to manage with nested elements > (at > > >>> least in > > >>> >>> Java - rewrite in Ruby anyone??). I'm open to suggestions > though. I > > >>> guess > > >>> >>> we could also make a rule that the data objects shouldn't have > > nested > > >>> >>> elements but that is a tough rule. > > >>> >>> > > >>> >> > > >>> >> I think the fields approach makes sense long-term; but, it is not > > >>> critical. > > >>> >> > > >>> >> > > >>> > > > >>> > I don't really know what the implementation looks like. If you > allow > > >>> > field filtering only on properties and deliver only properties > (i.e. > > >>> > no nested objects / associations) then I would assume it is pretty > > >>> > straightforward. > > >>> >>> > > >>> >>> > > > >>> >>> > GET /people/1 > > >>> >>> > { > > >>> >>> > meta: { ... }, > > >>> >>> > data: { id: 1, name: 'canonical', email: '[email protected] > ', > > >>> ... } > > >>> >>> > } > > >>> >>> > > > >>> >>> > GET /people/1?fields=id,name > > >>> >>> > { > > >>> >>> > meta: { ... }, > > >>> >>> > data: { id: 1, name: 'canonical' } > > >>> >>> > } > > >>> >>> > > > >>> >>> > 6d. Pagination. All requests that return a list should be > > paginated. > > >>> >>> > The query string parameters "limit" and "offset" should be used > > for > > >>> >>> > pagination. On any request in which either parameter is not > set, > > they > > >>> >>> > should default to 10 and 0 respectively. > > >>> >>> > > > >>> >>> > > >>> >>> +1 > > >>> >>> > > >>> >>> > > > >>> >>> > 6e. Use camelCase for properties. > > >>> >>> > > > >>> >>> > > >>> >>> +1 > > >>> >>> > > >>> >>> > > > >>> >>> > 7. Endpoints. > > >>> >>> > > > >>> >>> > 7a. Standard endpoints: there should be standard CRUD endpoints > > to > > >>> >>> > support each rave resource. In other words, any operation > > possible in > > >>> >>> > rave should be possible through a rest api action. > > >>> >>> > > > >>> >>> > > >>> >>> +1 > > >>> >>> > > >>> >>> > > > >>> >>> > 7b. Special endpoints. In the case of certain client needs, we > > can > > >>> >>> > implement a small number of special endpoints to fulfill a > > specific > > >>> >>> > role. The primary case in point is retrieving a page for > render, > > >>> which > > >>> >>> > returns a page, its regions, its regionWidgets, and their > render > > >>> data. > > >>> >>> > > > >>> >>> > > >>> >>> +1 > > >>> >>> > > >>> >>> > > > >>> >>> > > > >>> >>> > > > >>> >>> > Ok, I think that's it. This is meant as a proposal only - we > are > > >>> >>> > looking for feedback to go forward. Thoughts? > > >>> >>> > > > >>> >>> > > >>> > > >
