solution for warnings when using from_psgi_response with streaing
[catagits/Catalyst-Runtime.git] / lib / Catalyst / Response.pm
index 186eb0d..4f9c391 100644 (file)
@@ -3,11 +3,12 @@ package Catalyst::Response;
 use Moose;
 use HTTP::Headers;
 use Moose::Util::TypeConstraints;
-use namespace::autoclean;
 use Scalar::Util 'blessed';
 use Catalyst::Response::Writer;
 use Catalyst::Utils ();
 
+use namespace::clean -except => ['meta'];
+
 with 'MooseX::Emulate::Class::Accessor::Fast';
 
 our $DEFAULT_ENCODE_CONTENT_TYPE_MATCH = qr{text|xml$|javascript$};
@@ -20,7 +21,7 @@ has encodable_content_type => (
 
 has _response_cb => (
     is      => 'ro',
-    isa     => 'CodeRef', 
+    isa     => 'CodeRef',
     writer  => '_set_response_cb',
     clearer => '_clear_response_cb',
     predicate => '_has_response_cb',
@@ -67,7 +68,7 @@ sub _build_write_fh {
   my $requires_encoding = $_[0]->encodable_response;
   my %fields = (
     _writer => $writer,
-    _encoding => $_[0]->_context->encoding,
+    _context => $_[0]->_context,
     _requires_encoding => $requires_encoding,
   );
 
@@ -103,13 +104,25 @@ has _context => (
   clearer => '_clear_context',
 );
 
-before [qw(status headers content_encoding content_length content_type header)] => sub {
+before [qw(status headers content_encoding content_length content_type )] => sub {
   my $self = shift;
 
-  $self->_context->log->warn( 
+  $self->_context->log->warn(
     "Useless setting a header value after finalize_headers and the response callback has been called." .
-    " Not what you want." )
-      if ( $self->finalized_headers && !$self->_has_response_cb && @_ );
+    " Since we don't support tail headers this will not work as you might expect." )
+      if ( $self->_context && $self->finalized_headers && !$self->_has_response_cb && @_ );
+};
+
+# This has to be different since the first param to ->header is the header name and presumably
+# you should be able to request the header even after finalization, just not try to change it.
+before 'header' => sub {
+  my $self = shift;
+  my $header = shift;
+
+  $self->_context->log->warn(
+    "Useless setting a header value after finalize_headers and the response callback has been called." .
+    " Since we don't support tail headers this will not work as you might expect." )
+      if ( $self->_context && $self->finalized_headers && !$self->_has_response_cb && @_ );
 };
 
 sub output { shift->body(@_) }
@@ -134,6 +147,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;
@@ -148,22 +175,47 @@ sub from_psgi_response {
         my ($status, $headers, $body) = @$psgi_res;
         $self->status($status);
         $self->headers(HTTP::Headers->new(@$headers));
-        $self->body($body);
+        # Can be arrayref or filehandle...
+        if(defined $body) { # probably paranoia
+          ref $body eq 'ARRAY' ? $self->body(join('', @$body)) : $self->body($body);
+        }
     } elsif(ref $psgi_res eq 'CODE') {
+
+        # Its not clear to me this is correct.  Right now if the PSGI application wants
+        # to stream, we stream immediately and then completely bypass the rest of the
+        # Catalyst finalization process (unlike if the PSGI app sets an arrayref).  Part of
+        # me thinks we should override the current _response_cb and then let finalize_body
+        # call that.  I'm not sure the downside of bypassing those bits.  I'm going to leave
+        # this be for now and document the behavior.
+
         $psgi_res->(sub {
             my $response = shift;
             my ($status, $headers, $maybe_body) = @$response;
             $self->status($status);
             $self->headers(HTTP::Headers->new(@$headers));
             if(defined $maybe_body) {
-                $self->body($maybe_body);
+                # Can be arrayref or filehandle...
+                ref $maybe_body eq 'ARRAY' ? $self->body(join('', @$maybe_body)) : $self->body($maybe_body);
             } else {
                 return $self->write_fh;
             }
-        });  
+        });
      } else {
         die "You can't set a Catalyst response from that, expect a valid PSGI response";
     }
+
+    # Encoding compatibilty.   If the response set a charset, well... we need
+    # to assume its properly encoded and NOT encode for this response.  Otherwise
+    # We risk double encoding.
+
+    # We check first to make sure headers have not been finalized.  Headers might be finalized
+    # in the case where a PSGI response is streaming and the PSGI application already wrote
+    # to the output stream and close the filehandle.
+    if(!$self->finalized_headers && $self->content_type_charset) {
+      # We have to do this since for backcompat reasons having a charset doesn't always
+      # mean that the body is already encoded :(
+      $self->_context->clear_encoding;
+    }
 }
 
 =head1 NAME
@@ -199,7 +251,7 @@ will turn the Catalyst::Response into a HTTP Response and return it to the clien
     $c->response->body('Catalyst rocks!');
 
 Sets or returns the output (text or binary data). If you are returning a large body,
-you might want to use a L<IO::Handle> type of object (Something that implements the getline method 
+you might want to use a L<IO::Handle> type of object (Something that implements the getline method
 in the same fashion), or a filehandle GLOB. These will be passed down to the PSGI
 handler you are using and might be optimized using server specific abilities (for
 example L<Twiggy> will attempt to server a real local file in a non blocking manner).
@@ -224,7 +276,7 @@ When using a L<IO::Handle> type of object and no content length has been
 already set in the response headers Catalyst will make a reasonable attempt
 to determine the size of the Handle. Depending on the implementation of your
 handle object, setting the content length may fail. If it is at all possible
-for you to determine the content length of your handle object, 
+for you to determine the content length of your handle object,
 it is recommended that you set the content length in the response headers
 yourself, which will be respected and sent by Catalyst in the response.
 
@@ -449,12 +501,18 @@ even set a 'body' afterward.  So for example you might write your HTTP headers
 and the HEAD section of your document and then set the body from a template
 driven from a database.  In some cases this can seem to the client as if you had
 a faster overall response (but note that unless your server support chunked
-body your content is likely to get queued anyway (L<Starman> and most other 
+body your content is likely to get queued anyway (L<Starman> and most other
 http 1.1 webservers support this).
 
 If there is an encoding set, we encode each line of the response (the default
 encoding is UTF-8).
 
+=head2 $res->unencoded_write( $data )
+
+Works just like ->write but we don't apply any content encoding to C<$data>.  Use
+this if you are already encoding the $data or the data is arriving from an encoded
+storage.
+
 =head2 $res->write_fh
 
 Returns an instance of L<Catalyst::Response::Writer>, which is a lightweight
@@ -511,7 +569,7 @@ finalized (there isn't one anyway) and you need to call the close method.
 Prints @data to the output stream, separated by $,.  This lets you pass
 the response object to functions that want to write to an L<IO::Handle>.
 
-=head2 $self->finalize_headers($c)
+=head2 $res->finalize_headers()
 
 Writes headers to response if not already written
 
@@ -541,8 +599,32 @@ Example:
       $c->res->from_psgi_response($app->($c->req->env));
     }
 
+    sub streaming_body :Local {
+      my ($self, $c) = @_;
+      my $psgi_app = sub {
+          my $respond = shift;
+          my $writer = $respond->([200,["Content-Type" => "text/plain"]]);
+          $writer->write("body");
+          $writer->close;
+      };
+      $c->res->from_psgi_response($psgi_app);
+    }
+
 Please note this does not attempt to map or nest your PSGI application under
-the Controller and Action namespace or path.  
+the Controller and Action namespace or path. You may wish to review 'PSGI Helpers'
+under L<Catalyst::Utils> for help in properly nesting applications.
+
+B<NOTE> If your external PSGI application returns a response that has a character
+set associated with the content type (such as "text/html; charset=UTF-8") we set
+$c->clear_encoding to remove any additional content type encoding processing later
+in the application (this is done to avoid double encoding issues).
+
+B<NOTE> If your external PSGI application is streaming, we assume you completely
+handle the entire jobs (including closing the stream).  This will also bypass
+the output finalization methods on Catalyst (such as 'finalize_body' which gets
+called but then skipped when it finds that output is already finished.)  Its possible
+this might cause issue with some plugins that want to do 'things' during those
+finalization methods.  Just understand what is happening.
 
 =head2 encodable_content_type
 
@@ -572,7 +654,7 @@ Or you can alter the regular expression using this attribute.
 
 =head2 encodable_response
 
-Given a L<Catalyst::Response> return true if its one that can be encoded.  
+Given a L<Catalyst::Response> return true if its one that can be encoded.
 
      make sure there is an encoding set on the response
      make sure the content type is encodable
@@ -589,16 +671,28 @@ sub encodable_response {
   return 0 unless $self->_context; # Cases like returning a HTTP Exception response you don't have a context here...
   return 0 unless $self->_context->encoding;
 
+  # The response is considered to have a 'manual charset' when a charset is already set on
+  # the content type of the response AND it is not the same as the one we set in encoding.
+  # If there is no charset OR we are asking for the one which is the same as the current
+  # required encoding, that is a flag that we want Catalyst to encode the response automatically.
   my $has_manual_charset = 0;
   if(my $charset = $self->content_type_charset) {
     $has_manual_charset = (uc($charset) ne uc($self->_context->encoding->mime_name)) ? 1:0;
   }
 
+  # Content type is encodable if it matches the regular expression stored in this attribute
+  my $encodable_content_type = $self->content_type =~ m/${\$self->encodable_content_type}/ ? 1:0;
+
+  # The content encoding is allowed (for charset encoding) only if its empty or is set to identity
+  my $allowed_content_encoding = (!$self->content_encoding || $self->content_encoding eq 'identity') ? 1:0;
+
+  # The content type must be an encodable type, and there must be NO manual charset and also
+  # the content encoding must be the allowed values;
   if(
-      ($self->content_type =~ m/${\$self->encodable_content_type}/) and
-      (!$has_manual_charset) and
-      (!$self->content_encoding || $self->content_encoding eq 'identity' )
-  ) { 
+      $encodable_content_type and
+      !$has_manual_charset and
+      $allowed_content_encoding
+  ) {
     return 1;
   } else {
     return 0;