diff --git a/CHANGELOG.md b/CHANGELOG.md index c980eff4b7..eff2b0f456 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ * Fix `Text` with `spans` ignoring `overflow`: a rich text cut off by `max_lines` now ends in an ellipsis with `overflow=TextOverflow.ELLIPSIS`, as plain text does, instead of being clipped at a word ([#7057](https://github.com/flet-dev/flet/pull/7057)) by @FeodorFitsner. * Fix mypy inferring `type[Never]` for classes decorated with `@control("Name")`: the overload that takes arguments now accepts only a string `dart_widget_name` ([#7085](https://github.com/flet-dev/flet/issues/7085), [#7087](https://github.com/flet-dev/flet/pull/7087)) by @joaoaracheski. * Fix type checkers ignoring `dataclasses.field()` arguments such as `init=False` on `@control` and `@value` classes, by declaring `dataclasses.field` as a field specifier ([#6377](https://github.com/flet-dev/flet/issues/6377), [#7084](https://github.com/flet-dev/flet/pull/7084)) by @joaoaracheski. +* Fix `@ft.observable` not refreshing the UI after `+=` and `*=` on lists and `|=` on dicts, `extend()` never returning when given the list itself, and lists or dicts merged into a dict with `update()` or `|=` not notifying when changed later ([#6293](https://github.com/flet-dev/flet/issues/6293), [#7086](https://github.com/flet-dev/flet/pull/7086)) by @joaoaracheski. ## 1.1.2 diff --git a/sdk/python/packages/flet/src/flet/components/observable.py b/sdk/python/packages/flet/src/flet/components/observable.py index 5bb13bf913..84dce6b026 100644 --- a/sdk/python/packages/flet/src/flet/components/observable.py +++ b/sdk/python/packages/flet/src/flet/components/observable.py @@ -1,6 +1,7 @@ from __future__ import annotations import contextlib +import operator import weakref from dataclasses import InitVar, dataclass from typing import TYPE_CHECKING, Any, Callable, Optional @@ -34,6 +35,13 @@ class MyDataClass: x: int y: int ``` + + Note: + Listeners are called synchronously. With augmented assignments such as + `obj.items += [x]`, `*=` on lists and `|=` on dicts, they run before + Python assigns the result back to the field, so a listener that + replaces `obj.items` at that moment is overwritten. Direct calls such + as `obj.items.extend([x])` keep the listener's replacement. """ if Observable in cls.__mro__: return cls @@ -223,18 +231,21 @@ def __repr__(self): # Observable collections ----------------------------------------- +_MISSING = object() + -class ObservableList(list): +class _ObservableCollection: """ - List wrapper that reports mutating operations to its owning observable. + Base for collections that report changes to a field of an observable. + + Subclasses store a weak reference to the owning observable in `_owner_ref` + and the field's name in `_field`, both declared in their `__slots__`. """ - __slots__ = ("_owner_ref", "_field") + __slots__ = () - def __init__(self, owner: Observable, field: str, iterable=()): - super().__init__(iterable) - self._owner_ref = weakref.ref(owner) - self._field = field + _owner_ref: weakref.ref + _field: str def _touch(self): """ @@ -250,7 +261,7 @@ def _wrap(self, v): Wrap nested collections into observable variants before storing. Args: - v: Value being inserted into the list. + v: Value being inserted into the collection. Returns: Wrapped value when applicable. @@ -259,6 +270,44 @@ def _wrap(self, v): owner = self._owner_ref() return owner._wrap_if_collection(self._field, v) if owner else v + def _is_written_back(self, current: Any, value: Any) -> bool: + """ + Tell whether `value` is the observable child already stored in a slot. + + Python writes the result of an augmented assignment such as + `d["items"] += [1]` back into the slot. The child has notified already, + so writing the identical child back to the same owner and field is a + no-op. + + Args: + current: Value in the slot, or `_MISSING` when the slot is empty. + value: Value being written into the slot. + + Returns: + `True` when `value` is `current` and reports to the same owner and + field. + """ + + return ( + value is current + and isinstance(value, _ObservableCollection) + and value._owner_ref() is self._owner_ref() + and value._field == self._field + ) + + +class ObservableList(_ObservableCollection, list): + """ + List wrapper that reports mutating operations to its owning observable. + """ + + __slots__ = ("_owner_ref", "_field") + + def __init__(self, owner: Observable, field: str, iterable=()): + super().__init__(iterable) + self._owner_ref = weakref.ref(owner) + self._field = field + def append(self, x): """ Append an item and notify observers. @@ -270,11 +319,50 @@ def append(self, x): def extend(self, it): """ Extend list with iterable items and notify observers. + + Items are appended one at a time, as with `list.extend()`. + `xs.extend(xs)` appends the list's current items once, as the same + objects. If the iterable raises after some items were appended, + observers are still notified before the error propagates. """ - super().extend(self._wrap(v) for v in it) + size = len(self) + try: + if it is self: + super().extend(self) + else: + super().extend(self._wrap(v) for v in it) + except BaseException: + if len(self) != size: + self._touch() + raise self._touch() + def __iadd__(self, other): + """ + Extend list in place (`+=`) and notify observers. + + Observers are notified before Python assigns the list back to its + field, so a subscriber that replaces the field during `+=` has its + replacement overwritten. Call `extend()` to keep such a replacement. + """ + + self.extend(other) + return self + + def __imul__(self, n): + """ + Repeat list items in place (`*=`) and notify observers. + + Observers are notified before Python assigns the list back to its + field, so a subscriber that replaces the field during `*=` has its + replacement overwritten. + """ + + super().__imul__(n) + self._touch() + return self + def insert(self, i, x): """ Insert an item at a specific index and notify observers. @@ -331,6 +419,27 @@ def pop(self, i=-1): return v def __setitem__(self, i, v): + """ + Set an item or a slice and notify observers. + + Writing the item's own observable child back to its index, as Python + does after `xs[0] += [1]`, changes nothing and doesn't notify again. + An index object's `__index__()` is called once, as with `list`. + """ + + if not isinstance(i, slice): + try: + i = operator.index(i) + except TypeError: + pass + else: + current = ( + list.__getitem__(self, i) + if -len(self) <= i < len(self) + else _MISSING + ) + if self._is_written_back(current, v): + return super().__setitem__(i, self._wrap(v)) self._touch() @@ -339,7 +448,7 @@ def __delitem__(self, i): self._touch() -class ObservableDict(dict): +class ObservableDict(_ObservableCollection, dict): """ Dict wrapper that reports mutating operations to its owning observable. """ @@ -351,30 +460,16 @@ def __init__(self, owner: Observable, field: str, mapping=()): self._owner_ref = weakref.ref(owner) self._field = field - def _touch(self): - """ - Emit a change notification for the parent field. - """ - - owner = self._owner_ref() - if owner: - owner._notify(self._field) - - def _wrap(self, v): + def __setitem__(self, k, v): """ - Wrap nested collections into observable variants before storing. + Set a key and notify observers. - Args: - v: Value being inserted into the dict. - - Returns: - Wrapped value when applicable. + Writing the key's own observable child back, as Python does after + `d["items"] += [1]`, changes nothing and doesn't notify again. """ - owner = self._owner_ref() - return owner._wrap_if_collection(self._field, v) if owner else v - - def __setitem__(self, k, v): + if self._is_written_back(dict.get(self, k, _MISSING), v): + return super().__setitem__(k, self._wrap(v)) self._touch() @@ -394,13 +489,48 @@ def update(self, *a, **k): """ Update mapping with values and notify observers. - Positional mapping/iterable arguments are forwarded to `dict.update()`; - keyword values are wrapped when needed. + Takes the same arguments as `dict.update()` and inserts entries in the + same order: the mapping's keys or the key-value pairs first, one at a + time, then the keywords. List and dict values are wrapped as they are + inserted. Observers are notified once; if the input raises after some + entries were inserted, they are notified before the error propagates. """ - super().update(*a, **{kk: self._wrap(vv) for kk, vv in k.items()}) + if len(a) > 1: + raise TypeError(f"update expected at most 1 argument, got {len(a)}") + changed = False + try: + if a: + other = a[0] + if hasattr(other, "keys"): + for kk in other.keys(): # noqa: SIM118 + dict.__setitem__(self, kk, self._wrap(other[kk])) + changed = True + else: + for kk, vv in other: + dict.__setitem__(self, kk, self._wrap(vv)) + changed = True + for kk, vv in k.items(): + dict.__setitem__(self, kk, self._wrap(vv)) + changed = True + except BaseException: + if changed: + self._touch() + raise self._touch() + def __ior__(self, other): + """ + Merge a mapping in place (`|=`) and notify observers. + + Observers are notified before Python assigns the dict back to its + field, so a subscriber that replaces the field during `|=` has its + replacement overwritten. Call `update()` to keep such a replacement. + """ + + self.update(other) + return self + def pop(self, k, *d): """ Remove key and return its value, then notify observers. diff --git a/sdk/python/packages/flet/tests/test_observable.py b/sdk/python/packages/flet/tests/test_observable.py index 5fbb147939..86528bcb21 100644 --- a/sdk/python/packages/flet/tests/test_observable.py +++ b/sdk/python/packages/flet/tests/test_observable.py @@ -1,133 +1,458 @@ from dataclasses import dataclass, field +import pytest + import flet as ft class Foo(ft.Observable): + """Observable with annotated fields and no dataclass.""" + prop_a: str prop_b: str @dataclass class Bar: + """Plain dataclass base for `Baz`.""" + f: int d: list[int] = field(default_factory=list) @dataclass class Baz(Bar, ft.Observable): + """Observable dataclass that inherits fields from a plain dataclass.""" + prop_c: int = 3 prop_d: list[int] = field(default_factory=lambda: [1, 2, 3]) def add_item(self, item: int): + """Appends `item` to `prop_d`.""" self.prop_d.append(item) @dataclass class Qux(ft.Observable): + """Observable dataclass with a dict field.""" + prop_e: dict = field(default_factory=dict) def update_key(self, key, value): + """Sets `key` in `prop_e`.""" self.prop_e[key] = value -def test_foo_observable(): +@dataclass +class Store(ft.Observable): + """Observable dataclass with a list field and a dict field.""" + + items: list = field(default_factory=list) + mapping: dict = field(default_factory=dict) + + +def _watch(obj): + """Subscribes to `obj` and returns the list of changed field names. + + The listener is stored on `obj`, because observables hold listeners weakly. + """ changes = [] - def subscriber(sender, field): - changes.append((sender, field)) + def listener(sender, field): + changes.append(field) - foo = Foo() - foo.subscribe(subscriber) + obj._test_listener = listener + obj.subscribe(listener) + return changes - foo.prop_a = "test_value" - assert len(changes) == 1 - assert changes[0][0] == foo - assert changes[0][1] == "prop_a" - foo.prop_b = "another_value" - assert len(changes) == 2 - assert changes[1][0] == foo - assert changes[1][1] == "prop_b" +class TestObservable: + """Notifications and versioning of observable objects.""" + def test_field_assignment_notifies(self): + """Assigning a field notifies with the sender and the field name.""" + changes = [] -def test_bar_observable(): - changes = [] + def subscriber(sender, field): + changes.append((sender, field)) - def subscriber(sender, field): - changes.append((sender, field)) + foo = Foo() + foo.subscribe(subscriber) - baz = Baz(42) - baz.subscribe(subscriber) + foo.prop_a = "test_value" + assert len(changes) == 1 + assert changes[0][0] == foo + assert changes[0][1] == "prop_a" - baz.prop_a = "new_value" - assert len(changes) == 1 - assert changes[0][0] == baz - assert changes[0][1] == "prop_a" + foo.prop_b = "another_value" + assert len(changes) == 2 + assert changes[1][0] == foo + assert changes[1][1] == "prop_b" - baz.prop_c = 100 - assert len(changes) == 2 - assert changes[1][0] == baz - assert changes[1][1] == "prop_c" + def test_dataclass_subclass_notifies(self): + """Own, inherited and list fields of an observable dataclass notify.""" + changes = [] - baz.add_item(5) - assert baz.prop_d == [1, 2, 3, 5] - assert len(changes) == 3 - assert changes[2][0] == baz - assert changes[2][1] == "prop_d" + def subscriber(sender, field): + changes.append((sender, field)) - baz.f = 60 - assert len(changes) == 4 - assert changes[3][0] == baz - assert changes[3][1] == "f" + baz = Baz(42) + baz.subscribe(subscriber) - baz.d = [1, 2, 3] - assert len(changes) == 5 - assert changes[4][0] == baz - assert changes[4][1] == "d" + baz.prop_a = "new_value" + assert len(changes) == 1 + assert changes[0][0] == baz + assert changes[0][1] == "prop_a" - baz.d.reverse() - assert len(changes) == 6 - assert changes[5][0] == baz - assert changes[5][1] == "d" + baz.prop_c = 100 + assert len(changes) == 2 + assert changes[1][0] == baz + assert changes[1][1] == "prop_c" + baz.add_item(5) + assert baz.prop_d == [1, 2, 3, 5] + assert len(changes) == 3 + assert changes[2][0] == baz + assert changes[2][1] == "prop_d" + + baz.f = 60 + assert len(changes) == 4 + assert changes[3][0] == baz + assert changes[3][1] == "f" + + baz.d = [1, 2, 3] + assert len(changes) == 5 + assert changes[4][0] == baz + assert changes[4][1] == "d" + + baz.d.reverse() + assert len(changes) == 6 + assert changes[5][0] == baz + assert changes[5][1] == "d" + + def test_repr_shows_version(self): + """`repr` includes a version that grows with each change.""" + foo = Foo() + initial_repr = repr(foo) + assert "version=0" in initial_repr + + foo.prop_a = "value1" + updated_repr = repr(foo) + assert "version=1" in updated_repr + + foo.prop_b = "value2" + updated_repr_2 = repr(foo) + assert "version=2" in updated_repr_2 + + +class TestObservableList: + """Notifications from list fields of observable objects.""" + + def test_inplace_operators_notify(self): + """`+=` and `*=` change the list in place and notify once each.""" + changes = [] + + def subscriber(sender, field): + changes.append(field) + + baz = Baz(f=1) + baz.subscribe(subscriber) + + baz.prop_d += [4] + assert baz.prop_d == [1, 2, 3, 4] + assert changes == ["prop_d"] + + baz.prop_d *= 2 + assert baz.prop_d == [1, 2, 3, 4, 1, 2, 3, 4] + assert changes == ["prop_d", "prop_d"] + + def test_iadd_with_itself(self): + """`+=` with the list itself appends one copy of its items.""" + baz = Baz(f=1) + + baz.prop_d += baz.prop_d + assert baz.prop_d == [1, 2, 3, 1, 2, 3] + + def test_set_item_reads_custom_index_once(self): + """Setting an item calls the index object's `__index__()` once.""" + + class Index: + def __init__(self): + self.calls = 0 + + def __index__(self): + self.calls += 1 + return self.calls - 1 + + store = Store(items=[10, 20]) + changes = _watch(store) + index = Index() + + store.items[index] = 99 + assert store.items == [99, 20] + assert index.calls == 1 + assert changes == ["items"] + + def test_extend_with_itself(self): + """`extend` with the list itself appends its items once, as the same objects.""" + store = Store(items=[[1], 2]) + changes = _watch(store) + + store.items.extend(store.items) + assert store.items == [[1], 2, [1], 2] + assert store.items[2] is store.items[0] + assert changes == ["items"] + + def test_extend_reads_iterable_lazily(self): + """`extend` appends each item before reading the next one.""" + store = Store(items=[0]) + + def gen(): + for _ in range(2): + yield store.items[-1] + 1 + + store.items.extend(gen()) + assert store.items == [0, 1, 2] + + def test_extend_notifies_when_iterable_fails_partway(self): + """`extend` notifies about items appended before the iterable raised.""" + store = Store(items=[0]) + changes = _watch(store) + + def gen(): + yield 1 + raise RuntimeError("boom") + + with pytest.raises(RuntimeError): + store.items.extend(gen()) + assert store.items == [0, 1] + assert changes == ["items"] + + def test_extend_keeps_subscriber_replacement(self): + """A subscriber that replaces the field during `extend` keeps its value.""" + store = Store(items=[1]) + replaced = [] + + def listener(sender, field): + if not replaced: + replaced.append(True) + sender.items = [99] + + store.subscribe(listener) + store.items.extend([2]) + assert store.items == [99] + + def test_same_child_assignment_is_noop(self): + """Writing an item's own observable child back to its index doesn't notify.""" + store = Store(items=[[1]]) + store.items[0] = [1] + changes = _watch(store) + + store.items[0] = store.items[0] + store.items[-1] = store.items[-1] + assert changes == [] + + store.items[0] = [2] + store.items[0:1] = store.items[0:1] + assert changes == ["items", "items"] + + +class TestObservableDict: + """Notifications from dict fields of observable objects.""" + + def test_assignment_and_item_set_notify(self): + """Assigning the dict field and setting its keys notify.""" + changes = [] + + def subscriber(sender, field): + changes.append((sender, field)) + + qux = Qux() + qux.subscribe(subscriber) + + qux.prop_e = {"key1": "value1"} + assert len(changes) == 1 + assert changes[0][0] == qux + assert changes[0][1] == "prop_e" + + qux.update_key("key2", "value2") + assert qux.prop_e == {"key1": "value1", "key2": "value2"} + assert len(changes) == 2 + assert changes[1][0] == qux + assert changes[1][1] == "prop_e" + + qux.prop_e["key1"] = "updated_value1" + assert qux.prop_e["key1"] == "updated_value1" + assert len(changes) == 3 + assert changes[2][0] == qux + assert changes[2][1] == "prop_e" + + def test_inplace_union_notifies(self): + """`|=` merges into the dict in place and notifies once.""" + changes = [] + + def subscriber(sender, field): + changes.append(field) + + qux = Qux() + qux.subscribe(subscriber) + + qux.prop_e |= {"key1": "value1"} + assert qux.prop_e == {"key1": "value1"} + assert changes == ["prop_e"] + + @pytest.mark.parametrize( + "merge", + [ + lambda d: d.update({"items": []}), + lambda d: d.update([("items", [])]), + lambda d: d.update(items=[]), + lambda d: d.__ior__({"items": []}), + lambda d: d.__ior__([("items", [])]), + ], + ids=["mapping", "pairs", "keywords", "ior", "ior-pairs"], + ) + def test_merge_wraps_nested_values(self, merge): + """Lists merged into the dict notify when they change later.""" + changes = [] + + def subscriber(sender, field): + changes.append(field) + + qux = Qux() + qux.subscribe(subscriber) + + merge(qux.prop_e) + changes.clear() + qux.prop_e["items"].append(1) + assert qux.prop_e == {"items": [1]} + assert changes == ["prop_e"] + + def test_update_reads_input_lazily(self): + """`update` inserts each pair before reading the next one.""" + store = Store() + changes = _watch(store) + + def pairs(): + for key in ("x", "y"): + yield key, len(store.mapping) + + store.mapping.update(pairs()) + assert store.mapping == {"x": 0, "y": 1} + assert changes == ["mapping"] + + def test_update_applies_keywords_last(self): + """Keyword values override the mapping's values for the same key.""" + store = Store() + + store.mapping.update({"a": 1, "b": 2}, b=3) + assert store.mapping == {"a": 1, "b": 3} + + def test_update_reads_mapping_keys(self): + """`update` takes a mapping's entries from `keys()`, as `dict.update` does.""" + + class Mapping: + def keys(self): + return ["a"] + + def __getitem__(self, key): + return [key] + + def __iter__(self): + return iter(["ignored"]) + + store = Store() + changes = _watch(store) + + store.mapping.update(Mapping()) + assert store.mapping == {"a": ["a"]} + assert changes == ["mapping"] + changes.clear() + store.mapping["a"].append(1) + assert changes == ["mapping"] + + def test_update_notifies_when_input_fails_partway(self): + """`update` notifies about entries inserted before the input raised.""" + store = Store() + changes = _watch(store) + + with pytest.raises(ValueError): + store.mapping.update([("a", 1), ("b", 2, 3)]) + assert store.mapping == {"a": 1} + assert changes == ["mapping"] + + def test_update_rejects_extra_arguments(self): + """`update` with two positional arguments raises without notifying.""" + store = Store() + changes = _watch(store) + + with pytest.raises(TypeError): + store.mapping.update({"a": 1}, {"b": 2}) + assert store.mapping == {} + assert changes == [] + + def test_same_child_assignment_is_noop(self): + """Writing a key's own observable child back doesn't notify.""" + store = Store() + store.mapping["items"] = [] + changes = _watch(store) + + store.mapping["items"] = store.mapping["items"] + assert changes == [] + + store.mapping["items"] = [] + assert changes == ["mapping"] + + +def _iadd_list_in_dict(store): + store.mapping["list"] += [1] + + +def _imul_list_in_dict(store): + store.mapping["list"] *= 2 + + +def _ior_dict_in_dict(store): + store.mapping["dict"] |= {"a": 1} -def test_qux_observable(): - changes = [] - def subscriber(sender, field): - changes.append((sender, field)) +def _iadd_list_in_list(store): + store.items[0] += [1] - qux = Qux() - qux.subscribe(subscriber) - qux.prop_e = {"key1": "value1"} - assert len(changes) == 1 - assert changes[0][0] == qux - assert changes[0][1] == "prop_e" +def _imul_list_in_list(store): + store.items[-2] *= 2 - qux.update_key("key2", "value2") - assert qux.prop_e == {"key1": "value1", "key2": "value2"} - assert len(changes) == 2 - assert changes[1][0] == qux - assert changes[1][1] == "prop_e" - qux.prop_e["key1"] = "updated_value1" - assert qux.prop_e["key1"] == "updated_value1" - assert len(changes) == 3 - assert changes[2][0] == qux - assert changes[2][1] == "prop_e" +def _ior_dict_in_list(store): + store.items[1] |= {"a": 1} -def test_observable_repr(): - foo = Foo() - initial_repr = repr(foo) - assert "version=0" in initial_repr +class TestNestedInplaceOperators: + """In-place operators on collections nested in observable collections.""" - foo.prop_a = "value1" - updated_repr = repr(foo) - assert "version=1" in updated_repr + @pytest.mark.parametrize( + "op, field, child, expected", + [ + (_iadd_list_in_dict, "mapping", lambda s: s.mapping["list"], [0, 1]), + (_imul_list_in_dict, "mapping", lambda s: s.mapping["list"], [0, 0]), + (_ior_dict_in_dict, "mapping", lambda s: s.mapping["dict"], {"a": 1}), + (_iadd_list_in_list, "items", lambda s: s.items[0], [0, 1]), + (_imul_list_in_list, "items", lambda s: s.items[0], [0, 0]), + (_ior_dict_in_list, "items", lambda s: s.items[1], {"a": 1}), + ], + ids=["dict+=", "dict*=", "dict|=", "list+=", "list*=", "list|="], + ) + def test_notifies_once(self, op, field, child, expected): + """The nested collection notifies once; writing it back adds nothing.""" + store = Store() + store.mapping["list"] = [0] + store.mapping["dict"] = {} + store.items.extend([[0], {}]) + before = child(store) + changes = _watch(store) - foo.prop_b = "value2" - updated_repr_2 = repr(foo) - assert "version=2" in updated_repr_2 + op(store) + assert child(store) is before + assert child(store) == expected + assert changes == [field]