diff options
-rw-r--r-- | core/API/Request.php | 10 | ||||
-rw-r--r-- | core/Archive.php | 32 | ||||
-rw-r--r-- | core/Archive/Parameters.php | 13 | ||||
-rw-r--r-- | core/ArchiveProcessor.php | 34 | ||||
-rw-r--r-- | core/ArchiveProcessor/Loader.php | 6 | ||||
-rw-r--r-- | core/ArchiveProcessor/Parameters.php | 8 | ||||
-rw-r--r-- | core/ArchiveProcessor/Rules.php | 31 | ||||
-rw-r--r-- | core/DataAccess/ArchiveSelector.php | 41 | ||||
-rw-r--r-- | core/DataAccess/ArchiveWriter.php | 2 | ||||
-rw-r--r-- | core/DataTable.php | 35 | ||||
-rw-r--r-- | plugins/Actions/API.php | 28 | ||||
-rwxr-xr-x | tests/PHPUnit/Integration/OneVisitorOneWebsite_SeveralDaysDateRange_ArchivingTestsTest.php | 52 | ||||
-rw-r--r-- | tests/PHPUnit/Integration/expected/test_oneVisitor_oneWebsite_severalDays_DateRange__Actions.getPageUrls_range.xml | 39 | ||||
-rw-r--r-- | tests/PHPUnit/Integration/expected/test_oneVisitor_oneWebsite_severalDays_DateRange_periodIsRange_flattened___Actions.getPageUrls_range.xml | 13 |
14 files changed, 225 insertions, 119 deletions
diff --git a/core/API/Request.php b/core/API/Request.php index 0fc4d313d0..68cd3fff1d 100644 --- a/core/API/Request.php +++ b/core/API/Request.php @@ -366,7 +366,15 @@ class Request // we have to load all the child subtables. return Common::getRequestVar('filter_column_recursive', false) !== false && Common::getRequestVar('filter_pattern_recursive', false) !== false - && Common::getRequestVar('flat', false) === false; + && !self::shouldLoadFlatten(); + } + + /** + * @return bool + */ + public static function shouldLoadFlatten() + { + return Common::getRequestVar('flat', false) == 1; } /** diff --git a/core/Archive.php b/core/Archive.php index db1ca62ded..3f13396599 100644 --- a/core/Archive.php +++ b/core/Archive.php @@ -192,9 +192,10 @@ class Archive * or date range (ie, 'YYYY-MM-DD,YYYY-MM-DD'). * @param bool|false|string $segment Segment definition or false if no segment should be used. {@link Piwik\Segment} * @param bool|false|string $_restrictSitesToLogin Used only when running as a scheduled task. + * @param bool $skipAggregationOfSubTables Whether the archive, when it is processed, should also aggregate all sub-tables * @return Archive */ - public static function build($idSites, $period, $strDate, $segment = false, $_restrictSitesToLogin = false) + public static function build($idSites, $period, $strDate, $segment = false, $_restrictSitesToLogin = false, $skipAggregationOfSubTables = false) { $websiteIds = Site::getIdSitesFromIdSitesString($idSites, $_restrictSitesToLogin); @@ -209,7 +210,7 @@ class Archive $segment = new Segment($segment, $websiteIds); $idSiteIsAll = $idSites == self::REQUEST_ALL_WEBSITES_FLAG; $isMultipleDate = Period::isMultiplePeriod($strDate, $period); - return Archive::factory($segment, $allPeriods, $websiteIds, $idSiteIsAll, $isMultipleDate); + return Archive::factory($segment, $allPeriods, $websiteIds, $idSiteIsAll, $isMultipleDate, $skipAggregationOfSubTables); } /** @@ -231,9 +232,11 @@ class Archive * @param bool $isMultipleDate Whether multiple dates are being queried or not. If true, then * the result of querying functions will be indexed by period, * regardless of whether `count($periods) == 1`. + * @param bool $skipAggregationOfSubTables Whether the archive should skip aggregation of all sub-tables + * * @return Archive */ - public static function factory(Segment $segment, array $periods, array $idSites, $idSiteIsAll = false, $isMultipleDate = false) + public static function factory(Segment $segment, array $periods, array $idSites, $idSiteIsAll = false, $isMultipleDate = false, $skipAggregationOfSubTables = false) { $forceIndexedBySite = false; $forceIndexedByDate = false; @@ -244,7 +247,7 @@ class Archive $forceIndexedByDate = true; } - $params = new Parameters($idSites, $periods, $segment); + $params = new Parameters($idSites, $periods, $segment, $skipAggregationOfSubTables); return new Archive($params, $forceIndexedBySite, $forceIndexedByDate); } @@ -432,16 +435,21 @@ class Archive * @param string $segment @see {@link build()} * @param bool $expanded If true, loads all subtables. See {@link getDataTableExpanded()} * @param int|null $idSubtable See {@link getDataTableExpanded()} + * @param bool $skipAggregationOfSubTables Whether or not we should skip the aggregation of all sub-tables and only aggregate parent DataTable. * @param int|null $depth See {@link getDataTableExpanded()} * @return DataTable|DataTable\Map See {@link getDataTable()} and * {@link getDataTableExpanded()} for more * information */ public static function getDataTableFromArchive($name, $idSite, $period, $date, $segment, $expanded, - $idSubtable = null, $depth = null) + $idSubtable = null, $skipAggregationOfSubTables = false, $depth = null) { Piwik::checkUserHasViewAccess($idSite); - $archive = Archive::build($idSite, $period, $date, $segment); + + if($skipAggregationOfSubTables && ($expanded || $idSubtable)) { + throw new \Exception("Not expected to skipAggregationOfSubTables when expanded=1 or idSubtable is set."); + } + $archive = Archive::build($idSite, $period, $date, $segment, $_restrictSitesToLogin = false, $skipAggregationOfSubTables); if ($idSubtable === false) { $idSubtable = null; } @@ -620,7 +628,7 @@ class Archive private function cacheArchiveIdsWithoutLaunching($plugins) { $idarchivesByReport = ArchiveSelector::getArchiveIds( - $this->params->getIdSites(), $this->params->getPeriods(), $this->params->getSegment(), $plugins); + $this->params->getIdSites(), $this->params->getPeriods(), $this->params->getSegment(), $plugins, $this->params->isSkipAggregationOfSubTables()); // initialize archive ID cache for each report foreach ($plugins as $plugin) { @@ -644,7 +652,13 @@ class Archive */ private function getDoneStringForPlugin($plugin) { - return Rules::getDoneStringFlagFor($this->params->getIdSites(), $this->params->getSegment(), $this->getPeriodLabel(), $plugin); + return Rules::getDoneStringFlagFor( + $this->params->getIdSites(), + $this->params->getSegment(), + $this->getPeriodLabel(), + $plugin, + $this->params->isSkipAggregationOfSubTables() + ); } private function getPeriodLabel() @@ -776,7 +790,7 @@ class Archive */ private function prepareArchive(array $archiveGroups, Site $site, Period $period) { - $parameters = new ArchiveProcessor\Parameters($site, $period, $this->params->getSegment()); + $parameters = new ArchiveProcessor\Parameters($site, $period, $this->params->getSegment(), $this->params->isSkipAggregationOfSubTables()); $archiveLoader = new ArchiveProcessor\Loader($parameters); $periodString = $period->getRangeString(); diff --git a/core/Archive/Parameters.php b/core/Archive/Parameters.php index 0e2654c0f7..1f700819ee 100644 --- a/core/Archive/Parameters.php +++ b/core/Archive/Parameters.php @@ -36,16 +36,22 @@ class Parameters */ private $segment; + /** + * @var bool + */ + private $skipAggregationOfSubTables; + public function getSegment() { return $this->segment; } - public function __construct($idSites, $periods, Segment $segment) + public function __construct($idSites, $periods, Segment $segment, $skipAggregationOfSubTables) { $this->idSites = $idSites; $this->periods = $periods; $this->segment = $segment; + $this->skipAggregationOfSubTables = $skipAggregationOfSubTables; } public function getPeriods() @@ -58,5 +64,10 @@ class Parameters return $this->idSites; } + public function isSkipAggregationOfSubTables() + { + return $this->skipAggregationOfSubTables; + } + } diff --git a/core/ArchiveProcessor.php b/core/ArchiveProcessor.php index f578673d01..20c4c2df80 100644 --- a/core/ArchiveProcessor.php +++ b/core/ArchiveProcessor.php @@ -199,8 +199,14 @@ class ArchiveProcessor $table = $this->aggregateDataTableRecord($recordName, $columnsAggregationOperation, $columnsToRenameAfterAggregation); - $nameToCount[$recordName]['level0'] = $table->getRowsCount(); - $nameToCount[$recordName]['recursive'] = $table->getRowsCountRecursive(); + $rowsCount = $table->getRowsCount(); + $nameToCount[$recordName]['level0'] = $rowsCount; + + $rowsCountRecursive = $rowsCount; + if($this->isAggregateSubTables()) { + $rowsCountRecursive = $table->getRowsCountRecursive(); + } + $nameToCount[$recordName]['recursive'] = $rowsCountRecursive; $blob = $table->getSerialized($maximumRowsInDataTableLevelZero, $maximumRowsInSubDataTable, $columnToSortByBeforeTruncation); Common::destroy($table); @@ -322,7 +328,15 @@ class ArchiveProcessor */ protected function aggregateDataTableRecord($name, $columnsAggregationOperation = null, $columnsToRenameAfterAggregation = null) { - $dataTable = $this->getArchive()->getDataTableExpanded($name, $idSubTable = null, $depth = null, $addMetadataSubtableId = false); + if($this->isAggregateSubTables()) { + // By default we shall aggregate all sub-tables. + $dataTable = $this->getArchive()->getDataTableExpanded($name, $idSubTable = null, $depth = null, $addMetadataSubtableId = false); + } else { + // In some cases (eg. Actions plugin when period=range), + // for better performance we will only aggregate the parent table + $dataTable = $this->getArchive()->getDataTable($name, $idSubTable = null); + } + $dataTable = $this->getAggregatedDataTableMap($dataTable, $columnsAggregationOperation); $this->renameColumnsAfterAggregation($dataTable, $columnsToRenameAfterAggregation); return $dataTable; @@ -402,7 +416,7 @@ class ArchiveProcessor // as $date => $tableToSum $this->aggregatedDataTableMapsAsOne($data, $table); } else { - $table->addDataTable($data); + $table->addDataTable($data, $this->isAggregateSubTables()); } return $table; } @@ -418,7 +432,7 @@ class ArchiveProcessor if($tableToAggregate instanceof Map) { $this->aggregatedDataTableMapsAsOne($tableToAggregate, $aggregated); } else { - $aggregated->addDataTable($tableToAggregate); + $aggregated->addDataTable($tableToAggregate, $this->isAggregateSubTables()); } } } @@ -430,7 +444,7 @@ class ArchiveProcessor $columnsToRenameAfterAggregation = self::$columnsToRenameAfterAggregation; } foreach ($columnsToRenameAfterAggregation as $oldName => $newName) { - $table->renameColumn($oldName, $newName); + $table->renameColumn($oldName, $newName, $this->isAggregateSubTables()); } } @@ -464,4 +478,12 @@ class ArchiveProcessor } return $metrics; } + + /** + * @return bool + */ + protected function isAggregateSubTables() + { + return !$this->getParams()->isSkipAggregationOfSubTables(); + } } diff --git a/core/ArchiveProcessor/Loader.php b/core/ArchiveProcessor/Loader.php index b3ac4cd3f4..03a054a617 100644 --- a/core/ArchiveProcessor/Loader.php +++ b/core/ArchiveProcessor/Loader.php @@ -163,12 +163,8 @@ class Loader if ($this->isArchivingForcedToTrigger()) { return $noArchiveFound; } - $site = $this->params->getSite(); - $period = $this->params->getPeriod(); - $segment = $this->params->getSegment(); - $requestedPlugin = $this->params->getRequestedPlugin(); - $idAndVisits = ArchiveSelector::getArchiveIdAndVisits($site, $period, $segment, $minDatetimeArchiveProcessedUTC, $requestedPlugin); + $idAndVisits = ArchiveSelector::getArchiveIdAndVisits($this->params, $minDatetimeArchiveProcessedUTC); if (!$idAndVisits) { return $noArchiveFound; } diff --git a/core/ArchiveProcessor/Parameters.php b/core/ArchiveProcessor/Parameters.php index 1aac335994..9d9a908822 100644 --- a/core/ArchiveProcessor/Parameters.php +++ b/core/ArchiveProcessor/Parameters.php @@ -48,11 +48,12 @@ class Parameters * * @ignore */ - public function __construct(Site $site, Period $period, Segment $segment) + public function __construct(Site $site, Period $period, Segment $segment, $skipAggregationOfSubTables = false) { $this->site = $site; $this->period = $period; $this->segment = $segment; + $this->skipAggregationOfSubTables = $skipAggregationOfSubTables; } /** @@ -168,6 +169,11 @@ class Parameters return count($this->getIdSites()) == 1; } + public function isSkipAggregationOfSubTables() + { + return $this->skipAggregationOfSubTables; + } + public function logStatusDebug($isTemporary) { $temporary = 'definitive archive'; diff --git a/core/ArchiveProcessor/Rules.php b/core/ArchiveProcessor/Rules.php index 9aef10302d..05b37a93af 100644 --- a/core/ArchiveProcessor/Rules.php +++ b/core/ArchiveProcessor/Rules.php @@ -50,10 +50,10 @@ class Rules * @param string $plugin * @return string */ - public static function getDoneStringFlagFor(array $idSites, $segment, $periodLabel, $plugin) + public static function getDoneStringFlagFor(array $idSites, $segment, $periodLabel, $plugin, $isSkipAggregationOfSubTables) { if (!self::shouldProcessReportsAllPlugins($idSites, $segment, $periodLabel)) { - return self::getDoneFlagArchiveContainsOnePlugin($segment, $plugin); + return self::getDoneFlagArchiveContainsOnePlugin($segment, $plugin, $isSkipAggregationOfSubTables); } return self::getDoneFlagArchiveContainsAllPlugins($segment); } @@ -98,9 +98,10 @@ class Rules return $segmentsToProcess; } - private static function getDoneFlagArchiveContainsOnePlugin(Segment $segment, $plugin) + private static function getDoneFlagArchiveContainsOnePlugin(Segment $segment, $plugin, $isSkipAggregationOfSubTables = false) { - return 'done' . $segment->getHash() . '.' . $plugin; + $partial = self::isFlagArchivePartial($plugin, $isSkipAggregationOfSubTables); + return 'done' . $segment->getHash() . '.' . $plugin . $partial ; } private static function getDoneFlagArchiveContainsAllPlugins(Segment $segment) @@ -109,17 +110,35 @@ class Rules } /** + * @param $plugin + * @param $isSkipAggregationOfSubTables + * @return string + */ + private static function isFlagArchivePartial($plugin, $isSkipAggregationOfSubTables) + { + $partialArchive = ''; + if ($plugin != "VisitsSummary" // VisitsSummary is always called when segmenting and should not have its own .partial archive + && $isSkipAggregationOfSubTables + ) { + $partialArchive = '.partial'; + } + return $partialArchive; + } + + /** * @param array $plugins * @param $segment * @return array */ - public static function getDoneFlags(array $plugins, $segment) + public static function getDoneFlags(array $plugins, Segment $segment, $isSkipAggregationOfSubTables) { $doneFlags = array(); $doneAllPlugins = self::getDoneFlagArchiveContainsAllPlugins($segment); $doneFlags[$doneAllPlugins] = $doneAllPlugins; + + $plugins = array_unique($plugins); foreach ($plugins as $plugin) { - $doneOnePlugin = self::getDoneFlagArchiveContainsOnePlugin($segment, $plugin); + $doneOnePlugin = self::getDoneFlagArchiveContainsOnePlugin($segment, $plugin, $isSkipAggregationOfSubTables); $doneFlags[$plugin] = $doneOnePlugin; } return $doneFlags; diff --git a/core/DataAccess/ArchiveSelector.php b/core/DataAccess/ArchiveSelector.php index eb994ac756..4ba9b21b01 100644 --- a/core/DataAccess/ArchiveSelector.php +++ b/core/DataAccess/ArchiveSelector.php @@ -42,13 +42,13 @@ class ArchiveSelector const NB_VISITS_CONVERTED_RECORD_LOOKED_UP = "nb_visits_converted"; - static public function getArchiveIdAndVisits(Site $site, Period $period, Segment $segment, $minDatetimeArchiveProcessedUTC, $requestedPlugin) + static public function getArchiveIdAndVisits(ArchiveProcessor\Parameters $params, $minDatetimeArchiveProcessedUTC) { - $dateStart = $period->getDateStart(); - $bindSQL = array($site->getId(), + $dateStart = $params->getPeriod()->getDateStart(); + $bindSQL = array($params->getSite()->getId(), $dateStart->toString('Y-m-d'), - $period->getDateEnd()->toString('Y-m-d'), - $period->getId(), + $params->getPeriod()->getDateEnd()->toString('Y-m-d'), + $params->getPeriod()->getId(), ); $timeStampWhere = ''; @@ -57,9 +57,12 @@ class ArchiveSelector $bindSQL[] = Date::factory($minDatetimeArchiveProcessedUTC)->getDatetime(); } - $pluginOrVisitsSummary = array("VisitsSummary", $requestedPlugin); - $pluginOrVisitsSummary = array_unique($pluginOrVisitsSummary); - $sqlWhereArchiveName = self::getNameCondition($pluginOrVisitsSummary, $segment); + $requestedPlugin = $params->getRequestedPlugin(); + $segment = $params->getSegment(); + $isSkipAggregationOfSubTables = $params->isSkipAggregationOfSubTables(); + + $plugins = array("VisitsSummary", $requestedPlugin); + $sqlWhereArchiveName = self::getNameCondition($plugins, $segment, $isSkipAggregationOfSubTables); $sqlQuery = " SELECT idarchive, value, name, date1 as startDate FROM " . ArchiveTableCreator::getNumericTable($dateStart) . "`` @@ -77,8 +80,8 @@ class ArchiveSelector return false; } - $idArchive = self::getMostRecentIdArchiveFromResults($segment, $requestedPlugin, $results); - $idArchiveVisitsSummary = self::getMostRecentIdArchiveFromResults($segment, "VisitsSummary", $results); + $idArchive = self::getMostRecentIdArchiveFromResults($segment, $requestedPlugin, $isSkipAggregationOfSubTables, $results); + $idArchiveVisitsSummary = self::getMostRecentIdArchiveFromResults($segment, "VisitsSummary", $isSkipAggregationOfSubTables, $results); list($visits, $visitsConverted) = self::getVisitsMetricsFromResults($idArchive, $idArchiveVisitsSummary, $results); @@ -116,10 +119,10 @@ class ArchiveSelector return array($visits, $visitsConverted); } - protected static function getMostRecentIdArchiveFromResults(Segment $segment, $requestedPlugin, $results) + protected static function getMostRecentIdArchiveFromResults(Segment $segment, $requestedPlugin, $isSkipAggregationOfSubTables, $results) { $idArchive = false; - $namesRequestedPlugin = Rules::getDoneFlags(array($requestedPlugin), $segment); + $namesRequestedPlugin = Rules::getDoneFlags(array($requestedPlugin), $segment, $isSkipAggregationOfSubTables); foreach ($results as $result) { if ($idArchive === false && in_array($result['name'], $namesRequestedPlugin) @@ -138,6 +141,7 @@ class ArchiveSelector * @param array $periods * @param Segment $segment * @param array $plugins List of plugin names for which data is being requested. + * @param bool $isSkipAggregationOfSubTables Whether we are selecting an archive that may be partial (no sub-tables) * @return array Archive IDs are grouped by archive name and period range, ie, * array( * 'VisitsSummary.done' => array( @@ -145,12 +149,12 @@ class ArchiveSelector * ) * ) */ - static public function getArchiveIds($siteIds, $periods, $segment, $plugins) + static public function getArchiveIds($siteIds, $periods, $segment, $plugins, $isSkipAggregationOfSubTables = false) { $getArchiveIdsSql = "SELECT idsite, name, date1, date2, MAX(idarchive) as idarchive FROM %s WHERE %s - AND " . self::getNameCondition($plugins, $segment) . " + AND " . self::getNameCondition($plugins, $segment, $isSkipAggregationOfSubTables) . " AND idsite IN (" . implode(',', $siteIds) . ") GROUP BY idsite, date1, date2"; @@ -270,20 +274,21 @@ class ArchiveSelector * * @param array $plugins * @param Segment $segment + * @param bool $isSkipAggregationOfSubTables * @return string */ - static private function getNameCondition(array $plugins, $segment) + static private function getNameCondition(array $plugins, Segment $segment, $isSkipAggregationOfSubTables) { // the flags used to tell how the archiving process for a specific archive was completed, // if it was completed - $doneFlags = Rules::getDoneFlags($plugins, $segment); + $doneFlags = Rules::getDoneFlags($plugins, $segment, $isSkipAggregationOfSubTables); $allDoneFlags = "'" . implode("','", $doneFlags) . "'"; // create the SQL to find archives that are DONE - return "(name IN ($allDoneFlags)) AND " . + return "((name IN ($allDoneFlags)) AND " . " (value = '" . ArchiveWriter::DONE_OK . "' OR " . - " value = '" . ArchiveWriter::DONE_OK_TEMPORARY . "')"; + " value = '" . ArchiveWriter::DONE_OK_TEMPORARY . "'))"; } static public function purgeOutdatedArchives(Date $dateStart) diff --git a/core/DataAccess/ArchiveWriter.php b/core/DataAccess/ArchiveWriter.php index bd11053089..5650a6a828 100644 --- a/core/DataAccess/ArchiveWriter.php +++ b/core/DataAccess/ArchiveWriter.php @@ -66,7 +66,7 @@ class ArchiveWriter $this->segment = $params->getSegment(); $this->period = $params->getPeriod(); $idSites = array($this->idSite); - $this->doneFlag = Rules::getDoneStringFlagFor($idSites, $this->segment, $this->period->getLabel(), $params->getRequestedPlugin()); + $this->doneFlag = Rules::getDoneStringFlagFor($idSites, $this->segment, $this->period->getLabel(), $params->getRequestedPlugin(), $params->isSkipAggregationOfSubTables()); $this->isArchiveTemporary = $isArchiveTemporary; $this->dateStart = $this->period->getDateStart(); diff --git a/core/DataTable.php b/core/DataTable.php index 9edcaec373..0f8809821d 100644 --- a/core/DataTable.php +++ b/core/DataTable.php @@ -469,7 +469,7 @@ class DataTable implements DataTableInterface * * @param \Piwik\DataTable $tableToSum */ - public function addDataTable(DataTable $tableToSum) + public function addDataTable(DataTable $tableToSum, $doAggregateSubTables = true) { if($tableToSum instanceof Simple) { if($tableToSum->getRowsCount() > 1) { @@ -479,7 +479,7 @@ class DataTable implements DataTableInterface $this->aggregateRowFromSimpleTable($row); } else { foreach ($tableToSum->getRows() as $row) { - $this->aggregateRowWithLabel($row); + $this->aggregateRowWithLabel($row, $doAggregateSubTables); } } } @@ -891,12 +891,15 @@ class DataTable implements DataTableInterface * @param string $oldName Old column name. * @param string $newName New column name. */ - public function renameColumn($oldName, $newName) + public function renameColumn($oldName, $newName, $doRenameColumnsOfSubTables = true) { foreach ($this->getRows() as $row) { $row->renameColumn($oldName, $newName); - if (($idSubDataTable = $row->getIdSubDataTable()) !== null) { - Manager::getInstance()->getTable($idSubDataTable)->renameColumn($oldName, $newName); + + if($doRenameColumnsOfSubTables) { + if (($idSubDataTable = $row->getIdSubDataTable()) !== null) { + Manager::getInstance()->getTable($idSubDataTable)->renameColumn($oldName, $newName); + } } } if (!is_null($this->summaryRow)) { @@ -1582,7 +1585,7 @@ class DataTable implements DataTableInterface * @param $row * @throws \Exception */ - protected function aggregateRowWithLabel(Row $row) + protected function aggregateRowWithLabel(Row $row, $doAggregateSubTables = true) { $labelToLookFor = $row->getColumn('label'); if ($labelToLookFor === false) { @@ -1598,15 +1601,17 @@ class DataTable implements DataTableInterface } else { $rowFound->sumRow($row, $copyMeta = true, $this->getMetadata(self::COLUMN_AGGREGATION_OPS_METADATA_NAME)); - // if the row to add has a subtable whereas the current row doesn't - // we simply add it (cloning the subtable) - // if the row has the subtable already - // then we have to recursively sum the subtables - if (($idSubTable = $row->getIdSubDataTable()) !== null) { - $subTable = Manager::getInstance()->getTable($idSubTable); - $subTable->metadata[self::COLUMN_AGGREGATION_OPS_METADATA_NAME] - = $this->getMetadata(self::COLUMN_AGGREGATION_OPS_METADATA_NAME); - $rowFound->sumSubtable($subTable); + if($doAggregateSubTables) { + // if the row to add has a subtable whereas the current row doesn't + // we simply add it (cloning the subtable) + // if the row has the subtable already + // then we have to recursively sum the subtables + if (($idSubTable = $row->getIdSubDataTable()) !== null) { + $subTable = Manager::getInstance()->getTable($idSubTable); + $subTable->metadata[self::COLUMN_AGGREGATION_OPS_METADATA_NAME] + = $this->getMetadata(self::COLUMN_AGGREGATION_OPS_METADATA_NAME); + $rowFound->sumSubtable($subTable); + } } } } diff --git a/plugins/Actions/API.php b/plugins/Actions/API.php index ba96ce198b..14efde7db2 100644 --- a/plugins/Actions/API.php +++ b/plugins/Actions/API.php @@ -9,6 +9,7 @@ namespace Piwik\Plugins\Actions; use Exception; +use Piwik\API\Request; use Piwik\Archive; use Piwik\Common; @@ -119,8 +120,7 @@ class API extends \Piwik\Plugin\API public function getPageUrls($idSite, $period, $date, $segment = false, $expanded = false, $idSubtable = false, $depth = false) { - $dataTable = Archive::getDataTableFromArchive( - 'Actions_actions_url', $idSite, $period, $date, $segment, $expanded, $idSubtable, $depth); + $dataTable = $this->getDataTableFromArchive('Actions_actions_url', $idSite, $period, $date, $segment, $expanded, $idSubtable, $depth); $this->filterPageDatatable($dataTable); $this->filterActionsDataTable($dataTable, $expanded); return $dataTable; @@ -207,7 +207,7 @@ class API extends \Piwik\Plugin\API public function getPageTitles($idSite, $period, $date, $segment = false, $expanded = false, $idSubtable = false) { - $dataTable = Archive::getDataTableFromArchive('Actions_actions', $idSite, $period, $date, $segment, $expanded, $idSubtable); + $dataTable = $this->getDataTableFromArchive('Actions_actions', $idSite, $period, $date, $segment, $expanded, $idSubtable); $this->filterPageDatatable($dataTable); $this->filterActionsDataTable($dataTable, $expanded); return $dataTable; @@ -248,7 +248,7 @@ class API extends \Piwik\Plugin\API public function getDownloads($idSite, $period, $date, $segment = false, $expanded = false, $idSubtable = false) { - $dataTable = Archive::getDataTableFromArchive('Actions_downloads', $idSite, $period, $date, $segment, $expanded, $idSubtable); + $dataTable = $this->getDataTableFromArchive('Actions_downloads', $idSite, $period, $date, $segment, $expanded, $idSubtable); $this->filterActionsDataTable($dataTable, $expanded); return $dataTable; } @@ -263,7 +263,7 @@ class API extends \Piwik\Plugin\API public function getOutlinks($idSite, $period, $date, $segment = false, $expanded = false, $idSubtable = false) { - $dataTable = Archive::getDataTableFromArchive('Actions_outlink', $idSite, $period, $date, $segment, $expanded, $idSubtable); + $dataTable = $this->getDataTableFromArchive('Actions_outlink', $idSite, $period, $date, $segment, $expanded, $idSubtable); $this->filterActionsDataTable($dataTable, $expanded); return $dataTable; } @@ -299,7 +299,7 @@ class API extends \Piwik\Plugin\API protected function getSiteSearchKeywordsRaw($idSite, $period, $date, $segment) { - $dataTable = Archive::getDataTableFromArchive('Actions_sitesearch', $idSite, $period, $date, $segment, $expanded = false); + $dataTable = $this->getDataTableFromArchive('Actions_sitesearch', $idSite, $period, $date, $segment, $expanded = false); return $dataTable; } @@ -396,7 +396,7 @@ class API extends \Piwik\Plugin\API if ($table === false) { // fetch the data table - $table = call_user_func_array(array('Piwik\Archive', 'getDataTableFromArchive'), $callBackParameters); + $table = call_user_func_array(array($this, 'getDataTableFromArchive'), $callBackParameters); if ($table instanceof DataTable\Map) { // search an array of tables, e.g. when using date=last30 @@ -460,7 +460,7 @@ class API extends \Piwik\Plugin\API // match found on this level and more levels remaining: go deeper $idSubTable = $row->getIdSubDataTable(); $callBackParameters[6] = $idSubTable; - $table = call_user_func_array(array('Piwik\Archive', 'getDataTableFromArchive'), $callBackParameters); + $table = call_user_func_array(array($this, 'getDataTableFromArchive'), $callBackParameters); return $this->doFilterPageDatatableSearch($callBackParameters, $table, $searchTree); } @@ -552,4 +552,16 @@ class API extends \Piwik\Plugin\API { $dataTable->filter('ColumnCallbackDeleteRow', array('exit_nb_visits', function ($visits) { return !strlen($visits); })); } + + protected function getDataTableFromArchive($name, $idSite, $period, $date, $segment, $expanded = false, $idSubtable = null, $depth = null) + { + $skipAggregationOfSubTables = false; + if($period == 'range' + && empty($idSubtable) + && empty($expanded) + && !Request::shouldLoadFlatten()) { + $skipAggregationOfSubTables = true; + } + return Archive::getDataTableFromArchive($name, $idSite, $period, $date, $segment, $expanded, $idSubtable, $skipAggregationOfSubTables, $depth); + } } diff --git a/tests/PHPUnit/Integration/OneVisitorOneWebsite_SeveralDaysDateRange_ArchivingTestsTest.php b/tests/PHPUnit/Integration/OneVisitorOneWebsite_SeveralDaysDateRange_ArchivingTestsTest.php index b751a1b4a5..6c8997b935 100755 --- a/tests/PHPUnit/Integration/OneVisitorOneWebsite_SeveralDaysDateRange_ArchivingTestsTest.php +++ b/tests/PHPUnit/Integration/OneVisitorOneWebsite_SeveralDaysDateRange_ArchivingTestsTest.php @@ -54,18 +54,34 @@ class Test_Piwik_Integration_OneVisitorOneWebsite_SeveralDaysDateRange_Archiving for ($i = 0; $i <= 1; $i++) { foreach ($segments as $segment) { $result[] = array($apiToCall, array('idSite' => $idSite, 'date' => '2010-12-15,2011-01-15', - 'periods' => array('range'), 'segment' => $segment)); + 'periods' => array('range'), + 'segment' => $segment, + 'otherRequestParameters' => array( + 'flat' => '0', + 'expanded' => '0' + ), + )); } } // Testing Date range in January only // Because of flat=1, this test will archive all sub-tables $result[] = array('Actions.getPageUrls', array('idSite' => $idSite, 'date' => '2011-01-01,2011-02-01', - 'periods' => array('range'), - 'otherRequestParameters' => array( - 'flat' => '1', - ), - 'testSuffix' => '_periodIsRange_flattened_') + 'periods' => array('range'), + 'otherRequestParameters' => array( + 'flat' => '1', + 'expanded' => '0' + ), + 'testSuffix' => '_periodIsRange_flattened_') + ); + // testing the same with expanded=1 should not create new archive records + $result[] = array('Actions.getPageUrls', array('idSite' => $idSite, 'date' => '2011-01-01,2011-02-01', + 'periods' => array('range'), + 'otherRequestParameters' => array( + 'flat' => '0', + 'expanded' => '1' + ), + 'testSuffix' => '_periodIsRange_expanded_') ); return $result; } @@ -88,8 +104,7 @@ class Test_Piwik_Integration_OneVisitorOneWebsite_SeveralDaysDateRange_Archiving $expectedActionsBlobsWhenFlattened = $expectedActionsBlobs + 3; $tests = array( - // TODO Implement fix, then remove the +3 below - 'archive_blob_2010_12' => ( ($expectedActionsBlobs+3) /*Actions*/ + 'archive_blob_2010_12' => ( $expectedActionsBlobs /*Actions*/ + 8 /* UserSettings */ + 2 /* VisitTime */) * 3, @@ -137,12 +152,31 @@ class Test_Piwik_Integration_OneVisitorOneWebsite_SeveralDaysDateRange_Archiving $countBlobs = Db::get()->fetchOne($sql); if($expectedRows != $countBlobs) { - var_export(Db::get()->fetchAll("SELECT * FROM " . Common::prefixTable($table). " WHERE period = " . Piwik::$idPeriods['range'] . " ORDER BY idarchive ASC")); + $this->printDebugWhenTestFails($table); } $this->assertEquals($expectedRows, $countBlobs, "$table expected $expectedRows, got $countBlobs"); } } + /** + * @param $table + */ + protected function printDebugWhenTestFails($table) + { + $data = Db::get()->fetchAll("SELECT * FROM " . Common::prefixTable($table) . " WHERE period = " . Piwik::$idPeriods['range'] . " ORDER BY idarchive ASC"); + var_export($data); + + $idArchives = array(); + foreach ($data as $row) { + $idArchives[] = $row['idarchive']; + } + $idArchives = array_unique($idArchives); + foreach ($idArchives as $idArchive) { + $numericTable = str_replace("blob", "numeric", Common::prefixTable($table)); + var_export(Db::get()->fetchAll("SELECT idarchive, name FROM " . $numericTable . " WHERE idarchive = ? AND name LIKE 'done%' LIMIT 1 ", $idArchive)); + } + } + } Test_Piwik_Integration_OneVisitorOneWebsite_SeveralDaysDateRange_ArchivingTests::$fixture diff --git a/tests/PHPUnit/Integration/expected/test_oneVisitor_oneWebsite_severalDays_DateRange__Actions.getPageUrls_range.xml b/tests/PHPUnit/Integration/expected/test_oneVisitor_oneWebsite_severalDays_DateRange__Actions.getPageUrls_range.xml index c9d0409752..62ee4df14d 100644 --- a/tests/PHPUnit/Integration/expected/test_oneVisitor_oneWebsite_severalDays_DateRange__Actions.getPageUrls_range.xml +++ b/tests/PHPUnit/Integration/expected/test_oneVisitor_oneWebsite_severalDays_DateRange__Actions.getPageUrls_range.xml @@ -43,44 +43,5 @@ <avg_time_on_page>0</avg_time_on_page> <bounce_rate>0%</bounce_rate> <exit_rate>100%</exit_rate> - <subtable> - <row> - <label>sub2</label> - <nb_visits>3</nb_visits> - <nb_hits>3</nb_hits> - <sum_time_spent>0</sum_time_spent> - <exit_nb_visits>3</exit_nb_visits> - <avg_time_on_page>0</avg_time_on_page> - <bounce_rate>0%</bounce_rate> - <exit_rate>100%</exit_rate> - <subtable> - <row> - <label>sub3</label> - <nb_visits>3</nb_visits> - <nb_hits>3</nb_hits> - <sum_time_spent>0</sum_time_spent> - <exit_nb_visits>3</exit_nb_visits> - <avg_time_on_page>0</avg_time_on_page> - <bounce_rate>0%</bounce_rate> - <exit_rate>100%</exit_rate> - <subtable> - <row> - <label>/news</label> - <nb_visits>3</nb_visits> - <nb_hits>3</nb_hits> - <sum_time_spent>0</sum_time_spent> - <exit_nb_visits>3</exit_nb_visits> - <sum_daily_nb_uniq_visitors>2</sum_daily_nb_uniq_visitors> - <sum_daily_exit_nb_uniq_visitors>2</sum_daily_exit_nb_uniq_visitors> - <avg_time_on_page>0</avg_time_on_page> - <bounce_rate>0%</bounce_rate> - <exit_rate>100%</exit_rate> - <url>http://example.org/sub1/sub2/sub3/news</url> - </row> - </subtable> - </row> - </subtable> - </row> - </subtable> </row> </result>
\ No newline at end of file diff --git a/tests/PHPUnit/Integration/expected/test_oneVisitor_oneWebsite_severalDays_DateRange_periodIsRange_flattened___Actions.getPageUrls_range.xml b/tests/PHPUnit/Integration/expected/test_oneVisitor_oneWebsite_severalDays_DateRange_periodIsRange_flattened___Actions.getPageUrls_range.xml index bd8b87ef40..1f11de6d95 100644 --- a/tests/PHPUnit/Integration/expected/test_oneVisitor_oneWebsite_severalDays_DateRange_periodIsRange_flattened___Actions.getPageUrls_range.xml +++ b/tests/PHPUnit/Integration/expected/test_oneVisitor_oneWebsite_severalDays_DateRange_periodIsRange_flattened___Actions.getPageUrls_range.xml @@ -34,4 +34,17 @@ <exit_rate>100%</exit_rate> <url>http://example.org/news</url> </row> + <row> + <label>sub1/sub2/sub3/news</label> + <nb_visits>2</nb_visits> + <nb_hits>2</nb_hits> + <sum_time_spent>0</sum_time_spent> + <exit_nb_visits>2</exit_nb_visits> + <sum_daily_nb_uniq_visitors>2</sum_daily_nb_uniq_visitors> + <sum_daily_exit_nb_uniq_visitors>2</sum_daily_exit_nb_uniq_visitors> + <avg_time_on_page>0</avg_time_on_page> + <bounce_rate>0%</bounce_rate> + <exit_rate>100%</exit_rate> + <url>http://example.org/sub1/sub2/sub3/news</url> + </row> </result>
\ No newline at end of file |