Merge pull request #7244 from piwik/unique_visitors_metasites

Correctly process unique visitors across websites in MetaSite

7 files changed, 65 insertions, 26 deletions

diff --git a/core/ArchiveProcessor.php b/core/ArchiveProcessor.php
index ca2ebde323..17716000e8 100644
--- a/core/ArchiveProcessor.php
+++ b/core/ArchiveProcessor.php
@@ -389,19 +389,38 @@ class ArchiveProcessor
         ) {
             return;
         }
-        if ($row->getColumn('nb_uniq_visitors') !== false
-            || $row->getColumn('nb_users') !== false
+
+        if ($row->getColumn('nb_uniq_visitors') === false
+            && $row->getColumn('nb_users') === false
         ) {
-            if (SettingsPiwik::isUniqueVisitorsEnabled($this->getParams()->getPeriod()->getLabel())) {
-                $metrics = array(Metrics::INDEX_NB_UNIQ_VISITORS, Metrics::INDEX_NB_USERS);
-                $uniques = $this->computeNbUniques( $metrics );
-                $row->setColumn('nb_uniq_visitors', $uniques[Metrics::INDEX_NB_UNIQ_VISITORS]);
-                $row->setColumn('nb_users', $uniques[Metrics::INDEX_NB_USERS]);
-            } else {
-                $row->deleteColumn('nb_uniq_visitors');
-                $row->deleteColumn('nb_users');
+            return;
+        }
+
+        if (!SettingsPiwik::isUniqueVisitorsEnabled($this->getParams()->getPeriod()->getLabel())) {
+            $row->deleteColumn('nb_uniq_visitors');
+            $row->deleteColumn('nb_users');
+            return;
+        }
+
+        $metrics = array(
+            Metrics::INDEX_NB_USERS
+        );
+
+        if($this->getParams()->isSingleSite()) {
+            $uniqueVisitorsMetric = Metrics::INDEX_NB_UNIQ_VISITORS;
+        } else {
+            if(!SettingsPiwik::isSameFingerprintAcrossWebsites()) {
+                throw new Exception("Processing unique visitors across websites is enabled for this instance,
+                            but to process this metric you must first set enable_fingerprinting_across_websites=1
+                            in the config file, under the [Tracker] section.");
             }
+            $uniqueVisitorsMetric = Metrics::INDEX_NB_UNIQ_FINGERPRINTS;
         }
+        $metrics[] = $uniqueVisitorsMetric;
+
+        $uniques = $this->computeNbUniques( $metrics );
+        $row->setColumn('nb_uniq_visitors', $uniques[$uniqueVisitorsMetric]);
+        $row->setColumn('nb_users', $uniques[Metrics::INDEX_NB_USERS]);
     }
 
     protected function guessOperationForColumn($column)
diff --git a/core/DataAccess/LogAggregator.php b/core/DataAccess/LogAggregator.php
index 83946b1f6f..8b8d5175ff 100644
--- a/core/DataAccess/LogAggregator.php
+++ b/core/DataAccess/LogAggregator.php
@@ -158,14 +158,15 @@ class LogAggregator
     protected function getVisitsMetricFields()
     {
         return array(
-            Metrics::INDEX_NB_UNIQ_VISITORS    => "count(distinct " . self::LOG_VISIT_TABLE . ".idvisitor)",
-            Metrics::INDEX_NB_VISITS           => "count(*)",
-            Metrics::INDEX_NB_ACTIONS          => "sum(" . self::LOG_VISIT_TABLE . ".visit_total_actions)",
-            Metrics::INDEX_MAX_ACTIONS         => "max(" . self::LOG_VISIT_TABLE . ".visit_total_actions)",
-            Metrics::INDEX_SUM_VISIT_LENGTH    => "sum(" . self::LOG_VISIT_TABLE . ".visit_total_time)",
-            Metrics::INDEX_BOUNCE_COUNT        => "sum(case " . self::LOG_VISIT_TABLE . ".visit_total_actions when 1 then 1 when 0 then 1 else 0 end)",
-            Metrics::INDEX_NB_VISITS_CONVERTED => "sum(case " . self::LOG_VISIT_TABLE . ".visit_goal_converted when 1 then 1 else 0 end)",
-            Metrics::INDEX_NB_USERS            => "count(distinct " . self::LOG_VISIT_TABLE . ".user_id)",
+            Metrics::INDEX_NB_UNIQ_VISITORS               => "count(distinct " . self::LOG_VISIT_TABLE . ".idvisitor)",
+            Metrics::INDEX_NB_UNIQ_FINGERPRINTS           => "count(distinct " . self::LOG_VISIT_TABLE . ".config_id)",
+            Metrics::INDEX_NB_VISITS                      => "count(*)",
+            Metrics::INDEX_NB_ACTIONS                     => "sum(" . self::LOG_VISIT_TABLE . ".visit_total_actions)",
+            Metrics::INDEX_MAX_ACTIONS                    => "max(" . self::LOG_VISIT_TABLE . ".visit_total_actions)",
+            Metrics::INDEX_SUM_VISIT_LENGTH               => "sum(" . self::LOG_VISIT_TABLE . ".visit_total_time)",
+            Metrics::INDEX_BOUNCE_COUNT                   => "sum(case " . self::LOG_VISIT_TABLE . ".visit_total_actions when 1 then 1 when 0 then 1 else 0 end)",
+            Metrics::INDEX_NB_VISITS_CONVERTED            => "sum(case " . self::LOG_VISIT_TABLE . ".visit_goal_converted when 1 then 1 else 0 end)",
+            Metrics::INDEX_NB_USERS                       => "count(distinct " . self::LOG_VISIT_TABLE . ".user_id)",
         );
     }
 
@@ -445,8 +446,14 @@ class LogAggregator
 
     protected function isMetricRequested($metricId, $metricsRequested)
     {
-        return $metricsRequested === false
-            || in_array($metricId, $metricsRequested);
+        // do not process INDEX_NB_UNIQ_FINGERPRINTS unless specifically asked for
+        if($metricsRequested === false) {
+            if($metricId == Metrics::INDEX_NB_UNIQ_FINGERPRINTS) {
+                return false;
+            }
+            return true;
+        }
+        return in_array($metricId, $metricsRequested);
     }
 
     protected function getWhereStatement($tableName, $datetimeField, $extraWhere = false)
diff --git a/core/Metrics.php b/core/Metrics.php
index e76e8dd5e9..4b48bf13f2 100644
--- a/core/Metrics.php
+++ b/core/Metrics.php
@@ -86,6 +86,9 @@ class Metrics
     const INDEX_CONTENT_NB_IMPRESSIONS = 41;
     const INDEX_CONTENT_NB_INTERACTIONS = 42;
 
+    // Unique visitors fingerprints (useful to process unique visitors across websites)
+    const INDEX_NB_UNIQ_FINGERPRINTS = 43;
+
     // Goal reports
     const INDEX_GOAL_NB_CONVERSIONS = 1;
     const INDEX_GOAL_REVENUE = 2;
@@ -98,6 +101,7 @@ class Metrics
 
     public static $mappingFromIdToName = array(
         Metrics::INDEX_NB_UNIQ_VISITORS                      => 'nb_uniq_visitors',
+        Metrics::INDEX_NB_UNIQ_FINGERPRINTS                  => 'nb_uniq_fingerprints',
         Metrics::INDEX_NB_VISITS                             => 'nb_visits',
         Metrics::INDEX_NB_ACTIONS                            => 'nb_actions',
         Metrics::INDEX_NB_USERS                              => 'nb_users',
diff --git a/core/SettingsPiwik.php b/core/SettingsPiwik.php
index cc098ceca5..6a30ddfecb 100644
--- a/core/SettingsPiwik.php
+++ b/core/SettingsPiwik.php
@@ -9,7 +9,6 @@
 namespace Piwik;
 
 use Exception;
-use Piwik\Container\StaticContainer;
 use Piwik\Cache as PiwikCache;
 
 /**
@@ -444,4 +443,14 @@ class SettingsPiwik
         return Config::getInstance()->General['force_ssl'] == 1;
     }
 
+    /**
+     * Note: this config settig is also checked in the InterSites plugin
+     *
+     * @return bool
+     */
+    public static function isSameFingerprintAcrossWebsites()
+    {
+        return (bool)Config::getInstance()->Tracker['enable_fingerprinting_across_websites'];
+    }
+
 }
diff --git a/core/Tracker/Visit.php b/core/Tracker/Visit.php
index bb8db1ba7d..e5dbc69ef6 100644
--- a/core/Tracker/Visit.php
+++ b/core/Tracker/Visit.php
@@ -17,6 +17,7 @@ use Piwik\Exception\UnexpectedWebsiteFoundException;
 use Piwik\Network\IPUtils;
 use Piwik\Piwik;
 use Piwik\Plugin\Dimension\VisitDimension;
+use Piwik\SettingsPiwik;
 use Piwik\Tracker;
 
 /**
@@ -383,10 +384,7 @@ class Visit implements VisitInterface
     protected function getSettingsObject()
     {
         if (is_null($this->userSettings)) {
-            // Note: this config settig is also checked in the InterSites plugin
-            $isSameFingerprintAcrossWebsites = (bool)Config::getInstance()->Tracker['enable_fingerprinting_across_websites'];
-
-            $this->userSettings = new Settings( $this->request, $this->getVisitorIp(), $isSameFingerprintAcrossWebsites );
+            $this->userSettings = new Settings( $this->request, $this->getVisitorIp(), SettingsPiwik::isSameFingerprintAcrossWebsites());
         }
 
         return $this->userSettings;
diff --git a/tests/PHPUnit/System/MultipleSitesArchivingTest.php b/tests/PHPUnit/System/MultipleSitesArchivingTest.php
index b62feb8263..83ace8c9d4 100644
--- a/tests/PHPUnit/System/MultipleSitesArchivingTest.php
+++ b/tests/PHPUnit/System/MultipleSitesArchivingTest.php
@@ -34,6 +34,7 @@ class MultipleSitesArchivingTest extends SystemTestCase
         });
 
         Config::getInstance()->General['enable_processing_unique_visitors_multiple_sites'] = 1;
+        Config::getInstance()->Tracker['enable_fingerprinting_across_websites'] = 1;
     }
 
     public function getApiForTesting()
diff --git a/tests/PHPUnit/Unit/MetricsTest.php b/tests/PHPUnit/Unit/MetricsTest.php
index 112e7ef491..a943f2d2df 100644
--- a/tests/PHPUnit/Unit/MetricsTest.php
+++ b/tests/PHPUnit/Unit/MetricsTest.php
@@ -80,7 +80,8 @@ class MetricsTest extends \PHPUnit_Framework_TestCase
             'max_event_value' => 37,

             'nb_events_with_value' => 38,

             'nb_impressions' => 41,

-            'nb_interactions' => 42

+            'nb_interactions' => 42,

+            'nb_uniq_fingerprints' => 43,

         );

         $this->assertEquals($expectedMapping, $mapping);

     }