Fix archive invalidation timezone issue in tracker (#14318)

* Fix timezone bug in PHP tracker archive invalidation

* Reset timezone after each unit test

* Fix timezone issue in unit test

* More elegant method for getting midnight in UTC+5

3 files changed, 46 insertions, 9 deletions

diff --git a/core/Tracker/Visit.php b/core/Tracker/Visit.php
index f768ed6190..25ff938562 100644
--- a/core/Tracker/Visit.php
+++ b/core/Tracker/Visit.php
@@ -557,7 +557,11 @@ class Visit implements VisitInterface
 
         $date = Date::factory((int)$time, $timezone);
 
-        if (!$date->isToday()) { // we don't have to handle in case date is in future as it is not allowed by tracker
+        // $date->isToday() is buggy when server and website timezones don't match - so we'll do our own checking
+        $startOfToday = Date::factoryInTimezone('today', $timezone);
+        $isToday = $date->toString('Y-m-d') === $startOfToday->toString('Y-m-d');
+
+        if (!$isToday) { // we don't have to handle in case date is in future as it is not allowed by tracker
             $this->invalidator->rememberToInvalidateArchivedReportsLater($idSite, $date);
         }
     }
diff --git a/tests/PHPUnit/Integration/Tracker/VisitTest.php b/tests/PHPUnit/Integration/Tracker/VisitTest.php
index 7f30b331f5..9c2141f96c 100644
--- a/tests/PHPUnit/Integration/Tracker/VisitTest.php
+++ b/tests/PHPUnit/Integration/Tracker/VisitTest.php
@@ -414,11 +414,12 @@ class VisitTest extends IntegrationTestCase
 
     public function test_markArchivedReportsAsInvalidIfArchiveAlreadyFinished_shouldConsiderWebsitesTimezone()
     {
-        $timezone1 = 'UTC+4';
-        $timezone2 = 'UTC+6';
+        // The double-handling below is needed to work around weird behaviour when UTC and UTC+5 are different dates
+        // Example: 4:32am on 1 April in UTC+5 is 11:32pm on 31 March in UTC
+        $midnight = Date::factoryInTimezone('today', 'UTC+5')->setTimezone('UTC+5');
 
-        $currentActionTime1 = Date::today()->setTimezone($timezone1)->getDatetime();
-        $currentActionTime2 = Date::today()->setTimezone($timezone2)->getDatetime();
+        $oneHourAfterMidnight = $midnight->addHour(1)->getDatetime();
+        $oneHourBeforeMidnight = $midnight->subHour(1)->getDatetime();
         $idsite = API::getInstance()->addSite('name', 'http://piwik.net/', $ecommerce = null,
             $siteSearch = null,
             $searchKeywordParameters = null,
@@ -428,12 +429,12 @@ class VisitTest extends IntegrationTestCase
             $timezone = 'UTC+5');
 
         $expectedRemembered = array(
-            substr($currentActionTime1, 0, 10) => array($idsite)
+            substr($oneHourAfterMidnight, 0, 10) => array($idsite)
         );
 
         // if website timezone was von considered both would be today (expected = array())
-        $this->assertRememberedArchivedReportsThatShouldBeInvalidated($idsite, $currentActionTime1, array());
-        $this->assertRememberedArchivedReportsThatShouldBeInvalidated($idsite, $currentActionTime2, $expectedRemembered);
+        $this->assertRememberedArchivedReportsThatShouldBeInvalidated($idsite, $oneHourAfterMidnight, array());
+        $this->assertRememberedArchivedReportsThatShouldBeInvalidated($idsite, $oneHourBeforeMidnight, $expectedRemembered);
     }
 
     private function assertRememberedArchivedReportsThatShouldBeInvalidated($idsite, $requestDate, $expectedRemeberedArchivedReports)
diff --git a/tests/PHPUnit/Integration/TrackerTest.php b/tests/PHPUnit/Integration/TrackerTest.php
index cd19e43022..8635bede33 100644
--- a/tests/PHPUnit/Integration/TrackerTest.php
+++ b/tests/PHPUnit/Integration/TrackerTest.php
@@ -10,6 +10,8 @@ namespace Piwik\Tests\Integration;
 
 use Piwik\Common;
 use Piwik\Config;
+use Piwik\Date;
+use Piwik\Option;
 use Piwik\Piwik;
 use Piwik\SettingsServer;
 use Piwik\Tests\Framework\Fixture;
@@ -35,6 +37,8 @@ class TrackerTest extends IntegrationTestCase
      */
     private $request;
 
+    private $iniTimeZone;
+
     public function setUp()
     {
         parent::setUp();
@@ -43,6 +47,8 @@ class TrackerTest extends IntegrationTestCase
 
         $this->tracker = new TestTracker();
         $this->request = $this->buildRequest(array('idsite' => 1));
+
+        $this->iniTimeZone = ini_get('date.timezone');
     }
 
     public function tearDown()
@@ -56,7 +62,9 @@ class TrackerTest extends IntegrationTestCase
         if (array_key_exists('PIWIK_TRACKER_DEBUG', $GLOBALS)) {
             unset($GLOBALS['PIWIK_TRACKER_DEBUG']);
         }
-        
+
+        ini_set('date.timezone', $this->iniTimeZone);
+
         parent::tearDown();
     }
 
@@ -357,6 +365,30 @@ class TrackerTest extends IntegrationTestCase
         $this->assertTrue($called);
     }
 
+    public function test_archiveInvalidation_differentServerAndWebsiteTimezones()
+    {
+        // Server timezone is UTC
+        ini_set('date.timezone', 'America/New_York');
+
+        // Website timezone is New York
+        $idSite = Fixture::createWebsite('2014-01-01 00:00:00', 0, false, false,
+            1, null, null, 'America/New_York');
+
+        // It's 3 April in UTC but 2 April in New York
+        Date::$now = 1554257039;
+
+        $this->tracker = new TestTracker();
+
+        $this->request = $this->buildRequest(array('idsite' => $idSite));
+        $this->request->setParam('rec', 1);
+        $this->request->setCurrentTimestamp(Date::$now);
+        $this->tracker->trackRequest($this->request);
+
+        // Check for correct detection of whether the request's timestamp is 'today' in the appropriate timezone
+        // See Visit::markArchivedReportsAsInvalidIfArchiveAlreadyFinished() method
+        $this->assertEmpty(Option::getLike('report_to_invalidate_2_2019-04-02%'));
+    }
+
     private function getDefaultHandler()
     {
         return new Tracker\Handler();