From d94562fbac87736cee017aae81d2c6ce25aef0ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Mu=CC=88ller?= Date: Sat, 11 Nov 2023 17:35:33 +0100 Subject: [PATCH 01/21] !!!FEATURE: Clearer controller pattern WIP This change needs an accompanying adjustment to Neos to adjust the PluginImplementation as well as Modules. The new `SimpleActionController` gives you a direct and simple way to route an ActionRequest and return an ActionReponse with nothing in between. Routing should work just like with other ActionControllers. This is breaking if you implemented your own ControllerInterface or overwrote or expect some of the api methods of the ActionController. We now use a direct pattern of f(ActionRequest) => ActionResponse in more places. Adjusting existing controllers should be easy though. We discourage to manipulate `$this->reponse` in controllers, although it should still work fine in actions for now, please consider other options. --- .../Mvc/Controller/AbstractController.php | 51 ++++++---- .../Mvc/Controller/ActionController.php | 88 +++++++++-------- .../Mvc/Controller/ControllerInterface.php | 9 +- .../Classes/Mvc/Controller/RestController.php | 26 +++-- .../Mvc/Controller/SimpleActionController.php | 47 +++++++++ .../Controller/SpecialResponsesSupport.php | 99 +++++++++++++++++++ .../Mvc/Controller/StandardController.php | 3 +- Neos.Flow/Classes/Mvc/DispatchMiddleware.php | 4 +- Neos.Flow/Classes/Mvc/Dispatcher.php | 27 ++--- .../Mvc/Exception/StopActionException.php | 3 + .../Fixtures/Controller/FooController.php | 5 +- .../Mvc/Controller/AbstractControllerTest.php | 34 ++++--- .../Mvc/Controller/ActionControllerTest.php | 55 ++++++----- .../Tests/Unit/Mvc/DispatchMiddlewareTest.php | 7 +- Neos.Flow/Tests/Unit/Mvc/DispatcherTest.php | 21 ++-- .../Core/Widget/AbstractWidgetController.php | 7 +- .../Core/Widget/AjaxWidgetMiddleware.php | 6 +- .../Widget/AbstractWidgetControllerTest.php | 7 +- .../Core/Widget/AjaxWidgetMiddlewareTest.php | 6 +- 19 files changed, 352 insertions(+), 153 deletions(-) create mode 100644 Neos.Flow/Classes/Mvc/Controller/SimpleActionController.php create mode 100644 Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php diff --git a/Neos.Flow/Classes/Mvc/Controller/AbstractController.php b/Neos.Flow/Classes/Mvc/Controller/AbstractController.php index 21550f92f2..8056890b31 100644 --- a/Neos.Flow/Classes/Mvc/Controller/AbstractController.php +++ b/Neos.Flow/Classes/Mvc/Controller/AbstractController.php @@ -14,6 +14,9 @@ use Neos\Error\Messages as Error; use Neos\Flow\Annotations as Flow; use GuzzleHttp\Psr7\Uri; +use Neos\Flow\Mvc\Exception\NoSuchArgumentException; +use Neos\Flow\Mvc\Routing\Exception\MissingActionNameException; +use Neos\Flow\Property\Exception; use Psr\Http\Message\UriInterface; use Neos\Flow\Http\Helper\MediaTypeHelper; use Neos\Flow\Http\Helper\ResponseInformationHelper; @@ -36,6 +39,8 @@ */ abstract class AbstractController implements ControllerInterface { + use SpecialResponsesSupport; + /** * @var UriBuilder */ @@ -196,7 +201,7 @@ protected function forward($actionName, $controllerName = null, $packageKey = nu $nextRequest->setControllerName($controllerName); } if ($packageKey !== null && strpos($packageKey, '\\') !== false) { - list($packageKey, $subpackageKey) = explode('\\', $packageKey, 2); + [$packageKey, $subpackageKey] = explode('\\', $packageKey, 2); } else { $subpackageKey = null; } @@ -251,20 +256,24 @@ protected function forwardToRequest(ActionRequest $request) * if used with other request types. * * @param string $actionName Name of the action to forward to - * @param string $controllerName Unqualified object name of the controller to forward to. If not specified, the current controller is used. - * @param string $packageKey Key of the package containing the controller to forward to. If not specified, the current package is assumed. + * @param null $controllerName Unqualified object name of the controller to forward to. If not specified, the current controller is used. + * @param null $packageKey Key of the package containing the controller to forward to. If not specified, the current package is assumed. * @param array $arguments Array of arguments for the target action * @param integer $delay (optional) The delay in seconds. Default is no delay. * @param integer $statusCode (optional) The HTTP status code for the redirect. Default is "303 See Other" - * @param string $format The format to use for the redirect URI + * @param null $format The format to use for the redirect URI + * @return never * @throws StopActionException + * @throws UnsupportedRequestTypeException + * @throws \Neos\Flow\Http\Exception + * @throws MissingActionNameException * @see forward() * @api */ protected function redirect($actionName, $controllerName = null, $packageKey = null, array $arguments = [], $delay = 0, $statusCode = 303, $format = null): never { if ($packageKey !== null && strpos($packageKey, '\\') !== false) { - list($packageKey, $subpackageKey) = explode('\\', $packageKey, 2); + [$packageKey, $subpackageKey] = explode('\\', $packageKey, 2); } else { $subpackageKey = null; } @@ -291,7 +300,10 @@ protected function redirect($actionName, $controllerName = null, $packageKey = n * @param integer $delay (optional) The delay in seconds. Default is no delay. * @param integer $statusCode (optional) The HTTP status code for the redirect. Default is "303 See Other" * @return void + * @throws MissingActionNameException * @throws StopActionException + * @throws UnsupportedRequestTypeException + * @throws \Neos\Flow\Http\Exception * @see forwardToRequest() * @api */ @@ -311,22 +323,17 @@ protected function redirectToRequest(ActionRequest $request, $delay = 0, $status * @param mixed $uri Either a string representation of a URI or a \Neos\Flow\Http\Uri object * @param integer $delay (optional) The delay in seconds. Default is no delay. * @param integer $statusCode (optional) The HTTP status code for the redirect. Default is "303 See Other" - * @throws UnsupportedRequestTypeException If the request is not a web request * @throws StopActionException * @api */ - protected function redirectToUri($uri, $delay = 0, $statusCode = 303): never + protected function redirectToUri(string|UriInterface $uri, int $delay = 0, int $statusCode = 303): never { - if ($delay === 0) { - if (!$uri instanceof UriInterface) { - $uri = new Uri($uri); - } - $this->response->setRedirectUri($uri, $statusCode); - } else { - $this->response->setStatusCode($statusCode); - $this->response->setContent(''); + if (!$uri instanceof UriInterface) { + $uri = new Uri($uri); } - throw new StopActionException(); + + $response = $this->responseRedirectsToUri($uri,$delay, $statusCode, $this->response); + $this->throwStopActionWithReponse($response, '', 1699716808); } /** @@ -357,19 +364,23 @@ protected function throwStatus(int $statusCode, $statusMessage = null, $content /** * Maps arguments delivered by the request object to the local controller arguments. * + * @param ActionRequest $request * @return void * @throws RequiredArgumentMissingException + * @throws NoSuchArgumentException + * @throws Exception + * @throws \Neos\Flow\Security\Exception * @api */ - protected function mapRequestArgumentsToControllerArguments() + protected function mapRequestArgumentsToControllerArguments(ActionRequest $request) { /* @var $argument \Neos\Flow\Mvc\Controller\Argument */ foreach ($this->arguments as $argument) { $argumentName = $argument->getName(); if ($argument->getMapRequestBody()) { - $argument->setValue($this->request->getHttpRequest()->getParsedBody()); - } elseif ($this->request->hasArgument($argumentName)) { - $argument->setValue($this->request->getArgument($argumentName)); + $argument->setValue($request->getHttpRequest()->getParsedBody()); + } elseif ($request->hasArgument($argumentName)) { + $argument->setValue($request->getArgument($argumentName)); } elseif ($argument->isRequired()) { throw new RequiredArgumentMissingException('Required argument "' . $argumentName . '" is not set.', 1298012500); } diff --git a/Neos.Flow/Classes/Mvc/Controller/ActionController.php b/Neos.Flow/Classes/Mvc/Controller/ActionController.php index ed7f5f64f4..70288cacfc 100644 --- a/Neos.Flow/Classes/Mvc/Controller/ActionController.php +++ b/Neos.Flow/Classes/Mvc/Controller/ActionController.php @@ -202,8 +202,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 - * @param ActionResponse $response The response, modified by this handler - * @return void + * @return ActionResponse * @throws InvalidActionVisibilityException * @throws InvalidArgumentTypeException * @throws NoSuchActionException @@ -213,11 +212,12 @@ public function injectThrowableStorage(ThrowableStorageInterface $throwableStora * @throws StopActionException * @api */ - public function processRequest(ActionRequest $request, ActionResponse $response) + public function processRequest(ActionRequest $request): ActionResponse { + $response = new ActionResponse(); $this->initializeController($request, $response); - $this->actionMethodName = $this->resolveActionMethodName(); + $this->actionMethodName = $this->resolveActionMethodName($request); $this->initializeActionMethodArguments(); if ($this->enableDynamicTypeValidation !== true) { @@ -230,7 +230,7 @@ public function processRequest(ActionRequest $request, ActionResponse $response) call_user_func([$this, $actionInitializationMethodName]); } try { - $this->mvcPropertyMappingConfigurationService->initializePropertyMappingConfigurationFromRequest($this->request, $this->arguments); + $this->mvcPropertyMappingConfigurationService->initializePropertyMappingConfigurationFromRequest($request, $this->arguments); } catch (InvalidArgumentForHashGenerationException|InvalidHashException $e) { $message = $this->throwableStorage->logThrowable($e); $this->logger->notice('Property mapping configuration failed due to HMAC errors. ' . $message, LogEnvironment::fromMethodName(__METHOD__)); @@ -238,7 +238,7 @@ public function processRequest(ActionRequest $request, ActionResponse $response) } try { - $this->mapRequestArgumentsToControllerArguments(); + $this->mapRequestArgumentsToControllerArguments($request); } catch (RequiredArgumentMissingException $e) { $message = $this->throwableStorage->logThrowable($e); $this->logger->notice('Request argument mapping failed due to a missing required argument. ' . $message, LogEnvironment::fromMethodName(__METHOD__)); @@ -249,7 +249,7 @@ public function processRequest(ActionRequest $request, ActionResponse $response) } if ($this->view === null) { - $this->view = $this->resolveView(); + $this->view = $this->resolveView($request); } if ($this->view !== null) { $this->view->assign('settings', $this->settings); @@ -257,23 +257,27 @@ public function processRequest(ActionRequest $request, ActionResponse $response) $this->initializeView($this->view); } - $this->callActionMethod(); + // We still use a global response here as it might have been changed in any of the steps above + $response = $this->callActionMethod($request, $this->response); - if (!$this->response->hasContentType()) { - $this->response->setContentType($this->negotiatedMediaType); + if (!$response->hasContentType()) { + $response->setContentType($this->negotiatedMediaType); } + + return $response; } /** * Resolves and checks the current action method name * + * @param ActionRequest $request * @return string Method name of the current action - * @throws NoSuchActionException * @throws InvalidActionVisibilityException + * @throws NoSuchActionException */ - protected function resolveActionMethodName() + protected function resolveActionMethodName(ActionRequest $request): string { - $actionMethodName = $this->request->getControllerActionName() . 'Action'; + $actionMethodName = $request->getControllerActionName() . 'Action'; if (!is_callable([$this, $actionMethodName])) { throw new NoSuchActionException(sprintf('An action "%s" does not exist in controller "%s".', $actionMethodName, get_class($this)), 1186669086); } @@ -385,7 +389,7 @@ protected function getInformationNeededForInitializeActionMethodValidators() */ protected function initializeActionMethodValidators() { - list($validateGroupAnnotations, $actionMethodParameters, $actionValidateAnnotations, $actionIgnoredArguments) = $this->getInformationNeededForInitializeActionMethodValidators(); + [$validateGroupAnnotations, $actionMethodParameters, $actionValidateAnnotations, $actionIgnoredArguments] = $this->getInformationNeededForInitializeActionMethodValidators(); if (isset($validateGroupAnnotations[$this->actionMethodName])) { $validationGroups = $validateGroupAnnotations[$this->actionMethodName]; @@ -507,12 +511,11 @@ protected function initializeAction() * response object. If the action doesn't return anything and a valid * view exists, the view is rendered automatically. * - * TODO: In next major this will no longer append content and the response will probably be unique per call. - * - * - * @return void + * @param ActionRequest $request + * @param ActionResponse $response + * @return ActionResponse */ - protected function callActionMethod() + protected function callActionMethod(ActionRequest $request, ActionResponse $response): ActionResponse { $preparedArguments = []; foreach ($this->arguments as $argument) { @@ -554,10 +557,12 @@ protected function callActionMethod() } if ($actionResult === null && $this->view instanceof ViewInterface) { - $this->renderView(); + $response = $this->renderView($response); } else { - $this->response->setContent($actionResult); + $response->setContent($actionResult); } + + return $response; } /** @@ -624,14 +629,14 @@ public static function getPublicActionMethods($objectManager) * @return ViewInterface the resolved view * @throws ViewNotFoundException if no view can be resolved */ - protected function resolveView() + protected function resolveView(ActionRequest $request) { - $viewsConfiguration = $this->viewConfigurationManager->getViewConfiguration($this->request); + $viewsConfiguration = $this->viewConfigurationManager->getViewConfiguration($request); $viewObjectName = $this->defaultViewImplementation; if (!empty($this->defaultViewObjectName)) { $viewObjectName = $this->defaultViewObjectName; } - $viewObjectName = $this->resolveViewObjectName() ?: $viewObjectName; + $viewObjectName = $this->resolveViewObjectName($request) ?: $viewObjectName; if (isset($viewsConfiguration['viewObjectName'])) { $viewObjectName = $viewsConfiguration['viewObjectName']; } @@ -640,12 +645,12 @@ protected function resolveView() throw new ViewNotFoundException(sprintf( 'View class has to implement ViewInterface but "%s" in action "%s" of controller "%s" does not.', $viewObjectName, - $this->request->getControllerActionName(), + $request->getControllerActionName(), get_class($this) ), 1355153188); } - $viewOptions = isset($viewsConfiguration['options']) ? $viewsConfiguration['options'] : []; + $viewOptions = $viewsConfiguration['options'] ?? []; $view = $viewObjectName::createWithOptions($viewOptions); $this->emitViewResolved($view); @@ -671,19 +676,19 @@ protected function emitViewResolved(ViewInterface $view) * @return mixed The fully qualified view object name or false if no matching view could be found. * @api */ - protected function resolveViewObjectName() + protected function resolveViewObjectName(ActionRequest $request) { $possibleViewObjectName = $this->viewObjectNamePattern; - $packageKey = $this->request->getControllerPackageKey(); - $subpackageKey = $this->request->getControllerSubpackageKey(); - $format = $this->request->getFormat(); + $packageKey = $request->getControllerPackageKey(); + $subpackageKey = $request->getControllerSubpackageKey(); + $format = $request->getFormat(); if ($subpackageKey !== null && $subpackageKey !== '') { $packageKey .= '\\' . $subpackageKey; } $possibleViewObjectName = str_replace('@package', str_replace('.', '\\', $packageKey), $possibleViewObjectName); - $possibleViewObjectName = str_replace('@controller', $this->request->getControllerName(), $possibleViewObjectName); - $possibleViewObjectName = str_replace('@action', $this->request->getControllerActionName(), $possibleViewObjectName); + $possibleViewObjectName = str_replace('@controller', $request->getControllerName(), $possibleViewObjectName); + $possibleViewObjectName = str_replace('@action', $request->getControllerActionName(), $possibleViewObjectName); $viewObjectName = $this->objectManager->getCaseSensitiveObjectName(strtolower(str_replace('@format', $format, $possibleViewObjectName))); if ($viewObjectName === null) { @@ -822,32 +827,37 @@ protected function getErrorFlashMessage() /** * Renders the view and applies the result to the response object. + * + * @param ActionResponse $response + * @return ActionResponse */ - protected function renderView() + protected function renderView(ActionResponse $response): ActionResponse { $result = $this->view->render(); if (is_string($result)) { - $this->response->setContent($result); + $response->setContent($result); } if ($result instanceof ActionResponse) { - $result->mergeIntoParentResponse($this->response); + $result->mergeIntoParentResponse($response); } if ($result instanceof ResponseInterface) { - $this->response->replaceHttpResponse($result); + $response->replaceHttpResponse($result); if ($result->hasHeader('Content-Type')) { - $this->response->setContentType($result->getHeaderLine('Content-Type')); + $response->setContentType($result->getHeaderLine('Content-Type')); } } if (is_object($result) && is_callable([$result, '__toString'])) { - $this->response->setContent((string)$result); + $response->setContent((string)$result); } if ($result instanceof StreamInterface) { - $this->response->setContent($result); + $response->setContent($result); } + + return $response; } } diff --git a/Neos.Flow/Classes/Mvc/Controller/ControllerInterface.php b/Neos.Flow/Classes/Mvc/Controller/ControllerInterface.php index 93eca107a8..0062e5af9d 100644 --- a/Neos.Flow/Classes/Mvc/Controller/ControllerInterface.php +++ b/Neos.Flow/Classes/Mvc/Controller/ControllerInterface.php @@ -21,8 +21,8 @@ * Generic interface for controllers * * This interface serves as a common contract for all kinds of controllers. That is, - * in Flow it covers ActionController (dealing with ActionRequest) but also - * CommandController (dealing with CommandRequest). + * in Flow it covers typical ActionController scenarios. They deal with an incoming + * request and provide a response. * * Controllers implementing this interface are compatible with the MVC Dispatcher. * @@ -34,12 +34,11 @@ interface ControllerInterface * Processes a general request. The result can be returned by altering the given response. * * @param ActionRequest $request The request object - * @param ActionResponse $response The response, modified by the controller - * @return void + * @return ActionResponse $response The response, modified by the controller * @throws UnsupportedRequestTypeException if the controller doesn't support the current request type * @throws StopActionException * @throws ForwardException * @api */ - public function processRequest(ActionRequest $request, ActionResponse $response); + public function processRequest(ActionRequest $request): ActionResponse; } diff --git a/Neos.Flow/Classes/Mvc/Controller/RestController.php b/Neos.Flow/Classes/Mvc/Controller/RestController.php index ce4776b83b..bc694bf0f3 100644 --- a/Neos.Flow/Classes/Mvc/Controller/RestController.php +++ b/Neos.Flow/Classes/Mvc/Controller/RestController.php @@ -14,6 +14,8 @@ use Neos\Flow\Annotations as Flow; use Neos\Flow\Mvc\ActionRequest; use Neos\Flow\Mvc\ActionResponse; +use Neos\Flow\Mvc\Exception\InvalidActionNameException; +use Neos\Flow\Mvc\Exception\InvalidActionVisibilityException; use Neos\Flow\Mvc\Exception\NoSuchActionException; use Neos\Flow\Mvc\Exception\StopActionException; use Neos\Flow\Property\TypeConverter\PersistentObjectConverter; @@ -48,41 +50,45 @@ class RestController extends ActionController /** * Determines the action method and assures that the method exists. * + * @param ActionRequest $request * @return string The action method name * @throws NoSuchActionException if the action specified in the request object does not exist (and if there's no default action either). + * @throws StopActionException + * @throws InvalidActionNameException + * @throws InvalidActionVisibilityException */ - protected function resolveActionMethodName() + protected function resolveActionMethodName(ActionRequest $request): string { - if ($this->request->getControllerActionName() === 'index') { + if ($request->getControllerActionName() === 'index') { $actionName = 'index'; - switch ($this->request->getHttpRequest()->getMethod()) { + switch ($request->getHttpRequest()->getMethod()) { case 'HEAD': case 'GET': - $actionName = ($this->request->hasArgument($this->resourceArgumentName)) ? 'show' : 'list'; + $actionName = ($request->hasArgument($this->resourceArgumentName)) ? 'show' : 'list'; break; case 'POST': $actionName = 'create'; break; case 'PUT': - if (!$this->request->hasArgument($this->resourceArgumentName)) { + if (!$request->hasArgument($this->resourceArgumentName)) { $this->throwStatus(400, null, 'No resource specified'); } $actionName = 'update'; break; case 'DELETE': - if (!$this->request->hasArgument($this->resourceArgumentName)) { + if (!$request->hasArgument($this->resourceArgumentName)) { $this->throwStatus(400, null, 'No resource specified'); } $actionName = 'delete'; break; } - if ($this->request->getControllerActionName() !== $actionName) { + if ($request->getControllerActionName() !== $actionName) { // Clone the request, because it should not be mutated to prevent unexpected routing behavior - $this->request = clone $this->request; - $this->request->setControllerActionName($actionName); + $request = clone $request; + $request->setControllerActionName($actionName); } } - return parent::resolveActionMethodName(); + return parent::resolveActionMethodName($request); } /** diff --git a/Neos.Flow/Classes/Mvc/Controller/SimpleActionController.php b/Neos.Flow/Classes/Mvc/Controller/SimpleActionController.php new file mode 100644 index 0000000000..d0cb5f120c --- /dev/null +++ b/Neos.Flow/Classes/Mvc/Controller/SimpleActionController.php @@ -0,0 +1,47 @@ +setDispatched(true); + $actionMethodName = $this->resolveActionMethodName($request); + return $this->$actionMethodName($request); + } + + /** + * Resolves and checks the current action method name + * + * @return string Method name of the current action + * @throws NoSuchActionException + */ + protected function resolveActionMethodName(ActionRequest $request) + { + $actionMethodName = $request->getControllerActionName() . 'Action'; + if (!is_callable([$this, $actionMethodName])) { + throw new NoSuchActionException(sprintf('An action "%s" does not exist in controller "%s".', $actionMethodName, get_class($this)), 1186669086); + } + + return $actionMethodName; + } +} diff --git a/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php b/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php new file mode 100644 index 0000000000..e436e13439 --- /dev/null +++ b/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php @@ -0,0 +1,99 @@ +setStatusCode($statusCode); + if ($content === '') { + $content = sprintf( + '%s %s', + $statusCode, + $statusMessage ?? ResponseInformationHelper::getStatusMessageByCode($statusCode) + ); + } + $response->setContent($content); + $this->throwStopActionWithReponse($response, $content, 1558088618); + } + + /** + * Redirects to another URI + * + * @param UriInterface $uri Either a string representation of a URI or a UriInterface object + * @param integer $delay (optional) The delay in seconds. Default is no delay. + * @param integer $statusCode (optional) The HTTP status code for the redirect. Default is "303 See Other" + * @param ActionResponse|null $response + * @return ActionResponse + * @throws StopActionException + */ + protected function responseRedirectsToUri(UriInterface $uri, int $delay = 0, int $statusCode = 303, ?ActionResponse $response = null): ActionResponse + { + $response = $response ?? new ActionResponse; + + if ($delay < 1) { + $response->setRedirectUri($uri, $statusCode); + $this->throwStopActionWithReponse($response, '', 1699478812); + } + + $response->setStatusCode($statusCode); + $content = sprintf('', $delay, $uri); + $response->setContent($content); + return $response; + } + + /** + * @param ActionResponse $response + * @param string $message + * @param int $code + * @return never + * @throws StopActionException + */ + protected function throwStopActionWithReponse(ActionResponse $response, string $message = '', int $code = 0): never + { + $exception = new StopActionException($message, $code); + $exception->response = $response; + throw $exception; + } + + /** + * Forwards the request to another action and / or controller + * Request is directly transfered to the other action / controller + * + * NOTE that this will not try to convert any objects in the requests arguments, + * this can be a fine or a problem depending on context of usage. + * + * @param ActionRequest $request The request to redirect to + * @return never + * @throws ForwardException + */ + protected function _forwardToRequest(ActionRequest $request): never + { + $nextRequest = clone $request; + $forwardException = new ForwardException(); + $forwardException->setNextRequest($nextRequest); + throw $forwardException; + } +} diff --git a/Neos.Flow/Classes/Mvc/Controller/StandardController.php b/Neos.Flow/Classes/Mvc/Controller/StandardController.php index 101911f19a..f746e22ab5 100644 --- a/Neos.Flow/Classes/Mvc/Controller/StandardController.php +++ b/Neos.Flow/Classes/Mvc/Controller/StandardController.php @@ -25,9 +25,10 @@ class StandardController extends ActionController /** * Overrides the standard resolveView method * + * @param ActionRequest $request * @return ViewInterface $view The view */ - protected function resolveView() + protected function resolveView(ActionRequest $request) { $view = new SimpleTemplateView(['templateSource' => file_get_contents(FLOW_PATH_FLOW . 'Resources/Private/Mvc/StandardView_Template.html')]); $view->setControllerContext($this->controllerContext); diff --git a/Neos.Flow/Classes/Mvc/DispatchMiddleware.php b/Neos.Flow/Classes/Mvc/DispatchMiddleware.php index f80e117934..7e1b4c0bc4 100644 --- a/Neos.Flow/Classes/Mvc/DispatchMiddleware.php +++ b/Neos.Flow/Classes/Mvc/DispatchMiddleware.php @@ -41,8 +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); } - $actionResponse = new ActionResponse(); - $this->dispatcher->dispatch($actionRequest, $actionResponse); - return $actionResponse->buildHttpResponse(); + return $this->dispatcher->dispatch($actionRequest)->buildHttpResponse(); } } diff --git a/Neos.Flow/Classes/Mvc/Dispatcher.php b/Neos.Flow/Classes/Mvc/Dispatcher.php index b3ba774fbd..99e0780c93 100644 --- a/Neos.Flow/Classes/Mvc/Dispatcher.php +++ b/Neos.Flow/Classes/Mvc/Dispatcher.php @@ -85,8 +85,7 @@ public function injectFirewall(FirewallInterface $firewall) * Dispatches a request to a controller * * @param ActionRequest $request The request to dispatch - * @param ActionResponse $response The response, to be modified by the controller - * @return void + * @return ActionResponse * @throws AccessDeniedException * @throws AuthenticationRequiredException * @throws InfiniteLoopException @@ -95,13 +94,13 @@ public function injectFirewall(FirewallInterface $firewall) * @throws MissingConfigurationException * @api */ - public function dispatch(ActionRequest $request, ActionResponse $response) + public function dispatch(ActionRequest $request): ActionResponse { try { if ($this->securityContext->areAuthorizationChecksDisabled() !== true) { $this->firewall->blockIllegalRequests($request); } - $this->initiateDispatchLoop($request, $response); + $response = $this->initiateDispatchLoop($request); } catch (AuthenticationRequiredException $exception) { // Rethrow as the SecurityEntryPoint middleware will take care of the rest throw $exception->attachInterceptedRequest($request); @@ -111,6 +110,8 @@ public function dispatch(ActionRequest $request, ActionResponse $response) $securityLogger->warning('Access denied', LogEnvironment::fromMethodName(__METHOD__)); throw $exception; } + + return $response; } /** @@ -121,20 +122,22 @@ public function dispatch(ActionRequest $request, ActionResponse $response) * @return ActionResponse * @throws InvalidControllerException|InfiniteLoopException|NoSuchOptionException|UnsupportedRequestTypeException */ - protected function initiateDispatchLoop(ActionRequest $request, ActionResponse $parentResponse) + protected function initiateDispatchLoop(ActionRequest $request): ActionResponse { $dispatchLoopCount = 0; + // This response is just for legacy reasons so the signals have something. + $response = new ActionResponse(); while ($request !== null && $request->isDispatched() === false) { if ($dispatchLoopCount++ > 99) { throw new Exception\InfiniteLoopException(sprintf('Could not ultimately dispatch the request after %d iterations.', $dispatchLoopCount), 1217839467); } $controller = $this->resolveController($request); - $response = new ActionResponse(); try { $this->emitBeforeControllerInvocation($request, $response, $controller); - $controller->processRequest($request, $response); + $response = $controller->processRequest($request); $this->emitAfterControllerInvocation($request, $response, $controller); } catch (StopActionException $exception) { + $response = $exception->response ?? $response; $this->emitAfterControllerInvocation($request, $response, $controller); if ($exception instanceof ForwardException) { $request = $exception->getNextRequest(); @@ -142,16 +145,15 @@ protected function initiateDispatchLoop(ActionRequest $request, ActionResponse $ $request = $request->getParentRequest(); } } - $parentResponse = $response->mergeIntoParentResponse($parentResponse); } - return $parentResponse; + return $response; } /** * This signal is emitted directly before the request is been dispatched to a controller. * * @param ActionRequest $request - * @param ActionResponse $response + * @param ActionResponse $response This will always be an empty response, nothing to see here. * @param ControllerInterface $controller * @return void * @Flow\Signal @@ -180,10 +182,9 @@ protected function emitAfterControllerInvocation(ActionRequest $request, ActionR * * @param ActionRequest $request The request to dispatch * @return ControllerInterface - * @throws NoSuchOptionException - * @throws Controller\Exception\InvalidControllerException + * @throws InvalidControllerException */ - protected function resolveController(ActionRequest $request) + protected function resolveController(ActionRequest $request): ControllerInterface { /** @var ActionRequest $request */ $controllerObjectName = $request->getControllerObjectName(); diff --git a/Neos.Flow/Classes/Mvc/Exception/StopActionException.php b/Neos.Flow/Classes/Mvc/Exception/StopActionException.php index ae1b468dbb..8516720d00 100644 --- a/Neos.Flow/Classes/Mvc/Exception/StopActionException.php +++ b/Neos.Flow/Classes/Mvc/Exception/StopActionException.php @@ -11,6 +11,8 @@ * source code. */ +use Neos\Flow\Mvc\ActionResponse; + /** * This exception is thrown by a controller to stop the execution of the current * action and return the control to the dispatcher. The dispatcher catches this @@ -23,4 +25,5 @@ */ class StopActionException extends \Neos\Flow\Mvc\Exception { + public ActionResponse $response; } diff --git a/Neos.Flow/Tests/Functional/Http/Fixtures/Controller/FooController.php b/Neos.Flow/Tests/Functional/Http/Fixtures/Controller/FooController.php index d6a22b38ab..2fd4cab0ec 100644 --- a/Neos.Flow/Tests/Functional/Http/Fixtures/Controller/FooController.php +++ b/Neos.Flow/Tests/Functional/Http/Fixtures/Controller/FooController.php @@ -11,6 +11,7 @@ * source code. */ +use Neos\Flow\Mvc\ActionResponse; use Neos\Flow\Mvc\Controller\AbstractController; class FooController extends AbstractController @@ -18,9 +19,11 @@ class FooController extends AbstractController /** * @inheritDoc */ - public function processRequest($request, $response) + public function processRequest($request): ActionResponse { + $response = new ActionResponse(); $this->initializeController($request, $response); $response->setContent('FooController responded'); + return $response; } } diff --git a/Neos.Flow/Tests/Unit/Mvc/Controller/AbstractControllerTest.php b/Neos.Flow/Tests/Unit/Mvc/Controller/AbstractControllerTest.php index 699264f5a1..09b6d5bf42 100644 --- a/Neos.Flow/Tests/Unit/Mvc/Controller/AbstractControllerTest.php +++ b/Neos.Flow/Tests/Unit/Mvc/Controller/AbstractControllerTest.php @@ -291,13 +291,16 @@ 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 $response) + public function processRequest(ActionRequest $request): ActionResponse { + $response = new ActionResponse(); $mockUriBuilder = $this->uriBuilder; $this->initializeController($request, $response); $this->uriBuilder = $mockUriBuilder; $this->myIndexAction(); + + return $this->response; } public function myIndexAction(): void @@ -309,10 +312,11 @@ public function myIndexAction(): void $this->inject($controller, 'uriBuilder', $mockUriBuilder); try { - $controller->processRequest($this->mockActionRequest, $this->actionResponse); - } catch (StopActionException) { - Assert::assertSame('the_uri', $this->actionResponse->getRedirectUri()?->__toString()); - Assert::assertSame(303, $this->actionResponse->getStatusCode()); + $controller->processRequest($this->mockActionRequest); + } catch (StopActionException $exception) { + $actionReponse = $exception->response; + Assert::assertSame('the_uri', $actionReponse->getRedirectUri()?->__toString()); + Assert::assertSame(303, $actionReponse->getStatusCode()); return; } Assert::assertTrue(false, 'Expected to be redirected.'); @@ -332,13 +336,16 @@ 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 $response) + public function processRequest(ActionRequest $request): ActionResponse { + $response = new ActionResponse(); $mockUriBuilder = $this->uriBuilder; $this->initializeController($request, $response); $this->uriBuilder = $mockUriBuilder; $this->myIndexAction(); + + return $this->response; } public function myIndexAction(): void @@ -350,10 +357,11 @@ public function myIndexAction(): void $this->inject($controller, 'uriBuilder', $mockUriBuilder); try { - $controller->processRequest($this->mockActionRequest, $this->actionResponse); - } catch (StopActionException) { - Assert::assertSame('the_uri', $this->actionResponse->getRedirectUri()?->__toString()); - Assert::assertSame(303, $this->actionResponse->getStatusCode()); + $controller->processRequest($this->mockActionRequest); + } catch (StopActionException $exception) { + $actionReponse = $exception->response; + Assert::assertSame('the_uri', $actionReponse->getRedirectUri()?->__toString()); + Assert::assertSame(303, $actionReponse->getStatusCode()); return; } Assert::assertTrue(false, 'Expected to be redirected.'); @@ -489,13 +497,12 @@ public function mapRequestArgumentsToControllerArgumentsDoesJustThat() } $controller = $this->getAccessibleMock(AbstractController::class, ['processRequest']); - $controller->_call('initializeController', $this->mockActionRequest, $this->actionResponse); $controller->_set('arguments', $controllerArguments); $this->mockActionRequest->expects(self::atLeast(2))->method('hasArgument')->withConsecutive(['foo'], ['baz'])->willReturn(true); $this->mockActionRequest->expects(self::atLeast(2))->method('getArgument')->withConsecutive(['foo'], ['baz'])->willReturnOnConsecutiveCalls('bar', 'quux'); - $controller->_call('mapRequestArgumentsToControllerArguments'); + $controller->_call('mapRequestArgumentsToControllerArguments', $this->mockActionRequest); self::assertEquals('bar', $controllerArguments['foo']->getValue()); self::assertEquals('quux', $controllerArguments['baz']->getValue()); } @@ -518,12 +525,11 @@ public function mapRequestArgumentsToControllerArgumentsThrowsExceptionIfRequire } $controller = $this->getAccessibleMock(AbstractController::class, ['processRequest']); - $controller->_call('initializeController', $this->mockActionRequest, $this->actionResponse); $controller->_set('arguments', $controllerArguments); $this->mockActionRequest->expects(self::exactly(2))->method('hasArgument')->withConsecutive(['foo'], ['baz'])->willReturnOnConsecutiveCalls(true, false); $this->mockActionRequest->expects(self::once())->method('getArgument')->with('foo')->willReturn('bar'); - $controller->_call('mapRequestArgumentsToControllerArguments'); + $controller->_call('mapRequestArgumentsToControllerArguments', $this->mockActionRequest); } } diff --git a/Neos.Flow/Tests/Unit/Mvc/Controller/ActionControllerTest.php b/Neos.Flow/Tests/Unit/Mvc/Controller/ActionControllerTest.php index 523ece5c81..16a4378948 100644 --- a/Neos.Flow/Tests/Unit/Mvc/Controller/ActionControllerTest.php +++ b/Neos.Flow/Tests/Unit/Mvc/Controller/ActionControllerTest.php @@ -83,7 +83,7 @@ public function resolveViewObjectNameReturnsObjectNameOfCustomViewWithFormatSuff { $this->mockObjectManager->expects(self::once())->method('getCaseSensitiveObjectName')->with('some\package\subpackage\view\thecontroller\theactiontheformat')->will(self::returnValue('ResolvedObjectName')); - self::assertSame('ResolvedObjectName', $this->actionController->_call('resolveViewObjectName')); + self::assertSame('ResolvedObjectName', $this->actionController->_call('resolveViewObjectName', $this->mockRequest)); } /** @@ -97,7 +97,7 @@ public function resolveViewObjectNameReturnsObjectNameOfCustomViewWithoutFormatS ['some\package\subpackage\view\thecontroller\theaction'] )->willReturnOnConsecutiveCalls(null, 'ResolvedObjectName'); - self::assertSame('ResolvedObjectName', $this->actionController->_call('resolveViewObjectName')); + self::assertSame('ResolvedObjectName', $this->actionController->_call('resolveViewObjectName', $this->mockRequest)); } /** @@ -112,7 +112,7 @@ public function resolveViewObjectNameRespectsViewFormatToObjectNameMap() ['some\package\subpackage\view\thecontroller\theaction'] )->willReturn(null); - self::assertSame('Some\Custom\View\Object\Name', $this->actionController->_call('resolveViewObjectName')); + self::assertSame('Some\Custom\View\Object\Name', $this->actionController->_call('resolveViewObjectName', $this->mockRequest)); } /** @@ -121,7 +121,7 @@ public function resolveViewObjectNameRespectsViewFormatToObjectNameMap() public function resolveViewReturnsViewResolvedByResolveViewObjectName() { $this->mockObjectManager->expects(self::atLeastOnce())->method('getCaseSensitiveObjectName')->with('some\package\subpackage\view\thecontroller\theactiontheformat')->will(self::returnValue(SimpleTemplateView::class)); - self::assertInstanceOf(SimpleTemplateView::class, $this->actionController->_call('resolveView')); + self::assertInstanceOf(SimpleTemplateView::class, $this->actionController->_call('resolveView', $this->mockRequest)); } /** @@ -131,7 +131,7 @@ public function resolveViewReturnsDefaultViewIfNoViewObjectNameCouldBeResolved() { $this->mockObjectManager->expects(self::any())->method('getCaseSensitiveObjectName')->will(self::returnValue(null)); $this->actionController->_set('defaultViewObjectName', SimpleTemplateView::class); - self::assertInstanceOf(SimpleTemplateView::class, $this->actionController->_call('resolveView')); + self::assertInstanceOf(SimpleTemplateView::class, $this->actionController->_call('resolveView', $this->mockRequest)); } /** @@ -224,10 +224,11 @@ public function processRequestInjectsControllerContextToView() $mockView = $this->createMock(Mvc\View\ViewInterface::class); $mockView->expects(self::once())->method('setControllerContext')->with($this->mockControllerContext); - $this->actionController->expects(self::once())->method('resolveView')->will(self::returnValue($mockView)); - $this->actionController->expects(self::once())->method('resolveActionMethodName')->will(self::returnValue('someAction')); + $this->actionController->expects(self::once())->method('resolveView')->with($this->mockRequest)->will(self::returnValue($mockView)); + $this->actionController->expects(self::once())->method('callActionMethod')->willReturn($mockResponse); + $this->actionController->expects(self::once())->method('resolveActionMethodName')->with($this->mockRequest)->will(self::returnValue('someAction')); - $this->actionController->processRequest($this->mockRequest, $mockResponse); + $this->actionController->processRequest($this->mockRequest); } /** @@ -250,14 +251,13 @@ public function processRequestInjectsSettingsToView() $mockHttpRequest = $this->getMockBuilder(ServerRequestInterface::class)->disableOriginalConstructor()->getMock(); $this->mockRequest->expects(self::any())->method('getHttpRequest')->will(self::returnValue($mockHttpRequest)); - $mockResponse = new Mvc\ActionResponse; - + $mockResponse = new Mvc\ActionResponse(); $mockView = $this->createMock(Mvc\View\ViewInterface::class); $mockView->expects(self::once())->method('assign')->with('settings', $mockSettings); - $this->actionController->expects(self::once())->method('resolveView')->will(self::returnValue($mockView)); - $this->actionController->expects(self::once())->method('resolveActionMethodName')->will(self::returnValue('someAction')); - - $this->actionController->processRequest($this->mockRequest, $mockResponse); + $this->actionController->expects(self::once())->method('resolveView')->with($this->mockRequest)->will(self::returnValue($mockView)); + $this->actionController->expects(self::once())->method('callActionMethod')->willReturn($mockResponse); + $this->actionController->expects(self::once())->method('resolveActionMethodName')->with($this->mockRequest)->will(self::returnValue('someAction')); + $this->actionController->processRequest($this->mockRequest); } public function supportedAndRequestedMediaTypes() @@ -290,10 +290,11 @@ public function processRequestSetsNegotiatedContentTypeOnResponse($supportedMedi $this->mockRequest->method('getHttpRequest')->willReturn($mockHttpRequest); $mockResponse = new Mvc\ActionResponse; + $this->actionController->expects(self::once())->method('callActionMethod')->willReturn($mockResponse); $this->inject($this->actionController, 'supportedMediaTypes', $supportedMediaTypes); - $this->actionController->processRequest($this->mockRequest, $mockResponse); - self::assertSame($expected, $mockResponse->getContentType()); + $response = $this->actionController->processRequest($this->mockRequest); + self::assertSame($expected, $response->getContentType()); } /** @@ -304,9 +305,15 @@ public function processRequestUsesContentTypeFromActionResponse($supportedMediaT { $this->actionController = $this->getAccessibleMock(ActionController::class, ['resolveActionMethodName', 'initializeActionMethodArguments', 'initializeActionMethodValidators', 'resolveView', 'callActionMethod']); $this->actionController->method('resolveActionMethodName')->willReturn('indexAction'); - $this->inject($this->actionController, 'objectManager', $this->mockObjectManager); + $mockResponse = new Mvc\ActionResponse; + $mockResponse->setContentType('application/json'); + $this->inject($this->actionController, 'supportedMediaTypes', ['application/xml']); + + $this->actionController->expects(self::once())->method('callActionMethod')->willReturn($mockResponse); + + $mockMvcPropertyMappingConfigurationService = $this->createMock(Mvc\Controller\MvcPropertyMappingConfigurationService::class); $this->inject($this->actionController, 'mvcPropertyMappingConfigurationService', $mockMvcPropertyMappingConfigurationService); @@ -314,12 +321,10 @@ public function processRequestUsesContentTypeFromActionResponse($supportedMediaT $mockHttpRequest->method('getHeaderLine')->with('Accept')->willReturn('application/xml'); $this->mockRequest->method('getHttpRequest')->willReturn($mockHttpRequest); - $mockResponse = new Mvc\ActionResponse; - $mockResponse->setContentType('application/json'); - $this->inject($this->actionController, 'supportedMediaTypes', ['application/xml']); - $this->actionController->processRequest($this->mockRequest, $mockResponse); - self::assertSame('application/json', $mockResponse->getContentType()); + + $response = $this->actionController->processRequest($this->mockRequest); + self::assertSame('application/json', $response->getContentType()); } /** @@ -348,9 +353,9 @@ public function processRequestUsesContentTypeFromRenderedView($supportedMediaTyp $mockView = $this->createMock(Mvc\View\ViewInterface::class); $mockView->method('render')->willReturn(new Response(200, ['Content-Type' => 'application/json'])); - $this->actionController->expects(self::once())->method('resolveView')->will(self::returnValue($mockView)); + $this->actionController->expects(self::once())->method('resolveView')->with($this->mockRequest)->willReturn($mockView); - $this->actionController->processRequest($this->mockRequest, $mockResponse); + $mockResponse = $this->actionController->processRequest($this->mockRequest); self::assertSame('application/json', $mockResponse->getContentType()); } @@ -362,7 +367,7 @@ public function resolveViewThrowsExceptionIfResolvedViewDoesNotImplementViewInte $this->expectException(Mvc\Exception\ViewNotFoundException::class); $this->mockObjectManager->expects(self::any())->method('getCaseSensitiveObjectName')->will(self::returnValue(null)); $this->actionController->_set('defaultViewObjectName', 'ViewDefaultObjectName'); - $this->actionController->_call('resolveView'); + $this->actionController->_call('resolveView', $this->mockRequest); } public function ignoredValidationArgumentsProvider() diff --git a/Neos.Flow/Tests/Unit/Mvc/DispatchMiddlewareTest.php b/Neos.Flow/Tests/Unit/Mvc/DispatchMiddlewareTest.php index 408c518dda..319e5c588c 100644 --- a/Neos.Flow/Tests/Unit/Mvc/DispatchMiddlewareTest.php +++ b/Neos.Flow/Tests/Unit/Mvc/DispatchMiddlewareTest.php @@ -14,6 +14,7 @@ use GuzzleHttp\Psr7\Response; use Neos\Flow\Http\ServerRequestAttributes; use Neos\Flow\Mvc\ActionRequest; +use Neos\Flow\Mvc\ActionResponse; use Neos\Flow\Mvc\DispatchMiddleware; use Neos\Flow\Mvc\Dispatcher; use Neos\Flow\Tests\UnitTestCase; @@ -78,10 +79,14 @@ protected function setUp(): void */ public function processDispatchesTheRequest() { + $testContentType = 'audio/ogg'; $this->mockHttpRequest->method('getQueryParams')->willReturn([]); - $this->mockDispatcher->expects(self::once())->method('dispatch')->with($this->mockActionRequest); + $testReponse = new ActionResponse(); + $testReponse->setContentType($testContentType); + $this->mockDispatcher->expects(self::once())->method('dispatch')->with($this->mockActionRequest)->willReturn($testReponse); $response = $this->dispatchMiddleware->process($this->mockHttpRequest, $this->mockRequestHandler); self::assertInstanceOf(ResponseInterface::class, $response); + self::assertEquals([$testContentType], $response->getHeader('Content-Type')); } } diff --git a/Neos.Flow/Tests/Unit/Mvc/DispatcherTest.php b/Neos.Flow/Tests/Unit/Mvc/DispatcherTest.php index e632733101..56b134f6b5 100644 --- a/Neos.Flow/Tests/Unit/Mvc/DispatcherTest.php +++ b/Neos.Flow/Tests/Unit/Mvc/DispatcherTest.php @@ -142,9 +142,9 @@ public function dispatchCallsTheControllersProcessRequestMethodUntilTheIsDispatc { $this->mockActionRequest->expects(self::exactly(3))->method('isDispatched')->willReturnOnConsecutiveCalls(false, false, true); - $this->mockController->expects(self::exactly(2))->method('processRequest')->with($this->mockActionRequest); + $this->mockController->expects(self::exactly(2))->method('processRequest')->with($this->mockActionRequest)->willReturn($this->actionResponse); - $this->dispatcher->dispatch($this->mockActionRequest, $this->actionResponse); + $this->dispatcher->dispatch($this->mockActionRequest); } /** @@ -157,7 +157,7 @@ public function dispatchIgnoresStopExceptionsForFirstLevelActionRequests() $this->mockController->expects(self::atLeastOnce())->method('processRequest')->will(self::throwException(new StopActionException())); - $this->dispatcher->dispatch($this->mockParentRequest, $this->actionResponse); + $this->dispatcher->dispatch($this->mockParentRequest); } /** @@ -190,7 +190,7 @@ public function dispatchContinuesWithNextRequestFoundInAForwardException() ->withConsecutive([$this->mockActionRequest], [$this->mockParentRequest]) ->willReturnOnConsecutiveCalls(self::throwException(new StopActionException()), self::throwException($forwardException)); - $this->dispatcher->dispatch($this->mockActionRequest, $this->actionResponse); + $this->dispatcher->dispatch($this->mockActionRequest); } /** @@ -198,6 +198,9 @@ public function dispatchContinuesWithNextRequestFoundInAForwardException() */ public function dispatchThrowsAnInfiniteLoopExceptionIfTheRequestCouldNotBeDispachedAfter99Iterations() { + $response = new ActionResponse(); + $this->mockController->expects(self::any())->method('processRequest')->with($this->mockActionRequest)->willReturn($response); + $this->expectException(InfiniteLoopException::class); $requestCallCounter = 0; $requestCallBack = function () use (&$requestCallCounter) { @@ -205,7 +208,7 @@ public function dispatchThrowsAnInfiniteLoopExceptionIfTheRequestCouldNotBeDispa }; $this->mockParentRequest->method('isDispatched')->will(self::returnCallback($requestCallBack, '__invoke')); - $this->dispatcher->dispatch($this->mockParentRequest, $this->actionResponse); + $this->dispatcher->dispatch($this->mockParentRequest); } /** @@ -218,7 +221,7 @@ public function dispatchDoesNotBlockRequestsIfAuthorizationChecksAreDisabled() $this->mockSecurityContext->method('areAuthorizationChecksDisabled')->willReturn(true); $this->mockFirewall->expects(self::never())->method('blockIllegalRequests'); - $this->dispatcher->dispatch($this->mockActionRequest, $this->actionResponse); + $this->dispatcher->dispatch($this->mockActionRequest); } /** @@ -230,7 +233,7 @@ public function dispatchInterceptsActionRequestsByDefault() $this->mockFirewall->expects(self::once())->method('blockIllegalRequests')->with($this->mockActionRequest); - $this->dispatcher->dispatch($this->mockActionRequest, $this->actionResponse); + $this->dispatcher->dispatch($this->mockActionRequest); } /** @@ -245,7 +248,7 @@ public function dispatchThrowsAuthenticationExceptions() $this->mockFirewall->expects(self::once())->method('blockIllegalRequests')->will(self::throwException(new AuthenticationRequiredException())); - $this->dispatcher->dispatch($this->mockActionRequest, $this->actionResponse); + $this->dispatcher->dispatch($this->mockActionRequest); } /** @@ -258,7 +261,7 @@ public function dispatchRethrowsAccessDeniedException() $this->mockFirewall->expects(self::once())->method('blockIllegalRequests')->will(self::throwException(new AccessDeniedException())); - $this->dispatcher->dispatch($this->mockActionRequest, $this->actionResponse); + $this->dispatcher->dispatch($this->mockActionRequest); } /** diff --git a/Neos.FluidAdaptor/Classes/Core/Widget/AbstractWidgetController.php b/Neos.FluidAdaptor/Classes/Core/Widget/AbstractWidgetController.php index adc78f6d6e..871d1c010b 100644 --- a/Neos.FluidAdaptor/Classes/Core/Widget/AbstractWidgetController.php +++ b/Neos.FluidAdaptor/Classes/Core/Widget/AbstractWidgetController.php @@ -37,13 +37,12 @@ abstract class AbstractWidgetController extends ActionController * Handles a request. The result output is returned by altering the given response. * * @param ActionRequest $request The request object - * @param ActionResponse $response The response, modified by this handler - * @return void + * @return ActionResponse $response The response, modified by this handler * @throws WidgetContextNotFoundException * @throws \Neos\Flow\Mvc\Exception\UnsupportedRequestTypeException * @api */ - public function processRequest(ActionRequest $request, ActionResponse $response) + public function processRequest(ActionRequest $request): ActionResponse { /** @var $widgetContext WidgetContext */ $widgetContext = $request->getInternalArgument('__widgetContext'); @@ -51,6 +50,6 @@ public function processRequest(ActionRequest $request, ActionResponse $response) throw new WidgetContextNotFoundException('The widget context could not be found in the request.', 1307450180); } $this->widgetConfiguration = $widgetContext->getWidgetConfiguration(); - parent::processRequest($request, $response); + return parent::processRequest($request); } } diff --git a/Neos.FluidAdaptor/Classes/Core/Widget/AjaxWidgetMiddleware.php b/Neos.FluidAdaptor/Classes/Core/Widget/AjaxWidgetMiddleware.php index 4823b6a800..be4873b718 100644 --- a/Neos.FluidAdaptor/Classes/Core/Widget/AjaxWidgetMiddleware.php +++ b/Neos.FluidAdaptor/Classes/Core/Widget/AjaxWidgetMiddleware.php @@ -81,11 +81,7 @@ public function process(ServerRequestInterface $httpRequest, RequestHandlerInter $actionRequest->setControllerObjectName($widgetContext->getControllerObjectName()); $this->securityContext->setRequest($actionRequest); - $actionResponse = new ActionResponse(); - - $this->dispatcher->dispatch($actionRequest, $actionResponse); - - return $actionResponse->buildHttpResponse(); + return $this->dispatcher->dispatch($actionRequest)->buildHttpResponse(); } /** diff --git a/Neos.FluidAdaptor/Tests/Unit/Core/Widget/AbstractWidgetControllerTest.php b/Neos.FluidAdaptor/Tests/Unit/Core/Widget/AbstractWidgetControllerTest.php index 1a76194805..a18502df05 100644 --- a/Neos.FluidAdaptor/Tests/Unit/Core/Widget/AbstractWidgetControllerTest.php +++ b/Neos.FluidAdaptor/Tests/Unit/Core/Widget/AbstractWidgetControllerTest.php @@ -15,8 +15,10 @@ use GuzzleHttp\Psr7\Uri; use Neos\Flow\Mvc\ActionResponse; use Neos\Flow\Tests\UnitTestCase; +use Neos\FluidAdaptor\Core\Widget\AbstractWidgetController; use Neos\FluidAdaptor\Core\Widget\Exception\WidgetContextNotFoundException; use Neos\FluidAdaptor\Core\Widget\WidgetContext; +use PHPUnit\Framework\MockObject\MockObject; /** * Test case for AbstractWidgetController @@ -58,11 +60,12 @@ public function processRequestShouldSetWidgetConfiguration() $mockActionRequest->expects(self::atLeastOnce())->method('getInternalArgument')->with('__widgetContext')->will(self::returnValue($widgetContext)); + /** @var AbstractWidgetController|MockObject $abstractWidgetController */ $abstractWidgetController = $this->getAccessibleMock(\Neos\FluidAdaptor\Core\Widget\AbstractWidgetController::class, ['resolveActionMethodName', 'initializeActionMethodArguments', 'initializeActionMethodValidators', 'mapRequestArgumentsToControllerArguments', 'detectFormat', 'resolveView', 'callActionMethod']); $abstractWidgetController->method('resolveActionMethodName')->willReturn('indexAction'); $abstractWidgetController->_set('mvcPropertyMappingConfigurationService', $this->createMock(\Neos\Flow\Mvc\Controller\MvcPropertyMappingConfigurationService::class)); - - $abstractWidgetController->processRequest($mockActionRequest, $mockResponse); + $abstractWidgetController->expects(self::once())->method('callActionMethod')->willReturn($mockResponse); + $abstractWidgetController->processRequest($mockActionRequest); $actualWidgetConfiguration = $abstractWidgetController->_get('widgetConfiguration'); self::assertEquals($expectedWidgetConfiguration, $actualWidgetConfiguration); diff --git a/Neos.FluidAdaptor/Tests/Unit/Core/Widget/AjaxWidgetMiddlewareTest.php b/Neos.FluidAdaptor/Tests/Unit/Core/Widget/AjaxWidgetMiddlewareTest.php index 568db98704..82cf7ec3f7 100644 --- a/Neos.FluidAdaptor/Tests/Unit/Core/Widget/AjaxWidgetMiddlewareTest.php +++ b/Neos.FluidAdaptor/Tests/Unit/Core/Widget/AjaxWidgetMiddlewareTest.php @@ -14,6 +14,7 @@ use GuzzleHttp\Psr7\Response; use Neos\Flow\Mvc\ActionRequest; use Neos\Flow\Mvc\ActionRequestFactory; +use Neos\Flow\Mvc\ActionResponse; use Neos\Flow\Mvc\Dispatcher; use Neos\Flow\ObjectManagement\ObjectManagerInterface; use Neos\Flow\Security\Context; @@ -119,6 +120,8 @@ protected function setUp(): void $this->inject($this->ajaxWidgetMiddleware, 'hashService', $this->mockHashService); $this->mockDispatcher = $this->getMockBuilder(Dispatcher::class)->getMock(); + $actionReponse = new ActionResponse(); + $this->mockDispatcher->expects(self::any())->method('dispatch')->willReturn($actionReponse); $this->inject($this->ajaxWidgetMiddleware, 'dispatcher', $this->mockDispatcher); $this->mockSecurityContext = $this->getMockBuilder(Context::class)->getMock(); @@ -177,7 +180,8 @@ public function handleDispatchesActionRequestIfWidgetContextIsPresent() $mockActionRequest = $this->getMockBuilder(ActionRequest::class)->disableOriginalConstructor()->getMock(); $this->mockActionRequestFactory->method('prepareActionRequest')->willReturn($mockActionRequest); - $this->mockDispatcher->expects(self::once())->method('dispatch'); + $actionReponse = new ActionResponse(); + $this->mockDispatcher->expects(self::once())->method('dispatch')->willReturn($actionReponse);; $this->ajaxWidgetMiddleware->process($this->mockHttpRequest, $this->mockRequestHandler); } From 8ee71e95d60d55d9926bde03529c24222df90196 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Mu=CC=88ller?= Date: Tue, 26 Dec 2023 14:09:14 +0100 Subject: [PATCH 02/21] Controller cleanup --- Neos.Flow/Classes/Mvc/ActionRequest.php | 5 +- .../Mvc/Controller/AbstractController.php | 81 ++++++++----------- .../Mvc/Controller/ActionController.php | 12 +-- .../Mvc/Controller/ControllerInterface.php | 2 - .../Controller/SpecialResponsesSupport.php | 34 ++++---- Neos.Flow/Classes/Mvc/Dispatcher.php | 4 +- .../UnsupportedRequestTypeException.php | 2 +- Neos.Flow/Classes/Mvc/Routing/UriBuilder.php | 8 +- .../Mvc/Controller/AbstractControllerTest.php | 12 ++- .../Core/Widget/AbstractWidgetController.php | 18 ++++- 10 files changed, 89 insertions(+), 89 deletions(-) diff --git a/Neos.Flow/Classes/Mvc/ActionRequest.php b/Neos.Flow/Classes/Mvc/ActionRequest.php index 0ac0af4336..4878948d88 100644 --- a/Neos.Flow/Classes/Mvc/ActionRequest.php +++ b/Neos.Flow/Classes/Mvc/ActionRequest.php @@ -510,7 +510,7 @@ public function setArgument(string $argumentName, $value): void throw new Exception\InvalidArgumentNameException('Invalid argument name (must be a non-empty string).', 1210858767); } - if (strpos($argumentName, '__') === 0) { + if (str_starts_with($argumentName, '__')) { $this->internalArguments[$argumentName] = $value; return; } @@ -520,7 +520,7 @@ public function setArgument(string $argumentName, $value): void throw new Exception\InvalidArgumentTypeException('You are not allowed to store objects in the request arguments. Please convert the object of type "' . get_class($value) . '" given for argument "' . $argumentName . '" to a simple type first.', 1302783022); } - if (strpos($argumentName, '--') === 0) { + if (str_starts_with($argumentName, '--')) { $this->pluginArguments[substr($argumentName, 2)] = $value; return; } @@ -698,6 +698,7 @@ public function getFormat(): string * @return void * @Flow\Signal * @throws \Neos\Flow\SignalSlot\Exception\InvalidSlotException + * @deprecated Since Flow 9.0 as this signal has no meaning for quite some time, you might as well use Dispatcher::beforeControllerInvocation */ protected function emitRequestDispatched($request): void { diff --git a/Neos.Flow/Classes/Mvc/Controller/AbstractController.php b/Neos.Flow/Classes/Mvc/Controller/AbstractController.php index 8056890b31..6ee3e44ad4 100644 --- a/Neos.Flow/Classes/Mvc/Controller/AbstractController.php +++ b/Neos.Flow/Classes/Mvc/Controller/AbstractController.php @@ -14,8 +14,13 @@ use Neos\Error\Messages as Error; use Neos\Flow\Annotations as Flow; use GuzzleHttp\Psr7\Uri; +use Neos\Flow\Mvc\Exception\InvalidActionNameException; +use Neos\Flow\Mvc\Exception\InvalidArgumentNameException; +use Neos\Flow\Mvc\Exception\InvalidArgumentTypeException; +use Neos\Flow\Mvc\Exception\InvalidControllerNameException; use Neos\Flow\Mvc\Exception\NoSuchArgumentException; use Neos\Flow\Mvc\Routing\Exception\MissingActionNameException; +use Neos\Flow\Persistence\Exception\UnknownObjectException; use Neos\Flow\Property\Exception; use Psr\Http\Message\UriInterface; use Neos\Flow\Http\Helper\MediaTypeHelper; @@ -25,8 +30,6 @@ use Neos\Flow\Mvc\Exception\ForwardException; use Neos\Flow\Mvc\Exception\RequiredArgumentMissingException; use Neos\Flow\Mvc\Exception\StopActionException; -use Neos\Flow\Mvc\Exception\UnsupportedRequestTypeException; -use Neos\Flow\Mvc\FlashMessage\FlashMessageContainer; use Neos\Flow\Mvc\Routing\UriBuilder; use Neos\Flow\Persistence\PersistenceManagerInterface; use Neos\Flow\Validation\ValidatorResolver; @@ -106,7 +109,6 @@ abstract class AbstractController implements ControllerInterface * * @param ActionRequest $request * @param ActionResponse $response - * @throws UnsupportedRequestTypeException */ protected function initializeController(ActionRequest $request, ActionResponse $response) { @@ -185,14 +187,20 @@ public function addFlashMessage($messageBody, $messageTitle = '', $severity = Er * Request is directly transferred to the other action / controller * * @param string $actionName Name of the action to forward to - * @param string $controllerName Unqualified object name of the controller to forward to. If not specified, the current controller is used. - * @param string $packageKey Key of the package containing the controller to forward to. May also contain the sub package, concatenated with backslash (Vendor.Foo\Bar\Baz). If not specified, the current package is assumed. - * @param array $arguments Arguments to pass to the target action + * @param string|null $controllerName Unqualified object name of the controller to forward to. If not specified, the current controller is used. + * @param string|null $packageKey Key of the package containing the controller to forward to. May also contain the sub package, concatenated with backslash (Vendor.Foo\Bar\Baz). If not specified, the current package is assumed. + * @param array $arguments Arguments to pass to the target action + * @return never * @throws ForwardException + * @throws InvalidActionNameException + * @throws InvalidArgumentNameException + * @throws InvalidArgumentTypeException + * @throws InvalidControllerNameException + * @throws UnknownObjectException * @see redirect() * @api */ - protected function forward($actionName, $controllerName = null, $packageKey = null, array $arguments = []): never + protected function forward(string $actionName, string $controllerName = null, string $packageKey = null, array $arguments = []): never { $nextRequest = clone $this->request; $nextRequest->setControllerActionName($actionName); @@ -200,7 +208,7 @@ protected function forward($actionName, $controllerName = null, $packageKey = nu if ($controllerName !== null) { $nextRequest->setControllerName($controllerName); } - if ($packageKey !== null && strpos($packageKey, '\\') !== false) { + if ($packageKey !== null && str_contains($packageKey, '\\')) { [$packageKey, $subpackageKey] = explode('\\', $packageKey, 2); } else { $subpackageKey = null; @@ -212,7 +220,7 @@ protected function forward($actionName, $controllerName = null, $packageKey = nu $regularArguments = []; foreach ($arguments as $argumentName => $argumentValue) { - if (substr($argumentName, 0, 2) === '__') { + if (str_starts_with($argumentName, '__')) { $nextRequest->setArgument($argumentName, $argumentValue); } else { $regularArguments[$argumentName] = $argumentValue; @@ -221,30 +229,7 @@ protected function forward($actionName, $controllerName = null, $packageKey = nu $nextRequest->setArguments($this->persistenceManager->convertObjectsToIdentityArrays($regularArguments)); $this->arguments->removeAll(); - $forwardException = new ForwardException(); - $forwardException->setNextRequest($nextRequest); - throw $forwardException; - } - - /** - * Forwards the request to another action and / or controller. - * - * Request is directly transfered to the other action / controller - * - * @param ActionRequest $request The request to redirect to - * @return void - * @throws ForwardException - * @see redirectToRequest() - * @api - */ - protected function forwardToRequest(ActionRequest $request) - { - $packageKey = $request->getControllerPackageKey(); - $subpackageKey = $request->getControllerSubpackageKey(); - if ($subpackageKey !== null) { - $packageKey .= '\\' . $subpackageKey; - } - $this->forward($request->getControllerActionName(), $request->getControllerName(), $packageKey, $request->getArguments()); + $this->forwardToRequset($nextRequest); } /** @@ -256,23 +241,22 @@ protected function forwardToRequest(ActionRequest $request) * if used with other request types. * * @param string $actionName Name of the action to forward to - * @param null $controllerName Unqualified object name of the controller to forward to. If not specified, the current controller is used. - * @param null $packageKey Key of the package containing the controller to forward to. If not specified, the current package is assumed. - * @param array $arguments Array of arguments for the target action + * @param string|null $controllerName Unqualified object name of the controller to forward to. If not specified, the current controller is used. + * @param string|null $packageKey Key of the package containing the controller to forward to. If not specified, the current package is assumed. + * @param array $arguments Array of arguments for the target action * @param integer $delay (optional) The delay in seconds. Default is no delay. * @param integer $statusCode (optional) The HTTP status code for the redirect. Default is "303 See Other" - * @param null $format The format to use for the redirect URI + * @param string|null $format The format to use for the redirect URI * @return never * @throws StopActionException - * @throws UnsupportedRequestTypeException * @throws \Neos\Flow\Http\Exception * @throws MissingActionNameException * @see forward() * @api */ - protected function redirect($actionName, $controllerName = null, $packageKey = null, array $arguments = [], $delay = 0, $statusCode = 303, $format = null): never + protected function redirect(string $actionName, ?string $controllerName = null, ?string $packageKey = null, array $arguments = [], int $delay = 0, int $statusCode = 303, string $format = null): never { - if ($packageKey !== null && strpos($packageKey, '\\') !== false) { + if ($packageKey !== null && str_contains($packageKey, '\\') !== false) { [$packageKey, $subpackageKey] = explode('\\', $packageKey, 2); } else { $subpackageKey = null; @@ -299,15 +283,14 @@ protected function redirect($actionName, $controllerName = null, $packageKey = n * @param ActionRequest $request The request to redirect to * @param integer $delay (optional) The delay in seconds. Default is no delay. * @param integer $statusCode (optional) The HTTP status code for the redirect. Default is "303 See Other" - * @return void + * @return never * @throws MissingActionNameException * @throws StopActionException - * @throws UnsupportedRequestTypeException * @throws \Neos\Flow\Http\Exception * @see forwardToRequest() * @api */ - protected function redirectToRequest(ActionRequest $request, $delay = 0, $statusCode = 303) + protected function redirectToRequest(ActionRequest $request, int $delay = 0, int $statusCode = 303): never { $packageKey = $request->getControllerPackageKey(); $subpackageKey = $request->getControllerSubpackageKey(); @@ -320,7 +303,7 @@ protected function redirectToRequest(ActionRequest $request, $delay = 0, $status /** * Redirects to another URI * - * @param mixed $uri Either a string representation of a URI or a \Neos\Flow\Http\Uri object + * @param UriInterface|string $uri Either a string representation of a URI or a \Neos\Flow\Http\Uri object * @param integer $delay (optional) The delay in seconds. Default is no delay. * @param integer $statusCode (optional) The HTTP status code for the redirect. Default is "303 See Other" * @throws StopActionException @@ -332,7 +315,7 @@ protected function redirectToUri(string|UriInterface $uri, int $delay = 0, int $ $uri = new Uri($uri); } - $response = $this->responseRedirectsToUri($uri,$delay, $statusCode, $this->response); + $response = $this->responseRedirectsToUri($uri, $delay, $statusCode, $this->response); $this->throwStopActionWithReponse($response, '', 1699716808); } @@ -345,11 +328,11 @@ protected function redirectToUri(string|UriInterface $uri, int $delay = 0, int $ * @param string $statusMessage A custom HTTP status message * @param string $content Body content which further explains the status * @throws StopActionException - * @api + * @deprecated Use SpecialResponsesSupport::reponseThrowsStatus + * @see SpecialResponsesSupport::reponseThrowsStatus */ protected function throwStatus(int $statusCode, $statusMessage = null, $content = null): never { - $this->response->setStatusCode($statusCode); if ($content === null) { $content = sprintf( '%s %s', @@ -357,8 +340,8 @@ protected function throwStatus(int $statusCode, $statusMessage = null, $content $statusMessage ?? ResponseInformationHelper::getStatusMessageByCode($statusCode) ); } - $this->response->setContent($content); - throw new StopActionException($content, 1558088618); + + $this->reponseThrowsStatus($statusCode, $content, $this->response); } /** diff --git a/Neos.Flow/Classes/Mvc/Controller/ActionController.php b/Neos.Flow/Classes/Mvc/Controller/ActionController.php index 70288cacfc..9b85befbca 100644 --- a/Neos.Flow/Classes/Mvc/Controller/ActionController.php +++ b/Neos.Flow/Classes/Mvc/Controller/ActionController.php @@ -22,13 +22,14 @@ use Neos\Flow\Mvc\Exception\InvalidActionVisibilityException; use Neos\Flow\Mvc\Exception\InvalidArgumentTypeException; use Neos\Flow\Mvc\Exception\NoSuchActionException; +use Neos\Flow\Mvc\Exception\NoSuchArgumentException; use Neos\Flow\Mvc\Exception\RequiredArgumentMissingException; use Neos\Flow\Mvc\Exception\StopActionException; -use Neos\Flow\Mvc\Exception\UnsupportedRequestTypeException; use Neos\Flow\Mvc\Exception\ViewNotFoundException; use Neos\Flow\Mvc\View\ViewInterface; use Neos\Flow\Mvc\ViewConfigurationManager; use Neos\Flow\ObjectManagement\ObjectManagerInterface; +use Neos\Flow\Property\Exception; use Neos\Flow\Property\Exception\TargetNotFoundException; use Neos\Flow\Property\TypeConverter\Error\TargetNotFoundError; use Neos\Flow\Reflection\ReflectionService; @@ -206,10 +207,11 @@ public function injectThrowableStorage(ThrowableStorageInterface $throwableStora * @throws InvalidActionVisibilityException * @throws InvalidArgumentTypeException * @throws NoSuchActionException - * @throws UnsupportedRequestTypeException - * @throws ViewNotFoundException - * @throws \Neos\Flow\Mvc\Exception\RequiredArgumentMissingException * @throws StopActionException + * @throws ViewNotFoundException + * @throws NoSuchArgumentException + * @throws Exception + * @throws \Neos\Flow\Security\Exception * @api */ public function processRequest(ActionRequest $request): ActionResponse @@ -380,7 +382,7 @@ protected function getInformationNeededForInitializeActionMethodValidators() /** * Adds the needed validators to the Arguments: * - * - Validators checking the data type from the @param annotation + * - Validators checking the data type from the "@param" annotation * - Custom validators specified with validate annotations. * - Model-based validators (validate annotations in the model) * - Custom model validator classes diff --git a/Neos.Flow/Classes/Mvc/Controller/ControllerInterface.php b/Neos.Flow/Classes/Mvc/Controller/ControllerInterface.php index 0062e5af9d..3df03c026e 100644 --- a/Neos.Flow/Classes/Mvc/Controller/ControllerInterface.php +++ b/Neos.Flow/Classes/Mvc/Controller/ControllerInterface.php @@ -15,7 +15,6 @@ use Neos\Flow\Mvc\ActionResponse; use Neos\Flow\Mvc\Exception\ForwardException; use Neos\Flow\Mvc\Exception\StopActionException; -use Neos\Flow\Mvc\Exception\UnsupportedRequestTypeException; /** * Generic interface for controllers @@ -35,7 +34,6 @@ interface ControllerInterface * * @param ActionRequest $request The request object * @return ActionResponse $response The response, modified by the controller - * @throws UnsupportedRequestTypeException if the controller doesn't support the current request type * @throws StopActionException * @throws ForwardException * @api diff --git a/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php b/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php index e436e13439..c87b7dc535 100644 --- a/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php +++ b/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php @@ -6,6 +6,7 @@ use Neos\Flow\Mvc\ActionResponse; use Neos\Flow\Mvc\Exception\ForwardException; use Neos\Flow\Mvc\Exception\StopActionException; +use Neos\Flow\Persistence\Doctrine\PersistenceManager; use Psr\Http\Message\UriInterface; /** @@ -17,25 +18,20 @@ trait SpecialResponsesSupport * Sends the specified HTTP status immediately. * * @param integer $statusCode The HTTP status code - * @param ?string $statusMessage A custom HTTP status message - * @param string $content Body content which further explains the status - * @param ActionResponse $response + * @param string $content Body content which further explains the status the body of a given response will be overwritten if this is not empty + * @param ActionResponse|null $response The response to use or null for an empty response with the given status and message or content * @return never * @throws StopActionException */ - protected function reponseThrowsStatus(int $statusCode, ?string $statusMessage = null, string $content = '', ?ActionResponse $response = null): never + protected function reponseThrowsStatus(int $statusCode, string $content = '', ?ActionResponse $response = null): never { $response = $response ?? new ActionResponse; $response->setStatusCode($statusCode); - if ($content === '') { - $content = sprintf( - '%s %s', - $statusCode, - $statusMessage ?? ResponseInformationHelper::getStatusMessageByCode($statusCode) - ); + if ($content !== '') { + $response->setContent($content); } - $response->setContent($content); + $this->throwStopActionWithReponse($response, $content, 1558088618); } @@ -45,23 +41,23 @@ protected function reponseThrowsStatus(int $statusCode, ?string $statusMessage = * @param UriInterface $uri Either a string representation of a URI or a UriInterface object * @param integer $delay (optional) The delay in seconds. Default is no delay. * @param integer $statusCode (optional) The HTTP status code for the redirect. Default is "303 See Other" - * @param ActionResponse|null $response + * @param ActionResponse|null $response response that will have status, and location or body overwritten. * @return ActionResponse * @throws StopActionException */ protected function responseRedirectsToUri(UriInterface $uri, int $delay = 0, int $statusCode = 303, ?ActionResponse $response = null): ActionResponse { - $response = $response ?? new ActionResponse; + $nextResponse = $response !== null ? clone $response : new ActionResponse(); if ($delay < 1) { - $response->setRedirectUri($uri, $statusCode); - $this->throwStopActionWithReponse($response, '', 1699478812); + $nextResponse->setRedirectUri($uri, $statusCode); + $this->throwStopActionWithReponse($nextResponse, '', 1699478812); } - $response->setStatusCode($statusCode); + $nextResponse->setStatusCode($statusCode); $content = sprintf('', $delay, $uri); - $response->setContent($content); - return $response; + $nextResponse->setContent($content); + return $nextResponse; } /** @@ -89,7 +85,7 @@ protected function throwStopActionWithReponse(ActionResponse $response, string $ * @return never * @throws ForwardException */ - protected function _forwardToRequest(ActionRequest $request): never + protected function forwardToRequset(ActionRequest $request): never { $nextRequest = clone $request; $forwardException = new ForwardException(); diff --git a/Neos.Flow/Classes/Mvc/Dispatcher.php b/Neos.Flow/Classes/Mvc/Dispatcher.php index 99e0780c93..459c581497 100644 --- a/Neos.Flow/Classes/Mvc/Dispatcher.php +++ b/Neos.Flow/Classes/Mvc/Dispatcher.php @@ -20,7 +20,6 @@ use Neos\Flow\Mvc\Exception\InfiniteLoopException; use Neos\Flow\Mvc\Exception\StopActionException; use Neos\Flow\Mvc\Exception\ForwardException; -use Neos\Flow\Mvc\Exception\UnsupportedRequestTypeException; use Neos\Flow\ObjectManagement\ObjectManagerInterface; use Neos\Flow\Security\Authorization\FirewallInterface; use Neos\Flow\Security\Context; @@ -118,9 +117,8 @@ public function dispatch(ActionRequest $request): ActionResponse * Try processing the request until it is successfully marked "dispatched" * * @param ActionRequest $request - * @param ActionResponse $parentResponse * @return ActionResponse - * @throws InvalidControllerException|InfiniteLoopException|NoSuchOptionException|UnsupportedRequestTypeException + * @throws InvalidControllerException|InfiniteLoopException|NoSuchOptionException */ protected function initiateDispatchLoop(ActionRequest $request): ActionResponse { diff --git a/Neos.Flow/Classes/Mvc/Exception/UnsupportedRequestTypeException.php b/Neos.Flow/Classes/Mvc/Exception/UnsupportedRequestTypeException.php index aae2e5e83b..c173f22642 100644 --- a/Neos.Flow/Classes/Mvc/Exception/UnsupportedRequestTypeException.php +++ b/Neos.Flow/Classes/Mvc/Exception/UnsupportedRequestTypeException.php @@ -14,7 +14,7 @@ /** * An "Unsupported Request Type" exception * - * @api + * @deprecated since Flow 9.0 */ class UnsupportedRequestTypeException extends \Neos\Flow\Mvc\Exception { diff --git a/Neos.Flow/Classes/Mvc/Routing/UriBuilder.php b/Neos.Flow/Classes/Mvc/Routing/UriBuilder.php index 5eb57b2a94..e90eb50c03 100644 --- a/Neos.Flow/Classes/Mvc/Routing/UriBuilder.php +++ b/Neos.Flow/Classes/Mvc/Routing/UriBuilder.php @@ -279,10 +279,10 @@ public function reset() * Creates an URI used for linking to an Controller action. * * @param string $actionName Name of the action to be called - * @param array $controllerArguments Additional query parameters. Will be merged with $this->arguments. - * @param string $controllerName Name of the target controller. If not set, current ControllerName is used. - * @param string $packageKey Name of the target package. If not set, current Package is used. - * @param string $subPackageKey Name of the target SubPackage. If not set, current SubPackage is used. + * @param array $controllerArguments Additional query parameters. Will be merged with $this->arguments. + * @param string|null $controllerName Name of the target controller. If not set, current ControllerName is used. + * @param string|null $packageKey Name of the target package. If not set, current Package is used. + * @param string|null $subPackageKey Name of the target SubPackage. If not set, current SubPackage is used. * @return string the rendered URI * @api * @see build() diff --git a/Neos.Flow/Tests/Unit/Mvc/Controller/AbstractControllerTest.php b/Neos.Flow/Tests/Unit/Mvc/Controller/AbstractControllerTest.php index 09b6d5bf42..ca15d095d5 100644 --- a/Neos.Flow/Tests/Unit/Mvc/Controller/AbstractControllerTest.php +++ b/Neos.Flow/Tests/Unit/Mvc/Controller/AbstractControllerTest.php @@ -388,12 +388,16 @@ public function redirectToUriSetsStatus() $controller = $this->getAccessibleMock(AbstractController::class, ['processRequest']); $controller->_call('initializeController', $this->mockActionRequest, $this->actionResponse); + $response = null; try { $controller->_call('redirectToUri', 'http://some.uri'); } catch (StopActionException $e) { + // The dispatcher takes the response from the exception, so it makes sense to check that + $response = $e->response; } - self::assertSame(303, $this->actionResponse->getStatusCode()); + self::assertNotNull($response); + self::assertSame(303, $response->getStatusCode()); } /** @@ -406,12 +410,16 @@ public function redirectToUriSetsRedirectUri() $controller = $this->getAccessibleMock(AbstractController::class, ['processRequest']); $controller->_call('initializeController', $this->mockActionRequest, $this->actionResponse); + $response = null; try { $controller->_call('redirectToUri', $uri); } catch (StopActionException $e) { + // The dispatcher takes the response from the exception, so it makes sense to check that + $response = $e->response; } - self::assertSame($uri, (string)$this->actionResponse->getRedirectUri()); + self::assertNotNull($response); + self::assertSame($uri, (string)$response->getRedirectUri()); } /** diff --git a/Neos.FluidAdaptor/Classes/Core/Widget/AbstractWidgetController.php b/Neos.FluidAdaptor/Classes/Core/Widget/AbstractWidgetController.php index 871d1c010b..7ce14c80f8 100644 --- a/Neos.FluidAdaptor/Classes/Core/Widget/AbstractWidgetController.php +++ b/Neos.FluidAdaptor/Classes/Core/Widget/AbstractWidgetController.php @@ -14,6 +14,13 @@ use Neos\Flow\Mvc\ActionRequest; use Neos\Flow\Mvc\ActionResponse; use Neos\Flow\Mvc\Controller\ActionController; +use Neos\Flow\Mvc\Exception\InvalidActionVisibilityException; +use Neos\Flow\Mvc\Exception\InvalidArgumentTypeException; +use Neos\Flow\Mvc\Exception\NoSuchActionException; +use Neos\Flow\Mvc\Exception\NoSuchArgumentException; +use Neos\Flow\Mvc\Exception\StopActionException; +use Neos\Flow\Mvc\Exception\ViewNotFoundException; +use Neos\Flow\Property\Exception; use Neos\FluidAdaptor\Core\Widget\Exception\WidgetContextNotFoundException; /** @@ -39,12 +46,19 @@ abstract class AbstractWidgetController extends ActionController * @param ActionRequest $request The request object * @return ActionResponse $response The response, modified by this handler * @throws WidgetContextNotFoundException - * @throws \Neos\Flow\Mvc\Exception\UnsupportedRequestTypeException + * @throws InvalidActionVisibilityException + * @throws InvalidArgumentTypeException + * @throws NoSuchActionException + * @throws NoSuchArgumentException + * @throws StopActionException + * @throws ViewNotFoundException + * @throws Exception + * @throws \Neos\Flow\Security\Exception * @api */ public function processRequest(ActionRequest $request): ActionResponse { - /** @var $widgetContext WidgetContext */ + /** @var WidgetContext $widgetContext */ $widgetContext = $request->getInternalArgument('__widgetContext'); if (!$widgetContext instanceof WidgetContext) { throw new WidgetContextNotFoundException('The widget context could not be found in the request.', 1307450180); From 0fcf5223ab2475482212284e9b2c04df31b200ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Mu=CC=88ller?= Date: Tue, 26 Dec 2023 14:46:02 +0100 Subject: [PATCH 03/21] Style and suggestions cleanup --- .../Classes/Mvc/Controller/AbstractController.php | 8 ++++---- .../Classes/Mvc/Controller/SimpleActionController.php | 2 +- .../Classes/Mvc/Controller/SpecialResponsesSupport.php | 10 ++++------ .../Classes/Core/Widget/AjaxWidgetMiddleware.php | 1 - .../Unit/Core/Widget/AjaxWidgetMiddlewareTest.php | 2 +- 5 files changed, 10 insertions(+), 13 deletions(-) diff --git a/Neos.Flow/Classes/Mvc/Controller/AbstractController.php b/Neos.Flow/Classes/Mvc/Controller/AbstractController.php index 6ee3e44ad4..4cd42db358 100644 --- a/Neos.Flow/Classes/Mvc/Controller/AbstractController.php +++ b/Neos.Flow/Classes/Mvc/Controller/AbstractController.php @@ -316,7 +316,7 @@ protected function redirectToUri(string|UriInterface $uri, int $delay = 0, int $ } $response = $this->responseRedirectsToUri($uri, $delay, $statusCode, $this->response); - $this->throwStopActionWithReponse($response, '', 1699716808); + $this->throwStopActionWithResponse($response, '', 1699716808); } /** @@ -328,8 +328,8 @@ protected function redirectToUri(string|UriInterface $uri, int $delay = 0, int $ * @param string $statusMessage A custom HTTP status message * @param string $content Body content which further explains the status * @throws StopActionException - * @deprecated Use SpecialResponsesSupport::reponseThrowsStatus - * @see SpecialResponsesSupport::reponseThrowsStatus + * @deprecated Use SpecialResponsesSupport::responseThrowsStatus + * @see SpecialResponsesSupport::responseThrowsStatus */ protected function throwStatus(int $statusCode, $statusMessage = null, $content = null): never { @@ -341,7 +341,7 @@ protected function throwStatus(int $statusCode, $statusMessage = null, $content ); } - $this->reponseThrowsStatus($statusCode, $content, $this->response); + $this->responseThrowsStatus($statusCode, $content, $this->response); } /** diff --git a/Neos.Flow/Classes/Mvc/Controller/SimpleActionController.php b/Neos.Flow/Classes/Mvc/Controller/SimpleActionController.php index d0cb5f120c..e0e2dde2f3 100644 --- a/Neos.Flow/Classes/Mvc/Controller/SimpleActionController.php +++ b/Neos.Flow/Classes/Mvc/Controller/SimpleActionController.php @@ -35,7 +35,7 @@ public function processRequest(ActionRequest $request): ActionResponse * @return string Method name of the current action * @throws NoSuchActionException */ - protected function resolveActionMethodName(ActionRequest $request) + protected function resolveActionMethodName(ActionRequest $request): string { $actionMethodName = $request->getControllerActionName() . 'Action'; if (!is_callable([$this, $actionMethodName])) { diff --git a/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php b/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php index c87b7dc535..89fa8928a1 100644 --- a/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php +++ b/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php @@ -1,12 +1,10 @@ setContent($content); } - $this->throwStopActionWithReponse($response, $content, 1558088618); + $this->throwStopActionWithResponse($response, $content, 1558088618); } /** @@ -51,7 +49,7 @@ protected function responseRedirectsToUri(UriInterface $uri, int $delay = 0, int if ($delay < 1) { $nextResponse->setRedirectUri($uri, $statusCode); - $this->throwStopActionWithReponse($nextResponse, '', 1699478812); + $this->throwStopActionWithResponse($nextResponse, '', 1699478812); } $nextResponse->setStatusCode($statusCode); @@ -67,7 +65,7 @@ protected function responseRedirectsToUri(UriInterface $uri, int $delay = 0, int * @return never * @throws StopActionException */ - protected function throwStopActionWithReponse(ActionResponse $response, string $message = '', int $code = 0): never + protected function throwStopActionWithResponse(ActionResponse $response, string $message = '', int $code = 0): never { $exception = new StopActionException($message, $code); $exception->response = $response; diff --git a/Neos.FluidAdaptor/Classes/Core/Widget/AjaxWidgetMiddleware.php b/Neos.FluidAdaptor/Classes/Core/Widget/AjaxWidgetMiddleware.php index be4873b718..d8637a4965 100644 --- a/Neos.FluidAdaptor/Classes/Core/Widget/AjaxWidgetMiddleware.php +++ b/Neos.FluidAdaptor/Classes/Core/Widget/AjaxWidgetMiddleware.php @@ -15,7 +15,6 @@ use Neos\Flow\Annotations as Flow; use Neos\Flow\Mvc\ActionRequestFactory; -use Neos\Flow\Mvc\ActionResponse; use Neos\Flow\Mvc\Dispatcher; use Neos\Flow\Security\Context; use Neos\Flow\Security\Cryptography\HashService; diff --git a/Neos.FluidAdaptor/Tests/Unit/Core/Widget/AjaxWidgetMiddlewareTest.php b/Neos.FluidAdaptor/Tests/Unit/Core/Widget/AjaxWidgetMiddlewareTest.php index 82cf7ec3f7..19c352d2ad 100644 --- a/Neos.FluidAdaptor/Tests/Unit/Core/Widget/AjaxWidgetMiddlewareTest.php +++ b/Neos.FluidAdaptor/Tests/Unit/Core/Widget/AjaxWidgetMiddlewareTest.php @@ -181,7 +181,7 @@ public function handleDispatchesActionRequestIfWidgetContextIsPresent() $this->mockActionRequestFactory->method('prepareActionRequest')->willReturn($mockActionRequest); $actionReponse = new ActionResponse(); - $this->mockDispatcher->expects(self::once())->method('dispatch')->willReturn($actionReponse);; + $this->mockDispatcher->expects(self::once())->method('dispatch')->willReturn($actionReponse); $this->ajaxWidgetMiddleware->process($this->mockHttpRequest, $this->mockRequestHandler); } From e3fb34b5f02d2d26022f4f46d18dbdbd56d1a610 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Mu=CC=88ller?= Date: Tue, 26 Dec 2023 15:41:25 +0100 Subject: [PATCH 04/21] Tests for SimpleActionController --- .../SimpleActionControllerTestController.php | 48 ++++++++++ .../Mvc/SimpleActionControllerTest.php | 89 +++++++++++++++++++ .../Mvc/Controller/AbstractControllerTest.php | 12 +-- .../Fixtures/SimpleActionTestController.php | 19 ++++ .../Controller/SimpleActionControllerTest.php | 75 ++++++++++++++++ .../Core/Widget/AjaxWidgetMiddlewareTest.php | 8 +- 6 files changed, 241 insertions(+), 10 deletions(-) create mode 100644 Neos.Flow/Tests/Functional/Mvc/Fixtures/Controller/SimpleActionControllerTestController.php create mode 100644 Neos.Flow/Tests/Functional/Mvc/SimpleActionControllerTest.php create mode 100644 Neos.Flow/Tests/Unit/Mvc/Controller/Fixtures/SimpleActionTestController.php create mode 100644 Neos.Flow/Tests/Unit/Mvc/Controller/SimpleActionControllerTest.php diff --git a/Neos.Flow/Tests/Functional/Mvc/Fixtures/Controller/SimpleActionControllerTestController.php b/Neos.Flow/Tests/Functional/Mvc/Fixtures/Controller/SimpleActionControllerTestController.php new file mode 100644 index 0000000000..990b6c2256 --- /dev/null +++ b/Neos.Flow/Tests/Functional/Mvc/Fixtures/Controller/SimpleActionControllerTestController.php @@ -0,0 +1,48 @@ +setContent('index'); + + return $response; + } + + public function simpleReponseAction(ActionRequest $actionRequest): ActionResponse + { + $response = new ActionResponse(); + $response->setContent('Simple'); + + return $response; + } + + public function jsonResponseAction(ActionRequest $actionRequest): ActionResponse + { + $response = new ActionResponse(); + $response->setContent(json_encode(['foo' => 'bar', 'baz' => 123])); + $response->setContentType('application/json'); + + return $response; + } + + public function argumentsAction(ActionRequest $actionRequest): ActionResponse + { + $response = new ActionResponse(); + $response->setContent(strtolower($actionRequest->getArgument('testArgument'))); + if ($actionRequest->getFormat() === 'html') { + $response->setContentType('text/html'); + } + + return $response; + } +} diff --git a/Neos.Flow/Tests/Functional/Mvc/SimpleActionControllerTest.php b/Neos.Flow/Tests/Functional/Mvc/SimpleActionControllerTest.php new file mode 100644 index 0000000000..9e8d70b192 --- /dev/null +++ b/Neos.Flow/Tests/Functional/Mvc/SimpleActionControllerTest.php @@ -0,0 +1,89 @@ +registerRoute('test', 'test/mvc/simplecontrollertest(/{@action})', [ + '@package' => 'Neos.Flow', + '@subpackage' => 'Tests\Functional\Mvc\Fixtures', + '@controller' => 'SimpleActionControllerTest', + '@action' => 'index', + '@format' => 'html' + ]); + + $this->serverRequestFactory = $this->objectManager->get(ServerRequestFactoryInterface::class); + } + + /** + * Checks if a simple request is handled correctly. The route matching the + * specified URI defines a default action "index" which results in indexAction + * being called. + * + * @test + */ + public function defaultActionSpecifiedInRouteIsCalledAndResponseIsReturned() + { + $response = $this->browser->request('http://localhost/test/mvc/simplecontrollertest'); + self::assertEquals(200, $response->getStatusCode()); + self::assertEquals('index', $response->getBody()->getContents()); + } + + /** + * Checks if a simple request is handled correctly if another than the default + * action is specified. + * + * @test + */ + public function actionSpecifiedInActionRequestIsCalledAndResponseIsReturned() + { + $response = $this->browser->request('http://localhost/test/mvc/simplecontrollertest/simplereponse'); + self::assertEquals(200, $response->getStatusCode()); + self::assertEquals('Simple', $response->getBody()->getContents()); + } + + /** + * Checks if the content type and json content is correctly passed through + * + * @test + */ + public function responseContainsContentTypeAndContent() + { + $response = $this->browser->request('http://localhost/test/mvc/simplecontrollertest/jsonresponse'); + self::assertEquals(200, $response->getStatusCode()); + self::assertEquals(['application/json'], $response->getHeader('Content-Type')); + self::assertEquals(json_encode(['foo' => 'bar', 'baz' => 123]), $response->getBody()->getContents()); + } + + /** + * Checks if arguments and format are passed + * + * @test + */ + public function responseContainsArgumentContent() + { + $argument = '

Some markup

'; + $response = $this->browser->request('http://localhost/test/mvc/simplecontrollertest/arguments?testArgument=' . urlencode($argument)); + self::assertEquals(200, $response->getStatusCode()); + self::assertEquals(['text/html'], $response->getHeader('Content-Type')); + self::assertEquals(strtolower($argument), $response->getBody()->getContents()); + } +} diff --git a/Neos.Flow/Tests/Unit/Mvc/Controller/AbstractControllerTest.php b/Neos.Flow/Tests/Unit/Mvc/Controller/AbstractControllerTest.php index ca15d095d5..b8b994c207 100644 --- a/Neos.Flow/Tests/Unit/Mvc/Controller/AbstractControllerTest.php +++ b/Neos.Flow/Tests/Unit/Mvc/Controller/AbstractControllerTest.php @@ -314,9 +314,9 @@ public function myIndexAction(): void try { $controller->processRequest($this->mockActionRequest); } catch (StopActionException $exception) { - $actionReponse = $exception->response; - Assert::assertSame('the_uri', $actionReponse->getRedirectUri()?->__toString()); - Assert::assertSame(303, $actionReponse->getStatusCode()); + $actionResponse = $exception->response; + Assert::assertSame('the_uri', $actionResponse->getRedirectUri()?->__toString()); + Assert::assertSame(303, $actionResponse->getStatusCode()); return; } Assert::assertTrue(false, 'Expected to be redirected.'); @@ -359,9 +359,9 @@ public function myIndexAction(): void try { $controller->processRequest($this->mockActionRequest); } catch (StopActionException $exception) { - $actionReponse = $exception->response; - Assert::assertSame('the_uri', $actionReponse->getRedirectUri()?->__toString()); - Assert::assertSame(303, $actionReponse->getStatusCode()); + $actionResponse = $exception->response; + Assert::assertSame('the_uri', $actionResponse->getRedirectUri()?->__toString()); + Assert::assertSame(303, $actionResponse->getStatusCode()); return; } Assert::assertTrue(false, 'Expected to be redirected.'); diff --git a/Neos.Flow/Tests/Unit/Mvc/Controller/Fixtures/SimpleActionTestController.php b/Neos.Flow/Tests/Unit/Mvc/Controller/Fixtures/SimpleActionTestController.php new file mode 100644 index 0000000000..4670322eb7 --- /dev/null +++ b/Neos.Flow/Tests/Unit/Mvc/Controller/Fixtures/SimpleActionTestController.php @@ -0,0 +1,19 @@ +setContent('Simple'); + return $response; + } +} diff --git a/Neos.Flow/Tests/Unit/Mvc/Controller/SimpleActionControllerTest.php b/Neos.Flow/Tests/Unit/Mvc/Controller/SimpleActionControllerTest.php new file mode 100644 index 0000000000..5b71cda0c5 --- /dev/null +++ b/Neos.Flow/Tests/Unit/Mvc/Controller/SimpleActionControllerTest.php @@ -0,0 +1,75 @@ +createMock(ActionRequest::class); + $request->method('getControllerActionName')->willReturn($actionName); + + $this->expectException(NoSuchActionException::class); + + $testObject = new Fixtures\SimpleActionTestController(); + $testObject->processRequest($request); + } + + /** + * @return void + * @throws NoSuchActionException + * @throws \Neos\Flow\SignalSlot\Exception\InvalidSlotException + * @test + */ + public function responseOnlyContainsWhatActionSets() + { + $request = $this->createMock(ActionRequest::class); + $request->method('getControllerActionName')->willReturn('addTestContent'); + + $testObject = new Fixtures\SimpleActionTestController(); + $response = $testObject->processRequest($request); + self::assertInstanceOf(ActionResponse::class, $response); + self::assertEquals('Simple', $response->getContent()); + // default + self::assertEquals(200, $response->getStatusCode()); + self::assertFalse($response->hasContentType()); + + } +} diff --git a/Neos.FluidAdaptor/Tests/Unit/Core/Widget/AjaxWidgetMiddlewareTest.php b/Neos.FluidAdaptor/Tests/Unit/Core/Widget/AjaxWidgetMiddlewareTest.php index 19c352d2ad..4dacef28f3 100644 --- a/Neos.FluidAdaptor/Tests/Unit/Core/Widget/AjaxWidgetMiddlewareTest.php +++ b/Neos.FluidAdaptor/Tests/Unit/Core/Widget/AjaxWidgetMiddlewareTest.php @@ -120,8 +120,8 @@ protected function setUp(): void $this->inject($this->ajaxWidgetMiddleware, 'hashService', $this->mockHashService); $this->mockDispatcher = $this->getMockBuilder(Dispatcher::class)->getMock(); - $actionReponse = new ActionResponse(); - $this->mockDispatcher->expects(self::any())->method('dispatch')->willReturn($actionReponse); + $actionResponse = new ActionResponse(); + $this->mockDispatcher->expects(self::any())->method('dispatch')->willReturn($actionResponse); $this->inject($this->ajaxWidgetMiddleware, 'dispatcher', $this->mockDispatcher); $this->mockSecurityContext = $this->getMockBuilder(Context::class)->getMock(); @@ -180,8 +180,8 @@ public function handleDispatchesActionRequestIfWidgetContextIsPresent() $mockActionRequest = $this->getMockBuilder(ActionRequest::class)->disableOriginalConstructor()->getMock(); $this->mockActionRequestFactory->method('prepareActionRequest')->willReturn($mockActionRequest); - $actionReponse = new ActionResponse(); - $this->mockDispatcher->expects(self::once())->method('dispatch')->willReturn($actionReponse); + $actionResponse = new ActionResponse(); + $this->mockDispatcher->expects(self::once())->method('dispatch')->willReturn($actionResponse); $this->ajaxWidgetMiddleware->process($this->mockHttpRequest, $this->mockRequestHandler); } From b12f1df37de9ce73440c948faa2cb468feffeb04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Mu=CC=88ller?= Date: Wed, 27 Dec 2023 15:22:51 +0100 Subject: [PATCH 05/21] Fix AjaxWidget dispatching to use new StopActionException::response --- Neos.Flow/Classes/Mvc/Controller/ActionController.php | 6 +++--- .../Classes/Core/Widget/AbstractWidgetViewHelper.php | 4 ++-- .../Classes/Core/Widget/AjaxWidgetMiddleware.php | 1 - 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/Neos.Flow/Classes/Mvc/Controller/ActionController.php b/Neos.Flow/Classes/Mvc/Controller/ActionController.php index 9b85befbca..dcd32c525f 100644 --- a/Neos.Flow/Classes/Mvc/Controller/ActionController.php +++ b/Neos.Flow/Classes/Mvc/Controller/ActionController.php @@ -559,12 +559,12 @@ protected function callActionMethod(ActionRequest $request, ActionResponse $resp } if ($actionResult === null && $this->view instanceof ViewInterface) { - $response = $this->renderView($response); + $this->response = $this->renderView($this->response); } else { - $response->setContent($actionResult); + $this->response->setContent($actionResult); } - return $response; + return $this->response; } /** diff --git a/Neos.FluidAdaptor/Classes/Core/Widget/AbstractWidgetViewHelper.php b/Neos.FluidAdaptor/Classes/Core/Widget/AbstractWidgetViewHelper.php index 57a4befcc2..2fa376f802 100644 --- a/Neos.FluidAdaptor/Classes/Core/Widget/AbstractWidgetViewHelper.php +++ b/Neos.FluidAdaptor/Classes/Core/Widget/AbstractWidgetViewHelper.php @@ -226,7 +226,6 @@ protected function initiateSubRequest() $subRequest->setArgumentNamespace('--' . $this->widgetContext->getWidgetIdentifier()); $dispatchLoopCount = 0; - $subResponse = new ActionResponse(); $content = ''; while (!$subRequest->isDispatched()) { if ($dispatchLoopCount++ > 99) { @@ -240,12 +239,13 @@ protected function initiateSubRequest() } $subRequest->setControllerObjectName($this->widgetContext->getControllerObjectName()); try { - $this->controller->processRequest($subRequest, $subResponse); + $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(''); } catch (StopActionException $exception) { + $subResponse = $exception->response ?? $subResponse; if ($exception instanceof ForwardException) { $subRequest = $exception->getNextRequest(); continue; diff --git a/Neos.FluidAdaptor/Classes/Core/Widget/AjaxWidgetMiddleware.php b/Neos.FluidAdaptor/Classes/Core/Widget/AjaxWidgetMiddleware.php index d8637a4965..6641c8fbed 100644 --- a/Neos.FluidAdaptor/Classes/Core/Widget/AjaxWidgetMiddleware.php +++ b/Neos.FluidAdaptor/Classes/Core/Widget/AjaxWidgetMiddleware.php @@ -79,7 +79,6 @@ 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(); } From 33601301a8f9694961359c940ce7bde6e04fd150 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Mu=CC=88ller?= Date: Wed, 27 Dec 2023 15:33:53 +0100 Subject: [PATCH 06/21] Correct style issues --- Neos.Flow/Tests/Functional/Mvc/SimpleActionControllerTest.php | 1 - .../Tests/Unit/Mvc/Controller/SimpleActionControllerTest.php | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/Neos.Flow/Tests/Functional/Mvc/SimpleActionControllerTest.php b/Neos.Flow/Tests/Functional/Mvc/SimpleActionControllerTest.php index 9e8d70b192..32e44cf2cc 100644 --- a/Neos.Flow/Tests/Functional/Mvc/SimpleActionControllerTest.php +++ b/Neos.Flow/Tests/Functional/Mvc/SimpleActionControllerTest.php @@ -1,7 +1,6 @@ processRequest($request); self::assertInstanceOf(ActionResponse::class, $response); self::assertEquals('Simple', $response->getContent()); + self::assertFalse($response->hasContentType()); // default self::assertEquals(200, $response->getStatusCode()); - self::assertFalse($response->hasContentType()); - } } From 0689c7840a3fc2ab1753604c4d1d7e915438ceb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Mu=CC=88ller?= Date: Thu, 28 Dec 2023 13:15:06 +0100 Subject: [PATCH 07/21] Fix spelling error --- Neos.Flow/Classes/Mvc/Controller/AbstractController.php | 5 +---- Neos.Flow/Classes/Mvc/Controller/SimpleActionController.php | 6 ++++-- .../Classes/Mvc/Controller/SpecialResponsesSupport.php | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/Neos.Flow/Classes/Mvc/Controller/AbstractController.php b/Neos.Flow/Classes/Mvc/Controller/AbstractController.php index 4cd42db358..a7321ed649 100644 --- a/Neos.Flow/Classes/Mvc/Controller/AbstractController.php +++ b/Neos.Flow/Classes/Mvc/Controller/AbstractController.php @@ -229,7 +229,7 @@ protected function forward(string $actionName, string $controllerName = null, st $nextRequest->setArguments($this->persistenceManager->convertObjectsToIdentityArrays($regularArguments)); $this->arguments->removeAll(); - $this->forwardToRequset($nextRequest); + $this->forwardToRequest($nextRequest); } /** @@ -277,9 +277,6 @@ protected function redirect(string $actionName, ?string $controllerName = null, * * Redirect will be sent to the client which then performs another request to the new URI. * - * NOTE: This method only supports web requests and will throw an exception - * if used with other request types. - * * @param ActionRequest $request The request to redirect to * @param integer $delay (optional) The delay in seconds. Default is no delay. * @param integer $statusCode (optional) The HTTP status code for the redirect. Default is "303 See Other" diff --git a/Neos.Flow/Classes/Mvc/Controller/SimpleActionController.php b/Neos.Flow/Classes/Mvc/Controller/SimpleActionController.php index e0e2dde2f3..35b2e3e596 100644 --- a/Neos.Flow/Classes/Mvc/Controller/SimpleActionController.php +++ b/Neos.Flow/Classes/Mvc/Controller/SimpleActionController.php @@ -10,9 +10,11 @@ * Provides most direct access to our request/response abstraction, * the request comes directly from the dispatcher and goes directly back to it. * - * Views or other processing needs to be added to your controller as needed. - * For helpers to facilitate throws, forwards, redirects and such: + * For helpers to facilitate throws, forwards, redirects: * @see SpecialResponsesSupport + * + * Views or other processing needs to be added to your controller as needed, + * helpers will be suggested here as they become available. */ class SimpleActionController implements ControllerInterface { diff --git a/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php b/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php index 89fa8928a1..ff0090d73f 100644 --- a/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php +++ b/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php @@ -83,7 +83,7 @@ protected function throwStopActionWithResponse(ActionResponse $response, string * @return never * @throws ForwardException */ - protected function forwardToRequset(ActionRequest $request): never + protected function forwardToRequest(ActionRequest $request): never { $nextRequest = clone $request; $forwardException = new ForwardException(); From 88f43d5f59ec6c8c430777914e18a65abbf14953 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Thu, 25 Jan 2024 20:07:52 +0100 Subject: [PATCH 08/21] TASK: Adjust typos and doc of `ControllerInterface` --- .../Classes/Mvc/Controller/ControllerInterface.php | 13 ++++++++----- .../Mvc/Controller/SpecialResponsesSupport.php | 2 +- Neos.Flow/Tests/Unit/Mvc/DispatchMiddlewareTest.php | 6 +++--- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/Neos.Flow/Classes/Mvc/Controller/ControllerInterface.php b/Neos.Flow/Classes/Mvc/Controller/ControllerInterface.php index 3df03c026e..623bae911f 100644 --- a/Neos.Flow/Classes/Mvc/Controller/ControllerInterface.php +++ b/Neos.Flow/Classes/Mvc/Controller/ControllerInterface.php @@ -30,12 +30,15 @@ interface ControllerInterface { /** - * Processes a general request. The result can be returned by altering the given response. + * Processes a general request. * - * @param ActionRequest $request The request object - * @return ActionResponse $response The response, modified by the controller - * @throws StopActionException - * @throws ForwardException + * The action request can either be returned, or handed over + * by throwing a dedicated exception with response attached. + * + * @param ActionRequest $request The dispatched action request + * @return ActionResponse The resulting created response + * @throws StopActionException is allowed for exceptional control flow + * @throws ForwardException is allowed for exceptional control flow * @api */ public function processRequest(ActionRequest $request): ActionResponse; diff --git a/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php b/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php index ff0090d73f..3a5a6422f4 100644 --- a/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php +++ b/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php @@ -74,7 +74,7 @@ protected function throwStopActionWithResponse(ActionResponse $response, string /** * Forwards the request to another action and / or controller - * Request is directly transfered to the other action / controller + * Request is directly transferred to the other action / controller * * NOTE that this will not try to convert any objects in the requests arguments, * this can be a fine or a problem depending on context of usage. diff --git a/Neos.Flow/Tests/Unit/Mvc/DispatchMiddlewareTest.php b/Neos.Flow/Tests/Unit/Mvc/DispatchMiddlewareTest.php index 319e5c588c..2000dcc3dc 100644 --- a/Neos.Flow/Tests/Unit/Mvc/DispatchMiddlewareTest.php +++ b/Neos.Flow/Tests/Unit/Mvc/DispatchMiddlewareTest.php @@ -81,9 +81,9 @@ public function processDispatchesTheRequest() { $testContentType = 'audio/ogg'; $this->mockHttpRequest->method('getQueryParams')->willReturn([]); - $testReponse = new ActionResponse(); - $testReponse->setContentType($testContentType); - $this->mockDispatcher->expects(self::once())->method('dispatch')->with($this->mockActionRequest)->willReturn($testReponse); + $testResponse = new ActionResponse(); + $testResponse->setContentType($testContentType); + $this->mockDispatcher->expects(self::once())->method('dispatch')->with($this->mockActionRequest)->willReturn($testResponse); $response = $this->dispatchMiddleware->process($this->mockHttpRequest, $this->mockRequestHandler); self::assertInstanceOf(ResponseInterface::class, $response); From 03662a77a8c402e86c8088aa5a577609920ef72f Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Thu, 25 Jan 2024 21:45:45 +0100 Subject: [PATCH 09/21] TASK: Fix `RestController::redirectToUri` Original this method and logic was introduced via: https://github.com/neos/flow-development-collection/commit/b7801183c74bd1be72b80378fbf1cc220cadc195 The previous try catch would still work but mutating `$this->response` AFTER throwing an exception is pretty ugly. --- .../Mvc/Controller/AbstractController.php | 2 +- .../Classes/Mvc/Controller/RestController.php | 30 +++++++++---------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/Neos.Flow/Classes/Mvc/Controller/AbstractController.php b/Neos.Flow/Classes/Mvc/Controller/AbstractController.php index a7321ed649..a0fafc3e6c 100644 --- a/Neos.Flow/Classes/Mvc/Controller/AbstractController.php +++ b/Neos.Flow/Classes/Mvc/Controller/AbstractController.php @@ -300,7 +300,7 @@ protected function redirectToRequest(ActionRequest $request, int $delay = 0, int /** * Redirects to another URI * - * @param UriInterface|string $uri Either a string representation of a URI or a \Neos\Flow\Http\Uri object + * @param UriInterface|string $uri Either a string or a psr uri * @param integer $delay (optional) The delay in seconds. Default is no delay. * @param integer $statusCode (optional) The HTTP status code for the redirect. Default is "303 See Other" * @throws StopActionException diff --git a/Neos.Flow/Classes/Mvc/Controller/RestController.php b/Neos.Flow/Classes/Mvc/Controller/RestController.php index bc694bf0f3..174f9c4b5c 100644 --- a/Neos.Flow/Classes/Mvc/Controller/RestController.php +++ b/Neos.Flow/Classes/Mvc/Controller/RestController.php @@ -11,6 +11,7 @@ * source code. */ +use GuzzleHttp\Psr7\Uri; use Neos\Flow\Annotations as Flow; use Neos\Flow\Mvc\ActionRequest; use Neos\Flow\Mvc\ActionResponse; @@ -19,6 +20,7 @@ use Neos\Flow\Mvc\Exception\NoSuchActionException; use Neos\Flow\Mvc\Exception\StopActionException; use Neos\Flow\Property\TypeConverter\PersistentObjectConverter; +use Psr\Http\Message\UriInterface; /** * An action controller for RESTful web services @@ -116,28 +118,26 @@ protected function initializeUpdateAction() } /** - * Redirects the web request to another uri. + * Redirects to another URI * - * NOTE: This method only supports web requests and will throw an exception - * if used with other request types. - * - * @param mixed $uri Either a string representation of a URI or a \Neos\Flow\Http\Uri object + * @param UriInterface|string $uri Either a string or a psr uri * @param integer $delay (optional) The delay in seconds. Default is no delay. * @param integer $statusCode (optional) The HTTP status code for the redirect. Default is "303 See Other" * @throws StopActionException * @api */ - protected function redirectToUri($uri, $delay = 0, $statusCode = 303): never + protected function redirectToUri(string|UriInterface $uri, int $delay = 0, int $statusCode = 303): never { - // the parent method throws the exception, but we need to act afterwards - // thus the code in catch - it's the expected state - try { - parent::redirectToUri($uri, $delay, $statusCode); - } catch (StopActionException $exception) { - if ($this->request->getFormat() === 'json') { - $this->response->setContent(''); - } - throw $exception; + if (!$uri instanceof UriInterface) { + $uri = new Uri($uri); } + + $response = $this->responseRedirectsToUri($uri, $delay, $statusCode, $this->response); + if ($this->request->getFormat() === 'json') { + // send empty body on redirects for JSON requests + $response->setContent(''); + } + + $this->throwStopActionWithResponse($response, '', 1699716808); } } From 3df0dbd087f2216193560c9305158b16622edbcc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20M=C3=BCller?= Date: Fri, 26 Jan 2024 10:01:08 +0100 Subject: [PATCH 10/21] Update Neos.Flow/Tests/Functional/Http/Fixtures/Controller/FooController.php Co-authored-by: Marc Henry Schultz <85400359+mhsdesign@users.noreply.github.com> --- .../Tests/Functional/Http/Fixtures/Controller/FooController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Neos.Flow/Tests/Functional/Http/Fixtures/Controller/FooController.php b/Neos.Flow/Tests/Functional/Http/Fixtures/Controller/FooController.php index 2fd4cab0ec..411fa67efe 100644 --- a/Neos.Flow/Tests/Functional/Http/Fixtures/Controller/FooController.php +++ b/Neos.Flow/Tests/Functional/Http/Fixtures/Controller/FooController.php @@ -14,7 +14,7 @@ use Neos\Flow\Mvc\ActionResponse; use Neos\Flow\Mvc\Controller\AbstractController; -class FooController extends AbstractController +class FooController implements ControllerInterface { /** * @inheritDoc From 57d8995ef960e374bccadd2254ce68ac97f672e2 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Thu, 25 Jan 2024 21:45:45 +0100 Subject: [PATCH 11/21] TASK: Update `ControllerInterface::processRequest` doc --- .../Mvc/Controller/ControllerInterface.php | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/Neos.Flow/Classes/Mvc/Controller/ControllerInterface.php b/Neos.Flow/Classes/Mvc/Controller/ControllerInterface.php index 623bae911f..515db1d117 100644 --- a/Neos.Flow/Classes/Mvc/Controller/ControllerInterface.php +++ b/Neos.Flow/Classes/Mvc/Controller/ControllerInterface.php @@ -32,13 +32,20 @@ interface ControllerInterface /** * Processes a general request. * - * The action request can either be returned, or handed over - * by throwing a dedicated exception with response attached. + * The contract to the MVC Dispatcher is as follows. + * + * - If a request can be handled it must be set to `dispatched` {@see ActionRequest::setDispatched()} + * - The repose should be returned directly. + * - For outer ordinary control flow a {@see StopActionException} with response attached + * can be thrown which will be handled by the Dispatcher accordingly. + * + * - To forward the request to another controller, a {@see ForwardException} might be thrown + * wich the Dispatcher will catch and handle its attached next-request. * * @param ActionRequest $request The dispatched action request * @return ActionResponse The resulting created response - * @throws StopActionException is allowed for exceptional control flow - * @throws ForwardException is allowed for exceptional control flow + * @throws StopActionException + * @throws ForwardException * @api */ public function processRequest(ActionRequest $request): ActionResponse; From f3ba560f30b528f0cd536294e73af7b8023b4bb4 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Fri, 26 Jan 2024 10:32:24 +0100 Subject: [PATCH 12/21] TASK: Fix `FooController` test --- .../Functional/Http/Fixtures/Controller/FooController.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Neos.Flow/Tests/Functional/Http/Fixtures/Controller/FooController.php b/Neos.Flow/Tests/Functional/Http/Fixtures/Controller/FooController.php index 411fa67efe..35e6dbd723 100644 --- a/Neos.Flow/Tests/Functional/Http/Fixtures/Controller/FooController.php +++ b/Neos.Flow/Tests/Functional/Http/Fixtures/Controller/FooController.php @@ -14,7 +14,7 @@ use Neos\Flow\Mvc\ActionResponse; use Neos\Flow\Mvc\Controller\AbstractController; -class FooController implements ControllerInterface +class FooController extends AbstractController { /** * @inheritDoc @@ -22,6 +22,7 @@ class FooController implements ControllerInterface public function processRequest($request): ActionResponse { $response = new ActionResponse(); + // test's AbstractController::initializeController $this->initializeController($request, $response); $response->setContent('FooController responded'); return $response; From e3cdfb4e21b522a8837a87e2343f68c667b27bd8 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Thu, 25 Jan 2024 21:23:12 +0100 Subject: [PATCH 13/21] !!! TASK: Separate `ForwardException` from `StopActionException` --- .../Mvc/Controller/ActionController.php | 1 + .../Controller/SpecialResponsesSupport.php | 7 ++---- Neos.Flow/Classes/Mvc/Dispatcher.php | 9 ++++--- .../Mvc/Exception/ForwardException.php | 25 ++++++------------- .../Mvc/Exception/StopActionException.php | 21 ++++++++++++++-- Neos.Flow/Tests/Unit/Mvc/DispatcherTest.php | 9 +++---- .../Core/Widget/AbstractWidgetController.php | 2 ++ .../Core/Widget/AbstractWidgetViewHelper.php | 9 +++---- 8 files changed, 44 insertions(+), 39 deletions(-) diff --git a/Neos.Flow/Classes/Mvc/Controller/ActionController.php b/Neos.Flow/Classes/Mvc/Controller/ActionController.php index dcd32c525f..4e16b02bcc 100644 --- a/Neos.Flow/Classes/Mvc/Controller/ActionController.php +++ b/Neos.Flow/Classes/Mvc/Controller/ActionController.php @@ -208,6 +208,7 @@ public function injectThrowableStorage(ThrowableStorageInterface $throwableStora * @throws InvalidArgumentTypeException * @throws NoSuchActionException * @throws StopActionException + * @throws ForwardException * @throws ViewNotFoundException * @throws NoSuchArgumentException * @throws Exception diff --git a/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php b/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php index 3a5a6422f4..1b5b702450 100644 --- a/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php +++ b/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php @@ -67,9 +67,7 @@ protected function responseRedirectsToUri(UriInterface $uri, int $delay = 0, int */ protected function throwStopActionWithResponse(ActionResponse $response, string $message = '', int $code = 0): never { - $exception = new StopActionException($message, $code); - $exception->response = $response; - throw $exception; + throw StopActionException::create($response, $message, $code); } /** @@ -86,8 +84,7 @@ protected function throwStopActionWithResponse(ActionResponse $response, string protected function forwardToRequest(ActionRequest $request): never { $nextRequest = clone $request; - $forwardException = new ForwardException(); - $forwardException->setNextRequest($nextRequest); + $forwardException = ForwardException::create($nextRequest); throw $forwardException; } } diff --git a/Neos.Flow/Classes/Mvc/Dispatcher.php b/Neos.Flow/Classes/Mvc/Dispatcher.php index 459c581497..6b225a88bf 100644 --- a/Neos.Flow/Classes/Mvc/Dispatcher.php +++ b/Neos.Flow/Classes/Mvc/Dispatcher.php @@ -135,13 +135,14 @@ protected function initiateDispatchLoop(ActionRequest $request): ActionResponse $response = $controller->processRequest($request); $this->emitAfterControllerInvocation($request, $response, $controller); } catch (StopActionException $exception) { - $response = $exception->response ?? $response; + $response = $exception->response; $this->emitAfterControllerInvocation($request, $response, $controller); - if ($exception instanceof ForwardException) { - $request = $exception->getNextRequest(); - } elseif (!$request->isMainRequest()) { + if (!$request->isMainRequest()) { $request = $request->getParentRequest(); } + } catch (ForwardException $exception) { + $this->emitAfterControllerInvocation($request, $response, $controller); + $request = $exception->nextRequest; } } return $response; diff --git a/Neos.Flow/Classes/Mvc/Exception/ForwardException.php b/Neos.Flow/Classes/Mvc/Exception/ForwardException.php index 157d604374..095cd2b0c5 100644 --- a/Neos.Flow/Classes/Mvc/Exception/ForwardException.php +++ b/Neos.Flow/Classes/Mvc/Exception/ForwardException.php @@ -19,32 +19,21 @@ * * @api */ -class ForwardException extends StopActionException +final class ForwardException extends \Neos\Flow\Mvc\Exception { /** - * @var ActionRequest + * @var ActionRequest The next request, containing the information about the next action to execute. */ - protected $nextRequest; + public readonly ActionRequest $nextRequest; - /** - * Sets the next request, containing the information about the next action to - * execute. - * - * @param ActionRequest $nextRequest - * @return void - */ - public function setNextRequest(ActionRequest $nextRequest) + private function __construct(string $message, int $code, ?\Throwable $previous, ActionRequest $nextRequest) { + parent::__construct($message, $code, $previous); $this->nextRequest = $nextRequest; } - /** - * Returns the next request - * - * @return ActionRequest - */ - public function getNextRequest() + public static function create(ActionRequest $nextRequest) { - return $this->nextRequest; + return new self('', 0, null, $nextRequest); } } diff --git a/Neos.Flow/Classes/Mvc/Exception/StopActionException.php b/Neos.Flow/Classes/Mvc/Exception/StopActionException.php index 8516720d00..06379f9c2a 100644 --- a/Neos.Flow/Classes/Mvc/Exception/StopActionException.php +++ b/Neos.Flow/Classes/Mvc/Exception/StopActionException.php @@ -23,7 +23,24 @@ * * @api */ -class StopActionException extends \Neos\Flow\Mvc\Exception +final class StopActionException extends \Neos\Flow\Mvc\Exception { - public ActionResponse $response; + /** + * As throwing the exception allows for an unusual control flow, we attach the response for the dispatcher. + */ + public readonly ActionResponse $response; + + private function __construct(string $message, int $code, ?\Throwable $previous, ActionResponse $response) + { + parent::__construct($message, $code, $previous); + $this->response = $response; + } + + public static function create( + ActionResponse $response, + string $message, + int $code + ) { + return new self($message, $code, null, $response); + } } diff --git a/Neos.Flow/Tests/Unit/Mvc/DispatcherTest.php b/Neos.Flow/Tests/Unit/Mvc/DispatcherTest.php index 56b134f6b5..ee2cfd24f1 100644 --- a/Neos.Flow/Tests/Unit/Mvc/DispatcherTest.php +++ b/Neos.Flow/Tests/Unit/Mvc/DispatcherTest.php @@ -155,7 +155,7 @@ public function dispatchIgnoresStopExceptionsForFirstLevelActionRequests() $this->mockParentRequest->expects(self::exactly(2))->method('isDispatched')->willReturnOnConsecutiveCalls(false, true); $this->mockParentRequest->expects(self::once())->method('isMainRequest')->willReturn(true); - $this->mockController->expects(self::atLeastOnce())->method('processRequest')->will(self::throwException(new StopActionException())); + $this->mockController->expects(self::atLeastOnce())->method('processRequest')->will(self::throwException(StopActionException::create(new ActionResponse(), '', 0))); $this->dispatcher->dispatch($this->mockParentRequest); } @@ -168,7 +168,7 @@ public function dispatchCatchesStopExceptionOfActionRequestsAndRollsBackToThePar $this->mockActionRequest->expects(self::atLeastOnce())->method('isDispatched')->willReturn(false); $this->mockParentRequest->expects(self::atLeastOnce())->method('isDispatched')->willReturn(true); - $this->mockController->expects(self::atLeastOnce())->method('processRequest')->will(self::throwException(new StopActionException())); + $this->mockController->expects(self::atLeastOnce())->method('processRequest')->will(self::throwException(StopActionException::create(new ActionResponse(), '', 0))); $this->dispatcher->dispatch($this->mockActionRequest, $this->actionResponse); } @@ -181,14 +181,13 @@ public function dispatchContinuesWithNextRequestFoundInAForwardException() /** @var ActionRequest|MockObject $nextRequest */ $nextRequest = $this->getMockBuilder(ActionRequest::class)->disableOriginalConstructor()->getMock(); $nextRequest->expects(self::atLeastOnce())->method('isDispatched')->willReturn(true); - $forwardException = new ForwardException(); - $forwardException->setNextRequest($nextRequest); + $forwardException = ForwardException::create($nextRequest); $this->mockParentRequest->expects(self::atLeastOnce())->method('isDispatched')->willReturn(false); $this->mockController->expects(self::exactly(2))->method('processRequest') ->withConsecutive([$this->mockActionRequest], [$this->mockParentRequest]) - ->willReturnOnConsecutiveCalls(self::throwException(new StopActionException()), self::throwException($forwardException)); + ->willReturnOnConsecutiveCalls(self::throwException(StopActionException::create(new ActionResponse(), '', 0)), self::throwException($forwardException)); $this->dispatcher->dispatch($this->mockActionRequest); } diff --git a/Neos.FluidAdaptor/Classes/Core/Widget/AbstractWidgetController.php b/Neos.FluidAdaptor/Classes/Core/Widget/AbstractWidgetController.php index a52e85712b..ac4e300466 100644 --- a/Neos.FluidAdaptor/Classes/Core/Widget/AbstractWidgetController.php +++ b/Neos.FluidAdaptor/Classes/Core/Widget/AbstractWidgetController.php @@ -14,6 +14,7 @@ use Neos\Flow\Mvc\ActionRequest; use Neos\Flow\Mvc\ActionResponse; use Neos\Flow\Mvc\Controller\ActionController; +use Neos\Flow\Mvc\Exception\ForwardException; use Neos\Flow\Mvc\Exception\InvalidActionVisibilityException; use Neos\Flow\Mvc\Exception\InvalidArgumentTypeException; use Neos\Flow\Mvc\Exception\NoSuchActionException; @@ -52,6 +53,7 @@ abstract class AbstractWidgetController extends ActionController * @throws NoSuchActionException * @throws NoSuchArgumentException * @throws StopActionException + * @throws ForwardException * @throws ViewNotFoundException * @throws Exception * @throws \Neos\Flow\Security\Exception diff --git a/Neos.FluidAdaptor/Classes/Core/Widget/AbstractWidgetViewHelper.php b/Neos.FluidAdaptor/Classes/Core/Widget/AbstractWidgetViewHelper.php index cbd77f5855..3479a57fbf 100644 --- a/Neos.FluidAdaptor/Classes/Core/Widget/AbstractWidgetViewHelper.php +++ b/Neos.FluidAdaptor/Classes/Core/Widget/AbstractWidgetViewHelper.php @@ -244,13 +244,12 @@ protected function initiateSubRequest() $content = $subResponse->getContent(); $subResponse->setContent(''); } catch (StopActionException $exception) { - $subResponse = $exception->response ?? $subResponse; - if ($exception instanceof ForwardException) { - $subRequest = $exception->getNextRequest(); - continue; - } + $subResponse = $exception->response; $subResponse->mergeIntoParentResponse($this->controllerContext->getResponse()); throw $exception; + } catch (ForwardException $exception) { + $subRequest = $exception->nextRequest; + continue; } $subResponse->mergeIntoParentResponse($this->controllerContext->getResponse()); } From d507942e9bf226753e2695d63b7a92f0747b17c3 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Fri, 26 Jan 2024 08:59:40 +0100 Subject: [PATCH 14/21] !!! TASK: Adjust Signals Dispatcher `beforeControllerInvocation` and `afterControllerInvocation` The parameter `$response` was removed from the `beforeControllerInvocation` signal, as it would be just always empty at that point and misleading and any mutations would be ignored. The parameter `$response` was made nullable for the `afterControllerInvocation` signal, as it's not set after a `forwardToRequest`. Also, it's to be noted, that modifying the response in an `afterControllerInvocation` is not always going to take effect, for example if the dispatcher is still in the loop. --- Neos.Flow/Classes/Mvc/ActionRequest.php | 1 + Neos.Flow/Classes/Mvc/Dispatcher.php | 32 ++++++++++++------------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/Neos.Flow/Classes/Mvc/ActionRequest.php b/Neos.Flow/Classes/Mvc/ActionRequest.php index 78d14e9fc0..6d1e044a07 100644 --- a/Neos.Flow/Classes/Mvc/ActionRequest.php +++ b/Neos.Flow/Classes/Mvc/ActionRequest.php @@ -212,6 +212,7 @@ public function getMainRequest(): ActionRequest * Checks if this request is the uppermost ActionRequest, just one below the * HTTP request. * + * @phpstan-assert-if-true null $this->getParentRequest() * @return boolean * @api */ diff --git a/Neos.Flow/Classes/Mvc/Dispatcher.php b/Neos.Flow/Classes/Mvc/Dispatcher.php index 6b225a88bf..e952c7b0a3 100644 --- a/Neos.Flow/Classes/Mvc/Dispatcher.php +++ b/Neos.Flow/Classes/Mvc/Dispatcher.php @@ -123,41 +123,39 @@ public function dispatch(ActionRequest $request): ActionResponse protected function initiateDispatchLoop(ActionRequest $request): ActionResponse { $dispatchLoopCount = 0; - // This response is just for legacy reasons so the signals have something. - $response = new ActionResponse(); - while ($request !== null && $request->isDispatched() === false) { + while ($request->isDispatched() === false) { if ($dispatchLoopCount++ > 99) { throw new Exception\InfiniteLoopException(sprintf('Could not ultimately dispatch the request after %d iterations.', $dispatchLoopCount), 1217839467); } $controller = $this->resolveController($request); + $this->emitBeforeControllerInvocation($request, $controller); try { - $this->emitBeforeControllerInvocation($request, $response, $controller); $response = $controller->processRequest($request); $this->emitAfterControllerInvocation($request, $response, $controller); - } catch (StopActionException $exception) { - $response = $exception->response; - $this->emitAfterControllerInvocation($request, $response, $controller); + } catch (StopActionException $stopActionException) { + $this->emitAfterControllerInvocation($request, $stopActionException->response, $controller); + $response = $stopActionException->response; if (!$request->isMainRequest()) { $request = $request->getParentRequest(); } - } catch (ForwardException $exception) { - $this->emitAfterControllerInvocation($request, $response, $controller); - $request = $exception->nextRequest; + } catch (ForwardException $forwardException) { + $this->emitAfterControllerInvocation($request, null, $controller); + $request = $forwardException->nextRequest; } } - return $response; + // 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(); } /** - * This signal is emitted directly before the request is been dispatched to a controller. + * This signal is emitted directly before the request is being dispatched to a controller. * * @param ActionRequest $request - * @param ActionResponse $response This will always be an empty response, nothing to see here. * @param ControllerInterface $controller * @return void * @Flow\Signal */ - protected function emitBeforeControllerInvocation(ActionRequest $request, ActionResponse $response, ControllerInterface $controller) + protected function emitBeforeControllerInvocation(ActionRequest $request, ControllerInterface $controller) { } @@ -166,12 +164,14 @@ protected function emitBeforeControllerInvocation(ActionRequest $request, Action * returned control back to the dispatcher. * * @param ActionRequest $request - * @param ActionResponse $response + * @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 ControllerInterface $controller * @return void * @Flow\Signal */ - protected function emitAfterControllerInvocation(ActionRequest $request, ActionResponse $response, ControllerInterface $controller) + protected function emitAfterControllerInvocation(ActionRequest $request, ?ActionResponse $response, ControllerInterface $controller) { } From c1ce1bc064246d85a3f9ab410c1ffad1e31ad3d5 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Fri, 26 Jan 2024 11:50:26 +0100 Subject: [PATCH 15/21] TASK: Set fix $code of `StopActionException` --- Neos.Flow/Classes/Mvc/Controller/AbstractController.php | 2 +- Neos.Flow/Classes/Mvc/Controller/RestController.php | 2 +- .../Classes/Mvc/Controller/SpecialResponsesSupport.php | 9 ++++----- Neos.Flow/Classes/Mvc/Exception/StopActionException.php | 5 ++--- Neos.Flow/Tests/Unit/Mvc/DispatcherTest.php | 6 +++--- 5 files changed, 11 insertions(+), 13 deletions(-) diff --git a/Neos.Flow/Classes/Mvc/Controller/AbstractController.php b/Neos.Flow/Classes/Mvc/Controller/AbstractController.php index a0fafc3e6c..e4452524ac 100644 --- a/Neos.Flow/Classes/Mvc/Controller/AbstractController.php +++ b/Neos.Flow/Classes/Mvc/Controller/AbstractController.php @@ -313,7 +313,7 @@ protected function redirectToUri(string|UriInterface $uri, int $delay = 0, int $ } $response = $this->responseRedirectsToUri($uri, $delay, $statusCode, $this->response); - $this->throwStopActionWithResponse($response, '', 1699716808); + $this->throwStopActionWithResponse($response, ''); } /** diff --git a/Neos.Flow/Classes/Mvc/Controller/RestController.php b/Neos.Flow/Classes/Mvc/Controller/RestController.php index 174f9c4b5c..a6eb0c1a42 100644 --- a/Neos.Flow/Classes/Mvc/Controller/RestController.php +++ b/Neos.Flow/Classes/Mvc/Controller/RestController.php @@ -138,6 +138,6 @@ protected function redirectToUri(string|UriInterface $uri, int $delay = 0, int $ $response->setContent(''); } - $this->throwStopActionWithResponse($response, '', 1699716808); + $this->throwStopActionWithResponse($response, ''); } } diff --git a/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php b/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php index 1b5b702450..c80229d94a 100644 --- a/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php +++ b/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php @@ -30,7 +30,7 @@ protected function responseThrowsStatus(int $statusCode, string $content = '', ? $response->setContent($content); } - $this->throwStopActionWithResponse($response, $content, 1558088618); + $this->throwStopActionWithResponse($response, $content); } /** @@ -49,7 +49,7 @@ protected function responseRedirectsToUri(UriInterface $uri, int $delay = 0, int if ($delay < 1) { $nextResponse->setRedirectUri($uri, $statusCode); - $this->throwStopActionWithResponse($nextResponse, '', 1699478812); + $this->throwStopActionWithResponse($nextResponse, ''); } $nextResponse->setStatusCode($statusCode); @@ -61,13 +61,12 @@ protected function responseRedirectsToUri(UriInterface $uri, int $delay = 0, int /** * @param ActionResponse $response * @param string $message - * @param int $code * @return never * @throws StopActionException */ - protected function throwStopActionWithResponse(ActionResponse $response, string $message = '', int $code = 0): never + protected function throwStopActionWithResponse(ActionResponse $response, string $message = ''): never { - throw StopActionException::create($response, $message, $code); + throw StopActionException::create($response, $message); } /** diff --git a/Neos.Flow/Classes/Mvc/Exception/StopActionException.php b/Neos.Flow/Classes/Mvc/Exception/StopActionException.php index 06379f9c2a..8bf4a45e48 100644 --- a/Neos.Flow/Classes/Mvc/Exception/StopActionException.php +++ b/Neos.Flow/Classes/Mvc/Exception/StopActionException.php @@ -38,9 +38,8 @@ private function __construct(string $message, int $code, ?\Throwable $previous, public static function create( ActionResponse $response, - string $message, - int $code + string $message ) { - return new self($message, $code, null, $response); + return new self($message, 1558088618, null, $response); } } diff --git a/Neos.Flow/Tests/Unit/Mvc/DispatcherTest.php b/Neos.Flow/Tests/Unit/Mvc/DispatcherTest.php index ee2cfd24f1..33c21c165a 100644 --- a/Neos.Flow/Tests/Unit/Mvc/DispatcherTest.php +++ b/Neos.Flow/Tests/Unit/Mvc/DispatcherTest.php @@ -155,7 +155,7 @@ public function dispatchIgnoresStopExceptionsForFirstLevelActionRequests() $this->mockParentRequest->expects(self::exactly(2))->method('isDispatched')->willReturnOnConsecutiveCalls(false, true); $this->mockParentRequest->expects(self::once())->method('isMainRequest')->willReturn(true); - $this->mockController->expects(self::atLeastOnce())->method('processRequest')->will(self::throwException(StopActionException::create(new ActionResponse(), '', 0))); + $this->mockController->expects(self::atLeastOnce())->method('processRequest')->will(self::throwException(StopActionException::create(new ActionResponse(), ''))); $this->dispatcher->dispatch($this->mockParentRequest); } @@ -168,7 +168,7 @@ public function dispatchCatchesStopExceptionOfActionRequestsAndRollsBackToThePar $this->mockActionRequest->expects(self::atLeastOnce())->method('isDispatched')->willReturn(false); $this->mockParentRequest->expects(self::atLeastOnce())->method('isDispatched')->willReturn(true); - $this->mockController->expects(self::atLeastOnce())->method('processRequest')->will(self::throwException(StopActionException::create(new ActionResponse(), '', 0))); + $this->mockController->expects(self::atLeastOnce())->method('processRequest')->will(self::throwException(StopActionException::create(new ActionResponse(), ''))); $this->dispatcher->dispatch($this->mockActionRequest, $this->actionResponse); } @@ -187,7 +187,7 @@ public function dispatchContinuesWithNextRequestFoundInAForwardException() $this->mockController->expects(self::exactly(2))->method('processRequest') ->withConsecutive([$this->mockActionRequest], [$this->mockParentRequest]) - ->willReturnOnConsecutiveCalls(self::throwException(StopActionException::create(new ActionResponse(), '', 0)), self::throwException($forwardException)); + ->willReturnOnConsecutiveCalls(self::throwException(StopActionException::create(new ActionResponse(), '')), self::throwException($forwardException)); $this->dispatcher->dispatch($this->mockActionRequest); } From 18da350f5628c75a2ecea881edf7ededd14d3712 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Fri, 26 Jan 2024 13:37:59 +0100 Subject: [PATCH 16/21] TASK: Adjust docs of MVC Control flow exceptions Explain that the details/exception message dont matter, and set a default. --- .../Controller/SpecialResponsesSupport.php | 13 +++++----- .../Mvc/Exception/ForwardException.php | 23 +++++++++++++---- .../Mvc/Exception/StopActionException.php | 25 +++++++++++++------ Neos.Flow/Tests/Unit/Mvc/DispatcherTest.php | 2 +- 4 files changed, 43 insertions(+), 20 deletions(-) diff --git a/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php b/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php index c80229d94a..58456f9ef7 100644 --- a/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php +++ b/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php @@ -21,7 +21,7 @@ trait SpecialResponsesSupport * @return never * @throws StopActionException */ - protected function responseThrowsStatus(int $statusCode, string $content = '', ?ActionResponse $response = null): never + protected function responseThrowsStatus(int $statusCode, string $content = '', ?ActionResponse $response = null): never { $response = $response ?? new ActionResponse; @@ -59,14 +59,14 @@ protected function responseRedirectsToUri(UriInterface $uri, int $delay = 0, int } /** - * @param ActionResponse $response - * @param string $message + * @param ActionResponse $response The response to be received by the MVC Dispatcher. + * @param string $details Additional details just for the exception, in case it is logged (the regular exception message). * @return never * @throws StopActionException */ - protected function throwStopActionWithResponse(ActionResponse $response, string $message = ''): never + protected function throwStopActionWithResponse(ActionResponse $response, string $details = ''): never { - throw StopActionException::create($response, $message); + throw StopActionException::create($response, $details); } /** @@ -83,7 +83,6 @@ protected function throwStopActionWithResponse(ActionResponse $response, string protected function forwardToRequest(ActionRequest $request): never { $nextRequest = clone $request; - $forwardException = ForwardException::create($nextRequest); - throw $forwardException; + throw ForwardException::create($nextRequest, ''); } } diff --git a/Neos.Flow/Classes/Mvc/Exception/ForwardException.php b/Neos.Flow/Classes/Mvc/Exception/ForwardException.php index 095cd2b0c5..569054d7bc 100644 --- a/Neos.Flow/Classes/Mvc/Exception/ForwardException.php +++ b/Neos.Flow/Classes/Mvc/Exception/ForwardException.php @@ -14,15 +14,18 @@ /** * This exception is thrown by a controller to stop the execution of the current - * action and return the control to the dispatcher for the special case of a - * forward(). + * action and return the control to the dispatcher for the special case of a forward. + * + * See {@see SpecialResponsesSupport::forwardToRequest()} for more information. + * + * Other control flow exceptions: {@see StopActionException} * * @api */ final class ForwardException extends \Neos\Flow\Mvc\Exception { /** - * @var ActionRequest The next request, containing the information about the next action to execute. + * The next request the MVC Dispatcher should handle. */ public readonly ActionRequest $nextRequest; @@ -32,8 +35,18 @@ private function __construct(string $message, int $code, ?\Throwable $previous, $this->nextRequest = $nextRequest; } - public static function create(ActionRequest $nextRequest) + /** + * @param ActionRequest $nextRequest The next request the MVC Dispatcher should handle. + * @param string $details Additional details just for this exception, in case it is logged (the regular exception message). + */ + public static function create(ActionRequest $nextRequest, string $details): self { - return new self('', 0, null, $nextRequest); + if (empty($details)) { + $details = sprintf( + 'Forward action to %s controller.', + $nextRequest->getControllerObjectName() + ); + } + return new self($details, 1706272103, null, $nextRequest); } } diff --git a/Neos.Flow/Classes/Mvc/Exception/StopActionException.php b/Neos.Flow/Classes/Mvc/Exception/StopActionException.php index 8bf4a45e48..a9e0de703b 100644 --- a/Neos.Flow/Classes/Mvc/Exception/StopActionException.php +++ b/Neos.Flow/Classes/Mvc/Exception/StopActionException.php @@ -19,14 +19,17 @@ * exception and - depending on the "dispatched" status of the request - either * continues dispatching the request or returns control to the request handler. * - * See the Action Controller's forward() and redirectToUri() methods for more information. + * See {@see SpecialResponsesSupport::throwStopActionWithResponse()} + * or {@see SpecialResponsesSupport::responseRedirectsToUri()} for more information. + * + * Other control flow exceptions: {@see ForwardException} * * @api */ final class StopActionException extends \Neos\Flow\Mvc\Exception { /** - * As throwing the exception allows for an unusual control flow, we attach the response for the dispatcher. + * The response to be received by the MVC Dispatcher. */ public readonly ActionResponse $response; @@ -36,10 +39,18 @@ private function __construct(string $message, int $code, ?\Throwable $previous, $this->response = $response; } - public static function create( - ActionResponse $response, - string $message - ) { - return new self($message, 1558088618, null, $response); + /** + * @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). + */ + public static function create(ActionResponse $response, string $details): self + { + if (empty($details)) { + $details = sprintf( + 'Stop action with %s response.', + $response->getStatusCode() + ); + } + return new self($details, 1558088618, null, $response); } } diff --git a/Neos.Flow/Tests/Unit/Mvc/DispatcherTest.php b/Neos.Flow/Tests/Unit/Mvc/DispatcherTest.php index 33c21c165a..b988f163ed 100644 --- a/Neos.Flow/Tests/Unit/Mvc/DispatcherTest.php +++ b/Neos.Flow/Tests/Unit/Mvc/DispatcherTest.php @@ -181,7 +181,7 @@ public function dispatchContinuesWithNextRequestFoundInAForwardException() /** @var ActionRequest|MockObject $nextRequest */ $nextRequest = $this->getMockBuilder(ActionRequest::class)->disableOriginalConstructor()->getMock(); $nextRequest->expects(self::atLeastOnce())->method('isDispatched')->willReturn(true); - $forwardException = ForwardException::create($nextRequest); + $forwardException = ForwardException::create($nextRequest, ''); $this->mockParentRequest->expects(self::atLeastOnce())->method('isDispatched')->willReturn(false); From 73d721498817fb4b3c1b97708beeb0d24a2fb65e Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Fri, 26 Jan 2024 13:41:37 +0100 Subject: [PATCH 17/21] TASK: Rename factory methods of MVC Control flow exceptions --- .../Classes/Mvc/Controller/SpecialResponsesSupport.php | 4 ++-- Neos.Flow/Classes/Mvc/Exception/ForwardException.php | 2 +- Neos.Flow/Classes/Mvc/Exception/StopActionException.php | 2 +- Neos.Flow/Tests/Unit/Mvc/DispatcherTest.php | 8 ++++---- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php b/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php index 58456f9ef7..17deeca3e4 100644 --- a/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php +++ b/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php @@ -66,7 +66,7 @@ protected function responseRedirectsToUri(UriInterface $uri, int $delay = 0, int */ protected function throwStopActionWithResponse(ActionResponse $response, string $details = ''): never { - throw StopActionException::create($response, $details); + throw StopActionException::createForResponse($response, $details); } /** @@ -83,6 +83,6 @@ protected function throwStopActionWithResponse(ActionResponse $response, string protected function forwardToRequest(ActionRequest $request): never { $nextRequest = clone $request; - throw ForwardException::create($nextRequest, ''); + throw ForwardException::createForNextRequest($nextRequest, ''); } } diff --git a/Neos.Flow/Classes/Mvc/Exception/ForwardException.php b/Neos.Flow/Classes/Mvc/Exception/ForwardException.php index 569054d7bc..075f3b8cdf 100644 --- a/Neos.Flow/Classes/Mvc/Exception/ForwardException.php +++ b/Neos.Flow/Classes/Mvc/Exception/ForwardException.php @@ -39,7 +39,7 @@ private function __construct(string $message, int $code, ?\Throwable $previous, * @param ActionRequest $nextRequest The next request the MVC Dispatcher should handle. * @param string $details Additional details just for this exception, in case it is logged (the regular exception message). */ - public static function create(ActionRequest $nextRequest, string $details): self + public static function createForNextRequest(ActionRequest $nextRequest, string $details): self { if (empty($details)) { $details = sprintf( diff --git a/Neos.Flow/Classes/Mvc/Exception/StopActionException.php b/Neos.Flow/Classes/Mvc/Exception/StopActionException.php index a9e0de703b..18c4f7cbc4 100644 --- a/Neos.Flow/Classes/Mvc/Exception/StopActionException.php +++ b/Neos.Flow/Classes/Mvc/Exception/StopActionException.php @@ -43,7 +43,7 @@ private function __construct(string $message, int $code, ?\Throwable $previous, * @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). */ - public static function create(ActionResponse $response, string $details): self + public static function createForResponse(ActionResponse $response, string $details): self { if (empty($details)) { $details = sprintf( diff --git a/Neos.Flow/Tests/Unit/Mvc/DispatcherTest.php b/Neos.Flow/Tests/Unit/Mvc/DispatcherTest.php index b988f163ed..4e942bf4ac 100644 --- a/Neos.Flow/Tests/Unit/Mvc/DispatcherTest.php +++ b/Neos.Flow/Tests/Unit/Mvc/DispatcherTest.php @@ -155,7 +155,7 @@ public function dispatchIgnoresStopExceptionsForFirstLevelActionRequests() $this->mockParentRequest->expects(self::exactly(2))->method('isDispatched')->willReturnOnConsecutiveCalls(false, true); $this->mockParentRequest->expects(self::once())->method('isMainRequest')->willReturn(true); - $this->mockController->expects(self::atLeastOnce())->method('processRequest')->will(self::throwException(StopActionException::create(new ActionResponse(), ''))); + $this->mockController->expects(self::atLeastOnce())->method('processRequest')->will(self::throwException(StopActionException::createForResponse(new ActionResponse(), ''))); $this->dispatcher->dispatch($this->mockParentRequest); } @@ -168,7 +168,7 @@ public function dispatchCatchesStopExceptionOfActionRequestsAndRollsBackToThePar $this->mockActionRequest->expects(self::atLeastOnce())->method('isDispatched')->willReturn(false); $this->mockParentRequest->expects(self::atLeastOnce())->method('isDispatched')->willReturn(true); - $this->mockController->expects(self::atLeastOnce())->method('processRequest')->will(self::throwException(StopActionException::create(new ActionResponse(), ''))); + $this->mockController->expects(self::atLeastOnce())->method('processRequest')->will(self::throwException(StopActionException::createForResponse(new ActionResponse(), ''))); $this->dispatcher->dispatch($this->mockActionRequest, $this->actionResponse); } @@ -181,13 +181,13 @@ public function dispatchContinuesWithNextRequestFoundInAForwardException() /** @var ActionRequest|MockObject $nextRequest */ $nextRequest = $this->getMockBuilder(ActionRequest::class)->disableOriginalConstructor()->getMock(); $nextRequest->expects(self::atLeastOnce())->method('isDispatched')->willReturn(true); - $forwardException = ForwardException::create($nextRequest, ''); + $forwardException = ForwardException::createForNextRequest($nextRequest, ''); $this->mockParentRequest->expects(self::atLeastOnce())->method('isDispatched')->willReturn(false); $this->mockController->expects(self::exactly(2))->method('processRequest') ->withConsecutive([$this->mockActionRequest], [$this->mockParentRequest]) - ->willReturnOnConsecutiveCalls(self::throwException(StopActionException::create(new ActionResponse(), '')), self::throwException($forwardException)); + ->willReturnOnConsecutiveCalls(self::throwException(StopActionException::createForResponse(new ActionResponse(), '')), self::throwException($forwardException)); $this->dispatcher->dispatch($this->mockActionRequest); } From 554aa081f60e27c49bcc134935c4288473e5eb5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Mu=CC=88ller?= Date: Sat, 27 Jan 2024 19:43:58 +0100 Subject: [PATCH 18/21] SpecialResponses become final --- .../Classes/Mvc/Controller/SpecialResponsesSupport.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php b/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php index 17deeca3e4..733500c78c 100644 --- a/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php +++ b/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php @@ -21,7 +21,7 @@ trait SpecialResponsesSupport * @return never * @throws StopActionException */ - protected function responseThrowsStatus(int $statusCode, string $content = '', ?ActionResponse $response = null): never + final protected function responseThrowsStatus(int $statusCode, string $content = '', ?ActionResponse $response = null): never { $response = $response ?? new ActionResponse; @@ -43,7 +43,7 @@ protected function responseThrowsStatus(int $statusCode, string $content = '', ? * @return ActionResponse * @throws StopActionException */ - protected function responseRedirectsToUri(UriInterface $uri, int $delay = 0, int $statusCode = 303, ?ActionResponse $response = null): ActionResponse + final protected function responseRedirectsToUri(UriInterface $uri, int $delay = 0, int $statusCode = 303, ?ActionResponse $response = null): ActionResponse { $nextResponse = $response !== null ? clone $response : new ActionResponse(); @@ -53,7 +53,7 @@ protected function responseRedirectsToUri(UriInterface $uri, int $delay = 0, int } $nextResponse->setStatusCode($statusCode); - $content = sprintf('', $delay, $uri); + $content = sprintf('Redirect to %s', $delay, $uri, $uri); $nextResponse->setContent($content); return $nextResponse; } @@ -64,7 +64,7 @@ protected function responseRedirectsToUri(UriInterface $uri, int $delay = 0, int * @return never * @throws StopActionException */ - protected function throwStopActionWithResponse(ActionResponse $response, string $details = ''): never + final protected function throwStopActionWithResponse(ActionResponse $response, string $details = ''): never { throw StopActionException::createForResponse($response, $details); } @@ -80,7 +80,7 @@ protected function throwStopActionWithResponse(ActionResponse $response, string * @return never * @throws ForwardException */ - protected function forwardToRequest(ActionRequest $request): never + final protected function forwardToRequest(ActionRequest $request): never { $nextRequest = clone $request; throw ForwardException::createForNextRequest($nextRequest, ''); From 82caa27e6808256f47bbbed1c705e7a14ab89269 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Sat, 3 Feb 2024 13:35:32 +0100 Subject: [PATCH 19/21] TASK: Revert introduction of `SimpleActionController` Will be split up into an own pr https://github.com/neos/flow-development-collection/pull/3297 --- .../Mvc/Controller/SimpleActionController.php | 49 ----------- .../SimpleActionControllerTestController.php | 48 ---------- .../Mvc/SimpleActionControllerTest.php | 88 ------------------- .../Fixtures/SimpleActionTestController.php | 19 ---- .../Controller/SimpleActionControllerTest.php | 74 ---------------- 5 files changed, 278 deletions(-) delete mode 100644 Neos.Flow/Classes/Mvc/Controller/SimpleActionController.php delete mode 100644 Neos.Flow/Tests/Functional/Mvc/Fixtures/Controller/SimpleActionControllerTestController.php delete mode 100644 Neos.Flow/Tests/Functional/Mvc/SimpleActionControllerTest.php delete mode 100644 Neos.Flow/Tests/Unit/Mvc/Controller/Fixtures/SimpleActionTestController.php delete mode 100644 Neos.Flow/Tests/Unit/Mvc/Controller/SimpleActionControllerTest.php diff --git a/Neos.Flow/Classes/Mvc/Controller/SimpleActionController.php b/Neos.Flow/Classes/Mvc/Controller/SimpleActionController.php deleted file mode 100644 index 35b2e3e596..0000000000 --- a/Neos.Flow/Classes/Mvc/Controller/SimpleActionController.php +++ /dev/null @@ -1,49 +0,0 @@ -setDispatched(true); - $actionMethodName = $this->resolveActionMethodName($request); - return $this->$actionMethodName($request); - } - - /** - * Resolves and checks the current action method name - * - * @return string Method name of the current action - * @throws NoSuchActionException - */ - protected function resolveActionMethodName(ActionRequest $request): string - { - $actionMethodName = $request->getControllerActionName() . 'Action'; - if (!is_callable([$this, $actionMethodName])) { - throw new NoSuchActionException(sprintf('An action "%s" does not exist in controller "%s".', $actionMethodName, get_class($this)), 1186669086); - } - - return $actionMethodName; - } -} diff --git a/Neos.Flow/Tests/Functional/Mvc/Fixtures/Controller/SimpleActionControllerTestController.php b/Neos.Flow/Tests/Functional/Mvc/Fixtures/Controller/SimpleActionControllerTestController.php deleted file mode 100644 index 990b6c2256..0000000000 --- a/Neos.Flow/Tests/Functional/Mvc/Fixtures/Controller/SimpleActionControllerTestController.php +++ /dev/null @@ -1,48 +0,0 @@ -setContent('index'); - - return $response; - } - - public function simpleReponseAction(ActionRequest $actionRequest): ActionResponse - { - $response = new ActionResponse(); - $response->setContent('Simple'); - - return $response; - } - - public function jsonResponseAction(ActionRequest $actionRequest): ActionResponse - { - $response = new ActionResponse(); - $response->setContent(json_encode(['foo' => 'bar', 'baz' => 123])); - $response->setContentType('application/json'); - - return $response; - } - - public function argumentsAction(ActionRequest $actionRequest): ActionResponse - { - $response = new ActionResponse(); - $response->setContent(strtolower($actionRequest->getArgument('testArgument'))); - if ($actionRequest->getFormat() === 'html') { - $response->setContentType('text/html'); - } - - return $response; - } -} diff --git a/Neos.Flow/Tests/Functional/Mvc/SimpleActionControllerTest.php b/Neos.Flow/Tests/Functional/Mvc/SimpleActionControllerTest.php deleted file mode 100644 index 32e44cf2cc..0000000000 --- a/Neos.Flow/Tests/Functional/Mvc/SimpleActionControllerTest.php +++ /dev/null @@ -1,88 +0,0 @@ -registerRoute('test', 'test/mvc/simplecontrollertest(/{@action})', [ - '@package' => 'Neos.Flow', - '@subpackage' => 'Tests\Functional\Mvc\Fixtures', - '@controller' => 'SimpleActionControllerTest', - '@action' => 'index', - '@format' => 'html' - ]); - - $this->serverRequestFactory = $this->objectManager->get(ServerRequestFactoryInterface::class); - } - - /** - * Checks if a simple request is handled correctly. The route matching the - * specified URI defines a default action "index" which results in indexAction - * being called. - * - * @test - */ - public function defaultActionSpecifiedInRouteIsCalledAndResponseIsReturned() - { - $response = $this->browser->request('http://localhost/test/mvc/simplecontrollertest'); - self::assertEquals(200, $response->getStatusCode()); - self::assertEquals('index', $response->getBody()->getContents()); - } - - /** - * Checks if a simple request is handled correctly if another than the default - * action is specified. - * - * @test - */ - public function actionSpecifiedInActionRequestIsCalledAndResponseIsReturned() - { - $response = $this->browser->request('http://localhost/test/mvc/simplecontrollertest/simplereponse'); - self::assertEquals(200, $response->getStatusCode()); - self::assertEquals('Simple', $response->getBody()->getContents()); - } - - /** - * Checks if the content type and json content is correctly passed through - * - * @test - */ - public function responseContainsContentTypeAndContent() - { - $response = $this->browser->request('http://localhost/test/mvc/simplecontrollertest/jsonresponse'); - self::assertEquals(200, $response->getStatusCode()); - self::assertEquals(['application/json'], $response->getHeader('Content-Type')); - self::assertEquals(json_encode(['foo' => 'bar', 'baz' => 123]), $response->getBody()->getContents()); - } - - /** - * Checks if arguments and format are passed - * - * @test - */ - public function responseContainsArgumentContent() - { - $argument = '

Some markup

'; - $response = $this->browser->request('http://localhost/test/mvc/simplecontrollertest/arguments?testArgument=' . urlencode($argument)); - self::assertEquals(200, $response->getStatusCode()); - self::assertEquals(['text/html'], $response->getHeader('Content-Type')); - self::assertEquals(strtolower($argument), $response->getBody()->getContents()); - } -} diff --git a/Neos.Flow/Tests/Unit/Mvc/Controller/Fixtures/SimpleActionTestController.php b/Neos.Flow/Tests/Unit/Mvc/Controller/Fixtures/SimpleActionTestController.php deleted file mode 100644 index 4670322eb7..0000000000 --- a/Neos.Flow/Tests/Unit/Mvc/Controller/Fixtures/SimpleActionTestController.php +++ /dev/null @@ -1,19 +0,0 @@ -setContent('Simple'); - return $response; - } -} diff --git a/Neos.Flow/Tests/Unit/Mvc/Controller/SimpleActionControllerTest.php b/Neos.Flow/Tests/Unit/Mvc/Controller/SimpleActionControllerTest.php deleted file mode 100644 index ba7461a455..0000000000 --- a/Neos.Flow/Tests/Unit/Mvc/Controller/SimpleActionControllerTest.php +++ /dev/null @@ -1,74 +0,0 @@ -createMock(ActionRequest::class); - $request->method('getControllerActionName')->willReturn($actionName); - - $this->expectException(NoSuchActionException::class); - - $testObject = new Fixtures\SimpleActionTestController(); - $testObject->processRequest($request); - } - - /** - * @return void - * @throws NoSuchActionException - * @throws \Neos\Flow\SignalSlot\Exception\InvalidSlotException - * @test - */ - public function responseOnlyContainsWhatActionSets() - { - $request = $this->createMock(ActionRequest::class); - $request->method('getControllerActionName')->willReturn('addTestContent'); - - $testObject = new Fixtures\SimpleActionTestController(); - $response = $testObject->processRequest($request); - self::assertInstanceOf(ActionResponse::class, $response); - self::assertEquals('Simple', $response->getContent()); - self::assertFalse($response->hasContentType()); - // default - self::assertEquals(200, $response->getStatusCode()); - } -} From c96fd84431318e55297045beac984c73a01d94a1 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Sat, 3 Feb 2024 13:53:37 +0100 Subject: [PATCH 20/21] TASK: Revert introduction of `SpecialResponsesSupport` Will be split up into an own pr https://github.com/neos/flow-development-collection/pull/3298 --- .../Mvc/Controller/AbstractController.php | 41 ++++++--- .../Classes/Mvc/Controller/RestController.php | 19 ++-- .../Controller/SpecialResponsesSupport.php | 88 ------------------- .../Mvc/Exception/ForwardException.php | 3 +- .../Mvc/Exception/StopActionException.php | 4 +- 5 files changed, 43 insertions(+), 112 deletions(-) delete mode 100644 Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php diff --git a/Neos.Flow/Classes/Mvc/Controller/AbstractController.php b/Neos.Flow/Classes/Mvc/Controller/AbstractController.php index e4452524ac..69268ff928 100644 --- a/Neos.Flow/Classes/Mvc/Controller/AbstractController.php +++ b/Neos.Flow/Classes/Mvc/Controller/AbstractController.php @@ -42,8 +42,6 @@ */ abstract class AbstractController implements ControllerInterface { - use SpecialResponsesSupport; - /** * @var UriBuilder */ @@ -232,6 +230,23 @@ protected function forward(string $actionName, string $controllerName = null, st $this->forwardToRequest($nextRequest); } + /** + * Forwards the request to another action and / or controller. + * + * Request is directly transfered to the other action / controller + * + * @param ActionRequest $request The request to redirect to + * @return void + * @throws ForwardException + * @see redirectToRequest() + * @api + */ + protected function forwardToRequest(ActionRequest $request) + { + $nextRequest = clone $request; + throw ForwardException::createForNextRequest($nextRequest, ''); + } + /** * Redirects the request to another action and / or controller. * @@ -308,12 +323,16 @@ protected function redirectToRequest(ActionRequest $request, int $delay = 0, int */ protected function redirectToUri(string|UriInterface $uri, int $delay = 0, int $statusCode = 303): never { - if (!$uri instanceof UriInterface) { - $uri = new Uri($uri); + if ($delay === 0) { + if (!$uri instanceof UriInterface) { + $uri = new Uri($uri); + } + $this->response->setRedirectUri($uri, $statusCode); + } else { + $this->response->setStatusCode($statusCode); + $this->response->setContent(''); } - - $response = $this->responseRedirectsToUri($uri, $delay, $statusCode, $this->response); - $this->throwStopActionWithResponse($response, ''); + throw StopActionException::createForResponse($this->response, ''); } /** @@ -325,11 +344,11 @@ protected function redirectToUri(string|UriInterface $uri, int $delay = 0, int $ * @param string $statusMessage A custom HTTP status message * @param string $content Body content which further explains the status * @throws StopActionException - * @deprecated Use SpecialResponsesSupport::responseThrowsStatus - * @see SpecialResponsesSupport::responseThrowsStatus + * @api */ protected function throwStatus(int $statusCode, $statusMessage = null, $content = null): never { + $this->response->setStatusCode($statusCode); if ($content === null) { $content = sprintf( '%s %s', @@ -337,8 +356,8 @@ protected function throwStatus(int $statusCode, $statusMessage = null, $content $statusMessage ?? ResponseInformationHelper::getStatusMessageByCode($statusCode) ); } - - $this->responseThrowsStatus($statusCode, $content, $this->response); + $this->response->setContent($content); + throw StopActionException::createForResponse($this->response, $content); } /** diff --git a/Neos.Flow/Classes/Mvc/Controller/RestController.php b/Neos.Flow/Classes/Mvc/Controller/RestController.php index a6eb0c1a42..79ec113edf 100644 --- a/Neos.Flow/Classes/Mvc/Controller/RestController.php +++ b/Neos.Flow/Classes/Mvc/Controller/RestController.php @@ -128,16 +128,15 @@ protected function initializeUpdateAction() */ protected function redirectToUri(string|UriInterface $uri, int $delay = 0, int $statusCode = 303): never { - if (!$uri instanceof UriInterface) { - $uri = new Uri($uri); - } - - $response = $this->responseRedirectsToUri($uri, $delay, $statusCode, $this->response); - if ($this->request->getFormat() === 'json') { - // send empty body on redirects for JSON requests - $response->setContent(''); + // the parent method throws the exception, but we need to act afterwards + // thus the code in catch - it's the expected state + try { + parent::redirectToUri($uri, $delay, $statusCode); + } catch (StopActionException $exception) { + if ($this->request->getFormat() === 'json') { + $exception->response->setContent(''); + } + throw $exception; } - - $this->throwStopActionWithResponse($response, ''); } } diff --git a/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php b/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php deleted file mode 100644 index 733500c78c..0000000000 --- a/Neos.Flow/Classes/Mvc/Controller/SpecialResponsesSupport.php +++ /dev/null @@ -1,88 +0,0 @@ -setStatusCode($statusCode); - if ($content !== '') { - $response->setContent($content); - } - - $this->throwStopActionWithResponse($response, $content); - } - - /** - * Redirects to another URI - * - * @param UriInterface $uri Either a string representation of a URI or a UriInterface object - * @param integer $delay (optional) The delay in seconds. Default is no delay. - * @param integer $statusCode (optional) The HTTP status code for the redirect. Default is "303 See Other" - * @param ActionResponse|null $response response that will have status, and location or body overwritten. - * @return ActionResponse - * @throws StopActionException - */ - final protected function responseRedirectsToUri(UriInterface $uri, int $delay = 0, int $statusCode = 303, ?ActionResponse $response = null): ActionResponse - { - $nextResponse = $response !== null ? clone $response : new ActionResponse(); - - if ($delay < 1) { - $nextResponse->setRedirectUri($uri, $statusCode); - $this->throwStopActionWithResponse($nextResponse, ''); - } - - $nextResponse->setStatusCode($statusCode); - $content = sprintf('Redirect to %s', $delay, $uri, $uri); - $nextResponse->setContent($content); - return $nextResponse; - } - - /** - * @param ActionResponse $response The response to be received by the MVC Dispatcher. - * @param string $details Additional details just for the exception, in case it is logged (the regular exception message). - * @return never - * @throws StopActionException - */ - final protected function throwStopActionWithResponse(ActionResponse $response, string $details = ''): never - { - throw StopActionException::createForResponse($response, $details); - } - - /** - * Forwards the request to another action and / or controller - * Request is directly transferred to the other action / controller - * - * NOTE that this will not try to convert any objects in the requests arguments, - * this can be a fine or a problem depending on context of usage. - * - * @param ActionRequest $request The request to redirect to - * @return never - * @throws ForwardException - */ - final protected function forwardToRequest(ActionRequest $request): never - { - $nextRequest = clone $request; - throw ForwardException::createForNextRequest($nextRequest, ''); - } -} diff --git a/Neos.Flow/Classes/Mvc/Exception/ForwardException.php b/Neos.Flow/Classes/Mvc/Exception/ForwardException.php index 075f3b8cdf..2a1564c920 100644 --- a/Neos.Flow/Classes/Mvc/Exception/ForwardException.php +++ b/Neos.Flow/Classes/Mvc/Exception/ForwardException.php @@ -11,12 +11,13 @@ * source code. */ use Neos\Flow\Mvc\ActionRequest; +use Neos\Flow\Mvc\Controller\AbstractController; /** * This exception is thrown by a controller to stop the execution of the current * action and return the control to the dispatcher for the special case of a forward. * - * See {@see SpecialResponsesSupport::forwardToRequest()} for more information. + * See {@see AbstractController::forward()} for more information. * * Other control flow exceptions: {@see StopActionException} * diff --git a/Neos.Flow/Classes/Mvc/Exception/StopActionException.php b/Neos.Flow/Classes/Mvc/Exception/StopActionException.php index 18c4f7cbc4..50a4d6d9b9 100644 --- a/Neos.Flow/Classes/Mvc/Exception/StopActionException.php +++ b/Neos.Flow/Classes/Mvc/Exception/StopActionException.php @@ -12,6 +12,7 @@ */ use Neos\Flow\Mvc\ActionResponse; +use Neos\Flow\Mvc\Controller\AbstractController; /** * This exception is thrown by a controller to stop the execution of the current @@ -19,8 +20,7 @@ * exception and - depending on the "dispatched" status of the request - either * continues dispatching the request or returns control to the request handler. * - * See {@see SpecialResponsesSupport::throwStopActionWithResponse()} - * or {@see SpecialResponsesSupport::responseRedirectsToUri()} for more information. + * See {@see AbstractController::throwStatus()} or {@see AbstractController::redirectToUri()} for more information. * * Other control flow exceptions: {@see ForwardException} * From e540eb3354ab363bdc7dc80d961fc9d7b670ad0d Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Sat, 3 Feb 2024 14:07:04 +0100 Subject: [PATCH 21/21] TASK: Fix phpstan --- Neos.Flow/Classes/Mvc/Controller/AbstractController.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Neos.Flow/Classes/Mvc/Controller/AbstractController.php b/Neos.Flow/Classes/Mvc/Controller/AbstractController.php index 69268ff928..c36e276391 100644 --- a/Neos.Flow/Classes/Mvc/Controller/AbstractController.php +++ b/Neos.Flow/Classes/Mvc/Controller/AbstractController.php @@ -236,12 +236,11 @@ protected function forward(string $actionName, string $controllerName = null, st * Request is directly transfered to the other action / controller * * @param ActionRequest $request The request to redirect to - * @return void * @throws ForwardException * @see redirectToRequest() * @api */ - protected function forwardToRequest(ActionRequest $request) + protected function forwardToRequest(ActionRequest $request): never { $nextRequest = clone $request; throw ForwardException::createForNextRequest($nextRequest, '');