Merge pull request #3301 from neos/remove-dispatched-action-request

!!! TASK: Remove dispatched state from ActionRequest

Neos.Flow/Classes/Mvc/ActionRequest.php

@@ -18,15 +18,14 @@
 use Neos\Flow\ObjectManagement\ObjectManagerInterface;
 use Neos\Flow\Package\PackageManager;
 use Neos\Flow\Security\Cryptography\HashService;
-use Neos\Flow\SignalSlot\Dispatcher as SignalSlotDispatcher;
 use Neos\Utility\Arrays;
 
 /**
  * Represents an internal request targeted to a controller action
  *
  * @api
  */
-class ActionRequest implements RequestInterface
+class ActionRequest
 {
     /**
      * @Flow\Inject
@@ -264,38 +263,6 @@ public function getReferringRequest(): ?ActionRequest
         return $this->referringRequest;
     }
 
-    /**
-     * Sets the dispatched flag
-     *
-     * @param boolean $flag If this request has been dispatched
-     * @return void
-     * @throws \Neos\Flow\SignalSlot\Exception\InvalidSlotException
-     * @api
-     */
-    public function setDispatched($flag): void
-    {
-        $this->dispatched = (bool)$flag;
-
-        if ($flag) {
-            $this->emitRequestDispatched($this);
-        }
-    }
-
-    /**
-     * If this request has been dispatched and addressed by the responsible
-     * controller and the response is ready to be sent.
-     *
-     * The dispatcher will try to dispatch the request again if it has not been
-     * addressed yet.
-     *
-     * @return boolean true if this request has been dispatched successfully
-     * @api
-     */
-    public function isDispatched(): bool
-    {
-        return $this->dispatched;
-    }
-
     /**
      * Returns the object name of the controller defined by the package key and
      * controller name
@@ -689,28 +656,6 @@ public function getFormat(): string
         return $this->format;
     }
 
-    /**
-     * Emits a signal when a Request has been dispatched
-     *
-     * The action request is not proxyable, so the signal is dispatched manually here.
-     * The safeguard allows unit tests without the dispatcher dependency.
-     *
-     * @param ActionRequest $request
-     * @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
-    {
-        if ($this->objectManager !== null) {
-            $dispatcher = $this->objectManager->get(SignalSlotDispatcher::class);
-            if ($dispatcher !== null) {
-                $dispatcher->dispatch(ActionRequest::class, 'requestDispatched', [$request]);
-            }
-        }
-    }
-
     /**
      * Resets the dispatched status to false
      */

Neos.Flow/Classes/Mvc/Controller/AbstractController.php

@@ -115,8 +115,6 @@ protected function initializeController(ActionRequest $request, ActionResponse $
         $this->request = $request;
         $this->response = $response;
 
-        $this->request->setDispatched(true);
-
         $this->uriBuilder = new UriBuilder();
         $this->uriBuilder->setRequest($request);
 

Neos.Flow/Classes/Mvc/Controller/ControllerInterface.php

@@ -34,13 +34,12 @@ interface ControllerInterface
      *
      * 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.
+     * - The response 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.
+     * which the Dispatcher will catch and handle its attached next-request.
      *
      * @param ActionRequest $request The dispatched action request
      * @return ResponseInterface The resulting created response

Neos.Flow/Classes/Mvc/Dispatcher.php

@@ -11,7 +11,6 @@
  * source code.
  */
 
-use GuzzleHttp\Psr7\Response;
 use Neos\Flow\Annotations as Flow;
 use Neos\Flow\Configuration\Exception\NoSuchOptionException;
 use Neos\Flow\Log\PsrLoggerFactoryInterface;
@@ -123,28 +122,23 @@ public function dispatch(ActionRequest $request): ResponseInterface
     protected function initiateDispatchLoop(ActionRequest $request): ResponseInterface
     {
         $dispatchLoopCount = 0;
-        while ($request->isDispatched() === false) {
-            if ($dispatchLoopCount++ > 99) {
-                throw new Exception\InfiniteLoopException(sprintf('Could not ultimately dispatch the request after %d iterations.', $dispatchLoopCount), 1217839467);
-            }
+        do {
             $controller = $this->resolveController($request);
             $this->emitBeforeControllerInvocation($request, $controller);
             try {
                 $response = $controller->processRequest($request);
                 $this->emitAfterControllerInvocation($request, $response, $controller);
+                return $response;
             } catch (StopActionException $stopActionException) {
-                $this->emitAfterControllerInvocation($request, $stopActionException->response, $controller);
                 $response = $stopActionException->response;
-                if (!$request->isMainRequest()) {
-                    $request = $request->getParentRequest();
-                }
+                $this->emitAfterControllerInvocation($request, $response, $controller);
+                return $response;
             } catch (ForwardException $forwardException) {
                 $this->emitAfterControllerInvocation($request, null, $controller);
                 $request = $forwardException->nextRequest;
             }
-        }
-        // 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 Response();
+        } while ($dispatchLoopCount++ < 99);
+        throw new Exception\InfiniteLoopException(sprintf('Could not ultimately dispatch the request after %d iterations.', $dispatchLoopCount), 1217839467);
     }
 
     /**

Neos.Flow/Tests/Unit/Mvc/ActionRequestTest.php

@@ -21,7 +21,6 @@
 use Neos\Flow\Package\PackageManager;
 use Neos\Flow\Security\Cryptography\HashService;
 use Neos\Flow\Security\Exception\InvalidHashException;
-use Neos\Flow\SignalSlot\Dispatcher;
 use Neos\Flow\Tests\UnitTestCase;
 use Psr\Http\Message\ServerRequestInterface;
 
@@ -61,15 +60,6 @@ public function anActionRequestIsRequiredAsParentRequest()
         self::assertSame($this->actionRequest, $anotherActionRequest->getParentRequest());
     }
 
-    /**
-     * @test
-     */
-    public function constructorThrowsAnExceptionIfNoValidRequestIsPassed()
-    {
-        $this->expectException(\Error::class);
-        new ActionRequest(new \stdClass());
-    }
-
     /**
      * @test
      */
@@ -109,24 +99,6 @@ public function isMainRequestChecksIfTheParentRequestIsNotAnHttpRequest()
         self::assertFalse($yetAnotherActionRequest->isMainRequest());
     }
 
-    /**
-     * @test
-     */
-    public function requestIsDispatchable()
-    {
-        $mockDispatcher = $this->createMock(Dispatcher::class);
-
-        $mockObjectManager = $this->createMock(ObjectManagerInterface::class);
-        $mockObjectManager->expects(self::any())->method('get')->will(self::returnValue($mockDispatcher));
-        $this->inject($this->actionRequest, 'objectManager', $mockObjectManager);
-
-        self::assertFalse($this->actionRequest->isDispatched());
-        $this->actionRequest->setDispatched(true);
-        self::assertTrue($this->actionRequest->isDispatched());
-        $this->actionRequest->setDispatched(false);
-        self::assertFalse($this->actionRequest->isDispatched());
-    }
-
     /**
      * @test
      */
@@ -524,18 +496,6 @@ public function theRepresentationFormatCanBeSetAndRetrieved()
         self::assertEquals('html', $this->actionRequest->getFormat());
     }
 
-    /**
-     * @test
-     */
-    public function cloneResetsTheStatusToNotDispatched()
-    {
-        $this->actionRequest->setDispatched(true);
-        $cloneRequest = clone $this->actionRequest;
-
-        self::assertTrue($this->actionRequest->isDispatched());
-        self::assertFalse($cloneRequest->isDispatched());
-    }
-
     /**
      * @test
      */
@@ -558,21 +518,6 @@ public function getReferringRequestThrowsAnExceptionIfTheHmacOfTheArgumentsCould
         $this->actionRequest->getReferringRequest();
     }
 
-    /**
-     * @test
-     */
-    public function setDispatchedEmitsSignalIfDispatched()
-    {
-        $mockDispatcher = $this->createMock(Dispatcher::class);
-        $mockDispatcher->expects(self::once())->method('dispatch')->with(ActionRequest::class, 'requestDispatched', [$this->actionRequest]);
-
-        $mockObjectManager = $this->createMock(ObjectManagerInterface::class);
-        $mockObjectManager->expects(self::any())->method('get')->will(self::returnValue($mockDispatcher));
-        $this->inject($this->actionRequest, 'objectManager', $mockObjectManager);
-
-        $this->actionRequest->setDispatched(true);
-    }
-
     /**
      * @test
      */

Neos.Flow/Tests/Unit/Mvc/Controller/AbstractControllerTest.php

@@ -87,10 +87,8 @@ public function initializeControllerInitializesRequestUriBuilderArgumentsAndCont
 
         $controller = $this->getAccessibleMock(AbstractController::class, ['processRequest']);
 
-        self::assertFalse($request->isDispatched());
         $controller->_call('initializeController', $request, $this->actionResponse);
 
-        self::assertTrue($request->isDispatched());
         self::assertInstanceOf(Arguments::class, $controller->_get('arguments'));
         self::assertSame($request, $controller->_get('uriBuilder')->getRequest());
         self::assertSame($request, $controller->getControllerContext()->getRequest());