From bddff7d2b0c57d756b1ec1eadb3e7da7c8236622 Mon Sep 17 00:00:00 2001 From: Kim Biesbjerg Date: Wed, 13 Jul 2016 23:40:27 +0200 Subject: [PATCH 1/6] Dispatch afterDispatch event when exception is thrown --- lib/Cake/Error/ExceptionRenderer.php | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/lib/Cake/Error/ExceptionRenderer.php b/lib/Cake/Error/ExceptionRenderer.php index a245b64ee..40f551417 100644 --- a/lib/Cake/Error/ExceptionRenderer.php +++ b/lib/Cake/Error/ExceptionRenderer.php @@ -20,9 +20,12 @@ */ App::uses('Sanitize', 'Utility'); +App::uses('Dispatcher', 'Routing'); App::uses('Router', 'Routing'); -App::uses('CakeResponse', 'Network'); App::uses('Controller', 'Controller'); +App::uses('CakeRequest', 'Network'); +App::uses('CakeResponse', 'Network'); +App::uses('CakeEvent', 'Event'); /** * Exception Renderer. @@ -287,7 +290,7 @@ class ExceptionRenderer { protected function _outputMessage($template) { try { $this->controller->render($template); - $this->controller->afterFilter(); + $this->_shutdown(); $this->controller->response->send(); } catch (MissingViewException $e) { $attributes = $e->getAttributes(); @@ -327,4 +330,15 @@ class ExceptionRenderer { $this->controller->response->send(); } + protected function _shutdown() { + $this->controller->afterFilter(); + + $Dispatcher = new Dispatcher(); + $afterDispatchEvent = new CakeEvent('Dispatcher.afterDispatch', $Dispatcher, array( + 'request' => $this->controller->request, + 'response' => $this->controller->response + )); + $Dispatcher->getEventManager()->dispatch($afterDispatchEvent); + } + } From 5a63ee4e3e0daf076cf0f1e0dc957efa7fd9364e Mon Sep 17 00:00:00 2001 From: Kim Biesbjerg Date: Thu, 14 Jul 2016 00:17:02 +0200 Subject: [PATCH 2/6] Added tests to prove that Dispatcher.afterDispatch event is dispatched by exception renderer on error response --- .../Test/Case/Error/ExceptionRendererTest.php | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/lib/Cake/Test/Case/Error/ExceptionRendererTest.php b/lib/Cake/Test/Case/Error/ExceptionRendererTest.php index 51cc4ef70..2bff49aea 100644 --- a/lib/Cake/Test/Case/Error/ExceptionRendererTest.php +++ b/lib/Cake/Test/Case/Error/ExceptionRendererTest.php @@ -20,6 +20,7 @@ App::uses('ExceptionRenderer', 'Error'); App::uses('Controller', 'Controller'); App::uses('Component', 'Controller'); App::uses('Router', 'Routing'); +App::uses('CakeEvent', 'Event'); /** * Short description for class. @@ -61,6 +62,26 @@ class BlueberryComponent extends Component { } +/** + * AfterDispatchEventCallable class + * + * @package Cake.Test.Case.Error + */ +class AfterDispatchEventCallable { + + public $afterDispatchCalled = false; +/** + * AfterDispatchEventCallable::afterDispatch() + * + * @param mixed $event + * @return mixed boolean to stop the event dispatching or null to continue + */ + public function afterDispatch(CakeEvent $event) { + $this->afterDispatchCalled = true; + } + +} + /** * TestErrorController class * @@ -877,4 +898,25 @@ class ExceptionRendererTest extends CakeTestCase { $this->assertContains(h('SELECT * from poo_query < 5 and :seven'), $result); $this->assertContains("'seven' => (int) 7", $result); } + +/** + * Tests that exception renderer dispatches Dispatcher.afterDispatch event on error response + * + * @return void + */ + public function testAfterDispatchEventOnErrorResponse() { + $callable = new AfterDispatchEventCallable(); + Configure::write('Dispatcher.filters', array( + array('callable' => array($callable, 'afterDispatch'), 'on' => 'after') + )); + + $exception = new Exception('Oh no!'); + $ExceptionRenderer = new ExceptionRenderer($exception); + + ob_start(); + $ExceptionRenderer->render(); + $result = ob_get_clean(); + + $this->assertTrue($callable->afterDispatchCalled); + } } From a05639a30e8385c91fd0a1a12f4874f42ccecb49 Mon Sep 17 00:00:00 2001 From: Kim Biesbjerg Date: Thu, 14 Jul 2016 00:20:33 +0200 Subject: [PATCH 3/6] Don't save buffered output to var since it is not used --- lib/Cake/Test/Case/Error/ExceptionRendererTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Cake/Test/Case/Error/ExceptionRendererTest.php b/lib/Cake/Test/Case/Error/ExceptionRendererTest.php index 2bff49aea..abd620d68 100644 --- a/lib/Cake/Test/Case/Error/ExceptionRendererTest.php +++ b/lib/Cake/Test/Case/Error/ExceptionRendererTest.php @@ -915,7 +915,7 @@ class ExceptionRendererTest extends CakeTestCase { ob_start(); $ExceptionRenderer->render(); - $result = ob_get_clean(); + ob_get_clean(); $this->assertTrue($callable->afterDispatchCalled); } From c77b6288418fe504dee2491a8272a1255be5cfaa Mon Sep 17 00:00:00 2001 From: Kim Biesbjerg Date: Thu, 14 Jul 2016 04:51:49 +0200 Subject: [PATCH 4/6] Dispatch Controller.shutdown instead of calling afterFilter directly. Updated test --- lib/Cake/Error/ExceptionRenderer.php | 3 +- .../Test/Case/Error/ExceptionRendererTest.php | 39 ++++++------------- 2 files changed, 14 insertions(+), 28 deletions(-) diff --git a/lib/Cake/Error/ExceptionRenderer.php b/lib/Cake/Error/ExceptionRenderer.php index 40f551417..4da938167 100644 --- a/lib/Cake/Error/ExceptionRenderer.php +++ b/lib/Cake/Error/ExceptionRenderer.php @@ -331,7 +331,8 @@ class ExceptionRenderer { } protected function _shutdown() { - $this->controller->afterFilter(); + $afterFilterEvent = new CakeEvent('Controller.shutdown', $this->controller); + $this->controller->getEventManager()->dispatch($afterFilterEvent); $Dispatcher = new Dispatcher(); $afterDispatchEvent = new CakeEvent('Dispatcher.afterDispatch', $Dispatcher, array( diff --git a/lib/Cake/Test/Case/Error/ExceptionRendererTest.php b/lib/Cake/Test/Case/Error/ExceptionRendererTest.php index abd620d68..9decaa7e5 100644 --- a/lib/Cake/Test/Case/Error/ExceptionRendererTest.php +++ b/lib/Cake/Test/Case/Error/ExceptionRendererTest.php @@ -20,7 +20,7 @@ App::uses('ExceptionRenderer', 'Error'); App::uses('Controller', 'Controller'); App::uses('Component', 'Controller'); App::uses('Router', 'Routing'); -App::uses('CakeEvent', 'Event'); +App::uses('CakeEventManager', 'Event'); /** * Short description for class. @@ -62,26 +62,6 @@ class BlueberryComponent extends Component { } -/** - * AfterDispatchEventCallable class - * - * @package Cake.Test.Case.Error - */ -class AfterDispatchEventCallable { - - public $afterDispatchCalled = false; -/** - * AfterDispatchEventCallable::afterDispatch() - * - * @param mixed $event - * @return mixed boolean to stop the event dispatching or null to continue - */ - public function afterDispatch(CakeEvent $event) { - $this->afterDispatchCalled = true; - } - -} - /** * TestErrorController class * @@ -905,18 +885,23 @@ class ExceptionRendererTest extends CakeTestCase { * @return void */ public function testAfterDispatchEventOnErrorResponse() { - $callable = new AfterDispatchEventCallable(); - Configure::write('Dispatcher.filters', array( - array('callable' => array($callable, 'afterDispatch'), 'on' => 'after') - )); + $fired = array(); + $listener = function ($event) use (&$fired) { + $fired[] = $event->name(); + }; - $exception = new Exception('Oh no!'); + $EventManager = CakeEventManager::instance(); + $EventManager->attach($listener, 'Controller.shutdown'); + $EventManager->attach($listener, 'Dispatcher.afterDispatch'); + + $exception = new Exception('Terrible'); $ExceptionRenderer = new ExceptionRenderer($exception); ob_start(); $ExceptionRenderer->render(); ob_get_clean(); - $this->assertTrue($callable->afterDispatchCalled); + $expected = array('Controller.shutdown', 'Dispatcher.afterDispatch'); + $this->assertEquals($expected, $fired); } } From 38cad279d187c4e4f84e1fceba2ce7f4d78f3de5 Mon Sep 17 00:00:00 2001 From: Kim Biesbjerg Date: Thu, 14 Jul 2016 04:57:25 +0200 Subject: [PATCH 5/6] Updated test method name and description --- lib/Cake/Test/Case/Error/ExceptionRendererTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Cake/Test/Case/Error/ExceptionRendererTest.php b/lib/Cake/Test/Case/Error/ExceptionRendererTest.php index 9decaa7e5..cebc7dd2c 100644 --- a/lib/Cake/Test/Case/Error/ExceptionRendererTest.php +++ b/lib/Cake/Test/Case/Error/ExceptionRendererTest.php @@ -880,11 +880,11 @@ class ExceptionRendererTest extends CakeTestCase { } /** - * Tests that exception renderer dispatches Dispatcher.afterDispatch event on error response + * Test that rendering exceptions triggers shutdown events. * * @return void */ - public function testAfterDispatchEventOnErrorResponse() { + public function testRenderShutdownEvents() { $fired = array(); $listener = function ($event) use (&$fired) { $fired[] = $event->name(); From cbdc89ddeee1db2b01a357662cdaf3367642655a Mon Sep 17 00:00:00 2001 From: Kim Biesbjerg Date: Thu, 14 Jul 2016 05:50:37 +0200 Subject: [PATCH 6/6] Fix CS error --- lib/Cake/Error/ExceptionRenderer.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/Cake/Error/ExceptionRenderer.php b/lib/Cake/Error/ExceptionRenderer.php index 4da938167..d5d32ff95 100644 --- a/lib/Cake/Error/ExceptionRenderer.php +++ b/lib/Cake/Error/ExceptionRenderer.php @@ -330,6 +330,13 @@ class ExceptionRenderer { $this->controller->response->send(); } +/** + * Run the shutdown events. + * + * Triggers the afterFilter and afterDispatch events. + * + * @return void + */ protected function _shutdown() { $afterFilterEvent = new CakeEvent('Controller.shutdown', $this->controller); $this->controller->getEventManager()->dispatch($afterFilterEvent);