Fixes #4081. Using ElggBatch to delete recursive.

3 files changed, 71 insertions, 18 deletions

diff --git a/engine/lib/entities.php b/engine/lib/entities.php
index 2dc0eb8ae..c1ef683e5 100644
--- a/engine/lib/entities.php
+++ b/engine/lib/entities.php
@@ -1520,18 +1520,22 @@ function delete_entity($guid, $recursive = true) {
 					$entity_disable_override = access_get_show_hidden_status();
 					access_show_hidden_entities(true);
 					$ia = elgg_set_ignore_access(true);
-					$sub_entities = get_data("SELECT * from {$CONFIG->dbprefix}entities
-						WHERE container_guid=$guid
-							or owner_guid=$guid
-							or site_guid=$guid", 'entity_row_to_elggstar');
-					if ($sub_entities) {
-						foreach ($sub_entities as $e) {
-							// check for equality so that an entity that is its own
-							// owner or container does not cause infinite loop
-							if ($e->guid != $guid) {
-								$e->delete(true);
-							}
-						}
+
+					// @todo there was logic in the original code that ignored
+					// entities with owner or container guids of themselves.
+					// this should probably be prevented in ElggEntity instead of checked for here
+					$options = array(
+						'wheres' => array(
+							"((container_guid = $guid OR owner_guid = $guid OR site_guid = $guid)"
+							. " AND guid != $guid)"
+							),
+						'limit' => 0
+					);
+
+					$batch = new ElggBatch('elgg_get_entities', $options, 50, false);
+
+					foreach ($batch as $e) {
+						$e->delete(true);
 					}
 
 					access_show_hidden_entities($entity_disable_override);
diff --git a/engine/tests/api/helpers.php b/engine/tests/api/helpers.php
index a615be0c0..62e4471e0 100644
--- a/engine/tests/api/helpers.php
+++ b/engine/tests/api/helpers.php
@@ -526,7 +526,7 @@ class ElggCoreHelpersTest extends ElggCoreUnitTest {
 			'offset' => 0,
 			'limit' => 11
 		);
-		$batch = new ElggBatch(array('ElggCoreHelpersTest', 'test_elgg_batch_callback'), $options,
+		$batch = new ElggBatch(array('ElggCoreHelpersTest', 'elgg_batch_callback_test'), $options,
 				null, 5);
 		$j = 0;
 		foreach ($batch as $e) {
@@ -539,12 +539,12 @@ class ElggCoreHelpersTest extends ElggCoreUnitTest {
 		$this->assertEqual(11, $j);
 
 		// no increment, 0 start
-		ElggCoreHelpersTest::test_elgg_batch_callback(array(), true);
+		ElggCoreHelpersTest::elgg_batch_callback_test(array(), true);
 		$options = array(
 			'offset' => 0,
 			'limit' => 11
 		);
-		$batch = new ElggBatch(array('ElggCoreHelpersTest', 'test_elgg_batch_callback'), $options,
+		$batch = new ElggBatch(array('ElggCoreHelpersTest', 'elgg_batch_callback_test'), $options,
 				null, 5);
 		$batch->setIncrementOffset(false);
 
@@ -558,12 +558,12 @@ class ElggCoreHelpersTest extends ElggCoreUnitTest {
 		$this->assertEqual(11, $j);
 
 		// no increment, 3 start
-		ElggCoreHelpersTest::test_elgg_batch_callback(array(), true);
+		ElggCoreHelpersTest::elgg_batch_callback_test(array(), true);
 		$options = array(
 			'offset' => 3,
 			'limit' => 11
 		);
-		$batch = new ElggBatch(array('ElggCoreHelpersTest', 'test_elgg_batch_callback'), $options,
+		$batch = new ElggBatch(array('ElggCoreHelpersTest', 'elgg_batch_callback_test'), $options,
 				null, 5);
 		$batch->setIncrementOffset(false);
 
@@ -578,7 +578,7 @@ class ElggCoreHelpersTest extends ElggCoreUnitTest {
 		$this->assertEqual(11, $j);
 	}
 
-	static function test_elgg_batch_callback($options, $reset = false) {
+	static function elgg_batch_callback_test($options, $reset = false) {
 		static $count = 1;
 
 		if ($reset) {
diff --git a/engine/tests/objects/objects.php b/engine/tests/objects/objects.php
index cd507d5ab..915594e0a 100644
--- a/engine/tests/objects/objects.php
+++ b/engine/tests/objects/objects.php
@@ -239,6 +239,55 @@ class ElggCoreObjectTest extends ElggCoreUnitTest {
 		access_show_hidden_entities(false);
 	}
 
+	public function testElggRecursiveDelete() {
+		$types = array('ElggGroup', 'ElggObject', 'ElggUser', 'ElggSite');
+		$db_prefix = elgg_get_config('dbprefix');
+
+		foreach ($types as $type) {
+			$parent = new $type();
+			$this->assertTrue($parent->save());
+			
+			$child = new ElggObject();
+			$child->container_guid = $parent->guid;
+			$this->assertTrue($child->save());
+
+			$grandchild = new ElggObject();
+			$grandchild->container_guid = $child->guid;
+			$this->assertTrue($grandchild->save());
+
+			$this->assertTrue($parent->delete(true));
+
+			$q = "SELECT * FROM {$db_prefix}entities WHERE guid = $parent->guid";
+			$r = get_data($q);
+			$this->assertFalse($r);
+
+			$q = "SELECT * FROM {$db_prefix}entities WHERE guid = $child->guid";
+			$r = get_data($q);
+			$this->assertFalse($r);
+
+			$q = "SELECT * FROM {$db_prefix}entities WHERE guid = $grandchild->guid";
+			$r = get_data($q);
+			$this->assertFalse($r);
+		}
+
+		// object that owns itself
+		// can't check container_guid because of infinite loops in can_edit_entity()
+		$obj = new ElggObject();
+		$obj->save();
+		$obj->owner_guid = $obj->guid;
+		$obj->save();
+
+		$q = "SELECT * FROM {$db_prefix}entities WHERE guid = $obj->guid";
+		$r = get_data_row($q);
+		$this->assertEqual($obj->guid, $r->owner_guid);
+
+		$this->assertTrue($obj->delete(true));
+
+		$q = "SELECT * FROM {$db_prefix}entities WHERE guid = $obj->guid";
+		$r = get_data_row($q);
+		$this->assertFalse($r);
+	}
+
 	protected function get_object_row($guid) {
 		global $CONFIG;
 		return get_data_row("SELECT * FROM {$CONFIG->dbprefix}objects_entity WHERE guid='$guid'");