fix: Allow observing other nodes from an observer (#21653) (#21657)

Fixes #21603

Co-authored-by: Leif Åstrand <leif@vaadin.com>

Author: vaadin-bot

signals/src/main/java/com/vaadin/signals/impl/SignalTree.java

@@ -234,7 +234,7 @@ protected void notifyObservers(Snapshot oldSnapshot, Snapshot newSnapshot) {
         }
 
         runWithLock(() -> {
-            observers.forEach((nodeId, list) -> {
+            Map.copyOf(observers).forEach((nodeId, list) -> {
                 Data oldNode = oldSnapshot.data(nodeId).orElse(Node.EMPTY);
                 Data newNode = newSnapshot.data(nodeId).orElse(Node.EMPTY);
 

signals/src/test/java/com/vaadin/signals/impl/EffectTest.java

@@ -267,6 +267,27 @@ void changeTracking_noOpChange_effectNotRunButRemainsActive() {
         assertEquals(List.of("value", "update"), invocations);
     }
 
+    @Test
+    void changeTracking_readChildNodes_coveredByNextEffectInvocation() {
+        ListSignal<String> signal = new ListSignal<>(String.class);
+        ArrayList<List<String>> invocations = new ArrayList<>();
+
+        Signal.effect(() -> {
+            List<String> values = signal.value().stream().map(Signal::value)
+                    .toList();
+            invocations.add(values);
+        });
+
+        assertEquals(List.of(List.of()), invocations);
+
+        signal.insertLast("One");
+        assertEquals(List.of(List.of(), List.of("One")), invocations);
+
+        signal.insertLast("Two");
+        assertEquals(List.of(List.of(), List.of("One"), List.of("One", "Two")),
+                invocations);
+    }
+
     @Test
     void callback_updateSignal_throws() {
         ValueSignal<String> signal = new ValueSignal<>("value");

signals/src/test/java/com/vaadin/signals/impl/SynchronousSignalTreeTest.java

@@ -350,6 +350,37 @@ void observe_observeInCallback_registeredAgain() {
         assertEquals(1, count.get());
     }
 
+    @Test
+    void observe_observeAnotherNodeInCallback_observerAdded() {
+        SynchronousSignalTree tree = new SynchronousSignalTree(false);
+
+        Id childId = Id.random();
+        AtomicInteger count = new AtomicInteger();
+        tree.observeNextChange(Id.ZERO, () -> {
+            tree.observeNextChange(childId, () -> {
+                count.incrementAndGet();
+                return false;
+            });
+            return false;
+        });
+
+        tree.commitSingleCommand(new SignalCommand.InsertCommand(childId,
+                Id.ZERO, null, new DoubleNode(2), ListPosition.last()));
+
+        // Nothing yet since root observer not invoked
+        assertEquals(0, count.get());
+
+        tree.commitSingleCommand(TestUtil.writeRootValueCommand());
+
+        // Nothing yet since child observer not invoked
+        assertEquals(0, count.get());
+
+        tree.commitSingleCommand(new SignalCommand.SetCommand(Id.random(),
+                childId, new DoubleNode(3)));
+
+        assertEquals(1, count.get());
+    }
+
     @Test
     void subscribeToProcessed_noChanges_doesNotReceive() {
         SynchronousSignalTree tree = new SynchronousSignalTree(false);