moveUp|Down should return false on failure instead of throw

- Refactoring, remove usage of extract()
- Test cases updated

src/Model/Behavior/TreeBehavior.php

@@ -117,10 +117,11 @@ class TreeBehavior extends Behavior {
  * @throws \InvalidArgumentException When the 'for' key is not passed in $options
  */
 	public function findChildren($query, $options) {
-		extract($this->config());
-		extract($options);
+		$config = $this->config();
+		$options += ['for' => null, 'direct' => false];
+		list($parent, $left, $right) = [$config['parent'], $config['left'], $config['right']];
+		list($for, $direct) = [$options['for'], $options['direct']];
 		$primaryKey = $this->_table->primaryKey();
-		$direct = !isset($direct) ? false : $direct;
 
 		if (empty($for)) {
 			throw new \InvalidArgumentException("The 'for' key is required for find('children')");
@@ -157,11 +158,11 @@ class TreeBehavior extends Behavior {
  *
  * @param integer|string $id The ID of the record to move
  * @param integer|boolean $number How many places to move the node, or true to move to first position
- * @throws \Cake\ORM\Error\RecordNotFoundException When node was not found
  * @return boolean true on success, false on failure
  */
 	public function moveUp($id, $number = 1) {
-		extract($this->config());
+		$config = $this->config();
+		list($parent, $left, $right) = [$config['parent'], $config['left'], $config['right']];
 		$primaryKey = $this->_table->primaryKey();
 
 		if (!$number) {
@@ -174,7 +175,7 @@ class TreeBehavior extends Behavior {
 			->first();
 
 		if (!$node) {
-			throw new \Cake\ORM\Error\RecordNotFoundException("Node \"{$id}\" was not found in the tree.");
+			return false;
 		}
 
 		if ($node->{$parent}) {
@@ -217,11 +218,11 @@ class TreeBehavior extends Behavior {
  *
  * @param integer|string $id The ID of the record to move
  * @param integer|boolean $number How many places to move the node or true to move to last position
- * @throws \Cake\ORM\Error\RecordNotFoundException When node was not found
  * @return boolean true on success, false on failure
  */
 	public function moveDown($id, $number = 1) {
-		extract($this->config());
+		$config = $this->config();
+		list($parent, $left, $right) = [$config['parent'], $config['left'], $config['right']];
 		$primaryKey = $this->_table->primaryKey();
 
 		if (!$number) {
@@ -234,7 +235,7 @@ class TreeBehavior extends Behavior {
 			->first();
 
 		if (!$node) {
-			throw new \Cake\ORM\Error\RecordNotFoundException("Node \"{$id}\" was not found in the tree.");
+			return false;
 		}
 
 		if ($node->{$parent}) {

tests/TestCase/Model/Behavior/TreeBehaviorTest.php

@@ -172,8 +172,10 @@ class TreeBehaviorTest extends TestCase {
 		$this->assertEquals(false, $this->table->moveUp(1, 0));
 		$this->assertEquals(false, $this->table->moveUp(1, -10));
 
+		// move unexisting node
+		$this->assertEquals(false, $table->moveUp(500, 1));
+
 		// move inner node
-		$nodeIds = [];
 		$result = $table->moveUp(3, 1);
 		$nodes = $table->find('children', ['for' => 1])->all();
 		$this->assertEquals([3, 4, 5, 2], $nodes->extract('id')->toArray());
@@ -186,25 +188,13 @@ class TreeBehaviorTest extends TestCase {
 		$table->moveUp(8, true);
 		$nodes = $table->find()
 			->select(['id'])
-			->where(['parent_id' => 0, 'menu' => 'main-menu'])
+			->where(['parent_id IS' => null, 'menu' => 'main-menu'])
 			->order(['lft' => 'ASC'])
 			->all();
 		$this->assertEquals([8, 1, 6], $nodes->extract('id')->toArray());
 	}
 
 /**
- * Tests that moveUp() will throw an exception if the node was not found
- *
- * @expectedException \Cake\ORM\Error\RecordNotFoundException
- * @return void
- */
-	public function testMoveUpException() {
-		$table = TableRegistry::get('MenuLinkTrees');
-		$table->addBehavior('Tree', ['scope' => ['menu' => 'main-menu']]);
-		$table->moveUp(500, 1);
-	}
-
-/**
  * Tests the moveDown() method
  *
  * @return void
@@ -227,6 +217,9 @@ class TreeBehaviorTest extends TestCase {
 		$this->assertEquals([3, 4, 5, 2], $nodes->extract('id')->toArray());
 		$this->assertEquals(true, $result);
 
+		// move unexisting node
+		$this->assertEquals(false, $table->moveDown(500, 1));
+
 		// move leaf
 		$this->assertEquals(false, $table->moveDown(5, 1));
 
@@ -234,25 +227,13 @@ class TreeBehaviorTest extends TestCase {
 		$table->moveDown(1, true);
 		$nodes = $table->find()
 			->select(['id'])
-			->where(['parent_id' => 0, 'menu' => 'main-menu'])
+			->where(['parent_id IS' => null, 'menu' => 'main-menu'])
 			->order(['lft' => 'ASC'])
 			->all();
 		$this->assertEquals([6, 8, 1], $nodes->extract('id')->toArray());
 	}
 
 /**
- * Tests that moveDown() will throw an exception if the node was not found
- *
- * @expectedException \Cake\ORM\Error\RecordNotFoundException
- * @return void
- */
-	public function testMoveDownException() {
-		$table = TableRegistry::get('MenuLinkTrees');
-		$table->addBehavior('Tree', ['scope' => ['menu' => 'main-menu']]);
-		$table->moveDown(500, 1);
-	}
-
-/**
  * Tests the recover function
  *
  * @return void