[Catalyst-commits] r9568 - in Catalyst-Runtime/5.70/trunk: . lib/Catalyst/DispatchType t t/lib/TestApp/Controller

[t0m at dev.catalyst.perl.org]

Author: t0m
Date: 2009-03-24 00:14:08 +0000 (Tue, 24 Mar 2009)
New Revision: 9568

Added:
   Catalyst-Runtime/5.70/trunk/t/dead_recursive_chained_attributes.t
Removed:
   Catalyst-Runtime/5.70/trunk/t/unit_core_action_chained.t
Modified:
   Catalyst-Runtime/5.70/trunk/Changes
   Catalyst-Runtime/5.70/trunk/lib/Catalyst/DispatchType/Chained.pm
   Catalyst-Runtime/5.70/trunk/t/dead_load_multiple_chained_attributes.t
   Catalyst-Runtime/5.70/trunk/t/lib/TestApp/Controller/Root.pm
Log:
svn merge -r 9548:9549 http://dev.catalyst.perl.org/repos/Catalyst/Catalyst-Runtime/5.80/trunk
Make chaining to yourself explode at app load time

Modified: Catalyst-Runtime/5.70/trunk/Changes
===================================================================
--- Catalyst-Runtime/5.70/trunk/Changes	2009-03-23 23:53:47 UTC (rev 9567)
+++ Catalyst-Runtime/5.70/trunk/Changes	2009-03-24 00:14:08 UTC (rev 9568)
@@ -11,6 +11,9 @@
           for dispatchable actions so that ->visit or ->going to ActionChains
           with qw/Class::Name method_name/ works correctly (t0m)
           - Tests for this (Radoslaw Zielinski)
+        - Throw an exception rather than loading an app if an action
+          tries to chain to itself (t0m)
+          - Tests for this
         - Added Catalyst::Test::ctx_request to be able to inspect
           the context object after a request is made (Jos Boumans)
 

Modified: Catalyst-Runtime/5.70/trunk/lib/Catalyst/DispatchType/Chained.pm
===================================================================
--- Catalyst-Runtime/5.70/trunk/lib/Catalyst/DispatchType/Chained.pm	2009-03-23 23:53:47 UTC (rev 9567)
+++ Catalyst-Runtime/5.70/trunk/lib/Catalyst/DispatchType/Chained.pm	2009-03-24 00:14:08 UTC (rev 9568)
@@ -222,9 +222,14 @@
           "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} || [] };
 
     my $part = $action->name;

Modified: Catalyst-Runtime/5.70/trunk/t/dead_load_multiple_chained_attributes.t
===================================================================
--- Catalyst-Runtime/5.70/trunk/t/dead_load_multiple_chained_attributes.t	2009-03-23 23:53:47 UTC (rev 9567)
+++ Catalyst-Runtime/5.70/trunk/t/dead_load_multiple_chained_attributes.t	2009-03-24 00:14:08 UTC (rev 9568)
@@ -10,16 +10,16 @@
 
 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 {}

Copied: Catalyst-Runtime/5.70/trunk/t/dead_recursive_chained_attributes.t (from rev 9549, Catalyst-Runtime/5.80/trunk/t/dead_recursive_chained_attributes.t)
===================================================================
--- Catalyst-Runtime/5.70/trunk/t/dead_recursive_chained_attributes.t	                        (rev 0)
+++ Catalyst-Runtime/5.70/trunk/t/dead_recursive_chained_attributes.t	2009-03-24 00:14:08 UTC (rev 9568)
@@ -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 $@;
+

Modified: Catalyst-Runtime/5.70/trunk/t/lib/TestApp/Controller/Root.pm
===================================================================
--- Catalyst-Runtime/5.70/trunk/t/lib/TestApp/Controller/Root.pm	2009-03-23 23:53:47 UTC (rev 9567)
+++ Catalyst-Runtime/5.70/trunk/t/lib/TestApp/Controller/Root.pm	2009-03-24 00:14:08 UTC (rev 9568)
@@ -20,8 +20,4 @@
     $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;

Deleted: Catalyst-Runtime/5.70/trunk/t/unit_core_action_chained.t
===================================================================
--- Catalyst-Runtime/5.70/trunk/t/unit_core_action_chained.t	2009-03-23 23:53:47 UTC (rev 9567)
+++ Catalyst-Runtime/5.70/trunk/t/unit_core_action_chained.t	2009-03-24 00:14:08 UTC (rev 9568)
@@ -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");