From 82fe58de8541f2287ca28656c2121e3e3496c86f Mon Sep 17 00:00:00 2001 From: brettp Date: Thu, 6 Jan 2011 04:51:31 +0000 Subject: Refs #1242, Fixes #2775. Pulled common code between get_data() and get_data_row() into elgg_query_runner(). Caching is now done in the query runner instead of spread across execute_query() and get_data*(). Namespacing cache for callback and single row request. I'm open to better names for that function. Not closing #1242 because the elgg_get_entities*() functions can still return false. git-svn-id: http://code.elgg.org/elgg/trunk@7849 36083f99-b078-4883-b0ff-0f9b5a30f544 --- engine/lib/database.php | 133 ++++++++++++++++---------------------- engine/tests/objects/entities.php | 22 +++---- 2 files changed, 67 insertions(+), 88 deletions(-) (limited to 'engine') diff --git a/engine/lib/database.php b/engine/lib/database.php index da068effd..d02427029 100644 --- 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 56c8f7947..8e7e30c5b 100644 --- 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()); } -- cgit v1.2.3