fix RT#42025 by making C::Request::REST less unfriendly

diff --git a/Changes b/Changes
index e6d5887..f221a5c 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,6 +1,7 @@
 ??? ??? ?? ??:??:?? ??? ???? (???) - Release 0.67
   Fix RT#43840, improper app-level config handling - hdp
   Fix RT#42859, 'wrong' Catalyst dependency - hdp
+  Fix RT#42025, stepping on custom request classes - hdp
 
 Wed Aug 20 10:42:00 PST 2008 (jshirley) - Release 0.65
   Fully revamped tests to work without any JSON support

diff --git a/lib/Catalyst/Action/REST.pm b/lib/Catalyst/Action/REST.pm
index 79e8ef3..a1254a0 100644 (file)
--- a/lib/Catalyst/Action/REST.pm
+++ b/lib/Catalyst/Action/REST.pm
@@ -20,11 +20,12 @@ BEGIN { require 5.008001; }
 
 our $VERSION = '0.67';
 
-# This is wrong in several ways. First, there's no guarantee that
-# Catalyst.pm has not been subclassed. Two, there's no guarantee that
-# the user isn't already using their request subclass.
-Catalyst->request_class('Catalyst::Request::REST')
-  unless Catalyst->request_class->isa('Catalyst::Request::REST');
+sub new {
+  my $class  = shift;
+  my $config = shift;
+  Catalyst::Request::REST->_insert_self_into($config->{class});
+  return $class->SUPER::new($config, @_);
+}
 
 =head1 NAME
 

diff --git a/lib/Catalyst/Action/SerializeBase.pm b/lib/Catalyst/Action/SerializeBase.pm
index ef46070..5155140 100644 (file)
--- a/lib/Catalyst/Action/SerializeBase.pm
+++ b/lib/Catalyst/Action/SerializeBase.pm
@@ -13,9 +13,16 @@ use base 'Catalyst::Action';
 use Module::Pluggable::Object;
 use Data::Dump qw(dump);
 use Catalyst::Request::REST;
-
-Catalyst->request_class('Catalyst::Request::REST')
-    unless Catalyst->request_class->isa('Catalyst::Request::REST');
+use Catalyst::Utils ();
+
+sub new {
+  my $class  = shift;
+  my $config = shift;
+  Catalyst::Request::REST->_insert_self_into(
+    Catalyst::Utils::class2appclass($config->{class})
+  );
+  return $class->SUPER::new($config, @_);
+}
 
 __PACKAGE__->mk_accessors(qw(_serialize_plugins _loaded_plugins));
 

diff --git a/lib/Catalyst/Request/REST.pm b/lib/Catalyst/Request/REST.pm
index 447680e..795614c 100644 (file)
--- a/lib/Catalyst/Request/REST.pm
+++ b/lib/Catalyst/Request/REST.pm
@@ -13,6 +13,17 @@ use warnings;
 use base qw/Catalyst::Request Class::Accessor::Fast/;
 use HTTP::Headers::Util qw(split_header_words);
 
+sub _insert_self_into {
+  my ($class, $app) = @_;
+  my $req_class = $app->request_class;
+  return if $req_class->isa($class);
+  if ($req_class eq 'Catalyst::Request') {
+    $app->request_class($class);
+  } else {
+    die "$app has a custom request class $req_class, "
+      . "which is not a $class; see Catalyst::Request::REST";
+  }
+}
 
 =head1 NAME
 
@@ -32,6 +43,11 @@ This is a subclass of C<Catalyst::Request> that adds a few methods to
 the request object to faciliate writing REST-y code. Currently, these
 methods are all related to the content types accepted by the client.
 
+Note that if you have a custom request class in your application, and it does
+not inherit from C<Catalyst::Request::REST>, your application will fail with an
+error indicating a conflict the first time it tries to use
+C<Catalyst::Request::REST>'s functionality.  To fix this error, make sure your
+custom request class inherits from C<Catalyst::Request::REST>.
 
 =head1 METHODS
 

diff --git a/t/catalyst-request-rest.t b/t/catalyst-request-rest.t
index 98b3517..92ae166 100644 (file)
--- a/t/catalyst-request-rest.t
+++ b/t/catalyst-request-rest.t
@@ -1,8 +1,8 @@
 use strict;
 use warnings;
-use Test::More tests => 24;
+use Test::More tests => 28;
 use FindBin;
-use lib ( "$FindBin::Bin/../lib" );
+use lib ( "$FindBin::Bin/../lib", "$FindBin::Bin/../t/lib" );
 
 use Catalyst::Request::REST;
 use HTTP::Headers;
@@ -167,6 +167,25 @@ use HTTP::Headers;
                'each type appears only once' );
 }
 
+{
+  my $test = 'Test::Catalyst::Action::REST';
+  use_ok $test;
+  is($test->request_class, 'Catalyst::Request::REST',
+    'Request::REST took over for Request');
+
+  $test->request_class('Some::Other::Class');
+  eval { $test->setup };
+  like $@, qr/$test has a custom request class Some::Other::Class/;
+
+  {
+    package My::Request;
+    use base 'Catalyst::Request::REST';
+  }
+  $test->request_class('My::Request');
+  eval { $test->setup };
+  is $@, '', 'no error from Request::REST subclass';
+}
+
 package MockContext;
 
 sub prepare_body { }