Author: dallen6 Date: 2010-02-03 16:05:45 -0500 (Wed, 03 Feb 2010) New Revision: 5719 Modified: core/trunk/impl/src/main/java/org/jboss/weld/servlet/ConversationBeanStore.java core/trunk/impl/src/main/java/org/jboss/weld/servlet/HttpPassThruOnDemandSessionBeanStore.java core/trunk/impl/src/main/java/org/jboss/weld/servlet/HttpPassThruSessionBeanStore.java core/trunk/impl/src/main/java/org/jboss/weld/servlet/HttpSessionBeanStore.java Log: WELD-403 Removed concurrency issues with the pass through bean store. Modified: core/trunk/impl/src/main/java/org/jboss/weld/servlet/ConversationBeanStore.java =================================================================== --- core/trunk/impl/src/main/java/org/jboss/weld/servlet/ConversationBeanStore.java 2010-02-03 17:15:58 UTC (rev 5718) +++ core/trunk/impl/src/main/java/org/jboss/weld/servlet/ConversationBeanStore.java 2010-02-03 21:05:45 UTC (rev 5719) @@ -29,14 +29,12 @@ public class ConversationBeanStore extends HttpPassThruSessionBeanStore { - private final String cid; private final NamingScheme namingScheme; public ConversationBeanStore(HttpSession session, boolean sessionInvalidated, String cid) { - attachToSession(session); - this.cid = cid; this.namingScheme = new NamingScheme(ConversationContext.class.getName() + "[" + cid + "]", "#"); + attachToSession(session); } @Override Modified: core/trunk/impl/src/main/java/org/jboss/weld/servlet/HttpPassThruOnDemandSessionBeanStore.java =================================================================== --- core/trunk/impl/src/main/java/org/jboss/weld/servlet/HttpPassThruOnDemandSessionBeanStore.java 2010-02-03 17:15:58 UTC (rev 5718) +++ core/trunk/impl/src/main/java/org/jboss/weld/servlet/HttpPassThruOnDemandSessionBeanStore.java 2010-02-03 21:05:45 UTC (rev 5719) @@ -33,6 +33,10 @@ public HttpPassThruOnDemandSessionBeanStore(HttpServletRequest request) { this.request = request; + if (request.getSession(false) != null) + { + attachToSession(request.getSession()); + } } @Override Modified: core/trunk/impl/src/main/java/org/jboss/weld/servlet/HttpPassThruSessionBeanStore.java =================================================================== --- core/trunk/impl/src/main/java/org/jboss/weld/servlet/HttpPassThruSessionBeanStore.java 2010-02-03 17:15:58 UTC (rev 5718) +++ core/trunk/impl/src/main/java/org/jboss/weld/servlet/HttpPassThruSessionBeanStore.java 2010-02-03 21:05:45 UTC (rev 5719) @@ -17,31 +17,45 @@ package org.jboss.weld.servlet; +import static org.jboss.weld.logging.Category.CONTEXT; +import static org.jboss.weld.logging.LoggerFactory.loggerFactory; + +import java.util.ArrayList; import java.util.Collections; import java.util.Enumeration; +import java.util.List; import javax.servlet.http.HttpSession; import org.jboss.weld.context.api.ContextualInstance; -import org.jboss.weld.context.api.helpers.ConcurrentHashMapBeanStore; +import org.jboss.weld.context.beanstore.HashMapBeanStore; +import org.jboss.weld.context.beanstore.NamingScheme; +import org.jboss.weld.util.collections.EnumerationList; +import org.slf4j.cal10n.LocLogger; /** * A BeanStore that maintains Contextuals in a hash map and writes them through - * to a HttpSession. It also has the capability to reload the hash map from an - * existing session or to rewrite the map entries into a session. + * to a HttpSession. When this BeanStore is attached to a session, it will load + * all the existing contextuals from the session within the naming scheme for + * this BeanStore. All read operations are directly against the local map. * * @author David Allen */ public class HttpPassThruSessionBeanStore extends HttpSessionBeanStore { - private static final long serialVersionUID = 8923580660774253915L; + private static final long serialVersionUID = 8923580660774253915L; + private static final LocLogger log = loggerFactory().getLogger(CONTEXT); - private ConcurrentHashMapBeanStore delegateBeanStore = new ConcurrentHashMapBeanStore(); - private boolean attachedToSession = false; - private boolean invalidated = false; - private static final String SESSION_ATTRIBUTE_NAME = HttpPassThruSessionBeanStore.class.getName() + ".sessionBeanStore"; + private HashMapBeanStore delegateBeanStore = new HashMapBeanStore(); + private boolean attachedToSession = false; + private boolean invalidated = false; + public HttpPassThruSessionBeanStore() + { + log.trace("New bean store created: " + this); + } + /** * Attaches this pass-through bean store to the given session. * @@ -50,8 +64,8 @@ public void attachToSession(HttpSession session) { super.attachToSession(session); + attachedToSession = true; loadFromSession(session); - attachedToSession = true; } /** @@ -72,6 +86,7 @@ { detachFromSession(); invalidated = true; + log.trace("Bean store " + this + " is invalidated"); } /** @@ -95,24 +110,19 @@ } /** - * Loads the map from the given session into this map store, if it already - * exists in this session. If it does not already exist, then a new map is - * created since the session does not already have any contextuals stored in - * it. + * Loads the map with all contextuals currently stored in the session for + * this bean store. * * @param newSession a new HttpSession being attached */ protected void loadFromSession(HttpSession newSession) { - Object map = newSession.getAttribute(SESSION_ATTRIBUTE_NAME); - if (map != null) + log.trace("Loading bean store " + this + " map from session " + newSession.getId()); + for (String id : this.getFilteredAttributeNames()) { - delegateBeanStore = (ConcurrentHashMapBeanStore) map; + delegateBeanStore.put(id, (ContextualInstance<?>) super.getAttribute(id)); + log.trace("Added contextual " + super.getAttribute(id) + " under ID " + id); } - else - { - newSession.setAttribute(SESSION_ATTRIBUTE_NAME, delegateBeanStore); - } } @Override @@ -132,7 +142,14 @@ { if (attachedToSession && !isInvalidated()) { - super.removeAttribute(key); + try + { + super.removeAttribute(key); + } + catch (IllegalStateException e) + { + invalidate(); + } } delegateBeanStore.delegate().remove(key); } @@ -143,8 +160,44 @@ { if (attachedToSession && !isInvalidated()) { - super.setAttribute(key, instance); + try + { + super.setAttribute(key, instance); + log.trace("***** Added " + key + " to session " + this.getSession().getId()); + } + catch (IllegalStateException e) + { + invalidate(); + } } delegateBeanStore.put(key, (ContextualInstance<? extends Object>) instance); + log.trace("Added instance for key " + key); } + + /** + * Gets the list of attribute names that is held by the bean store + * + * @return The list of attribute names + */ + private List<String> getFilteredAttributeNames() + { + List<String> attributeNames = new ArrayList<String>(); + NamingScheme namingScheme = getNamingScheme(); + try + { + for (String attributeName : new EnumerationList<String>(super.getAttributeNames())) + { + if (namingScheme.acceptKey(attributeName)) + { + attributeNames.add(attributeName); + } + } + } + catch (IllegalStateException e) + { + invalidate(); + } + return attributeNames; + } + } Modified: core/trunk/impl/src/main/java/org/jboss/weld/servlet/HttpSessionBeanStore.java =================================================================== --- core/trunk/impl/src/main/java/org/jboss/weld/servlet/HttpSessionBeanStore.java 2010-02-03 17:15:58 UTC (rev 5718) +++ core/trunk/impl/src/main/java/org/jboss/weld/servlet/HttpSessionBeanStore.java 2010-02-03 21:05:45 UTC (rev 5719) @@ -29,7 +29,6 @@ * * @author Nicklas Karlsson * @author David Allen - * * @see org.jboss.weld.context.AbstractApplicationContext */ public class HttpSessionBeanStore extends AbstractAttributeBackedBeanStore @@ -38,7 +37,7 @@ private static final NamingScheme NAMING_SCHEME = new NamingScheme(SessionContext.class.getName(), "#"); // The HTTP session context to use as backing map - protected HttpSession session; + private HttpSession session; /** * Attaches this bean store to a session dynamically. This allows the session @@ -72,6 +71,11 @@ return session.getAttributeNames(); } + protected HttpSession getSession() + { + return session; + } + /** * @see org.jboss.weld.context.beanstore.AbstractAttributeBackedBeanStore#removeAttributes() */