Merge branch '3.x-dev' into 4.x-dev

alemonteiro · Apr 1, 2020 · 677fec1 · 677fec1
2 parents 53cc7a4 + 19af9b6
commit 677fec1
Show file tree

Hide file tree

Showing 19 changed files with 220 additions and 60 deletions.
diff --git a/core/ArchiveProcessor.php b/core/ArchiveProcessor.php
@@ -103,22 +103,11 @@ class ArchiveProcessor
 
     private $numberOfVisitsConverted = false;
 
-    /**
-     * If true, unique visitors are not calculated when we are aggregating data for multiple sites.
-     * The `[General] enable_processing_unique_visitors_multiple_sites` INI config option controls
-     * the value of this variable.
-     *
-     * @var bool
-     */
-    private $skipUniqueVisitorsCalculationForMultipleSites = true;
-
     public function __construct(Parameters $params, ArchiveWriter $archiveWriter, LogAggregator $logAggregator)
     {
         $this->params = $params;
         $this->logAggregator = $logAggregator;
         $this->archiveWriter = $archiveWriter;
-
-        $this->skipUniqueVisitorsCalculationForMultipleSites = Rules::shouldSkipUniqueVisitorsCalculationForMultipleSites();
     }
 
     protected function getArchive()
@@ -414,30 +403,40 @@ protected function getOperationForColumns($columns, $defaultOperation)
 
     protected function enrichWithUniqueVisitorsMetric(Row $row)
     {
-        // skip unique visitors metrics calculation if calculating for multiple sites is disabled
-        if (!$this->getParams()->isSingleSite()
-            && $this->skipUniqueVisitorsCalculationForMultipleSites
-        ) {
-            return;
-        }
-
         if ($row->getColumn('nb_uniq_visitors') === false
             && $row->getColumn('nb_users') === false
         ) {
             return;
         }
 
-        if (!SettingsPiwik::isUniqueVisitorsEnabled($this->getParams()->getPeriod()->getLabel())) {
+        $periodLabel = $this->getParams()->getPeriod()->getLabel();
+
+        if (!SettingsPiwik::isUniqueVisitorsEnabled($periodLabel)) {
             $row->deleteColumn('nb_uniq_visitors');
             $row->deleteColumn('nb_users');
             return;
         }
 
-        $metrics = array(
-            Metrics::INDEX_NB_USERS
-        );
+        $sites = $this->getIdSitesToComputeNbUniques();
+
+        if (count($sites) > 1 && Rules::shouldSkipUniqueVisitorsCalculationForMultipleSites()) {
+            if ($periodLabel != 'day') {
+                // for day we still keep the aggregated metric but for other periods we remove it as it becomes to
+                // inaccurate
+                $row->deleteColumn('nb_uniq_visitors');
+                $row->deleteColumn('nb_users');
+            }
+            return;
+        }
 
-        if ($this->getParams()->isSingleSite()) {
+        if (empty($sites)) {
+            // a plugin disabled running below query by removing all sites.
+            $row->deleteColumn('nb_uniq_visitors');
+            $row->deleteColumn('nb_users');
+            return;
+        }
+
+        if (count($sites) === 1) {
             $uniqueVisitorsMetric = Metrics::INDEX_NB_UNIQ_VISITORS;
         } else {
             if (!SettingsPiwik::isSameFingerprintAcrossWebsites()) {
@@ -447,9 +446,13 @@ protected function enrichWithUniqueVisitorsMetric(Row $row)
             }
             $uniqueVisitorsMetric = Metrics::INDEX_NB_UNIQ_FINGERPRINTS;
         }
-        $metrics[] = $uniqueVisitorsMetric;
 
-        $uniques = $this->computeNbUniques($metrics);
+        $metrics = array(
+            Metrics::INDEX_NB_USERS,
+            $uniqueVisitorsMetric
+        );
+
+        $uniques = $this->computeNbUniques($metrics, $sites);
 
         // see edge case as described in https://github.com/piwik/piwik/issues/9357 where uniq_visitors might be higher
         // than visits because we archive / process it after nb_visits. Between archiving nb_visits and nb_uniq_visitors
@@ -475,19 +478,44 @@ protected function guessOperationForColumn($column)
         return 'sum';
     }
 
+    private function getIdSitesToComputeNbUniques()
+    {
+        $sites = array($this->getParams()->getSite()->getId());
+
+        /**
+         * Triggered to change which site ids should be looked at when processing unique visitors and users.
+         *
+         * @param array &$idSites An array with one idSite. This site is being archived currently. To cancel the query
+         *                        you can change this value to an empty array. To include other sites in the query you
+         *                        can add more idSites to this list of idSites.
+         */
+        Piwik::postEvent('ArchiveProcessor.ComputeNbUniques.getIdSites', array(&$sites));
+
+        return $sites;
+    }
+
     /**
      * Processes number of unique visitors for the given period
      *
      * This is the only Period metric (ie. week/month/year/range) that we process from the logs directly,
      * since unique visitors cannot be summed like other metrics.
      *
-     * @param array Metrics Ids for which to aggregates count of values
-     * @return array of metrics, where the key is metricid and the value is the metric value
+     * @param array $metrics Metrics Ids for which to aggregates count of values
+     * @param int[] $sites  A list of idSites that should be included
+     * @return array|null An array of metrics, where the key is metricid and the value is the metric value or null if
+     *                      the query was cancelled and not executed.
      */
-    protected function computeNbUniques($metrics)
+    protected function computeNbUniques($metrics, $sites)
     {
         $logAggregator = $this->getLogAggregator();
-        $query = $logAggregator->queryVisitsByDimension(array(), false, array(), $metrics);
+        $sitesBackup = $logAggregator->getSites();
+
+        $logAggregator->setSites($sites);
+        try {
+            $query = $logAggregator->queryVisitsByDimension(array(), false, array(), $metrics);
+        } finally {
+            $logAggregator->setSites($sitesBackup);
+        }
         $data = $query->fetch();
         return $data;
     }

diff --git a/core/CronArchive.php b/core/CronArchive.php
@@ -18,11 +18,11 @@
 use Piwik\CronArchive\Performance\Logger;
 use Piwik\CronArchive\SharedSiteIds;
 use Piwik\Archive\ArchiveInvalidator;
+use Piwik\CronArchive\StopArchiverException;
 use Piwik\DataAccess\ArchiveSelector;
 use Piwik\DataAccess\RawLogDao;
 use Piwik\Exception\UnexpectedWebsiteFoundException;
 use Piwik\Metrics\Formatter;
-use Piwik\Period\Factory;
 use Piwik\Period\Factory as PeriodFactory;
 use Piwik\CronArchive\SitesToReprocessDistributedList;
 use Piwik\CronArchive\SegmentArchivingRequestUrlProvider;
@@ -326,10 +326,14 @@ public function main()
 
         $self = $this;
         Access::doAsSuperUser(function () use ($self) {
-            $self->init();
-            $self->run();
-            $self->runScheduledTasks();
-            $self->end();
+            try {
+                $self->init();
+                $self->run();
+                $self->runScheduledTasks();
+                $self->end();
+            } catch (StopArchiverException $e) {
+                $this->logger->info("Archiving stopped by stop archiver exception");
+            }
         });
     }
 
@@ -984,10 +988,10 @@ public function isThereAValidArchiveForPeriod($idSite, $period, $date, $segment
         $this->disconnectDb();
 
         if (Range::isMultiplePeriod($date, $period)) {
-            $rangePeriod = Factory::build($period, $date, Site::getTimezoneFor($idSite));
+            $rangePeriod = PeriodFactory::build($period, $date, Site::getTimezoneFor($idSite));
             $periodsToCheck = $rangePeriod->getSubperiods();
         } else {
-            $periodsToCheck = [Factory::build($period, $date, Site::getTimezoneFor($idSite))];
+            $periodsToCheck = [PeriodFactory::build($period, $date, Site::getTimezoneFor($idSite))];
         }
 
         $isTodayIncluded = $this->isTodayIncludedInPeriod($idSite, $periodsToCheck);
@@ -997,7 +1001,7 @@ public function isThereAValidArchiveForPeriod($idSite, $period, $date, $segment
         if ($isTodayIncluded
             && !$isLast
         ) {
-            return [false, null];
+            return [false, $date];
         }
 
         $periodsToCheckRanges = array_map(function (Period $p) { return $p->getRangeString(); }, $periodsToCheck);

diff --git a/core/CronArchive/StopArchiverException.php b/core/CronArchive/StopArchiverException.php
@@ -0,0 +1,15 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link https://matomo.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ *
+ */
+namespace Piwik\CronArchive;
+
+use Piwik\Exception\Exception;
+
+class StopArchiverException extends Exception
+{
+}
diff --git a/core/DataAccess/ArchivingDbAdapter.php b/core/DataAccess/ArchivingDbAdapter.php
@@ -9,6 +9,7 @@
 
 namespace Piwik\DataAccess;
 
+use Piwik\ArchiveProcessor\ArchivingStatus;
 use Piwik\Concurrency\Lock;
 use Piwik\Db\AdapterInterface;
 use Psr\Log\LoggerInterface;
@@ -30,6 +31,11 @@ class ArchivingDbAdapter
      */
     private $logger;
 
+    /**
+     * @var int
+     */
+    private $lastReexpireTime = null;
+
     public function __construct($wrapped, Lock $archivingLock = null, LoggerInterface $logger = null)
     {
         $this->wrapped = $wrapped;
@@ -101,7 +107,11 @@ private function logSql($sql)
     private function reexpireLock()
     {
         if ($this->archivingLock) {
-            $this->archivingLock->reexpireLock();
+            $timeBetweenReexpires = ArchivingStatus::DEFAULT_ARCHIVING_TTL / 4;
+            if ($this->lastReexpireTime + $timeBetweenReexpires < time()) {
+                $this->archivingLock->reexpireLock();
+                $this->lastReexpireTime = time();
+            }
         }
     }
 }
diff --git a/core/DataAccess/LogAggregator.php b/core/DataAccess/LogAggregator.php
@@ -180,6 +180,16 @@ public function __construct(Parameters $params, LoggerInterface $logger = null)
         $this->logger = $logger ?: StaticContainer::get('Psr\Log\LoggerInterface');
     }
 
+    public function setSites($sites)
+    {
+        $this->sites = array_map('intval', $sites);
+    }
+
+    public function getSites()
+    {
+        return $this->sites;
+    }
+
     public function getSegment()
     {
         return $this->segment;

diff --git a/plugins/GeoIp2/templates/_updaterManage.twig b/plugins/GeoIp2/templates/_updaterManage.twig
@@ -1,8 +1,9 @@
 <div ng-show="locationUpdater.geoipDatabaseInstalled" id="geoipdb-update-info">
     <p>
-		{{ 'GeoIp2_GeoIPUpdaterInstructions'|translate('<a href="http://www.maxmind.com/?rId=piwik">','</a>','<a rel="noreferrer noopener" href="https://db-ip.com/?refid=mtm">','</a>')|raw }}
+		{{ 'GeoIp2_GeoIPUpdaterInstructions'|translate('<a href="http://www.maxmind.com/?rId=piwik" rel="noreferrer noopener">','</a>','<a rel="noreferrer noopener" href="https://db-ip.com/?refid=mtm">','</a>')|raw }}
         <br/><br/>
-		{% if dbipLiteUrl|default is not empty %}{{ 'GeoIp2_GeoLiteCityLink'|translate('<a rel="noreferrer noopener" href="'~dbipLiteUrl|e('html_attr')~'">',dbipLiteUrl|e('html'),'</a>')|raw }}{% endif %}
+		{% if dbipLiteUrl|default is not empty %}{{ 'GeoIp2_GeoLiteCityLink'|translate('<a rel="noreferrer noopener" href="'~dbipLiteUrl|e('html_attr')~'">',dbipLiteUrl|e('html'),'</a>')|raw }}<br/><br/>{% endif %}
+		{{ 'UserCountry_MaxMindLinkExplanation'|translate('<a href="https://matomo.org/faq/how-to/how-do-i-get-the-geolocation-download-url-for-the-free-maxmind-db/" rel="noreferrer noopener" target="_blank">', '</a>')|raw }}
 
 		<span ng-show="locationUpdater.geoipDatabaseInstalled">
 			<br/><br/>{{ 'GeoIp2_GeoIPUpdaterIntro'|translate }}:

diff --git a/plugins/GeoIp2/templates/configuration.twig b/plugins/GeoIp2/templates/configuration.twig
@@ -13,7 +13,7 @@
                         <div id="manage-geoip-dbs">
                             <div class="row" id="geoipdb-screen1">
                                 <div class="geoipdb-column-1 col s6">
-                                    <p>{{ 'GeoIp2_IWantToDownloadFreeGeoIP'|translate|raw }}</p>
+                                    <p>{{ 'GeoIp2_IWantToDownloadFreeGeoIP'|translate|raw }}<sup><small>*</small></sup></p>
                                 </div>
                                 <div class="geoipdb-column-2 col s6">
                                     <p>{{ 'GeoIp2_IPurchasedGeoIPDBs'|translate('<a rel="noreferrer noopener" href="http://www.maxmind.com/en/geolocation_landing?rId=piwik">','</a>','<a rel="noreferrer noopener" href="https://db-ip.com/db/?refid=mtm">','</a>')|raw }}</p>
@@ -29,6 +29,9 @@
                                            value="{{ 'General_GetStarted'|translate }}..." id="start-automatic-update-geoip"/>
                                 </div>
                             </div>
+                            <div class="row">
+                                <p><sup>* <small>{{ 'UserCountry_GeoIpDbIpAccuracyNote'|translate('<a href="https://dev.maxmind.com/geoip/geoip2/geolite2/?rId=piwik" rel="noreferrer noopener" target="_blank">', '</a>')|raw }}.</small></sup></p>
+                            </div>
                         </div>
                     </div>
                     <div id="geoipdb-screen2-download" ng-show="locationUpdater.showFreeDownload">

diff --git a/plugins/GeoIp2/templates/setupguide.twig b/plugins/GeoIp2/templates/setupguide.twig
@@ -1,9 +1,15 @@
 <h3 style="margin-top:0;">{{ 'GeoIp2_HowToSetupGeoIP'|translate }}</h3>
 <p>{{ 'GeoIp2_HowToSetupGeoIPIntro'|translate }}</p>
 <ul style="list-style:disc !important;margin-left:2em;">
-    <li style="list-style-type: disc !important;">{{ 'GeoIp2_HowToSetupGeoIP_Step1'|translate('<a rel="noreferrer noopener" href="'~dbipLiteUrl~'">','</a>','<a rel="noreferrer noopener" target="_blank" href="http://db-ip.com/?refid=mtm">','</a>')|raw }}</li>
+    <li style="list-style-type: disc !important;">{{ 'GeoIp2_HowToSetupGeoIP_Step1'|translate('<a rel="noreferrer noopener" href="'~dbipLiteUrl~'">','</a>','<a rel="noreferrer noopener" target="_blank" href="http://db-ip.com/?refid=mtm">','</a>')|raw }}<sup>*</sup></li>
     <li style="list-style-type: disc !important;">{{ 'GeoIp2_HowToSetupGeoIP_Step2'|translate("'"~dbipLiteFilename~"'",'<strong>','</strong>','<strong>'~dbipLiteDesiredFilename~'</strong>')|raw }}</li>
     <li style="list-style-type: disc !important;">{{ 'GeoIp2_HowToSetupGeoIP_Step3'|translate('<strong>','</strong>','<span style="color:green"><strong>','</strong></span>')|raw }}</li>
     <li style="list-style-type: disc !important;">{{ 'GeoIp2_HowToSetupGeoIP_Step4'|translate }}</li>
 </ul>
-<p>&nbsp;</p>
+<p>&nbsp;</p>
+
+<div class="row">
+    <div class="col s12">
+        <p><sup>* <small>{{ 'UserCountry_GeoIpDbIpAccuracyNote'|translate('<a href="https://dev.maxmind.com/geoip/geoip2/geolite2/?rId=piwik" rel="noreferrer noopener" target="_blank">', '</a>')|raw }}.</small></sup></p>
+    </div>
+</div>
diff --git a/plugins/Live/Controller.php b/plugins/Live/Controller.php
@@ -195,6 +195,8 @@ public function getVisitList()
             return '';
         }
 
+        VisitorLog::groupActionsByPageviewId($nextVisits);
+
         $view = new View('@Live/getVisitList.twig');
         $view->idSite = $this->idSite;
         $view->startCounter = $startCounter < $nextVisits->getRowsCount() ? $nextVisits->getRowsCount() : $startCounter;

diff --git a/plugins/Live/tests/UI/Live_spec.js b/plugins/Live/tests/UI/Live_spec.js
@@ -75,6 +75,16 @@ describe("Live", function () {
         expect(await dialog.screenshot()).to.matchImage('visitor_profile');
     });
 
+    it('should load additional visits in visitor log', async function() {
+
+        await page.click('.visitor-profile-more-info a');
+
+        await page.waitForNetworkIdle();
+
+        var dialog = await page.$('.ui-dialog');
+        expect(await dialog.screenshot()).to.matchImage('visitor_profile_more_visits');
+    });
+
     it('should hide all action details', async function() {
         await page.evaluate(function(){
             $('.visitor-profile-toggle-actions').click();

diff --git a/plugins/Live/tests/UI/expected-screenshots/Live_visitor_profile_action_details.png b/plugins/Live/tests/UI/expected-screenshots/Live_visitor_profile_action_details.png
diff --git a/plugins/Live/tests/UI/expected-screenshots/Live_visitor_profile_actions_hidden.png b/plugins/Live/tests/UI/expected-screenshots/Live_visitor_profile_actions_hidden.png
diff --git a/plugins/Live/tests/UI/expected-screenshots/Live_visitor_profile_more_visits.png b/plugins/Live/tests/UI/expected-screenshots/Live_visitor_profile_more_visits.png
diff --git a/plugins/Live/tests/UI/expected-screenshots/Live_visitor_profile_visit_details.png b/plugins/Live/tests/UI/expected-screenshots/Live_visitor_profile_visit_details.png