Make Visitor Log live query more performant (#14700)

* starting to refactor live query

* more efficient way of fetching visitors for visitor log

* only execute logic when dateStart is set

* perform the same logic even when no date range is defined

* add comment

* fix variable is not defined

* trying to fix some tests

* fix limit, offset (maybe)

* fix date timestamp

* apply array_slice only when needed

* we cannot apply this logic when an offset is specified

* fix tests

* more efficient handling of offset

* apply some review feedback

2 files changed, 249 insertions, 58 deletions

diff --git a/plugins/Live/Model.php b/plugins/Live/Model.php
index 790935819f..c20f26462b 100644
--- a/plugins/Live/Model.php
+++ b/plugins/Live/Model.php
@@ -43,20 +43,124 @@ class Model
             $limit++;
         }
 
-        list($sql, $bind) = $this->makeLogVisitsQueryString($idSite, $period, $date, $segment, $offset, $limit, $visitorId, $minTimestamp, $filterSortOrder);
+        // If no other filter, only look at the last 24 hours of stats
+        if (empty($visitorId)
+            && empty($limit)
+            && empty($offset)
+            && empty($period)
+            && empty($date)
+        ) {
+            $period = 'day';
+            $date = 'yesterdaySameTime';
+        }
+
+        list($dateStart, $dateEnd) = $this->getStartAndEndDate($idSite, $period, $date);
+
+        $queries = $this->splitDatesIntoMultipleQueries($dateStart, $dateEnd, $limit, $offset);
 
-        $visits = Db::getReader()->fetchAll($sql, $bind);
+        $foundVisits = array();
+
+        foreach ($queries as $queryRange) {
+            $updatedLimit = $limit;
+            if (!empty($limit)) {
+                $updatedLimit = $limit - count($foundVisits);
+            }
+
+            $updatedOffset = $offset;
+            if (!empty($offset) && !empty($foundVisits)) {
+                $updatedOffset = 0; // we've already skipped enough rows
+            }
+
+            list($sql, $bind) = $this->makeLogVisitsQueryString($idSite, $queryRange[0], $queryRange[1], $segment, $updatedOffset, $updatedLimit, $visitorId, $minTimestamp, $filterSortOrder);
+
+            $visits = Db::getReader()->fetchAll($sql, $bind);
+
+            if (!empty($offset) && empty($visits)) {
+                // find out if there are any matches
+                $updatedOffset = 0;
+                list($sql, $bind) = $this->makeLogVisitsQueryString($idSite, $queryRange[0], $queryRange[1], $segment, $updatedOffset, $updatedLimit, $visitorId, $minTimestamp, $filterSortOrder);
+
+                $visits = Db::getReader()->fetchAll($sql, $bind);
+                if (!empty($visits)) {
+                    // found out the number of visits that we skipped in this query
+                    $offset = $offset - count($visits);
+                }
+                continue;
+            }
+
+            if (!empty($visits)) {
+                $foundVisits = array_merge($foundVisits, $visits);
+            }
+
+            if ($limit && count($foundVisits) >= $limit) {
+                if (count($foundVisits) > $limit) {
+                    $foundVisits = array_slice($foundVisits, 0, $limit);
+                }
+                break;
+            }
+        }
 
         if ($checkforMoreEntries) {
-            if (count($visits) == $limit) {
-                array_pop($visits);
-                return [$visits, true];
+            if (count($foundVisits) == $limit) {
+                array_pop($foundVisits);
+                return [$foundVisits, true];
             }
 
-            return [$visits, false];
+            return [$foundVisits, false];
+        }
+
+        return $foundVisits;
+    }
+
+    public function splitDatesIntoMultipleQueries($dateStart, $dateEnd, $limit, $offset)
+    {
+        $virtualDateEnd = $dateEnd;
+        if (empty($dateEnd)) {
+            $virtualDateEnd = Date::now()->addDay(1); // matomo always adds one day for some reason
+        }
+
+        $virtualDateStart = $dateStart;
+        if (empty($virtualDateStart)) {
+            $virtualDateStart = Date::factory(Date::FIRST_WEBSITE_TIMESTAMP);
         }
 
-        return $visits;
+        $queries = [];
+        $hasStartEndDateMoreThanOneDayInBetween = $virtualDateStart && $virtualDateStart->addDay(1)->isEarlier($virtualDateEnd);
+        if ($limit
+            && $hasStartEndDateMoreThanOneDayInBetween
+        ) {
+            $virtualDateEnd = $virtualDateEnd->subDay(1);
+            $queries[] = array($virtualDateEnd, $dateEnd); // need to use ",endDate" in case endDate is not set
+
+            if ($virtualDateStart->addDay(7)->isEarlier($virtualDateEnd)) {
+                $queries[] = array($virtualDateEnd->subDay(7), $virtualDateEnd->subSeconds(1));
+                $virtualDateEnd = $virtualDateEnd->subDay(7);
+            }
+
+            if (!$offset) {
+                // only when no offset
+                // we would in worst case - if not enough visits are found to bypass the offset - execute below queries too often.
+                // like we would need to execute each of the queries twice just to find out if there are some visits that
+                // need to be skipped...
+
+                if ($virtualDateStart->addDay(30)->isEarlier($virtualDateEnd)) {
+                    $queries[] = array($virtualDateEnd->subDay(30), $virtualDateEnd->subSeconds(1));
+                    $virtualDateEnd = $virtualDateEnd->subDay(30);
+                }
+                if ($virtualDateStart->addPeriod(1, 'year')->isEarlier($virtualDateEnd)) {
+                    $queries[] = array($virtualDateEnd->subYear(1), $virtualDateEnd->subSeconds(1));
+                    $virtualDateEnd = $virtualDateEnd->subYear(1);
+                }
+            }
+
+            if ($virtualDateStart->isEarlier($virtualDateEnd)) {
+                // need to use ",endDate" in case startDate is not set in which case we do not want to have any limit
+                $queries[] = array($dateStart, $virtualDateEnd->subSeconds(1));
+            }
+        } else {
+            $queries[] = array($dateStart, $dateEnd);
+        }
+        return $queries;
     }
 
     /**
@@ -240,8 +344,8 @@ class Model
 
     /**
      * @param $idSite
-     * @param $period
-     * @param $date
+     * @param Date $startDate
+     * @param Date $endDate
      * @param $segment
      * @param int $offset
      * @param int $limit
@@ -251,23 +355,11 @@ class Model
      * @return array
      * @throws Exception
      */
-    public function makeLogVisitsQueryString($idSite, $period, $date, $segment, $offset, $limit, $visitorId, $minTimestamp, $filterSortOrder)
+    public function makeLogVisitsQueryString($idSite, $startDate, $endDate, $segment, $offset, $limit, $visitorId, $minTimestamp, $filterSortOrder)
     {
-        // If no other filter, only look at the last 24 hours of stats
-        if (empty($visitorId)
-            && empty($limit)
-            && empty($offset)
-            && empty($period)
-            && empty($date)
-        ) {
-            $period = 'day';
-            $date = 'yesterdaySameTime';
-        }
-
-
         list($whereClause, $bindIdSites) = $this->getIdSitesWhereClause($idSite);
 
-        list($whereBind, $where) = $this->getWhereClauseAndBind($whereClause, $bindIdSites, $idSite, $period, $date, $visitorId, $minTimestamp);
+        list($whereBind, $where) = $this->getWhereClauseAndBind($whereClause, $bindIdSites, $startDate, $endDate, $visitorId, $minTimestamp);
 
         if (strtolower($filterSortOrder) !== 'asc') {
             $filterSortOrder = 'DESC';
@@ -325,35 +417,19 @@ class Model
     }
 
     /**
-     * @param string $whereClause
-     * @param array $bindIdSites
+     * for tests only
      * @param $idSite
      * @param $period
      * @param $date
-     * @param $visitorId
-     * @param $minTimestamp
-     * @return array
+     * @return Date[]
      * @throws Exception
+     * @internal
      */
-    private function getWhereClauseAndBind($whereClause, $bindIdSites, $idSite, $period, $date, $visitorId, $minTimestamp)
+    public function getStartAndEndDate($idSite, $period, $date)
     {
-        $where = array();
-        if (!empty($whereClause)) {
-            $where[] = $whereClause;
-        }
-        $whereBind = $bindIdSites;
+        $dateStart = null;
+        $dateEnd = null;
 
-        if (!empty($visitorId)) {
-            $where[] = "log_visit.idvisitor = ? ";
-            $whereBind[] = @Common::hex2bin($visitorId);
-        }
-
-        if (!empty($minTimestamp)) {
-            $where[] = "log_visit.visit_last_action_time > ? ";
-            $whereBind[] = date("Y-m-d H:i:s", $minTimestamp);
-        }
-
-        // SQL Filter with provided period
         if (!empty($period) && !empty($date)) {
             if ($idSite === 'all' || is_array($idSite)) {
                 $currentTimezone = Request::processRequest('SitesManager.getDefaultTimezone');
@@ -379,8 +455,6 @@ class Model
                 $processedPeriod = Period\Factory::build($period, $processedDate);
             }
             $dateStart = $processedPeriod->getDateStart()->setTimezone($currentTimezone);
-            $where[] = "log_visit.visit_last_action_time >= ?";
-            $whereBind[] = $dateStart->toString('Y-m-d H:i:s');
 
             if (!in_array($date, array('now', 'today', 'yesterdaySameTime'))
                 && strpos($date, 'last') === false
@@ -388,11 +462,51 @@ class Model
                 && Date::factory($dateString)->toString('Y-m-d') != Date::factory('now', $currentTimezone)->toString()
             ) {
                 $dateEnd = $processedPeriod->getDateEnd()->setTimezone($currentTimezone);
-                $where[] = " log_visit.visit_last_action_time <= ?";
-                $dateEndString = $dateEnd->addDay(1)->toString('Y-m-d H:i:s');
-                $whereBind[] = $dateEndString;
+                $dateEnd = $dateEnd->addDay(1);
             }
         }
+        return [$dateStart, $dateEnd];
+    }
+
+    /**
+     * @param string $whereClause
+     * @param array $bindIdSites
+     * @param Date $startDate
+     * @param Date $endDate
+     * @param $visitorId
+     * @param $minTimestamp
+     * @return array
+     * @throws Exception
+     */
+    private function getWhereClauseAndBind($whereClause, $bindIdSites, $startDate, $endDate, $visitorId, $minTimestamp)
+    {
+        $where = array();
+        if (!empty($whereClause)) {
+            $where[] = $whereClause;
+        }
+        $whereBind = $bindIdSites;
+
+        if (!empty($visitorId)) {
+            $where[] = "log_visit.idvisitor = ? ";
+            $whereBind[] = @Common::hex2bin($visitorId);
+        }
+
+        if (!empty($minTimestamp)) {
+            $where[] = "log_visit.visit_last_action_time > ? ";
+            $whereBind[] = date("Y-m-d H:i:s", $minTimestamp);
+        }
+
+        // SQL Filter with provided period
+        if (!empty($startDate)) {
+            $where[] = "log_visit.visit_last_action_time >= ?";
+            $whereBind[] = $startDate->toString('Y-m-d H:i:s');
+
+        }
+
+        if (!empty($endDate)) {
+            $where[] = " log_visit.visit_last_action_time <= ?";
+            $whereBind[] = $endDate->toString('Y-m-d H:i:s');
+        }
 
         if (count($where) > 0) {
             $where = join("
diff --git a/plugins/Live/tests/Integration/ModelTest.php b/plugins/Live/tests/Integration/ModelTest.php
index f9c244ee9e..9691583903 100644
--- a/plugins/Live/tests/Integration/ModelTest.php
+++ b/plugins/Live/tests/Integration/ModelTest.php
@@ -9,6 +9,7 @@
 namespace Piwik\Plugins\Live\tests\Integration;
 
 use Piwik\Common;
+use Piwik\Date;
 use Piwik\Piwik;
 use Piwik\Plugins\Live\Model;
 use Piwik\Tests\Framework\Fixture;
@@ -34,10 +35,11 @@ class ModelTest extends IntegrationTestCase
     public function test_makeLogVisitsQueryString()
     {
         $model = new Model();
+        list($dateStart, $dateEnd) = $model->getStartAndEndDate($idSite = 1, 'month', '2010-01-01');
         list($sql, $bind) = $model->makeLogVisitsQueryString(
                 $idSite = 1,
-                $period = 'month',
-                $date = '2010-01-01',
+                $dateStart,
+                $dateEnd,
                 $segment = false,
                 $offset = 0,
                 $limit = 100,
@@ -75,10 +77,11 @@ class ModelTest extends IntegrationTestCase
         });
 
         $model = new Model();
+        list($dateStart, $dateEnd) = $model->getStartAndEndDate($idSite = 1, 'month', '2010-01-01');
         list($sql, $bind) = $model->makeLogVisitsQueryString(
                 $idSite = 1,
-                $period = 'month',
-                $date = '2010-01-01',
+                $dateStart,
+                $dateEnd,
                 $segment = false,
                 $offset = 0,
                 $limit = 100,
@@ -114,10 +117,12 @@ class ModelTest extends IntegrationTestCase
     public function test_makeLogVisitsQueryStringWithOffset()
     {
         $model = new Model();
+
+        list($dateStart, $dateEnd) = $model->getStartAndEndDate($idSite = 1, 'month', '2010-01-01');
         list($sql, $bind) = $model->makeLogVisitsQueryString(
                 $idSite = 1,
-                $period = 'month',
-                $date = '2010-01-01',
+                $dateStart,
+                $dateEnd,
                 $segment = false,
                 $offset = 15,
                 $limit = 100,
@@ -152,10 +157,11 @@ class ModelTest extends IntegrationTestCase
     public function test_makeLogVisitsQueryString_whenSegment()
     {
         $model = new Model();
+        list($dateStart, $dateEnd) = $model->getStartAndEndDate($idSite = 1, 'month', '2010-01-01');
         list($sql, $bind) = $model->makeLogVisitsQueryString(
             $idSite = 1,
-            $period = 'month',
-            $date = '2010-01-01',
+            $dateStart,
+            $dateEnd,
             $segment = 'customVariablePageName1==Test',
             $offset = 10,
             $limit = 100,
@@ -197,6 +203,77 @@ class ModelTest extends IntegrationTestCase
         $this->assertEquals(SegmentTest::removeExtraWhiteSpaces($expectedBind), SegmentTest::removeExtraWhiteSpaces($bind));
     }
 
+    public function test_splitDatesIntoMultipleQueries_notMoreThanADayUsesOnlyOneQuery()
+    {
+        $dates = $this->splitDatesIntoMultipleQueries('2010-01-01 00:00:00', '2010-01-02 00:00:00', $limit = 5, $offset = 0);
+
+        $this->assertEquals(array('2010-01-01 00:00:00 2010-01-02 00:00:00'), $dates);
+    }
+
+
+    public function test_splitDatesIntoMultipleQueries_moreThanADayLessThanAWeek()
+    {
+        $dates = $this->splitDatesIntoMultipleQueries('2010-01-01 00:00:00', '2010-01-02 00:01:00', $limit = 5, $offset = 0);
+
+        $this->assertEquals(array('2010-01-01 00:01:00 2010-01-02 00:01:00', '2010-01-01 00:00:00 2010-01-01 00:00:59'), $dates);
+    }
+
+    public function test_splitDatesIntoMultipleQueries_moreThanAWeekLessThanMonth()
+    {
+        $dates = $this->splitDatesIntoMultipleQueries('2010-01-01 00:00:00', '2010-01-20 04:01:00', $limit = 5, $offset = 0);
+
+        $this->assertEquals(array('2010-01-19 04:01:00 2010-01-20 04:01:00', '2010-01-12 04:01:00 2010-01-19 04:00:59', '2010-01-01 00:00:00 2010-01-12 04:00:59'), $dates);
+    }
+
+    public function test_splitDatesIntoMultipleQueries_moreThanMonthLessThanYear()
+    {
+        $dates = $this->splitDatesIntoMultipleQueries('2010-01-01 00:00:00', '2010-02-20 04:01:00', $limit = 5, $offset = 0);
+
+        $this->assertEquals(array('2010-02-19 04:01:00 2010-02-20 04:01:00', '2010-02-12 04:01:00 2010-02-19 04:00:59', '2010-01-13 04:01:00 2010-02-12 04:00:59', '2010-01-01 00:00:00 2010-01-13 04:00:59'), $dates);
+    }
+
+    public function test_splitDatesIntoMultipleQueries_moreThanYear()
+    {
+        $dates = $this->splitDatesIntoMultipleQueries('2010-01-01 00:00:00', '2012-02-20 04:01:00', $limit = 5, $offset = 0);
+
+        $this->assertEquals(array('2012-02-19 04:01:00 2012-02-20 04:01:00', '2012-02-12 04:01:00 2012-02-19 04:00:59', '2012-01-13 04:01:00 2012-02-12 04:00:59', '2011-01-01 04:01:00 2012-01-13 04:00:59', '2010-01-01 00:00:00 2011-01-01 04:00:59'), $dates);
+    }
+
+    public function test_splitDatesIntoMultipleQueries_moreThanYear_withOffsetUsesLessQueries()
+    {
+        $dates = $this->splitDatesIntoMultipleQueries('2010-01-01 00:00:00', '2012-02-20 04:01:00', $limit = 5, $offset = 5);
+
+        $this->assertEquals(array('2012-02-19 04:01:00 2012-02-20 04:01:00', '2012-02-12 04:01:00 2012-02-19 04:00:59', '2010-01-01 00:00:00 2012-02-12 04:00:59'), $dates);
+    }
+
+    public function test_splitDatesIntoMultipleQueries_moreThanYear_noLimitDoesntUseMultipleQueries()
+    {
+        $dates = $this->splitDatesIntoMultipleQueries('2010-01-01 00:00:00', '2012-02-20 04:01:00', $limit = 0, $offset = 0);
+
+        $this->assertEquals(array('2010-01-01 00:00:00 2012-02-20 04:01:00'), $dates);
+    }
+
+    public function test_splitDatesIntoMultipleQueries_noStartDate()
+    {
+        $dates = $this->splitDatesIntoMultipleQueries(false, '2012-02-20 04:01:00', $limit = 5, $offset = 0);
+
+        $this->assertEquals(array('2012-02-19 04:01:00 2012-02-20 04:01:00', '2012-02-12 04:01:00 2012-02-19 04:00:59', '2012-01-13 04:01:00 2012-02-12 04:00:59', '2011-01-01 04:01:00 2012-01-13 04:00:59', ' 2011-01-01 04:00:59'), $dates);
+    }
+
+    private function splitDatesIntoMultipleQueries($startDate, $endDate, $limit, $offset)
+    {
+        if ($startDate) {
+            $startDate = Date::factory($startDate);
+        }
+        if ($endDate) {
+            $endDate = Date::factory($endDate);
+        }
+        $model = new Model();
+        $queries = $model->splitDatesIntoMultipleQueries($startDate, $endDate, $limit, $offset);
+
+        return array_map(function ($query) { return ($query[0] ? $query[0]->getDatetime() : '') . ' ' . ($query[1] ? $query[1]->getDatetime() : ''); }, $queries);
+    }
+
     protected function setSuperUser()
     {
         FakeAccess::$superUser = true;