Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

!!! TASK: Deprecate and replace ActionResponse in dispatcher #3294

Next Next commit
WIP: Experiment prefer PsrResponse over ActionResponse
mhsdesign committed Feb 6, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
commit b959935f22f11332c77ed729896ec16aaf0b026c
1 change: 1 addition & 0 deletions Neos.Flow/Classes/Mvc/ActionResponse.php
Original file line number Diff line number Diff line change
@@ -13,6 +13,7 @@
* The minimal MVC response object.
* It allows for simple interactions with the HTTP response from within MVC actions. More specific requirements can be implemented via HTTP middlewares.
*
* @deprecated
* @Flow\Proxy(false)
* @api
*/
31 changes: 13 additions & 18 deletions Neos.Flow/Classes/Mvc/Controller/ActionController.php
Original file line number Diff line number Diff line change
@@ -203,7 +203,7 @@ public function injectThrowableStorage(ThrowableStorageInterface $throwableStora
* Handles a request. The result output is returned by altering the given response.
*
* @param ActionRequest $request The request object
* @return ActionResponse
* @return ResponseInterface
* @throws InvalidActionVisibilityException
* @throws InvalidArgumentTypeException
* @throws NoSuchActionException
@@ -215,7 +215,7 @@ public function injectThrowableStorage(ThrowableStorageInterface $throwableStora
* @throws \Neos\Flow\Security\Exception
* @api
*/
public function processRequest(ActionRequest $request): ActionResponse
public function processRequest(ActionRequest $request): ResponseInterface
{
$response = new ActionResponse();
$this->initializeController($request, $response);
@@ -260,13 +260,13 @@ public function processRequest(ActionRequest $request): ActionResponse
$this->initializeView($this->view);
}

$response = $this->callActionMethod($request, $this->arguments, $response);
$httpResponse = $this->callActionMethod($request, $this->arguments, $response);

if (!$response->hasContentType()) {
$response->setContentType($this->negotiatedMediaType);
if (!$httpResponse->hasHeader('Content-Type')) {
$httpResponse = $httpResponse->withHeader('Content-Type', $this->negotiatedMediaType);
}

return $response;
return $httpResponse;
}

/**
@@ -515,10 +515,9 @@ protected function initializeAction()
*
* @param ActionRequest $request
* @param Arguments $arguments
* @param ActionResponse $response The most likely empty response to modify or replace.
* @return ActionResponse The final response for this request.
* @param ActionResponse $response The most likely empty response.
*/
protected function callActionMethod(ActionRequest $request, Arguments $arguments, ActionResponse $response): ActionResponse
protected function callActionMethod(ActionRequest $request, Arguments $arguments, ActionResponse $response): ResponseInterface
{
$preparedArguments = [];
foreach ($arguments as $argument) {
@@ -560,12 +559,12 @@ protected function callActionMethod(ActionRequest $request, Arguments $arguments
}

if ($actionResult === null && $this->view instanceof ViewInterface) {
$response = $this->renderView($response);
return $this->renderView($this->response);
} else {
$response->setContent($actionResult);
}

return $response;
return $this->response->buildHttpResponse();
}

/**
@@ -832,9 +831,8 @@ protected function getErrorFlashMessage()
* Renders the view and applies the result to the response object.
*
* @param ActionResponse $response
* @return ActionResponse
*/
protected function renderView(ActionResponse $response): ActionResponse
protected function renderView(ActionResponse $response): ResponseInterface
{
$result = $this->view->render();

@@ -847,10 +845,7 @@ protected function renderView(ActionResponse $response): ActionResponse
}

if ($result instanceof ResponseInterface) {
$response->replaceHttpResponse($result);
if ($result->hasHeader('Content-Type')) {
$response->setContentType($result->getHeaderLine('Content-Type'));
}
return $result;
}

if (is_object($result) && is_callable([$result, '__toString'])) {
@@ -861,6 +856,6 @@ protected function renderView(ActionResponse $response): ActionResponse
$response->setContent($result);
}

return $response;
return $response->buildHttpResponse();
}
}
6 changes: 3 additions & 3 deletions Neos.Flow/Classes/Mvc/Controller/ControllerInterface.php
Original file line number Diff line number Diff line change
@@ -12,9 +12,9 @@
*/

use Neos\Flow\Mvc\ActionRequest;
use Neos\Flow\Mvc\ActionResponse;
use Neos\Flow\Mvc\Exception\ForwardException;
use Neos\Flow\Mvc\Exception\StopActionException;
use Psr\Http\Message\ResponseInterface;

/**
* Generic interface for controllers
@@ -43,10 +43,10 @@ interface ControllerInterface
* wich the Dispatcher will catch and handle its attached next-request.
*
* @param ActionRequest $request The dispatched action request
* @return ActionResponse The resulting created response
* @return ResponseInterface The resulting created response
* @throws StopActionException
* @throws ForwardException
* @api
*/
public function processRequest(ActionRequest $request): ActionResponse;
public function processRequest(ActionRequest $request): ResponseInterface;
}
2 changes: 1 addition & 1 deletion Neos.Flow/Classes/Mvc/DispatchMiddleware.php
Original file line number Diff line number Diff line change
@@ -41,6 +41,6 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
throw new Exception('No ActionRequest was created before the DispatchMiddleware. Make sure you have the SecurityEntryPointMiddleware configured before dispatch.', 1605091292);
}

return $this->dispatcher->dispatch($actionRequest)->buildHttpResponse();
return $this->dispatcher->dispatch($actionRequest);
}
}
16 changes: 7 additions & 9 deletions Neos.Flow/Classes/Mvc/Dispatcher.php
Original file line number Diff line number Diff line change
@@ -11,6 +11,7 @@
* source code.
*/

use GuzzleHttp\Psr7\Response;
use Neos\Flow\Annotations as Flow;
use Neos\Flow\Configuration\Exception\NoSuchOptionException;
use Neos\Flow\Log\PsrLoggerFactoryInterface;
@@ -26,6 +27,7 @@
use Neos\Flow\Security\Exception\AccessDeniedException;
use Neos\Flow\Security\Exception\AuthenticationRequiredException;
use Neos\Flow\Security\Exception\MissingConfigurationException;
use Psr\Http\Message\ResponseInterface;
use Psr\Log\LoggerInterface;

/**
@@ -84,7 +86,6 @@ public function injectFirewall(FirewallInterface $firewall)
* Dispatches a request to a controller
*
* @param ActionRequest $request The request to dispatch
* @return ActionResponse
* @throws AccessDeniedException
* @throws AuthenticationRequiredException
* @throws InfiniteLoopException
@@ -93,7 +94,7 @@ public function injectFirewall(FirewallInterface $firewall)
* @throws MissingConfigurationException
* @api
*/
public function dispatch(ActionRequest $request): ActionResponse
public function dispatch(ActionRequest $request): ResponseInterface
{
try {
if ($this->securityContext->areAuthorizationChecksDisabled() !== true) {
@@ -117,10 +118,9 @@ public function dispatch(ActionRequest $request): ActionResponse
* Try processing the request until it is successfully marked "dispatched"
*
* @param ActionRequest $request
* @return ActionResponse
* @throws InvalidControllerException|InfiniteLoopException|NoSuchOptionException
*/
protected function initiateDispatchLoop(ActionRequest $request): ActionResponse
protected function initiateDispatchLoop(ActionRequest $request): ResponseInterface
{
$dispatchLoopCount = 0;
while ($request->isDispatched() === false) {
@@ -144,7 +144,7 @@ protected function initiateDispatchLoop(ActionRequest $request): ActionResponse
}
}
// TODO $response is never _null_ at this point, except a `forwardToRequest` and the `nextRequest` is already dispatched == true, which seems illegal af
return $response ?? new ActionResponse();
return $response ?? new Response();
}

/**
@@ -164,14 +164,12 @@ protected function emitBeforeControllerInvocation(ActionRequest $request, Contro
* returned control back to the dispatcher.
*
* @param ActionRequest $request
* @param ActionResponse|null $response The response the controller returned or null, if it was just forwarding a request.
* Modifying the response through this signal is not always going to take effect
* and might be ignored for example if the dispatcher is still in the loop.
* @param ResponseInterface|null $response The readonly response the controller returned or null, if it was just forwarding a request.
* @param ControllerInterface $controller
* @return void
* @Flow\Signal
*/
protected function emitAfterControllerInvocation(ActionRequest $request, ?ActionResponse $response, ControllerInterface $controller)
protected function emitAfterControllerInvocation(ActionRequest $request, ?ResponseInterface $response, ControllerInterface $controller)
{
}

8 changes: 5 additions & 3 deletions Neos.Flow/Classes/Mvc/Exception/StopActionException.php
Original file line number Diff line number Diff line change
@@ -12,6 +12,7 @@
*/

use Neos\Flow\Mvc\ActionResponse;
use Psr\Http\Message\ResponseInterface;
use Neos\Flow\Mvc\Controller\AbstractController;

/**
@@ -31,15 +32,16 @@ final class StopActionException extends \Neos\Flow\Mvc\Exception
/**
* The response to be received by the MVC Dispatcher.
*/
public readonly ActionResponse $response;
public readonly ResponseInterface $response;

private function __construct(string $message, int $code, ?\Throwable $previous, ActionResponse $response)
private function __construct(string $message, int $code, ?\Throwable $previous, ResponseInterface $response)
{
parent::__construct($message, $code, $previous);
$this->response = $response;
}

/**
* @deprecated
* @param ActionResponse $response The response to be received by the MVC Dispatcher.
* @param string $details Additional details just for this exception, in case it is logged (the regular exception message).
*/
@@ -51,6 +53,6 @@ public static function createForResponse(ActionResponse $response, string $detai
$response->getStatusCode()
);
}
return new self($details, 1558088618, null, $response);
return new self($details, 1558088618, null, $response->buildHttpResponse());
}
}
Original file line number Diff line number Diff line change
@@ -14,6 +14,7 @@
use GuzzleHttp\Psr7\ServerRequest;
use GuzzleHttp\Psr7\Uri;
use PHPUnit\Framework\Assert;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Neos\Flow\Mvc\ActionRequest;
use Neos\Flow\Mvc\ActionResponse;
@@ -291,7 +292,7 @@ public function redirectRedirectsToTheSpecifiedAction()
$mockUriBuilder->expects(self::once())->method('uriFor')->with('show', ['foo' => 'bar'], 'Stuff', 'Super', 'Duper\Package')->willReturn('the_uri');

$controller = new class extends AbstractController {
public function processRequest(ActionRequest $request): ActionResponse
public function processRequest(ActionRequest $request): ResponseInterface
{
$response = new ActionResponse();
$mockUriBuilder = $this->uriBuilder;
@@ -300,7 +301,7 @@ public function processRequest(ActionRequest $request): ActionResponse

$this->myIndexAction();

return $this->response;
return $this->response->buildHttpResponse();
}

public function myIndexAction(): void
@@ -336,7 +337,7 @@ public function redirectUsesRequestFormatAsDefaultAndUnsetsSubPackageKeyIfNecess
$mockUriBuilder->expects(self::once())->method('uriFor')->with('show', ['foo' => 'bar'], 'Stuff', 'Super', null)->willReturn('the_uri');

$controller = new class extends AbstractController {
public function processRequest(ActionRequest $request): ActionResponse
public function processRequest(ActionRequest $request): ResponseInterface
{
$response = new ActionResponse();
$mockUriBuilder = $this->uriBuilder;
@@ -345,7 +346,7 @@ public function processRequest(ActionRequest $request): ActionResponse

$this->myIndexAction();

return $this->response;
return $this->response->buildHttpResponse();
}

public function myIndexAction(): void
Original file line number Diff line number Diff line change
@@ -24,6 +24,7 @@
use Neos\Flow\Property\Exception;
use Neos\Flow\Persistence\QueryResultInterface;
use Neos\FluidAdaptor\Core\Widget\Exception\WidgetContextNotFoundException;
use Psr\Http\Message\ResponseInterface;

/**
* This is the base class for all widget controllers.
@@ -46,7 +47,7 @@ abstract class AbstractWidgetController extends ActionController
* Handles a request. The result output is returned by altering the given response.
*
* @param ActionRequest $request The request object
* @return ActionResponse $response The response, modified by this handler
* @return ResponseInterface The response, created by this handler
* @throws WidgetContextNotFoundException
* @throws InvalidActionVisibilityException
* @throws InvalidArgumentTypeException
@@ -59,7 +60,7 @@ abstract class AbstractWidgetController extends ActionController
* @throws \Neos\Flow\Security\Exception
* @api
*/
public function processRequest(ActionRequest $request): ActionResponse
public function processRequest(ActionRequest $request): ResponseInterface
{
/** @var WidgetContext $widgetContext */
$widgetContext = $request->getInternalArgument('__widgetContext');
Original file line number Diff line number Diff line change
@@ -11,6 +11,7 @@
* source code.
*/

use GuzzleHttp\Psr7\Utils;
use Neos\Flow\Mvc\ActionRequest;
use Neos\Flow\Mvc\ActionResponse;
use Neos\Flow\Mvc\Exception\ForwardException;
@@ -230,7 +231,6 @@ protected function initiateSubRequest()
if ($dispatchLoopCount++ > 99) {
throw new InfiniteLoopException('Could not ultimately dispatch the widget request after ' . $dispatchLoopCount . ' iterations.', 1380282310);
}
$subResponse = new ActionResponse();

$widgetControllerObjectName = $this->widgetContext->getControllerObjectName();
if ($subRequest->getControllerObjectName() !== '' && $subRequest->getControllerObjectName() !== $widgetControllerObjectName) {
@@ -241,17 +241,17 @@ protected function initiateSubRequest()
$subResponse = $this->controller->processRequest($subRequest);

// We need to make sure to not merge content up into the parent ActionResponse because that _could_ break the parent response.
$content = $subResponse->getContent();
$subResponse->setContent('');
$content = $subResponse->getBody()->getContents();
$subResponse = $subResponse->withBody(Utils::streamFor(''));
} catch (StopActionException $exception) {
$subResponse = $exception->response;
$subResponse->mergeIntoParentResponse($this->controllerContext->getResponse());
$this->controllerContext->getResponse()->replaceHttpResponse($subResponse);
throw $exception;
} catch (ForwardException $exception) {
$subRequest = $exception->nextRequest;
continue;
}
$subResponse->mergeIntoParentResponse($this->controllerContext->getResponse());
$this->controllerContext->getResponse()->replaceHttpResponse($subResponse);
}

return $content;
Original file line number Diff line number Diff line change
@@ -79,7 +79,7 @@ public function process(ServerRequestInterface $httpRequest, RequestHandlerInter
$actionRequest = $this->actionRequestFactory->createActionRequest($httpRequest, ['__widgetContext' => $widgetContext]);
$actionRequest->setControllerObjectName($widgetContext->getControllerObjectName());
$this->securityContext->setRequest($actionRequest);
return $this->dispatcher->dispatch($actionRequest)->buildHttpResponse();
return $this->dispatcher->dispatch($actionRequest);
}

/**