[jsr-314-open-mirror] [jsr-314-open] [Mojarra-1812-FacesServlet.service] PROPOSAL
Blake Sullivan
blake.sullivan at oracle.com
Wed Sep 8 12:37:27 EDT 2010
I agree. It isn't clear how this is the best solution to the problem
logged.
-- Blake Sullivan
On 9/8/10 9:05 AM, Matthias Wessendorf wrote:
> 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
>>
>
>
More information about the jsr-314-open-mirror
mailing list