Merge branch 'task/naderman/config-class' into develop

* task/naderman/config-class:
  [task/config-class] Do not create multiple cache driver instances.
  [task/config-class] Add an attribute for the table name in phpbb_config_db.
  [task/config-class] Correctly merge cached config with dynamically loaded data.
  [task/config-class] Always specify the config table to use.
  [task/config-class] Fix db config constructor param order
  [task/config-class] Implemented a config class to replace the global array.

phpBB/common.php

@@ -207,8 +207,8 @@ $class_loader->register();
 
 // set up caching
 $cache_factory = new phpbb_cache_factory($acm_type);
-$class_loader->set_cache($cache_factory->get_driver());
 $cache = $cache_factory->get_service();
+$class_loader->set_cache($cache->get_driver());
 
 // Instantiate some basic classes
 $request	= new phpbb_request();
@@ -227,7 +227,9 @@ $db->sql_connect($dbhost, $dbuser, $dbpasswd, $dbname, $dbport, false, defined('
 unset($dbpasswd);
 
 // Grab global variables, re-cache if necessary
-$config = $cache->obtain_config();
+$config = new phpbb_config_db($db, $cache->get_driver(), CONFIG_TABLE);
+set_config(null, null, null, $config);
+set_config_count(null, null, null, $config);
 
 // Add own hook handler
 require($phpbb_root_path . 'includes/hooks/index.' . $phpEx);

phpBB/download/file.php

@@ -55,8 +55,8 @@ if (isset($_GET['avatar']))
 
 	// set up caching
 	$cache_factory = new phpbb_cache_factory($acm_type);
-	$class_loader->set_cache($cache_factory->get_driver());
 	$cache = $cache_factory->get_service();
+	$class_loader->set_cache($cache->get_driver());
 
 	$db = new $sql_db();
 
@@ -70,7 +70,10 @@ if (isset($_GET['avatar']))
 	// worst-case default
 	$browser = (!empty($_SERVER['HTTP_USER_AGENT'])) ? htmlspecialchars((string) $_SERVER['HTTP_USER_AGENT']) : 'msie 6.0';
 
-	$config = $cache->obtain_config();
+	$config = new phpbb_config_db($db, $cache->get_driver(), CONFIG_TABLE);
+	set_config(null, null, null, $config);
+	set_config_count(null, null, null, $config);
+
 	$filename = $_GET['avatar'];
 	$avatar_group = false;
 	$exit = false;

phpBB/includes/cache/service.php

@@ -2,7 +2,6 @@
 /**
 *
 * @package acm
-* @version $Id$
 * @copyright (c) 2005 phpBB Group
 * @license http://opensource.org/licenses/gpl-license.php GNU Public License
 *
@@ -22,66 +21,41 @@ if (!defined('IN_PHPBB'))
 */
 class phpbb_cache_service
 {
-	private $acm;
+	private $driver;
 
-	public function __construct(phpbb_cache_driver_interface $acm = null)
+	/**
+	* Creates a cache service around a cache driver
+	*
+	* @param phpbb_cache_driver_interface $driver The cache driver
+	*/
+	public function __construct(phpbb_cache_driver_interface $driver = null)
 	{
-		$this->set_acm($acm);
+		$this->set_driver($driver);
 	}
 
-	public function set_acm(phpbb_cache_driver_interface $acm)
+	/**
+	* Returns the cache driver used by this cache service.
+	*
+	* @return phpbb_cache_driver_interface The cache driver
+	*/
+	public function get_driver()
 	{
-		$this->acm = $acm;
+		return $this->driver;
+	}
+
+	/**
+	* Replaces the cache driver used by this cache service.
+	*
+	* @param phpbb_cache_driver_interface $driver The cache driver
+	*/
+	public function set_driver(phpbb_cache_driver_interface $driver)
+	{
+		$this->driver = $driver;
 	}
 
 	public function __call($method, $arguments)
 	{
-		return call_user_func_array(array($this->acm, $method), $arguments);
+		return call_user_func_array(array($this->driver, $method), $arguments);
-	}
-
-	/**
-	* Get config values
-	*/
-	function obtain_config()
-	{
-		global $db;
-
-		if (($config = $this->acm->get('config')) !== false)
-		{
-			$sql = 'SELECT config_name, config_value
-				FROM ' . CONFIG_TABLE . '
-				WHERE is_dynamic = 1';
-			$result = $db->sql_query($sql);
-
-			while ($row = $db->sql_fetchrow($result))
-			{
-				$config[$row['config_name']] = $row['config_value'];
-			}
-			$db->sql_freeresult($result);
-		}
-		else
-		{
-			$config = $cached_config = array();
-
-			$sql = 'SELECT config_name, config_value, is_dynamic
-				FROM ' . CONFIG_TABLE;
-			$result = $db->sql_query($sql);
-
-			while ($row = $db->sql_fetchrow($result))
-			{
-				if (!$row['is_dynamic'])
-				{
-					$cached_config[$row['config_name']] = $row['config_value'];
-				}
-
-				$config[$row['config_name']] = $row['config_value'];
-			}
-			$db->sql_freeresult($result);
-
-			$this->acm->put('config', $cached_config);
-		}
-
-		return $config;
 	}
 
 	/**
@@ -92,7 +66,7 @@ class phpbb_cache_service
 	{
 		global $db;
 
-		if (($censors = $this->acm->get('_word_censors')) === false)
+		if (($censors = $this->driver->get('_word_censors')) === false)
 		{
 			$sql = 'SELECT word, replacement
 				FROM ' . WORDS_TABLE;
@@ -106,7 +80,7 @@ class phpbb_cache_service
 			}
 			$db->sql_freeresult($result);
 
-			$this->acm->put('_word_censors', $censors);
+			$this->driver->put('_word_censors', $censors);
 		}
 
 		return $censors;
@@ -117,7 +91,7 @@ class phpbb_cache_service
 	*/
 	function obtain_icons()
 	{
-		if (($icons = $this->acm->get('_icons')) === false)
+		if (($icons = $this->driver->get('_icons')) === false)
 		{
 			global $db;
 
@@ -137,7 +111,7 @@ class phpbb_cache_service
 			}
 			$db->sql_freeresult($result);
 
-			$this->acm->put('_icons', $icons);
+			$this->driver->put('_icons', $icons);
 		}
 
 		return $icons;
@@ -148,7 +122,7 @@ class phpbb_cache_service
 	*/
 	function obtain_ranks()
 	{
-		if (($ranks = $this->acm->get('_ranks')) === false)
+		if (($ranks = $this->driver->get('_ranks')) === false)
 		{
 			global $db;
 
@@ -178,7 +152,7 @@ class phpbb_cache_service
 			}
 			$db->sql_freeresult($result);
 
-			$this->acm->put('_ranks', $ranks);
+			$this->driver->put('_ranks', $ranks);
 		}
 
 		return $ranks;
@@ -193,7 +167,7 @@ class phpbb_cache_service
 	*/
 	function obtain_attach_extensions($forum_id)
 	{
-		if (($extensions = $this->acm->get('_extensions')) === false)
+		if (($extensions = $this->driver->get('_extensions')) === false)
 		{
 			global $db;
 
@@ -237,7 +211,7 @@ class phpbb_cache_service
 			}
 			$db->sql_freeresult($result);
 
-			$this->acm->put('_extensions', $extensions);
+			$this->driver->put('_extensions', $extensions);
 		}
 
 		// Forum post
@@ -298,7 +272,7 @@ class phpbb_cache_service
 	*/
 	function obtain_bots()
 	{
-		if (($bots = $this->acm->get('_bots')) === false)
+		if (($bots = $this->driver->get('_bots')) === false)
 		{
 			global $db;
 
@@ -337,7 +311,7 @@ class phpbb_cache_service
 			}
 			$db->sql_freeresult($result);
 
-			$this->acm->put('_bots', $bots);
+			$this->driver->put('_bots', $bots);
 		}
 
 		return $bots;
@@ -358,7 +332,7 @@ class phpbb_cache_service
 
 		foreach ($parsed_items as $key => $parsed_array)
 		{
-			$parsed_array = $this->acm->get('_cfg_' . $key . '_' . $theme[$key . '_path']);
+			$parsed_array = $this->driver->get('_cfg_' . $key . '_' . $theme[$key . '_path']);
 
 			if ($parsed_array === false)
 			{
@@ -384,7 +358,7 @@ class phpbb_cache_service
 				$parsed_array = parse_cfg_file($filename);
 				$parsed_array['filetime'] = @filemtime($filename);
 
-				$this->acm->put('_cfg_' . $key . '_' . $theme[$key . '_path'], $parsed_array);
+				$this->driver->put('_cfg_' . $key . '_' . $theme[$key . '_path'], $parsed_array);
 			}
 			$parsed_items[$key] = $parsed_array;
 		}
@@ -397,7 +371,7 @@ class phpbb_cache_service
 	*/
 	function obtain_disallowed_usernames()
 	{
-		if (($usernames = $this->acm->get('_disallowed_usernames')) === false)
+		if (($usernames = $this->driver->get('_disallowed_usernames')) === false)
 		{
 			global $db;
 
@@ -412,7 +386,7 @@ class phpbb_cache_service
 			}
 			$db->sql_freeresult($result);
 
-			$this->acm->put('_disallowed_usernames', $usernames);
+			$this->driver->put('_disallowed_usernames', $usernames);
 		}
 
 		return $usernames;
@@ -425,7 +399,7 @@ class phpbb_cache_service
 	{
 		global $phpbb_root_path, $phpEx;
 
-		if (($hook_files = $this->acm->get('_hooks')) === false)
+		if (($hook_files = $this->driver->get('_hooks')) === false)
 		{
 			$hook_files = array();
 
@@ -444,7 +418,7 @@ class phpbb_cache_service
 				closedir($dh);
 			}
 
-			$this->acm->put('_hooks', $hook_files);
+			$this->driver->put('_hooks', $hook_files);
 		}
 
 		return $hook_files;

phpBB/includes/captcha/plugins/phpbb_captcha_gd_plugin.php

@@ -152,9 +152,16 @@ class phpbb_captcha_gd extends phpbb_default_captcha
 		global $config;
 
 		$config_old = $config;
+
+		$config = new phpbb_config(array());
+		foreach ($config_old as $key => $value)
+		{
+			$config->set($key, $value);
+		}
+
 		foreach ($this->captcha_vars as $captcha_var => $template_var)
 		{
-				$config[$captcha_var] = request_var($captcha_var, (int) $config[$captcha_var]);
+			$config->set($captcha_var, request_var($captcha_var, (int) $config[$captcha_var]));
 		}
 		parent::execute_demo();
 		$config = $config_old;

phpBB/includes/config/config.php

@@ -0,0 +1,157 @@
+<?php
+/**
+*
+* @package phpBB3
+* @copyright (c) 2010 phpBB Group
+* @license http://opensource.org/licenses/gpl-license.php GNU Public License
+*
+*/
+
+/**
+* @ignore
+*/
+if (!defined('IN_PHPBB'))
+{
+	exit;
+}
+
+/**
+* Configuration container class
+* @package phpBB3
+*/
+class phpbb_config implements ArrayAccess, IteratorAggregate, Countable
+{
+	/**
+	* The configuration data
+	* @var array(string => string)
+	*/
+	protected $config;
+
+	/**
+	* Creates a configuration container with a default set of values
+	*
+	* @param array(string => string) $config The configuration data.
+	*/
+	public function __construct(array $config)
+	{
+		$this->config = $config;
+	}
+
+	/**
+	* Retrieves an ArrayIterator over the configuration values.
+	*
+	* @return ArrayIterator An iterator over all config data
+	*/
+	public function getIterator()
+	{
+		return new ArrayIterator($this->config);
+	}
+
+	/**
+	* Checks if the specified config value exists.
+	*
+	* @param  string $key The configuration option's name.
+	* @return bool        Whether the configuration option exists.
+	*/
+	public function offsetExists($key)
+	{
+		return isset($this->config[$key]);
+	}
+
+	/**
+	* Retrieves a configuration value.
+	*
+	* @param  string $key The configuration option's name.
+	* @return string      The configuration value
+	*/
+	public function offsetGet($key)
+	{
+		return (isset($this->config[$key])) ? $this->config[$key] : '';
+	}
+
+	/**
+	* Temporarily overwrites the value of a configuration variable.
+	*
+	* The configuration change will not persist. It will be lost
+	* after the request.
+	*
+	* @param string $key   The configuration option's name.
+	* @param string $value The temporary value.
+	*/
+	public function offsetSet($key, $value)
+	{
+		$this->config[$key] = $value;
+	}
+
+	/**
+	* Called when deleting a configuration value directly, triggers an error.
+	*
+	* @param string $key The configuration option's name.
+	*/
+	public function offsetUnset($key)
+	{
+		trigger_error('Config values have to be deleted explicitly with the phpbb_config::delete($key) method.', E_USER_ERROR);
+	}
+
+	/**
+	* Retrieves the number of configuration options currently set.
+	*
+	* @return int Number of config options
+	*/
+	public function count()
+	{
+		return count($this->config);
+	}
+
+	/**
+	* Sets a configuration option's value
+	*
+	* @param string $key   The configuration option's name
+	* @param string $value New configuration value
+	* @param bool   $cache Whether this variable should be cached or if it
+	*                      changes too frequently to be efficiently cached.
+	*/
+	public function set($key, $value, $cache = true)
+	{
+		$this->config[$key] = $value;
+	}
+
+	/**
+	* Sets a configuration option's value only if the old_value matches the
+	* current configuration value or the configuration value does not exist yet.
+	*
+	* @param  string $key       The configuration option's name
+	* @param  string $old_value Current configuration value
+	* @param  string $value     New configuration value
+	* @param  bool   $cache     Whether this variable should be cached or if it
+	*                           changes too frequently to be efficiently cached.
+	* @return bool              True if the value was changed, false otherwise.
+	*/
+	public function set_atomic($key, $old_value, $value, $cache = true)
+	{
+		if (!isset($this->config[$key]) || $this->config[$key] == $old_value)
+		{
+			$this->config[$key] = $value;
+			return true;
+		}
+		return false;
+	}
+
+	/**
+	* Increments an integer configuration value.
+	*
+	* @param string $key       The configuration option's name
+	* @param int    $increment Amount to increment by
+	* @param bool   $cache     Whether this variable should be cached or if it
+	*                          changes too frequently to be efficiently cached.
+	*/
+	function increment($key, $increment, $cache = true)
+	{
+		if (!isset($this->config[$key]))
+		{
+			$this->config[$key] = 0;
+		}
+
+		$this->config[$key] += $increment;
+	}
+}

phpBB/includes/config/db.php

@@ -0,0 +1,185 @@
+<?php
+/**
+*
+* @package phpBB3
+* @copyright (c) 2010 phpBB Group
+* @license http://opensource.org/licenses/gpl-license.php GNU Public License
+*
+*/
+
+/**
+* @ignore
+*/
+if (!defined('IN_PHPBB'))
+{
+	exit;
+}
+
+/**
+* Configuration container class
+* @package phpBB3
+*/
+class phpbb_config_db extends phpbb_config
+{
+	/**
+	* Cache instance
+	* @var phpbb_cache_driver_interface
+	*/
+	protected $cache;
+
+	/**
+	* Database connection
+	* @var dbal
+	*/
+	protected $db;
+
+	/**
+	* Name of the database table used for configuration.
+	* @var string
+	*/
+	protected $table;
+
+	/**
+	* Creates a configuration container with a default set of values
+	*
+	* @param dbal                         $db    Database connection
+	* @param phpbb_cache_driver_interface $cache Cache instance
+	* @param string                       $table Configuration table name
+	*/
+	public function __construct(dbal $db, phpbb_cache_driver_interface $cache, $table)
+	{
+		$this->db = $db;
+		$this->cache = $cache;
+		$this->table = $table;
+
+		if (($config = $cache->get('config')) !== false)
+		{
+			$sql = 'SELECT config_name, config_value
+				FROM ' . $this->table . '
+				WHERE is_dynamic = 1';
+			$result = $this->db->sql_query($sql);
+
+			while ($row = $this->db->sql_fetchrow($result))
+			{
+				$config[$row['config_name']] = $row['config_value'];
+			}
+			$this->db->sql_freeresult($result);
+		}
+		else
+		{
+			$config = $cached_config = array();
+
+			$sql = 'SELECT config_name, config_value, is_dynamic
+				FROM ' . $this->table;
+			$result = $this->db->sql_query($sql);
+
+			while ($row = $this->db->sql_fetchrow($result))
+			{
+				if (!$row['is_dynamic'])
+				{
+					$cached_config[$row['config_name']] = $row['config_value'];
+				}
+
+				$config[$row['config_name']] = $row['config_value'];
+			}
+			$this->db->sql_freeresult($result);
+
+			$cache->put('config', $cached_config);
+		}
+
+		parent::__construct($config);
+	}
+
+	/**
+	* Sets a configuration option's value
+	*
+	* @param string $key   The configuration option's name
+	* @param string $value New configuration value
+	* @param bool   $cache Whether this variable should be cached or if it
+	*                      changes too frequently to be efficiently cached.
+	*/
+	public function set($key, $value, $cache = true)
+	{
+		$this->set_atomic($key, false, $value, $cache);
+	}
+
+	/**
+	* Sets a configuration option's value only if the old_value matches the
+	* current configuration value or the configuration value does not exist yet.
+	*
+	* @param  string $key       The configuration option's name
+	* @param  mixed  $old_value Current configuration value or false to ignore
+	*                           the old value
+	* @param  string $new_value New configuration value
+	* @param  bool   $cache     Whether this variable should be cached or if it
+	*                           changes too frequently to be efficiently cached
+	* @return bool              True if the value was changed, false otherwise
+	*/
+	public function set_atomic($key, $old_value, $new_value, $cache = true)
+	{
+		$sql = 'UPDATE ' . $this->table . "
+			SET config_value = '" . $this->db->sql_escape($new_value) . "'
+			WHERE config_name = '" . $this->db->sql_escape($key) . "'";
+
+		if ($old_value !== false)
+		{
+			$sql .= " AND config_value = '" . $this->db->sql_escape($old_value) . "'";
+		}
+
+		$result = $this->db->sql_query($sql);
+
+		if (!$this->db->sql_affectedrows($result) && isset($this->config[$key]))
+		{
+			return false;
+		}
+
+		if (!isset($this->config[$key]))
+		{
+			$sql = 'INSERT INTO ' . $this->table . ' ' . $this->db->sql_build_array('INSERT', array(
+				'config_name'	=> $key,
+				'config_value'	=> $new_value,
+				'is_dynamic'	=> ($cache) ? 0 : 1));
+			$this->db->sql_query($sql);
+		}
+
+		if ($cache)
+		{
+			$this->cache->destroy('config');
+		}
+
+		$this->config[$key] = $new_value;
+		return true;
+	}
+
+	/**
+	* Increments an integer config value directly in the database.
+	*
+	* Using this method instead of setting the new value directly avoids race
+	* conditions and unlike set_atomic it cannot fail.
+	*
+	* @param string $key       The configuration option's name
+	* @param int    $increment Amount to increment by
+	* @param bool   $cache     Whether this variable should be cached or if it
+	*                          changes too frequently to be efficiently cached.
+	*/
+	function increment($key, $increment, $cache = true)
+	{
+		if (!isset($this->config[$key]))
+		{
+			$this->set($key, '0', $cache);
+		}
+
+		$sql_update = $this->db->cast_expr_to_string($this->db->cast_expr_to_bigint('config_value') . ' + ' . (int) $increment);
+
+		$this->db->sql_query('UPDATE ' . $this->table . '
+			SET config_value = ' . $sql_update . "
+			WHERE config_name = '" . $this->db->sql_escape($key) . "'");
+
+		if ($cache)
+		{
+			$this->cache->destroy('config');
+		}
+
+		$this->config[$key] += $increment;
+	}
+}

phpBB/includes/db/dbal.php

@@ -447,6 +447,28 @@ class dbal
 		return $column_name . ' | ' . (1 << $bit) . (($compare) ? ' ' . $compare : '');
 	}
 
+	/**
+	* Returns SQL string to cast a string expression to an int.
+	*
+	* @param  string $expression An expression evaluating to string
+	* @return string             Expression returning an int
+	*/
+	function cast_expr_to_bigint($expression)
+	{
+		return $expression;
+	}
+
+	/**
+	* Returns SQL string to cast an integer expression to a string.
+	*
+	* @param  string $expression An expression evaluating to int
+	* @return string             Expression returning a string
+	*/
+	function cast_expr_to_string($expression)
+	{
+		return $expression;
+	}
+
 	/**
 	* Run more than one insert statement.
 	*

phpBB/includes/db/firebird.php

@@ -465,6 +465,22 @@ class dbal_firebird extends dbal
 		return 'BIN_OR(' . $column_name . ', ' . (1 << $bit) . ')' . (($compare) ? ' ' . $compare : '');
 	}
 
+	/**
+	* @inheritdoc
+	*/
+	function cast_expr_to_bigint($expression)
+	{
+		return 'CAST(' . $expression . ' as DECIMAL(255, 0))';
+	}
+
+	/**
+	* @inheritdoc
+	*/
+	function cast_expr_to_string($expression)
+	{
+		return 'CAST(' . $expression . ' as VARCHAR(255))';
+	}
+
 	/**
 	* return sql error array
 	* @access private

phpBB/includes/db/postgres.php

@@ -365,6 +365,22 @@ class dbal_postgres extends dbal
 		return $expression;
 	}
 
+	/**
+	* @inheritdoc
+	*/
+	function cast_expr_to_bigint($expression)
+	{
+		return 'CAST(' . $expression . ' as DECIMAL(255, 0))';
+	}
+
+	/**
+	* @inheritdoc
+	*/
+	function cast_expr_to_string($expression)
+	{
+		return 'CAST(' . $expression . ' as VARCHAR(255))';
+	}
+
 	/**
 	* return sql error array
 	* @access private

phpBB/includes/functions.php

@@ -17,7 +17,11 @@ if (!defined('IN_PHPBB'))
 }
 
 // Common global functions
-
+/**
+* Casts a variable to the given type.
+*
+* @deprecated
+*/
 function set_var(&$result, $var, $type, $multibyte = false)
 {
 	// no need for dependency injection here, if you have the object, call the method yourself!
@@ -30,6 +34,7 @@ function set_var(&$result, $var, $type, $multibyte = false)
 * See {@link phpbb_request_interface::variable phpbb_request_interface::variable} for
 * documentation of this function's use.
 *
+* @deprecated
 * @param	mixed			$var_name	The form variable's name from which data shall be retrieved.
 * 										If the value is an array this may be an array of indizes which will give
 * 										direct access to a value at any depth. E.g. if the value of "var" is array(1 => "a")
@@ -78,59 +83,46 @@ function request_var($var_name, $default, $multibyte = false, $cookie = false, p
 
 /**
 * Set config value. Creates missing config entry.
+*
+* @deprecated
 */
-function set_config($config_name, $config_value, $is_dynamic = false)
+function set_config($config_name, $config_value, $is_dynamic = false, phpbb_config $set_config = null)
 {
-	global $db, $cache, $config;
+	static $config = null;
 
-	$sql = 'UPDATE ' . CONFIG_TABLE . "
+	if ($set_config !== null)
-		SET config_value = '" . $db->sql_escape($config_value) . "'
-		WHERE config_name = '" . $db->sql_escape($config_name) . "'";
-	$db->sql_query($sql);
-
-	if (!$db->sql_affectedrows() && !isset($config[$config_name]))
 	{
-		$sql = 'INSERT INTO ' . CONFIG_TABLE . ' ' . $db->sql_build_array('INSERT', array(
+		$config = $set_config;
-			'config_name'	=> $config_name,
+
-			'config_value'	=> $config_value,
+		if (empty($config_name))
-			'is_dynamic'	=> ($is_dynamic) ? 1 : 0));
+		{
-		$db->sql_query($sql);
+			return;
+		}
 	}
 
-	$config[$config_name] = $config_value;
+	$config->set($config_name, $config_value, !$is_dynamic);
-
-	if (!$is_dynamic)
-	{
-		$cache->destroy('config');
-	}
 }
 
 /**
 * Set dynamic config value with arithmetic operation.
+*
+* @deprecated
 */
-function set_config_count($config_name, $increment, $is_dynamic = false)
+function set_config_count($config_name, $increment, $is_dynamic = false, phpbb_config $set_config = null)
 {
-	global $db, $cache;
+	static $config = null;
 
-	switch ($db->sql_layer)
+	if ($set_config !== null)
 	{
-		case 'firebird':
+		$config = $set_config;
-		case 'postgres':
-			$sql_update = 'CAST(CAST(config_value as DECIMAL(255, 0)) + ' . (int) $increment . ' as VARCHAR(255))';
-		break;
 
-		// MySQL, SQlite, mssql, mssql_odbc, oracle
+		if (empty($config_name))
-		default:
+		{
-			$sql_update = 'config_value + ' . (int) $increment;
+			return;
-		break;
+		}
 	}
 
-	$db->sql_query('UPDATE ' . CONFIG_TABLE . ' SET config_value = ' . $sql_update . " WHERE config_name = '" . $db->sql_escape($config_name) . "'");
+	$config->increment($config_name, $increment, !$is_dynamic);
-
-	if (!$is_dynamic)
-	{
-		$cache->destroy('config');
-	}
 }
 
 /**
@@ -3546,7 +3538,7 @@ function phpbb_checkdnsrr($host, $type = 'MX')
 	// but until 5.3.3 it only works for MX records
 	// See: http://bugs.php.net/bug.php?id=51844
 
-	// Call checkdnsrr() if 
+	// Call checkdnsrr() if
 	// we're looking for an MX record or
 	// we're not on Windows or
 	// we're running a PHP version where #51844 has been fixed
@@ -3566,7 +3558,7 @@ function phpbb_checkdnsrr($host, $type = 'MX')
 	// dns_get_record() is available since PHP 5; since PHP 5.3 also on Windows,
 	// but on Windows it does not work reliable for AAAA records before PHP 5.3.1
 
-	// Call dns_get_record() if 
+	// Call dns_get_record() if
 	// we're not looking for an AAAA record or
 	// we're not on Windows or
 	// we're running a PHP version where AAAA lookups work reliable
@@ -3596,7 +3588,7 @@ function phpbb_checkdnsrr($host, $type = 'MX')
 		foreach ($resultset as $result)
 		{
 			if (
-				isset($result['host']) && $result['host'] == $host && 
+				isset($result['host']) && $result['host'] == $host &&
 				isset($result['type']) && $result['type'] == $type
 			)
 			{

phpBB/install/database_update.php

@@ -96,8 +96,8 @@ $class_loader->register();
 
 // set up caching
 $cache_factory = new phpbb_cache_factory($acm_type);
-$class_loader->set_cache($cache_factory->get_driver());
 $cache = $cache_factory->get_service();
+$class_loader->set_cache($cache->get_driver());
 
 $request = new phpbb_request();
 $user = new user();
@@ -165,17 +165,9 @@ include($phpbb_root_path . 'language/' . $language . '/install.' . $phpEx);
 $inline_update = (request_var('type', 0)) ? true : false;
 
 // To let set_config() calls succeed, we need to make the config array available globally
-$config = array();
+$config = new phpbb_config_db($db, $cache->get_driver(), CONFIG_TABLE);
-
+set_config(null, null, null, $config);
-$sql = 'SELECT *
+set_config_count(null, null, null, $config);
-	FROM ' . CONFIG_TABLE;
-$result = $db->sql_query($sql);
-
-while ($row = $db->sql_fetchrow($result))
-{
-	$config[$row['config_name']] = $row['config_value'];
-}
-$db->sql_freeresult($result);
 
 // We do not include DB Tools here, because we can not be sure the file is up-to-date ;)
 // Instead, this file defines a clean db_tools version (we are also not able to provide a different file, else the database update will not work standalone)

phpBB/install/index.php

@@ -172,8 +172,8 @@ $class_loader->register();
 
 // set up caching
 $cache_factory = new phpbb_cache_factory('file');
-$class_loader->set_cache($cache_factory->get_driver());
 $cache = $cache_factory->get_service();
+$class_loader->set_cache($cache->get_driver());
 
 $request = new phpbb_request();
 
@@ -281,9 +281,9 @@ else
 }
 
 // Set some standard variables we want to force
-$config = array(
+$config = new phpbb_config(array(
 	'load_tplcompile'	=> '1'
-);
+));
 
 $template->set_custom_template('../adm/style', 'admin');
 $template->assign_var('T_TEMPLATE_PATH', '../adm/style');

phpBB/install/install_convert.php

@@ -130,16 +130,9 @@ class install_convert extends module
 				unset($dbpasswd);
 
 				// We need to fill the config to let internal functions correctly work
-				$sql = 'SELECT *
+				$config = new phpbb_config_db($db, new phpbb_cache_driver_null, CONFIG_TABLE);
-					FROM ' . CONFIG_TABLE;
+				set_config(null, null, null, $config);
-				$result = $db->sql_query($sql);
+				set_config_count(null, null, null, $config);
-
-				$config = array();
-				while ($row = $db->sql_fetchrow($result))
-				{
-					$config[$row['config_name']] = $row['config_value'];
-				}
-				$db->sql_freeresult($result);
 
 				// Detect if there is already a conversion in progress at this point and offer to resume
 				// It's quite possible that the user will get disconnected during a large conversion so they need to be able to resume it
@@ -350,16 +343,9 @@ class install_convert extends module
 		$this->page_title = $lang['STAGE_SETTINGS'];
 
 		// We need to fill the config to let internal functions correctly work
-		$sql = 'SELECT *
+		$config = new phpbb_config_db($db, new phpbb_cache_driver_null, CONFIG_TABLE);
-			FROM ' . CONFIG_TABLE;
+		set_config(null, null, null, $config);
-		$result = $db->sql_query($sql);
+		set_config_count(null, null, null, $config);
-
-		$config = array();
-		while ($row = $db->sql_fetchrow($result))
-		{
-			$config[$row['config_name']] = $row['config_value'];
-		}
-		$db->sql_freeresult($result);
 
 		$convertor_tag = request_var('tag', '');
 
@@ -597,16 +583,10 @@ class install_convert extends module
 		$db->sql_connect($dbhost, $dbuser, $dbpasswd, $dbname, $dbport, false, true);
 		unset($dbpasswd);
 
-		$sql = 'SELECT *
+		// We need to fill the config to let internal functions correctly work
-			FROM ' . CONFIG_TABLE;
+		$config = new phpbb_config_db($db, new phpbb_cache_driver_null, CONFIG_TABLE);
-		$result = $db->sql_query($sql);
+		set_config(null, null, null, $config);
-
+		set_config_count(null, null, null, $config);
-		$config = array();
-		while ($row = $db->sql_fetchrow($result))
-		{
-			$config[$row['config_name']] = $row['config_value'];
-		}
-		$db->sql_freeresult($result);
 
 		// Override a couple of config variables for the duration
 		$config['max_quote_depth'] = 0;

phpBB/install/install_install.php

@@ -1460,17 +1460,10 @@ class install_install extends module
 		include_once($phpbb_root_path . 'includes/constants.' . $phpEx);
 		include_once($phpbb_root_path . 'includes/search/fulltext_native.' . $phpEx);
 
-		// Fill the config array - it is needed by those functions we call
+		// We need to fill the config to let internal functions correctly work
-		$sql = 'SELECT *
+		$config = new phpbb_config_db($db, new phpbb_cache_driver_null, CONFIG_TABLE);
-			FROM ' . CONFIG_TABLE;
+		set_config(null, null, null, $config);
-		$result = $db->sql_query($sql);
+		set_config_count(null, null, null, $config);
-
-		$config = array();
-		while ($row = $db->sql_fetchrow($result))
-		{
-			$config[$row['config_name']] = $row['config_value'];
-		}
-		$db->sql_freeresult($result);
 
 		$error = false;
 		$search = new fulltext_native($error);
@@ -1830,17 +1823,10 @@ class install_install extends module
 		// Obtain any submitted data
 		$data = $this->get_submitted_data();
 
-		// Fill the config array - it is needed by those functions we call
+		// We need to fill the config to let internal functions correctly work
-		$sql = 'SELECT *
+		$config = new phpbb_config_db($db, new phpbb_cache_driver_null, CONFIG_TABLE);
-			FROM ' . CONFIG_TABLE;
+		set_config(null, null, null, $config);
-		$result = $db->sql_query($sql);
+		set_config_count(null, null, null, $config);
-
-		$config = array();
-		while ($row = $db->sql_fetchrow($result))
-		{
-			$config[$row['config_name']] = $row['config_value'];
-		}
-		$db->sql_freeresult($result);
 
 		$sql = 'SELECT group_id
 			FROM ' . GROUPS_TABLE . "
@@ -1907,19 +1893,10 @@ class install_install extends module
 
 		$this->page_title = $lang['STAGE_FINAL'];
 
-		// Obtain any submitted data
+		// We need to fill the config to let internal functions correctly work
-		$data = $this->get_submitted_data();
+		$config = new phpbb_config_db($db, new phpbb_cache_driver_null, CONFIG_TABLE);
-
+		set_config(null, null, null, $config);
-		$sql = 'SELECT *
+		set_config_count(null, null, null, $config);
-			FROM ' . CONFIG_TABLE;
-		$result = $db->sql_query($sql);
-
-		$config = array();
-		while ($row = $db->sql_fetchrow($result))
-		{
-			$config[$row['config_name']] = $row['config_value'];
-		}
-		$db->sql_freeresult($result);
 
 		$user->session_begin();
 		$auth->login($data['admin_name'], $data['admin_pass1'], false, true, true);

phpBB/install/install_update.php

@@ -101,17 +101,10 @@ class install_update extends module
 		// We do not need this any longer, unset for safety purposes
 		unset($dbpasswd);
 
-		$config = array();
+		// We need to fill the config to let internal functions correctly work
-
+		$config = new phpbb_config_db($db, new phpbb_cache_driver_null, CONFIG_TABLE);
-		$sql = 'SELECT config_name, config_value
+		set_config(null, null, null, $config);
-			FROM ' . CONFIG_TABLE;
+		set_config_count(null, null, null, $config);
-		$result = $db->sql_query($sql);
-
-		while ($row = $db->sql_fetchrow($result))
-		{
-			$config[$row['config_name']] = $row['config_value'];
-		}
-		$db->sql_freeresult($result);
 
 		// Force template recompile
 		$config['load_tplcompile'] = 1;

phpBB/style.php

@@ -66,8 +66,8 @@ if ($id)
 
 	// set up caching
 	$cache_factory = new phpbb_cache_factory($acm_type);
-	$class_loader->set_cache($cache_factory->get_driver());
 	$cache = $cache_factory->get_service();
+	$class_loader->set_cache($cache->get_driver());
 
 	$request = new phpbb_request();
 	$db = new $sql_db();
@@ -82,7 +82,10 @@ if ($id)
 	}
 	unset($dbpasswd);
 
-	$config = $cache->obtain_config();
+	$config = new phpbb_config_db($db, $cache->get_driver(), CONFIG_TABLE);
+	set_config(null, null, null, $config);
+	set_config_count(null, null, null, $config);
+
 	$user = false;
 
 	// try to get a session ID from REQUEST array

tests/config/config_test.php

@@ -0,0 +1,112 @@
+<?php
+/**
+*
+* @package testing
+* @copyright (c) 2010 phpBB Group
+* @license http://opensource.org/licenses/gpl-license.php GNU Public License
+*
+*/
+
+class phpbb_config_test extends phpbb_test_case
+{
+	public function test_offset_exists()
+	{
+		$config = new phpbb_config(array('foo' => 'bar'));
+
+		$this->assertTrue(isset($config['foo']));
+		$this->assertFalse(isset($config['foobar']));
+	}
+
+	public function test_offset_get()
+	{
+		$config = new phpbb_config(array('foo' => 'bar'));
+		$this->assertEquals('bar', $config['foo']);
+	}
+
+	public function test_offset_get_missing()
+	{
+		$config = new phpbb_config(array());
+		$this->assertEquals('', $config['foo']);
+	}
+
+	public function test_offset_set()
+	{
+		$config = new phpbb_config(array());
+		$config['foo'] = 'x';
+		$this->assertEquals('x', $config['foo']);
+	}
+
+	public function test_offset_unset_fails()
+	{
+		$this->setExpectedTriggerError(E_USER_ERROR);
+		$config = new phpbb_config(array('foo' => 'x'));
+		unset($config['foo']);
+	}
+
+	public function test_count()
+	{
+		$config = new phpbb_config(array('foo' => 'bar'));
+		$this->assertEquals(1, count($config));
+	}
+
+	public function test_iterate()
+	{
+		$vars = array('foo' => '23', 'bar' => '42');
+		$config = new phpbb_config($vars);
+
+		$count = 0;
+		foreach ($config as $key => $value)
+		{
+			$this->assertTrue(isset($vars[$key]));
+			$this->assertEquals($vars[$key], $value);
+
+			$count++;
+		}
+
+		$this->assertEquals(count($vars), $count);
+	}
+
+	public function test_set_overwrite()
+	{
+		$config = new phpbb_config(array('foo' => 'x'));
+		$config->set('foo', 'bar');
+		$this->assertEquals('bar', $config['foo']);
+	}
+
+	public function test_set_new()
+	{
+		$config = new phpbb_config(array());
+		$config->set('foo', 'bar');
+		$this->assertEquals('bar', $config['foo']);
+	}
+
+	public function test_set_atomic_overwrite()
+	{
+		$config = new phpbb_config(array('foo' => 'bar'));
+		$this->assertTrue($config->set_atomic('foo', 'bar', '23'));
+		$this->assertEquals('23', $config['foo']);
+	}
+
+	public function test_set_atomic_new()
+	{
+		$config = new phpbb_config(array());
+		$this->assertTrue($config->set_atomic('foo', false, '23'));
+		$this->assertEquals('23', $config['foo']);
+	}
+
+	public function test_set_atomic_failure()
+	{
+		$config = new phpbb_config(array('foo' => 'bar'));
+		$this->assertFalse($config->set_atomic('foo', 'wrong', '23'));
+		$this->assertEquals('bar', $config['foo']);
+	}
+
+	public function test_increment()
+	{
+		$config = new phpbb_config(array('foo' => '23'));
+		$config->increment('foo', 3);
+		$this->assertEquals(26, $config['foo']);
+		$config->increment('foo', 1);
+		$this->assertEquals(27, $config['foo']);
+	}
+}

tests/config/db_test.php

@@ -0,0 +1,128 @@
+<?php
+/**
+*
+* @package testing
+* @copyright (c) 2010 phpBB Group
+* @license http://opensource.org/licenses/gpl-license.php GNU Public License
+*
+*/
+
+require_once __DIR__ . '/../mock/cache.php';
+
+class phpbb_config_db_test extends phpbb_database_test_case
+{
+	private $cache;
+	private $db;
+	private $config;
+
+	public function getDataSet()
+	{
+		return $this->createXMLDataSet(__DIR__ . '/fixtures/config.xml');
+	}
+
+	public function setUp()
+	{
+		parent::setUp();
+
+		$this->cache = new phpbb_mock_cache;
+		$this->db = $this->new_dbal();
+		$this->config = new phpbb_config_db($this->db, $this->cache, 'phpbb_config');
+	}
+
+	public function test_load_config()
+	{
+		$this->assertEquals('23', $this->config['foo']);
+		$this->assertEquals('42', $this->config['bar']);
+	}
+
+	public function test_load_cached()
+	{
+		$cache = new phpbb_mock_cache(array('config' => array('x' => 'y')));
+		$this->config = new phpbb_config_db($this->db, $cache, 'phpbb_config');
+
+		$this->assertTrue(!isset($this->config['foo']));
+		$this->assertEquals('42', $this->config['bar']);
+
+		$this->assertEquals('y', $this->config['x']);
+	}
+
+	public function test_offset_set()
+	{
+		$this->config['foo'] = 'x'; // temporary set
+		$this->assertEquals('x', $this->config['foo']);
+
+		$config2 = new phpbb_config_db($this->db, $this->cache, 'phpbb_config');
+		$this->assertEquals('23', $config2['foo']);
+	}
+
+	public function test_set_overwrite()
+	{
+		$this->config->set('foo', '17');
+		$this->assertEquals('17', $this->config['foo']);
+
+		// re-read config and populate cache
+		$config2 = new phpbb_config_db($this->db, $this->cache, 'phpbb_config');
+		$this->cache->checkVar($this, 'config', array('foo' => '17'));
+	}
+
+	public function test_set_overwrite_uncached()
+	{
+		$this->config->set('bar', '17', false);
+
+		// re-read config and populate cache
+		$config2 = new phpbb_config_db($this->db, $this->cache, 'phpbb_config');
+		$this->cache->checkVar($this, 'config', array('foo' => '23'));
+	}
+
+	public function test_set_new()
+	{
+		$this->config->set('foobar', '5');
+		$this->assertEquals('5', $this->config['foobar']);
+
+		// re-read config and populate cache
+		$config2 = new phpbb_config_db($this->db, $this->cache, 'phpbb_config');
+		$this->cache->checkVar($this, 'config', array('foo' => '23', 'foobar' => '5'));
+	}
+
+	public function test_set_new_uncached()
+	{
+		$this->config->set('foobar', '5', false);
+		$this->assertEquals('5', $this->config['foobar']);
+
+		// re-read config and populate cache
+		$config2 = new phpbb_config_db($this->db, $this->cache, 'phpbb_config');
+		$this->cache->checkVar($this, 'config', array('foo' => '23'));
+	}
+
+	public function test_set_atomic_overwrite()
+	{
+		$this->assertTrue($this->config->set_atomic('foo', '23', '17'));
+		$this->assertEquals('17', $this->config['foo']);
+	}
+
+	public function test_set_atomic_new()
+	{
+		$this->assertTrue($this->config->set_atomic('foobar', false, '5'));
+		$this->assertEquals('5', $this->config['foobar']);
+	}
+
+	public function test_set_atomic_failure()
+	{
+		$this->assertFalse($this->config->set_atomic('foo', 'wrong', '17'));
+		$this->assertEquals('23', $this->config['foo']);
+	}
+
+	public function test_increment()
+	{
+		$this->config->increment('foo', 3);
+		$this->assertEquals(26, $this->config['foo']);
+		$this->config->increment('foo', 1);
+		$this->assertEquals(27, $this->config['foo']);
+	}
+
+	public function test_increment_new()
+	{
+		$this->config->increment('foobar', 3);
+		$this->assertEquals(3, $this->config['foobar']);;
+	}
+}

tests/config/fixtures/config.xml

@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<dataset>
+	<table name="phpbb_config">
+		<column>config_name</column>
+		<column>config_value</column>
+		<column>is_dynamic</column>
+		<row>
+			<value>foo</value>
+			<value>23</value>
+			<value>0</value>
+		</row>
+		<row>
+			<value>bar</value>
+			<value>42</value>
+			<value>1</value>
+		</row>
+	</table>
+</dataset>

tests/mock/cache.php

@@ -37,6 +37,11 @@ class phpbb_mock_cache implements phpbb_cache_driver_interface
 		$test->assertEquals($data, $this->data[$var_name]);
 	}
 
+	public function checkVarUnset(PHPUnit_Framework_Assert $test, $var_name)
+	{
+		$test->assertFalse(isset($this->data[$var_name]));
+	}
+
 	public function check(PHPUnit_Framework_Assert $test, $data)
 	{
 		$test->assertEquals($data, $this->data);
@@ -59,6 +64,7 @@ class phpbb_mock_cache implements phpbb_cache_driver_interface
 	}
 	function destroy($var_name, $table = '')
 	{
+		unset($this->data[$var_name]);
 	}
 	public function _exists($var_name)
 	{