changeset 2617:81b70eeb710f

quick_frontend(contact list): refactored update: update is now called with appropriate constant value (C.UPDATE_ADD, C.UPDATE_DELETE, C.UPDATE_MODIFY and so on) when a widget change visibility according to current options. Before it was linked to cache only (C.UPDATE_ADD was only called when contact was first added to cache). This make widget handling in frontends more easy. Renamed entityToShow to entityVisible, which seems to correspond better. Started reducing lines lenght to 90 chars as a test. May become the new coding style soon.
author Goffi <goffi@goffi.org>
date Sun, 24 Jun 2018 21:59:29 +0200
parents 1cc88adb5142
children fe9888d3fcb6
files sat_frontends/primitivus/contact_list.py sat_frontends/quick_frontend/quick_contact_list.py
diffstat 2 files changed, 142 insertions(+), 59 deletions(-) [+]
line wrap: on
line diff
--- a/sat_frontends/primitivus/contact_list.py	Thu Jun 21 01:21:44 2018 +0200
+++ b/sat_frontends/primitivus/contact_list.py	Sun Jun 24 21:59:29 2018 +0200
@@ -231,7 +231,7 @@
         widgets = []  # list of built widgets
 
         for entity in entities:
-            if entity in self.contact_list._specials or not self.contact_list.entityToShow(entity):
+            if entity in self.contact_list._specials or not self.contact_list.entityVisible(entity):
                 continue
             markup_extra = []
             if self.contact_list.show_resources:
@@ -287,7 +287,7 @@
             data = self.contact_list.getGroupData(group)
             folded = data.get(C.GROUP_DATA_FOLDED, False)
             jids = list(data['jids'])
-            if group is not None and (self.contact_list.anyEntityToShow(jids) or self.contact_list.show_empty_groups):
+            if group is not None and (self.contact_list.anyEntityVisible(jids) or self.contact_list.show_empty_groups):
                 header = '[-]' if not folded else '[+]'
                 widget = sat_widgets.ClickableText(group, header=header + ' ')
                 content.append(widget)
--- a/sat_frontends/quick_frontend/quick_contact_list.py	Thu Jun 21 01:21:44 2018 +0200
+++ b/sat_frontends/quick_frontend/quick_contact_list.py	Sun Jun 24 21:59:29 2018 +0200
@@ -17,7 +17,8 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-"""Contact List handling multi profiles at once, should replace quick_contact_list module in the future"""
+"""Contact List handling multi profiles at once,
+    should replace quick_contact_list module in the future"""
 
 from sat.core.i18n import _
 from sat.core.log import getLogger
@@ -32,7 +33,8 @@
 try:
     # FIXME: to be removed when an acceptable solution is here
     unicode('')  # XXX: unicode doesn't exist in pyjamas
-except (TypeError, AttributeError):  # Error raised is not the same depending on pyjsbuild options
+except (TypeError, AttributeError):  # Error raised is not the same depending on
+                                     # pyjsbuild options
     # XXX: pyjamas' max doesn't support key argument, so we implement it ourself
     pyjamas_max = max
 
@@ -88,11 +90,24 @@
         self.show_empty_groups = True
         self.show_resources = False
         self.show_status = False
+        # do we show entities with notifications?
+        # if True, entities will be show even if they normally would not
+        # (e.g. not in contact list) if they have notifications attached
+        self.show_entities_with_notifs = True
 
-        self.host.bridge.asyncGetParamA(C.SHOW_EMPTY_GROUPS, "General", profile_key=profile, callback=self._showEmptyGroups)
-        self.host.bridge.asyncGetParamA(C.SHOW_OFFLINE_CONTACTS, "General", profile_key=profile, callback=self._showOfflineContacts)
+        self.host.bridge.asyncGetParamA(C.SHOW_EMPTY_GROUPS,
+                                        "General",
+                                        profile_key=profile,
+                                        callback=self._showEmptyGroups)
 
-        # FIXME: workaround for a pyjamas issue: calling hash on a class method always return a different value if that method is defined directly within the class (with the "def" keyword)
+        self.host.bridge.asyncGetParamA(C.SHOW_OFFLINE_CONTACTS,
+                                        "General",
+                                        profile_key=profile,
+                                        callback=self._showOfflineContacts)
+
+        # FIXME: workaround for a pyjamas issue: calling hash on a class method always
+        #        return a different value if that method is defined directly within the
+        #        class (with the "def" keyword)
         self.presenceListener = self.onPresenceUpdate
         self.host.addListener('presence', self.presenceListener, [self.profile])
         self.nickListener = self.onNickUpdate
@@ -116,12 +131,13 @@
         """Check if entity is in contact list
 
         An entity can be in contact list even if not in roster
-        @param entity (jid.JID): jid of the entity (resource is not ignored, use bare jid if needed)
+        @param entity (jid.JID): jid of the entity (resource is not ignored,
+            use bare jid if needed)
         """
         if entity.resource:
             try:
                 return entity.resource in self.getCache(entity.bare, C.CONTACT_RESOURCES)
-            except KeyError:
+            except exceptions.NotFound:
                 return False
         return entity in self._cache
 
@@ -139,7 +155,8 @@
 
         @return (set[jid.JID])
         """
-        return set([entity for entity in self._roster if self.getCache(entity, C.PRESENCE_SHOW) is not None])
+        return set([entity for entity in self._roster
+            if self.getCache(entity, C.PRESENCE_SHOW) is not None])
 
     @property
     def roster_entities_by_group(self):
@@ -186,7 +203,8 @@
         entities are not sorted
         key: bare jid, value: data
         """
-        return {jid_:cache for jid_, cache in self._cache.iteritems() if self.entityToShow(jid_)}
+        return {jid_:cache for jid_, cache in self._cache.iteritems()
+                if self.entityVisible(jid_)}
 
 
     def getItem(self, entity):
@@ -198,7 +216,11 @@
         return self._cache[entity]
 
     def _gotContacts(self, contacts):
-        """Called during filling, add contacts and notice parent that contacts are filled"""
+        """Add contacts and notice parent that contacts are filled
+
+        Called during initial contact list filling
+        @param contacts(tuple): all contacts
+        """
         for contact in contacts:
             self.host.newContactHandler(*contact, profile=self.profile)
         handler._contactsFilled(self.profile)
@@ -214,31 +236,43 @@
     def fill(self):
         handler.fill(self.profile)
 
-    def getCache(self, entity, name=None, bare_default=True):
+    def getCache(self, entity, name=None, bare_default=True, create_if_not_found=False):
         """Return a cache value for a contact
 
-        @param entity(jid.JID): entity of the contact from who we want data (resource is used if given)
+        @param entity(jid.JID): entity of the contact from who we want data
+            (resource is used if given)
             if a resource specific information is requested:
-                - if no resource is given (bare jid), the main resource is used, according to priority
+                - if no resource is given (bare jid), the main resource is used,
+                    according to priority
                 - if resource is given, it is used
         @param name(unicode): name the data to get, or None to get everything
-        @param bare_default(bool, None): if True and entity is a full jid, the value of bare jid
-            will be returned if not value is found for the requested resource.
+        @param bare_default(bool, None): if True and entity is a full jid,
+            the value of bare jid will be returned if not value is found for
+            the requested resource.
             If False, None is returned if no value is found for the requested resource.
             If None, bare_default will be set to False if entity is in a room, True else
+        @param create_if_not_found(bool): if True, create contact if it's not found
+            in cache
         @return: full cache if no name is given, or value of "name", or None
+        @raise NotFound: entity not found in cache
         """
         # FIXME: resource handling need to be reworked
-        # FIXME: bare_default work for requesting full jid to get bare jid, but not the other way
-        #        e.g.: if we have set an avatar for user@server.tld/resource and we request user@server.tld
+        # FIXME: bare_default work for requesting full jid to get bare jid,
+        #        but not the other way
+        #        e.g.: if we have set an avatar for user@server.tld/resource
+        #        and we request user@server.tld
         #        we won't get the avatar set in the resource
         try:
             cache = self._cache[entity.bare]
         except KeyError:
-            self.setContact(entity)
-            cache = self._cache[entity.bare]
+            if create_if_not_found:
+                self.setContact(entity)
+                cache = self._cache[entity.bare]
+            else:
+                raise exceptions.NotFound
 
         if name is None:
+            # full cache is requested
             return cache
 
         if name in ('status', C.PRESENCE_STATUSES, C.PRESENCE_PRIORITY, C.PRESENCE_SHOW):
@@ -337,7 +371,8 @@
 
         @param entity(jid.JID): jid of the special entity
             if the jid is full, will be added to special extras
-        @param special_type: one of special type (e.g. C.CONTACT_SPECIAL_GROUP) or None to remove special flag
+        @param special_type: one of special type (e.g. C.CONTACT_SPECIAL_GROUP)
+            or None to remove special flag
         """
         assert special_type in C.CONTACT_SPECIAL_ALLOWED + (None,)
         self.setCache(entity, C.CONTACT_SPECIAL, special_type)
@@ -345,14 +380,16 @@
     def getSpecials(self, special_type=None, bare=False):
         """Return all the bare JIDs of the special roster entities of with given type.
 
-        @param special_type(unicode, None): if not None, filter by special type (e.g. C.CONTACT_SPECIAL_GROUP)
+        @param special_type(unicode, None): if not None, filter by special type
+            (e.g. C.CONTACT_SPECIAL_GROUP)
         @param bare(bool): return only bare jids if True
         @return (iter[jid.JID]): found special entities
         """
         for entity in self._specials:
             if bare and entity.resource:
                 continue
-            if special_type is not None and self.getCache(entity, C.CONTACT_SPECIAL) != special_type:
+            if (special_type is not None
+                and self.getCache(entity, C.CONTACT_SPECIAL) != special_type):
                 continue
             yield entity
 
@@ -381,7 +418,8 @@
         attribute must not be set to the default group but ignored. If not,
         you may move your contact from its actual group(s) to the default one.
 
-        None value for 'groups' has a different meaning than [None] which is for the default group.
+        None value for 'groups' has a different meaning than [None]
+        which is for the default group.
 
         @param entity (jid.JID): entity to add or replace
             if entity is a full jid, attributes will be cached in for the full jid only
@@ -394,7 +432,10 @@
             attributes = {}
 
         entity_bare = entity.bare
-        update_type = C.UPDATE_MODIFY if entity_bare in self._cache else C.UPDATE_ADD
+        # we check if the entity is visible before changing anything
+        # this way we know if we need to do an UPDATE_ADD, UPDATE_MODIFY
+        # or an UPDATE_DELETE
+        was_visible = self.entityVisible(entity_bare)
 
         if in_roster:
             self._roster.add(entity_bare)
@@ -403,19 +444,23 @@
                                                      C.CONTACT_MAIN_RESOURCE: None,
                                                      C.CONTACT_SELECTED: set()})
 
-        assert not C.CONTACT_DATA_FORBIDDEN.intersection(attributes) # we don't want forbidden data in attributes
+        # we don't want forbidden data in attributes
+        assert not C.CONTACT_DATA_FORBIDDEN.intersection(attributes)
 
         # we set groups and fill self._groups accordingly
         if groups is not None:
             if not groups:
                 groups = [None]  # [None] is the default group
             if C.CONTACT_GROUPS in cache:
-                # XXX: don't use set(cache[C.CONTACT_GROUPS]).difference(groups) because it won't work in Pyjamas if None is in cache[C.CONTACT_GROUPS]
-                for group in [group for group in cache[C.CONTACT_GROUPS] if group not in groups]:
+                # XXX: don't use set(cache[C.CONTACT_GROUPS]).difference(groups) because
+                #      it won't work in Pyjamas if None is in cache[C.CONTACT_GROUPS]
+                for group in [group for group in cache[C.CONTACT_GROUPS]
+                              if group not in groups]:
                     self._groups[group]['jids'].remove(entity_bare)
             cache[C.CONTACT_GROUPS] = groups
             for group in groups:
-                self._groups.setdefault(group, {}).setdefault('jids', set()).add(entity_bare)
+                self._groups.setdefault(group, {}).setdefault('jids', set()).add(
+                    entity_bare)
 
         # special entities management
         if C.CONTACT_SPECIAL in attributes:
@@ -428,7 +473,8 @@
 
         # now the attributes we keep in cache
         # XXX: if entity is a full jid, we store the value for the resource only
-        cache_attr = cache[C.CONTACT_RESOURCES].setdefault(entity.resource, {}) if entity.resource else cache
+        cache_attr = (cache[C.CONTACT_RESOURCES].setdefault(entity.resource, {})
+                      if entity.resource else cache)
         for attribute, value in attributes.iteritems():
             if value is None:
                 # XXX: pyjamas hack: we need to use pop instead of del
@@ -439,23 +485,34 @@
             else:
                 if attribute == 'nick' and self.isSpecial(entity, C.CONTACT_SPECIAL_GROUP):
                     # we don't want to keep nick for MUC rooms
-                    # FIXME: this is here as plugin XEP-0054 can link resource's nick with bare jid
-                    #        which in the case of MUC set the nick for the whole MUC
+                    # FIXME: this is here as plugin XEP-0054 can link resource's nick
+                    #        with bare jid which in the case of MUC
+                    #        set the nick for the whole MUC
                     #        resulting in bad name displayed in some frontends
                     continue
                 cache_attr[attribute] = value
 
-        # we can update the display
-        self.update([entity], update_type, self.profile)
+        # we can update the display if needed
+        if self.entityVisible(entity_bare):
+            # if the contact was not visible, we need to add a widget
+            # else we just update id
+            update_type = C.UPDATE_MODIFY if was_visible else C.UPDATE_ADD
+            self.update([entity], update_type, self.profile)
+        elif was_visible:
+            # the entity was visible and is not anymore, we remove it
+            self.update([entity], C.UPDATE_DELETE, self.profile)
 
-    def entityToShow(self, entity, check_resource=False):
+    def entityVisible(self, entity, check_resource=False):
         """Tell if the contact should be showed or hidden.
 
         @param entity (jid.JID): jid of the contact
         @param check_resource (bool): True if resource must be significant
         @return (bool): True if that contact should be showed in the list
         """
-        show = self.getCache(entity, C.PRESENCE_SHOW)
+        try:
+            show = self.getCache(entity, C.PRESENCE_SHOW)
+        except exceptions.NotFound:
+            return False
 
         if check_resource:
             selected = self._selected
@@ -464,10 +521,11 @@
         return ((show is not None and show != C.PRESENCE_UNAVAILABLE)
                 or self.show_disconnected
                 or entity in selected
-                or next(self.host.getNotifs(entity.bare, profile=self.profile), None)
+                or (self.show_entities_with_notifs
+                and next(self.host.getNotifs(entity.bare, profile=self.profile), None))
                 )
 
-    def anyEntityToShow(self, entities, check_resources=False):
+    def anyEntityVisible(self, entities, check_resources=False):
         """Tell if in a list of entities, at least one should be shown
 
         @param entities (list[jid.JID]): list of jids
@@ -476,7 +534,7 @@
         """
         # FIXME: looks inefficient, really needed?
         for entity in entities:
-            if self.entityToShow(entity, check_resources):
+            if self.entityVisible(entity, check_resources):
                 return True
         return False
 
@@ -495,6 +553,7 @@
         @param entity(jid.JID): jid of the entity to remove (bare jid is used)
         """
         entity_bare = entity.bare
+        was_visible = self.entityVisible(entity_bare)
         try:
             groups = self._cache[entity_bare].get(C.CONTACT_GROUPS, set())
         except KeyError:
@@ -507,14 +566,17 @@
         for group in groups:
             self._groups[group]['jids'].remove(entity_bare)
             if not self._groups[group]['jids']:
-                self._groups.pop(group) # FIXME: we use pop because of pyjamas: http://wiki.goffi.org/wiki/Issues_with_Pyjamas/en
+                # FIXME: we use pop because of pyjamas:
+                #        http://wiki.goffi.org/wiki/Issues_with_Pyjamas/en
+                self._groups.pop(group)
         for iterable in (self._selected, self._specials):
             to_remove = set()
             for set_entity in iterable:
                 if set_entity.bare == entity.bare:
                     to_remove.add(set_entity)
             iterable.difference_update(to_remove)
-        self.update([entity], C.UPDATE_DELETE, self.profile)
+        if was_visible:
+            self.update([entity], C.UPDATE_DELETE, self.profile)
 
     def onPresenceUpdate(self, entity, show, priority, statuses, profile):
         """Update entity's presence status
@@ -525,7 +587,10 @@
         @param statuses: dict of statuses
         @param profile: %(doc_profile)s
         """
-        cache = self.getCache(entity)
+        # FIXME: cache modification should be done with setContact
+        #        the resources/presence handling logic should be moved there
+        was_visible = self.entityVisible(entity.bare)
+        cache = self.getCache(entity, create_if_not_found=True)
         if show == C.PRESENCE_UNAVAILABLE:
             if not entity.resource:
                 cache[C.CONTACT_RESOURCES].clear()
@@ -534,12 +599,14 @@
                 try:
                     del cache[C.CONTACT_RESOURCES][entity.resource]
                 except KeyError:
-                    log.error(u"Presence unavailable received for an unknown resource [{}]".format(entity))
+                    log.error(u"Presence unavailable received "
+                              u"for an unknown resource [{}]".format(entity))
                 if not cache[C.CONTACT_RESOURCES]:
                     cache[C.CONTACT_MAIN_RESOURCE] = None
         else:
             if not entity.resource:
-                log.warning(_(u"received presence from entity without resource: {}".format(entity)))
+                log.warning(_(u"received presence from entity "
+                              u"without resource: {}".format(entity)))
             resources_data = cache[C.CONTACT_RESOURCES]
             resource_data = resources_data.setdefault(entity.resource, {})
             resource_data[C.PRESENCE_SHOW] = show
@@ -549,9 +616,15 @@
             if entity.bare not in self._specials:
                 # we may have resources with no priority
                 # (when a cached value is added for a not connected resource)
-                priority_resource = max(resources_data, key=lambda res: resources_data[res].get(C.PRESENCE_PRIORITY, -2**32))
+                priority_resource = max(resources_data,
+                                        key=lambda res: resources_data[res].get(
+                                            C.PRESENCE_PRIORITY, -2**32))
                 cache[C.CONTACT_MAIN_RESOURCE] = priority_resource
-        self.update([entity], C.UPDATE_MODIFY, self.profile)
+        if self.entityVisible(entity.bare):
+            update_type = C.UPDATE_MODIFY if was_visible else C.UPDATE_ADD
+            self.update([entity], update_type, self.profile)
+        elif was_visible:
+            self.update([entity], C.UPDATE_DELETE, self.profile)
 
     def onNickUpdate(self, entity, new_nick, profile):
         """Update entity's nick
@@ -562,7 +635,6 @@
         """
         assert profile == self.profile
         self.setCache(entity, 'nick', new_nick)
-        self.update([entity], C.UPDATE_MODIFY, profile)
 
     def onNotification(self, entity, notif, profile):
         """Update entity with notification
@@ -572,7 +644,7 @@
         @param profile: %(doc_profile)s
         """
         assert profile == self.profile
-        if entity is not None:
+        if entity is not None and self.entityVisible(entity):
             self.update([entity], C.UPDATE_MODIFY, profile)
 
     def unselect(self, entity):
@@ -616,7 +688,7 @@
                 self.update([entity], C.UPDATE_SELECTION, profile=self.profile)
 
     def showOfflineContacts(self, show):
-        """Tell if offline contacts should shown
+        """Tell if offline contacts should be shown
 
         @param show(bool): True if offline contacts should be shown
         """
@@ -638,7 +710,7 @@
         if self.show_resources == show:
             return
         self.show_resources = show
-        self.update(profile=self.profile)
+        self.update(type_=C.UPDATE_STRUCTURE, profile=self.profile)
 
     def plug(self):
         handler.addProfile(self.profile)
@@ -657,7 +729,8 @@
         self.host = host
         global handler
         if handler is not None:
-            raise exceptions.InternalError(u"QuickContactListHandler must be instanciated only once")
+            raise exceptions.InternalError(u"QuickContactListHandler must be "
+                                           u"instanciated only once")
         handler = self
         self._clist = {} # key: profile, value: ProfileContactList
         self._widgets = set()
@@ -670,7 +743,8 @@
     def __contains__(self, entity):
         """Check if entity is in contact list
 
-        @param entity (jid.JID): jid of the entity (resource is not ignored, use bare jid if needed)
+        @param entity (jid.JID): jid of the entity (resource is not ignored,
+            use bare jid if needed)
         """
         for contact_list in self._clist.itervalues():
             if entity in contact_list:
@@ -825,7 +899,8 @@
 
         If special_type is None, return all special extras.
 
-        @param special_type(unicode, None): one of special type (e.g. C.CONTACT_SPECIAL_GROUP)
+        @param special_type(unicode, None): one of special type
+            (e.g. C.CONTACT_SPECIAL_GROUP)
             None to return all special extras.
         @return (set[jid.JID])
         """
@@ -838,6 +913,7 @@
         self._to_fill.remove(profile)
         if not self._to_fill:
             del self._to_fill
+            # we need a full update when all contacts are filled
             self.update()
 
     def fill(self, profile=None):
@@ -864,7 +940,8 @@
 
         remaining = to_fill.difference(filled)
         if remaining != to_fill:
-            log.debug(u"Not re-filling already filled contact list(s) for {}".format(u', '.join(to_fill.intersection(filled))))
+            log.debug(u"Not re-filling already filled contact list(s) for {}".format(
+                u', '.join(to_fill.intersection(filled))))
         for profile in remaining:
             self._clist[profile]._fill()
 
@@ -875,6 +952,7 @@
         """
         for contact_list in self._clist.itervalues():
             contact_list.clearContacts(keep_cache)
+        # we need a full update
         self.update()
 
     def select(self, entity):
@@ -895,7 +973,8 @@
             if set to False, widget state can be inconsistent, be sure to know
             what youa re doing!
         """
-        log.debug(u"Contact lists updates are now {}".format(u"LOCKED" if locked else u"UNLOCKED"))
+        log.debug(u"Contact lists updates are now {}".format(
+            u"LOCKED" if locked else u"UNLOCKED"))
         self._update_locked = locked
         if not locked and do_update:
             self.update()
@@ -910,7 +989,8 @@
     """This class manage the visual representation of contacts"""
     SINGLE=False
     PROFILES_MULTIPLE=True
-    PROFILES_ALLOW_NONE=True # Can be linked to no profile (e.g. at the early forntend start)
+    # Can be linked to no profile (e.g. at the early frontend start)
+    PROFILES_ALLOW_NONE=True
 
     def __init__(self, host, profiles):
         super(QuickContactList, self).__init__(host, None, profiles)
@@ -937,7 +1017,7 @@
 
     @property
     def items_sorted(self):
-        return handler.items
+        return handler.items_sorted
 
     def update(self, entities=None, type_=None, profile=None):
         """Update the display when something changed
@@ -950,8 +1030,11 @@
             - C.UPDATE_ADD: entity added
             - C.UPDATE_SELECTION: selection modified
             or None for undefined update
+            Note that events correspond to addition, modification and deletion
+            of items on the whole contact list. If the contact is visible or not
+            has no influence on the type_.
         @param profile(unicode, None): profile concerned with the update
-            None if unknown
+            None if all profiles need to be updated
         """
         raise NotImplementedError