Implement ORM_MPTT::delete() properly.

Inline ORM_MPTT::_grow() for now
Rewrite ORM_MPTT tests to be domain specific; they no longer use album/photo helpers.

2 files changed, 82 insertions, 82 deletions

diff --git a/core/libraries/ORM_MPTT.php b/core/libraries/ORM_MPTT.php
index 2cdc617d..bac4da92 100644
--- a/core/libraries/ORM_MPTT.php
+++ b/core/libraries/ORM_MPTT.php
@@ -56,8 +56,15 @@ class ORM_MPTT_Core extends ORM {
     $this->_lock();
 
     try {
+      // Make a hole in the parent for this new item
       $parent = ORM::factory($this->model_name, $parent_id);
-      $parent->_grow();
+      $this->db->query(
+        "UPDATE `{$this->table_name}` SET `left` = `left` + 2 WHERE `left` >= {$parent->right}");
+      $this->db->query(
+        "UPDATE `{$this->table_name}` SET `right` = `right` + 2 WHERE `right` >= {$parent->right}");
+      $parent->right += 2;
+
+      // Insert this item into the hole
       $this->left = $parent->right - 2;
       $this->right = $parent->right - 1;
       $this->parent_id = $parent->id;
@@ -73,11 +80,32 @@ class ORM_MPTT_Core extends ORM {
   }
 
   /**
-   * Deleted this node and adjust the left and right pointers
+   * Delete this node and all of its children.
    */
   public function delete() {
+    $children = $this->children();
+    if ($children) {
+      foreach ($this->children() as $item) {
+        $item->delete();
+      }
+
+      // Deleting children has affected this item
+      $this->reload();
+    }
+
+    $this->_lock();
+    try {
+      $this->db->query(
+        "UPDATE `{$this->table_name}` SET `left` = `left` - 2 WHERE `left` >= {$this->right}");
+      $this->db->query(
+        "UPDATE `{$this->table_name}` SET `right` = `right` - 2 WHERE `right` >= {$this->right}");
+    } catch (Exception $e) {
+      $this->_unlock();
+      throw $e;
+    }
+
+    $this->_unlock();
     parent::delete();
-    $this->_grow(-1);
   }
 
   /**
@@ -197,19 +225,6 @@ class ORM_MPTT_Core extends ORM {
     return parent::reload();
   }
 
-  /**
-   * Grow this node's space enough to make room for 1 or more new nodes.
-   *
-   * @param integer $count the number of new nodes to add
-   */
-  public function _grow($count=1) {
-    $size = $count * 2;
-    $this->db->query(
-      "UPDATE `{$this->table_name}` SET `left` = `left` + $size WHERE `left` >= {$this->right}");
-    $this->db->query(
-      "UPDATE `{$this->table_name}` SET `right` = `right` + $size WHERE `right` >= {$this->right}");
-    $this->right += 2;
-  }
 
   /**
    * Lock the tree to prevent concurrent modification.
diff --git a/core/tests/ORM_MPTT_Test.php b/core/tests/ORM_MPTT_Test.php
index e92f07fc..733b30ea 100644
--- a/core/tests/ORM_MPTT_Test.php
+++ b/core/tests/ORM_MPTT_Test.php
@@ -48,7 +48,7 @@ class ORM_MPTT_Test extends Unit_Test_Case {
     $album1_1_1 = ORM::factory("item")->add_to_parent($album1_1->id);
     $album1_1_2 = ORM::factory("item")->add_to_parent($album1_1->id);
 
-    $album1_1->reload()->delete();
+    $album1_1->delete();
     $album1->reload();
 
     // Now album1 contains only album1_2
@@ -56,19 +56,15 @@ class ORM_MPTT_Test extends Unit_Test_Case {
   }
 
   public function parent_test() {
-    $album = ORM::factory("item");
-    $album->add_to_parent(1);
+    $album = ORM::factory("item")->add_to_parent(1);
 
     $parent = ORM::factory("item", 1);
     $this->assert_equal($parent->id, $album->parent()->id);
   }
 
   public function parents_test() {
-    $outer = ORM::factory("item");
-    $outer->add_to_parent(1);
-
-    $inner = ORM::factory("item");
-    $inner->add_to_parent($outer->id);
+    $outer = ORM::factory("item")->add_to_parent(1);
+    $inner = ORM::factory("item")->add_to_parent($outer->id);
 
     $parent_ids = array();
     foreach ($inner->parents() as $parent) {
@@ -78,14 +74,9 @@ class ORM_MPTT_Test extends Unit_Test_Case {
   }
 
   public function children_test() {
-    $outer = ORM::factory("item");
-    $outer->add_to_parent(1);
-
-    $inner1 = ORM::factory("item");
-    $inner1->add_to_parent($outer->id);
-
-    $inner2 = ORM::factory("item");
-    $inner2->add_to_parent($outer->id);
+    $outer = ORM::factory("item")->add_to_parent(1);
+    $inner1 = ORM::factory("item")->add_to_parent($outer->id);
+    $inner2 = ORM::factory("item")->add_to_parent($outer->id);
 
     $child_ids = array();
     foreach ($outer->children() as $child) {
@@ -95,82 +86,76 @@ class ORM_MPTT_Test extends Unit_Test_Case {
   }
 
   public function children_limit_test() {
-    $outer = ORM::factory("item");
-    $outer->add_to_parent(1);
-
-    $inner1 = ORM::factory("item");
-    $inner1->add_to_parent($outer->id);
-
-    $inner2 = ORM::factory("item");
-    $inner2->add_to_parent($outer->id);
+    $outer = ORM::factory("item")->add_to_parent(1);
+    $inner1 = ORM::factory("item")->add_to_parent($outer->id);
+    $inner2 = ORM::factory("item")->add_to_parent($outer->id);
 
     $this->assert_equal(array($inner2->id => null), $outer->children(1, 1)->select_list('id'));
   }
 
   public function children_count_test() {
-    $outer = ORM::factory("item");
-    $outer->add_to_parent(1);
-
-    $inner1 = ORM::factory("item");
-    $inner1->add_to_parent($outer->id);
+    $outer = ORM::factory("item")->add_to_parent(1);
+    $inner1 = ORM::factory("item")->add_to_parent($outer->id);
+    $inner2 = ORM::factory("item")->add_to_parent($outer->id);
 
-    $inner2 = ORM::factory("item");
-    $inner2->add_to_parent($outer->id);
     $this->assert_equal(2, $outer->children_count());
   }
 
   public function descendant_test() {
-    $parent = album::create(1, "parent album", "parent album title");
-    photo::create($parent->id, DOCROOT . "themes/default/images/thumbnail.jpg", "photo1",
-                  "photo1", "parent album photo");
+    $parent = ORM::factory("item");
+    $parent->type = "album";
+    $parent->add_to_parent(1);
 
-    $album1 = album::create($parent->id, "album1", "album1 title");
-    photo::create($album1->id, DOCROOT . "themes/default/images/thumbnail.jpg", "photo1",
-                  "photo1", "album1 photo");
+    $photo = ORM::factory("item");
+    $photo->type = "photo";
+    $photo->add_to_parent($parent->id);
+
+    $album1 = ORM::factory("item");
+    $album1->type = "album";
+    $album1->add_to_parent($parent->id);
+
+    $photo1 = ORM::factory("item");
+    $photo1->type = "photo";
+    $photo1->add_to_parent($album1->id);
 
-    $album2 = album::create($parent->id, "album2", "album2 title");
-    photo::create($album2->id, DOCROOT . "themes/default/images/thumbnail.jpg", "photo2",
-                  "photo2", "album2 photo");
     $parent->reload();
 
-    $this->assert_equal(5, $parent->descendants()->count());
-    $this->assert_equal(3, $parent->descendants(null, 0, "photo")->count());
-    $this->assert_equal(2, $parent->descendants(null, 0, "album")->count());
+    $this->assert_equal(3, $parent->descendants()->count());
+    $this->assert_equal(2, $parent->descendants(null, 0, "photo")->count());
+    $this->assert_equal(1, $parent->descendants(null, 0, "album")->count());
   }
 
   public function descendant_limit_test() {
-    $parent = album::create(1, "parent album", "parent album title");
-    photo::create($parent->id, DOCROOT . "themes/default/images/thumbnail.jpg", "photo1",
-                  "photo1", "parent album photo");
-
-    $album1 = album::create($parent->id, "album1", "album1 title");
-    photo::create($album1->id, DOCROOT . "themes/default/images/thumbnail.jpg", "photo1",
-                  "photo1", "album1 photo");
+    $parent = ORM::factory("item")->add_to_parent(1);
+    $album1 = ORM::factory("item")->add_to_parent($parent->id);
+    $album2 = ORM::factory("item")->add_to_parent($parent->id);
+    $album3 = ORM::factory("item")->add_to_parent($parent->id);
 
-    $album2 = album::create($parent->id, "album2", "album2 title");
-    photo::create($album2->id, DOCROOT . "themes/default/images/thumbnail.jpg", "photo2",
-                  "photo2", "album2 photo");
     $parent->reload();
-
     $this->assert_equal(2, $parent->descendants(2)->count());
   }
 
   public function descendant_count_test() {
-    $parent = album::create(1, "parent album", "parent album title");
-    photo::create($parent->id, DOCROOT . "themes/default/images/thumbnail.jpg", "photo1",
-                  "photo1", "parent album photo");
+    $parent = ORM::factory("item");
+    $parent->type = "album";
+    $parent->add_to_parent(1);
+
+    $photo = ORM::factory("item");
+    $photo->type = "photo";
+    $photo->add_to_parent($parent->id);
+
+    $album1 = ORM::factory("item");
+    $album1->type = "album";
+    $album1->add_to_parent($parent->id);
 
-    $album1 = album::create($parent->id, "album1", "album1 title");
-    photo::create($album1->id, DOCROOT . "themes/default/images/thumbnail.jpg", "photo1",
-                  "photo1", "album1 photo");
+    $photo1 = ORM::factory("item");
+    $photo1->type = "photo";
+    $photo1->add_to_parent($album1->id);
 
-    $album2 = album::create($parent->id, "album2", "album2 title");
-    photo::create($album2->id, DOCROOT . "themes/default/images/thumbnail.jpg", "photo2",
-                  "photo2", "album2 photo");
     $parent->reload();
 
-    $this->assert_equal(5, $parent->descendants_count());
-    $this->assert_equal(3, $parent->descendants_count("photo"));
-    $this->assert_equal(2, $parent->descendants_count("album"));
+    $this->assert_equal(3, $parent->descendants_count());
+    $this->assert_equal(2, $parent->descendants_count("photo"));
+    $this->assert_equal(1, $parent->descendants_count("album"));
   }
 }