From c24a2c21779d47cd00bc04c05236a097fdce2aba Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Thu, 15 May 2014 14:25:23 +1200 Subject: [PATCH] BUG ArrayList failing to respect the SS_Sortable interface ref: CWPBUG-133 --- model/ArrayList.php | 55 ++++++++++++++++--- tests/model/ArrayListTest.php | 100 ++++++++++++++++++++++++++++++++-- 2 files changed, 140 insertions(+), 15 deletions(-) diff --git a/model/ArrayList.php b/model/ArrayList.php index 9d06e8219..89bba7143 100644 --- a/model/ArrayList.php +++ b/model/ArrayList.php @@ -345,9 +345,47 @@ 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('/^"?(?[^"\s]+)"?(\s+(?((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() @@ -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]; - if(strpos($column, ' ') !== false) { - throw new InvalidArgumentException("You can't pass SQL fragments to sort()"); - } - $columnsToSort[$column] = SORT_ASC; + list($column, $direction) = $this->parseSortColumn($args[0]); + $columnsToSort[$column] = $direction; - } else if(count($args)==2){ - $columnsToSort[$args[0]]=(strtolower($args[1])=='desc')?SORT_DESC:SORT_ASC; + } else if(count($args)==2) { + 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){ - $columnsToSort[$column] = (strtolower($sort_order)=='desc')?SORT_DESC:SORT_ASC; + foreach($args[0] as $key => $value) { + list($column, $direction) = $this->parseSortColumn($key, $value); + $columnsToSort[$column] = $direction; } } else { throw new InvalidArgumentException("Bad arguments passed to sort()"); diff --git a/tests/model/ArrayListTest.php b/tests/model/ArrayListTest.php index 1c9ce1a6c..e0f3c74b2 100644 --- a/tests/model/ArrayListTest.php +++ b/tests/model/ArrayListTest.php @@ -250,12 +250,36 @@ class ArrayListTest extends SapphireTest { array('Name' => 'John') )); - $list = $list->sort('Name'); - $this->assertEquals($list->toArray(), array( + // Unquoted name + $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'); - $this->assertEquals($list->toArray(), array( + // Sort two arguments + $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'); - $this->assertEquals($list->toArray(), array( + // Sort two arguments + $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() {