Skip to content

Commit

Permalink
TASK: Restrict #[Flow\Route] to ActionController's
Browse files Browse the repository at this point in the history
  • Loading branch information
mhsdesign committed Mar 27, 2024
1 parent f8f90df commit ee8a812
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 26 deletions.
50 changes: 33 additions & 17 deletions Neos.Flow/Classes/Mvc/Routing/AttributeRoutesProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

namespace Neos\Flow\Mvc\Routing;

use Neos\Flow\Mvc\Controller\ActionController;
use Neos\Flow\Mvc\Exception\InvalidActionNameException;
use Neos\Flow\Mvc\Routing\Exception\InvalidControllerException;
use Neos\Flow\ObjectManagement\ObjectManagerInterface;
Expand All @@ -24,33 +25,43 @@
/**
* Allows to annotate controller methods with route configurations
*
* Implementation:
* Internal implementation:
* -----------------------
*
* Flows routing configuration is declared via \@package, \@subpackage, \@controller and \@action
* The first three options will resolve to a fully qualified class name {@see \Neos\Flow\Mvc\ActionRequest::getControllerObjectName()}
* which is instantiated in the dispatcher {@see \Neos\Flow\Mvc\Dispatcher::dispatch()}
*
* The latter \@action option will be treated internally by each controller.
* By convention and implementation of the default ActionController inside processRequest
* {@see \Neos\Flow\Mvc\Controller\ActionController::callActionMethod()} will be used to concatenate the "Action" suffix
* to the action name and invoke it internally with prepared method arguments.
* The \@action is just another routing value while the dispatcher does not really know about "actions" from the "outside".
* The latter \@action option will be treated internally by each controller. From the perspective of the dispatcher \@action is just another routing value.
* By convention during processRequest in the default ActionController {@see \ActionController::resolveActionMethodName()} will be used
* to concatenate the "Action" suffix to the action name
* and {@see ActionController::callActionMethod()} will invoke the method internally with prepared method arguments.
*
* Creating routes by annotation must make a few assumptions to work.
* As not every FQ class name is representable via the routing configuration (e.g. the class has to end with "Controller"),
* Creating routes by annotation must make a few assumptions to work:
*
* 1. As not every FQ class name is representable via the routing configuration (e.g. the class has to end with "Controller"),
* only classes can be annotated that reside in a correct location and have the correct suffix.
* Otherwise, an exception will be thrown as the class is not discoverable by the dispatcher.
*
* The routing annotation is placed at methods.
* It is validated that the annotated method ends with "Action" and a routing value with the suffix trimmed will be generated.
* Using the annotations on any controller makes the assumption that the controller will delegate the request to the dedicate
* action by depending "Action".
* This thesis is true for the ActionController.
* 2. As the ActionController requires a little magic and is the main use case we currently only support this controller.
* For that reason it is validated that the annotation is inside an ActionController and the method ends with "Action".
* The routing value with the suffix trimmed will be generated:
*
* class MyThingController extends ActionController
* {
* #[Flow\Route(path: 'foo')]
* public function someAction()
* {
* }
* }
*
* The example will genrate the configuration:
*
* \@package My.Package
* \@controller MyThing
* \@action some
*
* As discussed in https://discuss.neos.io/t/rfc-future-of-routing-mvc-in-flow/6535 we want to refactor the routing values
* to include the fully qualified controller name, so it can be easier generated without strong restrictions.
* Additionally, the action mapping should include its full name and be guaranteed to called.
* Either by invoking the action in the dispatcher or by documenting this feature as part of a implementation of a ControllerInterface
* TODO for a future scope of `Flow\Action` see {@link https://github.com/neos/flow-development-collection/issues/3335}
*/
final class AttributeRoutesProvider implements RoutesProviderInterface
{
Expand Down Expand Up @@ -80,6 +91,11 @@ public function getRoutes(): Routes
if (!$includeClassName) {
continue;
}

if (!in_array(ActionController::class, class_parents($className), true)) {
throw new InvalidControllerException('TODO: Currently #[Flow\Route] is only supported for ActionController. See https://github.com/neos/flow-development-collection/issues/3335.');
}

$controllerObjectName = $this->objectManager->getCaseSensitiveObjectName($className);
$controllerPackageKey = $this->objectManager->getPackageKeyByObjectName($controllerObjectName);
$controllerPackageNamespace = str_replace('.', '\\', $controllerPackageKey);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,7 @@ Subroutes from Annotations
--------------------------

The ``Flow\Route`` attribute allows to define routes directly on the affected method.
(Currently only ActionController are supported https://github.com/neos/flow-development-collection/issues/3335)

.. code-block:: php
Expand All @@ -774,7 +775,7 @@ The ``Flow\Route`` attribute allows to define routes directly on the affected me
}
To find the annotation and tp specify the order of routes this has to be used together with the
`\Neos\Flow\Mvc\Routing\AttributeRoutesProviderFactory` as `providerFactory` in Setting `Neos.Flow.mvs.routes`
`\Neos\Flow\Mvc\Routing\AttributeRoutesProviderFactory` as `providerFactory` in Setting `Neos.Flow.mvc.routes`

.. code-block:: yaml
Expand All @@ -787,7 +788,7 @@ To find the annotation and tp specify the order of routes this has to be used to
providerFactory: \Neos\Flow\Mvc\Routing\AttributeRoutesProviderFactory
providerOptions:
classNames:
- Vendor\Example\Controller\ExampleController
- Vendor\Example\Controller\*
Route Loading Order and the Flow Application Context
====================================================
Expand Down
21 changes: 14 additions & 7 deletions Neos.Flow/Tests/Unit/Mvc/Routing/AttributeRoutesProviderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public function setUp(): void
$this->annotationRoutesProvider = new Routing\AttributeRoutesProvider(
$this->mockReflectionService,
$this->mockObjectManager,
['Vendor\Example\Controller\ExampleController']
['Vendor\\Example\\Controller\\*']
);
}

Expand All @@ -61,19 +61,26 @@ public function noAnnotationsYieldEmptyRoutes(): void
*/
public function routesFromAnnotationAreCreatedWhenClassNamesMatch(): void
{
$exampleFqnControllerName = 'Vendor\\Example\\Controller\\ExampleController';
eval('
namespace Vendor\Example\Controller;
class ExampleController extends \Neos\Flow\Mvc\Controller\ActionController {
}'
);

$this->mockReflectionService->expects($this->once())
->method('getClassesContainingMethodsAnnotatedWith')
->with(Flow\Route::class)
->willReturn(['Vendor\Example\Controller\ExampleController']);
->willReturn([$exampleFqnControllerName]);

$this->mockReflectionService->expects($this->once())
->method('getMethodsAnnotatedWith')
->with('Vendor\Example\Controller\ExampleController', Flow\Route::class)
->with($exampleFqnControllerName, Flow\Route::class)
->willReturn(['specialAction']);

$this->mockReflectionService->expects($this->once())
->method('getMethodAnnotations')
->with('Vendor\Example\Controller\ExampleController', 'specialAction', Flow\Route::class)
->with($exampleFqnControllerName, 'specialAction', Flow\Route::class)
->willReturn([
new Flow\Route(uriPattern: 'my/path'),
new Flow\Route(
Expand All @@ -86,12 +93,12 @@ public function routesFromAnnotationAreCreatedWhenClassNamesMatch(): void

$this->mockObjectManager->expects($this->once())
->method('getCaseSensitiveObjectName')
->with('Vendor\Example\Controller\ExampleController')
->willReturn('Vendor\Example\Controller\ExampleController');
->with($exampleFqnControllerName)
->willReturn($exampleFqnControllerName);

$this->mockObjectManager->expects($this->once())
->method('getPackageKeyByObjectName')
->with('Vendor\Example\Controller\ExampleController')
->with($exampleFqnControllerName)
->willReturn('Vendor.Example');

$expectedRoute1 = new Route();
Expand Down

0 comments on commit ee8a812

Please sign in to comment.