Cleanups; replace ObjectCache with a straight dict; fix signal name; doc cleanups

1 files changed, 18 insertions, 90 deletions

diff --git a/sugar/presence/presenceservice.py b/sugar/presence/presenceservice.py
index 7592cdd..13684a5 100644
--- a/sugar/presence/presenceservice.py
+++ b/sugar/presence/presenceservice.py
@@ -29,71 +29,6 @@ import logging
 # see PEP: http://docs.python.org/whatsnew/pep-328.html
 import buddy, activity
 
-class ObjectCache(object):
-    """Path to Activity/Buddy object cache
-    
-    On notification of a new object of either type the 
-    PresenceService client stores the object's representation
-    in this object.
-    
-    XXX Why not just sub-class dict?  We're only adding two
-        methods then and we would have all of the other 
-        standard operations on dictionaries.
-    """
-    def __init__(self):
-        """Initialise the cache"""
-        self._cache = {}
-
-    def get(self, object_path):
-        """Retrieve specified object from the cache 
-        
-        object_path -- full dbus path to the object
-        
-        returns a presence.buddy.Buddy or presence.activity.Activity
-        instance or None if the object_path is not yet cached.
-        
-        XXX could be written as return self._cache.get( object_path )
-        """
-        try:
-            return self._cache[object_path]
-        except KeyError:
-            return None
-
-    def add(self, obj):
-        """Adds given presence object to the cache 
-        
-        obj -- presence Buddy or Activity representation, the object's
-            object_path() method is used as the key for storage
-        
-        returns None 
-        
-        XXX should raise an error on collisions, shouldn't it? or 
-            return True/False to say whether the item was actually
-            added
-        """
-        op = obj.object_path()
-        if not self._cache.has_key(op):
-            self._cache[op] = obj
-
-    def remove(self, object_path):
-        """Remove the given presence object from the cache 
-        
-        object_path -- full dbus path to the object
-        
-        returns None 
-        
-        XXX does two checks instead of one with a try:except for the 
-            keyerror, normal case of deleting existing penalised as
-            a result.
-            
-            try:
-                return self._cache.pop( key )
-            except KeyError:
-                return None
-        """
-        if self._cache.has_key(object_path):
-            del self._cache[object_path]
-
 
 DBUS_SERVICE = "org.laptop.Sugar.Presence"
 DBUS_INTERFACE = "org.laptop.Sugar.Presence"
@@ -135,7 +70,7 @@ class PresenceService(gobject.GObject):
         """Initialise the service and attempt to connect to events
         """
         gobject.GObject.__init__(self)
-        self._objcache = ObjectCache()
+        self._objcache = {}
         # attempt to load the interface to the service...
         self._get_ps()
     
@@ -199,12 +134,16 @@ class PresenceService(gobject.GObject):
         
         Note that this method is called throughout the class whenever
         the representation of the object is required, it is not only 
-        called when the object is first discovered.
+        called when the object is first discovered.  The point is to only have
+        _one_ Python object for any D-Bus object represented by an object path,
+        effectively wrapping the D-Bus object in a single Python GObject.
         
         returns presence Buddy or Activity representation
         """
-        obj = self._objcache.get(object_path)
-        if not obj:
+        obj = None
+        try:
+            obj = self._objcache[object_path]
+        except KeyError:
             if object_path.startswith(self._PS_BUDDY_OP):
                 obj = buddy.Buddy(self._bus, self._new_object,
                         self._del_object, object_path)
@@ -213,12 +152,13 @@ class PresenceService(gobject.GObject):
                         self._del_object, object_path)
             else:
                 raise RuntimeError("Unknown object type")
-            self._objcache.add(obj)
+            self._objcache[object_path] = obj
         return obj
 
     def _del_object(self, object_path):
-        # FIXME
-        pass
+        """Fully remove an object from the object cache when it's no longer needed.
+        """
+        del self._objcache[object_path]
 
     def _emit_buddy_appeared_signal(self, object_path):
         """Emit GObject event with presence.buddy.Buddy object"""
@@ -248,13 +188,8 @@ class PresenceService(gobject.GObject):
         gobject.idle_add(self._emit_activity_invitation_signal, object_path)
 
     def _emit_private_invitation_signal(self, bus_name, connection, channel):
-        """Emit GObject event with bus_name, connection and channel
-        
-        XXX This seems to generate the wrong GObject event?  It generates 
-            'service-disappeared' instead of private-invitation for some 
-            reason.  That event doesn't even seem to be registered?
-        """
-        self.emit('service-disappeared', bus_name, connection, channel)
+        """Emit GObject event with bus_name, connection and channel"""
+        self.emit('private-invitation', bus_name, connection, channel)
         return False
 
     def _private_invitation_cb(self, bus_name, connection, channel):
@@ -281,10 +216,8 @@ class PresenceService(gobject.GObject):
         gobject.idle_add(self._emit_activity_disappeared_signal, object_path)
 
     def get(self, object_path):
-        """Retrieve given object path as a Buddy/Activity object
-        
-        XXX This is basically just an alias for _new_object, i.e. it 
-            just adds an extra function-call to the operation.
+        """Return the Buddy or Activity object corresponding to the given
+        D-Bus object path.
         """
         return self._new_object(object_path)
 
@@ -368,12 +301,7 @@ class PresenceService(gobject.GObject):
         return self._new_object(buddy_op)
 
     def get_owner(self):
-        """Retrieves "owner" as a Buddy
-        
-        XXX check that it really is a Buddy that's produced, what is 
-            this the owner of?  Shouldn't it be getting an activity 
-            and then asking who the owner of that is?
-        """
+        """Retrieves the laptop "owner" Buddy object."""
         try:
             owner_op = self._ps.GetOwner()
         except dbus.exceptions.DBusException, err:
@@ -381,7 +309,7 @@ class PresenceService(gobject.GObject):
                 """Unable to retrieve local user/owner from presence service: %s""",
                 err
             )
-            return None
+            raise RuntimeError("Could not get owner object from presence service.")
         return self._new_object(owner_op)
 
     def _share_activity_cb(self, activity, op):