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).
-- 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
>