Fix FELIX-4256 Avoid manipulation native and static methods from inner classes

Static inner classes and native method from inner classes are no more manipulated.

git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1527604 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/ipojo/manipulator/manipulator/pom.xml b/ipojo/manipulator/manipulator/pom.xml
index 0d796dd..8dd60c5 100644
--- a/ipojo/manipulator/manipulator/pom.xml
+++ b/ipojo/manipulator/manipulator/pom.xml
@@ -71,6 +71,12 @@
           <version>2.4</version>
           <scope>test</scope>
       </dependency>
+      <dependency>
+          <groupId>org.easytesting</groupId>
+          <artifactId>fest-assert</artifactId>
+          <version>1.4</version>
+          <scope>test</scope>
+      </dependency>
     </dependencies>
     <build>
         <resources>
diff --git a/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/ClassChecker.java b/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/ClassChecker.java
index cced500..3dbebb7 100644
--- a/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/ClassChecker.java
+++ b/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/ClassChecker.java
@@ -148,7 +148,10 @@
      */

     public void visitInnerClass(String name, String outerName, String innerName, int access) {

         if (m_className.equals(outerName)  || outerName == null) { // Anonymous classes does not have an outer class.

-            m_inners.add(name);

+            // Do not include inner static class

+            if (! ((access & ACC_STATIC) == ACC_STATIC)) {

+                m_inners.add(name);

+            }

         }

     }

 

diff --git a/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/InnerClassAdapter.java b/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/InnerClassAdapter.java
index 033d687..facb172 100644
--- a/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/InnerClassAdapter.java
+++ b/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/InnerClassAdapter.java
@@ -19,16 +19,16 @@
 
 package org.apache.felix.ipojo.manipulation;
 
-import java.util.Set;
+import org.objectweb.asm.*;
 
-import org.objectweb.asm.ClassAdapter;
-import org.objectweb.asm.ClassVisitor;
-import org.objectweb.asm.MethodVisitor;
-import org.objectweb.asm.Opcodes;
+import java.util.Set;
 
 /**
  * Adapts a inner class in order to allow accessing outer class fields.
  * A manipulated inner class has access to the managed field of the outer class.
+ *
+ * Only non-static inner classes are manipulated, others are not.
+ *
  * @author <a href="mailto:dev@felix.apache.org">Felix Project Team</a>
  */
 public class InnerClassAdapter extends ClassAdapter implements Opcodes {
@@ -37,7 +37,6 @@
      * Implementation class name.
      */
     private String m_outer;
-
     /**
      * List of fields of the implementation class.
      */
@@ -45,9 +44,10 @@
 
     /**
      * Creates the inner class adapter.
-     * @param arg0 parent class visitor
+     *
+     * @param arg0       parent class visitor
      * @param outerClass outer class (implementation class)
-     * @param fields fields of the implementation class
+     * @param fields     fields of the implementation class
      */
     public InnerClassAdapter(ClassVisitor arg0, String outerClass, Set<String> fields) {
         super(arg0);
@@ -58,15 +58,29 @@
     /**
      * Visits a method.
      * This methods create a code visitor manipulating outer class field accesses.
-     * @param access method visibility
-     * @param name method name
-     * @param desc method descriptor
-     * @param signature method signature
+     *
+     * @param access     method visibility
+     * @param name       method name
+     * @param desc       method descriptor
+     * @param signature  method signature
      * @param exceptions list of exceptions thrown by the method
      * @return a code adapter manipulating field accesses
      * @see org.objectweb.asm.ClassAdapter#visitMethod(int, java.lang.String, java.lang.String, java.lang.String, java.lang.String[])
      */
     public MethodVisitor visitMethod(int access, String name, String desc, String signature, String[] exceptions) {
+        // For non static and non constructor method, generate the wrapping method and move the method content within
+        // the manipulated method.
+
+        // Do nothing on static methods, should not happen in non-static inner classes.
+        if ((access & ACC_STATIC) == ACC_STATIC) {
+            return super.visitMethod(access, name, desc, signature, exceptions);
+        }
+
+        // Do nothing on native methods
+        if ((access & ACC_NATIVE) == ACC_NATIVE) {
+            return super.visitMethod(access, name, desc, signature, exceptions);
+        }
+
         MethodVisitor mv = super.visitMethod(access, name, desc, signature, exceptions);
         return new MethodCodeAdapter(mv, m_outer, access, name, desc, m_fields);
     }
diff --git a/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/InnerClassAdapterTest.java b/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/InnerClassAdapterTest.java
new file mode 100644
index 0000000..2e1bd61
--- /dev/null
+++ b/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/InnerClassAdapterTest.java
@@ -0,0 +1,149 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.felix.ipojo.manipulation;
+
+import junit.framework.Assert;
+import org.apache.felix.ipojo.InstanceManager;
+import org.apache.felix.ipojo.Pojo;
+import org.junit.Ignore;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import java.io.File;
+import java.io.IOException;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.Arrays;
+
+import static junit.framework.Assert.assertEquals;
+import static org.fest.assertions.Assertions.assertThat;
+
+/**
+ * Checks the inner class manipulation
+ */
+public class InnerClassAdapterTest {
+
+    public static String baseClassDirectory = "target/test-classes/";
+
+    private static ManipulatedClassLoader manipulate(String className, Manipulator manipulator) throws IOException {
+
+        byte[] clazz = manipulator.manipulate(ManipulatorTest.getBytesFromFile(new File
+                (baseClassDirectory + className.replace(".", "/") + ".class")));
+        ManipulatedClassLoader classloader = new ManipulatedClassLoader(className, clazz);
+
+        // Manipulate all inner classes
+        for (String s : manipulator.getInnerClasses()) {
+            String outerClassInternalName = className.replace(".", "/");
+            byte[] innerClassBytecode = ManipulatorTest.getBytesFromFile(new File(baseClassDirectory + s + "" +
+                    ".class"));
+            String innerClassName = s.replace("/", ".");
+            InnerClassManipulator innerManipulator = new InnerClassManipulator(outerClassInternalName, manipulator.getFields().keySet());
+            byte[] manipulated = innerManipulator.manipulate(innerClassBytecode, manipulator.getClassVersion());
+            classloader.addInnerClass(innerClassName, manipulated);
+        }
+
+        return classloader;
+    }
+
+    @Test
+    public void testManipulatingTheInner() throws Exception {
+        Manipulator manipulator = new Manipulator();
+        String className = "test.PojoWithInner";
+        byte[] origin =  ManipulatorTest.getBytesFromFile(new File(baseClassDirectory + className.replace(".",
+                "/") + ".class"));
+
+        ManipulatedClassLoader classloader = manipulate(className, manipulator);
+
+        Class cl = classloader.findClass(className);
+        Assert.assertNotNull(cl);
+        Assert.assertNotNull(manipulator.getManipulationMetadata());
+        Assert.assertFalse(manipulator.getInnerClasses().isEmpty());
+
+        System.out.println(manipulator.getManipulationMetadata());
+
+        // The manipulation add stuff to the class.
+        Assert.assertTrue(classloader.get(className).length > origin.length);
+
+
+        boolean found = false;
+        Constructor cst = null;
+        Constructor[] csts = cl.getDeclaredConstructors();
+        for (int i = 0; i < csts.length; i++) {
+            System.out.println(Arrays.asList(csts[i].getParameterTypes()));
+            if (csts[i].getParameterTypes().length == 1  &&
+                    csts[i].getParameterTypes()[0].equals(InstanceManager.class)) {
+                found = true;
+                cst = csts[i];
+            }
+        }
+        Assert.assertTrue(found);
+
+        // We still have the empty constructor
+        found = false;
+        csts = cl.getDeclaredConstructors();
+        for (Constructor cst1 : csts) {
+            System.out.println(Arrays.asList(cst1.getParameterTypes()));
+            if (cst1.getParameterTypes().length == 0) {
+                found = true;
+            }
+        }
+        Assert.assertTrue(found);
+
+        // Check the POJO interface
+        Assert.assertTrue(Arrays.asList(cl.getInterfaces()).contains(Pojo.class));
+
+        InstanceManager im = Mockito.mock(InstanceManager.class);
+        cst.setAccessible(true);
+        Object pojo = cst.newInstance(new Object[] {im});
+        Assert.assertNotNull(pojo);
+        Assert.assertTrue(pojo instanceof Pojo);
+        Method method = cl.getMethod("doSomething", new Class[0]);
+        Assert.assertTrue(((Boolean) method.invoke(pojo, new Object[0])).booleanValue());
+
+    }
+
+
+    @Test
+    public void testInnerClasses() throws IOException, ClassNotFoundException, NoSuchMethodException, IllegalAccessException, InvocationTargetException, InstantiationException {
+        Manipulator manipulator = new Manipulator();
+        String className = "test.inner.ComponentWithInnerClasses";
+        ManipulatedClassLoader classloader = manipulate(className, manipulator);
+
+        Class clazz = classloader.findClass(className);
+        Assert.assertNotNull(clazz);
+        Assert.assertNotNull(manipulator.getManipulationMetadata());
+        Assert.assertFalse(manipulator.getInnerClasses().isEmpty());
+        // We should have found only 2 inner classes.
+        assertThat(manipulator.getInnerClasses().size()).isEqualTo(3);
+
+        // Check that all inner classes are manipulated.
+        InstanceManager im = Mockito.mock(InstanceManager.class);
+        Constructor constructor = clazz.getDeclaredConstructor(InstanceManager.class);
+        constructor.setAccessible(true);
+        Object pojo = constructor.newInstance(new Object[] {im});
+        Assert.assertNotNull(pojo);
+        Assert.assertTrue(pojo instanceof Pojo);
+        Method method = clazz.getMethod("doSomething", new Class[0]);
+        String result = (String) method.invoke(pojo);
+        assertEquals(result, "foofoofoofoo");
+    }
+
+}
diff --git a/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/ManipulatedClassLoader.java b/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/ManipulatedClassLoader.java
index b511c75..eae2eb7 100644
--- a/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/ManipulatedClassLoader.java
+++ b/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/ManipulatedClassLoader.java
@@ -19,6 +19,9 @@
 
 package org.apache.felix.ipojo.manipulation;
 
+import java.util.LinkedHashMap;
+import java.util.Map;
+
 /**
  * A classloader used to load manipulated classes.
  */
@@ -27,19 +30,43 @@
     private String name;
     private byte[] clazz;
 
+    private Map<String, byte[]> inner = new LinkedHashMap<String, byte[]>();
+
     public ManipulatedClassLoader(String name, byte[] clazz) {
         this.name = name;
         this.clazz = clazz;
     }
 
+    public byte[] get(String name) {
+        if (name.equals(this.name)) {
+            return clazz;
+        }
+        if (inner.containsKey(name)) {
+            return inner.get(name);
+        }
+        return null;
+    }
+
+    public void addInnerClass(String name, byte[] clazz) {
+        inner.put(name, clazz);
+    }
+
     public Class findClass(String name) throws ClassNotFoundException {
         if (name.equals(this.name)) {
             return defineClass(name, clazz, 0, clazz.length);
         }
+
+        if (inner.containsKey(name)) {
+            return defineClass(name, inner.get(name), 0, inner.get(name).length);
+        }
+
         return super.findClass(name);
     }
 
-    public Class loadClass(String arg0) throws ClassNotFoundException {
-        return super.loadClass(arg0);
+    public Class loadClass(String classname) throws ClassNotFoundException {
+        if (inner.containsKey(classname)) {
+            return findClass(classname);
+        }
+        return super.loadClass(classname);
     }
 }
diff --git a/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/ManipulatorTest.java b/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/ManipulatorTest.java
index d98497b..ff0f772 100644
--- a/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/ManipulatorTest.java
+++ b/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/ManipulatorTest.java
@@ -197,61 +197,6 @@
 
     }
 
-    @Ignore("This test requires a classloader storing the inner class")
-    public void _testManipulatingTheInner() throws Exception {
-        Manipulator manipulator = new Manipulator();
-        byte[] clazz = manipulator.manipulate(getBytesFromFile(new File("target/test-classes/test/PojoWithInner.class")));
-        ManipulatedClassLoader classloader = new ManipulatedClassLoader("test.PojoWithInner", clazz);
-        Class cl = classloader.findClass("test.PojoWithInner");
-        Assert.assertNotNull(cl);
-        Assert.assertNotNull(manipulator.getManipulationMetadata());
-        Assert.assertFalse(manipulator.getInnerClasses().isEmpty());
-
-
-        System.out.println(manipulator.getManipulationMetadata());
-
-        // The manipulation add stuff to the class.
-        Assert.assertTrue(clazz.length > getBytesFromFile(new File("target/test-classes/test/PojoWithInner.class")).length);
-
-
-        boolean found = false;
-        Constructor cst = null;
-        Constructor[] csts = cl.getDeclaredConstructors();
-        for (int i = 0; i < csts.length; i++) {
-            System.out.println(Arrays.asList(csts[i].getParameterTypes()));
-            if (csts[i].getParameterTypes().length == 1  &&
-                    csts[i].getParameterTypes()[0].equals(InstanceManager.class)) {
-                found = true;
-                cst = csts[i];
-            }
-        }
-        Assert.assertTrue(found);
-
-        // We still have the empty constructor
-        found = false;
-        csts = cl.getDeclaredConstructors();
-        for (int i = 0; i < csts.length; i++) {
-            System.out.println(Arrays.asList(csts[i].getParameterTypes()));
-            if (csts[i].getParameterTypes().length == 0) {
-                found = true;
-            }
-        }
-        Assert.assertTrue(found);
-
-        // Check the POJO interface
-        Assert.assertTrue(Arrays.asList(cl.getInterfaces()).contains(Pojo.class));
-
-        InstanceManager im = (InstanceManager) Mockito.mock(InstanceManager.class);
-        cst.setAccessible(true);
-        Object pojo = cst.newInstance(new Object[] {im});
-        Assert.assertNotNull(pojo);
-        Assert.assertTrue(pojo instanceof Pojo);
-
-        Method method = cl.getMethod("doSomething", new Class[0]);
-        Assert.assertTrue(((Boolean) method.invoke(pojo, new Object[0])).booleanValue());
-
-    }
-
     public void testManipulatingWithConstructorModification() throws Exception {
         Manipulator manipulator = new Manipulator();
         byte[] clazz = manipulator.manipulate(getBytesFromFile(new File("target/test-classes/test/Child.class")));
diff --git a/ipojo/manipulator/manipulator/src/test/java/test/inner/ComponentWithInnerClasses.java b/ipojo/manipulator/manipulator/src/test/java/test/inner/ComponentWithInnerClasses.java
new file mode 100644
index 0000000..f568c7e
--- /dev/null
+++ b/ipojo/manipulator/manipulator/src/test/java/test/inner/ComponentWithInnerClasses.java
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package test.inner;
+
+import org.apache.felix.ipojo.annotations.Component;
+
+/**
+ * A component containing inner classes.
+ */
+@Component
+public class ComponentWithInnerClasses{
+
+    public String doSomething() {
+        MyInnerWithANativeMethod nat = new MyInnerWithANativeMethod();
+        MyInnerClass inn = new MyInnerClass();
+        Computation compute = new Computation() {
+
+            public String compute() {
+                return "foo";
+            }
+        };
+        return nat.foo() + MyStaticInnerClass.foo() + inn.foo() + compute.compute();
+    }
+
+    private class MyInnerWithANativeMethod {
+
+        public String foo() {
+            return "foo";
+        }
+
+        public native void baz();
+
+    }
+
+    public static class MyStaticInnerClass {
+
+        public static String foo() {
+            return "foo";
+        }
+
+        public String bar() {
+            return "bar";
+        }
+
+        public native void baz();
+    }
+
+    private class MyInnerClass {
+        public String foo() {
+            return "foo";
+        }
+    }
+
+
+}
diff --git a/ipojo/manipulator/manipulator/src/test/java/test/inner/Computation.java b/ipojo/manipulator/manipulator/src/test/java/test/inner/Computation.java
new file mode 100644
index 0000000..d9252b9
--- /dev/null
+++ b/ipojo/manipulator/manipulator/src/test/java/test/inner/Computation.java
@@ -0,0 +1,28 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package test.inner;
+
+/**
+ * A simple interface intended to be implemented by an anonymous class.
+ */
+public interface Computation {
+
+    public String compute();
+}