vlucas / bulletphp

A resource-oriented micro PHP framework
http://bulletphp.com
BSD 3-Clause "New" or "Revised" License
418 stars 50 forks source link

Register response content converters #9

Closed jcbwlkr closed 11 years ago

jcbwlkr commented 11 years ago

Add mechanism to allow for automatic conversion of response content and content type based on user defined test conditions.

I want to do this so I can do something like the following in my setup:

$app->registerContentConverter(
    function($content) {
        return $content instanceof Hal;
    },
    function($content) {
        return $content->asJson();
    },
    'application/hal+json'
);

And in my routes I can do this:

$app->path('/', function($request) use($app) {
    $hal = new Hal('/');
    // snip
    return $hal;
});

The default behavior of converting Arrays to JSON has been preserved by a call to registerContentConverter in \Bullet\App::__construct

vlucas commented 11 years ago

Great idea to have a response handling system. This will definitely be added to Bullet, we just have to get the details right.

  1. I'd like to change the name to registerResponseHandler, since Bullet already has the idea of "return a response", and "response types".
  2. Allow the condition (test) to be any valid PHP callable, or null (no condition) to have it applied to every response.
  3. Change the argument on the test and callback to be the Response object, not just the raw content, so the handler can test and modify more of the response than just the content. This would allow a broader range of requirements, like time-stamping every response or caching, etc.
  4. Drop the 3rd argument (for Content-Type), and move it into the callback using the request object

The updated code might look like this:

$app->registerResponseHandler(
    function($response) {
        return $response->content() instanceof Hal;
    },
    function($response) {
        $response->contentType('application/hal+json');
        return $response->content($response->content()->asJson());
    }
);

It's slightly more verbose, but adds more power for the handler to be able to modify any part of the response it needs to, and removes a method argument that may limit us later down the road.

jcbwlkr commented 11 years ago

That makes sense. I have made the requested changes. I added a test to ensure that multiple handlers can be applied.

While I was in there I took a few liberties. I will gladly rewrite if you suggest it.

vlucas commented 11 years ago

Good changes. I like it. :+1:

jcbwlkr commented 11 years ago

Great! I know it will be useful in my app and hopefully it will help someone else.