From 85e4c16f39a8b00b229644bcd175663541dfd51a Mon Sep 17 00:00:00 2001
From: Steve Clay <steve@mrclay.org>
Date: Mon, 4 Feb 2013 20:37:25 -0500
Subject: Doc fixes and inline type hints for variables (big static analysis
 cleanup)

---
 engine/lib/entities.php | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

(limited to 'engine/lib/entities.php')

diff --git a/engine/lib/entities.php b/engine/lib/entities.php
index ce736ce05..0361ae195 100644
--- a/engine/lib/entities.php
+++ b/engine/lib/entities.php
@@ -531,6 +531,7 @@ function can_write_to_container($user_guid = 0, $container_guid = 0, $type = 'al
 		// If still not approved, see if the user is a member of the group
 		// @todo this should be moved to the groups plugin/library
 		if (!$return && $user && $container instanceof ElggGroup) {
+			/* @var ElggGroup $container */
 			if ($container->isMember($user)) {
 				$return = true;
 			}
@@ -1463,7 +1464,7 @@ function elgg_list_entities(array $options = array(), $getter = 'elgg_get_entiti
  *
  * @param string $type           The type of entity
  * @param string $subtype        The subtype of entity
- * @param int    $container_guid The container GUID that the entinties belong to
+ * @param int    $container_guid The container GUID that the entities belong to
  * @param int    $site_guid      The site GUID
  * @param string $order_by       Order_by SQL order by clause
  *
@@ -2413,6 +2414,7 @@ function elgg_instanceof($entity, $type = NULL, $subtype = NULL, $class = NULL)
 	$return = ($entity instanceof ElggEntity);
 
 	if ($type) {
+		/* @var ElggEntity $entity */
 		$return = $return && ($entity->getType() == $type);
 	}
 
-- 
cgit v1.2.3


From 78e0ec583619cbb728cbe03df857c8219acaac7e Mon Sep 17 00:00:00 2001
From: Steve Clay <steve@mrclay.org>
Date: Mon, 4 Feb 2013 20:55:04 -0500
Subject: simplify flow, style

---
 engine/classes/ElggAttributeLoader.php |  8 ++++----
 engine/lib/annotations.php             |  2 +-
 engine/lib/entities.php                | 21 +++++++++++----------
 engine/lib/notification.php            |  2 +-
 engine/lib/objects.php                 |  2 +-
 engine/lib/plugins.php                 |  2 +-
 engine/lib/users.php                   |  4 ++--
 7 files changed, 21 insertions(+), 20 deletions(-)

(limited to 'engine/lib/entities.php')

diff --git a/engine/classes/ElggAttributeLoader.php b/engine/classes/ElggAttributeLoader.php
index 602bb8bae..2d1c1abde 100644
--- a/engine/classes/ElggAttributeLoader.php
+++ b/engine/classes/ElggAttributeLoader.php
@@ -148,11 +148,11 @@ class ElggAttributeLoader {
 				if (!is_callable($this->primary_loader)) {
 					throw new LogicException('Primary attribute loader must be callable');
 				}
-				if (!$this->requires_access_control) {
+				if ($this->requires_access_control) {
+					$fetched = (array) call_user_func($this->primary_loader, $row['guid']);
+				} else {
 					$ignoring_access = elgg_set_ignore_access();
-				}
-				$fetched = (array) call_user_func($this->primary_loader, $row['guid']);
-				if (!$this->requires_access_control) {
+					$fetched = (array) call_user_func($this->primary_loader, $row['guid']);
 					elgg_set_ignore_access($ignoring_access);
 				}
 				if (!$fetched) {
diff --git a/engine/lib/annotations.php b/engine/lib/annotations.php
index 3d94a1d55..a95aba8f8 100644
--- a/engine/lib/annotations.php
+++ b/engine/lib/annotations.php
@@ -469,7 +469,7 @@ function export_annotation_plugin_hook($hook, $entity_type, $returnvalue, $param
 
 	$result = elgg_get_annotations(array(
 		'guid' => $guid,
-		'limit' => 0
+		'limit' => 0,
 	));
 
 	if ($result) {
diff --git a/engine/lib/entities.php b/engine/lib/entities.php
index 0361ae195..e3535c741 100644
--- a/engine/lib/entities.php
+++ b/engine/lib/entities.php
@@ -1120,16 +1120,17 @@ function _elgg_fetch_entities_from_sql($sql) {
 	// Do secondary queries and merge rows
 	if ($lookup_types) {
 		$dbprefix = elgg_get_config('dbprefix');
-	}
-	foreach ($lookup_types as $type => $guids) {
-		$set = "(" . implode(',', $guids) . ")";
-		$sql = "SELECT * FROM {$dbprefix}{$type}s_entity WHERE guid IN $set";
-		$secondary_rows = get_data($sql);
-		if ($secondary_rows) {
-			foreach ($secondary_rows as $secondary_row) {
-				$key = $guid_to_key[$secondary_row->guid];
-				// cast to arrays to merge then cast back
-				$rows[$key] = (object)array_merge((array)$rows[$key], (array)$secondary_row);
+
+		foreach ($lookup_types as $type => $guids) {
+			$set = "(" . implode(',', $guids) . ")";
+			$sql = "SELECT * FROM {$dbprefix}{$type}s_entity WHERE guid IN $set";
+			$secondary_rows = get_data($sql);
+			if ($secondary_rows) {
+				foreach ($secondary_rows as $secondary_row) {
+					$key = $guid_to_key[$secondary_row->guid];
+					// cast to arrays to merge then cast back
+					$rows[$key] = (object)array_merge((array)$rows[$key], (array)$secondary_row);
+				}
 			}
 		}
 	}
diff --git a/engine/lib/notification.php b/engine/lib/notification.php
index 4191407fc..d72b1352a 100644
--- a/engine/lib/notification.php
+++ b/engine/lib/notification.php
@@ -264,7 +264,7 @@ array $params = NULL) {
 	$to = $to->email;
 
 	// From
-	$site = get_entity($CONFIG->site_guid);
+	$site = elgg_get_site_entity();
 	// If there's an email address, use it - but only if its not from a user.
 	if (!($from instanceof ElggUser) && $from->email) {
 		$from = $from->email;
diff --git a/engine/lib/objects.php b/engine/lib/objects.php
index 81b6b7f12..ff3cc733f 100644
--- a/engine/lib/objects.php
+++ b/engine/lib/objects.php
@@ -95,7 +95,7 @@ function get_object_sites($object_guid, $limit = 10, $offset = 0) {
 		'relationship_guid' => $object_guid,
 		'type' => 'site',
 		'limit' => $limit,
-		'offset' => $offset
+		'offset' => $offset,
 	));
 }
 
diff --git a/engine/lib/plugins.php b/engine/lib/plugins.php
index 973ca4026..d6f03c795 100644
--- a/engine/lib/plugins.php
+++ b/engine/lib/plugins.php
@@ -139,7 +139,7 @@ function elgg_generate_plugin_entities() {
 			$index = $id_map[$plugin_id];
 			$plugin = $known_plugins[$index];
 			// was this plugin deleted and its entity disabled?
-			if ($plugin->enabled != 'yes') {
+			if (!$plugin->isEnabled()) {
 				$plugin->enable();
 				$plugin->deactivate();
 				$plugin->setPriority('last');
diff --git a/engine/lib/users.php b/engine/lib/users.php
index 7365a6b68..c4e06895d 100644
--- a/engine/lib/users.php
+++ b/engine/lib/users.php
@@ -1071,10 +1071,10 @@ function collections_submenu_items() {
  * @return bool
  * @access private
  */
-function friends_page_handler($page_elements, $handler) {
+function friends_page_handler($segments, $handler) {
 	elgg_set_context('friends');
 	
-	if (isset($page_elements[0]) && $user = get_user_by_username($page_elements[0])) {
+	if (isset($segments[0]) && $user = get_user_by_username($segments[0])) {
 		elgg_set_page_owner_guid($user->getGUID());
 	}
 	if (elgg_get_logged_in_user_guid() == elgg_get_page_owner_guid()) {
-- 
cgit v1.2.3


From 011485988a8fa734c2ed74fec21031b3beebbcfd Mon Sep 17 00:00:00 2001
From: Steve Clay <steve@mrclay.org>
Date: Mon, 4 Feb 2013 21:09:02 -0500
Subject: Lots of new @todos :(

---
 engine/classes/ElggDiskFilestore.php | 1 +
 engine/classes/ElggFile.php          | 3 +++
 engine/classes/ElggGroup.php         | 3 +++
 engine/classes/ElggMenuBuilder.php   | 2 ++
 engine/classes/ElggPlugin.php        | 2 ++
 engine/lib/annotations.php           | 1 +
 engine/lib/calendar.php              | 2 ++
 engine/lib/database.php              | 2 ++
 engine/lib/entities.php              | 3 +++
 engine/lib/input.php                 | 5 +++++
 engine/lib/metastrings.php           | 1 +
 engine/lib/notification.php          | 3 ++-
 engine/lib/plugins.php               | 2 ++
 engine/lib/system_log.php            | 2 ++
 14 files changed, 31 insertions(+), 1 deletion(-)

(limited to 'engine/lib/entities.php')

diff --git a/engine/classes/ElggDiskFilestore.php b/engine/classes/ElggDiskFilestore.php
index 2d86e7c2d..7374aad35 100644
--- a/engine/classes/ElggDiskFilestore.php
+++ b/engine/classes/ElggDiskFilestore.php
@@ -60,6 +60,7 @@ class ElggDiskFilestore extends ElggFilestore {
 
 		$path = substr($fullname, 0, $ls);
 		$name = substr($fullname, $ls);
+		// @todo $name is unused, remove it or do we need to fix something?
 
 		// Try and create the directory
 		try {
diff --git a/engine/classes/ElggFile.php b/engine/classes/ElggFile.php
index 532db3617..3e9c24c17 100644
--- a/engine/classes/ElggFile.php
+++ b/engine/classes/ElggFile.php
@@ -93,6 +93,7 @@ class ElggFile extends ElggObject {
 			$container_guid = $this->container_guid;
 		}
 		$fs = $this->getFilestore();
+		// @todo add getSize() to ElggFilestore
 		return $fs->getSize($prefix, $container_guid);
 	}
 
@@ -289,6 +290,7 @@ class ElggFile extends ElggObject {
 	public function seek($position) {
 		$fs = $this->getFilestore();
 
+		// @todo add seek() to ElggFilestore
 		return $fs->seek($this->handle, $position);
 	}
 
@@ -393,6 +395,7 @@ class ElggFile extends ElggObject {
 
 			$this->filestore = new $filestore();
 			$this->filestore->setParameters($parameters);
+			// @todo explain why $parameters will always be set here (PhpStorm complains)
 		}
 
 		// this means the entity hasn't been saved so fallback to default
diff --git a/engine/classes/ElggGroup.php b/engine/classes/ElggGroup.php
index 24bdee79e..61f699f1a 100644
--- a/engine/classes/ElggGroup.php
+++ b/engine/classes/ElggGroup.php
@@ -220,6 +220,7 @@ class ElggGroup extends ElggEntity
 	 * @return array|false
 	 */
 	public function getObjects($subtype = "", $limit = 10, $offset = 0) {
+		// @todo are we deprecating this method, too?
 		return get_objects_in_group($this->getGUID(), $subtype, 0, 0, "", $limit, $offset, false);
 	}
 
@@ -233,6 +234,7 @@ class ElggGroup extends ElggEntity
 	 * @return array|false
 	 */
 	public function getFriendsObjects($subtype = "", $limit = 10, $offset = 0) {
+		// @todo are we deprecating this method, too?
 		return get_objects_in_group($this->getGUID(), $subtype, 0, 0, "", $limit, $offset, false);
 	}
 
@@ -244,6 +246,7 @@ class ElggGroup extends ElggEntity
 	 * @return array|false
 	 */
 	public function countObjects($subtype = "") {
+		// @todo are we deprecating this method, too?
 		return get_objects_in_group($this->getGUID(), $subtype, 0, 0, "", 10, 0, true);
 	}
 
diff --git a/engine/classes/ElggMenuBuilder.php b/engine/classes/ElggMenuBuilder.php
index 6da951597..0dce02be4 100644
--- a/engine/classes/ElggMenuBuilder.php
+++ b/engine/classes/ElggMenuBuilder.php
@@ -274,6 +274,8 @@ class ElggMenuBuilder {
 	 * @param ElggMenuItem $a
 	 * @param ElggMenuItem $b
 	 * @return bool
+	 *
+	 * @todo deprecate this? weight seems to be deprecated
 	 */
 	public static function compareByWeight($a, $b) {
 		$aw = $a->getWeight();
diff --git a/engine/classes/ElggPlugin.php b/engine/classes/ElggPlugin.php
index 066fd9a79..ae447bddb 100644
--- a/engine/classes/ElggPlugin.php
+++ b/engine/classes/ElggPlugin.php
@@ -597,6 +597,8 @@ class ElggPlugin extends ElggObject {
 	 * Checks if this plugin can be activated on the current
 	 * Elgg installation.
 	 *
+	 * @todo remove $site_guid param or implement it
+	 *
 	 * @param mixed $site_guid Optional site guid
 	 * @return bool
 	 */
diff --git a/engine/lib/annotations.php b/engine/lib/annotations.php
index 41a736aa1..f40a2cc6f 100644
--- a/engine/lib/annotations.php
+++ b/engine/lib/annotations.php
@@ -17,6 +17,7 @@
  */
 function row_to_elggannotation($row) {
 	if (!($row instanceof stdClass)) {
+		// @todo should throw in this case?
 		return $row;
 	}
 
diff --git a/engine/lib/calendar.php b/engine/lib/calendar.php
index 9a06c5292..e6f95934c 100644
--- a/engine/lib/calendar.php
+++ b/engine/lib/calendar.php
@@ -39,6 +39,8 @@ function get_day_end($day = null, $month = null, $year = null) {
 /**
  * Return the notable entities for a given time period.
  *
+ * @todo this function also accepts an array(type => subtypes) for 3rd arg. Should we document this?
+ *
  * @param int     $start_time     The start time as a unix timestamp.
  * @param int     $end_time       The end time as a unix timestamp.
  * @param string  $type           The type of entity (eg "user", "object" etc)
diff --git a/engine/lib/database.php b/engine/lib/database.php
index dcf50545d..2b348366d 100644
--- a/engine/lib/database.php
+++ b/engine/lib/database.php
@@ -123,6 +123,8 @@ function establish_db_link($dblinkname = "readwrite") {
 
 	// Set up cache if global not initialized and query cache not turned off
 	if ((!$DB_QUERY_CACHE) && (!$db_cache_off)) {
+		// @todo everywhere else this is assigned to array(), making it dangerous to call
+		// object methods on this. We should consider making this an plain array
 		$DB_QUERY_CACHE = new ElggStaticVariableCache('db_query_cache');
 	}
 }
diff --git a/engine/lib/entities.php b/engine/lib/entities.php
index e3535c741..25c927ac6 100644
--- a/engine/lib/entities.php
+++ b/engine/lib/entities.php
@@ -1220,6 +1220,7 @@ function elgg_get_entity_type_subtype_where_sql($table, $types, $subtypes, $pair
 			if ($subtypes) {
 				foreach ($subtypes as $subtype) {
 					// check that the subtype is valid (with ELGG_ENTITIES_NO_VALUE being a valid subtype)
+					// @todo simplify this logic
 					if (ELGG_ENTITIES_NO_VALUE === $subtype || $subtype_id = get_subtype_id($type, $subtype)) {
 						$subtype_ids[] = (ELGG_ENTITIES_NO_VALUE === $subtype) ? ELGG_ENTITIES_NO_VALUE : $subtype_id;
 					} else {
@@ -1461,6 +1462,8 @@ function elgg_list_entities(array $options = array(), $getter = 'elgg_get_entiti
  *
  * @tip Use this to generate a list of archives by month for when entities were added or updated.
  *
+ * @todo document how to pass in array for $subtype
+ *
  * @warning Months are returned in the form YYYYMM.
  *
  * @param string $type           The type of entity
diff --git a/engine/lib/input.php b/engine/lib/input.php
index bbb9ff94d..2d9bae4dd 100644
--- a/engine/lib/input.php
+++ b/engine/lib/input.php
@@ -226,6 +226,8 @@ function elgg_clear_sticky_value($form_name, $variable) {
 /**
  * Page handler for autocomplete endpoint.
  *
+ * @todo split this into functions/objects, this is way too big
+ *
  * /livesearch?q=<query>
  *
  * Other options include:
@@ -288,6 +290,7 @@ function input_livesearch_page_handler($page) {
 
 				if ($entities = get_data($query)) {
 					foreach ($entities as $entity) {
+						// @todo use elgg_get_entities (don't query in a loop!)
 						$entity = get_entity($entity->guid);
 						/* @var ElggUser $entity */
 						if (!$entity) {
@@ -338,6 +341,7 @@ function input_livesearch_page_handler($page) {
 				";
 				if ($entities = get_data($query)) {
 					foreach ($entities as $entity) {
+						// @todo use elgg_get_entities (don't query in a loop!)
 						$entity = get_entity($entity->guid);
 						/* @var ElggGroup $entity */
 						if (!$entity) {
@@ -386,6 +390,7 @@ function input_livesearch_page_handler($page) {
 
 				if ($entities = get_data($query)) {
 					foreach ($entities as $entity) {
+						// @todo use elgg_get_entities (don't query in a loop!)
 						$entity = get_entity($entity->guid);
 						/* @var ElggUser $entity */
 						if (!$entity) {
diff --git a/engine/lib/metastrings.php b/engine/lib/metastrings.php
index 7e18597a6..76c4bd8c4 100644
--- a/engine/lib/metastrings.php
+++ b/engine/lib/metastrings.php
@@ -804,6 +804,7 @@ function elgg_delete_metastring_based_object_by_id($id, $type) {
 			}
 
 			if ($metabyname_memcache) {
+				// @todo why name_id? is that even populated?
 				$metabyname_memcache->delete("{$obj->entity_guid}:{$obj->name_id}");
 			}
 		}
diff --git a/engine/lib/notification.php b/engine/lib/notification.php
index f0aff84e6..56e591192 100644
--- a/engine/lib/notification.php
+++ b/engine/lib/notification.php
@@ -174,7 +174,8 @@ function get_user_notification_settings($user_guid = 0) {
 		$user_guid = elgg_get_logged_in_user_guid();
 	}
 
-	// @todo: holy crap, really?
+	// @todo: there should be a better way now that metadata is cached. E.g. just query for MD names, then
+	// query user object directly
 	$all_metadata = elgg_get_metadata(array(
 		'guid' => $user_guid,
 		'limit' => 0
diff --git a/engine/lib/plugins.php b/engine/lib/plugins.php
index 0ea4404f3..f281b1416 100644
--- a/engine/lib/plugins.php
+++ b/engine/lib/plugins.php
@@ -91,7 +91,9 @@ function elgg_get_plugin_ids_in_dir($dir = null) {
  * @access private
  */
 function elgg_generate_plugin_entities() {
+	// @todo $site unused, can remove?
 	$site = get_config('site');
+
 	$dir = elgg_get_plugins_path();
 	$db_prefix = elgg_get_config('dbprefix');
 
diff --git a/engine/lib/system_log.php b/engine/lib/system_log.php
index 38bc4ba96..5a153afb2 100644
--- a/engine/lib/system_log.php
+++ b/engine/lib/system_log.php
@@ -10,6 +10,8 @@
 /**
  * Retrieve the system log based on a number of parameters.
  *
+ * @todo too many args, and the first arg is too confusing
+ *
  * @param int|array $by_user    The guid(s) of the user(s) who initiated the event.
  *                              Use 0 for unowned entries. Anything else falsey means anyone.
  * @param string    $event      The event you are searching on.
-- 
cgit v1.2.3


From 8f7bd6a446c85d8200e384ad320ea2507a850b0e Mon Sep 17 00:00:00 2001
From: Brett Profitt <brett.profitt@gmail.com>
Date: Wed, 27 Feb 2013 10:36:47 -0500
Subject: Fixes #5066. Empty subtypes are handled the same way for both
 'subtype' and 'subtypes'

---
 engine/lib/entities.php                      | 24 ++++++++++++++------
 engine/tests/api/entity_getter_functions.php | 34 ++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 7 deletions(-)

(limited to 'engine/lib/entities.php')

diff --git a/engine/lib/entities.php b/engine/lib/entities.php
index 25c927ac6..156eec040 100644
--- a/engine/lib/entities.php
+++ b/engine/lib/entities.php
@@ -1219,14 +1219,24 @@ function elgg_get_entity_type_subtype_where_sql($table, $types, $subtypes, $pair
 			$subtype_ids = array();
 			if ($subtypes) {
 				foreach ($subtypes as $subtype) {
-					// check that the subtype is valid (with ELGG_ENTITIES_NO_VALUE being a valid subtype)
-					// @todo simplify this logic
-					if (ELGG_ENTITIES_NO_VALUE === $subtype || $subtype_id = get_subtype_id($type, $subtype)) {
-						$subtype_ids[] = (ELGG_ENTITIES_NO_VALUE === $subtype) ? ELGG_ENTITIES_NO_VALUE : $subtype_id;
-					} else {
-						$valid_subtypes_count--;
-						elgg_log("Type-subtype '$type:$subtype' does not exist!", 'NOTICE');
+					// check that the subtype is valid
+					if (!$subtype && ELGG_ENTITIES_NO_VALUE === $subtype) {
+						// subtype value is 0
+						$subtype_ids[] = ELGG_ENTITIES_NO_VALUE;
+					} elseif (!$subtype) {
+						// subtype is ignored.
+						// this handles ELGG_ENTITIES_ANY_VALUE, '', and anything falsy that isn't 0
 						continue;
+					} else {
+						$subtype_id = get_subtype_id($type, $subtype);
+						
+						if ($subtype_id) {
+							$subtype_ids[] = $subtype_id;
+						} else {
+							$valid_subtypes_count--;
+							elgg_log("Type-subtype '$type:$subtype' does not exist!", 'NOTICE');
+							continue;
+						}
 					}
 				}
 
diff --git a/engine/tests/api/entity_getter_functions.php b/engine/tests/api/entity_getter_functions.php
index 6f7a6145e..52470e19d 100644
--- a/engine/tests/api/entity_getter_functions.php
+++ b/engine/tests/api/entity_getter_functions.php
@@ -2817,4 +2817,38 @@ class ElggCoreEntityGetterFunctionsTest extends ElggCoreUnitTest {
 		$entities = elgg_get_entities($options);
 		$this->assertFalse($entities);
 	}
+
+	public function testEGEEmptySubtypePlurality() {
+		$options = array(
+			'type' => 'user',
+			'subtypes' => ''
+		);
+
+		$entities = elgg_get_entities($options);
+		$this->assertTrue(is_array($entities));
+
+		$options = array(
+			'type' => 'user',
+			'subtype' => ''
+		);
+
+		$entities = elgg_get_entities($options);
+		$this->assertTrue(is_array($entities));
+
+		$options = array(
+			'type' => 'user',
+			'subtype' => array('')
+		);
+
+		$entities = elgg_get_entities($options);
+		$this->assertTrue(is_array($entities));
+
+		$options = array(
+			'type' => 'user',
+			'subtypes' => array('')
+		);
+
+		$entities = elgg_get_entities($options);
+		$this->assertTrue(is_array($entities));
+	}
 }
-- 
cgit v1.2.3