Chained Actions:

Fix Args() vs Args(0) action conflict with no further parts.
Args(0) should win.
Add tests to that affect.

diff --git a/lib/Catalyst/DispatchType/Chained.pm b/lib/Catalyst/DispatchType/Chained.pm
index f5ae7ad..0274dba 100644 (file)
--- a/lib/Catalyst/DispatchType/Chained.pm
+++ b/lib/Catalyst/DispatchType/Chained.pm
@@ -170,17 +170,28 @@ sub recurse_match {
                     local $c->req->{arguments} = [ @{$c->req->args}, @parts ];
                     next TRY_ACTION unless $action->match($c);
                 }
-                if (!$best_action || $#parts < $#{$best_action->{parts}}){
+                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 == 0)){
                     $best_action = {
                         actions => [ $action ],
                         captures=> [],
                         parts   => \@parts
-                        }
                     }
+                }
             }
         }
     }
-    return @$best_action{qw/actions captures parts /} if $best_action;
+    return @$best_action{qw/actions captures parts/} if $best_action;
     return ();
 }
 

diff --git a/t/lib/TestApp/Controller/Action/Chained/ArgsOrder.pm b/t/lib/TestApp/Controller/Action/Chained/ArgsOrder.pm
new file mode 100644 (file)
index 0000000..80e580e
--- /dev/null
+++ b/t/lib/TestApp/Controller/Action/Chained/ArgsOrder.pm
@@ -0,0 +1,35 @@
+package TestApp::Controller::Action::Chained::ArgsOrder;
+use warnings;
+use strict;
+
+use base qw( Catalyst::Controller );
+
+#
+#   This controller builds a simple chain of three actions that
+#   will output the arguments they got passed to @_ after the
+#   context object. We do this to test if that passing works
+#   as it should.
+#
+
+sub base  :Chained('/') PathPart('argsorder') CaptureArgs(0) {
+    my ( $self, $c, $arg ) = @_;
+    push @{ $c->stash->{ passed_args } }, 'base', $arg;
+}
+
+sub index :Chained('base') PathPart('') Args(0) {
+    my ( $self, $c, $arg ) = @_;
+    push @{ $c->stash->{ passed_args } }, 'index', $arg;
+}
+
+sub all  :Chained('base') PathPart('') Args() {
+    my ( $self, $c, $arg ) = @_;
+    push @{ $c->stash->{ passed_args } }, 'all', $arg;
+}
+
+sub end : Private {
+    my ( $self, $c ) = @_;
+    no warnings 'uninitialized';
+    $c->response->body( join '; ', @{ $c->stash->{ passed_args } } );
+}
+
+1;

diff --git a/t/live_component_controller_action_chained.t b/t/live_component_controller_action_chained.t
index a4879f6..708fc67 100644 (file)
--- a/t/live_component_controller_action_chained.t
+++ b/t/live_component_controller_action_chained.t
@@ -10,7 +10,7 @@ our $iters;
 
 BEGIN { $iters = $ENV{CAT_BENCH_ITERS} || 1; }
 
-use Test::More tests => 118*$iters;
+use Test::More tests => 124*$iters;
 use Catalyst::Test 'TestApp';
 
 if ( $ENV{CAT_BENCHMARK} ) {
@@ -766,66 +766,101 @@ sub run_tests {
         is( $response->content, '; ', 'Content OK' );
     }
 
-       #
-       #       Higher Args() hiding more specific CaptureArgs chains sections
-       #
-       {
-               my @expected = qw[
-                       TestApp::Controller::Action::Chained->begin
-                       TestApp::Controller::Action::Chained->cc_base
-                       TestApp::Controller::Action::Chained->cc_link
-                       TestApp::Controller::Action::Chained->cc_anchor
-                       TestApp::Controller::Action::Chained->end
-                       ];
-
-               my $expected = join ', ', @expected;
-
-               ok( my $response = request('http://localhost/chained/choose_capture/anchor.html'),
-                       'Choose between an early Args() and a later more ideal chain' );
-               is( $response->header('X-Catalyst-Executed') => $expected, 'Executed actions');
-               is( $response->content => '; ', 'Content OK' );
-       }
-
-       #
-       #       Less specific chain not being seen correctly due to earlier looser capture
-       #
-       {
-               my @expected = qw[
-                       TestApp::Controller::Action::Chained->begin
-                       TestApp::Controller::Action::Chained->cc_base
-                       TestApp::Controller::Action::Chained->cc_b
-                       TestApp::Controller::Action::Chained->cc_b_link
-                       TestApp::Controller::Action::Chained->cc_b_anchor
-                       TestApp::Controller::Action::Chained->end
-                       ];
-
-               my $expected = join ', ', @expected;
-
-               ok( my $response = request('http://localhost/chained/choose_capture/b/a/anchor.html'),
-                       'Choose between a more specific chain and an earlier looser one' );
-               is( $response->header('X-Catalyst-Executed') => $expected, 'Executed actions');
-               is( $response->content => 'a; ', 'Content OK' );
-       }
-
-       #
-       #       Check we get the looser one when it's the correct match
-       #
-       {
-               my @expected = qw[
-                       TestApp::Controller::Action::Chained->begin
-                       TestApp::Controller::Action::Chained->cc_base
-                       TestApp::Controller::Action::Chained->cc_a
-                       TestApp::Controller::Action::Chained->cc_a_link
-                       TestApp::Controller::Action::Chained->cc_a_anchor
-                       TestApp::Controller::Action::Chained->end
-                       ];
-
-               my $expected = join ', ', @expected;
-
-               ok( my $response = request('http://localhost/chained/choose_capture/a/a/anchor.html'),
-                       'Choose between a more specific chain and an earlier looser one' );
-               is( $response->header('X-Catalyst-Executed') => $expected, 'Executed actions');
-               is( $response->content => 'a; anchor.html', 'Content OK' );
-       }
+    #
+    #   Higher Args() hiding more specific CaptureArgs chains sections
+    #
+    {
+        my @expected = qw[
+            TestApp::Controller::Action::Chained->begin
+            TestApp::Controller::Action::Chained->cc_base
+            TestApp::Controller::Action::Chained->cc_link
+            TestApp::Controller::Action::Chained->cc_anchor
+            TestApp::Controller::Action::Chained->end
+            ];
+
+        my $expected = join ', ', @expected;
+
+        ok( my $response = request('http://localhost/chained/choose_capture/anchor.html'),
+            'Choose between an early Args() and a later more ideal chain' );
+        is( $response->header('X-Catalyst-Executed') => $expected, 'Executed actions');
+        is( $response->content => '; ', 'Content OK' );
+    }
+
+    #
+    #   Less specific chain not being seen correctly due to earlier looser capture
+    #
+    {
+        my @expected = qw[
+            TestApp::Controller::Action::Chained->begin
+            TestApp::Controller::Action::Chained->cc_base
+            TestApp::Controller::Action::Chained->cc_b
+            TestApp::Controller::Action::Chained->cc_b_link
+            TestApp::Controller::Action::Chained->cc_b_anchor
+            TestApp::Controller::Action::Chained->end
+            ];
+
+        my $expected = join ', ', @expected;
+
+        ok( my $response = request('http://localhost/chained/choose_capture/b/a/anchor.html'),
+            'Choose between a more specific chain and an earlier looser one' );
+        is( $response->header('X-Catalyst-Executed') => $expected, 'Executed actions');
+        is( $response->content => 'a; ', 'Content OK' );
+    }
+
+    #
+    #   Check we get the looser one when it's the correct match
+    #
+    {
+        my @expected = qw[
+            TestApp::Controller::Action::Chained->begin
+            TestApp::Controller::Action::Chained->cc_base
+            TestApp::Controller::Action::Chained->cc_a
+            TestApp::Controller::Action::Chained->cc_a_link
+            TestApp::Controller::Action::Chained->cc_a_anchor
+            TestApp::Controller::Action::Chained->end
+            ];
+
+        my $expected = join ', ', @expected;
+
+        ok( my $response = request('http://localhost/chained/choose_capture/a/a/anchor.html'),
+            'Choose between a more specific chain and an earlier looser one' );
+        is( $response->header('X-Catalyst-Executed') => $expected, 'Executed actions');
+        is( $response->content => 'a; anchor.html', 'Content OK' );
+    }
+
+    #
+    #   Args(0) should win over Args() if we actually have no arguments.
+    {
+        my @expected = qw[
+          TestApp::Controller::Action::Chained->begin
+          TestApp::Controller::Action::Chained::ArgsOrder->base
+          TestApp::Controller::Action::Chained::ArgsOrder->index
+          TestApp::Controller::Action::Chained::ArgsOrder->end
+        ];
+
+        my $expected = join( ", ", @expected );
+
+    # With no args, we should run "index"
+        ok( my $response = request('http://localhost/argsorder/'),
+            'Correct arg order ran' );
+        is( $response->header('X-Catalyst-Executed'),
+            $expected, 'Executed actions' );
+        is( $response->content, 'base; ; index; ', 'Content OK' );
+
+    # With args given, run "all"
+        ok( $response = request('http://localhost/argsorder/X'),
+            'Correct arg order ran' );
+        is( $response->header('X-Catalyst-Executed'), 
+        join(", ", 
+         qw[
+             TestApp::Controller::Action::Chained->begin
+             TestApp::Controller::Action::Chained::ArgsOrder->base
+             TestApp::Controller::Action::Chained::ArgsOrder->all
+             TestApp::Controller::Action::Chained::ArgsOrder->end
+          ])
+      );
+        is( $response->content, 'base; ; all; X', 'Content OK' );
+    
+    }
 
 }