Browse files

Migrating all model callbacks to the CakeEventManager, fixing some mi…

…nor bugs. All tests passing again
  • Loading branch information...
1 parent 35ecbfe commit 1651257919783336a3a4de4596fb9726949ac8b9 @lorenzo lorenzo committed Dec 26, 2011
View
5 lib/Cake/Controller/Controller.php
@@ -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()) {
View
20 lib/Cake/Model/BehaviorCollection.php
@@ -20,6 +20,7 @@
*/
App::uses('ObjectCollection', 'Utility');
+App::uses('CakeEventListener', 'Event');
/**
* Model behavior collection class.
@@ -28,7 +29,7 @@
*
* @package Cake.Model
*/
-class BehaviorCollection extends ObjectCollection {
+class BehaviorCollection extends ObjectCollection implements CakeEventListener {
/**
* Stores a reference to the attached name
@@ -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'
+ );
+ }
}
View
120 lib/Cake/Model/Model.php
@@ -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.
@@ -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
@@ -608,6 +611,14 @@ class Model extends Object {
);
/**
+ * 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.
*
* If `$id` is an array it can be used to pass several options into the model.
@@ -713,6 +724,40 @@ public function __construct($id = false, $table = null, $ds = null) {
}
/**
+ * 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.
*
@@ -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));

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?

@lorenzo
CakePHP member
lorenzo added a note Jul 27, 2012

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

@ADmad
CakePHP member
ADmad added a note Jul 27, 2012

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

@markstory
CakePHP 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ $this->getEventManager()->dispatch($event);
+ if (!$event->result) {
$this->whitelist = $_whitelist;
return false;
}
@@ -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);
@@ -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();
@@ -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;
@@ -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;
@@ -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;
}
/**
@@ -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;
}
View
20 lib/Cake/Test/Case/Model/BehaviorCollectionTest.php
@@ -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')));
@@ -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;
@@ -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));
}
/**
View
2 lib/Cake/Test/Case/Model/ModelReadTest.php
@@ -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')));
}
View
2 lib/Cake/Utility/ObjectCollection.php
@@ -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};
}

0 comments on commit 1651257

Please sign in to comment.