Skip to content

Commit

Permalink
MergeDataTables does not correctly merge if table2 has more data than…
Browse files Browse the repository at this point in the history
… table1 (matomo-org#15294)

* update submodules to fix build

* Handle DataTable Maps in VisitFrequency API and in DataTable merger, make sure to correctly copy child datatables.

* fix some tests

* Copy metadata over.

* Updated expected test results.

* Adding unit tests.
  • Loading branch information
diosmosis authored and jonasgrilleres committed Sep 22, 2020
1 parent 6d35fb4 commit 52b2c5c
Show file tree
Hide file tree
Showing 24 changed files with 3,413 additions and 32 deletions.
2 changes: 1 addition & 1 deletion core/DataTable/Renderer.php
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ public function __toString()
/**
* Set the DataTable to be rendered
*
* @param DataTable|Simple|DataTable\Map $table table to be rendered
* @param DataTableInterface $table table to be rendered
* @throws Exception
*/
public function setTable($table)
Expand Down
35 changes: 27 additions & 8 deletions plugins/API/DataTable/MergeDataTables.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@

class MergeDataTables
{

/**
* Merge the columns of two data tables.
* Merge the columns of two data tables. Only takes into consideration the first row of each table.
* Manipulates the first table.
*
* @param DataTable|DataTable\Map $table1 The table to eventually filter.
Expand All @@ -25,13 +24,14 @@ public function mergeDataTables($table1, $table2)
{
// handle table arrays
if ($table1 instanceof DataTable\Map && $table2 instanceof DataTable\Map) {
$subTables2 = $table2->getDataTables();
foreach ($table1->getDataTables() as $index => $subTable1) {
if (!array_key_exists($index, $subTables2)) {
// occurs when archiving starts on dayN and continues into dayN+1, see https://github.com/piwik/piwik/issues/5168#issuecomment-50959925
continue;
$subTables1 = $table1->getDataTables();
foreach ($table2->getDataTables() as $index => $subTable2) {
if (!array_key_exists($index, $subTables1)) {
$subTable1 = $this->makeNewDataTable($subTable2);
$table1->addTable($subTable1, $index);
} else {
$subTable1 = $subTables1[$index];
}
$subTable2 = $subTables2[$index];
$this->mergeDataTables($subTable1, $subTable2);
}
return;
Expand All @@ -52,4 +52,23 @@ public function mergeDataTables($table1, $table2)
}
}

private function makeNewDataTable(DataTable\DataTableInterface $subTable2)
{
if ($subTable2 instanceof DataTable\Map) {
$result = new DataTable\Map();
$result->setKeyName($subTable2->getKeyName());
return $result;
} else if ($subTable2 instanceof DataTable\Simple) {
$result = new DataTable\Simple();
$result->setAllTableMetadata($subTable2->getAllTableMetadata());
return $result;
} else if ($subTable2 instanceof DataTable) {
$result = new DataTable();
$result->setAllTableMetadata($subTable2->getAllTableMetadata());
return $result;
} else {
throw new \Exception("Unknown datatable type: " . get_class($subTable2));
}
}

}
2 changes: 1 addition & 1 deletion plugins/CustomDimensions
Original file line number Diff line number Diff line change
Expand Up @@ -4780,7 +4780,7 @@ <h2 id="VisitFrequency_get" style=" color:#0d0d0d;font-family:-apple-system, Bli
</h2>

<img alt=""
src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAArwAAADICAIAAACF9KXqAAAEfklEQVR4nO3cS1biQABA0ehxJ+x/SayFHtCHTufHE+IHuHckEqoqTuqRCG+n02nYw+FwOB6Pew01DMM9o+24mIebHQC+yPvhcDhv0heTh/f71IDnHfeGTffqLLufFwC8lPefXsD38e4fAO7xMQzD8XhcvKI+fmu+uOMuvne//PL8kvPD8R2HyQHz114Oni/gvM75auezLK4kLmA86WTGxTEXB9yYBQAe0cfaE5ONeXGf3t4jLzcaJkdODhiPOdmeFxew2DfzWcYP136+esrz38/HWRw8zggAD+Tv7YnJ2/rfLG7AO+7T20MtPqsSAHg+/640rN2k+Kxf/g+Jlzy67UwfJa0AYHertyduM7ksf8MB32DjNse237B4APgp/3164uabFM+3g37FGT3fXwmAl7J6pWHxwwtrB4yv+c+3xvGzvUuuLmBjlo3DNj7UsHhGG0uaDFgWCQCP622vb4R8PuX+hU9GAPA6dv6fhkfnsgEArHGlAQBIXuhrpAGAe4gGACARDQBAIhoAgEQ0AACJaAAAEtEAACSiAQBIRAMAkIgGACARDQBAIhoAgEQ0AACJaAAAEtEAACSiAQBIRAMAkIgGACARDQBAIhoAgEQ0AACJaAAAEtEAACSiAQBIRAMAkIgGACARDQBAIhoAgEQ0AACJaAAAEtEAACSiAQBIRAMAkIgGACARDQBAIhoAgEQ0AACJaAAAEtEAACSiAQBIRAMAkIgGACARDQBAIhoAgEQ0AACJaAAAEtEAACSiAQBIRAMAkIgGACARDQBAIhoAgEQ0AACJaAAAEtEAACSiAQBIRAMAkIgGACARDQBAIhoAgEQ0AACJaAAAEtEAACSiAQBIRAMAkIgGACARDQBAIhoAgEQ0AACJaAAAEtEAACSiAQBIRAMAkIgGACARDQBAIhoAgEQ0AACJaAAAEtEAACSiAQBIRAMAkIgGACARDQBAIhoAgEQ0AACJaAAAEtEAACSiAQBIRAMAkIgGACARDQBAIhoAgEQ0AACJaAAAEtEAACSiAQBIRAMAkIgGACARDQBAIhoAgEQ0AACJaAAAEtEAACSiAQBIRAMAkIgGACARDQBAIhoAgEQ0AACJaAAAEtEAACSiAQBIRAMAkIgGACARDQBAIhoAgEQ0AACJaAAAEtEAACSiAQBIRAMAkIgGACARDQBAIhoAgEQ0AACJaAAAEtEAACSiAQBIRAMAkIgGACARDQBAIhoAgEQ0AACJaAAAEtEAACSiAQBIRAMAkIgGACARDQBAIhoAgEQ0AACJaAAAEtEAACSiAQBIRAMAkIgGACARDQBAIhoAgEQ0AACJaAAAEtEAACSiAQBIRAMAkIgGACARDQBAIhoAgEQ0AACJaAAAEtEAACSiAQBIRAMAkIgGACARDQBAIhoAgEQ0AACJaAAAEtEAACSiAQBIRAMAkIgGACARDQBAIhoAgEQ0AACJaAAAEtEAACSiAQBIRAMAkIgGACARDQBAIhoAgEQ0AACJaAAAEtEAACSiAQBIRAMAkIgGACARDQBA8gczQRugNvGghAAAAABJRU5ErkJggg=="
src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAArwAAADICAIAAACF9KXqAAAPMklEQVR4nO3d748c50EH8Mf3w3Z8tu/Xns+JW9f2lZokFETakNC0CShJJUT7ohESL6BCQkICQV/kRRGhqHIQRUhFRBVCouJ/aIVoFASoqgIFSlQhkBKlL85uA5dw8cWOe77L3t7eHi/mcl7v3u7N7szc7j3z+bywnl3PPPvs7tzMd2eeZ54j29vboc3CwsLi4mL78wBAaY20P7WwsHDw7QAAhlxraHCOAQDY012hQWIAADoZ2y11SgxLS0vND5eXl8+ePdulxmq1OjIycvTo0byaSGS2traq1erExMSgG8LwWltbO378+Ojo6KAbwpCq1WqNRuP48eODbkhUfu+r3wsh/Onv/Ozpkx2P4GPND5p7M/R91qFWq42NjQkNdLK1tVWr1YQGuqjVauPj40IDndTr9Xq9LjTkaKux3VLY053Q0BwRmhPDuXPnmldoedhue3v72LFjlUqlp+ZSHmtra41G47777ht0Qxhe1Wp1bm5OsqSTlZWVjY0Nu5EcrdxcTwoP/uTFLovtMXoCACiV6zffS7OY0AAAZfelv3w5hPDZJ/a558LeocEYCgAom+nT+3QTcaYBAAhhmEPDwt0G0oCCKkme7FK/e24CMIQqU/d0X2CQZxoWm6Q8juZ4uC36EkyX+nf/S3oAYHjMTZ/ovoDLE5m0xx131QTgcFmvbiaF6cl9zjSMdf/vAdo9GLf8Lm8+SLccnpMD9u6/4f2DesuhPVmrfeH2OlsCQU9pYDc9dHnplrfZ5a0BQEF2b9IwPrbPqYQhDQ3Nv9d3y7vH+N3n06ze98P25zO+kZaH7W+nS6sAoCApb9IQhuTyRPfjZfoeD82rdHnYfeE929B99d3m7btW+hwgMQBwML7yN/8WQviVpy/vu+QgzzS0X4Bo/6/DrvlCSV5LAkDuZvYbbxlyCQ3PPPvN9At/44XP7ZY7nZkPcR04m3s2pDkPIToAcPBmpvYPDQd9eeLKlSvtT/ZxAWKoJO3vfm0i/cjSnsagAkAu5qb2GW8ZcjnT8I0XPnflypWLF3fmxbp58+bY2NipU6f2XPjatb0rae4V2NJDMKTrYZDvUTZ9p8vu0veN0PMRgAE6M3MgoSHcff5gaWmpv6mx23ND8381L9Nebn+YXZfX6q+SsFcPTUMuARigmz+uJoWJE0f3XfjI9vZ2vi/fd2gYcs4E5GVtbW15efnSpUuDbgjD6+rVq/Pz8xMTE4NuCENqZWVlY2Pj3Llzg25IDH7ww3ee+9rL4e5Oh50MxZDL4dT8u19iACBKSWL45cdTnU0f0ps7DYOWaweDbQwAFGff+S0TQkM3sgIAZTCbLjS4PAEAZTez36TYCaEBAMquMi00AACd1Ta3ksLsfpNiJ4QGACipt2+sJYWj46NplhcaAKCkVlJPip0QGgCgpP746/8aQnjmyY+kXF5oAIBSS3mThiA0AEDJzU4KDQBACpUU81smhAYAKLW5aaEBAOjsx7c3ksLJe8ZTriI0AEAZvX1jPSmMjqYNA0IDAJTR77/wnRDCpz9xIf0qQgMAlNdM6vGWQWgAgDKbSTfrREJoAIDyqqSbFDshNABAeaUfbxmEBgAooUZjOynMpb6zUxAaAKCEep0UOyE0AEDpLL+z3sdaQgMAlM7zf/3dEMJnnljoaS2hAQBKauZUDzdpCEIDAJRWZbqH8ZZBaACA0upp6EQQGgCgtOamhAYAoLP16mZSmDx1rKcVhQYAKJe3rt9OCuknxU4IDQBQLl/8i++EEB7/2Ad6XVFoAIAymu1lqqqE0AAAZdTT/JYJoQEAyqin+S0TQgMAlJHQAAB0szsp9pmZiV7XFRoAoETeeXdnfstjR3uYFDshNABAiSzf2AkNIyNHel1XaACAEvnyX/1LCOHJR873sa7QAACl00cvyCA0AEAJ9TopdkJoAIDS6WPoRBAaAKA8dsdbzs+4PAEAdLZR20oKs1NCAwDQ2ds31pJCH+Mtg9AAAOXx7Fe/HUJ46P75/lYXGgCgXPqYFDshNABAuZyd7WfoRBAaAKBszlaEBgAghTN9jbcMQgMAlMTWViMp3Dt3sr8ahAYAKIVbqxtJ4cTx8f5qEBoAoBSuv7uesQahAQBK4bmvvRxC+NRDH+i7BqEBAEqk716QQWgAgFLpb37LhNAAACXiTAMAkMr8rNAAAHRW29yZFNvlCQCgm+s3dsZb9jcp9s66OTUGABheX/izfwoh/NxP3ZulEqEBAMqi70mxE0IDAJTF3HSm0DDW/GBhYWG3vLi4mKVeAGDYzE33P3QiNIeGhYWF5qDQ8hAAOOzmZ/sfOhGaL0+ICAAQpd1JsbPc2Sno0wAA0bv93mZSOH3yWJZ6xvZ8tvnaxK1bt+564du3T58+3aXGWq22vb29urqapVlErFqt1ut1Wwhd1Ov19fX1RqMx6IYwpKrV6ubmpt1Iej9c2jmUZ/zQWkND0hey+VLFm2++2bzAtWvXmvtLttvY2BgZGVlbW8vSLCJWr9fr9XrLdgXNkkPC2Njev2pgc3Oz0WjUarVBN+TQeP7rr4UQPnZ5MuO+t3X0RHvPhvvvv7/Lw3ZLS0vHjh2rVCpZmkXE1tbWlpeXL126NOiGMLyuXr06Pz8/MZGpxxYRW1lZ2djYOHfu3KAbcoi8FkK49KGzly9fzlLLnT4NhksAQMQqU5l6QYYu92kIxlMAQEQy3tkpNIcGEQEAIlbJHBoMuQSAmOUyKXZCaACAmL1z672kcHR8NGNVQgMAxOx3v/KPeVUlNABA/D7zeLd7LKUkNABA/GamsvaCDEIDAJRBZVJoAABSmJsRGgCAFLKPtwxCAwBEbG19Z1qv6dPHs9cmNABAtD7/pRdzrE1oAIDIPfXohVzqERoAIHIzeVybCEIDAERvZkpoAABSmM3jJg1BaACA6M1lnhQ7ITQAQJw2642kUJk+kUuFQgMAxOnm+5Ninzg+nkuFQgMAxOm3/+Qf8q1QaACAmH32iQ/nVZXQAAAxm548lldVQgMAxKwylU8vyCA0AEDc8rpJQxAaACBued2kIQgNABCl9epmUshlUuyE0AAAEfr1576VFEZHczvWCw0AEK2nf/5CjrUJDQAQralTuY23DEIDAEQsxw4NQWgAgIjNTuU2dCIIDQAQsTM5zW+ZEBoAIDZbW+9Piu1MAwDQxburG0lh4sTRHKsVGgAgNr/1/N8XUa3QAABxynFS7ITQAABxmjqd500agtAAALHK9yYNQWgAgFhVch1vGYQGAIjV3JQzDQBAZ+9t1JPC9Ok8b9IQhAYAiMyv/cHfJYWj46P51iw0AECEnnr0Qu51Cg0AEKHJk3neCzIhNABAhPKd3zIhNABAhHK/SUMQGgAgSvnOb5kQGgAgQnN539kpCA0AEJN3V6tJ4WSuk2InhAYAiMdvfvmlpDAyciT3yoUGAIjNL33yUhHVCg0AEJuZAoZOBKEBAOIzPSk0AAApzE7mP94yCA0AEJ8ixlsGoQEAorFZbySFyrQzDQBAZ7/6xb9NCrlPip0QGgAgKr/w8Q8WVLPQAABRmSpmvGUQGgAgMkXMb5kQGgAgKkXMb5kQGgAgKkIDAJDKmZlCbtIQhAYAiMPqWi0pFDEpdkJoAIAY/MYfvZgURkeLOrgLDQAQj6ce/VBxlQsNABCP4sZbBqEBAGJS0PyWCaEBAOIxV8xUVQmhAQDiUZmZKK5yoQEA4jFf2E0agtAAABF45tlvJoWCJsVOCA0AEIlHPnpvofULDQAQiULHWwahAQCiUSly6EQQGgAgGpWpAntBBqEBAKJxxpkGAKCLRmM7KVSmiz3TMNb8YGFhYbe8uLhY6AsDALl4b6OeFIruCHknNCwsLDQHhZaHAMBw+vwffispFDcpdsLlCQCIwSd+5r6iX0JoAIAYFN2hIbT0acjFF/78P3Kvkxj916AbABCVualih06ENKHhlVdeaX74+uuvP/DAA4W1BwDox60bb33/+7cKfYn9Q8PDDz/c5WG7J1+vNrZHzp+rZGnWt7/3o49+ZG52MlNoevGfF5965EKWGt66vvq/y6uffOiDWSp57epKbXProfvPZqnk3//7zcrUPR8+P52lkrw+1V/8+PmRkSN91/DGW+/+aOnm04/9RJZm+FRb/N/12//z9uqnYtlWX3r5Bz99ef7euckslfhUW8S0rf7na0vV2uZjD13MUsmQfKpbW1svffdaxqNVdaN+ZubEpx+7ND5WbK+DI9vbO4M78xo9ce3atRMnTszPz+fTQKKzurr6xhtvPPjgg4NuCMPr1VdfPX/+/KlTpwbdEIbU8vLy+vr6xYuZQgN9uHOmYXFx0X0aAIBO7ro8ISgAAJ0YcgkApHKnTwMAQBd33UY63H2FIpcuDhmr3XPhTjUU9BYOuz0/loxfaCLHraKnttkqDkbGe8ln/5wPcl/R6UmamV6AndCQbArNf2C5DKbIWO2eC3eqoaC3QLOCtorm/+qvDbaKYZP9cz7IfUWnJ8uj5Y3nHhYzJshO0b/vqtKsK4buaSc0tL+rXN5nxkp6Wj2yLyZf7TvQRPsfQPNiBR1QO9WZcq9tqxiIll1ty5dV0A4kvexbhU0ll0+gp/R28NK/tBjaSf63keawGJ6/5IG/Ovtqj5IHkC8ZrO6/qsPBhsXmY3bL8bhTom3/3Z++eWJoJ/uEhvRncopmr5RRp5MNnRbbc+HFYm7m0feXa6sYoDSf/EB2ILaKXHT/Vb3nMoPSPdE2058pu31CQ5eNY8+zLimPSb20MM7LQgORMjd0se/Jif62ipYV0/eFtFUcmP7OsmbZgdhXHLCUB9T0H+/ul17Qj40uOr1Ef+dWhyQbDYn+L0/0/SHueZqrE9/W4dLfl9XrX7Kt4oC1/LjMpc4036B9xUHq3uepJy3pLa8fG7vhI83quRBD23UMDQfwF7jvaS57gdyl7Gk4JJ+8reLwyvdrsq8YiAO7aJjX6c9804ONak93hlw2FxYXF3M5odRebfOTvZ617t6wgt5CxPr4fIrbKvqrYbcZtopC9XGlIN9Npeh9Racn2ZXyCFr0gbaIysXQngzLHSF9Q7SzVQzWcH7+w9mqw67TgXO33BKtmp/stEqnFfv7ndDluN5SeZctpFOIzPhG9vxY9v2sDqmhCA32ArSzVQzcEH4FQ9gkDjsbVU+GIjQAQ8UpekpCYuiV0AAApGJqbAAgFaEBAEhFaAAAUvl/UDjdGeolPWcAAAAASUVORK5CYII="
height="200"
width="700"
margin="0 auto"/>
Expand Down
9 changes: 7 additions & 2 deletions plugins/VisitFrequency/API.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
namespace Piwik\Plugins\VisitFrequency;

use Piwik\API\Request;
use Piwik\Archive;
use Piwik\DataTable;
use Piwik\Period;
use Piwik\Piwik;
use Piwik\Plugins\API\DataTable\MergeDataTables;
use Piwik\Plugins\VisitsSummary\API as APIVisitsSummary;
Expand Down Expand Up @@ -49,7 +49,12 @@ public function get($idSite, $period, $date, $segment = false, $columns = false)
$columns = Piwik::getArrayFromApiParameter($columns);

/** @var \Piwik\DataTable\DataTableInterface $resultSet */
$resultSet = new DataTable\Simple();
if (Period::isMultiplePeriod($date, $period)) {
$resultSet = new DataTable\Map();
$resultSet->setKeyName('period');
} else {
$resultSet = new DataTable\Simple();
}

foreach ($visitTypes as $columnSuffix => $visitorTypeSegment) {
$modifiedSegment = $this->appendVisitorTypeSegment($segment, $visitorTypeSegment);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,22 @@
<?xml version="1.0" encoding="utf-8" ?>
<result />
<results>
<result period="2012-03-03" />
<result period="2012-03-04" />
<result period="2012-03-05" />
<result period="2012-03-06">
<nb_uniq_visitors_returning>1</nb_uniq_visitors_returning>
<nb_users_returning>0</nb_users_returning>
<nb_visits_returning>1</nb_visits_returning>
<nb_actions_returning>1</nb_actions_returning>
<nb_visits_converted_returning>1</nb_visits_converted_returning>
<bounce_count_returning>1</bounce_count_returning>
<sum_visit_length_returning>0</sum_visit_length_returning>
<max_actions_returning>1</max_actions_returning>
<bounce_rate_returning>100%</bounce_rate_returning>
<nb_actions_per_visit_returning>1</nb_actions_per_visit_returning>
<avg_time_on_site_returning>0</avg_time_on_site_returning>
</result>
<result period="2012-03-07" />
<result period="2012-03-08" />
<result period="2012-03-09" />
</results>
Original file line number Diff line number Diff line change
@@ -1,2 +1,25 @@
<?xml version="1.0" encoding="utf-8" ?>
<result />
<results>
<result period="2012-03">
<nb_uniq_visitors_returning>0</nb_uniq_visitors_returning>
<nb_users_returning>0</nb_users_returning>
<nb_visits_returning>1</nb_visits_returning>
<nb_actions_returning>1</nb_actions_returning>
<nb_visits_converted_returning>1</nb_visits_converted_returning>
<bounce_count_returning>1</bounce_count_returning>
<sum_visit_length_returning>0</sum_visit_length_returning>
<max_actions_returning>1</max_actions_returning>
<bounce_rate_returning>100%</bounce_rate_returning>
<nb_actions_per_visit_returning>1</nb_actions_per_visit_returning>
<avg_time_on_site_returning>0</avg_time_on_site_returning>
</result>
<result period="2012-04" />
<result period="2012-05" />
<result period="2012-06" />
<result period="2012-07" />
<result period="2012-08" />
<result period="2012-09" />
<result period="2012-10" />
<result period="2012-11" />
<result period="2012-12" />
</results>
Loading

0 comments on commit 52b2c5c

Please sign in to comment.