Merge 'namespace_handling_refactor' into 'trunk'
Rafael Kitover [Thu, 25 Jun 2009 09:49:35 +0000 (09:49 +0000)]
r5367@hlagh (orig r10459):  t0m | 2009-06-06 05:56:50 -0700
Branch for mo namespace handling refactor

r5368@hlagh (orig r10460):  mo | 2009-06-06 06:33:53 -0700
refactor of namespace handling
r5369@hlagh (orig r10461):  mo | 2009-06-06 06:34:42 -0700
TODO fix
r5371@hlagh (orig r10466):  mo | 2009-06-07 04:14:23 -0700
controller actions without attributes which are defined via config
r5372@hlagh (orig r10467):  mo | 2009-06-07 04:49:58 -0700
removed commented code
r5373@hlagh (orig r10468):  mo | 2009-06-07 05:28:53 -0700
Added support for ~ prefix to plugins and action classes
r5374@hlagh (orig r10469):  mo | 2009-06-07 05:36:55 -0700
Changes patches
r5686@hlagh (orig r10655):  caelum | 2009-06-24 19:22:27 -0700
cleanup TODO

Changes
Makefile.PL
TODO
lib/Catalyst.pm
lib/Catalyst/Controller.pm
lib/Catalyst/Dispatcher.pm
lib/Catalyst/Utils.pm
t/aggregate/live_component_controller_action_action.t
t/aggregate/live_component_controller_action_path.t
t/lib/TestApp/Controller/Action/Action.pm
t/lib/TestApp/Controller/Action/Path.pm

diff --git a/Changes b/Changes
index 98c8727..3b28c02 100644 (file)
--- a/Changes
+++ b/Changes
            ensuring child classes don't inherit their parent's config,
            except works correctly with closures.
 
+  New features:
+        -  Use ~ as prefix for plugins or action classes which are located in 
+           MyApp::Plugin / MyApp::Action (mo)
+        -  Controller methods without attributes are now considered actions if 
+           they are specified in config->{action(s)} (mo)
+
 5.80005 2009-06-06 14:40:00
 
   Behaviour changes:
index a1b635a..71898e1 100644 (file)
@@ -34,6 +34,7 @@ requires 'Tree::Simple::Visitor::FindByPath';
 requires 'URI' => '1.35';
 requires 'Text::Balanced'; # core in 5.8.x but mentioned for completeness
 requires 'MRO::Compat';
+requires 'String::RewritePrefix' => '0.004'; # Catalyst::Utils::resolve_namespace
 
 recommends 'B::Hooks::OP::Check::StashChange';
 
diff --git a/TODO b/TODO
index 8c5bc68..7d34863 100644 (file)
--- a/TODO
+++ b/TODO
@@ -22,4 +22,15 @@ Proposed functionality / feature additions:
       See also: Catalyst::Plugin::Log::Dispatch and
       http://github.com/willert/catalyst-plugin-log4perl-simple/tree
 
+TODO for brach namespace_handling_refactor:
 
+- refactor code in
+  * Catalyst::Dispatcher::get_containers           # No Idea
+  * Catalyst::Dispatcher::dispatch_type            # DONE
+
+  * Catalyst::Controller::_parse_ActionClass_attr  # DONE
+  * Catalyst::Dispatcher::_load_dispatch_types     # DONE
+  * Catalyst::setup_plugins                        # DONE
+  to use the same namespacing method
+
+- Ok, so can you add tests for ->config(actions => { foo => { ActionClass => '+Bar' }});
index f9ab9c4..063ee32 100644 (file)
@@ -2509,8 +2509,8 @@ the plugin name does not begin with C<Catalyst::Plugin::>.
 
         $class->_plugins( {} ) unless $class->_plugins;
         $plugins ||= [];
-
-        my @plugins = map { s/\A\+// ? $_ : "Catalyst::Plugin::$_" } @$plugins;
+                
+        my @plugins = Catalyst::Utils::resolve_namespace($class . '::Plugin', 'Catalyst::Plugin', @$plugins);
 
         for my $plugin ( reverse @plugins ) {
             Class::MOP::load_class($plugin);
index dd1dca0..45feacb 100644 (file)
@@ -178,13 +178,28 @@ around path_prefix => sub {
 sub get_action_methods {
     my $self = shift;
     my $meta = find_meta($self);
-    confess("Metaclass for " . ref($meta) ." for " . $meta->name
-        . " cannot support register_actions.")
-        unless $meta->can('get_nearest_methods_with_attributes');
+    confess("Metaclass for "
+          . ref($meta) . " for "
+          . $meta->name
+          . " cannot support register_actions." )
+      unless $meta->can('get_nearest_methods_with_attributes');
     my @methods = $meta->get_nearest_methods_with_attributes;
+
+    # actions specified via config are also action_methods
+    push(
+        @methods,
+        map {
+            $meta->get_method($_)
+              || confess( 'Action "' 
+                  . $_
+                  . '" is not available from controller '
+                  . ( ref $self ) )
+          } keys %{ $self->_controller_actions }
+    ) if ( ref $self );
     return @methods;
 }
 
+
 sub register_actions {
     my ( $self, $c ) = @_;
     $self->register_action_methods( $c, $self->get_action_methods );
@@ -199,7 +214,6 @@ sub register_action_methods {
     foreach my $method (@methods) {
         my $name = $method->name;
         my $attributes = $method->attributes;
-        next unless $attributes;
         my $attrs = $self->_parse_attrs( $c, $name, @{ $attributes } );
         if ( $attrs->{Private} && ( keys %$attrs > 1 ) ) {
             $c->log->debug( 'Bad action definition "'
@@ -376,9 +390,8 @@ sub _parse_PathPrefix_attr {
 
 sub _parse_ActionClass_attr {
     my ( $self, $c, $name, $value ) = @_;
-    unless ( $value =~ s/^\+// ) {
-      $value = join('::', $self->_action_class, $value );
-    }
+    my $appname = $self->_application;
+    $value = Catalyst::Utils::resolve_namespace($appname . '::Action', $self->_action_class, $value);
     return ( 'ActionClass', $value );
 }
 
index 9249f1c..be7787a 100644 (file)
@@ -654,12 +654,11 @@ sub _load_dispatch_types {
     my ( $self, @types ) = @_;
 
     my @loaded;
-
     # Preload action types
     for my $type (@types) {
-        my $class =
-          ( $type =~ /^\+(.*)$/ ) ? $1 : "Catalyst::DispatchType::${type}";
-
+        # first param is undef because we cannot get the appclass
+        my $class = Catalyst::Utils::resolve_namespace(undef, 'Catalyst::DispatchType', $type);
+        
         eval { Class::MOP::load_class($class) };
         Catalyst::Exception->throw( message => qq/Couldn't load "$class"/ )
           if $@;
@@ -681,10 +680,9 @@ of course it's being used.)
 
 sub dispatch_type {
     my ($self, $name) = @_;
-
-    unless ($name =~ s/^\+//) {
-        $name = "Catalyst::DispatchType::" . $name;
-    }
+    
+    # first param is undef because we cannot get the appclass
+    $name = Catalyst::Utils::resolve_namespace(undef, 'Catalyst::DispatchType', $name);
 
     for (@{ $self->_dispatch_types }) {
         return $_ if ref($_) eq $name;
index f47909d..5bff574 100644 (file)
@@ -9,6 +9,8 @@ use URI;
 use Carp qw/croak/;
 use Cwd;
 
+use String::RewritePrefix;
+
 use namespace::clean;
 
 =head1 NAME
@@ -377,6 +379,28 @@ sub term_width {
     return $_term_width = $width;
 }
 
+
+=head2 resolve_namespace
+
+Method which adds the namespace for plugins and actions.
+
+  __PACKAGE__->setup(qw(MyPlugin));
+  
+  # will load Catalyst::Plugin::MyPlugin
+
+=cut
+
+
+sub resolve_namespace {
+    my $appnamespace = shift;
+    my $namespace = shift;
+    my @classes = @_;
+    return String::RewritePrefix->rewrite(
+        { '' => $namespace.'::', '+' => '', '~' => $appnamespace . '::' }, @classes,
+      );
+}
+
+
 =head1 AUTHORS
 
 Catalyst Contributors, see Catalyst.pm
index ea52e78..1e11b18 100644 (file)
@@ -10,7 +10,7 @@ our $iters;
 
 BEGIN { $iters = $ENV{CAT_BENCH_ITERS} || 1; }
 
-use Test::More tests => 28 * $iters;
+use Test::More tests => 42 * $iters;
 use Catalyst::Test 'TestApp';
 
 if ( $ENV{CAT_BENCHMARK} ) {
@@ -106,5 +106,46 @@ sub run_tests {
             'Content is a serialized Catalyst::Request'
         );
     }
+    
+    {
+        ok( my $response = request('http://localhost/action_action_five'),
+            'Request' );
+        ok( $response->is_success, 'Response Successful 2xx' );
+        is( $response->content_type, 'text/plain', 'Response Content-Type' );
+        is( $response->header('X-Catalyst-Action'),
+            'action_action_five', 'Test Action' );
+        is(
+            $response->header('X-Test-Class'),
+            'TestApp::Controller::Action::Action',
+            'Test Class'
+        );
+        is( $response->header('X-Action'), 'works' );
+        like(
+            $response->content,
+            qr/^bless\( .* 'Catalyst::Request' \)$/s,
+            'Content is a serialized Catalyst::Request'
+        );
+    }
+    
+
+    {
+        ok( my $response = request('http://localhost/action_action_six'),
+            'Request' );
+        ok( $response->is_success, 'Response Successful 2xx' );
+        is( $response->content_type, 'text/plain', 'Response Content-Type' );
+        is( $response->header('X-Catalyst-Action'),
+            'action_action_six', 'Test Action' );
+        is(
+            $response->header('X-Test-Class'),
+            'TestApp::Controller::Action::Action',
+            'Test Class'
+        );
+        is( $response->header('X-TestAppActionTestMyAction'), 'MyAction works' );
+        like(
+            $response->content,
+            qr/^bless\( .* 'Catalyst::Request' \)$/s,
+            'Content is a serialized Catalyst::Request'
+        );
+    }
 
 }
index 338e696..950b4ac 100644 (file)
@@ -10,7 +10,7 @@ our $iters;
 
 BEGIN { $iters = $ENV{CAT_BENCH_ITERS} || 1; }
 
-use Test::More tests => 36*$iters;
+use Test::More tests => 42*$iters;
 use Catalyst::Test 'TestApp';
 
 if ( $ENV{CAT_BENCHMARK} ) {
@@ -142,4 +142,22 @@ sub run_tests {
             'Content is a serialized Catalyst::Request'
         );
     }
+
+    {
+        ok( my $response = request('http://localhost/action/path/six'), 'Request' );
+        ok( $response->is_success, 'Response Successful 2xx' );
+        is( $response->content_type, 'text/plain', 'Response Content-Type' );
+        is( $response->header('X-Catalyst-Action'),
+            'action/path/six', 'Test Action' );
+        is(
+            $response->header('X-Test-Class'),
+            'TestApp::Controller::Action::Path',
+            'Test Class'
+        );
+        like(
+            $response->content,
+            qr/^bless\( .* 'Catalyst::Request' \)$/s,
+            'Content is a serialized Catalyst::Request'
+        );
+    }
 }
index 787bb8d..5049427 100644 (file)
@@ -3,6 +3,8 @@ package TestApp::Controller::Action::Action;
 use strict;
 use base 'TestApp::Controller::Action';
 
+__PACKAGE__->config( actions => { action_action_five => { ActionClass => '+Catalyst::Action::TestBefore' } } );
+
 sub action_action_one : Global : ActionClass('TestBefore') {
     my ( $self, $c ) = @_;
     $c->res->header( 'X-Action', $c->stash->{test} );
@@ -25,4 +27,15 @@ sub action_action_four : Global : MyAction('TestMyAction') {
     $c->forward('TestApp::View::Dump::Request');
 }
 
+sub action_action_five : Global {
+    my ( $self, $c ) = @_;
+    $c->res->header( 'X-Action', $c->stash->{test} );
+    $c->forward('TestApp::View::Dump::Request');
+}
+
+sub action_action_six : Global : ActionClass('~TestMyAction') {
+    my ( $self, $c ) = @_;
+    $c->forward('TestApp::View::Dump::Request');
+}
+
 1;
index 3a93525..18fa71b 100644 (file)
@@ -7,6 +7,7 @@ __PACKAGE__->config(
     actions => {
       'one' => { 'Path' => [ 'a path with spaces' ] },
       'two' => { 'Path' => "åäö" },
+      'six' => { 'Local' => undef },
     },
 );
 
@@ -35,4 +36,9 @@ sub five : Path( "spaces_near_parens_doubleq" ) {
     $c->forward('TestApp::View::Dump::Request');
 }
 
+sub six {
+    my ( $self, $c ) = @_;
+    $c->forward('TestApp::View::Dump::Request');
+}
+
 1;