Make PresenceService responsible for tracking handle->activity mapping and activity property changed

4 files changed, 103 insertions, 68 deletions

diff --git a/src/activity.py b/src/activity.py
index b4a6b99..724e8d3 100644
--- a/src/activity.py
+++ b/src/activity.py
@@ -187,6 +187,13 @@ class Activity(ExportedGObject):
                     reply_handler=self.set_properties,
                     error_handler=got_properties_err)
 
+    @property
+    def room_details(self):
+        """Return the Telepathy plugin on which this Activity can be joined
+        and the handle of the room representing it.
+        """
+        return (self._tp, self._room)
+
     def do_get_property(self, pspec):
         """Gets the value of a property associated with this activity.
 
@@ -542,7 +549,7 @@ class Activity(ExportedGObject):
             if self._join_is_sharing:
                 self.send_properties()
                 self._ps.owner.add_activity(self)
-            self._join_cb(dbus.ObjectPath(self._object_path))
+            self._join_cb(self)
             _logger.debug("%s of activity %s succeeded" % (verb, self._id))
         except Exception, e:
             self._join_failed_cb(e)
@@ -606,7 +613,8 @@ class Activity(ExportedGObject):
     def join(self, async_cb, async_err_cb, sharing):
         """Local method for the local user to attempt to join the activity.
 
-        async_cb -- Callback method to be called if join attempt is successful
+        async_cb -- Callback method to be called with the Activity as a
+            parameter if join attempt is successful
         async_err_cb -- Callback method to be called if join attempt is
                         unsuccessful
 
diff --git a/src/presenceservice.py b/src/presenceservice.py
index 32dc5a9..818692e 100644
--- a/src/presenceservice.py
+++ b/src/presenceservice.py
@@ -38,6 +38,8 @@ from buddy import Buddy, ShellOwner
 from activity import Activity
 from psutils import pubkey_to_keyid
 
+CONN_INTERFACE_ACTIVITY_PROPERTIES = 'org.laptop.Telepathy.ActivityProperties'
+
 _PRESENCE_SERVICE = "org.laptop.Sugar.Presence"
 _PRESENCE_INTERFACE = "org.laptop.Sugar.Presence"
 _PRESENCE_PATH = "/org/laptop/Sugar/Presence"
@@ -45,7 +47,6 @@ _PRESENCE_PATH = "/org/laptop/Sugar/Presence"
 
 _logger = logging.getLogger('s-p-s.presenceservice')
 
-
 class NotFoundError(DBusException):
     def __init__(self, msg):
         DBusException.__init__(self, msg)
@@ -80,7 +81,12 @@ class PresenceService(ExportedGObject):
         self._handles_buddies = {}
 
         # activity id -> Activity
-        self._activities = {}
+        self._activities_by_id = {}
+        #: Tp plugin -> (handle -> Activity)
+        self._activities_by_handle = {}
+
+        #: Connection -> list of SignalMatch
+        self._conn_matches = {}
 
         self._session_bus = dbus.SessionBus()
         self._session_bus.add_signal_receiver(self._connection_disconnected_cb,
@@ -100,6 +106,7 @@ class PresenceService(ExportedGObject):
         # Set up the server connection
         self._server_plugin = ServerPlugin(self._registry, self._owner)
         self._handles_buddies[self._server_plugin] = {}
+        self._activities_by_handle[self._server_plugin] = {}
 
         self._server_plugin.connect('status', self._server_status_cb)
         self._server_plugin.connect('contact-online', self._contact_online)
@@ -113,13 +120,12 @@ class PresenceService(ExportedGObject):
                                     self._activity_invitation)
         self._server_plugin.connect('private-invitation',
                                     self._private_invitation)
-        self._server_plugin.connect('activity-properties-changed',
-                                    self._activity_properties_changed)
         self._server_plugin.start()
 
         # Set up the link local connection
         self._ll_plugin = LinkLocalPlugin(self._registry, self._owner)
         self._handles_buddies[self._ll_plugin] = {}
+        self._activities_by_handle[self._ll_plugin] = {}
 
         ExportedGObject.__init__(self, self._session_bus, _PRESENCE_PATH)
 
@@ -143,18 +149,45 @@ class PresenceService(ExportedGObject):
         old_status = self._connected
         if status == CONNECTION_STATUS_CONNECTED:
             self._connected = True
-            self._handles_buddies[plugin][plugin.self_handle] = self._owner
-            self._owner.add_telepathy_handle(plugin, plugin.self_handle)
+            self._tp_connected(plugin)
         else:
             self._connected = False
-            if plugin.self_handle is not None:
-                self._handles_buddies.setdefault(plugin, {}).pop(
-                        plugin.self_handle, None)
-                self._owner.remove_telepathy_handle(plugin, plugin.self_handle)
+            self._tp_disconnected(plugin)
 
         if self._connected != old_status:
             self.emit('connection-status', self._connected)
 
+    def _tp_connected(self, tp):
+        self._handles_buddies[tp][tp.self_handle] = self._owner
+        self._owner.add_telepathy_handle(tp, tp.self_handle)
+
+        conn = tp.get_connection()
+
+        self._conn_matches[conn] = []
+
+        def activity_properties_changed(room, properties):
+            self._activity_properties_changed(tp, room, properties)
+        m = conn[CONN_INTERFACE_ACTIVITY_PROPERTIES].connect_to_signal(
+                'ActivityPropertiesChanged',
+                activity_properties_changed)
+        self._conn_matches[conn].append(m)
+
+    def _tp_disconnected(self, tp):
+        if tp.self_handle is not None:
+            self._handles_buddies.setdefault(tp, {}).pop(
+                    tp.self_handle, None)
+            self._owner.remove_telepathy_handle(tp, tp.self_handle)
+
+        conn = tp.get_connection()
+
+        matches = self._conn_matches.get(conn)
+        try:
+            del self._conn_matches[conn]
+        except KeyError:
+            pass
+        for match in matches:
+            match.remove()
+
     def get_buddy(self, objid):
         buddy = self._buddies.get(objid)
         if buddy is None:
@@ -227,22 +260,35 @@ class PresenceService(ExportedGObject):
         except Exception:
             # FIXME: catching bare Exception considered harmful
             _logger.debug("Invalid activity:", exc_info=1)
-            return None
+            try:
+                del self._activities_by_handle[tp][room]
+            except KeyError:
+                pass
 
         activity.connect("validity-changed",
                          self._activity_validity_changed_cb)
         activity.connect("disappeared", self._activity_disappeared_cb)
-        self._activities[activity_id] = activity
+        self._activities_by_id[activity_id] = activity
+        self._activities_by_handle[tp][room] = activity
         return activity
 
     def _activity_disappeared_cb(self, activity):
         _logger.debug("activity %s disappeared" % activity.props.id)
 
         self.ActivityDisappeared(activity.object_path())
-        del self._activities[activity.props.id]
+        try:
+            del self._activities_by_id[activity.props.id]
+        except KeyError:
+            pass
+        tp, room = activity.room_details
+        try:
+            del self._activities_by_handle[tp][room]
+        except KeyError:
+            pass
 
     def _buddy_activities_changed(self, tp, contact_handle, activities):
-        _logger.debug("Handle %s activities changed: %s", contact_handle, activities)
+        _logger.debug("Handle %s activities changed: %s", contact_handle,
+                      activities)
         buddies = self._handles_buddies[tp]
         buddy = buddies.get(contact_handle)
 
@@ -260,12 +306,14 @@ class PresenceService(ExportedGObject):
         new_activities = set(activities.iterkeys())
 
         activities_joined = new_activities - old_activities
+
         for act in activities_joined:
+            room_handle = activities[act]
             _logger.debug("Handle %s joined activity %s", contact_handle, act)
-            activity = self._activities.get(act)
+            activity = self._activities_by_id.get(act)
             if activity is None:
                 # new activity, can fail
-                activity = self._new_activity(act, tp, activities[act])
+                activity = self._new_activity(act, tp, room_handle)
 
             if activity is not None:
                 activity.buddy_apparently_joined(buddy)
@@ -273,15 +321,20 @@ class PresenceService(ExportedGObject):
         activities_left = old_activities - new_activities
         for act in activities_left:
             _logger.debug("Handle %s left activity %s", contact_handle, act)
-            activity = self._activities.get(act)
-            if not activity:
+            activity = self._activities_by_id.get(act)
+            if activity is None:
+                # don't bother creating an Activity just so someone can leave
                 continue
 
             activity.buddy_apparently_left(buddy)
 
-    def _activity_invitation(self, tp, act_id, act_handle):
-        activity = self._activities.get(act_id)
-        if activity:
+    def _activity_invitation(self, tp, act_handle):
+        activity = self._activities_by_handle[tp].get(act_handle)
+        if activity is None:
+            # FIXME: we should synthesize an activity somehow, for the case of
+            # an invite to a non-public room
+            pass
+        else:
             self.ActivityInvitation(activity.object_path())
 
     def _private_invitation(self, tp, chan_path):
@@ -317,7 +370,7 @@ class PresenceService(ExportedGObject):
                          out_signature="ao")
     def GetActivities(self):
         ret = []
-        for act in self._activities.values():
+        for act in self._activities_by_id.values():
             if act.props.valid:
                 ret.append(act.object_path())
         return ret
@@ -325,7 +378,7 @@ class PresenceService(ExportedGObject):
     @dbus.service.method(_PRESENCE_INTERFACE, in_signature="s",
                          out_signature="o")
     def GetActivityById(self, actid):
-        act = self._activities.get(actid, None)
+        act = self._activities_by_id.get(actid, None)
         if not act or not act.props.valid:
             raise NotFoundError("The activity was not found.")
         return act.object_path()
@@ -476,8 +529,14 @@ class PresenceService(ExportedGObject):
                             name=name, color=color, local=True)
         activity.connect("validity-changed",
                          self._activity_validity_changed_cb)
-        self._activities[actid] = activity
-        activity.join(async_cb, async_err_cb, True)
+        self._activities_by_id[actid] = activity
+
+        def activity_shared():
+            tp, room = activity.room_details
+            self._activities_by_handle[tp][room] = activity
+            async_cb(activity.object_path())
+
+        activity.join(activity_shared, async_err_cb, True)
 
         # local activities are valid at creation by definition, but we can't
         # connect to the activity's validity-changed signal until its already
@@ -495,9 +554,12 @@ class PresenceService(ExportedGObject):
             _logger.debug("Activity disappeared: %s (%s)", activity.props.name,
                           activity.props.id)
 
-    def _activity_properties_changed(self, tp, act_id, act_handle, props):
-        activity = self._activities.get(act_id)
-        if activity:
+    def _activity_properties_changed(self, tp, act_handle, props):
+        activity = self._activities_by_handle[tp].get(act_handle)
+        if activity is None:
+            # FIXME: synthesize an activity
+            pass
+        else:
             activity.set_properties(props)
 
 
diff --git a/src/pstest.py b/src/pstest.py
index 7715fd3..3505a4d 100644
--- a/src/pstest.py
+++ b/src/pstest.py
@@ -176,7 +176,7 @@ class TestPresenceService(PresenceService):
                          self.__test_num, self.__randomize)
 
     def internal_get_activity(self, actid):
-        return self._activities.get(actid, None)
+        return self._activities_by_id.get(actid, None)
 
 
 def _extract_public_key(keyfile):
diff --git a/src/server_plugin.py b/src/server_plugin.py
index 31d14b1..10d5731 100644
--- a/src/server_plugin.py
+++ b/src/server_plugin.py
@@ -105,23 +105,13 @@ class ServerPlugin(gobject.GObject):
         'activity-invitation':
             # We were invited to join an activity
             # args:
-            #   activity ID: str
             #   activity room handle: int or long
-            (gobject.SIGNAL_RUN_FIRST, None, [object, object]),
+            (gobject.SIGNAL_RUN_FIRST, None, [object]),
         'private-invitation':
             # We were invited to join a chat or a media call
             # args:
             #   channel object path
             (gobject.SIGNAL_RUN_FIRST, None, [object]),
-        'activity-properties-changed':
-            # An activity's properties changed; as for
-            # ActivityPropertiesChanged
-            # args:
-            #   activity ID: str
-            #   activity room handle: int or long
-            #   properties: dict { str => object }
-            # FIXME: are these all the properties or just those that changed?
-            (gobject.SIGNAL_RUN_FIRST, None, [object, object, object]),
     }
 
     def __init__(self, registry, owner):
@@ -145,9 +135,6 @@ class ServerPlugin(gobject.GObject):
         self._registry = registry
         self._online_contacts = {}  # handle -> jid
 
-        # activity id -> handle
-        self._activities = {}
-
         self._owner = owner
         self.self_handle = None
 
@@ -364,10 +351,6 @@ class ServerPlugin(gobject.GObject):
         self._conn[CONN_INTERFACE_ALIASING].connect_to_signal('AliasesChanged',
                 self._alias_changed_cb)
         self._matches.append(m)
-        self._conn[CONN_INTERFACE_ACTIVITY_PROPERTIES].connect_to_signal(
-                'ActivityPropertiesChanged',
-                self._activity_properties_changed_cb)
-        self._matches.append(m)
 
         # Request presence for everyone we're subscribed to
         self._conn[CONN_INTERFACE_PRESENCE].RequestPresence(subscribe_handles)
@@ -469,7 +452,6 @@ class ServerPlugin(gobject.GObject):
         for handle in self._online_contacts.keys():
             self._contact_offline(handle)
         self._online_contacts = {}
-        self._activities = {}
 
         if self._reconnect_id > 0:
             gobject.source_remove(self._reconnect_id)
@@ -725,7 +707,6 @@ class ServerPlugin(gobject.GObject):
 
         activities_dict = {}
         for act_id, act_handle in activities:
-            self._activities[act_id] = act_handle
             activities_dict[act_id] = act_handle
         self.emit("buddy-activities-changed", handle, activities_dict)
 
@@ -753,18 +734,9 @@ class ServerPlugin(gobject.GObject):
         if (handle_type == HANDLE_TYPE_ROOM and
             channel_type == CHANNEL_TYPE_TEXT):
             def ready(channel):
-
-                for act_id, act_handle in self._activities.iteritems():
-                    if handle == act_handle:
-                        break
-                    else:
-                        return
-
                 def got_all_members(current, local_pending, remote_pending):
                     if local_pending:
-                        for act_id, act_handle in self._activities.iteritems():
-                            if handle == act_handle:
-                                self.emit('activity-invitation', act_id, handle)
+                        self.emit('activity-invitation', handle)
                 def got_all_members_err(e):
                     logger.debug('Unable to get channel members for %s:',
                                  object_path, exc_info=1)
@@ -782,13 +754,6 @@ class ServerPlugin(gobject.GObject):
                                CHANNEL_TYPE_STREAMED_MEDIA)):
             self.emit("private-invitation", object_path)
 
-    def _activity_properties_changed_cb(self, room, properties):
-        """Handle update of properties for a "room" (activity handle)"""
-        for act_id, act_handle in self._activities.items():
-            if room == act_handle:
-                self.emit("activity-properties-changed", act_id, room, properties)
-                return
-
     def _server_is_trusted(self, hostname):
         """Return True if the server with the given hostname is trusted to
         verify public-key ownership correctly, and only allows users to