preserve $c->state 5.90102
John Napiorkowski [Thu, 29 Oct 2015 18:59:38 +0000 (13:59 -0500)]
Changes
lib/Catalyst.pm
lib/Catalyst/Controller.pm
lib/Catalyst/Dispatcher.pm
lib/Catalyst/Runtime.pm
t/state.t [new file with mode: 0644]

diff --git a/Changes b/Changes
index 813edaf..8d2f360 100644 (file)
--- a/Changes
+++ b/Changes
@@ -8,6 +8,8 @@
   - Improvements to 'search_extra' configuration and tests around using 
     uri_for as a class method (cngarrison++)
   - Fix when Path() is set and not geting registered as action (grim8634++)
+  - $c->state is now preserved over actions in a chain, and across begin,
+    auto, ->forward and ->detach.
 
 5.90101 - 2015-09-04
   - Fixed a regression introduced in the last release which caused test
index 820fc35..266ab2b 100644 (file)
@@ -180,7 +180,7 @@ sub composed_stats_class {
 __PACKAGE__->_encode_check(Encode::FB_CROAK | Encode::LEAVE_SRC);
 
 # Remember to update this in Catalyst::Runtime as well!
-our $VERSION = '5.90101';
+our $VERSION = '5.90102';
 $VERSION = eval $VERSION if $VERSION =~ /_/; # numify for warning-free dev releases
 
 sub import {
@@ -606,13 +606,17 @@ sub error {
     return $c->{error} || [];
 }
 
-
 =head2 $c->state
 
 Contains the return value of the last executed action.
 Note that << $c->state >> operates in a scalar context which means that all
 values it returns are scalar.
 
+Please note that if an action throws an exception, the value of state
+should no longer be considered the return if the last action.  It is generally
+going to be 0, which indicates an error state.  Examine $c->error for error
+details.
+
 =head2 $c->clear_errors
 
 Clear errors.  You probably don't want to clear the errors unless you are
@@ -2001,7 +2005,7 @@ via $c->error.
 sub execute {
     my ( $c, $class, $code ) = @_;
     $class = $c->component($class) || $class;
-    $c->state(0);
+    #$c->state(0);
 
     if ( $c->depth >= $RECURSION ) {
         my $action = $code->reverse();
@@ -2053,7 +2057,7 @@ sub execute {
             }
             $c->error($error);
         }
-        $c->state(0);
+        #$c->state(0);
     }
     return $c->state;
 }
@@ -2400,7 +2404,6 @@ sub prepare {
     my $c = $class->context_class->new({ $uploadtmp ? (_uploadtmp => $uploadtmp) : ()});
 
     $c->response->_context($c);
-
     $c->stats($class->stats_class->new)->enable($c->use_stats);
 
     if ( $c->debug || $c->config->{enable_catalyst_header} ) {
index b50a2ff..1766168 100644 (file)
@@ -143,7 +143,12 @@ sub _BEGIN : Private {
     my $begin = ( $c->get_actions( 'begin', $c->namespace ) )[-1];
     return 1 unless $begin;
     $begin->dispatch( $c );
-    return !@{ $c->error };
+    #If there is an error, all bets off
+    if( @{ $c->error }) {
+      return !@{ $c->error };
+    } else {
+      return $c->state || 1;
+    }
 }
 
 sub _AUTO : Private {
@@ -153,7 +158,7 @@ sub _AUTO : Private {
         $auto->dispatch( $c );
         return 0 unless $c->state;
     }
-    return 1;
+    return $c->state || 1;
 }
 
 sub _ACTION : Private {
@@ -164,7 +169,12 @@ sub _ACTION : Private {
     {
         $c->action->dispatch( $c );
     }
-    return !@{ $c->error };
+    #If there is an error, all bets off
+    if( @{ $c->error }) {
+      return !@{ $c->error };
+    } else {
+      return $c->state || 1;
+    }
 }
 
 sub _END : Private {
index f63267b..46f41f8 100644 (file)
@@ -273,6 +273,7 @@ Documented in L<Catalyst>
 sub detach {
     my ( $self, $c, $command, @args ) = @_;
     $self->_do_forward(detach => $c, $command, @args ) if $command;
+    $c->state(0); # Needed in order to skip any auto functions
     Catalyst::Exception::Detach->throw;
 }
 
index a9c86d1..bfd4ffd 100644 (file)
@@ -7,7 +7,7 @@ BEGIN { require 5.008003; }
 
 # Remember to update this in Catalyst as well!
 
-our $VERSION = '5.90101';
+our $VERSION = '5.90102';
 $VERSION = eval $VERSION if $VERSION =~ /_/; # numify for warning-free dev releases
 
 =head1 NAME
diff --git a/t/state.t b/t/state.t
new file mode 100644 (file)
index 0000000..10542b7
--- /dev/null
+++ b/t/state.t
@@ -0,0 +1,87 @@
+use warnings;
+use strict;
+use Test::More;
+use HTTP::Request::Common;
+
+{
+  package MyApp::Controller::Root;
+  $INC{'MyApp/Controller/Root.pm'} = __FILE__;
+
+  use base 'Catalyst::Controller';
+
+  MyApp::Controller::Root->config(namespace=>'');
+
+  sub begin :Action {
+    my ($self, $c) = @_;
+    Test::More::is($c->state, 0);
+    return 'begin';
+  }
+
+  sub auto :Action {
+    my ($self, $c) = @_;
+    Test::More::is($c->state, 'begin'); # default state of 1 is new to 9.0102
+    return 'auto';
+
+  }
+
+  sub base :Chained('/') PathPrefix CaptureArgs(0) {
+    my ($self, $c) = @_;
+    Test::More::is($c->state, 'auto');
+    return 10;
+  }
+
+    sub one :Chained('base') PathPart('') CaptureArgs(0) {
+      my ($self, $c) = @_;
+      Test::More::is($c->state, 10);
+      return 20;
+    }
+
+      sub two :Chained('one') PathPart('') Args(1) {
+        my ($self, $c, $arg) = @_;
+        Test::More::is($c->state, 20);
+        my $ret = $c->forward('forward2');
+        Test::More::is($ret, 25);
+        Test::More::is($c->state, 25);
+        return 30;
+      }
+
+  sub end :Action {
+    my ($self, $c) = @_;
+    Test::More::is($c->state, 30);
+    my $ret = $c->forward('forward1');
+    Test::More::is($ret, 100);
+    Test::More::is($c->state, 100);
+    $c->detach('detach1');
+  }
+
+  sub forward1 :Action {
+    my ($self, $c) = @_;
+    Test::More::is($c->state, 30);
+    return 100;
+  }
+
+  sub forward2 :Action {
+    my ($self, $c) = @_;
+    Test::More::is($c->state, 20);
+    return 25;
+  }
+
+  sub detach1 :Action {
+    my ($self, $c) = @_;
+    Test::More::is($c->state, 100);
+  }
+
+  package MyApp;
+  use Catalyst;
+
+  MyApp->config(show_internal_actions=>1);
+  MyApp->setup;
+}
+
+use Catalyst::Test 'MyApp';
+
+{
+  ok my $res = request "/100";
+}
+
+done_testing;