Making the default serializer be specified by content-type, not module.

diff --git a/Changelog b/Changelog
index 3997004..5255c63 100644 (file)
--- a/Changelog
+++ b/Changelog
@@ -1,12 +1,17 @@
+Wed Jul 04 11:17:20 EDT 2007
+       Fixed 'default' serializer to set a valid Content-Type: header.  Fixes
+       RT ticket 27949.  Note that behavior has changed -- the default
+       serializer must now be specified as a content-type, not as a plugin
+       name. (dmo@roaringpenguin.com)
+
 Thu May 24 14:01:06 PDT 2007 (adam) - Release 0.41
-  Moved a bogus $self->class to $c->component($self->class)
+       Moved a bogus $self->class to $c->component($self->class)
 
 Fri Mar  9 14:13:29 PST 2007 (adam) - Release 0.40
-  Refactored the Content-Type negotiation to live in Catalyst::Request::REST. 
-    (drolsky)
-  Added some useful debugging. (drolsky)
-  Added a View serializer/deserializer, which simply calls the correct 
-    Catalyst view.  ('text/html' => [ 'View', 'TT' ]) (claco, adam)
+       Refactored the Content-Type negotiation to live in Catalyst::Request::REST.  (drolsky)
+       Added some useful debugging. (drolsky)
+       Added a View serializer/deserializer, which simply calls the correct
+       Catalyst view.  ('text/html' => [ 'View', 'TT' ]) (claco, adam)
 
 Wed Dec  6 00:45:02 PST 2006 (adam) - Release 0.31
        Fixed a bug where we would report a blank content-type negotiation.

diff --git a/README b/README
index 8da598b..172ac8e 100644 (file)
--- a/README
+++ b/README
@@ -140,9 +140,10 @@ AVAILABLE SERIALIZERS
     made. You can ensure that something is always returned by setting the
     "default" config option:
 
-       __PACKAGE__->config->{'serialize'}->{'default'} = 'YAML';
+       __PACKAGE__->config->{'serialize'}->{'default'} = 'text/x-yaml';
 
-    Would make it always fall back to YAML.
+    Would make it always fall back to the serializer plugin defined for
+    text/x-yaml.
 
     Implementing new Serialization formats is easy! Contributions are most
     welcome! See Catalyst::Action::Serialize and

diff --git a/lib/Catalyst/Action/Deserialize.pm b/lib/Catalyst/Action/Deserialize.pm
index c45d86b..f89447f 100644 (file)
--- a/lib/Catalyst/Action/Deserialize.pm
+++ b/lib/Catalyst/Action/Deserialize.pm
@@ -53,7 +53,7 @@ Catalyst::Action::Deserialize - Deserialize Data in a Request
 
     __PACKAGE__->config(
         serialize => {
-            'default'   => 'YAML',
+            'default'   => 'text/x-yaml',
             'stash_key' => 'rest',
             'map'       => {
                 'text/x-yaml'        => 'YAML',

diff --git a/lib/Catalyst/Action/Serialize.pm b/lib/Catalyst/Action/Serialize.pm
index 2ec7add..a55d531 100644 (file)
--- a/lib/Catalyst/Action/Serialize.pm
+++ b/lib/Catalyst/Action/Serialize.pm
@@ -66,10 +66,10 @@ Catalyst::Action::Serialize - Serialize Data in a Response
 
     __PACKAGE__->config(
         serialize => {
-            'default'   => 'YAML',
+            'default'   => 'text/x-yaml',
             'stash_key' => 'rest',
             'map'       => {
-                               'text/html'          => [ 'View', 'TT', ],
+                'text/html'          => [ 'View', 'TT', ],
                 'text/x-yaml'        => 'YAML',
                 'text/x-data-dumper' => [ 'Data::Serializer', 'Data::Dumper' ],
             },
@@ -106,9 +106,10 @@ L<Catalyst::Request::REST>.
 
 =item default
 
-The default Serialization format.  See the next section for
-available options.  This is used if a requested content-type
-is not recognized.
+The Content-Type of the default Serialization format.  This must be a
+Content-Type associated with a plugin in the "map" section below.  
+
+This is used if a requested content-type is not recognized.
 
 =item stash_key 
 

diff --git a/lib/Catalyst/Action/SerializeBase.pm b/lib/Catalyst/Action/SerializeBase.pm
index 2d15713..546f49a 100644 (file)
--- a/lib/Catalyst/Action/SerializeBase.pm
+++ b/lib/Catalyst/Action/SerializeBase.pm
@@ -44,6 +44,17 @@ sub _load_content_plugins {
     my $sclass = $search_path . "::";
     my $sarg;
     my $map = $controller->config->{'serialize'}->{'map'};
+
+    # If we don't have a handler for our preferred content type, try
+    # the default
+    if ( ! exists $map->{$content_type} ) {
+        if( exists $controller->config->{'serialize'}->{'default'} ) {
+            $content_type = $controller->config->{'serialize'}->{'default'} ;
+        } else {
+            return $self->_unsupported_media_type($c, $content_type);
+        }
+    }
+
     if ( exists( $map->{$content_type} ) ) {
         my $mc;
         if ( ref( $map->{$content_type} ) eq "ARRAY" ) {
@@ -64,11 +75,7 @@ sub _load_content_plugins {
             return $self->_unsupported_media_type($c, $content_type);
         }
     } else {
-        if ( exists( $controller->config->{'serialize'}->{'default'} ) ) {
-            $sclass .= $controller->config->{'serialize'}->{'default'};
-        } else {
-            return $self->_unsupported_media_type($c, $content_type);
-        }
+        return $self->_unsupported_media_type($c, $content_type);
     }
     unless ( exists( $self->_loaded_plugins->{$sclass} ) ) {
         my $load_class = $sclass;

diff --git a/lib/Catalyst/Controller/REST.pm b/lib/Catalyst/Controller/REST.pm
index 9547326..3761f07 100644 (file)
--- a/lib/Catalyst/Controller/REST.pm
+++ b/lib/Catalyst/Controller/REST.pm
@@ -170,13 +170,14 @@ Will do the trick nicely.
 
 =back
 
-By default, L<Catalyst::Controller::REST> will return a C<415 Unsupported Media Type> response if an attempt to use an unsupported content-type is made.  You
-can ensure that something is always returned by setting the C<default> config
-option:
+By default, L<Catalyst::Controller::REST> will return a C<415 Unsupported Media Type>
+response if an attempt to use an unsupported content-type is made.  You
+can ensure that something is always returned by setting the C<default>
+config option:
 
-   __PACKAGE__->config->{'serialize'}->{'default'} = 'YAML';
+   __PACKAGE__->config->{'serialize'}->{'default'} = 'text/x-yaml';
 
-Would make it always fall back to YAML.
+Would make it always fall back to the serializer plugin defined for text/x-yaml.
 
 Implementing new Serialization formats is easy!  Contributions
 are most welcome!  See L<Catalyst::Action::Serialize> and

diff --git a/t/catalyst-action-serialize-accept.t b/t/catalyst-action-serialize-accept.t
index b67062e..e91c27e 100644 (file)
--- a/t/catalyst-action-serialize-accept.t
+++ b/t/catalyst-action-serialize-accept.t
@@ -14,6 +14,7 @@ use Catalyst;
 __PACKAGE__->config(
     name => 'Test::Catalyst::Action::Serialize',
     serialize => {
+       'default'   => 'text/x-yaml',
         'stash_key' => 'rest',
         'map'       => {
             'text/x-yaml'        => 'YAML',
@@ -43,7 +44,7 @@ package main;
 
 use strict;
 use warnings;
-use Test::More tests => 3;
+use Test::More tests => 7;
 use Data::Serializer;
 use FindBin;
 use Data::Dump qw(dump);
@@ -56,15 +57,31 @@ my $t = Test::Rest->new('content_type' => 'text/x-yaml');
 
 use_ok 'Catalyst::Test', 'Test::Catalyst::Action::Serialize';
 
-my $req = $t->get(url => '/test');
-$req->remove_header('Content-Type');
-$req->header('Accept', 'text/x-yaml');
-my $res = request($req);
-ok( $res->is_success, 'GET the serialized request succeeded' );
 my $data = <<EOH;
 --- 
 lou: is my cat
 EOH
-is( $res->content, $data, "Request returned proper data");
+
+{
+       my $req = $t->get(url => '/test');
+       $req->remove_header('Content-Type');
+       $req->header('Accept', 'text/x-yaml');
+       my $res = request($req);
+       ok( $res->is_success, 'GET the serialized request succeeded' );
+       is( $res->content, $data, "Request returned proper data");
+       is( $res->header('Content-type'), 'text/x-yaml', '... with expected content-type')
+}
+
+# Make sure we don't get a bogus content-type when using default
+# serializer (rt.cpan.org ticket 27949)
+{
+       my $req = $t->get(url => '/test');
+       $req->remove_header('Content-Type');
+       $req->header('Accept', '*/*');
+       my $res = request($req);
+       ok( $res->is_success, 'GET the serialized request succeeded' );
+       is( $res->content, $data, "Request returned proper data");
+       is( $res->header('Content-type'), 'text/x-yaml', '... with expected content-type')
+}
 
 1;