Add support for ArquillianResource param injection on @deployment met…

…hods (#603)

Aggregate the TestEnricher argument generation into a single util class
Improve some of the javadoc
Fixes #601

Signed-off-by: Scott M Stark <starksm64@gmail.com>

container/test-impl-base/src/main/java/org/jboss/arquillian/container/test/impl/ContainerTestExtension.java

@@ -90,6 +90,7 @@ public void register(ExtensionBuilder builder) {
         builder.observer(ContainerEventController.class)
             .observer(ContainerRestarter.class)
             .observer(DeploymentGenerator.class)
+            .observer(AnnotationDeploymentScenarioGenerator.class)
             .observer(ArchiveDeploymentToolingExporter.class)
             .observer(ProtocolRegistryCreator.class)
             .observer(ClientContainerControllerCreator.class)

container/test-impl-base/src/main/java/org/jboss/arquillian/container/test/impl/client/deployment/AnnotationDeploymentScenarioGenerator.java

@@ -18,7 +18,9 @@
 
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
+import java.lang.reflect.Parameter;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
 import org.jboss.arquillian.container.spi.client.deployment.DeploymentScenario;
 import org.jboss.arquillian.container.test.api.Deployment;
@@ -27,7 +29,13 @@
 import org.jboss.arquillian.container.test.api.ShouldThrowException;
 import org.jboss.arquillian.container.test.api.TargetsContainer;
 import org.jboss.arquillian.container.test.spi.client.deployment.DeploymentScenarioGenerator;
+import org.jboss.arquillian.core.api.Instance;
+import org.jboss.arquillian.core.api.annotation.Inject;
+import org.jboss.arquillian.core.spi.ServiceLoader;
+import org.jboss.arquillian.test.api.ArquillianResource;
 import org.jboss.arquillian.test.spi.TestClass;
+import org.jboss.arquillian.test.spi.TestEnricher;
+import org.jboss.arquillian.test.spi.execution.ExecUtils;
 import org.jboss.shrinkwrap.api.Archive;
 import org.jboss.shrinkwrap.descriptor.api.Descriptor;
 
@@ -39,9 +47,10 @@
  * @version $Revision: $
  */
 public class AnnotationDeploymentScenarioGenerator extends AbstractDeploymentScenarioGenerator implements DeploymentScenarioGenerator {
+    @Inject
+    private Instance<ServiceLoader> serviceLoader;
 
     protected List<DeploymentConfiguration> generateDeploymentContent(TestClass testClass) {
-
         List<DeploymentConfiguration> deployments = new ArrayList<DeploymentConfiguration>();
         Method[] deploymentMethods = testClass.getMethods(Deployment.class);
 
@@ -71,26 +80,48 @@ private void validate(Method deploymentMethod) {
                     + ". "
                     + deploymentMethod);
         }
-        if (deploymentMethod.getParameterTypes().length != 0) {
-            throw new IllegalArgumentException("Method annotated with "
-                + Deployment.class.getName()
-                + " can not accept parameters. "
-                + deploymentMethod);
+            // This will throw IllegalArgumentException if check fails
+            hasZeroOrOnlyArquillianResourceArgs(deploymentMethod);
+    }
+
+    private void hasZeroOrOnlyArquillianResourceArgs(Method deploymentMethod) throws IllegalArgumentException{
+        boolean isOk = deploymentMethod.getParameterTypes().length == 0;
+        if (!isOk) {
+            ArrayList<String> badArgs = new ArrayList<>();
+            for (Parameter param : deploymentMethod.getParameters()) {
+                if (param.getAnnotation(ArquillianResource.class) == null) {
+                    badArgs.add(param.getName());
+                }
+            }
+            if (!badArgs.isEmpty()) {
+                throw new IllegalArgumentException("Method annotated with "
+                    + Deployment.class.getName()
+                    + " can not accept parameters that are not annotated with "
+                    + ArquillianResource.class.getName()
+                    + ". "
+                    + deploymentMethod
+                    + " has invalid parameters: "
+                    + badArgs);
+            }
         }
     }
 
     /**
-     * @param deploymentMethod
-     * @return
+     * Call the deployment method and generate the deployment content and return a {@link DeploymentConfiguration}
+     * populated with the content and any relevant deployment method annotation information
+     * @param deploymentMethod - {@link Deployment} annotated method
+     * @return configured {@link DeploymentConfiguration}
      */
     private DeploymentConfiguration generateDeploymentContent(Method deploymentMethod) {
 
         Deployment deploymentAnnotation = deploymentMethod.getAnnotation(Deployment.class);
         DeploymentConfiguration.DeploymentContentBuilder deploymentContentBuilder = null;
         if (Archive.class.isAssignableFrom(deploymentMethod.getReturnType())) {
-            deploymentContentBuilder = new DeploymentConfiguration.DeploymentContentBuilder(invoke(Archive.class, deploymentMethod));
+            Archive<?> archive = invoke(Archive.class, deploymentMethod);
+            deploymentContentBuilder = new DeploymentConfiguration.DeploymentContentBuilder(archive);
         } else if (Descriptor.class.isAssignableFrom(deploymentMethod.getReturnType())) {
-            deploymentContentBuilder = new DeploymentConfiguration.DeploymentContentBuilder(invoke(Descriptor.class, deploymentMethod));
+            Descriptor descriptor = invoke(Descriptor.class, deploymentMethod);
+            deploymentContentBuilder = new DeploymentConfiguration.DeploymentContentBuilder(descriptor);
         }
 
         if (deploymentMethod.isAnnotationPresent(OverProtocol.class)) {
@@ -118,12 +149,19 @@ private DeploymentConfiguration generateDeploymentContent(Method deploymentMetho
 
 
     /**
-     * @param deploymentMethod
-     * @return
+     * Invoke the deployment method to generate the test archive or descriptor
+     * @param type - the expected return type
+     * @param deploymentMethod - class deployment method
+     * @return the generated archive or descriptor
      */
     private <T> T invoke(Class<T> type, Method deploymentMethod) {
         try {
-            return type.cast(deploymentMethod.invoke(null));
+            Object[] args = null;
+            if(deploymentMethod.getParameterCount() > 0) {
+                Collection<TestEnricher> enrichers = serviceLoader.get().all(TestEnricher.class);
+                args = ExecUtils.enrichArguments(deploymentMethod, enrichers);
+            }
+            return type.cast(deploymentMethod.invoke(null, args));
         } catch (Exception e) {
             throw new RuntimeException("Could not invoke deployment method: " + deploymentMethod, e);
         }

container/test-impl-base/src/main/java/org/jboss/arquillian/container/test/impl/execution/LocalTestExecuter.java

@@ -16,8 +16,6 @@
  */
 package org.jboss.arquillian.container.test.impl.execution;
 
-import java.lang.reflect.Method;
-import java.util.Collection;
 import org.jboss.arquillian.container.test.impl.execution.event.LocalExecutionEvent;
 import org.jboss.arquillian.core.api.Injector;
 import org.jboss.arquillian.core.api.Instance;
@@ -28,6 +26,7 @@
 import org.jboss.arquillian.test.spi.TestEnricher;
 import org.jboss.arquillian.test.spi.TestResult;
 import org.jboss.arquillian.test.spi.annotation.TestScoped;
+import org.jboss.arquillian.test.spi.execution.ExecUtils;
 
 /**
  * A Handler for executing the Test Method.<br/>
@@ -53,10 +52,10 @@ public class LocalTestExecuter {
     public void execute(@Observes LocalExecutionEvent event) throws Exception {
         TestResult result = TestResult.passed();
         try {
-            event.getExecutor().invoke(
-                enrichArguments(
-                    event.getExecutor().getMethod(),
-                    serviceLoader.get().all(TestEnricher.class)));
+            Object[] args = ExecUtils.enrichArguments(
+                event.getExecutor().getMethod(),
+                serviceLoader.get().all(TestEnricher.class));
+            event.getExecutor().invoke(args);
         } catch (Throwable e) {
             result = TestResult.failed(e);
         } finally {
@@ -65,36 +64,4 @@ public void execute(@Observes LocalExecutionEvent event) throws Exception {
         testResult.set(result);
     }
 
-    /**
-     * Enrich the method arguments of a method call.<br/>
-     * The Object[] index will match the method parameterType[] index.
-     *
-     * @return the argument values
-     */
-    private Object[] enrichArguments(Method method, Collection<TestEnricher> enrichers) {
-        Object[] values = new Object[method.getParameterTypes().length];
-        if (method.getParameterTypes().length == 0) {
-            return values;
-        }
-        for (TestEnricher enricher : enrichers) {
-            mergeValues(values, enricher.resolve(method));
-        }
-        return values;
-    }
-
-    private void mergeValues(Object[] values, Object[] resolvedValues) {
-        if (resolvedValues == null || resolvedValues.length == 0) {
-            return;
-        }
-        if (values.length != resolvedValues.length) {
-            throw new IllegalStateException("TestEnricher resolved wrong argument count, expected " +
-                values.length + " returned " + resolvedValues.length);
-        }
-        for (int i = 0; i < resolvedValues.length; i++) {
-            Object resvoledValue = resolvedValues[i];
-            if (resvoledValue != null && values[i] == null) {
-                values[i] = resvoledValue;
-            }
-        }
-    }
 }

container/test-impl-base/src/test/java/org/jboss/arquillian/container/test/impl/client/deployment/DeploymentGeneratorTestCase.java

@@ -17,6 +17,8 @@
  */
 package org.jboss.arquillian.container.test.impl.client.deployment;
 
+import java.lang.annotation.Annotation;
+import java.math.BigDecimal;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -56,7 +58,11 @@
 import org.jboss.arquillian.core.api.annotation.Inject;
 import org.jboss.arquillian.core.api.annotation.Observer;
 import org.jboss.arquillian.core.spi.ServiceLoader;
+import org.jboss.arquillian.test.api.ArquillianResource;
+import org.jboss.arquillian.test.impl.enricher.resource.ArquillianResourceTestEnricher;
 import org.jboss.arquillian.test.spi.TestClass;
+import org.jboss.arquillian.test.spi.TestEnricher;
+import org.jboss.arquillian.test.spi.enricher.resource.ResourceProvider;
 import org.jboss.shrinkwrap.api.Archive;
 import org.jboss.shrinkwrap.api.ShrinkWrap;
 import org.jboss.shrinkwrap.api.spec.JavaArchive;
@@ -109,7 +115,7 @@ public void prepare() {
         Injector injector = injectorInst.get();
 
         final List<DeploymentScenarioGenerator> deploymentScenarioGenerators = new ArrayList<DeploymentScenarioGenerator>();
-        deploymentScenarioGenerators.add(new AnnotationDeploymentScenarioGenerator());
+        deploymentScenarioGenerators.add(injector.inject(new AnnotationDeploymentScenarioGenerator()));
         when(serviceLoader.all(DeploymentScenarioGenerator.class))
             .thenReturn(deploymentScenarioGenerators);
         when(serviceLoader.onlyOne(eq(DeployableContainer.class))).thenReturn(deployableContainer);
@@ -121,6 +127,13 @@ public void prepare() {
             .thenReturn(create(AuxiliaryArchiveProcessor.class, injector.inject(new TestAuxiliaryArchiveProcessor())));
         when(serviceLoader.all(eq(ApplicationArchiveProcessor.class)))
             .thenReturn(create(ApplicationArchiveProcessor.class, injector.inject(new TestApplicationArchiveAppender())));
+        when(serviceLoader.all(TestEnricher.class))
+            .thenReturn(create(TestEnricher.class, injector.inject(new ArquillianResourceTestEnricher())));
+        final List<ResourceProvider> resourceProviders = new ArrayList<>();
+        resourceProviders.add(new TestStringResourceProvider());
+        resourceProviders.add(new TestBigDecimalResourceProvider());
+        when(serviceLoader.all(ResourceProvider.class))
+            .thenReturn(resourceProviders);
 
         containerRegistry = new LocalContainerRegistry(injector);
         protocolRegistry = new ProtocolRegistry();
@@ -257,6 +270,37 @@ public void shouldAllowMultipleSameNamedArchiveDeploymentWithDifferentTargets()
         verifyScenario("X", "Y");
     }
 
+    /**
+     * https://github.com/arquillian/arquillian-core/issues/602
+     */
+    @Test
+    public void shouldAllowDeployMethodWithArqResource() {
+        addContainer(CONTAINER_NAME_1).getContainerConfiguration().setMode("suite");
+        addProtocol(PROTOCOL_NAME_1, true);
+
+        fire(createEvent(DeploymentWithArqResoureArg.class));
+        verifyScenario("DeploymentWithArqResoureArg");
+    }
+    @Test
+    public void shouldAllowDeployMethodWithMultipleArqResource() {
+        addContainer(CONTAINER_NAME_1).getContainerConfiguration().setMode("suite");
+        addProtocol(PROTOCOL_NAME_1, true);
+
+        fire(createEvent(DeploymentWithArqResoureArgsDifferentProviders.class));
+        verifyScenario("DeploymentWithArqResoureArgsDifferentProviders");
+    }
+
+
+    @Test(expected = IllegalArgumentException.class)
+    public void shouldFailDeployMethodWithNonArqResource() {
+        addContainer(CONTAINER_NAME_1).getContainerConfiguration().setMode("suite");
+        addProtocol(PROTOCOL_NAME_1, true);
+
+        fire(createEvent(DeploymentWithBadArg.class));
+        // Should not get here
+        Assert.fail("Should have failed with IllegalArgumentException");
+    }
+
     @Test // ARQ-971
     @SuppressWarnings("unchecked")
     public void shouldFilterNullAuxiliaryArchiveAppenderResulsts() throws Exception {
@@ -527,6 +571,32 @@ public static JavaArchive deploy() {
             return ShrinkWrap.create(JavaArchive.class);
         }
     }
+    private static class DeploymentWithArqResoureArg {
+        @Deployment(name = "DeploymentWithArqResoureArg", managed = false, testable = false)
+        @TargetsContainer(CONTAINER_NAME_1)
+        public static JavaArchive deploy(@ArquillianResource String resource) {
+            Assert.assertEquals("deploy-method-resource", resource);
+            return ShrinkWrap.create(JavaArchive.class);
+        }
+    }
+    private static class DeploymentWithArqResoureArgsDifferentProviders {
+        @Deployment(name = "DeploymentWithArqResoureArgsDifferentProviders", managed = false, testable = false)
+        @TargetsContainer(CONTAINER_NAME_1)
+        public static JavaArchive deploy(@ArquillianResource String resource, @ArquillianResource BigDecimal pi) {
+            Assert.assertEquals("deploy-method-resource", resource);
+            Assert.assertEquals("3.14159265358979323846", pi.toPlainString());
+            return ShrinkWrap.create(JavaArchive.class);
+        }
+    }
+    private static class DeploymentWithBadArg {
+        @Deployment(name = "DeploymentWithBadArg", managed = false, testable = false)
+        @TargetsContainer(CONTAINER_NAME_1)
+        public static JavaArchive deploy(String resource) {
+            // Should not be called
+            Assert.fail("DeploymentWithBadArg.deploy(String) should not be called");
+            return ShrinkWrap.create(JavaArchive.class);
+        }
+    }
 
     private static class CallMap {
         private Set<Class<?>> calls = new HashSet<Class<?>>();
@@ -600,4 +670,27 @@ public ProtocolConfiguration getConfig() {
             return config;
         }
     }
+
+    private static class TestStringResourceProvider implements ResourceProvider {
+        @Override
+        public boolean canProvide(Class<?> type) {
+            return String.class.isAssignableFrom(type);
+        }
+
+        @Override
+        public Object lookup(ArquillianResource resource, Annotation... qualifiers) {
+            return "deploy-method-resource";
+        }
+    }
+    private static class TestBigDecimalResourceProvider implements ResourceProvider {
+        @Override
+        public boolean canProvide(Class<?> type) {
+            return BigDecimal.class.isAssignableFrom(type);
+        }
+
+        @Override
+        public Object lookup(ArquillianResource resource, Annotation... qualifiers) {
+            return new BigDecimal("3.14159265358979323846");
+        }
+    }
 }

core/spi/src/main/java/org/jboss/arquillian/core/spi/LoadableExtension.java

@@ -22,7 +22,7 @@
 /**
  * LoadableExtension.
  * <p>
- * Loadable extensions are loaded on the local side of Arquillan. For extensions, components, observers etc to run on
+ * Loadable extensions are loaded on the local side of Arquillian. For extensions, components, observers etc to run on
  * the remote side, use {@code RemoteLoadableExtension} instead, and provide it via an
  * {@code AuxilliaryArchiveAppender}.
  *
@@ -58,6 +58,9 @@ <T> ExtensionBuilder override(Class<T> service, Class<? extends T> oldServiceImp
          * Register an observer for events. This observer will be injected according to any
          * {@link org.jboss.arquillian.core.api.annotation.Inject} annotated
          * {@link org.jboss.arquillian.core.api.Instance} fields.
+         * Note: the handler does not have to have any use of {@link org.jboss.arquillian.core.api.annotation.Observer}
+         * and this can be used to simply register a class that has needs to be instantiated and injected.
+         * @param handler A class with observer methods and/or Instance fields
          */
         ExtensionBuilder observer(Class<?> handler);