Merge branch 'master' of git://dev.laptop.org/sugar

services/presence/Makefile.am

@@ -17,8 +17,15 @@ sugar_PYTHON = \
 	psutils.py \
 	server_plugin.py
 
-bin_SCRIPTS = sugar-presence-service
+dist_bin_SCRIPTS = sugar-presence-service
 
 DISTCLEANFILES = $(service_DATA)
 
-EXTRA_DIST = $(service_in_files) $(bin_SCRIPTS)
+EXTRA_DIST = $(service_in_files)
+
+dist_check_SCRIPTS = test_psutils.py
+
+TESTS_ENVIRONMENT = \
+		    PYTHONPATH=$(top_srcdir):$(top_srcdir)/services/presence \
+		    $(PYTHON)
+TESTS = $(dist_check_SCRIPTS)

services/presence/activity.py

@@ -22,7 +22,8 @@ from dbus.gobject_service import ExportedGObject
 from sugar import util
 import logging
 
-from telepathy.interfaces import (CHANNEL_INTERFACE)
+from telepathy.constants import CHANNEL_GROUP_FLAG_CHANNEL_SPECIFIC_HANDLES
+from telepathy.interfaces import (CHANNEL_INTERFACE, CHANNEL_INTERFACE_GROUP)
 
 _ACTIVITY_PATH = "/org/laptop/Sugar/Presence/Activities/"
 _ACTIVITY_INTERFACE = "org.laptop.Sugar.Presence.Activity"
@@ -48,8 +49,17 @@ class Activity(ExportedGObject):
     __gtype_name__ = "Activity"
 
     __gsignals__ = {
-        'validity-changed': (gobject.SIGNAL_RUN_FIRST, gobject.TYPE_NONE,
+        'validity-changed':
-                            ([gobject.TYPE_BOOLEAN]))
+            # The activity's validity has changed.
+            # An activity is valid if its name, color, type and ID have been
+            # set.
+            # Arguments:
+            #   validity: bool
+            (gobject.SIGNAL_RUN_FIRST, None, [bool]),
+        'disappeared':
+            # Nobody is in this activity any more.
+            # No arguments.
+            (gobject.SIGNAL_RUN_FIRST, None, []),
     }
 
     __gproperties__ = {
@@ -71,7 +81,7 @@ class Activity(ExportedGObject):
 
     _RESERVED_PROPNAMES = __gproperties__.keys()
 
-    def __init__(self, bus, object_id, tp, **kwargs):
+    def __init__(self, bus, object_id, ps, tp, **kwargs):
         """Initializes the activity and sets its properties to default values.
 
         :Parameters:
@@ -79,6 +89,8 @@ class Activity(ExportedGObject):
                 A connection to the D-Bus session bus
             `object_id` : int
                 PS ID for this activity, used to construct the object-path
+            `ps` : presenceservice.PresenceService
+                The presence service
             `tp` : server plugin
                 The server plugin object (stands for "telepathy plugin")
         :Keywords:
@@ -103,16 +115,20 @@ class Activity(ExportedGObject):
         if not tp:
             raise ValueError("telepathy CM must be valid")
 
+        self._ps = ps
         self._object_id = object_id
         self._object_path = dbus.ObjectPath(_ACTIVITY_PATH +
                                             str(self._object_id))
 
-        self._buddies = []
+        self._buddies = set()
+        self._member_handles = set()
         self._joined = False
 
         # the telepathy client
         self._tp = tp
+        self._self_handle = None
         self._text_channel = None
+        self._text_channel_group_flags = 0
 
         self._valid = False
         self._id = None
@@ -367,8 +383,10 @@ class Activity(ExportedGObject):
                 ret.append(buddy)
         return ret
 
-    def buddy_joined(self, buddy):
+    def buddy_apparently_joined(self, buddy):
-        """Adds a buddy to this activity and sends a BuddyJoined signal
+        """Adds a buddy to this activity and sends a BuddyJoined signal,
+        unless we can already see who's in the activity by being in it
+        ourselves.
 
         buddy -- Buddy object representing the buddy being added
 
@@ -379,25 +397,54 @@ class Activity(ExportedGObject):
         This method is called by the PresenceService on the local machine.
 
         """
-        if buddy not in self._buddies:
+        if not self._joined:
-            self._buddies.append(buddy)
+            self._add_buddies((buddy,))
+
+    def _add_buddies(self, buddies):
+        buddies = set(buddies)
+
+        # disregard any who are already there
+        buddies -= self._buddies
+
+        self._buddies |= buddies
+
+        for buddy in buddies:
+            buddy.add_activity(self)
             if self.props.valid:
                 self.BuddyJoined(buddy.object_path())
 
-    def buddy_left(self, buddy):
+    def _remove_buddies(self, buddies):
-        """Removes a buddy from this activity and sends a BuddyLeft signal.
+        buddies = set(buddies)
+
+        # disregard any who are not already there
+        buddies &= self._buddies
+
+        self._buddies -= buddies
+
+        for buddy in buddies:
+            buddy.remove_activity(self)
+            if self.props.valid:
+                self.BuddyJoined(buddy.object_path())
+
+        if not self._buddies:
+            self.emit('disappeared')
+
+    def buddy_apparently_left(self, buddy):
+        """Removes a buddy from this activity and sends a BuddyLeft signal,
+        unless we can already see who's in the activity by being in it
+        ourselves.
 
         buddy -- Buddy object representing the buddy being removed
 
         Removes a buddy from this activity if the buddy is in the buddy list.
         If this activity is "valid", a BuddyLeft signal is also sent.
         This method is called by the PresenceService on the local machine.
-
         """
-        if buddy in self._buddies:
+        if not self._joined:
-            self._buddies.remove(buddy)
+            self._remove_buddies((buddy,))
-            if self.props.valid:
+
-                self.BuddyLeft(buddy.object_path())
+    def _text_channel_group_flags_changed_cb(self, flags):
+        self._text_channel_group_flags = flags
 
     def _handle_share_join(self, tp, text_channel):
         """Called when a join to a network activity was successful.
@@ -412,7 +459,36 @@ class Activity(ExportedGObject):
         self._text_channel = text_channel
         self._text_channel[CHANNEL_INTERFACE].connect_to_signal('Closed',
                 self._text_channel_closed_cb)
-        self._joined = True
+        if CHANNEL_INTERFACE_GROUP in self._text_channel:
+            group = self._text_channel[CHANNEL_INTERFACE_GROUP]
+
+            # FIXME: make these method calls async?
+
+            group.connect_to_signal('GroupFlagsChanged',
+                                    self._text_channel_group_flags_changed_cb)
+            self._text_channel_group_flags = group.GetGroupFlags()
+
+            self._self_handle = group.GetSelfHandle()
+
+            # by the time we hook this, we need to know the group flags
+            group.connect_to_signal('MembersChanged',
+                                    self._text_channel_members_changed_cb)
+            # bootstrap by getting the current state. This is where we find
+            # out whether anyone was lying to us in their PEP info
+            members = set(group.GetMembers())
+            added = members - self._member_handles
+            removed = self._member_handles - members
+            if added or removed:
+                self._text_channel_members_changed_cb('', added, removed,
+                                                      (), (), 0, 0)
+
+            # if we can see any member handles, we're probably able to see
+            # all members, so can stop caring about PEP announcements for this
+            # activity
+            self._joined = (self._self_handle in self._member_handles)
+        else:
+            self._joined = True
+
         return True
 
     def _shared_cb(self, tp, activity_id, text_channel, exc, userdata):
@@ -505,12 +581,59 @@ class Activity(ExportedGObject):
         if self._joined:
             self._text_channel[CHANNEL_INTERFACE].Close()
 
+    def _text_channel_members_changed_cb(self, message, added, removed,
+                                         local_pending, remote_pending,
+                                         actor, reason):
+        # Note: D-Bus calls this with list arguments, but after GetMembers()
+        # we call it with set and tuple arguments; we cope with any iterable.
+
+        if (self._text_channel_group_flags &
+            CHANNEL_GROUP_FLAG_CHANNEL_SPECIFIC_HANDLES):
+            map_chan = self._text_channel
+        else:
+            # we have global handles here
+            map_chan = None
+
+        # disregard any who are already there
+        added = set(added)
+        added -= self._member_handles
+        self._member_handles |= added
+
+        # for added people, we need a Buddy object
+        added_buddies = self._ps.map_handles_to_buddies(self._tp,
+                                                        map_chan,
+                                                        added)
+        self._add_buddies(added_buddies.itervalues())
+
+        # we treat all pending members as if they weren't there
+        removed = set(removed)
+        removed |= set(local_pending)
+        removed |= set(remote_pending)
+        # disregard any who aren't already there
+        removed &= self._member_handles
+        self._member_handles -= removed
+
+        # for removed people, don't bother creating a Buddy just so we can
+        # say it left. If we don't already have a Buddy object for someone,
+        # then obviously they're not in self._buddies!
+        removed_buddies = self._ps.map_handles_to_buddies(self._tp,
+                                                          map_chan,
+                                                          removed,
+                                                          create=False)
+        self._remove_buddies(removed_buddies.itervalues())
+
+        # if we were among those removed, we'll have to start believing
+        # the spoofable PEP-based activity tracking again.
+        if self._self_handle not in self._member_handles:
+            self._joined = False
+
     def _text_channel_closed_cb(self):
         """Callback method called when the text channel is closed.
 
         This callback is set up in the _handle_share_join method.
         """
         self._joined = False
+        self._self_handle = None
         self._text_channel = None
 
     def send_properties(self):

services/presence/buddy.py

@@ -37,6 +37,7 @@ _PROP_CURACT = "current-activity"
 _PROP_COLOR = "color"
 _PROP_OWNER = "owner"
 _PROP_VALID = "valid"
+_PROP_OBJID = 'objid'
 
 # Will go away soon
 _PROP_IP4_ADDRESS = "ip4-address"
@@ -90,15 +91,14 @@ class Buddy(ExportedGObject):
     }
 
     __gproperties__ = {
-        _PROP_KEY          : (str, None, None, None,
+        _PROP_KEY          : (str, None, None, None, gobject.PARAM_READWRITE),
-                              gobject.PARAM_READWRITE |
-                              gobject.PARAM_CONSTRUCT_ONLY),
         _PROP_ICON         : (object, None, None, gobject.PARAM_READWRITE),
         _PROP_NICK         : (str, None, None, None, gobject.PARAM_READWRITE),
         _PROP_COLOR        : (str, None, None, None, gobject.PARAM_READWRITE),
         _PROP_CURACT       : (str, None, None, None, gobject.PARAM_READWRITE),
         _PROP_VALID        : (bool, None, None, False, gobject.PARAM_READABLE),
         _PROP_OWNER        : (bool, None, None, False, gobject.PARAM_READABLE),
+        _PROP_OBJID        : (str, None, None, None, gobject.PARAM_READABLE),
         _PROP_IP4_ADDRESS  : (str, None, None, None, gobject.PARAM_READWRITE)
     }
 
@@ -106,16 +106,16 @@ class Buddy(ExportedGObject):
         """Initialize the Buddy object
 
         bus -- connection to the D-Bus session bus
-        object_id -- the activity's unique identifier
+        object_id -- the buddy's unique identifier, either based on their
+            key-ID or JID
         kwargs -- used to initialize the object's properties
 
         constructs a DBUS "object path" from the _BUDDY_PATH
         and object_id
         """
-        if not object_id or not isinstance(object_id, int):
-            raise ValueError("object id must be a valid number")
 
-        self._object_path = _BUDDY_PATH + str(object_id)
+        self._object_id = object_id
+        self._object_path = dbus.ObjectPath(_BUDDY_PATH + object_id)
 
         self._activities = {}   # Activity ID -> Activity
         self._activity_sigids = {}
@@ -130,9 +130,6 @@ class Buddy(ExportedGObject):
         self._color = None
         self._ip4_address = None
 
-        if not kwargs.get(_PROP_KEY):
-            raise ValueError("key required")
-
         _ALLOWED_INIT_PROPS = [_PROP_NICK, _PROP_KEY, _PROP_ICON,
                                _PROP_CURACT, _PROP_COLOR, _PROP_IP4_ADDRESS]
         for (key, value) in kwargs.items():
@@ -158,7 +155,9 @@ class Buddy(ExportedGObject):
 
         pspec -- property specifier with a "name" attribute
         """
-        if pspec.name == _PROP_KEY:
+        if pspec.name == _PROP_OBJID:
+            return self._object_id
+        elif pspec.name == _PROP_KEY:
             return self._key
         elif pspec.name == _PROP_ICON:
             return self._icon
@@ -422,32 +421,40 @@ class Buddy(ExportedGObject):
         """
         changed = False
         changed_props = {}
-        if _PROP_NICK in properties.keys():
+        if _PROP_NICK in properties:
             nick = properties[_PROP_NICK]
             if nick != self._nick:
                 self._nick = nick
                 changed_props[_PROP_NICK] = nick
                 changed = True
-        if _PROP_COLOR in properties.keys():
+        if _PROP_COLOR in properties:
             color = properties[_PROP_COLOR]
             if color != self._color:
                 self._color = color
                 changed_props[_PROP_COLOR] = color
                 changed = True
-        if _PROP_CURACT in properties.keys():
+        if _PROP_CURACT in properties:
             curact = properties[_PROP_CURACT]
             if curact != self._current_activity:
                 self._current_activity = curact
                 changed_props[_PROP_CURACT] = curact
                 changed = True
-        if _PROP_IP4_ADDRESS in properties.keys():
+        if _PROP_IP4_ADDRESS in properties:
             ip4addr = properties[_PROP_IP4_ADDRESS]
             if ip4addr != self._ip4_address:
                 self._ip4_address = ip4addr
                 changed_props[_PROP_IP4_ADDRESS] = ip4addr
                 changed = True
+        if _PROP_KEY in properties:
+            # don't allow key to be set more than once
+            if self._key is None:
+                key = properties[_PROP_KEY]
+                if key is not None:
+                    self._key = key
+                    changed_props[_PROP_KEY] = key
+                    changed = True
 
-        if not changed or not len(changed_props.keys()):
+        if not changed or not changed_props:
             return
 
         # Try emitting PropertyChanged before updating validity
@@ -558,13 +565,11 @@ class ShellOwner(GenericOwner):
     _SHELL_OWNER_INTERFACE = "org.laptop.Shell.Owner"
     _SHELL_PATH = "/org/laptop/Shell"
 
-    def __init__(self, ps, bus, object_id, test=False):
+    def __init__(self, ps, bus):
         """Initialize the ShellOwner instance
 
         ps -- presenceservice.PresenceService object
         bus -- a connection to the D-Bus session bus
-        object_id -- the activity's unique identifier
-        test -- ignored
 
         Retrieves initial property values from the profile
         module.  Loads the buddy icon from file as well.
@@ -584,8 +589,9 @@ class ShellOwner(GenericOwner):
         icon = f.read()
         f.close()
 
-        GenericOwner.__init__(self, ps, bus, object_id, key=key,
+        GenericOwner.__init__(self, ps, bus,
-                nick=nick, color=color, icon=icon, server=server,
+                'keyid/' + psutils.pubkey_to_keyid(key),
+                key=key, nick=nick, color=color, icon=icon, server=server,
                 key_hash=key_hash, registered=registered)
 
         # Ask to get notifications on Owner object property changes in the

services/presence/buddyiconcache.py

@@ -16,7 +16,6 @@
 # Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
 
 from sugar import env
-from sugar import util
 
 import os.path
 import cPickle

services/presence/presenceservice.py

@@ -1,4 +1,5 @@
 # Copyright (C) 2007, Red Hat, Inc.
+# Copyright (C) 2007 Collabora Ltd. <http://www.collabora.co.uk/>
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -14,25 +15,27 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
 
-import gobject
+import logging
+from weakref import WeakValueDictionary
+
 import dbus
 import dbus.service
+import gobject
 from dbus.gobject_service import ExportedGObject
 from dbus.mainloop.glib import DBusGMainLoop
-import logging
-
 from telepathy.client import ManagerRegistry, Connection
 from telepathy.interfaces import (CONN_MGR_INTERFACE, CONN_INTERFACE)
 from telepathy.constants import (CONNECTION_STATUS_CONNECTING,
     CONNECTION_STATUS_CONNECTED,
     CONNECTION_STATUS_DISCONNECTED)
 
-from server_plugin import ServerPlugin
-from linklocal_plugin import LinkLocalPlugin
 from sugar import util
 
+from server_plugin import ServerPlugin
+from linklocal_plugin import LinkLocalPlugin
 from buddy import Buddy, ShellOwner
 from activity import Activity
+from psutils import pubkey_to_keyid
 
 _PRESENCE_SERVICE = "org.laptop.Sugar.Presence"
 _PRESENCE_INTERFACE = "org.laptop.Sugar.Presence"
@@ -57,15 +60,26 @@ class PresenceService(ExportedGObject):
 
     def _create_owner(self):
         # Overridden by TestPresenceService
-        return ShellOwner(self, self._session_bus, self._get_next_object_id())
+        return ShellOwner(self, self._session_bus)
 
     def __init__(self):
         self._next_object_id = 0
         self._connected = False
 
-        self._buddies = {}      # key -> Buddy
+        # all Buddy objects
-        self._handles_buddies = {}      # tp client -> (handle -> Buddy)
+        # identifier -> Buddy, GC'd when no more refs exist
-        self._activities = {}   # activity id -> Activity
+        self._buddies = WeakValueDictionary()
+
+        # the online buddies for whom we know the full public key
+        # base64 public key -> Buddy
+        self._buddies_by_pubkey = {}
+
+        # The online buddies (those who're available via some CM)
+        # TP plugin -> (handle -> Buddy)
+        self._handles_buddies = {}
+
+        # activity id -> Activity
+        self._activities = {}
 
         self._session_bus = dbus.SessionBus()
         self._session_bus.add_signal_receiver(self._connection_disconnected_cb,
@@ -74,7 +88,10 @@ class PresenceService(ExportedGObject):
 
         # Create the Owner object
         self._owner = self._create_owner()
-        self._buddies[self._owner.props.key] = self._owner
+        key = self._owner.props.key
+        keyid = pubkey_to_keyid(key)
+        self._buddies['keyid/' + keyid] = self._owner
+        self._buddies_by_pubkey[key] = self._owner
 
         self._registry = ManagerRegistry()
         self._registry.LoadManagers()
@@ -133,49 +150,50 @@ class PresenceService(ExportedGObject):
         if self._connected != old_status:
             self.emit('connection-status', self._connected)
 
-    def _contact_online(self, tp, handle, props):
+    def get_buddy(self, objid):
-        new_buddy = False
+        buddy = self._buddies.get(objid)
-        key = props["key"]
+        if buddy is None:
-        buddy = self._buddies.get(key)
+            _logger.debug('Creating new buddy at .../%s', objid)
-        if not buddy:
             # we don't know yet this buddy
-            objid = self._get_next_object_id()
+            buddy = Buddy(self._session_bus, objid)
-            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
+            self._buddies[objid] = buddy
+        return buddy
+
+    def _contact_online(self, tp, objid, handle, props):
+        _logger.debug('Handle %u, .../%s is now online', handle, objid)
+        buddy = self.get_buddy(objid)
 
         self._handles_buddies[tp][handle] = buddy
         # store the handle of the buddy for this CM
         buddy.add_telepathy_handle(tp, handle)
-
         buddy.set_properties(props)
 
     def _buddy_validity_changed_cb(self, buddy, valid):
         if valid:
             self.BuddyAppeared(buddy.object_path())
+            self._buddies_by_pubkey[buddy.props.key] = buddy
             _logger.debug("New Buddy: %s (%s)", buddy.props.nick,
                           buddy.props.color)
         else:
             self.BuddyDisappeared(buddy.object_path())
+            self._buddies_by_pubkey.pop(buddy.props.key, None)
             _logger.debug("Buddy left: %s (%s)", buddy.props.nick,
                           buddy.props.color)
 
     def _buddy_disappeared_cb(self, buddy):
         if buddy.props.valid:
-            self.BuddyDisappeared(buddy.object_path())
+            self._buddy_validity_changed_cb(buddy, False)
-            _logger.debug('Buddy left: %s (%s)', buddy.props.nick,
-                          buddy.props.color)
-        self._buddies.pop(buddy.props.key)
 
     def _contact_offline(self, tp, handle):
         if not self._handles_buddies[tp].has_key(handle):
             return
 
         buddy = self._handles_buddies[tp].pop(handle)
-        key = buddy.props.key
-
         # the handle of the buddy for this CM is not valid anymore
+        # (this might trigger _buddy_disappeared_cb if they are not visible
+        # via any CM)
         buddy.remove_telepathy_handle(tp, handle)
 
     def _get_next_object_id(self):
@@ -199,7 +217,8 @@ class PresenceService(ExportedGObject):
     def _new_activity(self, activity_id, tp):
         try:
             objid = self._get_next_object_id()
-            activity = Activity(self._session_bus, objid, tp, id=activity_id)
+            activity = Activity(self._session_bus, objid, self, tp,
+                                id=activity_id)
         except Exception:
             # FIXME: catching bare Exception considered harmful
             _logger.debug("Invalid activity:", exc_info=1)
@@ -207,11 +226,12 @@ class PresenceService(ExportedGObject):
 
         activity.connect("validity-changed",
                          self._activity_validity_changed_cb)
+        activity.connect("disappeared", self._activity_disappeared_cb)
         self._activities[activity_id] = activity
         return activity
 
-    def _remove_activity(self, activity):
+    def _activity_disappeared_cb(self, activity):
-        _logger.debug("remove activity %s" % activity.props.id)
+        _logger.debug("activity %s disappeared" % activity.props.id)
 
         self.ActivityDisappeared(activity.object_path())
         del self._activities[activity.props.id]
@@ -246,8 +266,7 @@ class PresenceService(ExportedGObject):
                 activity = self._new_activity(act, tp)
 
             if activity is not None:
-                activity.buddy_joined(buddy)
+                activity.buddy_apparently_joined(buddy)
-                buddy.add_activity(activity)
 
         activities_left = old_activities - new_activities
         for act in activities_left:
@@ -256,11 +275,7 @@ class PresenceService(ExportedGObject):
             if not activity:
                 continue
 
-            activity.buddy_left(buddy)
+            activity.buddy_apparently_left(buddy)
-            buddy.remove_activity(activity)
-
-            if not activity.get_joined_buddies():
-                self._remove_activity(activity)
 
     def _activity_invitation(self, tp, act_id):
         activity = self._activities.get(act_id)
@@ -316,18 +331,31 @@ class PresenceService(ExportedGObject):
     @dbus.service.method(_PRESENCE_INTERFACE, in_signature='',
                          out_signature="ao")
     def GetBuddies(self):
-        ret = []
+        # in the presence of an out_signature, dbus-python will convert
-        for buddy in self._buddies.values():
+        # this set into an Array automatically (because it's iterable),
-            if buddy.props.valid:
+        # so it's easy to use for uniquification (we want to avoid returning
-                ret.append(buddy.object_path())
+        # buddies who're visible on both Salut and Gabble twice)
+
+        # always include myself even if I have no handles
+        ret = set((self._owner,))
+
+        for handles_buddies in self._handles_buddies.itervalues():
+            for buddy in handles_buddies.itervalues():
+                if buddy.props.valid:
+                    ret.add(buddy.object_path())
         return ret
 
     @dbus.service.method(_PRESENCE_INTERFACE,
                          in_signature="ay", out_signature="o",
                          byte_arrays=True)
     def GetBuddyByPublicKey(self, key):
-        if self._buddies.has_key(key):
+        buddy = self._buddies_by_pubkey.get(key)
-            buddy = self._buddies[key]
+        if buddy is not None:
+            if buddy.props.valid:
+                return buddy.object_path()
+        keyid = pubkey_to_keyid(key)
+        buddy = self._buddies.get('keyid/' + keyid)
+        if buddy is not None:
             if buddy.props.valid:
                 return buddy.object_path()
         raise NotFoundError("The buddy was not found.")
@@ -368,6 +396,48 @@ class PresenceService(ExportedGObject):
                             "connection to %s:%s" % (handle, tp_conn_name,
                                                      tp_conn_path))
 
+    def map_handles_to_buddies(self, tp, tp_chan, handles, create=True):
+        """
+
+        :Parameters:
+            `tp` : Telepathy plugin
+                The server or link-local plugin
+            `tp_chan` : telepathy.client.Channel or None
+                If not None, the channel in which these handles are
+                channel-specific
+            `handles` : iterable over int or long
+                The handles to be mapped to Buddy objects
+            `create` : bool
+                If true (default), if a corresponding `Buddy` object is not
+                found, create one.
+        :Returns:
+            A dict mapping handles from `handles` to `Buddy` objects.
+            If `create` is true, the dict's keys will be exactly the
+            items of `handles` in some order. If `create` is false,
+            the dict will contain no entry for handles for which no
+            `Buddy` is already available.
+        :Raises LookupError: if `tp` is not a plugin attached to this PS.
+        """
+        handle_to_buddy = self._handles_buddies[tp]
+
+        ret = {}
+        missing = []
+        for handle in handles:
+            buddy = handle_to_buddy.get(handle)
+            if buddy is None:
+                missing.append(handle)
+            else:
+                ret[handle] = buddy
+
+        if missing and create:
+            handle_to_objid = tp.identify_contacts(tp_chan, missing)
+            for handle, objid in handle_to_objid.iteritems():
+                buddy = self.get_buddy(objid)
+                ret[handle] = buddy
+                if tp_chan is None:
+                    handle_to_buddy[handle] = buddy
+        return ret
+
     @dbus.service.method(_PRESENCE_INTERFACE,
                          in_signature='', out_signature="o")
     def GetOwner(self):
@@ -397,9 +467,9 @@ 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._session_bus, objid, self._server_plugin,
+        activity = Activity(self._session_bus, objid, self,
-                            id=actid, type=atype, name=name, color=color,
+                            self._server_plugin, id=actid, type=atype,
-                            local=True)
+                            name=name, color=color, local=True)
         activity.connect("validity-changed",
                          self._activity_validity_changed_cb)
         self._activities[actid] = activity

services/presence/pstest.py

@@ -26,6 +26,7 @@ from sugar import env, util
 
 from buddy import GenericOwner, _PROP_NICK, _PROP_CURACT, _PROP_COLOR
 from presenceservice import PresenceService
+from psutils import pubkey_to_keyid
 
 
 _logger = logging.getLogger('s-p-s.pstest')
@@ -37,7 +38,7 @@ class TestOwner(GenericOwner):
 
     __gtype_name__ = "TestOwner"
 
-    def __init__(self, ps, bus, object_id, test_num, randomize):
+    def __init__(self, ps, bus, test_num, randomize):
         self._cp = ConfigParser()
         self._section = "Info"
         self._test_activities = []
@@ -62,8 +63,10 @@ class TestOwner(GenericOwner):
         icon = _get_random_image()
 
         _logger.debug("pubkey is %s" % pubkey)
-        GenericOwner.__init__(self, ps, bus, object_id, key=pubkey, nick=nick,
+        GenericOwner.__init__(self, ps, bus,
-                color=color, icon=icon, registered=registered, key_hash=privkey_hash)
+                'keyid/' + pubkey_to_keyid(pubkey),
+                key=pubkey, nick=nick, color=color, icon=icon,
+                registered=registered, key_hash=privkey_hash)
 
         # Only do the random stuff if randomize is true
         if randomize:
@@ -169,7 +172,7 @@ class TestPresenceService(PresenceService):
         PresenceService.__init__(self)
 
     def _create_owner(self):
-        return TestOwner(self, self._session_bus, self._get_next_object_id(),
+        return TestOwner(self, self._session_bus,
                          self.__test_num, self.__randomize)
 
     def internal_get_activity(self, actid):

services/presence/psutils.py

@@ -17,12 +17,15 @@
 
 import logging
 from string import ascii_letters, digits
+try:
+    from hashlib import sha1
+except ImportError:
+    # Python < 2.5
+    from sha import new as sha1
 
 import dbus
 import gobject
 
-from sugar import util
-
 
 _logger = logging.getLogger('s-p-s.psutils')
 
@@ -39,7 +42,7 @@ def pubkey_to_keyid(key):
     :Returns:
         The key ID as a string of hex digits
     """
-    return util.printable_hash(util._sha_data(key))
+    return sha1(key).hexdigest()
 
 
 def escape_identifier(identifier):

services/presence/server_plugin.py

@@ -20,6 +20,7 @@
 import logging
 import os
 import sys
+from string import hexdigits
 try:
     # Python >= 2.5
     from hashlib import md5
@@ -42,6 +43,7 @@ from telepathy.constants import (HANDLE_TYPE_CONTACT,
     CONNECTION_STATUS_CONNECTING,
     CONNECTION_STATUS_REASON_AUTHENTICATION_FAILED,
     CONNECTION_STATUS_REASON_NONE_SPECIFIED,
+    CHANNEL_GROUP_FLAG_CHANNEL_SPECIFIC_HANDLES,
     PROPERTY_FLAG_WRITE)
 from sugar import util
 
@@ -105,8 +107,11 @@ class ServerPlugin(gobject.GObject):
         'contact-online':
             # Contact has come online and we've discovered all their buddy
             # properties.
-            # args: contact handle: int; dict {name: str => property: object}
+            # args:
-            (gobject.SIGNAL_RUN_FIRST, None, [object, object]),
+            #   contact identification (based on key ID or JID): str
+            #   contact handle: int or long
+            #   dict {name: str => property: object}
+            (gobject.SIGNAL_RUN_FIRST, None, [str, object, object]),
         'contact-offline':
             # Contact has gone offline.
             # args: contact handle
@@ -263,7 +268,7 @@ class ServerPlugin(gobject.GObject):
 
         account_info['server'] = self._owner.get_server()
 
-        khash = util.printable_hash(util._sha_data(self._owner.props.key))
+        khash = psutils.pubkey_to_keyid(self._owner.props.key)
         account_info['account'] = "%s@%s" % (khash, account_info['server'])
 
         account_info['password'] = self._owner.get_key_hash()
@@ -770,10 +775,13 @@ class ServerPlugin(gobject.GObject):
             return
 
         props['nick'] = aliases[0]
+
         jid = self._conn[CONN_INTERFACE].InspectHandles(HANDLE_TYPE_CONTACT,
                                                         [handle])[0]
         self._online_contacts[handle] = jid
-        self.emit("contact-online", handle, props)
+        objid = self.identify_contacts(None, [handle])[handle]
+
+        self.emit("contact-online", objid, handle, props)
 
         self._conn[CONN_INTERFACE_BUDDY_INFO].GetActivities(handle,
             reply_handler=lambda *args: self._contact_online_activities_cb(
@@ -841,7 +849,7 @@ class ServerPlugin(gobject.GObject):
                 handle not in self._subscribe_local_pending and
                 handle not in self._subscribe_remote_pending):
             # it's probably a channel-specific handle - can't create a Buddy
-            # object
+            # object for those yet
             return
 
         self._online_contacts[handle] = None
@@ -855,8 +863,9 @@ class ServerPlugin(gobject.GObject):
 
         self._contact_online_request_properties(handle, 1)
 
-    def _subscribe_members_changed_cb(self, added, removed, local_pending,
+    def _subscribe_members_changed_cb(self, message, added, removed,
-            remote_pending, actor, reason):
+                                      local_pending, remote_pending,
+                                      actor, reason):
 
         added = set(added)
         removed = set(removed)
@@ -1063,3 +1072,100 @@ class ServerPlugin(gobject.GObject):
             if room == act_handle:
                 self.emit("activity-properties-changed", act_id, 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
+        register JIDs whose username part is either a public key fingerprint,
+        or of the wrong form to be a public key fingerprint (to allow for
+        ejabberd's admin@example.com address).
+
+        If we trust the server, we can skip verifying the key ourselves,
+        which leads to simplifications. In the current implementation we
+        never verify that people actually own the key they claim to, so
+        we will always give contacts on untrusted servers a JID- rather than
+        key-based identity.
+
+        For the moment we assume that the test server, olpc.collabora.co.uk,
+        does this verification.
+        """
+        return (hostname == 'olpc.collabora.co.uk')
+
+    def identify_contacts(self, tp_chan, handles):
+        """Work out the "best" unique identifier we can for the given handles,
+        in the context of the given channel (which may be None), using only
+        'fast' connection manager API (that does not involve network
+        round-trips).
+
+        For the XMPP server case, we proceed as follows:
+
+        * Find the owners of the given handles, if the channel has
+          channel-specific handles
+        * If the owner (globally-valid JID) is on a trusted server, return
+          'keyid/' plus the 'key fingerprint' (the user part of their JID,
+          currently implemented as the SHA-1 of the Base64 blob in
+          owner.key.pub)
+        * If the owner (globally-valid JID) cannot be found or is on an
+          untrusted server, return 'xmpp/' plus an escaped form of the JID
+
+        The idea is that we identify buddies by key-ID (i.e. by key, assuming
+        no collisions) if we can find it without making network round-trips,
+        but if that's not possible we just use their JIDs.
+
+        :Parameters:
+            `tp_chan` : telepathy.client.Channel or None
+                The channel in which the handles were found, or None if they
+                are known to be channel-specific handles
+            `handles` : iterable over (int or long)
+                The contacts' handles in that channel
+        :Returns:
+            A dict mapping the provided handles to the best available
+            unique identifier, which is a string that could be used as a
+            suffix to an object path
+        """
+        # we need to be able to index into handles, so force them to
+        # be a sequence
+        if not isinstance(handles, (tuple, list)):
+            handles = tuple(handles)
+
+        owners = handles
+
+        if tp_chan is not None and CHANNEL_INTERFACE_GROUP in tp_chan:
+            group = tp_chan[CHANNEL_INTERFACE_GROUP]
+            if (group.GetGroupFlags() &
+                CHANNEL_GROUP_FLAG_CHANNEL_SPECIFIC_HANDLES):
+                owners = group.GetHandleOwners(handles)
+                for i, owner in enumerate(owners):
+                    if owner == 0:
+                        owners[i] = handles[i]
+        else:
+            group = None
+
+        jids = self._conn[CONN_INTERFACE].InspectHandles(HANDLE_TYPE_CONTACT,
+                                                         owners)
+
+        ret = {}
+        for handle, jid in zip(handles, jids):
+            # special-case the Owner - we always know who we are
+            if (handle == self.self_handle or
+                (group is not None and handle == group.GetSelfHandle())):
+                ret[handle] = self._owner.props.objid
+                continue
+
+            if '/' in jid:
+                # the contact is unidentifiable (in an anonymous MUC) - create
+                # a temporary identity for them, based on their room-JID
+                ret[handle] = 'xmpp/' + psutils.escape_identifier(jid)
+            else:
+                user, host = jid.split('@', 1)
+                if (self._server_is_trusted(host) and len(user) == 40 and
+                    user.strip(hexdigits) == ''):
+                    # they're on a trusted server and their username looks
+                    # like a key-ID
+                    ret[handle] = 'keyid/' + user.lower()
+                else:
+                    # untrusted server, or not the right format to be a
+                    # key-ID - identify the contact by their JID
+                    ret[handle] = 'xmpp/' + psutils.escape_identifier(jid)
+
+        return ret

services/presence/test_psutils.py

@@ -1,4 +1,8 @@
-from psutils import escape_identifier
+print "Running test_psutils..."
+
+from psutils import escape_identifier, pubkey_to_keyid
+
+assert pubkey_to_keyid('abc') == 'a9993e364706816aba3e25717850c26c9cd0d89d'
 
 assert escape_identifier('') == '_'
 assert escape_identifier('_') == '_5f'