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?
>
> -- Adam
>
>
>
>>> 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
>>>
>>
>> ---------------------------------------------------------------------
>> 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
>