changeset 3685:36849fb5c854

plugin XEP-0054: fix wrongly used `await`: `_checkAvatarHash` was incorrectly awaited. This method is only called once, the code has been moved directly to `update`.
author Goffi <goffi@goffi.org>
date Thu, 30 Sep 2021 10:30:43 +0200
parents ba4ef64a6938
children fc209014524a
files sat/plugins/plugin_xep_0054.py
diffstat 1 files changed, 25 insertions(+), 34 deletions(-) [+]
line wrap: on
line diff
--- a/sat/plugins/plugin_xep_0054.py	Sun Sep 26 16:38:00 2021 +0200
+++ b/sat/plugins/plugin_xep_0054.py	Thu Sep 30 10:30:43 2021 +0200
@@ -45,10 +45,7 @@
         "Missing module pillow, please download/install it from https://python-pillow.github.io"
     )
 
-try:
-    from twisted.words.protocols.xmlstream import XMPPHandler
-except ImportError:
-    from wokkel.subprotocols import XMPPHandler
+from twisted.words.protocols.jabber.xmlstream import XMPPHandler
 
 IMPORT_NAME = "XEP-0054"
 
@@ -109,7 +106,7 @@
             NS_VCARD, client.profile)
         await client._xep_0054_avatar_hashes.load()
 
-    def savePhoto(self, client, photo_elt, entity_jid):
+    def savePhoto(self, client, photo_elt, entity):
         """Parse a <PHOTO> photo_elt and save the picture"""
         # XXX: this method is launched in a separate thread
         try:
@@ -131,7 +128,7 @@
             raise Failure(exceptions.NotFound())
 
         if not buf:
-            log.warning("empty avatar for {jid}".format(jid=entity_jid.full()))
+            log.warning("empty avatar for {jid}".format(jid=entity.full()))
             raise Failure(exceptions.NotFound())
 
         log.debug(_("Decoding binary"))
@@ -140,7 +137,7 @@
 
         if mime_type is None:
             log.debug(
-                f"no media type found specified for {entity_jid}'s avatar, trying to "
+                f"no media type found specified for {entity}'s avatar, trying to "
                 f"guess")
 
             try:
@@ -149,7 +146,7 @@
                 log.warning(f"Can't open avatar buffer: {e}")
 
             if mime_type is None:
-                msg = f"Can't find media type for {entity_jid}'s avatar"
+                msg = f"Can't find media type for {entity}'s avatar"
                 log.warning(msg)
                 raise Failure(exceptions.DataError(msg))
 
@@ -435,19 +432,6 @@
     def getDiscoItems(self, requestor, target, nodeIdentifier=""):
         return []
 
-    def _checkAvatarHash(self, client, entity, given_hash):
-        """Check that hash in cache (i.e. computed hash) is the same as given one"""
-        # XXX: if they differ, the avatar will be requested on each connection
-        # TODO: try to avoid re-requesting avatar in this case
-        computed_hash = client._xep_0054_avatar_hashes[entity.full()]
-        if computed_hash != given_hash:
-            log.warning(
-                "computed hash differs from given hash for {entity}:\n"
-                "computed: {computed}\ngiven: {given}".format(
-                    entity=entity, computed=computed_hash, given=given_hash
-                )
-            )
-
     async def update(self, presence):
         """Called on <presence/> stanza with vcard data
 
@@ -468,19 +452,19 @@
         except StopIteration:
             return
 
-        new_hash = str(photo_elt).strip()
-        if new_hash == HASH_SHA1_EMPTY:
-            new_hash = ""
+        given_hash = str(photo_elt).strip()
+        if given_hash == HASH_SHA1_EMPTY:
+            given_hash = ""
 
         hashes_cache = client._xep_0054_avatar_hashes
 
         old_hash = hashes_cache.get(entity_jid.full())
 
-        if old_hash == new_hash:
+        if old_hash == given_hash:
             # no change, we can return…
-            if new_hash:
+            if given_hash:
                 # …but we double check that avatar is in cache
-                avatar_cache = self.host.common_cache.getMetadata(new_hash)
+                avatar_cache = self.host.common_cache.getMetadata(given_hash)
                 if avatar_cache is None:
                     log.debug(
                         f"Avatar for [{entity_jid}] is known but not in cache, we get "
@@ -492,19 +476,19 @@
                     log.debug(f"avatar for {entity_jid} is already in cache")
             return
 
-        if new_hash is None:
+        if given_hash is None:
             # XXX: we use empty string to indicate that there is no avatar
-            new_hash = ""
+            given_hash = ""
 
-        await hashes_cache.aset(entity_jid.full(), new_hash)
+        await hashes_cache.aset(entity_jid.full(), given_hash)
 
-        if not new_hash:
+        if not given_hash:
             await self.plugin_parent._i.update(
                 client, IMPORT_NAME, "avatar", None, entity_jid)
             # the avatar has been removed, no need to go further
             return
 
-        avatar_cache = self.host.common_cache.getMetadata(new_hash)
+        avatar_cache = self.host.common_cache.getMetadata(given_hash)
         if avatar_cache is not None:
             log.debug(
                 f"New avatar found for [{entity_jid}], it's already in cache, we use it"
@@ -516,7 +500,7 @@
                     'path': avatar_cache['path'],
                     'filename': avatar_cache['filename'],
                     'media_type': avatar_cache['mime_type'],
-                    'cache_uid': new_hash,
+                    'cache_uid': given_hash,
                 },
                 entity_jid
             )
@@ -528,7 +512,14 @@
             if vcard is None:
                 log.warning(f"Unexpected empty vCard for {entity_jid}")
                 return
-            await self._checkAvatarHash(client, entity_jid, new_hash)
+            computed_hash = client._xep_0054_avatar_hashes[entity_jid.full()]
+            if computed_hash != given_hash:
+                log.warning(
+                    "computed hash differs from given hash for {entity}:\n"
+                    "computed: {computed}\ngiven: {given}".format(
+                        entity=entity_jid, computed=computed_hash, given=given_hash
+                    )
+                )
 
     def _update(self, presence):
         defer.ensureDeferred(self.update(presence))