Skip to content

Commit

Permalink
Merge pull request #1337 from Cinderella-Man/1.3.0-issue-1331
Browse files Browse the repository at this point in the history 
[BUG] Phalcon\Events\Manager->attach('event') fails when preceded by ->dettach('event').
  • Loading branch information
Phalcon committed Oct 6, 2013
2 parents e932823 + 6e6f3c9 commit 6cd15bf
Show file tree
Hide file tree
Showing 3 changed files with 158 additions and 15 deletions.
9 changes: 4 additions & 5 deletions ext/events/manager.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,9 +231,9 @@ PHP_METHOD(Phalcon_Events_Manager, getResponses){
*
* @param string $type
*/
PHP_METHOD(Phalcon_Events_Manager, dettachAll){
PHP_METHOD(Phalcon_Events_Manager, detachAll){

zval *type = NULL, *events = NULL, *null_value;
zval *type = NULL, *events = NULL;

PHALCON_MM_GROW();

Expand All @@ -249,11 +249,10 @@ PHP_METHOD(Phalcon_Events_Manager, dettachAll){
PHALCON_INIT_NVAR(events);
} else {
if (phalcon_array_isset(events, type)) {
PHALCON_INIT_VAR(null_value);
phalcon_array_update_zval(&events, type, &null_value, PH_COPY | PH_SEPARATE);
phalcon_array_unset(&events, type, PH_SEPARATE);
}
}

phalcon_update_property_this(this_ptr, SL("_events"), events TSRMLS_CC);

PHALCON_MM_RESTORE();
Expand Down
9 changes: 5 additions & 4 deletions ext/events/manager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ PHP_METHOD(Phalcon_Events_Manager, arePrioritiesEnabled);
PHP_METHOD(Phalcon_Events_Manager, collectResponses);
PHP_METHOD(Phalcon_Events_Manager, isCollecting);
PHP_METHOD(Phalcon_Events_Manager, getResponses);
PHP_METHOD(Phalcon_Events_Manager, dettachAll);
PHP_METHOD(Phalcon_Events_Manager, detachAll);
PHP_METHOD(Phalcon_Events_Manager, fireQueue);
PHP_METHOD(Phalcon_Events_Manager, fire);
PHP_METHOD(Phalcon_Events_Manager, hasListeners);
Expand All @@ -47,7 +47,7 @@ ZEND_BEGIN_ARG_INFO_EX(arginfo_phalcon_events_manager_collectresponses, 0, 0, 1)
ZEND_ARG_INFO(0, collect)
ZEND_END_ARG_INFO()

ZEND_BEGIN_ARG_INFO_EX(arginfo_phalcon_events_manager_dettachall, 0, 0, 0)
ZEND_BEGIN_ARG_INFO_EX(arginfo_phalcon_events_manager_detachall, 0, 0, 0)
ZEND_ARG_INFO(0, type)
ZEND_END_ARG_INFO()

Expand Down Expand Up @@ -78,11 +78,12 @@ PHALCON_INIT_FUNCS(phalcon_events_manager_method_entry){
PHP_ME(Phalcon_Events_Manager, collectResponses, arginfo_phalcon_events_manager_collectresponses, ZEND_ACC_PUBLIC)
PHP_ME(Phalcon_Events_Manager, isCollecting, NULL, ZEND_ACC_PUBLIC)
PHP_ME(Phalcon_Events_Manager, getResponses, NULL, ZEND_ACC_PUBLIC)
PHP_ME(Phalcon_Events_Manager, dettachAll, arginfo_phalcon_events_manager_dettachall, ZEND_ACC_PUBLIC)
PHP_ME(Phalcon_Events_Manager, detachAll, arginfo_phalcon_events_manager_detachall, ZEND_ACC_PUBLIC)
PHP_ME(Phalcon_Events_Manager, fireQueue, arginfo_phalcon_events_manager_firequeue, ZEND_ACC_PUBLIC)
PHP_ME(Phalcon_Events_Manager, fire, arginfo_phalcon_events_manager_fire, ZEND_ACC_PUBLIC)
PHP_ME(Phalcon_Events_Manager, hasListeners, arginfo_phalcon_events_manager_haslisteners, ZEND_ACC_PUBLIC)
PHP_ME(Phalcon_Events_Manager, getListeners, arginfo_phalcon_events_manager_getlisteners, ZEND_ACC_PUBLIC)
PHP_ME(Phalcon_Events_Manager, getListeners, arginfo_phalcon_events_manager_getlisteners, ZEND_ACC_PUBLIC)
PHP_MALIAS(Phalcon_Events_Manager, dettachAll, detachAll, arginfo_phalcon_events_manager_detachall, ZEND_ACC_PUBLIC | ZEND_ACC_DEPRECATED)
PHP_FE_END
};

155 changes: 149 additions & 6 deletions unit-tests/EventsTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,15 @@ class LeDummyComponent

protected $_eventsManager;

public function setEventManager($eventsManager)
public function setEventsManager($eventsManager)
{
$this->_eventsManager = $eventsManager;
}

public function getEventsManager()
{
return $this->_eventsManager;
}

public function leAction()
{
Expand All @@ -41,7 +46,7 @@ class LeAnotherComponent

protected $_eventsManager;

public function setEventManager($eventsManager)
public function setEventsManager($eventsManager)
{
$this->_eventsManager = $eventsManager;
}
Expand Down Expand Up @@ -125,10 +130,10 @@ public function testEvents()
$eventsManager->attach('dummy', $listener);

$component = new LeDummyComponent();
$component->setEventManager($eventsManager);
$component->setEventsManager($eventsManager);

$another = new LeAnotherComponent();
$another->setEventManager($eventsManager);
$another->setEventsManager($eventsManager);

$component->leAction();
$component->leAction();
Expand Down Expand Up @@ -156,9 +161,9 @@ public function testEvents()

/*
//This is failling :(
$eventsManager->dettach('dummy', $listener);*/
$eventsManager->detach('dummy', $listener);*/

/*$eventsManager->dettachAll('dummy');
/*$eventsManager->detachAll('dummy');
$component->leAction();
$component->leAction();
Expand Down Expand Up @@ -218,4 +223,142 @@ public function testEventsWeakref()

$this->assertEquals($data, "show first listener\n");
}

/**
* "Attaching event listeners by event name fails if preceded by
* detachment of all listeners for that type."
*
* Test contains 4 steps:
* - assigning event manager to dummy service with single log event
* listener attached
* - attaching second log listener
* - detaching all log listeners
* - attaching different listener
*
* @see https://github.com/phalcon/cphalcon/issues/1331
*/
public function testBug1331()
{
$di = new Phalcon\Di;
$di->set('componentX', function() use ($di) {
$component = new LeDummyComponent();
$eventsManager = new Phalcon\Events\Manager;
$eventsManager->attach('log', $di->get('MyFirstWeakrefListener'));
$component->setEventsManager($eventsManager);
return $component;
});

$di->set('firstListener', 'MyFirstWeakrefListener');
$di->set('secondListener', 'MySecondWeakrefListener');

// ----- TESTING STEP 1 - SIGNLE 'LOG' LISTENER ATTACHED

$component = $di->get('componentX');

$logListeners = $component->getEventsManager()->getListeners('log');

$this->assertInstanceOf('MyFirstWeakrefListener', $logListeners[0]);
$this->assertCount(1, $logListeners);

// ----- TESTING STEP 2 - SECOND 'LOG' LISTENER ATTACHED

$component->getEventsManager()->attach('log', $di->get('MySecondWeakrefListener'));

$logListeners = $component->getEventsManager()->getListeners('log');

$this->assertCount(2, $logListeners);
$firstLister = array_shift($logListeners);
$secondLister = array_shift($logListeners);
$this->assertInstanceOf('MyFirstWeakrefListener', $firstLister);
$this->assertInstanceOf('MySecondWeakrefListener', $secondLister);

// ----- TESTING STEP 3 - ALL 'LOG' LISTENER DETACHED

$component->getEventsManager()->detachAll('log');

$logListeners = $component->getEventsManager()->getListeners('log');
$this->assertEmpty($logListeners);

// ----- TESTING STEP 4 - SINGLE 'LOG' LISTENER ATTACHED SECOND TIME

$component->getEventsManager()->attach('log', $di->get('MySecondWeakrefListener'));

$logListeners = $component->getEventsManager()->getListeners('log');

$this->assertInstanceOf('MySecondWeakrefListener', $logListeners[0]);
$this->assertCount(1, $logListeners);
}

/**
* "Attaching event listeners by event name fails if preceded by
* detachment of all listeners for that type."
*
* Test contains 4 steps:
* - assigning event manager to dummy service with single log event
* listener attached
* - attaching second log listener
* - detaching all log listeners
* - attaching different listener
*
* NOTE: This test looks the same as above but it checks dettachAll()
* instead of detachAll() method. To be DELETED when dettachAll()
* will not supported any more.
*
* @see https://github.com/phalcon/cphalcon/issues/1331
*/
public function testBug1331BackwardCompatibility()
{
$di = new Phalcon\Di;
$di->set('componentX', function() use ($di) {
$component = new LeDummyComponent();
$eventsManager = new Phalcon\Events\Manager;
$eventsManager->attach('log', $di->get('MyFirstWeakrefListener'));
$component->setEventsManager($eventsManager);
return $component;
});

$di->set('firstListener', 'MyFirstWeakrefListener');
$di->set('secondListener', 'MySecondWeakrefListener');

// ----- TESTING STEP 1 - SIGNLE 'LOG' LISTENER ATTACHED

$component = $di->get('componentX');

$logListeners = $component->getEventsManager()->getListeners('log');

$this->assertInstanceOf('MyFirstWeakrefListener', $logListeners[0]);
$this->assertCount(1, $logListeners);

// ----- TESTING STEP 2 - SECOND 'LOG' LISTENER ATTACHED

$component->getEventsManager()->attach('log', $di->get('MySecondWeakrefListener'));

$logListeners = $component->getEventsManager()->getListeners('log');

$this->assertCount(2, $logListeners);
$firstLister = array_shift($logListeners);
$secondLister = array_shift($logListeners);
$this->assertInstanceOf('MyFirstWeakrefListener', $firstLister);
$this->assertInstanceOf('MySecondWeakrefListener', $secondLister);

// ----- TESTING STEP 3 - ALL 'LOG' LISTENER DETACHED

$oldErrorLevel = error_reporting(E_ALL & ~E_DEPRECATED);

@$component->getEventsManager()->dettachAll('log');

error_reporting($oldErrorLevel);

$logListeners = $component->getEventsManager()->getListeners('log');
$this->assertEmpty($logListeners);

// ----- TESTING STEP 4 - SINGLE 'LOG' LISTENER ATTACHED SECOND TIME

$component->getEventsManager()->attach('log', $di->get('MySecondWeakrefListener'));

$logListeners = $component->getEventsManager()->getListeners('log');

$this->assertInstanceOf('MySecondWeakrefListener', $logListeners[0]);
$this->assertCount(1, $logListeners);
}
}

0 comments on commit 6cd15bf

Please sign in to comment.