From a9a7c39540e196a4673775b6ec277a54b8436552 Mon Sep 17 00:00:00 2001
From: cash <cash.costello@gmail.com>
Date: Sat, 9 Feb 2013 12:10:19 -0500
Subject: cleaned up some coding standard issues

---
 engine/classes/ElggBatch.php | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

(limited to 'engine/classes/ElggBatch.php')

diff --git a/engine/classes/ElggBatch.php b/engine/classes/ElggBatch.php
index c1a77a0d9..5d59425d0 100644
--- a/engine/classes/ElggBatch.php
+++ b/engine/classes/ElggBatch.php
@@ -296,7 +296,8 @@ class ElggBatch
 	 * Increment the offset from the original options array? Setting to
 	 * false is required for callbacks that delete rows.
 	 *
-	 * @param bool $increment
+	 * @param bool $increment Set to false when deleting data
+	 * @return void
 	 */
 	public function setIncrementOffset($increment = true) {
 		$this->incrementOffset = (bool) $increment;
-- 
cgit v1.2.3


From 726007e5730f83340ea8ab294a9f5951586f42fc Mon Sep 17 00:00:00 2001
From: cash <cash.costello@gmail.com>
Date: Sat, 13 Apr 2013 14:10:02 -0400
Subject: introduces _elgg_invalidate_query_cache() to dry up the db query
 cache code

---
 engine/classes/ElggBatch.php                       |  5 +--
 engine/lib/database.php                            | 38 +++++++++-------------
 engine/lib/upgrade.php                             | 16 ++++-----
 engine/lib/upgrades/2009102801.php                 |  5 +--
 engine/lib/upgrades/2010061501.php                 |  6 ++--
 engine/lib/upgrades/2010071001.php                 |  5 +--
 engine/lib/upgrades/2010071002.php                 |  5 +--
 engine/lib/upgrades/2011052801.php                 |  5 +--
 ....8.13-update_user_location-8999eb8bf1bdd9a3.php |  4 +--
 9 files changed, 42 insertions(+), 47 deletions(-)

(limited to 'engine/classes/ElggBatch.php')

diff --git a/engine/classes/ElggBatch.php b/engine/classes/ElggBatch.php
index 5d59425d0..1912f89a2 100644
--- a/engine/classes/ElggBatch.php
+++ b/engine/classes/ElggBatch.php
@@ -229,8 +229,9 @@ class ElggBatch
 	private function getNextResultsChunk() {
 		// reset memory caches after first chunk load
 		if ($this->chunkIndex > 0) {
-			global $DB_QUERY_CACHE, $ENTITY_CACHE;
-			$DB_QUERY_CACHE = $ENTITY_CACHE = array();
+			global $ENTITY_CACHE;
+			$ENTITY_CACHE = array();
+			_elgg_invalidate_query_cache();
 		}
 
 		// always reset results.
diff --git a/engine/lib/database.php b/engine/lib/database.php
index 18235149d..b41eb4cda 100644
--- a/engine/lib/database.php
+++ b/engine/lib/database.php
@@ -460,19 +460,12 @@ function elgg_query_runner($query, $callback = null, $single = false) {
  * @access private
  */
 function insert_data($query) {
-	global $DB_QUERY_CACHE;
 
 	elgg_log("DB query $query", 'NOTICE');
 	
 	$dblink = get_db_link('write');
 
-	// Invalidate query cache
-	if ($DB_QUERY_CACHE) {
-		/* @var ElggStaticVariableCache $DB_QUERY_CACHE */
-		$DB_QUERY_CACHE->clear();
-	}
-
-	elgg_log("Query cache invalidated", 'NOTICE');
+	_elgg_invalidate_query_cache();
 
 	if (execute_query("$query", $dblink)) {
 		return mysql_insert_id($dblink);
@@ -492,18 +485,12 @@ function insert_data($query) {
  * @access private
  */
 function update_data($query) {
-	global $DB_QUERY_CACHE;
 
 	elgg_log("DB query $query", 'NOTICE');
 
 	$dblink = get_db_link('write');
 
-	// Invalidate query cache
-	if ($DB_QUERY_CACHE) {
-		/* @var ElggStaticVariableCache $DB_QUERY_CACHE */
-		$DB_QUERY_CACHE->clear();
-		elgg_log("Query cache invalidated", 'NOTICE');
-	}
+	_elgg_invalidate_query_cache();
 
 	if (execute_query("$query", $dblink)) {
 		return TRUE;
@@ -523,18 +510,12 @@ function update_data($query) {
  * @access private
  */
 function delete_data($query) {
-	global $DB_QUERY_CACHE;
 
 	elgg_log("DB query $query", 'NOTICE');
 
 	$dblink = get_db_link('write');
 
-	// Invalidate query cache
-	if ($DB_QUERY_CACHE) {
-		/* @var ElggStaticVariableCache $DB_QUERY_CACHE */
-		$DB_QUERY_CACHE->clear();
-		elgg_log("Query cache invalidated", 'NOTICE');
-	}
+	_elgg_invalidate_query_cache();
 
 	if (execute_query("$query", $dblink)) {
 		return mysql_affected_rows($dblink);
@@ -543,6 +524,19 @@ function delete_data($query) {
 	return FALSE;
 }
 
+/**
+ * Invalidate the query cache
+ * 
+ * @access private
+ */
+function _elgg_invalidate_query_cache() {
+	global $DB_QUERY_CACHE;
+	if ($DB_QUERY_CACHE) {
+		/* @var ElggStaticVariableCache $DB_QUERY_CACHE */
+		$DB_QUERY_CACHE->clear();
+		elgg_log("Query cache invalidated", 'NOTICE');
+	}
+}
 
 /**
  * Return tables matching the database prefix {@link $CONFIG->dbprefix}% in the currently
diff --git a/engine/lib/upgrade.php b/engine/lib/upgrade.php
index d684af862..0cc1e64dc 100644
--- a/engine/lib/upgrade.php
+++ b/engine/lib/upgrade.php
@@ -354,16 +354,12 @@ function _elgg_upgrade_unlock() {
  * @access private
  */
 function _elgg_upgrade_is_locked() {
-	global $CONFIG, $DB_QUERY_CACHE;
-	
+	global $CONFIG;
+
 	$is_locked = count(get_data("show tables like '{$CONFIG->dbprefix}upgrade_lock'"));
-	
-	// Invalidate query cache
-	if ($DB_QUERY_CACHE) {
-		/* @var ElggStaticVariableCache $DB_QUERY_CACHE */
-		$DB_QUERY_CACHE->clear();
-		elgg_log("Query cache invalidated", 'NOTICE');
-	}
-	
+
+	// @todo why?
+	_elgg_invalidate_query_cache();
+
 	return $is_locked;
 }
diff --git a/engine/lib/upgrades/2009102801.php b/engine/lib/upgrades/2009102801.php
index b91b99d95..3ad113fb2 100644
--- a/engine/lib/upgrades/2009102801.php
+++ b/engine/lib/upgrades/2009102801.php
@@ -203,14 +203,15 @@ function user_file_matrix($guid) {
 	return "$time_created/$user->guid/";
 }
 
-global $DB_QUERY_CACHE, $ENTITY_CACHE, $CONFIG;
+global $ENTITY_CACHE, $CONFIG;
 /**
  * Upgrade file locations
  */
 $users = mysql_query("SELECT guid, username
 	FROM {$CONFIG->dbprefix}users_entity WHERE username != ''");
 while ($user = mysql_fetch_object($users)) {
-	$DB_QUERY_CACHE = $ENTITY_CACHE = array();
+	$ENTITY_CACHE = array();
+	_elgg_invalidate_query_cache();
 
 	$to = $CONFIG->dataroot . user_file_matrix($user->guid);
 	foreach (array('1_0', '1_1', '1_6') as $version) {
diff --git a/engine/lib/upgrades/2010061501.php b/engine/lib/upgrades/2010061501.php
index b23ad0820..744c28fd5 100644
--- a/engine/lib/upgrades/2010061501.php
+++ b/engine/lib/upgrades/2010061501.php
@@ -45,7 +45,7 @@ if ($dbversion < 2009100701) {
 			}
 		}
 
-		global $DB_QUERY_CACHE, $ENTITY_CACHE;
+		global $ENTITY_CACHE;
 
 		/**
 			Upgrade file locations
@@ -60,7 +60,9 @@ if ($dbversion < 2009100701) {
 		$users = mysql_query("SELECT guid, username FROM {$CONFIG->dbprefix}users_entity
 			WHERE username != ''", $link);
 		while ($user = mysql_fetch_object($users)) {
-			$DB_QUERY_CACHE = $ENTITY_CACHE = array();
+			$ENTITY_CACHE = array();
+			_elgg_invalidate_query_cache();
+
 
 			$to = $CONFIG->dataroot . user_file_matrix($user->guid);
 			foreach (array('1_0', '1_1', '1_6') as $version) {
diff --git a/engine/lib/upgrades/2010071001.php b/engine/lib/upgrades/2010071001.php
index 34f5a773e..5594493a8 100644
--- a/engine/lib/upgrades/2010071001.php
+++ b/engine/lib/upgrades/2010071001.php
@@ -30,11 +30,12 @@ function user_file_matrix_2010071001($guid) {
 
 $sizes = array('large', 'medium', 'small', 'tiny', 'master', 'topbar');
 
-global $DB_QUERY_CACHE, $ENTITY_CACHE, $CONFIG;
+global $ENTITY_CACHE, $CONFIG;
 $users = mysql_query("SELECT guid, username FROM {$CONFIG->dbprefix}users_entity
 	WHERE username != ''");
 while ($user = mysql_fetch_object($users)) {
-	$DB_QUERY_CACHE = $ENTITY_CACHE = array();
+	$ENTITY_CACHE = array();
+	_elgg_invalidate_query_cache();
 
 	$user_directory = user_file_matrix_2010071001($user->guid);
 	if (!$user_directory) {
diff --git a/engine/lib/upgrades/2010071002.php b/engine/lib/upgrades/2010071002.php
index d1c74ed48..52aa15ef5 100644
--- a/engine/lib/upgrades/2010071002.php
+++ b/engine/lib/upgrades/2010071002.php
@@ -4,12 +4,13 @@
  */
 
 // loop through all users checking collections and notifications
-global $DB_QUERY_CACHE, $ENTITY_CACHE, $CONFIG;
+global $ENTITY_CACHE, $CONFIG;
 global $NOTIFICATION_HANDLERS;
 $users = mysql_query("SELECT guid, username FROM {$CONFIG->dbprefix}users_entity
 	WHERE username != ''");
 while ($user = mysql_fetch_object($users)) {
-	$DB_QUERY_CACHE = $ENTITY_CACHE = array();
+	$ENTITY_CACHE = array();
+	_elgg_invalidate_query_cache();
 
 	$user = get_entity($user->guid);
 	foreach ($NOTIFICATION_HANDLERS as $method => $foo) {
diff --git a/engine/lib/upgrades/2011052801.php b/engine/lib/upgrades/2011052801.php
index d68e0118e..b5a8e1018 100644
--- a/engine/lib/upgrades/2011052801.php
+++ b/engine/lib/upgrades/2011052801.php
@@ -2,7 +2,7 @@
 /**
  * Make sure all users have the relationship member_of_site
  */
-global $DB_QUERY_CACHE, $ENTITY_CACHE;
+global $ENTITY_CACHE;
 $db_prefix = get_config('dbprefix');
 
 $limit = 100;
@@ -17,7 +17,8 @@ $q = "SELECT e.* FROM {$db_prefix}entities e
 $users = get_data($q);
 
 while ($users) {
-	$DB_QUERY_CACHE = $ENTITY_CACHE = array();
+	$ENTITY_CACHE = array();
+	_elgg_invalidate_query_cache();
 
 	// do manually to not trigger any events because these aren't new users.
 	foreach ($users as $user) {
diff --git a/engine/lib/upgrades/2013030600-1.8.13-update_user_location-8999eb8bf1bdd9a3.php b/engine/lib/upgrades/2013030600-1.8.13-update_user_location-8999eb8bf1bdd9a3.php
index b38eb5100..8eccf05e2 100644
--- a/engine/lib/upgrades/2013030600-1.8.13-update_user_location-8999eb8bf1bdd9a3.php
+++ b/engine/lib/upgrades/2013030600-1.8.13-update_user_location-8999eb8bf1bdd9a3.php
@@ -7,8 +7,6 @@
  * This script turns that back into a string.
  */
 
-global $DB_QUERY_CACHE;
-
 $ia = elgg_set_ignore_access(true);
 $options = array(
 	'type' => 'user',
@@ -17,7 +15,7 @@ $options = array(
 $batch = new ElggBatch('elgg_get_entities', $options);
 
 foreach ($batch as $entity) {
-	$DB_QUERY_CACHE = array();
+	_elgg_invalidate_query_cache();
 	
 	if (is_array($entity->location)) {
 		$entity->location = implode(', ', $entity->location);
-- 
cgit v1.2.3


From 037cce80328318f7151c498d48f983cabd886505 Mon Sep 17 00:00:00 2001
From: cash <cash.costello@gmail.com>
Date: Sun, 14 Apr 2013 19:59:30 -0400
Subject: entity and query cache are size limited so don't need cache clearing
 in ElggBatch

---
 engine/classes/ElggBatch.php | 6 ------
 1 file changed, 6 deletions(-)

(limited to 'engine/classes/ElggBatch.php')

diff --git a/engine/classes/ElggBatch.php b/engine/classes/ElggBatch.php
index 1912f89a2..eb93b0f5d 100644
--- a/engine/classes/ElggBatch.php
+++ b/engine/classes/ElggBatch.php
@@ -227,12 +227,6 @@ class ElggBatch
 	 * @return bool
 	 */
 	private function getNextResultsChunk() {
-		// reset memory caches after first chunk load
-		if ($this->chunkIndex > 0) {
-			global $ENTITY_CACHE;
-			$ENTITY_CACHE = array();
-			_elgg_invalidate_query_cache();
-		}
 
 		// always reset results.
 		$this->results = array();
-- 
cgit v1.2.3


From 35acc4a297de7660a191d4e7f9d3e8d55561885a Mon Sep 17 00:00:00 2001
From: Steve Clay <steve@mrclay.org>
Date: Mon, 3 Jun 2013 17:02:33 -0400
Subject: Refs #5357: Introduces ElggBatch awareness of incomplete entities
 during fetch

---
 engine/classes/ElggBatch.php | 30 +++++++++++++++++++++------
 engine/lib/entities.php      | 14 ++++++++++---
 engine/tests/api/helpers.php | 48 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 83 insertions(+), 9 deletions(-)

(limited to 'engine/classes/ElggBatch.php')

diff --git a/engine/classes/ElggBatch.php b/engine/classes/ElggBatch.php
index eb93b0f5d..34520f2bc 100644
--- a/engine/classes/ElggBatch.php
+++ b/engine/classes/ElggBatch.php
@@ -149,6 +149,13 @@ class ElggBatch
 	 */
 	private $incrementOffset = true;
 
+	/**
+	 * Entities that could not be instantiated during a fetch
+	 *
+	 * @var stdClass[]
+	 */
+	private $incompleteEntities = array();
+
 	/**
 	 * Batches operations on any elgg_get_*() or compatible function that supports
 	 * an options array.
@@ -221,6 +228,17 @@ class ElggBatch
 		}
 	}
 
+	/**
+	 * Tell the process that an entity was incomplete during a fetch
+	 *
+	 * @param stdClass $row
+	 *
+	 * @access private
+	 */
+	public function reportIncompleteEntity(stdClass $row) {
+		$this->incompleteEntities[] = $row;
+	}
+
 	/**
 	 * Fetches the next chunk of results
 	 *
@@ -265,16 +283,16 @@ class ElggBatch
 
 		$current_options = array(
 			'limit' => $limit,
-			'offset' => $offset
+			'offset' => $offset,
+			'__ElggBatch' => $this,
 		);
 
 		$options = array_merge($this->options, $current_options);
-		$getter = $this->getter;
 
-		if (is_string($getter)) {
-			$this->results = $getter($options);
-		} else {
-			$this->results = call_user_func_array($getter, array($options));
+		$this->incompleteEntities = array();
+		$this->results = call_user_func_array($this->getter, array($options));
+		if ($this->incompleteEntities) {
+			// @todo what to do here?
 		}
 
 		if ($this->results) {
diff --git a/engine/lib/entities.php b/engine/lib/entities.php
index 5cfeca6f8..b7f8c1466 100644
--- a/engine/lib/entities.php
+++ b/engine/lib/entities.php
@@ -891,6 +891,8 @@ function elgg_get_entities(array $options = array()) {
 		'joins'					=>	array(),
 
 		'callback'				=> 'entity_row_to_elggstar',
+
+		'__ElggBatch'			=> null,
 	);
 
 	$options = array_merge($defaults, $options);
@@ -1008,7 +1010,7 @@ function elgg_get_entities(array $options = array()) {
 		}
 
 		if ($options['callback'] === 'entity_row_to_elggstar') {
-			$dt = _elgg_fetch_entities_from_sql($query);
+			$dt = _elgg_fetch_entities_from_sql($query, $options['__ElggBatch']);
 		} else {
 			$dt = get_data($query, $options['callback']);
 		}
@@ -1043,13 +1045,14 @@ function elgg_get_entities(array $options = array()) {
 /**
  * Return entities from an SQL query generated by elgg_get_entities.
  *
- * @param string $sql
+ * @param string    $sql
+ * @param ElggBatch $batch
  * @return ElggEntity[]
  *
  * @access private
  * @throws LogicException
  */
-function _elgg_fetch_entities_from_sql($sql) {
+function _elgg_fetch_entities_from_sql($sql, ElggBatch $batch = null) {
 	static $plugin_subtype;
 	if (null === $plugin_subtype) {
 		$plugin_subtype = get_subtype_id('object', 'plugin');
@@ -1126,6 +1129,11 @@ function _elgg_fetch_entities_from_sql($sql) {
 			} catch (IncompleteEntityException $e) {
 				// don't let incomplete entities throw fatal errors
 				unset($rows[$i]);
+
+				// report incompletes to the batch process that spawned this query
+				if ($batch) {
+					$batch->reportIncompleteEntity($row);
+				}
 			}
 		}
 	}
diff --git a/engine/tests/api/helpers.php b/engine/tests/api/helpers.php
index 62e4471e0..753d02915 100644
--- a/engine/tests/api/helpers.php
+++ b/engine/tests/api/helpers.php
@@ -578,6 +578,54 @@ class ElggCoreHelpersTest extends ElggCoreUnitTest {
 		$this->assertEqual(11, $j);
 	}
 
+	public function testElggBatchHandlesBrokenEntities() {
+		$num_test_entities = 4;
+		$guids = array();
+		$now = time();
+		for ($i = $num_test_entities; $i > 0; $i--) {
+			$entity = new ElggObject();
+			$entity->type = 'object';
+			$entity->subtype = 'test_5357_subtype';
+			$entity->access_id = ACCESS_PUBLIC;
+			$entity->time_created = ($now - $i);
+			$entity->save();
+			$guids[] = $entity->guid;
+			_elgg_invalidate_cache_for_entity($entity->guid);
+		}
+
+		// break the second entity
+		$db_prefix = elgg_get_config('dbprefix');
+		delete_data("DELETE FROM {$db_prefix}objects_entity WHERE guid = {$guids[1]}");
+
+		$options = array(
+			'type' => 'object',
+			'subtype' => 'test_5357_subtype',
+			'order' => 'e.time_created ASC',
+		);
+
+		$entities_visited = array();
+
+		$batch = new ElggBatch('elgg_get_entities', $options, null, 2);
+		foreach ($batch as $entity) {
+			$entities_visited[$entity->guid] = true;
+		}
+
+		// All but the broken entity should have been visited
+		$this->assertEqual(count($entities_visited), $num_test_entities - 1);
+
+		// cleanup (including leftovers from previous tests)
+		$entity_rows = elgg_get_entities(array_merge($options, array(
+			'callback' => '',
+			'limit' => false,
+		)));
+		$guids = array();
+		foreach ($entity_rows as $row) {
+			$guids[] = $row->guid;
+		}
+		delete_data("DELETE FROM {$db_prefix}entities WHERE guid IN (" . implode(',', $guids) . ")");
+		delete_data("DELETE FROM {$db_prefix}objects_entity WHERE guid IN (" . implode(',', $guids) . ")");
+	}
+
 	static function elgg_batch_callback_test($options, $reset = false) {
 		static $count = 1;
 
-- 
cgit v1.2.3


From 708d2fcdc07986ee8bce5838f03f1375e8cd6d5e Mon Sep 17 00:00:00 2001
From: Steve Clay <steve@mrclay.org>
Date: Tue, 4 Jun 2013 22:50:58 -0400
Subject: Fixes #5357: ElggBatch can now skip incomplete entities

---
 engine/classes/ElggBatch.php | 17 +++++++++++------
 engine/tests/api/helpers.php | 19 +++++++++++--------
 2 files changed, 22 insertions(+), 14 deletions(-)

(limited to 'engine/classes/ElggBatch.php')

diff --git a/engine/classes/ElggBatch.php b/engine/classes/ElggBatch.php
index 34520f2bc..83963ccee 100644
--- a/engine/classes/ElggBatch.php
+++ b/engine/classes/ElggBatch.php
@@ -291,14 +291,19 @@ class ElggBatch
 
 		$this->incompleteEntities = array();
 		$this->results = call_user_func_array($this->getter, array($options));
-		if ($this->incompleteEntities) {
-			// @todo what to do here?
-		}
 
-		if ($this->results) {
+		// If there were incomplete entities, we pretend they were at the beginning of the results,
+		// fool the local counter to think it's skipped by them already, and update the running
+		// total as if the results contained the incompletes.
+		if ($this->results || $this->incompleteEntities) {
 			$this->chunkIndex++;
-			$this->resultIndex = 0;
-			$this->retrievedResults += count($this->results);
+			$this->resultIndex = count($this->incompleteEntities);
+			$this->retrievedResults += (count($this->results) + count($this->incompleteEntities));
+			if (!$this->results) {
+				// This fetch was *all* incompletes! We need to fetch until we can either
+				// offer at least one row to iterate over, or give up.
+				return $this->getNextResultsChunk();
+			}
 			return true;
 		} else {
 			return false;
diff --git a/engine/tests/api/helpers.php b/engine/tests/api/helpers.php
index 753d02915..43244636b 100644
--- a/engine/tests/api/helpers.php
+++ b/engine/tests/api/helpers.php
@@ -579,7 +579,7 @@ class ElggCoreHelpersTest extends ElggCoreUnitTest {
 	}
 
 	public function testElggBatchHandlesBrokenEntities() {
-		$num_test_entities = 4;
+		$num_test_entities = 6;
 		$guids = array();
 		$now = time();
 		for ($i = $num_test_entities; $i > 0; $i--) {
@@ -593,25 +593,28 @@ class ElggCoreHelpersTest extends ElggCoreUnitTest {
 			_elgg_invalidate_cache_for_entity($entity->guid);
 		}
 
-		// break the second entity
+		// break entities such that the first fetch has one incomplete
+		// and the second fetch has only incompletes!
 		$db_prefix = elgg_get_config('dbprefix');
-		delete_data("DELETE FROM {$db_prefix}objects_entity WHERE guid = {$guids[1]}");
+		delete_data("
+			DELETE FROM {$db_prefix}objects_entity
+			WHERE guid IN ({$guids[1]}, {$guids[2]}, {$guids[3]})
+		");
 
 		$options = array(
 			'type' => 'object',
 			'subtype' => 'test_5357_subtype',
-			'order' => 'e.time_created ASC',
+			'order_by' => 'e.time_created ASC',
 		);
 
 		$entities_visited = array();
-
 		$batch = new ElggBatch('elgg_get_entities', $options, null, 2);
 		foreach ($batch as $entity) {
-			$entities_visited[$entity->guid] = true;
+			$entities_visited[] = $entity->guid;
 		}
 
-		// All but the broken entity should have been visited
-		$this->assertEqual(count($entities_visited), $num_test_entities - 1);
+		// The broken entities should not have been visited
+		$this->assertEqual($entities_visited, array($guids[0], $guids[4], $guids[5]));
 
 		// cleanup (including leftovers from previous tests)
 		$entity_rows = elgg_get_entities(array_merge($options, array(
-- 
cgit v1.2.3


From 3a3a520958e1eb31ac9d20e7cb818d8c3b5fff4a Mon Sep 17 00:00:00 2001
From: Steve Clay <steve@mrclay.org>
Date: Sat, 8 Jun 2013 21:12:03 -0400
Subject: ElggBatch with incrementOffset off now handles incomplete entities

---
 engine/classes/ElggBatch.php | 36 +++++++++++++++++++--------
 engine/tests/api/helpers.php | 58 +++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 80 insertions(+), 14 deletions(-)

(limited to 'engine/classes/ElggBatch.php')

diff --git a/engine/classes/ElggBatch.php b/engine/classes/ElggBatch.php
index 83963ccee..ac79cf084 100644
--- a/engine/classes/ElggBatch.php
+++ b/engine/classes/ElggBatch.php
@@ -242,9 +242,12 @@ class ElggBatch
 	/**
 	 * Fetches the next chunk of results
 	 *
+	 * @param int $num_incompletes_last_fetch When called recursively, this is the number of
+	 *                                        incomplete entities returned in the last fetch.
+	 *
 	 * @return bool
 	 */
-	private function getNextResultsChunk() {
+	private function getNextResultsChunk($num_incompletes_last_fetch = 0) {
 
 		// always reset results.
 		$this->results = array();
@@ -278,7 +281,7 @@ class ElggBatch
 		if ($this->incrementOffset) {
 			$offset = $this->offset + $this->retrievedResults;
 		} else {
-			$offset = $this->offset;
+			$offset = $this->offset + $num_incompletes_last_fetch;
 		}
 
 		$current_options = array(
@@ -292,17 +295,30 @@ class ElggBatch
 		$this->incompleteEntities = array();
 		$this->results = call_user_func_array($this->getter, array($options));
 
-		// If there were incomplete entities, we pretend they were at the beginning of the results,
-		// fool the local counter to think it's skipped by them already, and update the running
-		// total as if the results contained the incompletes.
-		if ($this->results || $this->incompleteEntities) {
+		$num_results = count($this->results);
+		$num_incomplete = count($this->incompleteEntities);
+
+		if ($this->incompleteEntities) {
+			// pad the front of the results with nulls representing the incompletes
+			array_splice($this->results, 0, 0, array_pad(array(), $num_incomplete, null));
+			// ...and skip past them
+			reset($this->results);
+			for ($i = 0; $i < $num_incomplete; $i++) {
+				next($this->results);
+			}
+		}
+
+		if ($this->results) {
 			$this->chunkIndex++;
-			$this->resultIndex = count($this->incompleteEntities);
-			$this->retrievedResults += (count($this->results) + count($this->incompleteEntities));
-			if (!$this->results) {
+
+			// let the system know we've jumped past the nulls
+			$this->resultIndex = $num_incomplete;
+
+			$this->retrievedResults += ($num_results + $num_incomplete);
+			if ($num_results == 0) {
 				// This fetch was *all* incompletes! We need to fetch until we can either
 				// offer at least one row to iterate over, or give up.
-				return $this->getNextResultsChunk();
+				return $this->getNextResultsChunk($num_incomplete);
 			}
 			return true;
 		} else {
diff --git a/engine/tests/api/helpers.php b/engine/tests/api/helpers.php
index 43244636b..06ef55138 100644
--- a/engine/tests/api/helpers.php
+++ b/engine/tests/api/helpers.php
@@ -578,16 +578,14 @@ class ElggCoreHelpersTest extends ElggCoreUnitTest {
 		$this->assertEqual(11, $j);
 	}
 
-	public function testElggBatchHandlesBrokenEntities() {
+	public function testElggBatchReadHandlesBrokenEntities() {
 		$num_test_entities = 6;
 		$guids = array();
-		$now = time();
 		for ($i = $num_test_entities; $i > 0; $i--) {
 			$entity = new ElggObject();
 			$entity->type = 'object';
 			$entity->subtype = 'test_5357_subtype';
 			$entity->access_id = ACCESS_PUBLIC;
-			$entity->time_created = ($now - $i);
 			$entity->save();
 			$guids[] = $entity->guid;
 			_elgg_invalidate_cache_for_entity($entity->guid);
@@ -604,11 +602,12 @@ class ElggCoreHelpersTest extends ElggCoreUnitTest {
 		$options = array(
 			'type' => 'object',
 			'subtype' => 'test_5357_subtype',
-			'order_by' => 'e.time_created ASC',
+			'order_by' => 'e.guid',
 		);
 
 		$entities_visited = array();
 		$batch = new ElggBatch('elgg_get_entities', $options, null, 2);
+		/* @var ElggEntity[] $batch */
 		foreach ($batch as $entity) {
 			$entities_visited[] = $entity->guid;
 		}
@@ -629,6 +628,57 @@ class ElggCoreHelpersTest extends ElggCoreUnitTest {
 		delete_data("DELETE FROM {$db_prefix}objects_entity WHERE guid IN (" . implode(',', $guids) . ")");
 	}
 
+	public function testElggBatchDeleteHandlesBrokenEntities() {
+		$num_test_entities = 6;
+		$guids = array();
+		for ($i = $num_test_entities; $i > 0; $i--) {
+			$entity = new ElggObject();
+			$entity->type = 'object';
+			$entity->subtype = 'test_5357_subtype';
+			$entity->access_id = ACCESS_PUBLIC;
+			$entity->save();
+			$guids[] = $entity->guid;
+			_elgg_invalidate_cache_for_entity($entity->guid);
+		}
+
+		// break entities such that the first fetch has one incomplete
+		// and the second fetch has only incompletes!
+		$db_prefix = elgg_get_config('dbprefix');
+		delete_data("
+			DELETE FROM {$db_prefix}objects_entity
+			WHERE guid IN ({$guids[1]}, {$guids[2]}, {$guids[3]})
+		");
+
+		$options = array(
+			'type' => 'object',
+			'subtype' => 'test_5357_subtype',
+			'order_by' => 'e.guid',
+		);
+
+		$entities_visited = array();
+		$batch = new ElggBatch('elgg_get_entities', $options, null, 2, false);
+		/* @var ElggEntity[] $batch */
+		foreach ($batch as $entity) {
+			$entities_visited[] = $entity->guid;
+			$entity->delete();
+		}
+
+		// The broken entities should not have been visited
+		$this->assertEqual($entities_visited, array($guids[0], $guids[4], $guids[5]));
+
+		// cleanup (including leftovers from previous tests)
+		$entity_rows = elgg_get_entities(array_merge($options, array(
+			'callback' => '',
+			'limit' => false,
+		)));
+		$guids = array();
+		foreach ($entity_rows as $row) {
+			$guids[] = $row->guid;
+		}
+		delete_data("DELETE FROM {$db_prefix}entities WHERE guid IN (" . implode(',', $guids) . ")");
+		delete_data("DELETE FROM {$db_prefix}objects_entity WHERE guid IN (" . implode(',', $guids) . ")");
+	}
+
 	static function elgg_batch_callback_test($options, $reset = false) {
 		static $count = 1;
 
-- 
cgit v1.2.3


From 8a47b2342e53c9cdf3093982486b19d6cc2f3e9b Mon Sep 17 00:00:00 2001
From: Steve Clay <steve@mrclay.org>
Date: Sat, 8 Jun 2013 21:34:11 -0400
Subject: Improved algorithm by tracking total incomplete entities

---
 engine/classes/ElggBatch.php | 18 ++++++++++++------
 engine/tests/api/helpers.php | 16 ++++++++--------
 2 files changed, 20 insertions(+), 14 deletions(-)

(limited to 'engine/classes/ElggBatch.php')

diff --git a/engine/classes/ElggBatch.php b/engine/classes/ElggBatch.php
index ac79cf084..d810ea066 100644
--- a/engine/classes/ElggBatch.php
+++ b/engine/classes/ElggBatch.php
@@ -156,6 +156,13 @@ class ElggBatch
 	 */
 	private $incompleteEntities = array();
 
+	/**
+	 * Total number of incomplete entities fetched
+	 *
+	 * @var int
+	 */
+	private $totalIncompletes = 0;
+
 	/**
 	 * Batches operations on any elgg_get_*() or compatible function that supports
 	 * an options array.
@@ -242,12 +249,9 @@ class ElggBatch
 	/**
 	 * Fetches the next chunk of results
 	 *
-	 * @param int $num_incompletes_last_fetch When called recursively, this is the number of
-	 *                                        incomplete entities returned in the last fetch.
-	 *
 	 * @return bool
 	 */
-	private function getNextResultsChunk($num_incompletes_last_fetch = 0) {
+	private function getNextResultsChunk() {
 
 		// always reset results.
 		$this->results = array();
@@ -281,7 +285,7 @@ class ElggBatch
 		if ($this->incrementOffset) {
 			$offset = $this->offset + $this->retrievedResults;
 		} else {
-			$offset = $this->offset + $num_incompletes_last_fetch;
+			$offset = $this->offset + $this->totalIncompletes;
 		}
 
 		$current_options = array(
@@ -298,6 +302,8 @@ class ElggBatch
 		$num_results = count($this->results);
 		$num_incomplete = count($this->incompleteEntities);
 
+		$this->totalIncompletes += $num_incomplete;
+
 		if ($this->incompleteEntities) {
 			// pad the front of the results with nulls representing the incompletes
 			array_splice($this->results, 0, 0, array_pad(array(), $num_incomplete, null));
@@ -318,7 +324,7 @@ class ElggBatch
 			if ($num_results == 0) {
 				// This fetch was *all* incompletes! We need to fetch until we can either
 				// offer at least one row to iterate over, or give up.
-				return $this->getNextResultsChunk($num_incomplete);
+				return $this->getNextResultsChunk();
 			}
 			return true;
 		} else {
diff --git a/engine/tests/api/helpers.php b/engine/tests/api/helpers.php
index 06ef55138..10216140f 100644
--- a/engine/tests/api/helpers.php
+++ b/engine/tests/api/helpers.php
@@ -579,7 +579,7 @@ class ElggCoreHelpersTest extends ElggCoreUnitTest {
 	}
 
 	public function testElggBatchReadHandlesBrokenEntities() {
-		$num_test_entities = 6;
+		$num_test_entities = 8;
 		$guids = array();
 		for ($i = $num_test_entities; $i > 0; $i--) {
 			$entity = new ElggObject();
@@ -592,11 +592,11 @@ class ElggCoreHelpersTest extends ElggCoreUnitTest {
 		}
 
 		// break entities such that the first fetch has one incomplete
-		// and the second fetch has only incompletes!
+		// and the second and third fetches have only incompletes!
 		$db_prefix = elgg_get_config('dbprefix');
 		delete_data("
 			DELETE FROM {$db_prefix}objects_entity
-			WHERE guid IN ({$guids[1]}, {$guids[2]}, {$guids[3]})
+			WHERE guid IN ({$guids[1]}, {$guids[2]}, {$guids[3]}, {$guids[4]}, {$guids[5]})
 		");
 
 		$options = array(
@@ -613,7 +613,7 @@ class ElggCoreHelpersTest extends ElggCoreUnitTest {
 		}
 
 		// The broken entities should not have been visited
-		$this->assertEqual($entities_visited, array($guids[0], $guids[4], $guids[5]));
+		$this->assertEqual($entities_visited, array($guids[0], $guids[6], $guids[7]));
 
 		// cleanup (including leftovers from previous tests)
 		$entity_rows = elgg_get_entities(array_merge($options, array(
@@ -629,7 +629,7 @@ class ElggCoreHelpersTest extends ElggCoreUnitTest {
 	}
 
 	public function testElggBatchDeleteHandlesBrokenEntities() {
-		$num_test_entities = 6;
+		$num_test_entities = 8;
 		$guids = array();
 		for ($i = $num_test_entities; $i > 0; $i--) {
 			$entity = new ElggObject();
@@ -642,11 +642,11 @@ class ElggCoreHelpersTest extends ElggCoreUnitTest {
 		}
 
 		// break entities such that the first fetch has one incomplete
-		// and the second fetch has only incompletes!
+		// and the second and third fetches have only incompletes!
 		$db_prefix = elgg_get_config('dbprefix');
 		delete_data("
 			DELETE FROM {$db_prefix}objects_entity
-			WHERE guid IN ({$guids[1]}, {$guids[2]}, {$guids[3]})
+			WHERE guid IN ({$guids[1]}, {$guids[2]}, {$guids[3]}, {$guids[4]}, {$guids[5]})
 		");
 
 		$options = array(
@@ -664,7 +664,7 @@ class ElggCoreHelpersTest extends ElggCoreUnitTest {
 		}
 
 		// The broken entities should not have been visited
-		$this->assertEqual($entities_visited, array($guids[0], $guids[4], $guids[5]));
+		$this->assertEqual($entities_visited, array($guids[0], $guids[6], $guids[7]));
 
 		// cleanup (including leftovers from previous tests)
 		$entity_rows = elgg_get_entities(array_merge($options, array(
-- 
cgit v1.2.3