[jsr-314-open-mirror] [jsr-314-open] [Mojarra-1812-FacesServlet.service] PROPOSAL

Ed Burns edward.burns at oracle.com
Wed Sep 8 11:44:32 EDT 2010


ACTION: Please review and respond by noon PDT Friday 10 September 2010.

New method on Lifecycle: cleanup() https://javaserverfaces.dev.java.net/issues/show_bug.cgi?id=1512


SECTION: Modified Files
----------------------------
M       jsf-api/src/main/java/javax/faces/lifecycle/Lifecycle.java
M       jsf-api/src/main/java/javax/faces/lifecycle/package.html
M       jsf-api/src/main/java/javax/faces/webapp/FacesServlet.java
M       jsf-ri/src/main/java/com/sun/faces/lifecycle/LifecycleImpl.java
M       jsf-ri/src/main/java/com/sun/faces/application/WebappLifecycleListener.java

Way back in the 2.0.4 Rev a cycle, I made this change to FacesServlet.java:

EB> r8293 | edburns | 2010-01-21 15:16:12 -0500 (Thu, 21 Jan 2010) | 46 lines
EB>

EB> Issue: 1512
EB> 
EB> No-one gave r= on this, but Neil Griffin did review the approach and
EB> said it would not break portlets.
EB> 
EB> SECTION: SPEC Considerations
EB> 
EB> First, let's assume I'll file a spec issue, targeted at 2.0 Rev a, that
EB> loosens the the following wording on the javadoc of
EB> FacesServlet.service().
EB> 
EB> The very last sentance in the javadoc for FacesServlet.service() is:
EB> 
EB> "In a finally block, FacesContext.release() must be called."
EB> 
EB> I want to change this to:
EB> 
EB> "The implementation must make it so FacesContext.release() is called
EB> within a finally block as late as possible in the processing for the JSF
EB> related portion of this request".

We have discovered that this breaks some scenarios.  The correct thing
is to, in fact, call facesContext.release() before returning from
FacesServlet.service().

This changebundle proposes a new method on Lifecycle so that
implementations may do any cleanup they need, and then call
facesContext.release() themselves.


SECTION: Diffs
----------------------------
Index: jsf-api/src/main/java/javax/faces/lifecycle/Lifecycle.java
===================================================================
--- jsf-api/src/main/java/javax/faces/lifecycle/Lifecycle.java	(revision 8594)
+++ jsf-api/src/main/java/javax/faces/lifecycle/Lifecycle.java	(working copy)
@@ -42,7 +42,7 @@
 
 
 /**
- * <p><strong>Lifecycle</strong> manages the
+ * <p><strong class="changed_modified_2_1">Lifecycle</strong> manages the
  * processing of the entire lifecycle of a particular JavaServer Faces
  * request.  It is responsible for executing all of the phases that have
  * been defined by the JavaServer Faces Specification, in the specified
@@ -131,5 +131,26 @@
      */
     public abstract void render(FacesContext context) throws FacesException;
 
+    /**
+     * <p class="changed_added_2_1">Allow the implementation to perform any
+     * cleanup necessary at the end of the JSF portion of this request.
+     * Implementations must ensure that {@link FacesContext#release} is called.
+     * The default implementation, which takes no action, is provided for
+     * backwards compatibility with existing implementations.
+     * </p>
+     *
+     * @param context FacesContext for the request being processed
+     *
+     * @throws FacesException if an exception is thrown during the execution
+     *  of the request processing lifecycle
+     * @throws NullPointerException if <code>context</code>
+     *  is <code>null</code>
+     *
+     * @since 2.1
+     */
+    public void cleanup(FacesContext context) throws FacesException {
+        
+    }
 
+
 }
Index: jsf-api/src/main/java/javax/faces/lifecycle/package.html
===================================================================
--- jsf-api/src/main/java/javax/faces/lifecycle/package.html	(revision 8594)
+++ jsf-api/src/main/java/javax/faces/lifecycle/package.html	(working copy)
@@ -3,7 +3,8 @@
 <title>Package Description for "javax.faces.lifecycle"</title>
 </head>
 <body bgcolor="white">
-    <p><span class="changed_modified_2_0">Classes</span> and interfaces defining lifecycle management for the
+    <p><span class="changed_modified_2_0 changed_modified_2_1">Classes</span>
+        and interfaces defining lifecycle management for the
 JavaServer Faces implementation.  The main class in this package is
 {@link javax.faces.lifecycle.Lifecycle}.  <code>Lifecycle</code> is the
 gateway to executing the request processing lifecycle.
Index: jsf-api/src/main/java/javax/faces/webapp/FacesServlet.java
===================================================================
--- jsf-api/src/main/java/javax/faces/webapp/FacesServlet.java	(revision 8594)
+++ jsf-api/src/main/java/javax/faces/webapp/FacesServlet.java	(working copy)
@@ -285,10 +285,8 @@
      * the cause, as the first argument, and the cause itself as the
      * second argument.</p>
 
-     * <p class="changed_modified_2_0_rev_a">The implementation must
-     * make it so {@link javax.faces.context.FacesContext#release} is
-     * called within a finally block as late as possible in the
-     * processing for the JSF related portion of this request.</p>
+     * <p class="changed_modified_2_1">In a finally block at the end of this
+     * method, {@Lifecycle#cleanup} is called.</p>
 
      * </div>
      *
@@ -348,6 +346,9 @@
                 }
             }
         }
+        finally {
+            lifecycle.cleanup(context);
+        }
 
         requestEnd(); // V3 Probe hook
     }
Index: jsf-ri/src/main/java/com/sun/faces/lifecycle/LifecycleImpl.java
===================================================================
--- jsf-ri/src/main/java/com/sun/faces/lifecycle/LifecycleImpl.java	(revision 8594)
+++ jsf-ri/src/main/java/com/sun/faces/lifecycle/LifecycleImpl.java	(working copy)
@@ -36,6 +36,8 @@
 
 package com.sun.faces.lifecycle;
 
+import com.sun.faces.application.ApplicationAssociate;
+import com.sun.faces.mgbean.BeanManager;
 import java.util.List;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.logging.Level;
@@ -48,6 +50,12 @@
 
 import com.sun.faces.util.FacesLogger;
 import com.sun.faces.util.MessageUtils;
+import java.util.Enumeration;
+import javax.faces.context.ExternalContext;
+import javax.faces.event.ExceptionQueuedEvent;
+import javax.faces.event.ExceptionQueuedEventContext;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpSession;
 
 
 /**
@@ -86,6 +94,8 @@
     private List<PhaseListener> listeners =
           new CopyOnWriteArrayList<PhaseListener>();
 
+    private ApplicationAssociate applicationAssociate;
+
         
 
     // ------------------------------------------------------- Lifecycle Methods
@@ -137,7 +147,70 @@
 
     }
 
+    @Override
+    public void cleanup(FacesContext context) throws FacesException {
+        try {
+            syncSessionScopedBeans(context);
+        } catch (Throwable t) {
+            ExceptionQueuedEventContext eventContext =
+                    new ExceptionQueuedEventContext(context, t);
+            context.getApplication().publishEvent(context,
+                    ExceptionQueuedEvent.class, eventContext);
+            context.getExceptionHandler().handle();
+        } finally {
+            if (null != context && !context.isReleased()) {
+                context.release();
+            }
+        }
 
+    }
+
+    /**
+     * This method ensures that session scoped managed beans will be
+     * synchronized properly in a clustered environment.
+     *
+     * @param request the current <code>ServletRequest</code>
+     */
+    private void syncSessionScopedBeans(FacesContext context) {
+        ExternalContext extContext = context.getExternalContext();
+        Object request = extContext.getRequest();
+
+        if (request instanceof HttpServletRequest) {
+            HttpSession session = (HttpSession)extContext.getSession(false);
+            if (session != null) {
+                ApplicationAssociate associate = getAssociate(extContext);
+                if (associate == null) {
+                    return;
+                }
+                BeanManager manager = associate.getBeanManager();
+                if (manager != null) {
+                    for (Enumeration e = session.getAttributeNames();
+                         e.hasMoreElements();) {
+                        String name = (String) e.nextElement();
+                        if (manager.isManaged(name)) {
+                            session
+                                  .setAttribute(name, session.getAttribute(name));
+                        }
+                    }
+                }
+            }
+        }
+
+    }
+
+    private ApplicationAssociate getAssociate(ExternalContext extContext) {
+
+        if (applicationAssociate == null) {
+            synchronized (extContext.getContext()) {
+                applicationAssociate = ApplicationAssociate.getInstance(extContext);
+            }
+        }
+
+        return applicationAssociate;
+    }
+
+
+
     // Add a new PhaseListener to the set of registered listeners
     public void addPhaseListener(PhaseListener listener) {
 
Index: jsf-ri/src/main/java/com/sun/faces/application/WebappLifecycleListener.java
===================================================================
--- jsf-ri/src/main/java/com/sun/faces/application/WebappLifecycleListener.java	(revision 8594)
+++ jsf-ri/src/main/java/com/sun/faces/application/WebappLifecycleListener.java	(working copy)
@@ -61,9 +61,6 @@
 import com.sun.faces.util.FacesLogger;
 import java.util.Map;
 import javax.faces.component.UIViewRoot;
-import javax.faces.context.FacesContext;
-import javax.faces.event.ExceptionQueuedEvent;
-import javax.faces.event.ExceptionQueuedEventContext;
 import javax.faces.event.SystemEvent;
 import javax.faces.event.PreDestroyViewMapEvent;
 import javax.faces.event.ViewMapListener;
@@ -103,30 +100,16 @@
      *
      * @param event the notification event
      */
-    public void requestDestroyed(ServletRequestEvent event) {
-        FacesContext context = FacesContext.getCurrentInstance();
-        try {
-            ServletRequest request = event.getServletRequest();
-            for (Enumeration e = request.getAttributeNames(); e.hasMoreElements();) {
-                String beanName = (String) e.nextElement();
-                handleAttributeEvent(beanName,
-                        request.getAttribute(beanName),
-                        ELUtils.Scope.REQUEST);
-            }
-            syncSessionScopedBeans(request);
-        } catch (Throwable t) {
-            ExceptionQueuedEventContext eventContext =
-                    new ExceptionQueuedEventContext(context, t);
-            context.getApplication().publishEvent(context,
-                    ExceptionQueuedEvent.class, eventContext);
-            context.getExceptionHandler().handle();
+    public void requestDestroyed(ServletRequestEvent event) {       
+        ServletRequest request = event.getServletRequest();
+        for (Enumeration e = request.getAttributeNames(); e.hasMoreElements(); ) {
+            String beanName = (String)e.nextElement();
+            handleAttributeEvent(beanName, 
+                                 request.getAttribute(beanName), 
+                                 ELUtils.Scope.REQUEST);
         }
-        finally {
-            if (null != context) {
-                context.release();
-            }
-            ApplicationAssociate.setCurrentInstance(null);
-        }
+        syncSessionScopedBeans(request);
+        ApplicationAssociate.setCurrentInstance(null);
     }
 
     /**



-- 
| edward.burns at oracle.com | office: +1 407 458 0017
| homepage:               | http://ridingthecrest.com/
|  0 work days until JSF 2.1 Milestone 5
|  8 work days until handoff of JSF 2.1 change log to jcp
|  7 work days until JavaOne 2010
| 42 work days until DOAG 2010
| 46 work days until JSF 2.1 Milestone 6



More information about the jsr-314-open-mirror mailing list