Zend_Navigation_Container::getChildren() returns null after recursive iteration

[huyi1985]

<?php
require 'load_zf.php';

$container = new Zend_Navigation(array(
    array(
        'label' => 'Page 1',
        'uri' => '/page1'
    ),
    array(
        'label' => 'Page 2',
        'uri' => '/page2',
        'pages' => array(
            array(
                'label' => 'Page 2.1',
                'uri' => '/page2-1',
            ),
            array(
                'label' => 'Page 2.2',
                'uri' => '/page2-2',
            ),
        )
    )
));

function doRecursiveIteration(Zend_Navigation_Container $container) {
    $iterator = new RecursiveIteratorIterator($container,
                            RecursiveIteratorIterator::SELF_FIRST);
    foreach ($iterator as $page) {}
}

doRecursiveIteration($container);

foreach ($container as $_page) {
    echo $_page->getLabel(), ' has ', $_page->count(), ' sub pages. ';
    if ($_page->hasChildren()) {
        assert($_page->getChildren() !== null);
        echo 'The first sub page is ', $_page->getChildren()->getLabel();
    } else {

    }
    echo PHP_EOL;
}

Output:

Page 1 has 0 sub pages.
Page 2 has 2 sub pages.
Warning: assert(): Assertion failed in E:\www\zend\zend_navigation_getChildren.php on line 36

It is because Zend_Navigation_Container uses $_index property to implement RecursiveIterator interface, and after a recursive iteration, the internal pointer points beyond the end of the $_index. key($this->_index) returns null.

    /**
     * Returns the child container.
     *
     * Implements RecursiveIterator interface.
     *
     * @return Zend_Navigation_Page|null
     */
    public function getChildren()
    {
        $hash = key($this->_index);

        if (isset($this->_pages[$hash])) {
            return $this->_pages[$hash];
        }

        return null;
    }

I think use this method can avoid the problem.

$keys = array_keys($this->_index);
$hash = isset($keys[0]) ? $keys[0] : null;

[froschdesign]

Maybe related: zendframework/zendframework#4517

[froschdesign]

@huyi1985

$keys = array_keys($this->_index);
$hash = isset($keys[0]) ? $keys[0] : null;

This will break existing unit tests:

• Zend_Navigation_ContainerTest::testRecursiveIteration
• Zend_Navigation_ContainerTest::testFindByWithRegex
• Zend_Navigation_ContainerTest::testFindAllByWithRegex
• Zend_Navigation_ContainerTest::testFindAllWithinArrayProperties
• Zend_Navigation_ContainerTest::testFindAllWithRegexWithinArrayProperties
• Zend_Navigation_ContainerTest::testFindAllByMagicLowercaseFindsBothNativeAndCustomProps

[huyi1985]

@froschdesign

I misunderstood the usage of getChildren(). It returns an iterator for the current iterator entry, not an array, containing children of current element. So maybe this issue is not a bug, we CAN NOT use hasChildren() + getChildren() to implement iteration, unless we call rewind() + next() manually. hasChildren() + getChildren() suit RecursiveIteratorIterator, not friendly with foreach

When someone wants to iterate a Navigation(Container), I think the 3 methods below are appropriate.

// 1. use the RecursiveIteratorIterator class
$mode = RecursiveIteratorIterator::SELF_FIRST;
// $mode = RecursiveIteratorIterator::LEAVES_ONLY;
// $mode = RecursiveIteratorIterator::CHILD_FIRST;
$iterator = new RecursiveIteratorIterator($container, $mode);
foreach ($iterator as $page) {
    echo $page->getLabel(), PHP_EOL;
}

Using RecursiveIteratorIterator::LEAVES_ONLY result in echo nothing, but I think this is acceptable.

// 2. use custom function and Zend_Navigation_Container::getPages()
function showLabels1(Zend_Navigation_Page $page) {
    echo $page->getLabel(), PHP_EOL;
    if ($page->hasPages()) {            // $page->hasChildren() also OK
        $subPages = $page->getPages();  // CANNOT use $page->getChildren()
        foreach ($subPages as $subPage) {
            showLabels1($subPage);
        }
    }
}

foreach ($container as $page) {
    showLabels1($page);
}

// 3. use custom function and Zend_Navigation_Container::getChildren()
function showLabels2(Zend_Navigation_Page $page) {
    echo $page->getLabel(), PHP_EOL;
    if ($page->hasChildren()) {
        $page->rewind();
        while($subPage = $page->getChildren()) {
            showLabels2($subPage);
            $page->next();
        }
    }
}

foreach ($container as $page) {
    showLabels2($page);
}

[froschdesign]

It returns an iterator for the current iterator entry, not an array, containing children of current element.

Right.