BUG ArrayList failing to respect the SS_Sortable interface

ref: CWPBUG-133

model/ArrayList.php

@@ -345,10 +345,48 @@ class ArrayList extends ViewableData implements SS_List, SS_Filterable, SS_Sorta
 		return $list;
 	}
 	
+	/**
+	 * Parses a specified column into a sort field and direction
+	 * 
+	 * @param type $column String to parse containing the column name
+	 * @param type $direction Optional Additional argument which may contain the direction
+	 * @return array Sort specification in the form array("Column", SORT_ASC).
+	 */
+	protected function parseSortColumn($column, $direction = null) {
+		// Substitute the direction for the column if column is a numeric index
+		if($direction && (empty($column) || is_numeric($column))) {
+			$column = $direction;
+			$direction = null;
+		}
+		
+		// Parse column specification, considering possible ansi sql quoting
+		if(preg_match('/^"?(?<column>[^"\s]+)"?(\s+(?<direction>((asc)|(desc))(ending)?))?$/i', $column, $match)) {
+			$column = $match['column'];
+			if(empty($direction) && !empty($match['direction'])) {
+				$direction = $match['direction'];
+			}
+		} else {
+			throw new InvalidArgumentException("Invalid sort() column");
+		}
+		
+		// Parse sort direction specification
+		if(empty($direction) || preg_match('/^asc(ending)?$/i', $direction)) {
+			$direction = SORT_ASC;
+		} elseif(preg_match('/^desc(ending)?$/i', $direction)) {
+			$direction = SORT_DESC;
+		} else {
+			throw new InvalidArgumentException("Invalid sort() direction");
+		}
+		
+		return array($column, $direction);
+	}
+	
 	/**
 	 * Sorts this list by one or more fields. You can either pass in a single
 	 * field name and direction, or a map of field names to sort directions.
 	 * 
+	 * Note that columns may be double quoted as per ANSI sql standard
+	 *
 	 * @return DataList
 	 * @see SS_List::sort()
 	 * @example $list->sort('Name'); // default ASC sorting
@@ -368,18 +406,17 @@ class ArrayList extends ViewableData implements SS_List, SS_Filterable, SS_Sorta
 		
 		// One argument and it's a string
 		if(count($args)==1 && is_string($args[0])){
-			$column = $args[0];
+			list($column, $direction) = $this->parseSortColumn($args[0]);
-			if(strpos($column, ' ') !== false) {
+			$columnsToSort[$column] = $direction;
-				throw new InvalidArgumentException("You can't pass SQL fragments to sort()");
-			}
-			$columnsToSort[$column] = SORT_ASC;
 
-		} else if(count($args)==2){
+		} else if(count($args)==2) {
-			$columnsToSort[$args[0]]=(strtolower($args[1])=='desc')?SORT_DESC:SORT_ASC;
+			list($column, $direction) = $this->parseSortColumn($args[0], $args[1]);
+			$columnsToSort[$column] = $direction;
 
 		} else if(is_array($args[0])) {
-			foreach($args[0] as $column => $sort_order){
+			foreach($args[0] as $key => $value) {
-				$columnsToSort[$column] = (strtolower($sort_order)=='desc')?SORT_DESC:SORT_ASC;
+				list($column, $direction) = $this->parseSortColumn($key, $value);
+				$columnsToSort[$column] = $direction;
 			}
 		} else {
 			throw new InvalidArgumentException("Bad arguments passed to sort()");

tests/model/ArrayListTest.php

@@ -250,12 +250,36 @@ class ArrayListTest extends SapphireTest {
 			array('Name' => 'John')
 		));
 
-		$list = $list->sort('Name');
+		// Unquoted name
-		$this->assertEquals($list->toArray(), array(
+		$list1 = $list->sort('Name');
+		$this->assertEquals($list1->toArray(), array(
 			(object) array('Name' => 'Bob'),
 			array('Name' => 'John'),
 			array('Name' => 'Steve')
 		));
+
+		// Quoted name name
+		$list2 = $list->sort('"Name"');
+		$this->assertEquals($list2->toArray(), array(
+			(object) array('Name' => 'Bob'),
+			array('Name' => 'John'),
+			array('Name' => 'Steve')
+		));
+
+		// Array (non-associative)
+		$list3 = $list->sort(array('"Name"'));
+		$this->assertEquals($list3->toArray(), array(
+			(object) array('Name' => 'Bob'),
+			array('Name' => 'John'),
+			array('Name' => 'Steve')
+		));
+		
+		// Check original list isn't altered
+		$this->assertEquals($list->toArray(), array(
+			array('Name' => 'Steve'),
+			(object) array('Name' => 'Bob'),
+			array('Name' => 'John')
+		));
 	}
 	
 	public function testSortSimpleASCOrder() {
@@ -265,12 +289,44 @@ class ArrayListTest extends SapphireTest {
 			array('Name' => 'John')
 		));
 
-		$list = $list->sort('Name','asc');
+		// Sort two arguments
-		$this->assertEquals($list->toArray(), array(
+		$list1 = $list->sort('Name','ASC');
+		$this->assertEquals($list1->toArray(), array(
 			(object) array('Name' => 'Bob'),
 			array('Name' => 'John'),
 			array('Name' => 'Steve')
 		));
+
+		// Sort single string
+		$list2 = $list->sort('Name asc');
+		$this->assertEquals($list2->toArray(), array(
+			(object) array('Name' => 'Bob'),
+			array('Name' => 'John'),
+			array('Name' => 'Steve')
+		));
+		
+		// Sort quoted string
+		$list3 = $list->sort('"Name" ASCENDING');
+		$this->assertEquals($list3->toArray(), array(
+			(object) array('Name' => 'Bob'),
+			array('Name' => 'John'),
+			array('Name' => 'Steve')
+		));
+
+		// Sort array specifier
+		$list4 = $list->sort(array('Name' => 'ascending'));
+		$this->assertEquals($list4->toArray(), array(
+			(object) array('Name' => 'Bob'),
+			array('Name' => 'John'),
+			array('Name' => 'Steve')
+		));
+		
+		// Check original list isn't altered
+		$this->assertEquals($list->toArray(), array(
+			array('Name' => 'Steve'),
+			(object) array('Name' => 'Bob'),
+			array('Name' => 'John')
+		));
 	}
 	
 	public function testSortSimpleDESCOrder() {
@@ -280,12 +336,44 @@ class ArrayListTest extends SapphireTest {
 			array('Name' => 'John')
 		));
 
-		$list = $list->sort('Name', 'DESC');
+		// Sort two arguments
-		$this->assertEquals($list->toArray(), array(
+		$list1 = $list->sort('Name', 'DESC');
+		$this->assertEquals($list1->toArray(), array(
 			array('Name' => 'Steve'),
 			array('Name' => 'John'),
 			(object) array('Name' => 'Bob')
 		));
+
+		// Sort single string
+		$list2 = $list->sort('Name desc');
+		$this->assertEquals($list2->toArray(), array(
+			array('Name' => 'Steve'),
+			array('Name' => 'John'),
+			(object) array('Name' => 'Bob')
+		));
+		
+		// Sort quoted string
+		$list3 = $list->sort('"Name" DESCENDING');
+		$this->assertEquals($list3->toArray(), array(
+			array('Name' => 'Steve'),
+			array('Name' => 'John'),
+			(object) array('Name' => 'Bob')
+		));
+
+		// Sort array specifier
+		$list4 = $list->sort(array('Name' => 'descending'));
+		$this->assertEquals($list4->toArray(), array(
+			array('Name' => 'Steve'),
+			array('Name' => 'John'),
+			(object) array('Name' => 'Bob')
+		));
+		
+		// Check original list isn't altered
+		$this->assertEquals($list->toArray(), array(
+			array('Name' => 'Steve'),
+			(object) array('Name' => 'Bob'),
+			array('Name' => 'John')
+		));
 	}
 	
 	public function testReverse() {