Merge pull request #7465 from piwik/datatable_tweaks

Various performance improvements and bugfixes.

8 files changed, 57 insertions, 49 deletions

diff --git a/core/DataTable/Filter/AddSegmentByLabel.php b/core/DataTable/Filter/AddSegmentByLabel.php
index 2e9fd693d1..d13eef97c5 100644
--- a/core/DataTable/Filter/AddSegmentByLabel.php
+++ b/core/DataTable/Filter/AddSegmentByLabel.php
@@ -63,10 +63,7 @@ class AddSegmentByLabel extends BaseFilter
         if (count($this->segments) === 1) {
             $segment = reset($this->segments);
 
-            foreach ($table->getRows() as $key => $row) {
-                if ($key == DataTable::ID_SUMMARY_ROW) {
-                    continue;
-                }
+            foreach ($table->getRowsWithoutSummaryRow() as $key => $row) {
 
                 $label = $row->getColumn('label');
 
@@ -78,10 +75,7 @@ class AddSegmentByLabel extends BaseFilter
             $numSegments  = count($this->segments);
             $conditionAnd = ';';
 
-            foreach ($table->getRows() as $key => $row) {
-                if ($key == DataTable::ID_SUMMARY_ROW) {
-                    continue;
-                }
+            foreach ($table->getRowsWithoutSummaryRow() as $key => $row) {
 
                 $label = $row->getColumn('label');
                 if (!empty($label)) {
diff --git a/core/DataTable/Filter/ColumnCallbackAddMetadata.php b/core/DataTable/Filter/ColumnCallbackAddMetadata.php
index c7945839cb..805a5db5ab 100644
--- a/core/DataTable/Filter/ColumnCallbackAddMetadata.php
+++ b/core/DataTable/Filter/ColumnCallbackAddMetadata.php
@@ -63,10 +63,13 @@ class ColumnCallbackAddMetadata extends BaseFilter
      */
     public function filter($table)
     {
-        foreach ($table->getRows() as $key => $row) {
-            if (!$this->applyToSummaryRow && $key == DataTable::ID_SUMMARY_ROW) {
-                continue;
-            }
+        if ($this->applyToSummaryRow) {
+            $rows = $table->getRows();
+        } else {
+            $rows = $table->getRowsWithoutSummaryRow();
+        }
+
+        foreach ($rows as $key => $row) {
 
             $parameters = array();
             foreach ($this->columnsToRead as $columnsToRead) {
diff --git a/core/DataTable/Filter/ColumnCallbackReplace.php b/core/DataTable/Filter/ColumnCallbackReplace.php
index 4d78831e88..84397d7a2c 100644
--- a/core/DataTable/Filter/ColumnCallbackReplace.php
+++ b/core/DataTable/Filter/ColumnCallbackReplace.php
@@ -99,6 +99,11 @@ class ColumnCallbackReplace extends BaseFilter
                 $this->filterSubTable($row);
             }
         }
+
+        if (in_array('label', $this->columnsToFilter)) {
+            // we need to force rebuilding the index
+            $table->setLabelsHaveChanged();
+        }
     }
 
     /**
diff --git a/core/DataTable/Filter/GroupBy.php b/core/DataTable/Filter/GroupBy.php
index 2ac79a6de9..07597fe6b5 100755
--- a/core/DataTable/Filter/GroupBy.php
+++ b/core/DataTable/Filter/GroupBy.php
@@ -76,11 +76,7 @@ class GroupBy extends BaseFilter
         $groupByRows = array();
         $nonGroupByRowIds = array();
 
-        foreach ($table->getRows() as $rowId => $row) {
-            // skip the summary row
-            if ($rowId == DataTable::ID_SUMMARY_ROW) {
-                continue;
-            }
+        foreach ($table->getRowsWithoutSummaryRow() as $rowId => $row) {
 
             $groupByColumnValue = $row->getColumn($this->groupByColumn);
             $groupByValue = $groupByColumnValue;
@@ -104,6 +100,10 @@ class GroupBy extends BaseFilter
             }
         }
 
+        if ($this->groupByColumn === 'label') {
+            $table->setLabelsHaveChanged();
+        }
+
         // delete the unneeded rows.
         $table->deleteRows($nonGroupByRowIds);
     }
diff --git a/core/DataTable/Filter/MetadataCallbackAddMetadata.php b/core/DataTable/Filter/MetadataCallbackAddMetadata.php
index 7f6aaef123..414f939183 100644
--- a/core/DataTable/Filter/MetadataCallbackAddMetadata.php
+++ b/core/DataTable/Filter/MetadataCallbackAddMetadata.php
@@ -62,11 +62,13 @@ class MetadataCallbackAddMetadata extends BaseFilter
      */
     public function filter($table)
     {
-        foreach ($table->getRows() as $key => $row) {
-            if (!$this->applyToSummaryRow && $key == DataTable::ID_SUMMARY_ROW) {
-                continue;
-            }
+        if ($this->applyToSummaryRow) {
+            $rows = $table->getRows();
+        } else {
+            $rows = $table->getRowsWithoutSummaryRow();
+        }
 
+        foreach ($rows as $key => $row) {
             $params = array();
             foreach ($this->metadataToRead as $name) {
                 $params[] = $row->getMetadata($name);
diff --git a/core/DataTable/Filter/ReplaceSummaryRowLabel.php b/core/DataTable/Filter/ReplaceSummaryRowLabel.php
index 1e550f6e3f..168794d097 100644
--- a/core/DataTable/Filter/ReplaceSummaryRowLabel.php
+++ b/core/DataTable/Filter/ReplaceSummaryRowLabel.php
@@ -53,18 +53,18 @@ class ReplaceSummaryRowLabel extends BaseFilter
      */
     public function filter($table)
     {
-        $rows = $table->getRows();
-        foreach ($rows as $id => $row) {
-            if ($row->getColumn('label') == DataTable::LABEL_SUMMARY_ROW
-                || $id == DataTable::ID_SUMMARY_ROW
-            ) {
+        $row = $table->getRowFromId(DataTable::ID_SUMMARY_ROW);
+        if ($row) {
+            $row->setColumn('label', $this->newLabel);
+        } else {
+            $row = $table->getRowFromLabel(DataTable::LABEL_SUMMARY_ROW);
+            if ($row) {
                 $row->setColumn('label', $this->newLabel);
-                break;
             }
         }
 
         // recurse
-        foreach ($rows as $row) {
+        foreach ($table->getRowsWithoutSummaryRow() as $row) {
             $subTable = $row->getSubtable();
             if ($subTable) {
                 $this->filter($subTable);
diff --git a/core/DataTable/Filter/Sort.php b/core/DataTable/Filter/Sort.php
index 7e91bb338d..3b11ec370c 100644
--- a/core/DataTable/Filter/Sort.php
+++ b/core/DataTable/Filter/Sort.php
@@ -21,11 +21,14 @@ use Piwik\Metrics;
  *
  * @api
  */
-class
-Sort extends BaseFilter
+class Sort extends BaseFilter
 {
     protected $columnToSort;
     protected $order;
+    protected $sign;
+
+    const ORDER_DESC = 'desc';
+    const ORDER_ASC  = 'asc';
 
     /**
      * Constructor.
@@ -36,7 +39,7 @@ Sort extends BaseFilter
      * @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.
      */
-    public function __construct($table, $columnToSort, $order = 'desc', $naturalSort = true, $recursiveSort = false)
+    public function __construct($table, $columnToSort, $order = 'desc', $naturalSort = true, $recursiveSort = true)
     {
         parent::__construct($table);
 
@@ -68,8 +71,8 @@ Sort extends BaseFilter
     /**
      * Sorting method used for sorting numbers
      *
-     * @param Row $a
-     * @param Row $b
+     * @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)
@@ -80,20 +83,20 @@ Sort extends BaseFilter
             } else {
                 return -1 * $this->sign * strnatcasecmp($rowA[1], $rowB[1]);
             }
-        } elseif (!isset($rowB[0])) {
-            return -1;
+        } elseif (!isset($rowB[0]) && !isset($rowA[0])) {
+            return -1 * $this->sign * strnatcasecmp($rowA[1], $rowB[1]);
         } elseif (!isset($rowA[0])) {
             return 1;
         }
 
-        return 0;
+        return -1;
     }
 
     /**
      * Sorting method used for sorting values natural
      *
-     * @param mixed $a
-     * @param mixed $b
+     * @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
      */
     function naturalSort($rowA, $rowB)
@@ -119,8 +122,8 @@ Sort extends BaseFilter
     /**
      * Sorting method used for sorting values
      *
-     * @param mixed $a
-     * @param mixed $b
+     * @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
      */
     function sortString($rowA, $rowB)
@@ -208,12 +211,11 @@ Sort extends BaseFilter
             return;
         }
 
-        $rows = $table->getRows();
-        if (count($rows) == 0) {
+        if (!$table->getRowsCount()) {
             return;
         }
 
-        $row = current($rows);
+        $row = $table->getFirstRow();
         if ($row === false) {
             return;
         }
@@ -268,10 +270,10 @@ Sort extends BaseFilter
 
         $sortedRows = array();
         foreach ($values as $key => $value) {
-            $sortedRows[$key] = $rows[$key];
+            $sortedRows[] = $rows[$key];
         }
 
-        $table->setRows(array_values($sortedRows));
+        $table->setRows($sortedRows);
 
         unset($rows);
         unset($sortedRows);
diff --git a/core/DataTable/Filter/Truncate.php b/core/DataTable/Filter/Truncate.php
index a8fa0bd08f..04b4cef2a8 100644
--- a/core/DataTable/Filter/Truncate.php
+++ b/core/DataTable/Filter/Truncate.php
@@ -90,16 +90,18 @@ class Truncate extends BaseFilter
      */
     private function addSummaryRow($table)
     {
-        $table->filter('Sort', array($this->columnToSortByBeforeTruncating, 'desc'));
-
         if ($table->getRowsCount() <= $this->truncateAfter + 1) {
             return;
         }
 
+        $table->filter('Sort', array($this->columnToSortByBeforeTruncating, 'desc', $naturalSort = true, $recursiveSort = false));
+
         $rows   = array_values($table->getRows());
         $count  = $table->getRowsCount();
         $newRow = new Row(array(Row::COLUMNS => array('label' => DataTable::LABEL_SUMMARY_ROW)));
 
+        $aggregationOps = $table->getMetadata(DataTable::COLUMN_AGGREGATION_OPS_METADATA_NAME);
+
         for ($i = $this->truncateAfter; $i < $count; $i++) {
             if (!isset($rows[$i])) {
                 // case when the last row is a summary row, it is not indexed by $cout but by DataTable::ID_SUMMARY_ROW
@@ -107,10 +109,10 @@ class Truncate extends BaseFilter
 
                 //FIXME: I'm not sure why it could return false, but it was reported in: http://forum.piwik.org/read.php?2,89324,page=1#msg-89442
                 if ($summaryRow) {
-                    $newRow->sumRow($summaryRow, $enableCopyMetadata = false, $table->getMetadata(DataTable::COLUMN_AGGREGATION_OPS_METADATA_NAME));
+                    $newRow->sumRow($summaryRow, $enableCopyMetadata = false, $aggregationOps);
                 }
             } else {
-                $newRow->sumRow($rows[$i], $enableCopyMetadata = false, $table->getMetadata(DataTable::COLUMN_AGGREGATION_OPS_METADATA_NAME));
+                $newRow->sumRow($rows[$i], $enableCopyMetadata = false, $aggregationOps);
             }
         }