Author: nfilotto Date: 2011-03-07 12:19:50 -0500 (Mon, 07 Mar 2011) New Revision: 4062 Modified: kernel/trunk/exo.kernel.container/src/main/java/org/exoplatform/container/ConcurrentPicoContainer.java kernel/trunk/exo.kernel.container/src/main/java/org/exoplatform/container/ExoContainer.java kernel/trunk/exo.kernel.container/src/main/java/org/exoplatform/container/jmx/MX4JComponentAdapter.java kernel/trunk/exo.kernel.container/src/test/java/org/exoplatform/container/TestExoContainer.java kernel/trunk/exo.kernel.container/src/test/resources/org/exoplatform/container/test-exo-container.xml Log: EXOJCR-1228: To avoid regressions, we propose a patch for it however this bug is mainly due to bad practices in WCM. Indeed they don't add the dependencies in the constructor as they are supposed to do but they get them within the body of the constructor using an Util class that call container.getComponentInstanceOfType(Class) Modified: kernel/trunk/exo.kernel.container/src/main/java/org/exoplatform/container/ConcurrentPicoContainer.java =================================================================== --- kernel/trunk/exo.kernel.container/src/main/java/org/exoplatform/container/ConcurrentPicoContainer.java 2011-03-05 11:29:46 UTC (rev 4061) +++ kernel/trunk/exo.kernel.container/src/main/java/org/exoplatform/container/ConcurrentPicoContainer.java 2011-03-07 17:19:50 UTC (rev 4062) @@ -88,6 +88,12 @@ private final AtomicBoolean disposed = new AtomicBoolean(); private final Set<PicoContainer> children = new CopyOnWriteArraySet<PicoContainer>(); + + /** + * Context used to keep in memory the components that are currently being created. + * This context is used to prevent cyclic resolution due to component plugins. + */ + private final ThreadLocal<Map<Object, Object>> depResolutionCtx = new ThreadLocal<Map<Object, Object>>(); /** * Creates a new container with a custom ComponentAdapterFactory and a parent container. @@ -391,12 +397,64 @@ } } + /** + * If no {@link ComponentAdapter} can be found it returns <tt>null</tt> otherwise + * it first try to get it from the dependency resolution context if it still cannot + * be found we get the instance from the {@link ComponentAdapter}. + * @see org.picocontainer.PicoContainer#getComponentInstanceOfType(java.lang.Class) + */ public Object getComponentInstanceOfType(Class componentType) { final ComponentAdapter componentAdapter = getComponentAdapterOfType(componentType); - return componentAdapter == null ? null : getInstance(componentAdapter); + if (componentAdapter == null) + return null; + Map<Object, Object> map = depResolutionCtx.get(); + if (map != null) + { + Object result = map.get(componentType); + if (result != null) + { + return result; + } + } + return getInstance(componentAdapter); } - + + /** + * Add the component corresponding to the given key, to the dependency resolution + * context + * @param key The key of the component to add to the context + * @param component The instance of the component to add to the context + */ + public void addComponentToCtx(Object key, Object component) + { + Map<Object, Object> map = depResolutionCtx.get(); + if (map == null) + { + map = new HashMap<Object, Object>(); + depResolutionCtx.set(map); + } + map.put(key, component); + } + + /** + * Remove the component corresponding to the given key, from the dependency resolution + * context + * @param key The key of the component to remove from the context + */ + public void removeComponentFromCtx(Object key) + { + Map<Object, Object> map = depResolutionCtx.get(); + if (map != null) + { + map.remove(key); + if (map.isEmpty()) + { + depResolutionCtx.set(null); + } + } + } + private Object getInstance(ComponentAdapter componentAdapter) { // check wether this is our adapter Modified: kernel/trunk/exo.kernel.container/src/main/java/org/exoplatform/container/ExoContainer.java =================================================================== --- kernel/trunk/exo.kernel.container/src/main/java/org/exoplatform/container/ExoContainer.java 2011-03-05 11:29:46 UTC (rev 4061) +++ kernel/trunk/exo.kernel.container/src/main/java/org/exoplatform/container/ExoContainer.java 2011-03-07 17:19:50 UTC (rev 4062) @@ -81,12 +81,6 @@ } static Log log = ExoLogger.getLogger("exo.kernel.container.ExoContainer"); - - /** - * Context used to keep in memory the components that are currently being created. - * This context is used to prevent cyclic resolution due to component plugins. - */ - private final ThreadLocal<Map<Object, Object>> depResolutionCtx = new ThreadLocal<Map<Object, Object>>(); private Map<String, ComponentLifecyclePlugin> componentLifecylePlugin_ = new HashMap<String, ComponentLifecyclePlugin>(); @@ -369,7 +363,7 @@ } else { - args[i] = getComponentInstanceOfTypeInternal(parameters[i]); + args[i] = getComponentInstanceOfType(parameters[i]); if (args[i] == null) { satisfied = false; @@ -393,59 +387,4 @@ ConfigurationManager cm = (ConfigurationManager)getComponentInstanceOfType(ConfigurationManager.class); return cm == null ? null : cm.getConfiguration(); } - - /** - * Add the component corresponding to the given key, to the dependency resolution - * context - * @param key The key of the component to add to the context - * @param component The instance of the component to add to the context - */ - public void addComponentToCtx(Object key, Object component) - { - Map<Object, Object> map = depResolutionCtx.get(); - if (map == null) - { - map = new HashMap<Object, Object>(); - depResolutionCtx.set(map); - } - map.put(key, component); - } - - /** - * Remove the component corresponding to the given key, from the dependency resolution - * context - * @param key The key of the component to remove from the context - */ - public void removeComponentFromCtx(Object key) - { - Map<Object, Object> map = depResolutionCtx.get(); - if (map != null) - { - map.remove(key); - if (map.isEmpty()) - { - depResolutionCtx.set(null); - } - } - } - - /** - * Try to get the component from the dependency resolution context and if it cannot be found - * it will try to get it from the method <code>getComponentInstanceOfType</code> - * @param componentType the type of the component to find - * @return the instance of the component corresponding to the given component type - */ - private Object getComponentInstanceOfTypeInternal(Class componentType) - { - Map<Object, Object> map = depResolutionCtx.get(); - if (map != null) - { - Object result = map.get(componentType); - if (result != null) - { - return result; - } - } - return getComponentInstanceOfType(componentType); - } } Modified: kernel/trunk/exo.kernel.container/src/main/java/org/exoplatform/container/jmx/MX4JComponentAdapter.java =================================================================== --- kernel/trunk/exo.kernel.container/src/main/java/org/exoplatform/container/jmx/MX4JComponentAdapter.java 2011-03-05 11:29:46 UTC (rev 4061) +++ kernel/trunk/exo.kernel.container/src/main/java/org/exoplatform/container/jmx/MX4JComponentAdapter.java 2011-03-07 17:19:50 UTC (rev 4062) @@ -182,7 +182,7 @@ } catch (Exception ex) { - log.error("Failed to instanciate plugin " + plugin.getName() + "for component " + component + ": " + log.error("Failed to instanciate plugin " + plugin.getName() + " for component " + component + ": " + ex.getMessage(), ex); } } Modified: kernel/trunk/exo.kernel.container/src/test/java/org/exoplatform/container/TestExoContainer.java =================================================================== --- kernel/trunk/exo.kernel.container/src/test/java/org/exoplatform/container/TestExoContainer.java 2011-03-05 11:29:46 UTC (rev 4061) +++ kernel/trunk/exo.kernel.container/src/test/java/org/exoplatform/container/TestExoContainer.java 2011-03-07 17:19:50 UTC (rev 4062) @@ -111,6 +111,30 @@ assertEquals(value, plugin.myClass_); } + public void testStackOverFlow2() + { + final RootContainer container = createRootContainer("test-exo-container.xml"); + SOE1 soe1 = (SOE1)container.getComponentInstanceOfType(SOE1.class); + assertNotNull(soe1); + SOEPlugin soe1Plugin = soe1.plugin; + assertNotNull(soe1Plugin); + assertNotNull(soe1Plugin.soe2); + assertEquals(soe1, soe1Plugin.soe2.soe1); + } + + public void testStackOverFlow3() + { + final RootContainer container = createRootContainer("test-exo-container.xml"); + SOE2 soe2 = (SOE2)container.getComponentInstanceOfType(SOE2.class); + assertNotNull(soe2); + assertNotNull(soe2.soe1); + SOEPlugin soe1Plugin = soe2.soe1.plugin; + assertNotNull(soe1Plugin); + assertNotNull(soe1Plugin.soe2); + assertEquals(soe2.soe1, soe1Plugin.soe2.soe1); + assertEquals(container.getComponentInstanceOfType(SOE1.class), soe2.soe1); + } + public void testCyclicRef() { final RootContainer container = createRootContainer("test-exo-container.xml", "testCyclicRef"); @@ -835,5 +859,32 @@ { disposed = true; } - } + } + + public static class SOE1 + { + public SOEPlugin plugin; + public void addPlugin(SOEPlugin plugin) + { + this.plugin = plugin; + } + } + + public static class SOEPlugin extends BaseComponentPlugin + { + public SOE2 soe2; + public SOEPlugin(SOE2 soe2) + { + this.soe2 = soe2; + } + } + + public static class SOE2 + { + public SOE1 soe1; + public SOE2() + { + this.soe1 = (SOE1)ExoContainerContext.getCurrentContainer().getComponentInstanceOfType(SOE1.class); + } + } } Modified: kernel/trunk/exo.kernel.container/src/test/resources/org/exoplatform/container/test-exo-container.xml =================================================================== --- kernel/trunk/exo.kernel.container/src/test/resources/org/exoplatform/container/test-exo-container.xml 2011-03-05 11:29:46 UTC (rev 4061) +++ kernel/trunk/exo.kernel.container/src/test/resources/org/exoplatform/container/test-exo-container.xml 2011-03-07 17:19:50 UTC (rev 4062) @@ -45,6 +45,19 @@ </component-plugin> </component-plugins> </component> + <component> + <type>org.exoplatform.container.TestExoContainer$SOE1</type> + <component-plugins> + <component-plugin> + <name>stackoverflow-test-plugin</name> + <set-method>addPlugin</set-method> + <type>org.exoplatform.container.TestExoContainer$SOEPlugin</type> + </component-plugin> + </component-plugins> + </component> + <component> + <type>org.exoplatform.container.TestExoContainer$SOE2</type> + </component> <component profiles="testCyclicRef"> <type>org.exoplatform.container.TestExoContainer$A</type> </component>