Fix the SmartURI recursion using a predicate and un-skip test

diff --git a/Changes b/Changes
index 99da146..51c2089 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,6 +1,10 @@
 # This file documents the revision history for Perl extension Catalyst.
 
 5.8000_04
+        - Use a predicate to avoid recursion in cases where the uri
+          method is overridden by a plugin, and calls the base method,
+          for example Catalyst::Plugin::SmartURI (t0m)
+          - Test for this (caelum)
         - Compose the MooseX::Emulate::Class::Accessor::Fast role to 
           Catalyst::Action, Catalyst::Request, and all other modules which 
           inherit from Class::Accessor::Fast in 5.70.

diff --git a/TODO b/TODO
index 318654a..d101589 100644 (file)
--- a/TODO
+++ b/TODO
@@ -1,8 +1,5 @@
 TBD pre 5.8000_04 dev release:
 
-  - Go through everything which used to use CAF, and ensure that it now uses MX::E::CAF + tests, 
-    extending t/caf_backcompat.t and replacing t/custom_request.t (t0m)
-
   - Simple test for NEXT compat in core (t0m).
 
   - Looks like stash is not available during prepare_path when running under
@@ -12,27 +9,6 @@ TBD pre 5.8000_04 dev release:
 
 ---
 
-  - Make the skipped test at the bottom of t/aggregate/live_engine_request_uri.t
-    pass / not be skipped. (From what C::P::SmartURI used to do)
-
-    This can be fixed by the following patch:
-
-Index: lib/Catalyst/Request.pm
-===================================================================
---- lib/Catalyst/Request.pm (revision 8709)
-+++ lib/Catalyst/Request.pm (working copy)
-@@ -96,7 +96,7 @@
-   lazy => 1,
-   default => sub {
-     my $self = shift;
--    return $self->path if $self->uri;
-+    return $self->{path} if $self->{uri};
-   },
- );
-
-    But I'd like a 2nd opinion from someone who knows core better than me
-    about if that is the correct fix.. (t0m / Caelum)
-
   - Common engine test failures, look into and get tests into core.
 
   - Catalyst-Plugin-Authorization-ACL, Can't locate object method "tree" via package "Catalyst::Dispatcher", fix the plugin as tree was never a public method.

diff --git a/lib/Catalyst/Request.pm b/lib/Catalyst/Request.pm
index c49d283..595c827 100644 (file)
--- a/lib/Catalyst/Request.pm
+++ b/lib/Catalyst/Request.pm
@@ -23,7 +23,7 @@ has protocol => (is => 'rw');
 has query_parameters  => (is => 'rw', default => sub { {} });
 has secure => (is => 'rw', default => 0);
 has captures => (is => 'rw', default => sub { [] });
-has uri => (is => 'rw');
+has uri => (is => 'rw', predicate => 'has_uri');
 has user => (is => 'rw');
 has headers => (
   is      => 'rw',
@@ -96,7 +96,7 @@ has base => (
   lazy => 1,
   default => sub {
     my $self = shift;
-    return $self->path if $self->uri;
+    return $self->path if $self->has_uri;
   },
 );
 

diff --git a/t/aggregate/live_engine_request_uri.t b/t/aggregate/live_engine_request_uri.t
index ee6fdfd..1722ccb 100644 (file)
--- a/t/aggregate/live_engine_request_uri.t
+++ b/t/aggregate/live_engine_request_uri.t
@@ -151,8 +151,8 @@ SKIP:
     is( $response->header( 'X-Catalyst-Param-c' ), '1', 'param "c" ok' );
 }
 
-SKIP: {
-    skip 'This currently causes infinite recursion', 2;
+# Test an overridden uri method which calls the base method, SmartURI does this.
+{
     require TestApp::RequestBaseBug;
     TestApp->request_class('TestApp::RequestBaseBug');
     ok( my $response = request('http://localhost/engine/request/uri'), 'Request' );