Merge 'add_captures_to_visit' into 'trunk'

r9541@dhcp-68 (orig r9539):  t0m | 2009-03-21 15:03:21 +0000
Visit currently does nothing with CaptureArgs, as failing tests in trunk demonstrate.
Branch as fixing this involves multiple commits and changing the visit api, etc..

r9542@dhcp-68 (orig r9540):  t0m | 2009-03-21 15:04:33 +0000
Make CaptureArgs get passed, this makes the test less fail, but not perfect yet. Also, needs docs..
r9543@dhcp-68 (orig r9541):  t0m | 2009-03-21 15:17:38 +0000
Fix the go test
r9544@dhcp-68 (orig r9542):  t0m | 2009-03-21 15:54:33 +0000
Doc changes for passing CaptureArgs
r9545@dhcp-68 (orig r9543):  t0m | 2009-03-21 15:56:08 +0000
Hack _invoke_as_component in a horrible way, so that it gives us back a path Action if it can, which makes >visit work correctly with qw/MyApp::Controller::Chained method_name/. I feel dirty, but it makes the test pass..
r9546@dhcp-68 (orig r9544):  t0m | 2009-03-21 18:04:06 +0000
Much cleaner. The visit class test wasn't actually testing visiting the class, so I fixed that and made the English in the error message more correct (IMO)
r9547@dhcp-68 (orig r9545):  t0m | 2009-03-21 18:11:04 +0000
Unfuck last commit, fix go tests, remove unneeded crud from TestApp, add FIXME for more cleanup
r9548@dhcp-68 (orig r9546):  t0m | 2009-03-21 20:39:52 +0000
Cleanup, changelog

diff --git a/Changes b/Changes
index 3f9c0a3..4433adf 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,5 +1,8 @@
 # This file documents the revision history for Perl extension Catalyst.
 
+        - Throw an exception rather than loading an app if an action
+          tries to chain to itself (t0m)
+          - Tests for this
         - Change the $c->visit and $c->go methods to optionally take
           CaptureArgs, making them useful to call ActionChains with (t0m)
           - Tests for this (radek)

diff --git a/lib/Catalyst/DispatchType/Chained.pm b/lib/Catalyst/DispatchType/Chained.pm
index 3b7502e..95d6559 100644 (file)
--- a/lib/Catalyst/DispatchType/Chained.pm
+++ b/lib/Catalyst/DispatchType/Chained.pm
@@ -258,8 +258,13 @@ sub register {
           "Multiple Chained attributes not supported registering ${action}"
         );
     }
+    my $chained_to = $chained_attr[0];
 
-    my $children = ($self->_children_of->{ $chained_attr[0] } ||= {});
+    Catalyst::Exception->throw(
+      "Actions cannot chain to themselves registering /${action}"
+    ) if ($chained_to eq '/' . $action);
+
+    my $children = ($self->_children_of->{ $chained_to } ||= {});
 
     my @path_part = @{ $action->attributes->{PathPart} || [] };
 

diff --git a/t/dead_load_multiple_chained_attributes.t b/t/dead_load_multiple_chained_attributes.t
index 7d1ad34..b870a75 100644 (file)
--- a/t/dead_load_multiple_chained_attributes.t
+++ b/t/dead_load_multiple_chained_attributes.t
@@ -10,16 +10,16 @@ plan tests => 4;
 
 use Catalyst::Test 'TestApp';
 
-eval q{  
+eval q{
     package TestApp::Controller::Action::Chained;
     sub should_fail : Chained('/') Chained('foo') Args(0) {}
 };
 ok(!$@);
 
-eval { TestApp->setup_actions; }; 
+eval { TestApp->setup_actions; };
 ok($@, 'Multiple chained attributes make action setup fail');
 
-eval q{      
+eval q{
     package TestApp::Controller::Action::Chained;
     no warnings 'redefine';
     sub should_fail {}

diff --git a/t/dead_recursive_chained_attributes.t b/t/dead_recursive_chained_attributes.t
new file mode 100644 (file)
index 0000000..77b9bcd
--- /dev/null
+++ b/t/dead_recursive_chained_attributes.t
@@ -0,0 +1,43 @@
+#!perl
+
+use strict;
+use warnings;
+use lib 't/lib';
+
+use Test::More tests => 6;
+
+use Catalyst::Test 'TestApp';
+
+eval q{
+    package TestApp::Controller::Action::Chained;
+    sub should_fail : Chained('should_fail') Args(0) {}
+};
+ok(!$@);
+
+eval { TestApp->setup_actions; };
+like($@, qr|Actions cannot chain to themselves registering /action/chained/should_fail|,
+    'Local self referencing attributes makes action setup fail');
+
+eval q{
+    package TestApp::Controller::Action::Chained;
+    no warnings 'redefine';
+    sub should_fail {}
+    use warnings 'redefine';
+    sub should_also_fail : Chained('/action/chained/should_also_fail') Args(0) {}
+};
+ok(!$@);
+
+eval { TestApp->setup_actions };
+like($@, qr|Actions cannot chain to themselves registering /action/chained/should_also_fail|,
+    'Full path self referencing attributes makes action setup fail');
+
+eval q{
+    package TestApp::Controller::Action::Chained;
+    no warnings 'redefine';
+    sub should_also_fail {}
+};
+ok(!$@);
+
+eval { TestApp->setup_actions };
+ok(!$@, 'And ok again') or warn $@;
+

diff --git a/t/lib/TestApp/Controller/Root.pm b/t/lib/TestApp/Controller/Root.pm
index a3978d6..53d79e2 100644 (file)
--- a/t/lib/TestApp/Controller/Root.pm
+++ b/t/lib/TestApp/Controller/Root.pm
@@ -20,8 +20,4 @@ sub localregex : LocalRegex('^localregex$') {
     $c->forward('TestApp::View::Dump::Request');
 }
 
-sub chain_to_self : Chained('chain_to_self') PathPart('') CaptureArgs(1) { }
-
-sub chain_recurse_endoint : Chained('chain_to_self') Args(0) { }
-
 1;

diff --git a/t/unit_core_action_chained.t b/t/unit_core_action_chained.t
deleted file mode 100644 (file)
index 1d4f4a8..0000000
--- a/t/unit_core_action_chained.t
+++ /dev/null
@@ -1,26 +0,0 @@
-#!perl
-
-use strict;
-use warnings;
-
-use FindBin;
-use lib "$FindBin::Bin/lib";
-
-use Test::More tests => 3;
-
-
-use TestApp;
-
-my $dispatch_type = TestApp->dispatcher->dispatch_type('Chained');
-isa_ok($dispatch_type, "Catalyst::DispatchType::Chained", "got dispatch type");
-
-# This test was failing due to recursion/OOM. set up an alarm so things dont
-# runaway
-local $SIG{ALRM} = sub { 
-    ok(0, "Chained->list didn't loop");
-    die "alarm expired - test probably looping";
-};
-alarm 10;
-
-$dispatch_type->list("TestApp");
-ok(1, "Chained->list didn't loop");