merged fix for regression from master
John Napiorkowski [Sun, 22 Dec 2013 16:22:43 +0000 (10:22 -0600)]
1  2 
Changes
lib/Catalyst.pm
lib/Catalyst/Delta.pod

diff --combined Changes
+++ b/Changes
@@@ -1,34 -1,12 +1,42 @@@
  # This file documents the revision history for Perl extension Catalyst.
  
 +  - Announcing the repo is not open for development of Perl Catalyst 'Runner'
 +  - http://questhub.io/realm/perl/explore/latest/tag/runner
 +
 +5.90059_002 - TBA
 +  - We now pass a scalar or filehandle directly to you Plack handler, rather
 +    than always use the streaming interface (we are still always using a
 +    delayed response callback).  This means that you can make use of Plack
 +    middleware like Plack::Middleware::XSendfile and we expect better use of
 +    server features (when they exist) like correct use of chunked encoding or
 +    properly non blocking streaming when running under a supporting server like
 +    Twiggy.  See Catalyst::Delta for more.  This change might cause issues if
 +    you are making heaving use of streaming (although in general we expect things
 +    to work much better.
 +  - In the case when we remove a content body from the response because you set
 +    an information status or a no content type status, warn that we are doing so
 +    when in debug mode.  You might see additional debugging information to help
 +    you find and remove unneeded response bodies.
 +  - Updated the code where Catalyst tries to guess a content length when you
 +    fail to provide one.  This should cause less issues when trying to guess the
 +    length of a funky filehandle.  This now uses Plack::Middleware::ContentLength
 +  - Removed custom code to remove body content when the request is HEAD and
-     swapped it for Plack::Middleware::Head.
++    swapped it for Plack::Middleware::Head
++  - Merged fix for regressions from stable..
 +
 +5.90059_001 - 2013-12-19
 +  - Removed deprecated Regexp dispatch type from dependency list.  If you are
 +    using Regex[p] type dispatching you need to add the standalone distribution
 +   'Catalyst::DispatchType::Regex' to you build system NOW or you application
 +   will be broken.
 +
+ 5.90053 - 2013-12-21
+   - Reverted a change in the previous release that moved the setup_log phase
+     to after setup_config.  This change was made to allow people to use
+     configuration that is late loaded (such as via the ConfigLoader Plugin)
+     to setup the plugin.  However it also broke the ability to use the log
+     during plugin setup (ie, it breaks lots of plugins).  Reverting the 
+     change.  See Catalyst::Delta for workarounds.
  
  5.90052 - 2013-12-18
  
diff --combined lib/Catalyst.pm
@@@ -41,8 -41,6 +41,8 @@@ use Plack::Middleware::ReverseProxy
  use Plack::Middleware::IIS6ScriptNameFix;
  use Plack::Middleware::IIS7KeepAliveFix;
  use Plack::Middleware::LighttpdScriptNameFix;
 +use Plack::Middleware::ContentLength;
 +use Plack::Middleware::Head;
  use Plack::Util;
  use Class::Load 'load_class';
  
@@@ -122,7 -120,7 +122,7 @@@ __PACKAGE__->stats_class('Catalyst::Sta
  
  # Remember to update this in Catalyst::Runtime as well!
  
 -our $VERSION = '5.90053';
 +our $VERSION = '5.90059_002';
  
  sub import {
      my ( $class, @arguments ) = @_;
@@@ -1126,6 -1124,7 +1126,7 @@@ sub setup 
  
      $class->setup_home( delete $flags->{home} );
  
+     $class->setup_log( delete $flags->{log} );
      $class->setup_plugins( delete $flags->{plugins} );
  
      # Call plugins setup, this is stupid and evil.
          $class->setup unless $Catalyst::__AM_RESTARTING;
      }
  
-     $class->setup_log( delete $flags->{log} );
      $class->setup_middleware();
      $class->setup_data_handlers();
      $class->setup_dispatcher( delete $flags->{dispatcher} );
@@@ -1859,6 -1857,11 +1859,6 @@@ sub finalize 
  
          $c->finalize_headers unless $c->response->finalized_headers;
  
 -        # HEAD request
 -        if ( $c->request->method eq 'HEAD' ) {
 -            $c->response->body('');
 -        }
 -
          $c->finalize_body;
      }
  
          }
      }
  
 -    # Content-Length
 -    if ( defined $response->body && length $response->body && !$response->content_length ) {
 +    # Remove incorrectly added body and content related meta data when returning
 +    # an information response, or a response the is required to not include a body
  
 -        # get the length from a filehandle
 -        if ( blessed( $response->body ) && $response->body->can('read') || ref( $response->body ) eq 'GLOB' )
 -        {
 -            my $size = -s $response->body;
 -            if ( $size ) {
 -                $response->content_length( $size );
 -            }
 -            else {
 -                $c->log->warn('Serving filehandle without a content-length');
 -            }
 -        }
 -        else {
 -            # everything should be bytes at this point, but just in case
 -            $response->content_length( length( $response->body ) );
 -        }
 -    }
 -
 -    # Errors
      if ( $response->status =~ /^(1\d\d|[23]04)$/ ) {
 -        $response->headers->remove_header("Content-Length");
 -        $response->body('');
 +        if($response->has_body) {
 +          $c->log->debug('Removing body for informational or no content http responses');
 +          $response->body('');
 +          $response->headers->remove_header("Content-Length");
 +        }
      }
  
      $c->finalize_cookies;
@@@ -3104,10 -3122,7 +3104,10 @@@ L<Catalyst::Plugin::EnableMiddleware> (
  sub registered_middlewares {
      my $class = shift;
      if(my $middleware = $class->_psgi_middleware) {
 -        return @$middleware;
 +        return (
 +          Plack::Middleware::ContentLength->new,
 +          Plack::Middleware::Head->new,
 +          @$middleware);
      } else {
          die "You cannot call ->registered_middlewares until middleware has been setup";
      }
diff --combined lib/Catalyst/Delta.pod
@@@ -4,45 -4,53 +4,90 @@@ Catalyst::Delta - Overview of changes b
  
  =head1 DESCRIPTION
  
 -This is an overview of the user-visible changes to Catalyst between major Catalyst releases.
 +This is an overview of the user-visible changes to Catalyst between major
 +Catalyst releases.
 +
 +=head2 VERSION 5.90060+
 +
 +We changed the way we return body content (from response) to whatever
 +Plack handler you are using (Starman, FastCGI, etc.)  We no longer
 +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 using chunked encoding when running
 +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
 +a filehandle that sets a readable path, your server might now correctly
 +handle the file (rather than as before where Catalyst would stream it
 +very likely very slowly).
 +
 +In other words, some things might be meaninglessly different and some
 +things that were broken codewise but worked because of Catalyst being
 +incorrect might suddenly be really broken.  The behavior is now more
 +correct in that Catalyst plays better with features that Plack offers
 +but if you are making heavy use of the streaming interface there could
 +be some differences so you should test carefully (this is probably not
 +the vast majority of people).  In particular if you are developing
 +using one server but deploying using a different one, differences in
 +what those server do with streaming should be noted.
 +
 +We also now more carefully distingush the different between a body set
 +to '' and a body that is undef.  This might lead to situations where
 +again you'll get a content-length were you didn't get one before or
 +where a supporting server will start chunking output.  If this is an
 +issue you can apply the middleware L<Plack::Middleware::BufferedStreaming>
 +or report specific problems to the dev team.
 +
 +Also, we have started migrating code in Catalyst to equivilent Plack
 +Middleware when such exists and is correct to do so. 
  
+ =head2 VERSION 5.90053
+ We are now clarifying the behavior of log, plugins and configuration during
+ the setup phase.  Since Plugins might require a log during setup, setup_log
+ must run BEFORE setup_plugins.   This has the unfortunate side effect that
+ anyone using the popular ConfigLoader plugin will not be able to supply
+ configuration to custom logs since the configuration is not yet finalized
+ when setup_log is run (when using ConfigLoader, which is a plugin and is
+ not loaded until later.)
+ As a workaround, you can supply custom log configuration directly into
+ the configuration:
+     package MyApp;
+     use Catalyst;
+     __PACKAGE__->config(
+       my_custom_log_info => { %custom_args },
+     );
+     __PACKAGE__->setup;
+ If you wish to configure the custom logger differently based on ENV, you can
+ try:
+     package MyApp;
+     use Catalyst;
+     use Catalyst::Utils;
+     __PACKAGE__->config(
+       Catalyst::Utils::merge_hashes(
+         +{ my_custom_log_info => { %base_custom_args } },
+         +{ do __PACKAGE__->path_to( $ENV{WHICH_CONF}."_conf.pl") },
+       ),
+     );
+     __PACKAGE__->setup;
+ Or create a standalone Configuration class that does the right thing.
+ Basically if you want to configure a logger via Catalyst global configuration
+ you can't use ConfigLoader because it will always be loaded too late to be of
+ any use.  Patches and workaround options welcomed!
  =head2 VERSION 5.9XXXX 'cataplack'
  
  The Catalyst::Engine sub-classes have all been removed and deprecated,