From fff3ed9b77d7a5c66bb8797e1f0ba245d83116f7 Mon Sep 17 00:00:00 2001 From: Chris Glass Date: Wed, 25 Jun 2014 17:34:51 +0200 Subject: [PATCH] Added a lot of tests for SimpleConfig Refactored the SImpleConfig class a lot to make sure the behavior is always defined. --- lib/simple_config.py | 231 ++++++++++++++++---------------- lib/tests/test_simple_config.py | 107 +++++++++++++++ 2 files changed, 219 insertions(+), 119 deletions(-) create mode 100644 lib/tests/test_simple_config.py diff --git a/lib/simple_config.py b/lib/simple_config.py index 30c7f9d12..c90a7379b 100644 --- a/lib/simple_config.py +++ b/lib/simple_config.py @@ -1,62 +1,95 @@ -import json import ast import threading import os from util import user_dir, print_error, print_msg +SYSTEM_CONFIG_PATH = "/etc/electrum.conf" config = None + + def get_config(): global config return config + def set_config(c): global config config = c -class SimpleConfig: +class SimpleConfig(object): """ -The SimpleConfig class is responsible for handling operations involving -configuration files. The constructor reads and stores the system and -user configurations from electrum.conf into separate dictionaries within -a SimpleConfig instance then reads the wallet file. -""" - def __init__(self, options={}): - self.lock = threading.Lock() - - # system conf, readonly - self.system_config = {} + The SimpleConfig class is responsible for handling operations involving + configuration files. + + There are 3 different sources of possible configuration values: + 1. Command line options. + 2. User configuration (in the user's config directory) + 3. System configuration (in /etc/) + They are taken in order (1. overrides config options set in 2., that + override config set in 3.) + """ + def __init__(self, options=None, read_system_config_function=None, + read_user_config_function=None, read_user_dir_function=None): + + # This is the holder of actual options for the current user. + self.current_options = {} + # This lock needs to be acquired for updating and reading the config in + # a thread-safe way. + self.lock = threading.RLock() + # The path for the config directory. This is set later by init_path() + self.path = None + + if options is None: + options = {} # Having a mutable as a default value is a bad idea. + + # The following two functions are there for dependency injection when + # testing. + if read_system_config_function is None: + read_system_config_function = read_system_config + if read_user_config_function is None: + read_user_config_function = read_user_config + if read_user_dir_function is None: + self.user_dir = user_dir + else: + self.user_dir = read_user_dir_function + + # Save the command-line keys to make sure we don't override them. + self.command_line_keys = options.keys() + # Save the system config keys to make sure we don't override them. + self.system_config_keys = [] + if options.get('portable') is not True: - self.read_system_config() + # system conf + system_config = read_system_config_function() + self.system_config_keys = system_config.keys() + self.current_options.update(system_config) - # command-line options - self.options_config = options + # update the current options with the command line options last (to + # override both others). + self.current_options.update(options) # init path self.init_path() - # user conf, writeable - self.user_config = {} - self.read_user_config() - - set_config(self) - + # user config. + self.user_config = read_user_config_function(self.path) + # The user config is overwritten by the current config! + self.user_config.update(self.current_options) + self.current_options = self.user_config + set_config(self) # Make a singleton instance of 'self' def init_path(self): # Read electrum path in the command line configuration - self.path = self.options_config.get('electrum_path') - - # Read electrum path in the system configuration - if self.path is None: - self.path = self.system_config.get('electrum_path') + self.path = self.current_options.get('electrum_path') # If not set, use the user's default data directory. if self.path is None: - self.path = user_dir() + self.path = self.user_dir() # Make directory if it does not yet exist. if not os.path.exists(self.path): @@ -64,110 +97,32 @@ a SimpleConfig instance then reads the wallet file. print_error( "electrum directory", self.path) - # portable wallet: use the same directory for wallet and headers file - #if options.get('portable'): - # self.wallet_config['blockchain_headers_path'] = os.path.dirname(self.path) - def set_key(self, key, value, save = True): - # find where a setting comes from and save it there - if self.options_config.get(key) is not None: - print "Warning: not changing '%s' because it was passed as a command-line option"%key + if not self.is_modifiable(key): + print "Warning: not changing key '%s' because it is not modifiable" \ + " (passed as command line option or defined in /etc/electrum.conf)"%key return - elif self.system_config.get(key) is not None: - if str(self.system_config[key]) != str(value): - print "Warning: not changing '%s' because it was set in the system configuration"%key - - else: - - with self.lock: - self.user_config[key] = value - if save: - self.save_user_config() - + with self.lock: + self.user_config[key] = value + self.current_options[key] = value + if save: + self.save_user_config() + return def get(self, key, default=None): - out = None - - # 1. command-line options always override everything - if self.options_config.has_key(key) and self.options_config.get(key) is not None: - out = self.options_config.get(key) - - # 2. user configuration - elif self.user_config.has_key(key): - out = self.user_config.get(key) - - # 2. system configuration - elif self.system_config.has_key(key): - out = self.system_config.get(key) - - if out is None and default is not None: - out = default - - # try to fix the type - if default is not None and type(out) != type(default): - import ast - try: - out = ast.literal_eval(out) - except Exception: - print "type error for '%s': using default value"%key - out = default - + with self.lock: + out = self.current_options.get(key, default) return out - def is_modifiable(self, key): - """Check if the config file is modifiable.""" - if self.options_config.has_key(key): + if key in self.command_line_keys: return False - elif self.user_config.has_key(key): - return True - elif self.system_config.has_key(key): + if key in self.system_config_keys: return False - else: - return True - - - def read_system_config(self): - """Parse and store the system config settings in electrum.conf into system_config[].""" - name = '/etc/electrum.conf' - if os.path.exists(name): - try: - import ConfigParser - except ImportError: - print "cannot parse electrum.conf. please install ConfigParser" - return - - p = ConfigParser.ConfigParser() - p.read(name) - try: - for k, v in p.items('client'): - self.system_config[k] = v - except ConfigParser.NoSectionError: - pass - - - def read_user_config(self): - """Parse and store the user config settings in electrum.conf into user_config[].""" - if not self.path: return - - path = os.path.join(self.path, "config") - if os.path.exists(path): - try: - with open(path, "r") as f: - data = f.read() - except IOError: - return - try: - d = ast.literal_eval( data ) #parse raw data from reading wallet file - except Exception: - print_msg("Error: Cannot read config file.") - return - - self.user_config = d - + return True def save_user_config(self): if not self.path: return @@ -180,3 +135,41 @@ a SimpleConfig instance then reads the wallet file. if self.get('gui') != 'android': import stat os.chmod(path, stat.S_IREAD | stat.S_IWRITE) + +def read_system_config(): + """Parse and return the system config settings in /etc/electrum.conf.""" + if os.path.exists(SYSTEM_CONFIG_PATH): + try: + import ConfigParser + except ImportError: + print "cannot parse electrum.conf. please install ConfigParser" + return + + p = ConfigParser.ConfigParser() + p.read(SYSTEM_CONFIG_PATH) + result = {} + try: + for k, v in p.items('client'): + result[k] = v + except ConfigParser.NoSectionError: + pass + return result + +def read_user_config(path): + """Parse and store the user config settings in electrum.conf into user_config[].""" + if not path: return + + config_path = os.path.join(path, "config") + if os.path.exists(config_path): + try: + with open(config_path, "r") as f: + data = f.read() + except IOError: + return + try: + d = ast.literal_eval( data ) #parse raw data from reading wallet file + except Exception: + print_msg("Error: Cannot read config file.") + return + + return d diff --git a/lib/tests/test_simple_config.py b/lib/tests/test_simple_config.py new file mode 100644 index 000000000..8704838d5 --- /dev/null +++ b/lib/tests/test_simple_config.py @@ -0,0 +1,107 @@ +import sys +import unittest +import tempfile +import shutil + +from StringIO import StringIO +from lib.simple_config import SimpleConfig + + +class Test_SimpleConfig(unittest.TestCase): + + def setUp(self): + super(Test_SimpleConfig, self).setUp() + # make sure "read_user_config" and "user_dir" return a temporary directory. + self.electrum_dir = tempfile.mkdtemp() + # Do the same for the user dir to avoid overwriting the real configuration + # for development machines with electrum installed :) + self.user_dir = tempfile.mkdtemp() + + self.options = {"electrum_path": self.electrum_dir} + self._saved_stdout = sys.stdout + self._stdout_buffer = StringIO() + sys.stdout = self._stdout_buffer + + def tearDown(self): + super(Test_SimpleConfig, self).tearDown() + # Remove the temporary directory after each test (to make sure we don't + # pollute /tmp for nothing. + shutil.rmtree(self.electrum_dir) + shutil.rmtree(self.user_dir) + + # Restore the "real" stdout + sys.stdout = self._saved_stdout + + def test_simple_config_command_line_overrides_everything(self): + """Options passed by command line override all other configuration + sources""" + fake_read_system = lambda : {"electrum_path": "a"} + fake_read_user = lambda _: {"electrum_path": "b"} + read_user_dir = lambda : self.user_dir + config = SimpleConfig(options=self.options, + read_system_config_function=fake_read_system, + read_user_config_function=fake_read_user, + read_user_dir_function=read_user_dir) + self.assertEqual(self.options.get("electrum_path"), + config.get("electrum_path")) + + def test_simple_config_system_config_overrides_user_config(self): + """Options passed in system config override user config.""" + fake_read_system = lambda : {"electrum_path": self.electrum_dir} + fake_read_user = lambda _: {"electrum_path": "b"} + read_user_dir = lambda : self.user_dir + config = SimpleConfig(options=None, + read_system_config_function=fake_read_system, + read_user_config_function=fake_read_user, + read_user_dir_function=read_user_dir) + self.assertEqual(self.options.get("electrum_path"), + config.get("electrum_path")) + + def test_simple_config_user_config_is_used_if_others_arent_specified(self): + """Options passed by command line override all other configuration + sources""" + fake_read_system = lambda : {} + fake_read_user = lambda _: {"electrum_path": self.electrum_dir} + read_user_dir = lambda : self.user_dir + config = SimpleConfig(options=None, + read_system_config_function=fake_read_system, + read_user_config_function=fake_read_user, + read_user_dir_function=read_user_dir) + self.assertEqual(self.options.get("electrum_path"), + config.get("electrum_path")) + + def test_cannot_set_options_passed_by_command_line(self): + fake_read_system = lambda : {} + fake_read_user = lambda _: {"electrum_path": "b"} + read_user_dir = lambda : self.user_dir + config = SimpleConfig(options=self.options, + read_system_config_function=fake_read_system, + read_user_config_function=fake_read_user, + read_user_dir_function=read_user_dir) + config.set_key("electrum_path", "c") + self.assertEqual(self.options.get("electrum_path"), + config.get("electrum_path")) + + def test_cannot_set_options_from_system_config(self): + fake_read_system = lambda : {"electrum_path": self.electrum_dir} + fake_read_user = lambda _: {} + read_user_dir = lambda : self.user_dir + config = SimpleConfig(options={}, + read_system_config_function=fake_read_system, + read_user_config_function=fake_read_user, + read_user_dir_function=read_user_dir) + config.set_key("electrum_path", "c") + self.assertEqual(self.options.get("electrum_path"), + config.get("electrum_path")) + + def test_can_set_options_set_in_user_config(self): + another_path = tempfile.mkdtemp() + fake_read_system = lambda : {} + fake_read_user = lambda _: {"electrum_path": self.electrum_dir} + read_user_dir = lambda : self.user_dir + config = SimpleConfig(options={}, + read_system_config_function=fake_read_system, + read_user_config_function=fake_read_user, + read_user_dir_function=read_user_dir) + config.set_key("electrum_path", another_path) + self.assertEqual(another_path, config.get("electrum_path"))