Caching id actions in general cache - can be enabled via INI setting...

Caching id actions in general cache - can be enabled via INI setting enable_segments_subquery_cache setting (disabled by default)

config/global.ini.php

@@ -157,6 +157,9 @@ enable_processing_unique_visitors_multiple_sites = 0
 enabled_periods_UI = "day,week,month,year,range"
 enabled_periods_API = "day,week,month,year,range"
 
+; whether to enable subquery cache for Custom Segment archiving queries
+enable_segments_subquery_cache = 0
+
 ; when set to 1, all requests to Piwik will return a maintenance message without connecting to the DB
 ; this is useful when upgrading using the shell command, to prevent other users from accessing the UI while Upgrade is in progress
 maintenance_mode = 0

core/Segment/SegmentExpression.php

@@ -192,6 +192,12 @@ class SegmentExpression
                 // eg. pageUrl!=DoesNotExist
                 // Not equal to NULL means it matches all rows
                 $sqlExpression = self::SQL_WHERE_MATCHES_ALL_ROWS;
+            } elseif($matchType == self::MATCH_CONTAINS
+                  || $matchType == self::MATCH_DOES_NOT_CONTAIN) {
+                // no action was found for CONTAINS / DOES NOT CONTAIN
+                // eg. pageUrl=@DoesNotExist -> matches no row
+                // eg. pageUrl!@DoesNotExist -> matches no rows
+                $sqlExpression = self::SQL_WHERE_DO_NOT_MATCH_ANY_ROW;
             } else {
                 // it is not expected to reach this code path
                 throw new Exception("Unexpected match type $matchType for your segment. " .
@@ -280,8 +286,13 @@ class SegmentExpression
             }
 
             $sqlExpressions[] = $sqlExpression;
+
             if ($value !== null) {
-                $values[] = $value;
+                if(is_array($value)) {
+                    $values = array_merge($values, $value);
+                } else {
+                    $values[] = $value;
+                }
             }
 
             $this->checkFieldIsAvailable($field, $availableTables);

core/Tracker/TableLogAction.php

@@ -10,6 +10,7 @@
 namespace Piwik\Tracker;
 
 use Piwik\Common;
+use Piwik\Config;
 use Piwik\Segment\SegmentExpression;
 
 /**
@@ -188,11 +189,8 @@ class TableLogAction
         // special case
         $sql = TableLogAction::getSelectQueryWhereNameContains($matchType, $actionType);
 
-        return array(
-            // mark that the returned value is an sql-expression instead of a literal value
-            'SQL'  => $sql,
-            'bind' => $valueToMatch,
-        );
+        $cache = new TableLogAction\Cache();
+        return $cache->getIdActionFromSegment($valueToMatch, $sql);
     }
 
     /**

core/Tracker/TableLogAction/Cache.php

+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ *
+ */
+
+namespace Piwik\Tracker\TableLogAction;
+
+use Piwik\Common;
+use Piwik\Config;
+
+class Cache
+{
+    public $enable;
+    protected $lifetime;
+
+    public function __construct()
+    {
+        $this->enable = Config::getInstance()->General['enable_segments_subquery_cache'];
+        $this->lifetime = 60 * 10;
+    }
+
+    /**
+     * @param $valueToMatch
+     * @param $sql
+     * @return array|null
+     * @throws \Exception
+     */
+    public function getIdActionFromSegment($valueToMatch, $sql)
+    {
+        if (!$this->enable) {
+            return array(
+                // mark that the returned value is an sql-expression instead of a literal value
+                'SQL' => $sql,
+                'bind' => $valueToMatch,
+            );
+        }
+
+        $ids = self::getIdsFromCache($valueToMatch, $sql);
+
+        if(is_null($ids)) {
+            return $ids;
+        }
+
+        $sql = Common::getSqlStringFieldsArray($ids);
+        $bind = $ids;
+
+        return array(
+            // mark that the returned value is an sql-expression instead of a literal value
+            'SQL'  => $sql,
+            'bind' => $bind,
+        );
+    }
+
+
+    /**
+     * @param $valueToMatch
+     * @param $sql
+     * @return array|bool|float|int|string
+     */
+    private function getIdsFromCache($valueToMatch, $sql)
+    {
+        $cache = \Piwik\Cache::getLazyCache();
+
+        $cacheKey = $this->getCacheKey($valueToMatch, $sql);
+
+        if ($cache->contains($cacheKey) === true) {
+            return $cache->fetch($cacheKey);
+        }
+
+        $ids = $this->fetchIdsFromDb($valueToMatch, $sql);
+
+        $cache->save($cacheKey, $ids, $this->lifetime);
+
+        return $ids;
+    }
+
+    /**
+     * @param $valueToMatch
+     * @param $sql
+     * @return string
+     * @throws
+     */
+    private function getCacheKey($valueToMatch, $sql)
+    {
+        if(is_array($valueToMatch)) {
+            throw new \Exception("value to match is an array: this is not expected");
+        }
+
+        $uniqueKey = md5($sql . $valueToMatch);
+        $cacheKey = 'TableLogAction.getIdActionFromSegment.' . $uniqueKey;
+        return $cacheKey;
+    }
+
+    /**
+     * @param $valueToMatch
+     * @param $sql
+     * @return array|null
+     * @throws \Exception
+     */
+    private function fetchIdsFromDb($valueToMatch, $sql)
+    {
+        $idActions = \Piwik\Db::fetchAll($sql, $valueToMatch);
+
+        $ids = array();
+        foreach ($idActions as $idAction) {
+            $ids[] = $idAction['idaction'];
+        }
+
+        if (!empty($ids)) {
+            return $ids;
+        }
+
+        // no action was found for CONTAINS / DOES NOT CONTAIN
+        return null;
+    }
+}
\ No newline at end of file

tests/PHPUnit/Integration/SegmentTest.php

@@ -10,6 +10,7 @@ namespace Piwik\Tests\Integration;
 
 use Exception;
 use Piwik\Common;
+use Piwik\Config;
 use Piwik\Db;
 use Piwik\Segment;
 use Piwik\Tests\Framework\Mock\FakeAccess;
@@ -636,10 +637,10 @@ class SegmentTest extends IntegrationTestCase
         $this->assertEquals($this->removeExtraWhiteSpaces($expected), $this->removeExtraWhiteSpaces($query));
     }
 
-    public function test_getSelectQuery_whenPageUrlDoesNotExist_asBothStatements_OR_AND()
+    public function test_getSelectQuery_whenPageUrlDoesNotExist_asBothStatements_OR_AND_withoutCache()
     {
-        $pageUrlFoundInDb = 'example.com/found-in-db';
-        $actionIdFoundInDb = $this->insertPageUrlAsAction($pageUrlFoundInDb);
+        Config::getInstance()->General['enable_segments_subquery_cache'] = 0;
+        list($pageUrlFoundInDb, $actionIdFoundInDb) = $this->insertActions();
 
         $select = 'log_visit.*';
         $from = 'log_visit';
@@ -650,10 +651,12 @@ class SegmentTest extends IntegrationTestCase
          * pageUrl==xyz                              -- Matches none
          * pageUrl!=abcdefg                          -- Matches all
          * pageUrl=@does-not-exist                   -- Matches none
+         * pageUrl=@found-in-db                      -- Matches all
          * pageUrl=='.urlencode($pageUrlFoundInDb)   -- Matches one
-         * pageUrl!@found                            -- Matches all
+         * pageUrl!@not-found                        -- matches all
+         * pageUrl!@found                            -- Matches none
          */
-        $segment = 'visitServerHour==12,pageUrl==xyz;pageUrl!=abcdefg,pageUrl=@does-not-exist,pageUrl=='.urlencode($pageUrlFoundInDb).',pageUrl!@found';
+        $segment = 'visitServerHour==12,pageUrl==xyz;pageUrl!=abcdefg,pageUrl=@does-not-exist,pageUrl=@found-in-db,pageUrl=='.urlencode($pageUrlFoundInDb).',pageUrl!@not-found,pageUrl!@found';
         $segment = new Segment($segment, $idSites = array());
 
         $query = $segment->getSelectQuery($select, $from, $where, $bind);
@@ -670,22 +673,97 @@ class SegmentTest extends IntegrationTestCase
                     " . Common::prefixTable('log_visit') . " AS log_visit
                     LEFT JOIN " . Common::prefixTable('log_link_visit_action') . " AS log_link_visit_action ON log_link_visit_action.idvisit = log_visit.idvisit
                 WHERE (HOUR(log_visit.visit_last_action_time) = ?
-                        OR (1 = 0))
-                      AND ((1 = 1)
-                        OR ( log_link_visit_action.idaction_url IN (SELECT idaction FROM log_action WHERE ( name LIKE CONCAT('%', ?, '%') AND type = 1 )) )
-                        OR   log_link_visit_action.idaction_url = ?
-                        OR ( log_link_visit_action.idaction_url IN (SELECT idaction FROM log_action WHERE ( name NOT LIKE CONCAT('%', ?, '%') AND type = 1 )) )
-                        )
+                        OR (1 = 0)) " . // pageUrl==xyz
+                    "AND ((1 = 1) " . // pageUrl!=abcdefg
+                    "    OR ( log_link_visit_action.idaction_url IN (SELECT idaction FROM log_action WHERE ( name LIKE CONCAT('%', ?, '%') AND type = 1 )) ) " . // pageUrl=@does-not-exist
+                    "    OR ( log_link_visit_action.idaction_url IN (SELECT idaction FROM log_action WHERE ( name LIKE CONCAT('%', ?, '%') AND type = 1 )) )" . // pageUrl=@found-in-db
+                    "    OR   log_link_visit_action.idaction_url = ?" . // pageUrl=='.urlencode($pageUrlFoundInDb)
+                    "    OR ( log_link_visit_action.idaction_url IN (SELECT idaction FROM log_action WHERE ( name NOT LIKE CONCAT('%', ?, '%') AND type = 1 )) )" . // pageUrl!@not-found
+                    "    OR ( log_link_visit_action.idaction_url IN (SELECT idaction FROM log_action WHERE ( name NOT LIKE CONCAT('%', ?, '%') AND type = 1 )) )" . // pageUrl!@found
+                    " )
                 GROUP BY log_visit.idvisit
                 ORDER BY NULL
                     ) AS log_inner",
             "bind" => array(
                 12,
                 "does-not-exist",
+                "found-in-db",
                 $actionIdFoundInDb,
-                "found"
+                "not-found",
+                "found",
+            ));
+
+        $cache = new TableLogAction\Cache();
+        $this->assertTrue( empty($cache->enable) );
+        $this->assertEquals($this->removeExtraWhiteSpaces($expected), $this->removeExtraWhiteSpaces($query));
+    }
+
+    public function test_getSelectQuery_whenPageUrlDoesNotExist_asBothStatements_OR_AND_withCache()
+    {
+        Config::getInstance()->General['enable_segments_subquery_cache'] = 1;
+
+        list($pageUrlFoundInDb, $actionIdFoundInDb) = $this->insertActions();
+        $select = 'log_visit.*';
+        $from = 'log_visit';
+        $where = false;
+        $bind = array();
+
+        /**
+         * pageUrl==xyz                              -- Matches none
+         * pageUrl!=abcdefg                          -- Matches all
+         * pageUrl=@does-not-exist                   -- Matches none
+         * pageUrl=@found-in-db                      -- Matches all
+         * pageUrl=='.urlencode($pageUrlFoundInDb)   -- Matches one
+         * pageUrl!@not-found                        -- matches all
+         * pageUrl!@found                            -- Matches none
+         */
+        $segment = 'visitServerHour==12,pageUrl==xyz;pageUrl!=abcdefg,pageUrl=@does-not-exist,pageUrl=@found-in-db,pageUrl=='.urlencode($pageUrlFoundInDb).',pageUrl!@not-found,pageUrl!@found';
+        $segment = new Segment($segment, $idSites = array());
+
+        $query = $segment->getSelectQuery($select, $from, $where, $bind);
+
+        $expected = array(
+            "sql"  => "
+                SELECT
+                    log_inner.*
+                FROM
+                    (
+                SELECT
+                    log_visit.*
+                FROM
+                    " . Common::prefixTable('log_visit') . " AS log_visit
+                    LEFT JOIN " . Common::prefixTable('log_link_visit_action') . " AS log_link_visit_action ON log_link_visit_action.idvisit = log_visit.idvisit
+                WHERE (HOUR(log_visit.visit_last_action_time) = ?
+                        OR (1 = 0))" . // pageUrl==xyz
+                "
+                      AND ((1 = 1) " . // pageUrl!=abcdefg
+                "
+                        OR (1 = 0) " . // pageUrl=@does-not-exist
+                "
+                        OR ( log_link_visit_action.idaction_url IN (?,?,?) )" . // pageUrl=@found-in-db
+                "
+                        OR   log_link_visit_action.idaction_url = ?" . // pageUrl=='.urlencode($pageUrlFoundInDb)
+                "
+                        OR ( log_link_visit_action.idaction_url IN (?,?,?) )" . // pageUrl!@not-found
+                "
+                        OR (1 = 0) " . // pageUrl!@found
+                ")
+                GROUP BY log_visit.idvisit
+                ORDER BY NULL
+                    ) AS log_inner",
+            "bind" => array(
+                12,
+                1, // pageUrl=@found-in-db
+                2, // pageUrl=@found-in-db
+                3, // pageUrl=@found-in-db
+                $actionIdFoundInDb, // pageUrl=='.urlencode($pageUrlFoundInDb)
+                1, // pageUrl!@not-found
+                2, // pageUrl!@not-found
+                3, // pageUrl!@not-found
             ));
 
+        $cache = new TableLogAction\Cache();
+        $this->assertTrue( !empty($cache->enable) );
         $this->assertEquals($this->removeExtraWhiteSpaces($expected), $this->removeExtraWhiteSpaces($query));
     }
 
@@ -711,4 +789,19 @@ class SegmentTest extends IntegrationTestCase
         $this->assertNotEmpty($actionIdFoundInDb, "Action $pageUrlFoundInDb was not found in the " . Common::prefixTable('log_action') . " table.");
         return $actionIdFoundInDb;
     }
+
+    /**
+     * @return array
+     */
+    private function insertActions()
+    {
+        $pageUrlFoundInDb = 'example.com/found-in-db';
+        $actionIdFoundInDb = $this->insertPageUrlAsAction($pageUrlFoundInDb);
+
+        // Adding some other actions to make test case more realistic
+        $this->insertPageUrlAsAction('example.net/found-in-db-bis');
+        $this->insertPageUrlAsAction('example.net/found-in-db-ter');
+
+        return array($pageUrlFoundInDb, $actionIdFoundInDb);
+    }
 }