[Catalyst-commits] r8721 - in Catalyst-Runtime/5.80/trunk: . lib/Catalyst t/aggregate

[t0m at dev.catalyst.perl.org]

Author: t0m
Date: 2008-12-04 22:48:12 +0000 (Thu, 04 Dec 2008)
New Revision: 8721

Modified:
   Catalyst-Runtime/5.80/trunk/Changes
   Catalyst-Runtime/5.80/trunk/TODO
   Catalyst-Runtime/5.80/trunk/lib/Catalyst/Request.pm
   Catalyst-Runtime/5.80/trunk/t/aggregate/live_engine_request_uri.t
Log:
Fix the SmartURI recursion using a predicate and un-skip test

Modified: Catalyst-Runtime/5.80/trunk/Changes
===================================================================
--- Catalyst-Runtime/5.80/trunk/Changes	2008-12-04 21:28:55 UTC (rev 8720)
+++ Catalyst-Runtime/5.80/trunk/Changes	2008-12-04 22:48:12 UTC (rev 8721)
@@ -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.

Modified: Catalyst-Runtime/5.80/trunk/TODO
===================================================================
--- Catalyst-Runtime/5.80/trunk/TODO	2008-12-04 21:28:55 UTC (rev 8720)
+++ Catalyst-Runtime/5.80/trunk/TODO	2008-12-04 22:48:12 UTC (rev 8721)
@@ -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 @@
 
 ---
 
-  - 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.

Modified: Catalyst-Runtime/5.80/trunk/lib/Catalyst/Request.pm
===================================================================
--- Catalyst-Runtime/5.80/trunk/lib/Catalyst/Request.pm	2008-12-04 21:28:55 UTC (rev 8720)
+++ Catalyst-Runtime/5.80/trunk/lib/Catalyst/Request.pm	2008-12-04 22:48:12 UTC (rev 8721)
@@ -23,7 +23,7 @@
 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 @@
   lazy => 1,
   default => sub {
     my $self = shift;
-    return $self->path if $self->uri;
+    return $self->path if $self->has_uri;
   },
 );
 

Modified: Catalyst-Runtime/5.80/trunk/t/aggregate/live_engine_request_uri.t
===================================================================
--- Catalyst-Runtime/5.80/trunk/t/aggregate/live_engine_request_uri.t	2008-12-04 21:28:55 UTC (rev 8720)
+++ Catalyst-Runtime/5.80/trunk/t/aggregate/live_engine_request_uri.t	2008-12-04 22:48:12 UTC (rev 8721)
@@ -151,8 +151,8 @@
     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' );