jakartaee / faces

Jakarta Faces
Other
109 stars 55 forks source link

Faces 4.1: Deprecate full state saving (FSS) #1829

Closed arjantijms closed 1 year ago

arjantijms commented 1 year ago

Full state saving is widely considered to be a historical mistake in Faces. Indeed, in old meeting minutes from before Faces (JSF) 1.0 it was already brought to the table that putting everything that static on the view description also in the state would be a monumental waste of resources.

We should have deprecated this a long time ago, but we simply forgot it every release.

Let's not forget this anymore, and let's actually deprecate (for removal) FFS.

BalusC commented 1 year ago

I like the FFS pun there :)

BalusC commented 1 year ago

For the record, these are the context params warranting removal as well

tandraschko commented 1 year ago

should we add a warning on startup when running with that configs?

VsevolodGolovanov commented 1 year ago

We have FSS enabled since ancient times because of cases like this (using PrimeFaces):

<p:tabView dynamic="true">
    <p:tab>
        <p:dataTable>
            <p:columns value="#{expression}">

State saving doesn't know about the dynamic attribute, so after rendering it executes expression which could be a heavy lazy loaded model, or it could depend on some prerequisites that haven't been satisfied.

at javax.faces.component.ComponentStateHelper.eval(ComponentStateHelper.java:182) [jboss-jsf-api_2.2_spec-2.2.5.jar:2.2.5]
at javax.faces.component.UIData.getValue(UIData.java:732) [jboss-jsf-api_2.2_spec-2.2.5.jar:2.2.5]
at org.primefaces.component.api.UIData.getDataModel(UIData.java:629) [primefaces-5.0.jar:5.0]
at javax.faces.component.UIData.getRowCount(UIData.java:356) [jboss-jsf-api_2.2_spec-2.2.5.jar:2.2.5]
...
at javax.faces.component.UIComponent.visitTree(UIComponent.java:1700) [jboss-jsf-api_2.2_spec-2.2.5.jar:2.2.5]
at com.sun.faces.application.view.FaceletPartialStateManagementStrategy.saveView(FaceletPartialStateManagementStrategy.java:472) [jsf-impl-2.2.5-jbossorg-3.jar:]
at com.sun.faces.application.StateManagerImpl.saveView(StateManagerImpl.java:89) [jsf-impl-2.2.5-jbossorg-3.jar:]
at com.sun.faces.application.view.WriteBehindStateWriter.flushToWriter(WriteBehindStateWriter.java:225) [jsf-impl-2.2.5-jbossorg-3.jar:]
at com.sun.faces.application.view.FaceletViewHandlingStrategy.renderView(FaceletViewHandlingStrategy.java:478) [jsf-impl-2.2.5-jbossorg-3.jar:]
at com.sun.faces.application.view.MultiViewHandler.renderView(MultiViewHandler.java:133) [jsf-impl-2.2.5-jbossorg-3.jar:]

Storing the whole state is less costly than working around this.

Maybe this case is considered in newer JSF & PF versions? In JSF 2.2 the only way a component could affect state saving tree walking is by overriding isTransient, and I'm not sure that would work here, because TabView would have to somehow know if it was dynamically rendered or not, which is kept in its state, before restoring the view.

tandraschko commented 1 year ago

@VsevolodGolovanov try to create a reproducer with latest PF and create a primefaces issue; might already be fixed

tandraschko commented 1 year ago

https://github.com/apache/myfaces/commit/f9aaff9abdca022da9392b32d00355af35cbd373

BalusC commented 1 year ago

Now in Mojarra as well.