Merged revisions 50683 via svnmerge from

http://svn.silverstripe.com/open/modules/sapphire/branches/2.2.2

........
  r50683 | aoneil | 2008-03-07 11:05:27 +1300 (Fri, 07 Mar 2008) | 2 lines
  
  #2295 - DataObjectSets cannot be iterated over multiple times concurrently
........


git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@50871 467b73ca-7a2a-4603-9d3b-597d59a354a9

core/ViewableData.php

@@ -18,7 +18,7 @@
  * @package sapphire
  * @subpackage view
  */
-class ViewableData extends Object implements Iterator {
+class ViewableData extends Object implements IteratorAggregate {
 	/**
 	 * The iterator position.
 	 * @var int
@@ -89,6 +89,18 @@ class ViewableData extends Object implements Iterator {
 		parent::defineMethods();
 	}
 	
+	/**
+	 * Returns a "1 record iterator"
+	 * Views <%control %> tags operate by looping over an item for as many instances as are 
+	 * available.  When you stick a single ViewableData object in a control tag, the foreach()
+	 * loop still needs to work.  We do this by creating an iterator that only returns one record.
+	 * This will always return the current ViewableData object.
+	 * @return ViewableData_Iterator A 1 record iterator
+	 */
+	function getIterator() {
+		return new ViewableData_Iterator($this);
+	}
+	
 	/**
 	 * Accessor overloader.
 	 * Allows default getting of fields via $this->getVal(), or mediation via a 
@@ -799,63 +811,6 @@ class ViewableData extends Object implements Iterator {
 		return SSViewer::topLevel();
 	}
 
-	/** 
-	 * Implementation of a "1 record iterator"
-	 * Views <%control %> tags operate by looping over an item for as many instances as are 
-	 * available.  When you stick a single ViewableData object in a control tag, the foreach()
-	 * loop still needs to work.  We do this by creating an iterator that only returns one record.
-	 * This will always return the current ViewableData object.
-	 */
-	public function current() { 
-		if($this->show) {
-			return $this;
-		}
-	}
-	
-	/** 
-	 * Implementation of a "1 record iterator"
-	 * Views <%control %> tags operate by looping over an item for as many instances as are 
-	 * available.  When you stick a single ViewableData object in a control tag, the foreach()
-	 * loop still needs to work.  We do this by creating an iterator that only returns one record.
-	 * Rewinds the iterator back to the start.
-	 */
-	public function rewind() { 
-		$this->show = true;
-	}
-	
-	/** 
-	 * Implementation of a "1 record iterator"
-	 * Views <%control %> tags operate by looping over an item for as many instances as are 
-	 * available.  When you stick a single ViewableData object in a control tag, the foreach()
-	 * loop still needs to work.  We do this by creating an iterator that only returns one record.
-	 * Return the key for the current object.
-	 */
-	public function key() { 
-		return 0;
-	}
-	
-	/** 
-	 * Implementation of a "1 record iterator"
-	 * Views <%control %> tags operate by looping over an item for as many instances as are 
-	 * available.  When you stick a single ViewableData object in a control tag, the foreach()
-	 * loop still needs to work.  We do this by creating an iterator that only returns one record.
-	 * Get the next object.
-	 */
-	public function next() {
-		return $this->show = false;
-	}
-	
-	/** 
-	 * Implementation of a "1 record iterator"
-	 * Views <%control %> tags operate by looping over an item for as many instances as are 
-	 * available.  When you stick a single ViewableData object in a control tag, the foreach()
-	 * loop still needs to work.  We do this by creating an iterator that only returns one record.
-	 * Check if there is a current object.
-	 */
-	public function valid() { 
-		return $this->show;
-	}
-
 
 	/**
 	 * Returns the root directory of the theme we're working with.
@@ -879,11 +834,7 @@ class ViewableData extends Object implements Iterator {
 		}
 	}
 	
-	/** 
+
-	 * Internal state toggler for the "1 record iterator"
-	 * @var bool
-	 */
-	private $show;
 
 	/**
 	 * Object-casting information for class methods
@@ -1148,4 +1099,61 @@ class ViewableData_Debugger extends ViewableData {
 	}
 }
 
+/**
+ * Implementation of a "1 record iterator"
+ * Views <%control %> tags operate by looping over an item for as many instances as are 
+ * available.  When you stick a single ViewableData object in a control tag, the foreach()
+ * loop still needs to work.  We do this by creating an iterator that only returns one record.
+ * This will always return the current ViewableData object.
+ */
+class ViewableData_Iterator implements Iterator {
+	function __construct($viewableData) {
+		$this->viewableData = $viewableData;
+		$this->show = true;
+	}
+
+	/** 
+	 * Internal state toggler
+	 * @var bool
+	 */
+	private $show;
+
+	/** 
+	 * This will always return the current ViewableData object.
+	 */
+	public function current() { 
+		if($this->show) {
+			return $this->viewableData;
+		}
+	}
+	
+	/** 
+	 * Rewinds the iterator back to the start.
+	 */
+	public function rewind() { 
+		$this->show = true;
+	}
+	
+	/** 
+	 * Return the key for the current object.
+	 */
+	public function key() { 
+		return 0;
+	}
+	
+	/** 
+	 * Get the next object.
+	 */
+	public function next() {
+		return $this->show = false;
+	}
+	
+	/** 
+	 * Check if there is a current object.
+	 */
+	public function valid() { 
+		return $this->show;
+	}
+}
+
 ?>

core/model/DataObjectSet.php

@@ -11,7 +11,7 @@
  * @package sapphire
  * @subpackage model
  */
-class DataObjectSet extends ViewableData implements Iterator {
+class DataObjectSet extends ViewableData implements IteratorAggregate {
 	/**
 	 * The DataObjects in this set.
 	 * @var array
@@ -90,7 +90,7 @@ class DataObjectSet extends ViewableData implements Iterator {
 				}
 			}
 
-			$this->current = $this->prepareItem(current($this->items));
+			
 		}
 		parent::__construct();
 	}
@@ -441,70 +441,6 @@ class DataObjectSet extends ViewableData implements Iterator {
 		}
 	}
 
-	/**
-	 * Rewind the iterator to the beginning of the set.
-	 * @return DataObject The first item in the set.
-	 */
-	public function rewind() {
-		$this->current = $this->prepareItem(reset($this->items));
-		return $this->current;
-	}
-	
-	/**
-	 * Return the current object of the iterator.
-	 * @return DataObject
-	 */
-	public function current() {
-		return $this->current;
-	}
-	
-	/**
-	 * Return the key of the current object of the iterator.
-	 * @return mixed
-	 */
-	public function key() {
-		return key($this->items);
-	}
-	
-	/**
-	 * Return the next item in this set.
-	 * @return DataObject
-	 */
-	public function next() {
-		$this->current = $this->prepareItem(next($this->items));
-		return $this->current;
-	}
-	
-	/**
-	 * Return the next item in this set without progressing the iterator.
-	 * @return DataObject
-	 */
-	public function peekNext() {
-		return $this->getOffset(1);
-	}
-	
-	/**
-	 * Return the prvious item in this set, without affecting the iterator.
-	 * @return DataObject
-	 */
-	public function peekPrev() {
-		return $this->getOffset(-1);
-	}
-	
-	/**
-	 * Return the object in this set offset by $offset from the iterator pointer.
-	 * @param int $offset The offset.
-	 * @return DataObject
-	 */
-	public function getOffset($offset) {
-		$keys = array_keys($this->items);
-		foreach($keys as $i => $key) {
-			if($key == key($this->items)) break;
-		}
-		$requiredKey = $keys[$i + $offset];
-		return $this->items [$requiredKey];
-	}
-
 	/**
 	 * Gets a specific slice of an existing set.
 	 * 
@@ -523,26 +459,12 @@ class DataObjectSet extends ViewableData implements Iterator {
 	}
 
 	/**
-	 * Prepare an item taken from the internal array for 
+	 * Returns an Iterator for this DataObjectSet.
-	 * output by this iterator.  Ensures that it is an object.
+	 * This function allows you to use DataObjectSets in foreach loops
-	 * @param DataObject $item Item to prepare
+	 * @return DataObjectSet_Iterator
-	 * @return DataObject
 	 */
-	protected function prepareItem($item) {
+	public function getIterator() {
-		if(is_object($item)) {
+		return new DataObjectSet_Iterator($this->items);
-			$item->iteratorProperties(key($this->items), sizeof($this->items));
-		}
-		// This gives some reliablity but it patches over the root cause of the bug...
-		// else if(key($this->items) !== null) $item = new ViewableData();
-		return $item;
-	}
-	
-	/**
-	 * Check the iterator is pointing to a valid item in the set.
-	 * @return boolean
-	 */
-	public function valid() {
-	 	return $this->current !== false;
 	}
 	
 	/**
@@ -1038,4 +960,103 @@ function column_sort_callback_basic($a, $b) {
 	return $result;
 }
 
+/**
+ * An Iterator for a DataObjectSet
+ */
+class DataObjectSet_Iterator implements Iterator {
+	function __construct($items) {
+		$this->items = $items;
+		
+		$this->current = $this->prepareItem(current($this->items));
+	}
+	
+	/**
+	 * Prepare an item taken from the internal array for 
+	 * output by this iterator.  Ensures that it is an object.
+	 * @param DataObject $item Item to prepare
+	 * @return DataObject
+	 */
+	protected function prepareItem($item) {
+		if(is_object($item)) {
+			$item->iteratorProperties(key($this->items), sizeof($this->items));
+		}
+		// This gives some reliablity but it patches over the root cause of the bug...
+		// else if(key($this->items) !== null) $item = new ViewableData();
+		return $item;
+	}
+	
+	
+	/**
+	 * Return the current object of the iterator.
+	 * @return DataObject
+	 */
+	public function current() {
+		return $this->current;
+	}
+	
+	/**
+	 * Return the key of the current object of the iterator.
+	 * @return mixed
+	 */
+	public function key() {
+		return key($this->items);
+	}
+	
+	/**
+	 * Return the next item in this set.
+	 * @return DataObject
+	 */
+	public function next() {
+		$this->current = $this->prepareItem(next($this->items));
+		return $this->current;
+	}
+	
+	/**
+	 * Rewind the iterator to the beginning of the set.
+	 * @return DataObject The first item in the set.
+	 */
+	public function rewind() {
+		$this->current = $this->prepareItem(reset($this->items));
+		return $this->current;
+	}
+	
+	/**
+	 * Check the iterator is pointing to a valid item in the set.
+	 * @return boolean
+	 */
+	public function valid() {
+	 	return $this->current !== false;
+	}
+	
+	/**
+	 * Return the next item in this set without progressing the iterator.
+	 * @return DataObject
+	 */
+	public function peekNext() {
+		return $this->getOffset(1);
+	}
+	
+	/**
+	 * Return the prvious item in this set, without affecting the iterator.
+	 * @return DataObject
+	 */
+	public function peekPrev() {
+		return $this->getOffset(-1);
+	}
+	
+	/**
+	 * Return the object in this set offset by $offset from the iterator pointer.
+	 * @param int $offset The offset.
+	 * @return DataObject
+	 */
+	public function getOffset($offset) {
+		$keys = array_keys($this->items);
+		foreach($keys as $i => $key) {
+			if($key == key($this->items)) break;
+		}
+		$requiredKey = $keys[$i + $offset];
+		return $this->items [$requiredKey];
+	}
+}
+
 ?>

core/model/SQLMap.php

@@ -11,7 +11,7 @@
  * @package sapphire
  * @subpackage model
  */
-class SQLMap extends Object implements Iterator {
+class SQLMap extends Object implements IteratorAggregate {
 	/**
 	 * The query used to generate the map.
 	 * @var SQLQuery
@@ -49,32 +49,11 @@ class SQLMap extends Object implements Iterator {
 	}
 	
 	/*
-	 * Iterator functions - necessary for foreach to work
+	 * Iterator - necessary for foreach to work
 	 */
-	public function rewind() {
+	public function getIterator() {
 		$this->genItems();
-		return $this->items->rewind() ? $this->items->rewind()->Title : null;
+		return $this->items->getIterator();
-	}
-	
-	public function current() {
-		$this->genItems();
-		return $this->items->current()->Title;
-	}
-	
-	public function key() {
-		$this->genItems();
-		return $this->items->current()->ID;
-	}
-	
-	public function next() {
-		$this->genItems();
-		$next = $this->items->next();
-		return isset($next->Title) ? $next->Title : null;
-	}
-	
-	public function valid() {
-		$this->genItems();
-	 	return $this->items->valid();
 	}
 	
 	/**

forms/ComplexTableField.php

@@ -495,7 +495,7 @@ JS;
 			return null;
 		}
 
-		$item = $this->unpagedSourceItems->getOffset($_REQUEST['ctf']['start'] + 1);
+		$item = $this->unpagedSourceItems->getIterator()->getOffset($_REQUEST['ctf']['start'] + 1);
 
 		$start = $_REQUEST['ctf']['start'] + 1;
 		return Convert::raw2att($this->PopupBaseLink() . "&methodName={$_REQUEST['methodName']}&ctf[childID]={$item->ID}&ctf[start]={$start}");
@@ -506,7 +506,7 @@ JS;
 			return null;
 		}
 
-		$item = $this->unpagedSourceItems->getOffset($_REQUEST['ctf']['start'] - 1);
+		$item = $this->unpagedSourceItems->getIterator()->getOffset($_REQUEST['ctf']['start'] - 1);
 
 		$start = $_REQUEST['ctf']['start'] - 1;
 		return Convert::raw2att($this->PopupBaseLink() . "&methodName={$_REQUEST['methodName']}&ctf[childID]={$item->ID}&ctf[start]={$start}");
@@ -532,7 +532,7 @@ JS;
         }
 		for($i = $offset;$i <= $offset + $this->pageSize && $i <= $this->totalCount;$i++) {
             $start = $i - 1;
-			$item = $this->unpagedSourceItems->getOffset($i-1);
+			$item = $this->unpagedSourceItems->getIterator()->getOffset($i-1);
 			$links['link'] = Convert::raw2att($this->PopupBaseLink() . "&methodName={$_REQUEST['methodName']}&ctf[childID]={$item->ID}&ctf[start]={$start}");
             $links['number'] = $i;
             $links['active'] = $i == $currentItem ? false : true;