Only accepting entity for custom methods in the TreeBehavior.

Implemented a non-recursive version of moveDown

src/Model/Behavior/TreeBehavior.php

@@ -268,22 +268,21 @@ class TreeBehavior extends Behavior {
 /**
  * Get the number of children nodes.
  *
- * @param integer|string $id The id of the record to read
+ * @param \Cake\ORM\Entity The entity to count children for
  * @param boolean $direct whether to count all nodes in the subtree or just
  * direct children
  * @return integer Number of children nodes.
  */
-	public function childCount($id, $direct = false) {
+	public function childCount($node, $direct = false) {
 		$config = $this->config();
 		list($parent, $left, $right) = [$config['parent'], $config['left'], $config['right']];
 
 		if ($direct) {
 			return $this->_scope($this->_table->find())
-				->where([$parent => $id])
+				->where([$parent => $node->get($this->_table->primaryKey())])
 				->count();
 		}
 
-		$node = $this->_getNode($id);
 		return ($node->{$right} - $node->{$left} - 1) / 2;
 	}
 
@@ -378,12 +377,12 @@ class TreeBehavior extends Behavior {
  * If the node is the first child, or is a top level node with no previous node
  * this method will return false
  *
- * @param integer|string $id The id of the record to move
+ * @param \Cake\ORM\Entity $node The node 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) {
+	public function moveUp($node, $number = 1) {
 		$config = $this->config();
 		list($parent, $left, $right) = [$config['parent'], $config['left'], $config['right']];
 
@@ -391,8 +390,6 @@ class TreeBehavior extends Behavior {
 			return false;
 		}
 
-		$node = $this->_getNode($id);
-
 		if ($node->{$parent}) {
 			$parentNode = $this->_table->get($node->{$parent}, ['fields' => [$left, $right]]);
 
@@ -420,7 +417,7 @@ class TreeBehavior extends Behavior {
 		}
 
 		if ($number) {
-			$this->moveUp($id, $number);
+			$this->moveUp($node, $number);
 		}
 
 		return true;
@@ -432,12 +429,12 @@ class TreeBehavior extends Behavior {
  * If the node is the last child, or is a top level node with no subsequent node
  * this method will return false
  *
- * @param integer|string $id The id of the record to move
+ * @param \Cake\ORM\Entity $node The node 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
+ * @return \Cake\ORM\Entity|boolean the entity after being moved or false on failure
  */
-	public function moveDown($id, $number = 1) {
+	public function moveDown($node, $number = 1) {
 		$config = $this->config();
 		list($parent, $left, $right) = [$config['parent'], $config['left'], $config['right']];
 
@@ -445,39 +442,49 @@ class TreeBehavior extends Behavior {
 			return false;
 		}
 
-		$node = $this->_getNode($id);
+		$parentRight = 0;
+		if ($node->get($parent)) {
+			$parentRight = $this->_getNode($node->get($parent))->get($right);
+		}
 
-		if ($node->{$parent}) {
-			$parentNode = $this->_table->get($node->{$parent}, ['fields' => [$left, $right]]);
+		if ($number === true) {
+			$number = PHP_INT_MAX;
+		}
 
-			if (($node->{$right} + 1) == $parentNode->{$right}) {
-				return false;
+		$edge = $this->_getMax();
+		while ($number-- > 0) {
+			list($nodeLeft, $nodeRight) = array_values($node->extract([$left, $right]));
+
+			if ($parentRight && ($right + 1 == $parentRight)) {
+				break;
 			}
-		}
 
-		$nextNode = $this->_scope($this->_table->find())
-			->select([$left, $right])
-			->where([$left => $node->{$right} + 1])
-			->first();
+			$nextNode = $this->_scope($this->_table->find())
+				->select([$left, $right])
+				->where([$left => $nodeRight + 1])
+				->first();
 
-		if (!$nextNode) {
-			return false;
-		}
+			if (!$nextNode) {
+				break;
+			}
 
-		$edge = $this->_getMax();
-		$this->_sync($edge - $node->{$left} + 1, '+', "BETWEEN {$node->{$left}} AND {$node->{$right}}");
-		$this->_sync($nextNode->{$left} - $node->{$left}, '-', "BETWEEN {$nextNode->{$left}} AND {$nextNode->{$right}}");
-		$this->_sync($edge - $node->{$left} - ($nextNode->{$right} - $nextNode->{$left}), '-', "> {$edge}");
+			$this->_sync($edge - $nodeLeft + 1, '+', "BETWEEN {$nodeLeft} AND {$nodeRight}");
+			$this->_sync($nextNode->{$left} - $nodeLeft, '-', "BETWEEN {$nextNode->{$left}} AND {$nextNode->{$right}}");
+			$this->_sync($edge - $nodeLeft - ($nextNode->{$right} - $nextNode->{$left}), '-', "> {$edge}");
 
-		if (is_int($number)) {
-			$number--;
-		}
+			$newLeft = $edge + 1;
+			if ($newLeft >= $nextNode->{$left} || $newLeft <= $nextNode->{$right}) {
+				$newLeft -= $nextNode->{$left} - $nodeLeft;
+			}
+			$newLeft -= $nextNode->{$right} - $nextNode->{$left} - 1;
 
-		if ($number) {
-			$this->moveDown($id, $number);
+			$node->set($left, $newLeft);
+			$node->set($right, $newLeft + ($nodeRight - $nodeLeft));
 		}
-
-		return true;
+		
+		$node->dirty($left, false);
+		$node->dirty($right, false);
+		return $node;
 	}
 
 /**

tests/TestCase/Model/Behavior/TreeBehaviorTest.php

@@ -79,29 +79,30 @@ class TreeBehaviorTest extends TestCase {
  */
 	public function testChildCount() {
 		// direct children for the root node
-		$countDirect = $this->table->childCount(1, true);
+		$table = $this->table;
+		$countDirect = $this->table->childCount($table->get(1), true);
 		$this->assertEquals(2, $countDirect);
 
 		// counts all the children of root
-		$count = $this->table->childCount(1, false);
+		$count = $this->table->childCount($table->get(1), false);
 		$this->assertEquals(9, $count);
 
 		// counts direct children
-		$count = $this->table->childCount(2, false);
+		$count = $this->table->childCount($table->get(2), false);
 		$this->assertEquals(3, $count);
 
 		// count children for a middle-node
-		$count = $this->table->childCount(6, false);
+		$count = $this->table->childCount($table->get(6), false);
 		$this->assertEquals(4, $count);
 
 		// count leaf children
-		$count = $this->table->childCount(10, false);
+		$count = $this->table->childCount($table->get(10), false);
 		$this->assertEquals(0, $count);
 
 		// test scoping
 		$table = TableRegistry::get('MenuLinkTrees');
 		$table->addBehavior('Tree', ['scope' => ['menu' => 'main-menu']]);
-		$count = $table->childCount(3, false);
+		$count = $table->childCount($table->get(3), false);
 		$this->assertEquals(2, $count);
 	}
 
@@ -117,7 +118,7 @@ class TreeBehaviorTest extends TestCase {
 				return $query->where(['menu' => 'main-menu']);
 			}
 		]);
-		$count = $table->childCount(1, false);
+		$count = $table->childCount($table->get(1), false);
 		$this->assertEquals(4, $count);
 	}
 
@@ -167,23 +168,23 @@ class TreeBehaviorTest extends TestCase {
 		$table->addBehavior('Tree', ['scope' => ['menu' => 'main-menu']]);
 
 		// top level, wont move
-		$this->assertFalse($this->table->moveUp(1, 10));
+		$this->assertFalse($this->table->moveUp($table->get(1), 10));
 
 		// edge cases
-		$this->assertFalse($this->table->moveUp(1, 0));
-		$this->assertFalse($this->table->moveUp(1, -10));
+		$this->assertFalse($this->table->moveUp($table->get(1), 0));
+		$this->assertFalse($this->table->moveUp($table->get(1), -10));
 
 		// move inner node
-		$result = $table->moveUp(3, 1);
+		$result = $table->moveUp($table->get(3), 1);
 		$nodes = $table->find('children', ['for' => 1])->all();
 		$this->assertEquals([3, 4, 5, 2], $nodes->extract('id')->toArray());
 		$this->assertTrue($result);
 
 		// move leaf
-		$this->assertFalse($table->moveUp(5, 1));
+		$this->assertFalse($table->moveUp($table->get(5), 1));
 
 		// move to first position
-		$table->moveUp(8, true);
+		$table->moveUp($table->get(8), true);
 		$nodes = $table->find()
 			->select(['id'])
 			->where(function($exp) {
@@ -196,18 +197,6 @@ class TreeBehaviorTest extends TestCase {
 	}
 
 /**
- * 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
@@ -215,25 +204,42 @@ class TreeBehaviorTest extends TestCase {
 	public function testMoveDown() {
 		$table = TableRegistry::get('MenuLinkTrees');
 		$table->addBehavior('Tree', ['scope' => ['menu' => 'main-menu']]);
-
 		// latest node, wont move
-		$this->assertFalse($this->table->moveDown(8, 10));
+		$node = $this->table->moveDown($table->get(8), 10);
+		$this->assertEquals(['lft' => 21, 'rght' => 22], $node->extract(['lft', 'rght']));
 
 		// edge cases
-		$this->assertFalse($this->table->moveDown(8, 0));
-		$this->assertFalse($this->table->moveUp(8, -10));
+		$this->assertFalse($this->table->moveDown($table->get(8), 0));
 
 		// move inner node
-		$result = $table->moveDown(2, 1);
+		$node = $table->moveDown($table->get(2), 1);
 		$nodes = $table->find('children', ['for' => 1])->all();
 		$this->assertEquals([3, 4, 5, 2], $nodes->extract('id')->toArray());
-		$this->assertTrue($result);
+		$this->assertEquals(['lft' => 11, 'rght' => 12], $node->extract(['lft', 'rght']));
+	}
 
-		// move leaf
-		$this->assertFalse( $table->moveDown(5, 1));
+/**
+ * Tests moving a node that has no siblings
+ *
+ * @return void
+ */
+	public function testMoveLeafDown() {
+		$table = TableRegistry::get('MenuLinkTrees');
+		$table->addBehavior('Tree', ['scope' => ['menu' => 'main-menu']]);
+		$node = $table->moveDown($table->get(5), 1);
+		$this->assertEquals(['lft' => 6, 'rght' => 7], $node->extract(['lft', 'rght']));
+	}
 
-		// move to last position
-		$table->moveDown(1, true);
+/**
+ * Tests moving a node to the bottom
+ *
+ * @return void
+ */
+	public function testMoveToBottom() {
+		$table = TableRegistry::get('MenuLinkTrees');
+		$table->addBehavior('Tree', ['scope' => ['menu' => 'main-menu']]);
+		$node = $table->moveDown($table->get(1), true);
+		$this->assertEquals(['lft' => 7, 'rght' => 16], $node->extract(['lft', 'rght']));
 		$nodes = $table->find()
 			->select(['id'])
 			->where(function($exp) {
@@ -246,18 +252,6 @@ class TreeBehaviorTest extends TestCase {
 	}
 
 /**
- * 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