Chained: Fix Args check treating undefind Args the same as 0.
[catagits/Catalyst-Runtime.git] / lib / Catalyst / DispatchType / Chained.pm
index 2fb0a0b..02f4c21 100644 (file)
@@ -85,7 +85,7 @@ sub list {
         $paths->row(@$_) for @rows;
     }
 
-    $c->log->debug( "Loaded Path Part actions:\n" . $paths->draw );
+    $c->log->debug( "Loaded Chained actions:\n" . $paths->draw . "\n" );
 }
 
 =head2 $self->match( $c, $path )
@@ -101,7 +101,8 @@ sub match {
 
     my @parts = split('/', $path);
 
-    my ($chain, $captures) = $self->recurse_match($c, '/', \@parts);
+    my ($chain, $captures, $parts) = $self->recurse_match($c, '/', \@parts);
+    push @{$c->req->args}, @$parts if $parts && @$parts;
 
     return 0 unless $chain;
 
@@ -126,6 +127,7 @@ sub recurse_match {
     my ( $self, $c, $parent, $path_parts ) = @_;
     my $children = $self->{children_of}{$parent};
     return () unless $children;
+    my $best_action;
     my @captures;
     TRY: foreach my $try_part (sort { length($b) <=> length($a) }
                                    keys %$children) {
@@ -141,6 +143,10 @@ sub recurse_match {
         my @try_actions = @{$children->{$try_part}};
         TRY_ACTION: foreach my $action (@try_actions) {
             if (my $capture_attr = $action->attributes->{CaptureArgs}) {
+
+                # Short-circuit if not enough remaining parts
+                next TRY_ACTION unless @parts >= $capture_attr->[0];
+
                 my @captures;
                 my @parts = @parts; # localise
 
@@ -148,22 +154,44 @@ sub recurse_match {
                 push(@captures, splice(@parts, 0, $capture_attr->[0]));
 
                 # try the remaining parts against children of this action
-                my ($actions, $captures) = $self->recurse_match(
+                my ($actions, $captures, $action_parts) = $self->recurse_match(
                                              $c, '/'.$action->reverse, \@parts
                                            );
-                if ($actions) {
-                    return [ $action, @$actions ], [ @captures, @$captures ];
+                if ($actions && (!$best_action || $#$action_parts < $#{$best_action->{parts}})){
+                    $best_action = {
+                        actions => [ $action, @$actions ],
+                        captures=> [ @captures, @$captures ],
+                        parts   => $action_parts
+                        };
                 }
-            } else {
+            }
+            else {
                 {
                     local $c->req->{arguments} = [ @{$c->req->args}, @parts ];
                     next TRY_ACTION unless $action->match($c);
                 }
-                push(@{$c->req->args}, @parts);
-                return [ $action ], [ ];
+                my $args_attr = $action->attributes->{Args}->[0];
+
+                #    No best action currently
+                # OR This one matches with fewer parts left than the current best action,
+                #    And therefore is a better match
+                # OR No parts and this expects 0 
+                #    The current best action might also be Args(0),
+                #    but we couldn't chose between then anyway so we'll take the last seen
+
+                if (!$best_action                       ||
+                    @parts < @{$best_action->{parts}}   ||
+                    (!@parts && $args_attr eq 0)){
+                    $best_action = {
+                        actions => [ $action ],
+                        captures=> [],
+                        parts   => \@parts
+                    }
+                }
             }
         }
     }
+    return @$best_action{qw/actions captures parts/} if $best_action;
     return ();
 }
 
@@ -192,7 +220,11 @@ sub register {
         if ($parent eq '.') {
             $parent = '/'.$action->namespace;
         } elsif ($parent !~ m/^\//) {
-            $parent = '/'.join('/', $action->namespace, $parent);
+            if ($action->namespace) {
+                $parent = '/'.join('/', $action->namespace, $parent);
+            } else {
+                $parent = '/'.$parent; # special case namespace '' (root)
+            }
         }
     } else {
         $parent = '/'
@@ -244,7 +276,7 @@ sub uri_for_action {
     my ( $self, $action, $captures ) = @_;
 
     return undef unless ($action->attributes->{Chained}
-                           && $action->attributes->{Args});
+                           && !$action->attributes->{CaptureArgs});
 
     my @parts = ();
     my @captures = @$captures;
@@ -253,11 +285,13 @@ sub uri_for_action {
     while ($curr) {
         if (my $cap = $curr->attributes->{CaptureArgs}) {
             return undef unless @captures >= $cap->[0]; # not enough captures
-            unshift(@parts, splice(@captures, -$cap->[0]));
+            if ($cap->[0]) {
+                unshift(@parts, splice(@captures, -$cap->[0]));
+            }
         }
         if (my $pp = $curr->attributes->{PartPath}) {
             unshift(@parts, $pp->[0])
-                if (defined $pp->[0] && length $pp->[0]);
+                if (defined($pp->[0]) && length($pp->[0]));
         }
         $parent = $curr->attributes->{Chained}->[0];
         $curr = $self->{actions}{$parent};
@@ -276,13 +310,13 @@ sub uri_for_action {
 =head2 Introduction
 
 The C<Chained> attribute allows you to chain public path parts together
-by their private names. A chain part's path can be specified with C<PathPart>
-and can be declared to expect an arbitrary number of arguments. The endpoint
-of the chain specifies how many arguments it gets through the C<Args>
-attribute. C<:Args(0)> would be none at all, C<:Args> without an integer
-would be unlimited. The path parts that aren't endpoints are using
-C<CaptureArgs> to specify how many parameters they expect to receive. As an
-example setup:
+by their private names. A chain part's path can be specified with
+C<PathPart> and can be declared to expect an arbitrary number of
+arguments. The endpoint of the chain specifies how many arguments it
+gets through the C<Args> attribute. C<:Args(0)> would be none at all,
+C<:Args> without an integer would be unlimited. The path parts that
+aren't endpoints are using C<CaptureArgs> to specify how many parameters
+they expect to receive. As an example setup:
 
   package MyApp::Controller::Greeting;
   use base qw/ Catalyst::Controller /;
@@ -305,8 +339,8 @@ example setup:
   }
 
 The debug output provides a separate table for chained actions, showing
-the whole chain as it would match and the actions it contains. Here's
-an example of the startup output with our actions above:
+the whole chain as it would match and the actions it contains. Here's an
+example of the startup output with our actions above:
 
   ...
   [debug] Loaded Path Part actions:
@@ -318,48 +352,49 @@ an example of the startup output with our actions above:
   '-----------------------+------------------------------'
   ...
 
-As you can see, Catalyst only deals with chains as whole path and 
-builds one for each endpoint, which are the actions with C<:Chained>
-but without C<:CaptureArgs>.
+As you can see, Catalyst only deals with chains as whole paths and
+builds one for each endpoint, which are the actions with C<:Chained> but
+without C<:CaptureArgs>.
 
 Let's assume this application gets a request at the path
-C</hello/23/world/12>, what happens then? First, Catalyst will dispatch
-to the C<hello> action and pass the value C<23> as argument to it after
-the context. It does so because we have previously used C<:CaptureArgs(1)>
-to declare that it has one path part after itself as it's argument. We
-told Catalyst that this is the beginning of the chain by specifying
-C<:Chained('/')>. Also note that instead of saying C<:PathPart('hello')>
-we could also just have said C<:PathPart>, as it defaults to the name of
-the action.
+C</hello/23/world/12>. What happens then? First, Catalyst will dispatch
+to the C<hello> action and pass the value C<23> as an argument to it
+after the context. It does so because we have previously used
+C<:CaptureArgs(1)> to declare that it has one path part after itself as
+its argument. We told Catalyst that this is the beginning of the chain
+by specifying C<:Chained('/')>. Also note that instead of saying
+C<:PathPart('hello')> we could also just have said C<:PathPart>, as it
+defaults to the name of the action.
 
 After C<hello> has run, Catalyst goes on to dispatch to the C<world>
-action. This is the last action to be called, as Catalyst knows this
-is an endpoint because we specified no C<:CaptureArgs> attribute. Nevertheless
-we specify that this action expects an argument, but at this point we're
-using C<:Args(1)> to do that. We could also have said C<:Args> or leave 
-it out alltogether, which would mean this action gets all arguments that
-are there. This action's C<:Chained> attribute says C<hello> and tells
-Catalyst that the C<hello> action in the current controller is it's 
-parent.
+action. This is the last action to be called: Catalyst knows this is an
+endpoint because we did not specify a C<:CaptureArgs>
+attribute. Nevertheless we specify that this action expects an argument,
+but at this point we're using C<:Args(1)> to do that. We could also have
+said C<:Args> or left it out altogether, which would mean this action
+would get all arguments that are there. This action's C<:Chained>
+attribute says C<hello> and tells Catalyst that the C<hello> action in
+the current controller is its parent.
 
 With this we have built a chain consisting of two public path parts.
-C<hello> captures one part of the path as it's argument, and also specifies
-the path root as it's parent. So this part is C</hello/$arg>. The next part
-is the endpoint C<world>, expecting one argument. It sums up to the path
-part C<world/$arg>. This leads to a complete chain of 
-C</hello/$arg/world/$arg> which is matched against the requested paths.
-
-This example application would, if run and called by e.g. 
-C</hello/23/world/12>, set the stash value C<message> to C<Hello > and
-the value C<arg_sum> to C<23>. The C<world> action would then append
-C<World!> to C<message> and add C<12> to the stash's C<arg_sum> value.
-For the sake of simplicity no view is shown. Instead we just put the
-values of the stash into our body. So the output would look like:
+C<hello> captures one part of the path as its argument, and also
+specifies the path root as its parent. So this part is
+C</hello/$arg>. The next part is the endpoint C<world>, expecting one
+argument. It sums up to the path part C<world/$arg>. This leads to a
+complete chain of C</hello/$arg/world/$arg> which is matched against the
+requested paths.
+
+This example application would, if run and called by e.g.
+C</hello/23/world/12>, set the stash value C<message> to "Hello" and the
+value C<arg_sum> to "23". The C<world> action would then append "World!"
+to C<message> and add C<12> to the stash's C<arg_sum> value.  For the
+sake of simplicity no view is shown. Instead we just put the values of
+the stash into our body. So the output would look like:
 
   Hello World!
   35
 
-And our test server would've given us this debugging output for the
+And our test server would have given us this debugging output for the
 request:
 
   ...
@@ -375,9 +410,9 @@ request:
   '------------------------------------------+-----------'
   ...
 
-What would be common usecases of this dispatching technique? It gives the
-possibility to split up logic that contains steps that each depend on each
-other. An example would be, for example, a wiki path like
+What would be common uses of this dispatch technique? It gives the
+possibility to split up logic that contains steps that each depend on
+each other. An example would be, for example, a wiki path like
 C</wiki/FooBarPage/rev/23/view>. This chain can be easily built with
 these actions:
 
@@ -389,31 +424,30 @@ these actions:
 
   sub rev : PathPart('rev') Chained('wiki') CaptureArgs(1) {
       my ( $self, $c, $revision_id ) = @_;
-      #  use the page object in the stash to get at it's
+      #  use the page object in the stash to get at its
       #  revision with number $revision_id
   }
 
   sub view : PathPart Chained('rev') Args(0) {
       my ( $self, $c ) = @_;
-      #  display the revision in our stash. An other option
+      #  display the revision in our stash. Another option
       #  would be to forward a compatible object to the action
       #  that displays the default wiki pages, unless we want
       #  a different interface here, for example restore
       #  functionality.
   }
 
-It would now be possible to add other endpoints. For example C<restore> to
-restore this specific revision as current state.
+It would now be possible to add other endpoints, for example C<restore>
+to restore this specific revision as the current state.
 
-Also, you of course don't have to put all the chained actions in one
-controller. The specification of the parent through C<:Chained> also takes
-an absolute action path as it's argument. Just specify it with a leading
-C</>.
+You don't have to put all the chained actions in one controller. The
+specification of the parent through C<:Chained> also takes an absolute
+action path as its argument. Just specify it with a leading C</>.
 
 If you want, for example, to have actions for the public paths
-C</foo/12/edit> and C</foo/12>, just specify two actions with 
+C</foo/12/edit> and C</foo/12>, just specify two actions with
 C<:PathPart('foo')> and C<:Chained('/')>. The handler for the former
-path needs a C<:CaptureArgs(1)> attribute and a endpoint with 
+path needs a C<:CaptureArgs(1)> attribute and a endpoint with
 C<:PathPart('edit')> and C<:Chained('foo')>. For the latter path give
 the action just a C<:Args(1)> to mark it as endpoint. This sums up to
 this debugging output:
@@ -451,16 +485,16 @@ effect as using C<:PathPart>, it would default to the action name.
 Has to be specified for every child in the chain. Possible values are
 absolute and relative private action paths, with the relatives pointing
 to the current controller, or a single slash C</> to tell Catalyst that
-this is the root of a chain. The attribute C<:Chained> without aguments
-also defaults to the C</> behaviour.
+this is the root of a chain. The attribute C<:Chained> without arguments
+also defaults to the C</> behavior.
 
-Due to the fact that you can specify an absolute path to the parent
-action, it doesn't matter to Catalyst where that parent is located. So,
-if your design requests it, you can redispatch a chain through every
-controller or namespace you want.
+Because you can specify an absolute path to the parent action, it
+doesn't matter to Catalyst where that parent is located. So, if your
+design requests it, you can redispatch a chain through any controller or
+namespace you want.
 
 Another interesting possibility gives C<:Chained('.')>, which chains
-itself to an action with the path of the current controllers namespace.
+itself to an action with the path of the current controller's namespace.
 For example:
 
   #   in MyApp::Controller::Foo
@@ -470,55 +504,56 @@ For example:
   sub baz : Chained('.') Args(1) { ... }
 
 This builds up a chain like C</bar/*/baz/*>. The specification of C<.>
-as argument to Chained here chains the C<baz> action to an action with
-the path of the current controller namespace, namely C</foo/bar>. That
-action chains directly to C</>, so the above chain comes out as end
-product.
+as the argument to Chained here chains the C<baz> action to an action
+with the path of the current controller namespace, namely
+C</foo/bar>. That action chains directly to C</>, so the C</bar/*/baz/*>
+chain comes out as the end product.
 
 =item CaptureArgs
 
-Also has to be specified for every part of the chain that is not an
+Must be specified for every part of the chain that is not an
 endpoint. With this attribute Catalyst knows how many of the following
-parts of the path (separated by C</>) this action wants to captures as
-it's arguments. If it doesn't expect any, just specify C<:CaptureArgs(0)>.
-The captures get passed to the action's C<@_> right after the context,
-but you can also find them as array reference in 
+parts of the path (separated by C</>) this action wants to capture as
+its arguments. If it doesn't expect any, just specify
+C<:CaptureArgs(0)>.  The captures get passed to the action's C<@_> right
+after the context, but you can also find them as array references in
 C<$c-E<gt>request-E<gt>captures-E<gt>[$level]>. The C<$level> is the
 level of the action in the chain that captured the parts of the path.
 
-An action that is part of a chain (read: that has a C<:Chained> attribute)
-but has no C<:CaptureArgs> attribute is treated by Catalyst as a chain end.
+An action that is part of a chain (that is, one that has a C<:Chained>
+attribute) but has no C<:CaptureArgs> attribute is treated by Catalyst
+as a chain end.
 
 =item Args
 
 By default, endpoints receive the rest of the arguments in the path. You
 can tell Catalyst through C<:Args> explicitly how many arguments your
 endpoint expects, just like you can with C<:CaptureArgs>. Note that this
-also influences if this chain is invoked on a request. A chain with an
+also affects whether this chain is invoked on a request. A chain with an
 endpoint specifying one argument will only match if exactly one argument
 exists in the path.
 
 You can specify an exact number of arguments like C<:Args(3)>, including
 C<0>. If you just say C<:Args> without any arguments, it is the same as
-leaving it out alltogether: The chain is matched independent of the number
+leaving it out altogether: The chain is matched regardless of the number
 of path parts after the endpoint.
 
-Just like with C<:CaptureArgs>, the arguments get passed to the action in
+Just as with C<:CaptureArgs>, the arguments get passed to the action in
 C<@_> after the context object. They can also be reached through
 C<$c-E<gt>request-E<gt>arguments>.
 
 =back
 
-=head2 auto actions, dispatching and forwarding
+=head2 Auto actions, dispatching and forwarding
 
 Note that the list of C<auto> actions called depends on the private path
-of the endpoint of the chain, not on the chained actions way. The C<auto>
-actions will be run before the chain dispatching begins. In every other
-aspect, C<auto> actions behave as documented.
+of the endpoint of the chain, not on the chained actions way. The
+C<auto> actions will be run before the chain dispatching begins. In
+every other aspect, C<auto> actions behave as documented.
 
 The C<forward>ing to other actions does just what you would expect. But if
 you C<detach> out of a chain, the rest of the chain will not get called
-after the C<detach> returned.
+after the C<detach>.
 
 =head1 AUTHOR