You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aries.apache.org by da...@apache.org on 2018/01/12 19:30:31 UTC

svn commit: r1821025 - in /aries/trunk/spi-fly: spi-fly-dynamic-bundle/src/test/java/org/apache/aries/spifly/dynamic/ spi-fly-weaver/src/main/java/org/apache/aries/spifly/weaver/

Author: davidb
Date: Fri Jan 12 19:30:30 2018
New Revision: 1821025

URL: http://svn.apache.org/viewvc?rev=1821025&view=rev
Log:
ARIES-1321 Support variable as argument for ServiceLoader.load

Patch applied on behalf of Olivier NOUGUIER and Michael N. Lipp with many thanks!
Closes #82

Modified:
    aries/trunk/spi-fly/spi-fly-dynamic-bundle/src/test/java/org/apache/aries/spifly/dynamic/ClientWeavingHookTest.java
    aries/trunk/spi-fly/spi-fly-dynamic-bundle/src/test/java/org/apache/aries/spifly/dynamic/TestClient.java
    aries/trunk/spi-fly/spi-fly-weaver/src/main/java/org/apache/aries/spifly/weaver/TCCLSetterVisitor.java

Modified: aries/trunk/spi-fly/spi-fly-dynamic-bundle/src/test/java/org/apache/aries/spifly/dynamic/ClientWeavingHookTest.java
URL: http://svn.apache.org/viewvc/aries/trunk/spi-fly/spi-fly-dynamic-bundle/src/test/java/org/apache/aries/spifly/dynamic/ClientWeavingHookTest.java?rev=1821025&r1=1821024&r2=1821025&view=diff
==============================================================================
--- aries/trunk/spi-fly/spi-fly-dynamic-bundle/src/test/java/org/apache/aries/spifly/dynamic/ClientWeavingHookTest.java (original)
+++ aries/trunk/spi-fly/spi-fly-dynamic-bundle/src/test/java/org/apache/aries/spifly/dynamic/ClientWeavingHookTest.java Fri Jan 12 19:30:30 2018
@@ -43,6 +43,7 @@ import java.util.concurrent.ConcurrentHa
 
 import javax.xml.parsers.DocumentBuilderFactory;
 
+import org.apache.aries.mytest.MySPI;
 import org.apache.aries.spifly.BaseActivator;
 import org.apache.aries.spifly.SpiFlyConstants;
 import org.apache.aries.spifly.Streams;
@@ -441,6 +442,9 @@ public class ClientWeavingHookTest {
         testConsumerBundleWeaving(consumerBundle1, wh, Collections.singleton("impl4"), "org.apache.aries.spifly.dynamic.impl3.MyAltDocumentBuilderFactory");
         testConsumerBundleWeaving(consumerBundle2, wh, Collections.singleton("olleh"), thisJVMsDBF);
         testConsumerBundleWeaving(consumerBundle3, wh, Collections.<String>emptySet(), thisJVMsDBF);
+        testConsumerBundleWeavingNonConst(consumerBundle1, wh, Collections.singleton("impl4"), "org.apache.aries.spifly.dynamic.impl3.MyAltDocumentBuilderFactory");
+        testConsumerBundleWeavingNonConst(consumerBundle2, wh, Collections.singleton("olleh"), thisJVMsDBF);
+        testConsumerBundleWeavingNonConst(consumerBundle3, wh, Collections.<String>emptySet(), thisJVMsDBF);
     }
 
     private void testConsumerBundleWeaving(Bundle consumerBundle, WeavingHook wh, Set<String> testClientResult, String jaxpClientResult) throws Exception {
@@ -466,6 +470,20 @@ public class ClientWeavingHookTest {
         Assert.assertEquals(jaxpClientResult, result2.getName());
     }
 
+    private void testConsumerBundleWeavingNonConst(Bundle consumerBundle, WeavingHook wh, Set<String> testClientResult, String jaxpClientResult) throws Exception {
+        // Weave the TestClient class.
+        URL clsUrl = getClass().getResource("TestClient.class");
+        WovenClass wc = new MyWovenClass(clsUrl, TestClient.class.getName(), consumerBundle);
+        wh.weave(wc);
+
+        // Invoke the woven class and check that it propertly sets the TCCL so that the
+        // META-INF/services/org.apache.aries.mytest.MySPI file from impl2 is visible.
+        Class<?> cls = wc.getDefinedClass();
+        Method method = cls.getMethod("testService", new Class [] {String.class, Class.class});
+        Object result = method.invoke(cls.newInstance(), "hello", MySPI.class);
+        Assert.assertEquals(testClientResult, result);
+    }
+
     @Test
     public void testJAXPClientWantsJREImplementation1() throws Exception {
         Bundle systembundle = mockSystemBundle();

Modified: aries/trunk/spi-fly/spi-fly-dynamic-bundle/src/test/java/org/apache/aries/spifly/dynamic/TestClient.java
URL: http://svn.apache.org/viewvc/aries/trunk/spi-fly/spi-fly-dynamic-bundle/src/test/java/org/apache/aries/spifly/dynamic/TestClient.java?rev=1821025&r1=1821024&r2=1821025&view=diff
==============================================================================
--- aries/trunk/spi-fly/spi-fly-dynamic-bundle/src/test/java/org/apache/aries/spifly/dynamic/TestClient.java (original)
+++ aries/trunk/spi-fly/spi-fly-dynamic-bundle/src/test/java/org/apache/aries/spifly/dynamic/TestClient.java Fri Jan 12 19:30:30 2018
@@ -25,6 +25,9 @@ import java.util.Set;
 import org.apache.aries.mytest.MySPI;
 
 public class TestClient {
+	/**
+	 * Load using a constant as parameter.
+	 */
     public Set<String> test(String input) {
         Set<String> results = new HashSet<String>();
 
@@ -32,6 +35,23 @@ public class TestClient {
         for (MySPI mySPI : loader) {
             results.add(mySPI.someMethod(input));
         }
+        return results;
+    }
+    
+	/**
+	 * Load using a variable as parameter.
+	 */
+    public Set<String> testService(String input, Class<MySPI> service) {
+        Set<String> results = new HashSet<String>();
+
+        // Try to irritate TCCLSetterVisitor by forcing an (irrelevant) LDC.
+        @SuppressWarnings("unused")
+		Class<?> causeLDC = String.class;
+        
+        ServiceLoader<MySPI> loader = ServiceLoader.load(service);
+        for (MySPI mySPI : loader) {
+            results.add(mySPI.someMethod(input));
+        }
         return results;
     }
 }

Modified: aries/trunk/spi-fly/spi-fly-weaver/src/main/java/org/apache/aries/spifly/weaver/TCCLSetterVisitor.java
URL: http://svn.apache.org/viewvc/aries/trunk/spi-fly/spi-fly-weaver/src/main/java/org/apache/aries/spifly/weaver/TCCLSetterVisitor.java?rev=1821025&r1=1821024&r2=1821025&view=diff
==============================================================================
--- aries/trunk/spi-fly/spi-fly-weaver/src/main/java/org/apache/aries/spifly/weaver/TCCLSetterVisitor.java (original)
+++ aries/trunk/spi-fly/spi-fly-weaver/src/main/java/org/apache/aries/spifly/weaver/TCCLSetterVisitor.java Fri Jan 12 19:30:30 2018
@@ -140,16 +140,19 @@ public class TCCLSetterVisitor extends C
 
     private class TCCLSetterMethodVisitor extends GeneratorAdapter {
         Type lastLDCType;
-
+        private int lastOpcode;
+        private int lastVar;
+        
         public TCCLSetterMethodVisitor(MethodVisitor mv, int access, String name, String descriptor) {
             super(Opcodes.ASM5, mv, access, name, descriptor);
         }
 
         /**
          * Store the last LDC call. When ServiceLoader.load(Class cls) is called
-         * the last LDC call before the ServiceLoader.load() visitMethodInsn call
-         * contains the class being passed in. We need to pass this class to $$FCCL$$ as well
-         * so we can copy the value found in here.
+         * with a class constant (XXX.class) as parameter, the last LDC call 
+         * before the ServiceLoader.load() visitMethodInsn call
+         * contains the class being passed in. We need to pass this class to 
+         * $$FCCL$$ as well so we can copy the value found in here.
          */
         @Override
         public void visitLdcInsn(Object cst) {
@@ -160,6 +163,23 @@ public class TCCLSetterVisitor extends C
         }
 
         /**
+         * Store the last ALOAD call. When ServiceLoader.load(Class cls) is called
+         * with using a variable as parameter, the last ALOAD call 
+         * before the ServiceLoader.load() visitMethodInsn call
+         * contains the class being passed in. Annihilate any previously
+         * found LDC, because it had nothing to do with the call to
+         * ServiceLoader.load(Class cls) if it is followed by an ALOAD
+         * (before the actual call).
+         */
+        @Override
+        public void visitVarInsn(int opcode, int var) {
+        	lastLDCType = null;
+        	this.lastOpcode = opcode;
+        	this.lastVar = var;
+        	super.visitVarInsn(opcode, var);
+        }
+
+        /**
          * Wrap selected method calls with
          *  Util.storeContextClassloader();
          *  $$FCCL$$(<class>)
@@ -189,9 +209,14 @@ public class TCCLSetterVisitor extends C
                     (wd.getArgClasses() == null || Arrays.equals(new String [] {Class.class.getName()}, wd.getArgClasses()))) {
                     // ServiceLoader.load() is a special case because it's a general-purpose service loader,
                     // therefore, the target class it the class being passed in to the ServiceLoader.load()
-                    // call itself.
-
-                    mv.visitLdcInsn(lastLDCType);
+                    // call itself. Use LDC if found (immediately) before the call
+                	if (lastLDCType != null) {
+                		mv.visitLdcInsn(lastLDCType);
+                	} else {
+                		// If there was no LDC (or an LDC was followed by an
+                		// ALOAD), the parameter is loaded from a variable.
+                		mv.visitVarInsn(lastOpcode, lastVar);	
+                	}
                 } else {
                     // In any other case, we're not dealing with a general-purpose service loader, but rather
                     // with a specific one, such as DocumentBuilderFactory.newInstance(). In that case the