Changeset 1571

Timestamp:

21.02.2010 15:30:02 (7 months ago)

Author:

francois

Message:

[1.5][nested set] Slight getChildren() refactoring:

• Added internal cache for children
• Renamed getNumberOfChildren() to countChildren()
• Added addNestedSetChild() to allow population of internal cache

(refs #854)

Location:

branches/1.5

Files:

• docs/behavior/nested_set.txt (modified) (2 diffs)
• generator/lib/behavior/nestedset/NestedSetBehaviorObjectBuilderModifier.php (modified) (8 diffs)
• generator/lib/behavior/nestedset/NestedSetBehaviorPeerBuilderModifier.php (modified) (1 diff)
• generator/lib/builder/om/PHP5ObjectBuilder.php (modified) (1 diff)
• test/testsuite/generator/behavior/nestedset/NestedSetBehaviorObjectBuilderModifierTest.php (modified) (5 diffs)

branches/1.5/docs/behavior/nested_set.txt

@@ -73 +73 @@
 echo $s2->getLevel();    // 1
 echo $s2->hasChildren(); // true
-echo $s2->getNumberOfChildren(); // 1
+echo $s2->countChildren(); // 1
 echo $s2->hasSiblings(); // true
 }}}
@@ -290 +290 @@
 bool  hasNextSibling()
 bool  hasChildren()
-int   getNumberOfChildren()
+int   countChildren()
 int   getNumberOfDescendants()
 

branches/1.5/generator/lib/behavior/nestedset/NestedSetBehaviorObjectBuilderModifier.php

@@ -78 +78 @@
  */
 protected \$nestedSetQueries = array();
+
+/**
+ * Internal cache for children nodes
+ * @var        null|PropelObjectCollection
+ */
+protected \$nestedSetChildren = null;
 ";
         }
@@ -102 +108 @@
 $peerClassname::shiftRLValues(-2, \$this->getRightValue() + 1, null" . ($this->behavior->useScope() ? ", \$this->getScopeValue()" : "") . ", \$con);
 ";
+        }
+        
+        public function objectClearReferences($builder)
+        {
+                return "\$this->clearNestedSetChildren();";
         }
         
@@ -144 +155 @@
                 $this->addGetNextSibling($script);
                 
+                $this->addNestedSetChildrenClear($script);
+                $this->addNestedSetChildrenInit($script);
+                $this->addNestedSetChildAdd($script);
                 $this->addHasChildren($script);
                 $this->addGetChildren($script);
-                $this->addGetNumberOfChildren($script);
+                $this->addCountChildren($script);
+                
                 $this->addGetFirstChild($script);
                 $this->addGetLastChild($script);
@@ -571 +586 @@
 ";
         }
-        
+
+        protected function addNestedSetChildrenClear(&$script)
+        {
+                $script .= "
+/**
+ * Clears out the \$nestedSetChildren collection
+ *
+ * This does not modify the database; however, it will remove any associated objects, causing
+ * them to be refetched by subsequent calls to accessor method.
+ *
+ * @return     void
+ */
+public function clearNestedSetChildren()
+{
+        \$this->nestedSetChildren = null;
+}
+";
+        }
+
+        protected function addNestedSetChildrenInit(&$script)
+        {
+                $script .= "
+/**
+ * Initializes the \$nestedSetChildren collection.
+ *
+ * @return     void
+ */
+public function initNestedSetChildren()
+{
+        \$this->nestedSetChildren = new PropelObjectCollection();
+        \$this->nestedSetChildren->setModel('" . $this->builder->getNewStubObjectBuilder($this->table)->getClassname() . "');
+}
+";
+        }
+
+        protected function addNestedSetChildAdd(&$script)
+        {
+                $objectClassname = $this->objectClassname;
+                $objectName = '$' . $this->table->getStudlyPhpName();
+                $script .= "
+/**
+ * Adds an element to the internal \$nestedSetChildren collection.
+ * Beware that this doesn't insert a node in the tree.
+ * This method is only used to facilitate children hydration.
+ *
+ * @param      $objectClassname $objectName
+ *
+ * @return     void
+ */
+public function addNestedSetChild($objectName)
+{
+        if (\$this->nestedSetChildren === null) {
+                \$this->initNestedSetChildren();
+        }
+        if (!\$this->nestedSetChildren->contains($objectName)) { // only add it if the **same** object is not already associated
+                \$this->nestedSetChildren[]= $objectName;
+                // {$objectName}->setParent(\$this);
+        }
+}
+";
+        }
+        
         protected function addHasChildren(&$script)
         {
@@ -596 +672 @@
  * Gets the children of the given node
  *
- * @param      Criteria \$query Criteria to filter results.
+ * @param      Criteria  \$criteria Criteria to filter results.
  * @param      PropelPDO \$con Connection to use.
- * @return     array            List of $objectClassname objects
- */
-public function getChildren(\$query = null, PropelPDO \$con = null)
-{
-        if(\$this->isLeaf()) {
-                return array();
+ * @return     array     List of $objectClassname objects
+ */
+public function getChildren(\$criteria = null, PropelPDO \$con = null)
+{
+        if(null === \$this->nestedSetChildren || null !== \$criteria) {
+                if (\$this->isLeaf() || (\$this->isNew() && null === \$this->nestedSetChildren)) {
+                        // return empty collection
+                        \$this->initNestedSetChildren();
+                } else {
+                        \$nestedSetChildren = $queryClassname::create(null, \$criteria)
+                        ->childrenOf(\$this)
+                        ->orderByBranch()
+                                ->find(\$con);
+                        if (null !== \$criteria) {
+                                return \$nestedSetChildren;
+                        }
+                        \$this->nestedSetChildren = \$nestedSetChildren;
+                }
+        }
+        return \$this->nestedSetChildren;
+}
+";
+        }
+
+        protected function addCountChildren(&$script)
+        {
+                $objectClassname = $this->objectClassname;
+                $peerClassname = $this->peerClassname;
+                $queryClassname = $this->queryClassname;
+                $script .= "
+/**
+ * Gets number of children for the given node
+ *
+ * @param      Criteria  \$criteria Criteria to filter results. 
+ * @param      PropelPDO \$con Connection to use.
+ * @return     int       Number of children
+ */
+public function countChildren(\$criteria = null, PropelPDO \$con = null)
+{
+        if(null === \$this->nestedSetChildren || null !== \$criteria) {
+                if (\$this->isLeaf() || (\$this->isNew() && null === \$this->nestedSetChildren)) {
+                        return 0;
+                } else {
+                        return $queryClassname::create(null, \$criteria)
+                                ->childrenOf(\$this)
+                                ->count(\$con);
+                }
         } else {
-                return $queryClassname::create(null, \$query)
-                        ->childrenOf(\$this)
-                        ->orderByBranch()
-                        ->find(\$con);
-        }
-}
-";
-        }
-
-        protected function addGetNumberOfChildren(&$script)
-        {
-                $objectClassname = $this->objectClassname;
-                $peerClassname = $this->peerClassname;
-                $queryClassname = $this->queryClassname;
-                $script .= "
-/**
- * Gets number of children for the given node
- *
- * @param      Criteria \$query Criteria to filter results. 
- * @param      PropelPDO \$con Connection to use.
- * @return     int              Number of children
- */
-public function getNumberOfChildren(\$query = null, PropelPDO \$con = null)
-{
-        if(\$this->isLeaf()) {
-                return 0;
-        } else {
-                return $queryClassname::create(null, \$query)
-                        ->childrenOf(\$this)
-                        ->count(\$con);
+                return count(\$this->nestedSetChildren);
         }
 }
@@ -890 +978 @@
                 }
                 $script .= "
+        // update the children collection of the parent
+        \$parent->addNestedSetChild(\$this);
+        
         // Keep the tree modification query for the save() transaction
         \$this->nestedSetQueries []= array(
@@ -932 +1023 @@
                 }
                 $script .= "
+        // update the children collection of the parent
+        \$parent->addNestedSetChild(\$this);
+        
         // Keep the tree modification query for the save() transaction
         \$this->nestedSetQueries []= array(
@@ -1351 +1445 @@
 
 /**
+ * Alias for countChildren(), for BC with Propel 1.4 nested sets
+ *
+ * @deprecated since 1.5
+ * @see        setParent
+ */
+public function getNumberOfChildren(PropelPDO \$con = null)
+{
+        return \$this->countChildren(null, \$con);
+}
+
+
+/**
  * Alias for getPrevSibling(), for BC with Propel 1.4 nested sets
  *

branches/1.5/generator/lib/behavior/nestedset/NestedSetBehaviorPeerBuilderModifier.php

@@ -442 +442 @@
                         } else if ($col->getPhpName() == $this->getColumnPhpName('level_column')) {
                                 $script .= "
-                                        \$object->setLevel(\$row[$n]);";
+                                        \$object->setLevel(\$row[$n]);
+                                        \$object->clearNestedSetChildren();";
                         }
                         $n++;

branches/1.5/generator/lib/builder/om/PHP5ObjectBuilder.php

@@ -4124 +4124 @@
 ";
 
+                $this->applyBehaviorModifier('objectClearReferences', $script, "                ");
+                
                 foreach ($vars as $varName) {
                         $script .= "

branches/1.5/test/testsuite/generator/behavior/nestedset/NestedSetBehaviorObjectBuilderModifierTest.php

@@ -385 +385 @@
                        t6 t7
                 */
-                $this->assertEquals(array(), $t2->getChildren(), 'getChildren() returns an empty array for leafs');
-                $descendants = $t3->getChildren();
+        $this->assertTrue($t2->getChildren() instanceof PropelObjectCollection, 'getChildren() returns a collection');
+                $this->assertEquals(0, count($t2->getChildren()), 'getChildren() returns an empty collection for leafs');
+                $children = $t3->getChildren();
                 $expected = array(
                         't4' => array(5, 6, 2), 
                         't5' => array(7, 12, 2), 
                 );
-                $this->assertEquals($expected, $this->dumpNodes($descendants, true), 'getChildren() returns an array of children');
+                $this->assertEquals($expected, $this->dumpNodes($children, true), 'getChildren() returns a collection of children');
                 $c = new Criteria();
                 $c->add(Table9Peer::TITLE, 't5');
-                $descendants = $t3->getDescendants($c);
+                $children = $t3->getChildren($c);
                 $expected = array(
                         't5' => array(7, 12, 2), 
                 );
-                $this->assertEquals($expected, $this->dumpNodes($descendants, true), 'getChildren() accepts a criteria as parameter');
-        }
-        
-        public function testGetNumberOfChildren()
-        {
-                list($t1, $t2, $t3, $t4, $t5, $t6, $t7) = $this->initTree();
-                /* Tree used for tests
-                 t1
-                 |  \
-                 t2 t3
-                    |  \
-                    t4 t5
-                       |  \
-                       t6 t7
-                */
-                $this->assertEquals(0, $t2->getNumberOfChildren(), 'getNumberOfChildren() returns 0 for leafs');
-                $this->assertEquals(2, $t3->getNumberOfChildren(), 'getNumberOfChildren() returns the number of children');
+                $this->assertEquals($expected, $this->dumpNodes($children, true), 'getChildren() accepts a criteria as parameter');
+        }
+
+        public function testGetChildrenCache()
+        {
+                list($t1, $t2, $t3, $t4, $t5, $t6, $t7) = $this->initTree();
+                $con = Propel::getConnection();
+                $count = $con->getQueryCount();
+                $children = $t3->getChildren(null, $con);
+                $children = $t3->getChildren(null, $con);
+                $this->assertEquals($count + 1, $con->getQueryCount(), 'getChildren() only issues a query once');
+                $expected = array(
+                        't4' => array(5, 6, 2), 
+                        't5' => array(7, 12, 2), 
+                );
+                $this->assertEquals($expected, $this->dumpNodes($children, true), 'getChildren() returns a collection of children');
+                // when using criteria, cache is not used
                 $c = new Criteria();
                 $c->add(Table9Peer::TITLE, 't5');
-                $this->assertEquals(1, $t3->getNumberOfChildren($c), 'getNumberOfChildren() accepts a criteria as parameter');
+                $children = $t3->getChildren($c, $con);
+                $this->assertEquals($count + 2, $con->getQueryCount(), 'getChildren() issues a new query when âssed a non-null Criteria');
+                $expected = array(
+                        't5' => array(7, 12, 2), 
+                );
+                $this->assertEquals($expected, $this->dumpNodes($children, true), 'getChildren() accepts a criteria as parameter');
+                // but not erased either
+                $children = $t3->getChildren(null, $con);
+                $this->assertEquals($count + 2, $con->getQueryCount(), 'getChildren() keeps its internal cache after being called with a Criteria');
+                $expected = array(
+                        't4' => array(5, 6, 2), 
+                        't5' => array(7, 12, 2), 
+                );
+        $this->assertEquals($expected, $this->dumpNodes($children, true), 'getChildren() returns a collection of children');
+        }
+        
+        public function testCountChildren()
+        {
+                list($t1, $t2, $t3, $t4, $t5, $t6, $t7) = $this->initTree();
+                /* Tree used for tests
+                 t1
+                 |  \
+                 t2 t3
+                    |  \
+                    t4 t5
+                       |  \
+                       t6 t7
+                */
+                $this->assertEquals(0, $t2->countChildren(), 'countChildren() returns 0 for leafs');
+                $this->assertEquals(2, $t3->countChildren(), 'countChildren() returns the number of children');
+                $c = new Criteria();
+                $c->add(Table9Peer::TITLE, 't5');
+                $this->assertEquals(1, $t3->countChildren($c), 'countChildren() accepts a criteria as parameter');
+        }
+
+        public function testCountChildrenCache()
+        {
+                list($t1, $t2, $t3, $t4, $t5, $t6, $t7) = $this->initTree();
+                /* Tree used for tests
+                 t1
+                 |  \
+                 t2 t3
+                    |  \
+                    t4 t5
+                       |  \
+                       t6 t7
+                */
+                $con = Propel::getConnection();
+                $count = $con->getQueryCount();
+                $children = $t3->getChildren(null, $con);
+                $nbChildren = $t3->countChildren(null, $con);
+                $this->assertEquals($count + 1, $con->getQueryCount(), 'countChildren() uses the internal collection when passed no Criteria');
+                $nbChildren = $t3->countChildren(new Criteria(), $con);
+                $this->assertEquals($count + 2, $con->getQueryCount(), 'countChildren() issues a new query when passed a Criteria');
         }
         
@@ -847 +901 @@
         }
 
+        public function testMoveToFirstChildOfAndChildrenCache()
+        {
+                list($t1, $t2, $t3, $t4, $t5, $t6, $t7) = $this->initTree();
+                /* Tree used for tests
+                 t1
+                 |  \
+                 t2 t3
+                    |  \
+                    t4 t5
+                       |  \
+                       t6 t7
+                */
+                // fill children cache
+                $t3->getChildren();
+                $t1->getChildren();
+                // move
+                $t5->moveToFirstChildOf($t1);
+                $children = $t3->getChildren();
+                $expected = array(
+                        't4' => array(11, 12, 2), 
+                );
+                $this->assertEquals($expected, $this->dumpNodes($children, true), 'moveToFirstChildOf() reinitializes the child collection of all concerned nodes');
+                $children = $t1->getChildren();
+                $expected = array(
+                        't5' => array(2, 7, 1), 
+                        't2' => array(8, 9, 1), 
+                        't3' => array(10, 13, 1), 
+                );
+                $this->assertEquals($expected, $this->dumpNodes($children, true), 'moveToFirstChildOf() reinitializes the child collection of all concerned nodes');
+        }
+        
         public function testMoveToLastChildOf()
         {
@@ -907 +992 @@
         }
 
+        public function testMoveToLastChildOfAndChildrenCache()
+        {
+                list($t1, $t2, $t3, $t4, $t5, $t6, $t7) = $this->initTree();
+                /* Tree used for tests
+                 t1
+                 |  \
+                 t2 t3
+                    |  \
+                    t4 t5
+                       |  \
+                       t6 t7
+                */
+                // fill children cache
+                $t3->getChildren();
+                $t1->getChildren();
+                // move
+                $t5->moveToLastChildOf($t1);
+                $children = $t3->getChildren();
+                $expected = array(
+                        't4' => array(5, 6, 2), 
+                );
+                $this->assertEquals($expected, $this->dumpNodes($children, true), 'moveToLastChildOf() reinitializes the child collection of all concerned nodes');
+                $children = $t1->getChildren();
+                $expected = array(
+                        't2' => array(2, 3, 1), 
+                        't3' => array(4, 7, 1), 
+                        't5' => array(8, 13, 1), 
+                );
+                $this->assertEquals($expected, $this->dumpNodes($children, true), 'moveToLastChildOf() reinitializes the child collection of all concerned nodes');
+        }
+
         public function testMoveToPrevSiblingOf()
         {
@@ -995 +1111 @@
                 $this->assertEquals($expected, $this->dumpTree(), 'moveToPrevSiblingOf() moves the entire subtree at the same level correctly');
         }
-        
+
+        public function testMoveToPrevSiblingOfAndChildrenCache()
+        {
+                list($t1, $t2, $t3, $t4, $t5, $t6, $t7) = $this->initTree();
+                /* Tree used for tests
+                 t1
+                 |  \
+                 t2 t3
+                    |  \
+                    t4 t5
+                       |  \
+                       t6 t7
+                */
+                // fill children cache
+                $t3->getChildren();
+                $t1->getChildren();
+                // move
+                $t5->moveToPrevSiblingOf($t2);
+                $children = $t3->getChildren();
+                $expected = array(
+                        't4' => array(11, 12, 2), 
+                );
+                $this->assertEquals($expected, $this->dumpNodes($children, true), 'moveToPrevSiblingOf() reinitializes the child collection of all concerned nodes');
+                $children = $t1->getChildren();
+                $expected = array(
+                        't5' => array(2, 7, 1), 
+                        't2' => array(8, 9, 1), 
+                        't3' => array(10, 13, 1), 
+                );
+                $this->assertEquals($expected, $this->dumpNodes($children, true), 'moveToPrevSiblingOf() reinitializes the child collection of all concerned nodes');
+        }
+
+        public function testMoveToNextSiblingOfAndChildrenCache()
+        {
+                list($t1, $t2, $t3, $t4, $t5, $t6, $t7) = $this->initTree();
+                /* Tree used for tests
+                 t1
+                 |  \
+                 t2 t3
+                    |  \
+                    t4 t5
+                       |  \
+                       t6 t7
+                */
+                // fill children cache
+                $t3->getChildren();
+                $t1->getChildren();
+                // move
+                $t5->moveToNextSiblingOf($t3);
+                $children = $t3->getChildren();
+                $expected = array(
+                        't4' => array(5, 6, 2), 
+                );
+                $this->assertEquals($expected, $this->dumpNodes($children, true), 'moveToNextSiblingOf() reinitializes the child collection of all concerned nodes');
+                $children = $t1->getChildren();
+                $expected = array(
+                        't2' => array(2, 3, 1), 
+                        't3' => array(4, 7, 1), 
+                        't5' => array(8, 13, 1), 
+                );
+                $this->assertEquals($expected, $this->dumpNodes($children, true), 'moveToNextSiblingOf() reinitializes the child collection of all concerned nodes');
+        }
+
         public function testMoveToNextSiblingOf()
         {
@@ -1139 +1317 @@
         public function testCompatibilityProxies()
         {
-                $proxies = array('createRoot', 'retrieveParent', 'setParentNode', 'retrievePrevSibling', 'retrieveNextSibling', 'retrieveFirstChild', 'retrieveLastChild', 'getPath');
+                $proxies = array('createRoot', 'retrieveParent', 'setParentNode', 'getNumberOfChildren', 'retrievePrevSibling', 'retrieveNextSibling', 'retrieveFirstChild', 'retrieveLastChild', 'getPath');
                 foreach ($proxies as $method) {
                         $this->assertFalse(method_exists('Table9', $method), 'proxies are not enabled by default');