Merge 'trunk' into 'fix_request_uri'

r13213@tomas-dorans-macbook-pro (orig r13177):  ajgb | 2010-04-21 11:10:51 +0000
Fix not stripping backslashes in DispatchType::Regex::uri_for_action
r15483@tomas-dorans-macbook-pro (orig r13190):  rafl | 2010-04-28 22:54:04 +0000
Make sure path_to returns an instance of the right Path::Class class.
r15484@tomas-dorans-macbook-pro (orig r13191):  edenc | 2010-04-28 23:29:02 +0000
minor documentation fix for handle_request
r15489@tomas-dorans-macbook-pro (orig r13193):  rafl | 2010-05-02 23:16:25 +0000
Allow parameterized roles to be applied as plugins.
r15490@tomas-dorans-macbook-pro (orig r13194):  t0m | 2010-05-02 23:27:43 +0000
Back out crazy heuristics

diff --git a/Changes b/Changes
index 2ec5d69..6052c85 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,9 +1,6 @@
 # This file documents the revision history for Perl extension Catalyst.
 
   Bug fixes:
-   - Additional fix for getting the base application path right when rewriting
-     requests into an application sub path with mod_alias and mod_rewrite on
-     Apache.
    - Ensure to always cleanup temporary uploaded files in all cases, even
      when exceptions occur during request processing, using HTTP::Body's
      ->cleanup feature. (RT#41442)
@@ -11,6 +8,7 @@
      dereferencing it. (RT#49267)
    - Fix regex special characters in REDIRECT_URL variable breaking
      the request base. (2nd part of RT#24951)
+   - Fix not stripping backslashes in DispatchType::Regex::uri_for_action
 
   New features:
    - Setting __PACKAGE__->config(enable_catalyst_header => 1); in your MyApp.pm
@@ -19,6 +17,7 @@
      HttpOnly flag
    - Allow the myapp_test.pl script to be given a list of paths which it
      will retrieve all of. (RT#53653)
+   - Allow parameterized roles to be applied as plugins.
 
   Documentation:
    - The Catalyst::Test::get method is documented as returning the raw

diff --git a/Makefile.PL b/Makefile.PL
index c71fa40..2590316 100644 (file)
--- a/Makefile.PL
+++ b/Makefile.PL
@@ -26,6 +26,7 @@ requires 'Carp';
 requires 'Class::C3::Adopt::NEXT' => '0.07';
 requires 'CGI::Simple::Cookie' => '1.109';
 requires 'Data::Dump';
+requires 'Data::OptList';
 requires 'HTML::Entities';
 requires 'HTTP::Body'    => '1.06'; # ->cleanup(1)
 requires 'HTTP::Headers' => '1.64';

diff --git a/lib/Catalyst.pm b/lib/Catalyst.pm
index 04a8ad4..9f2f836 100644 (file)
--- a/lib/Catalyst.pm
+++ b/lib/Catalyst.pm
@@ -14,6 +14,7 @@ use Catalyst::Request::Upload;
 use Catalyst::Response;
 use Catalyst::Utils;
 use Catalyst::Controller;
+use Data::OptList;
 use Devel::InnerPackage ();
 use File::stat;
 use Module::Pluggable::Object ();
@@ -2782,6 +2783,7 @@ the plugin name does not begin with C<Catalyst::Plugin::>.
         my ( $proto, $plugin, $instant ) = @_;
         my $class = ref $proto || $proto;
 
+        # FIXME: also pass along plugin options as soon as the mop has it
         Class::MOP::load_class( $plugin );
         $class->log->warn( "$plugin inherits from 'Catalyst::Component' - this is decated and will not work in 5.81" )
             if $plugin->isa( 'Catalyst::Component' );
@@ -2797,22 +2799,30 @@ the plugin name does not begin with C<Catalyst::Plugin::>.
         my ( $class, $plugins ) = @_;
 
         $class->_plugins( {} ) unless $class->_plugins;
-        $plugins ||= [];
+        $plugins = Data::OptList::mkopt($plugins || []);
 
-        my @plugins = Catalyst::Utils::resolve_namespace($class . '::Plugin', 'Catalyst::Plugin', @$plugins);
+        my @plugins = map {
+            [ Catalyst::Utils::resolve_namespace(
+                  $class . '::Plugin',
+                  'Catalyst::Plugin', $_->[0]
+              ),
+              $_->[1],
+            ]
+         } @{ $plugins };
 
         for my $plugin ( reverse @plugins ) {
-            Class::MOP::load_class($plugin);
-            my $meta = find_meta($plugin);
+            Class::MOP::load_class($plugin->[0]);
+            # pass along $plugin->[1] as well once cmop supports it
+            my $meta = find_meta($plugin->[0]);
             next if $meta && $meta->isa('Moose::Meta::Role');
 
-            $class->_register_plugin($plugin);
+            $class->_register_plugin($plugin->[0]);
         }
 
         my @roles =
-            map { $_->name }
-            grep { $_ && blessed($_) && $_->isa('Moose::Meta::Role') }
-            map { find_meta($_) }
+            map { $_->[0]->name, $_->[1] }
+            grep { $_->[0] && blessed($_->[0]) && $_->[0]->isa('Moose::Meta::Role') }
+            map { [find_meta($_->[0]), $_->[1]] }
             @plugins;
 
         Moose::Util::apply_all_roles(

diff --git a/lib/Catalyst/DispatchType/Regex.pm b/lib/Catalyst/DispatchType/Regex.pm
index 0d6da04..4b1beae 100644 (file)
--- a/lib/Catalyst/DispatchType/Regex.pm
+++ b/lib/Catalyst/DispatchType/Regex.pm
@@ -151,6 +151,7 @@ sub uri_for_action {
             my $re = "$orig";
             $re =~ s/^\^//;
             $re =~ s/\$$//;
+            $re =~ s/\\([^\\])/$1/g;
             my $final = '/';
             my @captures = @$captures;
             while (my ($front, $rest) = split(/\(/, $re, 2)) {

diff --git a/lib/Catalyst/Engine/CGI.pm b/lib/Catalyst/Engine/CGI.pm
index 318240d..a8a2b1b 100644 (file)
--- a/lib/Catalyst/Engine/CGI.pm
+++ b/lib/Catalyst/Engine/CGI.pm
@@ -158,16 +158,20 @@ sub prepare_path {
         if (my $req_uri = $ENV{REQUEST_URI}) {
             $req_uri =~ s/^\Q$base_path\E//;
             $req_uri =~ s/\?.*$//;
-            if ($req_uri && $req_uri ne '/') {
+            if ($req_uri) {
+                # Note that if REQUEST_URI doesn't start with a /, then the user
+                # is probably using mod_rewrite or something to rewrite requests
+                # into a sub-path of their application..
                 # This means that REQUEST_URI needs information from PATH_INFO
                 # prepending to it to be useful, otherwise the sub path which is
                 # being redirected to becomes the app base address which is
                 # incorrect.
-                # FIXME - This stuff is shit, we should get REDIRECT_URI, right?
-                my ($match) = $req_uri =~ m{^(/?[^/]+)};
-                my ($path_info_part) = $path_info =~ m|^(.*?\Q$match\E)|;
-                substr($req_uri, 0, length($match), $path_info_part)
-                    if $path_info_part;
+                if (substr($req_uri, 0, 1) ne '/') {
+                    my ($match) = $req_uri =~ m|^([^/]+)|;
+                    my ($path_info_part) = $path_info =~ m|^(.*?\Q$match\E)|;
+                    substr($req_uri, 0, length($match), $path_info_part)
+                        if $path_info_part;
+                }
                 $path_info = $req_uri;
             }
         }

diff --git a/t/aggregate/unit_core_engine_cgi-prepare_path.t b/t/aggregate/unit_core_engine_cgi-prepare_path.t
index 01ed6aa..12c21b4 100644 (file)
--- a/t/aggregate/unit_core_engine_cgi-prepare_path.t
+++ b/t/aggregate/unit_core_engine_cgi-prepare_path.t
@@ -87,6 +87,7 @@ use Catalyst::Engine::CGI;
 }
 
 {
+    local $TODO = 'Another mod_rewrite case';
     my $r = get_req (
         PATH_INFO => '/auth/login',
         SCRIPT_NAME => '/tx',

diff --git a/t/aggregate/unit_core_path_to.t b/t/aggregate/unit_core_path_to.t
index a89135c..f4fe89f 100644 (file)
--- a/t/aggregate/unit_core_path_to.t
+++ b/t/aggregate/unit_core_path_to.t
@@ -2,6 +2,9 @@ use strict;
 use warnings;
 
 use Test::More;
+use FindBin;
+use Path::Class;
+use File::Basename;
 
 my %non_unix = (
     MacOS   => 1,
@@ -16,17 +19,20 @@ my %non_unix = (
 
 my $os = $non_unix{$^O} ? $^O : 'Unix';
 
-if(  $os ne 'Unix' ) {
+if ( $os ne 'Unix' ) {
     plan skip_all => 'tests require Unix';
 }
-else {
-    plan tests => 3;
-}
 
 use_ok('Catalyst');
 
 my $context = 'Catalyst';
 
+$context->setup_home;
+my $base = dir($FindBin::Bin)->relative->stringify;
+
+isa_ok( Catalyst::path_to( $context, $base ), 'Path::Class::Dir' );
+isa_ok( Catalyst::path_to( $context, $base, basename $0 ), 'Path::Class::File' );
+
 my $config = Catalyst->config;
 
 $config->{home} = '/home/sri/my-app/';
@@ -37,3 +43,5 @@ $config->{home} = '/Users/sri/myapp/';
 
 is( Catalyst::path_to( $context, 'foo', 'bar' ),
     '/Users/sri/myapp/foo/bar', 'deep Unix path' );
+
+done_testing;

diff --git a/t/aggregate/unit_core_plugin.t b/t/aggregate/unit_core_plugin.t
index 11cef84..16a5e24 100644 (file)
--- a/t/aggregate/unit_core_plugin.t
+++ b/t/aggregate/unit_core_plugin.t
@@ -3,7 +3,7 @@
 use strict;
 use warnings;
 
-use Test::More tests => 24;
+use Test::More;
 
 use lib 't/lib';
 
@@ -61,3 +61,4 @@ my @expected = qw(
 is_deeply [ TestApp->registered_plugins ], \@expected,
   'registered_plugins() should only report the plugins for the current class';
 
+done_testing;

diff --git a/t/aggregate/unit_core_uri_for_action.t b/t/aggregate/unit_core_uri_for_action.t
index 4431f5a..ccc5910 100644 (file)
--- a/t/aggregate/unit_core_uri_for_action.t
+++ b/t/aggregate/unit_core_uri_for_action.t
@@ -8,7 +8,7 @@ use lib "$FindBin::Bin/../lib";
 
 use Test::More;
 
-plan tests => 30;
+plan tests => 33;
 
 use_ok('TestApp');
 
@@ -54,6 +54,21 @@ is($dispatcher->uri_for_action($regex_action, [ 'foo', 123 ]),
    "/action/regexp/foo/123",
    "Regex action interpolates captures correctly");
 
+my $regex_action_bs = $dispatcher->get_action_by_path(
+                     '/action/regexp/one_backslashes'
+                   );
+
+ok(!defined($dispatcher->uri_for_action($regex_action_bs)),
+   "Regex action without captures returns undef");
+
+ok(!defined($dispatcher->uri_for_action($regex_action_bs, [ 1, 2, 3 ])),
+   "Regex action with too many captures returns undef");
+
+is($dispatcher->uri_for_action($regex_action_bs, [ 'foo', 123 ]),
+   "/action/regexp/foo/123.html",
+   "Regex action interpolates captures correctly");
+
+
 #
 #   Index Action
 #

diff --git a/t/lib/PluginTestApp.pm b/t/lib/PluginTestApp.pm
index 1031586..29a02cd 100644 (file)
--- a/t/lib/PluginTestApp.pm
+++ b/t/lib/PluginTestApp.pm
@@ -1,10 +1,13 @@
 package PluginTestApp;
 use Test::More;
 
-use Catalyst qw(
-        Test::Plugin
-        +TestApp::Plugin::FullyQualified
-        );
+use Catalyst (
+    'Test::Plugin',
+    '+TestApp::Plugin::FullyQualified',
+    (eval { require MooseX::Role::Parameterized; 1 }
+        ? ('+TestApp::Plugin::ParameterizedRole' => { method_name => 'affe' })
+        : ()),
+);
 
 sub _test_plugins {
     my $c = shift;

diff --git a/t/lib/PluginTestApp/Controller/Root.pm b/t/lib/PluginTestApp/Controller/Root.pm
index 5358074..7bec366 100644 (file)
--- a/t/lib/PluginTestApp/Controller/Root.pm
+++ b/t/lib/PluginTestApp/Controller/Root.pm
@@ -34,6 +34,13 @@ sub run_time_plugins : Local {
     ref($c)->plugin( faux => $faux_plugin );
 
     isa_ok $c, 'Catalyst::Plugin::Test::Plugin';
+
+    # applied parameterized role
+    if (eval { require MooseX::Role::Parameterized; 1 }) {
+        can_ok $c, 'affe';
+        is $c->affe, 'birne', 'right method created by parameterized role';
+    }
+
     isa_ok $c, 'TestApp::Plugin::FullyQualified';
     ok !$c->isa($faux_plugin),
     '... and it should not inherit from the instant plugin';

diff --git a/t/lib/TestApp/Controller/Action/Regexp.pm b/t/lib/TestApp/Controller/Action/Regexp.pm
index 4b59d58..7966874 100644 (file)
--- a/t/lib/TestApp/Controller/Action/Regexp.pm
+++ b/t/lib/TestApp/Controller/Action/Regexp.pm
@@ -27,4 +27,9 @@ sub four : Action Regex('^action/regexp/redirect/(\w+)/universe/(\d+)/everything
     );
 }
 
+sub one_backslashes : Action Regex('^action/regexp/(\w+)/(\d+)\.html$') {
+    my ( $self, $c ) = @_;
+    $c->forward('TestApp::View::Dump::Request');
+}
+
 1;