You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@myfaces.apache.org by ta...@apache.org on 2026/07/30 07:23:21 UTC
(myfaces) branch main updated: Key UIData/UIRepeat per-row EVH state by clientId, not row index (#1034)
This is an automated email from the ASF dual-hosted git repository.
tandraschko pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/myfaces.git
The following commit(s) were added to refs/heads/main by this push:
new c65bda2e5 Key UIData/UIRepeat per-row EVH state by clientId, not row index (#1034)
c65bda2e5 is described below
commit c65bda2e57d4423af6222cea490672ca90467727
Author: Bauke Scholtz <ba...@gmail.com>
AuthorDate: Thu Jul 30 03:23:16 2026 -0400
Key UIData/UIRepeat per-row EVH state by clientId, not row index (#1034)
A nested iterating component is a single instance reused across the
enclosing component's rows, so keying its per-row EditableValueHolder
state by the bare row index made every enclosing row map to the same
key and overwrite the previous one. Only the last enclosing row's
submitted values survived: the rest were dropped silently, without
conversion, validation, model update or a message.
Key on getContainerClientId() instead, which is this component's
clientId plus its current row index and therefore carries the
enclosing rows' indices too. That is what _rowDeltaStates and
_rowTransientStates key on, and what _rowStates itself keyed on
before 6336caf1d.
The positional EditableValueHolderState list, its deferred allocation
and the flat iteration lists are unchanged; only the map key differs.
Mojarra hit the same defect and resolved it the same way, see
https://github.com/eclipse-ee4j/mojarra/issues/5874
Co-authored-by: Claude Opus 5 (1M context) <no...@anthropic.com>
---
.../main/java/jakarta/faces/component/UIData.java | 21 +++----
.../myfaces/view/facelets/component/UIRepeat.java | 20 +++----
.../java/jakarta/faces/component/UIDataTest.java | 66 ++++++++++++++++++++++
.../view/facelets/component/UIRepeatTest.java | 58 +++++++++++++++++++
4 files changed, 145 insertions(+), 20 deletions(-)
diff --git a/api/src/main/java/jakarta/faces/component/UIData.java b/api/src/main/java/jakarta/faces/component/UIData.java
index 2bb009725..ea76286d2 100644
--- a/api/src/main/java/jakarta/faces/component/UIData.java
+++ b/api/src/main/java/jakarta/faces/component/UIData.java
@@ -166,13 +166,13 @@ public class UIData extends UIComponentBase implements NamingContainer, UniqueId
private int _rowIndex = -1;
/**
- * Per-row EVH state. Outer key is the row index; value is the list of per-EVH states
- * indexed by position in {@link #_iterationEVHList} (null entry = component in default
- * empty state). A missing outer entry also means all EVH components are in default state.
- * Using an Integer row-index key avoids string concatenation and leverages the JVM Integer
- * cache for typical row counts, making map operations cheaper than string-key alternatives.
+ * Per-row EVH state. Outer key is {@link #getContainerClientId(FacesContext)}, i.e. this table's clientId plus
+ * its current row index, which for a table nested inside another iterating component also carries the enclosing
+ * rows' indices; value is the list of per-EVH states indexed by position in {@link #_iterationEVHList} (null
+ * entry = component in default empty state). A missing outer entry also means all EVH components are in default
+ * state.
*/
- private Map<Integer, List<EditableValueHolderState>> _rowStates = new HashMap<>();
+ private Map<String, List<EditableValueHolderState>> _rowStates = new HashMap<>();
private Map<String, Map<String, Object>> _rowDeltaStates = new HashMap<>();
private Map<String, Map<String, Object>> _rowTransientStates = new HashMap<>();
@@ -765,13 +765,14 @@ private void saveChildStates()
states.add(state);
}
}
+ String key = getContainerClientId(getFacesContext());
if (states != null)
{
- _rowStates.put(_rowIndex, states);
+ _rowStates.put(key, states);
}
else
{
- _rowStates.remove(_rowIndex);
+ _rowStates.remove(key);
}
}
@@ -789,7 +790,7 @@ private void restoreChildStates()
{
return;
}
- List<EditableValueHolderState> states = _rowStates.get(_rowIndex);
+ List<EditableValueHolderState> states = _rowStates.get(getContainerClientId(getFacesContext()));
for (int i = 0, n = evhList.size(); i < n; i++)
{
EditableValueHolderState state = (states != null && i < states.size()) ? states.get(i) : null;
@@ -1228,7 +1229,7 @@ public void restoreState(FacesContext context, Object state)
}
else
{
- _rowStates = (Map<Integer, List<EditableValueHolderState>>) rs;
+ _rowStates = (Map<String, List<EditableValueHolderState>>) rs;
}
}
if (values.length > 3)
diff --git a/impl/src/main/java/org/apache/myfaces/view/facelets/component/UIRepeat.java b/impl/src/main/java/org/apache/myfaces/view/facelets/component/UIRepeat.java
index 099c83e40..dcaf525dd 100644
--- a/impl/src/main/java/org/apache/myfaces/view/facelets/component/UIRepeat.java
+++ b/impl/src/main/java/org/apache/myfaces/view/facelets/component/UIRepeat.java
@@ -103,13 +103,13 @@ public class UIRepeat extends UIComponentBase implements NamingContainer
protected Map<String, Map<String, Object>> _rowTransientStates = new HashMap<>();
/**
- * Per-row EVH state. Outer key is the row index; value is the list of per-EVH states
- * indexed by position in {@link #_iterationEVHList} (null entry = component in default
- * empty state). A missing outer entry also means all EVH components are in default state.
- * Using an Integer row-index key avoids string concatenation and leverages the JVM Integer
- * cache for typical row counts, making map operations cheaper than string-key alternatives.
+ * Per-row EVH state. Outer key is {@link #getContainerClientId(FacesContext)}, i.e. this repeat's clientId plus
+ * its current row index, which for a repeat nested inside another iterating component also carries the enclosing
+ * rows' indices; value is the list of per-EVH states indexed by position in {@link #_iterationEVHList} (null
+ * entry = component in default empty state). A missing outer entry also means all EVH components are in default
+ * state.
*/
- private Map<Integer, List<EditableValueHolderState>> _rowStates = new HashMap<>();
+ private Map<String, List<EditableValueHolderState>> _rowStates = new HashMap<>();
/**
* Handle case where this table is nested inside another table. See method getDataModel for more details.
@@ -769,11 +769,11 @@ private void saveChildStates()
}
if (hasState)
{
- _rowStates.put(_index, states);
+ _rowStates.put(getContainerClientId(getFacesContext()), states);
}
else
{
- _rowStates.remove(_index);
+ _rowStates.remove(getContainerClientId(getFacesContext()));
}
}
@@ -791,7 +791,7 @@ private void restoreChildStates()
{
return;
}
- List<EditableValueHolderState> states = _rowStates.get(_index);
+ List<EditableValueHolderState> states = _rowStates.get(getContainerClientId(getFacesContext()));
for (int i = 0, n = evhList.size(); i < n; i++)
{
EditableValueHolderState state = (states != null && i < states.size()) ? states.get(i) : null;
@@ -1794,7 +1794,7 @@ public void restoreState(FacesContext context, Object state)
}
else
{
- _rowStates = (Map<Integer, List<EditableValueHolderState>>) rs;
+ _rowStates = (Map<String, List<EditableValueHolderState>>) rs;
}
}
if (values.length > 3)
diff --git a/impl/src/test/java/jakarta/faces/component/UIDataTest.java b/impl/src/test/java/jakarta/faces/component/UIDataTest.java
index 6108dbc3f..8e331b050 100644
--- a/impl/src/test/java/jakarta/faces/component/UIDataTest.java
+++ b/impl/src/test/java/jakarta/faces/component/UIDataTest.java
@@ -1109,6 +1109,72 @@ public void testStatefulNestedEVHStateIsPreserved()
"Nested UIInput row-1 state must be restored");
}
+ /**
+ * A UIData nested inside another UIData keeps its per-row state per enclosing row: the inner table's row 0 in
+ * outer row 0 is a different cell from the inner table's row 0 in outer row 1. Keying the inner table's state by
+ * its own row index alone lets each enclosing row overwrite the previous one, which silently drops the submitted
+ * values of every enclosing row but the last.
+ */
+ @Test
+ public void testNestedTableRowStateIsScopedPerOuterRow()
+ {
+ UIViewRoot root = facesContext.getViewRoot();
+
+ UIData outer = new UIData();
+ outer.setId("outer");
+ outer.setVar("group");
+ outer.setValue(java.util.Arrays.asList("g0", "g1"));
+
+ UIColumn outerColumn = new UIColumn();
+ outerColumn.setId("outerCol");
+
+ UIData inner = new UIData();
+ inner.setId("inner");
+ inner.setVar("row");
+ inner.setValue(java.util.Arrays.asList("r0", "r1"));
+
+ UIColumn innerColumn = new UIColumn();
+ innerColumn.setId("innerCol");
+ UIInput input = new UIInput();
+ input.setId("inp");
+
+ innerColumn.getChildren().add(input);
+ inner.getChildren().add(innerColumn);
+ outerColumn.getChildren().add(inner);
+ outer.getChildren().add(outerColumn);
+ root.getChildren().add(outer);
+
+ facesContext.setCurrentPhaseId(PhaseId.APPLY_REQUEST_VALUES);
+
+ // Submit a distinct value into every cell of the 2x2 grid.
+ for (int outerRow = 0; outerRow < 2; outerRow++)
+ {
+ outer.setRowIndex(outerRow);
+ for (int innerRow = 0; innerRow < 2; innerRow++)
+ {
+ inner.setRowIndex(innerRow);
+ input.setSubmittedValue("v" + outerRow + innerRow);
+ }
+ inner.setRowIndex(-1);
+ }
+ outer.setRowIndex(-1);
+
+ // Every cell must still hold its own value.
+ for (int outerRow = 0; outerRow < 2; outerRow++)
+ {
+ outer.setRowIndex(outerRow);
+ for (int innerRow = 0; innerRow < 2; innerRow++)
+ {
+ inner.setRowIndex(innerRow);
+ Assertions.assertEquals("v" + outerRow + innerRow, input.getSubmittedValue(),
+ "outer row " + outerRow + ", inner row " + innerRow
+ + " must keep the value submitted into it");
+ }
+ inner.setRowIndex(-1);
+ }
+ outer.setRowIndex(-1);
+ }
+
// -------------------------------------------------------------------------
// Read-only (no EVH) flat-list clientId reset
// -------------------------------------------------------------------------
diff --git a/impl/src/test/java/org/apache/myfaces/view/facelets/component/UIRepeatTest.java b/impl/src/test/java/org/apache/myfaces/view/facelets/component/UIRepeatTest.java
index 739de8a4f..4388644cb 100644
--- a/impl/src/test/java/org/apache/myfaces/view/facelets/component/UIRepeatTest.java
+++ b/impl/src/test/java/org/apache/myfaces/view/facelets/component/UIRepeatTest.java
@@ -224,6 +224,64 @@ public void testSaveAndRestorePortletLifecycleWithoutPss1() throws Exception
}
}
+ /**
+ * A UIRepeat nested inside another UIRepeat keeps its per-row state per enclosing row: the inner repeat's row 0
+ * in outer row 0 is a different cell from the inner repeat's row 0 in outer row 1. Keying the inner repeat's
+ * state by its own row index alone lets each enclosing row overwrite the previous one, which silently drops the
+ * submitted values of every enclosing row but the last.
+ */
+ @Test
+ public void testNestedRepeatRowStateIsScopedPerOuterRow()
+ {
+ UIViewRoot root = facesContext.getViewRoot();
+ root.setId(root.createUniqueId());
+
+ UIRepeat outer = new UIRepeat();
+ outer.setId(root.createUniqueId());
+ outer.setVar("group");
+ outer.setValue(Arrays.asList("g0", "g1"));
+
+ UIRepeat inner = new UIRepeat();
+ inner.setId(root.createUniqueId());
+ inner.setVar("row");
+ inner.setValue(Arrays.asList("r0", "r1"));
+
+ UIInput input = new UIInput();
+ input.setId(root.createUniqueId());
+
+ inner.getChildren().add(input);
+ outer.getChildren().add(inner);
+ root.getChildren().add(outer);
+
+ facesContext.setCurrentPhaseId(PhaseId.APPLY_REQUEST_VALUES);
+
+ for (int outerRow = 0; outerRow < 2; outerRow++)
+ {
+ outer.setRowIndex(outerRow);
+ for (int innerRow = 0; innerRow < 2; innerRow++)
+ {
+ inner.setRowIndex(innerRow);
+ input.setSubmittedValue("v" + outerRow + innerRow);
+ }
+ inner.setRowIndex(-1);
+ }
+ outer.setRowIndex(-1);
+
+ for (int outerRow = 0; outerRow < 2; outerRow++)
+ {
+ outer.setRowIndex(outerRow);
+ for (int innerRow = 0; innerRow < 2; innerRow++)
+ {
+ inner.setRowIndex(innerRow);
+ Assertions.assertEquals("v" + outerRow + innerRow, input.getSubmittedValue(),
+ "outer row " + outerRow + ", inner row " + innerRow
+ + " must keep the value submitted into it");
+ }
+ inner.setRowIndex(-1);
+ }
+ outer.setRowIndex(-1);
+ }
+
// -------------------------------------------------------------------------
// Helpers
// -------------------------------------------------------------------------