[jsr-314-open-mirror] [jsr-314-open] [Mojarra-1812-FacesServlet.service] PROPOSAL
Matthias Wessendorf
matzew at apache.org
Wed Sep 8 12:05:15 EDT 2010
Hello,
I think I don't get why the release() has been (re)moved at all and
why this now need to exposed as lifecyle API.
Doesn't the ticket talk about exception handling/reporting? I think I
don't understand how that is related to release the facesCtx.
-Matthias
On Wed, Sep 8, 2010 at 5:44 PM, Ed Burns <edward.burns at oracle.com> wrote:
> 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
>
--
Matthias Wessendorf
blog: http://matthiaswessendorf.wordpress.com/
sessions: http://www.slideshare.net/mwessendorf
twitter: http://twitter.com/mwessendorf
More information about the jsr-314-open-mirror
mailing list