tonic icon indicating copy to clipboard operation
tonic copied to clipboard

Response class should do content type negotiation

Open logistiker opened this issue 11 years ago • 3 comments

I think the Response class should support content type negotiation (Vary: Accept). This makes the content type output driven by the request as it should be. This could be an optional call but even if I were displaying only one content type for a resource, I'd still want to use this method to alert users that the requested content type cannot be displayed. Here's a method I use to accomplish this task when extending the Response class:

/**
 * Supported response formats
 */
protected $response_formats = array('json','wddx','php');

/**
 * Set format to first available accepted format in request
 *
 * @param string $format format to override detected format
 * @return void
 */
public function setFormat($format = null)
{
    // Add vary: accept header since the response uses content negotation based on the accept request header
    $this->addVary('accept');
    if ($format !== null) {
        if (! array_key_exists($format, $this->request->mimetypes)) {
            throw new Format_Response_Exception("Mimetype is not available for '$format'", 
                Response::INTERNALSERVERERROR);
        }
        $this->headers['Content-type'] = $this->request->mimetypes[$format];
        $this->format = $format;
        return;
    }
    $http_accept = $this->request->accept;
    // Default to $this->format if no format is requested in the accept header
    if (count($http_accept) === 0) {
        $http_accept = array(10 => array($this->request->mimetypes[$this->format]));
    }
    foreach ($http_accept as $priority => $values) {
        foreach ($values as $accept) {
            if (in_array($accept, $this->response_formats)) {
                if (! array_key_exists($accept, $this->request->mimetypes)) {
                    throw new Format_Response_Exception("Mimetype is not available for '$format'", 
                        Response::NOTACCEPTABLE);
                }
                $this->headers['Content-type'] = $this->request->mimetypes[$accept];
                $this->format = $accept;
                return;
            }
        }
    }
    // Set to default content type if there isn't a match for http-accept
    $this->headers['Content-type'] = $this->request->mimetypes[$this->format];
}

logistiker avatar Jul 16 '13 04:07 logistiker

I wrote this a while back based on an old version of Tonic but I believe the code could still be used even though for some reason the request is no longer passed into the Response class.

logistiker avatar Jul 16 '13 04:07 logistiker

It's possible this is not needed since a fair amount of this logic exists in resource. A notable exception is when @provides is set for multiple methods, it doesn't automatically add the Vary: Accept header to the response.

logistiker avatar Jul 16 '13 05:07 logistiker

I think there was a reason for not automatically adding a vary header as part of the @provides condition, possibly something to do with some subtitles surrounding the header in certain uses that made it easier/better to instead make people add their own vary header. I'm not sure what problems these might have been, but it seems so obvious to include a vary header at that point that it'd be there already unless there was an issue.

I guess one problem could be that if you have both @provides and @lang conditions then you'd want to set a single vary header "vary: accept,accept-language", so we 'd at least need a way of appending the existing headers in a sane way rather overwriting an existing value.

I'm not against automatically adding a vary header, I'm just wary of it breaking stuff.

peej avatar Jul 16 '13 08:07 peej