"Next Action" - Improving Perl Catalyst Chained Actions

One of the features I enjoy the most about Perl's Catalyst MVC web framework is action chaining. In action chaining you can use the chain of command pattern to logically group and reuse actions that can span and share a URL. Here's an example for a controller that is a classic CRUD style setup for webpages to view a list of posts, edit, delete and view an individual post and create a post. For ease of explanation I'm leaving off a lot of stuff I'd normally do like validation on incoming POST parameters, etc.

First a root controller that everything chains off from:

package Example::Controller::Root;

use Moose;
use MooseX::MethodAttributes;
use feature 'signatures';

extends 'Catalyst::Controller;

# URL-PART: /...
sub root :Chained('/') PathPart('') CaptureArgs(0) ($self, $c) {
  $c->stash->{user} = $c->user;
}

__PACKAGE__->config(namespace=>'');
__PACKAGE__->meta->make_immutable;

Next, the posts controller that continues the chain and defines endpoints:

package Example::Controller::Posts;

use Moose;
use MooseX::MethodAttributes;
use feature 'signatures';

extends 'Catalyst::Controller;

# URL-PART: /posts/...
sub root :Chained('../root') PathPart('posts') CaptureArgs(0) ($self, $c) {
  $c->stash->{posts} = $c->stash->{user}->posts;
}

# GET /posts
sub list :GET Chained('root') PathPart('') Args(0) ($self, $c) {
}

# POST /posts
sub create :POST Chained('root') PathPart('') Args(0) ($self, $c) {
  $c->stash->{posts}->create($c->req->body_parameters);
}

# GET /posts/new
sub build :GET Chained('root') PathPart('new') Args(0) ($self, $c) {
  $c->stash->{new_post} = $c->stash->{posts}->build;
}

# URL-PART: /posts/{$id}/...
sub find :Chained('root') PathPart('') CaptureArgs(1) ($self, $c, $id) {
  $c->stash->{post} = $c->stash->{posts}->find($id);
}

# GET /posts/{$id}
sub show :GET Chained('find') PathPart('') ($self, $c) {
}

# DELETE /posts/{$id}
sub delete :DELETE Chained('find') PathPart('') ($self, $c) {
  $c->stash->{post}->delete;
}

# GET /posts/{$id}/edit
sub edit :GET Via('find') At('edit') ($self, $c) {
}

# PATCH /posts/{$id}
sub update :PATCH Via('find') PathPart('') ($self, $c) {
  $c->stash->{post}->update($c->req->body_parameters);
}

__PACKAGE__->meta->make_immutable;

In this flow we use the stash a lot just to pass information from one action to another. Its a lot of extra typing that makes the code look messy and in a more real life example its easy to lose track of exactly whats in the stash at a given moment. Additionally you have a lot of namespace pollution in the stash (stash keys that are used by intermediate actions stick around to the end) and of course you have the classic issue with the stash of introducing hard to spot bugs with typos in the stash call:

$c->stash->{Posts}...

Instead, what if there was a way to directly pass arguments from a chain action directly to its children (and only its children?). Lets see what that would look like:

package Example::Controller::Root;

use Moose;
use MooseX::MethodAttributes;
use feature 'signatures';

extends 'Catalyst::Controller;

# URL-PART: /...
sub root :Chained('/') PathPart('') CaptureArgs(0) ($self, $c) {
  $c->action->next(my $user = $c->user);
}

__PACKAGE__->config(namespace=>'');
__PACKAGE__->meta->make_immutable;`

Next, the posts controller that continues the chain and defines endpoints:

package Example::Controller::Posts;

use Moose;
use MooseX::MethodAttributes;
use feature 'signatures';

extends 'Catalyst::Controller;

# URL-PART: /posts/...
sub root :Chained('../root') PathPart('posts') CaptureArgs(0) ($self, $c, $user) {
  $c->action->next(my $posts = $user->posts);
}

# GET /posts
sub list :GET Chained('root') PathPart('') Args(0) ($self, $c, $posts) {
}

# POST /posts
sub create :POST Chained('root') PathPart('') Args(0) ($self, $c, $posts) {
  $posts->create($c->req->body_parameters);
}

# GET /posts/new
sub build :GET Chained('root') PathPart('new') Args(0) ($self, $c, $posts) {
  my $new_post = $posts->build;
}

# URL-PART: /posts/{$id}/...
sub find :Chained('root') PathPart('') CaptureArgs(1) ($self, $c, $posts, $id) {
  $c->action->next(my $post = $posts->find($id));
}

# GET /posts/{$id}
sub show :GET Chained('find') PathPart('') ($self, $c, $post) {
}

# DELETE /posts/{$id}
sub delete :DELETE Chained('find') PathPart('') ($self, $c, $post) {
  $post->delete;
}

# GET /posts/{$id}/edit
sub edit :GET Chained('find') PathPart('edit') ($self, $c, $post) {
}

# PATCH /posts/{$id}
sub update :PATCH Chained('find') PathPart('') ($self, $c, $post) {
  $post->update($c->req->body_parameters);
}

__PACKAGE__->meta->make_immutable;

In this version, we've cleaned up the code considerably as well as tightened the relationships between actions. There's no unneeded stash namespace pollution and of course we have named variables where a typo is going to give you a compile time error not a nasty hard to figure out error at runtime.

This is a very small tweak to chaining that doesn't monumentally change how your code works but I find that especially with very complicated applications it really aids in understanding and reduces overall boilerplate mess that you end up with in classic Catalyst chained actions. Adding this does not significantly change how chaining works, but I believe it is a small, iterative refinement that plays nice in existing code as well as in any greenfield project.

In this example we passed a single variable from one action to another, however the "next_action" method accepts lists as well. Just be careful when doing this and the receiving action has args or captures since those get tacked onto the end of the argument list so if you need to pass arbitrary length lists you should pass it as a reference or wrap it in an object.

The next action feature is a proposed update to Catalyst chaining that exists as a pull request right now (https://github.com/perl-catalyst/catalyst-runtime/pull/184) for your review and comments. You can view a larger example of code using it here (https://github.com/jjn1056/Valiant/tree/main/example) but just a warning that also contains some other new ideas that Catalyst developers might be unfamiliar with. Stay tuned for more on that later :)

One More Thing...

When you invoke "$c->action->next(@args)" in your code, it does more than send arguments to the connected children actions. It creates a call stack that reverses when you hit the last action in the chain, returning control back upward the action chain and allowing you to return arguments. This
can be a great way to return control to a parent action. Example:

sub a :Chained('/') PathPart('') CaptureArgs(0) ($self, $c) {
  my $ret = $c->action->next(my $info1 = 'aaa'); # $ret = 'from b from c'
}

sub b :Chained('a') PathPart('') CaptureArgs(0) ($self, $c, $info1) {
  my $ret = $c->action->next(my $info2 = 'bbb'); # $ret = 'from c';
  return "from b $ret";
}

sub c :Chained('b') PathPart('') Args(0) ($self, $c, $info2) {
  return 'from c';
}

Please note however that due to existing limitation in Catalyst you will need to return only a scalar so if you want to return an array or hash of stuff you will need to make it an arrayref or hashref.

Comments

[Aristotle Pagaltzis]

This looks neat!

As a separate point, though, since we’re talking about messy stashing, I find the real problem with Catalyst chaining is that you can only say “this action captures two args”. The real solution would be if instead you could say “this action captures an arg called thread_id and an arg called post_id”.

This does mean you would be accessing the args by string key, so no strictures benefit there. But in every other respect it would be better. Most importantly it would remove all need to manually store or pass arg values – whether by stashing or as sub parameters or however – since all args would be available in the args hash at all times anyway. Only the code which actually deals with a particular arg value would ever look at it.

This would make it much easier to handle exceptions early in the setup part of a chain, like “let me fetch the record here, but if I’m ultimately going to the edit action then I need to include an optional parameter with the value of an arg from a later action”.

It would also allow arg names chosen to have meaning shared among actions that process similar data, and then a root action could create objects based on which keys show up in the args hash. “If there are thread_id and post_id keys in the args, use them to initialize a Model::Post object on the stash.” Then if there are several actions that capture a post ID, you don’t have to repeat this step in each one of them. This would particularly make it much nicer to work with things than don’t have a DBIx::Class-style “build up a query step by step and only run it at the end” lazy evaluation-ish interface.

Basically you could do work at whichever point up or down the chain was convenient, instead of having to wait until dispatch gets far enough along the chain that you get access to some particular arg, while in the meantime only being able to pass along values collected along the chain – even if the passing-along happens more neatly than by stashing, such as with this “next action” feature.

Of course this would constitute a major disruption of the whole chained dispatch API… whereas “next action” can be tacked on quietly and fits into the existing interface seamlessly.

[John Napiorkowski]

I did some experimenting with named args, you can check out the code here:

metacpan.org/pod/Catalyst::Control...

its scoped just to the enclosing action. However I never got feedback on it so wasn't sure people cared about it :)

what I'd like is something like

sub find_user :GET('/user/{id}') ($self, $c, $id) { }

to just work, where we'd match the $id to {id} but right now there's no introspection ability to signatures (unless that's changed). Alternatively if we could support attributes in signatures lots of frameworks do like:

sub find_user :GET('/user/{id}') ($self, $c, Param('id') $id) { }

Not sure what it would take to get that into Perl signatures. Any ideas? Then you'd have $c->request->named_args->{id} and $c->request->named_arg('id') but that's a bigger lift to support in Catalyst. I could do it but I won't if I can't get a simple patch like my current one in production.

Hit my on IRC and we can collaborate if you have time.

[Aristotle Pagaltzis]

I never got feedback on it so wasn't sure people cared about it :)

I never found out about it before! I’m not wild about the use of %_ but I wouldn’t mind it if I could just ignore it. But this doesn’t seem to give me what I want because:

its scoped just to the enclosing action.

No, no scoping. Scoping just forces the application code to thread the value through to where it’s needed (and makes it annoying to get at the value if it’s needed sooner than where dispatch makes it available).

I’d definitely want $c->req->named_args – but it must be populated up front with all named args across the entire action chain, with no scoping. Every action gets access to everything.

where we'd match the $id to {id}

I wouldn’t mind that being in there if I didn’t have to use it. Too much magic for my tastes. Using an attribute would be fine, but really I don’t care for any approach that goes through @_ anyway.

Strict safety – if that’s your motivation – is nice, but it’s not a sufficient benefit. Making most of the pass-the-potato code go away is a better way of protecting against fat-fingering the key name than making the compiler catch you passing the wrong potato. Code that doesn’t exist cannot have bugs – of any type.

[John Napiorkowski]

I'm not sure how I feel about the idea that all actions in a chain have access to args from action below them, I think that might be weird but if you could work up a proof of concept show how its useful that might help my understanding.

[Aristotle Pagaltzis]

Like for example, one app has a screen with a tab navigation which can be either a global view or can show only the records associated with some given organization. The tab navigation is the same in both cases, and it shows counts in the tab labels. So in my root controller begin action I have

$c->get_counts_for_tabs
    if $c->action->attributes->{'Chained'}
    and grep 'orgunit/item' eq $_->reverse;

and then ::Controller::Orgunit::item, which CaptureArgs(1) to get the OU ID, does this:

$c->get_counts_for_tabs( $orgunit_id )
    if $c->action->attributes->{'Chained'}
    and grep 'orgunit/item' eq $_->reverse;

and then get_counts_for_tabs runs one set of queries for the one case and another for the other, and then it does some munging to put the results on the stash. Only the queries differ, the munging is the same, and it’s easier to follow if they are in the same place as the munging, so this is why this all has ended up this way. It makes sure the appropriate set of counts is fetched no matter where dispatch ends up.

How much nicer, though, if the get_counts_for_tabs method didn’t exist and the logic could just be part of the begin action which would say

if ( defined ( my $orgunit_id = $c->req->named_param->{'ou_id'} ) ) {
    ... # do something with it here
} else { ... }
... # munging of the results here

This arguably isn’t the best example – but it’s a real one (though the details are not 1:1 identical). Often when I wish for this sort of thing it is because I am hacking around some particular unusual case – and it’d be nice if I had to fight with only the problem, not also the dispatch.

[John Napiorkowski]

I'm not sure I love the idea of checking for parameters like this outside the action they are declared on since I worry you're going to get very complex and hard to debug code, but I'd need to see a fuller example really to be able to be sure. You might want to watch out for when I talk more about per context views, which make it easy to create a view and add bits to it over the course of a request, that might be a more elegant solution. I do however in general like the idea of named args and hope to see that in a future revision.

[Aristotle Pagaltzis]

Are you saying the currently implemented version is better…?

I am fully aware that testing for the presence of particular args has the potential for messy code. It’s essentially a parallel form of dispatch. But that’s exactly why it can also clean code up.

And a great advantage of doing things in this particular way is that it’s a very low-magic way of providing a mechanism to do the things it enables. It doesn’t require understanding a sizable amount of features specific to a complex framework to understand the code written against this interface.

[John Napiorkowski]

I'm not sure if there's a better approach TBH I'd need to see a lot more code. Sorry I'm not really that good at talking code at this level of abstraction. I have seen a lot of Catalyst applications in the wild and stuff like this just makes me a bit nervous because I've seen it lead to really hard to understand and maintain code. Maybe I'd feel cool with it if I saw 'all the code' :). Generally I just like to follow a rule of using stuff as close to their point of original as possible because one thing with MVC is you can easily get too much action at a distance going on which makes it confusing. Generally I think if you need to open more than three files to understand how a given section of code works you are starting to push your luck.