ElggBatch with incrementOffset off now handles incomplete entities

diff --git a/engine/classes/ElggBatch.php b/engine/classes/ElggBatch.php
index 83963ccee3ecb26d7e1efc797a79ab056b324aef..ac79cf0841a4c23c837b2f3b2a87254b70f60dd5 100644 (file)
--- 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 43244636b9de312d5798e20fd79d5058a0730471..06ef55138f070ada55ff01af027ab1ce7af645dc 100644 (file)
--- 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;