FORGE-1565: Fixed possible CCE

impl/src/main/java/org/jboss/forge/furnace/container/simple/impl/SimpleServiceRegistryImpl.java

@@ -35,9 +35,9 @@ public class SimpleServiceRegistryImpl implements ServiceRegistry
    private final Addon addon;
    private final Set<Class<?>> serviceTypes;
 
-   private final Map<Integer, Class<?>> classCache = new WeakHashMap<>();
-   private final Map<Integer, ExportedInstance<?>> instanceCache = new WeakHashMap<>();
-   private final Map<Integer, Set<ExportedInstance<?>>> instancesCache = new WeakHashMap<>();
+   private final Map<String, Class<?>> classCache = new WeakHashMap<>();
+   private final Map<String, ExportedInstance<?>> instanceCache = new WeakHashMap<>();
+   private final Map<String, Set<ExportedInstance<?>>> instancesCache = new WeakHashMap<>();
 
    public SimpleServiceRegistryImpl(Furnace furnace, Addon addon, Set<Class<?>> serviceTypes)
    {
@@ -78,7 +78,7 @@ public <T> Set<ExportedInstance<T>> getExportedInstances(Class<T> clazz)
          return Collections.emptySet();
       }
 
-      Set<ExportedInstance<T>> result = (Set) instancesCache.get(actualLoadedType.hashCode());
+      Set<ExportedInstance<T>> result = (Set) instancesCache.get(actualLoadedType.getName());
 
       if (result == null)
       {
@@ -91,7 +91,7 @@ public <T> Set<ExportedInstance<T>> getExportedInstances(Class<T> clazz)
             }
          }
 
-         instancesCache.put(actualLoadedType.hashCode(), (Set) result);
+         instancesCache.put(actualLoadedType.getName(), (Set) result);
       }
       return result;
    }
@@ -130,7 +130,7 @@ public <T> ExportedInstance<T> getExportedInstance(final Class<T> clazz)
          return null;
       }
 
-      ExportedInstance<T> result = (ExportedInstance<T>) instanceCache.get(actualLoadedType.hashCode());
+      ExportedInstance<T> result = (ExportedInstance<T>) instanceCache.get(actualLoadedType.getName());
       if (result == null)
       {
          try
@@ -156,7 +156,7 @@ public ExportedInstance<T> call() throws Exception
             });
 
             if (result != null)
-               instanceCache.put(actualLoadedType.hashCode(), result);
+               instanceCache.put(actualLoadedType.getName(), result);
          }
          catch (Exception e)
          {
@@ -243,12 +243,12 @@ private <T> Class<T> loadAddonClass(Class<T> actualType) throws ClassNotFoundExc
 
    private Class<?> loadAddonClass(String className) throws ClassNotFoundException
    {
-      Class<?> cached = classCache.get(className.hashCode());
+      Class<?> cached = classCache.get(className);
       if (cached == null)
       {
          Class<?> result = Class.forName(className, false, addon.getClassLoader());
          // potentially not thread-safe
-         classCache.put(className.hashCode(), result);
+         classCache.put(className, result);
          cached = result;
       }
 

tests/src/main/java/org/jboss/forge/furnace/container/simple/Aa.java

@@ -0,0 +1,17 @@
+/**
+ * Copyright 2014 Red Hat, Inc. and/or its affiliates.
+ *
+ * Licensed under the Eclipse Public License version 1.0, available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.jboss.forge.furnace.container.simple;
+
+
+/**
+ * Keep the class name. The Hashcode should be the same as BB
+ * 
+ * @author <a href="ggastald@redhat.com">George Gastaldi</a>
+ */
+public class Aa
+{
+}

tests/src/main/java/org/jboss/forge/furnace/container/simple/BB.java

@@ -0,0 +1,16 @@
+/**
+ * Copyright 2014 Red Hat, Inc. and/or its affiliates.
+ *
+ * Licensed under the Eclipse Public License version 1.0, available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.jboss.forge.furnace.container.simple;
+
+/**
+ * Keep the class name. The Hashcode should be the same as Aa
+ * 
+ * @author <a href="ggastald@redhat.com">George Gastaldi</a>
+ */
+public class BB
+{
+}

tests/src/test/java/org/jboss/forge/furnace/container/simple/AddonRegistryMultipleAddonsTest.java

@@ -0,0 +1,126 @@
+/**
+ * Copyright 2013 Red Hat, Inc. and/or its affiliates.
+ *
+ * Licensed under the Eclipse Public License version 1.0, available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ */
+
+package org.jboss.forge.furnace.container.simple;
+
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.CoreMatchers.is;
+
+import java.util.Iterator;
+
+import org.jboss.arquillian.container.test.api.Deployment;
+import org.jboss.arquillian.junit.Arquillian;
+import org.jboss.forge.arquillian.AddonDependency;
+import org.jboss.forge.arquillian.Dependencies;
+import org.jboss.forge.arquillian.archive.ForgeArchive;
+import org.jboss.forge.furnace.addons.Addon;
+import org.jboss.forge.furnace.addons.AddonId;
+import org.jboss.forge.furnace.addons.AddonRegistry;
+import org.jboss.forge.furnace.container.simple.lifecycle.SimpleContainer;
+import org.jboss.forge.furnace.repositories.AddonDependencyEntry;
+import org.jboss.forge.furnace.services.Imported;
+import org.jboss.forge.furnace.spi.ServiceRegistry;
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+/**
+ * 
+ * @author <a href="ggastald@redhat.com">George Gastaldi</a>
+ */
+@RunWith(Arquillian.class)
+public class AddonRegistryMultipleAddonsTest
+{
+
+   @Deployment
+   @Dependencies({
+            @AddonDependency(name = "org.jboss.forge.furnace.container:simple")
+   })
+   public static ForgeArchive getDeployment()
+   {
+      ForgeArchive archive = ShrinkWrap.create(ForgeArchive.class)
+               .addBeansXML()
+               .addAsAddonDependencies(
+                        AddonDependencyEntry.create("org.jboss.forge.furnace.container:simple"),
+                        AddonDependencyEntry.create("test:dep2"),
+                        AddonDependencyEntry.create("test:dep1")
+
+               );
+
+      return archive;
+   }
+
+   @Deployment(name = "test:dep2,2", testable = false, order = 2)
+   public static ForgeArchive getDeploymentDep2()
+   {
+      ForgeArchive archive = ShrinkWrap.create(ForgeArchive.class)
+               .addClass(BB.class)
+               .addAsLocalServices(BB.class)
+               .addAsAddonDependencies(
+                        AddonDependencyEntry.create("org.jboss.forge.furnace.container:simple")
+               );
+      return archive;
+   }
+
+   @Deployment(name = "test:dep1,1", testable = false, order = 1)
+   public static ForgeArchive getDeploymentDep1()
+   {
+      ForgeArchive archive = ShrinkWrap
+               .create(ForgeArchive.class)
+               .addClass(Aa.class)
+               .addAsLocalServices(Aa.class)
+               .addAsAddonDependencies(
+                        AddonDependencyEntry.create("org.jboss.forge.furnace.container:simple")
+               );
+
+      return archive;
+   }
+
+   @Test
+   public void testServiceWithExpectedObjectsDifferentClassLoaders() throws Exception
+   {
+      AddonRegistry addonRegistry = SimpleContainer.getFurnace(this.getClass().getClassLoader()).getAddonRegistry();
+
+      AddonId depOneId = AddonId.from("test:dep1", "1");
+      AddonId depTwoId = AddonId.from("test:dep2", "2");
+
+      Addon depOne = addonRegistry.getAddon(depOneId);
+      Addon depTwo = addonRegistry.getAddon(depTwoId);
+
+      ServiceRegistry depOneServiceRegistry = depOne.getServiceRegistry();
+      ServiceRegistry depTwoServiceRegistry = depTwo.getServiceRegistry();
+
+      Assert.assertTrue(depOneServiceRegistry.hasService(Aa.class));
+      Assert.assertFalse(depOneServiceRegistry.hasService(BB.class));
+      Assert.assertFalse(depTwoServiceRegistry.hasService(Aa.class));
+      Assert.assertTrue(depTwoServiceRegistry.hasService(BB.class));
+
+      Assert.assertNull(depOneServiceRegistry.getExportedInstance(BB.class.getName()));
+      Assert.assertNull(depTwoServiceRegistry.getExportedInstance(Aa.class.getName()));
+
+      Assert.assertNotNull(depOneServiceRegistry.getExportedInstance(Aa.class.getName()));
+      Assert.assertNotNull(depTwoServiceRegistry.getExportedInstance(BB.class.getName()));
+
+      Imported<Aa> services = addonRegistry.getServices(Aa.class);
+      Assert.assertFalse("Imported<Aa> should have been satisfied", services.isUnsatisfied());
+      Assert.assertFalse("Imported<Aa> should not have been ambiguous", services.isAmbiguous());
+      Iterator<Aa> iterator = services.iterator();
+      Assert.assertTrue(iterator.hasNext());
+      Assert.assertThat(iterator.next(), is(instanceOf(Aa.class)));
+      Assert.assertFalse(iterator.hasNext());
+
+      Imported<BB> services2 = addonRegistry.getServices(BB.class);
+      Assert.assertFalse("Imported<BB> should have been satisfied", services.isUnsatisfied());
+      Assert.assertFalse("Imported<BB> should not have been ambiguous", services.isAmbiguous());
+      Iterator<BB> iterator2 = services2.iterator();
+      Assert.assertTrue(iterator2.hasNext());
+      Assert.assertThat(iterator2.next(), is(instanceOf(BB.class)));
+      Assert.assertFalse(iterator2.hasNext());
+   }
+
+}