Merge branch 'master' into psgi
Florian Ragwitz [Sun, 23 Jan 2011 15:42:37 +0000 (15:42 +0000)]
master:
Version 5.80030
Fix a warning with undef bodies
Changelog the Package::Stash bugfix
Changelog the resp->body improvements
Work with PP Package::Stash again now the incorrect assumption that broke with ::XS is fixed
Stop relying on Package::Stash PP bugs
Add conflict
Bad test failed to show bug.
Fix the case for body '0'
Make response body able to be undef to allow RenderView to see 'defined but empty' body, allowing X-Sendfile to work nicer. Also removes horrible modifier code. win/win if it doesn't break anything else.
Extra links in docs
Add test case for uri_for() with #fragment and query params, broken by 5.7008 when uri_for() was reimplemented without URI.pm.
Found a another fault in chained action dispatcher.

Conflicts:
lib/Catalyst/Engine/CGI.pm

13 files changed:
Changes
Makefile.PL
lib/Catalyst.pm
lib/Catalyst/Component.pm
lib/Catalyst/Engine.pm
lib/Catalyst/Response.pm
lib/Catalyst/Runtime.pm
t/aggregate/live__component_controller_action_chained2.t
t/aggregate/live_engine_response_emptybody.t [new file with mode: 0644]
t/aggregate/unit_core_uri_for.t
t/aggregate/unit_response.t
t/lib/ChainedActionsApp/Controller/Root.pm
t/lib/TestApp/Controller/Root.pm

diff --git a/Changes b/Changes
index 537dbf4..9076eb7 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,8 +1,16 @@
 # This file documents the revision history for Perl extension Catalyst.
 
+5.80030 2011-01-04 13:13:02
+
  New features:
   - Add a --proc_title option to the FCGI script to set the process
     title.
+  - Allow the response body to be set to `undef' explicitly to indicate the
+    absence of a body. It can be used to indicate that no body should be sent at
+    all and processing of views should be skipped. This is especially useful for
+    things like X-Sendfile, which now no longer require providing fake response
+    bodies to suppress view processing. In order for this to work, you will also
+    have upgrade Catalyst::Action::RenderView to at least version 0.15.
 
  Bug fixes:
   - Deal correctly with GLOB file handles in the response body (setting
     maximum number of actions). This means that (for example)
     a URI path /foo/* made out of 2 actions will take preference
     to a URI path /*/* made out of 3 actions. Please check your applications
-    if you are using chained action and please write new test to report 
+    if you are using chained action and please write new test to report
     failing case.
+  - Stop relying on bugs in the pure-perl version of Package::Stash. New
+    versions of Package::Stash load Package::Stash::XS if
+    available. Package::Stash::XS fixes some of the bugs of the pure-perl
+    version, exposing our faulty assumption and breaking things. We now work
+    with both old and new versions of Package::Stash, both with and without
+    Package::Stash::XS being installed. Older versions of Catalyst-Runtime also
+    work with both old and new versions of Package::Stash, but only if
+    Package::Stash::XS is *not* installed.
 
  Documentation:
   - Clarify that when forwarding or detaching, the end action associated
index c016f5a..5d0180e 100644 (file)
@@ -129,6 +129,7 @@ my %conflicts = (
     'Catalyst::Plugin::ENV' => '9999', # This plugin is just stupid, full stop
                                        # should have been a core fix.
     'Catalyst::Plugin::Unicode::Encoding' => '0.2',
+    'Catalyst::Plugin::Authentication' => '0.10010', # _config accessor in ::Credential::Password
     'Catalyst::Authentication::Credential::HTTP' => '1.009',
     'Catalyst::Plugin::Session::Store::File' => '0.16',
     'Catalyst::Plugin::Session' => '0.21',
index 173483a..5a34cc2 100644 (file)
@@ -81,7 +81,7 @@ __PACKAGE__->stats_class('Catalyst::Stats');
 
 # Remember to update this in Catalyst::Runtime as well!
 
-our $VERSION = '5.80029';
+our $VERSION = '5.80030';
 
 sub import {
     my ( $class, @arguments ) = @_;
@@ -1862,7 +1862,7 @@ sub finalize_headers {
     }
 
     # Content-Length
-    if ( $response->body && !$response->content_length ) {
+    if ( defined $response->body && length $response->body && !$response->content_length ) {
 
         # get the length from a filehandle
         if ( blessed( $response->body ) && $response->body->can('read') || ref( $response->body ) eq 'GLOB' )
@@ -2376,11 +2376,11 @@ sub prepare_write { my $c = shift; $c->engine->prepare_write( $c, @_ ) }
 
 =head2 $c->request_class
 
-Returns or sets the request class.
+Returns or sets the request class. Defaults to L<Catalyst::Request>.
 
 =head2 $c->response_class
 
-Returns or sets the response class.
+Returns or sets the response class. Defaults to L<Catalyst::Response>.
 
 =head2 $c->read( [$maxlength] )
 
index 28da155..1c61eb2 100644 (file)
@@ -127,7 +127,7 @@ sub config {
         # TODO maybe this should be a ClassData option?
         my $class = blessed($self) || $self;
         my $meta = Class::MOP::get_metaclass_by_name($class);
-        unless ($meta->has_package_symbol('$_config')) {
+        unless (${ $meta->get_or_add_package_symbol('$_config') }) {
             # Call merge_hashes to ensure we deep copy the parent
             # config onto the subclass
             $self->_config( Catalyst::Utils::merge_hashes($config, {}) );
index 52c38cb..407ceb5 100644 (file)
@@ -839,7 +839,7 @@ sub write {
         $self->_prepared_write(1);
     }
 
-    return 0 if !defined $buffer;
+    $buffer = q[] unless defined $buffer;
 
     my $len = length($buffer);
     $self->_writer->write($buffer);
index f268aef..9c8a4b2 100644 (file)
@@ -6,14 +6,8 @@ use HTTP::Headers;
 with 'MooseX::Emulate::Class::Accessor::Fast';
 
 has cookies   => (is => 'rw', default => sub { {} });
-has body      => (is => 'rw', default => '', lazy => 1, predicate => 'has_body',
-    clearer => '_clear_body'
-);
-after 'body' => sub { # If someone assigned undef, clear the body so we get ''
-    if (scalar(@_) == 2 && !defined($_[1])) {
-         $_[0]->_clear_body;
-    }
-};
+has body      => (is => 'rw', default => undef, lazy => 1, predicate => 'has_body');
+
 has location  => (is => 'rw');
 has status    => (is => 'rw', default => 200);
 has finalized_headers => (is => 'rw', default => 0);
index 4ea2826..dc1c673 100644 (file)
@@ -7,7 +7,7 @@ BEGIN { require 5.008004; }
 
 # Remember to update this in Catalyst as well!
 
-our $VERSION = '5.80029';
+our $VERSION = '5.80030';
 
 =head1 NAME
 
index 199db81..84d54ce 100644 (file)
@@ -16,6 +16,11 @@ content_like('/account/ferz', qr/This is account ferz/, 'account');
 content_like('/account/123', qr/This is account 123/, 'account');
 content_like('/account/profile/007/James Bond', qr/This is profile of James Bond/, 'account');
 
+TODO: {
+      local $TODO = q(new chained action test case that fails yet.);
+      content_like('/downloads/', qr/This is downloads index/, 'downloads');
+}
+
 action_notfound('/c');
 
 done_testing;
diff --git a/t/aggregate/live_engine_response_emptybody.t b/t/aggregate/live_engine_response_emptybody.t
new file mode 100644 (file)
index 0000000..825b48a
--- /dev/null
@@ -0,0 +1,27 @@
+#!perl
+
+use strict;
+use warnings;
+
+use FindBin;
+use lib "$FindBin::Bin/../lib";
+
+use Test::More;
+use Catalyst::Test 'TestApp';
+
+# body '0'
+{
+    my $res = request('/zerobody');
+    is $res->content, '0';
+    is $res->header('Content-Length'), '1';
+}
+
+# body ''
+{
+    my $res = request('/emptybody');
+    is $res->content, '';
+    ok !defined $res->header('Content-Length');
+}
+
+done_testing;
+
index 48003d3..dad5a1c 100644 (file)
@@ -58,6 +58,15 @@ is(
     'Plus is not encoded'
 );
 
+TODO: {
+    local $TODO = 'broken by 5.7008';
+    is(
+        Catalyst::uri_for( $context, '/bar#fragment', { param1 => 'value1' } )->as_string,
+        'http://127.0.0.1/foo/bar?param1=value1#fragment',
+        'URI for path with fragment and query params'
+    );
+}
+
 # test with utf-8
 is(
     Catalyst::uri_for( $context, 'quux', { param1 => "\x{2620}" } )->as_string,
index 31e397a..2380439 100644 (file)
@@ -1,8 +1,9 @@
 use strict;
 use warnings;
-use Test::More tests => 6;
+use Test::More;
 
 use_ok('Catalyst::Response');
+use_ok('Catalyst::Engine');
 
 my $res = Catalyst::Response->new;
 
@@ -12,7 +13,5 @@ is($res->code, 500, 'code sets itself');
 is($res->status, 500, 'code sets status');
 $res->status(501);
 is($res->code, 501, 'status sets code');
-is($res->body, '', "default response body ''");
-$res->body(undef);
-is($res->body, '', "response body '' after assigned undef");
 
+done_testing;
index a436087..07d0d2e 100644 (file)
@@ -60,6 +60,22 @@ sub profile : Chained('profile_base') PathPart('') Args(1) {
     $c->response->body( "This is profile of " . $acc );
 }
 
+=head2 downloads
+
+    This is a different test, this function is void, just to let following in the chain
+    to declare downloads as PathPart.
+
+=cut
+
+sub downloads : Chained('setup') PathPart('') CaptureArgs(0) {
+    my($self,$c) = @_;
+}
+
+sub downloads_index : Chained('downloads') PathPart('downloads') Args(0) {
+    my($self,$c) = @_;
+    $c->response->body( "This is download index");
+}
+
 sub default : Chained('setup') PathPart('') Args() {
     my ( $self, $c ) = @_;
     $c->response->body( 'Page not found' );
index 18c6db8..e34eacd 100644 (file)
@@ -14,6 +14,16 @@ sub zero : Path('0') {
     $c->forward('TestApp::View::Dump::Request');
 }
 
+sub zerobody : Local {
+    my ($self, $c) = @_;
+    $c->res->body('0');
+}
+
+sub emptybody : Local {
+    my ($self, $c) = @_;
+    $c->res->body('');
+}
+
 sub localregex : LocalRegex('^localregex$') {
     my ( $self, $c ) = @_;
     $c->res->header( 'X-Test-Class' => ref($self) );