From: André Walker Date: Mon, 2 Apr 2012 13:12:25 +0000 (-0300) Subject: Merge branch 'master' into gsoc_breadboard X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=catagits%2FCatalyst-Runtime.git;a=commitdiff_plain;h=bcb87e823493966b483e008df263b80adcedc776;hp=345358ea4f3fa844dcd691a424014be10a6145bd Merge branch 'master' into gsoc_breadboard --- diff --git a/Changes b/Changes index 5135713..c13291d 100644 --- 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 --- 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 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 --- 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 --- 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 --- 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 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 --- 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;