dev@javaserverfaces.java.net

Re: Seeking Review: 8-ViewLifecycle

From: Roger Kitain <Roger.Kitain_at_Sun.COM>
Date: Thu, 18 Nov 2004 09:06:00 -0500

Ed Burns wrote:

>>>>>>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
>
>
>
r=rogerk. Don't forget to port it over to web-tier alignment workspace...