Author: lincolnthree Date: 2010-03-12 12:13:21 -0500 (Fri, 12 Mar 2010) New Revision: 12197 Removed: modules/faces/trunk/faces-config.NavData modules/faces/trunk/seam-faces.iml Modified: modules/faces/trunk/src/main/java/org/jboss/seam/faces/context/FlashScopedContext.java modules/faces/trunk/src/main/java/org/jboss/seam/faces/context/ViewScopedContext.java Log: * Updated thread safety and code quality. Deleted: modules/faces/trunk/faces-config.NavData =================================================================== --- modules/faces/trunk/faces-config.NavData 2010-03-12 16:07:32 UTC (rev 12196) +++ modules/faces/trunk/faces-config.NavData 2010-03-12 17:13:21 UTC (rev 12197) @@ -1,6 +0,0 @@ -<?xml version="1.0" encoding="UTF-8"?> -<Scene Scope="Project" version="2"> - <Scope Scope="Faces Configuration Only"/> - <Scope Scope="Project"/> - <Scope Scope="All Faces Configurations"/> -</Scene> Deleted: modules/faces/trunk/seam-faces.iml =================================================================== --- modules/faces/trunk/seam-faces.iml 2010-03-12 16:07:32 UTC (rev 12196) +++ modules/faces/trunk/seam-faces.iml 2010-03-12 17:13:21 UTC (rev 12197) @@ -1,18 +0,0 @@ -<?xml version="1.0" encoding="UTF-8"?> -<module org.jetbrains.idea.maven.project.MavenProjectsManager.isMavenModule="true" type="JAVA_MODULE" version="4"> - <component name="NewModuleRootManager" LANGUAGE_LEVEL="JDK_1_6" inherit-compiler-output="false"> - <output url="file://$MODULE_DIR$/target/classes" /> - <output-test url="file://$MODULE_DIR$/target/test-classes" /> - <content url="file://$MODULE_DIR$"> - <sourceFolder url="file://$MODULE_DIR$/src/main/java" isTestSource="false" /> - <sourceFolder url="file://$MODULE_DIR$/src/main/resources" isTestSource="false" /> - <sourceFolder url="file://$MODULE_DIR$/src/test/java" isTestSource="true" /> - <sourceFolder url="file://$MODULE_DIR$/src/test/resources" isTestSource="true" /> - <excludeFolder url="file://$MODULE_DIR$/target" /> - </content> - <orderEntry type="inheritedJdk" /> - <orderEntry type="sourceFolder" forTests="false" /> - <orderEntry type="library" scope="PROVIDED" name="Maven: javax:javaee-web-api:6.0" level="project" /> - </component> -</module> - Modified: modules/faces/trunk/src/main/java/org/jboss/seam/faces/context/FlashScopedContext.java =================================================================== --- modules/faces/trunk/src/main/java/org/jboss/seam/faces/context/FlashScopedContext.java 2010-03-12 16:07:32 UTC (rev 12196) +++ modules/faces/trunk/src/main/java/org/jboss/seam/faces/context/FlashScopedContext.java 2010-03-12 17:13:21 UTC (rev 12197) @@ -24,6 +24,8 @@ */ public class FlashScopedContext implements Context, PhaseListener { + private static final long serialVersionUID = -1580689204988513798L; + private final static String COMPONENT_MAP_NAME = "org.jboss.seam.faces.flash.componentInstanceMap"; private final static String CREATIONAL_MAP_NAME = "org.jboss.seam.faces.flash.creationalInstanceMap"; private final ThreadLocal<Map<Contextual<?>, Object>> lastComponentInstanceMap = new ThreadLocal<Map<Contextual<?>, Object>>(); @@ -33,9 +35,7 @@ public <T> T get(final Contextual<T> component) { assertActive(); - Map<Contextual<?>, Object> componentInstanceMap = getComponentInstanceMap(); - T instance = (T) componentInstanceMap.get(component); - return instance; + return (T) getComponentInstanceMap().get(component); } @SuppressWarnings("unchecked") @@ -74,6 +74,57 @@ return FlashScoped.class; } + public boolean isActive() + { + return getFlash() != null; + } + + /** + * This method should, **in theory**, catch the current instanceMap (which + * is the previous lifecycle's next instanceMap.) These are the objects that + * we want cleaned up at the end of the current render-response phase, so we + * save them here until after the RENDER_RESPONSE phase, because otherwise + * they would have been destroyed by the Flash, and we would no longer have + * access to them. + */ + public void beforePhase(final PhaseEvent event) + { + this.lastComponentInstanceMap.set(getComponentInstanceMap()); + this.lastCreationalContextMap.set(getCreationalContextMap()); + } + + /** + * Do the object cleanup using our saved references. + */ + @SuppressWarnings("unchecked") + public void afterPhase(final PhaseEvent event) + { + // TODO verify that this is actually destroying the beans we want to be + // destroyed... flash is confusing, tests will make sense of it + Map<Contextual<?>, Object> componentInstanceMap = lastComponentInstanceMap.get(); + Map<Contextual<?>, CreationalContext<?>> creationalContextMap = lastCreationalContextMap.get(); + + if (componentInstanceMap != null) + { + for (Entry<Contextual<?>, Object> componentEntry : componentInstanceMap.entrySet()) + { + Contextual contextual = componentEntry.getKey(); + Object instance = componentEntry.getValue(); + CreationalContext creational = creationalContextMap.get(contextual); + + contextual.destroy(instance, creational); + } + } + + this.lastComponentInstanceMap.remove(); + this.lastCreationalContextMap.remove(); + } + + public PhaseId getPhaseId() + { + return PhaseId.RENDER_RESPONSE; + } + private Flash getFlash() { FacesContext currentInstance = FacesContext.getCurrentInstance(); @@ -85,17 +136,12 @@ return null; } - public boolean isActive() - { - return getFlash() != null; - } - private void assertActive() { if (!isActive()) { throw new ContextNotActiveException( - "WebBeans context with scope annotation @FlashScoped is not active with respect to the current thread"); + "Seam context with scope annotation @FlashScoped is not active with respect to the current thread"); } } @@ -127,39 +173,4 @@ return map; } - public void beforePhase(final PhaseEvent event) - { - this.lastComponentInstanceMap.set(getComponentInstanceMap()); - this.lastCreationalContextMap.set(getCreationalContextMap()); - } - - @SuppressWarnings("unchecked") - public void afterPhase(final PhaseEvent event) - { - // TODO verify that this is actually destroying the beans we want to be - // destroyed... flash is confusing, tests will make sense of it - Map<Contextual<?>, Object> componentInstanceMap = lastComponentInstanceMap.get(); - Map<Contextual<?>, CreationalContext<?>> creationalContextMap = lastCreationalContextMap.get(); - - if (componentInstanceMap != null) - { - for (Entry<Contextual<?>, Object> componentEntry : componentInstanceMap.entrySet()) - { - Contextual contextual = componentEntry.getKey(); - Object instance = componentEntry.getValue(); - CreationalContext creational = creationalContextMap.get(contextual); - - contextual.destroy(instance, creational); - } - } - - this.lastComponentInstanceMap.remove(); - this.lastCreationalContextMap.remove(); - } - - public PhaseId getPhaseId() - { - return PhaseId.RENDER_RESPONSE; - } - } Modified: modules/faces/trunk/src/main/java/org/jboss/seam/faces/context/ViewScopedContext.java =================================================================== --- modules/faces/trunk/src/main/java/org/jboss/seam/faces/context/ViewScopedContext.java 2010-03-12 16:07:32 UTC (rev 12196) +++ modules/faces/trunk/src/main/java/org/jboss/seam/faces/context/ViewScopedContext.java 2010-03-12 17:13:21 UTC (rev 12197) @@ -53,22 +53,11 @@ if (!isJsfSubscribed) { FacesContext.getCurrentInstance().getApplication().subscribeToEvent(PreDestroyViewMapEvent.class, this); - isJsfSubscribed = true; } - Map<String, Object> viewMap = getViewMap(); + T instance = (T) getComponentInstanceMap().get(component); - ConcurrentHashMap<Contextual<?>, Object> componentInstanceMap = (ConcurrentHashMap<Contextual<?>, Object>) viewMap - .get(COMPONENT_MAP_NAME); - - if (componentInstanceMap == null) - { - return null; - } - - T instance = (T) componentInstanceMap.get(component); - return instance; } @@ -77,54 +66,27 @@ { assertActive(); - Map<String, Object> viewMap = getViewMap(); - - ConcurrentHashMap<Contextual<?>, Object> componentInstanceMap = (ConcurrentHashMap<Contextual<?>, Object>) viewMap - .get(COMPONENT_MAP_NAME); - - if (componentInstanceMap == null) + T instance = get(component); + if (instance == null) { - // TODO we now need to start being carefull with reentrancy... - componentInstanceMap = new ConcurrentHashMap<Contextual<?>, Object>(); - viewMap.put(COMPONENT_MAP_NAME, componentInstanceMap); - } - - ConcurrentHashMap<Contextual<?>, CreationalContext<?>> creationalContextMap = (ConcurrentHashMap<Contextual<?>, CreationalContext<?>>) viewMap - .get(CREATIONAL_MAP_NAME); - if (creationalContextMap == null) - { - // TODO we now need to start being carefull with reentrancy... - creationalContextMap = new ConcurrentHashMap<Contextual<?>, CreationalContext<?>>(); - viewMap.put(CREATIONAL_MAP_NAME, creationalContextMap); - } - - T instance = (T) componentInstanceMap.get(component); - if (instance != null) - { - return instance; - } - - if (creationalContext == null) - { - return null; - } - - synchronized (componentInstanceMap) - { - // just to make sure... - T i = (T) componentInstanceMap.get(component); - if (i != null) + if (creationalContext != null) { - return i; + Map<Contextual<?>, Object> componentInstanceMap = getComponentInstanceMap(); + Map<Contextual<?>, CreationalContext<?>> creationalContextMap = getCreationalInstanceMap(); + synchronized (componentInstanceMap) + { + instance = (T) componentInstanceMap.get(component); + if (instance == null) + { + instance = component.create(creationalContext); + if (instance != null) + { + componentInstanceMap.put(component, instance); + creationalContextMap.put(component, creationalContext); + } + } + } } - - instance = component.create(creationalContext); - - if (instance != null) - { - componentInstanceMap.put(component, instance); - creationalContextMap.put(component, creationalContext); - } } return instance; @@ -135,9 +97,6 @@ return ViewScoped.class; } - /** - * The view context is active if a valid ViewRoot could be detected. - */ public boolean isActive() { return getViewRoot() != null; @@ -148,7 +107,7 @@ if (!isActive()) { throw new ContextNotActiveException( - "WebBeans context with scope annotation @ViewScoped is not active with respect to the current thread"); + "Seam context with scope annotation @ViewScoped is not active with respect to the current thread"); } } @@ -175,23 +134,17 @@ { if (event instanceof PreDestroyViewMapEvent) { - // better use the viewmap we get from the event to prevent - // concurrent modification problems - Map<String, Object> viewMap = ((UIViewRoot) event.getSource()).getViewMap(); + Map<Contextual<?>, Object> componentInstanceMap = getComponentInstanceMap(); + Map<Contextual<?>, CreationalContext<?>> creationalContextMap = getCreationalInstanceMap(); - ConcurrentHashMap<Contextual<?>, Object> componentInstanceMap = (ConcurrentHashMap<Contextual<?>, Object>) viewMap - .get(COMPONENT_MAP_NAME); - - ConcurrentHashMap<Contextual<?>, CreationalContext<?>> creationalContextMap = (ConcurrentHashMap<Contextual<?>, CreationalContext<?>>) viewMap - .get(CREATIONAL_MAP_NAME); - if (componentInstanceMap != null) { for (Entry<Contextual<?>, Object> componentEntry : componentInstanceMap.entrySet()) { - // there is no nice way to explain the Java Compiler that we - // are handling the same type T, - // therefore we need completely drop the type information :( + /* + * No way to inform the compiler of type <T> information, so + * it has to be abandoned here :( + */ Contextual contextual = componentEntry.getKey(); Object instance = componentEntry.getValue(); CreationalContext creational = creationalContextMap.get(contextual); @@ -225,4 +178,50 @@ return null; } + + @SuppressWarnings("unchecked") + private Map<Contextual<?>, Object> getComponentInstanceMap() + { + Map<String, Object> viewMap = getViewMap(); + Map<Contextual<?>, Object> componentInstanceMap = (ConcurrentHashMap<Contextual<?>, Object>) viewMap + .get(COMPONENT_MAP_NAME); + + if (componentInstanceMap == null) + { + synchronized (componentInstanceMap) + { + componentInstanceMap = (ConcurrentHashMap<Contextual<?>, Object>) viewMap.get(COMPONENT_MAP_NAME); + if (componentInstanceMap == null) + { + componentInstanceMap = new ConcurrentHashMap<Contextual<?>, Object>(); + viewMap.put(COMPONENT_MAP_NAME, componentInstanceMap); + } + } + } + + return componentInstanceMap; + } + + @SuppressWarnings("unchecked") + private Map<Contextual<?>, CreationalContext<?>> getCreationalInstanceMap() + { + Map<String, Object> viewMap = getViewMap(); + Map<Contextual<?>, CreationalContext<?>> creationalContextMap = (Map<Contextual<?>, CreationalContext<?>>) viewMap + .get(CREATIONAL_MAP_NAME); + + if (creationalContextMap == null) + { + synchronized (creationalContextMap) + { + creationalContextMap = (Map<Contextual<?>, CreationalContext<?>>) viewMap.get(CREATIONAL_MAP_NAME); + if (creationalContextMap == null) + { + creationalContextMap = new ConcurrentHashMap<Contextual<?>, CreationalContext<?>>(); + viewMap.put(CREATIONAL_MAP_NAME, creationalContextMap); + } + } + } + + return creationalContextMap; + } } \ No newline at end of file