Run queued filters after generic filters making visualizations much faster.

13 files changed, 194 insertions, 118 deletions

diff --git a/core/API/DataTableManipulator/LabelFilter.php b/core/API/DataTableManipulator/LabelFilter.php
index cd8300991b..0d4e11772a 100644
--- a/core/API/DataTableManipulator/LabelFilter.php
+++ b/core/API/DataTableManipulator/LabelFilter.php
@@ -147,6 +147,8 @@ class LabelFilter extends DataTableManipulator
         }
         $variations[] = $label;
 
+        $variations = array_unique($variations);
+
         return $variations;
     }
 
diff --git a/core/API/DataTablePostProcessor.php b/core/API/DataTablePostProcessor.php
index b13fff1439..dfa0434da6 100644
--- a/core/API/DataTablePostProcessor.php
+++ b/core/API/DataTablePostProcessor.php
@@ -59,6 +59,9 @@ class DataTablePostProcessor
      */
     private $formatter;
 
+    private $callbackBeforeGenericFilters;
+    private $callbackAfterGenericFilters;
+
     /**
      * Constructor.
      */
@@ -66,11 +69,31 @@ class DataTablePostProcessor
     {
         $this->apiModule = $apiModule;
         $this->apiMethod = $apiMethod;
-        $this->request = $request;
+        $this->setRequest($request);
 
         $this->report = Report::factory($apiModule, $apiMethod);
         $this->apiInconsistencies = new Inconsistencies();
-        $this->formatter = new Formatter();
+        $this->setFormatter(new Formatter());
+    }
+
+    public function setFormatter(Formatter $formatter)
+    {
+        $this->formatter = $formatter;
+    }
+
+    public function setRequest($request)
+    {
+        $this->request = $request;
+    }
+
+    public function setCallbackBeforeGenericFilters($callbackBeforeGenericFilters)
+    {
+        $this->callbackBeforeGenericFilters = $callbackBeforeGenericFilters;
+    }
+
+    public function setCallbackAfterGenericFilters($callbackAfterGenericFilters)
+    {
+        $this->callbackAfterGenericFilters = $callbackAfterGenericFilters;
     }
 
     /**
@@ -88,19 +111,24 @@ class DataTablePostProcessor
         $dataTable = $this->applyTotalsCalculator($dataTable);
         $dataTable = $this->applyFlattener($dataTable);
 
-        $dataTable = $this->applyGenericFilters($dataTable);
+        if ($this->callbackBeforeGenericFilters) {
+            call_user_func($this->callbackBeforeGenericFilters, $dataTable);
+        }
 
+        $dataTable = $this->applyGenericFilters($dataTable);
         $this->applyComputeProcessedMetrics($dataTable);
 
+        if ($this->callbackAfterGenericFilters) {
+            call_user_func($this->callbackAfterGenericFilters, $dataTable);
+        }
+
         // we automatically safe decode all datatable labels (against xss)
         $dataTable->queueFilter('SafeDecodeLabel');
-
         $dataTable = $this->convertSegmentValueToSegment($dataTable);
         $dataTable = $this->applyQueuedFilters($dataTable);
         $dataTable = $this->applyRequestedColumnDeletion($dataTable);
         $dataTable = $this->applyLabelFilter($dataTable);
         $dataTable = $this->applyMetricsFormatting($dataTable);
-
         return $dataTable;
     }
 
diff --git a/core/API/Proxy.php b/core/API/Proxy.php
index 0345693b62..0f8667d0d8 100644
--- a/core/API/Proxy.php
+++ b/core/API/Proxy.php
@@ -490,7 +490,7 @@ class Proxy extends Singleton
         $hideLine = trim($hideLine);
         $hideLine .= ' ';
 
-	$token = trim(strtok($hideLine, " "), "\n");
+        $token = trim(strtok($hideLine, " "), "\n");
 
         $hide = false;
 
@@ -529,18 +529,6 @@ class Proxy extends Singleton
     }
 
     /**
-     * Returns the number of required parameters (parameters without default values).
-     *
-     * @param string $class The class name
-     * @param string $name The method name
-     * @return int The number of required parameters
-     */
-    private function getNumberOfRequiredParameters($class, $name)
-    {
-        return $this->metadataArray[$class][$name]['numberOfRequiredParameters'];
-    }
-
-    /**
      * Returns true if the method is found in the API of the given class name.
      *
      * @param string $className The class name
diff --git a/core/API/Request.php b/core/API/Request.php
index 1db4009606..1c331fa7b9 100644
--- a/core/API/Request.php
+++ b/core/API/Request.php
@@ -18,6 +18,7 @@ use Piwik\SettingsServer;
 use Piwik\Url;
 use Piwik\UrlHelper;
 use Piwik\Log;
+use Piwik\Plugin\Manager as PluginManager;
 
 /**
  * Dispatches API requests to the appropriate API method.
@@ -219,11 +220,10 @@ class Request
             list($module, $method) = $this->extractModuleAndMethod($moduleMethod);
 
             list($module, $method) = self::getRenamedModuleAndAction($module, $method);
+            
+            PluginManager::getInstance()->checkIsPluginActivated($module);
 
-            if (!\Piwik\Plugin\Manager::getInstance()->isPluginActivated($module)) {
-                throw new PluginDeactivatedException($module);
-            }
-            $apiClassName = self::getClassNameAPI($module);
+            $apiClassName = $this->getClassNameAPI($module);
 
             self::reloadAuthUsingTokenAuth($this->request);
 
diff --git a/core/API/ResponseBuilder.php b/core/API/ResponseBuilder.php
index 4ebe50305d..68e448b0b4 100644
--- a/core/API/ResponseBuilder.php
+++ b/core/API/ResponseBuilder.php
@@ -25,6 +25,7 @@ class ResponseBuilder
     private $apiRenderer  = null;
     private $request      = null;
     private $sendHeader   = true;
+    private $postProcessDataTable = true;
 
     private $apiModule = false;
     private $apiMethod = false;
@@ -45,6 +46,11 @@ class ResponseBuilder
         $this->sendHeader = false;
     }
 
+    public function disableDataTablePostProcessor()
+    {
+        $this->postProcessDataTable = false;
+    }
+
     /**
      * This method processes the data resulting from the API call.
      *
@@ -164,8 +170,10 @@ class ResponseBuilder
 
     private function handleDataTable(DataTableInterface $datatable)
     {
-        $postProcessor = new DataTablePostProcessor($this->apiModule, $this->apiMethod, $this->request);
-        $datatable = $postProcessor->process($datatable);
+        if ($this->postProcessDataTable) {
+            $postProcessor = new DataTablePostProcessor($this->apiModule, $this->apiMethod, $this->request);
+            $datatable = $postProcessor->process($datatable);
+        }
 
         return $this->apiRenderer->renderDataTable($datatable);
     }
diff --git a/core/DataTable/Filter/PivotByDimension.php b/core/DataTable/Filter/PivotByDimension.php
index 61e68423e8..3eeff39b44 100644
--- a/core/DataTable/Filter/PivotByDimension.php
+++ b/core/DataTable/Filter/PivotByDimension.php
@@ -286,13 +286,12 @@ class PivotByDimension extends BaseFilter
             return null;
         }
 
-        if ($row->isSubtableLoaded()) {
-            $subtable = $row->getSubtable();
-        } else {
+        $subtable = $row->getSubtable();
+        if (!$subtable) {
             $subtable = $this->thisReport->fetchSubtable($idSubtable, $this->getRequestParamOverride($table));
         }
 
-        if ($subtable === null) { // sanity check
+        if (!$subtable) { // sanity check
             throw new Exception("Unexpected error: could not load subtable '$idSubtable'.");
         }
 
diff --git a/core/Plugin/Manager.php b/core/Plugin/Manager.php
index 6e0d5ba91a..fb11f0a664 100644
--- a/core/Plugin/Manager.php
+++ b/core/Plugin/Manager.php
@@ -21,6 +21,7 @@ use Piwik\Log;
 use Piwik\Option;
 use Piwik\Piwik;
 use Piwik\Plugin;
+use Piwik\PluginDeactivatedException;
 use Piwik\Singleton;
 use Piwik\Theme;
 use Piwik\Tracker;
@@ -266,6 +267,19 @@ class Manager extends Singleton
     }
 
     /**
+     * Checks whether the given plugin is activated, if not triggers an exception.
+     *
+     * @param  string $pluginName
+     * @throws PluginDeactivatedException
+     */
+    public function checkIsPluginActivated($pluginName)
+    {
+        if (!$this->isPluginActivated($pluginName)) {
+            throw new PluginDeactivatedException($pluginName);
+        }
+    }
+
+    /**
      * Returns `true` if plugin is loaded (in memory).
      *
      * @param string $name Name of plugin, eg, `'Acions'`.
diff --git a/core/Plugin/Metric.php b/core/Plugin/Metric.php
index 6d69350d92..4122dfeedf 100644
--- a/core/Plugin/Metric.php
+++ b/core/Plugin/Metric.php
@@ -124,10 +124,11 @@ abstract class Metric
 
             return $value;
 
-        } else {
-            $value = null;
+        } elseif (!empty($row)) {
+            
             if (array_key_exists($columnName, $row)) {
-                $value = $row[$columnName];
+                return $row[$columnName];
+
             } else {
 
                 if (empty($mappingNameToId)) {
@@ -142,10 +143,9 @@ abstract class Metric
                     }
                 }
             }
-
         }
 
-        return $value;
+        return null;
     }
 
     /**
diff --git a/core/Plugin/ViewDataTable.php b/core/Plugin/ViewDataTable.php
index 4f3b854127..a49358e48b 100644
--- a/core/Plugin/ViewDataTable.php
+++ b/core/Plugin/ViewDataTable.php
@@ -316,7 +316,7 @@ abstract class ViewDataTable implements ViewInterface
         return new VizRequest();
     }
 
-    protected function loadDataTableFromAPI($fixedRequestParams = array())
+    protected function loadDataTableFromAPI()
     {
         if (!is_null($this->dataTable)) {
             // data table is already there
@@ -324,7 +324,7 @@ abstract class ViewDataTable implements ViewInterface
             return $this->dataTable;
         }
 
-        $this->dataTable = $this->request->loadDataTableFromAPI($fixedRequestParams);
+        $this->dataTable = $this->request->loadDataTableFromAPI();
 
         return $this->dataTable;
     }
diff --git a/core/Plugin/Visualization.php b/core/Plugin/Visualization.php
index 15b468e362..3d48c24d15 100644
--- a/core/Plugin/Visualization.php
+++ b/core/Plugin/Visualization.php
@@ -10,6 +10,8 @@
 namespace Piwik\Plugin;
 
 use Piwik\API\DataTablePostProcessor;
+use Piwik\API\Proxy;
+use Piwik\API\ResponseBuilder;
 use Piwik\Common;
 use Piwik\DataTable;
 use Piwik\Date;
@@ -23,6 +25,8 @@ use Piwik\Plugins\API\API as ApiApi;
 use Piwik\Plugins\PrivacyManager\PrivacyManager;
 use Piwik\View;
 use Piwik\ViewDataTable\Manager as ViewDataTableManager;
+use Piwik\Plugin\Manager as PluginManager;
+use Piwik\API\Request as ApiRequest;
 
 /**
  * The base class for report visualizations that output HTML and use JavaScript.
@@ -173,8 +177,7 @@ class Visualization extends ViewDataTable
 
         try {
             $this->beforeLoadDataTable();
-
-            $this->loadDataTableFromAPI(array('disable_generic_filters' => 1, 'format_metrics' => 0));
+            $this->loadDataTableFromAPI();
             $this->postDataTableLoadedFromAPI();
 
             $requestPropertiesAfterLoadDataTable = $this->requestConfig->getProperties();
@@ -233,6 +236,35 @@ class Visualization extends ViewDataTable
         return $view;
     }
 
+    /**
+     * @internal
+     */
+    protected function loadDataTableFromAPI()
+    {
+        if (!is_null($this->dataTable)) {
+            // data table is already there
+            // this happens when setDataTable has been used
+            return $this->dataTable;
+        }
+
+        // we build the request (URL) to call the API
+        $request = $this->buildApiRequestArray();
+
+        $module = $this->requestConfig->getApiModuleToRequest();
+        $method = $this->requestConfig->getApiMethodToRequest();
+
+        PluginManager::getInstance()->checkIsPluginActivated($module);
+
+        $class     = ApiRequest::getClassNameAPI($module);
+        $dataTable = Proxy::getInstance()->call($class, $method, $request);
+
+        $response = new ResponseBuilder($format = 'original', $request);
+        $response->disableSendHeader();
+        $response->disableDataTablePostProcessor();
+
+        $this->dataTable = $response->getResponse($dataTable, $module, $method);
+    }
+
     private function getReportMetadata()
     {
         $request = $this->request->getRequestArray() + $_GET + $_POST;
@@ -255,11 +287,16 @@ class Visualization extends ViewDataTable
             $this->config->footer_icons = ViewDataTableManager::configureFooterIcons($this);
         }
 
-        if (!\Piwik\Plugin\Manager::getInstance()->isPluginActivated('Goals')) {
+        if (!$this->isPluginActivated('Goals')) {
             $this->config->show_goals = false;
         }
     }
 
+    private function isPluginActivated($pluginName)
+    {
+        return PluginManager::getInstance()->isPluginActivated($pluginName);
+    }
+
     /**
      * Assigns a template variable making it available in the Twig template specified by
      * {@link TEMPLATE_FILE}.
@@ -357,48 +394,39 @@ class Visualization extends ViewDataTable
 
     private function applyFilters()
     {
-        list($priorityFilters, $otherFilters) = $this->config->getFiltersToRun();
+        $postProcessor = $this->makeDataTablePostProcessor(); // must be created after requestConfig is final
+        $self = $this;
 
-        // First, filters that delete rows
-        foreach ($priorityFilters as $filter) {
-            $this->dataTable->filter($filter[0], $filter[1]);
-        }
+        $postProcessor->setCallbackBeforeGenericFilters(function (DataTable\DataTableInterface $dataTable) use ($self, $postProcessor) {
 
-        $this->beforeGenericFiltersAreAppliedToLoadedDataTable();
+            // First, filters that delete rows
+            foreach ($self->config->getPriorityFilters() as $filter) {
+                $dataTable->filter($filter[0], $filter[1]);
+            }
 
-        if (!in_array($this->requestConfig->filter_sort_column, $this->config->columns_to_display)) {
-            $hasNbUniqVisitors = in_array('nb_uniq_visitors', $this->config->columns_to_display);
-            $this->requestConfig->setDefaultSort($this->config->columns_to_display, $hasNbUniqVisitors, $this->dataTable->getColumns());
-        }
+            $self->beforeGenericFiltersAreAppliedToLoadedDataTable();
 
-        $postProcessor = $this->makeDataTablePostProcessor(); // must be created after requestConfig is final
+            if (!in_array($self->requestConfig->filter_sort_column, $self->config->columns_to_display)) {
+                $hasNbUniqVisitors = in_array('nb_uniq_visitors', $self->config->columns_to_display);
+                $columns = $dataTable->getColumns();
+                $self->requestConfig->setDefaultSort($self->config->columns_to_display, $hasNbUniqVisitors, $columns);
+            }
 
-        if (!$this->requestConfig->areGenericFiltersDisabled()) {
-            $this->dataTable = $postProcessor->applyGenericFilters($this->dataTable);
-        }
+            $postProcessor->setRequest($self->buildApiRequestArray());
+        });
 
-        $postProcessor->applyComputeProcessedMetrics($this->dataTable);
+        $postProcessor->setCallbackAfterGenericFilters(function (DataTable\DataTableInterface $dataTable) use ($self) {
 
-        $this->afterGenericFiltersAreAppliedToLoadedDataTable();
+            $self->afterGenericFiltersAreAppliedToLoadedDataTable();
 
-        // queue other filters so they can be applied later if queued filters are disabled
-        foreach ($otherFilters as $filter) {
-            $this->dataTable->queueFilter($filter[0], $filter[1]);
-        }
+            // queue other filters so they can be applied later if queued filters are disabled
+            foreach ($self->config->getPresentationFilters() as $filter) {
+                $dataTable->queueFilter($filter[0], $filter[1]);
+            }
 
-        // Finally, apply datatable filters that were queued (should be 'presentation' filters that
-        // do not affect the number of rows)
-        if (!$this->requestConfig->areQueuedFiltersDisabled()) {
-            $this->dataTable->applyQueuedFilters();
-        }
+        });
 
-        if ($this->requestConfig->shouldFormatMetrics()) {
-            $formatter = $this->metricsFormatter;
-            $report = $this->report;
-            $this->dataTable->filter(function (DataTable $table) use ($formatter, $report) {
-                $formatter->formatMetrics($table, $report);
-            });
-        }
+        $this->dataTable = $postProcessor->process($this->dataTable);
     }
 
     private function removeEmptyColumnsFromDisplay()
@@ -456,7 +484,7 @@ class Visualization extends ViewDataTable
      */
     private function hasReportBeenPurged()
     {
-        if (!\Piwik\Plugin\Manager::getInstance()->isPluginActivated('PrivacyManager')) {
+        if (!$this->isPluginActivated('PrivacyManager')) {
             return false;
         }
 
@@ -629,15 +657,14 @@ class Visualization extends ViewDataTable
 
     private function makeDataTablePostProcessor()
     {
-        $requestArray = $this->request->getRequestArray();
-        $request      = \Piwik\API\Request::getRequestArrayFromString($requestArray);
+        $request = $this->buildApiRequestArray();
+        $module  = $this->requestConfig->getApiModuleToRequest();
+        $method  = $this->requestConfig->getApiMethodToRequest();
 
-        if (false === $this->config->enable_sort) {
-            $request['filter_sort_column'] = '';
-            $request['filter_sort_order']  = '';
-        }
+        $processor = new DataTablePostProcessor($module, $method, $request);
+        $processor->setFormatter($this->metricsFormatter);
 
-        return new DataTablePostProcessor($this->requestConfig->getApiModuleToRequest(), $this->requestConfig->getApiMethodToRequest(), $request);
+        return $processor;
     }
 
     private function logMessageIfRequestPropertiesHaveChanged(array $requestPropertiesBefore)
@@ -685,4 +712,30 @@ class Visualization extends ViewDataTable
 
         return $result;
     }
+
+    /**
+     * @internal
+     * 
+     * @return array
+     */
+    public function buildApiRequestArray()
+    {
+        $requestArray = $this->request->getRequestArray();
+        $request = APIRequest::getRequestArrayFromString($requestArray);
+
+        if (false === $this->config->enable_sort) {
+            $request['filter_sort_column'] = '';
+            $request['filter_sort_order'] = '';
+        }
+
+        if (!array_key_exists('format_metrics', $request) || $request['format_metrics'] === 'bc') {
+            $request['format_metrics'] = '1';
+        }
+
+        if (!$this->requestConfig->disable_queued_filters && array_key_exists('disable_queued_filters', $request)) {
+            unset($request['disable_queued_filters']);
+        }
+
+        return $request;
+    }
 }
diff --git a/core/ViewDataTable/Config.php b/core/ViewDataTable/Config.php
index 220a253518..f4eef3c639 100644
--- a/core/ViewDataTable/Config.php
+++ b/core/ViewDataTable/Config.php
@@ -557,7 +557,7 @@ class Config
     /**
      * @ignore
      */
-    public function getFiltersToRun()
+    private function getFiltersToRun()
     {
         $priorityFilters     = array();
         $presentationFilters = array();
@@ -581,6 +581,20 @@ class Config
         return array($priorityFilters, $presentationFilters);
     }
 
+    public function getPriorityFilters()
+    {
+        $filters = $this->getFiltersToRun();
+
+        return $filters[0];
+    }
+
+    public function getPresentationFilters()
+    {
+        $filters = $this->getFiltersToRun();
+
+        return $filters[1];
+    }
+
     /**
      * Adds a related report to the {@link $related_reports} property. If the report
      * references the one that is currently being displayed, it will not be added to the related
diff --git a/core/ViewDataTable/Request.php b/core/ViewDataTable/Request.php
index 352ce25899..259d4caaa1 100644
--- a/core/ViewDataTable/Request.php
+++ b/core/ViewDataTable/Request.php
@@ -32,15 +32,11 @@ class Request
      * It builds the API request string and uses Request to call the API.
      * The requested DataTable object is stored in $this->dataTable.
      */
-    public function loadDataTableFromAPI($fixedRequestParams = array())
+    public function loadDataTableFromAPI()
     {
         // we build the request (URL) to call the API
         $requestArray = $this->getRequestArray();
 
-        foreach ($fixedRequestParams as $key => $value) {
-            $requestArray[$key] = $value;
-        }
-
         // we make the request to the API
         $request = new ApiRequest($requestArray);
 
@@ -104,6 +100,14 @@ class Request
             unset($requestArray['filter_limit']);
         }
 
+        if ($this->requestConfig->disable_generic_filters) {
+            $requestArray['disable_generic_filters'] = '0';
+        }
+
+        if ($this->requestConfig->disable_queued_filters) {
+            $requestArray['disable_queued_filters'] = 0;
+        }
+
         return $requestArray;
     }
 
diff --git a/core/ViewDataTable/RequestConfig.php b/core/ViewDataTable/RequestConfig.php
index 753ee55b98..a6c9b7bed8 100644
--- a/core/ViewDataTable/RequestConfig.php
+++ b/core/ViewDataTable/RequestConfig.php
@@ -314,35 +314,6 @@ class RequestConfig
         $this->filter_sort_order = 'desc';
     }
 
-    /**
-     * Returns `true` if queued filters have been disabled, `false` if otherwise.
-     *
-     * @return bool
-     */
-    public function areQueuedFiltersDisabled()
-    {
-        return isset($this->disable_queued_filters) && $this->disable_queued_filters;
-    }
-
-    /**
-     * Returns `true` if generic filters have been disabled, `false` if otherwise.
-     *
-     * @return bool
-     */
-    public function areGenericFiltersDisabled()
-    {
-        // if disable_generic_filters query param is set to '1', generic filters are disabled
-        if (Common::getRequestVar('disable_generic_filters', '0', 'string') == 1) {
-            return true;
-        }
-
-        if (isset($this->disable_generic_filters) && true === $this->disable_generic_filters) {
-            return true;
-        }
-
-        return false;
-    }
-
     public function getApiModuleToRequest()
     {
         list($module, $method) = explode('.', $this->apiMethodToRequestDataTable);
@@ -356,9 +327,4 @@ class RequestConfig
 
         return $method;
     }
-
-    public function shouldFormatMetrics()
-    {
-        return Common::getRequestVar('format_metrics', '1', 'string', $this->request_parameters_to_modify) != 0;
-    }
 }