ZF-8262

Removed dependency on calling Zend_Ldap_Collection::rewind() before using Zend_Ldap_Collection::current() and/or Zend_Ldap_Collection::key(). Zend_Ldap_Collection::rewind() will be called internally if iteration has not been started yet.

Added Zend_Ldap_Collection::getInnerIterator() to return the underlying Zend_Ldap_Collection_Iterator_Default.

Added Zend_Ldap_Collection::dn() to accompany Zend_Ldap_Collection::key(). The latter returns the numerical index of the current result item within the result set, the former returns its DN.

git-svn-id: http://framework.zend.com/svn/framework/standard/trunk@18921 44c647ce-9c0f-0410-b52a-842ac1e357ba

library/Zend/Ldap/Collection.php

@@ -41,7 +41,7 @@ class Zend_Ldap_Collection implements Iterator, Countable
      *
      * @var integer
      */
-    protected $_currentNumber = -1;
+    protected $_current = -1;
 
     /**
      * Container for item caching to speed up multiple iterations
@@ -99,8 +99,9 @@ class Zend_Ldap_Collection implements Iterator, Countable
         if ($this->count() > 0) {
             $this->rewind();
             return $this->current();
+        } else {
+            return null;
         }
-        else return null;
     }
 
     /**
@@ -128,17 +129,26 @@ class Zend_Ldap_Collection implements Iterator, Countable
      * Return the current result item
      * Implements Iterator
      *
-     * @return array
+     * @return array|null
      * @throws Zend_Ldap_Exception
      */
     public function current()
     {
-        if (!array_key_exists($this->_currentNumber, $this->_cache))
-        {
-            $this->_cache[$this->_currentNumber] =
-                $this->_createEntry($this->_iterator->current());
+        if ($this->count() > 0) {
+            if ($this->_current < 0) {
+                $this->rewind();
+            }
+            if (!array_key_exists($this->_current, $this->_cache)) {
+                $current = $this->_iterator->current();
+                if ($current === null) {
+                    return null;
+                }
+                $this->_cache[$this->_current] = $this->_createEntry($current);
+            }
+            return $this->_cache[$this->_current];
+        } else {
+            return null;
         }
-        return $this->_cache[$this->_currentNumber];
     }
 
     /**
@@ -153,14 +163,38 @@ class Zend_Ldap_Collection implements Iterator, Countable
     }
 
     /**
-     * Return the result item key
+     * Return the current result item DN
+     *
+     * @return string|null
+     */
+    public function dn()
+    {
+        if ($this->count() > 0) {
+            if ($this->_current < 0) {
+                $this->rewind();
+            }
+            return $this->_iterator->key();
+        } else {
+            return null;
+        }
+    }
+
+    /**
+     * Return the current result item key
      * Implements Iterator
      *
-     * @return int
+     * @return int|null
      */
     public function key()
     {
-        return $this->_currentNumber;
+        if ($this->count() > 0) {
+            if ($this->_current < 0) {
+                $this->rewind();
+            }
+            return $this->_current;
+        } else {
+            return null;
+        }
     }
 
     /**
@@ -172,7 +206,7 @@ class Zend_Ldap_Collection implements Iterator, Countable
     public function next()
     {
         $this->_iterator->next();
-        $this->_currentNumber++;
+        $this->_current++;
     }
 
     /**
@@ -184,7 +218,7 @@ class Zend_Ldap_Collection implements Iterator, Countable
     public function rewind()
     {
         $this->_iterator->rewind();
-        $this->_currentNumber = 0;
+        $this->_current = 0;
     }
 
     /**
@@ -196,7 +230,7 @@ class Zend_Ldap_Collection implements Iterator, Countable
      */
     public function valid()
     {
-        if (isset($this->_cache[$this->_currentNumber])) {
+        if (isset($this->_cache[$this->_current])) {
             return true;
         } else {
             return $this->_iterator->valid();

library/Zend/Ldap/Collection/Iterator/Default.php

@@ -52,13 +52,6 @@ class Zend_Ldap_Collection_Iterator_Default implements Iterator, Countable
     protected $_current = null;
 
     /**
-     * Current result entry DN
-     *
-     * @var string
-     */
-    protected $_currentDn = null;
-
-    /**
      * Number of items in query result
      *
      * @var integer
@@ -102,7 +95,6 @@ class Zend_Ldap_Collection_Iterator_Default implements Iterator, Countable
         if (is_resource($this->_resultId)) {
              $isClosed = @ldap_free_result($this->_resultId);
              $this->_resultId = null;
-             $this->_currentDn = null;
              $this->_current = null;
         }
         return $isClosed;
@@ -133,19 +125,23 @@ class Zend_Ldap_Collection_Iterator_Default implements Iterator, Countable
      * Return the current result item
      * Implements Iterator
      *
-     * @return array
+     * @return array|null
      * @throws Zend_Ldap_Exception
      */
     public function current()
     {
-        if (!is_resource($this->_current) || !is_string($this->_currentDn)) return null;
+        if (!is_resource($this->_current)) {
+            $this->rewind();
+        }
+        if (!is_resource($this->_current)) {
+            return null;
+        }
 
-        $entry = array('dn' => $this->_currentDn);
+        $entry = array('dn' => $this->key());
         $ber_identifier = null;
         $name = @ldap_first_attribute($this->_ldap->getResource(), $this->_current,
             $ber_identifier);
-        while ($name)
-        {
+        while ($name) {
             $data = @ldap_get_values_len($this->_ldap->getResource(), $this->_current, $name);
             unset($data['count']);
             $entry[strtolower($name)] = $data;
@@ -160,11 +156,24 @@ class Zend_Ldap_Collection_Iterator_Default implements Iterator, Countable
      * Return the result item key
      * Implements Iterator
      *
-     * @return int
+     * @return string|null
      */
     public function key()
     {
-        return $this->_currentDn;
+        if (!is_resource($this->_current)) {
+            $this->rewind();
+        }
+        if (is_resource($this->_current)) {
+            $currentDn = @ldap_get_dn($this->_ldap->getResource(), $this->_current);
+            if ($currentDn === false) {
+                /** @see Zend_Ldap_Exception */
+                require_once 'Zend/Ldap/Exception.php';
+                throw new Zend_Ldap_Exception($this->_ldap, 'getting dn');
+            }
+            return $currentDn;
+        } else {
+            return null;
+        }
     }
 
     /**
@@ -175,22 +184,20 @@ class Zend_Ldap_Collection_Iterator_Default implements Iterator, Countable
      */
     public function next()
     {
-        if (!is_resource($this->_current)) return;
-        $this->_current = @ldap_next_entry($this->_ldap->getResource(), $this->_current);
-        /**
-         * @see Zend_Ldap_Exception
-         */
-        require_once 'Zend/Ldap/Exception.php';
-        if ($this->_current === false) {
-            $msg = $this->_ldap->getLastError($code);
-            if ($code === Zend_Ldap_Exception::LDAP_SIZELIMIT_EXCEEDED) {
-                // we have reached the size limit enforced by the server
-                return;
-            } else if ($code > Zend_Ldap_Exception::LDAP_SUCCESS) {
-                 throw new Zend_Ldap_Exception($this->_ldap, 'getting next entry (' . $msg . ')');
+        if (is_resource($this->_current)) {
+            $this->_current = @ldap_next_entry($this->_ldap->getResource(), $this->_current);
+            /** @see Zend_Ldap_Exception */
+            require_once 'Zend/Ldap/Exception.php';
+            if ($this->_current === false) {
+                $msg = $this->_ldap->getLastError($code);
+                if ($code === Zend_Ldap_Exception::LDAP_SIZELIMIT_EXCEEDED) {
+                    // we have reached the size limit enforced by the server
+                    return;
+                } else if ($code > Zend_Ldap_Exception::LDAP_SUCCESS) {
+                     throw new Zend_Ldap_Exception($this->_ldap, 'getting next entry (' . $msg . ')');
+                }
             }
         }
-        $this->_storeCurrentDn();
     }
 
     /**
@@ -201,35 +208,14 @@ class Zend_Ldap_Collection_Iterator_Default implements Iterator, Countable
      */
     public function rewind()
     {
-        if (!is_resource($this->_resultId)) return;
-        $this->_current = @ldap_first_entry($this->_ldap->getResource(), $this->_resultId);
-        /**
-         * @see Zend_Ldap_Exception
-         */
-        require_once 'Zend/Ldap/Exception.php';
-        if ($this->_current === false &&
-                $this->_ldap->getLastErrorCode() > Zend_Ldap_Exception::LDAP_SUCCESS) {
-            throw new Zend_Ldap_Exception($this->_ldap, 'getting first entry');
-        }
-
-        $this->_storeCurrentDn();
-    }
-
-    /**
-     * Stores the current DN
-     *
-     * @return void
-     * @throws Zend_Ldap_Exception
-     */
-    protected function _storeCurrentDn()
-    {
-        if (is_resource($this->_current)) {
-            $this->_currentDn = @ldap_get_dn($this->_ldap->getResource(), $this->_current);
-            if ($this->_currentDn === false) {
-                throw new Zend_Ldap_Exception($this->_ldap, 'getting dn');
+        if (is_resource($this->_resultId)) {
+            $this->_current = @ldap_first_entry($this->_ldap->getResource(), $this->_resultId);
+            /** @see Zend_Ldap_Exception */
+            require_once 'Zend/Ldap/Exception.php';
+            if ($this->_current === false &&
+                    $this->_ldap->getLastErrorCode() > Zend_Ldap_Exception::LDAP_SUCCESS) {
+                throw new Zend_Ldap_Exception($this->_ldap, 'getting first entry');
             }
-        } else {
-            $this->_currentDn = null;
         }
     }
 
@@ -242,7 +228,7 @@ class Zend_Ldap_Collection_Iterator_Default implements Iterator, Countable
      */
     public function valid()
     {
-        return (is_resource($this->_current) && is_string($this->_currentDn));
+        return (is_resource($this->_current));
     }
 
 }

tests/Zend/Ldap/SearchTest.php

@@ -438,6 +438,88 @@ class Zend_Ldap_SearchTest extends Zend_Ldap_OnlineTestCase
         $entry = $ldap->getEntry($dn);
         $this->assertEquals($dn, $entry['dn']);
     }
+
+    public function testInnerIteratorIsOfRequiredType()
+    {
+        $items = $this->_getLdap()->search('(objectClass=organizationalUnit)',
+            TESTS_ZEND_LDAP_WRITEABLE_SUBTREE, Zend_Ldap::SEARCH_SCOPE_SUB);
+        $this->assertType('Zend_Ldap_Collection_Iterator_Default', $items->getInnerIterator());
+    }
+
+    /**
+     * @group ZF-8262
+     */
+    public function testCallingCurrentOnIteratorReturnsFirstElement()
+    {
+        $items = $this->_getLdap()->search('(objectClass=organizationalUnit)',
+            TESTS_ZEND_LDAP_WRITEABLE_SUBTREE, Zend_Ldap::SEARCH_SCOPE_SUB);
+        $this->assertEquals(TESTS_ZEND_LDAP_WRITEABLE_SUBTREE, $items->getInnerIterator()->key());
+        $current = $items->getInnerIterator()->current();
+        $this->assertType('array', $current);
+        $this->assertEquals(TESTS_ZEND_LDAP_WRITEABLE_SUBTREE, $current['dn']);
+    }
+
+    /**
+     * @group ZF-8262
+     */
+    public function testCallingCurrentOnCollectionReturnsFirstElement()
+    {
+        $items = $this->_getLdap()->search('(objectClass=organizationalUnit)',
+            TESTS_ZEND_LDAP_WRITEABLE_SUBTREE, Zend_Ldap::SEARCH_SCOPE_SUB);
+        $this->assertEquals(0, $items->key());
+        $this->assertEquals(TESTS_ZEND_LDAP_WRITEABLE_SUBTREE, $items->dn());
+        $current = $items->current();
+        $this->assertType('array', $current);
+        $this->assertEquals(TESTS_ZEND_LDAP_WRITEABLE_SUBTREE, $current['dn']);
+    }
+
+    /**
+     * @group ZF-8262
+     */
+    public function testCallingCurrentOnEmptyIteratorReturnsNull()
+    {
+        $items = $this->_getLdap()->search('(objectClass=account)', TESTS_ZEND_LDAP_WRITEABLE_SUBTREE,
+            Zend_Ldap::SEARCH_SCOPE_SUB);
+        $this->assertNull($items->getInnerIterator()->key());
+        $this->assertNull($items->getInnerIterator()->current());
+    }
+
+    /**
+     * @group ZF-8262
+     */
+    public function testCallingCurrentOnEmptyCollectionReturnsNull()
+    {
+        $items = $this->_getLdap()->search('(objectClass=account)', TESTS_ZEND_LDAP_WRITEABLE_SUBTREE,
+            Zend_Ldap::SEARCH_SCOPE_SUB);
+        $this->assertNull($items->key());
+        $this->assertNull($items->dn());
+        $this->assertNull($items->current());
+    }
+
+    /**
+     * @group ZF-8262
+     */
+    public function testResultIterationAfterCallingCurrent()
+    {
+        $items = $this->_getLdap()->search('(objectClass=organizationalUnit)',
+            TESTS_ZEND_LDAP_WRITEABLE_SUBTREE, Zend_Ldap::SEARCH_SCOPE_SUB);
+        $this->assertEquals(9, $items->count());
+        $this->assertEquals(TESTS_ZEND_LDAP_WRITEABLE_SUBTREE, $items->getInnerIterator()->key());
+        $current = $items->current();
+        $this->assertType('array', $current);
+        $this->assertEquals(TESTS_ZEND_LDAP_WRITEABLE_SUBTREE, $current['dn']);
+
+        $i=0;
+        foreach ($items as $key => $item)
+        {
+            $this->assertEquals($i, $key);
+            $i++;
+        }
+        $this->assertEquals(9, $i);
+        $j=0;
+        foreach ($items as $item) { $j++; }
+        $this->assertEquals($i, $j);
+    }
 }
 
 class Zend_Ldap_SearchTest_CollectionClassNotSubclassingZendLdapCollection