From 85e4c16f39a8b00b229644bcd175663541dfd51a Mon Sep 17 00:00:00 2001 From: Steve Clay Date: Mon, 4 Feb 2013 20:37:25 -0500 Subject: Doc fixes and inline type hints for variables (big static analysis cleanup) --- engine/lib/metadata.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'engine/lib/metadata.php') diff --git a/engine/lib/metadata.php b/engine/lib/metadata.php index f76c20f24..2fa491963 100644 --- a/engine/lib/metadata.php +++ b/engine/lib/metadata.php @@ -277,7 +277,7 @@ $access_id = ACCESS_PRIVATE, $allow_multiple = false) { * all metadata that match the query instead of returning * ElggMetadata objects. * - * @return mixed + * @return ElggMetadata[]|mixed * @since 1.8.0 */ function elgg_get_metadata(array $options = array()) { @@ -412,7 +412,7 @@ function elgg_enable_metadata(array $options) { * * metadata_owner_guids => NULL|ARR guids for metadata owners * - * @return mixed If count, int. If not count, array. false on errors. + * @return ElggEntity[]|mixed If count, int. If not count, array. false on errors. * @since 1.7.0 */ function elgg_get_entities_from_metadata(array $options = array()) { @@ -461,7 +461,7 @@ function elgg_get_entities_from_metadata(array $options = array()) { * @param array|null $order_by_metadata Array of names / direction * @param array|null $owner_guids Array of owner GUIDs * - * @return FALSE|array False on fail, array('joins', 'wheres') + * @return false|array False on fail, array('joins', 'wheres') * @since 1.7.0 * @access private */ -- cgit v1.2.3 From 597c0a4e519e14ba42c77b518a44789e3a8067b0 Mon Sep 17 00:00:00 2001 From: Cash Costello Date: Tue, 5 Mar 2013 07:34:18 -0500 Subject: Fixes #5165 handling html tags passed as tags --- engine/lib/metadata.php | 2 +- views/default/output/tag.php | 1 + views/default/output/tagcloud.php | 2 ++ views/default/output/tags.php | 1 + 4 files changed, 5 insertions(+), 1 deletion(-) (limited to 'engine/lib/metadata.php') diff --git a/engine/lib/metadata.php b/engine/lib/metadata.php index 2fa491963..35b7b4dfb 100644 --- a/engine/lib/metadata.php +++ b/engine/lib/metadata.php @@ -774,10 +774,10 @@ function string_to_tag_array($string) { $ar = explode(",", $string); $ar = array_map('trim', $ar); $ar = array_filter($ar, 'is_not_null'); + $ar = array_map('strip_tags', $ar); return $ar; } return false; - } /** diff --git a/views/default/output/tag.php b/views/default/output/tag.php index 3c002a31b..3e1f1c320 100644 --- a/views/default/output/tag.php +++ b/views/default/output/tag.php @@ -20,6 +20,7 @@ if (!empty($vars['object'])) { } if (isset($vars['value'])) { + $vars['value'] = htmlspecialchars($vars['value'], ENT_QUOTES, 'UTF-8', false); if (!empty($vars['type'])) { $type = "&type={$vars['type']}"; } else { diff --git a/views/default/output/tagcloud.php b/views/default/output/tagcloud.php index a212becd8..2fbf1cd0a 100644 --- a/views/default/output/tagcloud.php +++ b/views/default/output/tagcloud.php @@ -39,6 +39,8 @@ if (!empty($vars['tagcloud']) && is_array($vars['tagcloud'])) { $cloud = ''; foreach ($vars['tagcloud'] as $tag) { + $tag->tag = htmlspecialchars($tag->tag, ENT_QUOTES, 'UTF-8', false); + if ($cloud != '') { $cloud .= ', '; } diff --git a/views/default/output/tags.php b/views/default/output/tags.php index 3082dd41e..41fd5f168 100644 --- a/views/default/output/tags.php +++ b/views/default/output/tags.php @@ -55,6 +55,7 @@ if (!empty($vars['tags'])) { $list_items = '
  • ' . elgg_view_icon('tag', $icon_class) . '
    • '; foreach($vars['tags'] as $tag) { + $tag = htmlspecialchars($tag, ENT_QUOTES, 'UTF-8', false); if (!empty($vars['type'])) { $type = "&type={$vars['type']}"; } else { -- cgit v1.2.3 From 2dbb9772881131f5739bae2d1ce5955587e9ab78 Mon Sep 17 00:00:00 2001 From: cash Date: Wed, 6 Mar 2013 16:51:20 -0500 Subject: Fixes #4393 working around the overloaded 'count' parameter --- engine/lib/annotations.php | 16 ++++++++++++++++ engine/lib/metadata.php | 8 ++++++++ engine/lib/metastrings.php | 5 ----- 3 files changed, 24 insertions(+), 5 deletions(-) (limited to 'engine/lib/metadata.php') diff --git a/engine/lib/annotations.php b/engine/lib/annotations.php index f40a2cc6f..bd5ea1a1f 100644 --- a/engine/lib/annotations.php +++ b/engine/lib/annotations.php @@ -200,6 +200,18 @@ function update_annotation($annotation_id, $name, $value, $value_type, $owner_gu * @since 1.8.0 */ function elgg_get_annotations(array $options = array()) { + + // @todo remove support for count shortcut - see #4393 + if (isset($options['__egefac']) && $options['__egefac']) { + unset($options['__egefac']); + } else { + // support shortcut of 'count' => true for 'annotation_calculation' => 'count' + if (isset($options['count']) && $options['count']) { + $options['annotation_calculation'] = 'count'; + unset($options['count']); + } + } + $options['metastring_type'] = 'annotations'; return elgg_get_metastring_based_objects($options); } @@ -425,6 +437,10 @@ function elgg_get_entities_from_annotation_calculation($options) { $options['callback'] = 'entity_row_to_elggstar'; + // see #4393 + // @todo remove after the 'count' shortcut is removed from elgg_get_annotations() + $options['__egefac'] = true; + return elgg_get_annotations($options); } diff --git a/engine/lib/metadata.php b/engine/lib/metadata.php index 35b7b4dfb..96d446060 100644 --- a/engine/lib/metadata.php +++ b/engine/lib/metadata.php @@ -281,6 +281,14 @@ $access_id = ACCESS_PRIVATE, $allow_multiple = false) { * @since 1.8.0 */ function elgg_get_metadata(array $options = array()) { + + // @todo remove support for count shortcut - see #4393 + // support shortcut of 'count' => true for 'metadata_calculation' => 'count' + if (isset($options['count']) && $options['count']) { + $options['metadata_calculation'] = 'count'; + unset($options['count']); + } + $options['metastring_type'] = 'metadata'; return elgg_get_metastring_based_objects($options); } diff --git a/engine/lib/metastrings.php b/engine/lib/metastrings.php index 76c4bd8c4..f49b4a163 100644 --- a/engine/lib/metastrings.php +++ b/engine/lib/metastrings.php @@ -389,11 +389,6 @@ function elgg_get_metastring_based_objects($options) { $selects = $options['selects']; - // allow count shortcut - if ($options['count']) { - $options['metastring_calculation'] = 'count'; - } - // For performance reasons we don't want the joins required for metadata / annotations // unless we're going through one of their callbacks. // this means we expect the functions passing different callbacks to pass their required joins. -- cgit v1.2.3 From cf04d05bd44cd2ac935b6ed14c7af85e58aa7d17 Mon Sep 17 00:00:00 2001 From: Steve Clay Date: Sun, 10 Mar 2013 23:10:48 -0400 Subject: Fixes #5214: Successfully invalidates metadata cache in some corner cases --- engine/lib/metadata.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'engine/lib/metadata.php') diff --git a/engine/lib/metadata.php b/engine/lib/metadata.php index 96d446060..305e9918b 100644 --- a/engine/lib/metadata.php +++ b/engine/lib/metadata.php @@ -310,11 +310,14 @@ function elgg_delete_metadata(array $options) { if (!elgg_is_valid_options_for_batch_operation($options, 'metadata')) { return false; } + $options['metastring_type'] = 'metadata'; + $result = elgg_batch_metastring_based_objects($options, 'elgg_batch_delete_callback', false); + // This moved last in case an object's constructor sets metadata. Currently the batch + // delete process has to create the entity to delete its metadata. See #5214 elgg_get_metadata_cache()->invalidateByOptions('delete', $options); - $options['metastring_type'] = 'metadata'; - return elgg_batch_metastring_based_objects($options, 'elgg_batch_delete_callback', false); + return $result; } /** -- cgit v1.2.3 From a2ecf54d56d9f877e6f0f8ac6d841cee6187aac4 Mon Sep 17 00:00:00 2001 From: cash Date: Fri, 15 Mar 2013 11:18:05 -0400 Subject: more coding standard fixes --- engine/classes/ElggEntity.php | 11 +++++------ engine/classes/ElggGroup.php | 9 +++------ engine/classes/ElggMenuBuilder.php | 12 ++++++------ engine/classes/ElggObject.php | 9 +++------ engine/classes/ElggSite.php | 12 ++++-------- engine/classes/ElggTranslit.php | 26 +++++++++++++------------- engine/classes/ElggUser.php | 12 ++++-------- engine/lib/configuration.php | 4 ++-- engine/lib/elgglib.php | 6 +++--- engine/lib/languages.php | 3 +++ engine/lib/location.php | 2 +- engine/lib/metadata.php | 4 ++-- engine/lib/plugins.php | 2 +- engine/lib/relationships.php | 2 +- engine/lib/views.php | 20 +++++++++++--------- 15 files changed, 62 insertions(+), 72 deletions(-) (limited to 'engine/lib/metadata.php') diff --git a/engine/classes/ElggEntity.php b/engine/classes/ElggEntity.php index f44e73023..5a63c7b15 100644 --- a/engine/classes/ElggEntity.php +++ b/engine/classes/ElggEntity.php @@ -375,12 +375,11 @@ abstract class ElggEntity extends ElggData implements } return $result; - } - - // unsaved entity. store in temp array - // returning single entries instead of an array of 1 element is decided in - // getMetaData(), just like pulling from the db. - else { + } else { + // unsaved entity. store in temp array + // returning single entries instead of an array of 1 element is decided in + // getMetaData(), just like pulling from the db. + // // if overwrite, delete first if (!$multiple || !isset($this->temp_metadata[$name])) { $this->temp_metadata[$name] = array(); diff --git a/engine/classes/ElggGroup.php b/engine/classes/ElggGroup.php index 61f699f1a..7ab0bfa48 100644 --- a/engine/classes/ElggGroup.php +++ b/engine/classes/ElggGroup.php @@ -48,21 +48,18 @@ class ElggGroup extends ElggEntity $msg = elgg_echo('IOException:FailedToLoadGUID', array(get_class(), $guid->guid)); throw new IOException($msg); } - - // Is $guid is an ElggGroup? Use a copy constructor } else if ($guid instanceof ElggGroup) { + // $guid is an ElggGroup so this is a copy constructor elgg_deprecated_notice('This type of usage of the ElggGroup constructor was deprecated. Please use the clone method.', 1.7); foreach ($guid->attributes as $key => $value) { $this->attributes[$key] = $value; } - - // Is this is an ElggEntity but not an ElggGroup = ERROR! } else if ($guid instanceof ElggEntity) { + // @todo why separate from else throw new InvalidParameterException(elgg_echo('InvalidParameterException:NonElggGroup')); - - // Is it a GUID } else if (is_numeric($guid)) { + // $guid is a GUID so load entity if (!$this->load($guid)) { throw new IOException(elgg_echo('IOException:FailedToLoadGUID', array(get_class(), $guid))); } diff --git a/engine/classes/ElggMenuBuilder.php b/engine/classes/ElggMenuBuilder.php index 639e34755..198018f3c 100644 --- a/engine/classes/ElggMenuBuilder.php +++ b/engine/classes/ElggMenuBuilder.php @@ -235,8 +235,8 @@ class ElggMenuBuilder { /** * Compare two menu items by their display text * - * @param ElggMenuItem $a - * @param ElggMenuItem $b + * @param ElggMenuItem $a Menu item + * @param ElggMenuItem $b Menu item * @return bool */ public static function compareByText($a, $b) { @@ -253,8 +253,8 @@ class ElggMenuBuilder { /** * Compare two menu items by their identifiers * - * @param ElggMenuItem $a - * @param ElggMenuItem $b + * @param ElggMenuItem $a Menu item + * @param ElggMenuItem $b Menu item * @return bool */ public static function compareByName($a, $b) { @@ -271,8 +271,8 @@ class ElggMenuBuilder { /** * Compare two menu items by their priority * - * @param ElggMenuItem $a - * @param ElggMenuItem $b + * @param ElggMenuItem $a Menu item + * @param ElggMenuItem $b Menu item * @return bool * * @todo change name to compareByPriority diff --git a/engine/classes/ElggObject.php b/engine/classes/ElggObject.php index 6263f84f6..3cb76ffaf 100644 --- a/engine/classes/ElggObject.php +++ b/engine/classes/ElggObject.php @@ -66,21 +66,18 @@ class ElggObject extends ElggEntity { $msg = elgg_echo('IOException:FailedToLoadGUID', array(get_class(), $guid->guid)); throw new IOException($msg); } - - // Is $guid is an ElggObject? Use a copy constructor } else if ($guid instanceof ElggObject) { + // $guid is an ElggObject so this is a copy constructor elgg_deprecated_notice('This type of usage of the ElggObject constructor was deprecated. Please use the clone method.', 1.7); foreach ($guid->attributes as $key => $value) { $this->attributes[$key] = $value; } - - // Is this is an ElggEntity but not an ElggObject = ERROR! } else if ($guid instanceof ElggEntity) { + // @todo remove - do not need separate exception throw new InvalidParameterException(elgg_echo('InvalidParameterException:NonElggObject')); - - // Is it a GUID } else if (is_numeric($guid)) { + // $guid is a GUID so load if (!$this->load($guid)) { throw new IOException(elgg_echo('IOException:FailedToLoadGUID', array(get_class(), $guid))); } diff --git a/engine/classes/ElggSite.php b/engine/classes/ElggSite.php index 1a34df195..deba5087e 100644 --- a/engine/classes/ElggSite.php +++ b/engine/classes/ElggSite.php @@ -77,28 +77,24 @@ class ElggSite extends ElggEntity { $msg = elgg_echo('IOException:FailedToLoadGUID', array(get_class(), $guid->guid)); throw new IOException($msg); } - - // Is $guid is an ElggSite? Use a copy constructor } else if ($guid instanceof ElggSite) { + // $guid is an ElggSite so this is a copy constructor elgg_deprecated_notice('This type of usage of the ElggSite constructor was deprecated. Please use the clone method.', 1.7); foreach ($guid->attributes as $key => $value) { $this->attributes[$key] = $value; } - - // Is this is an ElggEntity but not an ElggSite = ERROR! } else if ($guid instanceof ElggEntity) { + // @todo remove and just use else clause throw new InvalidParameterException(elgg_echo('InvalidParameterException:NonElggSite')); - - // See if this is a URL } else if (strpos($guid, "http") !== false) { + // url so retrieve by url $guid = get_site_by_url($guid); foreach ($guid->attributes as $key => $value) { $this->attributes[$key] = $value; } - - // Is it a GUID } else if (is_numeric($guid)) { + // $guid is a GUID so load if (!$this->load($guid)) { throw new IOException(elgg_echo('IOException:FailedToLoadGUID', array(get_class(), $guid))); } diff --git a/engine/classes/ElggTranslit.php b/engine/classes/ElggTranslit.php index 79116fc01..601965c11 100644 --- a/engine/classes/ElggTranslit.php +++ b/engine/classes/ElggTranslit.php @@ -58,15 +58,15 @@ class ElggTranslit { // remove all ASCII except 0-9a-zA-Z, hyphen, underscore, and whitespace // note: "x" modifier did not work with this pattern. $string = preg_replace('~[' - . '\x00-\x08' # control chars - . '\x0b\x0c' # vert tab, form feed - . '\x0e-\x1f' # control chars - . '\x21-\x2c' # ! ... , - . '\x2e\x2f' # . slash - . '\x3a-\x40' # : ... @ - . '\x5b-\x5e' # [ ... ^ - . '\x60' # ` - . '\x7b-\x7f' # { ... DEL + . '\x00-\x08' // control chars + . '\x0b\x0c' // vert tab, form feed + . '\x0e-\x1f' // control chars + . '\x21-\x2c' // ! ... , + . '\x2e\x2f' // . slash + . '\x3a-\x40' // : ... @ + . '\x5b-\x5e' // [ ... ^ + . '\x60' // ` + . '\x7b-\x7f' // { ... DEL . ']~', '', $string); $string = strtr($string, '', ''); @@ -80,10 +80,10 @@ class ElggTranslit { // note: we cannot use [^0-9a-zA-Z] because that matches multibyte chars. // note: "x" modifier did not work with this pattern. $pattern = '~[' - . '\x00-\x2f' # controls ... slash - . '\x3a-\x40' # : ... @ - . '\x5b-\x60' # [ ... ` - . '\x7b-\x7f' # { ... DEL + . '\x00-\x2f' // controls ... slash + . '\x3a-\x40' // : ... @ + . '\x5b-\x60' // [ ... ` + . '\x7b-\x7f' // { ... DEL . ']+~x'; // ['internationalization', 'and', '日本語'] diff --git a/engine/classes/ElggUser.php b/engine/classes/ElggUser.php index 6c1cdc1de..b80065b27 100644 --- a/engine/classes/ElggUser.php +++ b/engine/classes/ElggUser.php @@ -65,30 +65,26 @@ class ElggUser extends ElggEntity $msg = elgg_echo('IOException:FailedToLoadGUID', array(get_class(), $guid->guid)); throw new IOException($msg); } - - // See if this is a username } else if (is_string($guid)) { + // $guid is a username $user = get_user_by_username($guid); if ($user) { foreach ($user->attributes as $key => $value) { $this->attributes[$key] = $value; } } - - // Is $guid is an ElggUser? Use a copy constructor } else if ($guid instanceof ElggUser) { + // $guid is an ElggUser so this is a copy constructor elgg_deprecated_notice('This type of usage of the ElggUser constructor was deprecated. Please use the clone method.', 1.7); foreach ($guid->attributes as $key => $value) { $this->attributes[$key] = $value; } - - // Is this is an ElggEntity but not an ElggUser = ERROR! } else if ($guid instanceof ElggEntity) { + // @todo why have a special case here throw new InvalidParameterException(elgg_echo('InvalidParameterException:NonElggUser')); - - // Is it a GUID } else if (is_numeric($guid)) { + // $guid is a GUID so load entity if (!$this->load($guid)) { throw new IOException(elgg_echo('IOException:FailedToLoadGUID', array(get_class(), $guid))); } diff --git a/engine/lib/configuration.php b/engine/lib/configuration.php index a0f297f0c..55e5bbd36 100644 --- a/engine/lib/configuration.php +++ b/engine/lib/configuration.php @@ -486,9 +486,9 @@ function get_config($name, $site_guid = 0) { // @todo these haven't really been implemented in Elgg 1.8. Complete in 1.9. // show dep message if ($new_name) { - // $msg = "Config value $name has been renamed as $new_name"; + // $msg = "Config value $name has been renamed as $new_name"; $name = $new_name; - // elgg_deprecated_notice($msg, $dep_version); + // elgg_deprecated_notice($msg, $dep_version); } // decide from where to return the value diff --git a/engine/lib/elgglib.php b/engine/lib/elgglib.php index 74b70f9fb..281b23535 100644 --- a/engine/lib/elgglib.php +++ b/engine/lib/elgglib.php @@ -1383,8 +1383,8 @@ function elgg_http_build_url(array $parts, $html_encode = TRUE) { * add tokens to the action. The form view automatically handles * tokens. * - * @param string $url Full action URL - * @param bool $html_encode HTML encode the url? (default: false) + * @param string $url Full action URL + * @param bool $html_encode HTML encode the url? (default: false) * * @return string URL with action tokens * @since 1.7.0 @@ -1446,7 +1446,7 @@ function elgg_http_remove_url_query_element($url, $element) { * Adds an element or elements to a URL's query string. * * @param string $url The URL - * @param array $elements Key/value pairs to add to the URL + * @param array $elements Key/value pairs to add to the URL * * @return string The new URL with the query strings added * @since 1.7.0 diff --git a/engine/lib/languages.php b/engine/lib/languages.php index 17db14d98..61ba91ddb 100644 --- a/engine/lib/languages.php +++ b/engine/lib/languages.php @@ -139,6 +139,9 @@ function get_language() { return false; } +/** + * @access private + */ function _elgg_load_translations() { global $CONFIG; diff --git a/engine/lib/location.php b/engine/lib/location.php index b319bb3bb..1534c7d7b 100644 --- a/engine/lib/location.php +++ b/engine/lib/location.php @@ -139,7 +139,7 @@ function elgg_get_entities_from_location(array $options = array()) { /** * Returns a viewable list of entities from location * - * @param array $options + * @param array $options Options array * * @see elgg_list_entities() * @see elgg_get_entities_from_location() diff --git a/engine/lib/metadata.php b/engine/lib/metadata.php index 305e9918b..a1ebfa5f1 100644 --- a/engine/lib/metadata.php +++ b/engine/lib/metadata.php @@ -920,8 +920,8 @@ function elgg_get_metadata_cache() { * Invalidate the metadata cache based on options passed to various *_metadata functions * * @param string $action Action performed on metadata. "delete", "disable", or "enable" - * - * @param array $options Options passed to elgg_(delete|disable|enable)_metadata + * @param array $options Options passed to elgg_(delete|disable|enable)_metadata + * @return void */ function elgg_invalidate_metadata_cache($action, array $options) { // remove as little as possible, optimizing for common cases diff --git a/engine/lib/plugins.php b/engine/lib/plugins.php index f281b1416..6fc000cf9 100644 --- a/engine/lib/plugins.php +++ b/engine/lib/plugins.php @@ -865,7 +865,7 @@ function elgg_set_plugin_user_setting($name, $value, $user_guid = null, $plugin_ * Unsets a user-specific plugin setting * * @param string $name Name of the setting - * @param int $user_guid Defaults to logged in user + * @param int $user_guid Defaults to logged in user * @param string $plugin_id Defaults to contextual plugin name * * @return bool diff --git a/engine/lib/relationships.php b/engine/lib/relationships.php index fe0b8364d..b0cd627fc 100644 --- a/engine/lib/relationships.php +++ b/engine/lib/relationships.php @@ -363,7 +363,7 @@ $relationship_guid = NULL, $inverse_relationship = FALSE) { /** * Returns a viewable list of entities by relationship * - * @param array $options + * @param array $options Options array for retrieval of entities * * @see elgg_list_entities() * @see elgg_get_entities_from_relationship() diff --git a/engine/lib/views.php b/engine/lib/views.php index 7d8347863..c4b349fc6 100644 --- a/engine/lib/views.php +++ b/engine/lib/views.php @@ -1107,7 +1107,7 @@ function elgg_view_entity_annotations(ElggEntity $entity, $full_view = true) { * This is a shortcut for {@elgg_view page/elements/title}. * * @param string $title The page title - * @param array $vars View variables (was submenu be displayed? (deprecated)) + * @param array $vars View variables (was submenu be displayed? (deprecated)) * * @return string The HTML (etc) */ @@ -1179,7 +1179,7 @@ function elgg_view_comments($entity, $add_comment = true, array $vars = array()) * * @param string $image The icon and other information * @param string $body Description content - * @param array $vars Additional parameters for the view + * @param array $vars Additional parameters for the view * * @return string * @since 1.8.0 @@ -1236,15 +1236,17 @@ function elgg_view_river_item($item, array $vars = array()) { // subject is disabled or subject/object deleted return ''; } + + // @todo this needs to be cleaned up // Don't hide objects in closed groups that a user can see. // see http://trac.elgg.org/ticket/4789 -// else { -// // hide based on object's container -// $visibility = ElggGroupItemVisibility::factory($object->container_guid); -// if ($visibility->shouldHideItems) { -// return ''; -// } -// } + // else { + // // hide based on object's container + // $visibility = ElggGroupItemVisibility::factory($object->container_guid); + // if ($visibility->shouldHideItems) { + // return ''; + // } + // } $vars['item'] = $item; -- cgit v1.2.3 From cb8a932702a66a8d8ec69982441e71bde8fd1b5c Mon Sep 17 00:00:00 2001 From: cash Date: Sat, 30 Mar 2013 12:04:19 -0400 Subject: Fixes #4867 handling boolean false values as metadata pair value --- engine/lib/metadata.php | 2 ++ engine/tests/api/metadata.php | 14 ++++++++++++++ 2 files changed, 16 insertions(+) (limited to 'engine/lib/metadata.php') diff --git a/engine/lib/metadata.php b/engine/lib/metadata.php index a1ebfa5f1..ad926a49a 100644 --- a/engine/lib/metadata.php +++ b/engine/lib/metadata.php @@ -619,6 +619,8 @@ $owner_guids = NULL) { // if the operand is IN don't quote it because quoting should be done already. if (is_numeric($pair['value'])) { $value = sanitise_string($pair['value']); + } else if (is_bool($pair['value'])) { + $value = (int) $pair['value']; } else if (is_array($pair['value'])) { $values_array = array(); diff --git a/engine/tests/api/metadata.php b/engine/tests/api/metadata.php index 825290d80..0862341c1 100644 --- a/engine/tests/api/metadata.php +++ b/engine/tests/api/metadata.php @@ -123,6 +123,20 @@ class ElggCoreMetadataAPITest extends ElggCoreUnitTest { $e->delete(); } + /** + * https://github.com/Elgg/Elgg/issues/4867 + */ + public function testElggGetEntityMetadataWhereSqlWithFalseValue() { + $pair = array('name' => 'test' , 'value' => false); + $result = elgg_get_entity_metadata_where_sql('e', 'metadata', null, null, $pair); + $where = preg_replace( '/\s+/', ' ', $result['wheres'][0]); + $this->assertTrue(strpos($where, "msn1.string = 'test' AND BINARY msv1.string = 0") > 0); + + $result = elgg_get_entity_metadata_where_sql('e', 'metadata', array('test'), array(false)); + $where = preg_replace( '/\s+/', ' ', $result['wheres'][0]); + $this->assertTrue(strpos($where, "msn.string IN ('test')) AND ( BINARY msv.string IN ('0')")); + } + // Make sure metadata with multiple values is correctly deleted when re-written // by another user // http://trac.elgg.org/ticket/2776 -- cgit v1.2.3 From 4129637ba28113cea9b27a9644d51354f67e9f55 Mon Sep 17 00:00:00 2001 From: Cash Costello Date: Sat, 11 May 2013 09:59:12 -0400 Subject: Fixes #5419 adds tests for enabling/disabling annotations and fixes bug with deleting disabled annotations when deleting an entity --- engine/lib/annotations.php | 24 +++++++----- engine/lib/entities.php | 11 +++++- engine/lib/metadata.php | 13 +++---- engine/tests/api/annotations.php | 80 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 108 insertions(+), 20 deletions(-) (limited to 'engine/lib/metadata.php') diff --git a/engine/lib/annotations.php b/engine/lib/annotations.php index bd5ea1a1f..81755f169 100644 --- a/engine/lib/annotations.php +++ b/engine/lib/annotations.php @@ -224,7 +224,7 @@ function elgg_get_annotations(array $options = array()) { * annotation_name(s), annotation_value(s), or guid(s) must be set. * * @param array $options An options array. {@See elgg_get_annotations()} - * @return mixed Null if the metadata name is invalid. Bool on success or fail. + * @return bool|null true on success, false on failure, null if no annotations to delete. * @since 1.8.0 */ function elgg_delete_annotations(array $options) { @@ -242,7 +242,7 @@ function elgg_delete_annotations(array $options) { * @warning Unlike elgg_get_annotations() this will not accept an empty options array! * * @param array $options An options array. {@See elgg_get_annotations()} - * @return mixed + * @return bool|null true on success, false on failure, null if no annotations disabled. * @since 1.8.0 */ function elgg_disable_annotations(array $options) { @@ -259,8 +259,11 @@ function elgg_disable_annotations(array $options) { * * @warning Unlike elgg_get_annotations() this will not accept an empty options array! * + * @warning In order to enable annotations, you must first use + * {@link access_show_hidden_entities()}. + * * @param array $options An options array. {@See elgg_get_annotations()} - * @return mixed + * @return bool|null true on success, false on failure, null if no metadata enabled. * @since 1.8.0 */ function elgg_enable_annotations(array $options) { @@ -532,15 +535,16 @@ function elgg_annotation_exists($entity_guid, $annotation_type, $owner_guid = NU return FALSE; } - $entity_guid = (int)$entity_guid; - $annotation_type = sanitise_string($annotation_type); + $entity_guid = sanitize_int($entity_guid); + $owner_guid = sanitize_int($owner_guid); + $annotation_type = sanitize_string($annotation_type); - $sql = "select a.id" . - " FROM {$CONFIG->dbprefix}annotations a, {$CONFIG->dbprefix}metastrings m " . - " WHERE a.owner_guid={$owner_guid} AND a.entity_guid={$entity_guid} " . - " AND a.name_id=m.id AND m.string='{$annotation_type}'"; + $sql = "SELECT a.id FROM {$CONFIG->dbprefix}annotations a" . + " JOIN {$CONFIG->dbprefix}metastrings m ON a.name_id = m.id" . + " WHERE a.owner_guid = $owner_guid AND a.entity_guid = $entity_guid" . + " AND m.string = '$annotation_type'"; - if ($check_annotation = get_data_row($sql)) { + if (get_data_row($sql)) { return TRUE; } diff --git a/engine/lib/entities.php b/engine/lib/entities.php index cb972b282..15ab1170e 100644 --- a/engine/lib/entities.php +++ b/engine/lib/entities.php @@ -1619,8 +1619,8 @@ function disable_entity($guid, $reason = "", $recursive = true) { /** * Enable an entity. * - * @warning In order to enable an entity using ElggEntity::enable(), - * you must first use {@link access_show_hidden_entities()}. + * @warning In order to enable an entity, you must first use + * {@link access_show_hidden_entities()}. * * @param int $guid GUID of entity to enable * @param bool $recursive Recursively enable all entities disabled with the entity? @@ -1748,6 +1748,10 @@ function delete_entity($guid, $recursive = true) { elgg_set_ignore_access($ia); } + $entity_disable_override = access_get_show_hidden_status(); + access_show_hidden_entities(true); + $ia = elgg_set_ignore_access(true); + // Now delete the entity itself $entity->deleteMetadata(); $entity->deleteOwnedMetadata(); @@ -1755,6 +1759,9 @@ function delete_entity($guid, $recursive = true) { $entity->deleteOwnedAnnotations(); $entity->deleteRelationships(); + access_show_hidden_entities($entity_disable_override); + elgg_set_ignore_access($ia); + elgg_delete_river(array('subject_guid' => $guid)); elgg_delete_river(array('object_guid' => $guid)); remove_all_private_settings($guid); diff --git a/engine/lib/metadata.php b/engine/lib/metadata.php index ad926a49a..43f7d5d6e 100644 --- a/engine/lib/metadata.php +++ b/engine/lib/metadata.php @@ -300,10 +300,8 @@ function elgg_get_metadata(array $options = array()) { * This requires at least one constraint: metadata_owner_guid(s), * metadata_name(s), metadata_value(s), or guid(s) must be set. * - * @warning This returns null on no ops. - * * @param array $options An options array. {@see elgg_get_metadata()} - * @return mixed Null if the metadata name is invalid. Bool on success or fail. + * @return bool|null true on success, false on failure, null if no metadata to delete. * @since 1.8.0 */ function elgg_delete_metadata(array $options) { @@ -325,10 +323,8 @@ function elgg_delete_metadata(array $options) { * * @warning Unlike elgg_get_metadata() this will not accept an empty options array! * - * @warning This returns null on no ops. - * * @param array $options An options array. {@See elgg_get_metadata()} - * @return mixed + * @return bool|null true on success, false on failure, null if no metadata disabled. * @since 1.8.0 */ function elgg_disable_metadata(array $options) { @@ -347,10 +343,11 @@ function elgg_disable_metadata(array $options) { * * @warning Unlike elgg_get_metadata() this will not accept an empty options array! * - * @warning This returns null on no ops. + * @warning In order to enable metadata, you must first use + * {@link access_show_hidden_entities()}. * * @param array $options An options array. {@See elgg_get_metadata()} - * @return mixed + * @return bool|null true on success, false on failure, null if no metadata enabled. * @since 1.8.0 */ function elgg_enable_metadata(array $options) { diff --git a/engine/tests/api/annotations.php b/engine/tests/api/annotations.php index 947292970..c0b0687cc 100644 --- a/engine/tests/api/annotations.php +++ b/engine/tests/api/annotations.php @@ -65,6 +65,86 @@ class ElggCoreAnnotationAPITest extends ElggCoreUnitTest { $annotations = elgg_get_annotations($options); $this->assertTrue(empty($annotations)); + // nothing to delete so null returned + $this->assertNull(elgg_delete_annotations($options)); + + $this->assertTrue($e->delete()); + } + + public function testElggDisableAnnotations() { + $e = new ElggObject(); + $e->save(); + + for ($i=0; $i<30; $i++) { + $e->annotate('test_annotation', rand(0,10000)); + } + + $options = array( + 'guid' => $e->getGUID(), + 'limit' => 0 + ); + + $this->assertTrue(elgg_disable_annotations($options)); + + $annotations = elgg_get_annotations($options); + $this->assertTrue(empty($annotations)); + + access_show_hidden_entities(true); + $annotations = elgg_get_annotations($options); + $this->assertIdentical(30, count($annotations)); + access_show_hidden_entities(false); + + $this->assertTrue($e->delete()); + } + + public function testElggEnableAnnotations() { + $e = new ElggObject(); + $e->save(); + + for ($i=0; $i<30; $i++) { + $e->annotate('test_annotation', rand(0,10000)); + } + + $options = array( + 'guid' => $e->getGUID(), + 'limit' => 0 + ); + + $this->assertTrue(elgg_disable_annotations($options)); + + // cannot see any annotations so returns null + $this->assertNull(elgg_enable_annotations($options)); + + access_show_hidden_entities(true); + $this->assertTrue(elgg_enable_annotations($options)); + access_show_hidden_entities(false); + + $annotations = elgg_get_annotations($options); + $this->assertIdentical(30, count($annotations)); + + $this->assertTrue($e->delete()); + } + + public function testElggAnnotationExists() { + $e = new ElggObject(); + $e->save(); + $guid = $e->getGUID(); + + $this->assertFalse(elgg_annotation_exists($guid, 'test_annotation')); + + $e->annotate('test_annotation', rand(0, 10000)); + $this->assertTrue(elgg_annotation_exists($guid, 'test_annotation')); + // this metastring should always exist but an annotation of this name should not + $this->assertFalse(elgg_annotation_exists($guid, 'email')); + + $options = array( + 'guid' => $guid, + 'limit' => 0 + ); + $this->assertTrue(elgg_disable_annotations($options)); + $this->assertTrue(elgg_annotation_exists($guid, 'test_annotation')); + $this->assertTrue($e->delete()); + $this->assertFalse(elgg_annotation_exists($guid, 'test_annotation')); } } -- cgit v1.2.3 From 5a1a793f8634e0f7133644e09e9ace60782864d4 Mon Sep 17 00:00:00 2001 From: Steve Clay Date: Sat, 8 Jun 2013 22:12:12 -0400 Subject: Fixes #5598: Properly update metadata cache in update_metadata() --- engine/lib/metadata.php | 10 +++++----- engine/tests/api/metadata_cache.php | 7 +++++++ 2 files changed, 12 insertions(+), 5 deletions(-) (limited to 'engine/lib/metadata.php') diff --git a/engine/lib/metadata.php b/engine/lib/metadata.php index 43f7d5d6e..046b85124 100644 --- a/engine/lib/metadata.php +++ b/engine/lib/metadata.php @@ -191,19 +191,19 @@ function update_metadata($id, $name, $value, $value_type, $owner_guid, $access_i } // Add the metastring - $value = add_metastring($value); - if (!$value) { + $value_id = add_metastring($value); + if (!$value_id) { return false; } - $name = add_metastring($name); - if (!$name) { + $name_id = add_metastring($name); + if (!$name_id) { return false; } // If ok then add it $query = "UPDATE {$CONFIG->dbprefix}metadata" - . " set name_id='$name', value_id='$value', value_type='$value_type', access_id=$access_id," + . " set name_id='$name_id', value_id='$value_id', value_type='$value_type', access_id=$access_id," . " owner_guid=$owner_guid where id=$id"; $result = update_data($query); diff --git a/engine/tests/api/metadata_cache.php b/engine/tests/api/metadata_cache.php index 846116a7b..7fb328169 100644 --- a/engine/tests/api/metadata_cache.php +++ b/engine/tests/api/metadata_cache.php @@ -166,4 +166,11 @@ class ElggCoreMetadataCacheTest extends ElggCoreUnitTest { $actual = $this->cache->filterMetadataHeavyEntities($guids, 6000); $this->assertIdentical($actual, $expected); } + + public function testCreateMetadataInvalidates() { + $this->obj1->foo = 1; + create_metadata($this->guid1, 'foo', 2, '', elgg_get_logged_in_user_guid(), ACCESS_FRIENDS); + + $this->assertEqual($this->obj1->foo, 2); + } } -- cgit v1.2.3 From 2cab755b9df35f5e338151cf1e94cbce5f544e98 Mon Sep 17 00:00:00 2001 From: Cash Costello Date: Sat, 13 Jul 2013 08:39:58 -0400 Subject: adds warning for those using a string value with the operand "IN" for metadata pairs --- engine/lib/metadata.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'engine/lib/metadata.php') diff --git a/engine/lib/metadata.php b/engine/lib/metadata.php index 046b85124..d2f8d4cd4 100644 --- a/engine/lib/metadata.php +++ b/engine/lib/metadata.php @@ -402,9 +402,11 @@ function elgg_enable_metadata(array $options) { * 'operand' => '=', * 'case_sensitive' => TRUE * ) - * Currently if multiple values are sent via + * Currently if multiple values are sent via * an array (value => array('value1', 'value2') * the pair's operand will be forced to "IN". + * If passing "IN" as the operand and a string as the value, + * the value must be a properly quoted and escaped string. * * metadata_name_value_pairs_operator => NULL|STR The operator to use for combining * (name = value) OPERATOR (name = value); default AND -- cgit v1.2.3 From 1e884c6abe933468ecfa035780bd3e9ff5e1ad61 Mon Sep 17 00:00:00 2001 From: Matt Beckett Date: Thu, 10 Oct 2013 13:29:50 -0700 Subject: #5952 - fix infinite loop when disabling > 50 annotations or metadata --- engine/lib/annotations.php | 6 +++++- engine/lib/metadata.php | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) (limited to 'engine/lib/metadata.php') 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); } /** -- cgit v1.2.3