diff src/plugins/plugin_xep_0054.py @ 1293:0541cb64217e frontends_multi_profiles

plugin XEP-0054: couple of fixes in VCard/avatar management: - fixed a confusion between full jid and bare jid, resulting in always requesting VCard - check of cache data loading before starting - stored cache are restored for all jid, not only ones in roster - client jid cache is correctly saved/restored - minor other fixes This plugin still need some work, but this patch fixes (hopefully) the major issues
author Goffi <goffi@goffi.org>
date Mon, 26 Jan 2015 02:03:16 +0100
parents faa1129559b8
children be3a301540c0
line wrap: on
line diff
--- a/src/plugins/plugin_xep_0054.py	Mon Jan 26 01:57:06 2015 +0100
+++ b/src/plugins/plugin_xep_0054.py	Mon Jan 26 02:03:16 2015 +0100
@@ -80,7 +80,7 @@
         if not os.path.exists(self.avatar_path):
             os.makedirs(self.avatar_path)
         self.avatars_cache = PersistentDict(NS_VCARD)
-        self.avatars_cache.load()  # FIXME: resulting deferred must be correctly managed
+        self.initialised = self.avatars_cache.load()  # FIXME: resulting deferred must be correctly managed
         host.bridge.addMethod("getCard", ".plugin", in_sign='ss', out_sign='s', method=self.getCard)
         host.bridge.addMethod("getAvatarFile", ".plugin", in_sign='s', out_sign='s', method=self.getAvatarFile)
         host.bridge.addMethod("setAvatar", ".plugin", in_sign='ss', out_sign='', method=self.setAvatar, async=True)
@@ -98,49 +98,49 @@
 
         return True
 
-    def _fillCachedValues(self, result, client):
+    def _fillCachedValues(self, profile):
         #FIXME: this is really suboptimal, need to be reworked
         #       the current naive approach keeps a map between all jids of all profiles
-        #       in persistent cache, and check if cached jid are in roster, then put avatar
-        #       hashs in memory.
-        for _jid in client.roster.getJids() + [client.jid]:
-            if _jid.userhost() in self.avatars_cache:
-                self.host.memory.updateEntityData(_jid, "avatar", self.avatars_cache[_jid.userhost()], client.profile)
+        #       in persistent cache, then put avatar
+        #       hashs in memory. Hashed should be shared between profiles
+        for jid_s, avatar_hash in self.avatars_cache.iteritems():
+            jid_ = jid.JID(jid_s)
+            self.host.memory.updateEntityData(jid_, "avatar", avatar_hash, profile)
 
+    @defer.inlineCallbacks
     def profileConnected(self, profile):
-        client = self.host.getClient(profile)
-        client.roster.got_roster.addCallback(self._fillCachedValues, client)
+        yield self.initialised
+        self._fillCachedValues(profile)
 
-    def update_cache(self, jid, name, value, profile):
+    def updateCache(self, jid_, name, value, profile):
         """update cache value
-        - save value in memory in case of change
-        @param jid: jid of the owner of the vcard
+
+        save value in memory in case of change
+        @param jid_: jid of the owner of the vcard
         @param name: name of the item which changed
         @param value: new value of the item
         @param profile: profile which received the update
         """
-        try:
-            cached = self.host.memory.getEntityData(jid, [name], profile)
-        except exceptions.UnknownEntityError:
-            cached = {}
-        if not name in cached or cached[name] != value:
-            self.host.memory.updateEntityData(jid, name, value, profile)
-            if name == "avatar":
-                self.avatars_cache[jid.userhost()] = value
+        assert not jid_.resource # VCard are retrieved with bare jid
+        self.host.memory.updateEntityData(jid_, name, value, profile)
+        if name == "avatar":
+            self.avatars_cache[jid_.userhost()] = value
 
-    def get_cache(self, entity_jid, name, profile):
+    def getCache(self, entity_jid, name, profile):
         """return cached value for jid
+
         @param entity_jid: target contact
         @param name: name of the value ('nick' or 'avatar')
         @param profile: %(doc_profile)s
         @return: wanted value or None"""
+        assert not entity_jid.resource # VCard are retrieved with bare jid
         try:
             data = self.host.memory.getEntityData(entity_jid, [name], profile)
         except exceptions.UnknownEntityError:
             return None
         return data.get(name)
 
-    def save_photo(self, photo_xml):
+    def savePhoto(self, photo_xml):
         """Parse a <PHOTO> elem and save the picture"""
         for elem in photo_xml.elements():
             if elem.name == 'TYPE':
@@ -169,7 +169,7 @@
                 dictionary['fullname'] = unicode(elem)
             elif elem.name == 'NICKNAME':
                 dictionary['nick'] = unicode(elem)
-                self.update_cache(target, 'nick', dictionary['nick'], profile)
+                self.updateCache(target, 'nick', dictionary['nick'], profile)
             elif elem.name == 'URL':
                 dictionary['website'] = unicode(elem)
             elif elem.name == 'EMAIL':
@@ -177,17 +177,17 @@
             elif elem.name == 'BDAY':
                 dictionary['birthday'] = unicode(elem)
             elif elem.name == 'PHOTO':
-                dictionary["avatar"] = yield threads.deferToThread(self.save_photo, elem)
+                dictionary["avatar"] = yield threads.deferToThread(self.savePhoto, elem)
                 if not dictionary["avatar"]:  # can happen in case of e.g. empty photo elem
                     del dictionary['avatar']
                 else:
-                    self.update_cache(target, 'avatar', dictionary['avatar'], profile)
+                    self.updateCache(target, 'avatar', dictionary['avatar'], profile)
             else:
                 log.info(_('FIXME: [%s] VCard tag is not managed yet') % elem.name)
 
         defer.returnValue(dictionary)
 
-    def vcard_ok(self, answer, profile):
+    def _VCardCb(self, answer, profile):
         """Called after the first get IQ"""
         log.debug(_("VCard found"))
 
@@ -203,13 +203,14 @@
             log.error(_("FIXME: vCard not found as first child element"))
             self.host.bridge.actionResult("SUPPRESS", answer['id'], {}, profile)  # FIXME: maybe an error message would be better
 
-    def vcard_err(self, failure, profile):
+    def _VCardEb(self, failure, profile):
         """Called when something is wrong with registration"""
         try:
-            log.error(_("Can't find VCard of %s") % failure.value.stanza['from'])
+            log.warning(_("Can't find VCard of %s") % failure.value.stanza['from'])
             self.host.bridge.actionResult("SUPPRESS", failure.value.stanza['id'], {}, profile)  # FIXME: maybe an error message would be better
+            self.updateCache(jid.JID(failure.value.stanza['from']), "avatar", '', profile)
         except AttributeError:  # 'ConnectionLost' object has no attribute 'stanza'
-            log.error(_("Can't find VCard: %s") % failure.getErrorMessage())
+            log.warning(_("Can't find VCard: %s") % failure.getErrorMessage())
 
     def getCard(self, target_s, profile_key=C.PROF_KEY_NONE):
         """Ask server for VCard
@@ -217,8 +218,7 @@
         @result: id to retrieve the profile"""
         current_jid, xmlstream = self.host.getJidNStream(profile_key)
         if not xmlstream:
-            log.error(_('Asking vcard for a non-existant or not connected profile'))
-            return ""
+            raise exceptions.ProfileUnknownError('Asking vcard for a non-existant or not connected profile ({})'.format(profile_key))
         profile = self.host.memory.getProfileName(profile_key)
         to_jid = jid.JID(target_s)
         log.debug(_("Asking for %s's VCard") % to_jid.userhost())
@@ -226,7 +226,7 @@
         reg_request["from"] = current_jid.full()
         reg_request["to"] = to_jid.userhost()
         reg_request.addElement('vCard', NS_VCARD)
-        reg_request.send(to_jid.userhost()).addCallbacks(self.vcard_ok, self.vcard_err, callbackArgs=[profile], errbackArgs=[profile])
+        reg_request.send(to_jid.userhost()).addCallbacks(self._VCardCb, self._VCardEb, callbackArgs=[profile], errbackArgs=[profile])
         return reg_request["id"]
 
     def getAvatarFile(self, avatar_hash):
@@ -282,9 +282,8 @@
         def elementBuilt(result):
             """Called once the image is at the right size/format, and the vcard set element is build"""
             set_avatar_elt, img_hash = result
-            self.avatars_cache[client.jid.userhost()] = img_hash  # we need to update the hash, so we can send a new presence
-                                                                 # element with the right hash
-            return set_avatar_elt.send().addCallback(lambda ignore: client.presence.available())
+            self.updateCache(client.jid.userhostJID(), 'avatar', img_hash, client.profile)
+            return set_avatar_elt.send().addCallback(lambda ignore: client.presence.available()) # FIXME: should send the current presence, not always "available" !
 
         d.addCallback(elementBuilt)
 
@@ -311,13 +310,16 @@
         """Request for VCard's nickname
         return the cached nickname if exists, else get VCard
         """
-        from_jid = jid.JID(presence['from'])
+        # FIXME: doesn't manage MUC correctly
+        from_jid = jid.JID(presence['from']).userhostJID()
         #FIXME: wokkel's data_form should be used here
         x_elem = filter(lambda x: x.name == "x", presence.elements())[0]  # We only want the "x" element
         for elem in x_elem.elements():
             if elem.name == 'photo':
                 _hash = str(elem)
-                old_avatar = self.plugin_parent.get_cache(from_jid, 'avatar', self.parent.profile)
+                old_avatar = self.plugin_parent.getCache(from_jid, 'avatar', self.parent.profile)
                 if not old_avatar or old_avatar != _hash:
                     log.debug(_('New avatar found, requesting vcard'))
                     self.plugin_parent.getCard(from_jid.userhost(), self.parent.profile)
+                else:
+                    log.debug("avatar for {} already in cache".format(from_jid))