Skip to content

Commit f4fca88

Browse files
sgiehlsgiehl
committed
fix some archive dependency issues
1 parent 2223aa1 commit f4fca88

2 files changed

Lines changed: 164 additions & 55 deletions

File tree

‎core/ArchiveProcessor/RecordBuilder.php‎

Lines changed: 89 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use Piwik\Archive;
1313
use Piwik\ArchiveProcessor;
1414
use Piwik\Common;
15+
use Piwik\Context;
1516
use Piwik\DataTable;
1617
use Piwik\DataTable\Row;
1718
use Piwik\Piwik;
@@ -149,6 +150,19 @@ public function buildForNonDayPeriod(ArchiveProcessor $archiveProcessor): void
149150
$blobRecordsByName[$blobRecord->getName()] = $blobRecord;
150151
}
151152

153+
foreach ($blobRecords as $record) {
154+
$flatRecordName = $record->getBuiltFromFlatRecord();
155+
if (
156+
empty($flatRecordName)
157+
|| !in_array($record->getName(), $requestedReports)
158+
|| in_array($flatRecordName, $requestedReports)
159+
) {
160+
continue;
161+
}
162+
163+
$requestedReports[] = $flatRecordName;
164+
}
165+
152166
$aggregatedCounts = [];
153167

154168
// make sure if there are requested numeric records that depend on blob records, that the blob records will be archived first
@@ -483,80 +497,90 @@ protected function aggregateDataTableFromBlobs(
483497
?array $columnsToRenameAfterAggregation,
484498
?array $periodsToInclude = null
485499
): array {
486-
$tableIdToResultRowMapping = [];
487-
$result = new DataTable();
488-
489-
if (!empty($columnsAggregationOperation)) {
490-
$result->setMetadata(DataTable::COLUMN_AGGREGATION_OPS_METADATA_NAME, $columnsAggregationOperation);
491-
}
492-
493-
$hasRows = false;
494-
foreach ($this->querySingleBlobRows($archiveProcessor, $recordName) as $archiveDataRow) {
495-
$period = $archiveDataRow['date1'] . ',' . $archiveDataRow['date2'];
496-
if ($periodsToInclude !== null && !isset($periodsToInclude[$period])) {
497-
continue;
500+
return $this->executeWithoutRequestedReport(function () use (
501+
$archiveProcessor,
502+
$recordName,
503+
$columnsAggregationOperation,
504+
$columnsToRenameAfterAggregation,
505+
$periodsToInclude
506+
) {
507+
$tableIdToResultRowMapping = [];
508+
$result = new DataTable();
509+
510+
if (!empty($columnsAggregationOperation)) {
511+
$result->setMetadata(DataTable::COLUMN_AGGREGATION_OPS_METADATA_NAME, $columnsAggregationOperation);
498512
}
499513

500-
$hasRows = true;
501-
$tableId = $archiveDataRow['name'] == $recordName ? null : $this->getSubtableIdFromBlobName($archiveDataRow['name']);
514+
$hasRows = false;
515+
foreach ($this->querySingleBlobRows($archiveProcessor, $recordName) as $archiveDataRow) {
516+
$period = $archiveDataRow['date1'] . ',' . $archiveDataRow['date2'];
517+
if ($periodsToInclude !== null && !isset($periodsToInclude[$period])) {
518+
continue;
519+
}
520+
521+
$hasRows = true;
522+
$tableId = $archiveDataRow['name'] == $recordName ? null : $this->getSubtableIdFromBlobName($archiveDataRow['name']);
502523

503-
$blobTable = DataTable::fromSerializedArray($archiveDataRow['value']);
504-
$blobTable->filter(function (DataTable $table) use ($archiveProcessor, $columnsToRenameAfterAggregation) {
505-
$archiveProcessor->renameColumnsAfterAggregation($table, $columnsToRenameAfterAggregation);
506-
});
524+
$blobTable = DataTable::fromSerializedArray($archiveDataRow['value']);
525+
$blobTable->filter(function (DataTable $table) use ($archiveProcessor, $columnsToRenameAfterAggregation) {
526+
$archiveProcessor->renameColumnsAfterAggregation($table, $columnsToRenameAfterAggregation);
527+
});
507528

508-
if ($tableId === null) {
509-
$tableToAddTo = $result;
510-
} elseif (!empty($tableIdToResultRowMapping[$period][$tableId])) {
511-
$rowToAddTo = $tableIdToResultRowMapping[$period][$tableId];
512-
if (!$rowToAddTo->getIdSubDataTable()) {
513-
$newTable = new DataTable();
514-
if (!empty($columnsAggregationOperation)) {
515-
$newTable->setMetadata(DataTable::COLUMN_AGGREGATION_OPS_METADATA_NAME, $columnsAggregationOperation);
529+
if ($tableId === null) {
530+
$tableToAddTo = $result;
531+
} elseif (!empty($tableIdToResultRowMapping[$period][$tableId])) {
532+
$rowToAddTo = $tableIdToResultRowMapping[$period][$tableId];
533+
if (!$rowToAddTo->getIdSubDataTable()) {
534+
$newTable = new DataTable();
535+
if (!empty($columnsAggregationOperation)) {
536+
$newTable->setMetadata(DataTable::COLUMN_AGGREGATION_OPS_METADATA_NAME, $columnsAggregationOperation);
537+
}
538+
$rowToAddTo->setSubtable($newTable);
516539
}
517-
$rowToAddTo->setSubtable($newTable);
540+
541+
$tableToAddTo = $rowToAddTo->getSubtable();
542+
} else {
543+
Common::destroy($blobTable);
544+
continue;
518545
}
519546

520-
$tableToAddTo = $rowToAddTo->getSubtable();
521-
} else {
522-
Common::destroy($blobTable);
523-
continue;
524-
}
547+
$tableToAddTo->addDataTable($blobTable);
525548

526-
$tableToAddTo->addDataTable($blobTable);
549+
foreach ($blobTable->getRows() as $blobTableRow) {
550+
$label = $blobTableRow->getColumn('label');
551+
$subtableId = $blobTableRow->getIdSubDataTable();
552+
if (empty($subtableId)) {
553+
continue;
554+
}
527555

528-
foreach ($blobTable->getRows() as $blobTableRow) {
529-
$label = $blobTableRow->getColumn('label');
530-
$subtableId = $blobTableRow->getIdSubDataTable();
531-
if (empty($subtableId)) {
532-
continue;
556+
$rowToAddTo = $tableToAddTo->getRowFromLabel($label);
557+
if ($rowToAddTo instanceof Row) {
558+
$tableIdToResultRowMapping[$period][$subtableId] = $rowToAddTo;
559+
}
533560
}
534561

535-
$rowToAddTo = $tableToAddTo->getRowFromLabel($label);
536-
if ($rowToAddTo instanceof Row) {
537-
$tableIdToResultRowMapping[$period][$subtableId] = $rowToAddTo;
538-
}
562+
Common::destroy($blobTable);
539563
}
540564

541-
Common::destroy($blobTable);
542-
}
543-
544-
return [$result, $hasRows];
565+
return [$result, $hasRows];
566+
});
545567
}
546568

547569
protected function getPeriodsWithRootBlob(ArchiveProcessor $archiveProcessor, string $recordName): array
548570
{
549-
$result = [];
550-
foreach ($this->querySingleBlobRows($archiveProcessor, $recordName) as $archiveDataRow) {
551-
if ($archiveDataRow['name'] !== $recordName) {
552-
continue;
553-
}
571+
return $this->executeWithoutRequestedReport(function () use ($archiveProcessor, $recordName) {
572+
$result = [];
573+
foreach ($this->querySingleBlobRows($archiveProcessor, $recordName) as $archiveDataRow) {
574+
if ($archiveDataRow['name'] !== $recordName) {
575+
continue;
576+
}
554577

555-
$period = $archiveDataRow['date1'] . ',' . $archiveDataRow['date2'];
556-
$result[$period] = true;
557-
}
578+
$period = $archiveDataRow['date1'] . ',' . $archiveDataRow['date2'];
579+
$result[$period] = true;
580+
}
558581

559-
return $result;
582+
return $result;
583+
});
560584
}
561585

562586
protected function querySingleBlobRows(ArchiveProcessor $archiveProcessor, string $recordName): iterable
@@ -595,6 +619,16 @@ protected function getSubtableIdFromBlobName(string $recordName): ?int
595619
return (int) $id;
596620
}
597621

622+
/**
623+
* @template T
624+
* @param callable(): T $callback
625+
* @return T
626+
*/
627+
protected function executeWithoutRequestedReport(callable $callback)
628+
{
629+
return Context::executeWithQueryParameters(['requestedReport' => ''], $callback);
630+
}
631+
598632
/**
599633
* Returns metadata for records primarily used when aggregating over non-day periods. Every numeric/blob
600634
* record your RecordBuilder creates should have an associated piece of record metadata.

‎plugins/Goals/tests/System/PreprocessedSegmentGoalsArchiveConsistencyTest.php‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
<?php
2+
3+
/**
4+
* Matomo - free/libre analytics platform
5+
*
6+
* @link https://matomo.org
7+
* @license https://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
8+
*/
9+
10+
namespace Piwik\Plugins\Goals\tests\System;
11+
12+
use Piwik\API\Request;
13+
use Piwik\DataTable;
14+
use Piwik\Tests\Framework\TestCase\SystemTestCase;
15+
use Piwik\Tests\Fixtures\SomePageGoalVisitsWithConversions;
16+
17+
/**
18+
* @group PreprocessedSegmentGoalsArchiveConsistencyTest
19+
* @group Goals
20+
* @group Plugins
21+
*/
22+
class PreprocessedSegmentGoalsArchiveConsistencyTest extends SystemTestCase
23+
{
24+
public static $fixture = null;
25+
26+
public function testRequestedActionArchiveDoesNotLoseWeeklyGoalConversionsInBaseSegment(): void
27+
{
28+
$segment = 'countryCode==' . self::$fixture->segmentCountryCode;
29+
$requestParams = [
30+
'idSite' => self::$fixture->idSite,
31+
'date' => self::$fixture->dateTime,
32+
'period' => 'week',
33+
];
34+
35+
// Warm up the week archive through an Actions report request with requestedReport set.
36+
Request::processRequest('Actions.getPageUrls', array_merge($requestParams, [
37+
'segment' => $segment,
38+
'idGoal' => 1,
39+
'requestedReport' => 'Actions_actions_url',
40+
]));
41+
42+
/** @var DataTable $baseSegmentTable */
43+
$baseSegmentTable = Request::processRequest('Goals.get', array_merge($requestParams, [
44+
'segment' => $segment,
45+
'idGoal' => 1,
46+
'columns' => 'nb_conversions',
47+
]));
48+
/** @var DataTable $newVisitorSegmentTable */
49+
$newVisitorSegmentTable = Request::processRequest('Goals.get', array_merge($requestParams, [
50+
'segment' => $segment . ';visitorType==new',
51+
'idGoal' => 1,
52+
'columns' => 'nb_conversions',
53+
]));
54+
55+
$baseRow = $baseSegmentTable->getFirstRow();
56+
$newVisitorRow = $newVisitorSegmentTable->getFirstRow();
57+
58+
$this->assertNotFalse($baseRow, 'Expected Goals.getConversions to return one row for base segment.');
59+
$this->assertNotFalse($newVisitorRow, 'Expected Goals.getConversions to return one row for new-visitor segment.');
60+
61+
$this->assertSame(
62+
1.0,
63+
(float) $newVisitorRow->getColumn('nb_conversions'),
64+
'Expected one conversion in new-visitor segment for goal 1.'
65+
);
66+
67+
$this->assertSame(
68+
1.0,
69+
(float) $baseRow->getColumn('nb_conversions'),
70+
'Expected one conversion in base segment for goal 1.'
71+
);
72+
}
73+
}
74+
75+
PreprocessedSegmentGoalsArchiveConsistencyTest::$fixture = new SomePageGoalVisitsWithConversions();

0 commit comments

Comments
 (0)
⚡