Force inconsistencies in API output to maintain backwards compatibility. format_processed_metrics query param changed to format_metrics. If set to 'bc', inconsistencies are enforced. Inconsistency logic is placed in a special Piwik\API\Inconsistencies class to allow it to be removed w/ ease in 3.0.

4 files changed, 41 insertions, 22 deletions

diff --git a/core/API/DataTablePostProcessor.php b/core/API/DataTablePostProcessor.php
index 444d3a1f2b..9a49c3f8ad 100644
--- a/core/API/DataTablePostProcessor.php
+++ b/core/API/DataTablePostProcessor.php
@@ -17,6 +17,7 @@ use Piwik\DataTable;
 use Piwik\DataTable\DataTableInterface;
 use Piwik\DataTable\Filter\PivotByDimension;
 use Piwik\Metrics\Formatter;
+use Piwik\Plugin\ProcessedMetric;
 use Piwik\Plugin\Report;
 
 /**
@@ -50,6 +51,11 @@ class DataTablePostProcessor
     private $apiMethod;
 
     /**
+     * @var Inconsistencies
+     */
+    private $apiInconsistencies;
+
+    /**
      * Constructor.
      */
     public function __construct($apiModule, $apiMethod, $request)
@@ -59,6 +65,7 @@ class DataTablePostProcessor
         $this->request = $request;
 
         $this->report = Report::factory($apiModule, $apiMethod);
+        $this->apiInconsistencies = new Inconsistencies();
     }
 
     /**
@@ -71,7 +78,7 @@ class DataTablePostProcessor
      * @param bool $applyFormatting Whether to format processed metrics or not.
      * @return DataTableInterface A new data table.
      */
-    public function process(DataTableInterface $dataTable, $applyFormatting = true)
+    public function process(DataTableInterface $dataTable)
     {
         // TODO: when calculating metrics before hand, only calculate for needed metrics, not all.
         $dataTable = $this->applyPivotByFilter($dataTable);
@@ -90,9 +97,7 @@ class DataTablePostProcessor
         $dataTable = $this->applyRequestedColumnDeletion($dataTable);
         $dataTable = $this->applyLabelFilter($dataTable);
 
-        if ($applyFormatting) {
-            $dataTable = $this->applyProcessedMetricsFormatting($dataTable);
-        }
+        $dataTable = $this->applyProcessedMetricsFormatting($dataTable);
 
         return $dataTable;
     }
@@ -265,20 +270,29 @@ class DataTablePostProcessor
     }
 
     /**
-     * TODO: remove applyFormatting parameter
-     *
      * @param DataTableInterface $dataTable
      * @param Formatter|null $formatter
      * @return DataTableInterface
      */
     public function applyProcessedMetricsFormatting($dataTable, Formatter $formatter = null)
     {
-        if ($formatter === null)
-        {
+        $formatMetrics = Common::getRequestVar('format_metrics', 0, 'string', $this->request);
+        if ($formatMetrics == '0') {
+            return $dataTable;
+        }
+
+        if ($formatter === null) {
             $formatter = new Formatter();
         }
 
-        $dataTable->filter(array($this, 'formatProcessedMetrics'), array($formatter));
+        // in Piwik 2.X & below, metrics are not formatted in API responses except for percents.
+        // this code implements this inconsistency
+        $metricsToFormat = null;
+        if ($formatMetrics === 'bc') {
+            $metricsToFormat = $this->apiInconsistencies->getPercentMetricsToFormat();
+        }
+
+        $dataTable->filter(array($this, 'formatProcessedMetrics'), array($formatter, $metricsToFormat));
         return $dataTable;
     }
 
@@ -346,19 +360,23 @@ class DataTablePostProcessor
     /**
      * public for use as callback.
      */
-    public function formatProcessedMetrics(DataTable $dataTable, Formatter $formatter)
+    public function formatProcessedMetrics(DataTable $dataTable, Formatter $formatter, $metricsToFormat = null)
     {
-        if ($dataTable->getMetadata(self::PROCESSED_METRICS_FORMATTED_FLAG)) {
-            return;
-        }
-
         $processedMetrics = Report::getProcessedMetricsFor($dataTable, $this->report);
-        if (empty($processedMetrics)) {
+        if (empty($processedMetrics)
+            || $dataTable->getMetadata(self::PROCESSED_METRICS_FORMATTED_FLAG)
+        ) {
             return;
         }
 
         $dataTable->setMetadata(self::PROCESSED_METRICS_FORMATTED_FLAG, true);
 
+        if ($metricsToFormat !== null) {
+            $processedMetrics = array_filter($processedMetrics, function (ProcessedMetric $metric) use ($metricsToFormat) {
+                return in_array($metric->getName(), $metricsToFormat);
+            });
+        }
+
         foreach ($processedMetrics as $name => $processedMetric) {
             if (!$processedMetric->beforeFormat($this->report, $dataTable)) {
                 continue;
@@ -372,7 +390,7 @@ class DataTablePostProcessor
 
                 $subtable = $row->getSubtable();
                 if (!empty($subtable)) {
-                    $this->formatProcessedMetrics($subtable, $formatter);
+                    $this->formatProcessedMetrics($subtable, $formatter, $metricsToFormat);
                 }
             }
         }
diff --git a/core/API/Request.php b/core/API/Request.php
index 5a52653c19..610cf80935 100644
--- a/core/API/Request.php
+++ b/core/API/Request.php
@@ -92,6 +92,11 @@ class Request
             if (!empty($requestRaw['segment'])) {
                 $defaultRequest['segment'] = $requestRaw['segment'];
             }
+
+            // TODO: comment on why this code is here, and make sure to mention in Inconsistencies class
+            if (empty($defaultRequest['format_metrics'])) {
+                $defaultRequest['format_metrics'] = 'bc';
+            }
         }
 
         $requestArray = $defaultRequest;
diff --git a/core/API/ResponseBuilder.php b/core/API/ResponseBuilder.php
index 97c27693db..4ebe50305d 100644
--- a/core/API/ResponseBuilder.php
+++ b/core/API/ResponseBuilder.php
@@ -164,10 +164,8 @@ class ResponseBuilder
 
     private function handleDataTable(DataTableInterface $datatable)
     {
-        $applyFormatting = Common::getRequestVar('force_format_processed_metrics', !($this->apiRenderer instanceof Original), null, $this->request) == 1;
-
         $postProcessor = new DataTablePostProcessor($this->apiModule, $this->apiMethod, $this->request);
-        $datatable = $postProcessor->process($datatable, $applyFormatting);
+        $datatable = $postProcessor->process($datatable);
 
         return $this->apiRenderer->renderDataTable($datatable);
     }
diff --git a/core/Plugin/Visualization.php b/core/Plugin/Visualization.php
index c82812b9f7..91a79d2a58 100644
--- a/core/Plugin/Visualization.php
+++ b/core/Plugin/Visualization.php
@@ -164,10 +164,9 @@ class Visualization extends ViewDataTable
         $this->overrideSomeConfigPropertiesIfNeeded();
 
         try {
-
             $this->beforeLoadDataTable();
 
-            $this->loadDataTableFromAPI(array('disable_generic_filters' => 1));
+            $this->loadDataTableFromAPI(array('disable_generic_filters' => 1, 'format_metrics' => 0));
             $this->postDataTableLoadedFromAPI();
 
             $requestPropertiesAfterLoadDataTable = $this->requestConfig->getProperties();
@@ -178,7 +177,6 @@ class Visualization extends ViewDataTable
             $this->beforeRender();
 
             $this->logMessageIfRequestPropertiesHaveChanged($requestPropertiesAfterLoadDataTable);
-
         } catch (NoAccessException $e) {
             throw $e;
         } catch (\Exception $e) {