diff --git a/lib/private/AppFramework/Http/Dispatcher.php b/lib/private/AppFramework/Http/Dispatcher.php index 6219ba47a41..7b9ad015de6 100644 --- a/lib/private/AppFramework/Http/Dispatcher.php +++ b/lib/private/AppFramework/Http/Dispatcher.php @@ -105,9 +105,6 @@ class Dispatcher { } catch(\Exception $exception){ $response = $this->middlewareDispatcher->afterException( $controller, $methodName, $exception); - if ($response === null) { - throw $exception; - } } $response = $this->middlewareDispatcher->afterController( diff --git a/lib/private/AppFramework/Middleware/MiddlewareDispatcher.php b/lib/private/AppFramework/Middleware/MiddlewareDispatcher.php index 60d2ae8b5c9..e1262b6c712 100644 --- a/lib/private/AppFramework/Middleware/MiddlewareDispatcher.php +++ b/lib/private/AppFramework/Middleware/MiddlewareDispatcher.php @@ -1,4 +1,5 @@ middlewares = array(); + $this->middlewares = []; $this->middlewareCounter = 0; } @@ -71,7 +72,7 @@ class MiddlewareDispatcher { * returns an array with all middleware elements * @return array the middlewares */ - public function getMiddlewares(){ + public function getMiddlewares(): array { return $this->middlewares; } @@ -84,10 +85,10 @@ class MiddlewareDispatcher { * @param string $methodName the name of the method that will be called on * the controller */ - public function beforeController(Controller $controller, $methodName){ + public function beforeController(Controller $controller, string $methodName){ // we need to count so that we know which middlewares we have to ask in // case there is an exception - $middlewareCount = count($this->middlewares); + $middlewareCount = \count($this->middlewares); for($i = 0; $i < $middlewareCount; $i++){ $this->middlewareCounter++; $middleware = $this->middlewares[$i]; @@ -111,7 +112,7 @@ class MiddlewareDispatcher { * exception * @throws \Exception the passed in exception if it can't handle it */ - public function afterException(Controller $controller, $methodName, \Exception $exception){ + public function afterException(Controller $controller, string $methodName, \Exception $exception): Response { for($i=$this->middlewareCounter-1; $i>=0; $i--){ $middleware = $this->middlewares[$i]; try { @@ -134,8 +135,8 @@ class MiddlewareDispatcher { * @param Response $response the generated response from the controller * @return Response a Response object */ - public function afterController(Controller $controller, $methodName, Response $response){ - for($i=count($this->middlewares)-1; $i>=0; $i--){ + public function afterController(Controller $controller, string $methodName, Response $response): Response { + for($i= \count($this->middlewares)-1; $i>=0; $i--){ $middleware = $this->middlewares[$i]; $response = $middleware->afterController($controller, $methodName, $response); } @@ -153,8 +154,8 @@ class MiddlewareDispatcher { * @param string $output the generated output from a response * @return string the output that should be printed */ - public function beforeOutput(Controller $controller, $methodName, $output){ - for($i=count($this->middlewares)-1; $i>=0; $i--){ + public function beforeOutput(Controller $controller, string $methodName, string $output): string { + for($i= \count($this->middlewares)-1; $i>=0; $i--){ $middleware = $this->middlewares[$i]; $output = $middleware->beforeOutput($controller, $methodName, $output); } diff --git a/lib/public/AppFramework/Http/Response.php b/lib/public/AppFramework/Http/Response.php index e47e01349e7..512b312dae1 100644 --- a/lib/public/AppFramework/Http/Response.php +++ b/lib/public/AppFramework/Http/Response.php @@ -232,11 +232,11 @@ class Response { /** * By default renders no output - * @return string|null + * @return string * @since 6.0.0 */ public function render() { - return null; + return ''; } diff --git a/tests/lib/AppFramework/Http/DispatcherTest.php b/tests/lib/AppFramework/Http/DispatcherTest.php index 95fa3c2a047..221e27e3b68 100644 --- a/tests/lib/AppFramework/Http/DispatcherTest.php +++ b/tests/lib/AppFramework/Http/DispatcherTest.php @@ -26,10 +26,12 @@ namespace Test\AppFramework\Http; use OC\AppFramework\Http\Dispatcher; use OC\AppFramework\Http\Request; +use OC\AppFramework\Middleware\MiddlewareDispatcher; use OC\AppFramework\Utility\ControllerMethodReflector; use OCP\AppFramework\Http; use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\DataResponse; +use OCP\AppFramework\Http\Response; use OCP\AppFramework\Controller; use OCP\IConfig; @@ -75,7 +77,9 @@ class TestController extends Controller { class DispatcherTest extends \Test\TestCase { + /** @var MiddlewareDispatcher */ private $middlewareDispatcher; + /** @var Dispatcher */ private $dispatcher; private $controllerMethod; private $response; @@ -120,14 +124,13 @@ class DispatcherTest extends \Test\TestCase { $this->reflector = new ControllerMethodReflector(); $this->dispatcher = new Dispatcher( - $this->http, $this->middlewareDispatcher, $this->reflector, + $this->http, + $this->middlewareDispatcher, + $this->reflector, $this->request ); - $this->response = $this->getMockBuilder( - '\OCP\AppFramework\Http\Response') - ->disableOriginalConstructor() - ->getMock(); + $this->response = $this->createMock(Response::class); $this->lastModified = new \DateTime(null, new \DateTimeZone('GMT')); $this->etag = 'hi'; @@ -162,7 +165,7 @@ class DispatcherTest extends \Test\TestCase { ->with($this->equalTo($this->controller), $this->equalTo($this->controllerMethod), $this->equalTo($exception)) - ->will($this->returnValue(null)); + ->willThrowException($exception); return; } } else { @@ -221,10 +224,9 @@ class DispatcherTest extends \Test\TestCase { public function testDispatcherReturnsArrayWith2Entries() { - $this->setMiddlewareExpectations(); + $this->setMiddlewareExpectations(''); - $response = $this->dispatcher->dispatch($this->controller, - $this->controllerMethod); + $response = $this->dispatcher->dispatch($this->controller, $this->controllerMethod); $this->assertNull($response[0]); $this->assertEquals(array(), $response[1]); $this->assertNull($response[2]); @@ -264,12 +266,14 @@ class DispatcherTest extends \Test\TestCase { public function testExceptionThrowsIfCanNotBeHandledByAfterException() { $out = 'yo'; $httpHeaders = 'Http'; - $responseHeaders = array('hell' => 'yeah'); + $responseHeaders = ['hell' => 'yeah']; $this->setMiddlewareExpectations($out, $httpHeaders, $responseHeaders, true, false); $this->expectException(\Exception::class); - $response = $this->dispatcher->dispatch($this->controller, - $this->controllerMethod); + $this->dispatcher->dispatch( + $this->controller, + $this->controllerMethod + ); } diff --git a/tests/lib/AppFramework/Middleware/MiddlewareDispatcherTest.php b/tests/lib/AppFramework/Middleware/MiddlewareDispatcherTest.php index b2f78c17fe6..e6c2c1b4f53 100644 --- a/tests/lib/AppFramework/Middleware/MiddlewareDispatcherTest.php +++ b/tests/lib/AppFramework/Middleware/MiddlewareDispatcherTest.php @@ -283,6 +283,8 @@ class MiddlewareDispatcherTest extends \Test\TestCase { ->method('beforeController'); $m3->expects($this->never()) ->method('afterController'); + $m3->method('beforeOutput') + ->will($this->returnArgument(2)); $this->dispatcher->registerMiddleware($m3);