Changeset 4759

Timestamp:

08/08/08 19:51:36 (5 months ago)

Author:

romanb

Message:

Fixed #1126. Fixed #1165.

Location:

branches/1.0

Files:

• lib/Doctrine/Data/Import.php (modified) (1 diff)
• lib/Doctrine/Node/NestedSet.php (modified) (16 diffs)
• lib/Doctrine/Tree/NestedSet.php (modified) (7 diffs)
• tests/Data/ImportTestCase.php (modified) (2 diffs)
• tests/models/NestedSet_MultiRootNode.php (added)
• tests/NestedSet/MultiRootTestCase.php (added)
• tests/run.php (modified) (1 diff)
• tests/Ticket/1215TestCase.php (modified) (2 diffs)

branches/1.0/lib/Doctrine/Data/Import.php

@@ -311 +311 @@
 
             if( ! $parent) {
+                $record->save(); // save, so that createRoot can do: root id = id
                 Doctrine::getTable($model)->getTree()->createRoot($record);
             } else {

branches/1.0/lib/Doctrine/Node/NestedSet.php

@@ -367 +367 @@
                 $newLevel = $dest->getNode()->getLevel();
                 
-                // Make space for new node
-        $this->shiftRLValues($dest->getNode()->getRightValue() + 1, 2, $newRoot);
-        
-        // Slide child nodes over one and down one to allow new parent to wrap them
-                $componentName = $this->_tree->getBaseComponent();              
-        $q = new Doctrine_Query();
-        $q->update($componentName);
-        $q->set("$componentName.lft", "$componentName.lft + 1");
-        $q->set("$componentName.rgt", "$componentName.rgt + 1");
-        $q->set("$componentName.level", "$componentName.level + 1");
-        $q->where("$componentName.lft >= ? AND $componentName.rgt <= ?", array($newLeft, $newRight));
-                $q = $this->_tree->returnQueryWithRootId($q, $newRoot);
-                $q->execute();
-        
-        $this->record['level'] = $newLevel;
-                $this->insertNode($newLeft, $newRight, $newRoot);
+                $conn = $this->record->getTable()->getConnection();
+                try {
+                    $conn->beginInternalTransaction();
+                    
+                    // Make space for new node
+            $this->shiftRLValues($dest->getNode()->getRightValue() + 1, 2, $newRoot);
+
+            // Slide child nodes over one and down one to allow new parent to wrap them
+                $componentName = $this->_tree->getBaseComponent();              
+            $q = new Doctrine_Query();
+            $q->update($componentName);
+            $q->set("$componentName.lft", "$componentName.lft + 1");
+            $q->set("$componentName.rgt", "$componentName.rgt + 1");
+            $q->set("$componentName.level", "$componentName.level + 1");
+            $q->where("$componentName.lft >= ? AND $componentName.rgt <= ?", array($newLeft, $newRight));
+                $q = $this->_tree->returnQueryWithRootId($q, $newRoot);
+                $q->execute();
+
+            $this->record['level'] = $newLevel;
+                $this->insertNode($newLeft, $newRight, $newRoot);
+                
+                $conn->commit();
+                } catch (Exception $e) {
+                    $conn->rollback();
+                    throw $e;
+                }
         
         return true;
@@ -413 +423 @@
         $newRoot = $dest->getNode()->getRootValue();
         
-        $this->shiftRLValues($newLeft, 2, $newRoot);
-        $this->record['level'] = $dest['level'];
-        $this->insertNode($newLeft, $newRight, $newRoot);
-        // update destination left/right values to prevent a refresh
-        // $dest->getNode()->setLeftValue($dest->getNode()->getLeftValue() + 2);
-        // $dest->getNode()->setRightValue($dest->getNode()->getRightValue() + 2);
+        $conn = $this->record->getTable()->getConnection();
+        try {
+            $conn->beginInternalTransaction();
+            
+            $this->shiftRLValues($newLeft, 2, $newRoot);
+            $this->record['level'] = $dest['level'];
+            $this->insertNode($newLeft, $newRight, $newRoot);
+            // update destination left/right values to prevent a refresh
+            // $dest->getNode()->setLeftValue($dest->getNode()->getLeftValue() + 2);
+            // $dest->getNode()->setRightValue($dest->getNode()->getRightValue() + 2);
+            
+            $conn->commit();
+        } catch (Exception $e) {
+            $conn->rollback();
+            throw $e;
+        }
                         
         return true;
@@ -449 +469 @@
         $newRoot = $dest->getNode()->getRootValue();
 
-        $this->shiftRLValues($newLeft, 2, $newRoot);
-        $this->record['level'] = $dest['level'];
-        $this->insertNode($newLeft, $newRight, $newRoot);
-
-        // update destination left/right values to prevent a refresh
-        // no need, node not affected
+        $conn = $this->record->getTable()->getConnection();
+        try {
+            $conn->beginInternalTransaction();
+            
+            $this->shiftRLValues($newLeft, 2, $newRoot);
+            $this->record['level'] = $dest['level'];
+            $this->insertNode($newLeft, $newRight, $newRoot);
+            // update destination left/right values to prevent a refresh
+            // no need, node not affected
+            
+            $conn->commit();
+        } catch (Exception $e) {
+            $conn->rollback();
+            throw $e;
+        }
 
         return true;
@@ -485 +514 @@
         $newRoot = $dest->getNode()->getRootValue();
 
-        $this->shiftRLValues($newLeft, 2, $newRoot);
-        $this->record['level'] = $dest['level'] + 1;
-        $this->insertNode($newLeft, $newRight, $newRoot);
-        
-        // update destination left/right values to prevent a refresh
-        // $dest->getNode()->setRightValue($dest->getNode()->getRightValue() + 2);
+        $conn = $this->record->getTable()->getConnection();
+        try {
+            $conn->beginInternalTransaction();
+            
+            $this->shiftRLValues($newLeft, 2, $newRoot);
+            $this->record['level'] = $dest['level'] + 1;
+            $this->insertNode($newLeft, $newRight, $newRoot);
+            
+            // update destination left/right values to prevent a refresh
+            // $dest->getNode()->setRightValue($dest->getNode()->getRightValue() + 2);
+            
+            $conn->commit();
+        } catch (Exception $e) {
+            $conn->rollback();
+            throw $e;
+        }
 
         return true;
@@ -521 +560 @@
         $newRoot = $dest->getNode()->getRootValue();
 
-        $this->shiftRLValues($newLeft, 2, $newRoot);
-        $this->record['level'] = $dest['level'] + 1;
-        $this->insertNode($newLeft, $newRight, $newRoot);
-
-        // update destination left/right values to prevent a refresh
-        // $dest->getNode()->setRightValue($dest->getNode()->getRightValue() + 2);
+        $conn = $this->record->getTable()->getConnection();
+        try {
+            $conn->beginInternalTransaction();
+            
+            $this->shiftRLValues($newLeft, 2, $newRoot);
+            $this->record['level'] = $dest['level'] + 1;
+            $this->insertNode($newLeft, $newRight, $newRoot);
+
+            // update destination left/right values to prevent a refresh
+            // $dest->getNode()->setRightValue($dest->getNode()->getRightValue() + 2);
+            
+            $conn->commit();
+        } catch (Exception $e) {
+            $conn->rollback();
+            throw $e;
+        }
         
         return true;
@@ -545 +594 @@
             
         try {
-            $conn->beginTransaction();
+            $conn->beginInternalTransaction();
 
             // Move between trees: Detach from old tree & insert into new tree
@@ -751 +800 @@
         $oldLevel = $this->record['level'];
         
+        $conn = $this->record->getTable()->getConnection();
         try {
-            $conn = $this->record->getTable()->getConnection();
-            $conn->beginTransaction();
+            $conn->beginInternalTransaction();
             
             // Detach from old tree (close gap in old tree)
@@ -867 +916 @@
     {
         if ($record === null) {
-          return ($this->getRightValue() > $this->getLeftValue());
+            return ($this->getRightValue() > $this->getLeftValue());
         } else if ( $record instanceof Doctrine_Record ) {
-          return ($record->getNode()->getRightValue() > $record->getNode()->getLeftValue());
+            return ($record->getNode()->getRightValue() > $record->getNode()->getLeftValue());
         } else {
-          return false;
+            return false;
         }
     }
@@ -891 +940 @@
     public function delete()
     {
-        // TODO: add the setting whether or not to delete descendants or relocate children
-        $oldRoot = $this->getRootValue();
-        $q = $this->_tree->getBaseQuery();
-        
-        $baseAlias = $this->_tree->getBaseAlias();
-        $componentName = $this->_tree->getBaseComponent();
-
-        $q = $q->addWhere("$baseAlias.lft >= ? AND $baseAlias.rgt <= ?", array($this->getLeftValue(), $this->getRightValue()));
-
-        $q = $this->_tree->returnQueryWithRootId($q, $oldRoot);
-        
-        $coll = $q->execute();
-
-        $coll->delete();
-
-        $first = $this->getRightValue() + 1;
-        $delta = $this->getLeftValue() - $this->getRightValue() - 1;
-        $this->shiftRLValues($first, $delta, $oldRoot);
+        $conn = $this->record->getTable()->getConnection();
+        try {
+            $conn->beginInternalTransaction();
+            
+            // TODO: add the setting whether or not to delete descendants or relocate children
+            $oldRoot = $this->getRootValue();
+            $q = $this->_tree->getBaseQuery();
+
+            $baseAlias = $this->_tree->getBaseAlias();
+            $componentName = $this->_tree->getBaseComponent();
+
+            $q = $q->addWhere("$baseAlias.lft >= ? AND $baseAlias.rgt <= ?", array($this->getLeftValue(), $this->getRightValue()));
+
+            $q = $this->_tree->returnQueryWithRootId($q, $oldRoot);
+
+            $coll = $q->execute();
+
+            $coll->delete();
+
+            $first = $this->getRightValue() + 1;
+            $delta = $this->getLeftValue() - $this->getRightValue() - 1;
+            $this->shiftRLValues($first, $delta, $oldRoot);
+            
+            $conn->commit();
+        } catch (Exception $e) {
+            $conn->rollback();
+            throw $e;
+        }
         
         return true; 
@@ -942 +1001 @@
         $treeSize = $right - $left + 1;
 
-        // Make room in the new branch
-        $this->shiftRLValues($destLeft, $treeSize, $rootId);
-
-        if ($left >= $destLeft) { // src was shifted too?
-            $left += $treeSize;
-            $right += $treeSize;
-        }
-
-        // update level for descendants
-        $q = new Doctrine_Query();
-        $q = $q->update($componentName)
-                ->set($componentName . '.level', 'level + ?')
-                ->where($componentName . '.lft > ? AND ' . $componentName . '.rgt < ?',
-                        array($levelDiff, $left, $right));
-        $q = $this->_tree->returnQueryWithRootId($q, $rootId);
-        $q->execute();
-        
-        // now there's enough room next to target to move the subtree
-        $this->shiftRLRange($left, $right, $destLeft - $left, $rootId);
-
-        // correct values after source (close gap in old tree)
-        $this->shiftRLValues($right + 1, -$treeSize, $rootId);
-
-        $this->record->save();
-        $this->record->refresh();
+        $conn = $this->record->getTable()->getConnection();
+        try {
+            $conn->beginInternalTransaction();
+            
+            // Make room in the new branch
+            $this->shiftRLValues($destLeft, $treeSize, $rootId);
+
+            if ($left >= $destLeft) { // src was shifted too?
+                $left += $treeSize;
+                $right += $treeSize;
+            }
+
+            // update level for descendants
+            $q = new Doctrine_Query();
+            $q = $q->update($componentName)
+                    ->set($componentName . '.level', 'level + ?')
+                    ->where($componentName . '.lft > ? AND ' . $componentName . '.rgt < ?',
+                            array($levelDiff, $left, $right));
+            $q = $this->_tree->returnQueryWithRootId($q, $rootId);
+            $q->execute();
+
+            // now there's enough room next to target to move the subtree
+            $this->shiftRLRange($left, $right, $destLeft - $left, $rootId);
+
+            // correct values after source (close gap in old tree)
+            $this->shiftRLValues($right + 1, -$treeSize, $rootId);
+
+            $this->record->save();
+            $this->record->refresh();
+            
+            $conn->commit();
+        } catch (Exception $e) {
+            $conn->rollback();
+            throw $e;
+        }
         
         return true;
@@ -973 +1042 @@
     /**
      * adds '$delta' to all Left and Right values that are >= '$first'. '$delta' can also be negative.
+     *
+     * Note: This method does wrap its database queries in a transaction. This should be done
+     * by the invoking code.
      *
      * @param int $first         First node to be shifted
@@ -985 +1057 @@
         $componentName = $this->_tree->getBaseComponent();
         $qLeft = $qLeft->update($componentName)
-                                ->set($componentName . '.lft', 'lft + ?')
-                                ->where($componentName . '.lft >= ?', array($delta, $first));
+                ->set($componentName . '.lft', 'lft + ?')
+                ->where($componentName . '.lft >= ?', array($delta, $first));
         
         $qLeft = $this->_tree->returnQueryWithRootId($qLeft, $rootId);
@@ -994 +1066 @@
         // shift right columns
         $resultRight = $qRight->update($componentName)
-                                ->set($componentName . '.rgt', 'rgt + ?')
-                                ->where($componentName . '.rgt >= ?', array($delta, $first));
+                ->set($componentName . '.rgt', 'rgt + ?')
+                ->where($componentName . '.rgt >= ?', array($delta, $first));
 
         $qRight = $this->_tree->returnQueryWithRootId($qRight, $rootId);
@@ -1005 +1077 @@
      * adds '$delta' to all Left and Right values that are >= '$first' and <= '$last'. 
      * '$delta' can also be negative.
+     *
+     * Note: This method does wrap its database queries in a transaction. This should be done
+     * by the invoking code.
      *
      * @param int $first     First node to be shifted (L value)
@@ -1018 +1093 @@
         $componentName = $this->_tree->getBaseComponent();
         $qLeft = $qLeft->update($componentName)
-                                ->set($componentName . '.lft', 'lft + ?')
-                                ->where($componentName . '.lft >= ? AND ' . $componentName . '.lft <= ?', array($delta, $first, $last));
+                ->set($componentName . '.lft', 'lft + ?')
+                ->where($componentName . '.lft >= ? AND ' . $componentName . '.lft <= ?', array($delta, $first, $last));
         
         $qLeft = $this->_tree->returnQueryWithRootId($qLeft, $rootId);
@@ -1027 +1102 @@
         // shift right column values
         $qRight = $qRight->update($componentName)
-                                ->set($componentName . '.rgt', 'rgt + ?')
-                                ->where($componentName . '.rgt >= ? AND ' . $componentName . '.rgt <= ?', array($delta, $first, $last));
+                ->set($componentName . '.rgt', 'rgt + ?')
+                ->where($componentName . '.rgt >= ? AND ' . $componentName . '.rgt <= ?', array($delta, $first, $last));
 
         $qRight = $this->_tree->returnQueryWithRootId($qRight, $rootId);

branches/1.0/lib/Doctrine/Tree/NestedSet.php

@@ -30 +30 @@
  * @version     $Revision$
  * @author      Joe Simms <joe.simms@websites4.com>
+ * @author      Roman Borschel <roman@code-factory.org>
  */
 class Doctrine_Tree_NestedSet extends Doctrine_Tree implements Doctrine_Tree_Interface
@@ -70 +71 @@
 
     /**
-     * creates root node from given record or from a new record
+     * Creates root node from given record or from a new record.
+     *
+     * Note: When using a tree with multiple root nodes (hasManyRoots), you MUST pass in a
+     * record to use as the root. This can either be a new/transient record that already has
+     * the root id column set to some numeric value OR a persistent record. In the latter case
+     * the records id will be assigned to the root id. You must use numeric columns for the id
+     * and root id columns.
      *
      * @param object $record        instance of Doctrine_Record
@@ -76 +83 @@
     public function createRoot(Doctrine_Record $record = null)
     {
+        if ($this->getAttribute('hasManyRoots')) {
+            if ( ! $record || ( ! $record->exists() && $record->getNode()->getRootValue() <= 0)
+                    || $record->getTable()->isIdentifierComposite()) {
+                throw new Doctrine_Tree_Exception("Node must have a root id set or must "
+                        . " be persistent and have a single-valued numeric primary key in order to"
+                        . " be created as a root node. Automatic assignment of a root id on"
+                        . " transient/new records is no longer supported.");
+            }
+            
+            if ($record->exists() && $record->getNode()->getRootValue() <= 0) {
+                // Default: root_id = id
+                $identifier = $record->getTable()->getIdentifier();
+                $record->getNode()->setRootValue($record->get($identifier));
+            }
+        }
+        
         if ( ! $record) {
             $record = $this->table->create();
-        }
-
-        // if tree is many roots, and no root id has been set, then get next root id
-        if ($root = $this->getAttribute('hasManyRoots') && $record->getNode()->getRootValue() <= 0) {
-            $record->getNode()->setRootValue($this->getNextRootId());
         }
 
@@ -109 +127 @@
      *
      * @param integer $rootId
+     * @todo Better $rootid = null and exception if $rootId == null && hasManyRoots?
+     *       Fetching with id = 1 is too magical and cant work reliably anyway.
      */
     public function fetchRoot($rootId = 1)
@@ -214 +234 @@
      *
      * @return integer
+     * @deprecated THIS METHOD IS DEPRECATED. ROOT IDS ARE NO LONGER AUTOMATICALLY GENERATED.
+     *             ROOT ID MUST BE ASSIGNED MANUALLY OR USING THE DEFAULT BEHAVIOR WHERE
+     *             ROOT_ID = ID. SEE createRoot() FOR DETAILS.
      */
     public function getNextRootId()
@@ -224 +247 @@
      *
      * @return integer
+     * @deprecated THIS METHOD IS DEPRECATED. ROOT IDS ARE NO LONGER AUTOMATICALLY GENERATED.
+     *             ROOT ID MUST BE ASSIGNED MANUALLY OR USING THE DEFAULT BEHAVIOR WHERE
+     *             ROOT_ID = ID. SEE createRoot() FOR DETAILS.
      */
     public function getMaxRootId()
     {
         $component = $this->table->getComponentName();
-        $column    = $this->getAttribute('rootColumnName');
+        $column = $this->getAttribute('rootColumnName');
 
         // cannot get this dql to work, cannot retrieve result using $coll[0]->max
@@ -326 +352 @@
         $this->_baseQuery = $this->_createBaseQuery();
     }
-
-    /**
-     * Enter description here...
-     *
-     * @param unknown_type $graph
-     */
-    /*
-    public function computeLevels($tree)
-    {
-        $right = array();
-        $isArray = is_array($tree);
-        $rootColumnName = $this->getAttribute('rootColumnName');
-
-        for ($i = 0, $count = count($tree); $i < $count; $i++) {
-            if ($rootColumnName && $i > 0 && $tree[$i][$rootColumnName] != $tree[$i-1][$rootColumnName]) {
-                $right = array();
-            }
-
-            if (count($right) > 0) {
-                while (count($right) > 0 && $right[count($right)-1] < $tree[$i]['rgt']) {
-                    //echo count($right);
-                    array_pop($right);
-                }
-            }
-
-            if ($isArray) {
-                $tree[$i]['level'] = count($right);
-            } else {
-                $tree[$i]->getNode()->setLevel(count($right));
-            }
-
-            $right[] = $tree[$i]['rgt'];
-        }
-        return $tree;
-    }
-    */
 }

branches/1.0/tests/Data/ImportTestCase.php

@@ -309 +309 @@
             $this->assertEqual($i[0]['rgt'], 6);
             $this->assertEqual($i[0]['level'], 0);
-            $this->assertEqual($i[0]['root_id'], '1');
+            $this->assertEqual($i[0]['root_id'], $i[0]['id']);
 
             $this->assertEqual($i[1]['name'], 'Item 1.1');
@@ -321 +321 @@
             $this->assertEqual($i[3]['rgt'], 12);
             $this->assertEqual($i[3]['level'], 0);
-            $this->assertEqual($i[3]['root_id'], '2');
+            $this->assertEqual($i[3]['root_id'], $i[3]['id']);
 
             $this->assertEqual($i[4]['name'], 'Item 2.1');

branches/1.0/tests/run.php

@@ -373 +373 @@
 $unsorted->addTestCase(new Doctrine_Template_TestCase());
 $unsorted->addTestCase(new Doctrine_NestedSet_SingleRoot_TestCase());
+$unsorted->addTestCase(new Doctrine_NestedSet_MultiRoot_TestCase());
 $unsorted->addTestCase(new Doctrine_PessimisticLocking_TestCase());
 $test->addTestCase($unsorted);

branches/1.0/tests/Ticket/1215TestCase.php

@@ -39 +39 @@
     }
     
+    /*
+    TEST NO LONGER VALID AS DOCTRINE-GENERATED ROOT IDS ARE NO LONGER SUPPORTED.
+    SEE Doctrine_Tree_NestedSet#createRoot(), Doctrine_Tree_NestedSet#getNextRootId() AND
+    Doctrine_Tree_NestedSet#getMaxRootId() FOR DETAILS.
+    
     public function prepareData()
     {
@@ -65 +70 @@
         $tree = Doctrine::getTable('Ticket_1215_TreeManyRoots')->getTree();
         $this->assertEqual($tree->getMaxRootId(), 2);
-    }
+    }*/
 }