Mailing List Archive: Catalyst: Users

Catalyst::Action::SerializeBase::_load_content_plugins default behavior

Index | Next | Previous | View Threaded

DGoehrig at synacor

Feb 8, 2010, 6:52 AM

Post #1 of 3 (636 views)
Permalink

Catalyst::Action::SerializeBase::_load_content_plugins default behavior

Dear List,

I recently upgraded a project I'm working on to track some of the latest
changes in Catalyst::Action::REST, and noticed that in our application's
case the change to the 'serialize' config parameter altered the default
behavior of the application. The previous behavior had a config that
looked like:

__PACKAGE__->config(
serialize => {
default => 'application/json',
map => {
'application/json' => 'JSONP',
'text/x-php-serialization' => ['Data::Serializer',
'PHP::Serialization' ],
}
}
);

Would and still does, result in the application returning
'application/json' by default, because on line 182 of
Catatlyst::Action::SerializeBase

$config = $controller->{'serialize'}

Overrides the default map!

However, if you use the new config style:

__PACKAGE__->config(
default => 'application/json',
map => {
'application/json' => 'JSONP',
'text/x-php-serialization' => ['Data::Serializer',
'PHP::Serialization' ],
}
);

The map entries are then appended to the default $config->map, and net
result is these additional mappings are never seen. This is because a
silly AJAX requests which never specified a content-type now use the
"default" behavior based on the browser's Accept-Encoding, which is
ALWAYS 'text/html'!

As a result, instead of returning a 'default' => 'application/json', it
quite rightly returns a default => 'text/html' which serializes based on
Catalyst::Action::YAML::HTML

Based on the documentation, both behaviors are correct, but it seems to
me that if I specify a local mapping that it should override the default
map as it did in the old behavior. Was this change a bug or a design
decision?

- Dave Goehrig

dgoehrig [at] synacor

_______________________________________________
List: Catalyst [at] lists
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst [at] lists/
Dev site: http://dev.catalyst.perl.org/

bobtfish at bobtfish

Feb 8, 2010, 7:37 AM

Post #2 of 3 (595 views)
Permalink

Re: Catalyst::Action::SerializeBase::_load_content_plugins default behavior [In reply to]

Dave Goehrig wrote:
> Based on the documentation, both behaviors are correct, but it seems to
> me that if I specify a local mapping that it should override the default
> map as it did in the old behavior. Was this change a bug or a design
> decision?

Bug.

If you could provide a failing test case that'd be most appreicated.

Cheers
t0m

_______________________________________________
List: Catalyst [at] lists
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst [at] lists/
Dev site: http://dev.catalyst.perl.org/

bobtfish at bobtfish

Feb 8, 2010, 12:21 PM

Post #3 of 3 (595 views)
Permalink

Re: Catalyst::Action::SerializeBase::_load_content_plugins default behavior [In reply to]

On 8 Feb 2010, at 14:52, Dave Goehrig wrote:
> However, if you use the new config style:
>
> __PACKAGE__->config(
> default => 'application/json',
> map => {
> 'application/json' => 'JSONP',
> 'text/x-php-serialization' => ['Data::Serializer',
> 'PHP::Serialization' ],
> }
> );
>
> The map entries are then appended to the default $config->map, and net
> result is these additional mappings are never seen.

Erm, nope..

They're merged, in the normal way that config is merged.

> This is because a
> silly AJAX requests which never specified a content-type now use the
> "default" behavior based on the browser's Accept-Encoding, which is
> ALWAYS 'text/html'!
>
> As a result, instead of returning a 'default' => 'application/json',
> it
> quite rightly returns a default => 'text/html' which serializes
> based on
> Catalyst::Action::YAML::HTML

Erm, this isn't anything to do with the 'default'.

The default is only used when you have NO Accept OR Content-Type
headers at all..

> Based on the documentation, both behaviors are correct, but it seems
> to
> me that if I specify a local mapping that it should override the
> default
> map as it did in the old behavior.

Yes, if you map text/html to JSONP, then it'll work (overriding the
default) :)

> Was this change a bug or a design
> decision?

I'm no longer sure, as I'm not sure what problem you're reporting /
what the actual behavior change is.

I wrote a test to try to prove your issue, but it works like I expect
it to:

http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=catagits/Catalyst-Action-REST.git;a=commitdiff;h=960e29eed9ccd049c3c0dcd78a65308242a64ae0

Can you write me a similar test which passes on the version you were
running, but fails on a later version (and tell me which versions
these are) - so that I can work out what the _actual_ behavior change
which caused your issue is?

Cheers
t0m

_______________________________________________
List: Catalyst [at] lists
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst [at] lists/
Dev site: http://dev.catalyst.perl.org/

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact Gossamer Threads