Re: Gallery 3.1 - REST API revisions
Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> Thu, 6 Jun 2013 13:09:09 +0200
| Newsgroups | gmane.comp.web.gallery.devel |
|---|---|
| Message-ID | <CA+z51A4r3FE5-t6i4wGWFvR4+ng4KBHtARoqYb-5h1Ab6C4+xA@mail.gmail.com> |
--===============0467339146456742583== Content-Type: multipart/alternative; boundary=20cf303b42a5206a3204de7a5909 --20cf303b42a5206a3204de7a5909 Content-Type: text/plain; charset=UTF-8 Hey everyone, As I mentioned below, there are some asymmetries in the current map that are bugging me, and now I think I've gathered my thoughts enough to address them :-) Executive summary: unless someone starts protesting soon, I'm going to start implementing some of the changes below. The changes will be absolutely 100% backwards compatible, so no functionality with existing external REST apps will be broken. They'll just be given a bit more power to be better :-). First, quick overview of how I personally interpret the current REST resources: Models: - "item" - item model - "tag" - tag model - "comment" - comment model Collections of models: - "items" - collection of item models - "tags" - collection of tag models - "comments" - collection of comment models Collections of related has-many models: - "item_comments" - collection of comment models that the item has - "item_tags" - collection of tag models that the item has Relationship between models (but not a model itself) - "tag_item" - tag/item relationship - "comment_item" - not needed since comment->item is belongs-to, not has-many. Collections of relationships between models (but not a model itself) - "tag_items" - collection of tag/item relationships - "comment_items" - not needed since comment->item is belongs-to, not has-many. Derivatives of item model: - "data" - get item data - "tree" - get items using different query parameters This makes sense to me, and I think it's a nice set to have. I don't think we need to add any *resources* to make my picture prettier, but rather just some *methods*... MISSING DELETES ------------------------------- It seems like there are a couple places where the DELETE method is missing. We have: - item/<item_id> - delete item - comment/<comment_id> - delete comment - item_tags/<item_id> - remove all tags from item - tag_item/<tag_id>,<item_id> - remove one tag from item - tag_items/<tag_id> - remove all items from one tag (i.e. delete tag) - admin only These make sense, but it seems that we're missing: - tag/<tag_id> - delete tag (i.e. remove all items from one tag) - admin only - item_comments/<comment_id> - remove all comments from item - admin only I realize that delete tag is a copy of delete tag_items, but I don't see why that's a reason it shouldn't be allowed... IMHO, it makes logical sense that if we can delete an item or comment, we can delete a tag, too :-) MISSING POST ------------------------- We can currently POST: - a "comment" entity to "comments" - a "tag" entity to "tags" - a "tag_item" entity to "tag_items" But we can't POST: - an "item" entity to "items" Of course, we *can* POST an "item" entity to "item" directly by considering the target item its parent (a special case, which follows convention http://en.wikipedia.org/wiki/REST#RESTful_web_APIs), so this new one is kinda/sorta duplicated. But, just like above, I think it's a logical way to do it, so it should be allowed. Of course, it'd have to have "parent" defined, but this is the same as a comment needing "item" defined, and both are reflections of how their entities are reported. ADDING TAG BROKEN ------------------------------------ Here's the current process for adding a not-yet-existent tag: - POST a "tag" resource, which creates a tag with count 0. - cross your fingers that nobody else touches your site and fires an event that runs Tag::compact(), which deletes empty tags. - POST a "tag_item" resource, which adds a item/tag relationship and makes the count non-zero. If I had to pick one specific case study that illustrated why the current asymmetries cause end-user problems, this would be it. In addition to being unreliable, this is also not a reflection of how the standard UI works, where tags are added to items using their names, not their ids. I spent a lot of time scribbling out various resource maps, and finally concluded that this could be neatly addressed with two, relatively minor changes: MISSING TAG ENTITY INFO -------------------------------------------- Currently, we have entities that include elements that are other REST resources. - item - "item" resource as parent and album_cover; "data" resource as thumb, resize, and full - comment - "item" resource as item - tag_item - "item" resource as item, "tag" resource as tag But, we don't have a tag's items as part if its entity. If it did, we'd have an entity with "name", "slug", "count", and "items" fields. Hey look, an entity with "items" and "name" fields - exactly what we're after to mimic how the standard UI works! Now we can POST a "tag" resource with "name" and "items", which solves our "adding tag broken" problem nicely. POST ITEM_TAGS MISLEADING -------------------------------------------------- IMHO, this method should be deprecated, and the already-existing (and more logical) POST to "tag_items" should be used instead. Following the pattern described in "missing post" above, we add an entity of type "foobar" to the rest resource "foobars". However, this breaks the pattern: we add a "tag_item" to "item_tags". Not only is this literally adding "barfoo" to "foobars", but it doesn't even match the members that "item_tags" spits out (which are of type tag, just like how "item_comments" spits out members of type comment). So... thoughts? Take care, Shad On 5 June 2013 18:46, Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> wrote: > Hey everyone, > > So, I'm starting to take the deep dive into refactoring the REST API for > Gallery 3.1, and I have some thoughts I wanted to toss out there for > feedback... > > Super top-level summary: > - Rest resources will change from being helpers (e.g. > "helpers/foo_rest.php) to controllers (e.g. > "classes/Controller/Rest/Foo.php"). This change is because the main job of > a rest resource is to take in a request and generate a response, which is > verbatim the definition of controller that K3 uses. All rest resources > extend a common Controller_Rest class, which will handle all of the common > controller functions (like how to translate between HTTP and REST requests > and responses). > - Externally, anything that worked in the REST API of Gallery 3.0.x will > still work in Gallery 3.1. But... > - I'd like to make a few changes (see below for some more details). This > would likely change our REST API version number from 3.0 to 3.1, and I can > add a "X-Gallery-Api-Deprecated" header to things that are deprecated along > with a human-readable description. > > Alright, now for a few possible changes... > > ERROR REPORTING > --------------------------------- > I'd like to be a bit more careful with error reporting. Among the minor > wiggles I've noticed so far: > - we previously never fired a 405 Method Not Allowed, even for methods > other than the 4 RESTful ones. > - we previously never set the "Allow" header, either. Should it be the > fixed list of four, or should it be tailored for each resource? > - most error messages are not translated (e.g. ORM validation errors, > "invalid"...), but a couple are (e.g. "Upload failed"). My personal > feeling is to be consistent with no translation, which lets REST clients > translate fixed messages however they wish. > > MORE LENIENT HEADER NAMES > ------------------------------------------------------ > While the API clearly defines "X-Gallery-Request-Method" as the thing to > use, there are several other semi-standards (i.e. commonly-used > non-standards) out there. Should we be flexible enough to search for them, > too? > > DOCUMENTATION > ----------------------------- > It might be handy to include in the GET response of each resource a list > of possible query parameters along with a human-readable (and > non-translated) description as a way of self-documentation... > > AVAILABLE METHODS/RESOURCES > ----------------------------------------- > I've been trying to make a nice, consistent map of our current interface, > and keep seeing odd asymmetries that are bugging me. My current thought is > that there are some methods that should be deprecated entirely and others > that should be added/refined. My map of this is still a work in progress - > I'll ping y'all once I can present an idea that I don't want to change > myself every time I look at it... > > ORM > ---------- > While the resources seem like a pretty natural reflection of ORM models, > this doesn't really appear to be exploited very much. As a result, it > seems like a lot of code is duplicated to do more-or-less the same thing. > I'm not sure exactly how yet, but I'd like to make the base > Controller_Rest class exploit this fact to simplify things. > > SUB-REQUESTS > ---------------------------- > In several places, it looks like the code is redefining the > url/entity/members output for a resource in multiple places. This > duplication can be eliminated using sub-requests - to get the > url/entity/members for a rest resource, just do a sub-request to its rest > URL... > > OUTPUT METHODS > -------------------------------- > Currently, all methods get JSON-only output with the exception of GET, > which can choose HTML or JSONP as well. As Wayne pointed out (see > http://galleryproject.org/node/106581), this has some shortcomings. > > While JSONP is nice for embedding, it's also not very secure (see > http://en.wikipedia.org/wiki/JSONP). That's why it's typically only > employed for GET responses, although even that can lend itself to hijacking > the data elsewhere. > > It seems that CORS would be a better approach (see > http://en.wikipedia.org/wiki/Cross-origin_resource_sharing). It tends to > be more secure, can be used on all types of responses (including POST, > which was one of Wayne's causes of frustration in the link above), etc. My > vote is that we should add CORS, then add a REST admin screen with the > option for disabling JSONP entirely. > > CLEAN URLS > ------------------------- > Another related non-REST issue is that Apache's mod_rewrite doesn't do > well with non-GET methods. This is what led to another of Wayne's > head-bashing-against-wall moments in his implementation. K3, however, is > savvy enough to understand this exact Apache limitation and ensure that it > works a bit more carefully to avoid it. So, when redoing the clean URLs > for 3.1, we should use Kohana, not Apache, to fire the 301 redirect. That > is, use the [PT] method to route the clean URLs where they need to go, but > do *not* use the [R] method to route the dirty URLs to clean ones - > instead, use Kohana. > > Alright, that's the end of my mind dump right now. Thoughts? > > Take care, > Shad > --20cf303b42a5206a3204de7a5909 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Hey everyone,<div><br></div><div>As I mentioned below, there are some asymm= etries in the current map that are bugging me, and now I think I've gat= hered my thoughts enough to address them :-)</div><div><br></div><div>Execu= tive summary: unless someone starts protesting soon, I'm going to start= implementing some of the changes below. =C2=A0The changes will be absolute= ly 100% backwards compatible, so no functionality with existing external RE= ST apps will be broken. =C2=A0They'll just be given a bit more power to= be better :-).</div> <div><br></div><div>First, quick overview of how I personally interpret the= current REST resources:</div><div><br></div><div>Models:</div><div>- "= ;item" - item model</div><div>- "tag" - tag model</div> <div> - "comment" - comment model</div><div>Collections of models:</div= ><div>- "items" - collection of item models</div><div>- "tag= s" - collection of tag models</div><div>- "comments" - colle= ction of comment models</div> <div>Collections of related has-many models:</div><div>- "item_comment= s" - collection of comment models that the item has</div><div>- "= item_tags" - collection of tag models that the item has</div><div> Relationship between models (but not a model itself)</div><div>- "tag_= item" - tag/item relationship</div><div>- "comment_item" - n= ot needed since comment->item is belongs-to, not has-many.</div><div> Collections of relationships between models (but not a model itself)</div><= div>- "tag_items" - collection of tag/item relationships</div><di= v><div>- "comment_items" - not needed since comment->item is b= elongs-to, not has-many.</div> </div><div>Derivatives of item model:</div><div>- "data" - get it= em data</div><div>- "tree" - get items using different query para= meters</div><div><br></div><div>This makes sense to me, and I think it'= s a nice set to have. =C2=A0I don't think we need to add any *resources= * to make my picture prettier, but rather just some *methods*...</div> <div><br></div><div>MISSING DELETES</div><div>-----------------------------= --</div><div>It seems like there are a couple places where the DELETE metho= d is missing. =C2=A0We have:</div><div>- item/<item_id> - delete item= </div> <div>- comment/<comment_id> - delete comment</div><div>- item_tags/&l= t;item_id> - remove all tags from item</div><div>- tag_item/<tag_id&g= t;,<item_id> - remove one tag from item</div><div>- tag_items/<tag= _id> - remove all items from one tag (i.e. delete tag) - admin only</div= > <div><br></div><div>These make sense, but it seems that we're missing:<= /div><div>- tag/<tag_id> - delete tag (i.e. remove all items from one= tag) - admin only</div><div>- item_comments/<comment_id> - remove al= l comments from item - admin only</div> <div><br></div><div>I realize that delete tag is a copy of delete tag_items= , but I don't see why that's a reason it shouldn't be allowed..= . IMHO, it makes logical sense that if we can delete an item or comment, we= can delete a tag, too :-)</div> <div><br></div><div>MISSING POST</div><div>-------------------------</div><= div>We can currently POST:</div><div>- a "comment" entity to &quo= t;comments"</div><div>- a "tag" entity to "tags"</= div> <div>- a "tag_item" entity to "tag_items"</div><div><br= ></div><div>But we can't POST:</div><div>- an "item" entity t= o "items"</div><div><br></div><div>Of course, we *can* POST an &q= uot;item" entity to "item" directly by considering the targe= t item its parent (a special case, which follows convention <a href=3D"http= ://en.wikipedia.org/wiki/REST#RESTful_web_APIs">http://en.wikipedia.org/wik= i/REST#RESTful_web_APIs</a>), so this new one is kinda/sorta duplicated. = =C2=A0But, just like above, I think it's a logical way to do it, so it = should be allowed. =C2=A0Of course, it'd have to have "parent"= ; defined, but this is the same as a comment needing "item" defin= ed, and both are reflections of how their entities are reported.</div> <div><br></div><div>ADDING TAG BROKEN</div><div>---------------------------= ---------</div><div>Here's the current process for adding a not-yet-exi= stent tag:</div><div>- POST a "tag" resource, which creates a tag= with count 0.</div> <div>- cross your fingers that nobody else touches your site and fires an e= vent that runs Tag::compact(), which deletes empty tags.</div><div>- POST a= "tag_item" resource, which adds a item/tag relationship and make= s the count non-zero.</div> <div><br></div><div>If I had to pick one specific case study that illustrat= ed why the current asymmetries cause end-user problems, this would be it. = =C2=A0In addition to being unreliable, this is also not a reflection of how= the standard UI works, where tags are added to items using their names, no= t their ids.</div> <div><br></div><div>I spent a lot of time scribbling out various resource m= aps, and finally concluded that this could be neatly addressed with two, re= latively minor changes:</div><div><br></div><div>MISSING TAG ENTITY INFO</d= iv> <div>--------------------------------------------</div><div>Currently, we h= ave entities that include elements that are other REST resources.</div><div= >- item - "item" resource as parent and album_cover; "data&q= uot; resource as thumb, resize, and full</div> <div>- comment - "item" resource as item</div><div>- tag_item - &= quot;item" resource as item, "tag" resource as tag</div><div= ><br></div><div>But, we don't have a tag's items as part if its ent= ity. =C2=A0If it did, we'd have an entity with "name", "= slug", "count", and "items" fields. =C2=A0Hey look= , an entity with "items" and "name" fields - exactly wh= at we're after to mimic how the standard UI works! =C2=A0Now we can POS= T a "tag" resource with "name" and "items", w= hich solves our "adding tag broken" problem nicely.</div> <div><br></div><div>POST ITEM_TAGS MISLEADING</div><div>-------------------= -------------------------------</div><div>IMHO, this method should be depre= cated, and the already-existing (and more logical) POST to "tag_items&= quot; should be used instead.</div> <div><br></div><div>Following the pattern described in "missing post&q= uot; above, we add an entity of type "foobar" to the rest resourc= e "foobars". =C2=A0However, this breaks the pattern: we add a &qu= ot;tag_item" to "item_tags". =C2=A0Not only is this literall= y adding "barfoo" to "foobars", but it doesn't even= match the members that "item_tags" spits out (which are of type = tag, just like how "item_comments" spits out members of type comm= ent).</div> <div><br></div><div>So... thoughts?</div><div><br></div><div>Take care,</di= v><div>Shad</div><div><br></div><div><br></div><div><br></div><div><br></di= v><div><div class=3D"gmail_quote">On 5 June 2013 18:46, Shad Laws <span dir= =3D"ltr"><<a href=3D"mailto:shad-xpYdmXCiSuZWk0Htik3J/[email protected]" target=3D"_blank">shad@sh= adlaws.com</a>></span> wrote:<br> <blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1p= x #ccc solid;padding-left:1ex"><div dir=3D"ltr">Hey everyone,<div><br></div= ><div>So, I'm starting to take the deep dive into refactoring the REST = API for Gallery 3.1, and I have some thoughts I wanted to toss out there fo= r feedback...</div> <div><br></div> <div>Super top-level summary:</div><div>- Rest resources will change from b= eing helpers (e.g. "helpers/foo_rest.php) to controllers (e.g. "c= lasses/Controller/Rest/Foo.php"). =C2=A0This change is because the mai= n job of a rest resource is to take in a request and generate a response, w= hich is verbatim the definition of controller that K3 uses. =C2=A0All rest = resources extend a common Controller_Rest class, which will handle all of t= he common controller functions (like how to translate between HTTP and REST= requests and responses).</div> <div>- Externally, anything that worked in the REST API of Gallery 3.0.x wi= ll still work in Gallery 3.1. =C2=A0But...<br></div><div>- I'd like to = make a few changes (see below for some more details). =C2=A0This would like= ly change our REST API version number from 3.0 to 3.1, and I can add a &quo= t;X-Gallery-Api-Deprecated" header to things that are deprecated along= with a human-readable description.</div> <div><br></div><div>Alright, now for a few possible changes...</div><div><b= r></div><div>ERROR REPORTING</div><div>---------------------------------</d= iv><div>I'd like to be a bit more careful with error reporting. =C2=A0A= mong the minor wiggles I've noticed so far:</div> <div>- we previously never fired a 405 Method Not Allowed, even for methods= other than the 4 RESTful ones.</div><div>- we previously never set the &qu= ot;Allow" header, either. =C2=A0Should it be the fixed list of four, o= r should it be tailored for each resource?</div> <div>- most error messages are not translated (e.g. ORM validation errors, = "invalid"...), but a couple are (e.g. "Upload failed").= =C2=A0My personal feeling is to be consistent with no translation, which l= ets REST clients translate fixed messages however they wish.</div> <div><br></div><div>MORE LENIENT HEADER NAMES</div><div>-------------------= -----------------------------------</div><div>While the API clearly defines= "X-Gallery-Request-Method" as the thing to use, there are severa= l other semi-standards (i.e. commonly-used non-standards) out there. =C2=A0= Should we be flexible enough to search for them, too?</div> <div><br></div><div>DOCUMENTATION</div><div>-----------------------------</= div><div>It might be handy to include in the GET response of each resource = a list of possible query parameters along with a human-readable (and non-tr= anslated) description as a way of self-documentation...</div> <div><br></div><div>AVAILABLE METHODS/RESOURCES</div><div>-----------------= ------------------------</div><div>I've been trying to make a nice, con= sistent map of our current interface, and keep seeing odd asymmetries that = are bugging me. =C2=A0My current thought is that there are some methods tha= t should be deprecated entirely and others that should be added/refined. = =C2=A0My map of this is still a work in progress - I'll ping y'all = once I can present an idea that I don't want to change myself every tim= e I look at it...</div> <div><br></div><div><div>ORM</div><div>----------</div><div>While the resou= rces seem like a pretty natural reflection of ORM models, this doesn't = really appear to be exploited very much. =C2=A0As a result, it seems like a= lot of code is duplicated to do more-or-less the same thing. =C2=A0I'm= not sure exactly how yet, but I'd like to make the base Controller_Res= t class exploit this fact to simplify things.</div> <div><br></div><div>SUB-REQUESTS</div><div>----------------------------</di= v><div>In several places, it looks like the code is redefining the url/enti= ty/members output for a resource in multiple places. =C2=A0This duplication= can be eliminated using sub-requests - to get the url/entity/members for a= rest resource, just do a sub-request to its rest URL...</div> <div><br></div><div>OUTPUT METHODS</div><div>------------------------------= --</div><div>Currently, all methods get JSON-only output with the exception= of GET, which can choose HTML or JSONP as well. =C2=A0As Wayne pointed out= (see=C2=A0<a href=3D"http://galleryproject.org/node/106581" target=3D"_bla= nk">http://galleryproject.org/node/106581</a>), this has some shortcomings.= </div> <div><br></div><div>While JSONP is nice for embedding, it's also not ve= ry secure (see=C2=A0<a href=3D"http://en.wikipedia.org/wiki/JSONP" target= =3D"_blank">http://en.wikipedia.org/wiki/JSONP</a>). =C2=A0That's why i= t's typically only employed for GET responses, although even that can l= end itself to hijacking the data elsewhere.</div> <div><br></div><div>It seems that CORS would be a better approach (see=C2= =A0<a href=3D"http://en.wikipedia.org/wiki/Cross-origin_resource_sharing" t= arget=3D"_blank">http://en.wikipedia.org/wiki/Cross-origin_resource_sharing= </a>). =C2=A0It tends to be more secure, can be used on all types of respon= ses (including POST, which was one of Wayne's causes of frustration in = the link above), etc. =C2=A0My vote is that we should add CORS, then add a = REST admin screen with the option for disabling JSONP entirely.</div> <div><br></div><div>CLEAN URLS</div><div>-------------------------</div><di= v>Another related non-REST issue is that Apache's mod_rewrite doesn'= ;t do well with non-GET methods. =C2=A0This is what led to another of Wayne= 's head-bashing-against-wall moments in his implementation. =C2=A0K3, h= owever, is savvy enough to understand this exact Apache limitation and ensu= re that it works a bit more carefully to avoid it. =C2=A0So, when redoing t= he clean URLs for 3.1, we should use Kohana, not Apache, to fire the 301 re= direct. =C2=A0That is, use the [PT] method to route the clean URLs where th= ey need to go, but do *not* use the [R] method to route the dirty URLs to c= lean ones - instead, use Kohana.</div> <div><br></div><div>Alright, that's the end of my mind dump right now. = =C2=A0Thoughts?</div><div><br></div><div>Take care,</div><div>Shad</div> </div></div> </blockquote></div><br></div> --20cf303b42a5206a3204de7a5909-- --===============0467339146456742583== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline ------------------------------------------------------------------------------ How ServiceNow helps IT people transform IT departments: 1. A cloud service to automate IT design, transition and operations 2. Dashboards that offer high-level views of enterprise services 3. A single system of record for all IT processes http://p.sf.net/sfu/servicenow-d2d-j --===============0467339146456742583== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline __[ g a l l e r y - d e v e l ]_________________________ [ list info/archive --> http://gallery.sf.net/lists.php ] [ gallery info/FAQ/download --> http://gallery.sf.net ] --===============0467339146456742583==--