Fix #1341 issue where Config would try to cancel the save_timer when it is None.

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:
 <version dict>
 <content dict>
 
-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)
-