[Catalyst-commits] r6829 - in trunk/Catalyst-Runtime:
lib/Catalyst/DispatchType t t/lib/TestApp/Controller/Action
gbjk at dev.catalyst.perl.org
gbjk at dev.catalyst.perl.org
Sun Sep 2 18:42:49 GMT 2007
Author: gbjk
Date: 2007-09-02 18:42:48 +0100 (Sun, 02 Sep 2007)
New Revision: 6829
Modified:
trunk/Catalyst-Runtime/lib/Catalyst/DispatchType/Chained.pm
trunk/Catalyst-Runtime/t/lib/TestApp/Controller/Action/Chained.pm
trunk/Catalyst-Runtime/t/live_component_controller_action_chained.t
Log:
Fix Chained dispatch for several first-come first-served bugs when comparing Args() to a chain or two separate chains.
Added tests to that effect.
Modified: trunk/Catalyst-Runtime/lib/Catalyst/DispatchType/Chained.pm
===================================================================
--- trunk/Catalyst-Runtime/lib/Catalyst/DispatchType/Chained.pm 2007-09-02 17:31:22 UTC (rev 6828)
+++ trunk/Catalyst-Runtime/lib/Catalyst/DispatchType/Chained.pm 2007-09-02 17:42:48 UTC (rev 6829)
@@ -101,7 +101,8 @@
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;
return 0 unless $chain;
@@ -126,6 +127,7 @@
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) {
@@ -152,22 +154,33 @@
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 ];
- }
- } else {
+ if ($actions && (!$best_action || $#$action_parts < $#{$best_action->{parts}})){
+ $best_action = {
+ actions => [ $action, @$actions ],
+ captures=> [ @captures, @$captures ],
+ parts => $action_parts
+ };
+ }
+ }
+ else {
{
local $c->req->{arguments} = [ @{$c->req->args}, @parts ];
next TRY_ACTION unless $action->match($c);
}
- push(@{$c->req->args}, @parts);
- return [ $action ], [ ];
+ if (!$best_action || $#parts < $#{$best_action->{parts}}){
+ $best_action = {
+ actions => [ $action ],
+ captures=> [],
+ parts => \@parts
+ }
+ }
}
}
}
+ return @$best_action{qw/actions captures parts /} if $best_action;
return ();
}
Modified: trunk/Catalyst-Runtime/t/lib/TestApp/Controller/Action/Chained.pm
===================================================================
--- trunk/Catalyst-Runtime/t/lib/TestApp/Controller/Action/Chained.pm 2007-09-02 17:31:22 UTC (rev 6828)
+++ trunk/Catalyst-Runtime/t/lib/TestApp/Controller/Action/Chained.pm 2007-09-02 17:42:48 UTC (rev 6829)
@@ -158,6 +158,23 @@
sub mult_nopp_idall : Chained('mult_nopp_id') PathPart('') Args(0) { }
sub mult_nopp_idnew : Chained('mult_nopp_id') PathPart('new') Args(0) { }
+#
+# Test Choice between branches and early return logic
+# Declaration order is important for $children->{$*}, since this is first match best.
+#
+sub cc_base : Chained('/') PathPart('chained/choose_capture') CaptureArgs(0) { }
+sub cc_link : Chained('cc_base') PathPart('') CaptureArgs(0) { }
+sub cc_anchor : Chained('cc_link') PathPart('anchor.html') Args(0) { }
+sub cc_all : Chained('cc_base') PathPart('') Args() { }
+
+sub cc_a : Chained('cc_base') PathPart('') CaptureArgs(1) { }
+sub cc_a_link : Chained('cc_a') PathPart('a') CaptureArgs(0) { }
+sub cc_a_anchor : Chained('cc_a_link') PathPart('') Args() { }
+
+sub cc_b : Chained('cc_base') PathPart('b') CaptureArgs(0) { }
+sub cc_b_link : Chained('cc_b') PathPart('') CaptureArgs(1) { }
+sub cc_b_anchor : Chained('cc_b_link') PathPart('anchor.html') Args() { }
+
sub end :Private {
my ($self, $c) = @_;
return if $c->stash->{no_end};
Modified: trunk/Catalyst-Runtime/t/live_component_controller_action_chained.t
===================================================================
--- trunk/Catalyst-Runtime/t/live_component_controller_action_chained.t 2007-09-02 17:31:22 UTC (rev 6828)
+++ trunk/Catalyst-Runtime/t/live_component_controller_action_chained.t 2007-09-02 17:42:48 UTC (rev 6829)
@@ -10,7 +10,7 @@
BEGIN { $iters = $ENV{CAT_BENCH_ITERS} || 1; }
-use Test::More tests => 109*$iters;
+use Test::More tests => 118*$iters;
use Catalyst::Test 'TestApp';
if ( $ENV{CAT_BENCHMARK} ) {
@@ -766,4 +766,66 @@
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' );
+ }
+
}
More information about the Catalyst-commits
mailing list