Merge remote-tracking branch 'origin/master' into ancona
Mark Ellis [Wed, 7 May 2014 17:46:53 +0000 (18:46 +0100)]
12 files changed:
Changes
lib/Catalyst.pm [changed mode: 0644->0755]
lib/Catalyst/Delta.pod
lib/Catalyst/Runtime.pm
lib/Catalyst/Utils.pm
t/aggregate/live_engine_response_headers.t
t/author/spelling.t
t/head_middleware.t [new file with mode: 0644]
t/http_exceptions.t
t/lib/TestLogger.pm
t/psgi-log.t
t/psgi_utils.t

diff --git a/Changes b/Changes
index cf79bd4..fa58fa2 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,5 +1,33 @@
 # This file documents the revision history for Perl extension Catalyst.
 
+  - Fix spelling, grammar and structural errors in POD
+  - Remove redundant ->setup call in t/head_middleware.t RT#95361
+  - Fix test failures when running under CATALYST_DEBUG. RT#95358
+
+5.90064 - 2014-05-05
+  - Fix for mindless broken tests on Win32 (Haarg++).
+  - Happy Cinco de Mayo!
+
+5.90063 - 2014-05-01
+  - 'end' and other special actions won't catch HTTP style exceptions anymore.
+  - Fix bug where Catalyst did not properly detect the terminal width when in
+    debug mode and thus making the debug output narrow and hard to read.
+  - Documentation corrections for Util methods around localized PSGI $env.
+  - Improvements to auto detection of terminal width.
+  - Updating deprecation list to include Class::Load and ensure_class_loaded
+  - Added a few docs around middleware and corrected the order that middleware
+    is loaded when registering it via ->setup_middleware instead of via
+    configuration.
+  - Added a test case to make sure default middleware order is correct.
+s
+5.90062 - 2014-04-14
+  - HTTP::Exception objects were not properly bubbled up to middleware since
+    there was some code in Catalyst that was triggering stringification.
+
+5.90061 - 2014-03-10
+  - Reverted a change related to how plugins get initialized that was
+    introduced by a change in December.
+
 5.90060 - 2014-02-07
   - Same as 5.90059_006, just marking it as stable, no functional changes.
 
old mode 100644 (file)
new mode 100755 (executable)
index 8173fb0..48f4949
@@ -126,7 +126,7 @@ __PACKAGE__->stats_class('Catalyst::Stats');
 
 # Remember to update this in Catalyst::Runtime as well!
 
-our $VERSION = '5.90060';
+our $VERSION = '5.90064';
 
 sub import {
     my ( $class, @arguments ) = @_;
@@ -1133,15 +1133,6 @@ sub setup {
     $class->setup_log( delete $flags->{log} );
     $class->setup_plugins( delete $flags->{plugins} );
 
-    # Call plugins setup, this is stupid and evil.
-    # Also screws C3 badly on 5.10, hack to avoid.
-    {
-        no warnings qw/redefine/;
-        local *setup = sub { };
-        $class->setup unless $Catalyst::__AM_RESTARTING;
-    }
-
-    $class->setup_middleware();
     $class->setup_data_handlers();
     $class->setup_dispatcher( delete $flags->{dispatcher} );
     if (my $engine = delete $flags->{engine}) {
@@ -1174,6 +1165,16 @@ You are running an old script!
 EOF
     }
 
+    # Call plugins setup, this is stupid and evil.
+    # Also screws C3 badly on 5.10, hack to avoid.
+    {
+        no warnings qw/redefine/;
+        local *setup = sub { };
+        $class->setup unless $Catalyst::__AM_RESTARTING;
+    }
+
+    $class->setup_middleware();
+
     # Initialize our data structure
     $class->components( {} );
 
@@ -1737,6 +1738,10 @@ sub execute {
     my $last = pop( @{ $c->stack } );
 
     if ( my $error = $@ ) {
+        #rethow if this can be handled by middleware
+        if(blessed $error && ($error->can('as_psgi') || $error->can('code'))) {
+            $error->can('rethrow') ? $error->rethrow : croak $error;
+        }
         if ( blessed($error) and $error->isa('Catalyst::Exception::Detach') ) {
             $error->rethrow if $c->depth > 1;
         }
@@ -2014,12 +2019,12 @@ sub handle_request {
         $c->dispatch;
         $status = $c->finalize;
     } catch {
-        chomp(my $error = $_);
-        $class->log->error(qq/Caught exception in engine "$error"/);
         #rethow if this can be handled by middleware
-        if(blessed $error && ($error->can('as_psgi') || $error->can('code'))) {
-            $error->can('rethrow') ? $error->rethrow : croak $error;
+        if(blessed $_ && ($_->can('as_psgi') || $_->can('code'))) {
+            $_->can('rethrow') ? $_->rethrow : croak $_;
         }
+        chomp(my $error = $_);
+        $class->log->error(qq/Caught exception in engine "$error"/);
     };
 
     $COUNT++;
@@ -3102,6 +3107,10 @@ which sounds odd but is likely how you expect it to work if you have prior
 experience with L<Plack::Builder> or if you previously used the plugin
 L<Catalyst::Plugin::EnableMiddleware> (which is now considered deprecated)
 
+So basically your middleware handles an incoming request from the first
+registered middleware, down and handles the response from the last middleware
+up.
+
 =cut
 
 sub registered_middlewares {
@@ -3123,7 +3132,7 @@ sub registered_middlewares {
 sub setup_middleware {
     my $class = shift;
     my @middleware_definitions = @_ ? 
-      @_ : reverse(@{$class->config->{'psgi_middleware'}||[]});
+      reverse(@_) : reverse(@{$class->config->{'psgi_middleware'}||[]});
 
     my @middleware = ();
     while(my $next = shift(@middleware_definitions)) {
@@ -3578,7 +3587,7 @@ example given above, which uses L<JSON::Maybe> to provide either L<JSON::PP>
 it installed (if you want the faster XS parser, add it to you project Makefile.PL
 or dist.ini, cpanfile, etc.)
 
-The C<data_handlers> configuation is a hashref whose keys are HTTP Content-Types
+The C<data_handlers> configuration is a hashref whose keys are HTTP Content-Types
 (matched against the incoming request type using a regexp such as to be case
 insensitive) and whose values are coderefs that receive a localized version of
 C<$_> which is a filehandle object pointing to received body.
index b7fcb37..398649f 100755 (executable)
@@ -9,6 +9,15 @@ Catalyst releases.
 
 =head2 VERSION 5.90060+
 
+=head3 Deprecate Catalyst::Utils::ensure_class_loaded
+
+Going forward we recommend you use L<Module::Runtime>.  In fact we will
+be converting all uses of L<Class::Load> to L<Module::Runtime>.  We will
+also convert L<Catalyst::Utils\ensure_class_loaded> to be based on
+L<Module::Runtime> to allow some time for you to update code, however at
+some future point this method will be removed so you should stop
+using it now.
+
 =head3 Support passing Body filehandles directly to your Plack server.
 
 We changed the way we return body content (from response) to whatever
@@ -17,7 +26,7 @@ always use the streaming interface for the cases when the body is a
 simple scalar, object or filehandle like.  In those cases we now just
 pass the simple response on to the plack handler.  This might lead to
 some minor differences in how streaming is handled.  For example, you
-might notice that streaming starts properly supportubg chunked encoding when
+might notice that streaming starts properly supporting chunked encoding when
 on a server that supports that, or that previously missing headers
 (possible content-length) might appear suddenly correct.  Also, if you
 are using middleware like L<Plack::Middleware::XSendfile> and are using
@@ -36,7 +45,7 @@ using one server but deploying using a different one, differences in
 what those server do with streaming should be noted.
 
 Please see note below about changes to filehandle support and existing
-Plack middleware to aid in back compatibility.
+Plack middleware to aid in backwards compatibility.
 
 =head3 Distinguish between body null versus undef.
 
@@ -59,7 +68,7 @@ so we benefit from better collaboration with developers outside Catalyst, 3)
 In the future you'll be able to change or trim the middleware stack to get
 additional performance when you don't need all the checks and constraints.
 
-=head3 Deprecation of Filehandle like objects that do read but not getline
+=head3 Deprecate Filehandle like objects that do read but not getline
 
 We also deprecated setting the response body to an object that does 'read'
 but not 'getline'.  If you are using a custom IO-Handle like object for
@@ -70,31 +79,31 @@ will issue a warning.  You also may run into this issue with L<MogileFS::Client>
 which does read but not getline.  For now we will just warn when encountering
 such an object and fallback to the previous behavior (where L<Catalyst::Engine>
 itself unrolls the filehandle and performs blocking streams).  However
-this backcompat will be removed in an upcoming release so you should either
+this backwards compatibility will be removed in an upcoming release so you should either
 rewrite your custom filehandle objects to support getline or start using the 
 middleware that adapts read for getline L<Plack::Middleware::AdaptFilehandleRead>.
 
-=head3 Response->headers become readonly after finalizing
+=head3 Response->headers become read-only after finalizing
 
 Once the response headers are finalized, trying to change them is not allowed
 (in the past you could change them and this would lead to unexpected results).
 
-=head3 Offically deprecation of L<Catalyst::Engine::PSGI>
+=head3 Officially deprecate L<Catalyst::Engine::PSGI>
 
 L<Catalyst::Engine::PSGI> is also officially no longer supported.  We will
-no long run test cases against this and can remove backcompat code for it
-as deemed necessary for the evolution of the platform.  You should simple
-discontinue use of this engine, as L<Catalyst> has been PSGI at the core
-for several years.
+no long run test cases against this and can remove backwards compatibility code for it
+as deemed necessary for the evolution of the platform.  You should simply
+discontinue use of this engine, as L<Catalyst> has been PSGI at the core for
+several years.
 
-=head2 Officially deprecate finding the PSGI $env anyplace other than Request
+=head3 Officially deprecate finding the PSGI $env anyplace other than Request
 
 A few early releases of Cataplack had the PSGI $env in L<Catalyst::Engine>.
-Code has been maintained here for backcompat reasons.  This is no longer
-supported and will be removed in upcoming release, so you should update
+Code has been maintained here for backwards compatibility reasons.  This is no
+longer supported and will be removed in upcoming release, so you should update
 your code and / or upgrade to a newer version of L<Catalyst>
 
-=head2 Deprecate setting Response->body after using write/write_fh
+=head3 Deprecate setting Response->body after using write/write_fh
 
 Setting $c->res->body to a filehandle after using $c->res->write or
 $c->res->write_fh is no longer considered allowed, since we can't send
@@ -232,7 +241,7 @@ Below is a brief list of features which have been deprecated in this release:
 
 =item MyApp->plugin method is deprecated, use L<Catalyst::Model::Adaptor> instead.
 
-=item __PACKAGE__->mk_accessors() is supported for backward compatibility only, use Moose attributes instead in new code.
+=item __PACKAGE__->mk_accessors() is supported for backwards compatibility only, use Moose attributes instead in new code.
 
 =item Use of Catalyst::Base now warns
 
index 75a6279..ea41e00 100644 (file)
@@ -7,7 +7,7 @@ BEGIN { require 5.008003; }
 
 # Remember to update this in Catalyst as well!
 
-our $VERSION = '5.90060';
+our $VERSION = '5.90064';
 
 =head1 NAME
 
index 3f594e6..78c293a 100644 (file)
@@ -393,17 +393,24 @@ sub term_width {
 
     my $width;
     eval '
-        require Term::Size::Any;
-        my ($columns, $rows) = Term::Size::Any::chars;
-        $width = $columns;
-        1;
+      use Term::Size::Any;
+      ($width) = Term::Size::Any::chars;
+      1;
     ' or do {
+          if($@ =~m[Can't locate Term/Size/Any.pm]) {
+            warn "Term::Size::Any is not installed, can't autodetect terminal column width\n";
+          } else {
+            warn "There was an error trying to detect your terminal size: $@\n";
+          }
+        warn 'Trouble trying to detect your terminal size, looking at $ENV{COLUMNS}'."\n";
         $width = $ENV{COLUMNS}
             if exists($ENV{COLUMNS})
             && $ENV{COLUMNS} =~ m/^\d+$/;
     };
 
-    $width = 80 unless ($width && $width >= 80);
+    do {
+      warn "Cannot determine desired terminal width, using default of 80 columns\n";
+      $width = 80 } unless ($width && $width >= 80);
     return $_term_width = $width;
 }
 
@@ -505,7 +512,7 @@ Localize C<$env> under the current controller path prefix:
       my $env = $c->Catalyst::Utils::env_at_path_prefix;
     }
 
-Assuming you have a requst like GET /user/name:
+Assuming you have a request like GET /user/name:
 
 In the example case C<$env> will have PATH_INFO of '/name' instead of
 '/user/name' and SCRIPT_NAME will now be '/user'.
@@ -530,7 +537,7 @@ sub env_at_path_prefix {
 
 =head2 env_at_action
 
-Localize C<$env> under the current controller path prefix:
+Localize C<$env> under the current action namespace.
 
     package MyApp::Controller::User;
 
@@ -543,11 +550,16 @@ Localize C<$env> under the current controller path prefix:
       my $env = $c->Catalyst::Utils::env_at_action;
     }
 
-Assuming you have a requst like GET /user/name:
+Assuming you have a request like GET /user/name:
 
 In the example case C<$env> will have PATH_INFO of '/' instead of
 '/user/name' and SCRIPT_NAME will now be '/user/name'.
 
+Alternatively, assuming you have a request like GET /user/name/foo:
+
+In this example case C<$env> will have PATH_INFO of '/foo' instead of
+'/user/name/foo' and SCRIPT_NAME will now be '/user/name'.
+
 This is probably a common case where you want to mount a PSGI application
 under an action but let the Args fall through to the PSGI app.
 
@@ -575,7 +587,7 @@ sub env_at_action {
 
 =head2 env_at_request_uri
 
-Localize C<$env> under the current controller path prefix:
+Localize C<$env> under the current request URI:
 
     package MyApp::Controller::User;
 
@@ -588,7 +600,7 @@ Localize C<$env> under the current controller path prefix:
       my $env = $c->Catalyst::Utils::env_at_request_uri
     }
 
-Assuming you have a requst like GET /user/name/hello:
+Assuming you have a request like GET /user/name/hello:
 
 In the example case C<$env> will have PATH_INFO of '/' instead of
 '/user/name' and SCRIPT_NAME will now be '/user/name/hello'.
index a0663e1..6cf9fdc 100644 (file)
@@ -37,8 +37,8 @@ foreach my $method (qw(HEAD GET)) {
             'HEAD method content is empty' );
     }
     elsif ( $method eq 'GET' ) {
-        # method name is echo'd back in content-body, which
-        # accounts for difference in content length.  In normal
+        # method name is echo'd back in content-body (twice under debug),
+        # which accounts for difference in content length.  In normal
         # cases the Content-Length should be the same regardless
         # of whether it's a GET or HEAD request.
         SKIP:
@@ -46,8 +46,10 @@ foreach my $method (qw(HEAD GET)) {
             if ( $ENV{CATALYST_SERVER} ) {
                 skip "Using remote server", 2;
             }
+            my $diff = TestApp->debug ? 2 : 1;
             is( $response->header('Content-Length'),
-                $content_length - 1, 'Response Header Content-Length' );
+                $content_length - $diff, 'Response Header Content-Length' )
+                or diag $response->content;
             is( length($response->content),
                 $response->header('Content-Length'),
                 'GET method content' );
index e98de80..1319a87 100644 (file)
@@ -20,8 +20,9 @@ add_stopwords(qw(
     FastCGI Stringifies Rethrows DispatchType Wishlist Refactor ROADMAP HTTPS Unescapes Restarter Nginx Refactored
     ActionClass LocalRegex LocalRegexp MyAction metadata cometd io psgix websockets
     UTF async codebase dev filenames params MyMiddleware
-    JSON POSTed RESTful configuation performant subref actionrole
+    JSON POSTed RESTful performant subref actionrole
     chunked chunking codewise distingush equivilent plack Javascript
+    ConfigLoader getline
     Andreas
     Ashton
     Axel
@@ -79,6 +80,7 @@ add_stopwords(qw(
     Szilakszi
     Tatsuhiko
     Ulf
+    Upasana
     Vilain
     Viljo
     Wardley
diff --git a/t/head_middleware.t b/t/head_middleware.t
new file mode 100644 (file)
index 0000000..baf560a
--- /dev/null
@@ -0,0 +1,49 @@
+use warnings;
+use strict;
+use Test::More;
+use HTTP::Request::Common;
+use Plack::Test;
+
+# Test to make sure we the order of some middleware is correct.  Basically
+# we want to make sure that if the request is a HEAD we properly remove the
+# body BUT not so quickly that we fail to calculate the length.  This test
+# exists mainly to prevent regressions.
+
+{
+  package MyApp::Controller::Root;
+  $INC{'MyApp/Controller/Root.pm'} = __FILE__;
+
+  use base 'Catalyst::Controller';
+
+  sub test :Local {
+    my ($self, $c) = @_;
+    $c->response->body("This is the body");
+  }
+
+  package MyApp;
+  use Catalyst;
+
+  Test::More::ok(MyApp->setup, 'setup app');
+}
+
+
+
+ok my $psgi = MyApp->psgi_app, 'build psgi app';
+
+test_psgi $psgi, sub {
+    my $cb = shift;
+    my $res = $cb->(GET "/root/test");
+    is $res->code, 200, 'OK';
+    is $res->content, 'This is the body', 'correct body';
+    is $res->content_length, 16, 'correct length';
+};
+
+test_psgi $psgi, sub {
+    my $cb = shift;
+    my $res = $cb->(HEAD "/root/test");
+    is $res->code, 200, 'OK';
+    is $res->content, '', 'correct body';
+    is $res->content_length, 16, 'correct length';    
+};
+
+done_testing;
index c8fae7c..5cb3117 100644 (file)
@@ -12,6 +12,10 @@ use Plack::Test;
 {
   package MyApp::Exception;
 
+  use overload
+    # Use the overloading thet HTTP::Exception uses
+    bool => sub { 1 }, '""' => 'as_string', fallback => 1;
+
   sub new {
     my ($class, $code, $headers, $body) = @_;
     return bless +{res => [$code, $headers, $body]}, $class;
@@ -30,6 +34,8 @@ use Plack::Test;
       $responder->([$code, $headers, $body]);
     };
   }
+
+  sub as_string { 'bad stringy bad' }
   
   package MyApp::Controller::Root;
 
@@ -64,15 +70,19 @@ use Plack::Test;
     die "I'm not dead yet";
   }
 
+  sub end :Private { die "We should never hit end for HTTPExceptions" }
+
   package MyApp;
   use Catalyst;
 
+  MyApp->config(abort_chain_on_error_fix=>1);
+
   sub debug { 1 }
 
   MyApp->setup_log('fatal');
 }
 
-$INC{'MyApp/Controller/Root.pm'} = '1'; # sorry...
+$INC{'MyApp/Controller/Root.pm'} = __FILE__; # sorry...
 MyApp->setup_log('error');
 
 Test::More::ok(MyApp->setup);
@@ -84,6 +94,7 @@ test_psgi $psgi, sub {
     my $res = $cb->(GET "/root/from_psgi_app");
     is $res->code, 404;
     is $res->content, 'Not Found', 'NOT FOUND';
+    unlike $res->content, qr'HTTPExceptions', 'HTTPExceptions';
 };
 
 test_psgi $psgi, sub {
@@ -91,6 +102,7 @@ test_psgi $psgi, sub {
     my $res = $cb->(GET "/root/from_catalyst");
     is $res->code, 403;
     is $res->content, 'Forbidden', 'Forbidden';
+    unlike $res->content, qr'HTTPExceptions', 'HTTPExceptions';
 };
 
 test_psgi $psgi, sub {
@@ -98,6 +110,7 @@ test_psgi $psgi, sub {
     my $res = $cb->(GET "/root/classic_error");
     is $res->code, 500;
     like $res->content, qr'Ex Parrot', 'Ex Parrot';
+    like $res->content, qr'HTTPExceptions', 'HTTPExceptions';
 };
 
 test_psgi $psgi, sub {
@@ -105,6 +118,7 @@ test_psgi $psgi, sub {
     my $res = $cb->(GET "/root/just_die");
     is $res->code, 500;
     like $res->content, qr'not dead yet', 'not dead yet';
+    like $res->content, qr'HTTPExceptions', 'HTTPExceptions';
 };
 
 
@@ -113,5 +127,5 @@ test_psgi $psgi, sub {
 # in the callbacks might never get run (thus all ran tests pass but not all
 # required tests run).
 
-done_testing(10);
+done_testing(14);
 
index f1dc7e6..6c1a26e 100644 (file)
@@ -3,6 +3,7 @@ use strict;
 use warnings;
 
 our @LOGS;
+our @ILOGS;
 our @ELOGS;
 
 sub new {
@@ -14,6 +15,11 @@ sub debug {
     push(@LOGS, shift());
 }
 
+sub info {
+    shift;
+    push(@ILOGS, shift());
+}
+
 sub warn {
     shift;
     push(@ELOGS, shift());
index e010d07..9e269c3 100644 (file)
@@ -46,6 +46,8 @@ use HTTP::Request::Common;
     no Moose;
 }
 
+my $cmp = TestApp->debug ? '>=' : '==';
+
 #subtest "psgi.errors" => sub
 {
 
@@ -69,8 +71,8 @@ use HTTP::Request::Common;
         my $cb = shift;
         my $res = $cb->(GET "/log/debug");
         my @logs = $handle->logs;
-        is(scalar(@logs), 1, "psgi.errors: one event output");
-        like($logs[0], qr/debug$/, "psgi.errors: event matches test data");
+        cmp_ok(scalar(@logs), $cmp, 1, "psgi.errors: one event output");
+        like($logs[0], qr/debug$/m, "psgi.errors: event matches test data");
     };
 };
 
@@ -96,8 +98,9 @@ use HTTP::Request::Common;
     test_psgi $app, sub {
         my $cb = shift;
         my $res = $cb->(GET "/log/debug");
-        is(scalar(@logs), 1, "psgix.logger: one event logged");
-        is_deeply($logs[0], { level => 'debug', message => "debug" }, "psgix.logger: right stuff");
+        cmp_ok(scalar(@logs), $cmp, 1, "psgix.logger: one event logged");
+        is(scalar(grep { $_->{level} eq 'debug' and $_->{message} eq 'debug' } @logs),
+           1, "psgix.logger: right stuff");
     };
 };
 
index bbdb2ad..078dd82 100644 (file)
@@ -3,17 +3,40 @@ use strict;
 
 # Make it easier to mount PSGI apps under catalyst
 
+my $psgi_app = sub {
+  my $req = Plack::Request->new(shift);
+  return [200,[],[$req->path]];
+};
+
 {
-  package MyApp::Controller::User;
+  package MyApp::Controller::Docs;
+  $INC{'MyApp/Controller/Docs.pm'} = __FILE__;
 
   use base 'Catalyst::Controller';
   use Plack::Request;
   use Catalyst::Utils;
 
-  my $psgi_app = sub {
-    my $req = Plack::Request->new(shift);
-    return [200,[],[$req->path]];
-  };
+  sub name :Local {
+    my ($self, $c) = @_;
+    my $env = $c->Catalyst::Utils::env_at_action;
+    $c->res->from_psgi_response(
+      $psgi_app->($env));
+
+  }
+
+  sub name_args :Local Args(1) {
+    my ($self, $c, $arg) = @_;
+    my $env = $c->Catalyst::Utils::env_at_action;
+    $c->res->from_psgi_response(
+      $psgi_app->($env));
+  }
+
+  package MyApp::Controller::User;
+  $INC{'MyApp/Controller/User.pm'} = __FILE__;
+
+  use base 'Catalyst::Controller';
+  use Plack::Request;
+  use Catalyst::Utils;
 
   sub local_example :Local {
     my ($self, $c) = @_;
@@ -89,8 +112,6 @@ use strict;
     }
   }
 
-  $INC{'MyApp/Controller/User.pm'} = __FILE__;
-
   package MyApp;
   use Catalyst;
   MyApp->setup;
@@ -324,6 +345,26 @@ use Catalyst::Test 'MyApp';
   is_deeply $c->req->args, [444];
 }
 
+{
+  my ($res, $c) = ctx_request('/docs/name');
+  is $c->action, 'docs/name';
+  is $res->content, '/';
+  is_deeply $c->req->args, [];
+}
+
+{
+  my ($res, $c) = ctx_request('/docs/name/111/222');
+  is $c->action, 'docs/name';
+  is $res->content, '/111/222';
+  is_deeply $c->req->args, [111,222];
+}
+
+{
+  my ($res, $c) = ctx_request('/docs/name_args/111');
+  is $c->action, 'docs/name_args';
+  is $res->content, '/111';
+  is_deeply $c->req->args, [111];
+}
 
 done_testing();