From: John Napiorkowski Date: Wed, 22 Jul 2015 17:22:10 +0000 (-0500) Subject: Merge branch 'fix/RouteMatching.pod' of https://github.com/dim0xff/catalyst-runtime... X-Git-Tag: 5.90094~4^2~5^2 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=catagits%2FCatalyst-Runtime.git;a=commitdiff_plain;h=26abb70733120828ebd6026e3ce8907385a53608;hp=6a226ee3384511ecfe42baf99ed67f9b90469b70 Merge branch 'fix/RouteMatching.pod' of https://github.com/dim0xff/catalyst-runtime into dim0xff-fix/RouteMatching.pod --- diff --git a/Changes b/Changes index fe57228..d842512 100644 --- a/Changes +++ b/Changes @@ -1,11 +1,23 @@ # This file documents the revision history for Perl extension Catalyst. -5.90091 - 2014-05-08 +5.90093 - 2015-05-29 + - Fixed a bug where if you used $res->write and then $res->body, the + contents of body would be double encoded (gshank++). + +5.90092 - 2015-05-19 + - Allows you to use a namespace suffix for request, response and stats + class traits. Docs and tests for this. + - Refactor the change introduced in 5.90091 to solve reported issues (for + example Catalyst::Controller::DBIC::API fails its tests) and to be a more + conservative refactor (new code more closely resembles the orginal code + that has proven to work for years.) + +5.90091 - 2015-05-08 - Fixed a bug where if an injected component expanded sub components, those sub components would not show up in the startup debug dev console ( even though they were actually created). -5.90090 - 2014-04-29 +5.90090 - 2015-04-29 - Updated some documention in Catalyst::Request::Upload to clarify behavior that RT ticket reported as confusing or unexpected - Merged all changes from 5.90089_XXX development cycle. diff --git a/lib/Catalyst.pm b/lib/Catalyst.pm index 016ef2d..0148bd2 100644 --- a/lib/Catalyst.pm +++ b/lib/Catalyst.pm @@ -82,8 +82,18 @@ sub _build_request_constructor_args { sub composed_request_class { my $class = shift; my @traits = (@{$class->request_class_traits||[]}, @{$class->config->{request_class_traits}||[]}); + + # For each trait listed, figure out what the namespace is. First we try the $trait + # as it is in the config. Then try $MyApp::TraitFor::Request:$trait. Last we try + # Catalyst::TraitFor::Request::$trait. If none load, throw error. + + my $trait_ns = 'TraitFor::Request'; + my @normalized_traits = map { + Class::Load::load_first_existing_class($_, $class.'::'.$trait_ns.'::'. $_, 'Catalyst::'.$trait_ns.'::'.$_) + } @traits; + return $class->_composed_request_class || - $class->_composed_request_class(Moose::Util::with_traits($class->request_class, @traits)); + $class->_composed_request_class(Moose::Util::with_traits($class->request_class, @normalized_traits)); } has response => ( @@ -106,8 +116,14 @@ sub _build_response_constructor_args { sub composed_response_class { my $class = shift; my @traits = (@{$class->response_class_traits||[]}, @{$class->config->{response_class_traits}||[]}); + + my $trait_ns = 'TraitFor::Response'; + my @normalized_traits = map { + Class::Load::load_first_existing_class($_, $class.'::'.$trait_ns.'::'. $_, 'Catalyst::'.$trait_ns.'::'.$_) + } @traits; + return $class->_composed_response_class || - $class->_composed_response_class(Moose::Util::with_traits($class->response_class, @traits)); + $class->_composed_response_class(Moose::Util::with_traits($class->response_class, @normalized_traits)); } has namespace => (is => 'rw'); @@ -132,7 +148,8 @@ our $RECURSION = 1000; our $DETACH = Catalyst::Exception::Detach->new; our $GO = Catalyst::Exception::Go->new; -#I imagine that very few of these really need to be class variables. if any. +#I imagine that very few of these really +#need to be class variables. if any. #maybe we should just make them attributes with a default? __PACKAGE__->mk_classdata($_) for qw/components arguments dispatcher engine log dispatcher_class @@ -150,14 +167,20 @@ __PACKAGE__->stats_class('Catalyst::Stats'); sub composed_stats_class { my $class = shift; my @traits = (@{$class->stats_class_traits||[]}, @{$class->config->{stats_class_traits}||[]}); + + my $trait_ns = 'TraitFor::Stats'; + my @normalized_traits = map { + Class::Load::load_first_existing_class($_, $class.'::'.$trait_ns.'::'. $_, 'Catalyst::'.$trait_ns.'::'.$_) + } @traits; + return $class->_composed_stats_class || - $class->_composed_stats_class(Moose::Util::with_traits($class->stats_class, @traits)); + $class->_composed_stats_class(Moose::Util::with_traits($class->stats_class, @normalized_traits)); } __PACKAGE__->_encode_check(Encode::FB_CROAK | Encode::LEAVE_SRC); # Remember to update this in Catalyst::Runtime as well! -our $VERSION = '5.90091'; +our $VERSION = '5.90093'; $VERSION = eval $VERSION if $VERSION =~ /_/; # numify for warning-free dev releases sub import { @@ -2205,9 +2228,10 @@ sub finalize_encoding { # Set the charset if necessary. This might be a bit bonkers since encodable response # is false when the set charset is not the same as the encoding mimetype (maybe # confusing action at a distance here.. - # Don't try to set the charset if one already exists + # Don't try to set the charset if one already exists or if headers are already finalized $c->res->content_type($c->res->content_type . "; charset=" . $c->encoding->mime_name) - unless($c->res->content_type_charset); + unless($c->res->content_type_charset || + ($c->res->_context && $c->res->finalized_headers && !$c->res->_has_response_cb)); } } @@ -2721,8 +2745,27 @@ Returns or sets the request class. Defaults to L. =head2 $app->request_class_traits -An arrayref of Ls which are applied to the request class. +An arrayref of Ls which are applied to the request class. You can +name the full namespace of the role, or a namespace suffix, which will then +be tried against the following standard namespace prefixes. + $MyApp::TraitFor::Request::$trait_suffix + Catalyst::TraitFor::Request::$trait_suffix + +So for example if you set: + + MyApp->request_class_traits(['Foo']); + +We try each possible role in turn (and throw an error if none load) + + Foo + MyApp::TraitFor::Request::Foo + Catalyst::TraitFor::Request::Foo + +The namespace part 'TraitFor::Request' was choosen to assist in backwards +compatibility with L which previously provided +these features in a stand alone package. + =head2 $app->composed_request_class This is the request class which has been composed with any request_class_traits. @@ -2733,7 +2776,27 @@ Returns or sets the response class. Defaults to L. =head2 $app->response_class_traits -An arrayref of Ls which are applied to the response class. +An arrayref of Ls which are applied to the response class. You can +name the full namespace of the role, or a namespace suffix, which will then +be tried against the following standard namespace prefixes. + + $MyApp::TraitFor::Response::$trait_suffix + Catalyst::TraitFor::Response::$trait_suffix + +So for example if you set: + + MyApp->response_class_traits(['Foo']); + +We try each possible role in turn (and throw an error if none load) + + Foo + MyApp::TraitFor::Response::Foo + Catalyst::TraitFor::Responset::Foo + +The namespace part 'TraitFor::Response' was choosen to assist in backwards +compatibility with L which previously provided +these features in a stand alone package. + =head2 $app->composed_response_class @@ -2854,10 +2917,10 @@ sub setup_components { # of named components in the configuration that are not actually existing (not a # real file). - $class->setup_injected_components; + my @injected = $class->setup_injected_components; # All components are registered, now we need to 'init' them. - foreach my $component_name (keys %{$class->components||+{}}) { + foreach my $component_name (@comps, @injected) { $class->components->{$component_name} = $class->components->{$component_name}->() if (ref($class->components->{$component_name}) || '') eq 'CODE'; } @@ -2878,6 +2941,9 @@ sub setup_injected_components { $injected_comp_name, $class->config->{inject_components}->{$injected_comp_name}); } + + return map { $class ."::" . $_ } + @injected_components; } =head2 $app->setup_injected_component( $injected_component_name, $config ) @@ -3898,7 +3964,26 @@ A arrayref of Ls that are applied to the stats_class before creatin =head2 $app->composed_stats_class -this is the stats_class composed with any 'stats_class_traits'. +this is the stats_class composed with any 'stats_class_traits'. You can +name the full namespace of the role, or a namespace suffix, which will then +be tried against the following standard namespace prefixes. + + $MyApp::TraitFor::Stats::$trait_suffix + Catalyst::TraitFor::Stats::$trait_suffix + +So for example if you set: + + MyApp->stats_class_traits(['Foo']); + +We try each possible role in turn (and throw an error if none load) + + Foo + MyApp::TraitFor::Stats::Foo + Catalyst::TraitFor::Stats::Foo + +The namespace part 'TraitFor::Stats' was choosen to assist in backwards +compatibility with L which previously provided +these features in a stand alone package. =head2 $c->use_stats diff --git a/lib/Catalyst/Engine.pm b/lib/Catalyst/Engine.pm index e4deb9e..5e87e9f 100644 --- a/lib/Catalyst/Engine.pm +++ b/lib/Catalyst/Engine.pm @@ -159,11 +159,11 @@ sub finalize_body { } else { - # Case where body was set afgter calling ->write. We'd prefer not to + # Case where body was set after calling ->write. We'd prefer not to # support this, but I can see some use cases with the way most of the - # views work. - - $self->write($c, $body ); + # views work. Since body has already been encoded, we need to do + # an 'unencoded_write' here. + $self->unencoded_write( $c, $body ); } } @@ -700,6 +700,20 @@ sub write { $c->response->write($buffer); } +=head2 $self->unencoded_write($c, $buffer) + +Writes the buffer to the client without encoding. Necessary for +already encoded buffers. Used when a $c->write has been done +followed by $c->res->body. + +=cut + +sub unencoded_write { + my ( $self, $c, $buffer ) = @_; + + $c->response->unencoded_write($buffer); +} + =head2 $self->read($c, [$maxlength]) Reads from the input stream by calling C<< $self->read_chunk >>. diff --git a/lib/Catalyst/Response.pm b/lib/Catalyst/Response.pm index eb99ab7..fa15afb 100644 --- a/lib/Catalyst/Response.pm +++ b/lib/Catalyst/Response.pm @@ -134,6 +134,20 @@ sub write { return $len; } +sub unencoded_write { + my ( $self, $buffer ) = @_; + + # Finalize headers if someone manually writes output + $self->_context->finalize_headers unless $self->finalized_headers; + + $buffer = q[] unless defined $buffer; + + my $len = length($buffer); + $self->_writer->write($buffer); + + return $len; +} + sub finalize_headers { my ($self) = @_; return; diff --git a/lib/Catalyst/Runtime.pm b/lib/Catalyst/Runtime.pm index b620293..8332312 100644 --- a/lib/Catalyst/Runtime.pm +++ b/lib/Catalyst/Runtime.pm @@ -7,7 +7,7 @@ BEGIN { require 5.008003; } # Remember to update this in Catalyst as well! -our $VERSION = '5.90091'; +our $VERSION = '5.90093'; $VERSION = eval $VERSION if $VERSION =~ /_/; # numify for warning-free dev releases =head1 NAME diff --git a/lib/Catalyst/UTF8.pod b/lib/Catalyst/UTF8.pod index eefb181..0f20099 100644 --- a/lib/Catalyst/UTF8.pod +++ b/lib/Catalyst/UTF8.pod @@ -462,6 +462,11 @@ B If you try to change the encoding after you start the stream, this will response. However since you've already started streaming this will not show up as an HTTP error status code, but rather error information in your body response and an error in your logs. +B If you use ->body AFTER using ->write (for example you may do this to write your HTML +HEAD information as fast as possible) we expect the contents to body to be encoded as it +normally would be if you never called ->write. In general unless you are doing weird custom +stuff with encoding this is likely to just already do the correct thing. + The second way to stream a response is to get the response writer object and invoke methods on that directly: diff --git a/t/class_traits.t b/t/class_traits.t index f796323..8c65b38 100644 --- a/t/class_traits.t +++ b/t/class_traits.t @@ -9,6 +9,26 @@ BEGIN { sub a { 'a' } sub b { 'b' } + + package Catalyst::TraitFor::Request::Foo; + use Moose::Role; + + sub c { 'c' } + + package TestApp::TraitFor::Request::Bar; + use Moose::Role; + + sub d { 'd' } + + package Catalyst::TraitFor::Response::Foo; + use Moose::Role; + + sub c { 'c' } + + package TestApp::TraitFor::Response::Bar; + use Moose::Role; + + sub d { 'd' } } { @@ -16,8 +36,8 @@ BEGIN { use Catalyst; - __PACKAGE__->request_class_traits([qw/TestRole/]); - __PACKAGE__->response_class_traits([qw/TestRole/]); + __PACKAGE__->request_class_traits([qw/TestRole Foo Bar/]); + __PACKAGE__->response_class_traits([qw/TestRole Foo Bar/]); __PACKAGE__->stats_class_traits([qw/TestRole/]); __PACKAGE__->setup; @@ -38,7 +58,11 @@ my ($res, $c) = ctx_request '/'; is $c->req->a, 'a'; is $c->req->b, 'b'; +is $c->req->c, 'c'; +is $c->req->d, 'd'; is $c->res->a, 'a'; is $c->res->b, 'b'; +is $c->res->c, 'c'; +is $c->res->d, 'd'; done_testing; diff --git a/t/inject_component_util.t b/t/inject_component_util.t index 5b9c2d0..3ac9dcc 100644 --- a/t/inject_component_util.t +++ b/t/inject_component_util.t @@ -1,60 +1,59 @@ use strict; use warnings; use Test::More; -use Catalyst::Utils; use FindBin; use lib "$FindBin::Bin/lib"; BEGIN { -package RoleTest1; + package RoleTest1; + use Moose::Role; -use Moose::Role; + sub aaa { 'aaa' } -sub aaa { 'aaa' } + $INC{'RoleTest1.pm'} = __FILE__; -package RoleTest2; + package RoleTest2; + use Moose::Role; -use Moose::Role; + sub bbb { 'bbb' } -sub bbb { 'bbb' } + $INC{'RoleTest2.pm'} = __FILE__; -package Model::Banana; - -use base qw/Catalyst::Model/; + package Model::Banana; + use base qw/Catalyst::Model/; -package Model::BananaMoose; - -use Moose; -extends 'Catalyst::Model'; + $INC{'Model/Banana.pm'} = __FILE__; -Model::BananaMoose->meta->make_immutable; + package Model::BananaMoose; -package TestCatalyst; $INC{'TestCatalyst.pm'} = 1; - -use Catalyst::Runtime '5.70'; - -use Moose; -BEGIN { extends qw/Catalyst/ } - -use Catalyst; - -after 'setup_components' => sub { - my $self = shift; - Catalyst::Utils::inject_component( into => __PACKAGE__, component => 'Model::Banana' ); - Catalyst::Utils::inject_component( into => __PACKAGE__, component => 'Test::Apple' ); - Catalyst::Utils::inject_component( into => __PACKAGE__, component => 'Model::Banana', as => 'Cherry' ); - Catalyst::Utils::inject_component( into => __PACKAGE__, component => 'Model::BananaMoose', as => 'CherryMoose', traits => ['RoleTest1', 'RoleTest2'] ); - Catalyst::Utils::inject_component( into => __PACKAGE__, component => 'Test::Apple', as => 'Apple' ); - Catalyst::Utils::inject_component( into => __PACKAGE__, component => 'Test::Apple', as => 'Apple2', traits => ['RoleTest1', 'RoleTest2'] ); -}; - -TestCatalyst->config( 'home' => '.' ); - -TestCatalyst->setup; - + use Moose; + extends 'Catalyst::Model'; + + Model::BananaMoose->meta->make_immutable; + $INC{'Model/BananaMoose.pm'} = __FILE__; +} + +{ + package TestCatalyst; + $INC{'TestCatalyst.pm'} = __FILE__; + + use Moose; + use Catalyst; + use Catalyst::Utils; + + after 'setup_components' => sub { + my $self = shift; + Catalyst::Utils::inject_component( into => __PACKAGE__, component => 'Model::Banana' ); + Catalyst::Utils::inject_component( into => __PACKAGE__, component => 'Test::Apple' ); + Catalyst::Utils::inject_component( into => __PACKAGE__, component => 'Model::Banana', as => 'Cherry' ); + Catalyst::Utils::inject_component( into => __PACKAGE__, component => 'Model::BananaMoose', as => 'CherryMoose', traits => ['RoleTest1', 'RoleTest2'] ); + Catalyst::Utils::inject_component( into => __PACKAGE__, component => 'Test::Apple', as => 'Apple' ); + Catalyst::Utils::inject_component( into => __PACKAGE__, component => 'Test::Apple', as => 'Apple2', traits => ['RoleTest1', 'RoleTest2'] ); + }; + + TestCatalyst->config( 'home' => '.' ); + TestCatalyst->setup; } - -package main; use Catalyst::Test qw/TestCatalyst/; diff --git a/t/utf_incoming.t b/t/utf_incoming.t index 6342025..ff61a6b 100644 --- a/t/utf_incoming.t +++ b/t/utf_incoming.t @@ -111,6 +111,14 @@ use Scalar::Util (); $c->response->body($contents); } + sub write_then_body :Local { + my ($self, $c) = @_; + + $c->res->content_type('text/html'); + $c->res->write("

This is early_write action ♥

"); + $c->res->body("

This is body_write action ♥

"); + } + sub file_upload :POST Consumes(Multipart) Local { my ($self, $c) = @_; Test::More::is $c->req->body_parameters->{'♥'}, '♥♥'; @@ -365,6 +373,14 @@ use Catalyst::Test 'MyApp'; } { + my $res = request "/root/write_then_body"; + + is $res->code, 200, 'OK'; + is decode_utf8($res->content), "

This is early_write action ♥

This is body_write action ♥

"; + is $res->content_charset, 'UTF-8'; +} + +{ ok my $path = File::Spec->catfile('t', 'utf8.txt'); ok my $req = POST '/root/file_upload', Content_Type => 'form-data',