flow-development-collection icon indicating copy to clipboard operation
flow-development-collection copied to clipboard

FEATURE: Tooling for working with response headers in controller

Open mhsdesign opened this issue 1 year ago • 0 comments

Initially the idea was was part of https://github.com/neos/flow-development-collection/pull/2219 but reverted and i reverted the commit which makes it a reimplementation here: https://github.com/neos/flow-development-collection/pull/3290

Maybe the ActionResponse Headers is stretching things a bit... have removed this for now and will create another PR suggesting to change to that. I do think the Headers class handles some cases for abstracting headers very well (e.g. Cache-Control and date based headers like Last-Modified) and should be the underlying ActionResponse API

Or differently expressed in https://github.com/neos/flow-development-collection/issues/2156#issuecomment-720162050 by @albe:

One idea: Let's move the currently deprecated Http\Headers class into the Mvc namespace and make it part of the ActionResponse API in order to specify headers to set in the response [...]. The idea is that ActionResponse is a higher level abstraction of a response and Headers is a (slightly) higher level abstraction of basic Http headers that hides some details like the Cache-Control stuff.


So i dug a bit into the history and it seem the Header class was deprecated in https://github.com/neos/flow-development-collection/pull/1366 (Flow 5.1)

Previously it was used in the legacy (pre prs7) \Neos\Flow\Http\Response and even shortly in the new ActionResponse https://github.com/neos/flow-development-collection/pull/1531 before psr7.

The Header class went with https://github.com/neos/flow-development-collection/pull/2626 through its last refactoring but wasnt touched really since.

Currently its only used as storage for \Neos\Flow\Http\Client\Browser::addAutomaticRequestHeader which is no real usecase. And even partially broken. The Header class is as good as dead.


After looking a bit into the Header class it might really well good that this is not super heavily in use. There are lots of tests, but the implementation looks rather complex, and there are missing features like case-insensitive header name matching.

Maybe we need a another utility / abstraction but just not the Header again :D Possibly even more utilities similar to the Header's \Neos\Flow\Http\CacheControlDirectives would be a good fit. The CacheControlDirectives should according to the docs be already preferred over the Header class and might be used like:

$cacheControlHeaderValue = $response->getHeaderLine('Cache-Control');
$cacheControlDirectives = CacheControlDirectives::fromRawHeader($cacheControlHeaderValue);
$cacheControlDirectives->setDirective('max-age', 3600);
$cacheControlHeaderValue = $cacheControlDirectives->getCacheControlHeaderValue();
$response = $response->withHeader('Cache-Control', $cacheControlHeaderValue);

mhsdesign avatar Jan 29 '24 21:01 mhsdesign