Merge branch 'master' into gsoc_breadboard

diff --git a/Changes b/Changes
index 5135713..c13291d 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,5 +1,9 @@
 # This file documents the revision history for Perl extension Catalyst.
 
+ Bug fixes:
+  - Fix request body parameters being multiply rebuilt. Fixes both
+    RT#75607 and CatalystX::DebugFilter
+
  Documentation:
   - Fix documentation in Catalyst::Component to show attributes and
     calling readers, rather than accessing elements in the $self->{} hash

diff --git a/lib/Catalyst/Component.pm b/lib/Catalyst/Component.pm
index 1f9b45e..1ea719f 100644 (file)
--- a/lib/Catalyst/Component.pm
+++ b/lib/Catalyst/Component.pm
@@ -61,7 +61,7 @@ It provides you with a generic new() for component construction through Catalyst
 component loader with config() support and a process() method placeholder.
 
 B<Note> that calling C<< $self->config >> inside a component is strongly
-disrecommended - the correctly merged config should have already been
+not recommended - the correctly merged config should have already been
 passed to the constructor and stored in attributes - accessing
 the config accessor directly from an instance is likely to get the
 wrong values (as it only holds the class wide config, not things loaded

diff --git a/lib/Catalyst/DispatchType/Chained.pm b/lib/Catalyst/DispatchType/Chained.pm
index ede2b69..3b7b0c4 100644 (file)
--- a/lib/Catalyst/DispatchType/Chained.pm
+++ b/lib/Catalyst/DispatchType/Chained.pm
@@ -201,9 +201,10 @@ sub recurse_match {
         my @try_actions = @{$children->{$try_part}};
         TRY_ACTION: foreach my $action (@try_actions) {
             if (my $capture_attr = $action->attributes->{CaptureArgs}) {
+                $capture_attr ||= 0;
 
                 # Short-circuit if not enough remaining parts
-                next TRY_ACTION unless @parts >= ($capture_attr->[0]||0);
+                next TRY_ACTION unless @parts >= $capture_attr->[0];
 
                 my @captures;
                 my @parts = @parts; # localise
@@ -360,7 +361,7 @@ sub uri_for_action {
     my $curr = $action;
     while ($curr) {
         if (my $cap = $curr->attributes->{CaptureArgs}) {
-            return undef unless @captures >= $cap->[0]; # not enough captures
+            return undef unless @captures >= ($cap->[0]||0); # not enough captures
             if ($cap->[0]) {
                 unshift(@parts, splice(@captures, -$cap->[0]));
             }

diff --git a/lib/Catalyst/Request.pm b/lib/Catalyst/Request.pm
index 9c43407..3d0c03a 100644 (file)
--- a/lib/Catalyst/Request.pm
+++ b/lib/Catalyst/Request.pm
@@ -127,7 +127,7 @@ has body_parameters => (
   is => 'rw',
   required => 1,
   lazy => 1,
-  default => sub { {} },
+  builder => 'prepare_body_parameters',
 );
 
 has uploads => (
@@ -152,8 +152,6 @@ has parameters => (
 
 sub prepare_parameters {
     my ( $self ) = @_;
-
-    $self->prepare_body;
     my $parameters = {};
     my $body_parameters = $self->body_parameters;
     my $query_parameters = $self->query_parameters;
@@ -175,12 +173,6 @@ sub prepare_parameters {
     $parameters;
 }
 
-before body_parameters => sub {
-    my ($self) = @_;
-    $self->prepare_body;
-    $self->prepare_body_parameters;
-};
-
 has _uploadtmp => (
     is => 'ro',
     predicate => '_has_uploadtmp',
@@ -225,9 +217,10 @@ sub prepare_body_chunk {
 sub prepare_body_parameters {
     my ( $self ) = @_;
 
+    $self->prepare_body if ! $self->_has_body;
     return unless $self->_body;
 
-    $self->{body_parameters} = $self->_body->param; # FIXME!! Recursion here.
+    return $self->_body->param;
 }
 
 sub prepare_connection {
@@ -277,7 +270,7 @@ has _body => (
 #             and provide a custom reader..
 sub body {
   my $self = shift;
-  $self->prepare_body();
+  $self->prepare_body unless ! $self->_has_body;
   croak 'body is a reader' if scalar @_;
   return blessed $self->_body ? $self->_body->body : $self->_body;
 }

diff --git a/t/aggregate/live_engine_request_parameters.t b/t/aggregate/live_engine_request_parameters.t
index 56a7074..e97ba81 100644 (file)
--- a/t/aggregate/live_engine_request_parameters.t
+++ b/t/aggregate/live_engine_request_parameters.t
@@ -6,7 +6,7 @@ use warnings;
 use FindBin;
 use lib "$FindBin::Bin/../lib";
 
-use Test::More tests => 53;
+use Test::More tests => 54;
 use Catalyst::Test 'TestApp';
 
 use Catalyst::Request;
@@ -71,8 +71,6 @@ use HTTP::Request::Common;
         'Content-Type' => 'application/x-www-form-urlencoded'
     );
 
-    unshift( @{ $parameters->{a} }, 1, 2, 3 );
-
     ok( my $response = request($request), 'Request' );
     ok( $response->is_success, 'Response Successful 2xx' );
     is( $response->content_type, 'text/plain', 'Response Content-Type' );
@@ -84,6 +82,9 @@ use HTTP::Request::Common;
     ok( eval '$creq = ' . $response->content, 'Unserialize Catalyst::Request' );
     isa_ok( $creq, 'Catalyst::Request' );
     is( $creq->method, 'POST', 'Catalyst::Request method' );
+    is_deeply( $creq->body_parameters, $parameters,
+               'Catalyst::Request body_parameters' );
+    unshift( @{ $parameters->{a} }, 1, 2, 3 );
     is_deeply( $creq->parameters, $parameters,
         'Catalyst::Request parameters' );
     is_deeply( $creq->arguments, [qw(a b)], 'Catalyst::Request arguments' );

diff --git a/t/lib/TestApp/Controller/BodyParams.pm b/t/lib/TestApp/Controller/BodyParams.pm
new file mode 100644 (file)
index 0000000..5732211
--- /dev/null
+++ b/t/lib/TestApp/Controller/BodyParams.pm
@@ -0,0 +1,13 @@
+package TestApp::Controller::BodyParams;
+
+use strict;
+use base 'Catalyst::Controller';
+
+sub default : Private {
+    my ( $self, $c ) = @_;
+    $c->req->body_params({override => 'that'});
+    $c->res->output($c->req->body_params->{override});
+    $c->res->status(200);
+}
+
+1;

diff --git a/t/live_catalyst_test.t b/t/live_catalyst_test.t
index 9fb299f..e7f8df9 100644 (file)
--- a/t/live_catalyst_test.t
+++ b/t/live_catalyst_test.t
@@ -5,6 +5,7 @@ use FindBin;
 use lib "$FindBin::Bin/lib";
 use Catalyst::Test 'TestApp', {default_host => 'default.com'};
 use Catalyst::Request;
+use HTTP::Request::Common;
 
 use Test::More;
 
@@ -44,5 +45,10 @@ my $req = '/dump/request';
     is( $creq->uri->host, $opts{host}, 'target host is mutable via options hashref' );
 }
 
+{
+       my $response = request( POST( '/bodyparams', { override => 'this' } ) )->content;
+    is($response, 'that', 'body param overridden');
+}
+
 done_testing;