changeset 2852:e2595c81eb6d

quick frontend(widgets): improved handling of multiple instances of widgets: when a widget was recreated, a new hash was created with a suffix, making hash matching difficult and bug prone. This has been changed to use a list of instances instead, this simplify the code and avoid issues while looking for existing instances of a widget.
author Goffi <goffi@goffi.org>
date Sun, 10 Mar 2019 18:03:14 +0100
parents 7764383a968c
children 6901a425d882
files sat_frontends/quick_frontend/quick_widgets.py
diffstat 1 files changed, 50 insertions(+), 70 deletions(-) [+]
line wrap: on
line diff
--- a/sat_frontends/quick_frontend/quick_widgets.py	Sun Mar 10 18:03:11 2019 +0100
+++ b/sat_frontends/quick_frontend/quick_widgets.py	Sun Mar 10 18:03:14 2019 +0100
@@ -24,7 +24,6 @@
 from sat_frontends.quick_frontend.constants import Const as C
 
 
-NEW_INSTANCE_SUFF = "_new_instance_"
 classes_map = {}
 
 
@@ -65,8 +64,9 @@
     def __iter__(self):
         """Iterate throught all widgets"""
         for widget_map in self._widgets.itervalues():
-            for widget in widget_map.itervalues():
-                yield widget
+            for widget_instances in widget_map.itervalues():
+                for widget in widget_instances:
+                    yield widget
 
     def getRealClass(self, class_):
         """Return class registered for given class_
@@ -86,21 +86,29 @@
             )
         return cls
 
-    def getRootHash(self, hash_):
-        """Return root hash (i.e. hash without new instance suffix for recreated widgets
+    def getWidgetInstances(self, widget):
+        """Get all instance of a widget
 
-        @param hash_(immutable): hash of a widget
-        @return (unicode): root hash (transtyped to unicode)
+        This is a helper method which call getWidgets
+        @param widget(QuickWidget): retrieve instances of this widget
+        @return: iterator on widgets
         """
-        return unicode(hash_).split(NEW_INSTANCE_SUFF)[0]
+        try:
+            target = widget.target
+        except AttributeError:
+            target = next(iter(widget.targets))
+        return self.getWidgets(widget.__class__, target, widget.profiles)
 
     def getWidgets(self, class_, target=None, profiles=None):
         """Get all subclassed widgets instances
 
-        @param class_: subclass of QuickWidget, same parameter as used in [getOrCreateWidget]
-        @param target: if not None, construct a hash with this target and filter corresponding widgets
-            recreated widgets (with new instance suffix) are handled
-        @param profiles(iterable, None): if not None, filter on instances linked to these profiles
+        @param class_: subclass of QuickWidget, same parameter as used in
+            [getOrCreateWidget]
+        @param target: if not None, construct a hash with this target and filter
+            corresponding widgets
+            recreated widgets are handled
+        @param profiles(iterable, None): if not None, filter on instances linked to these
+            profiles
         @return: iterator on widgets
         """
         class_ = self.getRealClass(class_)
@@ -113,18 +121,18 @@
                 filter_hash = unicode(class_.getWidgetHash(target, profiles))
             else:
                 filter_hash = None
-            for w_hash, w in widgets_map.iteritems():
-                if profiles is None or w.profiles.intersection(profiles):
-                    if (
-                        filter_hash is not None
-                        and self.getRootHash(w_hash) != filter_hash
-                    ):
-                        continue
-                    yield w
+            if filter_hash is not None:
+                for widget in widgets_map[filter_hash]:
+                    yield widget
+            else:
+                for widget_instances in widgets_map.itervalues():
+                    for widget in widget_instances:
+                        yield widget
 
     def getWidget(self, class_, target=None, profiles=None):
         """Get a widget without creating it if it doesn't exist.
 
+        if several instances of widgets with this hash exist, the first one is returned
         @param class_: subclass of QuickWidget, same parameter as used in [getOrCreateWidget]
         @param target: target depending of the widget, usually a JID instance
         @param profiles (unicode, iterable[unicode], None): profile(s) to use (may or may not be
@@ -137,7 +145,7 @@
         class_ = self.getRealClass(class_)
         hash_ = class_.getWidgetHash(target, profiles)
         try:
-            return self._widgets[class_.__name__][hash_]
+            return self._widgets[class_.__name__][hash_][0]
         except KeyError:
             return None
 
@@ -158,10 +166,10 @@
             if 'on_existing_widget' is present it can have the following values:
                 C.WIDGET_KEEP  [default]: return the existing widget
                 C.WIDGET_RAISE: raise WidgetAlreadyExistsError
-                C.WIDGET_RECREATE: create a new widget *WITH A NEW HASH*
+                C.WIDGET_RECREATE: create a new widget
                     if the existing widget has a "recreateArgs" method, it will be called with args list and kwargs dict
                     so the values can be completed to create correctly the new instance
-                [callable]: this method will be called with existing widget as argument
+                [callable]: this method will be called with existing widget as argument, the widget to use must be returned
             if 'force_hash' is present, the hash given in value will be used instead of the one returned by class_.getWidgetHash
             other keys will be used to instanciate class_ if the case happen (e.g. if type_ is present and class_ is a QuickChat subclass,
                 it will be used to create a new QuickChat instance).
@@ -211,16 +219,17 @@
             widget = None  # if the class is not SINGLE, we always create a new widget
         else:
             try:
-                widget = widgets_map[hash_]
-                widget.addTarget(target)
+                widget = widgets_map[hash_][0]
             except KeyError:
                 widget = None
+            else:
+                widget.addTarget(target)
 
         if widget is None:
             # we need to create a new widget
             log.debug(u"Creating new widget for target {} {}".format(target, cls))
             widget = cls(*_args, **_kwargs)
-            widgets_map[hash_] = widget
+            widgets_map[hash_] = [widget]
 
             if on_new_widget == C.WIDGET_NEW:
                 self.host.newWidget(widget)
@@ -235,55 +244,24 @@
             elif on_existing_widget == C.WIDGET_RAISE:
                 raise WidgetAlreadyExistsError(hash_)
             elif on_existing_widget == C.WIDGET_RECREATE:
-                # we use getOrCreateWidget to recreate the new widget
-                # /!\ we use args and kwargs and not _args and _kwargs because we need the original args
-                #     we need to get rid of kwargs special options
-                new_kwargs = kwargs.copy()
-                try:
-                    new_kwargs.pop(
-                        "force_hash"
-                    )  # FIXME: we use pop instead of del here because pyjamas doesn't raise error on del
-                except KeyError:
-                    pass
-                else:
-                    raise ValueError(
-                        "force_hash option can't be used with on_existing_widget=RECREATE"
-                    )
-
-                new_kwargs["on_new_widget"] = on_new_widget
-
-                # XXX: keep up-to-date if new special kwargs are added (i.e.: delete these keys here)
-                new_kwargs["on_existing_widget"] = C.WIDGET_RAISE
                 try:
                     recreateArgs = widget.recreateArgs
                 except AttributeError:
                     pass
                 else:
-                    recreateArgs(args, new_kwargs)
-                hash_idx = 1
-                while True:
-                    new_kwargs["force_hash"] = "{}{}{}".format(
-                        hash_, NEW_INSTANCE_SUFF, hash_idx
-                    )
-                    try:
-                        widget = self.getOrCreateWidget(
-                            class_, target, *args, **new_kwargs
-                        )
-                    except WidgetAlreadyExistsError:
-                        hash_idx += 1
-                    else:
-                        log.debug(
-                            u"Widget already exists, a new one has been recreated with hash {}".format(
-                                new_kwargs["force_hash"]
-                            )
-                        )
-                        break
+                    recreateArgs(_args, _kwargs)
+                widget = cls(*_args, **_kwargs)
+                widgets_map[hash_].append(widget)
+                log.debug(u"widget <{wid}> already exists, a new one has been recreated"
+                    .format(wid=widget))
             elif callable(on_existing_widget):
-                on_existing_widget(widget)
+                widget = on_existing_widget(widget)
+                if widget is None:
+                    raise exceptions.InternalError(
+                        u"on_existing_widget method must return the widget to use")
             else:
                 raise exceptions.InternalError(
-                    "Unexpected on_existing_widget value ({})".format(on_existing_widget)
-                )
+                    "Unexpected on_existing_widget value ({})".format(on_existing_widget))
 
         return widget
 
@@ -306,9 +284,11 @@
 
         for widget_map in self._widgets.itervalues():
             to_delete = set()
-            for hash_, widget in widget_map.iteritems():
-                if widget_to_delete is widget:
-                    to_delete.add(hash_)
+            for hash_, widget_instances in widget_map.iteritems():
+                if widget_to_delete in widget_instances:
+                    widget_instances.remove(widget_to_delete)
+                    if not widget_instances:
+                        to_delete.add(hash_)
             for hash_ in to_delete:
                 del widget_map[hash_]