Merge PR #974 branch 'bantu/ticket/10875' into develop

* bantu/ticket/10875:
  [ticket/10875] tests/cache/cache_test.php: Use single quotes where possible.
  [ticket/10875] Test for null cache driver and sql cache.
  [ticket/10875] Revise sql cache test.
  [ticket/10875] Must return query result on failure.
  [ticket/10875] More documentation.
  [ticket/10875] Spelling fix.
  [ticket/10875] Add docblocks for phpbb_cache_driver_interface
  [ticket/10875] Add comment about cache's sql_load() method.
  [ticket/10875] Fix logic in phpbb_cache_driver_file::sql_save().
  [ticket/10875] Fix phpbb_mock_cache::sql_save() to return $query_result.
  [ticket/10875] Fix return value of phpbb_cache_driver_null::sql_save().
  [ticket/10875] Remove useless assignment from phpbb_cache_driver_memory.

phpBB/includes/cache/driver/file.php

@@ -388,10 +388,10 @@ class phpbb_cache_driver_file extends phpbb_cache_driver_base
 
 		if ($this->_write('sql_' . md5($query), $this->sql_rowset[$query_id], $ttl + time(), $query))
 		{
-			$query_result = $query_id;
+			return $query_id;
 		}
 
-		return $query_id;
+		return $query_result;
 	}
 
 	/**

phpBB/includes/cache/driver/interface.php

@@ -63,42 +63,81 @@ interface phpbb_cache_driver_interface
 	public function destroy($var_name, $table = '');
 
 	/**
-	* Check if a given cache entry exist
+	* Check if a given cache entry exists
 	*/
 	public function _exists($var_name);
 
 	/**
-	* Load cached sql query
+	* Load result of an SQL query from cache.
+	*
+	* @param string $query			SQL query
+	*
+	* @return int|bool				Query ID (integer) if cache contains a rowset
+	*								for the specified query.
+	*								False otherwise.
 	*/
 	public function sql_load($query);
 
 	/**
-	* Save sql query
+	* Save result of an SQL query in cache.
+	*
+	* In persistent cache stores, this function stores the query
+	* result to persistent storage. In other words, there is no need
+	* to call save() afterwards.
+	*
+	* @param string $query			SQL query, should be used for generating storage key
+	* @param mixed $query_result	The result from dbal::sql_query, to be passed to
+	* 								dbal::sql_fetchrow to get all rows and store them
+	* 								in cache.
+	* @param int $ttl				Time to live, after this timeout the query should
+	*								expire from the cache.
+	* @return int|mixed				If storing in cache succeeded, an integer $query_id
+	* 								representing the query should be returned. Otherwise
+	* 								the original $query_result should be returned.
 	*/
 	public function sql_save($query, $query_result, $ttl);
 
 	/**
-	* Ceck if a given sql query exist in cache
+	* Check if result for a given SQL query exists in cache.
+	*
+	* @param int $query_id
+	* @return bool
 	*/
 	public function sql_exists($query_id);
 
 	/**
 	* Fetch row from cache (database)
+	*
+	* @param int $query_id
+	* @return array|bool 			The query result if found in the cache, otherwise
+	* 								false.
 	*/
 	public function sql_fetchrow($query_id);
 
 	/**
 	* Fetch a field from the current row of a cached database result (database)
+	*
+	* @param int $query_id
+	* @param $field 				The name of the column.
+	* @return string|bool 			The field of the query result if found in the cache,
+	* 								otherwise false.
 	*/
 	public function sql_fetchfield($query_id, $field);
 
 	/**
 	* Seek a specific row in an a cached database result (database)
+	*
+	* @param int $rownum 			Row to seek to.
+	* @param int $query_id
+	* @return bool
 	*/
 	public function sql_rowseek($rownum, $query_id);
 
 	/**
 	* Free memory used for a cached database result (database)
+	*
+	* @param int $query_id
+	* @return bool
 	*/
 	public function sql_freeresult($query_id);
 }

phpBB/includes/cache/driver/memory.php

@@ -299,7 +299,7 @@ abstract class phpbb_cache_driver_memory extends phpbb_cache_driver_base
 		if (!preg_match('/FROM \\(?(`?\\w+`?(?: \\w+)?(?:, ?`?\\w+`?(?: \\w+)?)*)\\)?/', $query, $regs))
 		{
 			// Bail out if the match fails.
-			return;
+			return $query_result;
 		}
 		$tables = array_map('trim', explode(',', $regs[1]));
 
@@ -339,8 +339,6 @@ abstract class phpbb_cache_driver_memory extends phpbb_cache_driver_base
 
 		$this->_write('sql_' . $hash, $this->sql_rowset[$query_id], $ttl);
 
-		$query_result = $query_id;
-
 		return $query_id;
 	}
 

phpBB/includes/cache/driver/null.php

@@ -109,6 +109,7 @@ class phpbb_cache_driver_null extends phpbb_cache_driver_base
 	*/
 	function sql_save($query, $query_result, $ttl)
 	{
+		return $query_result;
 	}
 
 	/**

tests/cache/cache_test.php

@@ -89,20 +89,55 @@ class phpbb_cache_test extends phpbb_database_test_case
 			WHERE config_name = 'foo'";
 		$result = $db->sql_query($sql, 300);
 		$first_result = $db->sql_fetchrow($result);
+		$expected = array('config_name' => 'foo', 'config_value' => '23', 'is_dynamic' => 0);
+		$this->assertEquals($expected, $first_result);
 
 		$this->assertFileExists($this->cache_dir . 'sql_' . md5(preg_replace('/[\n\r\s\t]+/', ' ', $sql)) . '.php');
 
+		$sql = 'DELETE FROM phpbb_config';
+		$result = $db->sql_query($sql);
+
 		$sql = "SELECT * FROM phpbb_config
 			WHERE config_name = 'foo'";
 		$result = $db->sql_query($sql, 300);
 
-		$this->assertEquals($first_result, $db->sql_fetchrow($result));
+		$this->assertEquals($expected, $db->sql_fetchrow($result));
 
 		$sql = "SELECT * FROM phpbb_config
-			WHERE config_name = 'bar'";
+			WHERE config_name = 'foo'";
+		$result = $db->sql_query($sql);
+
+		$no_cache_result = $db->sql_fetchrow($result);
+		$this->assertSame(false, $no_cache_result);
+
+		$db->sql_close();
+	}
+
+	public function test_null_cache_sql()
+	{
+		$driver = new phpbb_cache_driver_null($this->cache_dir);
+
+		global $db, $cache;
+		$db = $this->new_dbal();
+		$cache = new phpbb_cache_service($driver);
+
+		$sql = "SELECT * FROM phpbb_config
+			WHERE config_name = 'foo'";
+		$result = $db->sql_query($sql, 300);
+		$first_result = $db->sql_fetchrow($result);
+		$expected = array('config_name' => 'foo', 'config_value' => '23', 'is_dynamic' => 0);
+		$this->assertEquals($expected, $first_result);
+
+		$sql = 'DELETE FROM phpbb_config';
+		$result = $db->sql_query($sql);
+
+		// As null cache driver does not actually cache,
+		// this should return no results
+		$sql = "SELECT * FROM phpbb_config
+			WHERE config_name = 'foo'";
 		$result = $db->sql_query($sql, 300);
 
-		$this->assertNotEquals($first_result, $db->sql_fetchrow($result));
+		$this->assertSame(false, $db->sql_fetchrow($result));
 
 		$db->sql_close();
 	}

tests/mock/cache.php

@@ -123,6 +123,7 @@ class phpbb_mock_cache implements phpbb_cache_driver_interface
 	}
 	public function sql_save($query, $query_result, $ttl)
 	{
+		return $query_result;
 	}
 	public function sql_exists($query_id)
 	{