Merge pull request #6184 from Srokap/pr/6121

#5952 - fix infinite loop when disabling > 50 annotations or metadata

3 files changed, 51 insertions, 2 deletions

diff --git a/engine/lib/annotations.php b/engine/lib/annotations.php
index 124e67e0f..5e9b530de 100644
--- a/engine/lib/annotations.php
+++ b/engine/lib/annotations.php
@@ -249,9 +249,13 @@ function elgg_disable_annotations(array $options) {
 	if (!elgg_is_valid_options_for_batch_operation($options, 'annotations')) {
 		return false;
 	}
+	
+	// if we can see hidden (disabled) we need to use the offset
+	// otherwise we risk an infinite loop if there are more than 50
+	$inc_offset = access_get_show_hidden_status();
 
 	$options['metastring_type'] = 'annotations';
-	return elgg_batch_metastring_based_objects($options, 'elgg_batch_disable_callback', false);
+	return elgg_batch_metastring_based_objects($options, 'elgg_batch_disable_callback', $inc_offset);
 }
 
 /**
diff --git a/engine/lib/metadata.php b/engine/lib/metadata.php
index d2f8d4cd4..fdb1b85f6 100644
--- a/engine/lib/metadata.php
+++ b/engine/lib/metadata.php
@@ -333,9 +333,13 @@ function elgg_disable_metadata(array $options) {
 	}
 
 	elgg_get_metadata_cache()->invalidateByOptions('disable', $options);
+	
+	// if we can see hidden (disabled) we need to use the offset
+	// otherwise we risk an infinite loop if there are more than 50
+	$inc_offset = access_get_show_hidden_status();
 
 	$options['metastring_type'] = 'metadata';
-	return elgg_batch_metastring_based_objects($options, 'elgg_batch_disable_callback', false);
+	return elgg_batch_metastring_based_objects($options, 'elgg_batch_disable_callback', $inc_offset);
 }
 
 /**
diff --git a/engine/tests/regression/trac_bugs.php b/engine/tests/regression/trac_bugs.php
index f823825ab..ef1348cf6 100644
--- a/engine/tests/regression/trac_bugs.php
+++ b/engine/tests/regression/trac_bugs.php
@@ -332,4 +332,45 @@ class ElggCoreRegressionBugsTest extends ElggCoreUnitTest {
 		
 		$group->delete();
 	}
+
+	/**
+	 * Ensure that ElggBatch doesn't go into infinite loop when disabling annotations recursively when show hidden is enabled.
+	 *
+	 * https://github.com/Elgg/Elgg/issues/5952
+	 */
+	public function test_disabling_annotations_infinite_loop() {
+
+		//let's have some entity
+		$group = new ElggGroup();
+		$group->name = 'test_group';
+		$group->access_id = ACCESS_PUBLIC;
+		$this->assertTrue($group->save() !== false);
+
+		$total = 51;
+		//add some annotations
+		for ($cnt = 0; $cnt < $total; $cnt++) {
+			$group->annotate('test_annotation', 'value_' . $total);
+		}
+
+		//disable them
+		$show_hidden = access_get_show_hidden_status();
+		access_show_hidden_entities(true);
+		$options = array(
+			'guid' => $group->guid,
+			'limit' => $total, //using strict limit to avoid real infinite loop and just see ElggBatch limiting on it before finishing the work
+		);
+		elgg_disable_annotations($options);
+		access_show_hidden_entities($show_hidden);
+
+		//confirm all being disabled
+		$annotations = $group->getAnnotations(array(
+			'limit' => $total,
+		));
+		foreach ($annotations as $annotation) {
+			$this->assertTrue($annotation->enabled == 'no');
+		}
+
+		//delete group and annotations
+		$group->delete();
+	}
 }