Ryan,
Doesn't this break existing third-party StateManager
implementations? It might be OK to implement this
optimization in such a way that the RI's implementation
of RestoreViewPhase will still do its old thing when
StateManagerImpl is not used. Also, is the
new state manager implementation guaranteed to be
parent-first?
-- Adam
Ryan Lubke wrote:
>
> Section 2.2.1 of the specification states the
> following for 'post back' requests:
>
> For each component in the component tree, determine if a ValueExpression
> for
> “binding” is present. If so, call the setValue() method on this
> ValueExpression, passing the component instance on which it was found.
> Do this
> in a “parent-first” fashion, calling the setValue() method and then
> traversing the
> children.
>
> The current implementation would perform this action on the view
> returned by the StateManager by iterating over the view and processing
> each component.
>
> This change moves this logic into the StateManager implementation.
> When a component is recreated and it's state restored, check
> for a 'binding' ValueExpression and if found, set the component
> instance as its value.
>
> Doing this will save an additional iteration through the view.
>
>
> SECTION: Modified Files
> ----------------------------
> M src/com/sun/faces/application/StateManagerImpl.java
> M src/com/sun/faces/lifecycle/RestoreViewPhase.java
>
>
> SECTION: Diffs
> ----------------------------
> Index: src/com/sun/faces/application/StateManagerImpl.java
> ===================================================================
> RCS file:
> /cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/application/StateManagerImpl.java,v
>
> retrieving revision 1.57
> diff -u -r1.57 StateManagerImpl.java
> --- src/com/sun/faces/application/StateManagerImpl.java 11 Sep 2006
> 20:45:55 -0000 1.57
> +++ src/com/sun/faces/application/StateManagerImpl.java 19 Sep 2006
> 01:11:58 -0000
> @@ -33,6 +33,7 @@
> import javax.faces.context.ExternalContext;
> import javax.faces.context.FacesContext;
> import javax.faces.render.ResponseStateManager;
> +import javax.el.ValueExpression;
>
> import java.io.Externalizable;
> import java.io.IOException;
> @@ -73,7 +74,7 @@
> /** Number of views in logical view to be saved in session. */
> private int noOfViews;
> private int noOfViewsInLogicalView;
> - private Map<String,Class<?>> classMap =
> + private volatile Map<String,Class<?>> classMap =
> new ConcurrentHashMap<String,Class<?>>(32);
>
>
> @@ -497,9 +498,9 @@
>
> return (ReflectionUtils.lookupMethod(
> instance.getClass(),
> - "getState",
> - FacesContext.class,
> - String.class) != null);
> + "getState",
> + FacesContext.class,
> + String.class) != null);
>
> }
>
> @@ -523,6 +524,12 @@
> if (!n.trans) {
> c.restoreState(ctx, n.state);
> }
> +
> + ValueExpression ve = c.getValueExpression("binding");
> + if (ve != null) {
> + ve.setValue(ctx.getELContext(), c);
> + }
> +
> return c;
> } catch (Exception e) {
> throw new FacesException(e);
> Index: src/com/sun/faces/lifecycle/RestoreViewPhase.java
> ===================================================================
> RCS file:
> /cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/lifecycle/RestoreViewPhase.java,v
>
> retrieving revision 1.39
> diff -u -r1.39 RestoreViewPhase.java
> --- src/com/sun/faces/lifecycle/RestoreViewPhase.java 18 Sep 2006
> 23:15:05 -0000 1.39
> +++ src/com/sun/faces/lifecycle/RestoreViewPhase.java 19 Sep 2006
> 01:11:58 -0000
> @@ -38,6 +38,7 @@
> import javax.faces.component.UIComponent;
> import javax.faces.component.UIViewRoot;
> import javax.faces.context.FacesContext;
> +import javax.faces.context.ExternalContext;
> import javax.faces.event.PhaseId;
> import javax.faces.render.ResponseStateManager;
> import javax.servlet.http.HttpServletRequest;
> @@ -92,24 +93,24 @@
>
> // If an app had explicitely set the tree in the context, use that;
> //
> - UIViewRoot viewRoot = facesContext.getViewRoot();
> - Locale locale = null;
> + UIViewRoot viewRoot = facesContext.getViewRoot();
> + ExternalContext extContext = facesContext.getExternalContext();
> if (viewRoot != null) {
> if (logger.isLoggable(Level.FINE)) {
> logger.fine("Found a pre created view in FacesContext");
> }
> - locale = facesContext.getExternalContext().getRequestLocale();
> + Locale locale = extContext.getRequestLocale();
> facesContext.getViewRoot().setLocale(locale);
> doPerComponentActions(facesContext, viewRoot);
> return;
> }
>
> // Reconstitute or create the request tree
> - Map requestMap = facesContext.getExternalContext().getRequestMap();
> + Map requestMap = extContext.getRequestMap();
> String viewId = (String)
> requestMap.get("javax.servlet.include.path_info");
> if (viewId == null) {
> - viewId = facesContext.getExternalContext().getRequestPathInfo();
> + viewId = extContext.getRequestPathInfo();
> }
>
> // It could be that this request was mapped using
> @@ -121,7 +122,7 @@
> }
>
> if (viewId == null) {
> - Object request = facesContext.getExternalContext().getRequest();
> + Object request = extContext.getRequest();
> if (request instanceof HttpServletRequest) {
> viewId = ((HttpServletRequest) request).getServletPath();
> }
> @@ -142,7 +143,7 @@
> viewHandler.restoreView(facesContext, viewId))) {
> JSFVersionTracker tracker =
> ApplicationAssociate
> - .getInstance(facesContext.getExternalContext())
> + .getInstance(extContext)
> .getJSFVersionTracker();
>
> // The tracker will be null if the user turned off the
> @@ -155,16 +156,16 @@
> Version toTest = tracker.
> getVersionForTrackedClassName(viewHandler
> .getClass().getName());
> - Version currentVersion = tracker.getCurrentVersion();
> - boolean viewHandlerIsOld = false,
> - stateManagerIsOld = false;
> + Version currentVersion = tracker.getCurrentVersion();
>
> - viewHandlerIsOld = (toTest.compareTo(currentVersion) < 0);
> + boolean viewHandlerIsOld =
> + (toTest.compareTo(currentVersion) < 0);
> toTest = tracker.
> getVersionForTrackedClassName(facesContext
> .getApplication().getStateManager()
> .getClass().getName());
> - stateManagerIsOld = (toTest.compareTo(currentVersion) < 0);
> + boolean stateManagerIsOld =
> + (toTest.compareTo(currentVersion) < 0);
>
> if (viewHandlerIsOld || stateManagerIsOld) {
> viewRoot = viewHandler.createView(facesContext, viewId);
> @@ -180,7 +181,7 @@
> MessageUtils.RESTORE_VIEW_ERROR_MESSAGE_ID, params),
> viewId);
> }
> - }
> + }
> if (logger.isLoggable(Level.FINE)) {
> logger.fine("Postback: Restored view for " + viewId);
> }
> @@ -195,8 +196,7 @@
> }
> assert(null != viewRoot);
>
> - facesContext.setViewRoot(viewRoot);
> - doPerComponentActions(facesContext, viewRoot);
> + facesContext.setViewRoot(viewRoot);
>
> if (logger.isLoggable(Level.FINE)) {
> logger.fine("Exiting RestoreViewPhase");
> @@ -210,18 +210,23 @@
> return PhaseId.RESTORE_VIEW;
>
> }
> -
> - // ------------------------------------------------------- Protected
> Methods
> +
> +
> + // --------------------------------------------------------- Private
> Methods
>
>
> - /** <p>Do any per-component actions necessary during reconstitute</p> */
> - protected void doPerComponentActions(FacesContext context,
> + /**
> + * <p>This is called if a <code>ViewRoot</code> is already found
> + * in the <code>FacesContext</code> before any restoration is
> + * attempted.
> + */
> + private void doPerComponentActions(FacesContext context,
> UIComponent uic) {
>
> // if this component has a component value reference expression,
> // make sure to populate the ValueExpression for it.
> - ValueExpression valueExpression = null;
> - if (null != (valueExpression = uic.getValueExpression("binding"))) {
> + ValueExpression valueExpression = uic.getValueExpression("binding");
> + if (valueExpression != null) {
> valueExpression.setValue(context.getELContext(), uic);
> }
>
> @@ -230,9 +235,7 @@
> doPerComponentActions(context, kids.next());
> }
>
> - }
> -
> - // --------------------------------------------------------- Private
> Methods
> + }
>
>
> /**
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
> For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net
>