dev@javaserverfaces.java.net

Seeking Review: 8-ViewLifecycle

From: Ed Burns <Ed.Burns_at_Sun.COM>
Date: Wed, 17 Nov 2004 21:37:14 -0800

>>>>> On Thu, 11 Nov 2004 08:50:30 -0800, Adam Winer <adam.winer_at_oracle.com> said:

AW> Ed,
AW> Shouldn't we be calling all the UIViewRoot phase listener
AW> on all phases, not just encodeBegin() and encodeEnd()?

Ok, but we can't do RestoreView, per EG discussion.

AW> Also, it would greatly simplify the resulting code if
AW> we pulled this requirement out of UIViewRoot and into
AW> LifecycleImpl.

We can't do this, per EG discussion.

Here it is.

Issue: [8-ViewLifecycle]

M src/javax/faces/component/UIViewRoot.java

- Per EG discussion, make the phase listeners be called for every phase.

M test/javax/faces/component/UIViewRootTestCase.java

- lots more permutations!

Index: src/javax/faces/component/UIViewRoot.java
===================================================================
RCS file: /cvs/javaserverfaces-sources/jsf-api/src/javax/faces/component/UIViewRoot.java,v
retrieving revision 1.30
diff -u -r1.30 UIViewRoot.java
--- src/javax/faces/component/UIViewRoot.java 11 Nov 2004 20:39:30 -0000 1.30
+++ src/javax/faces/component/UIViewRoot.java 18 Nov 2004 05:34:30 -0000
@@ -35,7 +35,55 @@
 /**
  * <p><strong>UIViewRoot</strong> is the UIComponent that represents the
  * root of the UIComponent tree. This component has no rendering, it
- * just serves as the root of the component tree.</p>
+ * just serves as the root of the component tree, and as a place to hang
+ * per-view {_at_link PhaseListener}s.</p>
+ *
+ * <p>For each of the following lifecycle phase methods:</p>
+ *
+ * <ul>
+ *
+ * <li><p>{_at_link #processDecodes} </p></li>
+ *
+ * <li><p>{_at_link #processValidators} </p></li>
+ *
+ * <li><p>{_at_link #processUpdates} </p></li>
+ *
+ * <li><p>{_at_link #processApplication} </p></li>
+ *
+ * <li><p>RenderResponse, via {_at_link #encodeBegin} and {_at_link
+ * #encodeEnd} </p></li>
+ *
+ * </ul>
+ *
+ * <p>Take the following action regarding
+ * <code>PhaseListener</code>s.</p>
+ *
+ * <ul>
+ *
+ * <p>If {_at_link #getBeforePhaseListener} returns non-<code>null</code>,
+ * invoke the listener, passing in the correct corresponding {_at_link
+ * PhaseId} for this phase.</p>
+ *
+ * <p>If or one or more listeners have been added by a call to {_at_link
+ * #addPhaseListener}, invoke the <code>beforePhase</code> method on
+ * each one whose {_at_link PhaseListener#getPhaseId} matches the current
+ * phaseId, passing in the same <code>PhaseId</code> as in the previous
+ * step.</p>
+ *
+ * <p>Execute any processing for this phase.</p>
+ *
+ * <p>If {_at_link #getAfterPhaseListener} returns non-<code>null</code>,
+ * invoke the listener, passing in the correct corresponding {_at_link
+ * PhaseId} for this phase.</p>
+ *
+ * <p>If or one or more listeners have been added by a call to {_at_link
+ * #addPhaseListener}, invoke the <code>afterPhase</code> method on each
+ * one whose {_at_link PhaseListener#getPhaseId} matches the current
+ * phaseId, passing in the same <code>PhaseId</code> as in the previous
+ * step.</p>
+ *
+ *
+ * </ul>
  */
 
 public class UIViewRoot extends UIComponentBase {
@@ -192,6 +240,16 @@
     }
 
     /**
+ * <p>Allow an arbitrary method to be called for the "beforePhase"
+ * event as the UIViewRoot runs through its lifecycle. This method
+ * will be called for all phases except {_at_link
+ * PhaseId.RENDER_RESPONSE}. Unlike a true {_at_link PhaseListener},
+ * this approach doesn't allow for only receiving {_at_link
+ * PhaseEvent}s for a given phase.</p>
+ *
+ * <p>The method must conform to the signature of {_at_link
+ * PhaseListener#beforePhase}.</p>
+ *
      * @param newBeforePhase the {_at_link MethodBinding} that will be
      * invoked before this view is rendered.
      *
@@ -212,8 +270,18 @@
     }
 
     /**
+ * <p>Allow an arbitrary method to be called for the "afterPhase"
+ * event as the UIViewRoot runs through its lifecycle. This method
+ * will be called for all phases except {_at_link
+ * PhaseId.RENDER_RESPONSE}. Unlike a true {_at_link PhaseListener},
+ * this approach doesn't allow for only receiving {_at_link
+ * PhaseEvent}s for a given phase.</p>
+ *
+ * <p>The method must conform to the signature of {_at_link
+ * PhaseListener#afterPhase}.</p>
+ *
      * @param newAfterPhase the {_at_link MethodBinding} that will be
- * invoked after this view is rendered.
+ * invoked after this view is rendered.
      *
      */
 
@@ -368,12 +436,22 @@
      * is <code>null</code>
      */
     public void processDecodes(FacesContext context) {
+ // avoid creating the PhaseEvent if possible by doing redundant
+ // null checks.
+ if (null != beforePhase || null != phaseListeners) {
+ notifyPhaseListeners(context, PhaseId.APPLY_REQUEST_VALUES, true);
+ }
         super.processDecodes(context);
         broadcastEvents(context, PhaseId.APPLY_REQUEST_VALUES);
         // clear out the events if we're skipping to render-response
         if (context.getRenderResponse()) {
             events = null;
         }
+ // avoid creating the PhaseEvent if possible by doing redundant
+ // null checks.
+ if (null != beforePhase || null != phaseListeners) {
+ notifyPhaseListeners(context, PhaseId.APPLY_REQUEST_VALUES, false);
+ }
 
     }
 
@@ -398,29 +476,7 @@
         // avoid creating the PhaseEvent if possible by doing redundant
         // null checks.
         if (null != beforePhase || null != phaseListeners) {
- PhaseEvent event = createPhaseEvent(context);
- if (null != beforePhase) {
- try {
- beforePhase.invoke(context,
- new Object [] { event });
- }
- catch (Exception e) {
- // PENDING(edburns): log this
- }
- }
- if (null != phaseListeners) {
- Iterator iter = phaseListeners.iterator();
- PhaseListener curListener = null;
- while (iter.hasNext()) {
- curListener = (PhaseListener) iter.next();
- try {
- curListener.beforePhase(event);
- }
- catch (Exception e) {
- // PENDING(edburns): log this
- }
- }
- }
+ notifyPhaseListeners(context, PhaseId.RENDER_RESPONSE, true);
         }
 
         super.encodeBegin(context);
@@ -441,35 +497,67 @@
         // avoid creating the PhaseEvent if possible by doing redundant
         // null checks.
         if (null != afterPhase || null != phaseListeners) {
- PhaseEvent event = createPhaseEvent(context);
- if (null != afterPhase) {
- try {
- afterPhase.invoke(context,
- new Object [] { event });
- }
- catch (Exception e) {
- // PENDING(edburns): log this
- }
+ notifyPhaseListeners(context, PhaseId.RENDER_RESPONSE, false);
+ }
+
+ }
+
+ /**
+ * <p>Utility method that notifies phaseListeners for the given
+ * phaseId. Assumes that either or both the MethodBinding or
+ * phaseListeners data structure are non-null.</p>
+ *
+ * @param context the context for this request
+ *
+ * @param phaseId the {_at_link PhaseId} of the current phase
+ *
+ * @param isBefore, if true, notify beforePhase listeners. Notify
+ * afterPhase listeners otherwise.
+ */
+
+ private void notifyPhaseListeners(FacesContext context,
+ PhaseId phaseId,
+ boolean isBefore) {
+ PhaseEvent event = createPhaseEvent(context, phaseId);
+
+ boolean hasPhaseMethodBinding =
+ (isBefore && (null != beforePhase)) ||
+ (!isBefore && (null != afterPhase));
+ MethodBinding binding = isBefore ? beforePhase : afterPhase;
+
+ if (hasPhaseMethodBinding) {
+ try {
+ binding.invoke(context, new Object [] { event });
+ }
+ catch (Exception e) {
+ // PENDING(edburns): log this
             }
- if (null != phaseListeners) {
- Iterator iter = phaseListeners.iterator();
- PhaseListener curListener = null;
- while (iter.hasNext()) {
- curListener = (PhaseListener) iter.next();
+ }
+ if (null != phaseListeners) {
+ Iterator iter = phaseListeners.iterator();
+ PhaseListener curListener = null;
+ while (iter.hasNext()) {
+ curListener = (PhaseListener) iter.next();
+ if (phaseId == curListener.getPhaseId() ||
+ PhaseId.ANY_PHASE == curListener.getPhaseId()) {
                     try {
- curListener.afterPhase(event);
+ if (isBefore) {
+ curListener.beforePhase(event);
+ }
+ else {
+ curListener.afterPhase(event);
+ }
                     }
                     catch (Exception e) {
                         // PENDING(edburns): log this
                     }
                 }
             }
-
         }
-
     }
 
- private PhaseEvent createPhaseEvent(FacesContext context) throws FacesException {
+ private PhaseEvent createPhaseEvent(FacesContext context,
+ PhaseId phaseId) throws FacesException {
         Lifecycle lifecycle = null;
         LifecycleFactory lifecycleFactory = (LifecycleFactory)
             FactoryFinder.getFactory(FactoryFinder.LIFECYCLE_FACTORY);
@@ -480,8 +568,7 @@
         }
         lifecycle = lifecycleFactory.getLifecycle(lifecycleId);
             
- PhaseEvent result = new PhaseEvent(context, PhaseId.RENDER_RESPONSE,
- lifecycle);
+ PhaseEvent result = new PhaseEvent(context, phaseId, lifecycle);
         return result;
     }
         
@@ -499,6 +586,11 @@
      * is <code>null</code>
      */
     public void processValidators(FacesContext context) {
+ // avoid creating the PhaseEvent if possible by doing redundant
+ // null checks.
+ if (null != beforePhase || null != phaseListeners) {
+ notifyPhaseListeners(context, PhaseId.PROCESS_VALIDATIONS, true);
+ }
 
         super.processValidators(context);
         broadcastEvents(context, PhaseId.PROCESS_VALIDATIONS);
@@ -507,6 +599,11 @@
             events = null;
         }
 
+ // avoid creating the PhaseEvent if possible by doing redundant
+ // null checks.
+ if (null != beforePhase || null != phaseListeners) {
+ notifyPhaseListeners(context, PhaseId.PROCESS_VALIDATIONS, false);
+ }
     }
 
 
@@ -520,10 +617,20 @@
      * is <code>null</code>
      */
     public void processUpdates(FacesContext context) {
-
+ // avoid creating the PhaseEvent if possible by doing redundant
+ // null checks.
+ if (null != beforePhase || null != phaseListeners) {
+ notifyPhaseListeners(context, PhaseId.UPDATE_MODEL_VALUES, true);
+ }
+
         super.processUpdates(context);
         broadcastEvents(context, PhaseId.UPDATE_MODEL_VALUES);
 
+ // avoid creating the PhaseEvent if possible by doing redundant
+ // null checks.
+ if (null != beforePhase || null != phaseListeners) {
+ notifyPhaseListeners(context, PhaseId.UPDATE_MODEL_VALUES, false);
+ }
     }
 
 
@@ -537,10 +644,20 @@
      * is <code>null</code>
      */
     public void processApplication(FacesContext context) {
+ // avoid creating the PhaseEvent if possible by doing redundant
+ // null checks.
+ if (null != beforePhase || null != phaseListeners) {
+ notifyPhaseListeners(context, PhaseId.INVOKE_APPLICATION, true);
+ }
 
         // NOTE - no tree walk is performed; this is a UIViewRoot-only operation
         broadcastEvents(context, PhaseId.INVOKE_APPLICATION);
 
+ // avoid creating the PhaseEvent if possible by doing redundant
+ // null checks.
+ if (null != beforePhase || null != phaseListeners) {
+ notifyPhaseListeners(context, PhaseId.INVOKE_APPLICATION, false);
+ }
     }
 
     /**
Index: test/javax/faces/component/UIViewRootTestCase.java
===================================================================
RCS file: /cvs/javaserverfaces-sources/jsf-api/test/javax/faces/component/UIViewRootTestCase.java,v
retrieving revision 1.16
diff -u -r1.16 UIViewRootTestCase.java
--- test/javax/faces/component/UIViewRootTestCase.java 11 Nov 2004 18:03:07 -0000 1.16
+++ test/javax/faces/component/UIViewRootTestCase.java 18 Nov 2004 05:34:31 -0000
@@ -251,7 +251,18 @@
 
         
     public void doTestPhaseMethodBinding(UIViewRoot root) throws Exception {
- PhaseListenerBean phaseListenerBean = new PhaseListenerBean();
+ doTestPhaseMethodBindingWithPhaseId(root,
+ PhaseId.APPLY_REQUEST_VALUES);
+ doTestPhaseMethodBindingWithPhaseId(root, PhaseId.PROCESS_VALIDATIONS);
+ doTestPhaseMethodBindingWithPhaseId(root, PhaseId.UPDATE_MODEL_VALUES);
+ doTestPhaseMethodBindingWithPhaseId(root, PhaseId.INVOKE_APPLICATION);
+ doTestPhaseMethodBindingWithPhaseId(root, PhaseId.RENDER_RESPONSE);
+
+ }
+
+ public void doTestPhaseMethodBindingWithPhaseId(UIViewRoot root,
+ PhaseId phaseId) throws Exception {
+ PhaseListenerBean phaseListenerBean = new PhaseListenerBean(phaseId);
         facesContext.getExternalContext().getRequestMap().put("bean",
                                                             phaseListenerBean);
         Class [] args = new Class [] { PhaseEvent.class };
@@ -260,28 +271,58 @@
             afterBinding = facesContext.getApplication().createMethodBinding("#{bean.afterPhase}", args);
         root.setBeforePhaseListener(beforeBinding);
         root.setAfterPhaseListener(afterBinding);
- root.encodeBegin(facesContext);
- root.encodeEnd(facesContext);
+
+ callRightLifecycleMethodGivenPhaseId(root, phaseId);
+
         assertTrue(phaseListenerBean.isBeforePhaseCalled());
         assertTrue(phaseListenerBean.isAfterPhaseCalled());
         
         
     }
 
+
     public void doTestPhaseListener(UIViewRoot root) throws Exception {
- PhaseListenerBean phaseListener = new PhaseListenerBean();
+ doTestPhaseListenerWithPhaseId(root,
+ PhaseId.APPLY_REQUEST_VALUES);
+ doTestPhaseListenerWithPhaseId(root, PhaseId.PROCESS_VALIDATIONS);
+ doTestPhaseListenerWithPhaseId(root, PhaseId.UPDATE_MODEL_VALUES);
+ doTestPhaseListenerWithPhaseId(root, PhaseId.INVOKE_APPLICATION);
+ doTestPhaseListenerWithPhaseId(root, PhaseId.RENDER_RESPONSE);
+
+ }
+
+ public void doTestPhaseListenerWithPhaseId(UIViewRoot root,
+ PhaseId phaseId) throws Exception {
+ PhaseListenerBean phaseListener = new PhaseListenerBean(phaseId);
         root.addPhaseListener(phaseListener);
- root.encodeBegin(facesContext);
- root.encodeEnd(facesContext);
+
+ callRightLifecycleMethodGivenPhaseId(root, phaseId);
+
         assertTrue(phaseListener.isBeforePhaseCalled());
         assertTrue(phaseListener.isAfterPhaseCalled());
         
         
     }
 
+
     public void doTestPhaseMethodBindingAndListener(UIViewRoot root) throws Exception {
- PhaseListenerBean phaseListener = new PhaseListenerBean();
- PhaseListenerBean phaseListenerBean = new PhaseListenerBean();
+ doTestPhaseMethodBindingAndListenerWithPhaseId(root,
+ PhaseId.APPLY_REQUEST_VALUES);
+ doTestPhaseMethodBindingAndListenerWithPhaseId(root,
+ PhaseId.PROCESS_VALIDATIONS);
+ doTestPhaseMethodBindingAndListenerWithPhaseId(root,
+ PhaseId.UPDATE_MODEL_VALUES);
+ doTestPhaseMethodBindingAndListenerWithPhaseId(root,
+ PhaseId.INVOKE_APPLICATION);
+ doTestPhaseMethodBindingAndListenerWithPhaseId(root,
+ PhaseId.RENDER_RESPONSE);
+
+ }
+
+ public void doTestPhaseMethodBindingAndListenerWithPhaseId(UIViewRoot root,
+ PhaseId phaseId) throws Exception {
+ PhaseListenerBean phaseListener = new PhaseListenerBean(phaseId);
+ PhaseListenerBean phaseListenerBean = new PhaseListenerBean(phaseId);
         facesContext.getExternalContext().getRequestMap().put("bean",
                                                             phaseListenerBean);
         Class [] args = new Class [] { PhaseEvent.class };
@@ -291,8 +332,9 @@
         root.setBeforePhaseListener(beforeBinding);
         root.setAfterPhaseListener(afterBinding);
         root.addPhaseListener(phaseListener);
- root.encodeBegin(facesContext);
- root.encodeEnd(facesContext);
+
+ callRightLifecycleMethodGivenPhaseId(root, phaseId);
+
         assertTrue(phaseListenerBean.isBeforePhaseCalled());
         assertTrue(phaseListenerBean.isAfterPhaseCalled());
         assertTrue(phaseListener.isBeforePhaseCalled());
@@ -300,8 +342,22 @@
         
     }
 
-
-
+ private void callRightLifecycleMethodGivenPhaseId(UIViewRoot root,
+ PhaseId phaseId) throws Exception {
+ if (phaseId.getOrdinal() == PhaseId.APPLY_REQUEST_VALUES.getOrdinal()) {
+ root.processDecodes(facesContext);
+ }
+ else if(phaseId.getOrdinal() == PhaseId.PROCESS_VALIDATIONS.getOrdinal()) {
+ root.processValidators(facesContext);
+ } else if(phaseId.getOrdinal() == PhaseId.UPDATE_MODEL_VALUES.getOrdinal()) {
+ root.processUpdates(facesContext);
+ } else if(phaseId.getOrdinal() == PhaseId.INVOKE_APPLICATION.getOrdinal()) {
+ root.processApplication(facesContext);
+ } else if(phaseId.getOrdinal() == PhaseId.RENDER_RESPONSE.getOrdinal()) {
+ root.encodeBegin(facesContext);
+ root.encodeEnd(facesContext);
+ }
+ }
 
     // --------------------------------------------------------- Support Methods
 
@@ -398,8 +454,11 @@
     public static class PhaseListenerBean extends Object implements PhaseListener {
         private boolean beforePhaseCalled = false;
         private boolean afterPhaseCalled = false;
+ private PhaseId phaseId = null;
 
- public PhaseListenerBean() {}
+ public PhaseListenerBean(PhaseId phaseId) {
+ this.phaseId = phaseId;
+ }
 
         public boolean isBeforePhaseCalled() {
             return beforePhaseCalled;
@@ -417,7 +476,7 @@
             afterPhaseCalled = true;
         }
 
- public PhaseId getPhaseId() { return PhaseId.RENDER_RESPONSE; }
+ public PhaseId getPhaseId() { return phaseId; }
             
     }
 



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net