fix: Avoid NPE when tracking usage for removed nodes (#21739)

The change value cannot be extracted if there's no data node in the
tree. Check for this both when creating the usage instance and when
later evaluating whether it has changes.

Note that removing a node is not seen as changing the node itself but
only as a change to the node's parent.

Author: Legioth

signals/src/main/java/com/vaadin/signals/Signal.java

@@ -1,6 +1,7 @@
 package com.vaadin.signals;
 
 import java.util.Objects;
+import java.util.Optional;
 import java.util.concurrent.Executor;
 import java.util.function.Function;
 import java.util.function.Predicate;
@@ -219,6 +220,10 @@ protected Predicate<SignalCommand> mergeValidators(
      */
     protected abstract Object usageChangeValue(Data data);
 
+    private Optional<Object> usageChangeValue(Transaction transaction) {
+        return transaction.read(tree).data(id).map(this::usageChangeValue);
+    }
+
     boolean isValid(SignalCommand command) {
         if (command instanceof SignalCommand.ConditionCommand) {
             return true;
@@ -384,14 +389,22 @@ protected SignalTree tree() {
      * @return a usage instance, not <code>null</code>
      */
     protected Usage createUsage(Transaction transaction) {
+        Optional<Object> usageChangeValue = usageChangeValue(transaction);
+        if (usageChangeValue.isEmpty()) {
+            // Node is removed so no usage to track
+            return UsageTracker.NO_USAGE;
+        }
+
         // Capture so that we can use it later
-        Object originalValue = usageChangeValue(data(transaction));
+        Object originalValue = usageChangeValue.get();
 
         return new Usage() {
             @Override
             public boolean hasChanges() {
-                return !Objects.equals(originalValue,
-                        usageChangeValue(data(Transaction.getCurrent())));
+                return usageChangeValue(Transaction.getCurrent())
+                        .map(changeValue -> !Objects.equals(originalValue,
+                                changeValue))
+                        .orElse(Boolean.FALSE);
             }
 
             @Override

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

@@ -116,7 +116,10 @@ public interface Usage {
         Runnable onNextChange(TransientListener listener);
     }
 
-    private static final Usage NO_USAGE = new Usage() {
+    /**
+     * A usage that doesn't have any changes and never fires any events.
+     */
+    public static final Usage NO_USAGE = new Usage() {
         @Override
         public boolean hasChanges() {
             return false;

signals/src/test/java/com/vaadin/signals/ValueSignalTest.java

@@ -453,6 +453,34 @@ void usageTracking_registerAfterChange_listenerCalledImmediately() {
         assertEquals(2, trueCount.intValue());
     }
 
+    @Test
+    void usageTracking_removeSignalAfterTracking_hasNoChanges() {
+        ListSignal<String> list = new ListSignal<>(String.class);
+        ValueSignal<String> signal = list.insertLast("value").signal();
+
+        Usage usage = UsageTracker.track(() -> {
+            signal.value();
+        });
+
+        list.remove(signal);
+
+        assertFalse(usage.hasChanges());
+    }
+
+    @Test
+    void usageTracking_removeSignalBeforeTracking_hasNoChanges() {
+        ListSignal<String> list = new ListSignal<>(String.class);
+        ValueSignal<String> signal = list.insertLast("value").signal();
+
+        list.remove(signal);
+
+        Usage usage = UsageTracker.track(() -> {
+            signal.value();
+        });
+
+        assertFalse(usage.hasChanges());
+    }
+
     @Test
     void result_successfulOperation_resolvedThroughOverrideDispatcher() {
         TestExecutor dispatcher = useTestOverrideDispatcher();