RESTful dispatch

[dbrattli]

Hi,

I've made a RESTful dispatcher for CP3 based on http://microformats.org/wiki/rest/urls.
You can find it at:

http://code.google.com/p/dbrattli/source/browse/trunk/cherrypy/rest_dispatch.py

Please add comments or suggestions, so I can improve it enough to
submit a patch.

-- Dag

[Robert Brewer]

Well done! There are lots of good things in there I never would have
thought of, like the simple alternation of methods and params (much
easier than inventing a new specification language for which is which),
and calling find_handler twice! Brilliant.

Robert Brewer
fuma...@aminus.org

[Sylvain Hellegouarch]

I very much agree will probably be using it where I was using Routes or
Selector previously. Well done.

I know we are careful about getting new code in the trunk but that would
be a nice candidate considering the amount of people asking for that kind
of behavior.

- Sylvain

--
Sylvain Hellegouarch
http://www.defuze.org

[dbrattli]

Hi, thanks for the feedback. I've added a patch at http://www.cherrypy.org/ticket/897,
so you can play with it. I've included some tests in test_rest.py.

There are a few issues I would like to discuss:

1) I'm thinking that it might be better to add the arguments to kwargs
instead or args, and using the resources part of the URL as key. Thus
a GET on the URL /people/4/phones/ will call the GET method within
Phones with kwargs = { 'people' : 4 }. That would make it possible to
mount the Phones class within several controllers if needed. If would
also fix the second last test /people/phones/5/ringtones which
currently gets wrong since ringtones does not really know who the
argument 5 is really for.

2) It currently does not handle URLs such as /people/3/phones/
ringtones which is a short notation for /people/3/phones/all/ringtones/
all. I'm really not sure if we need to support such URLs without ids
in between? It currently supports such URLs until you start to use id
args, then you have to alternate after that (but has then problems
knowing who the params is for, but can be solved by 1). If it's
important we need to put a loop inside, and nest us down the objects
while inspecting if vpath is a param or path (or use something
recursive). It could get expensive since it would be calling
find_handler after every param. But I haven't really seen anybody
using such short URLs, so I'm not sure if it's necessary?

-- Dag

[Lakin Wecker]

I too want to commend you for such a simple implementation.  I had recently made some quite different changes to the dispatchers in trunk that had very similar goals.

See below for a bit more.

On 1/16/09, dbrattli <dbra...@gmail.com> wrote:

Hi, thanks for the feedback. I've added a patch at http://www.cherrypy.org/ticket/897,
so you can play with it. I've included some tests in test_rest.py.

There are a few issues I would like to discuss:

1) I'm thinking that it might be better to add the arguments to kwargs
instead or args, and using the resources part of the URL as key. Thus
a GET on the URL /people/4/phones/ will call the GET method within
Phones with kwargs = { 'people' : 4 }. That would make it possible to
mount the Phones class within several controllers if needed. If would
also fix the second last test /people/phones/5/ringtones which
currently gets wrong since ringtones does not really know who the
argument 5 is really for.

I would argue that even if you paired the argument '5' with the key 'phones' that the ringtones controller should not (necessarily) be forced to be aware of where it was mounted and how to instantiate everything that comes before it in the tree.  Somehow that seems backwards to me. Rather, the phones portion of the object tree is the object that knows how to deal with phones, and it should be the object that is passed the argument '5'.

With the current situation, we can't make use of URLs like the ones above without doing our own custom dispatching in the 'GET' method of the root object, so the GET method of the people object would be called like:
people.GET('phones', '5', 'ringtones')

And this method would need to pass out the arguments manually.

Similarly, in your suggested world all of the arguments are stored up and passed to the child-most object in the tree, so the resulting call is something like:
ringtones.GET(people=None, phones=5) ?

Either way, the object that receives the GET call must be coupled to the entire path and understand explicitly how to handle all arguments that are gathered up along the way.  To me this feels like it encourages heavily coupled object design.  Unfortunately, the only suggestion that I can think of resembles the way my patch works -

people.phones._cp_dispatch('5').ringtones.GET()

- and I'm not intending this to be shameless promotion of that patch. ;)  Especially considering that yours is so simple and popular.  

Is this how most people end up using Routes? 

Lakin

[dbrattli]

Hi Lakin,

For me:

/people/phones/5/ringtones

is just another way of writing:

/people/phones/ringtones?phone_id=5

For me it's just about endpoints with params. Personally I don't think
CherryPy
should know that there exists any underlaying data model.

PS. Where can I see your patch?

-- Dag

On Jan 16, 3:18 pm, "Lakin Wecker" <lakin.wec...@gmail.com> wrote:
> I too want to commend you for such a simple implementation.  I had recently
> made some quite different changes to the dispatchers in trunk that had very
> similar goals.
>
> See below for a bit more.
>

[Lakin Wecker]

http://cherrypy.org/ticket/869

In my opinion (and I realize I may be alone in this opinion)  - if all I have are endpoints with parameters, then I'd prefer a routes or urls.py like interface.

I honestly believe that allows the objects which represent the handler tree to do more will allow for less code in many situations.   But to each his own.

Lakin

[dbrattli]

Robert,

I've added a new version of the patch that fixes all the problems I
had with the first version. I can now also handle
URLs such as:

/people/phones/5/ringtones
/people/3/phones/ringtones

PS: And the dispatcher is even simpler than the last version!

-- Dag

> fuman...@aminus.org

[dbrattli]

New patch at:

http://www.cherrypy.org/attachment/ticket/897/rest_dispatch-v2.diff

-- Dag

[Robert Brewer]

dbrattli wrote:
> I've added a new version of the patch that fixes all the problems I
> had with the first version. I can now also handle
> URLs such as:
>
> /people/phones/5/ringtones
> /people/3/phones/ringtones
>
> PS: And the dispatcher is even simpler than the last version!

Yeah; I subscribe to cherrypy-tickets. ;)

This is some great work. It'll take me a bit of time to digest the
implications of Yet Another Dispatcher in the distro. I'd like to play
around a bit and see if:

1) perhaps we can separate the dispatching-by-HTTP-method code from the
grabbing-params-code a bit more cleanly. It bugs me that the
MethodDispatcher.__call__ is buried so deep in there. I'd like us to
explore ways to better enable composable graphs of dispatchers instead
of tightly coupling them.
2) contrarily, perhaps your approach, and lakin's recent trunk checkin,
might conflict or need a shootout to be built into the default
dispatcher.

Robert Brewer
fuma...@aminus.org

[dbrattli]

Hi,

1) Could be solved by allowing MethodDispatcher to also call
explicitly exposed methods within the class, if path_info resolved
into such an exposed method. Thus just moving a few lines from my
dispatcher to the method dispatcher. That way I can always call
MethodDispatcher, and it would only be the param collector that is the
difference.

2) Have no strong feelings about this. As long at it's easy to switch
dispachers in config there's really no problem. But I agree that
CherryPy should probably not include two dispatchers with overlapping
functionality. I'm not sure they overlap that much, but I need more
time to understand how it's working. Would be nice with an example.

-- Dag

> fuman...@aminus.org

[Lakin Wecker]

This is the best example code I have so far.  If I get a chance I may rewrite some of your examples to use the other dispatcher:

http://cherrypy.org/browser/trunk/cherrypy/test/test_dynamicobjectmapping.py#L69

Lakin

[Lakin Wecker]

I think that the most important thing that can come out of this is to make it even easier to write dispatchers.  In other words, these things can take quite a bit of mental effort to write at the moment.  It's amazing that CherryPy provides the facility to customize the dispatcher to begin with, but if we can make it dead easy to write dispatchers, then it matters much less which one of the two approaches are chosen to reside in trunk as it will be easy for any application to write and maintain their own dispatcher to facilitate their custom needs.

Lakin

[dbrattli]

Lakin,

I've looked over your patch, and I think it's very nice. I had a hard
time trying to use your _cp_dispatch to implement my REST stuff. But I
finally got it working after making a few changes to MethodDispatcher.
I've added both the solutions to http://www.cherrypy.org/ticket/897 so
you can see the difference. http://www.cherrypy.org/attachment/ticket/897/rest_dispatch-v3.diff.
I've reimplemented test_rest.py with _cp_dispatch called
test_rest_cp_dispatch.py.

Thus I only need to make myself a simple RestController and subclass
it for People, Phones, Ringtones etc. The good thing with this
solution is that mangling with the parameters is moved from CherryPy
and into the application. This is good since the application better
knows how it wants to deal with them.

def _cp_dispatch(self, vpath):
if vpath[0] != 'index':
if hasattr(self, vpath[0]):
return getattr(self, vpath[0])
else:
request.params[self.id_name] = vpath[0]
return self
return None

As for the shootout I think I will throw in the towel at this point.
Your solution is both more flexible and much faster than my solution.
If I can get the few lines into MethodDispatcher I will be happy. If
not I will have to make a MethodWithVerbsDispatcher ;-)

One suggestion is that the _cp_dispatch method could be allowed to
modify the vpath, in order to eat multiple entries from vpath in one
go, or to reorder or insert items to get a custom *args list when the
method is called: resource, new_vpath = dispatch(vpath)

-- Dag

On Jan 18, 3:21 am, "Lakin Wecker" <lakin.wec...@gmail.com> wrote:
> I think that the most important thing that can come out of this is to make
> it even easier to write dispatchers.  In other words, these things can take
> quite a bit of mental effort to write at the moment.  It's amazing that
> CherryPy provides the facility to customize the dispatcher to begin with,
> but if we can make it dead easy to write dispatchers, then it matters much
> less which one of the two approaches are chosen to reside in trunk as it
> will be easy for any application to write and maintain their own dispatcher
> to facilitate their custom needs.
>
> Lakin
>

> On Sat, Jan 17, 2009 at 1:50 PM, Lakin Wecker <lakin.wec...@gmail.com>wrote:
>
> > This is the best example code I have so far.  If I get a chance I may
> > rewrite some of your examples to use the other dispatcher:
>

> >http://cherrypy.org/browser/trunk/cherrypy/test/test_dynamicobjectmap...
>
> > Lakin

[Lakin Wecker]

On Sun, Jan 18, 2009 at 6:07 AM, dbrattli <dbra...@gmail.com> wrote:

Lakin,

I've looked over your patch, and I think it's very nice. I had a hard
time trying to use your _cp_dispatch to implement my REST stuff.

I would love help writing some documentation and examples  to help people to figure this out.
 

But I
finally got it working after making a few changes to MethodDispatcher.

It's not clear which changes you needed - but I'm assuming it was the changes on line 400.

Wouldn't you want those to be their own MethodBased object instead?  In other words, rather than  exposing a method, why not expose an object that implements the GET/POST methods?  I made those changes to your patch and uploaded a new one: http://www.cherrypy.org/attachment/ticket/897/rest_dispatch-v3-lakin.diff

While it is a bit more verbose (you have to declare a new object and then instantiate it) - it doesn't require that change to the MethodDispatcher _and_  provides a richer way to respond because you can now respond to POST on those URLs as well as GET. 
 
What do you think?

I've added both the solutions to http://www.cherrypy.org/ticket/897 so
you can see the difference. http://www.cherrypy.org/attachment/ticket/897/rest_dispatch-v3.diff.
I've reimplemented test_rest.py with _cp_dispatch called
test_rest_cp_dispatch.py.

Thus I only need to make myself a simple RestController and subclass
it for People, Phones, Ringtones etc. The good thing with this
solution is that mangling with the parameters is moved from CherryPy
and into the application. This is good since the application better
knows how it wants to deal with them.

 def _cp_dispatch(self, vpath):
     if vpath[0] != 'index':
         if hasattr(self, vpath[0]):
             return getattr(self, vpath[0])
         else:
             request.params[self.id_name] = vpath[0]
             return self
     return None

I took out the if hasattr(self, vpath[0])  if branch in my patch as it is redundant.  The current dispatch mechanism will  do that for you.

As for the shootout I think I will throw in the towel at this point.
Your solution is both more flexible and much faster than my solution.
If I can get the few lines into MethodDispatcher I will be happy. If
not I will have to make a MethodWithVerbsDispatcher ;-)

Heh - if you are happy with the changes that I made to your patch then you don't even need to make a new Method Dispatcher.  you only need that base class RestController with a (relatively simple) implementation of _cp_dispatch.
 

One suggestion is that the _cp_dispatch method could be allowed to
modify the vpath, in order to eat multiple entries from vpath in one
go, or to reorder or insert items to get a custom *args list when the
method is called: resource, new_vpath = dispatch(vpath)

I think this is a great idea and would be happy to apply a patch that allows this.  If I had the time I would do it myself but I probably won't for a few days.
 

Lakin

[dbrattli]

Lakin,

Thanks for making my _cp_dispatch even simpler. Adding "edit" and
"new" as classes is possible, but I would prefer to avoid it since you
should not want to do anything other than GET (they should only return
forms). The code becomes more chatty and harder to read. I have a
simpler patch for MethodDispatcher that might be acceptable. It's a
one liner:

Index: cherrypy/_cpdispatch.py
===================================================================
--- cherrypy/_cpdispatch.py (revision 2106)
+++ cherrypy/_cpdispatch.py (working copy)
@@ -370,7 +370,7 @@

# Find the subhandler
meth = request.method.upper()
- func = getattr(resource, meth, None)
+ func = getattr(resource, meth, resource)
if func is None and meth == "HEAD":
func = getattr(resource, "GET", None)
if func:

It just uses the resource itself if no other method was found. This
should be safe since the resource must have been exposed to get there
in the first place. And if you expose something, you want it to be
callable. Right?

-- Dag

On Jan 18, 5:45 pm, "Lakin Wecker" <lakin.wec...@gmail.com> wrote:

> On Sun, Jan 18, 2009 at 6:07 AM, dbrattli <dbrat...@gmail.com> wrote:
>
> > Lakin,
>
> > I've looked over your patch, and I think it's very nice. I had a hard
> > time trying to use your _cp_dispatch to implement my REST stuff.
>
> I would love help writing some documentation and examples  to help people to
> figure this out.
>
> > But I
> > finally got it working after making a few changes to MethodDispatcher.
>
> It's not clear which changes you needed - but I'm assuming it was the
> changes on line 400.
>
> Wouldn't you want those to be their own MethodBased object instead?  In
> other words, rather than  exposing a method, why not expose an object that
> implements the GET/POST methods?  I made those changes to your patch and

> uploaded a new one:http://www.cherrypy.org/attachment/ticket/897/rest_dispatch-v3-lakin....

[Lakin Wecker]

Yeah, I can understand your point.  I am not directly opposed to your patch, but am just making other suggestions as I see them.

What about a different expose method that effectively shortcuts the code that I wrote:

@cherrypy.expose_GET()
def edit(self, phone_id=None)

It would require a decorator that wrapped the method as a class with a GET method.

Lakin

[dbrattli]

I work on the TurboGears 1.x branch and there we don't use
cherrypy.expose at all. We use turbogears.expose(), and it would be
confusing that some methods would need this cherrypy.expose_GET
decorator. I would prefer the patch.

-- dag

On Jan 18, 8:05 pm, "Lakin Wecker" <lakin.wec...@gmail.com> wrote:
> Yeah, I can understand your point.  I am not directly opposed to your patch,
> but am just making other suggestions as I see them.
>
> What about a different expose method that effectively shortcuts the code
>
> that I wrote:
>
> @cherrypy.expose_GET()
> def edit(self, phone_id=None)
>
> It would require a decorator that wrapped the method as a class with a GET
> method.
>
> Lakin
>

[Sylvain Hellegouarch]

I wouldn't be encline to follow that track. I'm not a big fan of adding
more to the CP namespace but between a _cp_dispatch and this, I actually
the _cp_dispatch.

- Sylvain

Lakin Wecker a écrit :

> <mailto:lakin.wec...@gmail.com>> wrote:
> > > > I think that the most important thing that can come out of
> this is to
> > > make
> > > > it even easier to write dispatchers. In other words, these
> things can
> > > take
> > > > quite a bit of mental effort to write at the moment. It's
> amazing that
> > > > CherryPy provides the facility to customize the dispatcher
> to begin with,
> > > > but if we can make it dead easy to write dispatchers, then
> it matters
> > > much
> > > > less which one of the two approaches are chosen to reside in
> trunk as it
> > > > will be easy for any application to write and maintain their own
> > > dispatcher
> > > > to facilitate their custom needs.
> >
> > > > Lakin
> >
> > > > On Sat, Jan 17, 2009 at 1:50 PM, Lakin Wecker

> <lakin.wec...@gmail.com <mailto:lakin.wec...@gmail.com>

> > > > >> > fuman...@aminus.org <mailto:fuman...@aminus.org>
>
>
>
> >

[Lakin Wecker]

On Sun, Jan 18, 2009 at 12:32 PM, Sylvain Hellegouarch <s...@defuze.org> wrote:

I wouldn't be encline to follow that track. I'm not a big fan of adding
more to the CP namespace but between a _cp_dispatch and this, I actually
the _cp_dispatch.

This - which this? the export_GET? And do you prefer, or not prefer _cp_dispatch

Just trying to understand what you intended. :)

Lakin

[Sylvain Hellegouarch]

Lakin Wecker a écrit :

> On Sun, Jan 18, 2009 at 12:32 PM, Sylvain Hellegouarch <s...@defuze.org
> <mailto:s...@defuze.org>> wrote:
>
>
> I wouldn't be encline to follow that track. I'm not a big fan of
> adding
> more to the CP namespace but between a _cp_dispatch and this, I
> actually
> the _cp_dispatch.
>
>
> This - which this? the export_GET? And do you prefer, or not prefer
> _cp_dispatch

Damn I accidently the email as they say. Well I prefer _cp_dispatch as
it offers more flexibility and if we add something to the CP namespace,
it might as well be flexible.

- Sylvain

[dbrattli]

Lakin. I went back to having a RestDispatcher again instead of
modifying MethodController. I didn't like using a decorator such as
@method_dispatcher_expose around edit/new since the method does not
belong to the class anymore and looses the "self" param. I think that
is not acceptable.

RestDispatcher is now a combination of Dispatcher and
MethodDispatcher. It should be used together with RestController, to
get proper REST handing, but you don't have to. Applications can use
it without it, to get a mixed dispatcher, or easily modify
RestController for dynamic object dispatching etc. Also made it
possible to set idname and format in _cp_config since that's where CP
stores its config. The test also shows how you can have custom id
names. The People class uses "person_id" etc.

-- Dag

On Jan 18, 8:05 pm, "Lakin Wecker" <lakin.wec...@gmail.com> wrote:
> Yeah, I can understand your point.  I am not directly opposed to your patch,
> but am just making other suggestions as I see them.
>
> What about a different expose method that effectively shortcuts the code
>
> that I wrote:
>
> @cherrypy.expose_GET()
> def edit(self, phone_id=None)
>
> It would require a decorator that wrapped the method as a class with a GET
> method.
>
> Lakin
>