From 3ac47e89eb7cc6024cffad435e9a6ef2a29488ff Mon Sep 17 00:00:00 2001
From: Timo Besenreuther <timo.besenreuther@gmail.com>
Date: Wed, 20 Mar 2013 17:31:11 +0100
Subject: [PATCH] visitor log improvements

 * use a limit for the number of actions that are loaded for every visit. without a limit, the web server might crash (e.g. when there are 100K+ page views in one visit after a log import)
 * regular pagination in visitor log. (a) performance improvement: use indexes in the select query. (b) bug fix: the old logic was incorrect. in one setup, there are 10K+ visits but the visitor log only showed one full page and one page with one visit.
 * removed reference to trunk from tests read me
---
 config/global.ini.php                   |  3 ++
 core/ViewDataTable.php                  |  4 ++-
 plugins/CoreHome/templates/datatable.js |  3 --
 plugins/Live/API.php                    | 41 +++++++++++++------------
 plugins/Live/Controller.php             | 12 +++-----
 plugins/Live/templates/visitorLog.tpl   | 25 ++-------------
 tests/README.md                         |  8 ++---
 7 files changed, 39 insertions(+), 57 deletions(-)

diff --git a/config/global.ini.php b/config/global.ini.php
index 73294cfb62..b11a5e7328 100644
--- a/config/global.ini.php
+++ b/config/global.ini.php
@@ -248,6 +248,9 @@ datatable_archiving_maximum_rows_standard = 500
 ; amount of actions, referrers or custom variable name/value pairs.
 archiving_ranking_query_row_limit = 50000
 
+; maximum number of actions that is shown in the visitor log for each visitor
+visitor_log_maximum_actions_per_visit = 500
+
 ; by default, the real time Live! widget will update every 5 seconds and refresh with new visits/actions/etc.
 ; you can change the timeout so the widget refreshes more often, or not as frequently
 live_widget_refresh_after_seconds = 5
diff --git a/core/ViewDataTable.php b/core/ViewDataTable.php
index 27d1bdba25..c39cedf94a 100644
--- a/core/ViewDataTable.php
+++ b/core/ViewDataTable.php
@@ -772,7 +772,9 @@ abstract class Piwik_ViewDataTable
 		
 		if($this->dataTable &&
 			// Piwik_DataTable_Array doesn't have the method
-			!($this->dataTable instanceof Piwik_DataTable_Array))
+			!($this->dataTable instanceof Piwik_DataTable_Array)
+			&& empty($javascriptVariablesToSet['totalRows'])
+		)
 		{
 			$javascriptVariablesToSet['totalRows'] = $this->dataTable->getRowsCountBeforeLimitFilter();
 		}
diff --git a/plugins/CoreHome/templates/datatable.js b/plugins/CoreHome/templates/datatable.js
index 96714d1392..69a770eb60 100644
--- a/plugins/CoreHome/templates/datatable.js
+++ b/plugins/CoreHome/templates/datatable.js
@@ -279,9 +279,6 @@ dataTable.prototype =
 			{
 				params.filter_limit = value;
 				params.filter_offset = 0;
-				
-				// Hack for Visitor Log to not pass the maxIdVisit parameter when limit is changed
-				delete params.maxIdVisit;
 			};
 		}
 		
diff --git a/plugins/Live/API.php b/plugins/Live/API.php
index 622f95f379..fca088497c 100644
--- a/plugins/Live/API.php
+++ b/plugins/Live/API.php
@@ -100,8 +100,7 @@ class Piwik_Live_API
 	 */
 	public function getLastVisitsForVisitor( $visitorId, $idSite, $filter_limit = 10 )
 	{
-		Piwik::checkUserHasViewAccess($idSite);
-		$visitorDetails = $this->loadLastVisitorDetailsFromDatabase($idSite, $period = false, $date = false, $segment = false, $filter_limit, $maxIdVisit = false, $visitorId);
+		$visitorDetails = $this->loadLastVisitorDetailsFromDatabase($idSite, $period = false, $date = false, $segment = false, $filter_limit, $filter_offset = false, $visitorId);
 		$table = $this->getCleanedVisitorsFromDetails($visitorDetails, $idSite);
 		return $table;
 	}
@@ -114,21 +113,22 @@ class Piwik_Live_API
 	 * @param bool|string $period Period to restrict to when looking at the logs
 	 * @param bool|string $date Date to restrict to
 	 * @param bool|int $segment (optional) Number of visits rows to return
-	 * @param bool|int $filter_limit (optional)
-	 * @param bool|int $maxIdVisit (optional) Maximum idvisit to restrict the query to (useful when paginating)
+	 * @param bool|int $filter_limit (optional) Only return X visits
+	 * @param bool|int $filter_offset (optional) Skip the first X visits (useful when paginating)
 	 * @param bool|int $minTimestamp (optional) Minimum timestamp to restrict the query to (useful when paginating or refreshing visits)
 	 *
 	 * @return Piwik_DataTable
 	 */
-	public function getLastVisitsDetails( $idSite, $period, $date, $segment = false, $filter_limit = false, $maxIdVisit = false, $minTimestamp = false )
+	public function getLastVisitsDetails( $idSite, $period, $date, $segment = false, $filter_limit = false, $filter_offset = false, $minTimestamp = false )
 	{
 		if(empty($filter_limit)) 
 		{
 			$filter_limit = 10;
 		}
 		Piwik::checkUserHasViewAccess($idSite);
-		$visitorDetails = $this->loadLastVisitorDetailsFromDatabase($idSite, $period, $date, $segment, $filter_limit, $maxIdVisit, $visitorId = false, $minTimestamp); 
+		$visitorDetails = $this->loadLastVisitorDetailsFromDatabase($idSite, $period, $date, $segment, $filter_limit, $filter_offset, $visitorId = false, $minTimestamp); 
 		$dataTable = $this->getCleanedVisitorsFromDetails($visitorDetails, $idSite);
+		
 		return $dataTable;
 	}
 
@@ -138,7 +138,7 @@ class Piwik_Live_API
 	
 	public function getLastVisits( $idSite, $filter_limit = 10, $minTimestamp = false )
 	{
-		return $this->getLastVisitsDetails($idSite, $period = false, $date = false, $segment = false, $filter_limit, $maxIdVisit = false, $minTimestamp );
+		return $this->getLastVisitsDetails($idSite, $period = false, $date = false, $segment = false, $filter_limit, $filter_offset = false, $minTimestamp );
 	}
 
 	/**
@@ -150,6 +150,8 @@ class Piwik_Live_API
 	 */
 	private function getCleanedVisitorsFromDetails($visitorDetails, $idSite)
 	{
+		$actionsLimit = Piwik_Config::getInstance()->General['visitor_log_maximum_actions_per_visit'];
+		
 		$table = new Piwik_DataTable();
 
 		$site = new Piwik_Site($idSite);
@@ -198,6 +200,7 @@ class Piwik_Live_API
 					LEFT JOIN " .Piwik_Common::prefixTable('log_action')." AS log_action_title
 					ON  log_link_visit_action.idaction_name = log_action_title.idaction
 				WHERE log_link_visit_action.idvisit = ?
+				LIMIT 0, $actionsLimit
 				 ";
 			$actionDetails = Piwik_FetchAll($sql, array($idvisit));
 			
@@ -253,6 +256,7 @@ class Piwik_Live_API
 					AND goal.deleted = 0
 				WHERE log_conversion.idvisit = ?
 					AND log_conversion.idgoal > 0
+				LIMIT 0, $actionsLimit
 			";
 			$goalDetails = Piwik_FetchAll($sql, array($idvisit));
 
@@ -269,7 +273,8 @@ class Piwik_Live_API
 						log_conversion.server_time as serverTimePretty
 					FROM ".Piwik_Common::prefixTable('log_conversion')." AS log_conversion
 					WHERE idvisit = ?
-						AND idgoal <= ".Piwik_Tracker_GoalManager::IDGOAL_ORDER;
+						AND idgoal <= ".Piwik_Tracker_GoalManager::IDGOAL_ORDER."
+					LIMIT 0, $actionsLimit";
 			$ecommerceDetails = Piwik_FetchAll($sql, array($idvisit));
 
 			foreach($ecommerceDetails as &$ecommerceDetail)
@@ -316,6 +321,7 @@ class Piwik_Live_API
 						WHERE idvisit = ? 
 							AND idorder = ?
 							AND deleted = 0
+						LIMIT 0, $actionsLimit
 				";
 				$bind = array($idvisit, isset($ecommerceConversion['orderId']) 
 											? $ecommerceConversion['orderId'] 
@@ -400,12 +406,17 @@ class Piwik_Live_API
 						: 1 ); 
 	}
 	
-	private function loadLastVisitorDetailsFromDatabase($idSite, $period = false, $date = false, $segment = false, $filter_limit = false, $maxIdVisit = false, $visitorId = false, $minTimestamp = false)
+	private function loadLastVisitorDetailsFromDatabase($idSite, $period = false, $date = false, $segment = false, $filter_limit = false, $filter_offset = false, $visitorId = false, $minTimestamp = false)
 	{
 		if(empty($filter_limit))
 		{
 			$filter_limit = 100;
 		}
+		if(empty($filter_offset))
+		{
+			$filter_offset = 0;
+		}
+		
 		$where = $whereBind = array();
 		$where[] = "log_visit.idsite = ? ";
 		$whereBind[] = $idSite;
@@ -417,14 +428,6 @@ class Piwik_Live_API
 			$whereBind[] = @Piwik_Common::hex2bin($visitorId);
 		}
 
-		if(!empty($maxIdVisit))
-		{
-			$where[] = "log_visit.idvisit < ? ";
-			$whereBind[] = $maxIdVisit;
-			$orderBy = "idvisit DESC";
-			$orderByParent = "sub.idvisit DESC";
-		}
-		
 		if(!empty($minTimestamp))
 		{
 			$where[] = "log_visit.visit_last_action_time > ? ";
@@ -433,7 +436,7 @@ class Piwik_Live_API
 		
 		// If no other filter, only look at the last 24 hours of stats
 		if(empty($visitorId)
-			&& empty($maxIdVisit)
+			&& empty($filter_offset)
 			&& empty($period) 
 			&& empty($date))
 		{
@@ -500,7 +503,7 @@ class Piwik_Live_API
 		$from = "log_visit";
 		$subQuery = $segment->getSelectQuery($select, $from, $where, $whereBind, $orderBy);
 		
-		$sqlLimit = $filter_limit >= 1 ? " LIMIT ".(int)$filter_limit : "";
+		$sqlLimit = $filter_limit >= 1 ? " LIMIT ".$filter_offset.", ".(int)$filter_limit : "";
 		
 		// Group by idvisit so that a visitor converting 2 goals only appears once
 		$sql = "
diff --git a/plugins/Live/Controller.php b/plugins/Live/Controller.php
index 6fdc9fa9e4..080d6da409 100644
--- a/plugins/Live/Controller.php
+++ b/plugins/Live/Controller.php
@@ -49,12 +49,6 @@ class Piwik_Live_Controller extends Piwik_Controller
 	
 	public function getVisitorLog($fetch = false)
 	{
-		// If previous=1 is set, user clicked previous
-		// we can't deal with previous so we force display of the first page
-		if(Piwik_Common::getRequestVar('previous', 0, 'int') == 1) {
-			$_GET['maxIdVisit'] = '';
-		}
-		
 		$view = Piwik_ViewDataTable::factory();
 		$view->init( $this->pluginName,
 							__FUNCTION__,
@@ -83,9 +77,13 @@ class Piwik_Live_Controller extends Piwik_Controller
 		}
 		
 		$view->setReportDocumentation(Piwik_Translate('Live_VisitorLogDocumentation', array('<br />', '<br />')));
-		$view->setCustomParameter('dataTablePreviousIsFirst', 1);
+		
+		// set a very high row count so that the next link in the footer of the data table is always shown
+		$view->setCustomParameter('totalRows', 10000000);
+		
 		$view->setCustomParameter('filterEcommerce', Piwik_Common::getRequestVar('filterEcommerce', 0, 'int'));
 		$view->setCustomParameter('pageUrlNotDefined', Piwik_Translate('General_NotDefined', Piwik_Translate('Actions_ColumnPageURL')));
+		
 		return $this->renderView($view, $fetch);
 	}
 
diff --git a/plugins/Live/templates/visitorLog.tpl b/plugins/Live/templates/visitorLog.tpl
index e3153f5729..f8b9183738 100644
--- a/plugins/Live/templates/visitorLog.tpl
+++ b/plugins/Live/templates/visitorLog.tpl
@@ -10,7 +10,7 @@
 {capture assign='displayVisitorsInOwnColumn'}{if $isWidget}0{else}1{/if}{/capture}
 
 <a graphid="VisitsSummarygetEvolutionGraph" name="evolutionGraph"></a>
-{assign var=maxIdVisit value=0}
+
 {if isset($arrayDataTable.result) and $arrayDataTable.result == 'error'}
 		{$arrayDataTable.message}
 	{else}
@@ -39,9 +39,6 @@
 	<tbody>
 
 {foreach from=$arrayDataTable item=visitor}
-{if $maxIdVisit == 0 || $visitor.columns.idVisit < $maxIdVisit}
-{assign var=maxIdVisit value=$visitor.columns.idVisit}
-{/if}
 
 	{capture assign='visitorColumnContent'}
 		&nbsp;<img src="{$visitor.columns.countryFlag}" title="{$visitor.columns.location|escape:'html'}, Provider {$visitor.columns.provider|escape:'html'}" />
@@ -257,13 +254,6 @@
 </table>
 {/if}
 
-{if count($arrayDataTable) == $javascriptVariablesToSet.filter_limit}
-{* We set a fake large rows count so that 'Next' paginate link is forced to display
-   This is hard coded because the Visitor Log datatable is not fully loaded in memory,
-   but needs to fetch only the N rows in the logs
-   *}
-{php}$this->_tpl_vars['javascriptVariablesToSet']['totalRows'] = 100000; {/php}
-{/if}
 {if $properties.show_footer}
 	{include file="CoreHome/templates/datatable_footer.tpl"}
 {/if}
@@ -280,18 +270,7 @@ function Piwik_Live_LoadVisitorPopover(visitorId)
 {rdelim}
 
 $(document).ready(function(){ldelim}
-
-    var dataTableVisitorLog = piwik.DataTableManager.getDataTableInstanceByReport('{$properties.uniqueId}');
-    dataTableVisitorLog.param.maxIdVisit = {$maxIdVisit};
-    {literal}
-    function hidePreviousLink() {
-        if (dataTableVisitorLog.param.previous == 1) {
-            $('.dataTablePrevious').hide();
-            dataTableVisitorLog.param.previous = 0;
-        }
-    }
-    hidePreviousLink();
-
+	{literal}
     // Replace duplicated page views by a NX count instead of using too much vertical space
     $("ol.visitorLog").each(function () {
         var prevelement;
diff --git a/tests/README.md b/tests/README.md
index 3af99f1d45..f766013cf2 100644
--- a/tests/README.md
+++ b/tests/README.md
@@ -141,7 +141,7 @@ which you can use to run PHPUnit tests in your browser.
 
 ### Starting VisualPHPUnit
 
-To load VisualPHPUnit point your browser to http://path/to/piwik/trunk/tests/lib/visualphpunit/.
+To load VisualPHPUnit point your browser to http://path/to/piwik/tests/lib/visualphpunit/.
 
 VisualPHPUnit will already be configured for use with Piwik. 
 
@@ -215,7 +215,7 @@ First, XHProf must be built (this guide assumes you're using a linux variant):
 
  * 	Navigate to the XHProf extension directory.
 
-		$ cd /path/to/piwik/trunk/tests/lib/xhprof-0.9.2/extension
+		$ cd /path/to/piwik/tests/lib/xhprof-0.9.2/extension
     
  * 	Build XHProf.
 
@@ -228,7 +228,7 @@ First, XHProf must be built (this guide assumes you're using a linux variant):
       
 	```
 	[xhprof]
-	extension=/path/to/piwik/trunk/tests/lib/xhprof-0.9.2/extension/modules/xhprof.so
+	extension=/path/to/piwik/tests/lib/xhprof-0.9.2/extension/modules/xhprof.so
 	xhprof.output_dir=/path/to/output/dir
 	```
       
@@ -242,7 +242,7 @@ is installed and act accordingly.
 
 To use XHProf, first load VisualPHPUnit by pointing your browser to:
 
-http://path/to/piwik/trunk/tests/lib/visualphpunit/
+http://path/to/piwik/tests/lib/visualphpunit/
 
 Select a test or get ready to run a benchmark. Make sure the 'Profile with XHProf' select
 box is set to 'Yes' and click 'Run Tests'.
-- 
GitLab