services/presence/: Pass the SessionBus around, instead of a BusName object.

Delay making the BusName object until after the PS object has been exported,
to guarantee race-free activation.

4 files changed, 52 insertions, 39 deletions

diff --git a/services/presence/activity.py b/services/presence/activity.py
index 2e2ea55..4353fbb 100644
--- a/services/presence/activity.py
+++ b/services/presence/activity.py
@@ -39,7 +39,10 @@ _PROP_CUSTOM_PROPS = "custom-props"
 _logger = logging.getLogger('s-p-s.activity')
 
 class Activity(ExportedGObject):
-    """Represents a potentially shareable activity on the network.
+    """Represents a shared activity seen on the network, or a local activity
+    that has been, or will be, shared onto the network.
+
+    The activity might be public, restricted to a group, or invite-only.
     """
 
     __gtype_name__ = "Activity"
@@ -68,12 +71,12 @@ class Activity(ExportedGObject):
 
     _RESERVED_PROPNAMES = __gproperties__.keys()
 
-    def __init__(self, bus_name, object_id, tp, **kwargs):
+    def __init__(self, bus, object_id, tp, **kwargs):
         """Initializes the activity and sets its properties to default values.
 
         :Parameters:
-            `bus_name` : dbus.service.BusName
-                D-Bus well-known name for the Presence Service
+            `bus` : dbus.bus.BusConnection
+                A connection to the D-Bus session bus
             `object_id` : int
                 PS ID for this activity, used to construct the object-path
             `tp` : server plugin
@@ -95,8 +98,6 @@ class Activity(ExportedGObject):
                 Activity-specific properties
         """
 
-        if not bus_name:
-            raise ValueError("DBus bus name must be valid")
         if not object_id or not isinstance(object_id, int):
             raise ValueError("object id must be a valid number")
         if not tp:
@@ -133,7 +134,7 @@ class Activity(ExportedGObject):
         if not util.validate_activity_id(kwargs[_PROP_ID]):
             raise ValueError("Invalid activity id '%s'" % kwargs[_PROP_ID])
 
-        ExportedGObject.__init__(self, bus_name, self._object_path,
+        ExportedGObject.__init__(self, bus, self._object_path,
                                  gobject_properties=kwargs)
         if self.props.local and not self.props.valid:
             raise RuntimeError("local activities require color, type, and "
@@ -262,9 +263,9 @@ class Activity(ExportedGObject):
     @dbus.service.method(_ACTIVITY_INTERFACE,
                         in_signature="", out_signature="s")
     def GetId(self):
-        """DBUS method to get this activity's ID
+        """DBUS method to get this activity's (randomly generated) unique ID
 
-        returns Activity ID
+        :Returns: Activity ID as a string
         """
         return self.props.id
 
@@ -273,7 +274,7 @@ class Activity(ExportedGObject):
     def GetColor(self):
         """DBUS method to get this activity's colour
 
-        returns Activity colour
+        :Returns: Activity colour as a string in the format #RRGGBB,#RRGGBB
         """
         return self.props.color
 
@@ -282,7 +283,8 @@ class Activity(ExportedGObject):
     def GetType(self):
         """DBUS method to get this activity's type
 
-        returns Activity type
+        :Returns: Activity type as a string, in the same form as a D-Bus
+            well-known name
         """
         return self.props.type
 
@@ -305,7 +307,10 @@ class Activity(ExportedGObject):
         """DBUS method to return a list of valid buddies who are joined in
         this activity
 
-        returns A list of buddy object paths
+        :Returns:
+            A list of buddy object paths corresponding to those buddies
+            in this activity who are 'valid' (i.e. for whom we have complete
+            information)
         """
         ret = []
         for buddy in self._buddies:
@@ -319,8 +324,14 @@ class Activity(ExportedGObject):
         """DBUS method to get the list of channels associated with this
         activity
 
-        returns XXX - Not sure what this returns as get_channels doesn't
-        actually return a list of channels!
+        :Returns:
+            a tuple containing:
+                - the D-Bus well-known service name of the connection
+                  (FIXME: this is redundant; in Telepathy it can be derived
+                  from that of the connection)
+                - the D-Bus object path of the connection
+                - a list of D-Bus object paths representing the channels
+                  associated with this activity
         """
         return self.get_channels()
 
diff --git a/services/presence/buddy.py b/services/presence/buddy.py
index 60589b4..1b45fd5 100644
--- a/services/presence/buddy.py
+++ b/services/presence/buddy.py
@@ -102,22 +102,19 @@ class Buddy(ExportedGObject):
         _PROP_IP4_ADDRESS  : (str, None, None, None, gobject.PARAM_READWRITE)
     }
 
-    def __init__(self, bus_name, object_id, **kwargs):
+    def __init__(self, bus, object_id, **kwargs):
         """Initialize the Buddy object
 
-        bus_name -- DBUS object bus name (identifier)
+        bus -- connection to the D-Bus session bus
         object_id -- the activity's unique identifier
         kwargs -- used to initialize the object's properties
 
         constructs a DBUS "object path" from the _BUDDY_PATH
         and object_id
         """
-        if not bus_name:
-            raise ValueError("DBus bus name must be valid")
         if not object_id or not isinstance(object_id, int):
             raise ValueError("object id must be a valid number")
 
-        self._bus_name = bus_name
         self._object_path = _BUDDY_PATH + str(object_id)
 
         self._activities = {}   # Activity ID -> Activity
@@ -150,7 +147,7 @@ class Buddy(ExportedGObject):
             icon_data = kwargs[_PROP_ICON]
             del kwargs[_PROP_ICON]
 
-        ExportedGObject.__init__(self, bus_name, self._object_path,
+        ExportedGObject.__init__(self, bus, self._object_path,
                                  gobject_properties=kwargs)
 
         if icon_data:
@@ -499,11 +496,11 @@ class GenericOwner(Buddy):
     """
     __gtype_name__ = "GenericOwner"
 
-    def __init__(self, ps, bus_name, object_id, **kwargs):
+    def __init__(self, ps, bus, object_id, **kwargs):
         """Initialize the GenericOwner instance
 
         ps -- presenceservice.PresenceService object
-        bus_name -- DBUS object bus name (identifier)
+        bus -- a connection to the D-Bus session bus
         object_id -- the activity's unique identifier
         kwargs -- used to initialize the object's properties
 
@@ -520,7 +517,7 @@ class GenericOwner(Buddy):
         if self._ip4_addr_monitor.props.address:
             kwargs["ip4-address"] = self._ip4_addr_monitor.props.address
 
-        Buddy.__init__(self, bus_name, object_id, **kwargs)
+        Buddy.__init__(self, bus, object_id, **kwargs)
         self._owner = True
 
         self._bus = dbus.SessionBus()
@@ -561,11 +558,11 @@ class ShellOwner(GenericOwner):
     _SHELL_OWNER_INTERFACE = "org.laptop.Shell.Owner"
     _SHELL_PATH = "/org/laptop/Shell"
 
-    def __init__(self, ps, bus_name, object_id, test=False):
+    def __init__(self, ps, bus, object_id, test=False):
         """Initialize the ShellOwner instance
 
         ps -- presenceservice.PresenceService object
-        bus_name -- DBUS object bus name (identifier)
+        bus -- a connection to the D-Bus session bus
         object_id -- the activity's unique identifier
         test -- ignored
 
@@ -587,7 +584,7 @@ class ShellOwner(GenericOwner):
         icon = f.read()
         f.close()
 
-        GenericOwner.__init__(self, ps, bus_name, object_id, key=key,
+        GenericOwner.__init__(self, ps, bus, object_id, key=key,
                 nick=nick, color=color, icon=icon, server=server,
                 key_hash=key_hash, registered=registered)
 
diff --git a/services/presence/presenceservice.py b/services/presence/presenceservice.py
index dd76a66..bf058d3 100644
--- a/services/presence/presenceservice.py
+++ b/services/presence/presenceservice.py
@@ -57,7 +57,7 @@ class PresenceService(ExportedGObject):
 
     def _create_owner(self):
         # Overridden by TestPresenceService
-        return ShellOwner(self, self._bus_name, self._get_next_object_id())
+        return ShellOwner(self, self._session_bus, self._get_next_object_id())
 
     def __init__(self):
         self._next_object_id = 0
@@ -67,11 +67,10 @@ class PresenceService(ExportedGObject):
         self._handles_buddies = {}      # tp client -> (handle -> Buddy)
         self._activities = {}   # activity id -> Activity
 
-        bus = dbus.SessionBus()
-        self._bus_name = dbus.service.BusName(_PRESENCE_SERVICE, bus=bus)
-        bus.add_signal_receiver(self._connection_disconnected_cb,
-                                signal_name="Disconnected",
-                                dbus_interface="org.freedesktop.DBus")
+        self._session_bus = dbus.SessionBus()
+        self._session_bus.add_signal_receiver(self._connection_disconnected_cb,
+                signal_name="Disconnected",
+                dbus_interface="org.freedesktop.DBus")
 
         # Create the Owner object
         self._owner = self._create_owner()
@@ -104,7 +103,13 @@ class PresenceService(ExportedGObject):
         self._ll_plugin = LinkLocalPlugin(self._registry, self._owner)
         self._handles_buddies[self._ll_plugin] = {}
 
-        ExportedGObject.__init__(self, self._bus_name, _PRESENCE_PATH)
+        ExportedGObject.__init__(self, self._session_bus, _PRESENCE_PATH)
+
+        # for activation to work in a race-free way, we should really
+        # export the bus name only after we export our initial object;
+        # so this comes after the parent __init__
+        self._bus_name = dbus.service.BusName(_PRESENCE_SERVICE,
+                                              bus=self._session_bus)
 
     def _connection_disconnected_cb(self, foo=None):
         """Log event when D-Bus kicks us off the bus for some reason"""
@@ -135,7 +140,7 @@ class PresenceService(ExportedGObject):
         if not buddy:
             # we don't know yet this buddy
             objid = self._get_next_object_id()
-            buddy = Buddy(self._bus_name, objid, key=key)
+            buddy = Buddy(self._session_bus, objid, key=key)
             buddy.connect("validity-changed", self._buddy_validity_changed_cb)
             buddy.connect("disappeared", self._buddy_disappeared_cb)
             self._buddies[key] = buddy
@@ -194,7 +199,7 @@ class PresenceService(ExportedGObject):
     def _new_activity(self, activity_id, tp):
         try:
             objid = self._get_next_object_id()
-            activity = Activity(self._bus_name, objid, tp, id=activity_id)
+            activity = Activity(self._session_bus, objid, tp, id=activity_id)
         except Exception:
             # FIXME: catching bare Exception considered harmful
             _logger.debug("Invalid activity:", exc_info=1)
@@ -392,7 +397,7 @@ class PresenceService(ExportedGObject):
         objid = self._get_next_object_id()
         # FIXME check which tp client we should use to share the activity
         color = self._owner.props.color
-        activity = Activity(self._bus_name, objid, self._server_plugin,
+        activity = Activity(self._session_bus, objid, self._server_plugin,
                             id=actid, type=atype, name=name, color=color,
                             local=True)
         activity.connect("validity-changed",
diff --git a/services/presence/pstest.py b/services/presence/pstest.py
index edec3a8..55a1e32 100644
--- a/services/presence/pstest.py
+++ b/services/presence/pstest.py
@@ -20,7 +20,7 @@ class TestOwner(GenericOwner):
 
     __gtype_name__ = "TestOwner"
 
-    def __init__(self, ps, bus_name, object_id, test_num, randomize):
+    def __init__(self, ps, bus, object_id, test_num, randomize):
         self._cp = ConfigParser()
         self._section = "Info"
         self._test_activities = []
@@ -45,7 +45,7 @@ class TestOwner(GenericOwner):
         icon = _get_random_image()
 
         _logger.debug("pubkey is %s" % pubkey)
-        GenericOwner.__init__(self, ps, bus_name, object_id, key=pubkey, nick=nick,
+        GenericOwner.__init__(self, ps, bus, object_id, key=pubkey, nick=nick,
                 color=color, icon=icon, registered=registered, key_hash=privkey_hash)
 
         # Only do the random stuff if randomize is true
@@ -152,7 +152,7 @@ class TestPresenceService(PresenceService):
         PresenceService.__init__(self)
 
     def _create_owner(self):
-        return TestOwner(self, self._bus_name, self._get_next_object_id(),
+        return TestOwner(self, self._session_bus, self._get_next_object_id(),
                          self.__test_num, self.__randomize)
 
     def internal_get_activity(self, actid):