From 2f67ade05a426d41c95c146a86251d388c718b61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Fr=C3=A8rejean?= Date: Mon, 24 Jan 2011 15:13:15 +0100 Subject: [PATCH 1/5] [ticket/10006] Add phpbb_config::delete Add the missing `phpbb_config::delete` method to the config class PHPBB3-10006 --- phpBB/includes/config/config.php | 20 ++++++++++++++++++++ phpBB/includes/config/db.php | 32 ++++++++++++++++++++++++++++++++ tests/config/config_test.php | 9 +++++++++ tests/config/db_test.php | 7 +++++++ 4 files changed, 68 insertions(+) diff --git a/phpBB/includes/config/config.php b/phpBB/includes/config/config.php index 64fef28cfa1..9596b1e15bd 100644 --- a/phpBB/includes/config/config.php +++ b/phpBB/includes/config/config.php @@ -103,6 +103,26 @@ public function count() return count($this->config); } + /** + * Removes a configuration option + * + * @param String $key The configuration option's name + * @param bool $cache Whether this variable should be cached or if it + * changes too frequently to be efficiently cached + * @return bool True if the configuration entry was deleted successfully, + * otherwise false + */ + public function delete($key, $cache = true) + { + if (!isset($this->config[$key])) + { + return false; + } + + unset($this->config[$key]); + return true; + } + /** * Sets a configuration option's value * diff --git a/phpBB/includes/config/db.php b/phpBB/includes/config/db.php index 74fb0504ce6..f0c9a5d5919 100644 --- a/phpBB/includes/config/db.php +++ b/phpBB/includes/config/db.php @@ -90,6 +90,38 @@ public function __construct(dbal $db, phpbb_cache_driver_interface $cache, $tabl parent::__construct($config); } + /** + * Removes a configuration option + * + * @param String $key The configuration option's name + * @param bool $cache Whether this variable should be cached or if it + * changes too frequently to be efficiently cached + * @return bool True if the configuration entry was deleted successfully, + * otherwise false + */ + public function delete($key, $cache = true) + { + if (!isset($this->config[$key])) + { + return false; + } + + $sql = 'DELETE FROM ' . $this->table . " + WHERE config_name = '" . $this->db->sql_escape($key) . "'"; + $this->db->sql_query($sql); + if (!$this->db->sql_affectedrows()) + { + return false; + } + + unset($this->config[$key]); + + if ($cache) + { + $this->cache->destroy('config'); + } + } + /** * Sets a configuration option's value * diff --git a/tests/config/config_test.php b/tests/config/config_test.php index 73a365c8476..e4444eccda7 100644 --- a/tests/config/config_test.php +++ b/tests/config/config_test.php @@ -109,4 +109,13 @@ public function test_increment() $config->increment('foo', 1); $this->assertEquals(27, $config['foo']); } + + public function test_delete() + { + $config = new phpbb_config(array('foo' => 'bar')); + + $this->assertTrue(isset($config['foo'])); + $config->delete('foo'); + $this->assertFalse(isset($config['foo'])); + } } diff --git a/tests/config/db_test.php b/tests/config/db_test.php index e0d5252f191..d37b31dbbd0 100644 --- a/tests/config/db_test.php +++ b/tests/config/db_test.php @@ -125,4 +125,11 @@ public function test_increment_new() $this->config->increment('foobar', 3); $this->assertEquals(3, $this->config['foobar']);; } + + public function test_delete() + { + $this->assertTrue(isset($this->config['foo'])); + $this->config->delete('foo', false); + $this->assertFalse(isset($this->config['foo'])); + } } From 7b1638c37f9de64a0f6f7c712a063c7ef0d6d4aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Fr=C3=A8rejean?= Date: Mon, 24 Jan 2011 15:22:46 +0100 Subject: [PATCH 2/5] [ticket/10006] Tweak the tests a bit PHPBB3-10006 --- tests/config/config_test.php | 1 - tests/config/db_test.php | 6 +++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/config/config_test.php b/tests/config/config_test.php index e4444eccda7..9c91d9eb874 100644 --- a/tests/config/config_test.php +++ b/tests/config/config_test.php @@ -114,7 +114,6 @@ public function test_delete() { $config = new phpbb_config(array('foo' => 'bar')); - $this->assertTrue(isset($config['foo'])); $config->delete('foo'); $this->assertFalse(isset($config['foo'])); } diff --git a/tests/config/db_test.php b/tests/config/db_test.php index d37b31dbbd0..aa3d40999a9 100644 --- a/tests/config/db_test.php +++ b/tests/config/db_test.php @@ -129,7 +129,11 @@ public function test_increment_new() public function test_delete() { $this->assertTrue(isset($this->config['foo'])); - $this->config->delete('foo', false); + $this->config->delete('foo'); $this->assertFalse(isset($this->config['foo'])); + + // re-read config and populate cache + $config2 = new phpbb_config_db($this->db, $this->cache, 'phpbb_config'); + $this->cache->checkVarUnset($this, 'foo'); } } From 503b87da481d5a77091b519144e8b862b4569f80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Fr=C3=A8rejean?= Date: Mon, 24 Jan 2011 16:33:42 +0100 Subject: [PATCH 3/5] [ticket/10006] More testing Change the DB test to test agains new cache/config mock objects PHPBB3-10006 --- tests/config/db_test.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/config/db_test.php b/tests/config/db_test.php index aa3d40999a9..5bd5296e5a2 100644 --- a/tests/config/db_test.php +++ b/tests/config/db_test.php @@ -130,10 +130,13 @@ public function test_delete() { $this->assertTrue(isset($this->config['foo'])); $this->config->delete('foo'); + $this->cache->checkVarUnset($this, 'foo'); $this->assertFalse(isset($this->config['foo'])); // re-read config and populate cache - $config2 = new phpbb_config_db($this->db, $this->cache, 'phpbb_config'); - $this->cache->checkVarUnset($this, 'foo'); + $cache2 = new phpbb_mock_cache; + $config2 = new phpbb_config_db($this->db, $cache2, 'phpbb_config'); + $cache2->checkVarUnset($this, 'foo'); + $this->assertFalse(isset($config2['foo'])); } } From f93ac340a298dae9f45d99ea2b418532942bd423 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Fr=C3=A8rejean?= Date: Mon, 24 Jan 2011 16:43:09 +0100 Subject: [PATCH 4/5] [ticket/10006] Remove return values Remove some unneeded return values PHPBB3-10006 --- phpBB/includes/config/config.php | 6 ++---- phpBB/includes/config/db.php | 9 ++------- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/phpBB/includes/config/config.php b/phpBB/includes/config/config.php index 9596b1e15bd..ff352abe844 100644 --- a/phpBB/includes/config/config.php +++ b/phpBB/includes/config/config.php @@ -109,18 +109,16 @@ public function count() * @param String $key The configuration option's name * @param bool $cache Whether this variable should be cached or if it * changes too frequently to be efficiently cached - * @return bool True if the configuration entry was deleted successfully, - * otherwise false + * @return void */ public function delete($key, $cache = true) { if (!isset($this->config[$key])) { - return false; + return; } unset($this->config[$key]); - return true; } /** diff --git a/phpBB/includes/config/db.php b/phpBB/includes/config/db.php index f0c9a5d5919..caa2cff7f12 100644 --- a/phpBB/includes/config/db.php +++ b/phpBB/includes/config/db.php @@ -96,23 +96,18 @@ public function __construct(dbal $db, phpbb_cache_driver_interface $cache, $tabl * @param String $key The configuration option's name * @param bool $cache Whether this variable should be cached or if it * changes too frequently to be efficiently cached - * @return bool True if the configuration entry was deleted successfully, - * otherwise false + * @return void */ public function delete($key, $cache = true) { if (!isset($this->config[$key])) { - return false; + return; } $sql = 'DELETE FROM ' . $this->table . " WHERE config_name = '" . $this->db->sql_escape($key) . "'"; $this->db->sql_query($sql); - if (!$this->db->sql_affectedrows()) - { - return false; - } unset($this->config[$key]); From 27bbfde2437302ec0b7848513eb15cb91c8e07ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Fr=C3=A8rejean?= Date: Mon, 24 Jan 2011 22:35:42 +0100 Subject: [PATCH 5/5] [ticket/10006] Remove unneeded if statements Remove some of the additional `if (isset)` checks PHPBB3-10006 --- phpBB/includes/config/config.php | 5 ----- phpBB/includes/config/db.php | 5 ----- 2 files changed, 10 deletions(-) diff --git a/phpBB/includes/config/config.php b/phpBB/includes/config/config.php index ff352abe844..4ba05a98ece 100644 --- a/phpBB/includes/config/config.php +++ b/phpBB/includes/config/config.php @@ -113,11 +113,6 @@ public function count() */ public function delete($key, $cache = true) { - if (!isset($this->config[$key])) - { - return; - } - unset($this->config[$key]); } diff --git a/phpBB/includes/config/db.php b/phpBB/includes/config/db.php index caa2cff7f12..f98056e013d 100644 --- a/phpBB/includes/config/db.php +++ b/phpBB/includes/config/db.php @@ -100,11 +100,6 @@ public function __construct(dbal $db, phpbb_cache_driver_interface $cache, $tabl */ public function delete($key, $cache = true) { - if (!isset($this->config[$key])) - { - return; - } - $sql = 'DELETE FROM ' . $this->table . " WHERE config_name = '" . $this->db->sql_escape($key) . "'"; $this->db->sql_query($sql);