Bauke Scholtz created MYFACES-4778:
--------------------------------------
Summary: UIData and UIRepeat restore per-row state which is stale
or shared with an earlier restore
Key: MYFACES-4778
URL: https://issues.apache.org/jira/browse/MYFACES-4778
Project: MyFaces Core
Issue Type: Bug
Affects Versions: 4.1.5
Reporter: Bauke Scholtz
{{UIData}} and {{UIRepeat}} restore per-row state which does not belong to the
view being restored, in two cases. Both are covered by {{Issue6129IT}} of the
Jakarta Faces 5.0 TCK
([jakartaee/faces#2266|https://github.com/jakartaee/faces/pull/2266]), and
MyFaces 4.1.5-SNAPSHOT fails both tests on an equivalent page. Mojarra had both
bugs too, and fixed them in 4.0.30 and 4.1.21
([eclipse-ee4j/mojarra#6129|https://github.com/eclipse-ee4j/mojarra/issues/6129]).
h3. 1. With rowStatePreserved="true", the local value of a row is kept after
Update Model Values
With {{rowStatePreserved="true"}}, an {{<h:dataTable>}} or {{<ui:repeat>}}
keeps the local value of every row input in its saved row state, also after a
postback which passed validation and updated the model. That stale local value
then hides every later change of the model in that row, as long as the row is
not executed again.
h4. Reproducer
{code:xml}
<h:form id="form">
<h:dataTable id="preserved" value="#{bean.preserved}" var="i"
rowStatePreserved="true">
<h:column><h:inputText id="in" value="#{i.name}" /></h:column>
</h:dataTable>
<ui:repeat id="repeated" value="#{bean.repeated}" var="i"
rowStatePreserved="true">
<h:inputText id="in" value="#{i.name}" />
</ui:repeat>
<h:dataTable id="plain" value="#{bean.plain}" var="i">
<h:column><h:inputText id="in" value="#{i.name}" /></h:column>
</h:dataTable>
<h:commandButton id="submit" value="Submit" />
<h:commandButton id="change" value="Change">
<f:ajax execute="@this" render="@form" listener="#{bean.change}" />
</h:commandButton>
</h:form>
{code}
{code:java}
@Named
@ViewScoped
public class Bean implements Serializable {
private List<Item> preserved = List.of(new Item("original a"), new
Item("original b"));
private List<Item> repeated = List.of(new Item("original e"), new
Item("original f"));
private List<Item> plain = List.of(new Item("original c"), new
Item("original d"));
public void change(AjaxBehaviorEvent event) {
preserved.forEach(item -> item.setName("changed"));
repeated.forEach(item -> item.setName("changed"));
plain.forEach(item -> item.setName("changed"));
}
// Getters, and Item with a name property.
}
{code}
# Type a new value in every input and click Submit. Validation passes, and the
model holds the typed values.
# Click Change. The ajax request sets the name of every item to {{changed}} and
re-renders the form. It does not execute the tables.
h4. Expected
All six inputs show {{changed}}.
h4. Actual
The {{plain}} table shows {{changed}}. The {{preserved}} table and the
{{repeated}} repeat still show the typed values, although the model holds
{{changed}}.
Reproduced on MyFaces 4.1.5-SNAPSHOT on Tomcat 11, with server side state
saving. The view state after step 1 shows the cause: the {{rowDeltaStates}} of
the table still hold {{value}} and {{localValueSet = true}} for each row input.
h4. Cause
With {{rowStatePreserved="true"}},
{{UIData.setRowIndexPreserveComponentState()}} saves the state of a row when
the table leaves that row. {{child.saveState()}} returns the difference between
the current state and the initial state, so this is a delta:
{code:java}
Map<String, Object> sm = saveFullDescendantComponentStates(facesContext, null,
getChildren().iterator(), false);
if (sm != null && !sm.isEmpty())
{
_rowDeltaStates.put(getContainerClientId(facesContext), sm);
}
{code}
The delta is only stored when it is not empty, and an entry is never removed:
# Process Validations leaves a local value in each row input. When the table
leaves the row, the delta holds {{value}} and {{localValueSet}}, and it is
stored.
# Update Model Values clears the local value. The input is back in its initial
state.
# When the table leaves the row again, the delta is empty. It is not stored,
but the entry of step 1 is not removed either.
So the entry of step 1 is saved with the view. Every later request restores
that local value into the row, and there it takes precedence over the model.
{{UIRepeat}} has the same {{if (sm != null && !sm.isEmpty())}} when it saves a
row. Both are the same on {{main}}.
h4. Spec
The javadoc of {{UIInput.updateModel()}}:
{quote}
If the {{setValue()}} method returns successfully:
* Clear the local value of this {{UIInput}}.
* Set the {{localValueSet}} property of this {{UIInput}} to false.
{quote}
The restore puts back a local value which Update Model Values has cleared.
The javadoc of {{UIData.setRowStatePreserved()}}:
{quote}
To accomplish this, {{UIData}} must call {{StateHolder#saveState}} and
{{TransientStateHolder#saveTransientState}} on its children to capture their
state on exiting each row. When re-entering the row,
{{StateHolder#restoreState}} and {{TransientStateHolder#restoreTransientState}}
must be called in order to reinitialize the children to the correct state for
the new row.
{quote}
The restored state is the one of an earlier exit of the row, not of the last
one.
h4. Proposed fix
When the table leaves a row and the delta of that row is empty, remove the
entry of the row from {{_rowDeltaStates}}. The same in {{UIRepeat}}.
{code:java}
if (sm != null && !sm.isEmpty())
{
_rowDeltaStates.put(getContainerClientId(facesContext), sm);
}
else
{
_rowDeltaStates.remove(getContainerClientId(facesContext));
}
{code}
TCK: {{Issue6129IT.testRowStatePreservedDoesNotKeepLocalValueAfterUpdateModel}}.
h3. 2. Two restores of one saved state share the per-row values
With server side state saving, the saved state is kept in the session without
serializing it, as {{jakarta.faces.SERIALIZE_SERVER_STATE}} is {{false}} by
default. A request which restores that saved state changes the per-row values
in it. A later request which restores the same saved state again, such as after
the back button or in a second tab, then sees the per-row values of the request
before.
h4. Reproducer
{code:xml}
<h:form id="form">
<h:inputText id="required" value="#{bean.required}" required="true"
immediate="true" />
<h:dataTable id="table" value="#{bean.tableRows}" var="row">
<h:column><h:inputText id="in" value="#{row.name}" /></h:column>
</h:dataTable>
<ui:repeat id="repeat" value="#{bean.repeatRows}" var="row">
<h:inputText id="in" value="#{row.name}" />
</ui:repeat>
<h:commandButton id="submit" value="Submit" />
<h:commandButton id="refresh" value="Refresh">
<f:ajax execute="required" render="@form" />
</h:commandButton>
</h:form>
{code}
{code:java}
@Named
@ViewScoped
public class Bean implements Serializable {
private List<Row> tableRows = List.of(new Row("zero"), new Row("one"));
private List<Row> repeatRows = List.of(new Row("zero"), new Row("one"));
private String required;
// Getters and setters, and Row with a name property.
}
{code}
# Type {{first}} in row 0 of the table and of the repeat, leave the required
input empty, and click Submit. Validation fails. Keep the
{{jakarta.faces.ViewState}} value of this response.
# Type {{second}} in row 1 of the table and of the repeat, and click Submit.
Validation fails again.
# Put the {{jakarta.faces.ViewState}} value of step 1 back into the hidden
field, as the back button or a second tab would do, and click Refresh.
h4. Expected
The state of step 1: row 0 shows {{first}}, and row 1 shows {{one}}.
h4. Actual
Row 1 of the table and of the repeat shows {{second}}, the value of step 2.
Reproduced on MyFaces 4.1.5-SNAPSHOT on Tomcat 11, with server side state
saving.
h4. Cause
{{UIData.saveState()}} saves its per-row maps {{_rowDeltaStates}},
{{_rowStates}} and {{_rowTransientStates}} through
{{UIComponentBase.saveAttachedState()}}. A {{HashMap}} is no {{StateHolder}}
and no {{Collection}}, so {{saveAttachedState()}} returns it as it is:
{code:java}
else if (attachedObject instanceof Serializable)
{
return attachedObject;
}
{code}
{{UIData.restoreState()}} then takes the restored map as its own field, for
example {{_rowStates = (Map<String, List<EditableValueHolderState>>) rs;}}.
Without serialization, that is the instance in the saved state. When the table
saves the state of a row, it changes that map in place:
{code:java}
if (states != null)
{
_rowStates.put(key, states);
}
else
{
_rowStates.remove(key);
}
{code}
So step 2 changes the saved state of step 1. {{UIRepeat}} saves and restores
its {{_rowStates}} and {{_rowDeltaStates}} in the same way. Client side state
saving, and server side state saving with
{{jakarta.faces.SERIALIZE_SERVER_STATE}} set to {{true}}, are not affected,
because each restore then gets its own deserialized copy.
h4. Spec
No clause states that a saved state must stay as it was saved. These are the
clauses which a shared per-row state goes against.
The javadoc of {{UIData.setRowIndex()}}:
{quote}
To restore current state information for all descendant components, {{UIData}}
must reference its previously stored information for the current {{rowIndex}}
and call setters for each descendant as follows:
{quote}
After step 2, the restore of step 3 references the information which step 2
stored, not the information which was stored with the view state of step 1.
The specification, section "State Saving Alternatives and Implications", on the
{{server}} state saving method:
{quote}
Cause the saved state to be stored on the server in between requests. [...]
Serializing the view in server mode is optional but must be possible by setting
the _context-param jakarta.faces.SERIALIZE_SERVER_STATE_ to _true_.
{quote}
With that context parameter set to {{true}}, nothing is shared. So the result
of a restore depends on an option which the specification describes as optional
for serializing, not as a change of behavior.
h4. Proposed fix
Copy the per-row maps in {{restoreState()}} of {{UIData}} and {{UIRepeat}}, for
example {{_rowStates = new HashMap<>((Map<String,
List<EditableValueHolderState>>) rs);}}. The values in these maps are replaced
when a row is saved, not changed in place, so a copy of the outer map is enough.
TCK:
{{Issue6129IT.testSecondRestoreOfSavedStateDoesNotSeeChangesOfFirstRestore}}.
🤖 Generated with Claude Opus 5.5
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
