Re: [REVIEW] Restore view optimization
Looks good.
-- Adam
Ryan Lubke wrote:
> Adam Winer wrote:
>> Ryan Lubke wrote:
>>> Adam Winer wrote:
>>>> Ryan Lubke wrote:
>>>>> 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).
>>>>
>>>> I don't understand... if I wrote my own StateManager
>>>> (and I have), it's not OK for a new rev of JSF to just
>>>> break "binding" in all my pages. It's not OK to move
>>>> responsibility from one class to another in the RI
>>>> if either of those classes are replaceable.
>>>>
>>>> The issue isn't "can a third party be made to work",
>>>> it's "will a third party continue to work", and also
>>>> whether that code can be written portably (e.g.,
>>>> against MyFaces and the RI).
>>> Point taken. Would the check in RestoreViewPhase
>>> to bypass the doPerComponentActions() if the StateManager
>>> is StateManagerImpl be sufficient then?
>>
>> Probably... the only issue left would be what happens
>> if someone is using StateManagerImpl but a custom
>> lifecycle, in which case "binding" would get double-evaluated.
> I've attached take 2 of this change.
>>
>> -- Adam
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
>> For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net
>>
>
>
> ------------------------------------------------------------------------
>
>
> Take 2 of RestoreViewPhase optimization
>
>
> SECTION: Modified Files
> ----------------------------
> M src/com/sun/faces/RIConstants.java
> - added scope attribute constant names
> to flag that the default lifecycle and/or
> the default StateManager is being executed.
>
> M src/com/sun/faces/application/StateManagerImpl.java
> - Check to see if the execute method of the default
> Lifecycle was invoked - if it was, then handle
> the restoration of binding value expressions.
> - Add request scope attribute if the default
> StateManager is being used to restore the view.
>
> M src/com/sun/faces/lifecycle/LifecycleImpl.java
> - add request scoped attribute if execute is called
>
> M src/com/sun/faces/lifecycle/RestoreViewPhase.java
> - Check to see if the default StateManger is used
> to restore the view - if it isn't, then
> call doPerComponentActions
>
>
> SECTION: Diffs
> ----------------------------
> Index: src/com/sun/faces/RIConstants.java
> ===================================================================
> RCS file: /cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/RIConstants.java,v
> retrieving revision 1.89
> diff -u -r1.89 RIConstants.java
> --- src/com/sun/faces/RIConstants.java 30 Aug 2006 18:23:52 -0000 1.89
> +++ src/com/sun/faces/RIConstants.java 20 Sep 2006 17:32:55 -0000
> @@ -122,6 +122,10 @@
> public static final String ALL_MEDIA = "*/*";
> public static final String CHAR_ENCODING = "ISO-8859-1";
> public static final String SUN_JSF_JS_URI = "com_sun_faces_sunjsf.js";
> + public static final String DEFAULT_LIFECYCLE =
> + FACES_PREFIX + "DefaultLifecycle";
> + public static final String DEFAULT_STATEMANAGER =
> + FACES_PREFIX + "DefaultStateManager";
>
>
> private RIConstants() {
> 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 20 Sep 2006 17:32:55 -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;
> @@ -497,9 +498,9 @@
>
> return (ReflectionUtils.lookupMethod(
> instance.getClass(),
> - "getState",
> - FacesContext.class,
> - String.class) != null);
> + "getState",
> + FacesContext.class,
> + String.class) != null);
>
> }
>
> @@ -523,6 +524,13 @@
> if (!n.trans) {
> c.restoreState(ctx, n.state);
> }
> + if (ctx.getExternalContext().getRequestMap()
> + .get(RIConstants.DEFAULT_LIFECYCLE) != null) {
> + ValueExpression ve = c.getValueExpression("binding");
> + if (ve != null) {
> + ve.setValue(ctx.getELContext(), c);
> + }
> + }
> return c;
> } catch (Exception e) {
> throw new FacesException(e);
> @@ -571,6 +579,8 @@
> UIComponent c;
> FacetNode fn;
> TreeNode tn;
> + ctx.getExternalContext().getRequestMap().put(RIConstants.DEFAULT_STATEMANAGER,
> + Boolean.TRUE);
> for (int i = 0; i < tree.length; i++) {
> if (tree[i]instanceof FacetNode) {
> fn = (FacetNode) tree[i];
> Index: src/com/sun/faces/application/ViewHandlerImpl.java
> ===================================================================
> RCS file: /cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/application/ViewHandlerImpl.java,v
> retrieving revision 1.85
> diff -u -r1.85 ViewHandlerImpl.java
> --- src/com/sun/faces/application/ViewHandlerImpl.java 15 Sep 2006 17:19:18 -0000 1.85
> +++ src/com/sun/faces/application/ViewHandlerImpl.java 20 Sep 2006 17:32:55 -0000
> @@ -611,8 +611,7 @@
> context.getViewRoot().getViewId());
> }
>
> - context.getResponseWriter().writeText(
> - RIConstants.SAVESTATE_FIELD_MARKER, null);
> + context.getResponseWriter().write(RIConstants.SAVESTATE_FIELD_MARKER);
> if (logger.isLoggable(Level.FINE)) {
> logger.fine("End writing marker for viewId " +
> context.getViewRoot().getViewId());
> Index: src/com/sun/faces/lifecycle/LifecycleImpl.java
> ===================================================================
> RCS file: /cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/lifecycle/LifecycleImpl.java,v
> retrieving revision 1.68
> diff -u -r1.68 LifecycleImpl.java
> --- src/com/sun/faces/lifecycle/LifecycleImpl.java 6 Sep 2006 20:44:05 -0000 1.68
> +++ src/com/sun/faces/lifecycle/LifecycleImpl.java 20 Sep 2006 17:32:55 -0000
> @@ -46,6 +46,7 @@
> import com.sun.faces.renderkit.RenderKitUtils;
> import com.sun.faces.util.MessageUtils;
> import com.sun.faces.util.Util;
> +import com.sun.faces.RIConstants;
>
> /**
> * <p><b>LifecycleImpl</b> is the stock implementation of the standard
> @@ -101,7 +102,10 @@
>
> if (LOGGER.isLoggable(Level.FINE)) {
> LOGGER.fine("execute(" + context + ")");
> - }
> + }
> +
> + context.getExternalContext().getRequestMap().put(RIConstants.DEFAULT_LIFECYCLE,
> + Boolean.TRUE);
>
> for (int i = 1; i < phases.length; i++) { // Skip ANY_PHASE placeholder
>
> 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 20 Sep 2006 17:32:55 -0000
> @@ -54,6 +54,7 @@
> import com.sun.faces.renderkit.RenderKitUtils;
> import com.sun.faces.util.MessageUtils;
> import com.sun.faces.util.Util;
> +import com.sun.faces.RIConstants;
>
> /**
> * <B>Lifetime And Scope</B> <P> Same lifetime and scope as
> @@ -181,6 +182,14 @@
> viewId);
> }
> }
> +
> + // If our default state manager implementation didn't
> + // restore the view, then call doPerComponentActions
> + if (facesContext.getExternalContext().getRequestMap().get(
> + RIConstants.DEFAULT_STATEMANAGER) == null) {
> + doPerComponentActions(facesContext, viewRoot);
> + }
> +
> if (logger.isLoggable(Level.FINE)) {
> logger.fine("Postback: Restored view for " + viewId);
> }
> @@ -196,7 +205,6 @@
> assert(null != viewRoot);
>
> facesContext.setViewRoot(viewRoot);
> - doPerComponentActions(facesContext, viewRoot);
>
> if (logger.isLoggable(Level.FINE)) {
> logger.fine("Exiting RestoreViewPhase");
>
>
>
>
>
> ------------------------------------------------------------------------
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
> For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net