Change the config format

Add test_config

deluge/config.py

@@ -40,15 +40,15 @@ Deluge Config Module
 This module is used for loading and saving of configuration files.. or anything
 really.
 
-The format of the config file is as follows:
+The format of the config file is two json encoded dicts:
 
-<format version as int>
+<version dict>
-<config file version as int>
+<content dict>
-<content>
 
-The format version is controlled by the Config class.  It should only be changed
+The version dict contains two keys: file and format.  The format version is 
-when anything below it is changed directly by the Config class.  An example of
+controlled by the Config class.  It should only be changed when anything below 
-this would be if we changed the serializer for the content to something different.
+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
 to signify that there is a change in the naming of some config keys or something
@@ -90,6 +90,37 @@ def prop(func):
     """
     return property(doc=func.__doc__, **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
+    start = s.find("{")
+    offset = start
+
+    if start < 0:
+        return []
+
+    for index, c in enumerate(s[offset:]):
+        if c == "{":
+            opens += 1
+        elif c == "}":
+            opens -= 1
+        if opens == 0:
+            objects.append((start, index+offset+1))
+            start = index + offset + 1
+
+    return objects
+    
+    
 class Config(object):
     """
     This class is used to access/create/modify config files
@@ -105,12 +136,14 @@ class Config(object):
         self.__change_callbacks = []
 
         # These hold the version numbers and they will be set when loaded
-        self.__format_version = 1
+        self.__version = {
-        self.__file_version = 1
+            "format": 1,
+            "file": 1
+        }
 
         # This will get set with a reactor.callLater whenever a config option
         # is set.
-        self.__save_timer = None
+        self._save_timer = None
 
         if defaults:
             self.__config = defaults
@@ -134,12 +167,14 @@ class Config(object):
     def set_item(self, key, value):
         """
         Sets item 'key' to 'value' in the config dictionary, but does not allow
-        changing the item's type unless it is None
+        changing the item's type unless it is None.  If the types do not match,
+        it will attempt to convert it to the set type before raising a ValueError.
 
         :param key: string, item to change to change
         :param value: the value to change item to, must be same type as what is currently in the config
 
-        :raises ValueError: raised when the type of value is not the same as what is currently in the config
+        :raises ValueError: raised when the type of value is not the same as\
+what is currently in the config and it could not convert the value
 
         **Usage**
 
@@ -186,8 +221,8 @@ class Config(object):
             pass
 
         # We set the save_timer for 5 seconds if not already set
-        if not self.__save_timer or not self.__save_timer.active():
+        if not self._save_timer or not self._save_timer.active():
-            self.__save_timer = reactor.callLater(5, self.save)
+            self._save_timer = reactor.callLater(5, self.save)
 
     def __getitem__(self, key):
         """
@@ -304,38 +339,39 @@ class Config(object):
             filename = self.__config_file
 
         try:
-            data = open(filename, "rb")
+            data = open(filename, "rb").read()
         except IOError, e:
             log.warning("Unable to open config file %s: %s", filename, e)
             return
 
-        try:
+        objects = find_json_objects(data)
-            self.__format_version = int(data.readline())
-        except ValueError:
-            data.seek(0)
-        else:
-            try:
-                self.__file_version = int(data.readline())
-            except ValueError:
-                pass
-
-        fdata = data.read()
-        data.close()
         
+        if not len(objects):
+            # No json objects found, try depickling it
             try:
-            self.__config.update(json.loads(fdata))
+                self.__config.update(pickle.loads(data))
             except Exception, e:
                 log.exception(e)
+                log.warning("Unable to load config file: %s", filename)                
+        elif len(objects) == 1:
+            start, end = objects[0]
             try:
-                self.__config.update(pickle.loads(fdata))
+                self.__config.update(json.loads(data[start:end]))
+            except Exception, e:
+                log.exception(e)
+                log.warning("Unable to load config file: %s", filename)                
+        elif len(objects) == 2:
+            try:
+                start, end = objects[0]
+                self.__version.update(json.loads(data[start:end]))
+                start, end = objects[1]
+                self.__config.update(json.loads(data[start:end]))
             except Exception, e:
                 log.exception(e)
                 log.warning("Unable to load config file: %s", filename)                
             
-
-
         log.debug("Config %s version: %s.%s loaded: %s", filename,
-            self.__format_version, self.__file_version, self.__config)
+            self.__version["format"], self.__version["file"], self.__config)
 
     def save(self, filename=None):
         """
@@ -351,26 +387,27 @@ class Config(object):
         # Check to see if the current config differs from the one on disk
         # We will only write a new config file if there is a difference
         try:
-            data = open(filename, "rb")
+            data = open(filename, "rb").read()
-            data.readline()
+            objects = find_json_objects(data)
-            data.readline()
+            start, end = objects[0]
-            loaded_data = json.loads(data.read())
+            version = json.loads(data[start:end])
-            data.close()
+            start, end = objects[1]
-            if self.__config == loaded_data:
+            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 = None
+                self._save_timer = None
                 return
         except Exception, e:
             log.warning("Unable to open config file: %s", filename)
 
-        self.__save_timer = None
+        self._save_timer = None
 
         # 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")
-            f.write(str(self.__format_version) + "\n")
+            json.dump(self.__version, f, indent=2)            
-            f.write(str(self.__file_version) + "\n")
             json.dump(self.__config, f, indent=2)
             f.flush()
             os.fsync(f.fileno())
@@ -414,9 +451,9 @@ class Config(object):
         if output_version in input_range or output_version <= max(input_range):
             raise ValueError("output_version needs to be greater than input_range")
 
-        if self.__file_version not in input_range:
+        if self.__version["file"] not in input_range:
             log.debug("File version %s is not in input_range %s, ignoring converter function..",
-                self.__file_version, input_range)
+                self.__version["file"], input_range)
             return
 
         try:
@@ -424,10 +461,10 @@ class Config(object):
         except Exception, e:
             log.exception(e)
             log.error("There was an exception try to convert config file %s %s to %s",
-                self.__config_file, self.__file_version, output_version)
+                self.__config_file, self.__version["file"], output_version)
             raise e
         else:
-            self.__file_version = output_version
+            self.__version["file"] = output_version
             self.save()
 
     @property

tests/common.py

@@ -8,6 +8,7 @@ deluge.log.setupLogger("none")
 def set_tmp_config_dir():
     config_directory = tempfile.mkdtemp()
     deluge.configmanager.set_config_dir(config_directory)
+    return config_directory
 
 import gettext
 import locale

tests/test_config.py

@@ -0,0 +1,66 @@
+from twisted.trial import unittest
+from twisted.python.failure import Failure
+
+import common
+
+from deluge.config import Config
+
+class ConfigTestCase(unittest.TestCase):
+    def test_init(self):
+        defaults = {"string": "foobar", "int": 1, "float": 0.435, "bool": True, "tuple": (1, 2)}
+        
+        config = Config("test.conf", defaults=defaults, config_dir=".")
+        self.assertEquals(defaults, config.config)
+        
+        config = Config("test.conf", config_dir=".")
+        self.assertEquals({}, config.config)
+    
+    def test_set_get_item(self):
+        config = Config("test.conf", config_dir=".")
+        config["foo"] = 1
+        self.assertEquals(config["foo"], 1)
+        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):
+        d = {"string": "foobar", "int": 1, "float": 0.435, "bool": True, "tuple": (1, 2)}
+
+        def check_config():
+            config = Config("test.conf", 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(d, open("test.conf", "wb"))
+        
+        check_config()
+                
+        # Test opening a previous 1.2 config file of just a json object
+        import json
+        json.dump(d, open("test.conf", "wb"), indent=2)
+
+        check_config()
+        
+        # Test opening a previous 1.2 config file of having the format versions
+        # as ints
+        f = open("test.conf", "wb")
+        f.write(str(1) + "\n")
+        f.write(str(1) + "\n")
+        json.dump(d, f, indent=2)
+        f.close()
+        
+        check_config()
+        
+        # Test the 1.2 config format
+        v = {"format": 1, "file": 1}
+        f = open("test.conf", "wb")
+        json.dump(v, f, indent=2)
+        json.dump(d, f, indent=2)
+        f.close()
+        
+        check_config()