From 764a8c147f8992ae99a679573fc434d82dd7b0a6 Mon Sep 17 00:00:00 2001
From: mattab <matthieu.aubry@gmail.com>
Date: Fri, 5 Dec 2014 19:18:17 +1300
Subject: Moved SegmentExpression into namespace Piwik\Segment

---
 core/Segment.php                                   |   1 +
 core/Segment/SegmentExpression.php                 | 378 +++++++++++++++++++++
 core/SegmentExpression.php                         | 378 ---------------------
 core/Tracker/TableLogAction.php                    |   2 +-
 plugins/API/API.php                                |   2 +-
 plugins/Transitions/API.php                        |   2 +-
 plugins/VisitFrequency/API.php                     |   2 +-
 .../PHPUnit/Unit/Segment/SegmentExpressionTest.php | 132 +++++++
 tests/PHPUnit/Unit/SegmentExpressionTest.php       | 132 -------
 9 files changed, 515 insertions(+), 514 deletions(-)
 create mode 100644 core/Segment/SegmentExpression.php
 delete mode 100644 core/SegmentExpression.php
 create mode 100644 tests/PHPUnit/Unit/Segment/SegmentExpressionTest.php
 delete mode 100644 tests/PHPUnit/Unit/SegmentExpressionTest.php

diff --git a/core/Segment.php b/core/Segment.php
index 420946794d..d990ed16ff 100644
--- a/core/Segment.php
+++ b/core/Segment.php
@@ -10,6 +10,7 @@ namespace Piwik;
 
 use Exception;
 use Piwik\Plugins\API\API;
+use Piwik\Segment\SegmentExpression;
 
 /**
  * Limits the set of visits Piwik uses when aggregating analytics data.
diff --git a/core/Segment/SegmentExpression.php b/core/Segment/SegmentExpression.php
new file mode 100644
index 0000000000..e32bbe8723
--- /dev/null
+++ b/core/Segment/SegmentExpression.php
@@ -0,0 +1,378 @@
+<?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\Segment;
+
+use Exception;
+
+/**
+ *
+ */
+class SegmentExpression
+{
+    const AND_DELIMITER = ';';
+    const OR_DELIMITER = ',';
+
+    const MATCH_EQUAL = '==';
+    const MATCH_NOT_EQUAL = '!=';
+    const MATCH_GREATER_OR_EQUAL = '>=';
+    const MATCH_LESS_OR_EQUAL = '<=';
+    const MATCH_GREATER = '>';
+    const MATCH_LESS = '<';
+    const MATCH_CONTAINS = '=@';
+    const MATCH_DOES_NOT_CONTAIN = '!@';
+
+    // Note: you can't write this in the API, but access this feature
+    // via field!=        <- IS NOT NULL
+    // or via field==     <- IS NULL / empty
+    const MATCH_IS_NOT_NULL_NOR_EMPTY = '::NOT_NULL';
+    const MATCH_IS_NULL_OR_EMPTY = '::NULL';
+
+    // Special case, since we look up Page URLs/Page titles in a sub SQL query
+    const MATCH_ACTIONS_CONTAINS = 'IN';
+
+    const INDEX_BOOL_OPERATOR = 0;
+    const INDEX_OPERAND = 1;
+
+    function __construct($string)
+    {
+        $this->string = $string;
+        $this->tree = $this->parseTree();
+    }
+
+    protected $joins = array();
+    protected $valuesBind = array();
+    protected $parsedTree = array();
+    protected $tree = array();
+    protected $parsedSubExpressions = array();
+
+    /**
+     * Given the array of parsed filters containing, for each filter,
+     * the boolean operator (AND/OR) and the operand,
+     * Will return the array where the filters are in SQL representation
+     *
+     * @throws Exception
+     * @return array
+     */
+    public function parseSubExpressions()
+    {
+        $parsedSubExpressions = array();
+        foreach ($this->tree as $leaf) {
+            $operand = $leaf[self::INDEX_OPERAND];
+
+            $operand = urldecode($operand);
+
+            $operator = $leaf[self::INDEX_BOOL_OPERATOR];
+            $pattern = '/^(.+?)(' . self::MATCH_EQUAL . '|'
+                . self::MATCH_NOT_EQUAL . '|'
+                . self::MATCH_GREATER_OR_EQUAL . '|'
+                . self::MATCH_GREATER . '|'
+                . self::MATCH_LESS_OR_EQUAL . '|'
+                . self::MATCH_LESS . '|'
+                . self::MATCH_CONTAINS . '|'
+                . self::MATCH_DOES_NOT_CONTAIN
+                . '){1}(.*)/';
+            $match = preg_match($pattern, $operand, $matches);
+            if ($match == 0) {
+                throw new Exception('The segment \'' . $operand . '\' is not valid.');
+            }
+
+            $leftMember = $matches[1];
+            $operation  = $matches[2];
+            $valueRightMember = urldecode($matches[3]);
+
+            // is null / is not null
+            if ($valueRightMember === '') {
+                if ($operation == self::MATCH_NOT_EQUAL) {
+                    $operation = self::MATCH_IS_NOT_NULL_NOR_EMPTY;
+                } elseif ($operation == self::MATCH_EQUAL) {
+                    $operation = self::MATCH_IS_NULL_OR_EMPTY;
+                } else {
+                    throw new Exception('The segment \'' . $operand . '\' has no value specified. You can leave this value empty ' .
+                        'only when you use the operators: ' . self::MATCH_NOT_EQUAL . ' (is not) or ' . self::MATCH_EQUAL . ' (is)');
+                }
+            }
+
+            $parsedSubExpressions[] = array(
+                self::INDEX_BOOL_OPERATOR => $operator,
+                self::INDEX_OPERAND       => array(
+                    $leftMember,
+                    $operation,
+                    $valueRightMember,
+                ));
+        }
+        $this->parsedSubExpressions = $parsedSubExpressions;
+        return $parsedSubExpressions;
+    }
+
+    /**
+     * Set the given expression
+     * @param $parsedSubExpressions
+     */
+    public function setSubExpressionsAfterCleanup($parsedSubExpressions)
+    {
+        $this->parsedSubExpressions = $parsedSubExpressions;
+    }
+
+    /**
+     * @param array $availableTables
+     */
+    public function parseSubExpressionsIntoSqlExpressions(&$availableTables = array())
+    {
+        $sqlSubExpressions = array();
+        $this->valuesBind = array();
+        $this->joins = array();
+
+        foreach ($this->parsedSubExpressions as $leaf) {
+            $operator = $leaf[self::INDEX_BOOL_OPERATOR];
+            $operandDefinition = $leaf[self::INDEX_OPERAND];
+
+            $operand = $this->getSqlMatchFromDefinition($operandDefinition, $availableTables);
+
+            if ($operand[1] !== null) {
+                $this->valuesBind[] = $operand[1];
+            }
+
+            $operand = $operand[0];
+            $sqlSubExpressions[] = array(
+                self::INDEX_BOOL_OPERATOR => $operator,
+                self::INDEX_OPERAND       => $operand,
+            );
+        }
+
+        $this->tree = $sqlSubExpressions;
+    }
+
+    /**
+     * Given an array representing one filter operand ( left member , operation , right member)
+     * Will return an array containing
+     * - the SQL substring,
+     * - the values to bind to this substring
+     *
+     * @param array $def
+     * @param array $availableTables
+     * @throws Exception
+     * @return array
+     */
+    protected function getSqlMatchFromDefinition($def, &$availableTables)
+    {
+        $field     = $def[0];
+        $matchType = $def[1];
+        $value     = $def[2];
+
+        $alsoMatchNULLValues = false;
+        switch ($matchType) {
+            case self::MATCH_EQUAL:
+                $sqlMatch = '=';
+                break;
+            case self::MATCH_NOT_EQUAL:
+                $sqlMatch = '<>';
+                $alsoMatchNULLValues = true;
+                break;
+            case self::MATCH_GREATER:
+                $sqlMatch = '>';
+                break;
+            case self::MATCH_LESS:
+                $sqlMatch = '<';
+                break;
+            case self::MATCH_GREATER_OR_EQUAL:
+                $sqlMatch = '>=';
+                break;
+            case self::MATCH_LESS_OR_EQUAL:
+                $sqlMatch = '<=';
+                break;
+            case self::MATCH_CONTAINS:
+                $sqlMatch = 'LIKE';
+                $value    = '%' . $this->escapeLikeString($value) . '%';
+                break;
+            case self::MATCH_DOES_NOT_CONTAIN:
+                $sqlMatch = 'NOT LIKE';
+                $value    = '%' . $this->escapeLikeString($value) . '%';
+                $alsoMatchNULLValues = true;
+                break;
+
+            case self::MATCH_IS_NOT_NULL_NOR_EMPTY:
+                $sqlMatch = 'IS NOT NULL AND (' . $field . ' <> \'\' OR ' . $field . ' = 0)';
+                $value    = null;
+                break;
+
+            case self::MATCH_IS_NULL_OR_EMPTY:
+                $sqlMatch = 'IS NULL OR ' . $field . ' = \'\' ';
+                $value    = null;
+                break;
+
+            case self::MATCH_ACTIONS_CONTAINS:
+                // this match type is not accessible from the outside
+                // (it won't be matched in self::parseSubExpressions())
+                // it can be used internally to inject sub-expressions into the query.
+                // see Segment::getCleanedExpression()
+                $sqlMatch = 'IN (' . $value['SQL'] . ')';
+                $value    = $this->escapeLikeString($value['bind']);
+                break;
+            default:
+                throw new Exception("Filter contains the match type '" . $matchType . "' which is not supported");
+                break;
+        }
+
+        // We match NULL values when rows are excluded only when we are not doing a
+        $alsoMatchNULLValues = $alsoMatchNULLValues && !empty($value);
+
+        if ($matchType === self::MATCH_ACTIONS_CONTAINS
+            || is_null($value)
+        ) {
+            $sqlExpression = "( $field $sqlMatch )";
+        } else {
+            if ($alsoMatchNULLValues) {
+                $sqlExpression = "( $field IS NULL OR $field $sqlMatch ? )";
+            } else {
+                $sqlExpression = "$field $sqlMatch ?";
+            }
+        }
+
+        $this->checkFieldIsAvailable($field, $availableTables);
+
+        return array($sqlExpression, $value);
+    }
+
+    /**
+     * Check whether the field is available
+     * If not, add it to the available tables
+     *
+     * @param string $field
+     * @param array $availableTables
+     */
+    private function checkFieldIsAvailable($field, &$availableTables)
+    {
+        $fieldParts = explode('.', $field);
+
+        $table = count($fieldParts) == 2 ? $fieldParts[0] : false;
+
+        // remove sql functions from field name
+        // example: `HOUR(log_visit.visit_last_action_time)` gets `HOUR(log_visit` => remove `HOUR(`
+        $table = preg_replace('/^[A-Z_]+\(/', '', $table);
+        $tableExists = !$table || in_array($table, $availableTables);
+
+        if (!$tableExists) {
+            $availableTables[] = $table;
+        }
+    }
+
+    /**
+     * Escape the characters % and _ in the given string
+     * @param string $str
+     * @return string
+     */
+    private function escapeLikeString($str)
+    {
+        $str = str_replace("%", "\%", $str);
+        $str = str_replace("_", "\_", $str);
+        return $str;
+    }
+
+    /**
+     * Given a filter string,
+     * will parse it into an array where each row contains the boolean operator applied to it,
+     * and the operand
+     *
+     * @return array
+     */
+    protected function parseTree()
+    {
+        $string = $this->string;
+        if (empty($string)) {
+            return array();
+        }
+        $tree = array();
+        $i = 0;
+        $length = strlen($string);
+        $isBackslash = false;
+        $operand = '';
+        while ($i <= $length) {
+            $char = $string[$i];
+
+            $isAND = ($char == self::AND_DELIMITER);
+            $isOR = ($char == self::OR_DELIMITER);
+            $isEnd = ($length == $i + 1);
+
+            if ($isEnd) {
+                if ($isBackslash && ($isAND || $isOR)) {
+                    $operand = substr($operand, 0, -1);
+                }
+                $operand .= $char;
+                $tree[] = array(self::INDEX_BOOL_OPERATOR => '', self::INDEX_OPERAND => $operand);
+                break;
+            }
+
+            if ($isAND && !$isBackslash) {
+                $tree[] = array(self::INDEX_BOOL_OPERATOR => 'AND', self::INDEX_OPERAND => $operand);
+                $operand = '';
+            } elseif ($isOR && !$isBackslash) {
+                $tree[] = array(self::INDEX_BOOL_OPERATOR => 'OR', self::INDEX_OPERAND => $operand);
+                $operand = '';
+            } else {
+                if ($isBackslash && ($isAND || $isOR)) {
+                    $operand = substr($operand, 0, -1);
+                }
+                $operand .= $char;
+            }
+            $isBackslash = ($char == "\\");
+            $i++;
+        }
+        return $tree;
+    }
+
+    /**
+     * Given the array of parsed boolean logic, will return
+     * an array containing the full SQL string representing the filter,
+     * the needed joins and the values to bind to the query
+     *
+     * @throws Exception
+     * @return array SQL Query, Joins and Bind parameters
+     */
+    public function getSql()
+    {
+        if (count($this->tree) == 0) {
+            throw new Exception("Invalid segment, please specify a valid segment.");
+        }
+        $sql = '';
+        $subExpression = false;
+        foreach ($this->tree as $expression) {
+            $operator = $expression[self::INDEX_BOOL_OPERATOR];
+            $operand = $expression[self::INDEX_OPERAND];
+
+            if ($operator == 'OR'
+                && !$subExpression
+            ) {
+                $sql .= ' (';
+                $subExpression = true;
+            } else {
+                $sql .= ' ';
+            }
+
+            $sql .= $operand;
+
+            if ($operator == 'AND'
+                && $subExpression
+            ) {
+                $sql .= ')';
+                $subExpression = false;
+            }
+
+            $sql .= " $operator";
+        }
+        if ($subExpression) {
+            $sql .= ')';
+        }
+        return array(
+            'where' => $sql,
+            'bind'  => $this->valuesBind,
+            'join'  => implode(' ', $this->joins)
+        );
+    }
+}
diff --git a/core/SegmentExpression.php b/core/SegmentExpression.php
deleted file mode 100644
index cbd2a3eb9e..0000000000
--- a/core/SegmentExpression.php
+++ /dev/null
@@ -1,378 +0,0 @@
-<?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;
-
-use Exception;
-
-/**
- *
- */
-class SegmentExpression
-{
-    const AND_DELIMITER = ';';
-    const OR_DELIMITER = ',';
-
-    const MATCH_EQUAL = '==';
-    const MATCH_NOT_EQUAL = '!=';
-    const MATCH_GREATER_OR_EQUAL = '>=';
-    const MATCH_LESS_OR_EQUAL = '<=';
-    const MATCH_GREATER = '>';
-    const MATCH_LESS = '<';
-    const MATCH_CONTAINS = '=@';
-    const MATCH_DOES_NOT_CONTAIN = '!@';
-
-    // Note: you can't write this in the API, but access this feature
-    // via field!=        <- IS NOT NULL
-    // or via field==     <- IS NULL / empty
-    const MATCH_IS_NOT_NULL_NOR_EMPTY = '::NOT_NULL';
-    const MATCH_IS_NULL_OR_EMPTY = '::NULL';
-
-    // Special case, since we look up Page URLs/Page titles in a sub SQL query
-    const MATCH_ACTIONS_CONTAINS = 'IN';
-
-    const INDEX_BOOL_OPERATOR = 0;
-    const INDEX_OPERAND = 1;
-
-    function __construct($string)
-    {
-        $this->string = $string;
-        $this->tree = $this->parseTree();
-    }
-
-    protected $joins = array();
-    protected $valuesBind = array();
-    protected $parsedTree = array();
-    protected $tree = array();
-    protected $parsedSubExpressions = array();
-
-    /**
-     * Given the array of parsed filters containing, for each filter,
-     * the boolean operator (AND/OR) and the operand,
-     * Will return the array where the filters are in SQL representation
-     *
-     * @throws Exception
-     * @return array
-     */
-    public function parseSubExpressions()
-    {
-        $parsedSubExpressions = array();
-        foreach ($this->tree as $leaf) {
-            $operand = $leaf[self::INDEX_OPERAND];
-
-            $operand = urldecode($operand);
-
-            $operator = $leaf[self::INDEX_BOOL_OPERATOR];
-            $pattern = '/^(.+?)(' . self::MATCH_EQUAL . '|'
-                . self::MATCH_NOT_EQUAL . '|'
-                . self::MATCH_GREATER_OR_EQUAL . '|'
-                . self::MATCH_GREATER . '|'
-                . self::MATCH_LESS_OR_EQUAL . '|'
-                . self::MATCH_LESS . '|'
-                . self::MATCH_CONTAINS . '|'
-                . self::MATCH_DOES_NOT_CONTAIN
-                . '){1}(.*)/';
-            $match = preg_match($pattern, $operand, $matches);
-            if ($match == 0) {
-                throw new Exception('The segment \'' . $operand . '\' is not valid.');
-            }
-
-            $leftMember = $matches[1];
-            $operation  = $matches[2];
-            $valueRightMember = urldecode($matches[3]);
-
-            // is null / is not null
-            if ($valueRightMember === '') {
-                if ($operation == self::MATCH_NOT_EQUAL) {
-                    $operation = self::MATCH_IS_NOT_NULL_NOR_EMPTY;
-                } elseif ($operation == self::MATCH_EQUAL) {
-                    $operation = self::MATCH_IS_NULL_OR_EMPTY;
-                } else {
-                    throw new Exception('The segment \'' . $operand . '\' has no value specified. You can leave this value empty ' .
-                        'only when you use the operators: ' . self::MATCH_NOT_EQUAL . ' (is not) or ' . self::MATCH_EQUAL . ' (is)');
-                }
-            }
-
-            $parsedSubExpressions[] = array(
-                self::INDEX_BOOL_OPERATOR => $operator,
-                self::INDEX_OPERAND       => array(
-                    $leftMember,
-                    $operation,
-                    $valueRightMember,
-                ));
-        }
-        $this->parsedSubExpressions = $parsedSubExpressions;
-        return $parsedSubExpressions;
-    }
-
-    /**
-     * Set the given expression
-     * @param $parsedSubExpressions
-     */
-    public function setSubExpressionsAfterCleanup($parsedSubExpressions)
-    {
-        $this->parsedSubExpressions = $parsedSubExpressions;
-    }
-
-    /**
-     * @param array $availableTables
-     */
-    public function parseSubExpressionsIntoSqlExpressions(&$availableTables = array())
-    {
-        $sqlSubExpressions = array();
-        $this->valuesBind = array();
-        $this->joins = array();
-
-        foreach ($this->parsedSubExpressions as $leaf) {
-            $operator = $leaf[self::INDEX_BOOL_OPERATOR];
-            $operandDefinition = $leaf[self::INDEX_OPERAND];
-
-            $operand = $this->getSqlMatchFromDefinition($operandDefinition, $availableTables);
-
-            if ($operand[1] !== null) {
-                $this->valuesBind[] = $operand[1];
-            }
-
-            $operand = $operand[0];
-            $sqlSubExpressions[] = array(
-                self::INDEX_BOOL_OPERATOR => $operator,
-                self::INDEX_OPERAND       => $operand,
-            );
-        }
-
-        $this->tree = $sqlSubExpressions;
-    }
-
-    /**
-     * Given an array representing one filter operand ( left member , operation , right member)
-     * Will return an array containing
-     * - the SQL substring,
-     * - the values to bind to this substring
-     *
-     * @param array $def
-     * @param array $availableTables
-     * @throws Exception
-     * @return array
-     */
-    protected function getSqlMatchFromDefinition($def, &$availableTables)
-    {
-        $field     = $def[0];
-        $matchType = $def[1];
-        $value     = $def[2];
-
-        $alsoMatchNULLValues = false;
-        switch ($matchType) {
-            case self::MATCH_EQUAL:
-                $sqlMatch = '=';
-                break;
-            case self::MATCH_NOT_EQUAL:
-                $sqlMatch = '<>';
-                $alsoMatchNULLValues = true;
-                break;
-            case self::MATCH_GREATER:
-                $sqlMatch = '>';
-                break;
-            case self::MATCH_LESS:
-                $sqlMatch = '<';
-                break;
-            case self::MATCH_GREATER_OR_EQUAL:
-                $sqlMatch = '>=';
-                break;
-            case self::MATCH_LESS_OR_EQUAL:
-                $sqlMatch = '<=';
-                break;
-            case self::MATCH_CONTAINS:
-                $sqlMatch = 'LIKE';
-                $value    = '%' . $this->escapeLikeString($value) . '%';
-                break;
-            case self::MATCH_DOES_NOT_CONTAIN:
-                $sqlMatch = 'NOT LIKE';
-                $value    = '%' . $this->escapeLikeString($value) . '%';
-                $alsoMatchNULLValues = true;
-                break;
-
-            case self::MATCH_IS_NOT_NULL_NOR_EMPTY:
-                $sqlMatch = 'IS NOT NULL AND (' . $field . ' <> \'\' OR ' . $field . ' = 0)';
-                $value    = null;
-                break;
-
-            case self::MATCH_IS_NULL_OR_EMPTY:
-                $sqlMatch = 'IS NULL OR ' . $field . ' = \'\' ';
-                $value    = null;
-                break;
-
-            case self::MATCH_ACTIONS_CONTAINS:
-                // this match type is not accessible from the outside
-                // (it won't be matched in self::parseSubExpressions())
-                // it can be used internally to inject sub-expressions into the query.
-                // see Segment::getCleanedExpression()
-                $sqlMatch = 'IN (' . $value['SQL'] . ')';
-                $value    = $this->escapeLikeString($value['bind']);
-                break;
-            default:
-                throw new Exception("Filter contains the match type '" . $matchType . "' which is not supported");
-                break;
-        }
-
-        // We match NULL values when rows are excluded only when we are not doing a
-        $alsoMatchNULLValues = $alsoMatchNULLValues && !empty($value);
-
-        if ($matchType === self::MATCH_ACTIONS_CONTAINS
-            || is_null($value)
-        ) {
-            $sqlExpression = "( $field $sqlMatch )";
-        } else {
-            if ($alsoMatchNULLValues) {
-                $sqlExpression = "( $field IS NULL OR $field $sqlMatch ? )";
-            } else {
-                $sqlExpression = "$field $sqlMatch ?";
-            }
-        }
-
-        $this->checkFieldIsAvailable($field, $availableTables);
-
-        return array($sqlExpression, $value);
-    }
-
-    /**
-     * Check whether the field is available
-     * If not, add it to the available tables
-     *
-     * @param string $field
-     * @param array $availableTables
-     */
-    private function checkFieldIsAvailable($field, &$availableTables)
-    {
-        $fieldParts = explode('.', $field);
-
-        $table = count($fieldParts) == 2 ? $fieldParts[0] : false;
-
-        // remove sql functions from field name
-        // example: `HOUR(log_visit.visit_last_action_time)` gets `HOUR(log_visit` => remove `HOUR(`
-        $table = preg_replace('/^[A-Z_]+\(/', '', $table);
-        $tableExists = !$table || in_array($table, $availableTables);
-
-        if (!$tableExists) {
-            $availableTables[] = $table;
-        }
-    }
-
-    /**
-     * Escape the characters % and _ in the given string
-     * @param string $str
-     * @return string
-     */
-    private function escapeLikeString($str)
-    {
-        $str = str_replace("%", "\%", $str);
-        $str = str_replace("_", "\_", $str);
-        return $str;
-    }
-
-    /**
-     * Given a filter string,
-     * will parse it into an array where each row contains the boolean operator applied to it,
-     * and the operand
-     *
-     * @return array
-     */
-    protected function parseTree()
-    {
-        $string = $this->string;
-        if (empty($string)) {
-            return array();
-        }
-        $tree = array();
-        $i = 0;
-        $length = strlen($string);
-        $isBackslash = false;
-        $operand = '';
-        while ($i <= $length) {
-            $char = $string[$i];
-
-            $isAND = ($char == self::AND_DELIMITER);
-            $isOR = ($char == self::OR_DELIMITER);
-            $isEnd = ($length == $i + 1);
-
-            if ($isEnd) {
-                if ($isBackslash && ($isAND || $isOR)) {
-                    $operand = substr($operand, 0, -1);
-                }
-                $operand .= $char;
-                $tree[] = array(self::INDEX_BOOL_OPERATOR => '', self::INDEX_OPERAND => $operand);
-                break;
-            }
-
-            if ($isAND && !$isBackslash) {
-                $tree[] = array(self::INDEX_BOOL_OPERATOR => 'AND', self::INDEX_OPERAND => $operand);
-                $operand = '';
-            } elseif ($isOR && !$isBackslash) {
-                $tree[] = array(self::INDEX_BOOL_OPERATOR => 'OR', self::INDEX_OPERAND => $operand);
-                $operand = '';
-            } else {
-                if ($isBackslash && ($isAND || $isOR)) {
-                    $operand = substr($operand, 0, -1);
-                }
-                $operand .= $char;
-            }
-            $isBackslash = ($char == "\\");
-            $i++;
-        }
-        return $tree;
-    }
-
-    /**
-     * Given the array of parsed boolean logic, will return
-     * an array containing the full SQL string representing the filter,
-     * the needed joins and the values to bind to the query
-     *
-     * @throws Exception
-     * @return array SQL Query, Joins and Bind parameters
-     */
-    public function getSql()
-    {
-        if (count($this->tree) == 0) {
-            throw new Exception("Invalid segment, please specify a valid segment.");
-        }
-        $sql = '';
-        $subExpression = false;
-        foreach ($this->tree as $expression) {
-            $operator = $expression[self::INDEX_BOOL_OPERATOR];
-            $operand = $expression[self::INDEX_OPERAND];
-
-            if ($operator == 'OR'
-                && !$subExpression
-            ) {
-                $sql .= ' (';
-                $subExpression = true;
-            } else {
-                $sql .= ' ';
-            }
-
-            $sql .= $operand;
-
-            if ($operator == 'AND'
-                && $subExpression
-            ) {
-                $sql .= ')';
-                $subExpression = false;
-            }
-
-            $sql .= " $operator";
-        }
-        if ($subExpression) {
-            $sql .= ')';
-        }
-        return array(
-            'where' => $sql,
-            'bind'  => $this->valuesBind,
-            'join'  => implode(' ', $this->joins)
-        );
-    }
-}
diff --git a/core/Tracker/TableLogAction.php b/core/Tracker/TableLogAction.php
index 709936f2a5..fe620035d7 100644
--- a/core/Tracker/TableLogAction.php
+++ b/core/Tracker/TableLogAction.php
@@ -10,7 +10,7 @@
 namespace Piwik\Tracker;
 
 use Piwik\Common;
-use Piwik\SegmentExpression;
+use Piwik\Segment\SegmentExpression;
 use Piwik\Tracker;
 
 /**
diff --git a/plugins/API/API.php b/plugins/API/API.php
index f9f60dd261..f7aa20d08b 100644
--- a/plugins/API/API.php
+++ b/plugins/API/API.php
@@ -23,7 +23,7 @@ use Piwik\Period\Range;
 use Piwik\Piwik;
 use Piwik\Plugin\Dimension\VisitDimension;
 use Piwik\Plugins\CoreAdminHome\CustomLogo;
-use Piwik\SegmentExpression;
+use Piwik\Segment\SegmentExpression;
 use Piwik\Translate;
 use Piwik\Version;
 
diff --git a/plugins/Transitions/API.php b/plugins/Transitions/API.php
index 32abffdc06..44a39fdcf5 100644
--- a/plugins/Transitions/API.php
+++ b/plugins/Transitions/API.php
@@ -24,7 +24,7 @@ use Piwik\Plugins\Actions\Actions;
 use Piwik\Plugins\Actions\ArchivingHelper;
 use Piwik\RankingQuery;
 use Piwik\Segment;
-use Piwik\SegmentExpression;
+use Piwik\Segment\SegmentExpression;
 use Piwik\Site;
 use Piwik\Tracker\Action;
 use Piwik\Tracker\PageUrl;
diff --git a/plugins/VisitFrequency/API.php b/plugins/VisitFrequency/API.php
index 97a98f27f8..a6f9971480 100644
--- a/plugins/VisitFrequency/API.php
+++ b/plugins/VisitFrequency/API.php
@@ -13,7 +13,7 @@ use Piwik\Archive;
 use Piwik\DataTable;
 use Piwik\Piwik;
 use Piwik\Plugins\VisitsSummary\API as APIVisitsSummary;
-use Piwik\SegmentExpression;
+use Piwik\Segment\SegmentExpression;
 
 /**
  * VisitFrequency API lets you access a list of metrics related to Returning Visitors.
diff --git a/tests/PHPUnit/Unit/Segment/SegmentExpressionTest.php b/tests/PHPUnit/Unit/Segment/SegmentExpressionTest.php
new file mode 100644
index 0000000000..ede256707b
--- /dev/null
+++ b/tests/PHPUnit/Unit/Segment/SegmentExpressionTest.php
@@ -0,0 +1,132 @@
+<?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;
+
+use Piwik\Segment\SegmentExpression;
+
+class SegmentExpressionTest extends \PHPUnit_Framework_TestCase
+{
+    /**
+     * Dataprovider for testSegmentSqlSimpleNoOperation
+     * @return array
+     */
+    public function getSimpleSegmentExpressions()
+    {
+        return array(
+            // classic expressions
+            array('A', " A "),
+            array('A,B', " (A OR B )"),
+            array('A;B', " A AND B "),
+            array('A;B;C', " A AND B AND C "),
+            array('A,B;C,D;E,F,G', " (A OR B) AND (C OR D) AND (E OR F OR G )"),
+
+            // unescape the backslash
+            array('A\,B\,C,D', " (A,B,C OR D )"),
+            array('\,A', ' ,A '),
+            // unescape only when it was escaping a known delimiter
+            array('\\\A', ' \\\A '),
+            // unescape at the end
+            array('\,\;\A\B,\,C,D\;E\,', ' (,;\A\B OR ,C OR D;E, )'),
+
+            // only replace when a following expression is detected
+            array('A,', ' A, '),
+            array('A;', ' A; '),
+            array('A;B;', ' A AND B; '),
+            array('A,B,', ' (A OR B, )'),
+        );
+    }
+
+    /**
+     * @dataProvider getSimpleSegmentExpressions
+     * @group Core
+     */
+    public function testSegmentSqlSimpleNoOperation($expression, $expectedSql)
+    {
+        $segment = new SegmentExpression($expression);
+        $expected = array('where' => $expectedSql, 'bind' => array(), 'join' => '');
+        $processed = $segment->getSql();
+        $this->assertEquals($expected, $processed);
+    }
+
+    /**
+     * Dataprovider for testSegmentSqlWithOperations
+     * @return array
+     */
+    public function getOperationSegmentExpressions()
+    {
+        // Filter expression => SQL string + Bind values
+        return array(
+            array('A==B%', array('where' => " A = ? ", 'bind' => array('B%'))),
+            array('ABCDEF====B===', array('where' => " ABCDEF = ? ", 'bind' => array('==B==='))),
+            array('A===B;CDEF!=C!=', array('where' => " A = ? AND ( CDEF IS NULL OR CDEF <> ? ) ", 'bind' => array('=B', 'C!='))),
+            array('A==B,C==D', array('where' => " (A = ? OR C = ? )", 'bind' => array('B', 'D'))),
+            array('A!=B;C==D', array('where' => " ( A IS NULL OR A <> ? ) AND C = ? ", 'bind' => array('B', 'D'))),
+            array('A!=B;C==D,E!=Hello World!=', array('where' => " ( A IS NULL OR A <> ? ) AND (C = ? OR ( E IS NULL OR E <> ? ) )", 'bind' => array('B', 'D', 'Hello World!='))),
+
+            array('A>B', array('where' => " A > ? ", 'bind' => array('B'))),
+            array('A<B', array('where' => " A < ? ", 'bind' => array('B'))),
+            array('A<=B', array('where' => " A <= ? ", 'bind' => array('B'))),
+            array('A>=B', array('where' => " A >= ? ", 'bind' => array('B'))),
+            array('ABCDEF>=>=>=B===', array('where' => " ABCDEF >= ? ", 'bind' => array('>=>=B==='))),
+            array('A>=<=B;CDEF>G;H>=I;J<K;L<=M', array('where' => " A >= ? AND CDEF > ? AND H >= ? AND J < ? AND L <= ? ", 'bind' => array('<=B', 'G', 'I', 'K', 'M'))),
+            array('A>=B;C>=D,E<w_ow great!', array('where' => " A >= ? AND (C >= ? OR E < ? )", 'bind' => array('B', 'D', 'w_ow great!'))),
+
+            array('A=@B_', array('where' => " A LIKE ? ", 'bind' => array('%B\_%'))),
+            array('A!@B%', array('where' => " ( A IS NULL OR A NOT LIKE ? ) ", 'bind' => array('%B\%%'))),
+        );
+    }
+
+    /**
+     * @dataProvider getOperationSegmentExpressions
+     * @group Core
+     */
+    public function testSegmentSqlWithOperations($expression, $expectedSql)
+    {
+        $segment = new SegmentExpression($expression);
+        $segment->parseSubExpressions();
+        $segment->parseSubExpressionsIntoSqlExpressions();
+        $processed = $segment->getSql();
+        $expectedSql['join'] = '';
+        $this->assertEquals($expectedSql, $processed);
+    }
+
+    /**
+     * Dataprovider for testBogusFiltersExpectExceptionThrown
+     * @return array
+     */
+    public function getBogusFilters()
+    {
+        return array(
+            array('A=B'),
+            array('C!D'),
+            array(''),
+            array('      '),
+            array(',;,'),
+            array(','),
+            array(',,'),
+            array('!='),
+        );
+    }
+
+    /**
+     * @dataProvider getBogusFilters
+     * @group Core
+     */
+    public function testBogusFiltersExpectExceptionThrown($bogus)
+    {
+        try {
+            $segment = new SegmentExpression($bogus);
+            $segment->parseSubExpressions();
+            $segment->getSql();
+        } catch (\Exception $e) {
+            return;
+        }
+        $this->fail('Expected exception not raised for:' . var_export($segment->getSql(), true));
+    }
+}
diff --git a/tests/PHPUnit/Unit/SegmentExpressionTest.php b/tests/PHPUnit/Unit/SegmentExpressionTest.php
deleted file mode 100644
index aadecc8f0e..0000000000
--- a/tests/PHPUnit/Unit/SegmentExpressionTest.php
+++ /dev/null
@@ -1,132 +0,0 @@
-<?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;
-
-use Piwik\SegmentExpression;
-
-class SegmentExpressionTest extends \PHPUnit_Framework_TestCase
-{
-    /**
-     * Dataprovider for testSegmentSqlSimpleNoOperation
-     * @return array
-     */
-    public function getSimpleSegmentExpressions()
-    {
-        return array(
-            // classic expressions
-            array('A', " A "),
-            array('A,B', " (A OR B )"),
-            array('A;B', " A AND B "),
-            array('A;B;C', " A AND B AND C "),
-            array('A,B;C,D;E,F,G', " (A OR B) AND (C OR D) AND (E OR F OR G )"),
-
-            // unescape the backslash
-            array('A\,B\,C,D', " (A,B,C OR D )"),
-            array('\,A', ' ,A '),
-            // unescape only when it was escaping a known delimiter
-            array('\\\A', ' \\\A '),
-            // unescape at the end
-            array('\,\;\A\B,\,C,D\;E\,', ' (,;\A\B OR ,C OR D;E, )'),
-
-            // only replace when a following expression is detected
-            array('A,', ' A, '),
-            array('A;', ' A; '),
-            array('A;B;', ' A AND B; '),
-            array('A,B,', ' (A OR B, )'),
-        );
-    }
-
-    /**
-     * @dataProvider getSimpleSegmentExpressions
-     * @group Core
-     */
-    public function testSegmentSqlSimpleNoOperation($expression, $expectedSql)
-    {
-        $segment = new SegmentExpression($expression);
-        $expected = array('where' => $expectedSql, 'bind' => array(), 'join' => '');
-        $processed = $segment->getSql();
-        $this->assertEquals($expected, $processed);
-    }
-
-    /**
-     * Dataprovider for testSegmentSqlWithOperations
-     * @return array
-     */
-    public function getOperationSegmentExpressions()
-    {
-        // Filter expression => SQL string + Bind values
-        return array(
-            array('A==B%', array('where' => " A = ? ", 'bind' => array('B%'))),
-            array('ABCDEF====B===', array('where' => " ABCDEF = ? ", 'bind' => array('==B==='))),
-            array('A===B;CDEF!=C!=', array('where' => " A = ? AND ( CDEF IS NULL OR CDEF <> ? ) ", 'bind' => array('=B', 'C!='))),
-            array('A==B,C==D', array('where' => " (A = ? OR C = ? )", 'bind' => array('B', 'D'))),
-            array('A!=B;C==D', array('where' => " ( A IS NULL OR A <> ? ) AND C = ? ", 'bind' => array('B', 'D'))),
-            array('A!=B;C==D,E!=Hello World!=', array('where' => " ( A IS NULL OR A <> ? ) AND (C = ? OR ( E IS NULL OR E <> ? ) )", 'bind' => array('B', 'D', 'Hello World!='))),
-
-            array('A>B', array('where' => " A > ? ", 'bind' => array('B'))),
-            array('A<B', array('where' => " A < ? ", 'bind' => array('B'))),
-            array('A<=B', array('where' => " A <= ? ", 'bind' => array('B'))),
-            array('A>=B', array('where' => " A >= ? ", 'bind' => array('B'))),
-            array('ABCDEF>=>=>=B===', array('where' => " ABCDEF >= ? ", 'bind' => array('>=>=B==='))),
-            array('A>=<=B;CDEF>G;H>=I;J<K;L<=M', array('where' => " A >= ? AND CDEF > ? AND H >= ? AND J < ? AND L <= ? ", 'bind' => array('<=B', 'G', 'I', 'K', 'M'))),
-            array('A>=B;C>=D,E<w_ow great!', array('where' => " A >= ? AND (C >= ? OR E < ? )", 'bind' => array('B', 'D', 'w_ow great!'))),
-
-            array('A=@B_', array('where' => " A LIKE ? ", 'bind' => array('%B\_%'))),
-            array('A!@B%', array('where' => " ( A IS NULL OR A NOT LIKE ? ) ", 'bind' => array('%B\%%'))),
-        );
-    }
-
-    /**
-     * @dataProvider getOperationSegmentExpressions
-     * @group Core
-     */
-    public function testSegmentSqlWithOperations($expression, $expectedSql)
-    {
-        $segment = new SegmentExpression($expression);
-        $segment->parseSubExpressions();
-        $segment->parseSubExpressionsIntoSqlExpressions();
-        $processed = $segment->getSql();
-        $expectedSql['join'] = '';
-        $this->assertEquals($expectedSql, $processed);
-    }
-
-    /**
-     * Dataprovider for testBogusFiltersExpectExceptionThrown
-     * @return array
-     */
-    public function getBogusFilters()
-    {
-        return array(
-            array('A=B'),
-            array('C!D'),
-            array(''),
-            array('      '),
-            array(',;,'),
-            array(','),
-            array(',,'),
-            array('!='),
-        );
-    }
-
-    /**
-     * @dataProvider getBogusFilters
-     * @group Core
-     */
-    public function testBogusFiltersExpectExceptionThrown($bogus)
-    {
-        try {
-            $segment = new SegmentExpression($bogus);
-            $segment->parseSubExpressions();
-            $segment->getSql();
-        } catch (\Exception $e) {
-            return;
-        }
-        $this->fail('Expected exception not raised for:' . var_export($segment->getSql(), true));
-    }
-}
-- 
cgit v1.2.3


From 5c14cf8e5bff649230f91ef7e90dc3d3dd89f9a9 Mon Sep 17 00:00:00 2001
From: mattab <matthieu.aubry@gmail.com>
Date: Fri, 5 Dec 2014 19:31:25 +1300
Subject: move logic that creates the SQL query into LogQueryBuilder class

---
 core/DataAccess/LogQueryBuilder.php | 261 ++++++++++++++++++++++++++++++++++++
 core/Segment.php                    | 249 +++-------------------------------
 core/Segment/SegmentExpression.php  |   5 +
 3 files changed, 284 insertions(+), 231 deletions(-)
 create mode 100644 core/DataAccess/LogQueryBuilder.php

diff --git a/core/DataAccess/LogQueryBuilder.php b/core/DataAccess/LogQueryBuilder.php
new file mode 100644
index 0000000000..254334ca8c
--- /dev/null
+++ b/core/DataAccess/LogQueryBuilder.php
@@ -0,0 +1,261 @@
+<?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;
+
+
+use Exception;
+use Piwik\Common;
+use Piwik\Segment\SegmentExpression;
+
+class LogQueryBuilder
+{
+
+
+    /**
+     * @param $segmentExpression
+     * @param $select
+     * @param $from
+     * @param $where
+     * @param $bind
+     * @param $orderBy
+     * @param $groupBy
+     * @return array
+     * @throws Exception
+     */
+    public function getSelectQueryString(SegmentExpression $segmentExpression, $select, $from, $where, $bind, $orderBy, $groupBy)
+    {
+        if (!is_array($from)) {
+            $from = array($from);
+        }
+
+        if (!$segmentExpression->isEmpty()) {
+            $segmentExpression->parseSubExpressionsIntoSqlExpressions($from);
+
+            $joins = $this->generateJoins($from);
+            $from = $joins['sql'];
+            $joinWithSubSelect = $joins['joinWithSubSelect'];
+
+            $segmentSql = $segmentExpression->getSql();
+            $segmentWhere = $segmentSql['where'];
+            if (!empty($segmentWhere)) {
+                if (!empty($where)) {
+                    $where = "( $where )
+				AND
+				($segmentWhere)";
+                } else {
+                    $where = $segmentWhere;
+                }
+            }
+
+            $bind = array_merge($bind, $segmentSql['bind']);
+        } else {
+            $joins = $this->generateJoins($from);
+            $from = $joins['sql'];
+            $joinWithSubSelect = $joins['joinWithSubSelect'];
+        }
+
+        if ($joinWithSubSelect) {
+            $sql = $this->buildWrappedSelectQuery($select, $from, $where, $orderBy, $groupBy);
+        } else {
+            $sql = $this->buildSelectQuery($select, $from, $where, $orderBy, $groupBy);
+        }
+        return array(
+            'sql' => $sql,
+            'bind' => $bind
+        );
+    }
+    /**
+     * 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 generateJoins($tables)
+    {
+        $knownTables = array("log_visit", "log_link_visit_action", "log_conversion", "log_conversion_item");
+        $visitsAvailable = $actionsAvailable = $conversionsAvailable = $conversionItemAvailable = false;
+        $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 joined on idlink_va
+        $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";
+        }
+
+        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 ($actionsAvailable && $table == "log_conversion") {
+                    // have actions, need conversions => join on idlink_va
+                    $join = "log_conversion.idlink_va = log_link_visit_action.idlink_va "
+                        . "AND log_conversion.idsite = log_link_visit_action.idsite";
+                } else if ($actionsAvailable && $table == "log_visit") {
+                    // have actions, need visits => join on idvisit
+                    $join = "log_visit.idvisit = log_link_visit_action.idvisit";
+                } else if ($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";
+                } else if ($conversionsAvailable && $table == "log_link_visit_action") {
+                    // have conversions, need actions => join on idlink_va
+                    $join = "log_conversion.idlink_va = log_link_visit_action.idlink_va";
+                } else if (($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");
+            $actionsAvailable = ($actionsAvailable || $table == "log_link_visit_action");
+            $conversionsAvailable = ($conversionsAvailable || $table == "log_conversion");
+            $conversionItemAvailable = ($conversionItemAvailable || $table == "log_conversion_item");
+        }
+
+        $return = array(
+            'sql'               => $sql,
+            'joinWithSubSelect' => $joinWithSubSelect
+        );
+        return $return;
+
+    }
+
+    /**
+     * Build select query the normal way
+     * @param string $select fieldlist to be selected
+     * @param string $from tablelist to select from
+     * @param string $where where clause
+     * @param string $orderBy order by clause
+     * @param string $groupBy group by clause
+     * @return string
+     */
+    private function buildSelectQuery($select, $from, $where, $orderBy, $groupBy)
+    {
+        $sql = "
+			SELECT
+				$select
+			FROM
+				$from";
+
+        if ($where) {
+            $sql .= "
+			WHERE
+				$where";
+        }
+
+        if ($groupBy) {
+            $sql .= "
+			GROUP BY
+				$groupBy";
+        }
+
+        if ($orderBy) {
+            $sql .= "
+			ORDER BY
+				$orderBy";
+        }
+
+        return $sql;
+    }
+
+    /**
+     * 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
+     * @param string $select
+     * @param string $from
+     * @param string $where
+     * @param string $orderBy
+     * @param string $groupBy
+     * @throws Exception
+     * @return string
+     */
+    private function buildWrappedSelectQuery($select, $from, $where, $orderBy, $groupBy)
+    {
+        $matchTables = "(log_visit|log_conversion_item|log_conversion|log_action)";
+        preg_match_all("/". $matchTables ."\.[a-z0-9_\*]+/", $select, $matches);
+        $neededFields = array_unique($matches[0]);
+
+        if (count($neededFields) == 0) {
+            throw new Exception("No needed fields found in select expression. "
+                . "Please use a table prefix.");
+        }
+
+        $select = preg_replace('/'.$matchTables.'\./', 'log_inner.', $select);
+        $orderBy = preg_replace('/'.$matchTables.'\./', 'log_inner.', $orderBy);
+        $groupBy = preg_replace('/'.$matchTables.'\./', 'log_inner.', $groupBy);
+
+        $from = "(
+			SELECT
+				" . implode(",
+				", $neededFields) . "
+			FROM
+				$from
+			WHERE
+				$where
+			GROUP BY log_visit.idvisit
+				) AS log_inner";
+
+        $where = false;
+        $query = $this->buildSelectQuery($select, $from, $where, $orderBy, $groupBy);
+        return $query;
+    }
+
+} 
\ No newline at end of file
diff --git a/core/Segment.php b/core/Segment.php
index d990ed16ff..5229caf3c5 100644
--- a/core/Segment.php
+++ b/core/Segment.php
@@ -9,6 +9,7 @@
 namespace Piwik;
 
 use Exception;
+use Piwik\DataAccess\LogQueryBuilder;
 use Piwik\Plugins\API\API;
 use Piwik\Segment\SegmentExpression;
 
@@ -60,6 +61,16 @@ class Segment
      */
     protected $segment = null;
 
+    /**
+     * @var string
+     */
+    protected $string = null;
+
+    /**
+     * @var array
+     */
+    protected $idSites = null;
+
     /**
      * Truncate the Segments to 8k
      */
@@ -69,9 +80,8 @@ class Segment
      * Constructor.
      *
      * @param string $segmentCondition The segment condition, eg, `'browserCode=ff;countryCode=CA'`.
-     * @param array $idSites The list of sites the segment will be used with. Some segments are
-     *                       dependent on the site, such as goal segments.
-     * @throws Exception
+     * @param array $idSites The list of sites the st
+     * @throws
      */
     public function __construct($segmentCondition, $idSites)
     {
@@ -128,7 +138,7 @@ class Segment
      */
     public function isEmpty()
     {
-        return empty($this->string);
+        return $this->segment->isEmpty();
     }
 
     protected $availableSegments = array();
@@ -227,234 +237,10 @@ class Segment
      */
     public function getSelectQuery($select, $from, $where = false, $bind = array(), $orderBy = false, $groupBy = false)
     {
-        if (!is_array($from)) {
-            $from = array($from);
-        }
-
-        if (!$this->isEmpty()) {
-            $this->segment->parseSubExpressionsIntoSqlExpressions($from);
-
-            $joins = $this->generateJoins($from);
-            $from = $joins['sql'];
-            $joinWithSubSelect = $joins['joinWithSubSelect'];
-
-            $segmentSql = $this->segment->getSql();
-            $segmentWhere = $segmentSql['where'];
-            if (!empty($segmentWhere)) {
-                if (!empty($where)) {
-                    $where = "( $where )
-				AND
-				($segmentWhere)";
-                } else {
-                    $where = $segmentWhere;
-                }
-            }
-
-            $bind = array_merge($bind, $segmentSql['bind']);
-        } else {
-            $joins = $this->generateJoins($from);
-            $from = $joins['sql'];
-            $joinWithSubSelect = $joins['joinWithSubSelect'];
-        }
-
-        if ($joinWithSubSelect) {
-            $sql = $this->buildWrappedSelectQuery($select, $from, $where, $orderBy, $groupBy);
-        } else {
-            $sql = $this->buildSelectQuery($select, $from, $where, $orderBy, $groupBy);
-        }
-        return array(
-            'sql'  => $sql,
-            'bind' => $bind
-        );
+        $segmentExpression = $this->segment;
+        $segmentQuery = new LogQueryBuilder();
+        return $segmentQuery->getSelectQueryString($segmentExpression, $select, $from, $where, $bind, $orderBy, $groupBy);
     }
-
-    /**
-     * 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 generateJoins($tables)
-    {
-        $knownTables = array("log_visit", "log_link_visit_action", "log_conversion", "log_conversion_item");
-        $visitsAvailable = $actionsAvailable = $conversionsAvailable = $conversionItemAvailable = false;
-        $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 joined on idlink_va
-        $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";
-        }
-
-        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 ($actionsAvailable && $table == "log_conversion") {
-                    // have actions, need conversions => join on idlink_va
-                    $join = "log_conversion.idlink_va = log_link_visit_action.idlink_va "
-                        . "AND log_conversion.idsite = log_link_visit_action.idsite";
-                } else if ($actionsAvailable && $table == "log_visit") {
-                    // have actions, need visits => join on idvisit
-                    $join = "log_visit.idvisit = log_link_visit_action.idvisit";
-                } else if ($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";
-                } else if ($conversionsAvailable && $table == "log_link_visit_action") {
-                    // have conversions, need actions => join on idlink_va
-                    $join = "log_conversion.idlink_va = log_link_visit_action.idlink_va";
-                } else if (($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");
-            $actionsAvailable = ($actionsAvailable || $table == "log_link_visit_action");
-            $conversionsAvailable = ($conversionsAvailable || $table == "log_conversion");
-            $conversionItemAvailable = ($conversionItemAvailable || $table == "log_conversion_item");
-        }
-
-        $return = array(
-            'sql'               => $sql,
-            'joinWithSubSelect' => $joinWithSubSelect
-        );
-        return $return;
-
-    }
-
-    /**
-     * Build select query the normal way
-     * @param string $select fieldlist to be selected
-     * @param string $from tablelist to select from
-     * @param string $where where clause
-     * @param string $orderBy order by clause
-     * @param string $groupBy group by clause
-     * @return string
-     */
-    private function buildSelectQuery($select, $from, $where, $orderBy, $groupBy)
-    {
-        $sql = "
-			SELECT
-				$select
-			FROM
-				$from";
-
-        if ($where) {
-            $sql .= "
-			WHERE
-				$where";
-        }
-
-        if ($groupBy) {
-            $sql .= "
-			GROUP BY
-				$groupBy";
-        }
-
-        if ($orderBy) {
-            $sql .= "
-			ORDER BY
-				$orderBy";
-        }
-
-        return $sql;
-    }
-
-    /**
-     * 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
-     * @param string $select
-     * @param string $from
-     * @param string $where
-     * @param string $orderBy
-     * @param string $groupBy
-     * @throws Exception
-     * @return string
-     */
-    private function buildWrappedSelectQuery($select, $from, $where, $orderBy, $groupBy)
-    {
-        $matchTables = "(log_visit|log_conversion_item|log_conversion|log_action)";
-        preg_match_all("/". $matchTables ."\.[a-z0-9_\*]+/", $select, $matches);
-        $neededFields = array_unique($matches[0]);
-
-        if (count($neededFields) == 0) {
-            throw new Exception("No needed fields found in select expression. "
-                . "Please use a table prefix.");
-        }
-
-        $select = preg_replace('/'.$matchTables.'\./', 'log_inner.', $select);
-        $orderBy = preg_replace('/'.$matchTables.'\./', 'log_inner.', $orderBy);
-        $groupBy = preg_replace('/'.$matchTables.'\./', 'log_inner.', $groupBy);
-
-        $from = "(
-			SELECT
-				" . implode(",
-				", $neededFields) . "
-			FROM
-				$from
-			WHERE
-				$where
-			GROUP BY log_visit.idvisit
-				) AS log_inner";
-
-        $where = false;
-        $query = $this->buildSelectQuery($select, $from, $where, $orderBy, $groupBy);
-        return $query;
-    }
-
     /**
      * Returns the segment string.
      *
@@ -464,4 +250,5 @@ class Segment
     {
         return (string) $this->getString();
     }
+
 }
\ No newline at end of file
diff --git a/core/Segment/SegmentExpression.php b/core/Segment/SegmentExpression.php
index e32bbe8723..4078321ce0 100644
--- a/core/Segment/SegmentExpression.php
+++ b/core/Segment/SegmentExpression.php
@@ -46,6 +46,11 @@ class SegmentExpression
         $this->tree = $this->parseTree();
     }
 
+    public function isEmpty()
+    {
+        return empty($this->string);
+    }
+
     protected $joins = array();
     protected $valuesBind = array();
     protected $parsedTree = array();
-- 
cgit v1.2.3


From 3980f9eeed4535032c1e563f39adf8efa4520914 Mon Sep 17 00:00:00 2001
From: mattab <matthieu.aubry@gmail.com>
Date: Fri, 5 Dec 2014 19:45:31 +1300
Subject: Renamed method to have distinct name

---
 core/RankingQuery.php                   | 4 ++--
 plugins/Actions/Archiver.php            | 4 ++--
 plugins/Contents/Archiver.php           | 2 +-
 plugins/Events/Archiver.php             | 2 +-
 tests/PHPUnit/Unit/RankingQueryTest.php | 2 +-
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/core/RankingQuery.php b/core/RankingQuery.php
index f44845f075..cd4f830669 100644
--- a/core/RankingQuery.php
+++ b/core/RankingQuery.php
@@ -214,7 +214,7 @@ class RankingQuery
      */
     public function execute($innerQuery, $bind = array())
     {
-        $query = $this->generateQuery($innerQuery);
+        $query = $this->generateRankingQuery($innerQuery);
         $data  = Db::fetchAll($query, $bind);
 
         if ($this->columnToMarkExcludedRows !== false) {
@@ -268,7 +268,7 @@ class RankingQuery
      *                            itself.
      * @return string             The entire ranking query SQL.
      */
-    public function generateQuery($innerQuery)
+    public function generateRankingQuery($innerQuery)
     {
         // +1 to include "Others"
         $limit = $this->limit + 1;
diff --git a/plugins/Actions/Archiver.php b/plugins/Actions/Archiver.php
index 67408ced64..ab5fea8739 100644
--- a/plugins/Actions/Archiver.php
+++ b/plugins/Actions/Archiver.php
@@ -254,7 +254,7 @@ class Archiver extends \Piwik\Plugin\Archiver
         return $this->isSiteSearchEnabled;
     }
 
-    protected function archiveDayQueryProcess($select, $from, $where, $orderBy, $groupBy, $sprintfField, $rankingQuery = false)
+    protected function archiveDayQueryProcess($select, $from, $where, $orderBy, $groupBy, $sprintfField, RankingQuery $rankingQuery = null)
     {
         $select = sprintf($select, $sprintfField);
 
@@ -266,7 +266,7 @@ class Archiver extends \Piwik\Plugin\Archiver
 
         // apply ranking query
         if ($rankingQuery) {
-            $querySql = $rankingQuery->generateQuery($querySql);
+            $querySql = $rankingQuery->generateRankingQuery($querySql);
         }
 
         // get result
diff --git a/plugins/Contents/Archiver.php b/plugins/Contents/Archiver.php
index 55668b4c3a..45607bd7b7 100644
--- a/plugins/Contents/Archiver.php
+++ b/plugins/Contents/Archiver.php
@@ -188,7 +188,7 @@ class Archiver extends \Piwik\Plugin\Archiver
 
         // apply ranking query
         if ($rankingQuery) {
-            $query['sql'] = $rankingQuery->generateQuery($query['sql']);
+            $query['sql'] = $rankingQuery->generateRankingQuery($query['sql']);
         }
 
         // get result
diff --git a/plugins/Events/Archiver.php b/plugins/Events/Archiver.php
index eb33d4899f..b396401de0 100644
--- a/plugins/Events/Archiver.php
+++ b/plugins/Events/Archiver.php
@@ -189,7 +189,7 @@ class Archiver extends \Piwik\Plugin\Archiver
 
         // apply ranking query
         if ($rankingQuery) {
-            $query['sql'] = $rankingQuery->generateQuery($query['sql']);
+            $query['sql'] = $rankingQuery->generateRankingQuery($query['sql']);
         }
 
         // get result
diff --git a/tests/PHPUnit/Unit/RankingQueryTest.php b/tests/PHPUnit/Unit/RankingQueryTest.php
index 49c24e56be..c242732ee7 100644
--- a/tests/PHPUnit/Unit/RankingQueryTest.php
+++ b/tests/PHPUnit/Unit/RankingQueryTest.php
@@ -139,7 +139,7 @@ class RankingQueryTest extends \PHPUnit_Framework_TestCase
      */
     private function checkQuery($rankingQuery, $innerQuerySql, $expected)
     {
-        $query = $rankingQuery->generateQuery($innerQuerySql);
+        $query = $rankingQuery->generateRankingQuery($innerQuerySql);
 
         $queryNoWhitespace = preg_replace("/\s+/", "", $query);
         $expectedNoWhitespace = preg_replace("/\s+/", "", $expected);
-- 
cgit v1.2.3


From 2cd889046e16ed3b737e4734b70ac240f6515b3a Mon Sep 17 00:00:00 2001
From: mattab <matthieu.aubry@gmail.com>
Date: Fri, 5 Dec 2014 20:40:25 +1300
Subject: Refactoring without changing logic, so I can understand the code

---
 core/DataAccess/LogQueryBuilder.php | 136 ++++++++++++++++++------------------
 core/Segment.php                    |   8 +--
 core/Segment/SegmentExpression.php  |   4 +-
 3 files changed, 73 insertions(+), 75 deletions(-)

diff --git a/core/DataAccess/LogQueryBuilder.php b/core/DataAccess/LogQueryBuilder.php
index 254334ca8c..2be8052d4c 100644
--- a/core/DataAccess/LogQueryBuilder.php
+++ b/core/DataAccess/LogQueryBuilder.php
@@ -16,51 +16,23 @@ use Piwik\Segment\SegmentExpression;
 
 class LogQueryBuilder
 {
-
-
-    /**
-     * @param $segmentExpression
-     * @param $select
-     * @param $from
-     * @param $where
-     * @param $bind
-     * @param $orderBy
-     * @param $groupBy
-     * @return array
-     * @throws Exception
-     */
     public function getSelectQueryString(SegmentExpression $segmentExpression, $select, $from, $where, $bind, $orderBy, $groupBy)
     {
         if (!is_array($from)) {
             $from = array($from);
         }
 
-        if (!$segmentExpression->isEmpty()) {
+        if(!$segmentExpression->isEmpty()) {
             $segmentExpression->parseSubExpressionsIntoSqlExpressions($from);
-
-            $joins = $this->generateJoins($from);
-            $from = $joins['sql'];
-            $joinWithSubSelect = $joins['joinWithSubSelect'];
-
             $segmentSql = $segmentExpression->getSql();
-            $segmentWhere = $segmentSql['where'];
-            if (!empty($segmentWhere)) {
-                if (!empty($where)) {
-                    $where = "( $where )
-				AND
-				($segmentWhere)";
-                } else {
-                    $where = $segmentWhere;
-                }
-            }
-
+            $where = $this->getWhereMatchBoth($where, $segmentSql['where']);
             $bind = array_merge($bind, $segmentSql['bind']);
-        } else {
-            $joins = $this->generateJoins($from);
-            $from = $joins['sql'];
-            $joinWithSubSelect = $joins['joinWithSubSelect'];
         }
 
+        $joins = $this->generateJoins($from);
+        $joinWithSubSelect = $joins['joinWithSubSelect'];
+        $from = $joins['sql'];
+
         if ($joinWithSubSelect) {
             $sql = $this->buildWrappedSelectQuery($select, $from, $where, $orderBy, $groupBy);
         } else {
@@ -71,6 +43,8 @@ class LogQueryBuilder
             'bind' => $bind
         );
     }
+
+
     /**
      * Generate the join sql based on the needed tables
      * @param array $tables tables to join
@@ -178,8 +152,53 @@ class LogQueryBuilder
 
     }
 
+
+    /**
+     * 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
+     * @param string $select
+     * @param string $from
+     * @param string $where
+     * @param string $orderBy
+     * @param string $groupBy
+     * @throws Exception
+     * @return string
+     */
+    private function buildWrappedSelectQuery($select, $from, $where, $orderBy, $groupBy)
+    {
+        $matchTables = "(log_visit|log_conversion_item|log_conversion|log_action)";
+        preg_match_all("/". $matchTables ."\.[a-z0-9_\*]+/", $select, $matches);
+        $neededFields = array_unique($matches[0]);
+
+        if (count($neededFields) == 0) {
+            throw new Exception("No needed fields found in select expression. "
+                . "Please use a table prefix.");
+        }
+
+        $select = preg_replace('/'.$matchTables.'\./', 'log_inner.', $select);
+        $orderBy = preg_replace('/'.$matchTables.'\./', 'log_inner.', $orderBy);
+        $groupBy = preg_replace('/'.$matchTables.'\./', 'log_inner.', $groupBy);
+
+        $from = "(
+			SELECT
+				" . implode(",
+				", $neededFields) . "
+			FROM
+				$from
+			WHERE
+				$where
+			GROUP BY log_visit.idvisit
+				) AS log_inner";
+
+        $where = false;
+        $query = $this->buildSelectQuery($select, $from, $where, $orderBy, $groupBy);
+        return $query;
+    }
+
+
     /**
      * Build select query the normal way
+     *
      * @param string $select fieldlist to be selected
      * @param string $from tablelist to select from
      * @param string $where where clause
@@ -217,45 +236,24 @@ class LogQueryBuilder
     }
 
     /**
-     * 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
-     * @param string $select
-     * @param string $from
-     * @param string $where
-     * @param string $orderBy
-     * @param string $groupBy
-     * @throws Exception
+     * @param $where
+     * @param $segmentWhere
      * @return string
      */
-    private function buildWrappedSelectQuery($select, $from, $where, $orderBy, $groupBy)
+    protected function getWhereMatchBoth($where, $segmentWhere)
     {
-        $matchTables = "(log_visit|log_conversion_item|log_conversion|log_action)";
-        preg_match_all("/". $matchTables ."\.[a-z0-9_\*]+/", $select, $matches);
-        $neededFields = array_unique($matches[0]);
-
-        if (count($neededFields) == 0) {
-            throw new Exception("No needed fields found in select expression. "
-                . "Please use a table prefix.");
+        if (empty($segmentWhere) && empty($where)) {
+            throw new \Exception("Segment where clause should be non empty.");
         }
-
-        $select = preg_replace('/'.$matchTables.'\./', 'log_inner.', $select);
-        $orderBy = preg_replace('/'.$matchTables.'\./', 'log_inner.', $orderBy);
-        $groupBy = preg_replace('/'.$matchTables.'\./', 'log_inner.', $groupBy);
-
-        $from = "(
-			SELECT
-				" . implode(",
-				", $neededFields) . "
-			FROM
-				$from
-			WHERE
-				$where
-			GROUP BY log_visit.idvisit
-				) AS log_inner";
-
-        $where = false;
-        $query = $this->buildSelectQuery($select, $from, $where, $orderBy, $groupBy);
-        return $query;
+        if (empty($segmentWhere)) {
+            return $where;
+        }
+        if (empty($where)) {
+            return $segmentWhere;
+        }
+        return "( $where )
+                AND
+                ($segmentWhere)";
     }
 
 } 
\ No newline at end of file
diff --git a/core/Segment.php b/core/Segment.php
index 5229caf3c5..7c48df3c07 100644
--- a/core/Segment.php
+++ b/core/Segment.php
@@ -59,7 +59,7 @@ class Segment
     /**
      * @var SegmentExpression
      */
-    protected $segment = null;
+    protected $segmentExpression = null;
 
     /**
      * @var string
@@ -114,7 +114,7 @@ class Segment
         $this->string  = $string;
         $this->idSites = $idSites;
         $segment = new SegmentExpression($string);
-        $this->segment = $segment;
+        $this->segmentExpression = $segment;
 
         // parse segments
         $expressions = $segment->parseSubExpressions();
@@ -138,7 +138,7 @@ class Segment
      */
     public function isEmpty()
     {
-        return $this->segment->isEmpty();
+        return $this->segmentExpression->isEmpty();
     }
 
     protected $availableSegments = array();
@@ -237,7 +237,7 @@ class Segment
      */
     public function getSelectQuery($select, $from, $where = false, $bind = array(), $orderBy = false, $groupBy = false)
     {
-        $segmentExpression = $this->segment;
+        $segmentExpression = $this->segmentExpression;
         $segmentQuery = new LogQueryBuilder();
         return $segmentQuery->getSelectQueryString($segmentExpression, $select, $from, $where, $bind, $orderBy, $groupBy);
     }
diff --git a/core/Segment/SegmentExpression.php b/core/Segment/SegmentExpression.php
index 4078321ce0..10cf0294e3 100644
--- a/core/Segment/SegmentExpression.php
+++ b/core/Segment/SegmentExpression.php
@@ -48,7 +48,7 @@ class SegmentExpression
 
     public function isEmpty()
     {
-        return empty($this->string);
+        return count($this->tree) == 0;
     }
 
     protected $joins = array();
@@ -342,7 +342,7 @@ class SegmentExpression
      */
     public function getSql()
     {
-        if (count($this->tree) == 0) {
+        if ($this->isEmpty()) {
             throw new Exception("Invalid segment, please specify a valid segment.");
         }
         $sql = '';
-- 
cgit v1.2.3


From 46629d99f6942e46abaeeb835ebb67a5b3969c60 Mon Sep 17 00:00:00 2001
From: mattab <matthieu.aubry@gmail.com>
Date: Fri, 5 Dec 2014 20:54:17 +1300
Subject: Building the inner query reusing buildSelectQuery

---
 core/DataAccess/LogQueryBuilder.php | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/core/DataAccess/LogQueryBuilder.php b/core/DataAccess/LogQueryBuilder.php
index 2be8052d4c..fb1f1c1c47 100644
--- a/core/DataAccess/LogQueryBuilder.php
+++ b/core/DataAccess/LogQueryBuilder.php
@@ -175,22 +175,21 @@ class LogQueryBuilder
                 . "Please use a table prefix.");
         }
 
-        $select = preg_replace('/'.$matchTables.'\./', 'log_inner.', $select);
-        $orderBy = preg_replace('/'.$matchTables.'\./', 'log_inner.', $orderBy);
-        $groupBy = preg_replace('/'.$matchTables.'\./', 'log_inner.', $groupBy);
+        $innerSelect = implode(", \n", $neededFields);
+        $innerFrom = $from;
+        $innerWhere = $where;
+        $innerGroupBy = "log_visit.idvisit";
+        $innerOrderBy = "NULL";
 
-        $from = "(
-			SELECT
-				" . implode(",
-				", $neededFields) . "
-			FROM
-				$from
-			WHERE
-				$where
-			GROUP BY log_visit.idvisit
-				) AS log_inner";
+        $innerQuery = $this->buildSelectQuery($innerSelect, $innerFrom, $innerWhere, $innerOrderBy, $innerGroupBy);
 
+        $select = preg_replace('/'.$matchTables.'\./', 'log_inner.', $select);
+        $from = "(
+            $innerQuery
+        ) AS log_inner";
         $where = false;
+        $orderBy = preg_replace('/'.$matchTables.'\./', 'log_inner.', $orderBy);
+        $groupBy = preg_replace('/'.$matchTables.'\./', 'log_inner.', $groupBy);
         $query = $this->buildSelectQuery($select, $from, $where, $orderBy, $groupBy);
         return $query;
     }
-- 
cgit v1.2.3


From 64bc69c93eac5b7b3af514e625678fb7b3aff76d Mon Sep 17 00:00:00 2001
From: mattab <matthieu.aubry@gmail.com>
Date: Fri, 5 Dec 2014 20:57:07 +1300
Subject: Added ORDER BY NULL optimisation, fixing unit tests

---
 tests/PHPUnit/Integration/SegmentTest.php | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/PHPUnit/Integration/SegmentTest.php b/tests/PHPUnit/Integration/SegmentTest.php
index 889f4a8e04..41fe39a392 100644
--- a/tests/PHPUnit/Integration/SegmentTest.php
+++ b/tests/PHPUnit/Integration/SegmentTest.php
@@ -210,6 +210,7 @@ class SegmentTest extends IntegrationTestCase
                     AND
                     ( log_link_visit_action.custom_var_k1 = ? AND log_visit.visitor_returning = ? )
                 GROUP BY log_visit.idvisit
+                ORDER BY NULL
                     ) AS log_inner",
             "bind" => array(1, 'Test', 0));
 
@@ -300,6 +301,7 @@ class SegmentTest extends IntegrationTestCase
                     AND
                     ( log_conversion.idgoal = ? )
                 GROUP BY log_visit.idvisit
+                ORDER BY NULL
                     ) AS log_inner",
             "bind" => array(1, 1));
 
@@ -423,6 +425,7 @@ class SegmentTest extends IntegrationTestCase
                 WHERE
                      log_conversion.idgoal = ? AND HOUR(log_visit.visit_last_action_time) = ? AND log_link_visit_action.custom_var_k1 = ?
                 GROUP BY log_visit.idvisit
+                ORDER BY NULL
                     ) AS log_inner",
             "bind" => array(1, 12, 'Test'));
 
-- 
cgit v1.2.3


From 7f509b059ec54109cd2d7200dc466c4944c33d53 Mon Sep 17 00:00:00 2001
From: mattab <matthieu.aubry@gmail.com>
Date: Fri, 5 Dec 2014 22:20:06 +1300
Subject: Refactor Live SQL into Model

---
 core/DataAccess/LogQueryBuilder.php |  20 ++-
 core/Segment.php                    |   4 +-
 plugins/Live/API.php                | 205 +++-------------------------
 plugins/Live/Model.php              | 262 ++++++++++++++++++++++++++++++++++++
 4 files changed, 292 insertions(+), 199 deletions(-)
 create mode 100644 plugins/Live/Model.php

diff --git a/core/DataAccess/LogQueryBuilder.php b/core/DataAccess/LogQueryBuilder.php
index fb1f1c1c47..6e5b77561b 100644
--- a/core/DataAccess/LogQueryBuilder.php
+++ b/core/DataAccess/LogQueryBuilder.php
@@ -16,20 +16,25 @@ use Piwik\Segment\SegmentExpression;
 
 class LogQueryBuilder
 {
-    public function getSelectQueryString(SegmentExpression $segmentExpression, $select, $from, $where, $bind, $orderBy, $groupBy)
+    public function __construct(SegmentExpression $segmentExpression)
+    {
+        $this->segmentExpression = $segmentExpression;
+    }
+
+    public function getSelectQueryString($select, $from, $where, $bind, $orderBy, $groupBy)
     {
         if (!is_array($from)) {
             $from = array($from);
         }
 
-        if(!$segmentExpression->isEmpty()) {
-            $segmentExpression->parseSubExpressionsIntoSqlExpressions($from);
-            $segmentSql = $segmentExpression->getSql();
+        if(!$this->segmentExpression->isEmpty()) {
+            $this->segmentExpression->parseSubExpressionsIntoSqlExpressions($from);
+            $segmentSql = $this->segmentExpression->getSql();
             $where = $this->getWhereMatchBoth($where, $segmentSql['where']);
             $bind = array_merge($bind, $segmentSql['bind']);
         }
 
-        $joins = $this->generateJoins($from);
+        $joins = $this->generateJoinsString($from);
         $joinWithSubSelect = $joins['joinWithSubSelect'];
         $from = $joins['sql'];
 
@@ -51,7 +56,7 @@ class LogQueryBuilder
      * @throws Exception if tables can't be joined
      * @return array
      */
-    private function generateJoins($tables)
+    private function generateJoinsString($tables)
     {
         $knownTables = array("log_visit", "log_link_visit_action", "log_conversion", "log_conversion_item");
         $visitsAvailable = $actionsAvailable = $conversionsAvailable = $conversionItemAvailable = false;
@@ -184,7 +189,8 @@ class LogQueryBuilder
         $innerQuery = $this->buildSelectQuery($innerSelect, $innerFrom, $innerWhere, $innerOrderBy, $innerGroupBy);
 
         $select = preg_replace('/'.$matchTables.'\./', 'log_inner.', $select);
-        $from = "(
+        $from = "
+        (
             $innerQuery
         ) AS log_inner";
         $where = false;
diff --git a/core/Segment.php b/core/Segment.php
index 7c48df3c07..91318a0197 100644
--- a/core/Segment.php
+++ b/core/Segment.php
@@ -238,8 +238,8 @@ class Segment
     public function getSelectQuery($select, $from, $where = false, $bind = array(), $orderBy = false, $groupBy = false)
     {
         $segmentExpression = $this->segmentExpression;
-        $segmentQuery = new LogQueryBuilder();
-        return $segmentQuery->getSelectQueryString($segmentExpression, $select, $from, $where, $bind, $orderBy, $groupBy);
+        $segmentQuery = new LogQueryBuilder($segmentExpression);
+        return $segmentQuery->getSelectQueryString($select, $from, $where, $bind, $orderBy, $groupBy);
     }
     /**
      * Returns the segment string.
diff --git a/plugins/Live/API.php b/plugins/Live/API.php
index 7a36822534..0b66e75f64 100644
--- a/plugins/Live/API.php
+++ b/plugins/Live/API.php
@@ -69,49 +69,8 @@ class API extends \Piwik\Plugin\API
     public function getCounters($idSite, $lastMinutes, $segment = false)
     {
         Piwik::checkUserHasViewAccess($idSite);
-        $lastMinutes = (int) $lastMinutes;
-
-        $counters = array(
-            'visits'   => 0,
-            'actions'  => 0,
-            'visitors' => 0,
-            'visitsConverted' => 0,
-        );
-
-        if (empty($lastMinutes)) {
-            return array($counters);
-        }
-
-        list($whereIdSites, $idSites) = $this->getIdSitesWhereClause($idSite);
-
-        $select  = "count(*) as visits, COUNT(DISTINCT log_visit.idvisitor) as visitors";
-        $where   = $whereIdSites . "AND log_visit.visit_last_action_time >= ?";
-        $bind    = $idSites;
-        $bind[]  = Date::factory(time() - $lastMinutes * 60)->toString('Y-m-d H:i:s');
-
-        $segment = new Segment($segment, $idSite);
-        $query   = $segment->getSelectQuery($select, 'log_visit', $where, $bind);
-
-        $data    = Db::fetchAll($query['sql'], $query['bind']);
-
-        $counters['visits']   = $data[0]['visits'];
-        $counters['visitors'] = $data[0]['visitors'];
-
-        $select = "count(*)";
-        $from   = 'log_link_visit_action';
-        list($whereIdSites) = $this->getIdSitesWhereClause($idSite, $from);
-        $where  = $whereIdSites . "AND log_link_visit_action.server_time >= ?";
-        $query  = $segment->getSelectQuery($select, $from, $where, $bind);
-        $counters['actions'] = Db::fetchOne($query['sql'], $query['bind']);
-
-        $select = "count(*)";
-        $from   = 'log_conversion';
-        list($whereIdSites) = $this->getIdSitesWhereClause($idSite, $from);
-        $where  = $whereIdSites . "AND log_conversion.server_time >= ?";
-        $query  = $segment->getSelectQuery($select, $from, $where, $bind);
-        $counters['visitsConverted'] = Db::fetchOne($query['sql'], $query['bind']);
-
-        return array($counters);
+        $model = new Model();
+        return $model->queryCounters($idSite, $lastMinutes, $segment);
     }
 
     /**
@@ -455,38 +414,8 @@ class API extends \Piwik\Plugin\API
      */
     private function getAdjacentVisitorId($idSite, $visitorId, $visitLastActionTime, $segment, $getNext)
     {
-        if ($getNext) {
-            $visitLastActionTimeCondition = "sub.visit_last_action_time <= ?";
-            $orderByDir = "DESC";
-        } else {
-            $visitLastActionTimeCondition = "sub.visit_last_action_time >= ?";
-            $orderByDir = "ASC";
-        }
-
-        $visitLastActionDate = Date::factory($visitLastActionTime);
-        $dateOneDayAgo       = $visitLastActionDate->subDay(1);
-        $dateOneDayInFuture  = $visitLastActionDate->addDay(1);
-
-        $select = "log_visit.idvisitor, MAX(log_visit.visit_last_action_time) as visit_last_action_time";
-        $from = "log_visit";
-        $where = "log_visit.idsite = ? AND log_visit.idvisitor <> ? AND visit_last_action_time >= ? and visit_last_action_time <= ?";
-        $whereBind = array($idSite, @Common::hex2bin($visitorId), $dateOneDayAgo->toString('Y-m-d H:i:s'), $dateOneDayInFuture->toString('Y-m-d H:i:s'));
-        $orderBy = "MAX(log_visit.visit_last_action_time) $orderByDir";
-        $groupBy = "log_visit.idvisitor";
-
-        $segment = new Segment($segment, $idSite);
-        $queryInfo = $segment->getSelectQuery($select, $from, $where, $whereBind, $orderBy, $groupBy);
-
-        $sql = "SELECT sub.idvisitor, sub.visit_last_action_time FROM ({$queryInfo['sql']}) as sub
-                 WHERE $visitLastActionTimeCondition
-                 LIMIT 1";
-        $bind = array_merge($queryInfo['bind'], array($visitLastActionTime));
-
-        $visitorId = Db::fetchOne($sql, $bind);
-        if (!empty($visitorId)) {
-            $visitorId = bin2hex($visitorId);
-        }
-        return $visitorId;
+        $model = new Model();
+        return $model->queryAdjacentVisitorId($idSite, $visitorId, $visitLastActionTime, $segment, $getNext);
     }
 
     /**
@@ -614,112 +543,21 @@ class API extends \Piwik\Plugin\API
 
     private function loadLastVisitorDetailsFromDatabase($idSite, $period, $date, $segment = false, $countVisitorsToFetch = 100, $visitorId = false, $minTimestamp = false, $filterSortOrder = false)
     {
-        $where = $whereBind = array();
-
-        list($whereClause, $idSites) = $this->getIdSitesWhereClause($idSite);
+        $model = new Model();
+        $data = $model->queryLogVisits($idSite, $period, $date, $segment, $countVisitorsToFetch, $visitorId, $minTimestamp, $filterSortOrder);
 
-        $where[] = $whereClause;
-        $whereBind = $idSites;
-
-        if (strtolower($filterSortOrder) !== 'asc') {
-            $filterSortOrder = 'DESC';
-        }
-
-        $orderBy = "idsite, visit_last_action_time " . $filterSortOrder;
-        $orderByParent = "sub.visit_last_action_time " . $filterSortOrder;
-
-        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);
-        }
-
-        // If no other filter, only look at the last 24 hours of stats
-        if (empty($visitorId)
-            && empty($countVisitorsToFetch)
-            && empty($period)
-            && empty($date)
-        ) {
-            $period = 'day';
-            $date = 'yesterdaySameTime';
-        }
-
-        // SQL Filter with provided period
-        if (!empty($period) && !empty($date)) {
-            $currentSite = new Site($idSite);
-            $currentTimezone = $currentSite->getTimezone();
-
-            $dateString = $date;
-            if ($period == 'range') {
-                $processedPeriod = new Range('range', $date);
-                if ($parsedDate = Range::parseDateRange($date)) {
-                    $dateString = $parsedDate[2];
-                }
-            } else {
-                $processedDate = Date::factory($date);
-                if ($date == 'today'
-                    || $date == 'now'
-                    || $processedDate->toString() == Date::factory('now', $currentTimezone)->toString()
-                ) {
-                    $processedDate = $processedDate->subDay(1);
-                }
-                $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
-                && strpos($date, 'previous') === false
-                && 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;
-            }
-        }
-
-        if (count($where) > 0) {
-            $where = join("
-				AND ", $where);
-        } else {
-            $where = false;
-        }
-
-        $segment = new Segment($segment, $idSite);
-
-        // Subquery to use the indexes for ORDER BY
-        $select = "log_visit.*";
-        $from = "log_visit";
-        $subQuery = $segment->getSelectQuery($select, $from, $where, $whereBind, $orderBy);
-
-        $sqlLimit = $countVisitorsToFetch >= 1 ? " LIMIT 0, " . (int)$countVisitorsToFetch : "";
-
-        // Group by idvisit so that a visitor converting 2 goals only appears once
-        $sql = "
-			SELECT sub.* FROM (
-				" . $subQuery['sql'] . "
-				$sqlLimit
-			) AS sub
-			GROUP BY sub.idvisit
-			ORDER BY $orderByParent
-		";
-        try {
-            $data = Db::fetchAll($sql, $subQuery['bind']);
-        } catch (Exception $e) {
-            echo $e->getMessage();
-            exit;
-        }
+        return $this->makeVisitorTableFromArray($data);
+    }
 
+    /**
+     * @param $data
+     * @return DataTable
+     * @throws Exception
+     */
+    private function makeVisitorTableFromArray($data)
+    {
         $dataTable = new DataTable();
         $dataTable->addRowsFromSimpleArray($data);
-       // $dataTable->disableFilter('Truncate');
 
         if (!empty($data[0])) {
             $columnsToNotAggregate = array_map(function () {
@@ -732,18 +570,5 @@ class API extends \Piwik\Plugin\API
         return $dataTable;
     }
 
-    /**
-     * @param $idSite
-     * @param string $table
-     * @return array
-     */
-    private function getIdSitesWhereClause($idSite, $table = 'log_visit')
-    {
-        $idSites = array($idSite);
-        Piwik::postEvent('Live.API.getIdSitesString', array(&$idSites));
 
-        $idSitesBind = Common::getSqlStringFieldsArray($idSites);
-        $whereClause = $table . ".idsite in ($idSitesBind) ";
-        return array($whereClause, $idSites);
-    }
 }
diff --git a/plugins/Live/Model.php b/plugins/Live/Model.php
new file mode 100644
index 0000000000..7bf5485c5a
--- /dev/null
+++ b/plugins/Live/Model.php
@@ -0,0 +1,262 @@
+<?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\Live;
+
+use Exception;
+use Piwik\Common;
+use Piwik\Date;
+use Piwik\Db;
+use Piwik\Period;
+use Piwik\Period\Range;
+use Piwik\Piwik;
+use Piwik\Segment;
+use Piwik\Site;
+
+class Model
+{
+
+    /**
+     * @param $idSite
+     * @param $period
+     * @param $date
+     * @param $segment
+     * @param $countVisitorsToFetch
+     * @param $visitorId
+     * @param $minTimestamp
+     * @param $filterSortOrder
+     * @return array
+     * @throws Exception
+     */
+    public function queryLogVisits($idSite, $period, $date, $segment, $countVisitorsToFetch, $visitorId, $minTimestamp, $filterSortOrder)
+    {
+        $where = $whereBind = array();
+
+        list($whereClause, $idSites) = $this->getIdSitesWhereClause($idSite);
+
+        $where[] = $whereClause;
+        $whereBind = $idSites;
+
+        if (strtolower($filterSortOrder) !== 'asc') {
+            $filterSortOrder = 'DESC';
+        }
+
+        $orderBy = "idsite, visit_last_action_time " . $filterSortOrder;
+        $orderByParent = "sub.visit_last_action_time " . $filterSortOrder;
+
+        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);
+        }
+
+        // If no other filter, only look at the last 24 hours of stats
+        if (empty($visitorId)
+            && empty($countVisitorsToFetch)
+            && empty($period)
+            && empty($date)
+        ) {
+            $period = 'day';
+            $date = 'yesterdaySameTime';
+        }
+
+        // SQL Filter with provided period
+        if (!empty($period) && !empty($date)) {
+            $currentSite = new Site($idSite);
+            $currentTimezone = $currentSite->getTimezone();
+
+            $dateString = $date;
+            if ($period == 'range') {
+                $processedPeriod = new Range('range', $date);
+                if ($parsedDate = Range::parseDateRange($date)) {
+                    $dateString = $parsedDate[2];
+                }
+            } else {
+                $processedDate = Date::factory($date);
+                if ($date == 'today'
+                    || $date == 'now'
+                    || $processedDate->toString() == Date::factory('now', $currentTimezone)->toString()
+                ) {
+                    $processedDate = $processedDate->subDay(1);
+                }
+                $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
+                && strpos($date, 'previous') === false
+                && 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;
+            }
+        }
+
+        if (count($where) > 0) {
+            $where = join("
+				AND ", $where);
+        } else {
+            $where = false;
+        }
+
+        $segment = new Segment($segment, $idSite);
+
+        // Subquery to use the indexes for ORDER BY
+        $select = "log_visit.*";
+        $from = "log_visit";
+        $groupBy = false;
+        $subQuery = $segment->getSelectQuery($select, $from, $where, $whereBind, $orderBy, $groupBy);
+
+        $sqlLimit = $countVisitorsToFetch >= 1 ? " LIMIT 0, " . (int)$countVisitorsToFetch : "";
+
+        // Group by idvisit so that a visitor converting 2 goals only appears once
+        $sql = "
+			SELECT sub.* FROM (
+				" . $subQuery['sql'] . "
+				$sqlLimit
+			) AS sub
+			GROUP BY sub.idvisit
+			ORDER BY $orderByParent
+		";
+        try {
+            $data = Db::fetchAll($sql, $subQuery['bind']);
+            return $data;
+        } catch (Exception $e) {
+            echo $e->getMessage();
+            exit;
+        }
+        return $data;
+    }
+
+    /**
+     * @param $idSite
+     * @param $lastMinutes
+     * @param $segment
+     * @return array
+     * @throws Exception
+     */
+    public function queryCounters($idSite, $lastMinutes, $segment)
+    {
+        $lastMinutes = (int)$lastMinutes;
+
+        $counters = array(
+            'visits' => 0,
+            'actions' => 0,
+            'visitors' => 0,
+            'visitsConverted' => 0,
+        );
+
+        if (empty($lastMinutes)) {
+            return array($counters);
+        }
+
+        list($whereIdSites, $idSites) = $this->getIdSitesWhereClause($idSite);
+
+        $select = "count(*) as visits, COUNT(DISTINCT log_visit.idvisitor) as visitors";
+        $where = $whereIdSites . "AND log_visit.visit_last_action_time >= ?";
+        $bind = $idSites;
+        $bind[] = Date::factory(time() - $lastMinutes * 60)->toString('Y-m-d H:i:s');
+
+        $segment = new Segment($segment, $idSite);
+        $query = $segment->getSelectQuery($select, 'log_visit', $where, $bind);
+
+        $data = Db::fetchAll($query['sql'], $query['bind']);
+
+        $counters['visits'] = $data[0]['visits'];
+        $counters['visitors'] = $data[0]['visitors'];
+
+        $select = "count(*)";
+        $from = 'log_link_visit_action';
+        list($whereIdSites) = $this->getIdSitesWhereClause($idSite, $from);
+        $where = $whereIdSites . "AND log_link_visit_action.server_time >= ?";
+        $query = $segment->getSelectQuery($select, $from, $where, $bind);
+        $counters['actions'] = Db::fetchOne($query['sql'], $query['bind']);
+
+        $select = "count(*)";
+        $from = 'log_conversion';
+        list($whereIdSites) = $this->getIdSitesWhereClause($idSite, $from);
+        $where = $whereIdSites . "AND log_conversion.server_time >= ?";
+        $query = $segment->getSelectQuery($select, $from, $where, $bind);
+        $counters['visitsConverted'] = Db::fetchOne($query['sql'], $query['bind']);
+
+        return array($counters);
+    }
+
+
+
+    /**
+     * @param $idSite
+     * @param string $table
+     * @return array
+     */
+    private function getIdSitesWhereClause($idSite, $table = 'log_visit')
+    {
+        $idSites = array($idSite);
+        Piwik::postEvent('Live.API.getIdSitesString', array(&$idSites));
+
+        $idSitesBind = Common::getSqlStringFieldsArray($idSites);
+        $whereClause = $table . ".idsite in ($idSitesBind) ";
+        return array($whereClause, $idSites);
+    }
+
+
+    /**
+     * @param $idSite
+     * @param $visitorId
+     * @param $visitLastActionTime
+     * @param $segment
+     * @param $getNext
+     * @return string
+     * @throws Exception
+     */
+    public function queryAdjacentVisitorId($idSite, $visitorId, $visitLastActionTime, $segment, $getNext)
+    {
+        if ($getNext) {
+            $visitLastActionTimeCondition = "sub.visit_last_action_time <= ?";
+            $orderByDir = "DESC";
+        } else {
+            $visitLastActionTimeCondition = "sub.visit_last_action_time >= ?";
+            $orderByDir = "ASC";
+        }
+
+        $visitLastActionDate = Date::factory($visitLastActionTime);
+        $dateOneDayAgo = $visitLastActionDate->subDay(1);
+        $dateOneDayInFuture = $visitLastActionDate->addDay(1);
+
+        $select = "log_visit.idvisitor, MAX(log_visit.visit_last_action_time) as visit_last_action_time";
+        $from = "log_visit";
+        $where = "log_visit.idsite = ? AND log_visit.idvisitor <> ? AND visit_last_action_time >= ? and visit_last_action_time <= ?";
+        $whereBind = array($idSite, @Common::hex2bin($visitorId), $dateOneDayAgo->toString('Y-m-d H:i:s'), $dateOneDayInFuture->toString('Y-m-d H:i:s'));
+        $orderBy = "MAX(log_visit.visit_last_action_time) $orderByDir";
+        $groupBy = "log_visit.idvisitor";
+
+        $segment = new Segment($segment, $idSite);
+        $queryInfo = $segment->getSelectQuery($select, $from, $where, $whereBind, $orderBy, $groupBy);
+
+        $sql = "SELECT sub.idvisitor, sub.visit_last_action_time FROM ({$queryInfo['sql']}) as sub
+                 WHERE $visitLastActionTimeCondition
+                 LIMIT 1";
+        $bind = array_merge($queryInfo['bind'], array($visitLastActionTime));
+
+        $visitorId = Db::fetchOne($sql, $bind);
+        if (!empty($visitorId)) {
+            $visitorId = bin2hex($visitorId);
+        }
+        return $visitorId;
+    }
+} 
\ No newline at end of file
-- 
cgit v1.2.3


From 8ba4ba154aab7ac286b66ff16b59bb798ac3fb45 Mon Sep 17 00:00:00 2001
From: mattab <matthieu.aubry@gmail.com>
Date: Fri, 5 Dec 2014 22:42:43 +1300
Subject: Refactor Live API Visitor Profile logic in own class

---
 plugins/Live/API.php            | 262 +------------------------------------
 plugins/Live/Controller.php     |   2 +-
 plugins/Live/Model.php          |  20 ++-
 plugins/Live/VisitorProfile.php | 279 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 295 insertions(+), 268 deletions(-)
 create mode 100644 plugins/Live/VisitorProfile.php

diff --git a/plugins/Live/API.php b/plugins/Live/API.php
index 0b66e75f64..fc08e510db 100644
--- a/plugins/Live/API.php
+++ b/plugins/Live/API.php
@@ -16,10 +16,8 @@ use Piwik\DataTable\Row;
 use Piwik\Date;
 use Piwik\Db;
 use Piwik\Metrics\Formatter;
-use Piwik\Period\Range;
 use Piwik\Period;
 use Piwik\Piwik;
-use Piwik\Plugins\Referrers\API as APIReferrers;
 use Piwik\Plugins\SitesManager\API as APISitesManager;
 use Piwik\Segment;
 use Piwik\Site;
@@ -55,8 +53,6 @@ require_once PIWIK_INCLUDE_PATH . '/plugins/UserCountry/functions.php';
 class API extends \Piwik\Plugin\API
 {
     const VISITOR_PROFILE_MAX_VISITS_TO_AGGREGATE = 100;
-    const VISITOR_PROFILE_MAX_VISITS_TO_SHOW = 10;
-    const VISITOR_PROFILE_DATE_FORMAT = '%day% %shortMonth% %longYear%';
 
     /**
      * This will return simple counters, for a given website ID, for visits over the last N minutes
@@ -166,190 +162,8 @@ class API extends \Piwik\Plugin\API
             return array();
         }
 
-        $isEcommerceEnabled = Site::isEcommerceEnabledFor($idSite);
-
-        $result = array();
-        $result['totalVisits'] = 0;
-        $result['totalVisitDuration'] = 0;
-        $result['totalActions'] = 0;
-        $result['totalSearches'] = 0;
-        $result['totalPageViews'] = 0;
-        $result['totalGoalConversions'] = 0;
-        $result['totalConversionsByGoal'] = array();
-
-        if ($isEcommerceEnabled) {
-            $result['totalEcommerceConversions'] = 0;
-            $result['totalEcommerceRevenue'] = 0;
-            $result['totalEcommerceItems'] = 0;
-            $result['totalAbandonedCarts'] = 0;
-            $result['totalAbandonedCartsRevenue'] = 0;
-            $result['totalAbandonedCartsItems'] = 0;
-        }
-
-        $countries  = array();
-        $continents = array();
-        $cities     = array();
-        $siteSearchKeywords = array();
-
-        $pageGenerationTimeTotal = 0;
-
-        // aggregate all requested visits info for total_* info
-        foreach ($visits->getRows() as $visit) {
-            ++$result['totalVisits'];
-
-            $result['totalVisitDuration'] += $visit->getColumn('visitDuration');
-            $result['totalActions'] += $visit->getColumn('actions');
-            $result['totalGoalConversions'] += $visit->getColumn('goalConversions');
-
-            // individual goal conversions are stored in action details
-            foreach ($visit->getColumn('actionDetails') as $action) {
-                if ($action['type'] == 'goal') {
-                    // handle goal conversion
-                    $idGoal = $action['goalId'];
-                    $idGoalKey = 'idgoal=' . $idGoal;
-
-                    if (!isset($result['totalConversionsByGoal'][$idGoalKey])) {
-                        $result['totalConversionsByGoal'][$idGoalKey] = 0;
-                    }
-                    ++$result['totalConversionsByGoal'][$idGoalKey];
-
-                    if (!empty($action['revenue'])) {
-                        if (!isset($result['totalRevenueByGoal'][$idGoalKey])) {
-                            $result['totalRevenueByGoal'][$idGoalKey] = 0;
-                        }
-                        $result['totalRevenueByGoal'][$idGoalKey] += $action['revenue'];
-                    }
-                } else if ($action['type'] == Piwik::LABEL_ID_GOAL_IS_ECOMMERCE_ORDER // handle ecommerce order
-                    && $isEcommerceEnabled
-                ) {
-                    ++$result['totalEcommerceConversions'];
-                    $result['totalEcommerceRevenue'] += $action['revenue'];
-                    $result['totalEcommerceItems'] += $action['items'];
-                } else if ($action['type'] == Piwik::LABEL_ID_GOAL_IS_ECOMMERCE_CART // handler abandoned cart
-                    && $isEcommerceEnabled
-                ) {
-                    ++$result['totalAbandonedCarts'];
-                    $result['totalAbandonedCartsRevenue'] += $action['revenue'];
-                    $result['totalAbandonedCartsItems'] += $action['items'];
-                }
-
-                if (isset($action['siteSearchKeyword'])) {
-                    $keyword = $action['siteSearchKeyword'];
-
-                    if (!isset($siteSearchKeywords[$keyword])) {
-                        $siteSearchKeywords[$keyword] = 0;
-                        ++$result['totalSearches'];
-                    }
-                    ++$siteSearchKeywords[$keyword];
-                }
-
-                if (isset($action['generationTime'])) {
-                    $pageGenerationTimeTotal += $action['generationTime'];
-                    ++$result['totalPageViews'];
-                }
-            }
-
-            $countryCode = $visit->getColumn('countryCode');
-            if (!isset($countries[$countryCode])) {
-                $countries[$countryCode] = 0;
-            }
-            ++$countries[$countryCode];
-
-            $continentCode = $visit->getColumn('continentCode');
-            if (!isset($continents[$continentCode])) {
-                $continents[$continentCode] = 0;
-            }
-            ++$continents[$continentCode];
-
-            if ($countryCode && !array_key_exists($countryCode, $cities)) {
-                $cities[$countryCode] = array();
-            }
-            $city = $visit->getColumn('city');
-            if (!empty($city)) {
-                $cities[$countryCode][] = $city;
-            }
-        }
-
-        // sort countries/continents/search keywords by visit/action
-        asort($countries);
-        asort($continents);
-        arsort($siteSearchKeywords);
-
-        // transform country/continents/search keywords into something that will look good in XML
-        $result['countries'] = $result['continents'] = $result['searches'] = array();
-
-        foreach ($countries as $countryCode => $nbVisits) {
-
-            $countryInfo = array('country'    => $countryCode,
-                                   'nb_visits'  => $nbVisits,
-                                   'flag'       => \Piwik\Plugins\UserCountry\getFlagFromCode($countryCode),
-                                   'prettyName' => \Piwik\Plugins\UserCountry\countryTranslate($countryCode));
-            if (!empty($cities[$countryCode])) {
-                $countryInfo['cities'] = array_unique($cities[$countryCode]);
-            }
-            $result['countries'][] = $countryInfo;
-        }
-        foreach ($continents as $continentCode => $nbVisits) {
-            $result['continents'][] = array('continent'  => $continentCode,
-                                            'nb_visits'  => $nbVisits,
-                                            'prettyName' => \Piwik\Plugins\UserCountry\continentTranslate($continentCode));
-        }
-        foreach ($siteSearchKeywords as $keyword => $searchCount) {
-            $result['searches'][] = array('keyword'  => $keyword,
-                                          'searches' => $searchCount);
-        }
-
-        if ($result['totalPageViews']) {
-            $result['averagePageGenerationTime'] =
-                round($pageGenerationTimeTotal / $result['totalPageViews'], $precision = 2);
-        }
-
-        $formatter = new Formatter();
-        $result['totalVisitDurationPretty'] = $formatter->getPrettyTimeFromSeconds($result['totalVisitDuration'], true);
-
-        // use requested visits for first/last visit info
-        $rows = $visits->getRows();
-        $result['firstVisit'] = $this->getVisitorProfileVisitSummary(end($rows));
-        $result['lastVisit'] = $this->getVisitorProfileVisitSummary(reset($rows));
-
-        // check if requested visits have lat/long
-        if ($checkForLatLong) {
-            $result['hasLatLong'] = false;
-            foreach ($rows as $visit) {
-                if ($visit->getColumn('latitude') !== false) { // realtime map only checks for latitude
-                    $result['hasLatLong'] = true;
-                    break;
-                }
-            }
-        }
-
-        // save count of visits we queries
-        $result['visitsAggregated'] = count($rows);
-
-        // use N most recent visits for last_visits
-        $visits->deleteRowsOffset(self::VISITOR_PROFILE_MAX_VISITS_TO_SHOW);
-        $result['lastVisits'] = $visits;
-
-        // use the right date format for the pretty server date
-        $timezone = Site::getTimezoneFor($idSite);
-        foreach ($result['lastVisits']->getRows() as $visit) {
-            $dateTimeVisitFirstAction = Date::factory($visit->getColumn('firstActionTimestamp'), $timezone);
-
-            $datePretty = $dateTimeVisitFirstAction->getLocalized(self::VISITOR_PROFILE_DATE_FORMAT);
-            $visit->setColumn('serverDatePrettyFirstAction', $datePretty);
-
-            $dateTimePretty = $datePretty . ' ' . $visit->getColumn('serverTimePrettyFirstAction');
-            $visit->setColumn('serverDateTimePrettyFirstAction', $dateTimePretty);
-        }
-
-        $result['userId'] = $visit->getColumn('userId');
-
-        // get visitor IDs that are adjacent to this one in log_visit
-        // TODO: make sure order of visitor ids is not changed if a returning visitor visits while the user is
-        //       looking at the popup.
-        $latestVisitTime = reset($rows)->getColumn('lastActionDateTime');
-        $result['nextVisitorId'] = $this->getAdjacentVisitorId($idSite, $visitorId, $latestVisitTime, $segment, $getNext = true);
-        $result['previousVisitorId'] = $this->getAdjacentVisitorId($idSite, $visitorId, $latestVisitTime, $segment, $getNext = false);
+        $profile = new VisitorProfile();
+        $result = $profile->makeVisitorProfile($visits, $idSite, $visitorId, $segment, $checkForLatLong);
 
         /**
          * Triggered in the Live.getVisitorProfile API method. Plugins can use this event
@@ -398,77 +212,6 @@ class API extends \Piwik\Plugin\API
         return $visitor->getVisitorId();
     }
 
-    /**
-     * Returns the ID of a visitor that is adjacent to another visitor (by time of last action)
-     * in the log_visit table.
-     *
-     * @param int $idSite The ID of the site whose visits should be looked at.
-     * @param string $visitorId The ID of the visitor to get an adjacent visitor for.
-     * @param string $visitLastActionTime The last action time of the latest visit for $visitorId.
-     * @param string $segment
-     * @param bool $getNext Whether to retrieve the next visitor or the previous visitor. The next
-     *                      visitor will be the visitor that appears chronologically later in the
-     *                      log_visit table. The previous visitor will be the visitor that appears
-     *                      earlier.
-     * @return string The hex visitor ID.
-     */
-    private function getAdjacentVisitorId($idSite, $visitorId, $visitLastActionTime, $segment, $getNext)
-    {
-        $model = new Model();
-        return $model->queryAdjacentVisitorId($idSite, $visitorId, $visitLastActionTime, $segment, $getNext);
-    }
-
-    /**
-     * Returns a summary for an important visit. Used to describe the first & last visits of a visitor.
-     *
-     * @param Row $visit
-     * @return array
-     */
-    private function getVisitorProfileVisitSummary($visit)
-    {
-        $today = Date::today();
-
-        $serverDate = $visit->getColumn('firstActionTimestamp');
-        return array(
-            'date'            => $serverDate,
-            'prettyDate'      => Date::factory($serverDate)->getLocalized(self::VISITOR_PROFILE_DATE_FORMAT),
-            'daysAgo'         => (int)Date::secondsToDays($today->getTimestamp() - Date::factory($serverDate)->getTimestamp()),
-            'referrerType'    => $visit->getColumn('referrerType'),
-            'referralSummary' => self::getReferrerSummaryForVisit($visit),
-        );
-    }
-
-    /**
-     * Returns a summary for a visit's referral.
-     *
-     * @param Row $visit
-     * @return bool|mixed|string
-     * @ignore
-     */
-    public static function getReferrerSummaryForVisit($visit)
-    {
-        $referrerType = $visit->getColumn('referrerType');
-        if ($referrerType === false
-            || $referrerType == 'direct'
-        ) {
-            $result = Piwik::translate('Referrers_DirectEntry');
-        } else if ($referrerType == 'search') {
-            $result = $visit->getColumn('referrerName');
-
-            $keyword = $visit->getColumn('referrerKeyword');
-            if ($keyword !== false
-                && $keyword != APIReferrers::getKeywordNotDefinedString()
-            ) {
-                $result .= ' (' . $keyword . ')';
-            }
-        } else if ($referrerType == 'campaign') {
-            $result = Piwik::translate('Referrers_ColumnCampaign') . ' (' . $visit->getColumn('referrerName') . ')';
-        } else {
-            $result = $visit->getColumn('referrerName');
-        }
-
-        return $result;
-    }
 
     /**
      * @deprecated
@@ -545,7 +288,6 @@ class API extends \Piwik\Plugin\API
     {
         $model = new Model();
         $data = $model->queryLogVisits($idSite, $period, $date, $segment, $countVisitorsToFetch, $visitorId, $minTimestamp, $filterSortOrder);
-
         return $this->makeVisitorTableFromArray($data);
     }
 
diff --git a/plugins/Live/Controller.php b/plugins/Live/Controller.php
index 9df3e26ff8..da434412dd 100644
--- a/plugins/Live/Controller.php
+++ b/plugins/Live/Controller.php
@@ -133,7 +133,7 @@ class Controller extends \Piwik\Plugin\Controller
                                                                             'date'    => false
                                                                        ));
         $view->visitData = $visits->getFirstRow()->getColumns();
-        $view->visitReferralSummary = API::getReferrerSummaryForVisit($visits->getFirstRow());
+        $view->visitReferralSummary = VisitorProfile::getReferrerSummaryForVisit($visits->getFirstRow());
         $view->showLocation = true;
         $this->setWidgetizedVisitorProfileUrl($view);
         $view->exportLink = $this->getVisitorProfileExportLink();
diff --git a/plugins/Live/Model.php b/plugins/Live/Model.php
index 7bf5485c5a..2353450572 100644
--- a/plugins/Live/Model.php
+++ b/plugins/Live/Model.php
@@ -7,7 +7,7 @@
  *
  */
 
-namespace Piwik\plugins\Live;
+namespace Piwik\Plugins\Live;
 
 use Exception;
 use Piwik\Common;
@@ -216,12 +216,18 @@ class Model
 
 
     /**
-     * @param $idSite
-     * @param $visitorId
-     * @param $visitLastActionTime
-     * @param $segment
-     * @param $getNext
-     * @return string
+     * Returns the ID of a visitor that is adjacent to another visitor (by time of last action)
+     * in the log_visit table.
+     *
+     * @param int $idSite The ID of the site whose visits should be looked at.
+     * @param string $visitorId The ID of the visitor to get an adjacent visitor for.
+     * @param string $visitLastActionTime The last action time of the latest visit for $visitorId.
+     * @param string $segment
+     * @param bool $getNext Whether to retrieve the next visitor or the previous visitor. The next
+     *                      visitor will be the visitor that appears chronologically later in the
+     *                      log_visit table. The previous visitor will be the visitor that appears
+     *                      earlier.
+     * @return string The hex visitor ID.
      * @throws Exception
      */
     public function queryAdjacentVisitorId($idSite, $visitorId, $visitLastActionTime, $segment, $getNext)
diff --git a/plugins/Live/VisitorProfile.php b/plugins/Live/VisitorProfile.php
new file mode 100644
index 0000000000..807028d6ad
--- /dev/null
+++ b/plugins/Live/VisitorProfile.php
@@ -0,0 +1,279 @@
+<?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\Live;
+
+use Exception;
+use Piwik\DataTable;
+use Piwik\Date;
+use Piwik\Metrics\Formatter;
+use Piwik\Piwik;
+use Piwik\Site;
+use Piwik\Plugins\Referrers\API as APIReferrers;
+
+class VisitorProfile
+{
+    const VISITOR_PROFILE_MAX_VISITS_TO_SHOW = 10;
+    const VISITOR_PROFILE_DATE_FORMAT = '%day% %shortMonth% %longYear%';
+
+    /**
+     * @param $visits
+     * @param $idSite
+     * @param $visitorId
+     * @param $segment
+     * @param $checkForLatLong
+     * @return array
+     * @throws Exception
+     */
+    public function makeVisitorProfile(DataTable $visits, $idSite, $visitorId, $segment, $checkForLatLong)
+    {
+        $isEcommerceEnabled = Site::isEcommerceEnabledFor($idSite);
+
+        $result = array();
+        $result['totalVisits'] = 0;
+        $result['totalVisitDuration'] = 0;
+        $result['totalActions'] = 0;
+        $result['totalSearches'] = 0;
+        $result['totalPageViews'] = 0;
+        $result['totalGoalConversions'] = 0;
+        $result['totalConversionsByGoal'] = array();
+
+        if ($isEcommerceEnabled) {
+            $result['totalEcommerceConversions'] = 0;
+            $result['totalEcommerceRevenue'] = 0;
+            $result['totalEcommerceItems'] = 0;
+            $result['totalAbandonedCarts'] = 0;
+            $result['totalAbandonedCartsRevenue'] = 0;
+            $result['totalAbandonedCartsItems'] = 0;
+        }
+
+        $countries = array();
+        $continents = array();
+        $cities = array();
+        $siteSearchKeywords = array();
+
+        $pageGenerationTimeTotal = 0;
+
+        // aggregate all requested visits info for total_* info
+        /** @var DataTable\Row $visit */
+        foreach ($visits->getRows() as $visit) {
+            ++$result['totalVisits'];
+
+            $result['totalVisitDuration'] += $visit->getColumn('visitDuration');
+            $result['totalActions'] += $visit->getColumn('actions');
+            $result['totalGoalConversions'] += $visit->getColumn('goalConversions');
+
+            // individual goal conversions are stored in action details
+            foreach ($visit->getColumn('actionDetails') as $action) {
+                if ($action['type'] == 'goal') {
+                    // handle goal conversion
+                    $idGoal = $action['goalId'];
+                    $idGoalKey = 'idgoal=' . $idGoal;
+
+                    if (!isset($result['totalConversionsByGoal'][$idGoalKey])) {
+                        $result['totalConversionsByGoal'][$idGoalKey] = 0;
+                    }
+                    ++$result['totalConversionsByGoal'][$idGoalKey];
+
+                    if (!empty($action['revenue'])) {
+                        if (!isset($result['totalRevenueByGoal'][$idGoalKey])) {
+                            $result['totalRevenueByGoal'][$idGoalKey] = 0;
+                        }
+                        $result['totalRevenueByGoal'][$idGoalKey] += $action['revenue'];
+                    }
+                } else if ($action['type'] == Piwik::LABEL_ID_GOAL_IS_ECOMMERCE_ORDER // handle ecommerce order
+                    && $isEcommerceEnabled
+                ) {
+                    ++$result['totalEcommerceConversions'];
+                    $result['totalEcommerceRevenue'] += $action['revenue'];
+                    $result['totalEcommerceItems'] += $action['items'];
+                } else if ($action['type'] == Piwik::LABEL_ID_GOAL_IS_ECOMMERCE_CART // handler abandoned cart
+                    && $isEcommerceEnabled
+                ) {
+                    ++$result['totalAbandonedCarts'];
+                    $result['totalAbandonedCartsRevenue'] += $action['revenue'];
+                    $result['totalAbandonedCartsItems'] += $action['items'];
+                }
+
+                if (isset($action['siteSearchKeyword'])) {
+                    $keyword = $action['siteSearchKeyword'];
+
+                    if (!isset($siteSearchKeywords[$keyword])) {
+                        $siteSearchKeywords[$keyword] = 0;
+                        ++$result['totalSearches'];
+                    }
+                    ++$siteSearchKeywords[$keyword];
+                }
+
+                if (isset($action['generationTime'])) {
+                    $pageGenerationTimeTotal += $action['generationTime'];
+                    ++$result['totalPageViews'];
+                }
+            }
+
+            $countryCode = $visit->getColumn('countryCode');
+            if (!isset($countries[$countryCode])) {
+                $countries[$countryCode] = 0;
+            }
+            ++$countries[$countryCode];
+
+            $continentCode = $visit->getColumn('continentCode');
+            if (!isset($continents[$continentCode])) {
+                $continents[$continentCode] = 0;
+            }
+            ++$continents[$continentCode];
+
+            if ($countryCode && !array_key_exists($countryCode, $cities)) {
+                $cities[$countryCode] = array();
+            }
+            $city = $visit->getColumn('city');
+            if (!empty($city)) {
+                $cities[$countryCode][] = $city;
+            }
+        }
+
+        // sort countries/continents/search keywords by visit/action
+        asort($countries);
+        asort($continents);
+        arsort($siteSearchKeywords);
+
+        // transform country/continents/search keywords into something that will look good in XML
+        $result['countries'] = $result['continents'] = $result['searches'] = array();
+
+        foreach ($countries as $countryCode => $nbVisits) {
+
+            $countryInfo = array('country' => $countryCode,
+                'nb_visits' => $nbVisits,
+                'flag' => \Piwik\Plugins\UserCountry\getFlagFromCode($countryCode),
+                'prettyName' => \Piwik\Plugins\UserCountry\countryTranslate($countryCode));
+            if (!empty($cities[$countryCode])) {
+                $countryInfo['cities'] = array_unique($cities[$countryCode]);
+            }
+            $result['countries'][] = $countryInfo;
+        }
+        foreach ($continents as $continentCode => $nbVisits) {
+            $result['continents'][] = array('continent' => $continentCode,
+                'nb_visits' => $nbVisits,
+                'prettyName' => \Piwik\Plugins\UserCountry\continentTranslate($continentCode));
+        }
+        foreach ($siteSearchKeywords as $keyword => $searchCount) {
+            $result['searches'][] = array('keyword' => $keyword,
+                'searches' => $searchCount);
+        }
+
+        if ($result['totalPageViews']) {
+            $result['averagePageGenerationTime'] =
+                round($pageGenerationTimeTotal / $result['totalPageViews'], $precision = 2);
+        }
+
+        $formatter = new Formatter();
+        $result['totalVisitDurationPretty'] = $formatter->getPrettyTimeFromSeconds($result['totalVisitDuration'], true);
+
+        // use requested visits for first/last visit info
+        $rows = $visits->getRows();
+        $result['firstVisit'] = $this->getVisitorProfileVisitSummary(end($rows));
+        $result['lastVisit'] = $this->getVisitorProfileVisitSummary(reset($rows));
+
+        // check if requested visits have lat/long
+        if ($checkForLatLong) {
+            $result['hasLatLong'] = false;
+            foreach ($rows as $visit) {
+                if ($visit->getColumn('latitude') !== false) { // realtime map only checks for latitude
+                    $result['hasLatLong'] = true;
+                    break;
+                }
+            }
+        }
+
+        // save count of visits we queries
+        $result['visitsAggregated'] = count($rows);
+
+        // use N most recent visits for last_visits
+        $visits->deleteRowsOffset(self::VISITOR_PROFILE_MAX_VISITS_TO_SHOW);
+        $result['lastVisits'] = $visits;
+
+        // use the right date format for the pretty server date
+        $timezone = Site::getTimezoneFor($idSite);
+        foreach ($result['lastVisits']->getRows() as $visit) {
+            $dateTimeVisitFirstAction = Date::factory($visit->getColumn('firstActionTimestamp'), $timezone);
+
+            $datePretty = $dateTimeVisitFirstAction->getLocalized(self::VISITOR_PROFILE_DATE_FORMAT);
+            $visit->setColumn('serverDatePrettyFirstAction', $datePretty);
+
+            $dateTimePretty = $datePretty . ' ' . $visit->getColumn('serverTimePrettyFirstAction');
+            $visit->setColumn('serverDateTimePrettyFirstAction', $dateTimePretty);
+        }
+
+        $result['userId'] = $visit->getColumn('userId');
+
+        // get visitor IDs that are adjacent to this one in log_visit
+        // TODO: make sure order of visitor ids is not changed if a returning visitor visits while the user is
+        //       looking at the popup.
+        $latestVisitTime = reset($rows)->getColumn('lastActionDateTime');
+
+
+        $model = new Model();
+        $result['nextVisitorId'] = $model->queryAdjacentVisitorId($idSite, $visitorId, $latestVisitTime, $segment, $getNext = true);
+        $result['previousVisitorId'] = $model->queryAdjacentVisitorId($idSite, $visitorId, $latestVisitTime, $segment, $getNext = false);
+        return $result;
+    }
+
+    /**
+     * Returns a summary for an important visit. Used to describe the first & last visits of a visitor.
+     *
+     * @param DataTable\Row $visit
+     * @return array
+     */
+    private function getVisitorProfileVisitSummary($visit)
+    {
+        $today = Date::today();
+
+        $serverDate = $visit->getColumn('firstActionTimestamp');
+        return array(
+            'date'            => $serverDate,
+            'prettyDate'      => Date::factory($serverDate)->getLocalized(self::VISITOR_PROFILE_DATE_FORMAT),
+            'daysAgo'         => (int)Date::secondsToDays($today->getTimestamp() - Date::factory($serverDate)->getTimestamp()),
+            'referrerType'    => $visit->getColumn('referrerType'),
+            'referralSummary' => self::getReferrerSummaryForVisit($visit),
+        );
+    }
+
+
+    /**
+     * Returns a summary for a visit's referral.
+     *
+     * @param DataTable\Row $visit
+     * @return bool|mixed|string
+     */
+    public static function getReferrerSummaryForVisit($visit)
+    {
+        $referrerType = $visit->getColumn('referrerType');
+        if ($referrerType === false
+            || $referrerType == 'direct'
+        ) {
+            $result = Piwik::translate('Referrers_DirectEntry');
+        } else if ($referrerType == 'search') {
+            $result = $visit->getColumn('referrerName');
+
+            $keyword = $visit->getColumn('referrerKeyword');
+            if ($keyword !== false
+                && $keyword != APIReferrers::getKeywordNotDefinedString()
+            ) {
+                $result .= ' (' . $keyword . ')';
+            }
+        } else if ($referrerType == 'campaign') {
+            $result = Piwik::translate('Referrers_ColumnCampaign') . ' (' . $visit->getColumn('referrerName') . ')';
+        } else {
+            $result = $visit->getColumn('referrerName');
+        }
+
+        return $result;
+    }
+
+} 
\ No newline at end of file
-- 
cgit v1.2.3


From af73717d45280335b33707295fc4a72f3937d74e Mon Sep 17 00:00:00 2001
From: mattab <matthieu.aubry@gmail.com>
Date: Fri, 5 Dec 2014 22:58:46 +1300
Subject: Removed checkLatLong API parameter and always set the info

---
 plugins/Live/API.php        | 6 ++----
 plugins/Live/Controller.php | 2 +-
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/plugins/Live/API.php b/plugins/Live/API.php
index fc08e510db..970056001b 100644
--- a/plugins/Live/API.php
+++ b/plugins/Live/API.php
@@ -138,11 +138,9 @@ class API extends \Piwik\Plugin\API
      * @param int $idSite Site ID
      * @param bool|false|string $visitorId The ID of the visitor whose profile to retrieve.
      * @param bool|false|string $segment
-     * @param bool $checkForLatLong If true, hasLatLong will appear in the output and be true if
-     *                              one of the first 100 visits has a latitude/longitude.
      * @return array
      */
-    public function getVisitorProfile($idSite, $visitorId = false, $segment = false, $checkForLatLong = false)
+    public function getVisitorProfile($idSite, $visitorId = false, $segment = false)
     {
         Piwik::checkUserHasViewAccess($idSite);
 
@@ -163,7 +161,7 @@ class API extends \Piwik\Plugin\API
         }
 
         $profile = new VisitorProfile();
-        $result = $profile->makeVisitorProfile($visits, $idSite, $visitorId, $segment, $checkForLatLong);
+        $result = $profile->makeVisitorProfile($visits, $idSite, $visitorId, $segment);
 
         /**
          * Triggered in the Live.getVisitorProfile API method. Plugins can use this event
diff --git a/plugins/Live/Controller.php b/plugins/Live/Controller.php
index da434412dd..82dccb92bc 100644
--- a/plugins/Live/Controller.php
+++ b/plugins/Live/Controller.php
@@ -109,7 +109,7 @@ class Controller extends \Piwik\Plugin\Controller
         $view = new View('@Live/getVisitorProfilePopup.twig');
         $view->idSite = $idSite;
         $view->goals = APIGoals::getInstance()->getGoals($idSite);
-        $view->visitorData = Request::processRequest('Live.getVisitorProfile', array('checkForLatLong' => true));
+        $view->visitorData = Request::processRequest('Live.getVisitorProfile');
         $view->exportLink = $this->getVisitorProfileExportLink();
 
         if (Common::getRequestVar('showMap', 1) == 1
-- 
cgit v1.2.3


From 13bb9a7ecdd65ea61caf70a449af13edf9cb2913 Mon Sep 17 00:00:00 2001
From: mattab <matthieu.aubry@gmail.com>
Date: Fri, 5 Dec 2014 23:33:37 +1300
Subject: Visitor Profile clean code

---
 plugins/Live/API.php                               |   4 +-
 plugins/Live/VisitorProfile.php                    | 444 ++++++++++++---------
 plugins/Live/templates/getVisitorProfilePopup.twig |   2 +-
 3 files changed, 266 insertions(+), 184 deletions(-)

diff --git a/plugins/Live/API.php b/plugins/Live/API.php
index 970056001b..ed11c6c277 100644
--- a/plugins/Live/API.php
+++ b/plugins/Live/API.php
@@ -160,8 +160,8 @@ class API extends \Piwik\Plugin\API
             return array();
         }
 
-        $profile = new VisitorProfile();
-        $result = $profile->makeVisitorProfile($visits, $idSite, $visitorId, $segment);
+        $profile = new VisitorProfile($idSite);
+        $result = $profile->makeVisitorProfile($visits, $visitorId, $segment);
 
         /**
          * Triggered in the Live.getVisitorProfile API method. Plugins can use this event
diff --git a/plugins/Live/VisitorProfile.php b/plugins/Live/VisitorProfile.php
index 807028d6ad..d52adbf076 100644
--- a/plugins/Live/VisitorProfile.php
+++ b/plugins/Live/VisitorProfile.php
@@ -22,206 +22,69 @@ class VisitorProfile
     const VISITOR_PROFILE_MAX_VISITS_TO_SHOW = 10;
     const VISITOR_PROFILE_DATE_FORMAT = '%day% %shortMonth% %longYear%';
 
+    protected $profile = array();
+    private $siteSearchKeywords = array();
+    private $continents = array();
+    private $countries = array();
+    private $cities = array();
+    private $pageGenerationTimeTotal = 0;
+
+    public function __construct($idSite)
+    {
+        $this->idSite = $idSite;
+        $this->isEcommerceEnabled = Site::isEcommerceEnabledFor($this->idSite);
+    }
+
     /**
      * @param $visits
-     * @param $idSite
      * @param $visitorId
      * @param $segment
-     * @param $checkForLatLong
      * @return array
      * @throws Exception
      */
-    public function makeVisitorProfile(DataTable $visits, $idSite, $visitorId, $segment, $checkForLatLong)
+    public function makeVisitorProfile(DataTable $visits, $visitorId, $segment)
     {
-        $isEcommerceEnabled = Site::isEcommerceEnabledFor($idSite);
-
-        $result = array();
-        $result['totalVisits'] = 0;
-        $result['totalVisitDuration'] = 0;
-        $result['totalActions'] = 0;
-        $result['totalSearches'] = 0;
-        $result['totalPageViews'] = 0;
-        $result['totalGoalConversions'] = 0;
-        $result['totalConversionsByGoal'] = array();
-
-        if ($isEcommerceEnabled) {
-            $result['totalEcommerceConversions'] = 0;
-            $result['totalEcommerceRevenue'] = 0;
-            $result['totalEcommerceItems'] = 0;
-            $result['totalAbandonedCarts'] = 0;
-            $result['totalAbandonedCartsRevenue'] = 0;
-            $result['totalAbandonedCartsItems'] = 0;
-        }
-
-        $countries = array();
-        $continents = array();
-        $cities = array();
-        $siteSearchKeywords = array();
+        $this->initVisitorProfile();
 
-        $pageGenerationTimeTotal = 0;
-
-        // aggregate all requested visits info for total_* info
         /** @var DataTable\Row $visit */
         foreach ($visits->getRows() as $visit) {
-            ++$result['totalVisits'];
+            ++$this->profile['totalVisits'];
 
-            $result['totalVisitDuration'] += $visit->getColumn('visitDuration');
-            $result['totalActions'] += $visit->getColumn('actions');
-            $result['totalGoalConversions'] += $visit->getColumn('goalConversions');
+            $this->profile['totalVisitDuration'] += $visit->getColumn('visitDuration');
+            $this->profile['totalActions'] += $visit->getColumn('actions');
+            $this->profile['totalGoalConversions'] += $visit->getColumn('goalConversions');
 
             // individual goal conversions are stored in action details
             foreach ($visit->getColumn('actionDetails') as $action) {
-                if ($action['type'] == 'goal') {
-                    // handle goal conversion
-                    $idGoal = $action['goalId'];
-                    $idGoalKey = 'idgoal=' . $idGoal;
-
-                    if (!isset($result['totalConversionsByGoal'][$idGoalKey])) {
-                        $result['totalConversionsByGoal'][$idGoalKey] = 0;
-                    }
-                    ++$result['totalConversionsByGoal'][$idGoalKey];
-
-                    if (!empty($action['revenue'])) {
-                        if (!isset($result['totalRevenueByGoal'][$idGoalKey])) {
-                            $result['totalRevenueByGoal'][$idGoalKey] = 0;
-                        }
-                        $result['totalRevenueByGoal'][$idGoalKey] += $action['revenue'];
-                    }
-                } else if ($action['type'] == Piwik::LABEL_ID_GOAL_IS_ECOMMERCE_ORDER // handle ecommerce order
-                    && $isEcommerceEnabled
-                ) {
-                    ++$result['totalEcommerceConversions'];
-                    $result['totalEcommerceRevenue'] += $action['revenue'];
-                    $result['totalEcommerceItems'] += $action['items'];
-                } else if ($action['type'] == Piwik::LABEL_ID_GOAL_IS_ECOMMERCE_CART // handler abandoned cart
-                    && $isEcommerceEnabled
-                ) {
-                    ++$result['totalAbandonedCarts'];
-                    $result['totalAbandonedCartsRevenue'] += $action['revenue'];
-                    $result['totalAbandonedCartsItems'] += $action['items'];
-                }
-
-                if (isset($action['siteSearchKeyword'])) {
-                    $keyword = $action['siteSearchKeyword'];
-
-                    if (!isset($siteSearchKeywords[$keyword])) {
-                        $siteSearchKeywords[$keyword] = 0;
-                        ++$result['totalSearches'];
-                    }
-                    ++$siteSearchKeywords[$keyword];
-                }
-
-                if (isset($action['generationTime'])) {
-                    $pageGenerationTimeTotal += $action['generationTime'];
-                    ++$result['totalPageViews'];
-                }
-            }
-
-            $countryCode = $visit->getColumn('countryCode');
-            if (!isset($countries[$countryCode])) {
-                $countries[$countryCode] = 0;
-            }
-            ++$countries[$countryCode];
-
-            $continentCode = $visit->getColumn('continentCode');
-            if (!isset($continents[$continentCode])) {
-                $continents[$continentCode] = 0;
-            }
-            ++$continents[$continentCode];
-
-            if ($countryCode && !array_key_exists($countryCode, $cities)) {
-                $cities[$countryCode] = array();
-            }
-            $city = $visit->getColumn('city');
-            if (!empty($city)) {
-                $cities[$countryCode][] = $city;
-            }
-        }
-
-        // sort countries/continents/search keywords by visit/action
-        asort($countries);
-        asort($continents);
-        arsort($siteSearchKeywords);
-
-        // transform country/continents/search keywords into something that will look good in XML
-        $result['countries'] = $result['continents'] = $result['searches'] = array();
-
-        foreach ($countries as $countryCode => $nbVisits) {
-
-            $countryInfo = array('country' => $countryCode,
-                'nb_visits' => $nbVisits,
-                'flag' => \Piwik\Plugins\UserCountry\getFlagFromCode($countryCode),
-                'prettyName' => \Piwik\Plugins\UserCountry\countryTranslate($countryCode));
-            if (!empty($cities[$countryCode])) {
-                $countryInfo['cities'] = array_unique($cities[$countryCode]);
+                $this->handleIfGoalAction($action);
+                $this->handleIfEcommerceAction($action);
+                $this->handleIfSiteSearchAction($action);
+                $this->handleIfPageGenerationTime($action);
             }
-            $result['countries'][] = $countryInfo;
-        }
-        foreach ($continents as $continentCode => $nbVisits) {
-            $result['continents'][] = array('continent' => $continentCode,
-                'nb_visits' => $nbVisits,
-                'prettyName' => \Piwik\Plugins\UserCountry\continentTranslate($continentCode));
-        }
-        foreach ($siteSearchKeywords as $keyword => $searchCount) {
-            $result['searches'][] = array('keyword' => $keyword,
-                'searches' => $searchCount);
+            $this->handleGeoLocation($visit);
         }
 
-        if ($result['totalPageViews']) {
-            $result['averagePageGenerationTime'] =
-                round($pageGenerationTimeTotal / $result['totalPageViews'], $precision = 2);
-        }
+        $this->handleGeoLocationCountries();
+        $this->handleGeoLocationContinents();
+        $this->handleSiteSearches();
+        $this->handleAveragePageGenerationTime();
 
         $formatter = new Formatter();
-        $result['totalVisitDurationPretty'] = $formatter->getPrettyTimeFromSeconds($result['totalVisitDuration'], true);
+        $this->profile['totalVisitDurationPretty'] = $formatter->getPrettyTimeFromSeconds($this->profile['totalVisitDuration'], true);
 
-        // use requested visits for first/last visit info
-        $rows = $visits->getRows();
-        $result['firstVisit'] = $this->getVisitorProfileVisitSummary(end($rows));
-        $result['lastVisit'] = $this->getVisitorProfileVisitSummary(reset($rows));
-
-        // check if requested visits have lat/long
-        if ($checkForLatLong) {
-            $result['hasLatLong'] = false;
-            foreach ($rows as $visit) {
-                if ($visit->getColumn('latitude') !== false) { // realtime map only checks for latitude
-                    $result['hasLatLong'] = true;
-                    break;
-                }
-            }
-        }
-
-        // save count of visits we queries
-        $result['visitsAggregated'] = count($rows);
+        $this->handleVisitsSummary($visits);
+        $this->handleAdjacentVisitorIds($visits, $visitorId, $segment);
 
         // use N most recent visits for last_visits
         $visits->deleteRowsOffset(self::VISITOR_PROFILE_MAX_VISITS_TO_SHOW);
-        $result['lastVisits'] = $visits;
-
-        // use the right date format for the pretty server date
-        $timezone = Site::getTimezoneFor($idSite);
-        foreach ($result['lastVisits']->getRows() as $visit) {
-            $dateTimeVisitFirstAction = Date::factory($visit->getColumn('firstActionTimestamp'), $timezone);
-
-            $datePretty = $dateTimeVisitFirstAction->getLocalized(self::VISITOR_PROFILE_DATE_FORMAT);
-            $visit->setColumn('serverDatePrettyFirstAction', $datePretty);
-
-            $dateTimePretty = $datePretty . ' ' . $visit->getColumn('serverTimePrettyFirstAction');
-            $visit->setColumn('serverDateTimePrettyFirstAction', $dateTimePretty);
-        }
 
-        $result['userId'] = $visit->getColumn('userId');
+        $this->enrichVisitsWithFirstActionDatetime($visits);
 
-        // get visitor IDs that are adjacent to this one in log_visit
-        // TODO: make sure order of visitor ids is not changed if a returning visitor visits while the user is
-        //       looking at the popup.
-        $latestVisitTime = reset($rows)->getColumn('lastActionDateTime');
+        $this->profile['lastVisits'] = $visits;
 
+        $this->profile['userId'] = $visit->getColumn('userId');
 
-        $model = new Model();
-        $result['nextVisitorId'] = $model->queryAdjacentVisitorId($idSite, $visitorId, $latestVisitTime, $segment, $getNext = true);
-        $result['previousVisitorId'] = $model->queryAdjacentVisitorId($idSite, $visitorId, $latestVisitTime, $segment, $getNext = false);
-        return $result;
+        return $this->profile;
     }
 
     /**
@@ -257,23 +120,242 @@ class VisitorProfile
         if ($referrerType === false
             || $referrerType == 'direct'
         ) {
-            $result = Piwik::translate('Referrers_DirectEntry');
-        } else if ($referrerType == 'search') {
-            $result = $visit->getColumn('referrerName');
+            return Piwik::translate('Referrers_DirectEntry');
+        }
+
+        if ($referrerType == 'search') {
+            $referrerName = $visit->getColumn('referrerName');
 
             $keyword = $visit->getColumn('referrerKeyword');
             if ($keyword !== false
                 && $keyword != APIReferrers::getKeywordNotDefinedString()
             ) {
-                $result .= ' (' . $keyword . ')';
+                $referrerName .= ' (' . $keyword . ')';
+            }
+            return $referrerName;
+        }
+
+        if ($referrerType == 'campaign') {
+            return Piwik::translate('Referrers_ColumnCampaign') . ' (' . $visit->getColumn('referrerName') . ')';
+        }
+
+        return $visit->getColumn('referrerName');
+    }
+
+    private function isEcommerceEnabled()
+    {
+        return $this->isEcommerceEnabled;
+    }
+
+    /**
+     * @param $action
+     */
+    private function handleIfEcommerceAction($action)
+    {
+        if (!$this->isEcommerceEnabled()) {
+            return;
+        }
+        if ($action['type'] == Piwik::LABEL_ID_GOAL_IS_ECOMMERCE_ORDER) {
+            ++$this->profile['totalEcommerceConversions'];
+            $this->profile['totalEcommerceRevenue'] += $action['revenue'];
+            $this->profile['totalEcommerceItems'] += $action['items'];
+        } else if ($action['type'] == Piwik::LABEL_ID_GOAL_IS_ECOMMERCE_CART) {
+            ++$this->profile['totalAbandonedCarts'];
+            $this->profile['totalAbandonedCartsRevenue'] += $action['revenue'];
+            $this->profile['totalAbandonedCartsItems'] += $action['items'];
+        }
+    }
+
+    private function handleIfGoalAction($action)
+    {
+        if ($action['type'] != 'goal') {
+            return;
+        }
+        $idGoal = $action['goalId'];
+        $idGoalKey = 'idgoal=' . $idGoal;
+
+        if (!isset($this->profile['totalConversionsByGoal'][$idGoalKey])) {
+            $this->profile['totalConversionsByGoal'][$idGoalKey] = 0;
+        }
+        ++$this->profile['totalConversionsByGoal'][$idGoalKey];
+
+        if (!empty($action['revenue'])) {
+            if (!isset($this->profile['totalRevenueByGoal'][$idGoalKey])) {
+                $this->profile['totalRevenueByGoal'][$idGoalKey] = 0;
+            }
+            $this->profile['totalRevenueByGoal'][$idGoalKey] += $action['revenue'];
+        }
+    }
+
+    private function handleIfSiteSearchAction($action)
+    {
+        if (!isset($action['siteSearchKeyword'])) {
+            return;
+        }
+        $keyword = $action['siteSearchKeyword'];
+
+        if (!isset($this->siteSearchKeywords[$keyword])) {
+            $this->siteSearchKeywords[$keyword] = 0;
+            ++$this->profile['totalSearches'];
+        }
+        ++$this->siteSearchKeywords[$keyword];
+    }
+
+    private function handleGeoLocation(DataTable\Row $visit)
+    {
+        // realtime map only checks for latitude
+        $hasLatitude = $visit->getColumn('latitude') !== false;
+        if ($hasLatitude) {
+            $this->profile['hasLatLong'] = true;
+        }
+
+        $countryCode = $visit->getColumn('countryCode');
+        if (!isset($this->countries[$countryCode])) {
+            $this->countries[$countryCode] = 0;
+        }
+        ++$this->countries[$countryCode];
+
+        $continentCode = $visit->getColumn('continentCode');
+        if (!isset($this->continents[$continentCode])) {
+            $this->continents[$continentCode] = 0;
+        }
+        ++$this->continents[$continentCode];
+
+        if ($countryCode && !array_key_exists($countryCode, $this->cities)) {
+            $this->cities[$countryCode] = array();
+        }
+        $city = $visit->getColumn('city');
+        if (!empty($city)) {
+            $this->cities[$countryCode][] = $city;
+        }
+    }
+
+    private function handleSiteSearches()
+    {
+        // sort by visit/action
+        arsort($this->siteSearchKeywords);
+
+        foreach ($this->siteSearchKeywords as $keyword => $searchCount) {
+            $this->profile['searches'][] = array('keyword' => $keyword,
+                'searches' => $searchCount);
+        }
+    }
+
+    private function handleGeoLocationContinents()
+    {
+        // sort by visit/action
+        asort($this->continents);
+        foreach ($this->continents as $continentCode => $nbVisits) {
+            $this->profile['continents'][] = array('continent' => $continentCode,
+                'nb_visits' => $nbVisits,
+                'prettyName' => \Piwik\Plugins\UserCountry\continentTranslate($continentCode));
+        }
+    }
+
+    private function handleGeoLocationCountries()
+    {
+        // sort by visit/action
+        asort($this->countries);
+
+        // transform country/continents/search keywords into something that will look good in XML
+        $this->profile['countries'] = $this->profile['continents'] = $this->profile['searches'] = array();
+
+        foreach ($this->countries as $countryCode => $nbVisits) {
+
+            $countryInfo = array('country' => $countryCode,
+                'nb_visits' => $nbVisits,
+                'flag' => \Piwik\Plugins\UserCountry\getFlagFromCode($countryCode),
+                'prettyName' => \Piwik\Plugins\UserCountry\countryTranslate($countryCode));
+            if (!empty($this->cities[$countryCode])) {
+                $countryInfo['cities'] = array_unique($this->cities[$countryCode]);
             }
-        } else if ($referrerType == 'campaign') {
-            $result = Piwik::translate('Referrers_ColumnCampaign') . ' (' . $visit->getColumn('referrerName') . ')';
-        } else {
-            $result = $visit->getColumn('referrerName');
+            $this->profile['countries'][] = $countryInfo;
+        }
+    }
+
+    private function initVisitorProfile()
+    {
+        $this->profile['totalVisits'] = 0;
+        $this->profile['totalVisitDuration'] = 0;
+        $this->profile['totalActions'] = 0;
+        $this->profile['totalSearches'] = 0;
+        $this->profile['totalPageViewsWithTiming'] = 0;
+        $this->profile['totalGoalConversions'] = 0;
+        $this->profile['totalConversionsByGoal'] = array();
+        $this->profile['hasLatLong'] = false;
+
+        if ($this->isEcommerceEnabled()) {
+            $this->profile['totalEcommerceConversions'] = 0;
+            $this->profile['totalEcommerceRevenue'] = 0;
+            $this->profile['totalEcommerceItems'] = 0;
+            $this->profile['totalAbandonedCarts'] = 0;
+            $this->profile['totalAbandonedCartsRevenue'] = 0;
+            $this->profile['totalAbandonedCartsItems'] = 0;
+        }
+    }
+
+    private function handleAveragePageGenerationTime()
+    {
+        if ($this->profile['totalPageViewsWithTiming']) {
+            $this->profile['averagePageGenerationTime'] =
+                round($this->pageGenerationTimeTotal / $this->profile['totalPageViewsWithTiming'], $precision = 2);
+        }
+    }
+
+    private function handleIfPageGenerationTime($action)
+    {
+        if (isset($action['generationTime'])) {
+            $this->pageGenerationTimeTotal += $action['generationTime'];
+            ++$this->profile['totalPageViewsWithTiming'];
         }
+    }
+
+    /**
+     * @param DataTable $visits
+     * @param $visitorId
+     * @param $segment
+     */
+    private function handleAdjacentVisitorIds(DataTable $visits, $visitorId, $segment)
+    {
+        // get visitor IDs that are adjacent to this one in log_visit
+        // TODO: make sure order of visitor ids is not changed if a returning visitor visits while the user is
+        //       looking at the popup.
+        $rows = $visits->getRows();
+        $latestVisitTime = reset($rows)->getColumn('lastActionDateTime');
+
+        $model = new Model();
+        $this->profile['nextVisitorId'] = $model->queryAdjacentVisitorId($this->idSite, $visitorId, $latestVisitTime, $segment, $getNext = true);
+        $this->profile['previousVisitorId'] = $model->queryAdjacentVisitorId($this->idSite, $visitorId, $latestVisitTime, $segment, $getNext = false);
+    }
+
+    /**
+     * @param DataTable $visits
+     */
+    private function handleVisitsSummary(DataTable $visits)
+    {
+        $rows = $visits->getRows();
+        $this->profile['firstVisit'] = $this->getVisitorProfileVisitSummary(end($rows));
+        $this->profile['lastVisit'] = $this->getVisitorProfileVisitSummary(reset($rows));
+        $this->profile['visitsAggregated'] = count($rows);
+    }
+
+    /**
+     * @param DataTable $visits
+     * @return DataTable\Row
+     * @throws Exception
+     */
+    private function enrichVisitsWithFirstActionDatetime(DataTable $visits)
+    {
+        $timezone = Site::getTimezoneFor($this->idSite);
+        foreach ($visits->getRows() as $visit) {
+            $dateTimeVisitFirstAction = Date::factory($visit->getColumn('firstActionTimestamp'), $timezone);
+
+            $datePretty = $dateTimeVisitFirstAction->getLocalized(self::VISITOR_PROFILE_DATE_FORMAT);
+            $visit->setColumn('serverDatePrettyFirstAction', $datePretty);
 
-        return $result;
+            $dateTimePretty = $datePretty . ' ' . $visit->getColumn('serverTimePrettyFirstAction');
+            $visit->setColumn('serverDateTimePrettyFirstAction', $dateTimePretty);
+        }
     }
 
 } 
\ No newline at end of file
diff --git a/plugins/Live/templates/getVisitorProfilePopup.twig b/plugins/Live/templates/getVisitorProfilePopup.twig
index 0a25de6a39..afb9551321 100644
--- a/plugins/Live/templates/getVisitorProfilePopup.twig
+++ b/plugins/Live/templates/getVisitorProfilePopup.twig
@@ -61,7 +61,7 @@
                         </p>
                         {% endif %}
                         {% if visitorData.averagePageGenerationTime is defined %}
-                        <p title="{{ 'Live_CalculatedOverNPageViews'|translate(visitorData.totalPageViews) }}">
+                        <p title="{{ 'Live_CalculatedOverNPageViews'|translate(visitorData.totalPageViewsWithTiming) }}">
                             {{ 'Live_AveragePageGenerationTime'|translate('<strong>' ~ visitorData.averagePageGenerationTime ~ 's</strong>')|raw }}
                         </p>
                         {% endif %}
-- 
cgit v1.2.3


From 3006097403d7499849adffc5ec3d7d39d3c07cc0 Mon Sep 17 00:00:00 2001
From: mattab <matthieu.aubry@gmail.com>
Date: Fri, 5 Dec 2014 23:58:38 +1300
Subject: Fix one test

---
 ...ge_dateIsLastN_MetadataAndNormalAPI__Live.getVisitorProfile.xml | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tests/PHPUnit/System/expected/test_periodIsRange_dateIsLastN_MetadataAndNormalAPI__Live.getVisitorProfile.xml b/tests/PHPUnit/System/expected/test_periodIsRange_dateIsLastN_MetadataAndNormalAPI__Live.getVisitorProfile.xml
index 60e389c7ab..845a1a23a9 100644
--- a/tests/PHPUnit/System/expected/test_periodIsRange_dateIsLastN_MetadataAndNormalAPI__Live.getVisitorProfile.xml
+++ b/tests/PHPUnit/System/expected/test_periodIsRange_dateIsLastN_MetadataAndNormalAPI__Live.getVisitorProfile.xml
@@ -4,11 +4,12 @@
 	<totalVisitDuration>361</totalVisitDuration>
 	<totalActions>2</totalActions>
 	<totalSearches>0</totalSearches>
-	<totalPageViews>0</totalPageViews>
+	<totalPageViewsWithTiming>0</totalPageViewsWithTiming>
 	<totalGoalConversions>1</totalGoalConversions>
 	<totalConversionsByGoal>
 		<row idgoal="1">1</row>
 	</totalConversionsByGoal>
+	<hasLatLong>0</hasLatLong>
 	<searches>
 	</searches>
 	<continents>
@@ -42,6 +43,8 @@
 		<referralSummary>Direct Entry</referralSummary>
 	</lastVisit>
 	<visitsAggregated>2</visitsAggregated>
+	
+	
 	<lastVisits>
 		<row>
 			<idSite>1</idSite>
@@ -263,6 +266,4 @@
 		</row>
 	</lastVisits>
 	<userId>0</userId>
-	
-	
 </result>
\ No newline at end of file
-- 
cgit v1.2.3


From d84fbf324a8a4522ad96c4edc2f1328a9218b37a Mon Sep 17 00:00:00 2001
From: mattab <matthieu.aubry@gmail.com>
Date: Sat, 6 Dec 2014 00:00:45 +1300
Subject: Introduce new $limit parameter to the query builder, which will LIMIT
 all queries including inner ones - do not use when aggregating or it will
 sample

---
 core/DataAccess/LogQueryBuilder.php | 24 +++++++++++++++++-------
 core/Segment.php                    |  8 ++++++--
 2 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/core/DataAccess/LogQueryBuilder.php b/core/DataAccess/LogQueryBuilder.php
index 6e5b77561b..2bb71d4335 100644
--- a/core/DataAccess/LogQueryBuilder.php
+++ b/core/DataAccess/LogQueryBuilder.php
@@ -21,7 +21,7 @@ class LogQueryBuilder
         $this->segmentExpression = $segmentExpression;
     }
 
-    public function getSelectQueryString($select, $from, $where, $bind, $orderBy, $groupBy)
+    public function getSelectQueryString($select, $from, $where, $bind, $orderBy, $groupBy, $limit)
     {
         if (!is_array($from)) {
             $from = array($from);
@@ -39,9 +39,9 @@ class LogQueryBuilder
         $from = $joins['sql'];
 
         if ($joinWithSubSelect) {
-            $sql = $this->buildWrappedSelectQuery($select, $from, $where, $orderBy, $groupBy);
+            $sql = $this->buildWrappedSelectQuery($select, $from, $where, $orderBy, $groupBy, $limit);
         } else {
-            $sql = $this->buildSelectQuery($select, $from, $where, $orderBy, $groupBy);
+            $sql = $this->buildSelectQuery($select, $from, $where, $orderBy, $groupBy, $limit);
         }
         return array(
             'sql' => $sql,
@@ -166,10 +166,11 @@ class LogQueryBuilder
      * @param string $where
      * @param string $orderBy
      * @param string $groupBy
+     * @param string $limit
      * @throws Exception
      * @return string
      */
-    private function buildWrappedSelectQuery($select, $from, $where, $orderBy, $groupBy)
+    private function buildWrappedSelectQuery($select, $from, $where, $orderBy, $groupBy, $limit)
     {
         $matchTables = "(log_visit|log_conversion_item|log_conversion|log_action)";
         preg_match_all("/". $matchTables ."\.[a-z0-9_\*]+/", $select, $matches);
@@ -185,8 +186,9 @@ class LogQueryBuilder
         $innerWhere = $where;
         $innerGroupBy = "log_visit.idvisit";
         $innerOrderBy = "NULL";
+        $innerLimit = $limit;
 
-        $innerQuery = $this->buildSelectQuery($innerSelect, $innerFrom, $innerWhere, $innerOrderBy, $innerGroupBy);
+        $innerQuery = $this->buildSelectQuery($innerSelect, $innerFrom, $innerWhere, $innerOrderBy, $innerGroupBy, $innerLimit);
 
         $select = preg_replace('/'.$matchTables.'\./', 'log_inner.', $select);
         $from = "
@@ -196,7 +198,7 @@ class LogQueryBuilder
         $where = false;
         $orderBy = preg_replace('/'.$matchTables.'\./', 'log_inner.', $orderBy);
         $groupBy = preg_replace('/'.$matchTables.'\./', 'log_inner.', $groupBy);
-        $query = $this->buildSelectQuery($select, $from, $where, $orderBy, $groupBy);
+        $query = $this->buildSelectQuery($select, $from, $where, $orderBy, $groupBy, $limit);
         return $query;
     }
 
@@ -209,9 +211,10 @@ class LogQueryBuilder
      * @param string $where where clause
      * @param string $orderBy order by clause
      * @param string $groupBy group by clause
+     * @param string $limit limit by clause
      * @return string
      */
-    private function buildSelectQuery($select, $from, $where, $orderBy, $groupBy)
+    private function buildSelectQuery($select, $from, $where, $orderBy, $groupBy, $limit)
     {
         $sql = "
 			SELECT
@@ -237,6 +240,13 @@ class LogQueryBuilder
 				$orderBy";
         }
 
+        $limit = (int)$limit;
+        if ($limit >= 1) {
+            $sql .= "
+            LIMIT
+                $limit";
+        }
+
         return $sql;
     }
 
diff --git a/core/Segment.php b/core/Segment.php
index 91318a0197..e350e03afb 100644
--- a/core/Segment.php
+++ b/core/Segment.php
@@ -233,14 +233,18 @@ class Segment
      * @param array|string $bind (optional) Bind parameters, eg, `array($col1Value, $col2Value)`.
      * @param false|string $orderBy (optional) Order by clause, eg, `"t1.col1 ASC"`.
      * @param false|string $groupBy (optional) Group by clause, eg, `"t2.col2"`.
+     * @param int $limit Limit by clause
+     * @param int If set to value >= 1 then the Select query (and All inner queries) will be LIMIT'ed by this value.
+     *              Use only when you're not aggregating or it will sample the data.
      * @return string The entire select query.
      */
-    public function getSelectQuery($select, $from, $where = false, $bind = array(), $orderBy = false, $groupBy = false)
+    public function getSelectQuery($select, $from, $where = false, $bind = array(), $orderBy = false, $groupBy = false, $limit = 0)
     {
         $segmentExpression = $this->segmentExpression;
         $segmentQuery = new LogQueryBuilder($segmentExpression);
-        return $segmentQuery->getSelectQueryString($select, $from, $where, $bind, $orderBy, $groupBy);
+        return $segmentQuery->getSelectQueryString($select, $from, $where, $bind, $orderBy, $groupBy, $limit);
     }
+
     /**
      * Returns the segment string.
      *
-- 
cgit v1.2.3


From 09196f61430b50039adcbd7ad69938c12b171f62 Mon Sep 17 00:00:00 2001
From: mattab <matthieu.aubry@gmail.com>
Date: Sat, 6 Dec 2014 00:14:54 +1300
Subject: Added unit test for $limit parameter

---
 tests/PHPUnit/Integration/SegmentTest.php | 66 +++++++++++++++++++++++++------
 1 file changed, 54 insertions(+), 12 deletions(-)

diff --git a/tests/PHPUnit/Integration/SegmentTest.php b/tests/PHPUnit/Integration/SegmentTest.php
index 41fe39a392..e1439cddda 100644
--- a/tests/PHPUnit/Integration/SegmentTest.php
+++ b/tests/PHPUnit/Integration/SegmentTest.php
@@ -126,7 +126,7 @@ class SegmentTest extends IntegrationTestCase
         $this->assertEquals(32, strlen($segment->getHash()));
     }
 
-    public function testGetSelectQueryNoJoin()
+    public function test_getSelectQuery_whenNoJoin()
     {
         $select = '*';
         $from = 'log_visit';
@@ -153,7 +153,7 @@ class SegmentTest extends IntegrationTestCase
         $this->assertEquals($this->_filterWhitsSpaces($expected), $this->_filterWhitsSpaces($query));
     }
 
-    public function testGetSelectQueryJoinVisitOnAction()
+    public function test_getSelectQuery_whenJoinVisitOnAction()
     {
         $select = '*';
         $from = 'log_link_visit_action';
@@ -181,7 +181,7 @@ class SegmentTest extends IntegrationTestCase
         $this->assertEquals($this->_filterWhitsSpaces($expected), $this->_filterWhitsSpaces($query));
     }
 
-    public function testGetSelectQueryJoinActionOnVisit()
+    public function test_getSelectQuery_whenJoinActionOnVisit()
     {
         $select = 'sum(log_visit.visit_total_actions) as nb_actions, max(log_visit.visit_total_actions) as max_actions, sum(log_visit.visit_total_time) as sum_visit_length';
         $from = 'log_visit';
@@ -217,7 +217,7 @@ class SegmentTest extends IntegrationTestCase
         $this->assertEquals($this->_filterWhitsSpaces($expected), $this->_filterWhitsSpaces($query));
     }
 
-    public function testGetSelectQueryJoinConversionOnAction()
+    public function test_getSelectQuery_whenJoinConversionOnAction()
     {
         $select = '*';
         $from = 'log_link_visit_action';
@@ -245,7 +245,7 @@ class SegmentTest extends IntegrationTestCase
         $this->assertEquals($this->_filterWhitsSpaces($expected), $this->_filterWhitsSpaces($query));
     }
 
-    public function testGetSelectQueryJoinActionOnConversion()
+    public function test_getSelectQuery_whenJoinActionOnConversion()
     {
         $select = '*';
         $from = 'log_conversion';
@@ -273,7 +273,7 @@ class SegmentTest extends IntegrationTestCase
         $this->assertEquals($this->_filterWhitsSpaces($expected), $this->_filterWhitsSpaces($query));
     }
 
-    public function testGetSelectQueryJoinConversionOnVisit()
+    public function test_getSelectQuery_whenJoinConversionOnVisit()
     {
         $select = 'log_visit.*';
         $from = 'log_visit';
@@ -308,7 +308,7 @@ class SegmentTest extends IntegrationTestCase
         $this->assertEquals($this->_filterWhitsSpaces($expected), $this->_filterWhitsSpaces($query));
     }
 
-    public function testGetSelectQueryConversionOnly()
+    public function test_getSelectQuery_whenJoinConversionOnly()
     {
         $select = 'log_conversion.*';
         $from = 'log_conversion';
@@ -335,7 +335,7 @@ class SegmentTest extends IntegrationTestCase
         $this->assertEquals($this->_filterWhitsSpaces($expected), $this->_filterWhitsSpaces($query));
     }
 
-    public function testGetSelectQueryJoinVisitOnConversion()
+    public function test_getSelectQuery_whenJoinVisitOnConversion()
     {
         $select = '*';
         $from = 'log_conversion';
@@ -367,7 +367,7 @@ class SegmentTest extends IntegrationTestCase
      * visit is joined on action, then conversion is joined
      * make sure that conversion is joined on action not visit
      */
-    public function testGetSelectQueryJoinVisitAndConversionOnAction()
+    public function test_getSelectQuery_whenJoinVisitAndConversionOnAction()
     {
         $select = '*';
         $from = 'log_link_visit_action';
@@ -398,7 +398,7 @@ class SegmentTest extends IntegrationTestCase
      * join conversion on visit, then actions
      * make sure actions are joined before conversions
      */
-    public function testGetSelectQueryJoinConversionAndActionOnVisit()
+    public function test_getSelectQuery_whenJoinConversionAndActionOnVisit()
     {
         $select = 'log_visit.*';
         $from = 'log_visit';
@@ -433,7 +433,7 @@ class SegmentTest extends IntegrationTestCase
     }
 
     /**
-     * Dataprovider for testBogusSegmentThrowsException
+     * Dataprovider for test_bogusSegment_shouldThrowException
      */
     public function getBogusSegments()
     {
@@ -447,7 +447,7 @@ class SegmentTest extends IntegrationTestCase
     /**
      * @dataProvider getBogusSegments
      */
-    public function testBogusSegmentThrowsException($segment)
+    public function test_bogusSegment_shouldThrowException($segment)
     {
         try {
             new Segment($segment, $idSites = array());
@@ -456,4 +456,46 @@ class SegmentTest extends IntegrationTestCase
         }
         $this->fail('Expected exception not raised');
     }
+
+
+    public function test_getSelectQuery_whenLimit_innerQueryShouldHaveLimit()
+    {
+        $select = 'sum(log_visit.visit_total_time) as sum_visit_length';
+        $from = 'log_visit';
+        $where = 'log_visit.idvisit = ?';
+        $bind = array(1);
+
+        $segment = 'customVariablePageName1==Test';
+        $segment = new Segment($segment, $idSites = array());
+
+        $orderBy = false;
+        $groupBy = false;
+        $limit = 33;
+
+        $query = $segment->getSelectQuery($select, $from, $where, $bind, $orderBy, $groupBy, $limit);
+
+        $expected = array(
+            "sql"  => "
+                SELECT
+                    sum(log_visit.visit_total_time) as sum_visit_length
+                FROM
+                    (
+                SELECT
+                    log_visit.visit_total_time
+                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
+                    ( log_visit.idvisit = ? )
+                    AND
+                    ( log_link_visit_action.custom_var_k1 = ? )
+                GROUP BY log_visit.idvisit
+                ORDER BY NULL
+                LIMIT 33
+                    ) AS log_inner
+                LIMIT 33",
+            "bind" => array(1, 'Test'));
+
+        $this->assertEquals($this->_filterWhitsSpaces($expected), $this->_filterWhitsSpaces($query));
+    }
 }
-- 
cgit v1.2.3


From 04e3b93fea8ed2ccfe911ce56507e8c5c9091491 Mon Sep 17 00:00:00 2001
From: mattab <matthieu.aubry@gmail.com>
Date: Sat, 6 Dec 2014 00:17:25 +1300
Subject: Re-ordering $orderBy, $groupBy  --> $groupBy, $orderBy as it's proper
 order in SQL logic

---
 core/DataAccess/LogQueryBuilder.php | 19 ++++++++++---------
 core/Segment.php                    |  2 +-
 plugins/Actions/Archiver.php        | 18 +++++++++---------
 plugins/Contents/Archiver.php       |  6 +++---
 plugins/Events/Archiver.php         |  4 ++--
 5 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/core/DataAccess/LogQueryBuilder.php b/core/DataAccess/LogQueryBuilder.php
index 2bb71d4335..132480514b 100644
--- a/core/DataAccess/LogQueryBuilder.php
+++ b/core/DataAccess/LogQueryBuilder.php
@@ -21,7 +21,7 @@ class LogQueryBuilder
         $this->segmentExpression = $segmentExpression;
     }
 
-    public function getSelectQueryString($select, $from, $where, $bind, $orderBy, $groupBy, $limit)
+    public function getSelectQueryString($select, $from, $where, $bind, $groupBy, $orderBy, $limit)
     {
         if (!is_array($from)) {
             $from = array($from);
@@ -39,9 +39,9 @@ class LogQueryBuilder
         $from = $joins['sql'];
 
         if ($joinWithSubSelect) {
-            $sql = $this->buildWrappedSelectQuery($select, $from, $where, $orderBy, $groupBy, $limit);
+            $sql = $this->buildWrappedSelectQuery($select, $from, $where, $groupBy, $orderBy, $limit);
         } else {
-            $sql = $this->buildSelectQuery($select, $from, $where, $orderBy, $groupBy, $limit);
+            $sql = $this->buildSelectQuery($select, $from, $where, $groupBy, $orderBy, $limit);
         }
         return array(
             'sql' => $sql,
@@ -164,13 +164,13 @@ class LogQueryBuilder
      * @param string $select
      * @param string $from
      * @param string $where
-     * @param string $orderBy
      * @param string $groupBy
+     * @param string $orderBy
      * @param string $limit
      * @throws Exception
      * @return string
      */
-    private function buildWrappedSelectQuery($select, $from, $where, $orderBy, $groupBy, $limit)
+    private function buildWrappedSelectQuery($select, $from, $where, $groupBy, $orderBy, $limit)
     {
         $matchTables = "(log_visit|log_conversion_item|log_conversion|log_action)";
         preg_match_all("/". $matchTables ."\.[a-z0-9_\*]+/", $select, $matches);
@@ -188,7 +188,7 @@ class LogQueryBuilder
         $innerOrderBy = "NULL";
         $innerLimit = $limit;
 
-        $innerQuery = $this->buildSelectQuery($innerSelect, $innerFrom, $innerWhere, $innerOrderBy, $innerGroupBy, $innerLimit);
+        $innerQuery = $this->buildSelectQuery($innerSelect, $innerFrom, $innerWhere, $innerGroupBy, $innerOrderBy, $innerLimit);
 
         $select = preg_replace('/'.$matchTables.'\./', 'log_inner.', $select);
         $from = "
@@ -198,7 +198,7 @@ class LogQueryBuilder
         $where = false;
         $orderBy = preg_replace('/'.$matchTables.'\./', 'log_inner.', $orderBy);
         $groupBy = preg_replace('/'.$matchTables.'\./', 'log_inner.', $groupBy);
-        $query = $this->buildSelectQuery($select, $from, $where, $orderBy, $groupBy, $limit);
+        $query = $this->buildSelectQuery($select, $from, $where, $groupBy, $orderBy, $limit);
         return $query;
     }
 
@@ -209,12 +209,12 @@ class LogQueryBuilder
      * @param string $select fieldlist to be selected
      * @param string $from tablelist to select from
      * @param string $where where clause
-     * @param string $orderBy order by clause
      * @param string $groupBy group by clause
+     * @param string $orderBy order by clause
      * @param string $limit limit by clause
      * @return string
      */
-    private function buildSelectQuery($select, $from, $where, $orderBy, $groupBy, $limit)
+    private function buildSelectQuery($select, $from, $where, $groupBy, $orderBy, $limit)
     {
         $sql = "
 			SELECT
@@ -254,6 +254,7 @@ class LogQueryBuilder
      * @param $where
      * @param $segmentWhere
      * @return string
+     * @throws
      */
     protected function getWhereMatchBoth($where, $segmentWhere)
     {
diff --git a/core/Segment.php b/core/Segment.php
index e350e03afb..7fe4c061fd 100644
--- a/core/Segment.php
+++ b/core/Segment.php
@@ -242,7 +242,7 @@ class Segment
     {
         $segmentExpression = $this->segmentExpression;
         $segmentQuery = new LogQueryBuilder($segmentExpression);
-        return $segmentQuery->getSelectQueryString($select, $from, $where, $bind, $orderBy, $groupBy, $limit);
+        return $segmentQuery->getSelectQueryString($select, $from, $where, $bind, $groupBy, $orderBy, $limit);
     }
 
     /**
diff --git a/plugins/Actions/Archiver.php b/plugins/Actions/Archiver.php
index ab5fea8739..199438445e 100644
--- a/plugins/Actions/Archiver.php
+++ b/plugins/Actions/Archiver.php
@@ -244,9 +244,9 @@ class Archiver extends \Piwik\Plugin\Archiver
             $this->updateQuerySelectFromForSiteSearch($select, $from);
         }
 
-        $this->archiveDayQueryProcess($select, $from, $where, $orderBy, $groupBy, "idaction_name", $rankingQuery);
+        $this->archiveDayQueryProcess($select, $from, $where, $groupBy, $orderBy, "idaction_name", $rankingQuery);
 
-        $this->archiveDayQueryProcess($select, $from, $where, $orderBy, $groupBy, "idaction_url", $rankingQuery);
+        $this->archiveDayQueryProcess($select, $from, $where, $groupBy, $orderBy, "idaction_url", $rankingQuery);
     }
 
     protected function isSiteSearchEnabled()
@@ -254,7 +254,7 @@ class Archiver extends \Piwik\Plugin\Archiver
         return $this->isSiteSearchEnabled;
     }
 
-    protected function archiveDayQueryProcess($select, $from, $where, $orderBy, $groupBy, $sprintfField, RankingQuery $rankingQuery = null)
+    protected function archiveDayQueryProcess($select, $from, $where, $groupBy, $orderBy, $sprintfField, RankingQuery $rankingQuery = null)
     {
         $select = sprintf($select, $sprintfField);
 
@@ -321,9 +321,9 @@ class Archiver extends \Piwik\Plugin\Archiver
 
         $groupBy = "log_visit.%s, idaction";
 
-        $this->archiveDayQueryProcess($select, $from, $where, $orderBy, $groupBy, "visit_entry_idaction_url", $rankingQuery);
+        $this->archiveDayQueryProcess($select, $from, $where, $groupBy, $orderBy, "visit_entry_idaction_url", $rankingQuery);
 
-        $this->archiveDayQueryProcess($select, $from, $where, $orderBy, $groupBy, "visit_entry_idaction_name", $rankingQuery);
+        $this->archiveDayQueryProcess($select, $from, $where, $groupBy, $orderBy, "visit_entry_idaction_name", $rankingQuery);
     }
 
     /**
@@ -366,9 +366,9 @@ class Archiver extends \Piwik\Plugin\Archiver
 
         $groupBy = "log_visit.%s, idaction";
 
-        $this->archiveDayQueryProcess($select, $from, $where, $orderBy, $groupBy, "visit_exit_idaction_url", $rankingQuery);
+        $this->archiveDayQueryProcess($select, $from, $where, $groupBy, $orderBy, "visit_exit_idaction_url", $rankingQuery);
 
-        $this->archiveDayQueryProcess($select, $from, $where, $orderBy, $groupBy, "visit_exit_idaction_name", $rankingQuery);
+        $this->archiveDayQueryProcess($select, $from, $where, $groupBy, $orderBy, "visit_exit_idaction_name", $rankingQuery);
         return array($rankingQuery, $extraSelects, $from, $orderBy, $select, $where, $groupBy);
     }
 
@@ -412,9 +412,9 @@ class Archiver extends \Piwik\Plugin\Archiver
 
         $groupBy = "log_link_visit_action.%s, idaction";
 
-        $this->archiveDayQueryProcess($select, $from, $where, $orderBy, $groupBy, "idaction_url_ref", $rankingQuery);
+        $this->archiveDayQueryProcess($select, $from, $where, $groupBy, $orderBy, "idaction_url_ref", $rankingQuery);
 
-        $this->archiveDayQueryProcess($select, $from, $where, $orderBy, $groupBy, "idaction_name_ref", $rankingQuery);
+        $this->archiveDayQueryProcess($select, $from, $where, $groupBy, $orderBy, "idaction_name_ref", $rankingQuery);
     }
 
     /**
diff --git a/plugins/Contents/Archiver.php b/plugins/Contents/Archiver.php
index 45607bd7b7..6d3a2e05ce 100644
--- a/plugins/Contents/Archiver.php
+++ b/plugins/Contents/Archiver.php
@@ -117,7 +117,7 @@ class Archiver extends \Piwik\Plugin\Archiver
             $rankingQuery->addColumn(array(Metrics::INDEX_CONTENT_NB_IMPRESSIONS, Metrics::INDEX_NB_VISITS), 'sum');
         }
 
-        $resultSet = $this->archiveDayQueryProcess($select, $from, $where, $orderBy, $groupBy, $rankingQuery);
+        $resultSet = $this->archiveDayQueryProcess($select, $from, $where, $groupBy, $orderBy, $rankingQuery);
 
         while ($row = $resultSet->fetch()) {
             $this->aggregateImpressionRow($row);
@@ -174,14 +174,14 @@ class Archiver extends \Piwik\Plugin\Archiver
             $rankingQuery->addColumn(array(Metrics::INDEX_CONTENT_NB_INTERACTIONS), 'sum');
         }
 
-        $resultSet = $this->archiveDayQueryProcess($select, $from, $where, $orderBy, $groupBy, $rankingQuery);
+        $resultSet = $this->archiveDayQueryProcess($select, $from, $where, $groupBy, $orderBy, $rankingQuery);
 
         while ($row = $resultSet->fetch()) {
             $this->aggregateInteractionRow($row);
         }
     }
 
-    private function archiveDayQueryProcess($select, $from, $where, $orderBy, $groupBy, RankingQuery $rankingQuery)
+    private function archiveDayQueryProcess($select, $from, $where, $groupBy, $orderBy, RankingQuery $rankingQuery)
     {
         // get query with segmentation
         $query = $this->getLogAggregator()->generateQuery($select, $from, $where, $groupBy, $orderBy);
diff --git a/plugins/Events/Archiver.php b/plugins/Events/Archiver.php
index b396401de0..5a5160bdb2 100644
--- a/plugins/Events/Archiver.php
+++ b/plugins/Events/Archiver.php
@@ -179,10 +179,10 @@ class Archiver extends \Piwik\Plugin\Archiver
             $rankingQuery->addColumn(Metrics::INDEX_EVENT_MAX_EVENT_VALUE, 'max');
         }
 
-        $this->archiveDayQueryProcess($select, $from, $where, $orderBy, $groupBy, $rankingQuery);
+        $this->archiveDayQueryProcess($select, $from, $where, $groupBy, $orderBy, $rankingQuery);
     }
 
-    protected function archiveDayQueryProcess($select, $from, $where, $orderBy, $groupBy, RankingQuery $rankingQuery)
+    protected function archiveDayQueryProcess($select, $from, $where, $groupBy, $orderBy, RankingQuery $rankingQuery)
     {
         // get query with segmentation
         $query = $this->getLogAggregator()->generateQuery($select, $from, $where, $groupBy, $orderBy);
-- 
cgit v1.2.3


From 966fb421bcf0d8efe792797e138d6a7afce31ebe Mon Sep 17 00:00:00 2001
From: mattab <matthieu.aubry@gmail.com>
Date: Sat, 6 Dec 2014 00:23:20 +1300
Subject: Fix test

---
 tests/PHPUnit/Integration/SegmentTest.php | 32 +++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/tests/PHPUnit/Integration/SegmentTest.php b/tests/PHPUnit/Integration/SegmentTest.php
index e1439cddda..e0fd9cd3c4 100644
--- a/tests/PHPUnit/Integration/SegmentTest.php
+++ b/tests/PHPUnit/Integration/SegmentTest.php
@@ -36,11 +36,11 @@ class SegmentTest extends IntegrationTestCase
         parent::tearDown();
     }
 
-    protected function _filterWhitsSpaces($valueToFilter)
+    protected function _filterWhiteSpaces($valueToFilter)
     {
         if (is_array($valueToFilter)) {
             foreach ($valueToFilter as $key => $value) {
-                $valueToFilter[$key] = $this->_filterWhitsSpaces($value);
+                $valueToFilter[$key] = $this->_filterWhiteSpaces($value);
             }
             return $valueToFilter;
         } else {
@@ -117,11 +117,11 @@ class SegmentTest extends IntegrationTestCase
         $segment = new Segment($segment, $idSites = array());
         $sql = $segment->getSelectQuery($select, $from, false);
 
-        $this->assertEquals($this->_filterWhitsSpaces($expected), $this->_filterWhitsSpaces($sql));
+        $this->assertEquals($this->_filterWhiteSpaces($expected), $this->_filterWhiteSpaces($sql));
 
         // calling twice should give same results
         $sql = $segment->getSelectQuery($select, array($from));
-        $this->assertEquals($this->_filterWhitsSpaces($expected), $this->_filterWhitsSpaces($sql));
+        $this->assertEquals($this->_filterWhiteSpaces($expected), $this->_filterWhiteSpaces($sql));
 
         $this->assertEquals(32, strlen($segment->getHash()));
     }
@@ -150,7 +150,7 @@ class SegmentTest extends IntegrationTestCase
                     ( log_visit.custom_var_k1 = ? AND log_visit.visitor_returning = ? )",
             "bind" => array(1, 'Test', 0));
 
-        $this->assertEquals($this->_filterWhitsSpaces($expected), $this->_filterWhitsSpaces($query));
+        $this->assertEquals($this->_filterWhiteSpaces($expected), $this->_filterWhiteSpaces($query));
     }
 
     public function test_getSelectQuery_whenJoinVisitOnAction()
@@ -178,7 +178,7 @@ class SegmentTest extends IntegrationTestCase
                     ( log_link_visit_action.custom_var_k1 = ? AND log_visit.visitor_returning = ? )",
             "bind" => array(1, 'Test', 0));
 
-        $this->assertEquals($this->_filterWhitsSpaces($expected), $this->_filterWhitsSpaces($query));
+        $this->assertEquals($this->_filterWhiteSpaces($expected), $this->_filterWhiteSpaces($query));
     }
 
     public function test_getSelectQuery_whenJoinActionOnVisit()
@@ -214,7 +214,7 @@ class SegmentTest extends IntegrationTestCase
                     ) AS log_inner",
             "bind" => array(1, 'Test', 0));
 
-        $this->assertEquals($this->_filterWhitsSpaces($expected), $this->_filterWhitsSpaces($query));
+        $this->assertEquals($this->_filterWhiteSpaces($expected), $this->_filterWhiteSpaces($query));
     }
 
     public function test_getSelectQuery_whenJoinConversionOnAction()
@@ -242,7 +242,7 @@ class SegmentTest extends IntegrationTestCase
                     ( log_link_visit_action.custom_var_k1 = ? AND log_conversion.idgoal = ? AND log_link_visit_action.custom_var_k2 = ? )",
             "bind" => array(1, 'Test', 1, 'Test2'));
 
-        $this->assertEquals($this->_filterWhitsSpaces($expected), $this->_filterWhitsSpaces($query));
+        $this->assertEquals($this->_filterWhiteSpaces($expected), $this->_filterWhiteSpaces($query));
     }
 
     public function test_getSelectQuery_whenJoinActionOnConversion()
@@ -270,7 +270,7 @@ class SegmentTest extends IntegrationTestCase
                     ( ( log_conversion.idgoal IS NULL OR log_conversion.idgoal <> ? ) AND log_link_visit_action.custom_var_k1 = ? AND log_conversion.idgoal = ? )",
             "bind" => array(1, 2, 'Test', 1));
 
-        $this->assertEquals($this->_filterWhitsSpaces($expected), $this->_filterWhitsSpaces($query));
+        $this->assertEquals($this->_filterWhiteSpaces($expected), $this->_filterWhiteSpaces($query));
     }
 
     public function test_getSelectQuery_whenJoinConversionOnVisit()
@@ -305,7 +305,7 @@ class SegmentTest extends IntegrationTestCase
                     ) AS log_inner",
             "bind" => array(1, 1));
 
-        $this->assertEquals($this->_filterWhitsSpaces($expected), $this->_filterWhitsSpaces($query));
+        $this->assertEquals($this->_filterWhiteSpaces($expected), $this->_filterWhiteSpaces($query));
     }
 
     public function test_getSelectQuery_whenJoinConversionOnly()
@@ -332,7 +332,7 @@ class SegmentTest extends IntegrationTestCase
                     ( log_conversion.idgoal = ? )",
             "bind" => array(1, 1));
 
-        $this->assertEquals($this->_filterWhitsSpaces($expected), $this->_filterWhitsSpaces($query));
+        $this->assertEquals($this->_filterWhiteSpaces($expected), $this->_filterWhiteSpaces($query));
     }
 
     public function test_getSelectQuery_whenJoinVisitOnConversion()
@@ -360,7 +360,7 @@ class SegmentTest extends IntegrationTestCase
                     ( (log_conversion.idgoal = ? OR HOUR(log_visit.visit_last_action_time) = ? ))",
             "bind" => array(1, 1, 12));
 
-        $this->assertEquals($this->_filterWhitsSpaces($expected), $this->_filterWhitsSpaces($query));
+        $this->assertEquals($this->_filterWhiteSpaces($expected), $this->_filterWhiteSpaces($query));
     }
 
     /**
@@ -391,7 +391,7 @@ class SegmentTest extends IntegrationTestCase
                      HOUR(log_visit.visit_last_action_time) = ? AND log_conversion.idgoal = ? ",
             "bind" => array(12, 1));
 
-        $this->assertEquals($this->_filterWhitsSpaces($expected), $this->_filterWhitsSpaces($query));
+        $this->assertEquals($this->_filterWhiteSpaces($expected), $this->_filterWhiteSpaces($query));
     }
 
     /**
@@ -429,7 +429,7 @@ class SegmentTest extends IntegrationTestCase
                     ) AS log_inner",
             "bind" => array(1, 12, 'Test'));
 
-        $this->assertEquals($this->_filterWhitsSpaces($expected), $this->_filterWhitsSpaces($query));
+        $this->assertEquals($this->_filterWhiteSpaces($expected), $this->_filterWhiteSpaces($query));
     }
 
     /**
@@ -477,7 +477,7 @@ class SegmentTest extends IntegrationTestCase
         $expected = array(
             "sql"  => "
                 SELECT
-                    sum(log_visit.visit_total_time) as sum_visit_length
+                    sum(log_inner.visit_total_time) as sum_visit_length
                 FROM
                     (
                 SELECT
@@ -496,6 +496,6 @@ class SegmentTest extends IntegrationTestCase
                 LIMIT 33",
             "bind" => array(1, 'Test'));
 
-        $this->assertEquals($this->_filterWhitsSpaces($expected), $this->_filterWhitsSpaces($query));
+        $this->assertEquals($this->_filterWhiteSpaces($expected), $this->_filterWhiteSpaces($query));
     }
 }
-- 
cgit v1.2.3


From 9610d1b785cb05e06b94eb2c749e7a7f80dcc90b Mon Sep 17 00:00:00 2001
From: mattab <matthieu.aubry@gmail.com>
Date: Sat, 6 Dec 2014 01:09:12 +1300
Subject: Added integration test for the model, to test the SQL query generated

---
 plugins/Live/Model.php                     | 220 ++++++++++++++++-------------
 plugins/Live/tests/Integration/APITest.php | 152 --------------------
 plugins/Live/tests/System/APITest.php      | 152 ++++++++++++++++++++
 plugins/Live/tests/System/ModelTest.php    | 123 ++++++++++++++++
 tests/PHPUnit/Integration/SegmentTest.php  |  30 ++--
 5 files changed, 413 insertions(+), 264 deletions(-)
 delete mode 100644 plugins/Live/tests/Integration/APITest.php
 create mode 100644 plugins/Live/tests/System/APITest.php
 create mode 100644 plugins/Live/tests/System/ModelTest.php

diff --git a/plugins/Live/Model.php b/plugins/Live/Model.php
index 2353450572..2d438bff9b 100644
--- a/plugins/Live/Model.php
+++ b/plugins/Live/Model.php
@@ -36,105 +36,10 @@ class Model
      */
     public function queryLogVisits($idSite, $period, $date, $segment, $countVisitorsToFetch, $visitorId, $minTimestamp, $filterSortOrder)
     {
-        $where = $whereBind = array();
-
-        list($whereClause, $idSites) = $this->getIdSitesWhereClause($idSite);
-
-        $where[] = $whereClause;
-        $whereBind = $idSites;
-
-        if (strtolower($filterSortOrder) !== 'asc') {
-            $filterSortOrder = 'DESC';
-        }
-
-        $orderBy = "idsite, visit_last_action_time " . $filterSortOrder;
-        $orderByParent = "sub.visit_last_action_time " . $filterSortOrder;
-
-        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);
-        }
-
-        // If no other filter, only look at the last 24 hours of stats
-        if (empty($visitorId)
-            && empty($countVisitorsToFetch)
-            && empty($period)
-            && empty($date)
-        ) {
-            $period = 'day';
-            $date = 'yesterdaySameTime';
-        }
-
-        // SQL Filter with provided period
-        if (!empty($period) && !empty($date)) {
-            $currentSite = new Site($idSite);
-            $currentTimezone = $currentSite->getTimezone();
-
-            $dateString = $date;
-            if ($period == 'range') {
-                $processedPeriod = new Range('range', $date);
-                if ($parsedDate = Range::parseDateRange($date)) {
-                    $dateString = $parsedDate[2];
-                }
-            } else {
-                $processedDate = Date::factory($date);
-                if ($date == 'today'
-                    || $date == 'now'
-                    || $processedDate->toString() == Date::factory('now', $currentTimezone)->toString()
-                ) {
-                    $processedDate = $processedDate->subDay(1);
-                }
-                $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
-                && strpos($date, 'previous') === false
-                && 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;
-            }
-        }
+        list($sql, $bind) = $this->makeLogVisitsQueryString($idSite, $period, $date, $segment, $countVisitorsToFetch, $visitorId, $minTimestamp, $filterSortOrder);
 
-        if (count($where) > 0) {
-            $where = join("
-				AND ", $where);
-        } else {
-            $where = false;
-        }
-
-        $segment = new Segment($segment, $idSite);
-
-        // Subquery to use the indexes for ORDER BY
-        $select = "log_visit.*";
-        $from = "log_visit";
-        $groupBy = false;
-        $subQuery = $segment->getSelectQuery($select, $from, $where, $whereBind, $orderBy, $groupBy);
-
-        $sqlLimit = $countVisitorsToFetch >= 1 ? " LIMIT 0, " . (int)$countVisitorsToFetch : "";
-
-        // Group by idvisit so that a visitor converting 2 goals only appears once
-        $sql = "
-			SELECT sub.* FROM (
-				" . $subQuery['sql'] . "
-				$sqlLimit
-			) AS sub
-			GROUP BY sub.idvisit
-			ORDER BY $orderByParent
-		";
         try {
-            $data = Db::fetchAll($sql, $subQuery['bind']);
+            $data = Db::fetchAll($sql, $bind);
             return $data;
         } catch (Exception $e) {
             echo $e->getMessage();
@@ -265,4 +170,125 @@ class Model
         }
         return $visitorId;
     }
+
+    /**
+     * @param $idSite
+     * @param $period
+     * @param $date
+     * @param $segment
+     * @param $countVisitorsToFetch
+     * @param $visitorId
+     * @param $minTimestamp
+     * @param $filterSortOrder
+     * @return array
+     * @throws Exception
+     */
+    public function makeLogVisitsQueryString($idSite, $period, $date, $segment, $countVisitorsToFetch, $visitorId, $minTimestamp, $filterSortOrder)
+    {
+        $where = $whereBind = array();
+
+        list($whereClause, $idSites) = $this->getIdSitesWhereClause($idSite);
+
+        $where[] = $whereClause;
+        $whereBind = $idSites;
+
+        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);
+        }
+
+        // If no other filter, only look at the last 24 hours of stats
+        if (empty($visitorId)
+            && empty($countVisitorsToFetch)
+            && empty($period)
+            && empty($date)
+        ) {
+            $period = 'day';
+            $date = 'yesterdaySameTime';
+        }
+
+        // SQL Filter with provided period
+        if (!empty($period) && !empty($date)) {
+            $currentSite = $this->makeSite($idSite);
+            $currentTimezone = $currentSite->getTimezone();
+
+            $dateString = $date;
+            if ($period == 'range') {
+                $processedPeriod = new Range('range', $date);
+                if ($parsedDate = Range::parseDateRange($date)) {
+                    $dateString = $parsedDate[2];
+                }
+            } else {
+                $processedDate = Date::factory($date);
+                if ($date == 'today'
+                    || $date == 'now'
+                    || $processedDate->toString() == Date::factory('now', $currentTimezone)->toString()
+                ) {
+                    $processedDate = $processedDate->subDay(1);
+                }
+                $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
+                && strpos($date, 'previous') === false
+                && 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;
+            }
+        }
+
+        if (count($where) > 0) {
+            $where = join("
+				AND ", $where);
+        } else {
+            $where = false;
+        }
+
+        if (strtolower($filterSortOrder) !== 'asc') {
+            $filterSortOrder = 'DESC';
+        }
+        $segment = new Segment($segment, $idSite);
+
+        // Subquery to use the indexes for ORDER BY
+        $select = "log_visit.*";
+        $from = "log_visit";
+        $groupBy = false;
+        $limit = $countVisitorsToFetch >= 1 ? (int)$countVisitorsToFetch : 0;
+        $orderBy = "idsite, visit_last_action_time " . $filterSortOrder;
+        $orderByParent = "sub.visit_last_action_time " . $filterSortOrder;
+
+        $subQuery = $segment->getSelectQuery($select, $from, $where, $whereBind, $orderBy, $groupBy, $limit);
+
+        $bind = $subQuery['bind'];
+        // Group by idvisit so that a visitor converting 2 goals only appears once
+        $sql = "
+			SELECT sub.* FROM (
+				" . $subQuery['sql'] . "
+			) AS sub
+			GROUP BY sub.idvisit
+			ORDER BY $orderByParent
+		";
+        return array($sql, $bind);
+    }
+
+    /**
+     * @param $idSite
+     * @return Site
+     */
+    protected function makeSite($idSite)
+    {
+        return new Site($idSite);
+    }
 } 
\ No newline at end of file
diff --git a/plugins/Live/tests/Integration/APITest.php b/plugins/Live/tests/Integration/APITest.php
deleted file mode 100644
index 2c85088a19..0000000000
--- a/plugins/Live/tests/Integration/APITest.php
+++ /dev/null
@@ -1,152 +0,0 @@
-<?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\Live\tests\Integration;
-
-use Piwik\Date;
-use Piwik\Db;
-use Piwik\Plugins\Goals\API as GoalsApi;
-use Piwik\Plugins\Live\API;
-use Piwik\Access;
-use Piwik\Tests\Framework\Fixture;
-use Piwik\Tests\Framework\Mock\FakeAccess;
-use Piwik\Tests\Framework\TestCase\SystemTestCase;
-
-/**
- * @group Live
- * @group APITest
- * @group Plugins
- */
-class APITest extends SystemTestCase
-{
-    /**
-     * @var API
-     */
-    private $api;
-    private $idSite = 1;
-
-    public function setUp()
-    {
-        parent::setUp();
-
-        $this->api = API::getInstance();
-        $this->setSuperUser();
-        $this->createSite();
-    }
-
-    /**
-     * @expectedException \Exception
-     * @expectedExceptionMessage checkUserHasViewAccess Fake exception
-     */
-    public function test_GetCounters_ShouldFail_IfUserHasNoPermission()
-    {
-        $this->setAnonymous();
-        $this->api->getCounters($this->idSite, 5);
-    }
-
-    public function test_GetCounters_ShouldReturnZeroForAllCounters_IfThereAreNoVisitsEtc()
-    {
-        $counters = $this->api->getCounters($this->idSite, 5);
-
-        $this->assertEquals($this->buildCounter(0, 0, 0, 0), $counters);
-    }
-
-    public function test_GetCounters_ShouldOnlyReturnResultsOfLastMinutes()
-    {
-        $this->trackSomeVisits();
-
-        $counters = $this->api->getCounters($this->idSite, 5);
-        $this->assertEquals($this->buildCounter(19, 32, 16, 16), $counters);
-
-        $counters = $this->api->getCounters($this->idSite, 20);
-        $this->assertEquals($this->buildCounter(24, 60, 20, 40), $counters);
-
-        $counters = $this->api->getCounters($this->idSite, 0);
-        $this->assertEquals($this->buildCounter(0, 0, 0, 0), $counters);
-    }
-
-    private function trackSomeVisits()
-    {
-        $nowTimestamp = time();
-
-        // use local tracker so mock location provider can be used
-        $t = Fixture::getTracker($this->idSite, $nowTimestamp, $defaultInit = true, $useLocal = false);
-        $t->enableBulkTracking();
-
-        for ($i = 0; $i != 20; ++$i) {
-            $t->setForceNewVisit();
-            $t->setVisitorId( substr(md5($i * 1000), 0, $t::LENGTH_VISITOR_ID));
-
-            $factor = 10;
-            if ($i > 15) {
-                $factor = 30; // make sure first 15 visits are always within 5 minutes to prevent any random fails
-            }
-            $time = $nowTimestamp - ($i * $factor);
-
-            // first visit -> this one is > 5 minutes and should be ignored in one test
-            $date = Date::factory($time - 600);
-            $t->setForceVisitDateTime($date->getDatetime());
-            $t->setUrl("http://piwik.net/space/quest/iv");
-            $t->doTrackPageView("Space Quest XV");
-
-            $t->doTrackGoal(1); // this one is > 5 minutes and should be ignored in one test
-
-            // second visit
-            $date = Date::factory($time - 1);
-            $t->setForceVisitDateTime($date->getDatetime());
-            $t->setUrl("http://piwik.net/space/quest/iv");
-            $t->doTrackPageView("Space Quest XII");
-
-            if ($i % 6 == 0) {
-                $t->setForceNewVisit(); // to test visitors vs visits
-            }
-
-            // third visit
-            $date = Date::factory($time);
-            $t->setForceVisitDateTime($date->getDatetime());
-            $t->setUrl("http://piwik.net/grue/$i");
-            $t->doTrackPageView('It is pitch black...');
-
-            $t->doTrackGoal(2);
-        }
-
-        $t->doBulkTrack();
-    }
-
-    private function buildCounter($visits, $actions, $visitors, $visitsConverted)
-    {
-        return array(array(
-            'visits'   => $visits,
-            'actions'  => $actions,
-            'visitors' => $visitors,
-            'visitsConverted' => $visitsConverted,
-        ));
-    }
-
-    private function createSite()
-    {
-        Fixture::createWebsite('2013-01-23 01:23:45');
-        GoalsApi::getInstance()->addGoal(1, 'MyName', 'manually', '', 'contains');
-        GoalsApi::getInstance()->addGoal(1, 'MyGoal', 'manually', '', 'contains');
-    }
-
-    private function setSuperUser()
-    {
-        $pseudoMockAccess = new FakeAccess();
-        FakeAccess::$superUser = true;
-        Access::setSingletonInstance($pseudoMockAccess);
-    }
-
-    private function setAnonymous()
-    {
-        $pseudoMockAccess = new FakeAccess();
-        FakeAccess::$superUser = false;
-        Access::setSingletonInstance($pseudoMockAccess);
-    }
-
-}
diff --git a/plugins/Live/tests/System/APITest.php b/plugins/Live/tests/System/APITest.php
new file mode 100644
index 0000000000..2c85088a19
--- /dev/null
+++ b/plugins/Live/tests/System/APITest.php
@@ -0,0 +1,152 @@
+<?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\Live\tests\Integration;
+
+use Piwik\Date;
+use Piwik\Db;
+use Piwik\Plugins\Goals\API as GoalsApi;
+use Piwik\Plugins\Live\API;
+use Piwik\Access;
+use Piwik\Tests\Framework\Fixture;
+use Piwik\Tests\Framework\Mock\FakeAccess;
+use Piwik\Tests\Framework\TestCase\SystemTestCase;
+
+/**
+ * @group Live
+ * @group APITest
+ * @group Plugins
+ */
+class APITest extends SystemTestCase
+{
+    /**
+     * @var API
+     */
+    private $api;
+    private $idSite = 1;
+
+    public function setUp()
+    {
+        parent::setUp();
+
+        $this->api = API::getInstance();
+        $this->setSuperUser();
+        $this->createSite();
+    }
+
+    /**
+     * @expectedException \Exception
+     * @expectedExceptionMessage checkUserHasViewAccess Fake exception
+     */
+    public function test_GetCounters_ShouldFail_IfUserHasNoPermission()
+    {
+        $this->setAnonymous();
+        $this->api->getCounters($this->idSite, 5);
+    }
+
+    public function test_GetCounters_ShouldReturnZeroForAllCounters_IfThereAreNoVisitsEtc()
+    {
+        $counters = $this->api->getCounters($this->idSite, 5);
+
+        $this->assertEquals($this->buildCounter(0, 0, 0, 0), $counters);
+    }
+
+    public function test_GetCounters_ShouldOnlyReturnResultsOfLastMinutes()
+    {
+        $this->trackSomeVisits();
+
+        $counters = $this->api->getCounters($this->idSite, 5);
+        $this->assertEquals($this->buildCounter(19, 32, 16, 16), $counters);
+
+        $counters = $this->api->getCounters($this->idSite, 20);
+        $this->assertEquals($this->buildCounter(24, 60, 20, 40), $counters);
+
+        $counters = $this->api->getCounters($this->idSite, 0);
+        $this->assertEquals($this->buildCounter(0, 0, 0, 0), $counters);
+    }
+
+    private function trackSomeVisits()
+    {
+        $nowTimestamp = time();
+
+        // use local tracker so mock location provider can be used
+        $t = Fixture::getTracker($this->idSite, $nowTimestamp, $defaultInit = true, $useLocal = false);
+        $t->enableBulkTracking();
+
+        for ($i = 0; $i != 20; ++$i) {
+            $t->setForceNewVisit();
+            $t->setVisitorId( substr(md5($i * 1000), 0, $t::LENGTH_VISITOR_ID));
+
+            $factor = 10;
+            if ($i > 15) {
+                $factor = 30; // make sure first 15 visits are always within 5 minutes to prevent any random fails
+            }
+            $time = $nowTimestamp - ($i * $factor);
+
+            // first visit -> this one is > 5 minutes and should be ignored in one test
+            $date = Date::factory($time - 600);
+            $t->setForceVisitDateTime($date->getDatetime());
+            $t->setUrl("http://piwik.net/space/quest/iv");
+            $t->doTrackPageView("Space Quest XV");
+
+            $t->doTrackGoal(1); // this one is > 5 minutes and should be ignored in one test
+
+            // second visit
+            $date = Date::factory($time - 1);
+            $t->setForceVisitDateTime($date->getDatetime());
+            $t->setUrl("http://piwik.net/space/quest/iv");
+            $t->doTrackPageView("Space Quest XII");
+
+            if ($i % 6 == 0) {
+                $t->setForceNewVisit(); // to test visitors vs visits
+            }
+
+            // third visit
+            $date = Date::factory($time);
+            $t->setForceVisitDateTime($date->getDatetime());
+            $t->setUrl("http://piwik.net/grue/$i");
+            $t->doTrackPageView('It is pitch black...');
+
+            $t->doTrackGoal(2);
+        }
+
+        $t->doBulkTrack();
+    }
+
+    private function buildCounter($visits, $actions, $visitors, $visitsConverted)
+    {
+        return array(array(
+            'visits'   => $visits,
+            'actions'  => $actions,
+            'visitors' => $visitors,
+            'visitsConverted' => $visitsConverted,
+        ));
+    }
+
+    private function createSite()
+    {
+        Fixture::createWebsite('2013-01-23 01:23:45');
+        GoalsApi::getInstance()->addGoal(1, 'MyName', 'manually', '', 'contains');
+        GoalsApi::getInstance()->addGoal(1, 'MyGoal', 'manually', '', 'contains');
+    }
+
+    private function setSuperUser()
+    {
+        $pseudoMockAccess = new FakeAccess();
+        FakeAccess::$superUser = true;
+        Access::setSingletonInstance($pseudoMockAccess);
+    }
+
+    private function setAnonymous()
+    {
+        $pseudoMockAccess = new FakeAccess();
+        FakeAccess::$superUser = false;
+        Access::setSingletonInstance($pseudoMockAccess);
+    }
+
+}
diff --git a/plugins/Live/tests/System/ModelTest.php b/plugins/Live/tests/System/ModelTest.php
new file mode 100644
index 0000000000..d5dea34767
--- /dev/null
+++ b/plugins/Live/tests/System/ModelTest.php
@@ -0,0 +1,123 @@
+<?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\Live\tests\Integration;
+
+use Piwik\Access;
+use Piwik\Common;
+use Piwik\Plugins\Live\Model;
+use Piwik\Tests\Framework\Fixture;
+use Piwik\Tests\Framework\Mock\FakeAccess;
+use Piwik\Tests\Framework\TestCase\SystemTestCase;
+use Piwik\Tests\Integration\SegmentTest;
+
+/**
+ * @group LiveModelTest
+ * @group Plugins
+ */
+class ModelTest extends SystemTestCase
+{
+    function setUp()
+    {
+        $this->setSuperUser();
+        Fixture::createWebsite('2010-01-01');
+    }
+
+    public function test_makeLogVisitsQueryString()
+    {
+        $model = new Model();
+        list($sql, $bind) = $model->makeLogVisitsQueryString(
+                $idSite = 1,
+                $period = 'month',
+                $date = '2010-01-01',
+                $segment = false,
+                $countVisitorsToFetch = 100,
+                $visitorId = false,
+                $minTimestamp = false,
+                $filterSortOrder = false
+        );
+        $expectedSql = ' SELECT sub.* FROM
+                (
+                    SELECT log_visit.*
+                    FROM piwiktests_log_visit AS log_visit
+                    WHERE log_visit.idsite in (?)
+                      AND log_visit.visit_last_action_time >= ?
+                      AND log_visit.visit_last_action_time <= ?
+                    ORDER BY idsite, visit_last_action_time DESC
+                    LIMIT 100
+                 ) AS sub
+                 GROUP BY sub.idvisit
+                 ORDER BY sub.visit_last_action_time DESC
+        ';
+        $expectedBind = array(
+            '1',
+            '2010-01-01 00:00:00',
+            '2010-02-01 00:00:00',
+        );
+        $this->assertEquals(SegmentTest::removeExtraWhiteSpaces($expectedSql), SegmentTest::removeExtraWhiteSpaces($sql));
+        $this->assertEquals(SegmentTest::removeExtraWhiteSpaces($expectedBind), SegmentTest::removeExtraWhiteSpaces($bind));
+    }
+
+
+    public function test_makeLogVisitsQueryString_whenSegment()
+    {
+        $model = new Model();
+        list($sql, $bind) = $model->makeLogVisitsQueryString(
+            $idSite = 1,
+            $period = 'month',
+            $date = '2010-01-01',
+            $segment = 'customVariablePageName1==Test',
+            $countVisitorsToFetch = 100,
+            $visitorId = 'abc',
+            $minTimestamp = false,
+            $filterSortOrder = false
+        );
+        $expectedSql = ' SELECT sub.* FROM
+                (
+
+                    SELECT log_inner.*
+                    FROM (
+                        SELECT log_visit.*
+                        FROM piwiktests_log_visit AS log_visit
+                          LEFT JOIN piwiktests_log_link_visit_action AS log_link_visit_action
+                          ON log_link_visit_action.idvisit = log_visit.idvisit
+                        WHERE ( log_visit.idsite in (?)
+                          AND log_visit.idvisitor = ?
+                          AND log_visit.visit_last_action_time >= ?
+                          AND log_visit.visit_last_action_time <= ? )
+                          AND ( log_link_visit_action.custom_var_k1 = ? )
+                        GROUP BY log_visit.idvisit
+                        ORDER BY NULL
+                        LIMIT 100
+                        ) AS log_inner
+                    ORDER BY idsite, visit_last_action_time DESC
+                    LIMIT 100
+                 ) AS sub
+                 GROUP BY sub.idvisit
+                 ORDER BY sub.visit_last_action_time DESC
+        ';
+        $expectedBind = array(
+            '1',
+            Common::hex2bin('abc'),
+            '2010-01-01 00:00:00',
+            '2010-02-01 00:00:00',
+            'Test',
+        );
+        $this->assertEquals(SegmentTest::removeExtraWhiteSpaces($expectedSql), SegmentTest::removeExtraWhiteSpaces($sql));
+        $this->assertEquals(SegmentTest::removeExtraWhiteSpaces($expectedBind), SegmentTest::removeExtraWhiteSpaces($bind));
+    }
+
+
+    protected function setSuperUser()
+    {
+        $pseudoMockAccess = new FakeAccess();
+        FakeAccess::$superUser = true;
+        Access::setSingletonInstance($pseudoMockAccess);
+    }
+
+}
\ No newline at end of file
diff --git a/tests/PHPUnit/Integration/SegmentTest.php b/tests/PHPUnit/Integration/SegmentTest.php
index e0fd9cd3c4..864420a161 100644
--- a/tests/PHPUnit/Integration/SegmentTest.php
+++ b/tests/PHPUnit/Integration/SegmentTest.php
@@ -36,11 +36,11 @@ class SegmentTest extends IntegrationTestCase
         parent::tearDown();
     }
 
-    protected function _filterWhiteSpaces($valueToFilter)
+    static public function removeExtraWhiteSpaces($valueToFilter)
     {
         if (is_array($valueToFilter)) {
             foreach ($valueToFilter as $key => $value) {
-                $valueToFilter[$key] = $this->_filterWhiteSpaces($value);
+                $valueToFilter[$key] = self::removeExtraWhiteSpaces($value);
             }
             return $valueToFilter;
         } else {
@@ -117,11 +117,11 @@ class SegmentTest extends IntegrationTestCase
         $segment = new Segment($segment, $idSites = array());
         $sql = $segment->getSelectQuery($select, $from, false);
 
-        $this->assertEquals($this->_filterWhiteSpaces($expected), $this->_filterWhiteSpaces($sql));
+        $this->assertEquals($this->removeExtraWhiteSpaces($expected), $this->removeExtraWhiteSpaces($sql));
 
         // calling twice should give same results
         $sql = $segment->getSelectQuery($select, array($from));
-        $this->assertEquals($this->_filterWhiteSpaces($expected), $this->_filterWhiteSpaces($sql));
+        $this->assertEquals($this->removeExtraWhiteSpaces($expected), $this->removeExtraWhiteSpaces($sql));
 
         $this->assertEquals(32, strlen($segment->getHash()));
     }
@@ -150,7 +150,7 @@ class SegmentTest extends IntegrationTestCase
                     ( log_visit.custom_var_k1 = ? AND log_visit.visitor_returning = ? )",
             "bind" => array(1, 'Test', 0));
 
-        $this->assertEquals($this->_filterWhiteSpaces($expected), $this->_filterWhiteSpaces($query));
+        $this->assertEquals($this->removeExtraWhiteSpaces($expected), $this->removeExtraWhiteSpaces($query));
     }
 
     public function test_getSelectQuery_whenJoinVisitOnAction()
@@ -178,7 +178,7 @@ class SegmentTest extends IntegrationTestCase
                     ( log_link_visit_action.custom_var_k1 = ? AND log_visit.visitor_returning = ? )",
             "bind" => array(1, 'Test', 0));
 
-        $this->assertEquals($this->_filterWhiteSpaces($expected), $this->_filterWhiteSpaces($query));
+        $this->assertEquals($this->removeExtraWhiteSpaces($expected), $this->removeExtraWhiteSpaces($query));
     }
 
     public function test_getSelectQuery_whenJoinActionOnVisit()
@@ -214,7 +214,7 @@ class SegmentTest extends IntegrationTestCase
                     ) AS log_inner",
             "bind" => array(1, 'Test', 0));
 
-        $this->assertEquals($this->_filterWhiteSpaces($expected), $this->_filterWhiteSpaces($query));
+        $this->assertEquals($this->removeExtraWhiteSpaces($expected), $this->removeExtraWhiteSpaces($query));
     }
 
     public function test_getSelectQuery_whenJoinConversionOnAction()
@@ -242,7 +242,7 @@ class SegmentTest extends IntegrationTestCase
                     ( log_link_visit_action.custom_var_k1 = ? AND log_conversion.idgoal = ? AND log_link_visit_action.custom_var_k2 = ? )",
             "bind" => array(1, 'Test', 1, 'Test2'));
 
-        $this->assertEquals($this->_filterWhiteSpaces($expected), $this->_filterWhiteSpaces($query));
+        $this->assertEquals($this->removeExtraWhiteSpaces($expected), $this->removeExtraWhiteSpaces($query));
     }
 
     public function test_getSelectQuery_whenJoinActionOnConversion()
@@ -270,7 +270,7 @@ class SegmentTest extends IntegrationTestCase
                     ( ( log_conversion.idgoal IS NULL OR log_conversion.idgoal <> ? ) AND log_link_visit_action.custom_var_k1 = ? AND log_conversion.idgoal = ? )",
             "bind" => array(1, 2, 'Test', 1));
 
-        $this->assertEquals($this->_filterWhiteSpaces($expected), $this->_filterWhiteSpaces($query));
+        $this->assertEquals($this->removeExtraWhiteSpaces($expected), $this->removeExtraWhiteSpaces($query));
     }
 
     public function test_getSelectQuery_whenJoinConversionOnVisit()
@@ -305,7 +305,7 @@ class SegmentTest extends IntegrationTestCase
                     ) AS log_inner",
             "bind" => array(1, 1));
 
-        $this->assertEquals($this->_filterWhiteSpaces($expected), $this->_filterWhiteSpaces($query));
+        $this->assertEquals($this->removeExtraWhiteSpaces($expected), $this->removeExtraWhiteSpaces($query));
     }
 
     public function test_getSelectQuery_whenJoinConversionOnly()
@@ -332,7 +332,7 @@ class SegmentTest extends IntegrationTestCase
                     ( log_conversion.idgoal = ? )",
             "bind" => array(1, 1));
 
-        $this->assertEquals($this->_filterWhiteSpaces($expected), $this->_filterWhiteSpaces($query));
+        $this->assertEquals($this->removeExtraWhiteSpaces($expected), $this->removeExtraWhiteSpaces($query));
     }
 
     public function test_getSelectQuery_whenJoinVisitOnConversion()
@@ -360,7 +360,7 @@ class SegmentTest extends IntegrationTestCase
                     ( (log_conversion.idgoal = ? OR HOUR(log_visit.visit_last_action_time) = ? ))",
             "bind" => array(1, 1, 12));
 
-        $this->assertEquals($this->_filterWhiteSpaces($expected), $this->_filterWhiteSpaces($query));
+        $this->assertEquals($this->removeExtraWhiteSpaces($expected), $this->removeExtraWhiteSpaces($query));
     }
 
     /**
@@ -391,7 +391,7 @@ class SegmentTest extends IntegrationTestCase
                      HOUR(log_visit.visit_last_action_time) = ? AND log_conversion.idgoal = ? ",
             "bind" => array(12, 1));
 
-        $this->assertEquals($this->_filterWhiteSpaces($expected), $this->_filterWhiteSpaces($query));
+        $this->assertEquals($this->removeExtraWhiteSpaces($expected), $this->removeExtraWhiteSpaces($query));
     }
 
     /**
@@ -429,7 +429,7 @@ class SegmentTest extends IntegrationTestCase
                     ) AS log_inner",
             "bind" => array(1, 12, 'Test'));
 
-        $this->assertEquals($this->_filterWhiteSpaces($expected), $this->_filterWhiteSpaces($query));
+        $this->assertEquals($this->removeExtraWhiteSpaces($expected), $this->removeExtraWhiteSpaces($query));
     }
 
     /**
@@ -496,6 +496,6 @@ class SegmentTest extends IntegrationTestCase
                 LIMIT 33",
             "bind" => array(1, 'Test'));
 
-        $this->assertEquals($this->_filterWhiteSpaces($expected), $this->_filterWhiteSpaces($query));
+        $this->assertEquals($this->removeExtraWhiteSpaces($expected), $this->removeExtraWhiteSpaces($query));
     }
 }
-- 
cgit v1.2.3


From 269ffeb4b71926fa4973aa0075943dcc2426bd08 Mon Sep 17 00:00:00 2001
From: mattab <matthieu.aubry@gmail.com>
Date: Sat, 6 Dec 2014 01:12:57 +1300
Subject: ORDER BY the inner query so the order is deterministic + It uses the
 INDEX

---
 core/DataAccess/LogQueryBuilder.php     | 7 ++++++-
 plugins/Live/tests/System/ModelTest.php | 2 +-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/core/DataAccess/LogQueryBuilder.php b/core/DataAccess/LogQueryBuilder.php
index 132480514b..b062b91b3f 100644
--- a/core/DataAccess/LogQueryBuilder.php
+++ b/core/DataAccess/LogQueryBuilder.php
@@ -185,9 +185,14 @@ class LogQueryBuilder
         $innerFrom = $from;
         $innerWhere = $where;
         $innerGroupBy = "log_visit.idvisit";
-        $innerOrderBy = "NULL";
         $innerLimit = $limit;
 
+        $innerOrderBy = "NULL";
+        // Only when there is a LIMIT then we can apply to the inner query the same ORDER BY as the parent query
+        if($innerLimit) {
+            $innerOrderBy = $orderBy;
+        }
+
         $innerQuery = $this->buildSelectQuery($innerSelect, $innerFrom, $innerWhere, $innerGroupBy, $innerOrderBy, $innerLimit);
 
         $select = preg_replace('/'.$matchTables.'\./', 'log_inner.', $select);
diff --git a/plugins/Live/tests/System/ModelTest.php b/plugins/Live/tests/System/ModelTest.php
index d5dea34767..90196e7309 100644
--- a/plugins/Live/tests/System/ModelTest.php
+++ b/plugins/Live/tests/System/ModelTest.php
@@ -92,7 +92,7 @@ class ModelTest extends SystemTestCase
                           AND log_visit.visit_last_action_time <= ? )
                           AND ( log_link_visit_action.custom_var_k1 = ? )
                         GROUP BY log_visit.idvisit
-                        ORDER BY NULL
+                        ORDER BY idsite, visit_last_action_time DESC
                         LIMIT 100
                         ) AS log_inner
                     ORDER BY idsite, visit_last_action_time DESC
-- 
cgit v1.2.3


From dac71ca1347dbde11618e99b2dfe589d3d1812b8 Mon Sep 17 00:00:00 2001
From: mattab <matthieu.aubry@gmail.com>
Date: Sat, 6 Dec 2014 01:19:33 +1300
Subject: Only set order by if it's set, otherwise we ORDER BY NULL

---
 core/DataAccess/LogQueryBuilder.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/core/DataAccess/LogQueryBuilder.php b/core/DataAccess/LogQueryBuilder.php
index b062b91b3f..ac55ccf454 100644
--- a/core/DataAccess/LogQueryBuilder.php
+++ b/core/DataAccess/LogQueryBuilder.php
@@ -189,7 +189,7 @@ class LogQueryBuilder
 
         $innerOrderBy = "NULL";
         // Only when there is a LIMIT then we can apply to the inner query the same ORDER BY as the parent query
-        if($innerLimit) {
+        if($innerLimit && $orderBy) {
             $innerOrderBy = $orderBy;
         }
 
-- 
cgit v1.2.3


From a37254a976502a342a863ece8b285b96faff9494 Mon Sep 17 00:00:00 2001
From: mattab <matthieu.aubry@gmail.com>
Date: Sat, 6 Dec 2014 01:53:27 +1300
Subject: The ultimate prize and final step to close the issue: do not group by
 the whole set of matched rows, way too slow on huge datasets!

---
 core/DataAccess/LogQueryBuilder.php       | 10 +++++++---
 plugins/Live/tests/System/ModelTest.php   |  1 -
 tests/PHPUnit/Integration/SegmentTest.php |  3 +--
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/core/DataAccess/LogQueryBuilder.php b/core/DataAccess/LogQueryBuilder.php
index ac55ccf454..a5b2fcf986 100644
--- a/core/DataAccess/LogQueryBuilder.php
+++ b/core/DataAccess/LogQueryBuilder.php
@@ -184,14 +184,18 @@ class LogQueryBuilder
         $innerSelect = implode(", \n", $neededFields);
         $innerFrom = $from;
         $innerWhere = $where;
-        $innerGroupBy = "log_visit.idvisit";
-        $innerLimit = $limit;
 
+        $innerLimit = $limit;
+        $innerGroupBy = "log_visit.idvisit";
         $innerOrderBy = "NULL";
-        // Only when there is a LIMIT then we can apply to the inner query the same ORDER BY as the parent query
         if($innerLimit && $orderBy) {
+            // only When LIMITing we can apply to the inner query the same ORDER BY as the parent query
             $innerOrderBy = $orderBy;
         }
+        if($innerLimit) {
+            // When LIMITing, no need to GROUP BY (GROUPing by is done before the LIMIT which is super slow when large amount of rows is matched)
+            $innerGroupBy = false;
+        }
 
         $innerQuery = $this->buildSelectQuery($innerSelect, $innerFrom, $innerWhere, $innerGroupBy, $innerOrderBy, $innerLimit);
 
diff --git a/plugins/Live/tests/System/ModelTest.php b/plugins/Live/tests/System/ModelTest.php
index 90196e7309..268a7861d4 100644
--- a/plugins/Live/tests/System/ModelTest.php
+++ b/plugins/Live/tests/System/ModelTest.php
@@ -91,7 +91,6 @@ class ModelTest extends SystemTestCase
                           AND log_visit.visit_last_action_time >= ?
                           AND log_visit.visit_last_action_time <= ? )
                           AND ( log_link_visit_action.custom_var_k1 = ? )
-                        GROUP BY log_visit.idvisit
                         ORDER BY idsite, visit_last_action_time DESC
                         LIMIT 100
                         ) AS log_inner
diff --git a/tests/PHPUnit/Integration/SegmentTest.php b/tests/PHPUnit/Integration/SegmentTest.php
index 864420a161..19108b1312 100644
--- a/tests/PHPUnit/Integration/SegmentTest.php
+++ b/tests/PHPUnit/Integration/SegmentTest.php
@@ -458,7 +458,7 @@ class SegmentTest extends IntegrationTestCase
     }
 
 
-    public function test_getSelectQuery_whenLimit_innerQueryShouldHaveLimit()
+    public function test_getSelectQuery_whenLimit_innerQueryShouldHaveLimitAndNoGroupBy()
     {
         $select = 'sum(log_visit.visit_total_time) as sum_visit_length';
         $from = 'log_visit';
@@ -489,7 +489,6 @@ class SegmentTest extends IntegrationTestCase
                     ( log_visit.idvisit = ? )
                     AND
                     ( log_link_visit_action.custom_var_k1 = ? )
-                GROUP BY log_visit.idvisit
                 ORDER BY NULL
                 LIMIT 33
                     ) AS log_inner
-- 
cgit v1.2.3


From 020dd94590ca601dd5db7de2e8785284b27546fc Mon Sep 17 00:00:00 2001
From: mattab <matthieu.aubry@gmail.com>
Date: Sat, 6 Dec 2014 02:08:26 +1300
Subject: Adding little test case for the culprit pageUrl!=

---
 tests/PHPUnit/Integration/SegmentTest.php | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tests/PHPUnit/Integration/SegmentTest.php b/tests/PHPUnit/Integration/SegmentTest.php
index 19108b1312..91be56af12 100644
--- a/tests/PHPUnit/Integration/SegmentTest.php
+++ b/tests/PHPUnit/Integration/SegmentTest.php
@@ -398,14 +398,14 @@ class SegmentTest extends IntegrationTestCase
      * join conversion on visit, then actions
      * make sure actions are joined before conversions
      */
-    public function test_getSelectQuery_whenJoinConversionAndActionOnVisit()
+    public function test_getSelectQuery_whenJoinConversionAndActionOnVisit_andPageUrlSet()
     {
         $select = 'log_visit.*';
         $from = 'log_visit';
         $where = false;
         $bind = array();
 
-        $segment = 'visitConvertedGoalId==1;visitServerHour==12;customVariablePageName1==Test';
+        $segment = 'visitConvertedGoalId==1;visitServerHour==12;customVariablePageName1==Test;pageUrl!=';
         $segment = new Segment($segment, $idSites = array());
 
         $query = $segment->getSelectQuery($select, $from, $where, $bind);
@@ -424,6 +424,11 @@ class SegmentTest extends IntegrationTestCase
                     LEFT JOIN " . Common::prefixTable('log_conversion') . " AS log_conversion ON log_conversion.idlink_va = log_link_visit_action.idlink_va AND log_conversion.idsite = log_link_visit_action.idsite
                 WHERE
                      log_conversion.idgoal = ? AND HOUR(log_visit.visit_last_action_time) = ? AND log_link_visit_action.custom_var_k1 = ?
+                      AND (
+                            log_link_visit_action.idaction_url IS NOT NULL
+                            AND (log_link_visit_action.idaction_url <> ''
+                                OR log_link_visit_action.idaction_url = 0)
+                            )
                 GROUP BY log_visit.idvisit
                 ORDER BY NULL
                     ) AS log_inner",
-- 
cgit v1.2.3


From 814f3790741fce0843d3074e77bf785aaccb88a6 Mon Sep 17 00:00:00 2001
From: mattab <matthieu.aubry@gmail.com>
Date: Sat, 6 Dec 2014 13:04:52 +1300
Subject: Refactoring SQL into the Model

---
 plugins/Live/Model.php   | 256 ++++++++++++++++++++++++++++++++++++++---------
 plugins/Live/Visitor.php | 107 ++------------------
 2 files changed, 216 insertions(+), 147 deletions(-)

diff --git a/plugins/Live/Model.php b/plugins/Live/Model.php
index 2d438bff9b..81215f864c 100644
--- a/plugins/Live/Model.php
+++ b/plugins/Live/Model.php
@@ -11,17 +11,163 @@ namespace Piwik\Plugins\Live;
 
 use Exception;
 use Piwik\Common;
+use Piwik\DataAccess\LogAggregator;
 use Piwik\Date;
 use Piwik\Db;
 use Piwik\Period;
 use Piwik\Period\Range;
 use Piwik\Piwik;
+use Piwik\Plugins\CustomVariables\CustomVariables;
 use Piwik\Segment;
 use Piwik\Site;
+use Piwik\Tracker\GoalManager;
 
 class Model
 {
 
+    /**
+     * @param $idVisit
+     * @param $actionsLimit
+     * @return array
+     * @throws \Exception
+     */
+    public function queryActionsForVisit($idVisit, $actionsLimit)
+    {
+        $maxCustomVariables = CustomVariables::getMaxCustomVariables();
+
+        $sqlCustomVariables = '';
+        for ($i = 1; $i <= $maxCustomVariables; $i++) {
+            $sqlCustomVariables .= ', custom_var_k' . $i . ', custom_var_v' . $i;
+        }
+        // The second join is a LEFT join to allow returning records that don't have a matching page title
+        // eg. Downloads, Outlinks. For these, idaction_name is set to 0
+        $sql = "
+				SELECT
+					COALESCE(log_action_event_category.type, log_action.type, log_action_title.type) AS type,
+					log_action.name AS url,
+					log_action.url_prefix,
+					log_action_title.name AS pageTitle,
+					log_action.idaction AS pageIdAction,
+					log_link_visit_action.server_time as serverTimePretty,
+					log_link_visit_action.time_spent_ref_action as timeSpentRef,
+					log_link_visit_action.idlink_va AS pageId,
+					log_link_visit_action.custom_float
+					" . $sqlCustomVariables . ",
+					log_action_event_category.name AS eventCategory,
+					log_action_event_action.name as eventAction
+				FROM " . Common::prefixTable('log_link_visit_action') . " AS log_link_visit_action
+					LEFT JOIN " . Common::prefixTable('log_action') . " AS log_action
+					ON  log_link_visit_action.idaction_url = log_action.idaction
+					LEFT JOIN " . Common::prefixTable('log_action') . " AS log_action_title
+					ON  log_link_visit_action.idaction_name = log_action_title.idaction
+					LEFT JOIN " . Common::prefixTable('log_action') . " AS log_action_event_category
+					ON  log_link_visit_action.idaction_event_category = log_action_event_category.idaction
+					LEFT JOIN " . Common::prefixTable('log_action') . " AS log_action_event_action
+					ON  log_link_visit_action.idaction_event_action = log_action_event_action.idaction
+				WHERE log_link_visit_action.idvisit = ?
+				ORDER BY server_time ASC
+				LIMIT 0, $actionsLimit
+				 ";
+        $actionDetails = Db::fetchAll($sql, array($idVisit));
+        return $actionDetails;
+    }
+
+    /**
+     * @param $idVisit
+     * @param $limit
+     * @return array
+     * @throws \Exception
+     */
+    public function queryGoalConversionsForVisit($idVisit, $limit)
+    {
+        $sql = "
+				SELECT
+						'goal' as type,
+						goal.name as goalName,
+						goal.idgoal as goalId,
+						goal.revenue as revenue,
+						log_conversion.idlink_va as goalPageId,
+						log_conversion.server_time as serverTimePretty,
+						log_conversion.url as url
+				FROM " . Common::prefixTable('log_conversion') . " AS log_conversion
+				LEFT JOIN " . Common::prefixTable('goal') . " AS goal
+					ON (goal.idsite = log_conversion.idsite
+						AND
+						goal.idgoal = log_conversion.idgoal)
+					AND goal.deleted = 0
+				WHERE log_conversion.idvisit = ?
+					AND log_conversion.idgoal > 0
+                ORDER BY server_time ASC
+				LIMIT 0, $limit
+			";
+        $goalDetails = Db::fetchAll($sql, array($idVisit));
+        return $goalDetails;
+    }
+
+    /**
+     * @param $idVisit
+     * @param $limit
+     * @return array
+     * @throws \Exception
+     */
+    public function queryEcommerceConversionsForVisit($idVisit, $limit)
+    {
+        $sql = "SELECT
+						case idgoal when " . GoalManager::IDGOAL_CART
+                            . " then '" . Piwik::LABEL_ID_GOAL_IS_ECOMMERCE_CART
+                            . "' else '" . Piwik::LABEL_ID_GOAL_IS_ECOMMERCE_ORDER . "' end as type,
+						idorder as orderId,
+						" . LogAggregator::getSqlRevenue('revenue') . " as revenue,
+						" . LogAggregator::getSqlRevenue('revenue_subtotal') . " as revenueSubTotal,
+						" . LogAggregator::getSqlRevenue('revenue_tax') . " as revenueTax,
+						" . LogAggregator::getSqlRevenue('revenue_shipping') . " as revenueShipping,
+						" . LogAggregator::getSqlRevenue('revenue_discount') . " as revenueDiscount,
+						items as items,
+						log_conversion.server_time as serverTimePretty
+					FROM " . Common::prefixTable('log_conversion') . " AS log_conversion
+					WHERE idvisit = ?
+						AND idgoal <= " . GoalManager::IDGOAL_ORDER . "
+					ORDER BY server_time ASC
+					LIMIT 0, $limit";
+        $ecommerceDetails = Db::fetchAll($sql, array($idVisit));
+        return $ecommerceDetails;
+    }
+
+
+    /**
+     * @param $idVisit
+     * @param $idOrder
+     * @param $actionsLimit
+     * @return array
+     * @throws \Exception
+     */
+    public function queryEcommerceItemsForOrder($idVisit, $idOrder, $actionsLimit)
+    {
+        $sql = "SELECT
+							log_action_sku.name as itemSKU,
+							log_action_name.name as itemName,
+							log_action_category.name as itemCategory,
+							" . LogAggregator::getSqlRevenue('price') . " as price,
+							quantity as quantity
+						FROM " . Common::prefixTable('log_conversion_item') . "
+							INNER JOIN " . Common::prefixTable('log_action') . " AS log_action_sku
+							ON  idaction_sku = log_action_sku.idaction
+							LEFT JOIN " . Common::prefixTable('log_action') . " AS log_action_name
+							ON  idaction_name = log_action_name.idaction
+							LEFT JOIN " . Common::prefixTable('log_action') . " AS log_action_category
+							ON idaction_category = log_action_category.idaction
+						WHERE idvisit = ?
+							AND idorder = ?
+							AND deleted = 0
+						LIMIT 0, $actionsLimit
+				";
+
+        $bind = array($idVisit, $idOrder);
+
+        $itemsDetails = Db::fetchAll($sql, $bind);
+        return $itemsDetails;
+    }
+
     /**
      * @param $idSite
      * @param $period
@@ -185,12 +331,70 @@ class Model
      */
     public function makeLogVisitsQueryString($idSite, $period, $date, $segment, $countVisitorsToFetch, $visitorId, $minTimestamp, $filterSortOrder)
     {
-        $where = $whereBind = array();
+        // If no other filter, only look at the last 24 hours of stats
+        if (empty($visitorId)
+            && empty($countVisitorsToFetch)
+            && empty($period)
+            && empty($date)
+        ) {
+            $period = 'day';
+            $date = 'yesterdaySameTime';
+        }
+
+        list($whereBind, $where) = $this->getWhereClauseAndBind($idSite, $period, $date, $visitorId, $minTimestamp);
+
+        if (strtolower($filterSortOrder) !== 'asc') {
+            $filterSortOrder = 'DESC';
+        }
+        $segment = new Segment($segment, $idSite);
+
+        // Subquery to use the indexes for ORDER BY
+        $select = "log_visit.*";
+        $from = "log_visit";
+        $groupBy = false;
+        $limit = $countVisitorsToFetch >= 1 ? (int)$countVisitorsToFetch : 0;
+        $orderBy = "idsite, visit_last_action_time " . $filterSortOrder;
+        $orderByParent = "sub.visit_last_action_time " . $filterSortOrder;
+
+        $subQuery = $segment->getSelectQuery($select, $from, $where, $whereBind, $orderBy, $groupBy, $limit);
+
+        $bind = $subQuery['bind'];
+        // Group by idvisit so that a visitor converting 2 goals only appears once
+        $sql = "
+			SELECT sub.* FROM (
+				" . $subQuery['sql'] . "
+			) AS sub
+			GROUP BY sub.idvisit
+			ORDER BY $orderByParent
+		";
+        return array($sql, $bind);
+    }
+
+    /**
+     * @param $idSite
+     * @return Site
+     */
+    protected function makeSite($idSite)
+    {
+        return new Site($idSite);
+    }
 
-        list($whereClause, $idSites) = $this->getIdSitesWhereClause($idSite);
+    /**
+     * @param $idSite
+     * @param $period
+     * @param $date
+     * @param $visitorId
+     * @param $minTimestamp
+     * @return array
+     * @throws Exception
+     */
+    private function getWhereClauseAndBind($idSite, $period, $date, $visitorId, $minTimestamp)
+    {
+        list($whereClause, $bindIdSites) = $this->getIdSitesWhereClause($idSite);
 
+        $where = array();
         $where[] = $whereClause;
-        $whereBind = $idSites;
+        $whereBind = $bindIdSites;
 
         if (!empty($visitorId)) {
             $where[] = "log_visit.idvisitor = ? ";
@@ -202,16 +406,6 @@ class Model
             $whereBind[] = date("Y-m-d H:i:s", $minTimestamp);
         }
 
-        // If no other filter, only look at the last 24 hours of stats
-        if (empty($visitorId)
-            && empty($countVisitorsToFetch)
-            && empty($period)
-            && empty($date)
-        ) {
-            $period = 'day';
-            $date = 'yesterdaySameTime';
-        }
-
         // SQL Filter with provided period
         if (!empty($period) && !empty($date)) {
             $currentSite = $this->makeSite($idSite);
@@ -255,40 +449,6 @@ class Model
         } else {
             $where = false;
         }
-
-        if (strtolower($filterSortOrder) !== 'asc') {
-            $filterSortOrder = 'DESC';
-        }
-        $segment = new Segment($segment, $idSite);
-
-        // Subquery to use the indexes for ORDER BY
-        $select = "log_visit.*";
-        $from = "log_visit";
-        $groupBy = false;
-        $limit = $countVisitorsToFetch >= 1 ? (int)$countVisitorsToFetch : 0;
-        $orderBy = "idsite, visit_last_action_time " . $filterSortOrder;
-        $orderByParent = "sub.visit_last_action_time " . $filterSortOrder;
-
-        $subQuery = $segment->getSelectQuery($select, $from, $where, $whereBind, $orderBy, $groupBy, $limit);
-
-        $bind = $subQuery['bind'];
-        // Group by idvisit so that a visitor converting 2 goals only appears once
-        $sql = "
-			SELECT sub.* FROM (
-				" . $subQuery['sql'] . "
-			) AS sub
-			GROUP BY sub.idvisit
-			ORDER BY $orderByParent
-		";
-        return array($sql, $bind);
-    }
-
-    /**
-     * @param $idSite
-     * @return Site
-     */
-    protected function makeSite($idSite)
-    {
-        return new Site($idSite);
+        return array($whereBind, $where);
     }
 } 
\ No newline at end of file
diff --git a/plugins/Live/Visitor.php b/plugins/Live/Visitor.php
index 5cbe823cd9..eec1ecb6ce 100644
--- a/plugins/Live/Visitor.php
+++ b/plugins/Live/Visitor.php
@@ -246,44 +246,11 @@ class Visitor implements VisitorInterface
     {
         $idVisit = $visitorDetailsArray['idVisit'];
 
-        $maxCustomVariables = CustomVariables::getMaxCustomVariables();
-
-        $sqlCustomVariables = '';
-        for ($i = 1; $i <= $maxCustomVariables; $i++) {
-            $sqlCustomVariables .= ', custom_var_k' . $i . ', custom_var_v' . $i;
-        }
-        // The second join is a LEFT join to allow returning records that don't have a matching page title
-        // eg. Downloads, Outlinks. For these, idaction_name is set to 0
-        $sql = "
-				SELECT
-					COALESCE(log_action_event_category.type, log_action.type, log_action_title.type) AS type,
-					log_action.name AS url,
-					log_action.url_prefix,
-					log_action_title.name AS pageTitle,
-					log_action.idaction AS pageIdAction,
-					log_link_visit_action.server_time as serverTimePretty,
-					log_link_visit_action.time_spent_ref_action as timeSpentRef,
-					log_link_visit_action.idlink_va AS pageId,
-					log_link_visit_action.custom_float
-					". $sqlCustomVariables . ",
-					log_action_event_category.name AS eventCategory,
-					log_action_event_action.name as eventAction
-				FROM " . Common::prefixTable('log_link_visit_action') . " AS log_link_visit_action
-					LEFT JOIN " . Common::prefixTable('log_action') . " AS log_action
-					ON  log_link_visit_action.idaction_url = log_action.idaction
-					LEFT JOIN " . Common::prefixTable('log_action') . " AS log_action_title
-					ON  log_link_visit_action.idaction_name = log_action_title.idaction
-					LEFT JOIN " . Common::prefixTable('log_action') . " AS log_action_event_category
-					ON  log_link_visit_action.idaction_event_category = log_action_event_category.idaction
-					LEFT JOIN " . Common::prefixTable('log_action') . " AS log_action_event_action
-					ON  log_link_visit_action.idaction_event_action = log_action_event_action.idaction
-				WHERE log_link_visit_action.idvisit = ?
-				ORDER BY server_time ASC
-				LIMIT 0, $actionsLimit
-				 ";
-        $actionDetails = Db::fetchAll($sql, array($idVisit));
+        $model = new Model();
+        $actionDetails = $model->queryActionsForVisit($idVisit, $actionsLimit);
 
         $formatter = new Formatter();
+        $maxCustomVariables = CustomVariables::getMaxCustomVariables();
 
         foreach ($actionDetails as $actionIdx => &$actionDetail) {
             $actionDetail =& $actionDetails[$actionIdx];
@@ -353,46 +320,9 @@ class Visitor implements VisitorInterface
         }
 
         // If the visitor converted a goal, we shall select all Goals
-        $sql = "
-				SELECT
-						'goal' as type,
-						goal.name as goalName,
-						goal.idgoal as goalId,
-						goal.revenue as revenue,
-						log_conversion.idlink_va as goalPageId,
-						log_conversion.server_time as serverTimePretty,
-						log_conversion.url as url
-				FROM " . Common::prefixTable('log_conversion') . " AS log_conversion
-				LEFT JOIN " . Common::prefixTable('goal') . " AS goal
-					ON (goal.idsite = log_conversion.idsite
-						AND
-						goal.idgoal = log_conversion.idgoal)
-					AND goal.deleted = 0
-				WHERE log_conversion.idvisit = ?
-					AND log_conversion.idgoal > 0
-                ORDER BY server_time ASC
-				LIMIT 0, $actionsLimit
-			";
-        $goalDetails = Db::fetchAll($sql, array($idVisit));
-
-        $sql = "SELECT
-						case idgoal when " . GoalManager::IDGOAL_CART . " then '" . Piwik::LABEL_ID_GOAL_IS_ECOMMERCE_CART . "' else '" . Piwik::LABEL_ID_GOAL_IS_ECOMMERCE_ORDER . "' end as type,
-						idorder as orderId,
-						" . LogAggregator::getSqlRevenue('revenue') . " as revenue,
-						" . LogAggregator::getSqlRevenue('revenue_subtotal') . " as revenueSubTotal,
-						" . LogAggregator::getSqlRevenue('revenue_tax') . " as revenueTax,
-						" . LogAggregator::getSqlRevenue('revenue_shipping') . " as revenueShipping,
-						" . LogAggregator::getSqlRevenue('revenue_discount') . " as revenueDiscount,
-						items as items,
-
-						log_conversion.server_time as serverTimePretty
-					FROM " . Common::prefixTable('log_conversion') . " AS log_conversion
-					WHERE idvisit = ?
-						AND idgoal <= " . GoalManager::IDGOAL_ORDER . "
-					ORDER BY server_time ASC
-					LIMIT 0, $actionsLimit";
-        $ecommerceDetails = Db::fetchAll($sql, array($idVisit));
+        $goalDetails = $model->queryGoalConversionsForVisit($idVisit, $actionsLimit);
 
+        $ecommerceDetails = $model->queryEcommerceConversionsForVisit($idVisit, $actionsLimit);
         foreach ($ecommerceDetails as &$ecommerceDetail) {
             if ($ecommerceDetail['type'] == Piwik::LABEL_ID_GOAL_IS_ECOMMERCE_CART) {
                 unset($ecommerceDetail['orderId']);
@@ -415,30 +345,9 @@ class Visitor implements VisitorInterface
         // Enrich ecommerce carts/orders with the list of products
         usort($ecommerceDetails, array('static', 'sortByServerTime'));
         foreach ($ecommerceDetails as &$ecommerceConversion) {
-            $sql = "SELECT
-							log_action_sku.name as itemSKU,
-							log_action_name.name as itemName,
-							log_action_category.name as itemCategory,
-							" . LogAggregator::getSqlRevenue('price') . " as price,
-							quantity as quantity
-						FROM " . Common::prefixTable('log_conversion_item') . "
-							INNER JOIN " . Common::prefixTable('log_action') . " AS log_action_sku
-							ON  idaction_sku = log_action_sku.idaction
-							LEFT JOIN " . Common::prefixTable('log_action') . " AS log_action_name
-							ON  idaction_name = log_action_name.idaction
-							LEFT JOIN " . Common::prefixTable('log_action') . " AS log_action_category
-							ON idaction_category = log_action_category.idaction
-						WHERE idvisit = ?
-							AND idorder = ?
-							AND deleted = 0
-						LIMIT 0, $actionsLimit
-				";
-            $bind = array($idVisit, isset($ecommerceConversion['orderId'])
-                ? $ecommerceConversion['orderId']
-                : GoalManager::ITEM_IDORDER_ABANDONED_CART
-            );
-
-            $itemsDetails = Db::fetchAll($sql, $bind);
+            $idOrder = isset($ecommerceConversion['orderId']) ? $ecommerceConversion['orderId'] : GoalManager::ITEM_IDORDER_ABANDONED_CART;
+
+            $itemsDetails = $model->queryEcommerceItemsForOrder($idVisit, $idOrder, $actionsLimit);
             foreach ($itemsDetails as &$detail) {
                 if ($detail['price'] == round($detail['price'])) {
                     $detail['price'] = round($detail['price']);
-- 
cgit v1.2.3


From 8f079b21b1f635a6bb9bbbdfd8396001fc2bdfa3 Mon Sep 17 00:00:00 2001
From: mattab <matthieu.aubry@gmail.com>
Date: Sat, 6 Dec 2014 13:23:28 +1300
Subject: Prefix tables in expected strings (travis runs without prefix)

---
 plugins/Live/tests/System/ModelTest.php | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/plugins/Live/tests/System/ModelTest.php b/plugins/Live/tests/System/ModelTest.php
index 268a7861d4..80f1c8d86f 100644
--- a/plugins/Live/tests/System/ModelTest.php
+++ b/plugins/Live/tests/System/ModelTest.php
@@ -17,7 +17,8 @@ use Piwik\Tests\Framework\TestCase\SystemTestCase;
 use Piwik\Tests\Integration\SegmentTest;
 
 /**
- * @group LiveModelTest
+ * @group Live
+ * @group ModelTest
  * @group Plugins
  */
 class ModelTest extends SystemTestCase
@@ -44,7 +45,7 @@ class ModelTest extends SystemTestCase
         $expectedSql = ' SELECT sub.* FROM
                 (
                     SELECT log_visit.*
-                    FROM piwiktests_log_visit AS log_visit
+                    FROM ' . Common::prefixTable('log_visit') . ' AS log_visit
                     WHERE log_visit.idsite in (?)
                       AND log_visit.visit_last_action_time >= ?
                       AND log_visit.visit_last_action_time <= ?
@@ -83,8 +84,8 @@ class ModelTest extends SystemTestCase
                     SELECT log_inner.*
                     FROM (
                         SELECT log_visit.*
-                        FROM piwiktests_log_visit AS log_visit
-                          LEFT JOIN piwiktests_log_link_visit_action AS log_link_visit_action
+                        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 ( log_visit.idsite in (?)
                           AND log_visit.idvisitor = ?
-- 
cgit v1.2.3