From dad0dae15cd8b62dbc91125e2d0e4078909aab73 Mon Sep 17 00:00:00 2001
From: Thomas Steur <tsteur@users.noreply.github.com>
Date: Wed, 13 Jul 2016 15:11:06 +1200
Subject: [PATCH] Improved join generation for segments (#10264)
* better join generation for segments
* do not update visit if there are no values to be updated
---
core/DataAccess/LogQueryBuilder.php | 211 ++------------
.../LogQueryBuilder/JoinGenerator.php | 264 ++++++++++++++++++
.../DataAccess/LogQueryBuilder/JoinTables.php | 114 ++++++++
core/Plugin/LogTablesProvider.php | 65 +++++
core/Tracker/LogTable.php | 75 +++++
core/Tracker/Visit.php | 5 +
plugins/CoreHome/Tracker/LogTable/Action.php | 30 ++
.../CoreHome/Tracker/LogTable/Conversion.php | 24 ++
.../Tracker/LogTable/ConversionItem.php | 25 ++
.../Tracker/LogTable/LinkVisitAction.php | 29 ++
plugins/CoreHome/Tracker/LogTable/Visit.php | 30 ++
.../SegmentEditor/SegmentQueryDecorator.php | 4 +-
.../tests/Unit/SegmentQueryDecoratorTest.php | 5 +-
.../Mock/Plugin/LogTablesProvider.php | 28 ++
tests/PHPUnit/Integration/SegmentTest.php | 18 +-
.../LogQueryBuilder/JoinGeneratorTest.php | 171 ++++++++++++
.../LogQueryBuilder/JoinTablesTest.php | 146 ++++++++++
17 files changed, 1046 insertions(+), 198 deletions(-)
create mode 100644 core/DataAccess/LogQueryBuilder/JoinGenerator.php
create mode 100644 core/DataAccess/LogQueryBuilder/JoinTables.php
create mode 100644 core/Plugin/LogTablesProvider.php
create mode 100644 core/Tracker/LogTable.php
create mode 100644 plugins/CoreHome/Tracker/LogTable/Action.php
create mode 100644 plugins/CoreHome/Tracker/LogTable/Conversion.php
create mode 100644 plugins/CoreHome/Tracker/LogTable/ConversionItem.php
create mode 100644 plugins/CoreHome/Tracker/LogTable/LinkVisitAction.php
create mode 100644 plugins/CoreHome/Tracker/LogTable/Visit.php
create mode 100644 tests/PHPUnit/Framework/Mock/Plugin/LogTablesProvider.php
create mode 100644 tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinGeneratorTest.php
create mode 100644 tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinTablesTest.php
diff --git a/core/DataAccess/LogQueryBuilder.php b/core/DataAccess/LogQueryBuilder.php
index c65f36bf10..186ed0d96e 100644
--- a/core/DataAccess/LogQueryBuilder.php
+++ b/core/DataAccess/LogQueryBuilder.php
@@ -10,11 +10,23 @@
namespace Piwik\DataAccess;
use Exception;
-use Piwik\Common;
+use Piwik\DataAccess\LogQueryBuilder\JoinGenerator;
+use Piwik\DataAccess\LogQueryBuilder\JoinTables;
+use Piwik\Plugin\LogTablesProvider;
use Piwik\Segment\SegmentExpression;
class LogQueryBuilder
{
+ /**
+ * @var LogTablesProvider
+ */
+ private $logTableProvider;
+
+ public function __construct(LogTablesProvider $logTablesProvider)
+ {
+ $this->logTableProvider = $logTablesProvider;
+ }
+
public function getSelectQueryString(SegmentExpression $segmentExpression, $select, $from, $where, $bind, $groupBy,
$orderBy, $limitAndOffset)
{
@@ -31,9 +43,11 @@ class LogQueryBuilder
$bind = array_merge($bind, $segmentSql['bind']);
}
- $joins = $this->generateJoinsString($from);
- $joinWithSubSelect = $joins['joinWithSubSelect'];
- $from = $joins['sql'];
+ $tables = new JoinTables($this->logTableProvider, $from);
+ $join = new JoinGenerator($tables);
+ $join->generate();
+ $from = $join->getJoinString();
+ $joinWithSubSelect = $join->shouldJoinWithSelect();
// hack for https://github.com/piwik/piwik/issues/9194#issuecomment-164321612
$useSpecialConversionGroupBy = (!empty($segmentSql)
@@ -55,192 +69,15 @@ class LogQueryBuilder
);
}
- private function hasJoinedTableAlreadyManually($tableToFind, $joinToFind, $tables)
+ private function getKnownTables()
{
- foreach ($tables as $index => $table) {
- if (is_array($table)
- && !empty($table['table'])
- && $table['table'] === $tableToFind
- && (!isset($table['tableAlias']) || $table['tableAlias'] === $tableToFind)
- && isset($table['joinOn']) && $table['joinOn'] === $joinToFind) {
- return true;
- }
+ $names = array();
+ foreach ($this->logTableProvider->getAllLogTables() as $logTable) {
+ $names[] = $logTable->getName();
}
-
- return false;
+ return $names;
}
- private function findIndexOfManuallyAddedTable($tableToFind, $tables)
- {
- foreach ($tables as $index => $table) {
- if (is_array($table)
- && !empty($table['table'])
- && $table['table'] === $tableToFind
- && (!isset($table['tableAlias']) || $table['tableAlias'] === $tableToFind)) {
- return $index;
- }
- }
- }
-
- private function hasTableAddedManually($tableToFind, $tables)
- {
- $table = $this->findIndexOfManuallyAddedTable($tableToFind, $tables);
-
- return isset($table);
- }
-
- /**
- * Generate the join sql based on the needed tables
- * @param array $tables tables to join
- * @throws Exception if tables can't be joined
- * @return array
- */
- private function generateJoinsString(&$tables)
- {
- $knownTables = array("log_action", "log_visit", "log_link_visit_action", "log_conversion", "log_conversion_item");
- $visitsAvailable = $linkVisitActionsTableAvailable = $conversionsAvailable = $conversionItemAvailable = $actionsTableAvailable = false;
- $defaultLogActionJoin = "log_link_visit_action.idaction_url = log_action.idaction";
-
- $joinWithSubSelect = false;
- $sql = '';
-
- // make sure the tables are joined in the right order
- // base table first, then action before conversion
- // this way, conversions can be left joined on idvisit
- $actionIndex = array_search("log_link_visit_action", $tables);
- $conversionIndex = array_search("log_conversion", $tables);
- if ($actionIndex > 0 && $conversionIndex > 0 && $actionIndex > $conversionIndex) {
- $tables[$actionIndex] = "log_conversion";
- $tables[$conversionIndex] = "log_link_visit_action";
- }
- // same as above: action before visit
- $actionIndex = array_search("log_link_visit_action", $tables);
- $visitIndex = array_search("log_visit", $tables);
- if ($actionIndex > 0 && $visitIndex > 0 && $actionIndex > $visitIndex) {
- $tables[$actionIndex] = "log_visit";
- $tables[$visitIndex] = "log_link_visit_action";
- }
-
- // we need to add log_link_visit_action dynamically to join eg visit with action
- $linkVisitAction = array_search("log_link_visit_action", $tables);
- $actionIndex = array_search("log_action", $tables);
- if ($linkVisitAction === false && $actionIndex > 0) {
- $tables[] = "log_link_visit_action";
- }
-
- if ($actionIndex > 0
- && $this->hasTableAddedManually('log_action', $tables)
- && !$this->hasJoinedTableAlreadyManually('log_action', $defaultLogActionJoin, $tables)) {
- // we cannot join the same table with same alias twice, therefore we need to combine the join via AND
- $tableIndex = $this->findIndexOfManuallyAddedTable('log_action', $tables);
- $defaultLogActionJoin = '(' . $tables[$tableIndex]['joinOn'] . ' AND ' . $defaultLogActionJoin . ')';
- unset($tables[$tableIndex]);
- }
-
- $linkVisitAction = array_search("log_link_visit_action", $tables);
- $actionIndex = array_search("log_action", $tables);
- if ($linkVisitAction > 0 && $actionIndex > 0 && $linkVisitAction > $actionIndex) {
- $tables[$actionIndex] = "log_link_visit_action";
- $tables[$linkVisitAction] = "log_action";
- }
-
- foreach ($tables as $i => $table) {
- if (is_array($table)) {
- // join condition provided
- $alias = isset($table['tableAlias']) ? $table['tableAlias'] : $table['table'];
- $sql .= "
- LEFT JOIN " . Common::prefixTable($table['table']) . " AS " . $alias
- . " ON " . $table['joinOn'];
- continue;
- }
-
- if (!in_array($table, $knownTables)) {
- throw new Exception("Table '$table' can't be used for segmentation");
- }
-
- $tableSql = Common::prefixTable($table) . " AS $table";
-
- if ($i == 0) {
- // first table
- $sql .= $tableSql;
- } else {
-
- if ($linkVisitActionsTableAvailable && $table === 'log_action') {
- $join = $defaultLogActionJoin;
-
- if ($this->hasJoinedTableAlreadyManually($table, $join, $tables)) {
- $actionsTableAvailable = true;
- continue;
- }
-
- } elseif ($linkVisitActionsTableAvailable && $table == "log_conversion") {
- // have actions, need conversions => join on idvisit
- $join = "log_conversion.idvisit = log_link_visit_action.idvisit";
- } elseif ($linkVisitActionsTableAvailable && $table == "log_visit") {
- // have actions, need visits => join on idvisit
- $join = "log_visit.idvisit = log_link_visit_action.idvisit";
-
- if ($this->hasJoinedTableAlreadyManually($table, $join, $tables)) {
- $visitsAvailable = true;
- continue;
- }
-
- } elseif ($visitsAvailable && $table == "log_link_visit_action") {
- // have visits, need actions => we have to use a more complex join
- // we don't hande this here, we just return joinWithSubSelect=true in this case
- $joinWithSubSelect = true;
- $join = "log_link_visit_action.idvisit = log_visit.idvisit";
-
- if ($this->hasJoinedTableAlreadyManually($table, $join, $tables)) {
- $linkVisitActionsTableAvailable = true;
- continue;
- }
-
- } elseif ($conversionsAvailable && $table == "log_link_visit_action") {
- // have conversions, need actions => join on idvisit
- $join = "log_conversion.idvisit = log_link_visit_action.idvisit";
- } elseif (($visitsAvailable && $table == "log_conversion")
- || ($conversionsAvailable && $table == "log_visit")
- ) {
- // have visits, need conversion (or vice versa) => join on idvisit
- // notice that joining conversions on visits has lower priority than joining it on actions
- $join = "log_conversion.idvisit = log_visit.idvisit";
-
- // if conversions are joined on visits, we need a complex join
- if ($table == "log_conversion") {
- $joinWithSubSelect = true;
- }
- } elseif ($conversionItemAvailable && $table === 'log_visit') {
- $join = "log_conversion_item.idvisit = log_visit.idvisit";
- } elseif ($conversionItemAvailable && $table === 'log_link_visit_action') {
- $join = "log_conversion_item.idvisit = log_link_visit_action.idvisit";
- } elseif ($conversionItemAvailable && $table === 'log_conversion') {
- $join = "log_conversion_item.idvisit = log_conversion.idvisit";
- } else {
- throw new Exception("Table '$table' can't be joined for segmentation");
- }
-
- // the join sql the default way
- $sql .= "
- LEFT JOIN $tableSql ON $join";
- }
-
- // remember which tables are available
- $visitsAvailable = ($visitsAvailable || $table == "log_visit");
- $linkVisitActionsTableAvailable = ($linkVisitActionsTableAvailable || $table == "log_link_visit_action");
- $actionsTableAvailable = ($actionsTableAvailable || $table == "log_action");
- $conversionsAvailable = ($conversionsAvailable || $table == "log_conversion");
- $conversionItemAvailable = ($conversionItemAvailable || $table == "log_conversion_item");
- }
-
- $return = array(
- 'sql' => $sql,
- 'joinWithSubSelect' => $joinWithSubSelect
- );
- return $return;
- }
-
-
/**
* Build a select query where actions have to be joined on visits (or conversions)
* In this case, the query gets wrapped in another query so that grouping by visit is possible
@@ -256,7 +93,7 @@ class LogQueryBuilder
*/
private function buildWrappedSelectQuery($select, $from, $where, $groupBy, $orderBy, $limitAndOffset, $innerGroupBy = null)
{
- $matchTables = "(log_visit|log_conversion_item|log_conversion|log_action)";
+ $matchTables = '(' . implode('|', $this->getKnownTables()) . ')';
preg_match_all("/". $matchTables ."\.[a-z0-9_\*]+/", $select, $matches);
$neededFields = array_unique($matches[0]);
diff --git a/core/DataAccess/LogQueryBuilder/JoinGenerator.php b/core/DataAccess/LogQueryBuilder/JoinGenerator.php
new file mode 100644
index 0000000000..1bd3ab3912
--- /dev/null
+++ b/core/DataAccess/LogQueryBuilder/JoinGenerator.php
@@ -0,0 +1,264 @@
+<?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\DataAccess\LogQueryBuilder;
+
+use Exception;
+use Piwik\Common;
+use Piwik\Tracker\LogTable;
+
+class JoinGenerator
+{
+ /**
+ * @var JoinTables
+ */
+ protected $tables;
+
+ /**
+ * @var bool
+ */
+ private $joinWithSubSelect = false;
+
+ /**
+ * @var string
+ */
+ private $joinString = '';
+
+ /**
+ * @var array
+ */
+ private $nonVisitJoins = array();
+
+ public function __construct(JoinTables $tables)
+ {
+ $this->tables = $tables;
+ $this->addMissingTablesNeededForJoins();
+ }
+
+ private function addMissingTablesNeededForJoins()
+ {
+ foreach ($this->tables as $index => $table) {
+ if (is_array($table)) {
+ continue;
+ }
+
+ $logTable = $this->tables->getLogTable($table);
+
+ if (!$logTable->getColumnToJoinOnIdVisit()) {
+ $tableNameToJoin = $logTable->getLinkTableToBeAbleToJoinOnVisit();
+
+ if ($index > 0 && !$this->tables->hasJoinedTable($tableNameToJoin)) {
+ $this->tables->addTableToJoin($tableNameToJoin);
+ }
+
+ if ($this->tables->hasJoinedTable($tableNameToJoin)) {
+ $this->generateNonVisitJoins($table, $tableNameToJoin, $index);
+ }
+ }
+ }
+ }
+
+ /**
+ * Generate the join sql based on the needed tables
+ * @throws Exception if tables can't be joined
+ * @return array
+ */
+ public function generate()
+ {
+ /** @var LogTable[] $availableLogTables */
+ $availableLogTables = array();
+
+ $this->tables->sort(array($this, 'sortTablesForJoin'));
+
+ foreach ($this->tables as $i => $table) {
+ if (is_array($table)) {
+
+ // join condition provided
+ $alias = isset($table['tableAlias']) ? $table['tableAlias'] : $table['table'];
+ $this->joinString .= " LEFT JOIN " . Common::prefixTable($table['table']) . " AS " . $alias
+ . " ON " . $table['joinOn'];
+ continue;
+ }
+
+ $tableSql = Common::prefixTable($table) . " AS $table";
+
+ $logTable = $this->tables->getLogTable($table);
+
+ if ($i == 0) {
+ // first table
+ $this->joinString .= $tableSql;
+ } else {
+
+ $join = $this->findJoinCriteriasForTables($logTable, $availableLogTables);
+
+ if ($join === null) {
+ $availableLogTables[$table] = $logTable;
+ continue;
+ }
+
+ // the join sql the default way
+ $this->joinString .= " LEFT JOIN $tableSql ON " . $join;
+ }
+
+ $availableLogTables[$table] = $logTable;
+ }
+ }
+
+ public function getJoinString()
+ {
+ return $this->joinString;
+ }
+
+ public function shouldJoinWithSelect()
+ {
+ return $this->joinWithSubSelect;
+ }
+
+ /**
+ * @param LogTable $logTable
+ * @param LogTable[] $availableLogTables
+ * @return string|null returns null in case the table is already joined, or the join string if the table needs
+ * to be joined
+ * @throws Exception if table cannot be joined for segmentation
+ */
+ protected function findJoinCriteriasForTables(LogTable $logTable, $availableLogTables)
+ {
+ $join = null;
+ $alternativeJoin = null;
+ $table = $logTable->getName();
+
+ foreach ($availableLogTables as $availableLogTable) {
+ if ($logTable->getColumnToJoinOnIdVisit() && $availableLogTable->getColumnToJoinOnIdVisit()) {
+
+ $join = sprintf("%s.%s = %s.%s", $table, $logTable->getColumnToJoinOnIdVisit(),
+ $availableLogTable->getName(), $availableLogTable->getColumnToJoinOnIdVisit());
+ $alternativeJoin = sprintf("%s.%s = %s.%s", $availableLogTable->getName(), $availableLogTable->getColumnToJoinOnIdVisit(),
+ $table, $logTable->getColumnToJoinOnIdVisit());
+
+ if ($availableLogTable->shouldJoinWithSubSelect()) {
+ $this->joinWithSubSelect = true;
+ }
+
+ break;
+ }
+
+ if ($logTable->getColumnToJoinOnIdAction() && $availableLogTable->getColumnToJoinOnIdAction()) {
+ if (isset($this->nonVisitJoins[$logTable->getName()][$availableLogTable->getName()])) {
+ $join = $this->nonVisitJoins[$logTable->getName()][$availableLogTable->getName()];
+ }
+
+ break;
+ }
+ }
+
+ if (!isset($join)) {
+ throw new Exception("Table '$table' can't be joined for segmentation");
+ }
+
+ if ($this->tables->hasJoinedTableManually($table, $join)
+ || $this->tables->hasJoinedTableManually($table, $alternativeJoin)) {
+ // already joined, no need to join it again
+ return null;
+ }
+
+ return $join;
+ }
+
+ /**
+ * This code is a bit tricky. We have to execute this right at the beginning before actually iterating over all the
+ * tables and generating the join string as we may have to delete a table from the tables. If we did not delete
+ * this table upfront, we would have maybe already added a joinString for that table, even though it will be later
+ * removed by another table. This means if we wouldn't delete/unset that table upfront, we would need to alter
+ * an already generated join string which would not be really nice code as well.
+ *
+ * Next problem is, because we are deleting a table, we have to remember the "joinOn" string for that table in a
+ * property "nonVisitJoins". Otherwise we would not be able to generate the correct "joinOn" string when actually
+ * iterating over all the tables to generate that string.
+ *
+ * @param $tableName
+ * @param $tableNameToJoin
+ * @param $index
+ */
+ protected function generateNonVisitJoins($tableName, $tableNameToJoin, $index)
+ {
+ $logTable = $this->tables->getLogTable($tableName);
+ $logTableToJoin = $this->tables->getLogTable($tableNameToJoin);
+
+ $nonVisitJoin = sprintf("%s.%s = %s.%s", $logTableToJoin->getName(), $logTableToJoin->getColumnToJoinOnIdAction(),
+ $tableName, $logTable->getColumnToJoinOnIdAction());
+
+ $altNonVisitJoin = sprintf("%s.%s = %s.%s", $tableName, $logTable->getColumnToJoinOnIdAction(),
+ $logTableToJoin->getName(), $logTableToJoin->getColumnToJoinOnIdAction());
+
+ if ($index > 0
+ && $this->tables->hasAddedTableManually($tableName)
+ && !$this->tables->hasJoinedTableManually($tableName, $nonVisitJoin)
+ && !$this->tables->hasJoinedTableManually($tableName, $altNonVisitJoin)) {
+ $tableIndex = $this->tables->findIndexOfManuallyAddedTable($tableName);
+ $nonVisitJoin = '(' . $this->tables[$tableIndex]['joinOn'] . ' AND ' . $nonVisitJoin . ')';
+ unset($this->tables[$tableIndex]);
+ }
+
+ if (!isset($this->nonVisitJoins[$tableName])) {
+ $this->nonVisitJoins[$tableName] = array();
+ }
+
+ if (!isset($this->nonVisitJoins[$tableNameToJoin])) {
+ $this->nonVisitJoins[$tableNameToJoin] = array();
+ }
+
+ $this->nonVisitJoins[$tableName][$tableNameToJoin] = $nonVisitJoin;
+ $this->nonVisitJoins[$tableNameToJoin][$tableName] = $nonVisitJoin;
+ }
+
+ public function sortTablesForJoin($tA, $tB)
+ {
+ $coreSort = array(
+ 'log_link_visit_action' => 0,
+ 'log_action' => 1,
+ 'log_visit' => 2,
+ 'log_conversion' => 3,
+ 'log_conversion_item' => 4
+ );
+
+ if (is_array($tA) && is_array($tB)) {
+ return 0;
+ }
+
+ if (is_array($tA)) {
+ return -1;
+ }
+
+ if (is_array($tB)) {
+ return 1;
+ }
+
+ if (isset($coreSort[$tA])) {
+ $weightA = $coreSort[$tA];
+ } else {
+ $weightA = 999;
+ }
+ if (isset($coreSort[$tB])) {
+ $weightB = $coreSort[$tB];
+ } else {
+ $weightB = 999;
+ }
+
+ if ($weightA === $weightB) {
+ return 0;
+ }
+
+ if ($weightA > $weightB) {
+ return 1;
+ }
+
+ return -1;
+ }
+
+}
diff --git a/core/DataAccess/LogQueryBuilder/JoinTables.php b/core/DataAccess/LogQueryBuilder/JoinTables.php
new file mode 100644
index 0000000000..d840cfba3f
--- /dev/null
+++ b/core/DataAccess/LogQueryBuilder/JoinTables.php
@@ -0,0 +1,114 @@
+<?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\DataAccess\LogQueryBuilder;
+
+use Exception;
+use Piwik\Plugin\LogTablesProvider;
+
+class JoinTables extends \ArrayObject
+{
+ /**
+ * @var LogTablesProvider
+ */
+ private $logTableProvider;
+
+ /**
+ * Tables constructor.
+ * @param LogTablesProvider $logTablesProvider
+ * @param array $tables
+ */
+ public function __construct(LogTablesProvider $logTablesProvider, $tables)
+ {
+ $this->logTableProvider = $logTablesProvider;
+
+ foreach ($tables as $table) {
+ $this->checkTableCanBeUsedForSegmentation($table);
+ }
+
+ $this->exchangeArray(array_values($tables));
+ }
+
+ public function getTables()
+ {
+ return $this->getArrayCopy();
+ }
+
+ public function addTableToJoin($tableName)
+ {
+ $this->checkTableCanBeUsedForSegmentation($tableName);
+ $this->append($tableName);
+ }
+
+ public function hasJoinedTable($tableName)
+ {
+ return in_array($tableName, $this->getTables());
+ }
+
+ public function hasJoinedTableManually($tableToFind, $joinToFind)
+ {
+ foreach ($this as $table) {
+ if (is_array($table)
+ && !empty($table['table'])
+ && $table['table'] === $tableToFind
+ && (!isset($table['tableAlias']) || $table['tableAlias'] === $tableToFind)
+ && isset($table['joinOn']) && $table['joinOn'] === $joinToFind) {
+ return true;
+ }
+ }
+
+ return false;
+ }
+
+ public function getLogTable($tableName)
+ {
+ return $this->logTableProvider->getLogTable($tableName);
+ }
+
+ public function findIndexOfManuallyAddedTable($tableNameToFind)
+ {
+ foreach ($this as $index => $table) {
+ if (is_array($table)
+ && !empty($table['table'])
+ && $table['table'] === $tableNameToFind
+ && (!isset($table['tableAlias']) || $table['tableAlias'] === $tableNameToFind)) {
+ return $index;
+ }
+ }
+ }
+
+ public function hasAddedTableManually($tableToFind)
+ {
+ $table = $this->findIndexOfManuallyAddedTable($tableToFind);
+
+ return isset($table);
+ }
+
+ public function sort($cmpFunction)
+ {
+ // we do not use $this->uasort as we do not want to maintain keys
+ $tables = $this->getTables();
+
+ // we need to make sure first table always comes first, only sort tables after the first table
+ $firstTable = array_shift($tables);
+ usort($tables, $cmpFunction);
+ array_unshift($tables, $firstTable);
+
+ $this->exchangeArray($tables);
+ }
+
+ private function checkTableCanBeUsedForSegmentation($tableName)
+ {
+ if (!is_array($tableName) && !$this->getLogTable($tableName)) {
+ throw new Exception("Table '$tableName' can't be used for segmentation");
+ }
+ }
+
+
+}
diff --git a/core/Plugin/LogTablesProvider.php b/core/Plugin/LogTablesProvider.php
new file mode 100644
index 0000000000..92afca14c7
--- /dev/null
+++ b/core/Plugin/LogTablesProvider.php
@@ -0,0 +1,65 @@
+<?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\Plugin;
+
+use Piwik\Container\StaticContainer;
+use Piwik\Tracker\LogTable;
+
+class LogTablesProvider {
+
+ /**
+ * @var Manager
+ */
+ private $pluginManager;
+
+ /**
+ * @var LogTable[]
+ */
+ private $tablesCache;
+
+ public function __construct(Manager $pluginManager)
+ {
+ $this->pluginManager = $pluginManager;
+ }
+
+ /**
+ * Get an instance of a specific log table if such a log table exists.
+ *
+ * @param string $tableNameWithoutPrefix eg "log_visit"
+ * @return LogTable|null
+ */
+ public function getLogTable($tableNameWithoutPrefix)
+ {
+ foreach ($this->getAllLogTables() as $table) {
+ if ($table->getName() === $tableNameWithoutPrefix) {
+ return $table;
+ }
+ }
+ }
+
+ /**
+ * Get all log table instances defined by any activated and loaded plugin. The returned tables are not sorted in
+ * any order.
+ * @return LogTable[]
+ */
+ public function getAllLogTables()
+ {
+ if (!isset($this->tablesCache)) {
+ $tables = $this->pluginManager->findMultipleComponents('Tracker', 'Piwik\\Tracker\\LogTable');
+
+ $this->tablesCache = array();
+ foreach ($tables as $table) {
+ $this->tablesCache[] = StaticContainer::get($table);
+ }
+ }
+
+ return $this->tablesCache;
+ }
+
+}
diff --git a/core/Tracker/LogTable.php b/core/Tracker/LogTable.php
new file mode 100644
index 0000000000..8e66f43463
--- /dev/null
+++ b/core/Tracker/LogTable.php
@@ -0,0 +1,75 @@
+<?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;
+
+/**
+ * Base class for LogTables. You need to create a log table eg if you want to be able to create a segment for a custom
+ * log table.
+ */
+abstract class LogTable {
+
+ /**
+ * Get the unprefixed database table name. For example 'log_visit' or 'log_action'.
+ * @return string
+ */
+ abstract public function getName();
+
+ /**
+ * Get the name of the column that can be used to join a visit with another table. This is the name of the column
+ * that represents the "idvisit".
+ * @return string
+ */
+ public function getColumnToJoinOnIdVisit()
+ {
+ return '';
+ }
+
+ /**
+ * Get the name of the column that can be used to join an action with another table. This is the name of the column
+ * that represents the "idaction".
+ *
+ * This could be more generic eg by specifiying "$this->joinableOn = array('action' => 'idaction') and this
+ * would allow to also add more complex structures in the future but not needed for now I'd say. Let's go with
+ * simpler, more clean and expressive solution for now until needed.
+ *
+ * @return string
+ */
+ public function getColumnToJoinOnIdAction()
+ {
+ return '';
+ }
+
+ /**
+ * Defines whether this table should be joined via a subselect. Return true if a complex join is needed. (eg when
+ * having visits and needing actions, or when having visits and needing conversions, or vice versa).
+ * @return bool
+ */
+ public function shouldJoinWithSubSelect()
+ {
+ return false;
+ }
+
+ /**
+ * Returns the name of a log table that allows to join on a visit. Eg if there is a table "action", and it is not
+ * joinable with "visit" table, it can return "log_link_visit_action" to be able to join the action table on visit
+ * via this link table.
+ *
+ * In theory there could be case where it may be needed to join via two tables, so it could be needed at some
+ * point to return an array of tables here. not sure if we should handle this case just yet. Alternatively,
+ * once needed eg in LogQueryBuilder, we should maybe better call instead ->getLinkTableToBeAbleToJoinOnVisit()
+ * again on the returned table until we have found a table that can be joined with visit.
+ *
+ * @return string
+ */
+ public function getLinkTableToBeAbleToJoinOnVisit()
+ {
+ return;
+ }
+
+}
diff --git a/core/Tracker/Visit.php b/core/Tracker/Visit.php
index f4485f52fb..9de1490424 100644
--- a/core/Tracker/Visit.php
+++ b/core/Tracker/Visit.php
@@ -397,6 +397,11 @@ class Visit implements VisitInterface
*/
protected function updateExistingVisit($valuesToUpdate)
{
+ if (empty($valuesToUpdate)) {
+ Common::printDebug('There are no values to be updated for this visit');
+ return;
+ }
+
$idSite = $this->request->getIdSite();
$idVisit = (int)$this->visitProperties->getProperty('idvisit');
diff --git a/plugins/CoreHome/Tracker/LogTable/Action.php b/plugins/CoreHome/Tracker/LogTable/Action.php
new file mode 100644
index 0000000000..fdb69e5ec6
--- /dev/null
+++ b/plugins/CoreHome/Tracker/LogTable/Action.php
@@ -0,0 +1,30 @@
+<?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\Plugins\CoreHome\Tracker\LogTable;
+
+use Piwik\Tracker\LogTable;
+
+class Action extends LogTable
+{
+ public function getName()
+ {
+ return 'log_action';
+ }
+
+ public function getColumnToJoinOnIdAction()
+ {
+ return 'idaction';
+ }
+
+ public function getLinkTableToBeAbleToJoinOnVisit()
+ {
+ return 'log_link_visit_action';
+ }
+
+}
diff --git a/plugins/CoreHome/Tracker/LogTable/Conversion.php b/plugins/CoreHome/Tracker/LogTable/Conversion.php
new file mode 100644
index 0000000000..e920864088
--- /dev/null
+++ b/plugins/CoreHome/Tracker/LogTable/Conversion.php
@@ -0,0 +1,24 @@
+<?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\Plugins\CoreHome\Tracker\LogTable;
+
+use Piwik\Tracker\LogTable;
+
+class Conversion extends LogTable
+{
+ public function getName()
+ {
+ return 'log_conversion';
+ }
+
+ public function getColumnToJoinOnIdVisit()
+ {
+ return 'idvisit';
+ }
+}
\ No newline at end of file
diff --git a/plugins/CoreHome/Tracker/LogTable/ConversionItem.php b/plugins/CoreHome/Tracker/LogTable/ConversionItem.php
new file mode 100644
index 0000000000..566841b4fa
--- /dev/null
+++ b/plugins/CoreHome/Tracker/LogTable/ConversionItem.php
@@ -0,0 +1,25 @@
+<?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\Plugins\CoreHome\Tracker\LogTable;
+
+use Piwik\Tracker\LogTable;
+
+class ConversionItem extends LogTable
+{
+ public function getName()
+ {
+ return 'log_conversion_item';
+ }
+
+ public function getColumnToJoinOnIdVisit()
+ {
+ return 'idvisit';
+ }
+
+}
diff --git a/plugins/CoreHome/Tracker/LogTable/LinkVisitAction.php b/plugins/CoreHome/Tracker/LogTable/LinkVisitAction.php
new file mode 100644
index 0000000000..cb102d407c
--- /dev/null
+++ b/plugins/CoreHome/Tracker/LogTable/LinkVisitAction.php
@@ -0,0 +1,29 @@
+<?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\Plugins\CoreHome\Tracker\LogTable;
+
+use Piwik\Tracker\LogTable;
+
+class LinkVisitAction extends LogTable
+{
+ public function getName()
+ {
+ return 'log_link_visit_action';
+ }
+
+ public function getColumnToJoinOnIdAction()
+ {
+ return 'idaction_url';
+ }
+
+ public function getColumnToJoinOnIdVisit()
+ {
+ return 'idvisit';
+ }
+}
diff --git a/plugins/CoreHome/Tracker/LogTable/Visit.php b/plugins/CoreHome/Tracker/LogTable/Visit.php
new file mode 100644
index 0000000000..f403c37697
--- /dev/null
+++ b/plugins/CoreHome/Tracker/LogTable/Visit.php
@@ -0,0 +1,30 @@
+<?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\Plugins\CoreHome\Tracker\LogTable;
+
+use Piwik\Tracker\LogTable;
+
+class Visit extends LogTable
+{
+ public function getName()
+ {
+ return 'log_visit';
+ }
+
+ public function getColumnToJoinOnIdVisit()
+ {
+ return 'idvisit';
+ }
+
+ public function shouldJoinWithSubSelect()
+ {
+ return true;
+ }
+
+}
\ No newline at end of file
diff --git a/plugins/SegmentEditor/SegmentQueryDecorator.php b/plugins/SegmentEditor/SegmentQueryDecorator.php
index d501f6f9b1..130cc59bd5 100644
--- a/plugins/SegmentEditor/SegmentQueryDecorator.php
+++ b/plugins/SegmentEditor/SegmentQueryDecorator.php
@@ -9,6 +9,7 @@
namespace Piwik\Plugins\SegmentEditor;
use Piwik\DataAccess\LogQueryBuilder;
+use Piwik\Plugin\LogTablesProvider;
use Piwik\Plugins\SegmentEditor\Services\StoredSegmentService;
use Piwik\Segment\SegmentExpression;
use Piwik\SettingsServer;
@@ -26,9 +27,10 @@ class SegmentQueryDecorator extends LogQueryBuilder
*/
private $storedSegmentService;
- public function __construct(StoredSegmentService $storedSegmentService)
+ public function __construct(StoredSegmentService $storedSegmentService, LogTablesProvider $logTablesProvider)
{
$this->storedSegmentService = $storedSegmentService;
+ parent::__construct($logTablesProvider);
}
public function getSelectQueryString(SegmentExpression $segmentExpression, $select, $from, $where, $bind, $groupBy,
diff --git a/plugins/SegmentEditor/tests/Unit/SegmentQueryDecoratorTest.php b/plugins/SegmentEditor/tests/Unit/SegmentQueryDecoratorTest.php
index 6ecad82f2f..7e38188966 100644
--- a/plugins/SegmentEditor/tests/Unit/SegmentQueryDecoratorTest.php
+++ b/plugins/SegmentEditor/tests/Unit/SegmentQueryDecoratorTest.php
@@ -11,6 +11,7 @@ namespace Piwik\Plugins\SegmentEditor\tests\Unit;
use Piwik\Plugins\SegmentEditor\SegmentQueryDecorator;
use Piwik\Plugins\SegmentEditor\Services\StoredSegmentService;
use Piwik\Segment\SegmentExpression;
+use Piwik\Tests\Framework\Mock\Plugin\LogTablesProvider;
/**
* @group SegmentEditor
@@ -36,7 +37,8 @@ class SegmentQueryDecoratorTest extends \PHPUnit_Framework_TestCase
/** @var StoredSegmentService $service */
$service = $this->getMockSegmentEditorService();
- $this->decorator = new SegmentQueryDecorator($service);
+ $logTables = new LogTablesProvider();
+ $this->decorator = new SegmentQueryDecorator($service, $logTables);
}
public function test_getSelectQueryString_DoesNotDecorateSql_WhenNoSegmentUsed()
@@ -86,4 +88,5 @@ class SegmentQueryDecoratorTest extends \PHPUnit_Framework_TestCase
$mock->expects($this->any())->method('getAllSegmentsAndIgnoreVisibility')->willReturn(self::$storedSegments);
return $mock;
}
+
}
diff --git a/tests/PHPUnit/Framework/Mock/Plugin/LogTablesProvider.php b/tests/PHPUnit/Framework/Mock/Plugin/LogTablesProvider.php
new file mode 100644
index 0000000000..c563e250cd
--- /dev/null
+++ b/tests/PHPUnit/Framework/Mock/Plugin/LogTablesProvider.php
@@ -0,0 +1,28 @@
+<?php
+
+namespace Piwik\Tests\Framework\Mock\Plugin;
+
+use Piwik\Plugins\CoreHome\Tracker\LogTable\Action;
+use Piwik\Plugins\CoreHome\Tracker\LogTable\Conversion;
+use Piwik\Plugins\CoreHome\Tracker\LogTable\ConversionItem;
+use Piwik\Plugins\CoreHome\Tracker\LogTable\LinkVisitAction;
+use Piwik\Plugins\CoreHome\Tracker\LogTable\Visit;
+
+class LogTablesProvider extends \Piwik\Plugin\LogTablesProvider
+{
+ public function __construct()
+ {
+ }
+
+ public function getAllLogTables()
+ {
+ return array(
+ new Visit(),
+ new Action(),
+ new LinkVisitAction(),
+ new ConversionItem(),
+ new Conversion()
+ );
+ }
+
+}
diff --git a/tests/PHPUnit/Integration/SegmentTest.php b/tests/PHPUnit/Integration/SegmentTest.php
index 9f626e7969..37d13cc2e7 100644
--- a/tests/PHPUnit/Integration/SegmentTest.php
+++ b/tests/PHPUnit/Integration/SegmentTest.php
@@ -271,7 +271,7 @@ class SegmentTest extends IntegrationTestCase
*
FROM
" . Common::prefixTable('log_conversion') . " AS log_conversion
- LEFT JOIN " . Common::prefixTable('log_link_visit_action') . " AS log_link_visit_action ON log_conversion.idvisit = log_link_visit_action.idvisit
+ LEFT JOIN " . Common::prefixTable('log_link_visit_action') . " AS log_link_visit_action ON log_link_visit_action.idvisit = log_conversion.idvisit
WHERE
( log_conversion.idvisit = ? )
AND
@@ -361,7 +361,7 @@ class SegmentTest extends IntegrationTestCase
*
FROM
" . Common::prefixTable('log_conversion') . " AS log_conversion
- LEFT JOIN " . Common::prefixTable('log_visit') . " AS log_visit ON log_conversion.idvisit = log_visit.idvisit
+ LEFT JOIN " . Common::prefixTable('log_visit') . " AS log_visit ON log_visit.idvisit = log_conversion.idvisit
WHERE
( log_conversion.idvisit = ? )
AND
@@ -524,7 +524,7 @@ class SegmentTest extends IntegrationTestCase
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
- LEFT JOIN " . Common::prefixTable('log_conversion') . " AS log_conversion ON log_conversion.idvisit = log_link_visit_action.idvisit
+ LEFT JOIN " . Common::prefixTable('log_conversion') . " AS log_conversion ON log_conversion.idvisit = log_visit.idvisit
WHERE
log_conversion.idgoal = ? AND HOUR(log_visit.visit_last_action_time) = ? AND log_link_visit_action.custom_var_k1 = ?
AND (
@@ -693,7 +693,7 @@ class SegmentTest extends IntegrationTestCase
SELECT log_conversion.idgoal AS `idgoal`, log_conversion.custom_dimension_1 AS `custom_dimension_1`, count(*) AS `1`, count(distinct log_conversion.idvisit) AS `3`,
FROM $logConversionsTable AS log_conversion
LEFT JOIN $logLinkVisitActionTable AS log_link_visit_action
- ON log_conversion.idvisit = log_link_visit_action.idvisit
+ ON log_link_visit_action.idvisit = log_conversion.idvisit
LEFT JOIN $logActionTable AS log_action
ON log_link_visit_action.idaction_url = log_action.idaction
WHERE ( log_conversion.server_time >= ?
@@ -761,7 +761,7 @@ class SegmentTest extends IntegrationTestCase
SUM(log_conversion.items) AS `8`
FROM
" . Common::prefixTable('log_conversion') . " AS log_conversion
- LEFT JOIN " . Common::prefixTable('log_link_visit_action') . " AS log_link_visit_action ON log_conversion.idvisit = log_link_visit_action.idvisit
+ LEFT JOIN " . Common::prefixTable('log_link_visit_action') . " AS log_link_visit_action ON log_link_visit_action.idvisit = log_conversion.idvisit
WHERE
( log_conversion.idsite IN (?) )
AND
@@ -1289,7 +1289,7 @@ class SegmentTest extends IntegrationTestCase
FROM (
SELECT log_conversion.idgoal, log_conversion.idvisit, log_conversion.revenue, log_conversion.items
FROM $logConversionTable AS log_conversion
- LEFT JOIN $logLinkVisitActionTable AS log_link_visit_action ON log_conversion.idvisit = log_link_visit_action.idvisit
+ LEFT JOIN $logLinkVisitActionTable AS log_link_visit_action ON log_link_visit_action.idvisit = log_conversion.idvisit
WHERE ( log_conversion.server_time >= ?
AND log_conversion.server_time <= ?
AND log_conversion.idsite IN (?) )
@@ -1340,7 +1340,7 @@ class SegmentTest extends IntegrationTestCase
FROM (
SELECT log_conversion.idgoal, log_conversion.referer_type, log_conversion.referer_name, log_conversion.referer_keyword, log_conversion.idvisit, log_conversion.revenue
FROM $logConversionTable AS log_conversion
- LEFT JOIN $logLinkVisitActionTable AS log_link_visit_action ON log_conversion.idvisit = log_link_visit_action.idvisit
+ LEFT JOIN $logLinkVisitActionTable AS log_link_visit_action ON log_link_visit_action.idvisit = log_conversion.idvisit
WHERE ( log_conversion.server_time >= ?
AND log_conversion.server_time <= ?
AND log_conversion.idsite IN (?) )
@@ -1385,8 +1385,8 @@ class SegmentTest extends IntegrationTestCase
FROM (
SELECT log_conversion.idgoal, log_conversion.idvisit, log_conversion.revenue
FROM $logConversionTable AS log_conversion
- LEFT JOIN $logLinkVisitActionTable AS log_link_visit_action ON log_conversion.idvisit = log_link_visit_action.idvisit
- LEFT JOIN $logVisitTable AS log_visit ON log_visit.idvisit = log_link_visit_action.idvisit
+ LEFT JOIN $logLinkVisitActionTable AS log_link_visit_action ON log_link_visit_action.idvisit = log_conversion.idvisit
+ LEFT JOIN $logVisitTable AS log_visit ON log_visit.idvisit = log_conversion.idvisit
WHERE ( log_conversion.server_time >= ?
AND log_conversion.server_time <= ?
AND log_conversion.idsite IN (?) )
diff --git a/tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinGeneratorTest.php b/tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinGeneratorTest.php
new file mode 100644
index 0000000000..cd572e4849
--- /dev/null
+++ b/tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinGeneratorTest.php
@@ -0,0 +1,171 @@
+<?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\Tests\Unit\DataAccess;
+
+use Piwik\DataAccess\LogQueryBuilder\JoinGenerator;
+use Piwik\DataAccess\LogQueryBuilder\JoinTables;
+use Piwik\Tests\Framework\Mock\Plugin\LogTablesProvider;
+use Piwik\Tracker\Visit;
+
+/**
+ * @group Core
+ */
+class JoinGeneratorTest extends \PHPUnit_Framework_TestCase
+{
+ /**
+ * @var JoinGenerator
+ */
+ private $generator;
+
+ public function setUp()
+ {
+ $this->generator = $this->makeTables(array(
+ 'log_visit',
+ array('table' => 'log_conversion', 'joinOn' => 'log_conversion.idvisit = log_visit.idvisit'),
+ 'log_action'));
+ }
+
+ public function test_constructor_shouldAddTablesIfNeeded()
+ {
+ $tables = $this->makeTables(array('log_visit', 'log_action'));
+ $this->makeGenerator($tables);
+
+ $this->assertEquals(array('log_visit', 'log_action', 'log_link_visit_action'), $tables->getTables());
+ }
+
+ public function test_generate_shouldJoinWithSubselect_IfBaseTableIsLogVisit()
+ {
+ $generator = $this->generate(array('log_visit', 'log_action'));
+ $this->assertTrue($generator->shouldJoinWithSelect());
+ }
+
+ public function test_generate_shouldNotJoinWithSubselect_IfBaseTableIsLogVisitButNoTableToJoin()
+ {
+ $generator = $this->generate(array('log_visit'));
+ $this->assertFalse($generator->shouldJoinWithSelect());
+ }
+
+ public function test_generate_shouldNotJoinWithSubselect_IfLogVisitIsGivenButItIsNotBaseTable()
+ {
+ $generator = $this->generate(array('log_conversion', 'log_visit'));
+ $this->assertFalse($generator->shouldJoinWithSelect());
+ }
+
+ public function test_generate_getJoinString()
+ {
+ $generator = $this->generate(array('log_action', 'log_link_visit_action', 'log_visit'));
+
+ $expected = 'log_action AS log_action ';
+ $expected .= 'LEFT JOIN log_link_visit_action AS log_link_visit_action ON log_link_visit_action.idaction_url = log_action.idaction ';
+ $expected .= 'LEFT JOIN log_visit AS log_visit ON log_visit.idvisit = log_link_visit_action.idvisit';
+ $this->assertEquals($expected, $generator->getJoinString());
+ }
+
+ public function test_generate_getJoinString_OnlyOneTable()
+ {
+ $generator = $this->generate(array('log_visit'));
+ $this->assertEquals('log_visit AS log_visit', $generator->getJoinString());
+ }
+
+ public function test_generate_getJoinString_OnlyOneActionTable()
+ {
+ $generator = $this->generate(array('log_action'));
+ $this->assertEquals('log_action AS log_action', $generator->getJoinString());
+ }
+
+ public function test_generate_getJoinString_OnlyActionTables()
+ {
+ $generator = $this->generate(array('log_link_visit_action', 'log_action'));
+ $expected = 'log_link_visit_action AS log_link_visit_action';
+ $expected .= ' LEFT JOIN log_action AS log_action ON log_link_visit_action.idaction_url = log_action.idaction';
+ $this->assertEquals($expected, $generator->getJoinString());
+ }
+
+ public function test_generate_getJoinString_manuallyJoinedAlready()
+ {
+ $generator = $this->generate(array(
+ 'log_link_visit_action',
+ array('table' => 'log_visit', 'joinOn' => 'log_visit.idvisit = log_link_visit_action.idvisit'),
+ array('table' => 'log_action', 'joinOn' => 'log_link_visit_action.idaction_name = log_action.idaction'),
+ 'log_action'
+ ));
+
+ $expected = 'log_link_visit_action AS log_link_visit_action ';
+ $expected .= 'LEFT JOIN log_visit AS log_visit ON log_visit.idvisit = log_link_visit_action.idvisit ';
+ $expected .= 'LEFT JOIN log_action AS log_action ON (log_link_visit_action.idaction_name = log_action.idaction AND log_link_visit_action.idaction_url = log_action.idaction)';
+ $this->assertEquals($expected, $generator->getJoinString());
+ }
+
+ public function test_generate_getJoinString_allTables()
+ {
+ $generator = $this->generate(array(
+ 'log_action',
+ 'log_conversion_item',
+ 'log_link_visit_action',
+ 'log_conversion',
+ 'log_visit',
+ ));
+
+ $expected = 'log_action AS log_action ';
+ $expected .= 'LEFT JOIN log_link_visit_action AS log_link_visit_action ON log_link_visit_action.idaction_url = log_action.idaction ';
+ $expected .= 'LEFT JOIN log_visit AS log_visit ON log_visit.idvisit = log_link_visit_action.idvisit ';
+ $expected .= 'LEFT JOIN log_conversion AS log_conversion ON log_conversion.idvisit = log_link_visit_action.idvisit ';
+ $expected .= 'LEFT JOIN log_conversion_item AS log_conversion_item ON log_conversion_item.idvisit = log_link_visit_action.idvisit';
+ $this->assertEquals($expected, $generator->getJoinString());
+ }
+
+ public function test_sortTablesForJoin_shouldSortTablesAsSpecified()
+ {
+ $tables = array(
+ 'log_action',
+ array('table' => 'log_conversion', 'joinOn' => 'log_conversion.idvisit = log_visit.idvisit'),
+ 'log_conversion_item',
+ 'log_link_visit_action',
+ 'log_conversion',
+ 'log_visit',
+ );
+
+ $generator = $this->makeGenerator($tables);
+ $tables[] = 'log_foo_bar';
+ usort($tables, array($generator, 'sortTablesForJoin'));
+
+ $expected = array(
+ array('table' => 'log_conversion', 'joinOn' => 'log_conversion.idvisit = log_visit.idvisit'),
+ 'log_link_visit_action',
+ 'log_action',
+ 'log_visit',
+ 'log_conversion',
+ 'log_conversion_item',
+ 'log_foo_bar'
+ );
+
+ $this->assertEquals($expected, $tables);
+ }
+
+ private function generate($tables)
+ {
+ $generator = $this->makeGenerator($tables);
+ $generator->generate();
+ return $generator;
+ }
+
+ private function makeGenerator($tables)
+ {
+ if (is_array($tables)) {
+ $tables = $this->makeTables($tables);
+ }
+
+ return new JoinGenerator($tables);
+ }
+
+ private function makeTables($tables)
+ {
+ return new JoinTables(new LogTablesProvider(), $tables);
+ }
+}
\ No newline at end of file
diff --git a/tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinTablesTest.php b/tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinTablesTest.php
new file mode 100644
index 0000000000..8f5a52fd0b
--- /dev/null
+++ b/tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinTablesTest.php
@@ -0,0 +1,146 @@
+<?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\Tests\Unit\DataAccess;
+
+use Piwik\DataAccess\LogQueryBuilder\JoinTables;
+use Piwik\Tests\Framework\Mock\Plugin\LogTablesProvider;
+use Piwik\Tracker\Visit;
+
+/**
+ * @group Core
+ */
+class JoinTablesTest extends \PHPUnit_Framework_TestCase
+{
+ /**
+ * @var JoinTables
+ */
+ private $tables;
+
+ public function setUp()
+ {
+ $this->tables = $this->makeTables(array(
+ 'log_visit',
+ array('table' => 'log_conversion', 'joinOn' => 'log_conversion.idvisit = log_visit.idvisit'),
+ 'log_action'));
+ }
+
+ /**
+ * @expectedException \Exception
+ * @expectedExceptionMessage Table 'log_foo_bar_baz' can't be used for segmentation
+ */
+ public function test_construct_shouldThrowException_IfTableIsNotPossibleToJoin()
+ {
+ $this->makeTables(array('log_visit', 'log_foo_bar_baz'));
+ }
+
+ public function test_hasJoinedTable_shouldDetectIfTableIsAlreadyAdded()
+ {
+ $this->assertTrue($this->tables->hasJoinedTable('log_visit'));
+ $this->assertTrue($this->tables->hasJoinedTable('log_action'));
+
+ $this->assertFalse($this->tables->hasJoinedTable('log_foo_bar_baz'));
+ $this->assertFalse($this->tables->hasJoinedTable('log_conversion')); // we do not check for manually joined tables
+ }
+
+ public function test_addTableToJoin_shouldAddGivenTable()
+ {
+ $table = 'log_conversion_item';
+ $this->assertFalse($this->tables->hasJoinedTable($table));
+
+ $this->tables->addTableToJoin($table);
+
+ $this->assertTrue($this->tables->hasJoinedTable($table));
+ }
+
+ /**
+ * @expectedException \Exception
+ * @expectedExceptionMessage Table 'log_foo_bar_baz' can't be used for segmentation
+ */
+ public function test_addTableToJoin_shouldCheckIfTableCanBeUsedForSegmentation()
+ {
+ $table = 'log_foo_bar_baz';
+ $this->assertFalse($this->tables->hasJoinedTable($table));
+
+ $this->tables->addTableToJoin($table);
+
+ $this->assertTrue($this->tables->hasJoinedTable($table));
+ }
+
+ public function test_hasJoinedTableManually_shouldReturnTrue_IfTableJoinExistsExactlyAsGiven()
+ {
+ $result = $this->tables->hasJoinedTableManually('log_conversion', 'log_conversion.idvisit = log_visit.idvisit');
+
+ $this->assertTrue($result);
+ }
+
+ public function test_hasJoinedTableManually_shouldReturnFalse_IfTableOrJoinDoesNotMatch()
+ {
+ $result = $this->tables->hasJoinedTableManually('log_foo_bar_baz', 'log_conversion.idvisit = log_visit.idvisit');
+ $this->assertFalse($result);
+
+ $result = $this->tables->hasJoinedTableManually('log_conversion', 'log_foo_bar_baz.idvisit = log_visit.idvisit');
+ $this->assertFalse($result);
+ }
+
+ public function test_hasAddedTableManually_shouldReturnTrue_IfTableWasAddedManually()
+ {
+ $result = $this->tables->hasAddedTableManually('log_conversion');
+
+ $this->assertTrue($result);
+ }
+
+ public function test_hasAddedTableManually_shouldReturnFalse_IfTableWasNotAddedManually()
+ {
+ $result = $this->tables->hasAddedTableManually('log_foo_bar_baz');
+ $this->assertFalse($result);
+
+ $result = $this->tables->hasAddedTableManually('log_conversion_item');
+ $this->assertFalse($result);
+ }
+
+ public function test_getLogTable_shouldReturnInstanceOfLogTable_IfTableExists()
+ {
+ $visit = $this->tables->getLogTable('log_visit');
+ $this->assertFalse($visit instanceof Visit);
+ }
+
+ public function test_getLogTable_shouldReturnNull_IfLogTableDoesNotExist()
+ {
+ $visit = $this->tables->getLogTable('log_foo_bar_baz');
+ $this->assertNull($visit);
+ }
+
+ public function test_findIndexOfManuallyAddedTable_shouldReturnTheIndex_IfTableWasAddedManually()
+ {
+ $this->assertSame(1, $this->tables->findIndexOfManuallyAddedTable('log_conversion'));
+ }
+
+ public function test_findIndexOfManuallyAddedTable_shouldReturnNull_IfTableWasNotAddedManually()
+ {
+ $this->assertNull($this->tables->findIndexOfManuallyAddedTable('log_visit'));
+ $this->assertNull($this->tables->findIndexOfManuallyAddedTable('log_action'));
+ $this->assertNull($this->tables->findIndexOfManuallyAddedTable('log_foo_bar_baz'));
+ }
+
+ public function test_sort_shouldNeverSortFirstEntry_AndNotMaintainKeys()
+ {
+ $tables = $this->makeTables(array('log_conversion', 'log_visit', 'log_action', 'log_conversion_item'));
+ $tables->sort(function($a, $b) {
+ return strcmp($a, $b);
+ });
+
+ $expected = array('log_conversion', 'log_action', 'log_conversion_item', 'log_visit');
+ $this->assertEquals($expected, $tables->getTables());
+ }
+
+ private function makeTables($tables)
+ {
+ return new JoinTables(new LogTablesProvider(), $tables);
+ }
+}
\ No newline at end of file
--
GitLab