From d93fcf6eea640908108e8a997a5a0c4c094367b1 Mon Sep 17 00:00:00 2001 From: Andrew Resch Date: Wed, 18 Aug 2010 12:32:11 -0700 Subject: [PATCH] Fix #1341 issue where Config would try to cancel the save_timer when it is None. --- deluge/config.py | 40 ++++++++++++++++++------------------ tests/test_config.py | 48 +++++++++++++++++++++++++------------------- 2 files changed, 46 insertions(+), 42 deletions(-) diff --git a/deluge/config.py b/deluge/config.py index 5a6202d07..f7ad405e9 100644 --- a/deluge/config.py +++ b/deluge/config.py @@ -45,9 +45,9 @@ The format of the config file is two json encoded dicts: -The version dict contains two keys: file and format. The format version is -controlled by the Config class. It should only be changed when anything below -it is changed directly by the Config class. An example of this would be if we +The version dict contains two keys: file and format. The format version is +controlled by the Config class. It should only be changed when anything below +it is changed directly by the Config class. An example of this would be if we changed the serializer for the content to something different. The config file version is changed by the 'owner' of the config file. This is @@ -93,13 +93,13 @@ def prop(func): def find_json_objects(s): """ Find json objects in a string. - + :param s: the string to find json objects in :type s: string - + :returns: a list of tuples containing start and end locations of json objects in the string `s` :rtype: [(start, end), ...] - + """ objects = [] opens = 0 @@ -119,8 +119,8 @@ def find_json_objects(s): start = index + offset + 1 return objects - - + + class Config(object): """ This class is used to access/create/modify config files @@ -348,21 +348,21 @@ what is currently in the config and it could not convert the value return objects = find_json_objects(data) - + if not len(objects): # No json objects found, try depickling it try: self.__config.update(pickle.loads(data)) except Exception, e: log.exception(e) - log.warning("Unable to load config file: %s", filename) + log.warning("Unable to load config file: %s", filename) elif len(objects) == 1: start, end = objects[0] try: self.__config.update(json.loads(data[start:end])) except Exception, e: log.exception(e) - log.warning("Unable to load config file: %s", filename) + log.warning("Unable to load config file: %s", filename) elif len(objects) == 2: try: start, end = objects[0] @@ -371,8 +371,8 @@ what is currently in the config and it could not convert the value self.__config.update(json.loads(data[start:end])) except Exception, e: log.exception(e) - log.warning("Unable to load config file: %s", filename) - + log.warning("Unable to load config file: %s", filename) + log.debug("Config %s version: %s.%s loaded: %s", filename, self.__version["format"], self.__version["file"], self.__config) @@ -396,26 +396,24 @@ what is currently in the config and it could not convert the value version = json.loads(data[start:end]) start, end = objects[1] loaded_data = json.loads(data[start:end]) - if self.__config == loaded_data and self.__version == version: # The config has not changed so lets just return - self._save_timer.cancel() + if self._save_timer: + self._save_timer.cancel() return - except Exception, e: - log.warning("Unable to open config file: %s", filename) - - + except IOError, e: + log.warning("Unable to open config file: %s because: %s", filename, e) # Save the new config and make sure it's written to disk try: log.debug("Saving new config file %s", filename + ".new") f = open(filename + ".new", "wb") - json.dump(self.__version, f, indent=2) + json.dump(self.__version, f, indent=2) json.dump(self.__config, f, indent=2) f.flush() os.fsync(f.fileno()) f.close() - except Exception, e: + except IOError, e: log.error("Error writing new config file: %s", e) return False diff --git a/tests/test_config.py b/tests/test_config.py index f29ceae6f..d61920e20 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -6,7 +6,7 @@ import os from deluge.config import Config -DEFAULTS = {"string": "foobar", "int": 1, "float": 0.435, "bool": True, "tuple": (1, 2)} +DEFAULTS = {"string": "foobar", "int": 1, "float": 0.435, "bool": True} class ConfigTestCase(unittest.TestCase): def setUp(self): @@ -15,10 +15,10 @@ class ConfigTestCase(unittest.TestCase): def test_init(self): config = Config("test.conf", defaults=DEFAULTS, config_dir=self.config_dir) self.assertEquals(DEFAULTS, config.config) - + config = Config("test.conf", config_dir=self.config_dir) self.assertEquals({}, config.config) - + def test_set_get_item(self): config = Config("test.conf", config_dir=self.config_dir) config["foo"] = 1 @@ -26,28 +26,28 @@ class ConfigTestCase(unittest.TestCase): self.assertRaises(ValueError, config.set_item, "foo", "bar") config["foo"] = 2 self.assertEquals(config.get_item("foo"), 2) - + config._save_timer.cancel() def test_load(self): def check_config(): config = Config("test.conf", config_dir=self.config_dir) - + self.assertEquals(config["string"], "foobar") self.assertEquals(config["float"], 0.435) - + # Test loading an old config from 1.1.x import pickle pickle.dump(DEFAULTS, open(os.path.join(self.config_dir, "test.conf"), "wb")) - + check_config() - + # Test opening a previous 1.2 config file of just a json object import json json.dump(DEFAULTS, open(os.path.join(self.config_dir, "test.conf"), "wb"), indent=2) check_config() - + # Test opening a previous 1.2 config file of having the format versions # as ints f = open(os.path.join(self.config_dir, "test.conf"), "wb") @@ -55,26 +55,33 @@ class ConfigTestCase(unittest.TestCase): f.write(str(1) + "\n") json.dump(DEFAULTS, f, indent=2) f.close() - + check_config() - + # Test the 1.2 config format v = {"format": 1, "file": 1} f = open(os.path.join(self.config_dir, "test.conf"), "wb") json.dump(v, f, indent=2) json.dump(DEFAULTS, f, indent=2) f.close() - + check_config() def test_save(self): config = Config("test.conf", defaults=DEFAULTS, config_dir=self.config_dir) + # We do this twice because the first time we need to save the file to disk + # and the second time we do a compare and we should not write + ret = config.save() + self.assertTrue(ret) + ret = config.save() + self.assertTrue(ret) + config["string"] = "baz" config["int"] = 2 ret = config.save() self.assertTrue(ret) del config - + config = Config("test.conf", defaults=DEFAULTS, config_dir=self.config_dir) self.assertEquals(config["string"], "baz") self.assertEquals(config["int"], 2) @@ -84,14 +91,14 @@ class ConfigTestCase(unittest.TestCase): config["string"] = "baz" config["int"] = 2 self.assertTrue(config._save_timer.active()) - + def check_config(config): self.assertTrue(not config._save_timer.active()) del config config = Config("test.conf", defaults=DEFAULTS, config_dir=self.config_dir) self.assertEquals(config["string"], "baz") self.assertEquals(config["int"], 2) - + from twisted.internet.task import deferLater from twisted.internet import reactor d = deferLater(reactor, 7, check_config, config) @@ -99,16 +106,15 @@ class ConfigTestCase(unittest.TestCase): def test_find_json_objects(self): s = """{ - "file": 1, + "file": 1, "format": 1 }{ - "ssl": true, - "enabled": false, + "ssl": true, + "enabled": false, "port": 8115 }\n""" - + from deluge.config import find_json_objects - + objects = find_json_objects(s) self.assertEquals(len(objects), 2) -