khrt / Raisin

Raisin - a REST API micro framework for Perl 🐫 🐪
61 stars 29 forks source link

Added support for dynamic changes to content type. #116

Closed budney closed 2 years ago

budney commented 2 years ago

Raisin selects a response content type (and hence an Encoder) based on the "Accept" header of the request and the "produces" list for the resource. Applications can change the Content-Type header, but can't change the encoder. This patch changes Raisin's post-processing step to consult the 'raisinx.encoder' value in the attribute, which may have changed since it was initially set by the call() method.

This is a minimal change: the application is fully responsible for making sure the headers are consistent, that the new content-type is appropriate, and that the specified encoder actually exists. It just gives you the power to change the encoder, if you need to and know what you're doing.

budney commented 2 years ago

Hi! Is it possible to merge this change? If not, is there an alternate approach you'd prefer to see?

Thanks!

khrt commented 2 years ago

Hi, I see that you've defined a variable, but how is it used?

budney commented 2 years ago

$format is actually the same variable that's defined a few lines earlier in the same sub. Sub call defines $format this way at line 47:

my $format = $self->negotiate_format($req);

A few lines later it calls Plack::Util::response_cb() using a closure that references $format, which means that nothing the endpoint does can ever change the formatter's mind about what format to return. But shortly after line 47, in line 51, the negotiated format is saved to the environment like this:

$env->{'raisinx.encoder'} = $format;

So all my change does is, instead of referencing $format directly, to fetch it from the environment again. That way if the endpoint changes the value of rasinx.encoder, the callback will pick up the change.

Taking advantage of this change requires the endpoint to modify the value of raisinx.encoder, which is a bit hacky, but it was a much smaller change than adding some sort of "change_encoder()" method.

budney commented 2 years ago

The documentation change in my other pull request does describe how to take advantage of this, so users won't have to discover it by mining the source code.

khrt commented 2 years ago

Right, this makes sense.

I'm willing to merge this, if you add a test for it. Would please mind adding the test?

budney commented 2 years ago

OK, I've added a test in which the encoder is overridden after forcing a different format to be negotiated by using an "Accept" header.