[Catalyst] Re: Catalyst::Action::REST

[Matt S Trout]

Garrett Goebel wrote:
> 
> On Nov 20, 2006, at 9:02 AM, A. Pagaltzis wrote:
> 
>> * Matt S Trout <dbix-class at trout.me.uk> [2006-11-20 15:25]:
>>> Feel free to propose a variant on the syntax marcus proposed
>>> that  will allow the implementation of equivalent functionality
>>> without the  addition of multimethods to the perl core.
>>
>> Oh! Now I feel silly.
>>
>> Eh.
>>
>> OK, to lay out my own thinking a little, when I was doing my
>> silly PATHINFO-based hack-up of CGI::Prototype::Hidden, the plan
>> eventually became to have one class per handler, with methods
>> named after, err, methods, eg.:
>>
>>     sub GET  { ... }
>>     sub POST { ... }
>>     sub HEAD { ... }
>>
>> I still think that’s cleanest approach.
>>
> 
> That's pretty much what I wound up doing. Except I was more focused on a 
> RESTful rewrite of InstantCRUD to generate one controller class per 
> table. It is very clean and simple. All my per-table controllers 
> inherited from a single base controller which implemented actions for 
> create, read, update, delete, edit, show, index.

This is basically how Reaction handles CRUD

>> Catalyst as it stands somewhat encourages a confusion between
>> nouns (URIs) and verbs (methods), with URIs like
>> `/entry/1234/comments/add`, where the `/entry/1234/comments` part
>> identifies a resource, but the `/add` bit at the end is really an
>> verb. That should simply be a POST to `/entry/1234/comments`.
> 
> I ended up disagreeing with the RoR folks and Audrey's REST work in 
> Jifty. I think leaving the key unspecified is a bad idea. Sometimes a 
> table has more than one primary key candidate. Or you may want to limit 
> the result set based on field that isn't a primary key. What I 
> implemented back around the same time as John Napiorkowski was working 
> on the same thing was:
> 
> /entry/id/1234/comments
> 
> vs.
> 
> /entry/surname,given_name/Pagaltzis,Aristotle/comments
> 
> vs.
> 
> /entry/status/open

Again, pretty much exactly how Reaction handles it, except that I've shied 
away from dealing with multi-col PKs -just- yet :)

>> And
>> most of the time, if the design is RESTful from the start, you
>> can implement a web app as pure CRUD using the HTTP methods; eg.
>> the methods in a controller should simply correspond 1:1 to HTTP
>> verbs. That was as far as I had gotten my own thoughts.
>>
>> Of course, there are going to be some cases where that isn’t
>> quite enough; for these, you could have the option of adding
>> methods that work as if they were custom invented HTTP methods.
>> The framework could permit tunneling unsupported methods inside
>> POST requests, which it would automatically unravel before
>> dispatching. The Rails guys were doing something along these
>> lines; I have to take another look sometime.
> 
> I tunneled PUT and DELETE inside a POST via a "_method" body parameter, 
> which was automatically unravelled before dispatch within an overridden 
> Catalyst::prepare_body_parameters method.

Clever. Would you be willing to separate that out as a plugin with a 
configurable parameter name? I think that might be well-received (and actually 
something that *should* be a plugin for once :)

>> What I don’t like about the current proposals in Catalyst-land
>> is that they make the RESTful approach a wordy non-default
>> option. It takes quite a bit of extra annotation with any of
>> them, even if each annotation is concise. It ends easier to just
>> put verbs in your URIs, where it should be just as easy to do it
>> properly.
> 
> I'm not sure I follow you. RESTful uri's end up being shorter. I suppose 
> you're talking about the pain associated with implementing a RESTful 
> interface within a catalyst app.
> 
> The annoying problem that I ran up against was:
> 
> 1) Catalyst is designed with the assumption of dispatch based on first 
> match instead of best match. I.e. Catalyst::Dispatcher short circuits on 
> the first dispatch type attribute which matches. So side effects in the 
> dispatch type's ->match method aren't guaranteed if multiple dispatch 
> type parameters were specified.

It's not -exactly- first match, more an implicit assumption that the match of 
the most path parts is the best match - I effectively consider the current 
dispatcher to be half refactored from the 5.0 implementation, but haven't yet 
found a design for the second half I consider satisfactory.

> 2) All dispatch types are based on the request's path. If you want to 
> dispatch based on additional criterion, you're supposed to bury the 
> dispatch match logic in action classes.

You seem to consider dispatching on the path above all to be a bad thing; I 
don't quite understand this since the nature of HTTP/REST is that the URI 
identifies the entity and -then- the query parameters are applied, if present, 
representing a search on that entity, and only then is the method (and body if 
any) applied to the entity resulting.

> Fast is good and well, but I'd rather be correct than fast.

That's nice, but mapping path -> entity first *is* correct, at least for 
implementing REST :)

> I ended up subclassing the Catalyst::Dispatcher to make prepare_action 
> match all the dispatch types attributes specified for an action. This 
> allowed me to move my dispatch logic out of action classes and into 
> dispatch types where IMHO they belong.


I think what we really need to do is re-think the dispatch logic somewhat - 
entity, then search, then apply method+body is definitely correct, but 
matching the action first (and having the actions be singleton-per-app) is 
almost certainly wrong.

What I think we really want to be doing is something like Chained, but where 
the controllers rather than being a singleton-per-app are instantiated 
per-request, so

POST /foo/id/1/bar/id/3?baz=quux would do something like

my $foo_c = Controller::Foo->new(id => 1, parent => $app);

my $bar_c = Controller::Bar->new(id => 3, parent => $foo_c);

$bar_c->apply_search(baz => 'quux');

$bar_c->POST($body);

or similar. I don't think the above is -quite- right but I think it -is- 
closer than how things currently work.