[jsr-314-open-mirror] [jsr-314-open] REMOVED: Re: [Mojarra-1512-FacesServlet.service] PROPOSAL
by Ed Burns
>>>>> On Wed, 08 Sep 2010 10:48:02 -0700, Blake Sullivan <blake.sullivan(a)oracle.com> said:
B> I haven't checked, but why does Faces have to do anything special to
B> handles Session-scoped beans for fail-over? Isn't the application
B> server going to handle this anyway? And if there is a Serialization
B> failure, isn't the user pretty much hosed at this point? In this case,
B> if a fail-over occurred the user would at best lose state. Also, this
B> kind of error is almost certainly an application coding error and there
B> is nothing that the appliction listener can really do that is helpful at
B> this point. If you really feel like publishing the event, how bad would
B> it be to create a one shot FacesContext just for delivering the event
B> and then release it immediately after?
Blake has given a litany of compelling reasons for not doing this as a
spec issue.
Therefore, I'm removing this from consideration for 2.1.
Sorry for the wasted time.
Ed
--
| edward.burns(a)oracle.com | office: +1 407 458 0017
| homepage: | http://ridingthecrest.com/
| 8 work days until handoff of JSF 2.1 change log to jcp
16 years, 1 month
Re: [jsr-314-open-mirror] [jsr-314-open] [Mojarra-1512-FacesServlet.service] PROPOSAL
by Ed Burns
>>>>> On Wed, 08 Sep 2010 09:37:27 -0700, Blake Sullivan <blake.sullivan(a)oracle.com> said:
B> I agree. It isn't clear how this is the best solution to the problem
B> logged.
The initial problem for issue 1512 (not 1812 as I incorrectly included
in the subject line) was that an exception was not being sucked in by
the JSF ExceptionHandler.
The cause is the use of ServletRequestListener.requestDestroyed() to
perform the action that caused the exception.
ServletRequestListener.requestDestroyed() is called *after* the
FacesContext is released, and therefore the ExceptionHandler can't
possibly be used.
The first iteration of the fix for 1512 was to defer the release of the
FacesContext to the requestDestroyed() implementation. This has been
shown to be incorrect.
The second iteration (just suggested today) was to add API that allows
the action to be taken at the right time by the implementation.
Your comments have now alerted me to the existence of another approach:
simply perform this action *in* the implementation's
FacesContext.release() method.
One problem with this approach, however, is that decorated FacesContext
instances, such as in the portlet case, may completely override
FacesContext.release(), and therefore break clustering, and also not
solve the intial problem of 1512.
Any other ideas? Should we just do it at the end of Lifecycle.render()?
[1] https://javaserverfaces.dev.java.net/issues/show_bug.cgi?id=1512
--
| edward.burns(a)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
16 years, 1 month
[jsr-314-open-mirror] [jsr-314-open] [Mojarra-1812-FacesServlet.service] PROPOSAL
by Ed Burns
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(a)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
16 years, 1 month
Re: [jsr-314-open-mirror] [jsr-314-open] update on Red Hat's participation in JSR-314 and JSF 2.1
by Ed Burns
>>>>> On Thu, 19 Aug 2010 11:25:43 -0400, Dan Allen <dan.j.allen(a)gmail.com> said:
DA> Ed, Roger and other members of the JSR-314 EG,
[...]
Hi Dan and other experts,
I'd like to remind everyone that the "EG disbands after the JSR is done"
and "JSR sponsor is encouraged but not required to consult the former
EG" processes are the way we've done things for years. Also, we also
limit the changes we make within an MR to small, well understood
changes. Nothing I've said changes that.
Dan, you're certainly within your rights in maintaining this stance. As
we continue to make minor enhancements to JSF through the JCP minor
revision process, I sincerely hope that RedHat will follow through on
its responsibilities as a JCP member when it comes to reviewing
change-logs and spec drafts in the minor revision process.
Roger and I feel the same way about the esprit de corp of this expert
group. It really is a great group of people and I will do everything
I can to ensure the JSF team in the JCP stays a fun place to work.
Per the process I've outlined here, the JCP MR process does provide a
way to reject proposed changes and send them to a JCP body for full
discussion. Of course, in the spirit of the way the expert group has
been run since day one, I hope changes can happen amicably here, at
least as a first resort, and only via the EC as a last resort.
Sincerely,
Ed Burns
--
| edward.burns(a)oracle.com | office: +1 407 458 0017
| homepage: | http://ridingthecrest.com/
16 years, 1 month
