I guess it is fixed
Romain Manni-Bucau @rmannibucau <https://twitter.com/rmannibucau> | Blog <http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> | LinkedIn <https://www.linkedin.com/in/rmannibucau> | Tomitriber <http://www.tomitribe.com> 2015-03-05 16:28 GMT+01:00 Thiago Veronezi <[email protected]>: > Hi Romain, > > This change breaks > > tomee/apache-tomee/src/test/java/org/apache/tomee/RemoteTomEEEJBContainerIT.java > with... > > java.lang.IllegalStateException: Cannot create a session after the response > has been committed > org.apache.catalina.connector.Request.doGetSession(Request.java:2872) > org.apache.catalina.connector.Request.getSession(Request.java:2232) > > org.apache.openejb.cdi.CdiAppContextsService.currentSessionId(CdiAppContextsService.java:153) > > org.apache.openejb.cdi.CdiAppContextsService.getConversationSessionId(CdiAppContextsService.java:145) > > org.apache.webbeans.portable.ConversationProducer.newInstance(ConversationProducer.java:54) > . > . > . > > []s, > Thiago. > > > On Wed, Mar 4, 2015 at 3:14 PM, <[email protected]> wrote: > > > Repository: tomee > > Updated Branches: > > refs/heads/develop d2ee62393 -> 01a80f1ba > > > > > > one more step for @ConversationScoped tcks > > > > > > Project: http://git-wip-us.apache.org/repos/asf/tomee/repo > > Commit: http://git-wip-us.apache.org/repos/asf/tomee/commit/01a80f1b > > Tree: http://git-wip-us.apache.org/repos/asf/tomee/tree/01a80f1b > > Diff: http://git-wip-us.apache.org/repos/asf/tomee/diff/01a80f1b > > > > Branch: refs/heads/develop > > Commit: 01a80f1baef4c9ee5b64c462d4f8fb010f1608a0 > > Parents: d2ee623 > > Author: Romain Manni-Bucau <[email protected]> > > Authored: Wed Mar 4 21:14:05 2015 +0100 > > Committer: Romain Manni-Bucau <[email protected]> > > Committed: Wed Mar 4 21:14:05 2015 +0100 > > > > ---------------------------------------------------------------------- > > .../openejb/cdi/CdiAppContextsService.java | 80 > ++++++-------------- > > .../server/httpd/EndWebBeansListener.java | 22 +++++- > > tck/cdi-embedded/src/test/resources/failing.xml | 2 +- > > 3 files changed, 42 insertions(+), 62 deletions(-) > > ---------------------------------------------------------------------- > > > > > > > > > http://git-wip-us.apache.org/repos/asf/tomee/blob/01a80f1b/container/openejb-core/src/main/java/org/apache/openejb/cdi/CdiAppContextsService.java > > ---------------------------------------------------------------------- > > diff --git > > > a/container/openejb-core/src/main/java/org/apache/openejb/cdi/CdiAppContextsService.java > > > b/container/openejb-core/src/main/java/org/apache/openejb/cdi/CdiAppContextsService.java > > index 3870dfa..166a992 100644 > > --- > > > a/container/openejb-core/src/main/java/org/apache/openejb/cdi/CdiAppContextsService.java > > +++ > > > b/container/openejb-core/src/main/java/org/apache/openejb/cdi/CdiAppContextsService.java > > @@ -38,6 +38,11 @@ import org.apache.webbeans.spi.ConversationService; > > import org.apache.webbeans.web.context.ServletRequestContext; > > import > > org.apache.webbeans.web.intercept.RequestScopedBeanInterceptorHandler; > > > > +import java.lang.annotation.Annotation; > > +import java.lang.reflect.Constructor; > > +import java.util.ArrayList; > > +import java.util.Collection; > > +import java.util.Map; > > import javax.enterprise.context.ApplicationScoped; > > import javax.enterprise.context.BusyConversationException; > > import javax.enterprise.context.ContextException; > > @@ -54,11 +59,6 @@ import javax.inject.Singleton; > > import javax.servlet.ServletRequestEvent; > > import javax.servlet.http.HttpServletRequest; > > import javax.servlet.http.HttpSession; > > -import java.lang.annotation.Annotation; > > -import java.lang.reflect.Constructor; > > -import java.util.ArrayList; > > -import java.util.Collection; > > -import java.util.Map; > > > > public class CdiAppContextsService extends AbstractContextsService > > implements ContextsService, ConversationService { > > public static final Object EJB_REQUEST_EVENT = new Object(); > > @@ -262,7 +262,7 @@ public class CdiAppContextsService extends > > AbstractContextsService implements Co > > } else if (scopeType.equals(ApplicationScoped.class)) { > > return getApplicationContext(); > > } else if (supportsConversation() && > > scopeType.equals(ConversationScoped.class)) { > > - return getConversationContext(true); > > + return getConversationContext(); > > } else if (scopeType.equals(Dependent.class)) { > > return dependentContext; > > } else if (scopeType.equals(Singleton.class)) { > > @@ -285,7 +285,7 @@ public class CdiAppContextsService extends > > AbstractContextsService implements Co > > initSingletonContext(); > > } else if (scopeType.equals(Singleton.class)) { //NOPMD > > // Do nothing > > - } else if (supportsConversation() && > > scopeType.equals(ConversationScoped.class)) { > > + } else if (supportsConversation() && > > scopeType.equals(ConversationScoped.class) && !isTimeout()) { > > initConversationContext(startParameter); > > } else { > > if (logger.isWarningEnabled()) { > > @@ -337,29 +337,26 @@ public class CdiAppContextsService extends > > AbstractContextsService implements Co > > //Re-initialize thread local for session > > final HttpSession session = request.getSession(false); > > > > - String cid = null; > > + final String cid = request.getParameter("cid"); > > if (session != null) { > > initSessionContext(session); > > - > > - final ServletRequestContext rc = > > getRequestContext(false); > > - if (rc != null && rc.getServletRequest() != null && > > conversationService != null && !isConversationSkipped(rc)) { > > - cid = > rc.getServletRequest().getParameter("cid"); > > + if (conversationService != null && > > !isConversationSkipped(request)) { > > if (cid != null) { > > final ConversationManager > conversationManager > > = webBeansContext.getConversationManager(); > > final ConversationImpl c = > > conversationManager.getPropogatedConversation(cid, session.getId()); > > if (c != null) { > > - > > conversationContext.set(conversationManager.getConversationContext(c)); > > + final ConversationContext context = > > conversationManager.getConversationContext(c); > > + context.setActive(true); > > + conversationContext.set(context); > > + return; > > } > > } > > } > > - } else { > > - cid = getConversationId(); > > } > > - if (cid == null && conversationContext.get() == null) { > > + > > + if (cid == null && !isTimeout()) { > > // transient but active > > - final ConversationContext context = new > > ConversationContext(); > > - context.setActive(true); > > - conversationContext.set(context); > > + initConversationContext(request); > > } > > } > > } else if (event == EJB_REQUEST_EVENT) { > > @@ -428,7 +425,7 @@ public class CdiAppContextsService extends > > AbstractContextsService implements Co > > return; > > } > > > > - final ConversationContext cc = getConversationContext(false); > > + final ConversationContext cc = getConversationContext(); > > if (cc == null) { > > return; > > } > > @@ -557,9 +554,8 @@ public class CdiAppContextsService extends > > AbstractContextsService implements Co > > > > final HttpServletRequest req = > > HttpServletRequest.class.isInstance(request) ? > > HttpServletRequest.class.cast(request) : null; > > ConversationContext context = > > ConversationContext.class.isInstance(request) ? > > ConversationContext.class.cast(request) : null; > > - final ThreadContext tc = ThreadContext.getThreadContext(); > > Object event = null; > > - if (context == null && (tc == null || tc.getCurrentOperation() > != > > Operation.TIMEOUT)) { > > + if (context == null) { > > final ConversationContext existingContext = > > conversationContext.get(); > > if (existingContext == null) { > > context = new ConversationContext(); > > @@ -593,8 +589,7 @@ public class CdiAppContextsService extends > > AbstractContextsService implements Co > > return; > > } > > > > - final ConversationContext context = > getConversationContext(false); > > - > > + final ConversationContext context = getConversationContext(); > > if (context != null) { > > context.destroy(); > > final ServletRequestContext servletRequestContext = > > getRequestContext(false); > > @@ -610,7 +605,7 @@ public class CdiAppContextsService extends > > AbstractContextsService implements Co > > } > > > > > > - private ServletRequestContext getRequestContext(final boolean > create) > > { > > + public ServletRequestContext getRequestContext(final boolean > create) { > > ServletRequestContext context = requestContext.get(); > > if (context == null && create) { > > initRequestContext(null); > > @@ -655,40 +650,11 @@ public class CdiAppContextsService extends > > AbstractContextsService implements Co > > * > > * @return conversation context > > */ > > - private ConversationContext getConversationContext(final boolean > > createIfPropagated) { > > - ConversationContext context = conversationContext.get(); > > - if (context == null && createIfPropagated && !isTimeout()) { > > - final ServletRequestContext rc = getRequestContext(true); > // > > needs to exist for Conversation scope > > - if (rc != null && rc.getServletRequest() != null && > > !isConversationSkipped(rc)) { > > - final HttpServletRequest servletRequest = > > rc.getServletRequest(); > > - final HttpSession session = > > servletRequest.getSession(false); > > - if (session != null) { > > - final String cid = > > rc.getServletRequest().getParameter("cid"); > > - if (cid != null) { > > - final ConversationManager conversationManager = > > webBeansContext.getConversationManager(); > > - final ConversationImpl conversation = > > conversationManager.getPropogatedConversation(cid, session.getId()); > > - if (conversation != null) { > > - final ConversationContext ctx = > > conversationManager.getConversationContext(conversation); > > - if (ctx != null) { > > - conversationContext.set(ctx); > > - ctx.setActive(true); > > - return ctx; > > - } > > - } > > - } > > - // else create a new one, we ensure we have a > session > > before doing it cause in several cases - stateless - we don't want to > > create a new one if not > > - return initConversationContext(servletRequest); > > - } > > - } > > - } > > - if (context != null && !context.isActive() && > createIfPropagated) > > { > > - context.setActive(true); > > - } > > - return context; > > + private ConversationContext getConversationContext() { > > + return conversationContext.get(); > > } > > > > - private boolean isConversationSkipped(final ServletRequestContext > rc) > > { > > - final HttpServletRequest servletRequest = > rc.getServletRequest(); > > + private boolean isConversationSkipped(final HttpServletRequest > > servletRequest) { > > return > > "none".equals(servletRequest.getParameter("conversationPropagation")) || > > "true".equals(servletRequest.getParameter("nocid")); > > } > > > > > > > > > http://git-wip-us.apache.org/repos/asf/tomee/blob/01a80f1b/server/openejb-http/src/main/java/org/apache/openejb/server/httpd/EndWebBeansListener.java > > ---------------------------------------------------------------------- > > diff --git > > > a/server/openejb-http/src/main/java/org/apache/openejb/server/httpd/EndWebBeansListener.java > > > b/server/openejb-http/src/main/java/org/apache/openejb/server/httpd/EndWebBeansListener.java > > index e1a7968..af340ac 100644 > > --- > > > a/server/openejb-http/src/main/java/org/apache/openejb/server/httpd/EndWebBeansListener.java > > +++ > > > b/server/openejb-http/src/main/java/org/apache/openejb/server/httpd/EndWebBeansListener.java > > @@ -154,7 +154,7 @@ public class EndWebBeansListener implements > > ServletContextListener, ServletReque > > * {@inheritDoc} > > */ > > @Override > > - public void sessionDestroyed(HttpSessionEvent event) { > > + public void sessionDestroyed(final HttpSessionEvent event) { > > if (webBeansContext == null) { > > return; > > } > > @@ -171,16 +171,30 @@ public class EndWebBeansListener implements > > ServletContextListener, ServletReque > > > > > WebappWebBeansContext.class.cast(webBeansContext).getParent().getContextsService().endContext(SessionScoped.class, > > event.getSession()); > > } > > > > + final CdiAppContextsService appContextsService = > > CdiAppContextsService.class.cast(webBeansContext.getContextsService()); > > + if (appContextsService.getRequestContext(false) != null) { > > + appContextsService.pushRequestReleasable(new Runnable() { > > + @Override > > + public void run() { > > + doDestroyConversations(event.getSession().getId()); > > + } > > + }); > > + } else { > > + doDestroyConversations(event.getSession().getId()); > > + } > > + > > + destroyFakedRequest(); > > + } > > + > > + private void doDestroyConversations(final String id) { > > final ConversationManager conversationManager = > > webBeansContext.getConversationManager(); > > - final Map<Conversation, ConversationContext> cc = > > > conversationManager.getAndRemoveConversationMapWithSessionId(event.getSession().getId()); > > + final Map<Conversation, ConversationContext> cc = > > conversationManager.getAndRemoveConversationMapWithSessionId(id); > > for (final Map.Entry<Conversation, ConversationContext> c : > > cc.entrySet()) { > > if (c != null) { > > c.getValue().destroy(); > > > > webBeansContext.getBeanManagerImpl().fireEvent(c.getKey().getId(), > > CdiAppContextsService.DestroyedLiteral.CONVERSATION); > > } > > } > > - > > - destroyFakedRequest(); > > } > > > > private void destroyFakedRequest() { > > > > > > > http://git-wip-us.apache.org/repos/asf/tomee/blob/01a80f1b/tck/cdi-embedded/src/test/resources/failing.xml > > ---------------------------------------------------------------------- > > diff --git a/tck/cdi-embedded/src/test/resources/failing.xml > > b/tck/cdi-embedded/src/test/resources/failing.xml > > index 81a4955..613b81e 100644 > > --- a/tck/cdi-embedded/src/test/resources/failing.xml > > +++ b/tck/cdi-embedded/src/test/resources/failing.xml > > @@ -31,7 +31,7 @@ > > -Dopenejb.embedded.try-jsp=true > > --> > > <classes> > > - <class > > > name="org.jboss.cdi.tck.tests.deployment.initialization.ApplicationInitializationLifecycleTest" > > /> > > + <class > > > name="org.jboss.cdi.tck.tests.context.conversation.servlet.ServletConversationTest" > > /> > > </classes> > > </test> > > </suite> > > > > >
