diff options
author | Matthieu Aubry <matt@piwik.org> | 2015-02-11 08:11:53 +0300 |
---|---|---|
committer | Matthieu Aubry <matt@piwik.org> | 2015-02-11 08:11:53 +0300 |
commit | 75e1cf15f871ca85c84a597536e75348a1c96de9 (patch) | |
tree | 5089e28f73714d5fd1c31d89d313a9b5b38698c3 | |
parent | cd3f96bc51d742442f25f7285b5c906052524446 (diff) | |
parent | 1e4e73c5aa9cb05d3ea202e74d3c588124527957 (diff) |
Merge pull request #7162 from piwik/6809
Some performance improvements for the all websites dashboard
-rw-r--r-- | plugins/API/ProcessedReport.php | 5 | ||||
-rwxr-xr-x | plugins/MultiSites/API.php | 128 | ||||
-rw-r--r-- | tests/PHPUnit/System/BackwardsCompatibility1XTest.php | 12 | ||||
-rw-r--r-- | tests/PHPUnit/System/expected/test_BackwardsCompatibility1XTest__MultiSites.getAll_day.xml | 17 |
4 files changed, 106 insertions, 56 deletions
diff --git a/plugins/API/ProcessedReport.php b/plugins/API/ProcessedReport.php index bd1a885f4d..7c4bd46c9e 100644 --- a/plugins/API/ProcessedReport.php +++ b/plugins/API/ProcessedReport.php @@ -674,8 +674,9 @@ class ProcessedReport // if we handle MultiSites.getAll we do not always have the same idSite but different ones for // each site, see https://github.com/piwik/piwik/issues/5006 $idSiteForRow = $idSite; - if ($row->getMetadata('idsite') && is_numeric($row->getMetadata('idsite'))) { - $idSiteForRow = (int) $row->getMetadata('idsite'); + $idSiteMetadata = $row->getMetadata('idsite'); + if ($idSiteMetadata && is_numeric($idSiteMetadata)) { + $idSiteForRow = (int) $idSiteMetadata; } $prettyValue = self::getPrettyValue($formatter, $idSiteForRow, $columnName, $columnValue, $htmlAllowed = false); diff --git a/plugins/MultiSites/API.php b/plugins/MultiSites/API.php index fbc3068d2b..c85dc94024 100755 --- a/plugins/MultiSites/API.php +++ b/plugins/MultiSites/API.php @@ -87,13 +87,14 @@ class API extends \Piwik\Plugin\API { Piwik::checkUserHasSomeViewAccess(); - $idSites = $this->getSitesIdFromPattern($pattern); + $sites = $this->getSitesIdFromPattern($pattern, $_restrictSitesToLogin); - if (empty($idSites)) { + if (empty($sites)) { return new DataTable(); } + return $this->buildDataTable( - $idSites, + $sites, $period, $date, $segment, @@ -106,27 +107,56 @@ class API extends \Piwik\Plugin\API /** * Fetches the list of sites which names match the string pattern * - * @param $pattern + * @param string $pattern + * @param bool $_restrictSitesToLogin * @return array|string */ - private function getSitesIdFromPattern($pattern) + private function getSitesIdFromPattern($pattern, $_restrictSitesToLogin) { - $idSites = 'all'; + // First clear cache + Site::clearCache(); + if (empty($pattern)) { - return $idSites; - } - $idSites = array(); - $sites = Request::processRequest('SitesManager.getPatternMatchSites', - array('pattern' => $pattern, - // added because caller could overwrite these - 'serialize' => 0, - 'format' => 'original')); - if (!empty($sites)) { - foreach ($sites as $site) { - $idSites[] = $site['idsite']; + + /** @var Scheduler $scheduler */ + $scheduler = StaticContainer::getContainer()->get('Piwik\Scheduler\Scheduler'); + // Then, warm the cache with only the data we should have access to + if (Piwik::hasUserSuperUserAccess() + // Hack: when this API function is called as a Scheduled Task, Super User status is enforced. + // This means this function would return ALL websites in all cases. + // Instead, we make sure that only the right set of data is returned + && !$scheduler->isRunningTask() + ) { + APISitesManager::getInstance()->getAllSites(); + } else { + APISitesManager::getInstance()->getSitesWithAtLeastViewAccess($limit = false, $_restrictSitesToLogin); + } + + } else { + $sites = Request::processRequest('SitesManager.getPatternMatchSites', + array('pattern' => $pattern, + // added because caller could overwrite these + 'showColumns' => '', + 'hideColumns' => '', + 'serialize' => 0, + 'format' => 'original')); + + if (!empty($sites)) { + $idSites = array(); + foreach ($sites as $site) { + $idSites[] = $site['idsite']; + } + + $model = new ModelSitesManager(); + $sites = $model->getSitesFromIds($idSites); // getPatternMatchSites does not return all sites information... + Site::setSitesFromArray($sites); } } - return $idSites; + + // Both calls above have called Site::setSitesFromArray. We now get these sites: + $sitesToProblablyAdd = Site::getSites(); + + return $sitesToProblablyAdd; } /** @@ -145,8 +175,11 @@ class API extends \Piwik\Plugin\API public function getOne($idSite, $period, $date, $segment = false, $_restrictSitesToLogin = false, $enhanced = false) { Piwik::checkUserHasViewAccess($idSite); + + $sites = $this->getSiteFromId($idSite); + return $this->buildDataTable( - $idSite, + $sites, $period, $date, $segment, @@ -156,37 +189,26 @@ class API extends \Piwik\Plugin\API ); } - private function buildDataTable($idSitesOrIdSite, $period, $date, $segment, $_restrictSitesToLogin, $enhanced, $multipleWebsitesRequested) + private function getSiteFromId($idSite) { - $allWebsitesRequested = ($idSitesOrIdSite == 'all'); - if ($allWebsitesRequested) { - // First clear cache - Site::clearCache(); - /** @var Scheduler $scheduler */ - $scheduler = StaticContainer::getContainer()->get('Piwik\Scheduler\Scheduler'); - // Then, warm the cache with only the data we should have access to - if (Piwik::hasUserSuperUserAccess() - // Hack: when this API function is called as a Scheduled Task, Super User status is enforced. - // This means this function would return ALL websites in all cases. - // Instead, we make sure that only the right set of data is returned - && !$scheduler->isRunningTask() - ) { - APISitesManager::getInstance()->getAllSites(); - } else { - APISitesManager::getInstance()->getSitesWithAtLeastViewAccess($limit = false, $_restrictSitesToLogin); + $idSite = (int) $idSite; + $sites = array(APISitesManager::getInstance()->getSiteFromId($idSite)); + + return $sites; + } + + private function buildDataTable($sitesToProblablyAdd, $period, $date, $segment, $_restrictSitesToLogin, $enhanced, $multipleWebsitesRequested) + { + $idSites = array(); + if (!empty($sitesToProblablyAdd)) { + foreach ($sitesToProblablyAdd as $site) { + $idSites[] = $site['idsite']; } - // Both calls above have called Site::setSitesFromArray. We now get these sites: - $sitesToProblablyAdd = Site::getSites(); - } else if (is_array($idSitesOrIdSite)) { - $model = new ModelSitesManager(); - $sitesToProblablyAdd = $model->getSitesFromIds($idSitesOrIdSite); - } else { - $sitesToProblablyAdd = array(APISitesManager::getInstance()->getSiteFromId($idSitesOrIdSite)); } // build the archive type used to query archive data $archive = Archive::build( - $idSitesOrIdSite, + $idSites, $period, $date, $segment, @@ -211,7 +233,10 @@ class API extends \Piwik\Plugin\API // $dataTable instanceOf Set $dataTable = $archive->getDataTableFromNumeric($fieldsToGet); - $dataTable = $this->mergeDataTableMapAndPopulateLabel($idSitesOrIdSite, $multipleWebsitesRequested, $dataTable); + if ($multipleWebsitesRequested && count($idSites) === 1 && Range::isMultiplePeriod($date, $period)) { + } else { + $dataTable = $this->mergeDataTableMapAndPopulateLabel($idSites, $multipleWebsitesRequested, $dataTable); + } if ($dataTable instanceof DataTable\Map) { foreach ($dataTable->getDataTables() as $table) { @@ -237,11 +262,15 @@ class API extends \Piwik\Plugin\API $dataTable->setMetadata(self::getLastPeriodMetadataName('date'), $lastPeriod); } - $pastArchive = Archive::build($idSitesOrIdSite, $period, $strLastDate, $segment, $_restrictSitesToLogin); + $pastArchive = Archive::build($idSites, $period, $strLastDate, $segment, $_restrictSitesToLogin); $pastData = $pastArchive->getDataTableFromNumeric($fieldsToGet); - $pastData = $this->mergeDataTableMapAndPopulateLabel($idSitesOrIdSite, $multipleWebsitesRequested, $pastData); + if ($multipleWebsitesRequested && count($idSites) === 1 && Range::isMultiplePeriod($date, $period)) { + + } else { + $pastData = $this->mergeDataTableMapAndPopulateLabel($idSites, $multipleWebsitesRequested, $pastData); + } // use past data to calculate evolution percentages $this->calculateEvolutionPercentages($dataTable, $pastData, $apiMetrics); @@ -259,14 +288,9 @@ class API extends \Piwik\Plugin\API $dataTable->queueFilter('ColumnDelete', array('label')); } - Site::clearCache(); - // replace record names with user friendly metric names $dataTable->queueFilter('ReplaceColumnNames', array($columnNameRewrites)); - // Ensures data set sorted, for Metadata output - $dataTable->filter('Sort', array(self::NB_VISITS_METRIC, 'desc', $naturalSort = false)); - // filter rows without visits // note: if only one website is queried and there are no visits, we can not remove the row otherwise // ResponseBuilder throws 'Call to a member function getColumns() on a non-object' diff --git a/tests/PHPUnit/System/BackwardsCompatibility1XTest.php b/tests/PHPUnit/System/BackwardsCompatibility1XTest.php index fa8de721a3..de34f10739 100644 --- a/tests/PHPUnit/System/BackwardsCompatibility1XTest.php +++ b/tests/PHPUnit/System/BackwardsCompatibility1XTest.php @@ -104,7 +104,10 @@ class BackwardsCompatibility1XTest extends SystemTestCase // the Action.getPageTitles test fails for unknown reason, so skipping it // eg. https://travis-ci.org/piwik/piwik/jobs/24449365 - 'Action.getPageTitles' + 'Action.getPageTitles', + + // the label column is not the first column here + 'MultiSites.getAll' ); return array( @@ -127,7 +130,12 @@ class BackwardsCompatibility1XTest extends SystemTestCase 'periods' => array('range'), 'disableArchiving' => true)), array('VisitFrequency.get', array('idSite' => $idSite, 'date' => '2012-03-03,2012-12-12', 'periods' => array('month'), - 'testSuffix' => '_multipleOldNew', 'disableArchiving' => true)) + 'testSuffix' => '_multipleOldNew', 'disableArchiving' => true)), + array('MultiSites.getAll', array('idSite' => $idSite, 'date' => $dateTime, + 'disableArchiving' => true, + 'otherRequestParameters' => array( + 'hideColumns' => 'nb_users', + ))), ); } } diff --git a/tests/PHPUnit/System/expected/test_BackwardsCompatibility1XTest__MultiSites.getAll_day.xml b/tests/PHPUnit/System/expected/test_BackwardsCompatibility1XTest__MultiSites.getAll_day.xml new file mode 100644 index 0000000000..01a5e25002 --- /dev/null +++ b/tests/PHPUnit/System/expected/test_BackwardsCompatibility1XTest__MultiSites.getAll_day.xml @@ -0,0 +1,17 @@ +<?xml version="1.0" encoding="utf-8" ?> +<result> + <row> + <nb_visits>2</nb_visits> + <nb_actions>8</nb_actions> + <nb_pageviews>4</nb_pageviews> + <revenue>43</revenue> + <label>new name</label> + <visits_evolution>100%</visits_evolution> + <actions_evolution>100%</actions_evolution> + <pageviews_evolution>100%</pageviews_evolution> + <revenue_evolution>100%</revenue_evolution> + <group /> + <main_url>http://site.com</main_url> + <idsite>1</idsite> + </row> +</result>
\ No newline at end of file |