dev@javaserverfaces.java.net

Re: [REVIEW] Restore view optimization

From: Ryan Lubke <Ryan.Lubke_at_Sun.COM>
Date: Tue, 19 Sep 2006 09:46:00 -0700

Adam Winer wrote:
> Ryan,
>
> Doesn't this break existing third-party StateManager
> implementations?
The way I saw it was either way, the default behavior could be
replaced (if the third party provided their own Lifecycle that
didn't decorate the default).
> 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.
I thought about this (see above). I can certainly add
a check though.
> Also, is the
> new state manager implementation guaranteed to be
> parent-first?
Yes, the tree is restored in parent first fashion, so
this would work.
>
> -- 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
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
> For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net
>