Skip to content

Commit

Permalink
!!!FEATURE: Clearer controller pattern
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@kitsunet
kitsunet committed Nov 11, 2023
1 parent c64ef2a commit d94562f
Show file tree
Hide file tree
Showing 19 changed files with 352 additions and 153 deletions.
51 changes: 31 additions & 20 deletions Neos.Flow/Classes/Mvc/Controller/AbstractController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -36,6 +39,8 @@
*/
abstract class AbstractController implements ControllerInterface
{
use SpecialResponsesSupport;

/**
* @var UriBuilder
*/
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand All @@ -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
*/
Expand All @@ -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('<html><head><meta http-equiv="refresh" content="' . (int)$delay . ';url=' . $uri . '"/></head></html>');
if (!$uri instanceof UriInterface) {
$uri = new Uri($uri);
}
throw new StopActionException();

$response = $this->responseRedirectsToUri($uri,$delay, $statusCode, $this->response);
$this->throwStopActionWithReponse($response, '', 1699716808);
}

/**
Expand Down Expand Up @@ -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);
}
Expand Down
88 changes: 49 additions & 39 deletions Neos.Flow/Classes/Mvc/Controller/ActionController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand All @@ -230,15 +230,15 @@ 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__));
$this->throwStatus(400, null, 'Invalid HMAC submitted');
}

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__));
Expand All @@ -249,31 +249,35 @@ 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);
$this->view->setControllerContext($this->controllerContext);
$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);
}
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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'];
}
Expand All @@ -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);
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
}
}
9 changes: 4 additions & 5 deletions Neos.Flow/Classes/Mvc/Controller/ControllerInterface.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -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;
}
Loading

0 comments on commit d94562f

Please sign in to comment.