From: John Napiorkowski <jjnapiork@cpan.org>
Date: Wed, 23 Oct 2013 19:00:51 +0000 (-0500)
Subject: removed plack.request keys
X-Git-Tag: 5.90050~1^2~13
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=catagits%2FCatalyst-Runtime.git;a=commitdiff_plain;h=f152ae23b886a4f0bcfaeaf401ea2bf71cd30ab1;hp=efba3342e7c14be407683c876adcda8684b564d5

removed plack.request keys
---

diff --git a/Changes b/Changes
index 7723053..8679020 100644
--- a/Changes
+++ b/Changes
@@ -7,7 +7,11 @@
     as actions that only handle JSON or actions that only understand classic
     HTML forms.
   - TODO: body_data should slurp classic formdata
-  - TODO: remove non public Plack::Request $env keys added in previous release
+  - Removed PSGI $env keys that are added on the 'plack.request.*' namespace
+    since after discussion it was clear those keys are not part of the public
+    API.  Keys removed: 'plack.request.query', 'plack.request.body', 
+    'plack.request.merged' and 'plack.request.http.body'. Altered some test
+    cases to reflect this change.
 
 5.90049_004 - 2013-10-18
   - JSON Data handler looks for both JSON::MaybeXS and JSON, and uses
diff --git a/lib/Catalyst.pm b/lib/Catalyst.pm
index c7b8bce..f65448c 100644
--- a/lib/Catalyst.pm
+++ b/lib/Catalyst.pm
@@ -3203,6 +3203,12 @@ sub setup_data_handlers {
 sub default_data_handlers {
     my ($class) = @_;
     return +{
+      'application/x-www-form-urlencoded' => sub {
+          my ($fh, $req) = @_;
+          my $params = $req->_use_hash_multivalue ? $self->body_parameters->mixed : $self->body_parameters;
+          Class::Load::load_first_existing_class('CGI::Struct::XS', 'CGI::Struct')
+            ->('build_cgi_struct')->($params)
+      },
       'application/json' => sub {
           Class::Load::load_first_existing_class('JSON::MaybeXS', 'JSON')
             ->can('decode_json')->(do { local $/; $_->getline });
diff --git a/lib/Catalyst/Engine.pm b/lib/Catalyst/Engine.pm
index 3b4ceb0..ba606ba 100644
--- a/lib/Catalyst/Engine.pm
+++ b/lib/Catalyst/Engine.pm
@@ -507,10 +507,6 @@ sub prepare_query_parameters {
     # (yes, index() is faster than a regex :))
     if ( index( $query_string, '=' ) < 0 ) {
         $c->request->query_keywords($self->unescape_uri($query_string));
-        $env->{'plack.request.query'} ||= Hash::MultiValue->new(
-          map { (URI::Escape::uri_unescape($_), '') }
-            split(/\+/, $query_string, -1));
-
         return;
     }
 
@@ -542,10 +538,9 @@ sub prepare_query_parameters {
         }
     }
 
-    $env->{'plack.request.query'} ||= Hash::MultiValue->from_mixed(\%query);
     $c->request->query_parameters( 
       $c->request->_use_hash_multivalue ?
-        $env->{'plack.request.query'}->clone :
+        Hash::MultiValue->from_mixed(\%query) :
         \%query);
 }
 
@@ -588,7 +583,6 @@ sub prepare_uploads {
 
     my $uploads = $request->_body->upload;
     my $parameters = $request->parameters;
-    my @plack_uploads;
     foreach my $name (keys %$uploads) {
         my $files = $uploads->{$name};
         my @uploads;
@@ -603,14 +597,9 @@ sub prepare_uploads {
                filename => $upload->{filename},
               );
             push @uploads, $u;
-
-            # Plack compatibility.
-            my %copy = (%$upload, headers=>$headers);
-            push @plack_uploads, $name, Plack::Request::Upload->new(%copy);
         }
         $request->uploads->{$name} = @uploads > 1 ? \@uploads : $uploads[0];
 
-
         # support access to the filename as a normal param
         my @filenames = map { $_->{filename} } @uploads;
         # append, if there's already params with this name
@@ -626,8 +615,6 @@ sub prepare_uploads {
             $parameters->{$name} = @filenames > 1 ? \@filenames : $filenames[0];
         }
     }
-
-    $self->env->{'plack.request.upload'} ||= Hash::MultiValue->new(@plack_uploads);
 }
 
 =head2 $self->write($c, $buffer)
diff --git a/lib/Catalyst/Request.pm b/lib/Catalyst/Request.pm
index 87ce66a..15f7b7b 100644
--- a/lib/Catalyst/Request.pm
+++ b/lib/Catalyst/Request.pm
@@ -60,7 +60,7 @@ has query_keywords => (is => 'rw');
 has match => (is => 'rw');
 has method => (is => 'rw');
 has protocol => (is => 'rw');
-has query_parameters  => (is => 'rw', default => sub { {} });
+has query_parameters  => (is => 'rw', lazy=>1, default => sub { shift->_use_hash_multivalue ? Hash::MultiValue->new : +{} });
 has secure => (is => 'rw', default => 0);
 has captures => (is => 'rw', default => sub { [] });
 has uri => (is => 'rw', predicate => 'has_uri');
@@ -210,15 +210,8 @@ sub _build_parameters {
     my $body_parameters = $self->body_parameters;
     my $query_parameters = $self->query_parameters;
 
-    ## setup for downstream plack
-    $self->env->{'plack.request.merged'} ||= do {
-        my $query = $self->env->{'plack.request.query'} || Hash::MultiValue->new;
-        my $body  = $self->env->{'plack.request.body'} || Hash::MultiValue->new;
-        Hash::MultiValue->new($query->flatten, $body->flatten);
-    };
-
     if($self->_use_hash_multivalue) {
-        return $self->env->{'plack.request.merged'}->clone; # We want a copy, in case your App is evil
+        return Hash::MultiValue->new($query_parameters->flatten, $body_parameters->flatten);
     }
 
     # We copy, no references
@@ -256,13 +249,6 @@ sub prepare_body {
         return;
     }
 
-    # Define PSGI ENV placeholders, or for empty should there be no content
-    # body (typical in HEAD or GET).  Looks like from Plack::Request that
-    # middleware would probably expect to see this, even if empty
-
-    $self->env->{'plack.request.body'}   = Hash::MultiValue->new;
-    $self->env->{'plack.request.upload'} = Hash::MultiValue->new;
-
     # If there is nothing to read, set body to naught and return.  This
     # will cause all body code to be skipped
 
@@ -312,9 +298,6 @@ sub prepare_body {
         $self->env->{'psgi.input'}->seek(0, 0); # Reset the buffer for downstream middleware or apps
     }
 
-    $self->env->{'plack.request.http.body'} = $self->_body;
-    $self->env->{'plack.request.body'} = Hash::MultiValue->from_mixed($self->_body->param);
-
     # paranoia against wrong Content-Length header
     my $remaining = $length - $self->_read_position;
     if ( $remaining > 0 ) {
@@ -332,10 +315,13 @@ sub prepare_body_parameters {
     my ( $self ) = @_;
 
     $self->prepare_body if ! $self->_has_body;
-    return {} unless $self->_body;
+
+    unless($self->_body) {
+      return $self->_use_hash_multivalue ? Hash::MultiValue->new : {};
+    }
 
     return $self->_use_hash_multivalue ?
-        $self->env->{'plack.request.body'}->clone :
+        Hash::MultiValue->from_mixed($self->_body->param) :
         $self->_body->param;
 }
 
diff --git a/t/lib/TestFromPSGI/Controller/Root.pm b/t/lib/TestFromPSGI/Controller/Root.pm
index f3a0477..1f82453 100644
--- a/t/lib/TestFromPSGI/Controller/Root.pm
+++ b/t/lib/TestFromPSGI/Controller/Root.pm
@@ -5,9 +5,9 @@ use MooseX::MethodAttributes;
 
 extends 'Catalyst::Controller';
 
-sub test_psgi_keys :Local Args(1) {
-  my ($self, $c, $key) = @_;
-  $c->res->body($c->req->env->{$key});
+sub test_psgi_keys :Local {
+  my ($self, $c) = @_;
+  $c->res->body('ok');
 }
 
 sub from_psgi_array : Local {
diff --git a/t/more-psgi-compat.t b/t/more-psgi-compat.t
index d7c6101..a176472 100644
--- a/t/more-psgi-compat.t
+++ b/t/more-psgi-compat.t
@@ -38,19 +38,6 @@ use Catalyst::Test 'TestFromPSGI';
   ok my($res, $c) = ctx_request(POST '/test_psgi_keys?a=1&b=2', [c=>3,d=>4]);
 
   ok $c->req->env->{"psgix.input.buffered"}, "input is buffered";
-  ok $c->req->env->{"plack.request.http.body"};
-  ok my $body = $c->req->env->{"plack.request.body"};
-  ok my $query = $c->req->env->{"plack.request.query"};
-  ok my $merged = $c->req->env->{"plack.request.merged"};
-
-  is $body->get('c'), 3;
-  is $body->get('d'), 4;
-  is $query->get('a'), 1;
-  is $query->get('b'), 2;
-  is $merged->get('c'), 3;
-  is $merged->get('d'), 4;
-  is $merged->get('a'), 1;
-  is $merged->get('b'), 2;
 
   is $c->req->parameters->get('c'), 3;
   is $c->req->parameters->get('d'), 4;