vlucas / bulletphp

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

Response Handlers cannot be overloaded #68

Closed shadowhand closed 9 years ago

shadowhand commented 9 years ago

Currently, once a handler has been added, it cannot be overloaded. Because new handlers are not prepended, new handlers that match similar, but perhaps more restrictive, conditions will not be executed. Problem was introduced by f1f3c21c6031626ed39cf6e7df906a85a192d2bb.

Example of code I want to use:

$app->registerResponseHandler(
    function($response) {
        return is_array($response->content());
    },
    function($response) use ($app) {
        if (BULLET_ENV !== 'production') {
            $options = JSON_PRETTY_PRINT;
        } else {
            $options = null;
        }
        $response->contentType('application/vnd.api+json');
        $response->content(json_encode($response->content(), $options));
    }
);
ellotheth commented 9 years ago

This fix is a BC break if users were registering their own handlers expecting the array-to-JSON transformation to happen first. An alternate solution for @shadowhand could be to keep the existing handler logic the same, but let users specify the JSON options ahead of time, maybe in the constructor:

$app = new App([ 'json_options' => [
    'type' => 'application/vnd.api+json',
    'options' => JSON_PRETTY_PRINT | JSON_NUMERIC_CHECK
]]);

and then have those options overwrite the defaults:

$jsonOptions = [
    'type' => 'application/json',
    'options' => null
];
if (isset($values['json_options']) && is_array($values['json_options'])) {
    $jsonOptions = array_merge($jsonOptions, $values['json_options']);
}

$this->registerResponseHandler(
    function($response) {
        return is_array($response->content());
    },
    function($response) use ($jsonOptions) {
        $response->contentType($jsonOptions['type']);
        $response->content(json_encode($response->content(), $jsonOptions['options']));
    }
);

/cc @vlucas

vlucas commented 9 years ago

Seems like the ultimate solution is to name the response handlers so that they can be removed and over-ridden at any point. A third parameter could be added for the name that would not break BC. Thoughts?

ellotheth commented 9 years ago

Much nicer, I'll get a PR up in a few. Any preferences on the name for the default array handler? I think it makes sense to name the handlers by what's being handled rather than the result (so e.g. somebody who wanted to output arrays as XML wouldn't have to make a handler called json). How about array-content?