Repository: primefaces/primefaces (java)

Issue:
TreeTable: Swapping nodes with Collections.swap causes NPE

**Describe the defect**
If you swap items inside TreeNode#getChildren with Collections.swap, it causes an NPE with mojarra when the update="treeID" is triggered. With MyFaces NPE is only  thrown with quarkus for some reason, with jetty it doesn't throw an NPE but it doesn't seem to work either. @FlipWarthog has noticed that this is because TreeNodeChildren#.set clears the parent node  for some reason

```
java.lang.NullPointerException: Cannot invoke "org.primefaces.model.TreeNode.getRowKey()" because the return value of "org.primefaces.model.TreeNode.getParent()" is null
    at org.primefaces.component.treetable.TreeTableRenderer.encodeNode(TreeTableRenderer.java:528)
    at org.primefaces.component.treetable.TreeTableRenderer.encodeNodeChildren(TreeTableRenderer.java:849)
    at org.primefaces.component.treetable.TreeTableRenderer.encodeNodeChildren(TreeTableRenderer.java:836)
    at org.primefaces.component.treetable.TreeTableRenderer.encodeTbody(TreeTableRenderer.java:514)
    at org.primefaces.component.treetable.TreeTableRenderer.encodeRegularMarkup(TreeTableRenderer.java:386)
    at org.primefaces.component.treetable.TreeTableRenderer.encodeMarkup(TreeTableRenderer.java:262)
    at org.primefaces.component.treetable.TreeTableRenderer.render(TreeTableRenderer.java:145)
    at org.primefaces.component.treetable.TreeTableRenderer.encodeEnd(TreeTableRenderer.java:138)
    at javax.faces.component.UIComponentBase.encodeEnd(UIComponentBase.java:649)
    at javax.faces.component.UIComponentBase.encodeAll(UIComponentBase.java:530)
```

**Reproducer**
[primefaces-test.zip](https://github.com/primefaces/primefaces/files/7935585/primefaces-test.zip)


**Environment:**
 - PF Version: _11.0_
 - JSF + version: all profiles on the primefaces-test project
 - Affected browsers: _ALL_

**To Reproduce**
Steps to reproduce the behavior:
1. Run with mojarra
2. Click on 'Move up' button on the last row




Pull request:
Fix #8330: TreeTable: Swapping nodes with Collections.swap causes NPE

Patch:
diff --git a/pom.xml b/pom.xml
index c1975f24d7..412383eb3e 100644
--- a/pom.xml
+++ b/pom.xml
@@ -98,7 +98,7 @@
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-compiler-plugin</artifactId>
-                <version>3.9.0</version>
+                <version>3.10.0</version>
                 <configuration>
                     <encoding>${project.build.sourceEncoding}</encoding>
                 </configuration>
diff --git a/primefaces/src/main/java/org/primefaces/component/api/UIData.java b/primefaces/src/main/java/org/primefaces/component/api/UIData.java
index 224a616f0c..5a19ba4194 100644
--- a/primefaces/src/main/java/org/primefaces/component/api/UIData.java
+++ b/primefaces/src/main/java/org/primefaces/component/api/UIData.java
@@ -26,6 +26,7 @@ package org.primefaces.component.api;
 import java.io.IOException;
 import java.sql.ResultSet;
 import java.util.*;
+import java.util.logging.Logger;
 
 import javax.faces.FacesException;
 import javax.faces.application.Application;
@@ -61,6 +62,7 @@ import org.primefaces.util.SharedStringBuilder;
 @SuppressWarnings("unchecked")
 public class UIData extends javax.faces.component.UIData {
 
+    private static final Logger LOGGER = Logger.getLogger(UIData.class.getName());
     private static final String SB_ID = UIData.class.getName() + "#id";
 
     private final Map<String, Object> _rowTransientStates = new HashMap<>();
@@ -82,16 +84,20 @@ public class UIData extends javax.faces.component.UIData {
 
     public boolean isLazy() {
         return ComponentUtils.eval(getStateHelper(), PropertyKeys.lazy, () -> {
+            boolean lazy = false;
+
             // if not set by xhtml, we need to check the type of the value binding
             Class<?> type = ELUtils.getType(getFacesContext(),
                     getValueExpression("value"),
                     () -> getValue());
             if (type == null) {
-                throw new FacesException("Unable to automatically determine the `lazy` attribute. "
+                LOGGER.warning("Unable to automatically determine the `lazy` attribute, fallback to false. "
                         + "Either define the `lazy` attribute on the component or make sure the `value` attribute doesn't resolve to `null`. "
                         + "clientId: " + this.getClientId());
             }
-            boolean lazy = LazyDataModel.class.isAssignableFrom(type);
+            else {
+                lazy = LazyDataModel.class.isAssignableFrom(type);
+            }
 
             // remember in ViewState, to not do the same check again
             setLazy(lazy);
diff --git a/primefaces/src/main/java/org/primefaces/component/datatable/feature/FilterFeature.java b/primefaces/src/main/java/org/primefaces/component/datatable/feature/FilterFeature.java
index 6233c12c71..15552970a2 100644
--- a/primefaces/src/main/java/org/primefaces/component/datatable/feature/FilterFeature.java
+++ b/primefaces/src/main/java/org/primefaces/component/datatable/feature/FilterFeature.java
@@ -39,6 +39,7 @@ import javax.faces.event.PhaseId;
 import java.io.IOException;
 import java.util.*;
 import java.util.concurrent.atomic.AtomicBoolean;
+import org.primefaces.util.ComponentUtils;
 
 public class FilterFeature implements DataTableFeature {
 
@@ -208,7 +209,7 @@ public class FilterFeature implements DataTableFeature {
         }
 
         //Metadata for callback
-        if (table.isPaginator() || table.isVirtualScroll()) {
+        if ((table.isPaginator() || table.isVirtualScroll()) && ComponentUtils.isRequestSource(table, context)) {
             PrimeFaces.current().ajax().addCallbackParam("totalRecords", filtered.size());
         }
 
diff --git a/primefaces/src/main/java/org/primefaces/component/datatable/feature/SortFeature.java b/primefaces/src/main/java/org/primefaces/component/datatable/feature/SortFeature.java
index 980a57f9b8..b02dc6d7b7 100644
--- a/primefaces/src/main/java/org/primefaces/component/datatable/feature/SortFeature.java
+++ b/primefaces/src/main/java/org/primefaces/component/datatable/feature/SortFeature.java
@@ -40,6 +40,7 @@ import java.util.concurrent.atomic.AtomicInteger;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 import javax.el.ValueExpression;
+import org.primefaces.util.ComponentUtils;
 
 public class SortFeature implements DataTableFeature {
 
@@ -120,7 +121,7 @@ public class SortFeature implements DataTableFeature {
 
             sort(context, table);
 
-            if (table.isPaginator()) {
+            if (table.isPaginator() && ComponentUtils.isRequestSource(table, context)) {
                 PrimeFaces.current().ajax().addCallbackParam("totalRecords", table.getRowCount());
             }
 
diff --git a/primefaces/src/main/java/org/primefaces/component/dataview/DataView.java b/primefaces/src/main/java/org/primefaces/component/dataview/DataView.java
index 9c2a995365..7f9020bb67 100644
--- a/primefaces/src/main/java/org/primefaces/component/dataview/DataView.java
+++ b/primefaces/src/main/java/org/primefaces/component/dataview/DataView.java
@@ -158,7 +158,7 @@ public class DataView extends DataViewBase {
             lazyModel.setWrappedData(data);
 
             //Update paginator
-            if (ComponentUtils.isRequestSource(this, getFacesContext()) && isPaginator() ) {
+            if (ComponentUtils.isRequestSource(this, getFacesContext()) && isPaginator()) {
                 PrimeFaces.current().ajax().addCallbackParam("totalRecords", lazyModel.getRowCount());
             }
         }
diff --git a/primefaces/src/main/java/org/primefaces/component/treetable/feature/FilterFeature.java b/primefaces/src/main/java/org/primefaces/component/treetable/feature/FilterFeature.java
index 085483316d..19390f295a 100644
--- a/primefaces/src/main/java/org/primefaces/component/treetable/feature/FilterFeature.java
+++ b/primefaces/src/main/java/org/primefaces/component/treetable/feature/FilterFeature.java
@@ -44,6 +44,7 @@ import org.primefaces.component.treetable.TreeTableState;
 import org.primefaces.model.CheckboxTreeNode;
 import org.primefaces.model.DefaultTreeNode;
 import org.primefaces.model.TreeNode;
+import org.primefaces.util.ComponentUtils;
 import org.primefaces.util.LocaleUtils;
 
 public class FilterFeature implements TreeTableFeature {
@@ -136,11 +137,13 @@ public class FilterFeature implements TreeTableFeature {
         tt.setRowKey(root, null);
 
         //Metadata for callback
-        if (tt.isPaginator()) {
-            PrimeFaces.current().ajax().addCallbackParam("totalRecords", filteredValue.getChildCount());
-        }
-        if (tt.getSelectedRowKeysAsString() != null) {
-            PrimeFaces.current().ajax().addCallbackParam("selection", tt.getSelectedRowKeysAsString());
+        if (ComponentUtils.isRequestSource(tt, context)) {
+            if (tt.isPaginator()) {
+                PrimeFaces.current().ajax().addCallbackParam("totalRecords", filteredValue.getChildCount());
+            }
+            if (tt.getSelectedRowKeysAsString() != null) {
+                PrimeFaces.current().ajax().addCallbackParam("selection", tt.getSelectedRowKeysAsString());
+            }
         }
     }
 
diff --git a/primefaces/src/main/java/org/primefaces/component/treetable/feature/SelectionFeature.java b/primefaces/src/main/java/org/primefaces/component/treetable/feature/SelectionFeature.java
index 74fa5f723f..d1fed4e4f7 100644
--- a/primefaces/src/main/java/org/primefaces/component/treetable/feature/SelectionFeature.java
+++ b/primefaces/src/main/java/org/primefaces/component/treetable/feature/SelectionFeature.java
@@ -33,6 +33,7 @@ import org.primefaces.PrimeFaces;
 import org.primefaces.component.treetable.TreeTable;
 import org.primefaces.component.treetable.TreeTableRenderer;
 import org.primefaces.model.TreeNode;
+import org.primefaces.util.ComponentUtils;
 import org.primefaces.util.SharedStringBuilder;
 
 public class SelectionFeature implements TreeTableFeature {
@@ -109,7 +110,9 @@ public class SelectionFeature implements TreeTableFeature {
                 builder.append(descendantRowKeys.get(i));
             }
 
-            PrimeFaces.current().ajax().addCallbackParam("descendantRowKeys", builder.toString());
+            if (ComponentUtils.isRequestSource(table, context)) {
+                PrimeFaces.current().ajax().addCallbackParam("descendantRowKeys", builder.toString());
+            }
         }
     }
 
diff --git a/primefaces/src/main/java/org/primefaces/component/treetable/feature/SortFeature.java b/primefaces/src/main/java/org/primefaces/component/treetable/feature/SortFeature.java
index 4af0db92b1..570c3c5a7e 100644
--- a/primefaces/src/main/java/org/primefaces/component/treetable/feature/SortFeature.java
+++ b/primefaces/src/main/java/org/primefaces/component/treetable/feature/SortFeature.java
@@ -38,6 +38,7 @@ import java.util.stream.IntStream;
 import org.primefaces.component.treetable.TreeTable;
 import org.primefaces.component.treetable.TreeTableRenderer;
 import org.primefaces.component.treetable.TreeTableState;
+import org.primefaces.util.ComponentUtils;
 
 public class SortFeature implements TreeTableFeature {
 
@@ -109,11 +110,11 @@ public class SortFeature implements TreeTableFeature {
         renderer.encodeTbody(context, table, table.getValue(), true);
 
         String selectedRowKeys = table.getSelectedRowKeysAsString();
-        if (selectedRowKeys != null) {
+        if (selectedRowKeys != null && ComponentUtils.isRequestSource(table, context)) {
             PrimeFaces.current().ajax().addCallbackParam("selection", selectedRowKeys);
         }
 
-        if (table.isPaginator()) {
+        if (table.isPaginator() && ComponentUtils.isRequestSource(table, context)) {
             PrimeFaces.current().ajax().addCallbackParam("totalRecords", table.getRowCount());
         }
 
@@ -185,7 +186,7 @@ public class SortFeature implements TreeTableFeature {
             });
 
             for (int i = 0; i < childrenArray.length; i++) {
-                children.setSibling(i, (TreeNode) childrenArray[i]);
+                children.set(i, (TreeNode) childrenArray[i]);
             }
             for (int i = 0; i < children.size(); i++) {
                 sortNode(table, sortBy, comparisonResult, children.get(i), context, var, locale, collator);
diff --git a/primefaces/src/main/java/org/primefaces/model/CheckboxTreeNodeChildren.java b/primefaces/src/main/java/org/primefaces/model/CheckboxTreeNodeChildren.java
index 5effcbaa86..d341a9ed61 100644
--- a/primefaces/src/main/java/org/primefaces/model/CheckboxTreeNodeChildren.java
+++ b/primefaces/src/main/java/org/primefaces/model/CheckboxTreeNodeChildren.java
@@ -150,35 +150,6 @@ public class CheckboxTreeNodeChildren<T> extends TreeNodeList<T> {
         }
     }
 
-    /**
-     * Optimized set implementation to be used in sorting
-     *
-     * @param index index of the element to replace
-     * @param node node to be stored at the specified position
-     * @return the node previously at the specified position
-     */
-    @Override
-    public TreeNode setSibling(int index, TreeNode node) {
-        if (node == null) {
-            throw new NullPointerException();
-        }
-        else if ((index < 0) || (index >= size())) {
-            throw new IndexOutOfBoundsException();
-        }
-        else {
-            if (!parent.equals(node.getParent())) {
-                eraseParent(node);
-            }
-
-            TreeNode previous = get(index);
-            super.set(index, node);
-            node.setParent(parent);
-            updateRowKeys(parent);
-            updateSelectionState(parent);
-            return previous;
-        }
-    }
-
     @Override
     public TreeNode remove(int index) {
         TreeNode node = get(index);
diff --git a/primefaces/src/main/java/org/primefaces/model/TreeNodeChildren.java b/primefaces/src/main/java/org/primefaces/model/TreeNodeChildren.java
index faa43771fa..9fa05402c8 100644
--- a/primefaces/src/main/java/org/primefaces/model/TreeNodeChildren.java
+++ b/primefaces/src/main/java/org/primefaces/model/TreeNodeChildren.java
@@ -135,34 +135,6 @@ public class TreeNodeChildren<T> extends TreeNodeList<T> {
             eraseParent(node);
         }
 
-        TreeNode previous = get(index);
-        super.set(index, node);
-        previous.setParent(null);
-        node.setParent(parent);
-        updateRowKeys(parent, node, index);
-        return previous;
-    }
-
-    /**
-     * Optimized set implementation to be used in sorting
-     *
-     * @param index index of the element to replace
-     * @param node node to be stored at the specified position
-     * @return the node previously at the specified position
-     */
-    @Override
-    public TreeNode setSibling(int index, TreeNode node) {
-        if (node == null) {
-            throw new NullPointerException();
-        }
-        if (index < 0 || index >= size()) {
-            throw new IndexOutOfBoundsException();
-        }
-
-        if (!parent.equals(node.getParent())) {
-            eraseParent(node);
-        }
-
         TreeNode previous = get(index);
         super.set(index, node);
         node.setParent(parent);
diff --git a/primefaces/src/main/java/org/primefaces/model/TreeNodeList.java b/primefaces/src/main/java/org/primefaces/model/TreeNodeList.java
index 71425d6556..68e240cb60 100644
--- a/primefaces/src/main/java/org/primefaces/model/TreeNodeList.java
+++ b/primefaces/src/main/java/org/primefaces/model/TreeNodeList.java
@@ -29,14 +29,4 @@ public class TreeNodeList<T> extends ArrayList<TreeNode<T>> {
 
     private static final long serialVersionUID = 1L;
 
-    /**
-     * Optimized set implementation to be used in sorting
-     *
-     * @param index index of the element to replace
-     * @param node node to be stored at the specified position
-     * @return the node previously at the specified position
-     */
-    public TreeNode<T> setSibling(int index, TreeNode<T> node) {
-        throw new UnsupportedOperationException();
-    }
 }


Test patch:
diff --git a/primefaces/src/test/java/org/primefaces/model/TreeNodeChildrenTest.java b/primefaces/src/test/java/org/primefaces/model/TreeNodeChildrenTest.java
new file mode 100644
index 0000000000..f4f43a8cbe
--- /dev/null
+++ b/primefaces/src/test/java/org/primefaces/model/TreeNodeChildrenTest.java
@@ -0,0 +1,62 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2009-2021 PrimeTek
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package org.primefaces.model;
+
+import java.util.Collections;
+import java.util.List;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public class TreeNodeChildrenTest {
+
+    @Test
+    public void swapRetainsParent() {
+        // Arrange
+        TreeNode<String> root = new DefaultTreeNode<>("Parent", null);
+        TreeNode<String> child1 = new DefaultTreeNode<>("Child1", root);
+        TreeNode<String> child2 = new DefaultTreeNode<>("Child2", root);
+
+        List<TreeNode<String>> values = new TreeNodeChildren<>(root);
+
+        values.add(child1);
+        values.add(child2);
+
+        // Pre-assert
+        assertEquals(child1, values.get(0));
+        assertEquals(root, values.get(0).getParent());
+        assertEquals(child2, values.get(1));
+        assertEquals(root, values.get(1).getParent());
+
+        // Act
+        Collections.swap(values, 0, 1);
+
+        // Assert
+        assertEquals(child2, values.get(0));
+        assertEquals(root, values.get(0).getParent());
+        assertEquals(child1, values.get(1));
+        assertEquals(root, values.get(1).getParent());
+    }
+
+}