Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

!!! FEATURE: WIP Dispatcher and controller return ActionResponse (simpler controller pattern) #3232

Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
d94562f
!!!FEATURE: Clearer controller pattern
kitsunet Nov 11, 2023
8ee71e9
Controller cleanup
kitsunet Dec 26, 2023
0fcf522
Style and suggestions cleanup
kitsunet Dec 26, 2023
e3fb34b
Tests for SimpleActionController
kitsunet Dec 26, 2023
b12f1df
Fix AjaxWidget dispatching to use new StopActionException::response
kitsunet Dec 27, 2023
3360130
Correct style issues
kitsunet Dec 27, 2023
0689c78
Fix spelling error
kitsunet Dec 28, 2023
465c80b
Merge remote-tracking branch 'origin/9.0' into feature/actioncontroll…
mhsdesign Jan 25, 2024
88f43d5
TASK: Adjust typos and doc of `ControllerInterface`
mhsdesign Jan 25, 2024
03662a7
TASK: Fix `RestController::redirectToUri`
mhsdesign Jan 25, 2024
3df0dbd
Update Neos.Flow/Tests/Functional/Http/Fixtures/Controller/FooControl…
kitsunet Jan 26, 2024
57d8995
TASK: Update `ControllerInterface::processRequest` doc
mhsdesign Jan 25, 2024
f3ba560
TASK: Fix `FooController` test
mhsdesign Jan 26, 2024
e3cdfb4
!!! TASK: Separate `ForwardException` from `StopActionException`
mhsdesign Jan 25, 2024
d507942
!!! TASK: Adjust Signals Dispatcher `beforeControllerInvocation` and …
mhsdesign Jan 26, 2024
c1ce1bc
TASK: Set fix $code of `StopActionException`
mhsdesign Jan 26, 2024
18da350
TASK: Adjust docs of MVC Control flow exceptions
mhsdesign Jan 26, 2024
73d7214
TASK: Rename factory methods of MVC Control flow exceptions
mhsdesign Jan 26, 2024
a59bdaa
Merge pull request #3 from mhsdesign/test-can-i-push-to-christian-que…
kitsunet Jan 26, 2024
554aa08
SpecialResponses become final
kitsunet Jan 27, 2024
82caa27
TASK: Revert introduction of `SimpleActionController`
mhsdesign Feb 3, 2024
c96fd84
TASK: Revert introduction of `SpecialResponsesSupport`
mhsdesign Feb 3, 2024
e540eb3
TASK: Fix phpstan
mhsdesign Feb 3, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions Neos.Flow/Classes/Mvc/ActionRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -510,7 +511,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;
}
Expand All @@ -520,7 +521,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;
}
Expand Down Expand Up @@ -698,6 +699,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
{
Expand Down
90 changes: 50 additions & 40 deletions Neos.Flow/Classes/Mvc/Controller/AbstractController.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@
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;
use Neos\Flow\Http\Helper\ResponseInformationHelper;
Expand All @@ -22,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;
Expand Down Expand Up @@ -101,7 +107,6 @@
*
* @param ActionRequest $request
* @param ActionResponse $response
* @throws UnsupportedRequestTypeException
*/
protected function initializeController(ActionRequest $request, ActionResponse $response)
{
Expand Down Expand Up @@ -180,23 +185,29 @@
* 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<string, mixed> $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);

if ($controllerName !== null) {
$nextRequest->setControllerName($controllerName);
}
if ($packageKey !== null && strpos($packageKey, '\\') !== false) {
list($packageKey, $subpackageKey) = explode('\\', $packageKey, 2);
if ($packageKey !== null && str_contains($packageKey, '\\')) {
[$packageKey, $subpackageKey] = explode('\\', $packageKey, 2);
} else {
$subpackageKey = null;
}
Expand All @@ -207,7 +218,7 @@

$regularArguments = [];
foreach ($arguments as $argumentName => $argumentValue) {
if (substr($argumentName, 0, 2) === '__') {
if (str_starts_with($argumentName, '__')) {
$nextRequest->setArgument($argumentName, $argumentValue);
} else {
$regularArguments[$argumentName] = $argumentValue;
Expand All @@ -216,9 +227,7 @@
$nextRequest->setArguments($this->persistenceManager->convertObjectsToIdentityArrays($regularArguments));
$this->arguments->removeAll();

$forwardException = new ForwardException();
$forwardException->setNextRequest($nextRequest);
throw $forwardException;
$this->forwardToRequest($nextRequest);

Check failure on line 230 in Neos.Flow/Classes/Mvc/Controller/AbstractController.php

View workflow job for this annotation

GitHub Actions / PHP 8.2 Test static analysis (deps: highest)

Method Neos\Flow\Mvc\Controller\AbstractController::forward() should always throw an exception or terminate script execution but doesn't do that.
}

/**
Expand All @@ -234,12 +243,8 @@
*/
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());
$nextRequest = clone $request;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we only need to clone as far as i know, because that will unset the dispatch state? But that will be hopefully be fixed via #3301

throw ForwardException::createForNextRequest($nextRequest, '');
}

/**
Expand All @@ -251,20 +256,23 @@
* 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 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<string, string> $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 string|null $format The format to use for the redirect URI
* @return never
* @throws StopActionException
* @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) {
list($packageKey, $subpackageKey) = explode('\\', $packageKey, 2);
if ($packageKey !== null && str_contains($packageKey, '\\') !== false) {
[$packageKey, $subpackageKey] = explode('\\', $packageKey, 2);
} else {
$subpackageKey = null;
}
Expand All @@ -284,18 +292,17 @@
*
* 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"
* @return void
* @return never
* @throws MissingActionNameException
* @throws StopActionException
* @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();
Expand All @@ -308,14 +315,13 @@
/**
* 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 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 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) {
Expand All @@ -326,7 +332,7 @@
$this->response->setStatusCode($statusCode);
$this->response->setContent('<html><head><meta http-equiv="refresh" content="' . (int)$delay . ';url=' . $uri . '"/></head></html>');
}
throw new StopActionException();
throw StopActionException::createForResponse($this->response, '');
}

/**
Expand All @@ -351,25 +357,29 @@
);
}
$this->response->setContent($content);
throw new StopActionException($content, 1558088618);
throw StopActionException::createForResponse($this->response, $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
Loading
Loading