From 07df94def66a78bda40560a5bdd31058f61e2238 Mon Sep 17 00:00:00 2001 From: Bart Visscher Date: Sun, 3 Mar 2013 12:06:00 +0100 Subject: [PATCH 01/17] Convert OC_Config to object interface --- lib/config.php | 111 ++++++++++++++++++----------------------- lib/hintexception.php | 27 ++++++++++ lib/legacy/config.php | 98 ++++++++++++++++++++++++++++++++++++ lib/setup.php | 16 +----- tests/lib/config.php | 113 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 288 insertions(+), 77 deletions(-) create mode 100644 lib/hintexception.php create mode 100644 lib/legacy/config.php create mode 100644 tests/lib/config.php diff --git a/lib/config.php b/lib/config.php index 9b87d4ce4e5..dcc659395a7 100644 --- a/lib/config.php +++ b/lib/config.php @@ -34,17 +34,27 @@ * */ +namespace OC; + /** * This class is responsible for reading and writing config.php, the very basic * configuration file of owncloud. */ -class OC_Config{ +class Config { // associative array key => value - private static $cache = array(); + protected $cache = array(); + + protected $config_dir; + protected $config_filename; - // Is the cache filled? - private static $init = false; + protected $debug_mode; + public function __construct($config_dir, $debug_mode) { + $this->config_dir = $config_dir; + $this->debug_mode = $debug_mode; + $this->config_filename = $this->config_dir.'config.php'; + $this->readData(); + } /** * @brief Lists all available config keys * @return array with key names @@ -52,10 +62,8 @@ class OC_Config{ * This function returns all keys saved in config.php. Please note that it * does not return the values. */ - public static function getKeys() { - self::readData(); - - return array_keys( self::$cache ); + public function getKeys() { + return array_keys( $this->cache ); } /** @@ -67,11 +75,9 @@ class OC_Config{ * This function gets the value from config.php. If it does not exist, * $default will be returned. */ - public static function getValue( $key, $default = null ) { - self::readData(); - - if( array_key_exists( $key, self::$cache )) { - return self::$cache[$key]; + public function getValue( $key, $default = null ) { + if( array_key_exists( $key, $this->cache )) { + return $this->cache[$key]; } return $default; @@ -81,57 +87,43 @@ class OC_Config{ * @brief Sets a value * @param string $key key * @param string $value value - * @return bool * * This function sets the value and writes the config.php. If the file can * not be written, false will be returned. */ - public static function setValue( $key, $value ) { - self::readData(); - + public function setValue( $key, $value ) { // Add change - self::$cache[$key] = $value; + $this->cache[$key] = $value; // Write changes - self::writeData(); - return true; + $this->writeData(); } /** * @brief Removes a key from the config * @param string $key key - * @return bool * * This function removes a key from the config.php. If owncloud has no * write access to config.php, the function will return false. */ - public static function deleteKey( $key ) { - self::readData(); - - if( array_key_exists( $key, self::$cache )) { + public function deleteKey( $key ) { + if( array_key_exists( $key, $this->cache )) { // Delete key from cache - unset( self::$cache[$key] ); + unset( $this->cache[$key] ); // Write changes - self::writeData(); + $this->writeData(); } - - return true; } /** * @brief Loads the config file - * @return bool * * Reads the config file and saves it to the cache */ - private static function readData() { - if( self::$init ) { - return true; - } - + private function readData() { // read all file in config dir ending by config.php - $config_files = glob( OC::$SERVERROOT."/config/*.config.php"); + $config_files = glob( $this->config_dir.'*.config.php'); //Filter only regular files $config_files = array_filter($config_files, 'is_file'); @@ -140,54 +132,49 @@ class OC_Config{ natsort($config_files); // Add default config - array_unshift($config_files,OC::$SERVERROOT."/config/config.php"); + array_unshift($config_files, $this->config_filename); //Include file and merge config - foreach($config_files as $file){ + foreach($config_files as $file) { + if( !file_exists( $file) ) { + continue; + } + unset($CONFIG); include $file; if( isset( $CONFIG ) && is_array( $CONFIG )) { - self::$cache = array_merge(self::$cache, $CONFIG); + $this->cache = array_merge($this->cache, $CONFIG); } } - - // We cached everything - self::$init = true; - - return true; } /** * @brief Writes the config file - * @return bool * * Saves the config to the config file. * */ - public static function writeData() { + private function writeData() { // Create a php file ... - $content = "debug_mode) { $content .= "define('DEBUG',true);\n"; } - $content .= "\$CONFIG = "; - $content .= var_export(self::$cache, true); + $content .= '$CONFIG = '; + $content .= var_export($this->cache, true); $content .= ";\n"; + //var_dump($content, $this); - $filename = OC::$SERVERROOT."/config/config.php"; // Write the file - $result=@file_put_contents( $filename, $content ); + $result=@file_put_contents( $this->config_filename, $content ); if(!$result) { - $tmpl = new OC_Template( '', 'error', 'guest' ); - $tmpl->assign('errors', array(1=>array( - 'error'=>"Can't write into config directory 'config'", - 'hint'=>'You can usually fix this by giving the webserver user write access' - .' to the config directory in owncloud'))); - $tmpl->printPage(); - exit; + throw new HintException( + "Can't write into config directory 'config'", + 'You can usually fix this by giving the webserver user write access' + .' to the config directory in owncloud'); } // Prevent others not to read the config - @chmod($filename, 0640); - - return true; + @chmod($this->config_filename, 0640); } } + +require_once __DIR__.'/legacy/'.basename(__FILE__); diff --git a/lib/hintexception.php b/lib/hintexception.php new file mode 100644 index 00000000000..8c64258435b --- /dev/null +++ b/lib/hintexception.php @@ -0,0 +1,27 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace OC; + +class HintException extends \Exception +{ + private $hint; + + public function __construct($message, $hint, $code = 0, Exception $previous = null) { + $this->hint = $hint; + parent::__construct($message, $code, $previous); + } + + public function __toString() { + return __CLASS__ . ": [{$this->code}]: {$this->message} ({$this->hint})\n"; + } + + public function getHint() { + return $this->hint; + } +} diff --git a/lib/legacy/config.php b/lib/legacy/config.php new file mode 100644 index 00000000000..d030bbe3676 --- /dev/null +++ b/lib/legacy/config.php @@ -0,0 +1,98 @@ +. + * + */ +/* + * + * An example of config.php + * + * "mysql", + * "firstrun" => false, + * "pi" => 3.14 + * ); + * ?> + * + */ + +/** + * This class is responsible for reading and writing config.php, the very basic + * configuration file of owncloud. + */ +OC_Config::$object = new \OC\Config(OC::$SERVERROOT.'/config/', defined('DEBUG') && DEBUG); +class OC_Config{ + public static $object; + /** + * @brief Lists all available config keys + * @return array with key names + * + * This function returns all keys saved in config.php. Please note that it + * does not return the values. + */ + public static function getKeys() { + return self::$object->getKeys(); + } + + /** + * @brief Gets a value from config.php + * @param string $key key + * @param string $default = null default value + * @return string the value or $default + * + * This function gets the value from config.php. If it does not exist, + * $default will be returned. + */ + public static function getValue( $key, $default = null ) { + return self::$object->getValue( $key, $default ); + } + + /** + * @brief Sets a value + * @param string $key key + * @param string $value value + * + * This function sets the value and writes the config.php. If the file can + * not be written, false will be returned. + */ + public static function setValue( $key, $value ) { + try { + self::$object->setValue( $key, $value ); + } catch (\OC\HintException $e) { + \OC_Template::printErrorPage( $e->getMessage(), $e->getHint() ); + } + } + + /** + * @brief Removes a key from the config + * @param string $key key + * + * This function removes a key from the config.php. If owncloud has no + * write access to config.php, the function will return false. + */ + public static function deleteKey( $key ) { + try { + self::$object->deleteKey( $key ); + } catch (\OC\HintException $e) { + \OC_Template::printErrorPage( $e->getMessage(), $e->getHint() ); + } + } +} diff --git a/lib/setup.php b/lib/setup.php index d1197b3ebf3..e05db554320 100644 --- a/lib/setup.php +++ b/lib/setup.php @@ -1,21 +1,7 @@ hint = $hint; - parent::__construct($message, $code, $previous); - } - - public function __toString() { - return __CLASS__ . ": [{$this->code}]: {$this->message} ({$this->hint})\n"; - } - - public function getHint() { - return $this->hint; - } } class OC_Setup { diff --git a/tests/lib/config.php b/tests/lib/config.php new file mode 100644 index 00000000000..e22bf3fd7de --- /dev/null +++ b/tests/lib/config.php @@ -0,0 +1,113 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +class Test_Config extends PHPUnit_Framework_TestCase { + const CONFIG_FILE = 'static://config.php'; + const CONFIG_DIR = 'static://'; + const TESTCONTENT = '"bar");'; + + public function testReadData() + { + $config = new OC\Config(self::CONFIG_DIR, false); + $this->assertAttributeEquals(array(), 'cache', $config); + + file_put_contents(self::CONFIG_FILE, self::TESTCONTENT); + $config = new OC\Config(self::CONFIG_DIR, false); + $this->assertAttributeEquals(array('foo'=>'bar'), 'cache', $config); + } + + public function testGetKeys() + { + file_put_contents(self::CONFIG_FILE, self::TESTCONTENT); + $config = new OC\Config(self::CONFIG_DIR, false); + $this->assertEquals(array('foo'), $config->getKeys()); + } + + public function testGetValue() + { + file_put_contents(self::CONFIG_FILE, self::TESTCONTENT); + $config = new OC\Config(self::CONFIG_DIR, false); + $this->assertEquals('bar', $config->getValue('foo')); + $this->assertEquals(null, $config->getValue('bar')); + $this->assertEquals('moo', $config->getValue('bar', 'moo')); + } + + public function testSetValue() + { + file_put_contents(self::CONFIG_FILE, self::TESTCONTENT); + $config = new OC\Config(self::CONFIG_DIR, false); + $config->setValue('foo', 'moo'); + $this->assertAttributeEquals(array('foo'=>'moo'), 'cache', $config); + $content = file_get_contents(self::CONFIG_FILE); + $this->assertEquals(<< 'moo', +); + +EOL +, $content); + $config->setValue('bar', 'red'); + $this->assertAttributeEquals(array('foo'=>'moo', 'bar'=>'red'), 'cache', $config); + $content = file_get_contents(self::CONFIG_FILE); + $this->assertEquals(<< 'moo', + 'bar' => 'red', +); + +EOL +, $content); + } + + public function testDeleteKey() + { + file_put_contents(self::CONFIG_FILE, self::TESTCONTENT); + $config = new OC\Config(self::CONFIG_DIR, false); + $config->deleteKey('foo'); + $this->assertAttributeEquals(array(), 'cache', $config); + $content = file_get_contents(self::CONFIG_FILE); + $this->assertEquals(<<deleteKey('foo'); // change something so we save to the config file + $this->assertAttributeEquals(array(), 'cache', $config); + $this->assertAttributeEquals(true, 'debug_mode', $config); + $content = file_get_contents(self::CONFIG_FILE); + $this->assertEquals(<<setValue('foo', 'bar'); + } catch (\OC\HintException $e) { + return; + } + $this->fail(); + } +} From e620286b253bcd43806fa9deab5e2e67decda582 Mon Sep 17 00:00:00 2001 From: Bart Visscher Date: Tue, 19 Mar 2013 08:47:29 +0100 Subject: [PATCH 02/17] Fix returns of values in OCP\Config --- lib/public/config.php | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/lib/public/config.php b/lib/public/config.php index 8076d640b49..73476d7551d 100644 --- a/lib/public/config.php +++ b/lib/public/config.php @@ -49,7 +49,7 @@ class Config { * $default will be returned. */ public static function getSystemValue( $key, $default = null ) { - return(\OC_Config::getValue( $key, $default )); + return \OC_Config::getValue( $key, $default ); } /** @@ -62,7 +62,12 @@ class Config { * not be written, false will be returned. */ public static function setSystemValue( $key, $value ) { - return(\OC_Config::setValue( $key, $value )); + try { + \OC_Config::setValue( $key, $value ); + } catch (Exception $e) { + return false; + } + return true; } /** @@ -76,7 +81,7 @@ class Config { * not exist the default value will be returned */ public static function getAppValue( $app, $key, $default = null ) { - return(\OC_Appconfig::getValue( $app, $key, $default )); + return \OC_Appconfig::getValue( $app, $key, $default ); } /** @@ -89,7 +94,12 @@ class Config { * Sets a value. If the key did not exist before it will be created. */ public static function setAppValue( $app, $key, $value ) { - return(\OC_Appconfig::setValue( $app, $key, $value )); + try { + \OC_Appconfig::setValue( $app, $key, $value ); + } catch (Exception $e) { + return false; + } + return true; } /** @@ -104,7 +114,7 @@ class Config { * not exist the default value will be returned */ public static function getUserValue( $user, $app, $key, $default = null ) { - return(\OC_Preferences::getValue( $user, $app, $key, $default )); + return \OC_Preferences::getValue( $user, $app, $key, $default ); } /** @@ -119,6 +129,11 @@ class Config { * will be added automagically. */ public static function setUserValue( $user, $app, $key, $value ) { - return(\OC_Preferences::setValue( $user, $app, $key, $value )); + try { + \OC_Preferences::setValue( $user, $app, $key, $value ); + } catch (Exception $e) { + return false; + } + return true; } } From 9f5b7657fb27d86792a034d3665efdac6eb304e5 Mon Sep 17 00:00:00 2001 From: Bart Visscher Date: Wed, 8 May 2013 18:17:26 +0200 Subject: [PATCH 03/17] Remove include for loading legacy class --- lib/config.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/config.php b/lib/config.php index dcc659395a7..1c27292d6b3 100644 --- a/lib/config.php +++ b/lib/config.php @@ -176,5 +176,3 @@ class Config { @chmod($this->config_filename, 0640); } } - -require_once __DIR__.'/legacy/'.basename(__FILE__); From 83444d9c641b77144cd74e5dc71c5ad18964944e Mon Sep 17 00:00:00 2001 From: Bart Visscher Date: Wed, 8 May 2013 18:20:44 +0200 Subject: [PATCH 04/17] camelCase class properties --- lib/config.php | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/lib/config.php b/lib/config.php index 1c27292d6b3..63301cf0ab2 100644 --- a/lib/config.php +++ b/lib/config.php @@ -44,15 +44,15 @@ class Config { // associative array key => value protected $cache = array(); - protected $config_dir; - protected $config_filename; + protected $configDir; + protected $configFilename; - protected $debug_mode; + protected $debugMode; - public function __construct($config_dir, $debug_mode) { - $this->config_dir = $config_dir; - $this->debug_mode = $debug_mode; - $this->config_filename = $this->config_dir.'config.php'; + public function __construct($configDir, $debugMode) { + $this->configDir = $configDir; + $this->debugMode = $debugMode; + $this->configFilename = $this->configDir.'config.php'; $this->readData(); } /** @@ -123,19 +123,19 @@ class Config { */ private function readData() { // read all file in config dir ending by config.php - $config_files = glob( $this->config_dir.'*.config.php'); + $configFiles = glob( $this->configDir.'*.config.php'); //Filter only regular files - $config_files = array_filter($config_files, 'is_file'); + $configFiles = array_filter($configFiles, 'is_file'); //Sort array naturally : - natsort($config_files); + natsort($configFiles); // Add default config - array_unshift($config_files, $this->config_filename); + array_unshift($configFiles, $this->configFilename); //Include file and merge config - foreach($config_files as $file) { + foreach($configFiles as $file) { if( !file_exists( $file) ) { continue; } @@ -156,16 +156,15 @@ class Config { private function writeData() { // Create a php file ... $content = "debug_mode) { + if ($this->debugMode) { $content .= "define('DEBUG',true);\n"; } $content .= '$CONFIG = '; $content .= var_export($this->cache, true); $content .= ";\n"; - //var_dump($content, $this); // Write the file - $result=@file_put_contents( $this->config_filename, $content ); + $result=@file_put_contents( $this->configFilename, $content ); if(!$result) { throw new HintException( "Can't write into config directory 'config'", @@ -173,6 +172,6 @@ class Config { .' to the config directory in owncloud'); } // Prevent others not to read the config - @chmod($this->config_filename, 0640); + @chmod($this->configFilename, 0640); } } From d50d663928a924a359b504c20406550758da7c2d Mon Sep 17 00:00:00 2001 From: Michael Gapczynski Date: Mon, 3 Jun 2013 18:05:38 -0400 Subject: [PATCH 05/17] Style and comment fixes --- lib/config.php | 38 +++++++++++++++++++------------------- lib/hintexception.php | 4 ++-- lib/legacy/config.php | 30 ++++++++++++++++-------------- 3 files changed, 37 insertions(+), 35 deletions(-) diff --git a/lib/config.php b/lib/config.php index 63301cf0ab2..563e8ec8696 100644 --- a/lib/config.php +++ b/lib/config.php @@ -38,7 +38,7 @@ namespace OC; /** * This class is responsible for reading and writing config.php, the very basic - * configuration file of owncloud. + * configuration file of ownCloud. */ class Config { // associative array key => value @@ -63,7 +63,7 @@ class Config { * does not return the values. */ public function getKeys() { - return array_keys( $this->cache ); + return array_keys($this->cache); } /** @@ -75,8 +75,8 @@ class Config { * This function gets the value from config.php. If it does not exist, * $default will be returned. */ - public function getValue( $key, $default = null ) { - if( array_key_exists( $key, $this->cache )) { + public function getValue($key, $default = null) { + if (isset($this->cache[$key])) { return $this->cache[$key]; } @@ -88,10 +88,10 @@ class Config { * @param string $key key * @param string $value value * - * This function sets the value and writes the config.php. If the file can - * not be written, false will be returned. + * This function sets the value and writes the config.php. + * */ - public function setValue( $key, $value ) { + public function setValue($key, $value) { // Add change $this->cache[$key] = $value; @@ -103,13 +103,13 @@ class Config { * @brief Removes a key from the config * @param string $key key * - * This function removes a key from the config.php. If owncloud has no - * write access to config.php, the function will return false. + * This function removes a key from the config.php. + * */ - public function deleteKey( $key ) { - if( array_key_exists( $key, $this->cache )) { + public function deleteKey($key) { + if (isset($this->cache[$key])) { // Delete key from cache - unset( $this->cache[$key] ); + unset($this->cache[$key]); // Write changes $this->writeData(); @@ -123,7 +123,7 @@ class Config { */ private function readData() { // read all file in config dir ending by config.php - $configFiles = glob( $this->configDir.'*.config.php'); + $configFiles = glob($this->configDir.'*.config.php'); //Filter only regular files $configFiles = array_filter($configFiles, 'is_file'); @@ -135,13 +135,13 @@ class Config { array_unshift($configFiles, $this->configFilename); //Include file and merge config - foreach($configFiles as $file) { - if( !file_exists( $file) ) { + foreach ($configFiles as $file) { + if (!file_exists($file)) { continue; } unset($CONFIG); include $file; - if( isset( $CONFIG ) && is_array( $CONFIG )) { + if (isset($CONFIG) && is_array($CONFIG)) { $this->cache = array_merge($this->cache, $CONFIG); } } @@ -164,12 +164,12 @@ class Config { $content .= ";\n"; // Write the file - $result=@file_put_contents( $this->configFilename, $content ); - if(!$result) { + $result = @file_put_contents( $this->configFilename, $content); + if (!$result) { throw new HintException( "Can't write into config directory 'config'", 'You can usually fix this by giving the webserver user write access' - .' to the config directory in owncloud'); + .' to the config directory in ownCloud'); } // Prevent others not to read the config @chmod($this->configFilename, 0640); diff --git a/lib/hintexception.php b/lib/hintexception.php index 8c64258435b..c8bff750033 100644 --- a/lib/hintexception.php +++ b/lib/hintexception.php @@ -8,8 +8,8 @@ namespace OC; -class HintException extends \Exception -{ +class HintException extends \Exception { + private $hint; public function __construct($message, $hint, $code = 0, Exception $previous = null) { diff --git a/lib/legacy/config.php b/lib/legacy/config.php index d030bbe3676..635f0af66f8 100644 --- a/lib/legacy/config.php +++ b/lib/legacy/config.php @@ -36,11 +36,13 @@ /** * This class is responsible for reading and writing config.php, the very basic - * configuration file of owncloud. + * configuration file of ownCloud. */ OC_Config::$object = new \OC\Config(OC::$SERVERROOT.'/config/', defined('DEBUG') && DEBUG); -class OC_Config{ +class OC_Config { + public static $object; + /** * @brief Lists all available config keys * @return array with key names @@ -61,8 +63,8 @@ class OC_Config{ * This function gets the value from config.php. If it does not exist, * $default will be returned. */ - public static function getValue( $key, $default = null ) { - return self::$object->getValue( $key, $default ); + public static function getValue($key, $default = null) { + return self::$object->getValue($key, $default); } /** @@ -70,14 +72,14 @@ class OC_Config{ * @param string $key key * @param string $value value * - * This function sets the value and writes the config.php. If the file can - * not be written, false will be returned. + * This function sets the value and writes the config.php. + * */ - public static function setValue( $key, $value ) { + public static function setValue($key, $value) { try { - self::$object->setValue( $key, $value ); + self::$object->setValue($key, $value); } catch (\OC\HintException $e) { - \OC_Template::printErrorPage( $e->getMessage(), $e->getHint() ); + \OC_Template::printErrorPage($e->getMessage(), $e->getHint()); } } @@ -85,14 +87,14 @@ class OC_Config{ * @brief Removes a key from the config * @param string $key key * - * This function removes a key from the config.php. If owncloud has no - * write access to config.php, the function will return false. + * This function removes a key from the config.php. + * */ - public static function deleteKey( $key ) { + public static function deleteKey($key) { try { - self::$object->deleteKey( $key ); + self::$object->deleteKey($key); } catch (\OC\HintException $e) { - \OC_Template::printErrorPage( $e->getMessage(), $e->getHint() ); + \OC_Template::printErrorPage($e->getMessage(), $e->getHint()); } } } From e0359b0b24a2fbc490fa1c96ee722ef82a191c04 Mon Sep 17 00:00:00 2001 From: Michael Gapczynski Date: Mon, 3 Jun 2013 18:17:32 -0400 Subject: [PATCH 06/17] One more style fix --- lib/config.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/config.php b/lib/config.php index 563e8ec8696..a6095296453 100644 --- a/lib/config.php +++ b/lib/config.php @@ -164,7 +164,7 @@ class Config { $content .= ";\n"; // Write the file - $result = @file_put_contents( $this->configFilename, $content); + $result = @file_put_contents($this->configFilename, $content); if (!$result) { throw new HintException( "Can't write into config directory 'config'", From b7b6075d55abdf656128c0044d6649c976e40a00 Mon Sep 17 00:00:00 2001 From: Michael Gapczynski Date: Mon, 10 Jun 2013 11:42:20 -0400 Subject: [PATCH 07/17] Fix potential glob error --- lib/config.php | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/lib/config.php b/lib/config.php index a6095296453..4003339ea5c 100644 --- a/lib/config.php +++ b/lib/config.php @@ -122,21 +122,17 @@ class Config { * Reads the config file and saves it to the cache */ private function readData() { - // read all file in config dir ending by config.php - $configFiles = glob($this->configDir.'*.config.php'); - - //Filter only regular files - $configFiles = array_filter($configFiles, 'is_file'); - - //Sort array naturally : - natsort($configFiles); - - // Add default config - array_unshift($configFiles, $this->configFilename); - - //Include file and merge config + // Default config + $configFiles = array($this->configFilename); + // Add all files in the config dir ending with config.php + $extra = glob($this->configDir.'*.config.php'); + if (is_array($extra)) { + natsort($extra); + $configFiles = array_merge($configFiles, $extra); + } + // Include file and merge config foreach ($configFiles as $file) { - if (!file_exists($file)) { + if (!is_file($file)) { continue; } unset($CONFIG); From 969e43c87b7afb6184846fe27849167c9c6f5eab Mon Sep 17 00:00:00 2001 From: Michael Gapczynski Date: Mon, 10 Jun 2013 12:07:25 -0400 Subject: [PATCH 08/17] Can't determine if debug mode is defined until we read the config --- lib/config.php | 9 +++------ lib/legacy/config.php | 2 +- tests/lib/config.php | 18 +++++++++--------- 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/lib/config.php b/lib/config.php index 4003339ea5c..a3663949d16 100644 --- a/lib/config.php +++ b/lib/config.php @@ -47,11 +47,8 @@ class Config { protected $configDir; protected $configFilename; - protected $debugMode; - - public function __construct($configDir, $debugMode) { + public function __construct($configDir) { $this->configDir = $configDir; - $this->debugMode = $debugMode; $this->configFilename = $this->configDir.'config.php'; $this->readData(); } @@ -152,7 +149,7 @@ class Config { private function writeData() { // Create a php file ... $content = "debugMode) { + if (defined('DEBUG') && DEBUG) { $content .= "define('DEBUG',true);\n"; } $content .= '$CONFIG = '; @@ -167,7 +164,7 @@ class Config { 'You can usually fix this by giving the webserver user write access' .' to the config directory in ownCloud'); } - // Prevent others not to read the config + // Prevent others from reading the config @chmod($this->configFilename, 0640); } } diff --git a/lib/legacy/config.php b/lib/legacy/config.php index 635f0af66f8..f68d7c31b25 100644 --- a/lib/legacy/config.php +++ b/lib/legacy/config.php @@ -38,7 +38,7 @@ * This class is responsible for reading and writing config.php, the very basic * configuration file of ownCloud. */ -OC_Config::$object = new \OC\Config(OC::$SERVERROOT.'/config/', defined('DEBUG') && DEBUG); +OC_Config::$object = new \OC\Config(OC::$SERVERROOT.'/config/'); class OC_Config { public static $object; diff --git a/tests/lib/config.php b/tests/lib/config.php index e22bf3fd7de..acc2a536fd0 100644 --- a/tests/lib/config.php +++ b/tests/lib/config.php @@ -13,25 +13,25 @@ class Test_Config extends PHPUnit_Framework_TestCase { public function testReadData() { - $config = new OC\Config(self::CONFIG_DIR, false); + $config = new OC\Config(self::CONFIG_DIR); $this->assertAttributeEquals(array(), 'cache', $config); file_put_contents(self::CONFIG_FILE, self::TESTCONTENT); - $config = new OC\Config(self::CONFIG_DIR, false); + $config = new OC\Config(self::CONFIG_DIR); $this->assertAttributeEquals(array('foo'=>'bar'), 'cache', $config); } public function testGetKeys() { file_put_contents(self::CONFIG_FILE, self::TESTCONTENT); - $config = new OC\Config(self::CONFIG_DIR, false); + $config = new OC\Config(self::CONFIG_DIR); $this->assertEquals(array('foo'), $config->getKeys()); } public function testGetValue() { file_put_contents(self::CONFIG_FILE, self::TESTCONTENT); - $config = new OC\Config(self::CONFIG_DIR, false); + $config = new OC\Config(self::CONFIG_DIR); $this->assertEquals('bar', $config->getValue('foo')); $this->assertEquals(null, $config->getValue('bar')); $this->assertEquals('moo', $config->getValue('bar', 'moo')); @@ -40,7 +40,7 @@ class Test_Config extends PHPUnit_Framework_TestCase { public function testSetValue() { file_put_contents(self::CONFIG_FILE, self::TESTCONTENT); - $config = new OC\Config(self::CONFIG_DIR, false); + $config = new OC\Config(self::CONFIG_DIR); $config->setValue('foo', 'moo'); $this->assertAttributeEquals(array('foo'=>'moo'), 'cache', $config); $content = file_get_contents(self::CONFIG_FILE); @@ -69,7 +69,7 @@ EOL public function testDeleteKey() { file_put_contents(self::CONFIG_FILE, self::TESTCONTENT); - $config = new OC\Config(self::CONFIG_DIR, false); + $config = new OC\Config(self::CONFIG_DIR); $config->deleteKey('foo'); $this->assertAttributeEquals(array(), 'cache', $config); $content = file_get_contents(self::CONFIG_FILE); @@ -84,11 +84,11 @@ EOL public function testSavingDebugMode() { + define('DEBUG',true); file_put_contents(self::CONFIG_FILE, self::TESTCONTENT); - $config = new OC\Config(self::CONFIG_DIR, true); + $config = new OC\Config(self::CONFIG_DIR); $config->deleteKey('foo'); // change something so we save to the config file $this->assertAttributeEquals(array(), 'cache', $config); - $this->assertAttributeEquals(true, 'debug_mode', $config); $content = file_get_contents(self::CONFIG_FILE); $this->assertEquals(<<setValue('foo', 'bar'); } catch (\OC\HintException $e) { From 64f16f1db1f05e032080c885ebf91f38f659e62f Mon Sep 17 00:00:00 2001 From: Bart Visscher Date: Thu, 27 Jun 2013 21:57:59 +0200 Subject: [PATCH 09/17] Fix stupid namespace separator --- lib/config.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/config.php b/lib/config.php index e204de0baee..19d58c90443 100644 --- a/lib/config.php +++ b/lib/config.php @@ -166,6 +166,6 @@ class Config { } // Prevent others from reading the config @chmod($this->configFilename, 0640); - OC_Util::clearOpcodeCache(); + \OC_Util::clearOpcodeCache(); } } From ae2b3732de4eeced50a71f9074d0b5dadf625edd Mon Sep 17 00:00:00 2001 From: Bart Visscher Date: Thu, 27 Jun 2013 22:23:53 +0200 Subject: [PATCH 10/17] Use file_exists to fix the unittests --- lib/config.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/config.php b/lib/config.php index 19d58c90443..7ccbc050508 100644 --- a/lib/config.php +++ b/lib/config.php @@ -129,7 +129,7 @@ class Config { } // Include file and merge config foreach ($configFiles as $file) { - if (!is_file($file)) { + if (!file_exists($file)) { continue; } unset($CONFIG); From 194b61b4c507e58eab0750ab40ed6eb6f085c06a Mon Sep 17 00:00:00 2001 From: Bart Visscher Date: Thu, 27 Jun 2013 22:24:17 +0200 Subject: [PATCH 11/17] Revert "Can't determine if debug mode is defined until we read the config" This reverts commit 969e43c87b7afb6184846fe27849167c9c6f5eab. --- lib/config.php | 9 ++++++--- lib/legacy/config.php | 2 +- tests/lib/config.php | 18 +++++++++--------- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/lib/config.php b/lib/config.php index 7ccbc050508..adf70ac841a 100644 --- a/lib/config.php +++ b/lib/config.php @@ -47,8 +47,11 @@ class Config { protected $configDir; protected $configFilename; - public function __construct($configDir) { + protected $debugMode; + + public function __construct($configDir, $debugMode) { $this->configDir = $configDir; + $this->debugMode = $debugMode; $this->configFilename = $this->configDir.'config.php'; $this->readData(); } @@ -149,7 +152,7 @@ class Config { private function writeData() { // Create a php file ... $content = "debugMode) { $content .= "define('DEBUG',true);\n"; } $content .= '$CONFIG = '; @@ -164,7 +167,7 @@ class Config { 'You can usually fix this by giving the webserver user write access' .' to the config directory in ownCloud'); } - // Prevent others from reading the config + // Prevent others not to read the config @chmod($this->configFilename, 0640); \OC_Util::clearOpcodeCache(); } diff --git a/lib/legacy/config.php b/lib/legacy/config.php index f68d7c31b25..635f0af66f8 100644 --- a/lib/legacy/config.php +++ b/lib/legacy/config.php @@ -38,7 +38,7 @@ * This class is responsible for reading and writing config.php, the very basic * configuration file of ownCloud. */ -OC_Config::$object = new \OC\Config(OC::$SERVERROOT.'/config/'); +OC_Config::$object = new \OC\Config(OC::$SERVERROOT.'/config/', defined('DEBUG') && DEBUG); class OC_Config { public static $object; diff --git a/tests/lib/config.php b/tests/lib/config.php index acc2a536fd0..e22bf3fd7de 100644 --- a/tests/lib/config.php +++ b/tests/lib/config.php @@ -13,25 +13,25 @@ class Test_Config extends PHPUnit_Framework_TestCase { public function testReadData() { - $config = new OC\Config(self::CONFIG_DIR); + $config = new OC\Config(self::CONFIG_DIR, false); $this->assertAttributeEquals(array(), 'cache', $config); file_put_contents(self::CONFIG_FILE, self::TESTCONTENT); - $config = new OC\Config(self::CONFIG_DIR); + $config = new OC\Config(self::CONFIG_DIR, false); $this->assertAttributeEquals(array('foo'=>'bar'), 'cache', $config); } public function testGetKeys() { file_put_contents(self::CONFIG_FILE, self::TESTCONTENT); - $config = new OC\Config(self::CONFIG_DIR); + $config = new OC\Config(self::CONFIG_DIR, false); $this->assertEquals(array('foo'), $config->getKeys()); } public function testGetValue() { file_put_contents(self::CONFIG_FILE, self::TESTCONTENT); - $config = new OC\Config(self::CONFIG_DIR); + $config = new OC\Config(self::CONFIG_DIR, false); $this->assertEquals('bar', $config->getValue('foo')); $this->assertEquals(null, $config->getValue('bar')); $this->assertEquals('moo', $config->getValue('bar', 'moo')); @@ -40,7 +40,7 @@ class Test_Config extends PHPUnit_Framework_TestCase { public function testSetValue() { file_put_contents(self::CONFIG_FILE, self::TESTCONTENT); - $config = new OC\Config(self::CONFIG_DIR); + $config = new OC\Config(self::CONFIG_DIR, false); $config->setValue('foo', 'moo'); $this->assertAttributeEquals(array('foo'=>'moo'), 'cache', $config); $content = file_get_contents(self::CONFIG_FILE); @@ -69,7 +69,7 @@ EOL public function testDeleteKey() { file_put_contents(self::CONFIG_FILE, self::TESTCONTENT); - $config = new OC\Config(self::CONFIG_DIR); + $config = new OC\Config(self::CONFIG_DIR, false); $config->deleteKey('foo'); $this->assertAttributeEquals(array(), 'cache', $config); $content = file_get_contents(self::CONFIG_FILE); @@ -84,11 +84,11 @@ EOL public function testSavingDebugMode() { - define('DEBUG',true); file_put_contents(self::CONFIG_FILE, self::TESTCONTENT); - $config = new OC\Config(self::CONFIG_DIR); + $config = new OC\Config(self::CONFIG_DIR, true); $config->deleteKey('foo'); // change something so we save to the config file $this->assertAttributeEquals(array(), 'cache', $config); + $this->assertAttributeEquals(true, 'debug_mode', $config); $content = file_get_contents(self::CONFIG_FILE); $this->assertEquals(<<setValue('foo', 'bar'); } catch (\OC\HintException $e) { From 12976fb2e1f6a4d6a054ba2b620f0e7707ce2c69 Mon Sep 17 00:00:00 2001 From: Bart Visscher Date: Thu, 27 Jun 2013 22:50:28 +0200 Subject: [PATCH 12/17] Set debugMode after reading the config file --- lib/config.php | 9 ++++++-- lib/legacy/config.php | 2 +- tests/lib/config.php | 50 +++++++++++++++++++------------------------ 3 files changed, 30 insertions(+), 31 deletions(-) diff --git a/lib/config.php b/lib/config.php index adf70ac841a..afd74c56b40 100644 --- a/lib/config.php +++ b/lib/config.php @@ -49,12 +49,17 @@ class Config { protected $debugMode; - public function __construct($configDir, $debugMode) { + public function __construct($configDir) { $this->configDir = $configDir; - $this->debugMode = $debugMode; $this->configFilename = $this->configDir.'config.php'; $this->readData(); + $this->setDebugMode(defined('DEBUG') && DEBUG); } + + public function setDebugMode($enable) { + $this->debugMode = $enable; + } + /** * @brief Lists all available config keys * @return array with key names diff --git a/lib/legacy/config.php b/lib/legacy/config.php index 635f0af66f8..f68d7c31b25 100644 --- a/lib/legacy/config.php +++ b/lib/legacy/config.php @@ -38,7 +38,7 @@ * This class is responsible for reading and writing config.php, the very basic * configuration file of ownCloud. */ -OC_Config::$object = new \OC\Config(OC::$SERVERROOT.'/config/', defined('DEBUG') && DEBUG); +OC_Config::$object = new \OC\Config(OC::$SERVERROOT.'/config/'); class OC_Config { public static $object; diff --git a/tests/lib/config.php b/tests/lib/config.php index e22bf3fd7de..8f52cf4ae76 100644 --- a/tests/lib/config.php +++ b/tests/lib/config.php @@ -11,38 +11,35 @@ class Test_Config extends PHPUnit_Framework_TestCase { const CONFIG_DIR = 'static://'; const TESTCONTENT = '"bar");'; + function setUp() { + file_put_contents(self::CONFIG_FILE, self::TESTCONTENT); + $this->config = new OC\Config(self::CONFIG_DIR); + } + public function testReadData() { - $config = new OC\Config(self::CONFIG_DIR, false); + $config = new OC\Config('/non-existing'); $this->assertAttributeEquals(array(), 'cache', $config); - file_put_contents(self::CONFIG_FILE, self::TESTCONTENT); - $config = new OC\Config(self::CONFIG_DIR, false); - $this->assertAttributeEquals(array('foo'=>'bar'), 'cache', $config); + $this->assertAttributeEquals(array('foo'=>'bar'), 'cache', $this->config); } public function testGetKeys() { - file_put_contents(self::CONFIG_FILE, self::TESTCONTENT); - $config = new OC\Config(self::CONFIG_DIR, false); - $this->assertEquals(array('foo'), $config->getKeys()); + $this->assertEquals(array('foo'), $this->config->getKeys()); } public function testGetValue() { - file_put_contents(self::CONFIG_FILE, self::TESTCONTENT); - $config = new OC\Config(self::CONFIG_DIR, false); - $this->assertEquals('bar', $config->getValue('foo')); - $this->assertEquals(null, $config->getValue('bar')); - $this->assertEquals('moo', $config->getValue('bar', 'moo')); + $this->assertEquals('bar', $this->config->getValue('foo')); + $this->assertEquals(null, $this->config->getValue('bar')); + $this->assertEquals('moo', $this->config->getValue('bar', 'moo')); } public function testSetValue() { - file_put_contents(self::CONFIG_FILE, self::TESTCONTENT); - $config = new OC\Config(self::CONFIG_DIR, false); - $config->setValue('foo', 'moo'); - $this->assertAttributeEquals(array('foo'=>'moo'), 'cache', $config); + $this->config->setValue('foo', 'moo'); + $this->assertAttributeEquals(array('foo'=>'moo'), 'cache', $this->config); $content = file_get_contents(self::CONFIG_FILE); $this->assertEquals(<<setValue('bar', 'red'); - $this->assertAttributeEquals(array('foo'=>'moo', 'bar'=>'red'), 'cache', $config); + $this->config->setValue('bar', 'red'); + $this->assertAttributeEquals(array('foo'=>'moo', 'bar'=>'red'), 'cache', $this->config); $content = file_get_contents(self::CONFIG_FILE); $this->assertEquals(<<deleteKey('foo'); - $this->assertAttributeEquals(array(), 'cache', $config); + $this->config->deleteKey('foo'); + $this->assertAttributeEquals(array(), 'cache', $this->config); $content = file_get_contents(self::CONFIG_FILE); $this->assertEquals(<<deleteKey('foo'); // change something so we save to the config file - $this->assertAttributeEquals(array(), 'cache', $config); - $this->assertAttributeEquals(true, 'debug_mode', $config); + $this->config->setDebugMode(true); + $this->config->deleteKey('foo'); // change something so we save to the config file + $this->assertAttributeEquals(array(), 'cache', $this->config); + $this->assertAttributeEquals(true, 'debugMode', $this->config); $content = file_get_contents(self::CONFIG_FILE); $this->assertEquals(<<setValue('foo', 'bar'); } catch (\OC\HintException $e) { From e789e056759aedb93d9eba3a8598acea67c842c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Tue, 2 Jul 2013 00:15:42 +0200 Subject: [PATCH 13/17] on unit test use @expectedException some phpdoc added --- lib/legacy/config.php | 3 +++ tests/lib/config.php | 15 +++++++++------ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/lib/legacy/config.php b/lib/legacy/config.php index f68d7c31b25..5294a48ea44 100644 --- a/lib/legacy/config.php +++ b/lib/legacy/config.php @@ -41,6 +41,9 @@ OC_Config::$object = new \OC\Config(OC::$SERVERROOT.'/config/'); class OC_Config { + /** + * @var \OC\Config + */ public static $object; /** diff --git a/tests/lib/config.php b/tests/lib/config.php index 8f52cf4ae76..c17d2ae7eff 100644 --- a/tests/lib/config.php +++ b/tests/lib/config.php @@ -11,6 +11,11 @@ class Test_Config extends PHPUnit_Framework_TestCase { const CONFIG_DIR = 'static://'; const TESTCONTENT = '"bar");'; + /** + * @var \OC\Config + */ + private $config; + function setUp() { file_put_contents(self::CONFIG_FILE, self::TESTCONTENT); $this->config = new OC\Config(self::CONFIG_DIR); @@ -94,14 +99,12 @@ EOL , $content); } + /** + * @expectedException \OC\HintException + */ public function testWriteData() { $config = new OC\Config('/non-writable'); - try { - $config->setValue('foo', 'bar'); - } catch (\OC\HintException $e) { - return; - } - $this->fail(); + $config->setValue('foo', 'bar'); } } From f29dd1c784b736e5d5398f936733409c0db0160d Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 5 Jul 2013 15:25:53 +0200 Subject: [PATCH 14/17] fix test case whitespace --- tests/lib/config.php | 35 ++++++++++++++--------------------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/tests/lib/config.php b/tests/lib/config.php index c17d2ae7eff..12473eb6676 100644 --- a/tests/lib/config.php +++ b/tests/lib/config.php @@ -21,30 +21,26 @@ class Test_Config extends PHPUnit_Framework_TestCase { $this->config = new OC\Config(self::CONFIG_DIR); } - public function testReadData() - { + public function testReadData() { $config = new OC\Config('/non-existing'); $this->assertAttributeEquals(array(), 'cache', $config); - $this->assertAttributeEquals(array('foo'=>'bar'), 'cache', $this->config); + $this->assertAttributeEquals(array('foo' => 'bar'), 'cache', $this->config); } - public function testGetKeys() - { + public function testGetKeys() { $this->assertEquals(array('foo'), $this->config->getKeys()); } - public function testGetValue() - { + public function testGetValue() { $this->assertEquals('bar', $this->config->getValue('foo')); $this->assertEquals(null, $this->config->getValue('bar')); $this->assertEquals('moo', $this->config->getValue('bar', 'moo')); } - public function testSetValue() - { + public function testSetValue() { $this->config->setValue('foo', 'moo'); - $this->assertAttributeEquals(array('foo'=>'moo'), 'cache', $this->config); + $this->assertAttributeEquals(array('foo' => 'moo'), 'cache', $this->config); $content = file_get_contents(self::CONFIG_FILE); $this->assertEquals(<<config->setValue('bar', 'red'); - $this->assertAttributeEquals(array('foo'=>'moo', 'bar'=>'red'), 'cache', $this->config); + $this->assertAttributeEquals(array('foo' => 'moo', 'bar' => 'red'), 'cache', $this->config); $content = file_get_contents(self::CONFIG_FILE); $this->assertEquals(<<config->deleteKey('foo'); $this->assertAttributeEquals(array(), 'cache', $this->config); $content = file_get_contents(self::CONFIG_FILE); @@ -79,11 +74,10 @@ EOL ); EOL -, $content); + , $content); } - public function testSavingDebugMode() - { + public function testSavingDebugMode() { $this->config->setDebugMode(true); $this->config->deleteKey('foo'); // change something so we save to the config file $this->assertAttributeEquals(array(), 'cache', $this->config); @@ -96,14 +90,13 @@ define('DEBUG',true); ); EOL -, $content); + , $content); } /** * @expectedException \OC\HintException */ - public function testWriteData() - { + public function testWriteData() { $config = new OC\Config('/non-writable'); $config->setValue('foo', 'bar'); } From 492a35737c634fee27b0eb9d3ea6425bc6d98396 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 5 Jul 2013 15:26:39 +0200 Subject: [PATCH 15/17] fix \OC\Config test cases when debug mode is enabled --- tests/lib/config.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/lib/config.php b/tests/lib/config.php index 12473eb6676..87ee2807c2d 100644 --- a/tests/lib/config.php +++ b/tests/lib/config.php @@ -39,6 +39,7 @@ class Test_Config extends PHPUnit_Framework_TestCase { } public function testSetValue() { + $this->config->setDebugMode(false); $this->config->setValue('foo', 'moo'); $this->assertAttributeEquals(array('foo' => 'moo'), 'cache', $this->config); $content = file_get_contents(self::CONFIG_FILE); @@ -65,6 +66,7 @@ EOL } public function testDeleteKey() { + $this->config->setDebugMode(false); $this->config->deleteKey('foo'); $this->assertAttributeEquals(array(), 'cache', $this->config); $content = file_get_contents(self::CONFIG_FILE); From abe9abab997a01c3784a475d4cd16068647ed71d Mon Sep 17 00:00:00 2001 From: Bart Visscher Date: Mon, 8 Jul 2013 18:01:32 +0200 Subject: [PATCH 16/17] Add constructor documentation --- lib/config.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/config.php b/lib/config.php index c01cb4e152f..b03302ef09d 100644 --- a/lib/config.php +++ b/lib/config.php @@ -49,6 +49,9 @@ class Config { protected $debugMode; + /** + * @param $configDir path to the config dir, needs to end with '/' + */ public function __construct($configDir) { $this->configDir = $configDir; $this->configFilename = $this->configDir.'config.php'; From e7b882a4fcfbc5210d64372ba170efa825ebc237 Mon Sep 17 00:00:00 2001 From: Bart Visscher Date: Mon, 8 Jul 2013 18:29:43 +0200 Subject: [PATCH 17/17] stupid namespace --- lib/config.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/config.php b/lib/config.php index b03302ef09d..00d9f5b4247 100644 --- a/lib/config.php +++ b/lib/config.php @@ -159,7 +159,7 @@ class Config { */ private function writeData() { // Create a php file ... - $defaults = new OC_Defaults; + $defaults = new \OC_Defaults; $content = "debugMode) { $content .= "define('DEBUG',true);\n";