From 2ad273f58cdd387ee23b09d03a7f42ada8e0d563 Mon Sep 17 00:00:00 2001
From: Dave O'Neill <dmo@roaringpenguin.com>
Date: Wed, 4 Jul 2007 11:25:00 -0400
Subject: [PATCH] Correctly set Content-type: header when using default serializer.
This changes the {serialize}->{default} config setting to take the name of a
valid content-type defined in {serialize}->{map}, rather than a plugin name.
This gives us enough information to set the Content-type: header appropriately
when serializing.
Fixes rt.cpan.org 27949
---
Changelog | 17 +++++++++++------
README | 5 +++--
lib/Catalyst/Action/Deserialize.pm | 2 +-
lib/Catalyst/Action/Serialize.pm | 11 ++++++-----
lib/Catalyst/Action/SerializeBase.pm | 17 ++++++++++++-----
lib/Catalyst/Controller/REST.pm | 11 ++++++-----
t/catalyst-action-serialize-accept.t | 31 ++++++++++++++++++++++++-------
7 files changed, 63 insertions(+), 31 deletions(-)
diff --git a/Changelog b/Changelog
index 3997004..5255c63 100644
--- 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
--- 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
--- 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
--- 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
--- 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
--- 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
--- 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;
--
1.5.2.2.239.g89630
