document deprecations, refactor finalize body a bit
John Napiorkowski [Tue, 21 Jan 2014 20:05:28 +0000 (14:05 -0600)]
lib/Catalyst/Delta.pod
lib/Catalyst/Engine.pm
lib/Catalyst/Response.pm

index 2521e26..1c3de6e 100755 (executable)
@@ -75,6 +75,11 @@ this backcompat 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
+
+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>
 
 L<Catalyst::Engine::PSGI> is also officially no longer supported.  We will
@@ -90,6 +95,13 @@ Code has been maintained here for backcompat 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
+
+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
+the filehandle to the underlying Plack handler.  For now we will continue
+to support setting body to a simple value since 
+
 =head2 VERSION 5.90053
 
 We are now clarifying the behavior of log, plugins and configuration during
index 149d772..d48d9a4 100644 (file)
@@ -75,6 +75,7 @@ sub finalize_body {
     ## doing something custom and is expected to close the response
     return if $res->_has_write_fh;
 
+    my $body = $res->body; # save some typing
     if($res->_has_response_cb) {
         ## we have not called the response callback yet, so we are safe to send
         ## the whole body to PSGI
@@ -82,42 +83,55 @@ sub finalize_body {
         my @headers;
         $res->headers->scan(sub { push @headers, @_ });
 
-        ## We need to figure out what kind of body we have...
-        my $body = $res->body;
+        # We need to figure out what kind of body we have and normalize it to something
+        # PSGI can deal with
         if(defined $body) {
-            if( 
-                (blessed($body) && $body->can('getline'))
-                or ref($body) eq 'GLOB'
-            ) {
-              # Body is an IO handle that meets the PSGI spec
-            } elsif(blessed($body) && $body->can('read')) {
-                # In the past, Catalyst only looked for read not getline.  It is very possible
-                # that one might have an object that respected read but did not have getline.
-                # As a result, we need to handle this case for backcompat.
+            # Handle objects first
+            if(blessed($body)) {
+                if($body->can('getline')) {
+                    # Body is an IO handle that meets the PSGI spec.  Nothing to normalize
+                } elsif($body->can('read')) {
+
+                    # In the past, Catalyst only looked for ->read not ->getline.  It is very possible
+                    # that one might have an object that respected read but did not have getline.
+                    # As a result, we need to handle this case for backcompat.
                 
-                # We will just do the old loop for now but someone could write a proxy
-                # object to wrap getline and proxy read
-                my $got;
-                do {
-                    $got = read $body, my ($buffer), $CHUNKSIZE;
-                    $got = 0 unless $self->write($c, $buffer );
-                } while $got > 0;
-
-                # I really am guessing this case is pathological.  I'd like to remove it
-                # but need to give people a bit of heads up
-                $c->log->warn('!!! Setting $response->body to an object that supports "read" but not "getline" is deprecated. !!!')
-                  unless $self->{__FH_READ_DEPRECATION_NOTICE_qwvsretf43}++;
-
-                close $body;
-                return;
+                    # We will just do the old loop for now.  In a future version of Catalyst this support
+                    # will be removed and one will have to rewrite their custom object or use 
+                    # Plack::Middleware::AdaptFilehandleRead.  In anycase support for this is officially
+                    # deprecated and described as such as of 5.90060
+                   
+                    my $got;
+                    do {
+                        $got = read $body, my ($buffer), $CHUNKSIZE;
+                        $got = 0 unless $self->write($c, $buffer );
+                    } while $got > 0;
+
+                    close $body;
+                    return;
+                } else {
+                    # Looks like for  backcompat reasons we need to be able to deal
+                    # with stringyfiable objects.
+                    $body = ["$body"]; 
+                }
+            } elsif(ref $body) {
+                if( (ref($body) eq 'GLOB') or (ref($body) eq 'ARRAY')) {
+                  # Again, PSGI can just accept this, no transform needed.  We don't officially
+                  # document the body as arrayref at this time (and there's not specific test
+                  # cases.  we support it because it simplifies some plack compatibility logic
+                  # and we might make it official at some point.
+                } else {
+                   $c->log->error("${\ref($body)} is not a valid value for Response->body");
+                   return;
+                }
             } else {
-              # Looks like for  backcompat reasons we need to be able to deal
-              # with stringyfiable objects.
-              $body = "$body" if blessed($body); # Assume there's some sort of overloading..
-              $body = [$body];  
+                # Body is defined and not an object or reference.  We assume a simple value
+                # and wrap it in an array for PSGI
+                $body = [$body];
             }
         } else {
-          $body = [];
+            # There's no body...
+            $body = [];
         }
 
         $res->_response_cb->([ $res->status, \@headers, $body]);
@@ -133,11 +147,11 @@ sub finalize_body {
         ## We'll just use the old, existing code for this (or most of it)
 
         if(my $body = $res->body) {
-          no warnings 'uninitialized';
+
           if ( blessed($body) && $body->can('read') or ref($body) eq 'GLOB' ) {
 
               ## In this case we have no choice and will fall back on the old
-              ## manual streaming stuff.
+              ## manual streaming stuff.  Not optimal.  This is deprecated as of 5.900560+
 
               my $got;
               do {
index 8c26ae4..fa3b146 100644 (file)
@@ -111,23 +111,15 @@ sub from_psgi_response {
         my ($status, $headers, $body) = @$psgi_res;
         $self->status($status);
         $self->headers(HTTP::Headers->new(@$headers));
-        if(ref $body eq 'ARRAY') {
-          $self->body(join '', grep defined, @$body);
-        } else {
-          $self->body($body);
-        }
+        $self->body($body);
     } elsif(ref $psgi_res eq 'CODE') {
         $psgi_res->(sub {
             my $response = shift;
             my ($status, $headers, $maybe_body) = @$response;
             $self->status($status);
             $self->headers(HTTP::Headers->new(@$headers));
-            if($maybe_body) {
-                if(ref $maybe_body eq 'ARRAY') {
-                  $self->body(join '', grep defined, @$maybe_body);
-                } else {
-                  $self->body($maybe_body);
-                }
+            if(defined $maybe_body) {
+                $self->body($maybe_body);
             } else {
                 return $self->write_fh;
             }