Browse files

Fix null not stopping model callbacks.

Add a compatibility shim into CakeEventManager to fix `null` not
breaking model callbacks.  This was a regression created when model
callbacks were re-factored to use the event manager. This code should be
removed in 3.x as its inconsistent with events used everywhere else in
the framework.

Fixes #3123
  • Loading branch information...
1 parent cf4a9f4 commit 82a8400deff6fe56f3ea2b3bc8c59556419d1075 @markstory markstory committed Sep 13, 2012
Showing with 44 additions and 1 deletion.
  1. +4 −0 lib/Cake/Event/CakeEventManager.php
  2. +1 −1 lib/Cake/Model/Model.php
  3. +39 −0 lib/Cake/Test/Case/Model/ModelWriteTest.php
View
4 lib/Cake/Event/CakeEventManager.php
@@ -248,6 +248,10 @@ public function dispatch($event) {
if ($result === false) {
$event->stopPropagation();
}
+ // TODO remove this in 3.0 as its a compatibility shim for model callbacks.
+ if (isset($event->break, $event->breakOn) && in_array($result, (array)$event->breakOn)) {
+ $event->stopPropagation();
+ }
if ($result !== null) {
$event->result = $result;
}
View
2 lib/Cake/Model/Model.php
@@ -1667,7 +1667,7 @@ public function save($data = null, $validate = true, $fieldList = array()) {
$event = new CakeEvent('Model.beforeSave', $this, array($options));
list($event->break, $event->breakOn) = array(true, array(false, null));
$this->getEventManager()->dispatch($event);
- if (!$event->result) {
+ if ($event->isStopped()) {
$this->whitelist = $_whitelist;
return false;
}
View
39 lib/Cake/Test/Case/Model/ModelWriteTest.php
@@ -540,6 +540,25 @@ public function testBeforeValidateSaveAbortion() {
}
/**
+ * test that beforeValidate returning false can abort saves.
+ *
+ * @return void
+ */
+ public function testBeforeValidateNullSaveAbortion() {
+ $this->loadFixtures('Post');
+ $Model = new CallbackPostTestModel();
+ $Model->beforeValidateReturn = null;
+
+ $data = array(
+ 'title' => 'new article',
+ 'body' => 'this is some text.'
+ );
+ $Model->create();
+ $result = $Model->save($data);
+ $this->assertFalse($result);
+ }
+
+/**
* test that beforeSave returning false can abort saves.
*
* @return void
@@ -559,6 +578,26 @@ public function testBeforeSaveSaveAbortion() {
}
/**
+ * Test that beforeSave returnning null can abort saves.
+ *
+ * @return void
+ */
+ public function testBeforeSaveNullAbort() {
+ $this->loadFixtures('Post');
+ $Model = new CallbackPostTestModel();
+ $Model->beforeSaveReturn = null;
+
+ $data = array(
+ 'title' => 'new article',
+ 'body' => 'this is some text.'
+ );
+ $Model->create();
+ $result = $Model->save($data);
+ $this->assertFalse($result);
+
+ }
+
+/**
* testSaveField method
*
* @return void

0 comments on commit 82a8400

Please sign in to comment.