Merge with W. Trevor King's tree.

6 files changed, 248 insertions, 33 deletions

diff --git a/libbe/bug.py b/libbe/bug.py
index 43974dd..7418933 100644
--- a/libbe/bug.py
+++ b/libbe/bug.py
@@ -18,6 +18,7 @@ import os
 import os.path
 import errno
 import time
+import types
 import xml.sax.saxutils
 import doctest
 
@@ -184,6 +185,27 @@ class Bug(settings_object.SavedSettingsObject):
                     fset=_set_time,
                     doc="An integer version of .time_string")
 
+    def _extra_strings_check_fn(value):
+        "Require an iterable full of strings"
+        if value == settings_object.EMPTY:
+            return True
+        elif not hasattr(value, "__iter__"):
+            return False
+        for x in value:
+            if type(x) not in types.StringTypes:
+                return False
+        return True
+    def _extra_strings_change_hook(self, old, new):
+        self.extra_strings.sort() # to make merging easier
+        self._prop_save_settings(old, new)
+    @_versioned_property(name="extra_strings",
+                         doc="Space for an array of extra strings.  Useful for storing state for functionality implemented purely in becommands/<some_function>.py.",
+                         default=[],
+                         check_fn=_extra_strings_check_fn,
+                         change_hook=_extra_strings_change_hook,
+                         mutable=True)
+    def extra_strings(): return {}
+
     @_versioned_property(name="summary",
                          doc="A one-line bug description")
     def summary(): return {}
diff --git a/libbe/bugdir.py b/libbe/bugdir.py
index a9ec42e..3c2c247 100644
--- a/libbe/bugdir.py
+++ b/libbe/bugdir.py
@@ -54,9 +54,9 @@ class AlreadyInitialized(Exception):
 class MultipleBugMatches(ValueError):
     def __init__(self, shortname, matches):
         msg = ("More than one bug matches %s.  "
-               "Please be more specific.\n%s" % shortname, matches)
+               "Please be more specific.\n%s" % (shortname, matches))
         ValueError.__init__(self, msg)
-        self.shortname = shortnamename
+        self.shortname = shortname
         self.matches = matches
 
 
diff --git a/libbe/comment.py b/libbe/comment.py
index 80b97a1..df5a63f 100644
--- a/libbe/comment.py
+++ b/libbe/comment.py
@@ -151,10 +151,10 @@ class Comment(Tree, settings_object.SavedSettingsObject):
         if self.rcs != None and self.sync_with_disk == True:
             import rcs
             return self.rcs.get_file_contents(self.get_path("body"))
-    def _set_comment_body(self, value, force=False):
+    def _set_comment_body(self, old=None, new=None, force=False):
         if (self.rcs != None and self.sync_with_disk == True) or force==True:
-            assert value != None, "Can't save empty comment"
-            self.rcs.set_file_contents(self.get_path("body"), value)
+            assert new != None, "Can't save empty comment"
+            self.rcs.set_file_contents(self.get_path("body"), new)
 
     @Property
     @change_hook_property(hook=_set_comment_body)
@@ -323,7 +323,7 @@ class Comment(Tree, settings_object.SavedSettingsObject):
         #    raise Exception, str(self)+'\n'+str(self.settings)+'\n'+str(self._settings_loaded)
         #assert self.in_reply_to != None, "Comment must be a reply to something"
         self.save_settings()
-        self._set_comment_body(self.body, force=True)
+        self._set_comment_body(new=self.body, force=True)
 
     def remove(self):
         for comment in self.traverse():
diff --git a/libbe/mapfile.py b/libbe/mapfile.py
index c36d454..0272890 100644
--- a/libbe/mapfile.py
+++ b/libbe/mapfile.py
@@ -113,7 +113,7 @@ def parse(contents):
             else:
                 newlines.append(line)
         contents = '\n'.join(newlines)
-    return yaml.load(contents)
+    return yaml.load(contents) or {}
 
 def map_save(rcs, path, map, allow_no_rcs=False):
     """Save the map as a mapfile to the specified path"""
diff --git a/libbe/properties.py b/libbe/properties.py
index a8e89fb..956ecc3 100644
--- a/libbe/properties.py
+++ b/libbe/properties.py
@@ -26,12 +26,17 @@ and
 for more information on decorators.
 """
 
+import copy
+import types
 import unittest
 
 
 class ValueCheckError (ValueError):
     def __init__(self, name, value, allowed):
-        msg = "%s not in %s for %s" % (value, allowed, name)
+        action = "in" # some list of allowed values
+        if type(allowed) == types.FunctionType:
+            action = "allowed by" # some allowed-value check function
+        msg = "%s not %s %s for %s" % (value, action, allowed, name)
         ValueError.__init__(self, msg)
         self.name = name
         self.value = value
@@ -66,7 +71,7 @@ def doc_property(doc=None):
         return funcs
     return decorator
 
-def local_property(name, null=None):
+def local_property(name, null=None, mutable_null=False):
     """
     Define get/set access to per-parent-instance local storage.  Uses
     ._<name>_value to store the value for a particular owner instance.
@@ -80,7 +85,11 @@ def local_property(name, null=None):
         def _fget(self):
             if fget is not None:
                 fget(self)
-            value = getattr(self, "_%s_value" % name, null)
+            if mutable_null == True:
+                ret_null = copy.deepcopy(null)
+            else:
+                ret_null = null
+            value = getattr(self, "_%s_value" % name, ret_null)
             return value
         def _fset(self, value):
             setattr(self, "_%s_value" % name, value)
@@ -118,7 +127,46 @@ def settings_property(name, null=None):
         return funcs
     return decorator
 
-def defaulting_property(default=None, null=None):
+
+# Allow comparison and caching with _original_ values for mutables,
+# since
+#
+# >>> a = []
+# >>> b = a
+# >>> b.append(1)
+# >>> a
+# [1]
+# >>> a==b
+# True
+def _hash_mutable_value(value):
+    return repr(value)
+def _init_mutable_property_cache(self):
+    if not hasattr(self, "_mutable_property_cache_hash"):
+        # first call to _fget for any mutable property
+        self._mutable_property_cache_hash = {}
+        self._mutable_property_cache_copy = {}
+def _set_cached_mutable_property(self, cacher_name, property_name, value):
+    _init_mutable_property_cache(self)
+    self._mutable_property_cache_hash[(cacher_name, property_name)] = \
+        _hash_mutable_value(value)
+    self._mutable_property_cache_copy[(cacher_name, property_name)] = \
+        copy.deepcopy(value)
+def _get_cached_mutable_property(self, cacher_name, property_name, default=None):
+    _init_mutable_property_cache(self)
+    if (cacher_name, property_name) not in self._mutable_property_cache_copy:
+        return default
+    return self._mutable_property_cache_copy[(cacher_name, property_name)]
+def _cmp_cached_mutable_property(self, cacher_name, property_name, value):
+    _init_mutable_property_cache(self)
+    if (cacher_name, property_name) not in self._mutable_property_cache_hash:
+        return 1 # any value > non-existant old hash
+    old_hash = self._mutable_property_cache_hash[(cacher_name, property_name)]
+    return cmp(_hash_mutable_value(value), old_hash)
+
+
+def defaulting_property(default=None, null=None,
+                        default_mutable=False,
+                        null_mutable=False):
     """
     Define a default value for get access to a property.
     If the stored value is null, then default is returned.
@@ -127,12 +175,25 @@ def defaulting_property(default=None, null=None):
         if hasattr(funcs, "__call__"):
             funcs = funcs()
         fget = funcs.get("fget")
+        fset = funcs.get("fset")
+        name = funcs.get("name", "<unknown>")
         def _fget(self):
             value = fget(self)
             if value == null:
-                return default
+                if default_mutable == True:
+                    return copy.deepcopy(default)
+                else:
+                    return default
             return value
+        def _fset(self, value):
+            if value == default:
+                if null_mutable == True:
+                    value = copy.deepcopy(null)
+                else:
+                    value = null
+            fset(self, value)
         funcs["fget"] = _fget
+        funcs["fset"] = _fset
         return funcs
     return decorator
 
@@ -184,7 +245,7 @@ def checked_property(allowed=[]):
         return funcs
     return decorator
 
-def cached_property(generator, initVal=None):
+def cached_property(generator, initVal=None, mutable=False):
     """
     Allow caching of values generated by generator(instance), where
     instance is the instance to which this property belongs.  Uses
@@ -193,7 +254,7 @@ def cached_property(generator, initVal=None):
 
     When the cache flag is True or missing and the stored value is
     initVal, the first fget call triggers the generator function,
-    whiose output is stored in _<name>_cached_value.  That and
+    whose output is stored in _<name>_cached_value.  That and
     subsequent calls to fget will return this cached value.
 
     If the input value is no longer initVal (e.g. a value has been
@@ -205,25 +266,27 @@ def cached_property(generator, initVal=None):
 
     The cache flag is missing on initialization.  Particular instances
     may override by setting their own flag.
+
+    In the case that mutable == True, all caching is disabled and the
+    generator is called whenever the cached value would otherwise be
+    used.  This avoids uncertainties in the value of stored mutables.
     """
     def decorator(funcs):
         if hasattr(funcs, "__call__"):
             funcs = funcs()
         fget = funcs.get("fget")
-        fset = funcs.get("fset")
         name = funcs.get("name", "<unknown>")
         def _fget(self):
             cache = getattr(self, "_%s_cache" % name, True)
             value = fget(self)
-            if cache == True:
-                if value == initVal:
+            if value == initVal:
+                if cache == True and mutable == False:
                     if hasattr(self, "_%s_cached_value" % name):
                         value = getattr(self, "_%s_cached_value" % name)
                     else:
                         value = generator(self)
                         setattr(self, "_%s_cached_value" % name, value)
-            else:
-                if value == initVal:
+                else:
                     value = generator(self)
             return value
         funcs["fget"] = _fget
@@ -258,12 +321,15 @@ def primed_property(primer, initVal=None):
         return funcs
     return decorator
 
-def change_hook_property(hook):
+def change_hook_property(hook, mutable=False):
     """
     Call the function hook(instance, old_value, new_value) whenever a
     value different from the current value is set (instance is a a
     reference to the class instance to which this property belongs).
-    This is useful for saving changes to disk, etc.
+    This is useful for saving changes to disk, etc.  This function is
+    called _after_ the new value has been stored, allowing you to
+    change the stored value if you want.
+
     """
     def decorator(funcs):
         if hasattr(funcs, "__call__"):
@@ -271,15 +337,31 @@ def change_hook_property(hook):
         fget = funcs.get("fget")
         fset = funcs.get("fset")
         name = funcs.get("name", "<unknown>")
+        def _fget(self, new_value=None, from_fset=False): # only used if mutable == True
+            value = fget(self)
+            if _cmp_cached_mutable_property(self, "change hook property", name, value) != 0:
+                # there has been a change, cache new value
+                old_value = _get_cached_mutable_property(self, "change hook property", name)
+                _set_cached_mutable_property(self, "change hook property", name, value)
+                if from_fset == True: # return previously cached value
+                    value = old_value
+                else: # the value changed while we weren't looking
+                    hook(self, old_value, value)
+            return value
         def _fset(self, value):
-            old_value = fget(self)
+            if mutable == True: # get cached previous value
+                old_value = _fget(self, new_value=value, from_fset=True)
+            else:
+                old_value = fget(self)
+            fset(self, value)
             if value != old_value:
                 hook(self, old_value, value)
-            fset(self, value)
+        if mutable == True:
+            funcs["fget"] = _fget
         funcs["fset"] = _fset
         return funcs
     return decorator
-
+        
 
 class DecoratorTests(unittest.TestCase):
     def testLocalDoc(self):
@@ -320,16 +402,18 @@ class DecoratorTests(unittest.TestCase):
         class Test(object):
             @Property
             @defaulting_property(default='y', null='x')
-            @local_property(name="DEFAULT")
+            @local_property(name="DEFAULT", null=5)
             def x(): return {}
         t = Test()
-        self.failUnless(t.x == None, str(t.x)) 
+        self.failUnless(t.x == 5, str(t.x)) 
         t.x = 'x'
         self.failUnless(t.x == 'y', str(t.x))
         t.x = 'y'
         self.failUnless(t.x == 'y', str(t.x))
         t.x = 'z'
         self.failUnless(t.x == 'z', str(t.x))
+        t.x = 5
+        self.failUnless(t.x == 5, str(t.x))
     def testCheckedLocalProperty(self):
         class Test(object):
             @Property
@@ -474,6 +558,57 @@ class DecoratorTests(unittest.TestCase):
         t.x = 2
         self.failUnless(t.old == 1, t.old)
         self.failUnless(t.new == 2, t.new)
+    def testChangeHookMutableProperty(self):
+        class Test(object):
+            def _hook(self, old, new):
+                self.old = old
+                self.new = new
+                self.hook_calls += 1
+
+            @Property
+            @change_hook_property(_hook, mutable=True)
+            @local_property(name="HOOKED")
+            def x(): return {}
+        t = Test()
+        t.hook_calls = 0
+        t.x = []
+        self.failUnless(t.old == None, t.old)
+        self.failUnless(t.new == [], t.new)
+        a = t.x
+        a.append(5)
+        t.x = a
+        self.failUnless(t.old == [], t.old)
+        self.failUnless(t.new == [5], t.new)
+        t.x = []
+        self.failUnless(t.old == [5], t.old)
+        self.failUnless(t.new == [], t.new)
+        # now append without reassigning.  this doesn't trigger the
+        # change, since we don't ever set t.x, only get it and mess
+        # with it.  It does, however, update our t.new, since t.new =
+        # t.x and is not a static copy.
+        t.x.append(5)
+        self.failUnless(t.old == [5], t.old)
+        self.failUnless(t.new == [5], t.new)
+        # however, the next t.x get _will_ notice the change...
+        a = t.x
+        self.failUnless(t.old == [], t.old)
+        self.failUnless(t.new == [5], t.new)
+        self.failUnless(t.hook_calls == 6, t.hook_calls)
+        t.x.append(6) # this append(6) is not noticed yet
+        self.failUnless(t.old == [], t.old)
+        self.failUnless(t.new == [5,6], t.new)
+        self.failUnless(t.hook_calls == 6, t.hook_calls)
+        # this append(7) is not noticed, but the t.x get causes the
+        # append(6) to be noticed
+        t.x.append(7)
+        self.failUnless(t.old == [5], t.old)
+        self.failUnless(t.new == [5,6,7], t.new)
+        self.failUnless(t.hook_calls == 7, t.hook_calls)
+        a = t.x # now the append(7) is noticed
+        self.failUnless(t.old == [5,6], t.old)
+        self.failUnless(t.new == [5,6,7], t.new)
+        self.failUnless(t.hook_calls == 8, t.hook_calls)
+
 
 suite = unittest.TestLoader().loadTestsFromTestCase(DecoratorTests)
 
diff --git a/libbe/settings_object.py b/libbe/settings_object.py
index 1df3e6b..60fddb9 100644
--- a/libbe/settings_object.py
+++ b/libbe/settings_object.py
@@ -87,6 +87,7 @@ def attr_name_to_setting_name(self, name):
 def versioned_property(name, doc,
                        default=None, generator=None,
                        change_hook=prop_save_settings,
+                       mutable=False,
                        primer=prop_load_settings,
                        allowed=None, check_fn=None,
                        settings_properties=[],
@@ -125,10 +126,12 @@ def versioned_property(name, doc,
     def decorator(funcs):
         fulldoc = doc
         if default != None:
-            defaulting  = defaulting_property(default=default, null=EMPTY)
+            defaulting  = defaulting_property(default=default, null=EMPTY,
+                                              default_mutable=mutable)
             fulldoc += "\n\nThis property defaults to %s" % default
         if generator != None:
-            cached = cached_property(generator=generator, initVal=EMPTY)
+            cached = cached_property(generator=generator, initVal=EMPTY,
+                                     mutable=mutable)
             fulldoc += "\n\nThis property is generated with %s" % generator
         if check_fn != None:
             fn_checked = fn_checked_property(value_allowed_fn=check_fn)
@@ -137,7 +140,7 @@ def versioned_property(name, doc,
             checked = checked_property(allowed=allowed)
             fulldoc += "\n\nThe allowed values for this property are: %s." \
                        % (', '.join(allowed))
-        hooked      = change_hook_property(hook=change_hook)
+        hooked      = change_hook_property(hook=change_hook, mutable=mutable)
         primed      = primed_property(primer=primer, initVal=UNPRIMED)
         settings    = settings_property(name=name, null=UNPRIMED)
         docp        = doc_property(doc=fulldoc)
@@ -146,12 +149,10 @@ def versioned_property(name, doc,
             deco = defaulting(deco)
         if generator != None:
             deco = cached(deco)
-        if default != None:
-            deco = defaulting(deco)
-        if allowed != None:
-            deco = checked(deco)
         if check_fn != None:
             deco = fn_checked(deco)
+        if allowed != None:
+            deco = checked(deco)
         return Property(deco)
     return decorator
 
@@ -219,6 +220,7 @@ class SavedSettingsObject(object):
 
 class SavedSettingsObjectTests(unittest.TestCase):
     def testSimpleProperty(self):
+        """Testing a minimal versioned property"""
         class Test(SavedSettingsObject):
             settings_properties = []
             required_saved_properties = []
@@ -276,6 +278,7 @@ class SavedSettingsObjectTests(unittest.TestCase):
         self.failUnless(t.settings["Content-type"] == EMPTY,
                         t.settings["Content-type"])
     def testDefaultingProperty(self):
+        """Testing a defaulting versioned property"""
         class Test(SavedSettingsObject):
             settings_properties = []
             required_saved_properties = []
@@ -305,6 +308,7 @@ class SavedSettingsObjectTests(unittest.TestCase):
         self.failUnless(t._get_saved_settings()=={"Content-type":"text/html"},
                         t._get_saved_settings())
     def testRequiredDefaultingProperty(self):
+        """Testing a required defaulting versioned property"""
         class Test(SavedSettingsObject):
             settings_properties = []
             required_saved_properties = []
@@ -324,6 +328,7 @@ class SavedSettingsObjectTests(unittest.TestCase):
         self.failUnless(t._get_saved_settings()=={"Content-type":"text/html"},
                         t._get_saved_settings())
     def testClassVersionedPropertyDefinition(self):
+        """Testing a class-specific _versioned property decorator"""
         class Test(SavedSettingsObject):
             settings_properties = []
             required_saved_properties = []
@@ -348,6 +353,59 @@ class SavedSettingsObjectTests(unittest.TestCase):
         t.content_type = "text/html"
         self.failUnless(t._get_saved_settings()=={"Content-type":"text/html"},
                         t._get_saved_settings())
+    def testMutableChangeHookedProperty(self):
+        """Testing a mutable change-hooked property"""
+        SAVES = []
+        def prop_log_save_settings(self, old, new, saves=SAVES):
+            saves.append("'%s' -> '%s'" % (str(old), str(new)))
+            prop_save_settings(self, old, new)
+        class Test(SavedSettingsObject):
+            settings_properties = []
+            required_saved_properties = []
+            @versioned_property(name="List-type",
+                                doc="A test property",
+                                mutable=True,
+                                change_hook=prop_log_save_settings,
+                                settings_properties=settings_properties,
+                                required_saved_properties=required_saved_properties)
+            def list_type(): return {}
+            def __init__(self):
+                SavedSettingsObject.__init__(self)
+        t = Test()
+        self.failUnless(t._settings_loaded == False, t._settings_loaded)
+        t.load_settings()
+        self.failUnless(SAVES == [], SAVES)
+        self.failUnless(t._settings_loaded == True, t._settings_loaded)
+        self.failUnless(t.list_type == EMPTY, t.list_type)
+        self.failUnless(SAVES == [
+                "'None' -> '<class 'libbe.settings_object.EMPTY'>'"
+                ], SAVES)
+        self.failUnless(t.settings["List-type"]==EMPTY,t.settings["List-type"])
+        t.list_type = []
+        self.failUnless(t.settings["List-type"] == [], t.settings["List-type"])
+        self.failUnless(SAVES == [
+                "'None' -> '<class 'libbe.settings_object.EMPTY'>'",
+                "'<class 'libbe.settings_object.EMPTY'>' -> '[]'"
+                ], SAVES)
+        t.list_type.append(5)
+        self.failUnless(SAVES == [
+                "'None' -> '<class 'libbe.settings_object.EMPTY'>'",
+                "'<class 'libbe.settings_object.EMPTY'>' -> '[]'",
+                "'<class 'libbe.settings_object.EMPTY'>' -> '[]'" # <- TODO.  Where did this come from?
+                ], SAVES)
+        self.failUnless(t.settings["List-type"] == [5],t.settings["List-type"])
+        self.failUnless(SAVES == [ # the append(5) has not yet been saved
+                "'None' -> '<class 'libbe.settings_object.EMPTY'>'",
+                "'<class 'libbe.settings_object.EMPTY'>' -> '[]'",
+                "'<class 'libbe.settings_object.EMPTY'>' -> '[]'",
+                ], SAVES)
+        self.failUnless(t.list_type == [5], t.list_type) # <-get triggers saved
+        self.failUnless(SAVES == [ # now the append(5) has been saved.
+                "'None' -> '<class 'libbe.settings_object.EMPTY'>'",
+                "'<class 'libbe.settings_object.EMPTY'>' -> '[]'",
+                "'<class 'libbe.settings_object.EMPTY'>' -> '[]'",
+                "'[]' -> '[5]'"
+                ], SAVES)
 
 unitsuite=unittest.TestLoader().loadTestsFromTestCase(SavedSettingsObjectTests)
 suite = unittest.TestSuite([unitsuite, doctest.DocTestSuite()])