Refs #1242, Fixes #2775. Pulled common code between get_data() and get_data_row(...

git-svn-id: http://code.elgg.org/elgg/trunk@7849 36083f99-b078-4883-b0ff-0f9b5a30f544

diff --git a/engine/lib/database.php b/engine/lib/database.php
index da068effdc3ccac5bf5f7549fe4185c876200743..d024270295c25d2a0b6c5db13861e36df01b2ab7 100644 (file)
--- a/engine/lib/database.php
+++ b/engine/lib/database.php
@@ -244,14 +244,11 @@ function explain_query($query, $link) {
  * @throws DatabaseException
  */
 function execute_query($query, $dblink) {
-       global $CONFIG, $dbcalls, $DB_QUERY_CACHE;
+       global $CONFIG, $dbcalls;
 
        $dbcalls++;
 
        $result = mysql_query($query, $dblink);
-       if ($DB_QUERY_CACHE) {
-               $DB_QUERY_CACHE[$query] = -1; // Set initial cache to -1
-       }
 
        if (mysql_errno($dblink)) {
                throw new DatabaseException(mysql_error($dblink) . "\n\n QUERY: " . $query);
@@ -327,59 +324,14 @@ function execute_delayed_read_query($query, $handler = "") {
  * argument to $callback.  If no callback function is defined, the
  * entire result set is returned as an array.
  *
- * If no results are matched, FALSE is returned.
- *
  * @param mixed  $query    The query being passed.
  * @param string $callback Optionally, the name of a function to call back to on each row
  *
- * @return array|false An array of database result objects or callback function results or false
+ * @return array An array of database result objects or callback function results. If the query
+ *               returned nothing, an empty array.
  */
 function get_data($query, $callback = "") {
-       global $CONFIG, $DB_QUERY_CACHE;
-
-       // Is cached?
-       if ($DB_QUERY_CACHE) {
-               $cached_query = $DB_QUERY_CACHE[$query];
-       }
-
-       if ((isset($cached_query)) && ($cached_query)) {
-               elgg_log("$query results returned from cache");
-
-               if ($cached_query === -1) {
-                       // Last time this query returned nothing, so return an empty array
-                       return array();
-               }
-
-               return $cached_query;
-       }
-
-       $dblink = get_db_link('read');
-       $resultarray = array();
-
-       if ($result = execute_query("$query", $dblink)) {
-               while ($row = mysql_fetch_object($result)) {
-                       if (!empty($callback) && is_callable($callback)) {
-                               $row = $callback($row);
-                       }
-                       if ($row) {
-                               $resultarray[] = $row;
-                       }
-               }
-       }
-
-       if (empty($resultarray)) {
-               elgg_log("DB query \"$query\" returned no results.");
-               // @todo consider changing this to return empty array #1242
-               return FALSE;
-       }
-
-       // Cache result
-       if ($DB_QUERY_CACHE) {
-               $DB_QUERY_CACHE[$query] = $resultarray;
-               elgg_log("$query results cached");
-       }
-
-       return $resultarray;
+       return elgg_query_runner($query, $callback, false);
 }
 
 /**
@@ -395,47 +347,74 @@ function get_data($query, $callback = "") {
  * @return mixed A single database result object or the result of the callback function.
  */
 function get_data_row($query, $callback = "") {
+       return elgg_query_runner($query, $callback, true);
+}
+
+/**
+ * Handles returning data from a query, running it through a callback function,
+ * and caching the results.
+ *
+ * @access private
+ *
+ * @param string $query    The query to execute
+ * @param string $callback An optional callback function to run on each row
+ * @param bool   $single   Return only a single result?
+ *
+ * @return array An array of database result objects or callback function results. If the query
+ *               returned nothing, an empty array.
+ * @since 1.8
+ */
+function elgg_query_runner($query, $callback = null, $single = false) {
        global $CONFIG, $DB_QUERY_CACHE;
 
-       // Is cached
-       if ($DB_QUERY_CACHE) {
-               $cached_query = $DB_QUERY_CACHE[$query];
-       }
+       // since we want to cache results of running the callback, we need to
+       // need to namespace the query with the callback, and single result request.
+       $hash = (string)$callback . (string)$single . $query;
 
-       if ((isset($cached_query)) && ($cached_query)) {
-               elgg_log("$query results returned from cache");
+       // Is cached?
+       if ($DB_QUERY_CACHE) {
+               $cached_query = $DB_QUERY_CACHE[$hash];
 
-               if ($cached_query === -1) {
-                       // Last time this query returned nothing, so return false
-                       //@todo fix me this should return array().
-                       return FALSE;
+               if ($cached_query !== FALSE) {
+                       elgg_log("$query results returned from cache (hash: $hash)");
+                       return $cached_query;
                }
-
-               return $cached_query;
        }
 
        $dblink = get_db_link('read');
+       $return = array();
 
        if ($result = execute_query("$query", $dblink)) {
-               $row = mysql_fetch_object($result);
 
-               // Cache result (even if query returned no data)
-               if ($DB_QUERY_CACHE) {
-                       $DB_QUERY_CACHE[$query] = $row;
-                       elgg_log("$query results cached");
-               }
-
-               if (!empty($callback) && is_callable($callback)) {
+               // test for callback once instead of on each iteration.
+               // @todo check profiling to see if this needs to be broken out into
+               // explicit cases instead of checking in the interation.
+               $is_callable = is_callable($callback);
+               while ($row = mysql_fetch_object($result)) {
+                       if ($is_callable) {
                                $row = $callback($row);
-               }
+                       }
 
-               if ($row) {
-                       return $row;
+                       if ($single) {
+                               $return = $row;
+                               break;
+                       } else {
+                               $return[] = $row;
+                       }
                }
        }
 
-       elgg_log("$query returned no results.");
-       return FALSE;
+       if (empty($return)) {
+               elgg_log("DB query \"$query\" returned no results.");
+       }
+
+       // Cache result
+       if ($DB_QUERY_CACHE) {
+               $DB_QUERY_CACHE[$hash] = $return;
+               elgg_log("$query results cached (hash: $hash)");
+       }
+
+       return $return;
 }
 
 /**

diff --git a/engine/tests/objects/entities.php b/engine/tests/objects/entities.php
index 56c8f79471108e7a3ffae84bfa3b295b0a5e2c8e..8e7e30c5bbca14ec1ff747faf7c896b2f68b77a4 100644 (file)
--- a/engine/tests/objects/entities.php
+++ b/engine/tests/objects/entities.php
@@ -118,7 +118,7 @@ class ElggCoreEntityTest extends ElggCoreUnitTest {
                $this->assertIsA($annotations[0], 'ElggAnnotation');
                $this->assertIdentical($annotations[0]->name, 'non_existent');
                $this->assertEqual($this->entity->countAnnotations('non_existent'), 1);
-               
+
                $this->assertIdentical($annotations, get_annotations($this->entity->getGUID()));
                $this->assertIdentical($annotations, get_annotations($this->entity->getGUID(), 'site'));
                $this->assertIdentical($annotations, get_annotations($this->entity->getGUID(), 'site', 'testing'));
@@ -127,10 +127,10 @@ class ElggCoreEntityTest extends ElggCoreUnitTest {
                //  clear annotation
                $this->assertTrue($this->entity->clearAnnotations());
                $this->assertEqual($this->entity->countAnnotations('non_existent'), 0);
-               
-               $this->assertIdentical(FALSE, get_annotations($this->entity->getGUID()));
-               $this->assertIdentical(FALSE, get_annotations($this->entity->getGUID(), 'site'));
-               $this->assertIdentical(FALSE, get_annotations($this->entity->getGUID(), 'site', 'testing'));
+
+               $this->assertIdentical(array(), get_annotations($this->entity->getGUID()));
+               $this->assertIdentical(array(), get_annotations($this->entity->getGUID(), 'site'));
+               $this->assertIdentical(array(), get_annotations($this->entity->getGUID(), 'site', 'testing'));
 
                // clean up
                $this->assertTrue($this->entity->delete());
@@ -201,30 +201,30 @@ class ElggCoreEntityTest extends ElggCoreUnitTest {
                $this->assertTrue($this->entity->enable());
                $this->assertTrue($this->entity->delete());
        }
-       
+
        public function testElggEntityMetadata() {
                // let's delte a non-existent metadata
                $this->assertFalse($this->entity->clearMetaData('important'));
-               
+
                // let's add the meatadata
                $this->assertTrue($this->entity->important = 'indeed!');
                $this->assertTrue($this->entity->less_important = 'true, too!');
                $this->save_entity();
-               
+
                // test deleting incorrectly
                // @link http://trac.elgg.org/ticket/2273
                $this->assertFalse($this->entity->clearMetaData('impotent'));
                $this->assertEqual($this->entity->important, 'indeed!');
-               
+
                // get rid of one metadata
                $this->assertEqual($this->entity->important, 'indeed!');
                $this->assertTrue($this->entity->clearMetaData('important'));
                $this->assertEqual($this->entity->important, '');
-               
+
                // get rid of all metadata
                $this->assertTrue($this->entity->clearMetaData());
                $this->assertEqual($this->entity->less_important, '');
-               
+
                // clean up database
                $this->assertTrue($this->entity->delete());
        }