Updated Catalyst::Request and Catalyst::Response to have sensible defaults for attributes

Updated Catalyst.pm to reflect sensible defaults
Worked through some TODO items added by groditi
Fixed up a test that was testing against object internals rather than accessors
r17794@martha (orig r7725):  konobi | 2008-05-08 14:42:41 -0400

diff --git a/lib/Catalyst.pm b/lib/Catalyst.pm
index c7007ab..37425b1 100644 (file)
--- a/lib/Catalyst.pm
+++ b/lib/Catalyst.pm
@@ -29,14 +29,14 @@ use Carp qw/croak carp/;
 
 BEGIN { require 5.008001; }
 
-has stack => (is => 'rw');
-has stash => (is => 'rw');
-has state => (is => 'rw');
+has stack => (is => 'rw', default => sub { [] });
+has stash => (is => 'rw', default => sub { {} });
+has state => (is => 'rw', default => 0);
 has stats => (is => 'rw');
 has action => (is => 'rw');
-has counter => (is => 'rw');
-has request => (is => 'rw');
-has response => (is => 'rw');
+has counter => (is => 'rw', default => sub { {} });
+has request => (is => 'rw', default => sub { $_[0]->request_class->new({}) }, required => 1, lazy => 1);
+has response => (is => 'rw', default => sub { $_[0]->response_class->new({}) }, required => 1, lazy => 1);
 has namespace => (is => 'rw');
 
 
@@ -46,8 +46,15 @@ sub depth { scalar @{ shift->stack || [] }; }
 
 # Laziness++
 *comp = \&component;
-*req  = \&request;
-*res  = \&response;
+
+sub req {
+    # carp "the use of req() is deprecated in favour of request()";
+    my $self = shift; return $self->request(@_);
+}
+sub res {
+    # carp "the use of res() is deprecated in favour of response()";
+    my $self = shift; return $self->response(@_);
+}
 
 # For backwards compatibility
 *finalize_output = \&finalize_body;
@@ -330,7 +337,7 @@ your code like this:
 
 =cut
 
-sub forward { my $c = shift; $c->dispatcher->forward( $c, @_ ) }
+sub forward { my $c = shift; no warnings 'recursion'; $c->dispatcher->forward( $c, @_ ) }
 
 =head2 $c->detach( $action [, \@arguments ] )
 
@@ -375,14 +382,16 @@ Catalyst).
 around stash => sub {
     my $orig = shift;
     my $c = shift;
+
+    my $orig_stash = $c->$orig();
     if (@_) {
         my $stash = @_ > 1 ? {@_} : $_[0];
         croak('stash takes a hash or hashref') unless ref $stash;
         foreach my $key ( keys %$stash ) {
-            $c->$orig()->{$key} = $stash->{$key};
+            $orig_stash->{$key} = $stash->{$key};
         }
     }
-    return $c->$orig();
+    return $orig_stash;
 };
 
 =head2 $c->error
@@ -1429,9 +1438,8 @@ sub finalize_headers {
 
     my $response = $c->response; #accessor calls can add up?
 
-    # Moose TODO: Maybe this should be an attribute too?
     # Check if we already finalized headers
-    return if $response->{_finalized_headers};
+    return if $response->finalized_headers;
 
     # Handle redirects
     if ( my $location = $response->redirect ) {
@@ -1478,7 +1486,7 @@ sub finalize_headers {
     $c->engine->finalize_headers( $c, @_ );
 
     # Done
-    $response->{_finalized_headers} = 1;
+    $response->finalized_headers(1);
 }
 
 =head2 $c->finalize_output
@@ -1548,8 +1556,10 @@ sub handle_request {
     }
 
     $COUNT++;
-    #todo: reuse coderef from can
-    $class->log->_flush() if $class->log->can('_flush');
+    
+    if(my $coderef = $class->log->can('_flush')){
+        $class->log->$coderef();
+    }
     return $status;
 }
 
@@ -1563,39 +1573,16 @@ etc.).
 sub prepare {
     my ( $class, @arguments ) = @_;
 
-    #moose todo: context_class as attr with default
+    # XXX
+    # After the app/ctxt split, this should become an attribute based on something passed
+    # into the application.
     $class->context_class( ref $class || $class ) unless $class->context_class;
-    #Moose TODO: if we make empty containers the defaults then that can be
-    #handled by the context class itself instead of having this here
-    my $c = $class->context_class->new(
-        {
-            counter => {},
-            stack   => [],
-            request => $class->request_class->new(
-                {
-                    arguments        => [],
-                    body_parameters  => {},
-                    cookies          => {},
-                    headers          => HTTP::Headers->new,
-                    parameters       => {},
-                    query_parameters => {},
-                    secure           => 0,
-                    captures         => [],
-                    uploads          => {}
-                }
-            ),
-            response => $class->response_class->new(
-                {
-                    body    => '',
-                    cookies => {},
-                    headers => HTTP::Headers->new(),
-                    status  => 200
-                }
-            ),
-            stash => {},
-            state => 0
-        }
-    );
+   
+    my $c = $class->context_class->new({});
+
+    # For on-demand data
+    $c->request->_context($c);
+    $c->response->_context($c);
 
     #surely this is not the most efficient way to do things...
     $c->stats($class->stats_class->new)->enable($c->use_stats);
@@ -1603,10 +1590,6 @@ sub prepare {
         $c->res->headers->header( 'X-Catalyst' => $Catalyst::VERSION );            
     }
 
-    # For on-demand data
-    $c->request->_context($c);
-    $c->response->_context($c);
-
     #XXX reuse coderef from can
     # Allow engine to direct the prepare flow (for POE)
     if ( $c->engine->can('prepare') ) {

diff --git a/lib/Catalyst/Action.pm b/lib/Catalyst/Action.pm
index 2023303..4469a1d 100644 (file)
--- a/lib/Catalyst/Action.pm
+++ b/lib/Catalyst/Action.pm
@@ -49,15 +49,15 @@ sub dispatch {    # Execute ourselves against a context
     my ( $self, $c ) = @_;
     #Moose todo: grrrrrr. this is no good. i don't know enough about it to
     # debug it though. why can't we just call the accessor?
-    local $c->{namespace} = $self->namespace;
-    return $c->execute( $self->class, $self );
+    #local $c->{namespace} = $self->namespace;
+    #return $c->execute( $self->class, $self );
 
     #believed to be equivalent:
-    #my $orig = $c->namespace;
-    #$c->namespace($self->namespace);
-    #my $ret = $c->execute( $self->class, $self );
-    #$c->namespace($orig);
-    #return $ret;
+    my $orig = $c->namespace;
+    $c->namespace($self->namespace);
+    my $ret = $c->execute( $self->class, $self );
+    $c->namespace($orig);
+    return $ret;
 }
 
 sub execute {

diff --git a/lib/Catalyst/ActionChain.pm b/lib/Catalyst/ActionChain.pm
index e018280..819b894 100644 (file)
--- a/lib/Catalyst/ActionChain.pm
+++ b/lib/Catalyst/ActionChain.pm
@@ -57,6 +57,7 @@ sub from_chain {
     return $self->new({ %$final, chain => $actions });
 }
 
+no Moose;
 __PACKAGE__->meta->make_immutable;
 1;
 

diff --git a/lib/Catalyst/DispatchType.pm b/lib/Catalyst/DispatchType.pm
index dcc442f..99da4c6 100644 (file)
--- a/lib/Catalyst/DispatchType.pm
+++ b/lib/Catalyst/DispatchType.pm
@@ -71,6 +71,7 @@ the same terms as Perl itself.
 
 =cut
 
+no Moose;
 __PACKAGE__->meta->make_immutable;
 
 1;

diff --git a/lib/Catalyst/DispatchType/Chained.pm b/lib/Catalyst/DispatchType/Chained.pm
index 1b8219a..25617da 100644 (file)
--- a/lib/Catalyst/DispatchType/Chained.pm
+++ b/lib/Catalyst/DispatchType/Chained.pm
@@ -328,6 +328,7 @@ sub uri_for_action {
    
 }
 
+no Moose;
 __PACKAGE__->meta->make_immutable;
 
 =head1 USAGE

diff --git a/lib/Catalyst/DispatchType/Default.pm b/lib/Catalyst/DispatchType/Default.pm
index 7981ac2..1216b44 100644 (file)
--- a/lib/Catalyst/DispatchType/Default.pm
+++ b/lib/Catalyst/DispatchType/Default.pm
@@ -61,6 +61,7 @@ the same terms as Perl itself.
 
 =cut
 
+no Moose;
 __PACKAGE__->meta->make_immutable;
 
 1;

diff --git a/lib/Catalyst/DispatchType/Index.pm b/lib/Catalyst/DispatchType/Index.pm
index 63b864d..bbda89c 100644 (file)
--- a/lib/Catalyst/DispatchType/Index.pm
+++ b/lib/Catalyst/DispatchType/Index.pm
@@ -69,6 +69,7 @@ the same terms as Perl itself.
 
 =cut
 
+no Moose;
 __PACKAGE__->meta->make_immutable;
 
 1;

diff --git a/lib/Catalyst/DispatchType/Path.pm b/lib/Catalyst/DispatchType/Path.pm
index 95cf445..46902c4 100644 (file)
--- a/lib/Catalyst/DispatchType/Path.pm
+++ b/lib/Catalyst/DispatchType/Path.pm
@@ -140,6 +140,7 @@ the same terms as Perl itself.
 
 =cut
 
+no Moose;
 __PACKAGE__->meta->make_immutable;
 
 1;

diff --git a/lib/Catalyst/DispatchType/Regex.pm b/lib/Catalyst/DispatchType/Regex.pm
index ed26885..c417a4c 100644 (file)
--- a/lib/Catalyst/DispatchType/Regex.pm
+++ b/lib/Catalyst/DispatchType/Regex.pm
@@ -161,6 +161,7 @@ the same terms as Perl itself.
 
 =cut
 
+no Moose;
 __PACKAGE__->meta->make_immutable;
 
 1;

diff --git a/lib/Catalyst/Dispatcher.pm b/lib/Catalyst/Dispatcher.pm
index 44f2705..0818f85 100644 (file)
--- a/lib/Catalyst/Dispatcher.pm
+++ b/lib/Catalyst/Dispatcher.pm
@@ -171,10 +171,11 @@ sub forward {
 
     no warnings 'recursion';
 
-    #moose todo: reaching inside another object is bad
-    local $c->request->{arguments} = \@args;
+    my $orig_args = $c->request->arguments();
+    $c->request->arguments(\@args);
     $action->dispatch( $c );
-
+    $c->request->arguments($orig_args);
+    
     return $c->state;
 }
 
@@ -282,7 +283,6 @@ sub prepare_action {
         unshift @args, $arg;
     }
 
-    #Moose todo: This seems illegible, even if efficient.
     s/%([0-9A-Fa-f]{2})/chr(hex($1))/eg for grep { defined } @{$req->captures||[]};
 
     $c->log->debug( 'Path is "' . $req->match . '"' )
@@ -531,6 +531,7 @@ sub _load_dispatch_types {
     return @loaded;
 }
 
+no Moose;
 __PACKAGE__->meta->make_immutable;
 
 =head2 meta

diff --git a/lib/Catalyst/Engine.pm b/lib/Catalyst/Engine.pm
index aa8215c..71773fe 100644 (file)
--- a/lib/Catalyst/Engine.pm
+++ b/lib/Catalyst/Engine.pm
@@ -130,9 +130,6 @@ sub finalize_error {
         # Don't show body parser in the dump
         delete $c->req->{_body};
 
-        # Don't show response header state in dump
-        delete $c->res->{_finalized_headers};
-
         my @infos;
         my $i = 0;
         for my $dump ( $c->dump_these ) {

diff --git a/lib/Catalyst/Exception.pm b/lib/Catalyst/Exception.pm
index 7a59a57..deeb3c0 100644 (file)
--- a/lib/Catalyst/Exception.pm
+++ b/lib/Catalyst/Exception.pm
@@ -71,6 +71,7 @@ BEGIN {
     extends($CATALYST_EXCEPTION_CLASS || 'Catalyst::Exception::Base');
 }
 
-Catalyst::Exception->meta->make_immutable;
+no Moose;
+__PACKAGE__->meta->make_immutable;
 
 1;

diff --git a/lib/Catalyst/Log.pm b/lib/Catalyst/Log.pm
index 726ef7d..1a33c98 100644 (file)
--- a/lib/Catalyst/Log.pm
+++ b/lib/Catalyst/Log.pm
@@ -99,6 +99,9 @@ sub _send_to_log {
     print STDERR @_;
 }
 
+no Moose;
+__PACKAGE__->meta->make_immutable();
+
 1;
 
 __END__

diff --git a/lib/Catalyst/Model.pm b/lib/Catalyst/Model.pm
index 0cd5dd2..deb2d24 100644 (file)
--- a/lib/Catalyst/Model.pm
+++ b/lib/Catalyst/Model.pm
@@ -3,6 +3,9 @@ package Catalyst::Model;
 use Moose;
 extends qw/Catalyst::Component/;
 
+no Moose;
+__PACKAGE__->meta->make_immutable();
+
 =head1 NAME
 
 Catalyst::Model - Catalyst Model base class

diff --git a/lib/Catalyst/Request.pm b/lib/Catalyst/Request.pm
index 099b137..ec7a96a 100644 (file)
--- a/lib/Catalyst/Request.pm
+++ b/lib/Catalyst/Request.pm
@@ -6,6 +6,7 @@ use utf8;
 use URI::http;
 use URI::https;
 use URI::QueryParam;
+use HTTP::Headers;
 
 use Moose;
 
@@ -26,6 +27,9 @@ has headers => (
   is      => 'rw',
   isa     => 'HTTP::Headers',
   handles => [qw(content_encoding content_length content_type header referer user_agent)],
+  default => sub { HTTP::Headers->new() },
+  required => 1,
+  lazy => 1,
 );
 
 has _context => (
@@ -54,7 +58,7 @@ has uploads => (
 
 before uploads => sub {
   my ($self) = @_;
-  $self->_context->prepare_body;
+  #$self->_context->prepare_body;
 };
 
 has parameters => (
@@ -66,7 +70,7 @@ has parameters => (
 
 before parameters => sub {
   my ($self, $params) = @_;
-  $self->_context->prepare_body();
+  #$self->_context->prepare_body();
   if ( $params && !ref $params ) {
     $self->_context->log->warn(
         "Attempt to retrieve '$params' with req->params(), " .

diff --git a/lib/Catalyst/Response.pm b/lib/Catalyst/Response.pm
index cb280dd..2c63ac7 100644 (file)
--- a/lib/Catalyst/Response.pm
+++ b/lib/Catalyst/Response.pm
@@ -1,16 +1,20 @@
 package Catalyst::Response;
 
 use Moose;
+use HTTP::Headers;
 
-has cookies   => (is => 'rw');
-has body      => (is => 'rw');
+has cookies   => (is => 'rw', default => sub { {} });
+has body      => (is => 'rw', default => '');
 has location  => (is => 'rw');
-has status    => (is => 'rw');
+has status    => (is => 'rw', default => 200);
+has finalized_headers => (is => 'rw', default => 0);
 has headers   => (
   is      => 'rw',
   handles => [qw(content_encoding content_length content_type header)],
+  default => sub { HTTP::Headers->new() },
+  required => 1,
+  lazy => 1,
 );
-
 has _context => (
   is => 'rw',
   weak_ref => 1,

diff --git a/lib/Catalyst/Stats.pm b/lib/Catalyst/Stats.pm
index 0c39915..daa5a0e 100644 (file)
--- a/lib/Catalyst/Stats.pm
+++ b/lib/Catalyst/Stats.pm
@@ -120,6 +120,9 @@ sub _get_uid {
     return $visitor->getResult;
 } 
 
+no Moose;
+__PACKAGE__->meta->make_immutable();
+
 1;
 
 __END__

diff --git a/lib/Catalyst/View.pm b/lib/Catalyst/View.pm
index 29964b9..a08bbbf 100644 (file)
--- a/lib/Catalyst/View.pm
+++ b/lib/Catalyst/View.pm
@@ -64,4 +64,7 @@ the same terms as Perl itself.
 
 =cut
 
+no Moose;
+__PACKAGE__->meta->make_immutable();
+
 1;

diff --git a/t/live_engine_request_parameters.t b/t/live_engine_request_parameters.t
index 81c9ba9..4b46009 100644 (file)
--- a/t/live_engine_request_parameters.t
+++ b/t/live_engine_request_parameters.t
@@ -32,7 +32,7 @@ use HTTP::Request::Common;
     ok( eval '$creq = ' . $response->content, 'Unserialize Catalyst::Request' );
     isa_ok( $creq, 'Catalyst::Request' );
     is( $creq->method, 'GET', 'Catalyst::Request method' );
-    is_deeply( $creq->{parameters}, $parameters,
+    is_deeply( $creq->parameters, $parameters,
         'Catalyst::Request parameters' );
 }
 
@@ -43,7 +43,7 @@ use HTTP::Request::Common;
     ok( $response->is_success, 'Response Successful 2xx' );
     is( $response->content_type, 'text/plain', 'Response Content-Type' );
     ok( eval '$creq = ' . $response->content );
-    is $creq->{parameters}->{q}, 'foo+bar', '%2b not double decoded';
+    is $creq->parameters->{q}, 'foo+bar', '%2b not double decoded';
 }
 
 {
@@ -53,7 +53,7 @@ use HTTP::Request::Common;
     ok( $response->is_success, 'Response Successful 2xx' );
     is( $response->content_type, 'text/plain', 'Response Content-Type' );
     ok( eval '$creq = ' . $response->content );
-    is $creq->{parameters}->{q}, 'foo=bar', '= not ignored';
+    is $creq->parameters->{q}, 'foo=bar', '= not ignored';
 }
 
 {
@@ -84,10 +84,10 @@ use HTTP::Request::Common;
     ok( eval '$creq = ' . $response->content, 'Unserialize Catalyst::Request' );
     isa_ok( $creq, 'Catalyst::Request' );
     is( $creq->method, 'POST', 'Catalyst::Request method' );
-    is_deeply( $creq->{parameters}, $parameters,
+    is_deeply( $creq->parameters, $parameters,
         'Catalyst::Request parameters' );
     is_deeply( $creq->arguments, [qw(a b)], 'Catalyst::Request arguments' );
-    is_deeply( $creq->{uploads}, {}, 'Catalyst::Request uploads' );
+    is_deeply( $creq->uploads,   {}, 'Catalyst::Request uploads' );
     is_deeply( $creq->cookies,   {}, 'Catalyst::Request cookie' );
 }
 
@@ -109,7 +109,7 @@ use HTTP::Request::Common;
 
     ok( my $response = request($request), 'Request' );
     ok( eval '$creq = ' . $response->content, 'Unserialize Catalyst::Request' );
-    is_deeply( $creq->{parameters}, $parameters, 'Catalyst::Request parameters' );
+    is_deeply( $creq->parameters, $parameters, 'Catalyst::Request parameters' );
 }
 
 # raw query string support
@@ -129,11 +129,11 @@ use HTTP::Request::Common;
     
     ok( my $response = request($request), 'Request' );
     ok( eval '$creq = ' . $response->content, 'Unserialize Catalyst::Request' );
-    is( $creq->{uri}->query, 'query+string', 'Catalyst::Request POST query_string' );
+    is( $creq->uri->query, 'query+string', 'Catalyst::Request POST query_string' );
     is( $creq->query_keywords, 'query string', 'Catalyst::Request query_keywords' );
-    is_deeply( $creq->{parameters}, $parameters, 'Catalyst::Request parameters' );
+    is_deeply( $creq->parameters, $parameters, 'Catalyst::Request parameters' );
     
     ok( $response = request('http://localhost/dump/request/a/b?x=1&y=1&z=1'), 'Request' );
     ok( eval '$creq = ' . $response->content, 'Unserialize Catalyst::Request' );
-    is( $creq->{uri}->query, 'x=1&y=1&z=1', 'Catalyst::Request GET query_string' );
+    is( $creq->uri->query, 'x=1&y=1&z=1', 'Catalyst::Request GET query_string' );
 }