Skip to content

Commit

Permalink
Refs #4278 Remove shouldArchive, make getMinTimeArchiveProcessed prot…
Browse files Browse the repository at this point in the history 
…ected
  • Loading branch information
@mattab
mattab committed Nov 4, 2013
1 parent e8bec08 commit 670b24f
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 50 deletions.
38 changes: 17 additions & 21 deletions core/ArchiveProcessor.php
View file
Expand Up @@ -157,11 +157,22 @@ class ArchiveProcessor
*/
private $segment = null;

/**
* @var Archive
*/
protected $archive = null;

public function __construct(Period $period, Site $site, Segment $segment)
{
$this->period = $period;
$this->site = $site;
$this->segment = $segment;

// If we are aggregating multiple reports: prepare the Archive object needed for aggregate* methods
if(!$this->isDayArchive()) {
$subPeriods = $this->getPeriod()->getSubperiods();
$this->archive = Archive::factory($this->getSegment(), $subPeriods, array($this->getSite()->getId()));
}
}

/**
Expand Down Expand Up @@ -401,7 +412,7 @@ protected function computeNewArchive($requestedPlugin, $enforceProcessCoreMetric
*
* @return int|bool Datetime timestamp, or false if must look at any archive available
*/
public function getMinTimeArchiveProcessed()
protected function getMinTimeArchiveProcessed()
{
$endDateTimestamp = self::determineIfArchivePermanent($this->getDateEnd());
$isArchiveTemporary = ($endDateTimestamp === false);
Expand Down Expand Up @@ -486,11 +497,11 @@ protected function compute()
{
$archivers = $this->getPluginArchivers();

foreach($archivers as $archiverClass) {
foreach($archivers as $pluginName => $archiverClass) {
/** @var Archiver $archiver */
$archiver = new $archiverClass( $this );

if($archiver->shouldArchive()) {
if($this->shouldProcessReportsForPlugin($pluginName)) {
if($this->isDayArchive()) {
$archiver->aggregateDayReport();
} else {
Expand Down Expand Up @@ -578,7 +589,7 @@ protected function compress($data)
* @param string $pluginName
* @return bool
*/
public function shouldProcessReportsForPlugin($pluginName)
protected function shouldProcessReportsForPlugin($pluginName)
{
if (Rules::shouldProcessReportsAllPlugins($this->getSegment(), $this->getPeriod()->getLabel())) {
return true;
Expand Down Expand Up @@ -646,11 +657,6 @@ protected function convertMetricsIdToName($data)
Metrics::INDEX_NB_UNIQ_VISITORS => Metrics::INDEX_SUM_DAILY_NB_UNIQ_VISITORS
);

/**
* @var Archive
*/
protected $archive = null;

/**
* Sums records for every subperiod of the current period and inserts the result as the record
* for this period.
Expand Down Expand Up @@ -688,10 +694,9 @@ public function aggregateDataTableRecords($recordNames,
if (!is_array($recordNames)) {
$recordNames = array($recordNames);
}
$this->initArchive();
$nameToCount = array();
foreach ($recordNames as $recordName) {
$table = $this->getRecordDataTableSum($recordName, $invalidSummedColumnNameToRenamedName, $columnAggregationOperations);
$table = $this->aggregateDataTableRecord($recordName, $invalidSummedColumnNameToRenamedName, $columnAggregationOperations);

$nameToCount[$recordName]['level0'] = $table->getRowsCount();
$nameToCount[$recordName]['recursive'] = $table->getRowsCountRecursive();
Expand Down Expand Up @@ -727,7 +732,6 @@ public function aggregateNumericMetrics($columns, $operationToApply = false)
if (!is_array($columns)) {
$columns = array($columns);
}
$this->initArchive();
$data = $this->archive->getNumeric($columns);
$operationForColumn = $this->getOperationForColumns($columns, $operationToApply);
$results = $this->aggregateDataArray($data, $operationForColumn);
Expand All @@ -747,14 +751,6 @@ public function aggregateNumericMetrics($columns, $operationToApply = false)
return $results;
}

protected function initArchive()
{
if (empty($this->archive)) {
$subPeriods = $this->getPeriod()->getSubperiods();
$this->archive = Archive::factory($this->getSegment(), $subPeriods, array($this->getSite()->getId()));
}
}

/**
* This method selects all DataTables that have the name $name over the period.
* All these DataTables are then added together, and the resulting DataTable is returned.
Expand All @@ -764,7 +760,7 @@ protected function initArchive()
* @param array $columnAggregationOperations Operations for aggregating columns, @see Row::sumRow()
* @return DataTable
*/
protected function getRecordDataTableSum($name, $invalidSummedColumnNameToRenamedName, $columnAggregationOperations = null)
protected function aggregateDataTableRecord($name, $invalidSummedColumnNameToRenamedName, $columnAggregationOperations = null)
{
$table = new DataTable();
if (!empty($columnAggregationOperations)) {
Expand Down
21 changes: 4 additions & 17 deletions core/Plugin/Archiver.php
View file
Expand Up @@ -48,6 +48,9 @@
*/
abstract class Archiver
{
/**
* @var \Piwik\ArchiveProcessor
*/
protected $processor;

/**
Expand All @@ -74,28 +77,12 @@ abstract public function aggregateDayReport();
*/
abstract public function aggregateMultipleReports();

// todo: review this concept, each plugin should somehow maintain the list of report names they generate
/**
* Returns true if the current plugin should be archived or not.
*
* @return bool
*/
public function shouldArchive()
{
$className = get_class($this);
$pluginName = str_replace(array("Piwik\\Plugins\\", "\\Archiver"), "", $className);
if (strpos($pluginName, "\\") !== false) {
throw new \Exception("unexpected plugin name $pluginName in shouldArchive()");
}
return $this->getProcessor()->shouldProcessReportsForPlugin($pluginName);
}

/**
* @return \Piwik\ArchiveProcessor
*/
protected function getProcessor()
{
return $this->processor;
return $this->processor->getDateEnd();
}

/**
Expand Down
31 changes: 19 additions & 12 deletions tests/PHPUnit/Core/ArchiveProcessingTest.php
View file
Expand Up @@ -22,6 +22,13 @@
use Piwik\SettingsServer;
use Piwik\Site;

class ArchiveProcessorTest extends ArchiveProcessor {
public function public_getMinTimeArchiveProcessed()
{
return $this->getMinTimeArchiveProcessed();
}
}

class ArchiveProcessingTest extends DatabaseTestCase
{
public function setUp()
Expand Down Expand Up @@ -61,7 +68,7 @@ private function _createWebsite($timezone = 'UTC')
* @param string $periodLabel
* @param string $dateLabel
* @param string $siteTimezone
* @return ArchiveProcessor
* @return ArchiveProcessorTest
*/
private function _createArchiveProcessor($periodLabel, $dateLabel, $siteTimezone)
{
Expand All @@ -70,7 +77,7 @@ private function _createArchiveProcessor($periodLabel, $dateLabel, $siteTimezone
$period = Period::factory($periodLabel, $date);
$segment = new Segment('', $site->getId());

return new ArchiveProcessor($period, $site, $segment);
return new \ArchiveProcessorTest($period, $site, $segment);
}

/**
Expand All @@ -94,7 +101,7 @@ public function testInitCurrentMonth()
$timeout = Rules::getTodayArchiveTimeToLive();
$this->assertTrue($timeout >= 10);
$dateMinArchived = $now - $timeout;
$this->compareTimestamps($dateMinArchived, $archiveProcessor->getMinTimeArchiveProcessed());
$this->compareTimestamps($dateMinArchived, $archiveProcessor->public_getMinTimeArchiveProcessed());

$this->assertTrue($archiveProcessor->isArchiveTemporary());
}
Expand All @@ -116,7 +123,7 @@ public function testInitDayInPast()

// min finished timestamp considered when looking at archive timestamp
$dateMinArchived = Date::factory('2010-01-02')->getTimestamp();
$this->assertEquals($archiveProcessor->getMinTimeArchiveProcessed() + 1, $dateMinArchived);
$this->assertEquals($archiveProcessor->public_getMinTimeArchiveProcessed() + 1, $dateMinArchived);

$this->assertEquals('2010-01-01 00:00:00', $archiveProcessor->getDateStart()->getDateStartUTC());
$this->assertEquals('2010-01-01 23:59:59', $archiveProcessor->getDateEnd()->getDateEndUTC());
Expand All @@ -133,7 +140,7 @@ public function testInitDayInPastNonUTCWebsite()
$archiveProcessor = $this->_createArchiveProcessor('day', '2010-01-01', $timezone);
// min finished timestamp considered when looking at archive timestamp
$dateMinArchived = Date::factory('2010-01-01 18:30:00');
$this->assertEquals($archiveProcessor->getMinTimeArchiveProcessed() + 1, $dateMinArchived->getTimestamp());
$this->assertEquals($archiveProcessor->public_getMinTimeArchiveProcessed() + 1, $dateMinArchived->getTimestamp());

$this->assertEquals('2009-12-31 18:30:00', $archiveProcessor->getDateStart()->getDateStartUTC());
$this->assertEquals('2010-01-01 18:29:59', $archiveProcessor->getDateEnd()->getDateEndUTC());
Expand All @@ -150,7 +157,7 @@ public function testInitMonthInPastNonUTCWebsite()
$archiveProcessor = $this->_createArchiveProcessor('month', '2010-01-02', $timezone);
// min finished timestamp considered when looking at archive timestamp
$dateMinArchived = Date::factory('2010-02-01 05:30:00');
$this->assertEquals($archiveProcessor->getMinTimeArchiveProcessed() + 1, $dateMinArchived->getTimestamp());
$this->assertEquals($archiveProcessor->public_getMinTimeArchiveProcessed() + 1, $dateMinArchived->getTimestamp());

$this->assertEquals('2010-01-01 05:30:00', $archiveProcessor->getDateStart()->getDateStartUTC());
$this->assertEquals('2010-02-01 05:29:59', $archiveProcessor->getDateEnd()->getDateEndUTC());
Expand All @@ -175,7 +182,7 @@ public function testInitToday()

// we look at anything processed within the time to live range
$dateMinArchived = $now - Rules::getTodayArchiveTimeToLive();
$this->compareTimestamps($dateMinArchived, $archiveProcessor->getMinTimeArchiveProcessed() );
$this->compareTimestamps($dateMinArchived, $archiveProcessor->public_getMinTimeArchiveProcessed() );
$this->assertTrue($archiveProcessor->isArchiveTemporary());

// when browsers don't trigger archives, we force ArchiveProcessor
Expand All @@ -187,7 +194,7 @@ public function testInitToday()
if (!Common::isPhpCliMode()) {
$dateMinArchived = 0;
}
$this->compareTimestamps($archiveProcessor->getMinTimeArchiveProcessed(), $dateMinArchived);
$this->compareTimestamps($archiveProcessor->public_getMinTimeArchiveProcessed(), $dateMinArchived);

$this->assertEquals(date('Y-m-d', $timestamp) . ' 01:00:00', $archiveProcessor->getDateStart()->getDateStartUTC());
$this->assertEquals(date('Y-m-d', $timestamp + 86400) . ' 00:59:59', $archiveProcessor->getDateEnd()->getDateEndUTC());
Expand Down Expand Up @@ -216,7 +223,7 @@ public function testInitTodayEurope()

// we look at anything processed within the time to live range
$dateMinArchived = $now - Rules::getTodayArchiveTimeToLive();
$minTimeArchivedProcessed = $archiveProcessor->getMinTimeArchiveProcessed();
$minTimeArchivedProcessed = $archiveProcessor->public_getMinTimeArchiveProcessed();
$this->compareTimestamps($dateMinArchived, $minTimeArchivedProcessed);
$this->assertTrue($archiveProcessor->isArchiveTemporary());

Expand All @@ -229,7 +236,7 @@ public function testInitTodayEurope()
if (!Common::isPhpCliMode()) {
$dateMinArchived = 0;
}
$this->compareTimestamps($dateMinArchived, $archiveProcessor->getMinTimeArchiveProcessed());
$this->compareTimestamps($dateMinArchived, $archiveProcessor->public_getMinTimeArchiveProcessed());

// this test varies with DST
$this->assertTrue($archiveProcessor->getDateStart()->getDateStartUTC() == date('Y-m-d', $timestamp - 86400) . ' 22:00:00' ||
Expand Down Expand Up @@ -262,7 +269,7 @@ public function testInitTodayToronto()

// we look at anything processed within the time to live range
$dateMinArchived = $now - Rules::getTodayArchiveTimeToLive();
$this->compareTimestamps($dateMinArchived, $archiveProcessor->getMinTimeArchiveProcessed() );
$this->compareTimestamps($dateMinArchived, $archiveProcessor->public_getMinTimeArchiveProcessed() );
$this->assertTrue($archiveProcessor->isArchiveTemporary());

// when browsers don't trigger archives, we force ArchiveProcessor
Expand All @@ -274,7 +281,7 @@ public function testInitTodayToronto()
if (!Common::isPhpCliMode()) {
$dateMinArchived = 0;
}
$this->compareTimestamps($dateMinArchived, $archiveProcessor->getMinTimeArchiveProcessed());
$this->compareTimestamps($dateMinArchived, $archiveProcessor->public_getMinTimeArchiveProcessed());

// this test varies with DST
$this->assertTrue($archiveProcessor->getDateStart()->getDateStartUTC() == date('Y-m-d', $timestamp) . ' 04:00:00' ||
Expand Down

0 comments on commit 670b24f

Please sign in to comment.