From 35acc4a297de7660a191d4e7f9d3e8d55561885a Mon Sep 17 00:00:00 2001 From: Steve Clay 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') 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 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') 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 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') 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 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') 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