Mercurial > libervia-backend
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_]