From 100d7181bcc9814078748272b0df182c95631bca Mon Sep 17 00:00:00 2001 From: Evan Winslow Date: Thu, 24 May 2012 10:22:42 -0700 Subject: Fixes #4543: Cache entities fetched with get_entity and elgg_get_entities --- engine/lib/entities.php | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) (limited to 'engine') diff --git a/engine/lib/entities.php b/engine/lib/entities.php index 4875b2c2f..05916ddf4 100644 --- a/engine/lib/entities.php +++ b/engine/lib/entities.php @@ -52,9 +52,16 @@ function invalidate_cache_for_entity($guid) { * @see retrieve_cached_entity() * @see invalidate_cache_for_entity() * @access private + * TODO(evan): Use an ElggCache object */ function cache_entity(ElggEntity $entity) { global $ENTITY_CACHE; + + // Don't store too many or we'll have memory problems + // TODO(evan): Pick a less arbitrary limit + if (count($ENTITY_CACHE) > 256) { + unset($ENTITY_CACHE[array_rand($ENTITY_CACHE)]); + } $ENTITY_CACHE[$entity->guid] = $entity; } @@ -675,7 +682,14 @@ function get_entity($guid) { if (!is_numeric($guid) || $guid === 0 || $guid === '0') { return FALSE; } + + // Check local cache first + $new_entity = retrieve_cached_entity($guid); + if ($new_entity) { + return $new_entity; + } + // Check shared memory cache, if available if ((!$newentity_cache) && (is_memcache_available())) { $newentity_cache = new ElggMemcache('new_entity_cache'); } @@ -683,12 +697,14 @@ function get_entity($guid) { if ($newentity_cache) { $new_entity = $newentity_cache->load($guid); } - + if ($new_entity) { return $new_entity; } - return entity_row_to_elggstar(get_entity_as_row($guid)); + $new_entity = entity_row_to_elggstar(get_entity_as_row($guid)); + cache_entity($new_entity); + return $new_entity; } /** @@ -930,6 +946,13 @@ function elgg_get_entities(array $options = array()) { } $dt = get_data($query, $options['callback']); + foreach ($dt as $entity) { + // If a custom callback is provided, it could return something other than ElggEntity, + // so we have to do an explicit check here. + if ($entity instanceof ElggEntity) { + cache_entity($entity); + } + } return $dt; } else { $total = get_data_row($query); -- cgit v1.2.3 From 6dafbe06733f6cee4a52b169f5e4cf07d9d11c72 Mon Sep 17 00:00:00 2001 From: Steve Clay Date: Tue, 29 May 2012 08:24:49 -0400 Subject: small fixes --- engine/lib/entities.php | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) (limited to 'engine') diff --git a/engine/lib/entities.php b/engine/lib/entities.php index 05916ddf4..20921b41a 100644 --- a/engine/lib/entities.php +++ b/engine/lib/entities.php @@ -30,7 +30,7 @@ $SUBTYPE_CACHE = NULL; * * @param int $guid The entity guid * - * @return void + * @return null * @access private */ function invalidate_cache_for_entity($guid) { @@ -48,7 +48,7 @@ function invalidate_cache_for_entity($guid) { * * @param ElggEntity $entity Entity to cache * - * @return void + * @return null * @see retrieve_cached_entity() * @see invalidate_cache_for_entity() * @access private @@ -56,7 +56,13 @@ function invalidate_cache_for_entity($guid) { */ function cache_entity(ElggEntity $entity) { global $ENTITY_CACHE; - + + // Don't cache entities while access control is off, otherwise they could be + // exposed to users who shouldn't see them when control is re-enabled. + if (elgg_get_ignore_access()) { + return; + } + // Don't store too many or we'll have memory problems // TODO(evan): Pick a less arbitrary limit if (count($ENTITY_CACHE) > 256) { @@ -703,7 +709,9 @@ function get_entity($guid) { } $new_entity = entity_row_to_elggstar(get_entity_as_row($guid)); - cache_entity($new_entity); + if ($new_entity) { + cache_entity($new_entity); + } return $new_entity; } @@ -1425,6 +1433,7 @@ function disable_entity($guid, $reason = "", $recursive = true) { $entity->disableMetadata(); $entity->disableAnnotations(); + invalidate_cache_for_entity($guid); $res = update_data("UPDATE {$CONFIG->dbprefix}entities SET enabled = 'no' -- cgit v1.2.3 From e274214a3827f702b91639180433a7b534a303ef Mon Sep 17 00:00:00 2001 From: Steve Clay Date: Wed, 13 Jun 2012 16:53:54 -0400 Subject: Added entity caching to elgg_get_entities with workaround for recursive delete problem (#4568) --- engine/lib/entities.php | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) (limited to 'engine') diff --git a/engine/lib/entities.php b/engine/lib/entities.php index 20921b41a..90e62fac7 100644 --- a/engine/lib/entities.php +++ b/engine/lib/entities.php @@ -77,7 +77,7 @@ function cache_entity(ElggEntity $entity) { * * @param int $guid The guid * - * @return void + * @return ElggEntity|bool false if entity not cached, or not fully loaded * @see cache_entity() * @see invalidate_cache_for_entity() * @access private @@ -954,13 +954,18 @@ function elgg_get_entities(array $options = array()) { } $dt = get_data($query, $options['callback']); - foreach ($dt as $entity) { - // If a custom callback is provided, it could return something other than ElggEntity, - // so we have to do an explicit check here. - if ($entity instanceof ElggEntity) { - cache_entity($entity); + if ($dt) { + foreach ($dt as $entity) { + // If a custom callback is provided, it could return something other than ElggEntity, + // so we have to do an explicit check here. + if ($entity instanceof ElggEntity) { + cache_entity($entity); + } } + // @todo Without this, recursive delete fails. See #4568 + reset($dt); } + return $dt; } else { $total = get_data_row($query); -- cgit v1.2.3 From 24ee620c1b33d550d704df0fc3259c0a9bce59cb Mon Sep 17 00:00:00 2001 From: Sem Date: Tue, 31 Jul 2012 21:25:47 +0200 Subject: Fixes #4774. Added tokens insted is_action param in delete river menu item. --- engine/lib/navigation.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'engine') diff --git a/engine/lib/navigation.php b/engine/lib/navigation.php index 10b11acfe..8c3952594 100644 --- a/engine/lib/navigation.php +++ b/engine/lib/navigation.php @@ -339,11 +339,10 @@ function elgg_river_menu_setup($hook, $type, $return, $params) { if (elgg_is_admin_logged_in()) { $options = array( 'name' => 'delete', - 'href' => "action/river/delete?id=$item->id", + 'href' => elgg_add_action_tokens_to_url("action/river/delete?id=$item->id"), 'text' => elgg_view_icon('delete'), 'title' => elgg_echo('delete'), 'confirm' => elgg_echo('deleteconfirm'), - 'is_action' => true, 'priority' => 200, ); $return[] = ElggMenuItem::factory($options); -- cgit v1.2.3 From 82226dc8f71e0e820640f0d416d2eb119f02cece Mon Sep 17 00:00:00 2001 From: Matt Beckett Date: Tue, 7 Aug 2012 16:05:41 -0600 Subject: fixes river upgrade for comments --- engine/lib/upgrades/2010121602.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'engine') diff --git a/engine/lib/upgrades/2010121602.php b/engine/lib/upgrades/2010121602.php index 2d55c8214..5b0996b5e 100644 --- a/engine/lib/upgrades/2010121602.php +++ b/engine/lib/upgrades/2010121602.php @@ -4,7 +4,7 @@ */ $query = "UPDATE {$CONFIG->dbprefix}river - SET view='river/annotation/generic_comment/create', action_type='create' + SET view='river/annotation/generic_comment/create' WHERE view='annotation/annotate' AND action_type='comment'"; update_data($query); -- cgit v1.2.3 From f9c83e896f59dcbb11dc5a876aa71cf581a49afe Mon Sep 17 00:00:00 2001 From: Steve Clay Date: Wed, 22 Aug 2012 11:44:31 -0400 Subject: Refactored for clarity --- engine/lib/entities.php | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) (limited to 'engine') diff --git a/engine/lib/entities.php b/engine/lib/entities.php index 90e62fac7..b8ebbd68a 100644 --- a/engine/lib/entities.php +++ b/engine/lib/entities.php @@ -679,8 +679,10 @@ function entity_row_to_elggstar($row) { * @link http://docs.elgg.org/DataModel/Entities */ function get_entity($guid) { - static $newentity_cache; - $new_entity = false; + // This should not be a static local var. Notice that cache writing occurs in a completely + // different instance outside this function. + // @todo We need a single Memcache instance with a shared pool of namespace wrappers. This function would pull an instance from the pool. + static $shared_cache; // We could also use: if (!(int) $guid) { return FALSE }, // but that evaluates to a false positive for $guid = TRUE. @@ -696,16 +698,18 @@ function get_entity($guid) { } // Check shared memory cache, if available - if ((!$newentity_cache) && (is_memcache_available())) { - $newentity_cache = new ElggMemcache('new_entity_cache'); - } - - if ($newentity_cache) { - $new_entity = $newentity_cache->load($guid); + if (null === $shared_cache) { + if (is_memcache_available()) { + $shared_cache = new ElggMemcache('new_entity_cache'); + } else { + $shared_cache = false; + } } - - if ($new_entity) { - return $new_entity; + if ($shared_cache) { + $new_entity = $shared_cache->load($guid); + if ($new_entity) { + return $new_entity; + } } $new_entity = entity_row_to_elggstar(get_entity_as_row($guid)); -- cgit v1.2.3 From 3325f3c3d07836f9afe122cfc3aa72377f6bd35e Mon Sep 17 00:00:00 2001 From: Steve Clay Date: Sat, 25 Aug 2012 09:21:45 -0300 Subject: Fixed @return doc on elgg_extract --- engine/lib/elgglib.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'engine') diff --git a/engine/lib/elgglib.php b/engine/lib/elgglib.php index 3026a78e3..554b0561f 100644 --- a/engine/lib/elgglib.php +++ b/engine/lib/elgglib.php @@ -1575,7 +1575,7 @@ function elgg_http_url_is_identical($url1, $url2, $ignore_params = array('offset * @param bool $strict Return array key if it's set, even if empty. If false, * return $default if the array key is unset or empty. * - * @return void + * @return mixed * @since 1.8.0 */ function elgg_extract($key, array $array, $default = null, $strict = true) { -- cgit v1.2.3