[
https://issues.apache.org/jira/browse/MYFACES-4778?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Thomas Andraschko resolved MYFACES-4778.
----------------------------------------
Resolution: Fixed
> 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
> Assignee: Thomas Andraschko
> Priority: Major
> Fix For: 5.0.0, 4.1.5
>
>
> {{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)
