Clean up some of the Impl code and ensure test method executor waits …

…for deployment to finish before running
forge · Mar 18, 2013 · a69c7c2 · a69c7c2
1 parent 42dd2ab
commit a69c7c2
Show file tree

Hide file tree

Showing 6 changed files with 63 additions and 70 deletions.
diff --git a/arquillian/core/src/main/java/org/jboss/forge/arquillian/ForgeTestMethodExecutor.java b/arquillian/core/src/main/java/org/jboss/forge/arquillian/ForgeTestMethodExecutor.java
@@ -19,6 +19,7 @@
 import java.lang.annotation.Annotation;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.util.concurrent.Callable;
 import java.util.concurrent.Future;
 
 import org.jboss.arquillian.container.test.spi.ContainerMethodExecutor;
@@ -29,6 +30,7 @@
 import org.jboss.forge.container.Forge;
 import org.jboss.forge.container.addons.Addon;
 import org.jboss.forge.container.addons.AddonRegistry;
+import org.jboss.forge.container.lock.LockMode;
 import org.jboss.forge.container.services.ExportedInstance;
 import org.jboss.forge.container.services.ServiceRegistry;
 import org.jboss.forge.container.util.ClassLoaders;
@@ -64,38 +66,47 @@ public TestResult invoke(final TestMethodExecutor testMethodExecutor)
 
       try
       {
-         String testClassName = testMethodExecutor.getInstance().getClass().getName();
-         AddonRegistry addonRegistry = forge.getAddonRegistry();
-         Object instance = null;
+         final String testClassName = testMethodExecutor.getInstance().getClass().getName();
+         final AddonRegistry addonRegistry = forge.getAddonRegistry();
 
-         for (Addon addon : addonRegistry.getRegisteredAddons())
+         final Object instance = forge.getLockManager().performLocked(LockMode.WRITE, new Callable<Object>()
          {
-            if (!addon.getStatus().isMissing() && ClassLoaders.containsClass(addon.getClassLoader(), testClassName))
+            @Override
+            public Object call() throws Exception
             {
-               Future<Addon> future = addonRegistry.start(addon.getId());
-               future.get();
-            }
-
-            if (addon.getStatus().isStarted())
-            {
-               ServiceRegistry registry = addon.getServiceRegistry();
-               ExportedInstance<?> result = registry.getExportedInstance(testClassName);
-
-               if (result != null)
+               Object result = null;
+               for (Addon addon : addonRegistry.getRegisteredAddons())
                {
-                  if (instance == null)
+                  if (!addon.getStatus().isMissing()
+                           && ClassLoaders.containsClass(addon.getClassLoader(), testClassName))
                   {
-                     instance = result.get();
+                     Future<Addon> future = addonRegistry.start(addon.getId());
+                     future.get();
                   }
-                  else
+
+                  if (addon.getStatus().isStarted())
                   {
-                     throw new IllegalStateException(
-                              "Multiple test classes found in deployed addons. " +
-                                       "You must have only one @Deployment(testable=true\"); deployment");
+                     ServiceRegistry registry = addon.getServiceRegistry();
+                     ExportedInstance<?> testInstance = registry.getExportedInstance(testClassName);
+
+                     if (testInstance != null)
+                     {
+                        if (result == null)
+                        {
+                           result = testInstance.get();
+                        }
+                        else
+                        {
+                           throw new IllegalStateException(
+                                    "Multiple test classes found in deployed addons. " +
+                                             "You must have only one @Deployment(testable=true\"); deployment");
+                        }
+                     }
                   }
                }
+               return result;
             }
-         }
+         });
 
          if (instance == null)
             throw new IllegalStateException(

diff --git a/container/src/main/java/org/jboss/forge/container/impl/AddonRepositoryImpl.java b/container/src/main/java/org/jboss/forge/container/impl/AddonRepositoryImpl.java
@@ -68,7 +68,8 @@ public static MutableAddonRepository forDirectory(Forge forge, File dir)
 
    public static MutableAddonRepository forDefaultDirectory(Forge forge)
    {
-      return new AddonRepositoryImpl(forge.getLockManager(), new File(OperatingSystemUtils.getUserHomePath(), DEFAULT_ADDON_DIR));
+      return new AddonRepositoryImpl(forge.getLockManager(), new File(OperatingSystemUtils.getUserHomePath(),
+               DEFAULT_ADDON_DIR));
    }
 
    public static String getRuntimeAPIVersion()
@@ -118,7 +119,8 @@ public Boolean call() throws Exception
                   {
                      if (resource.isDirectory())
                      {
-                        Files.copyDirectory(resource, new File(addonSlotDir, resource.getName()));
+                        Files.copyDirectory(resource, new File(addonSlotDir, addon.getName()
+                                 + resource.getParentFile().getParentFile().getName()));
                      }
                      else
                      {

diff --git a/container/src/main/java/org/jboss/forge/container/impl/AddonRunnable.java b/container/src/main/java/org/jboss/forge/container/impl/AddonRunnable.java
@@ -20,7 +20,7 @@
 import org.jboss.forge.container.lock.LockMode;
 import org.jboss.forge.container.modules.ModularURLScanner;
 import org.jboss.forge.container.modules.ModularWeld;
-import org.jboss.forge.container.modules.ModuleResourceLoader;
+import org.jboss.forge.container.modules.AddonResourceLoader;
 import org.jboss.forge.container.modules.ModuleScanResult;
 import org.jboss.forge.container.services.ServiceRegistry;
 import org.jboss.forge.container.util.Assert;
@@ -92,7 +92,7 @@ public Callable<Object> call() throws Exception
       {
          try
          {
-            ResourceLoader resourceLoader = new ModuleResourceLoader(addon.getModule());
+            ResourceLoader resourceLoader = new AddonResourceLoader(addon);
             ModularURLScanner scanner = new ModularURLScanner(resourceLoader, "META-INF/beans.xml");
             ModuleScanResult scanResult = scanner.scan();
 

diff --git a/...ntainer/modules/ModuleResourceLoader.java → ...ontainer/modules/AddonResourceLoader.java b/...ntainer/modules/ModuleResourceLoader.java → ...ontainer/modules/AddonResourceLoader.java
@@ -1,53 +1,41 @@
 package org.jboss.forge.container.modules;
 
-import org.jboss.modules.Module;
-import org.jboss.modules.ModuleClassLoader;
-import org.jboss.weld.resources.spi.ResourceLoader;
-import org.jboss.weld.resources.spi.ResourceLoadingException;
-
 import java.net.URL;
 import java.util.Collection;
 import java.util.Enumeration;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 
+import org.jboss.forge.container.addons.Addon;
+import org.jboss.weld.resources.spi.ResourceLoader;
+import org.jboss.weld.resources.spi.ResourceLoadingException;
+
 /**
- * A {@link ResourceLoader} that can load classes from a {@link Module}
- * <p>
- * Thread Safety: This class is thread safe, and does not require a happens before even between construction and usage
- * 
- * @author Stuart Douglas
+ * A {@link ResourceLoader} that can load classes from an {@link Addon}
  * 
+ * @author <a href="mailto:lincolnbaxter@gmail.com">Lincoln Baxter, III</a>
  */
-public class ModuleResourceLoader implements ResourceLoader
+public class AddonResourceLoader implements ResourceLoader
 {
-   @Override
-   public String toString()
-   {
-      return "ModuleResourceLoader [module=" + module + "]";
-   }
-
-   private final Module module;
-
-   /**
-    * Additional classes that have been added to the bean archive by the container or by a portable extension
-    */
    private final Map<String, Class<?>> classes;
-   private ModuleClassLoader classLoader;
 
-   public ModuleResourceLoader(Module module)
+   private ClassLoader classLoader;
+   private Addon addon;
+
+   public AddonResourceLoader(Addon addon)
    {
-      this.module = module;
       this.classes = new ConcurrentHashMap<String, Class<?>>();
-      this.classLoader = module.getClassLoader();
+      this.addon = addon;
+      this.classLoader = addon.getClassLoader();
+   }
+
+   @Override
+   public String toString()
+   {
+      return addon.getId().toCoordinates();
    }
 
-   /**
-    * If the class name is found in additionalClasses then return it.
-    * 
-    * Otherwise the class will be loaded from the module ClassLoader
-    */
    @Override
    public Class<?> classForName(String name)
    {
@@ -80,32 +68,26 @@ public void addAdditionalClass(Class<?> clazz)
       this.classes.put(clazz.getName(), clazz);
    }
 
-   /**
-    * Loads a resource from the module class loader
-    */
    @Override
    public URL getResource(String name)
    {
       try
       {
-         return module.getClassLoader().getResource(name);
+         return classLoader.getResource(name);
       }
       catch (Exception e)
       {
          throw new ResourceLoadingException(e);
       }
    }
 
-   /**
-    * Loads resources from the module class loader
-    */
    @Override
    public Collection<URL> getResources(String name)
    {
       try
       {
          final HashSet<URL> resources = new HashSet<URL>();
-         Enumeration<URL> urls = module.getClassLoader().getResources(name);
+         Enumeration<URL> urls = classLoader.getResources(name);
          while (urls.hasMoreElements())
          {
             resources.add(urls.nextElement());
@@ -122,7 +104,6 @@ public Collection<URL> getResources(String name)
    @Override
    public void cleanup()
    {
-      // noop
    }
 
 }
diff --git a/container/src/main/java/org/jboss/forge/container/modules/ModularFileSystemURLHandler.java b/container/src/main/java/org/jboss/forge/container/modules/ModularFileSystemURLHandler.java
@@ -146,9 +146,9 @@ protected void handle(String name, URL url, List<String> discoveredClasses, List
          }
          catch (Exception e)
          {
-            log.warn("Not loading Bean definition from class: ["
-                     + className + "] because of underlying class loading error. " +
-                     "Class found in [" + resourceLoader + "] but could not be loaded.)", e);
+            log.warn("Not loading class: ["
+                     + className + "] from addon [" + resourceLoader
+                     + "] because a required import could not be resolved.");
          }
       }
       else if (name.endsWith("beans.xml"))

diff --git a/resources/tests/src/test/java/test/org/jboss/forge/resource/FileResourceGeneratorTest.java b/resources/tests/src/test/java/test/org/jboss/forge/resource/FileResourceGeneratorTest.java
@@ -10,7 +10,6 @@
 import org.jboss.forge.arquillian.Addon;
 import org.jboss.forge.arquillian.Dependencies;
 import org.jboss.forge.arquillian.archive.ForgeArchive;
-import org.jboss.forge.container.addons.AddonDependency;
 import org.jboss.forge.container.addons.AddonId;
 import org.jboss.forge.container.repositories.AddonDependencyEntry;
 import org.jboss.forge.resource.DirectoryResource;