From 1510afcf0467d7736bbbd197a15722c8a7b65b27 Mon Sep 17 00:00:00 2001
From: Benaka Moorthi <benaka.moorthi@gmail.com>
Date: Mon, 9 Sep 2013 15:01:27 -0400
Subject: [PATCH] Fixes #4063, allow csv renderer to handle arbitrarily nested
data structures at least a little bit gracefully.
---
core/DataTable/Renderer/Csv.php | 81 +++++++++++++--------
tests/PHPUnit/Integration/CsvExportTest.php | 8 ++
tests/PHPUnit/IntegrationTestCase.php | 9 ++-
3 files changed, 66 insertions(+), 32 deletions(-)
diff --git a/core/DataTable/Renderer/Csv.php b/core/DataTable/Renderer/Csv.php
index 48b107877f..a9c569680f 100644
--- a/core/DataTable/Renderer/Csv.php
+++ b/core/DataTable/Renderer/Csv.php
@@ -202,36 +202,11 @@ class Csv extends Renderer
}
}
}
+
$csv = array();
+ $allColumns = array();
foreach ($table->getRows() as $row) {
- $csvRow = array();
-
- $columns = $row->getColumns();
- foreach ($columns as $name => $value) {
- //goals => array( 'idgoal=1' =>array(..), 'idgoal=2' => array(..))
- if (is_array($value)) {
- foreach ($value as $key => $subValues) {
- if (is_array($subValues)) {
- foreach ($subValues as $subKey => $subValue) {
- if ($this->translateColumnNames) {
- $subName = $name != 'goals' ? $name . ' ' . $key
- : Piwik_Translate('Goals_GoalX', $key);
- $columnName = $this->translateColumnName($subKey)
- . ' (' . $subName . ')';
- } else {
- // goals_idgoal=1
- $columnName = $name . "_" . $key . "_" . $subKey;
- }
- $allColumns[$columnName] = true;
- $csvRow[$columnName] = $subValue;
- }
- }
- }
- } else {
- $allColumns[$name] = true;
- $csvRow[$name] = $value;
- }
- }
+ $csvRow = $this->flattenColumnArray($row->getColumns());
if ($this->exportMetadata) {
$metadata = $row->getMetadata();
@@ -246,7 +221,6 @@ class Csv extends Renderer
$name = 'metadata_' . $name;
}
- $allColumns[$name] = true;
$csvRow[$name] = $value;
}
}
@@ -260,6 +234,10 @@ class Csv extends Renderer
}
}
+ foreach ($csvRow as $name => $value) {
+ $allColumns[$name] = true;
+ }
+
$csv[] = $csvRow;
}
@@ -378,4 +356,47 @@ class Csv extends Renderer
@header('Content-Disposition: attachment; filename="' . $fileName . '"');
Piwik::overrideCacheControlHeaders();
}
-}
+
+ /**
+ * Flattens an array of column values so they can be outputted as CSV (which does not support
+ * nested structures).
+ */
+ private function flattenColumnArray($columns, &$csvRow = array(), $csvColumnNameTemplate = '%s')
+ {
+ foreach ($columns as $name => $value) {
+ $csvName = sprintf($csvColumnNameTemplate, $this->getCsvColumnName($name));
+
+ if (is_array($value)) {
+ // if we're translating column names and this is an array of arrays, the column name
+ // format becomes a bit more complicated. also in this case, we assume $value is not
+ // nested beyond 2 levels (ie, array(0 => array(0 => 1, 1 => 2)), but not array(
+ // 0 => array(0 => array(), 1 => array())) )
+ if ($this->translateColumnNames
+ && is_array(reset($value))
+ ) {
+ foreach ($value as $level1Key => $level1Value) {
+ $inner = $name == 'goals' ? Piwik_Translate('Goals_GoalX', $level1Key) : $name . ' ' . $level1Key;
+ $columnNameTemplate = '%s (' . $inner . ')';
+
+ $this->flattenColumnArray($level1Value, $csvRow, $columnNameTemplate);
+ }
+ } else {
+ $this->flattenColumnArray($value, $csvRow, $csvName . '_%s');
+ }
+ } else {
+ $csvRow[$csvName] = $value;
+ }
+ }
+
+ return $csvRow;
+ }
+
+ private function getCsvColumnName($name)
+ {
+ if ($this->translateColumnNames) {
+ return $this->translateColumnName($name);
+ } else {
+ return $name;
+ }
+ }
+}
\ No newline at end of file
diff --git a/tests/PHPUnit/Integration/CsvExportTest.php b/tests/PHPUnit/Integration/CsvExportTest.php
index 7e84ababdb..a5a9dfd475 100755
--- a/tests/PHPUnit/Integration/CsvExportTest.php
+++ b/tests/PHPUnit/Integration/CsvExportTest.php
@@ -44,6 +44,14 @@ class Test_Piwik_Integration_CsvExport extends IntegrationTestCase
'otherRequestParameters' => $deExtraParam,
'language' => 'de',
'testSuffix' => '_xp1_inner1_trans-de')),
+
+ // TODO: we cannot currently test the csv output of Live.getLastVisitsDetails. The API method includes
+ // random output. Normally we remove this when it's in XML output, but it's harder to do that w/
+ // CSV output. For now, we use compareOutput => false to just check that no errors result.
+ array('Live.getLastVisitsDetails', array('idSite' => $idSite,
+ 'date' => $dateTime,
+ 'format' => 'csv',
+ 'compareOutput' => false))
);
}
diff --git a/tests/PHPUnit/IntegrationTestCase.php b/tests/PHPUnit/IntegrationTestCase.php
index 19b4a92ca6..1925561934 100755
--- a/tests/PHPUnit/IntegrationTestCase.php
+++ b/tests/PHPUnit/IntegrationTestCase.php
@@ -710,7 +710,7 @@ abstract class IntegrationTestCase extends PHPUnit_Framework_TestCase
}
}
- protected function _testApiUrl($testName, $apiId, $requestUrl)
+ protected function _testApiUrl($testName, $apiId, $requestUrl, $compareOutput = true)
{
$isTestLogImportReverseChronological = strpos($testName, 'ImportedInRandomOrderTest') === false;
$isLiveMustDeleteDates = (strpos($requestUrl, 'Live.getLastVisits') !== false
@@ -743,6 +743,10 @@ abstract class IntegrationTestCase extends PHPUnit_Framework_TestCase
file_put_contents($processedFilePath, $response);
+ if (!$compareOutput) {
+ return;
+ }
+
$expected = $this->loadExpectedFile($expectedFilePath);
if (empty($expected)) {
print("The expected file is not found at '$expectedFilePath'. The Processed response was:");
@@ -1014,7 +1018,8 @@ abstract class IntegrationTestCase extends PHPUnit_Framework_TestCase
isset($params['fileExtension']) ? $params['fileExtension'] : false);
foreach ($requestUrls as $apiId => $requestUrl) {
- $this->_testApiUrl($testName . $testSuffix, $apiId, $requestUrl);
+ $this->_testApiUrl($testName . $testSuffix, $apiId, $requestUrl,
+ isset($params['compareOutput']) ? $params['compareOutput'] : true);
}
// change the language back to en
--
GitLab