diff --git a/core/API/DataTableGenericFilter.php b/core/API/DataTableGenericFilter.php index 5d4ba49422fec0aab6a40491b678ce6378622639..7aa05713846390112ea3ecec35b784cb990cee5b 100644 --- a/core/API/DataTableGenericFilter.php +++ b/core/API/DataTableGenericFilter.php @@ -98,6 +98,9 @@ class DataTableGenericFilter array( 'filter_sort_column' => array('string'), 'filter_sort_order' => array('string', 'desc'), + $naturalSort = true, + $recursiveSort = false, + $doSortBySecondaryColumn = true )), array('Truncate', array( @@ -159,22 +162,27 @@ class DataTableGenericFilter } foreach ($filterParams as $name => $info) { - // parameter type to cast to - $type = $info[0]; - - // default value if specified, when the parameter doesn't have a value - $defaultValue = null; - if (isset($info[1])) { - $defaultValue = $info[1]; - } - - try { - $value = Common::getRequestVar($name, $defaultValue, $type, $this->request); - settype($value, $type); - $filterParameters[] = $value; - } catch (Exception $e) { - $exceptionRaised = true; - break; + if (!is_array($info)) { + // hard coded value that cannot be changed via API, see eg $naturalSort = true in 'Sort' + $filterParameters[] = $info; + } else { + // parameter type to cast to + $type = $info[0]; + + // default value if specified, when the parameter doesn't have a value + $defaultValue = null; + if (isset($info[1])) { + $defaultValue = $info[1]; + } + + try { + $value = Common::getRequestVar($name, $defaultValue, $type, $this->request); + settype($value, $type); + $filterParameters[] = $value; + } catch (Exception $e) { + $exceptionRaised = true; + break; + } } } diff --git a/core/DataTable.php b/core/DataTable.php index f55bc42a1a49182f4af2f04fd93cce3c55348ab2..a297b6c70b2a44f098194dbea65642a6f7cecf35 100644 --- a/core/DataTable.php +++ b/core/DataTable.php @@ -877,6 +877,14 @@ class DataTable implements DataTableInterface, \IteratorAggregate, \ArrayAccess return $this->rows; } + /** + * @ignore + */ + public function getRowsCountWithoutSummaryRow() + { + return count($this->rows); + } + /** * Returns an array containing all column values for the requested column. * diff --git a/core/DataTable/Filter/Sort.php b/core/DataTable/Filter/Sort.php index 8f2df8c9f66921c32be37bce713fd28d39a4de60..f3174e88de03b74ea8c1894e06f42224af98d528 100644 --- a/core/DataTable/Filter/Sort.php +++ b/core/DataTable/Filter/Sort.php @@ -13,6 +13,7 @@ use Piwik\DataTable\Row; use Piwik\DataTable\Simple; use Piwik\DataTable; use Piwik\Metrics; +use Piwik\Metrics\Sorter; /** * Sorts a {@link DataTable} based on the value of a specific column. @@ -25,7 +26,8 @@ class Sort extends BaseFilter { protected $columnToSort; protected $order; - protected $sign; + protected $naturalSort; + protected $isSecondaryColumnSortEnabled; const ORDER_DESC = 'desc'; const ORDER_ASC = 'asc'; @@ -38,8 +40,10 @@ class Sort extends BaseFilter * @param string $order order `'asc'` or `'desc'`. * @param bool $naturalSort Whether to use a natural sort or not (see {@link http://php.net/natsort}). * @param bool $recursiveSort Whether to sort all subtables or not. + * @param bool $doSortBySecondaryColumn If true will sort by a secondary column. The column is automatically + * detected and will be either nb_visits or label, if possible. */ - public function __construct($table, $columnToSort, $order = 'desc', $naturalSort = true, $recursiveSort = true) + public function __construct($table, $columnToSort, $order = 'desc', $naturalSort = true, $recursiveSort = true, $doSortBySecondaryColumn = false) { parent::__construct($table); @@ -48,144 +52,9 @@ class Sort extends BaseFilter } $this->columnToSort = $columnToSort; - $this->naturalSort = $naturalSort; - $this->setOrder($order); - } - - /** - * Updates the order - * - * @param string $order asc|desc - */ - public function setOrder($order) - { - if ($order == 'asc') { - $this->order = 'asc'; - $this->sign = 1; - } else { - $this->order = 'desc'; - $this->sign = -1; - } - } - - /** - * Sorting method used for sorting numbers - * - * @param array $rowA array[0 => value of column to sort, 1 => label] - * @param array $rowB array[0 => value of column to sort, 1 => label] - * @return int - */ - public function numberSort($rowA, $rowB) - { - if (isset($rowA[0]) && isset($rowB[0])) { - if ($rowA[0] != $rowB[0] || !isset($rowA[1])) { - return $this->sign * ($rowA[0] < $rowB[0] ? -1 : 1); - } else { - return -1 * $this->sign * strnatcasecmp($rowA[1], $rowB[1]); - } - } elseif (!isset($rowB[0]) && !isset($rowA[0])) { - return -1 * $this->sign * strnatcasecmp($rowA[1], $rowB[1]); - } elseif (!isset($rowA[0])) { - return 1; - } - - return -1; - } - - /** - * Sorting method used for sorting values natural - * - * @param mixed $valA - * @param mixed $valB - * @return int - */ - public function naturalSort($valA, $valB) - { - return !isset($valA) - && !isset($valB) - ? 0 - : (!isset($valA) - ? 1 - : (!isset($valB) - ? -1 - : $this->sign * strnatcasecmp( - $valA, - $valB - ) - ) - ); - } - - /** - * Sorting method used for sorting values - * - * @param mixed $valA - * @param mixed $valB - * @return int - */ - public function sortString($valA, $valB) - { - return !isset($valA) - && !isset($valB) - ? 0 - : (!isset($valA) - ? 1 - : (!isset($valB) - ? -1 - : $this->sign * - strcasecmp($valA, - $valB - ) - ) - ); - } - - protected function getColumnValue(Row $row) - { - $value = $row->getColumn($this->columnToSort); - - if ($value === false || is_array($value)) { - return null; - } - - return $value; - } - - /** - * Sets the column to be used for sorting - * - * @param Row $row - * @return int - */ - protected function selectColumnToSort($row) - { - $value = $row->hasColumn($this->columnToSort); - if ($value) { - return $this->columnToSort; - } - - $columnIdToName = Metrics::getMappingFromNameToId(); - // sorting by "nb_visits" but the index is Metrics::INDEX_NB_VISITS in the table - if (isset($columnIdToName[$this->columnToSort])) { - $column = $columnIdToName[$this->columnToSort]; - $value = $row->hasColumn($column); - - if ($value) { - return $column; - } - } - - // eg. was previously sorted by revenue_per_visit, but this table - // doesn't have this column; defaults with nb_visits - $column = Metrics::INDEX_NB_VISITS; - $value = $row->hasColumn($column); - if ($value) { - return $column; - } - - // even though this column is not set properly in the table, - // we select it for the sort, so that the table's internal state is set properly - return $this->columnToSort; + $this->naturalSort = $naturalSort; + $this->order = strtolower($order); + $this->isSecondaryColumnSortEnabled = $doSortBySecondaryColumn; } /** @@ -204,87 +73,48 @@ class Sort extends BaseFilter return; } - if (!$table->getRowsCount()) { + if (!$table->getRowsCountWithoutSummaryRow()) { return; } $row = $table->getFirstRow(); + if ($row === false) { return; } - $this->columnToSort = $this->selectColumnToSort($row); + $config = new Sorter\Config(); + $sorter = new Sorter($config); - $value = $this->getFirstValueFromDataTable($table); + $config->naturalSort = $this->naturalSort; + $config->primaryColumnToSort = $sorter->getPrimaryColumnToSort($table, $this->columnToSort); + $config->primarySortOrder = $sorter->getPrimarySortOrder($this->order); + $config->primarySortFlags = $sorter->getBestSortFlags($table, $config->primaryColumnToSort); + $config->secondaryColumnToSort = $sorter->getSecondaryColumnToSort($row, $config->primaryColumnToSort); + $config->secondarySortOrder = $sorter->getSecondarySortOrder($this->order, $config->secondaryColumnToSort); + $config->secondarySortFlags = $sorter->getBestSortFlags($table, $config->secondaryColumnToSort); - if (is_numeric($value) && $this->columnToSort !== 'label') { - $methodToUse = "numberSort"; - } else { - if ($this->naturalSort) { - $methodToUse = "naturalSort"; - } else { - $methodToUse = "sortString"; - } - } + // secondary sort should not be needed for all other sort flags (eg string/natural sort) as label is unique and would make it slower + $isSecondaryColumnSortNeeded = $config->primarySortFlags === SORT_NUMERIC; + $config->isSecondaryColumnSortEnabled = $this->isSecondaryColumnSortEnabled && $isSecondaryColumnSortNeeded; - $this->sort($table, $methodToUse); + $this->sort($sorter, $table); } - private function getFirstValueFromDataTable($table) + private function sort(Sorter $sorter, DataTable $table) { - foreach ($table->getRowsWithoutSummaryRow() as $row) { - $value = $this->getColumnValue($row); - if (!is_null($value)) { - return $value; - } - } - } - - /** - * Sorts the DataTable rows using the supplied callback function. - * - * @param string $functionCallback A comparison callback compatible with {@link usort}. - * @param string $columnSortedBy The column name `$functionCallback` sorts by. This is stored - * so we can determine how the DataTable was sorted in the future. - */ - private function sort(DataTable $table, $functionCallback) - { - $table->setTableSortedBy($this->columnToSort); - - $rows = $table->getRowsWithoutSummaryRow(); - - // get column value and label only once for performance tweak - $values = array(); - if ($functionCallback === 'numberSort') { - foreach ($rows as $key => $row) { - $values[$key] = array($this->getColumnValue($row), $row->getColumn('label')); - } - } else { - foreach ($rows as $key => $row) { - $values[$key] = $this->getColumnValue($row); - } - } - - uasort($values, array($this, $functionCallback)); - - $sortedRows = array(); - foreach ($values as $key => $value) { - $sortedRows[] = $rows[$key]; - } - - $table->setRows($sortedRows); - - unset($rows); - unset($sortedRows); + $sorter->sort($table); if ($table->isSortRecursiveEnabled()) { foreach ($table->getRowsWithoutSummaryRow() as $row) { $subTable = $row->getSubtable(); + if ($subTable) { $subTable->enableRecursiveSort(); - $this->sort($subTable, $functionCallback); + $this->sort($sorter, $subTable); } } } } -} + +} \ No newline at end of file diff --git a/core/Metrics/Sorter.php b/core/Metrics/Sorter.php new file mode 100644 index 0000000000000000000000000000000000000000..48be5a25538ddb7368f5f686b7c513fa5aa5d085 --- /dev/null +++ b/core/Metrics/Sorter.php @@ -0,0 +1,237 @@ +<?php +/** + * Piwik - free/libre analytics platform + * + * @link http://piwik.org + * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later + */ +namespace Piwik\Metrics; + +use Piwik\DataTable; +use Piwik\DataTable\Row; +use Piwik\Metrics; +use Piwik\Plugin\Metric; +use Piwik\Plugin\Report; + +class Sorter +{ + /** + * @var Sorter\Config + */ + private $config; + + public function __construct(Sorter\Config $config) + { + $this->config = $config; + } + + /** + * Sorts the DataTable rows using the supplied callback function. + * + * @param DataTable $table The table to sort. + */ + public function sort(DataTable $table) + { + // all that code is in here and not in separate methods for best performance. It does make a difference once + // php has to copy many (eg 50k) rows otherwise. + + $table->setTableSortedBy($this->config->primaryColumnToSort); + + $rows = $table->getRowsWithoutSummaryRow(); + + // we need to sort rows that have a value separately from rows that do not have a value since we always want + // to append rows that do not have a value at the end. + $rowsWithValues = array(); + $rowsWithoutValues = array(); + + $valuesToSort = array(); + foreach ($rows as $key => $row) { + $value = $this->getColumnValue($row); + if (isset($value)) { + $valuesToSort[] = $value; + $rowsWithValues[] = $row; + } else { + $rowsWithoutValues[] = $row; + } + } + + unset($rows); + + if ($this->config->isSecondaryColumnSortEnabled && $this->config->secondaryColumnToSort) { + $secondaryValues = array(); + foreach ($rowsWithValues as $key => $row) { + $secondaryValues[$key] = $row->getColumn($this->config->secondaryColumnToSort); + } + + array_multisort($valuesToSort, $this->config->primarySortOrder, $this->config->primarySortFlags, $secondaryValues, $this->config->secondarySortOrder, $this->config->secondarySortFlags, $rowsWithValues); + + } else { + array_multisort($valuesToSort, $this->config->primarySortOrder, $this->config->primarySortFlags, $rowsWithValues); + } + + if (!empty($rowsWithoutValues) && $this->config->secondaryColumnToSort) { + $secondaryValues = array(); + foreach ($rowsWithoutValues as $key => $row) { + $secondaryValues[$key] = $row->getColumn($this->config->secondaryColumnToSort); + } + + array_multisort($secondaryValues, $this->config->secondarySortOrder, $this->config->secondarySortFlags, $rowsWithoutValues); + } + + unset($secondaryValues); + + foreach ($rowsWithoutValues as $row) { + $rowsWithValues[] = $row; + } + + $table->setRows(array_values($rowsWithValues)); + } + + private function getColumnValue(Row $row) + { + $value = $row->getColumn($this->config->primaryColumnToSort); + + if ($value === false || is_array($value)) { + return null; + } + + return $value; + } + + /** + * @param string $order 'asc' or 'desc' + * @return int + */ + public function getPrimarySortOrder($order) + { + if ($order === 'asc') { + return SORT_ASC; + } + + return SORT_DESC; + } + + /** + * @param string $order 'asc' or 'desc' + * @param string|int $secondarySortColumn column name or column id + * @return int + */ + public function getSecondarySortOrder($order, $secondarySortColumn) + { + if ($secondarySortColumn === 'label') { + + $secondaryOrder = SORT_ASC; + if ($order === 'asc') { + $secondaryOrder = SORT_DESC; + } + + return $secondaryOrder; + } + + return $this->getPrimarySortOrder($order); + } + + /** + * Detect the column to be used for sorting + * + * @param DataTable $table + * @param string|int $columnToSort column name or column id + * @return int + */ + public function getPrimaryColumnToSort(DataTable $table, $columnToSort) + { + // we fallback to nb_visits in case columnToSort does not exist + $columnsToCheck = array($columnToSort, 'nb_visits'); + + $row = $table->getFirstRow(); + + foreach ($columnsToCheck as $column) { + $column = Metric::getActualMetricColumn($table, $column); + + if ($row->hasColumn($column)) { + // since getActualMetricColumn() returns a default value, we need to make sure it actually has that column + return $column; + } + } + + return $columnToSort; + } + + /** + * Detect the secondary sort column to be used for sorting + * + * @param Row $row + * @param int|string $primaryColumnToSort + * @return int + */ + public function getSecondaryColumnToSort(Row $row, $primaryColumnToSort) + { + $defaultSecondaryColumn = array(Metrics::INDEX_NB_VISITS, 'nb_visits'); + + if (in_array($primaryColumnToSort, $defaultSecondaryColumn)) { + // if sorted by visits, then sort by label as a secondary column + $column = 'label'; + $value = $row->hasColumn($column); + if ($value !== false) { + return $column; + } + + return null; + } + + if ($primaryColumnToSort !== 'label') { + // we do not add this by default to make sure we do not sort by label as a first and secondary column + $defaultSecondaryColumn[] = 'label'; + } + + foreach ($defaultSecondaryColumn as $column) { + $value = $row->hasColumn($column); + if ($value !== false) { + return $column; + } + } + } + + /** + * @param DataTable $table + * @param string|int $columnToSort A column name or column id. Make sure that column actually exists in the row. + * You might want to get a valid column via {@link getPrimaryColumnToSort()} or + * {@link getSecondaryColumnToSort()} + * @return int + */ + public function getBestSortFlags(DataTable $table, $columnToSort) + { + // when column is label we always to sort by string or natural + if ($columnToSort !== 'label') { + foreach ($table->getRowsWithoutSummaryRow() as $row) { + $value = $row->getColumn($columnToSort); + + if ($value !== false && $value !== null && !is_array($value)) { + + if (is_numeric($value)) { + $sortFlags = SORT_NUMERIC; + } else { + $sortFlags = $this->getStringSortFlags(); + } + + return $sortFlags; + } + } + } + + return $this->getStringSortFlags(); + } + + private function getStringSortFlags() + { + if ($this->config->naturalSort) { + $sortFlags = SORT_NATURAL | SORT_FLAG_CASE; + } else { + $sortFlags = SORT_STRING | SORT_FLAG_CASE; + } + + return $sortFlags; + } + + +} \ No newline at end of file diff --git a/core/Metrics/Sorter/Config.php b/core/Metrics/Sorter/Config.php new file mode 100644 index 0000000000000000000000000000000000000000..a62b4e4c081c5742efc43c59485ac5c4799c80d8 --- /dev/null +++ b/core/Metrics/Sorter/Config.php @@ -0,0 +1,24 @@ +<?php +/** + * Piwik - free/libre analytics platform + * + * @link http://piwik.org + * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later + */ +namespace Piwik\Metrics\Sorter; + +class Config +{ + public $naturalSort = false; + + public $primaryColumnToSort; + public $primarySortFlags; + public $primarySortOrder; + + public $secondaryColumnToSort; + public $secondarySortOrder; + public $secondarySortFlags; + + public $isSecondaryColumnSortEnabled = true; + +} \ No newline at end of file diff --git a/core/Plugin/Metric.php b/core/Plugin/Metric.php index 6349ca699983060be54a3d9558d581a4c42d5efa..e1bdb78cad2086e8dbf83fc99b29d41b824a868f 100644 --- a/core/Plugin/Metric.php +++ b/core/Plugin/Metric.php @@ -174,16 +174,19 @@ abstract class Metric */ public static function getActualMetricColumn(DataTable $table, $columnName, $mappingNameToId = null) { - if (empty($mappingIdToName)) { - $mappingNameToId = Metrics::getMappingFromNameToId(); - } - $firstRow = $table->getFirstRow(); - if (!empty($firstRow) - && $firstRow->getColumn($columnName) === false - ) { - $columnName = $mappingNameToId[$columnName]; + + if (!empty($firstRow) && $firstRow->hasColumn($columnName) === false) { + + if (empty($mappingIdToName)) { + $mappingNameToId = Metrics::getMappingFromNameToId(); + } + + if (array_key_exists($columnName, $mappingNameToId)) { + $columnName = $mappingNameToId[$columnName]; + } } + return $columnName; } } diff --git a/tests/PHPUnit/Unit/DataTable/Filter/SortTest.php b/tests/PHPUnit/Unit/DataTable/Filter/SortTest.php index a1f173ba61b6a52f5608dc272cbbb04658de3f6c..8a405b697a2f5960c174e94c69dbf68241d85385 100644 --- a/tests/PHPUnit/Unit/DataTable/Filter/SortTest.php +++ b/tests/PHPUnit/Unit/DataTable/Filter/SortTest.php @@ -14,8 +14,10 @@ use Piwik\DataTable\Row; /** * @group DataTableTest + * @group Core + * @group sort */ -class DataTable_Filter_SortTest extends \PHPUnit_Framework_TestCase +class SortTest extends \PHPUnit_Framework_TestCase { public function testNormalSortDescending() @@ -47,7 +49,6 @@ class DataTable_Filter_SortTest extends \PHPUnit_Framework_TestCase $this->assertEquals($expectedOrder, $table->getColumn('label')); } - public function testMissingColumnValuesShouldAppearLastAfterSortAsc() { $table = new DataTable(); @@ -83,8 +84,6 @@ class DataTable_Filter_SortTest extends \PHPUnit_Framework_TestCase /** * Test to sort by label - * - * @group Core */ public function testFilterSortString() { @@ -125,36 +124,31 @@ class DataTable_Filter_SortTest extends \PHPUnit_Framework_TestCase /** * Test to sort by visit - * - * @group Core */ public function testFilterSortNumeric() { - $idcol = Row::COLUMNS; - $table = new DataTable(); - $rows = array( - array($idcol => array('label' => 'google', 'nb_visits' => 897)), //0 - array($idcol => array('label' => 'ask', 'nb_visits' => -152)), //1 - array($idcol => array('label' => 'piwik', 'nb_visits' => 1.5)), //2 - array($idcol => array('label' => 'yahoo', 'nb_visits' => 154)), //3 - array($idcol => array('label' => 'amazon', 'nb_visits' => 30)), //4 - array($idcol => array('label' => '238949', 'nb_visits' => 0)), //5 - array($idcol => array('label' => 'Q*(%&*', 'nb_visits' => 1)) //6 - ); - $table->addRowsFromArray($rows); - $expectedtable = new DataTable(); + $table = $this->createDataTable(array( + array('label' => 'google', 'nb_visits' => 897), //0 + array('label' => 'ask', 'nb_visits' => -152), //1 + array('label' => 'piwik', 'nb_visits' => 1.5), //2 + array('label' => 'yahoo', 'nb_visits' => 154), //3 + array('label' => 'amazon', 'nb_visits' => 30), //4 + array('label' => '238949', 'nb_visits' => 0), //5 + array('label' => 'Q*(%&*', 'nb_visits' => 1) //6 + )); + $rows = array( - array($idcol => array('label' => 'ask', 'nb_visits' => -152)), //1 - array($idcol => array('label' => '238949', 'nb_visits' => 0)), //5 - array($idcol => array('label' => 'Q*(%&*', 'nb_visits' => 1)), //6 - array($idcol => array('label' => 'piwik', 'nb_visits' => 1.5)), //2 - array($idcol => array('label' => 'amazon', 'nb_visits' => 30)), //4 - array($idcol => array('label' => 'yahoo', 'nb_visits' => 154)), //3 - array($idcol => array('label' => 'google', 'nb_visits' => 897)) //0 + array('label' => 'ask', 'nb_visits' => -152), //1 + array('label' => '238949', 'nb_visits' => 0), //5 + array('label' => 'Q*(%&*', 'nb_visits' => 1), //6 + array('label' => 'piwik', 'nb_visits' => 1.5), //2 + array('label' => 'amazon', 'nb_visits' => 30), //4 + array('label' => 'yahoo', 'nb_visits' => 154), //3 + array('label' => 'google', 'nb_visits' => 897) //0 ); - $expectedtable->addRowsFromArray($rows); - $expectedtableReverse = new DataTable(); - $expectedtableReverse->addRowsFromArray(array_reverse($rows)); + + $expectedtable = $this->createDataTable($rows); + $expectedtableReverse = $this->createDataTable(array_reverse($rows)); $filter = new Sort($table, 'nb_visits', 'asc'); $filter->filter($table); @@ -165,6 +159,47 @@ class DataTable_Filter_SortTest extends \PHPUnit_Framework_TestCase $this->assertTrue(DataTable::isEqual($table, $expectedtableReverse)); } + /** + * Test to sort by visit + */ + public function testFilterSortNumeric_withSecondaryColumnSortLabel() + { + $rows = array( + array('label' => 'google', 'nb_visits' => array()), + array('label' => 'ask', 'nb_visits' => false), + array('label' => 'piwik', 'nb_visits' => 143), + array('label' => 'yahoo', 'nb_visits' => 154), + array('label' => 'zzzzz', 'nb_visits' => false), + array('label' => 'amazon', 'nb_visits' => 154), + array('label' => '238949', 'nb_visits' => 154), + array('label' => 'Q*(%&*', 'nb_visits' => 1) + ); + $table = $this->createDataTable($rows); + + $rows = array( + array('label' => '238949', 'nb_visits' => 154), + array('label' => 'amazon', 'nb_visits' => 154), + array('label' => 'yahoo', 'nb_visits' => 154), + array('label' => 'piwik', 'nb_visits' => 143), + array('label' => 'Q*(%&*', 'nb_visits' => 1), + array('label' => 'ask', 'nb_visits' => false), + array('label' => 'google', 'nb_visits' => array()), + array('label' => 'zzzzz', 'nb_visits' => false) + ); + + $expectedtable = $this->createDataTable($rows); + $expectedtableReverse = $this->createDataTable(array_reverse($rows)); + + $filter = new Sort($table, 'nb_visits', 'desc', $natural = true, $reverse = false, $secondaryColumn = true); + $filter->filter($table); + + $this->assertTrue(DataTable::isEqual($table, $expectedtable)); + + $filter = new Sort($table, 'nb_visits', 'asc', $natural = true, $reverse = false, $secondaryColumn = true); + $filter->filter($table); + $this->assertTrue(DataTable::isEqual($table, $expectedtableReverse)); + } + public function test_sortingArrayValues_doesNotError() { $table = new DataTable(); @@ -180,4 +215,56 @@ class DataTable_Filter_SortTest extends \PHPUnit_Framework_TestCase $filter->filter($table); $this->assertTrue(DataTable::isEqual($tableOriginal, $table)); } + + + /** + * Test to sort by label + */ + public function testFilter_shouldPickStringSearchEvenIfFirstLabelIsNumeric() + { + $table = $this->createDataTable(array( + array('label' => '238975247578949'), + array('label' => 'google'), + array('label' => '013494'), + array('label' => '9'), + array('label' => '999yahoo'), + array('label' => '494'), + array('label' => 'Q*(%&*("$&%*(&"$*")"))') + )); + + $rows = array( + array('label' => '9'), + array('label' => '494'), + array('label' => '999yahoo'), + array('label' => '013494'), + array('label' => '238975247578949'), + array('label' => 'google'), + array('label' => 'Q*(%&*("$&%*(&"$*")"))') + ); + + $expectedtable = $this->createDataTable($rows); + $expectedtableReverse = $this->createDataTable(array_reverse($rows)); + + $filter = new Sort($table, 'label', 'asc', $natural = true); + $filter->filter($table); + $this->assertTrue(DataTable::isEqual($expectedtable, $table)); + + $filter = new Sort($table, 'label', 'desc'); + $filter->filter($table); + $this->assertTrue(DataTable::isEqual($table, $expectedtableReverse)); + } + + private function createDataTable($rows) + { + $table = new DataTable(); + foreach ($rows as $columns) { + $table->addRow($this->createRow($columns)); + } + return $table; + } + + private function createRow($columns) + { + return new Row(array(Row::COLUMNS => $columns)); + } } diff --git a/tests/PHPUnit/Unit/Metrics/SorterTest.php b/tests/PHPUnit/Unit/Metrics/SorterTest.php new file mode 100644 index 0000000000000000000000000000000000000000..103995e638eddd675a0b032e65f78748d4a0ab9a --- /dev/null +++ b/tests/PHPUnit/Unit/Metrics/SorterTest.php @@ -0,0 +1,405 @@ +<?php +/** + * Piwik - free/libre analytics platform + * + * @link http://piwik.org + * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later + */ +namespace Piwik\Tests\Unit\Metrics; + +use Piwik\DataTable; +use Piwik\DataTable\Row; +use Piwik\Metrics; +use Piwik\Metrics\Sorter; +use Piwik\Metrics\Sorter\Config; +use Piwik\Tests\Framework\TestCase\UnitTestCase; + +/** + * @group Core + * @group sort + */ +class SorterTest extends UnitTestCase +{ + /** + * @var Sorter + */ + private $sorter; + + /** + * @var Config + */ + private $config; + + public function setUp() + { + parent::setUp(); + + $this->config = new Config(); + $this->config->primaryColumnToSort = 'nb_visits'; + $this->config->primarySortOrder = SORT_DESC; + $this->config->primarySortFlags = SORT_NUMERIC; + $this->sorter = new Sorter($this->config); + } + + public function test_getPrimarySortOrder_shouldReturnDescByDefault() + { + $this->assertSame(SORT_DESC, $this->sorter->getPrimarySortOrder(null)); + $this->assertSame(SORT_DESC, $this->sorter->getPrimarySortOrder('whatever')); + $this->assertSame(SORT_DESC, $this->sorter->getPrimarySortOrder('desc')); + } + + public function test_getPrimarySortOrder_shouldReturnAscIfRequestedLowerCase() + { + $this->assertSame(SORT_ASC, $this->sorter->getPrimarySortOrder('asc')); + $this->assertSame(SORT_DESC, $this->sorter->getPrimarySortOrder('AsC')); // we require 'asc' to be lowercase + } + + public function test_getSecondarySortOrder_shouldReturnInvertedOrder_IfColumnIsLabel() + { + $this->assertSame(SORT_DESC, $this->sorter->getSecondarySortOrder('asc', 'label')); + $this->assertSame(SORT_ASC, $this->sorter->getSecondarySortOrder('whatever', 'label')); + $this->assertSame(SORT_ASC, $this->sorter->getSecondarySortOrder('desc', 'label')); + $this->assertSame(SORT_ASC, $this->sorter->getSecondarySortOrder('AsC', 'label')); + } + public function test_getPrimarySortOrder_shouldReturnDescByDefault_IfNotLabelColumnIsRequested() + { + $this->assertSame(SORT_DESC, $this->sorter->getSecondarySortOrder(null, 'nb_visits')); + $this->assertSame(SORT_DESC, $this->sorter->getSecondarySortOrder('whatever', 'nb_visits')); + $this->assertSame(SORT_DESC, $this->sorter->getSecondarySortOrder('desc', 'nb_visits')); + } + + public function test_getSecondarySortOrder_shouldReturnAscIfRequestedLowerCase_IfNotLabelColumnIsRequested() + { + $this->assertSame(SORT_ASC, $this->sorter->getSecondarySortOrder('asc', 'nb_visits')); + $this->assertSame(SORT_DESC, $this->sorter->getSecondarySortOrder('AsC', 'nb_visits')); // we require 'asc' to be lowercase + } + + /** + * @dataProvider getPrimaryColumnsToSort + */ + public function test_getPrimaryColumnToSort_shouldPickCorrectPrimaryColumnAndMapMetricNameToIdIfNeededAndReverse($expectedUsedColumn, $columnToSortBy) + { + $table = $this->createDataTable(array( + array('label' => 'nintendo', 'nb_visits' => false, 'nb_hits' => 0, Metrics::INDEX_NB_VISITS_CONVERTED => false, Metrics::INDEX_BOUNCE_COUNT => 5) + )); + + $this->assertSame($expectedUsedColumn, $this->sorter->getPrimaryColumnToSort($table, $columnToSortBy)); + } + + public function getPrimaryColumnsToSort() + { + return array( + array('nb_visits', 'nb_visits'), // it is present in the row and should be used even though the value is false + array('nb_hits', 'nb_hits'), // it is present in the row and should be used even though the value is zero + array(Metrics::INDEX_NB_VISITS_CONVERTED, 'nb_visits_converted'), // the column name is not present but it should find the column id even though the value is false + array(Metrics::INDEX_NB_VISITS_CONVERTED, Metrics::INDEX_NB_VISITS_CONVERTED), // if a column is present as id it should still be able to find it + array(Metrics::INDEX_BOUNCE_COUNT, 'bounce_count'), // should resolve column name to id, column has a value + array(Metrics::INDEX_BOUNCE_COUNT, Metrics::INDEX_BOUNCE_COUNT), // should find a column with a value + ); + } + + public function test_getPrimaryColumnToSort_shouldFallbackToNbVisitsIfPossible() + { + $table = $this->createDataTable(array( + array('label' => 'nintendo', 'nb_visits' => false) + )); + + $this->assertSame('nb_visits', $this->sorter->getPrimaryColumnToSort($table, 'any_random_column_that_doesnt_exist')); + } + + public function test_getPrimaryColumnToSort_shouldFallbackToThePassedColumnNameIfColumnCannotBeFoundAndNbVisitsDoesNotExist() + { + $table = $this->createDataTable(array(array('label' => 'nintendo'))); + + $this->assertSame('any_random_column_that_doesnt_exist', $this->sorter->getPrimaryColumnToSort($table, 'any_random_column_that_doesnt_exist')); + } + + public function test_getSecondaryColumnToSort_shouldNotFindASecondaryColumnToSort_IfSortedByLabelButNoVisitsColumnPresent() + { + $row = $this->createRow(array('label' => 'nintendo')); + + $this->assertNull($this->sorter->getSecondaryColumnToSort($row, 'label')); + } + + public function test_getSecondaryColumnToSort_shouldPreferVisitsColumn_IfColumnIsPresent_EvenIfValueIsFalse() + { + $row = $this->createRow(array('label' => 'nintendo', 'nb_visits' => false, 'nb_hits' => 10)); + + $this->assertSame('nb_visits', $this->sorter->getSecondaryColumnToSort($row, 'nb_hits')); + $this->assertSame('nb_visits', $this->sorter->getSecondaryColumnToSort($row, 'label')); + } + + public function test_getSecondaryColumnToSort_shouldPreferVisitsColumn_IfColumnIsPresent_EvenIfVisitsColumnIsId() + { + $row = $this->createRow(array('label' => 'nintendo', Metrics::INDEX_NB_VISITS => false, 'nb_hits' => 10)); + + $this->assertSame(Metrics::INDEX_NB_VISITS, $this->sorter->getSecondaryColumnToSort($row, 'nb_hits')); + $this->assertSame(Metrics::INDEX_NB_VISITS, $this->sorter->getSecondaryColumnToSort($row, 'label')); + } + + public function test_getSecondaryColumnToSort_shouldUseLabelColumn_IfColumnIsPresentButNotNbVisitsColumn() + { + $row = $this->createRow(array('label' => 'nintendo', 'nb_hits' => 10)); + + $this->assertSame('label', $this->sorter->getSecondaryColumnToSort($row, 'nb_hits')); + } + + public function test_getSecondaryColumnToSort_shouldUseLabelColumn_IfPrimaryColumnIsNbVisitsColumn() + { + $row = $this->createRow(array('label' => 'nintendo', 'nb_visits' => 10)); + + $this->assertSame('label', $this->sorter->getSecondaryColumnToSort($row, 'nb_visits')); + $this->assertSame('label', $this->sorter->getSecondaryColumnToSort($row, Metrics::INDEX_NB_VISITS)); + } + + public function test_getSecondaryColumnToSort_shouldNotBeAbleToFallback_IfVisitsColumnIsUsedButThereIsNoLabelColumn() + { + $row = $this->createRow(array('nb_visits' => 10, 'nb_hits' => 10)); + + $this->assertNull($this->sorter->getSecondaryColumnToSort($row, 'nb_visits')); + } + + public function test_getSecondaryColumnToSort_shouldUseVisitsAsSecondaryColumn_IfLabelIsUsedAsPrimaryColumn() + { + $row = $this->createRow(array('label' => 'nintendo', 'nb_visits' => false)); + + $this->assertSame('nb_visits', $this->sorter->getSecondaryColumnToSort($row, 'label')); + } + + /** + * @dataProvider getLabelsForNaturalSortTest + */ + public function test_getBestSortFlags_shouldAlwaysPickStringOrNaturalSortCaseInsensitive($label) + { + $table = $this->createDataTable(array(array('label' => $label))); + + $this->config->naturalSort = false; // even if natural sort is not preferred it should be still used + $this->assertSame(SORT_STRING | SORT_FLAG_CASE, $this->sorter->getBestSortFlags($table, 'label')); + + $this->config->naturalSort = true; + $this->assertSame(SORT_NATURAL | SORT_FLAG_CASE, $this->sorter->getBestSortFlags($table, 'label')); + } + + public function getLabelsForNaturalSortTest() + { + return array(array('nintendo'), array('2015'), array('240.4'), array(2015), array('/test')); + } + + /** + * @dataProvider getColumnsForBestSortFlagsTest + */ + public function test_getBestSortFlags($expectedSortFlags, $columnToReadFrom, $naturalSort = false) + { + $this->config->naturalSort = $naturalSort; + + $table = $this->createDataTable(array( + array('label' => 'nintendo1', 'nb_visits' => false, 'nb_hits' => 0, Metrics::INDEX_NB_VISITS_CONVERTED => false, Metrics::INDEX_BOUNCE_COUNT => 5), + array('label' => 'nintendo2', 'nb_visits' => 100, 'nb_pageviews' => 100, Metrics::INDEX_NB_VISITS_CONVERTED => null, 'sum_visit_length' => '5.5s'), + array('label' => 'nintendo2', Metrics::INDEX_NB_VISITS_CONVERTED => array(), 'min_time_generation' => '5.5') + )); + + $this->assertSame($expectedSortFlags, $this->sorter->getBestSortFlags($table, $columnToReadFrom)); + } + + public function getColumnsForBestSortFlagsTest() + { + return array( + array(SORT_NUMERIC, 'nb_visits'), // should find a numeric value in the first row + array(SORT_NUMERIC, 'nb_pageviews'), // should find a numeric value in the second row + array(SORT_STRING | SORT_FLAG_CASE, Metrics::INDEX_NB_VISITS_CONVERTED), // should not find any value in any row and use default value + array(SORT_NATURAL | SORT_FLAG_CASE, Metrics::INDEX_NB_VISITS_CONVERTED, true), // should not find any value in any row and use default value, natural preferred + array(SORT_STRING | SORT_FLAG_CASE, 'sum_visit_length'), // it is not numeric so should use string as natural is disabled + array(SORT_NATURAL | SORT_FLAG_CASE, 'sum_visit_length', true), // it is not numeric but natural is preferred so should use natural sort + array(SORT_NUMERIC, 'min_time_generation') // value is a string but numeric so should use numeric + ); + } + + public function test_sort_shouldNotFailIfNoRowsAreSet() + { + $table = $this->createDataTable(array()); + + $this->sorter->sort($table); + + $this->assertSame(0, $table->getRowsCount()); + } + + public function test_sort_shouldSetTheSortedColumnNameOnTheTable() + { + $table = $this->createDataTable(array(array('nb_test' => 5))); + $this->config->primaryColumnToSort = 'nb_test'; + + $this->sorter->sort($table); + + $this->assertSame('nb_test', $table->getSortedByColumnName()); + } + + public function test_sort_shouldKeepTheAmountOfColumns() + { + $table = $this->createDataTableFromValues(array(5, null)); + $table->addSummaryRow($this->createRow(array('nb_test' => 10))); + + $this->sorter->sort($table); + + $this->assertSame(3, $table->getRowsCount()); + $this->assertSame(2, $table->getRowsCountWithoutSummaryRow()); + } + + public function test_sort_shouldNotSortOrChangeTheSummaryRow() + { + $table = $this->createDataTableFromValues(array(5, null)); + $table->addSummaryRow($this->createRow(array('nb_test' => 10))); + + $this->sorter->sort($table); + + $summaryRow = $table->getRowFromId(DataTable::ID_SUMMARY_ROW); + + $this->assertSame(10, $summaryRow->getColumn('nb_test')); + } + + public function test_sort_shouldSortNumeric_AndShouldAddEmptyValuesAlwaysAtTheEnd() + { + $table = $this->createDataTableFromValues(array(5, null, 61, array(), 10, false, 20, 15)); + + $this->config->primarySortFlags = SORT_NUMERIC; + $this->config->primarySortOrder = SORT_ASC; + $this->sorter->sort($table); + + $expected = array(5, 10, 15, 20, 61, false, array(), false); + $this->assertExpectedRowsOrder($expected, $table); + + $this->config->primarySortOrder = SORT_DESC; + $this->sorter->sort($table); + + $expected = array(61, 20, 15, 10, 5, false, array(), false); + $this->assertExpectedRowsOrder($expected, $table); + } + + public function test_sort_sortNatural_ShoudAddEmptyValuesAlwaysAtTheEnd() + { + $table = $this->createDataTableFromValues(array('nintendo', null, 'abc', array(), 'DeF', 'def', false, '1210', 'piwik')); + + $this->config->primarySortFlags = SORT_NATURAL; + $this->config->primarySortOrder = SORT_ASC; + $this->sorter->sort($table); + + $expected = array('1210', 'DeF', 'abc', 'def', 'nintendo', 'piwik', false, array(), false); + $this->assertExpectedRowsOrder($expected, $table); + + $this->config->primarySortOrder = SORT_DESC; + $this->sorter->sort($table); + + $expected = array('piwik', 'nintendo', 'def', 'abc', 'DeF', '1210', false, array(), false); + $this->assertExpectedRowsOrder($expected, $table); + } + + public function test_sort_ShoudIgnoreASecondColumnSort_IfDisabled() + { + $table = $this->createDataTableFromValues(array('abc', 'abc', 'abc', 'abc', 'abc')); + + $this->config->primarySortFlags = SORT_NATURAL; + $this->config->isSecondaryColumnSortEnabled = false; + $this->sorter->sort($table); + + // we make sure the labels order did not change neither when ASC nor DESC + $expected = array('My Label 0', 'My Label 1', 'My Label 2', 'My Label 3', 'My Label 4'); + $this->assertExpectedRowsOrder($expected, $table, 'label'); + + $this->config->secondarySortOrder = SORT_DESC; + $this->sorter->sort($table); + + $this->assertExpectedRowsOrder($expected, $table, 'label'); + } + + public function test_sort_ShoudIgnoreASecondColumnSort_IfSortIsNumericButNoSecondaryColumnIsSet() + { + $table = $this->createDataTableFromValues(array('abc', 'abc', 'abc', 'abc', 'abc')); + + $this->config->primarySortFlags = SORT_NUMERIC; + $this->sorter->sort($table); + + // we make sure the labels order did not change neither when ASC nor DESC + $expected = array('My Label 0', 'My Label 1', 'My Label 2', 'My Label 3', 'My Label 4'); + $this->assertExpectedRowsOrder($expected, $table, 'label'); + + $this->config->secondarySortOrder = SORT_DESC; + $this->sorter->sort($table); + + $this->assertExpectedRowsOrder($expected, $table, 'label'); + } + + public function test_sort_ShoudSortBySecondColumn_IfSortedNumeric() + { + $table = $this->createDataTableFromValues(array('abc', 'abc', 'abc', 'abc', 'abc')); + + $this->config->primarySortFlags = SORT_NUMERIC; + $this->config->secondaryColumnToSort = 'label'; + $this->config->secondarySortOrder = SORT_ASC; + $this->config->secondarySortFlags = SORT_NATURAL; + + $this->sorter->sort($table); + + // we make sure the labels order did not change neither when ASC nor DESC + $expected = array('My Label 0', 'My Label 1', 'My Label 2', 'My Label 3', 'My Label 4'); + $this->assertExpectedRowsOrder($expected, $table, 'label'); + + $this->config->secondarySortOrder = SORT_DESC; + $this->sorter->sort($table); + + $expected = array_reverse($expected); + $this->assertExpectedRowsOrder($expected, $table, 'label'); + } + + public function test_sort_ShoudSortEmptyValues_BySecondColumn_IfSortedNumeric() + { + $table = $this->createDataTableFromValues(array(null, null, null, null, null)); + + $this->config->primarySortFlags = SORT_NUMERIC; + $this->config->secondaryColumnToSort = 'label'; + $this->config->secondarySortOrder = SORT_ASC; + $this->config->secondarySortFlags = SORT_NATURAL; + + $this->sorter->sort($table); + + // we make sure the labels order did not change neither when ASC nor DESC + $expected = array('My Label 0', 'My Label 1', 'My Label 2', 'My Label 3', 'My Label 4'); + $this->assertExpectedRowsOrder($expected, $table, 'label'); + + $this->config->secondarySortOrder = SORT_DESC; + $this->sorter->sort($table); + + $expected = array_reverse($expected); + $this->assertExpectedRowsOrder($expected, $table, 'label'); + } + + private function assertExpectedRowsOrder($expectedValuesOrder, $table, $column = 'nb_visits') + { + foreach ($table->getRows() as $index => $row) { + $this->assertSame($expectedValuesOrder[$index], $row->getColumn($column)); + } + } + + private function createDataTableFromValues($values) + { + $rows = array(); + foreach ($values as $index => $value) { + $rows[] = array('nb_visits' => $value, 'label' => 'My Label ' . $index); + } + + return $this->createDataTable($rows); + } + + private function createDataTable($rows) + { + $table = new DataTable(); + foreach ($rows as $columns) { + $table->addRow($this->createRow($columns)); + } + return $table; + } + + private function createRow($columns) + { + return new Row(array(Row::COLUMNS => $columns)); + } + +} \ No newline at end of file