Philipp Hörist pushed to branch master at gajim / gajim

Commits: f7f8b8b0 by lovetox at 2022-08-14T13:26:11+02:00 cfix: Contacts: Fix logic error when calling add_private_contact()

- - - - - a706dfc6 by lovetox at 2022-08-14T13:28:35+02:00 perf: Use Task Queue to request avatars on MUC join

Previously we requested all avatars at once. On bigger MUCs this can lead to other network tasks, like joining other MUCs, executing more slowly.

- - - - - 33d27496 by lovetox at 2022-08-14T13:28:58+02:00 cq: TaskManager: Add comment

- - - - -

3 changed files:

- gajim/common/modules/contacts.py - gajim/common/modules/vcard_avatars.py - gajim/common/task_manager.py

Changes:

===================================== gajim/common/modules/contacts.py ===================================== @@ -129,7 +129,7 @@ def add_chat_contact(self, jid: Union[str, JID]) -> BareContact: contact = self._contacts.get(jid) if contact is not None: if not isinstance(contact, BareContact): - raise ValueError(f'Trying to add GroupchatContact {jid}, ' + raise ValueError(f'Trying to add BareContact {jid}, ' f'but contact exists already as {contact}') return contact

@@ -162,14 +162,14 @@ def add_private_contact(self, jid: Union[str, JID]) -> GroupchatParticipant: raise ValueError(f'Trying to add a bare JID as private {jid}')

contact = self._contacts.get(jid.bare) - if not isinstance(contact, GroupchatContact): - raise ValueError(f'Trying to add GroupchatParticipant {jid}, ' - f'to BareContact {contact}') - if contact is None: group_chat_contact = self.add_group_chat_contact(jid.bare) return group_chat_contact.add_resource(jid.resource)

+ if not isinstance(contact, GroupchatContact): + raise ValueError(f'Trying to add GroupchatParticipant {jid}, ' + f'to BareContact {contact}') + return contact.add_resource(jid.resource)

def add_contact(self,

===================================== gajim/common/modules/vcard_avatars.py ===================================== @@ -16,9 +16,13 @@

from __future__ import annotations

+from typing import Any +from typing import Callable from typing import Generator from typing import Optional

+import weakref + from nbxmpp.const import AvatarState from nbxmpp.modules.util import is_error from nbxmpp.modules.vcard_temp import VCard @@ -31,8 +35,12 @@

from gajim.common import app from gajim.common import types +from gajim.common.task_manager import Task from gajim.common.modules.base import BaseModule from gajim.common.modules.util import as_task +from gajim.common.modules.contacts import GroupchatContact +from gajim.common.modules.contacts import GroupchatParticipant +from gajim.common.modules.contacts import BareContact

class VCardAvatars(BaseModule): @@ -62,13 +70,13 @@ def get_avatar_sha(self, jid: JID) -> Optional[str]:

@as_task def _request_vcard(self, - jid: JID, - expected_sha: str, - type_: str + contact: types.ChatContactT, + expected_sha: str ) -> Generator[VCard, None, None]: _task = yield # noqa: F841

- vcard = yield self._con.get_module('VCardTemp').request_vcard(jid=jid) + vcard = yield self._con.get_module('VCardTemp').request_vcard( + jid=contact.jid)

if is_error(vcard): self._log.warning(vcard) @@ -76,29 +84,27 @@ def _request_vcard(self,

avatar, avatar_sha = vcard.get_avatar() if avatar is None: - self._log.info('Avatar missing: %s %s', jid, expected_sha) + self._log.info('Avatar missing: %s %s', contact.jid, expected_sha) return

if expected_sha != avatar_sha: self._log.warning('Avatar mismatch: %s %s != %s', - jid, + contact.jid, expected_sha, avatar_sha) return

- self._log.info('Received: %s %s', jid, avatar_sha) + self._log.info('Received: %s %s', contact.jid, avatar_sha) app.interface.save_avatar(avatar)

- contact = self._con.get_module('Contacts').get_contact(jid) - - if type_ == 'contact': - app.storage.cache.set_contact(jid, 'avatar', avatar_sha) + if isinstance(contact, BareContact): + app.storage.cache.set_contact(contact.jid, 'avatar', avatar_sha)

- elif type_ == 'muc': - app.storage.cache.set_muc(jid, 'avatar', avatar_sha) + elif isinstance(contact, GroupchatContact): + app.storage.cache.set_muc(contact.jid, 'avatar', avatar_sha)

- elif type_ == 'muc-user': - self._muc_avatar_cache[jid] = avatar_sha + else: + self._muc_avatar_cache[contact.jid] = avatar_sha

contact.update_avatar(avatar_sha)

@@ -180,10 +186,11 @@ def _process_update(self,

if avatar_sha not in self._requested_shas: self._requested_shas.append(avatar_sha) - if groupchat: - self._request_vcard(jid, avatar_sha, 'muc') - else: - self._request_vcard(jid, avatar_sha, 'contact') + + task = VCardAvatarsTask(contact, + avatar_sha, + self._request_vcard) + app.task_manager.add_task(task)

def _muc_update_received(self, properties: PresenceProperties) -> None: contact = self._con.get_module('Contacts').get_contact(properties.jid, @@ -202,9 +209,11 @@ def _muc_update_received(self, properties: PresenceProperties) -> None: if properties.avatar_sha not in self._requested_shas: app.log('avatar').info('Request: %s', nick) self._requested_shas.append(properties.avatar_sha) - self._request_vcard(properties.jid, - properties.avatar_sha, - 'muc-user') + + task = VCardAvatarsTask(contact, + properties.avatar_sha, + self._request_vcard) + app.task_manager.add_task(task) return

current_avatar_sha = self._muc_avatar_cache.get(properties.jid) @@ -216,3 +225,43 @@ def _muc_update_received(self, properties: PresenceProperties) -> None:

else: self._log.info('Avatar already known: %s', nick) + + +class VCardAvatarsTask(Task): + def __init__(self, + contact: Any, + sha: str, + callback: Callable[..., Any] + ) -> None: + + Task.__init__(self) + self._contact = contact + self._sha = sha + self._callback = weakref.WeakMethod(callback) + + def execute(self) -> None: + callback = self._callback() + if callback is not None: + callback(self._contact, self._sha) + + def preconditions_met(self) -> bool: + try: + client = app.get_client(self._contact.account) + except Exception: + return False + + if not client.state.is_available: + return False + + elif isinstance(self._contact, GroupchatParticipant): + if not self._contact.room.is_joined: + self.set_obsolete() + return False + + return True + + def __repr__(self) -> str: + return f'VCardAvatars ({self._contact.jid} {self._sha})' + + def __hash__(self) -> int: + return hash((self._contact, self._sha))

===================================== gajim/common/task_manager.py ===================================== @@ -43,6 +43,8 @@ def _process_queue(self) -> bool: continue

if not task.preconditions_met(): + # precondition_met() can change the obsolete flag, so we need + # to check again here if task.is_obsolete(): log.info('Task obsolete: %r', task) else:

View it on GitLab: https://dev.gajim.org/gajim/gajim/-/compare/09731d2fb45e931f012d2387fbbc6e47...