Skip to content

Commit bba9d70

Browse files
committed
Improve patch application and add list insertion tests
Refactored Control patch logic to use path-based indexing for more accurate patch target resolution. Updated DiffBuilder to fix path handling in dataclass comparison. Added comprehensive tests for list insertions and updates in both frozen and in-place diff scenarios, improving coverage for control list operations.
1 parent d65134f commit bba9d70

4 files changed

Lines changed: 160 additions & 30 deletions

File tree

packages/flet/lib/src/models/control.dart

Lines changed: 32 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -305,43 +305,51 @@ class Control extends ChangeNotifier {
305305
"Patch must be a list with at least 2 elements: tree_index, operation");
306306
}
307307

308-
// build map of "to-be-patched" tree nodes
309-
Map<int, PatchTarget> treeIndex = {};
310-
buildTreeIndex(Control control, dynamic obj, List<dynamic> node) {
308+
Map<int, List<dynamic>> pathIndex = {};
309+
310+
buildPathIndex(List<dynamic> node, List<dynamic> path) {
311311
// node[0] - index
312312
// node[1] - map of child properties or indexes
313-
treeIndex[node[0]] =
314-
PatchTarget(obj is Control ? obj.properties : obj, control);
313+
pathIndex[node[0]] = path;
315314
if (node.length > 1 && node[1] is Map) {
316315
for (var entry in (node[1] as Map).entries) {
317316
// key - property name or list index
318317
// value - child node
319-
dynamic child;
320-
if (obj is Control) {
321-
child = obj.properties[entry.key];
322-
} else if (obj is Map) {
323-
child = obj[entry.key];
324-
} else if (obj is List) {
325-
child = obj[entry.key];
326-
}
327-
if (child is Control) {
328-
control = child;
329-
}
330-
buildTreeIndex(control, child, entry.value);
318+
buildPathIndex(entry.value, [...path, entry.key]);
331319
}
332320
}
333321
}
334322

335-
buildTreeIndex(this, this, patch[0]);
336-
//debugPrint("TREE INDEX: $treeIndex");
323+
buildPathIndex(patch[0], []);
324+
325+
//debugPrint("PATH INDEX: $pathIndex");
326+
327+
getPatchTarget(int index) {
328+
var path = pathIndex[index]!;
329+
dynamic obj = this;
330+
Control? control = this;
331+
for (var p in path) {
332+
if (obj is Control) {
333+
obj = obj.properties[p];
334+
} else if (obj is Map) {
335+
obj = obj[p];
336+
} else if (obj is List) {
337+
obj = obj[p];
338+
}
339+
if (obj is Control) {
340+
control = obj;
341+
}
342+
}
343+
return PatchTarget(obj is Control ? obj.properties : obj, control!);
344+
}
337345

338346
// apply patch commands
339347
for (int i = 1; i < patch.length; i++) {
340348
var op = patch[i] as List<dynamic>;
341349
var opType = OperationType.fromInt(op[0]);
342350
if (opType == OperationType.replace) {
343351
// REPLACE
344-
var node = treeIndex[op[1]]!;
352+
var node = getPatchTarget(op[1]);
345353
var key = op[2];
346354
var value = op[3];
347355
node.obj[key] = _transformIfControl(value, node.control, backend);
@@ -353,7 +361,7 @@ class Control extends ChangeNotifier {
353361
}
354362
} else if (opType == OperationType.add) {
355363
// ADD
356-
var node = treeIndex[op[1]]!;
364+
var node = getPatchTarget(op[1]);
357365
var index = op[2];
358366
var value = op[3];
359367
if (node.obj is! List) {
@@ -366,7 +374,7 @@ class Control extends ChangeNotifier {
366374
}
367375
} else if (opType == OperationType.remove) {
368376
// REMOVE
369-
var node = treeIndex[op[1]]!;
377+
var node = getPatchTarget(op[1]);
370378
var index = op[2];
371379
if (node.obj is! List) {
372380
throw Exception("Remove operation can be applied to lists only: $op");
@@ -377,9 +385,9 @@ class Control extends ChangeNotifier {
377385
}
378386
} else if (opType == OperationType.move) {
379387
// MOVE
380-
var fromNode = treeIndex[op[1]]!;
388+
var fromNode = getPatchTarget(op[1]);
381389
var fromIndex = op[2];
382-
var toNode = treeIndex[op[3]]!;
390+
var toNode = getPatchTarget(op[3]);
383391
var toIndex = op[4];
384392
if (fromNode.obj is! List || toNode.obj is! List) {
385393
throw Exception("Move operation can be applied to lists only: $op");

sdk/python/packages/flet/src/flet/controls/object_patch.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,7 @@ def _item_added(self, parent, path, key, item, item_key=None, frozen=False):
458458
):
459459
self._compare_dataclasses(
460460
parent,
461-
op.location,
461+
_path_join(path, key),
462462
src,
463463
dst,
464464
frozen,
@@ -515,7 +515,7 @@ def _item_removed(self, path, key, item, item_key=None, frozen=False):
515515
):
516516
self._compare_dataclasses(
517517
dst.parent,
518-
_path_join(path, key),
518+
_path_join(op.path, op.key),
519519
src,
520520
dst,
521521
frozen,

sdk/python/packages/flet/tests/test_object_diff_frozen.py

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -861,3 +861,73 @@ def test_component_list_diff():
861861
assert c1.parent == comp
862862
assert btn1.parent == c1
863863
assert txt1.parent == comp
864+
865+
866+
def test_list_insertions_no_keys():
867+
col_1 = ft.Column(
868+
[
869+
ft.Text("Line 2", key=2),
870+
ft.Text("Line 4", key=4),
871+
ft.Text("Line 6", key=6),
872+
ft.Text("Line 8", key=8),
873+
]
874+
)
875+
col_1._frozen = True
876+
col_2 = ft.Column(
877+
[
878+
ft.Text("Line 1", key=1),
879+
ft.Text("Line 2 (updated)", key=2),
880+
ft.Text("Line 3", key=3),
881+
ft.Text("Line 4 (updated)", key=4),
882+
ft.Text("Line 5", key=5),
883+
ft.Text("Line 6 (updated)", key=6),
884+
ft.Text("Line 7", key=7),
885+
]
886+
)
887+
patch, msg, added_controls, removed_controls = make_diff(col_2, col_1)
888+
889+
assert cmp_ops(
890+
patch,
891+
[
892+
{
893+
"op": "add",
894+
"path": ["controls", 0],
895+
"value": ft.Text(value="Line 1", key=1),
896+
},
897+
{
898+
"op": "replace",
899+
"path": ["controls", 1, "value"],
900+
"value": "Line 2 (updated)",
901+
},
902+
{
903+
"op": "add",
904+
"path": ["controls", 2],
905+
"value": ft.Text(value="Line 3", key=3),
906+
},
907+
{
908+
"op": "remove",
909+
"path": ["controls", 5],
910+
"value": ft.Text(value="Line 8", key=8),
911+
},
912+
{
913+
"op": "replace",
914+
"path": ["controls", 3, "value"],
915+
"value": "Line 4 (updated)",
916+
},
917+
{
918+
"op": "add",
919+
"path": ["controls", 4],
920+
"value": ft.Text(value="Line 5", key=5),
921+
},
922+
{
923+
"op": "replace",
924+
"path": ["controls", 5, "value"],
925+
"value": "Line 6 (updated)",
926+
},
927+
{
928+
"op": "add",
929+
"path": ["controls", 6],
930+
"value": ft.Text(value="Line 7", key=7),
931+
},
932+
],
933+
)

sdk/python/packages/flet/tests/test_object_diff_in_place.py

Lines changed: 56 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
from dataclasses import field
22
from typing import Any, Optional
33

4+
from pytest import raises
5+
46
import flet as ft
57
from flet.components.component import Component
68
from flet.controls.base_control import control
@@ -39,8 +41,7 @@ class SuperButton(Button):
3941
prop_2: Optional[str] = None
4042

4143
def init(self):
42-
print("SuperButton.init()")
43-
assert not self.page
44+
pass
4445

4546
def build(self):
4647
print("SuperButton.build()")
@@ -213,7 +214,8 @@ def test_simple_page():
213214
SuperButton("Another Button"),
214215
]
215216
del page.fonts["font2"]
216-
assert page.controls[0].controls[0].page is None
217+
with raises(RuntimeError):
218+
assert page.controls[0].controls[0].page is None
217219

218220
page._user_services._services[0].prop_2 = [2, 6]
219221

@@ -291,7 +293,7 @@ def test_simple_page():
291293
{
292294
"op": "add",
293295
"path": ["views", 0, "controls", 0, "controls", 0],
294-
"value": SuperButton("Bar"),
296+
"value_type": SuperButton,
295297
},
296298
{
297299
"op": "replace",
@@ -554,3 +556,53 @@ def test_overriding_controls_with_component():
554556
page.theme_mode = ft.ThemeMode.DARK
555557
patch, _, added_controls, removed_controls = make_diff(page, show_details=True)
556558
print(patch)
559+
560+
561+
def test_list_insertions():
562+
col = ft.Column(
563+
[
564+
ft.Text("Line 2"),
565+
ft.Text("Line 4"),
566+
ft.Text("Line 6"),
567+
ft.Text("Line 8"),
568+
]
569+
)
570+
_, patch, _, _, _ = make_msg(col, {})
571+
572+
# 1st update
573+
col.controls[0] = ft.Text("Line 2 (updated)")
574+
col.controls[1] = ft.Text("Line 4 (updated)")
575+
col.controls[2] = ft.Text("Line 6 (updated)")
576+
577+
patch, _, _, _ = make_diff(col)
578+
assert cmp_ops(
579+
patch,
580+
[
581+
{"op": "replace", "path": ["controls", 0], "value_type": Text},
582+
{"op": "replace", "path": ["controls", 1], "value_type": Text},
583+
{"op": "replace", "path": ["controls", 2], "value_type": Text},
584+
],
585+
)
586+
587+
# 2nd update
588+
col.controls.insert(0, ft.Text("Line 1"))
589+
col.controls.insert(2, ft.Text("Line 3"))
590+
col.controls.insert(4, ft.Text("Line 5"))
591+
col.controls.insert(6, ft.Text("Line 7"))
592+
col.controls[3].value = "Line 4 (updated again)"
593+
594+
patch, _, _, _ = make_diff(col)
595+
assert cmp_ops(
596+
patch,
597+
[
598+
{"op": "add", "path": ["controls", 0], "value": ft.Text("Line 1")},
599+
{"op": "add", "path": ["controls", 2], "value": ft.Text("Line 3")},
600+
{
601+
"op": "replace",
602+
"path": ["controls", 3, "value"],
603+
"value": "Line 4 (updated again)",
604+
},
605+
{"op": "add", "path": ["controls", 4], "value": ft.Text("Line 5")},
606+
{"op": "add", "path": ["controls", 6], "value": ft.Text("Line 7")},
607+
],
608+
)

0 commit comments

Comments
 (0)