Skip to content

Commit

Permalink
Migrating all model callbacks to the CakeEventManager, fixing some mi…
Browse files Browse the repository at this point in the history 
…nor bugs. All tests passing again
  • Loading branch information
@lorenzo
lorenzo committed Dec 26, 2011
1 parent 35ecbfe commit 1651257
Show file tree
Hide file tree
Showing 6 changed files with 104 additions and 65 deletions.
5 changes: 2 additions & 3 deletions lib/Cake/Controller/Controller.php
View file
Expand Up @@ -732,9 +732,8 @@ public function redirect($url, $status = null, $exit = true) {
extract($status, EXTR_OVERWRITE);
}
$event = new CakeEvent('Controller.beforeRedirect', $this, array($url, $status, $exit));
//TODO: Remove the following two lines when the events are fully migrated to the CakeEventManager
$event->breakOn = false;
$event->collectReturn = true;
//TODO: Remove the following line when the events are fully migrated to the CakeEventManager
list($event->break, $event->breakOn, $event->collectReturn) = array(true, false, true);
$this->getEventManager()->dispatch($event);

if ($event->isStopped()) {
Expand Down
20 changes: 19 additions & 1 deletion lib/Cake/Model/BehaviorCollection.php
View file
Expand Up @@ -20,6 +20,7 @@
*/

App::uses('ObjectCollection', 'Utility');
App::uses('CakeEventListener', 'Event');

/**
* Model behavior collection class.
Expand All @@ -28,7 +29,7 @@
*
* @package Cake.Model
*/
class BehaviorCollection extends ObjectCollection {
class BehaviorCollection extends ObjectCollection implements CakeEventListener {

/**
* Stores a reference to the attached name
Expand Down Expand Up @@ -271,4 +272,21 @@ public function hasMethod($method, $callback = false) {
return false;
}

/**
* Returns the implemented events that will get routed to the trigger function
* in order to dispatch them separately on each behavior
*
* @return array
*/
public function implementedEvents() {
return array(
'Model.beforeFind' => 'trigger',
'Model.afterFind' => 'trigger',
'Model.beforeValidate' => 'trigger',
'Model.beforeSave' => 'trigger',
'Model.afterSave' => 'trigger',
'Model.beforeDelete' => 'trigger',
'Model.afterDelete' => 'trigger'
);
}
}
120 changes: 71 additions & 49 deletions lib/Cake/Model/Model.php
View file
Expand Up @@ -27,6 +27,9 @@
App::uses('ModelBehavior', 'Model');
App::uses('ConnectionManager', 'Model');
App::uses('Xml', 'Utility');
App::uses('CakeEvent', 'Event');
App::uses('CakeEventListener', 'Event');
App::uses('CakeEventManager', 'Event');

/**
* Object-relational mapper.
Expand All @@ -39,7 +42,7 @@
* @package Cake.Model
* @link http://book.cakephp.org/2.0/en/models.html
*/
class Model extends Object {
class Model extends Object implements CakeEventListener {

/**
* The name of the DataSource connection that this Model uses
Expand Down Expand Up @@ -607,6 +610,14 @@ class Model extends Object {
'neighbors' => true, 'list' => true, 'threaded' => true
);

/**
* Instance of the CakeEventManager this model is using
* to dispatch inner events.
*
* @var CakeEventManager
*/
protected $_eventManager = null;

/**
* Constructor. Binds the model's database table to the object.
*
Expand Down Expand Up @@ -712,6 +723,40 @@ public function __construct($id = false, $table = null, $ds = null) {
$this->Behaviors->init($this->alias, $this->actsAs);
}

/**
* Returns a list of all events that will fire in the model during it's lifecycle.
* You can override this function to add you own listener callbacks
*
* @return array
*/
public function implementedEvents() {
return array(
'Model.beforeFind' => array('callable' => 'beforeFind', 'passParams' => true),
'Model.afterFind' => array('callable' => 'afterFind', 'passParams' => true),
'Model.beforeValidate' => array('callable' => 'beforeValidate', 'passParams' => true),
'Model.beforeSave' => array('callable' => 'beforeSave', 'passParams' => true),
'Model.afterSave' => array('callable' => 'afterSave', 'passParams' => true),
'Model.beforeDelete' => array('callable' => 'beforeDelete', 'passParams' => true, 'priority' => 9),
'Model.afterDelete' => array('callable' => 'afterDelete'),
);
}

/**
* Returns the CakeEventManager manager instance that is handling any callbacks.
* You can use this instance to register any new listeners or callbacks to the
* controller events, or create your own events and trigger them at will.
*
* @return CakeEventManager
*/
public function getEventManager() {
if (empty($this->_eventManager)) {
$this->_eventManager = new CakeEventManager();
$this->_eventManager->attach($this->Behaviors);
$this->_eventManager->attach($this);
}
return $this->_eventManager;
}

/**
* Handles custom method calls, like findBy<field> for DB models,
* and custom RPC calls for remote data sources.
Expand Down Expand Up @@ -1585,10 +1630,10 @@ public function save($data = null, $validate = true, $fieldList = array()) {
}

if ($options['callbacks'] === true || $options['callbacks'] === 'before') {
$result = $this->Behaviors->trigger('beforeSave', array(&$this, $options), array(
'break' => true, 'breakOn' => array(false, null)
));
if (!$result || !$this->beforeSave($options)) {
$event = new CakeEvent('Model.beforeSave', $this, array($options));
list($event->break, $event->breakOn) = array(true, array(false, null));

This comment has been minimized.

Sign in to view

Copy link
@SimonEast

SimonEast Jul 27, 2012

Lorenzo,
If a behavior returns null from a beforeSave() (or neglects to return anything), this code appears to appears to stop other behaviors' beforeSave() functions running, but still allows the save to go ahead. Is that the intended outcome?

This comment has been minimized.

Sign in to view

Copy link
@lorenzo

lorenzo Jul 27, 2012

Author Member

Yes that is the intended behavior for model callbacks, as I had to maintain backwards compatibility.

This comment has been minimized.

Sign in to view

Copy link
@ADmad

ADmad Jul 27, 2012

Member

We should remove this BC in 3.0. Event propagation should stop only if you explicitly return false.

This comment has been minimized.

Sign in to view

Copy link
@markstory

markstory Jul 27, 2012

Member

I think removing the BC behavior in the event system is on the list for the 3.0 roadmap. If not we should add it.

This comment has been minimized.

Sign in to view

Copy link
@SimonEast

SimonEast via email Jul 27, 2012

$this->getEventManager()->dispatch($event);
if (!$event->result) {
$this->whitelist = $_whitelist;
return false;
}
Expand Down Expand Up @@ -1672,8 +1717,8 @@ public function save($data = null, $validate = true, $fieldList = array()) {
}
}
if ($options['callbacks'] === true || $options['callbacks'] === 'after') {
$this->Behaviors->trigger('afterSave', array(&$this, $created, $options));
$this->afterSave($created);
$event = new CakeEvent('Model.afterSave', $this, array($created, $options));
$this->getEventManager()->dispatch($event);
}
if (!empty($this->data)) {
$success = Set::merge($success, $this->data);
Expand Down Expand Up @@ -2236,13 +2281,11 @@ public function delete($id = null, $cascade = true) {
}
$id = $this->id;

if ($this->beforeDelete($cascade)) {
$filters = $this->Behaviors->trigger(
'beforeDelete',
array(&$this, $cascade),
array('break' => true, 'breakOn' => array(false, null))
);
if (!$filters || !$this->exists()) {
$event = new CakeEvent('Model.beforeDelete', $this, array($cascade));
list($event->break, $event->breakOn) = array(true, array(false, null));
$this->getEventManager()->dispatch($event);
if (!$event->isStopped()) {
if (!$this->exists()) {
return false;
}
$db = $this->getDataSource();
Expand Down Expand Up @@ -2272,8 +2315,7 @@ public function delete($id = null, $cascade = true) {
if ($updateCounterCache) {
$this->updateCounterCache($keys[$this->alias]);
}
$this->Behaviors->trigger('afterDelete', array(&$this));
$this->afterDelete();
$this->getEventManager()->dispatch(new CakeEvent('Model.afterDelete', $this));
$this->_clearCache();
$this->id = false;
return true;
Expand Down Expand Up @@ -2566,24 +2608,13 @@ public function buildQuery($type = 'first', $query = array()) {
$query['order'] = array($query['order']);

if ($query['callbacks'] === true || $query['callbacks'] === 'before') {
$return = $this->Behaviors->trigger(
'beforeFind',
array(&$this, $query),
array('break' => true, 'breakOn' => array(false, null), 'modParams' => 1)
);

$query = (is_array($return)) ? $return : $query;

if ($return === false) {
return null;
}

$return = $this->beforeFind($query);
$query = (is_array($return)) ? $return : $query;

if ($return === false) {
$event = new CakeEvent('Model.beforeFind', $this, array($query));
list($event->break, $event->breakOn, $event->modParams) = array(true, array(false, null), 0);
$this->getEventManager()->dispatch($event);
if ($event->isStopped()) {
return null;
}
$query = $event->data[0];
}

return $query;
Expand Down Expand Up @@ -2814,15 +2845,10 @@ protected function _findThreaded($state, $query, $results = array()) {
* @return array Set of filtered results
*/
protected function _filterResults($results, $primary = true) {
$return = $this->Behaviors->trigger(
'afterFind',
array(&$this, $results, $primary),
array('modParams' => 1)
);
if ($return !== true) {
$results = $return;
}
return $this->afterFind($results, $primary);
$event = new CakeEvent('Model.afterFind', $this, array($results, $primary));
$event->modParams = 0;
$this->getEventManager()->dispatch($event);
return $event->result;
}

/**
Expand Down Expand Up @@ -2936,14 +2962,10 @@ public function validates($options = array()) {
* @see Model::validates()
*/
public function invalidFields($options = array()) {
if (
!$this->Behaviors->trigger(
'beforeValidate',
array(&$this, $options),
array('break' => true, 'breakOn' => false)
) ||
$this->beforeValidate($options) === false
) {
$event = new CakeEvent('Model.beforeValidate', $this, array($options));
list($event->break, $event->breakOn) = array(true, false);
$this->getEventManager()->dispatch($event);
if ($event->isStopped()) {
return false;
}

Expand Down
20 changes: 10 additions & 10 deletions lib/Cake/Test/Case/Model/BehaviorCollectionTest.php
View file
Expand Up @@ -859,31 +859,31 @@ public function testBehaviorSaveCallbacks() {

$Sample->Behaviors->attach('Test', array('beforeSave' => 'on'));
$Sample->create();
$this->assertSame($Sample->save($record), false);
$this->assertSame(false, $Sample->save($record));

$Sample->Behaviors->attach('Test', array('beforeSave' => 'off'));
$Sample->create();
$result = $Sample->save($record);
$expected = $record;
$expected['Sample']['id'] = $Sample->id;
$this->assertSame($result, $expected);
$this->assertSame($expected, $result);

$Sample->Behaviors->attach('Test', array('beforeSave' => 'test'));
$Sample->create();
$result = $Sample->save($record);
$expected = $record;
$expected['Sample']['id'] = $Sample->id;
$this->assertSame($result, $expected);
$this->assertSame($expected, $result);

$Sample->Behaviors->attach('Test', array('beforeSave' => 'modify'));
$expected = Set::insert($record, 'Sample.name', 'sample99 modified before');
$Sample->create();
$result = $Sample->save($record);
$expected['Sample']['id'] = $Sample->id;
$this->assertSame($result, $expected);
$this->assertSame($expected, $result);

$Sample->Behaviors->disable('Test');
$this->assertSame($Sample->save($record), $record);
$this->assertSame($record, $Sample->save($record));

$Sample->Behaviors->attach('Test', array('beforeSave' => 'off', 'afterSave' => 'on'));
$expected = Set::merge($record, array('Sample' => array('aftersave' => 'modified after on create')));
Expand All @@ -897,21 +897,21 @@ public function testBehaviorSaveCallbacks() {
$Sample->create();
$result = $Sample->save($record);
$expected['Sample']['id'] = $Sample->id;
$this->assertSame($result, $expected);
$this->assertSame($expected, $result);

$Sample->Behaviors->attach('Test', array('beforeSave' => 'off', 'afterSave' => 'test'));
$Sample->create();
$expected = $record;
$result = $Sample->save($record);
$expected['Sample']['id'] = $Sample->id;
$this->assertSame($result, $expected);
$this->assertSame($expected, $result);

$Sample->Behaviors->attach('Test', array('afterSave' => 'test2'));
$Sample->create();
$expected = $record;
$result = $Sample->save($record);
$expected['Sample']['id'] = $Sample->id;
$this->assertSame($result, $expected);
$this->assertSame($expected, $result);

$Sample->Behaviors->attach('Test', array('beforeFind' => 'off', 'afterFind' => 'off'));
$Sample->recursive = -1;
Expand All @@ -920,12 +920,12 @@ public function testBehaviorSaveCallbacks() {
$Sample->Behaviors->attach('Test', array('afterSave' => 'on'));
$expected = Set::merge($record2, array('Sample' => array('aftersave' => 'modified after')));
$Sample->create();
$this->assertSame($Sample->save($record2), $expected);
$this->assertSame($expected, $Sample->save($record2));

$Sample->Behaviors->attach('Test', array('afterSave' => 'modify'));
$expected = Set::merge($record2, array('Sample' => array('name' => 'sample1 modified after')));
$Sample->create();
$this->assertSame($Sample->save($record2), $expected);
$this->assertSame($expected, $Sample->save($record2));
}

/**
Expand Down
2 changes: 1 addition & 1 deletion lib/Cake/Test/Case/Model/ModelReadTest.php
View file
Expand Up @@ -5041,7 +5041,7 @@ public function testCallbackSourceChange() {
* @return void
*/
public function testCallbackSourceChangeUnknownDatasource() {
$this->loadFixtures('Post');
$this->loadFixtures('Post', 'Author');
$TestModel = new Post();
$this->assertFalse($TestModel->find('all', array('connection' => 'foo')));
}
Expand Down
2 changes: 1 addition & 1 deletion lib/Cake/Utility/ObjectCollection.php
View file
Expand Up @@ -104,7 +104,7 @@ public function trigger($callback, $params = array(), $options = array()) {
$subject = $event->subject();
}
//TODO: Temporary BC check, while we move all the triggers system into the CakeEventManager
foreach (array('breakOn', 'collectReturn', 'modParams') as $opt) {
foreach (array('break', 'breakOn', 'collectReturn', 'modParams') as $opt) {
if (isset($event->{$opt})) {
$options[$opt] = $event->{$opt};
}
Expand Down

0 comments on commit 1651257

Please sign in to comment.