# HG changeset patch # User Goffi # Date 1422234196 -3600 # Node ID 0541cb64217ed04308c3bda4de7b6740451481ff # Parent b29a065a66f0e8519ad533c941782db1fe75ac14 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 diff -r b29a065a66f0 -r 0541cb64217e src/plugins/plugin_xep_0054.py --- 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 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))