From e8984a90331ed926753dc881fffe2e051c5704e1 Mon Sep 17 00:00:00 2001 From: Andy Hobbs Date: Fri, 13 Sep 2013 09:56:38 +0100 Subject: [PATCH 1/6] Profiles and optimised EventManager::listeners() function, reducing execution time by 14.5% Refs #2105 --- lib/Cake/Event/CakeEventManager.php | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/Cake/Event/CakeEventManager.php b/lib/Cake/Event/CakeEventManager.php index 799e0b733..354497935 100644 --- a/lib/Cake/Event/CakeEventManager.php +++ b/lib/Cake/Event/CakeEventManager.php @@ -267,7 +267,18 @@ class CakeEventManager { if (empty($this->_listeners[$eventKey])) { return array(); } - ksort($this->_listeners[$eventKey]); + + $listeners = $this->_listeners[$eventKey]; + foreach ($globalListeners as $priority => $priorityQ) { + if (!empty($listeners[$priority])) { + $listeners[$priority] = array_merge($listeners[$priority], $priorityQ); + unset($globalListeners[$priority]); + } + + $listeners = $listeners + $globalListeners; + } + + ksort($listeners); $result = array(); foreach ($this->_listeners[$eventKey] as $priorityQ) { $result = array_merge($result, $priorityQ); From 615f700d22c40160af265f93a7a39fd3b67879c7 Mon Sep 17 00:00:00 2001 From: Andy Hobbs Date: Fri, 13 Sep 2013 11:41:41 +0100 Subject: [PATCH 2/6] Added EventManager::prioritisedListeners() function to alow the global event manager to return unprocessed listeners array Refs #2105 --- lib/Cake/Event/CakeEventManager.php | 21 +++- .../Test/Case/Event/CakeEventManagerTest.php | 95 ++++++++++++++++++- 2 files changed, 112 insertions(+), 4 deletions(-) diff --git a/lib/Cake/Event/CakeEventManager.php b/lib/Cake/Event/CakeEventManager.php index 354497935..9424180a6 100644 --- a/lib/Cake/Event/CakeEventManager.php +++ b/lib/Cake/Event/CakeEventManager.php @@ -264,14 +264,19 @@ class CakeEventManager { * @return array */ public function listeners($eventKey) { - if (empty($this->_listeners[$eventKey])) { + $globalListeners = array(); + if (!$this->_isGlobal) { + $globalListeners = self::instance()->prioritisedListeners($eventKey); + } + + if (empty($this->_listeners[$eventKey]) && empty($globalListeners)) { return array(); } $listeners = $this->_listeners[$eventKey]; foreach ($globalListeners as $priority => $priorityQ) { if (!empty($listeners[$priority])) { - $listeners[$priority] = array_merge($listeners[$priority], $priorityQ); + $listeners[$priority] = array_merge($priorityQ, $listeners[$priority]); unset($globalListeners[$priority]); } @@ -286,4 +291,16 @@ class CakeEventManager { return $result; } +/** + * Returns the listeners for the specified event key indexed by priority + * + * @param string $eventKey + * @return array + */ + public function prioritisedListeners($eventKey) { + if (empty($this->_listeners[$eventKey])) { + return array(); + } + return $this->_listeners[$eventKey]; + } } diff --git a/lib/Cake/Test/Case/Event/CakeEventManagerTest.php b/lib/Cake/Test/Case/Event/CakeEventManagerTest.php index 79461305e..7a4ea15e1 100644 --- a/lib/Cake/Test/Case/Event/CakeEventManagerTest.php +++ b/lib/Cake/Test/Case/Event/CakeEventManagerTest.php @@ -213,6 +213,29 @@ class CakeEventManagerTest extends CakeTestCase { $manager->dispatch($event); } +/** + * Tests event dispatching + * + * @return void + */ + public function testDispatchClosure() { + $this->skipIf( + version_compare(PHP_VERSION, '5.3.0', '<'), + 'These tests fail in PHP version < 5.3' + ); + + $manager = new CakeEventManager; + $listener = $this->getMock('CakeEventTestListener'); + $anotherListener = $this->getMock('CakeEventTestListener'); + + $manager->attach(function($testEvent) use ($listener) { $listener->listenerFunction($testEvent); }, 'fake.event'); + + $event = new CakeEvent('fake.event'); + + $listener->expects($this->once())->method('listenerFunction')->with($event); + $manager->dispatch($event); + } + /** * Tests event dispatching using event key name * @@ -388,12 +411,12 @@ class CakeEventManagerTest extends CakeTestCase { * @return void */ public function testDispatchWithGlobal() { - $generalManager = $this->getMock('CakeEventManager', array('dispatch')); + $generalManager = $this->getMock('CakeEventManager', array('prioritisedListeners')); $manager = new CakeEventManager; $event = new CakeEvent('fake.event'); CakeEventManager::instance($generalManager); - $generalManager->expects($this->once())->method('dispatch')->with($event); + $generalManager->expects($this->once())->method('prioritisedListeners')->with('fake.event'); $manager->dispatch($event); } @@ -405,6 +428,13 @@ class CakeEventManagerTest extends CakeTestCase { public function testStopPropagation() { $manager = new CakeEventManager; $listener = new CakeEventTestListener; + + CakeEventManager::instance($generalManager); + $generalManager->expects($this->any()) + ->method('prioritisedListeners') + ->with('fake.event') + ->will($this->returnValue(array())); + $manager->attach(array($listener, 'listenerFunction'), 'fake.event'); $manager->attach(array($listener, 'stopListener'), 'fake.event', array('priority' => 8)); $manager->attach(array($listener, 'secondListenerFunction'), 'fake.event', array('priority' => 5)); @@ -414,4 +444,65 @@ class CakeEventManagerTest extends CakeTestCase { $expected = array('secondListenerFunction'); $this->assertEquals($expected, $listener->callStack); } + +/** + * Tests event dispatching using priorities + * + * @return void + */ + public function testDispatchPrioritizedWithGlobal() { + $generalManager = $this->getMock('CakeEventManager'); + $manager = new CakeEventManager; + $listener = new CustomTestEventListerner; + $event = new CakeEvent('fake.event'); + + CakeEventManager::instance($generalManager); + $generalManager->expects($this->any()) + ->method('prioritisedListeners') + ->with('fake.event') + ->will($this->returnValue( + array(11 => array( + array('callable' => array($listener, 'secondListenerFunction'), 'passParams' => false) + )) + )); + + + $manager->attach(array($listener, 'listenerFunction'), 'fake.event'); + $manager->attach(array($listener, 'thirdListenerFunction'), 'fake.event', array('priority' => 15)); + + $manager->dispatch($event); + + $expected = array('listenerFunction', 'secondListenerFunction', 'thirdListenerFunction'); + $this->assertEquals($expected, $listener->callStack); + } + + /** + * Tests event dispatching using priorities + * + * @return void + */ + public function testDispatchGlobalBeforeLocal() { + $generalManager = $this->getMock('CakeEventManager'); + $manager = new CakeEventManager; + $listener = new CustomTestEventListerner; + $event = new CakeEvent('fake.event'); + + CakeEventManager::instance($generalManager); + $generalManager->expects($this->any()) + ->method('prioritisedListeners') + ->with('fake.event') + ->will($this->returnValue( + array(10 => array( + array('callable' => array($listener, 'listenerFunction'), 'passParams' => false) + )) + )); + + + $manager->attach(array($listener, 'secondListenerFunction'), 'fake.event'); + + $manager->dispatch($event); + + $expected = array('listenerFunction', 'secondListenerFunction'); + $this->assertEquals($expected, $listener->callStack); + } } From 11db7c16318df6bd9023ce48821a4aa0197283f5 Mon Sep 17 00:00:00 2001 From: Andy Hobbs Date: Sun, 15 Sep 2013 21:46:54 +0100 Subject: [PATCH 3/6] Removed EventManager test that tested using a closure as it breaks compatability with php < 5.3.0 Refs #2105 --- .../Test/Case/Event/CakeEventManagerTest.php | 23 ------------------- 1 file changed, 23 deletions(-) diff --git a/lib/Cake/Test/Case/Event/CakeEventManagerTest.php b/lib/Cake/Test/Case/Event/CakeEventManagerTest.php index 7a4ea15e1..87e8c2f5e 100644 --- a/lib/Cake/Test/Case/Event/CakeEventManagerTest.php +++ b/lib/Cake/Test/Case/Event/CakeEventManagerTest.php @@ -213,29 +213,6 @@ class CakeEventManagerTest extends CakeTestCase { $manager->dispatch($event); } -/** - * Tests event dispatching - * - * @return void - */ - public function testDispatchClosure() { - $this->skipIf( - version_compare(PHP_VERSION, '5.3.0', '<'), - 'These tests fail in PHP version < 5.3' - ); - - $manager = new CakeEventManager; - $listener = $this->getMock('CakeEventTestListener'); - $anotherListener = $this->getMock('CakeEventTestListener'); - - $manager->attach(function($testEvent) use ($listener) { $listener->listenerFunction($testEvent); }, 'fake.event'); - - $event = new CakeEvent('fake.event'); - - $listener->expects($this->once())->method('listenerFunction')->with($event); - $manager->dispatch($event); - } - /** * Tests event dispatching using event key name * From 7656ffd12e59597b3fb073e4110d71772126e00e Mon Sep 17 00:00:00 2001 From: Andy Hobbs Date: Sun, 15 Sep 2013 22:10:47 +0100 Subject: [PATCH 4/6] Corrected coding standards violations in CateEventMAnagerTest.pnp Refs #2105 --- lib/Cake/Test/Case/Event/CakeEventManagerTest.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/Cake/Test/Case/Event/CakeEventManagerTest.php b/lib/Cake/Test/Case/Event/CakeEventManagerTest.php index 87e8c2f5e..6742f4685 100644 --- a/lib/Cake/Test/Case/Event/CakeEventManagerTest.php +++ b/lib/Cake/Test/Case/Event/CakeEventManagerTest.php @@ -443,7 +443,6 @@ class CakeEventManagerTest extends CakeTestCase { )) )); - $manager->attach(array($listener, 'listenerFunction'), 'fake.event'); $manager->attach(array($listener, 'thirdListenerFunction'), 'fake.event', array('priority' => 15)); @@ -453,7 +452,7 @@ class CakeEventManagerTest extends CakeTestCase { $this->assertEquals($expected, $listener->callStack); } - /** +/** * Tests event dispatching using priorities * * @return void @@ -474,7 +473,6 @@ class CakeEventManagerTest extends CakeTestCase { )) )); - $manager->attach(array($listener, 'secondListenerFunction'), 'fake.event'); $manager->dispatch($event); From 3e373771d7f7ec5eb821a5ed41f10e5dc0a4efce Mon Sep 17 00:00:00 2001 From: Andy Hobbs Date: Mon, 16 Sep 2013 08:38:54 +0100 Subject: [PATCH 5/6] Corrected coding standards violations in CaKeEventManagerTest.php - Changed combining of arrays in CakeEventManager::listeners() to be more efficient Refs #2105 --- lib/Cake/Event/CakeEventManager.php | 14 ++---- .../Test/Case/Event/CakeEventManagerTest.php | 48 ++++++++++--------- 2 files changed, 30 insertions(+), 32 deletions(-) diff --git a/lib/Cake/Event/CakeEventManager.php b/lib/Cake/Event/CakeEventManager.php index 9424180a6..c88c6b977 100644 --- a/lib/Cake/Event/CakeEventManager.php +++ b/lib/Cake/Event/CakeEventManager.php @@ -230,15 +230,12 @@ class CakeEventManager { $event = new CakeEvent($event); } - if (!$this->_isGlobal) { - self::instance()->dispatch($event); - } - - if (empty($this->_listeners[$event->name()])) { + $listeners = $this->listeners($event->name()); + if (empty($listeners)) { return; } - foreach ($this->listeners($event->name()) as $listener) { + foreach ($listeners as $listener) { if ($event->isStopped()) { break; } @@ -279,13 +276,12 @@ class CakeEventManager { $listeners[$priority] = array_merge($priorityQ, $listeners[$priority]); unset($globalListeners[$priority]); } - - $listeners = $listeners + $globalListeners; } + $listeners = $listeners + $globalListeners; ksort($listeners); $result = array(); - foreach ($this->_listeners[$eventKey] as $priorityQ) { + foreach ($listeners as $priorityQ) { $result = array_merge($result, $priorityQ); } return $result; diff --git a/lib/Cake/Test/Case/Event/CakeEventManagerTest.php b/lib/Cake/Test/Case/Event/CakeEventManagerTest.php index 6742f4685..7053dddcf 100644 --- a/lib/Cake/Test/Case/Event/CakeEventManagerTest.php +++ b/lib/Cake/Test/Case/Event/CakeEventManagerTest.php @@ -104,7 +104,7 @@ class CakeEventManagerTest extends CakeTestCase { * @return void */ public function testAttachListeners() { - $manager = new CakeEventManager; + $manager = new CakeEventManager(); $manager->attach('fakeFunction', 'fake.event'); $expected = array( array('callable' => 'fakeFunction', 'passParams' => false) @@ -136,7 +136,7 @@ class CakeEventManagerTest extends CakeTestCase { * @return void */ public function testAttachMultipleEventKeys() { - $manager = new CakeEventManager; + $manager = new CakeEventManager(); $manager->attach('fakeFunction', 'fake.event'); $manager->attach('fakeFunction2', 'another.event'); $manager->attach('fakeFunction3', 'another.event', array('priority' => 1, 'passParams' => true)); @@ -158,7 +158,7 @@ class CakeEventManagerTest extends CakeTestCase { * @return void */ public function testDetach() { - $manager = new CakeEventManager; + $manager = new CakeEventManager(); $manager->attach(array('AClass', 'aMethod'), 'fake.event'); $manager->attach(array('AClass', 'anotherMethod'), 'another.event'); $manager->attach('fakeFunction', 'another.event', array('priority' => 1)); @@ -182,7 +182,7 @@ class CakeEventManagerTest extends CakeTestCase { * @return void */ public function testDetachFromAll() { - $manager = new CakeEventManager; + $manager = new CakeEventManager(); $manager->attach(array('AClass', 'aMethod'), 'fake.event'); $manager->attach(array('AClass', 'aMethod'), 'another.event'); $manager->attach('fakeFunction', 'another.event', array('priority' => 1)); @@ -201,7 +201,7 @@ class CakeEventManagerTest extends CakeTestCase { * @return void */ public function testDispatch() { - $manager = new CakeEventManager; + $manager = new CakeEventManager(); $listener = $this->getMock('CakeEventTestListener'); $anotherListener = $this->getMock('CakeEventTestListener'); $manager->attach(array($listener, 'listenerFunction'), 'fake.event'); @@ -219,8 +219,8 @@ class CakeEventManagerTest extends CakeTestCase { * @return void */ public function testDispatchWithKeyName() { - $manager = new CakeEventManager; - $listener = new CakeEventTestListener; + $manager = new CakeEventManager(); + $listener = new CakeEventTestListener(); $manager->attach(array($listener, 'listenerFunction'), 'fake.event'); $event = 'fake.event'; $manager->dispatch($event); @@ -239,7 +239,7 @@ class CakeEventManagerTest extends CakeTestCase { version_compare(PHPUnit_Runner_Version::id(), '3.7', '<'), 'These tests fail in PHPUnit 3.6' ); - $manager = new CakeEventManager; + $manager = new CakeEventManager(); $listener = $this->getMock('CakeEventTestListener'); $anotherListener = $this->getMock('CakeEventTestListener'); $manager->attach(array($listener, 'listenerFunction'), 'fake.event'); @@ -267,7 +267,7 @@ class CakeEventManagerTest extends CakeTestCase { 'These tests fail in PHPUnit 3.6' ); - $manager = new CakeEventManager; + $manager = new CakeEventManager(); $listener = $this->getMock('CakeEventTestListener'); $anotherListener = $this->getMock('CakeEventTestListener'); $manager->attach(array($listener, 'listenerFunction'), 'fake.event'); @@ -289,8 +289,8 @@ class CakeEventManagerTest extends CakeTestCase { * @return void */ public function testDispatchPrioritized() { - $manager = new CakeEventManager; - $listener = new CakeEventTestListener; + $manager = new CakeEventManager(); + $listener = new CakeEventTestListener(); $manager->attach(array($listener, 'listenerFunction'), 'fake.event'); $manager->attach(array($listener, 'secondListenerFunction'), 'fake.event', array('priority' => 5)); $event = new CakeEvent('fake.event'); @@ -306,7 +306,7 @@ class CakeEventManagerTest extends CakeTestCase { * @return void */ public function testDispatchPassingParams() { - $manager = new CakeEventManager; + $manager = new CakeEventManager(); $listener = $this->getMock('CakeEventTestListener'); $anotherListener = $this->getMock('CakeEventTestListener'); $manager->attach(array($listener, 'listenerFunction'), 'fake.event'); @@ -324,7 +324,7 @@ class CakeEventManagerTest extends CakeTestCase { * @return void */ public function testAttachSubscriber() { - $manager = new CakeEventManager; + $manager = new CakeEventManager(); $listener = $this->getMock('CustomTestEventListener', array('secondListenerFunction')); $manager->attach($listener); $event = new CakeEvent('fake.event'); @@ -338,7 +338,7 @@ class CakeEventManagerTest extends CakeTestCase { $event = new CakeEvent('another.event', $this, array('some' => 'data')); $manager->dispatch($event); - $manager = new CakeEventManager; + $manager = new CakeEventManager(); $listener = $this->getMock('CustomTestEventListener', array('listenerFunction', 'thirdListenerFunction')); $manager->attach($listener); $event = new CakeEvent('multiple.handlers'); @@ -353,7 +353,7 @@ class CakeEventManagerTest extends CakeTestCase { * @return void */ public function testDetachSubscriber() { - $manager = new CakeEventManager; + $manager = new CakeEventManager(); $listener = $this->getMock('CustomTestEventListener', array('secondListenerFunction')); $manager->attach($listener); $expected = array( @@ -376,7 +376,7 @@ class CakeEventManagerTest extends CakeTestCase { */ public function testGlobalDispatcherGetter() { $this->assertInstanceOf('CakeEventManager', CakeEventManager::instance()); - $manager = new CakeEventManager; + $manager = new CakeEventManager(); CakeEventManager::instance($manager); $this->assertSame($manager, CakeEventManager::instance()); @@ -389,7 +389,7 @@ class CakeEventManagerTest extends CakeTestCase { */ public function testDispatchWithGlobal() { $generalManager = $this->getMock('CakeEventManager', array('prioritisedListeners')); - $manager = new CakeEventManager; + $manager = new CakeEventManager(); $event = new CakeEvent('fake.event'); CakeEventManager::instance($generalManager); @@ -403,8 +403,9 @@ class CakeEventManagerTest extends CakeTestCase { * @return void */ public function testStopPropagation() { - $manager = new CakeEventManager; - $listener = new CakeEventTestListener; + $generalManager = $this->getMock('CakeEventManager'); + $manager = new CakeEventManager(); + $listener = new CakeEventTestListener(); CakeEventManager::instance($generalManager); $generalManager->expects($this->any()) @@ -429,8 +430,8 @@ class CakeEventManagerTest extends CakeTestCase { */ public function testDispatchPrioritizedWithGlobal() { $generalManager = $this->getMock('CakeEventManager'); - $manager = new CakeEventManager; - $listener = new CustomTestEventListerner; + $manager = new CakeEventManager(); + $listener = new CustomTestEventListener(); $event = new CakeEvent('fake.event'); CakeEventManager::instance($generalManager); @@ -459,8 +460,8 @@ class CakeEventManagerTest extends CakeTestCase { */ public function testDispatchGlobalBeforeLocal() { $generalManager = $this->getMock('CakeEventManager'); - $manager = new CakeEventManager; - $listener = new CustomTestEventListerner; + $manager = new CakeEventManager(); + $listener = new CustomTestEventListener(); $event = new CakeEvent('fake.event'); CakeEventManager::instance($generalManager); @@ -480,4 +481,5 @@ class CakeEventManagerTest extends CakeTestCase { $expected = array('listenerFunction', 'secondListenerFunction'); $this->assertEquals($expected, $listener->callStack); } + } From 63d867c98b3472316ea250b242b759ef8d386375 Mon Sep 17 00:00:00 2001 From: mark_story Date: Sun, 10 Nov 2013 22:16:09 -0500 Subject: [PATCH 6/6] Restore global event managers that were causing cascading failures. Refs #2105 --- lib/Cake/Test/Case/Event/CakeEventManagerTest.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/Cake/Test/Case/Event/CakeEventManagerTest.php b/lib/Cake/Test/Case/Event/CakeEventManagerTest.php index 7053dddcf..7df403da4 100644 --- a/lib/Cake/Test/Case/Event/CakeEventManagerTest.php +++ b/lib/Cake/Test/Case/Event/CakeEventManagerTest.php @@ -395,6 +395,7 @@ class CakeEventManagerTest extends CakeTestCase { $generalManager->expects($this->once())->method('prioritisedListeners')->with('fake.event'); $manager->dispatch($event); + CakeEventManager::instance(new CakeEventManager()); } /** @@ -421,6 +422,7 @@ class CakeEventManagerTest extends CakeTestCase { $expected = array('secondListenerFunction'); $this->assertEquals($expected, $listener->callStack); + CakeEventManager::instance(new CakeEventManager()); } /** @@ -451,6 +453,7 @@ class CakeEventManagerTest extends CakeTestCase { $expected = array('listenerFunction', 'secondListenerFunction', 'thirdListenerFunction'); $this->assertEquals($expected, $listener->callStack); + CakeEventManager::instance(new CakeEventManager()); } /** @@ -480,6 +483,7 @@ class CakeEventManagerTest extends CakeTestCase { $expected = array('listenerFunction', 'secondListenerFunction'); $this->assertEquals($expected, $listener->callStack); + CakeEventManager::instance(new CakeEventManager()); } }