ENHANCEMENT #4599 DataObjectSet now uses more array functions instead of performing equivalent tasks - thanks simon_w!

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

core/model/DataObjectSet.php

@@ -180,6 +180,18 @@ class DataObjectSet extends ViewableData implements IteratorAggregate, Countable
 		$this->totalSize = $totalSize;
 	}
 	
+	/**
+	 * Get the page limits
+	 * @return array
+	 */
+	public function getPageLimits() {
+		return array(
+			'pageStart' => $this->pageStart,
+			'pageLength' => $this->pageLength,
+			'totalSize' => $this->totalSize,
+		);
+	}
+
 	/**
 	 * Use the limit from the given query to add prev/next buttons to this DataObjectSet.
 	 * @param SQLQuery $query The query used to generate this DataObjectSet
@@ -190,7 +202,7 @@ class DataObjectSet extends ViewableData implements IteratorAggregate, Countable
 				$length = $query->limit['limit'];
 				$start = $query->limit['start'];
 			} else if(stripos($query->limit, 'OFFSET')) {
-				list($length, $start) = preg_split("/ +OFFSET +/", trim($query->limit));
+				list($length, $start) = preg_split("/ +OFFSET +/i", trim($query->limit));
 			} else {
 				$result = preg_split("/ *, */", trim($query->limit));
 				$start = $result[0];
@@ -438,11 +450,7 @@ class DataObjectSet extends ViewableData implements IteratorAggregate, Countable
 		if($key == null) {
 			array_unshift($this->items, $item);
 		} else {
-			// Not very efficient :-(
+			$this->items = array_merge(array($key=>$item), $this->items); 
-			$newItems = array();
-			$newItems[$key] = $item;
-			foreach($this->items as $k => $v) $newItems[$k] = $v;
-			$this->items = $newItems;
 		}
 	}
 
@@ -487,7 +495,7 @@ class DataObjectSet extends ViewableData implements IteratorAggregate, Countable
 	 */
 	public function merge($anotherSet){
 		if($anotherSet) {
-			foreach($anotherSet->items as $item){
+			foreach($anotherSet as $item){
 				$this->push($item);
 			}
 		}
@@ -501,13 +509,8 @@ class DataObjectSet extends ViewableData implements IteratorAggregate, Countable
 	 * @return DataObjectSet
 	 */
 	public function getRange($offset, $length) {
-		$set = new DataObjectSet();
+		$set = array_slice($this->items, (int)$offset, (int)$length);
-		$offset = (int)$offset;
+		return new DataObjectSet($set);
-		$length = (int)$length;
-		for($i=$offset; $i<($offset+$length); $i++) {
-			if(isset($this->items[$i])) $set->push($this->items[$i]);
-		}
-		return $set;
 	}
 
 	/**
@@ -524,7 +527,7 @@ class DataObjectSet extends ViewableData implements IteratorAggregate, Countable
 	 * @return boolean
 	 */
 	public function exists() {
-		return sizeof($this->items) > 0;
+		return (bool)$this->items;
 	}
 	
 	/**
@@ -536,7 +539,7 @@ class DataObjectSet extends ViewableData implements IteratorAggregate, Countable
 			return null;
 
 		$keys = array_keys($this->items);
-		return sizeof($keys) > 0 ? $this->items[$keys[0]] : null;
+		return $this->items[$keys[0]];
 	}
 	
 	/**

tests/DataObjectSetTest.php

@@ -219,6 +219,111 @@ class DataObjectSetTest extends SapphireTest {
 
 		$this->assertEquals($allComments->Count(), 7, 'There are 7 uniquely named commentators');
 	}
+
+	/**
+	 * Test {@link DataObjectSet->parseQueryLimit()}
+	 */
+	function testParseQueryLimit() {
+		// Create empty objects, because they don't need to have contents
+		$sql = new SQLQuery('*', '"Member"');
+		$max = $sql->unlimitedRowCount();
+		$set = new DataObjectSet();
+		
+		// Test handling an array
+		$set->parseQueryLimit($sql->limit(array('limit'=>5, 'start'=>2)));
+		$expected = array(
+			'pageStart' => 2,
+			'pageLength' => 5,
+			'totalSize' => $max,
+		);
+		$this->assertEquals($expected, $set->getPageLimits(), 'The page limits match expected values.');
+		
+		// Test handling OFFSET string
+		// uppercase
+		$set->parseQueryLimit($sql->limit('3 OFFSET   1'));
+		$expected = array(
+			'pageStart' => 1,
+			'pageLength' => 3,
+			'totalSize' => $max,
+		);
+		$this->assertEquals($expected, $set->getPageLimits(), 'The page limits match expected values.');
+		// and lowercase
+		$set->parseQueryLimit($sql->limit('32   offset   3'));
+		$expected = array(
+			'pageStart' => 3,
+			'pageLength' => 32,
+			'totalSize' => $max,
+		);
+		$this->assertEquals($expected, $set->getPageLimits(), 'The page limits match expected values.');
+		
+		// Finally check MySQL LIMIT syntax
+		$set->parseQueryLimit($sql->limit('7, 7'));
+		$expected = array(
+			'pageStart' => 7,
+			'pageLength' => 7,
+			'totalSize' => $max,
+		);
+		$this->assertEquals($expected, $set->getPageLimits(), 'The page limits match expected values.');
+	}
+
+	/**
+	 * Test {@link DataObjectSet->insertFirst()}
+	 */
+	function testInsertFirst() {
+		// Get one comment
+		$comment = DataObject::get_one('PageComment', 'Name = \'Joe\'');
+		// Get all other comments
+		$set = DataObject::get('PageComment', 'Name != \'Joe\'');
+		
+		// Duplicate so we can use it later without another lookup
+		$otherSet = clone $set;
+		// insert without a key
+		$otherSet->insertFirst($comment);
+		$this->assertEquals($comment, $otherSet->First(), 'Comment should be first');
+		
+		// Give us another copy
+		$otherSet = clone $set;
+		// insert with a numeric key
+		$otherSet->insertFirst($comment, 2);
+		$this->assertEquals($comment, $otherSet->First(), 'Comment should be first');
+		
+		// insert with a non-numeric key
+		$set->insertFirst($comment, 'SomeRandomKey');
+		$this->assertEquals($comment, $set->First(), 'Comment should be first');
+	}
+
+	/**
+	 * Test {@link DataObjectSet->getRange()}
+	 */
+	function testGetRange() {
+		$comments = DataObject::get('PageComment', '', "\"ID\" ASC");
+		
+		// Make sure we got all 8 comments
+		$this->assertEquals($comments->Count(), 8, 'Eight comments in the database.');
+		
+		// Grab a range
+		$range = $comments->getRange(1, 5);
+		$this->assertEquals($range->Count(), 5, 'Five comments in the range.');
+		
+		// And now grab a range that shouldn't be full. Remember counting starts at 0.
+		$range = $comments->getRange(7, 5);
+		$this->assertEquals($range->Count(), 1, 'One comment in the range.');
+		// Make sure it's the last one
+		$this->assertEquals($range->First(), $comments->Last(), 'The only item in the range should be the last one.');
+	}
+
+	/**
+	 * Test {@link DataObjectSet->exists()}
+	 */
+	function testExists() {
+		// Test an empty set
+		$set = new DataObjectSet();
+		$this->assertFalse($set->exists(), 'Empty set doesn\'t exist.');
+		// Test a non-empty set
+		$set = DataObject::get('PageComment', '', "\"ID\" ASC");
+		$this->assertTrue($set->exists(), 'Non-empty set does exist.');
+	}
+
 }
 
 /**