[seam-commits] Seam SVN: r12197 - in modules/faces/trunk: src/main/java/org/jboss/seam/faces/context and 1 other directory.

seam-commits at lists.jboss.org seam-commits at lists.jboss.org
Fri Mar 12 12:13:22 EST 2010


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



More information about the seam-commits mailing list