Fix https://issues.apache.org/jira/browse/FELIX-4347
Detect inner classes that should have been marked static but are not.
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1550146 13f79535-47bb-0310-9956-ffa450edef68
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 479a026..333c9db 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
@@ -97,7 +97,6 @@
*/
public FieldVisitor visitField(int access, String name, String desc,
String signature, Object value) {
-
if (name.equals(MethodCreator.IM_FIELD)
&& desc.equals("Lorg/apache/felix/ipojo/InstanceManager;")) {
m_isAlreadyManipulated = true;
@@ -232,6 +231,10 @@
}
+ if (name.equals("<clinit>")) {
+ return new InnerClassAssignedToStaticFieldDetector();
+ }
+
return null;
}
@@ -367,7 +370,7 @@
public void visitLocalVariable(String name, String desc, String signature, Label start, Label end, int index) {
m_method.addLocalVariable(name, desc, signature, index);
}
-
+
public void visitEnd() {
m_method.end();
}
@@ -788,4 +791,20 @@
}
+ /**
+ * Class required to detect inner classes assigned to static field and thus must not be manipulated (FELIX-4347).
+ * If an inner class is assigned to a static field, it must not be manipulated.
+ *
+ * However notice that this is only useful when AspectJ is used, because aspectJ is changing the 'staticity' of
+ * the inner class.
+ */
+ private class InnerClassAssignedToStaticFieldDetector extends EmptyVisitor implements MethodVisitor {
+
+ @Override
+ public void visitTypeInsn(int opcode, String type) {
+ if (opcode == NEW && m_inners.containsKey(type)) {
+ m_inners.remove(type);
+ }
+ }
+ }
}
diff --git a/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/InnerClassChecker.java b/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/InnerClassChecker.java
index b30a5b7..cd93eee 100644
--- a/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/InnerClassChecker.java
+++ b/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/InnerClassChecker.java
@@ -42,7 +42,6 @@
@Override
public MethodVisitor visitMethod(int access, String name, String desc, String signature, String[] exceptions) {
// Do not collect static and native method.
-
if ((access & ACC_STATIC) == ACC_STATIC) {
return null;
}
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
index e5f745e..0216535 100644
--- 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
@@ -23,10 +23,12 @@
import org.apache.felix.ipojo.InstanceManager;
import org.apache.felix.ipojo.Pojo;
import org.apache.felix.ipojo.metadata.Element;
+import org.junit.Ignore;
import org.junit.Test;
import org.mockito.Mockito;
import java.io.File;
+import java.io.FilenameFilter;
import java.io.IOException;
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
@@ -46,8 +48,9 @@
public static String baseClassDirectory = "target/test-classes/";
public static ManipulatedClassLoader manipulate(String className, Manipulator manipulator) throws IOException {
+ final File mainClassFile = new File(baseClassDirectory + className.replace(".", "/") + ".class");
byte[] bytecode = ManipulatorTest.getBytesFromFile(
- new File(baseClassDirectory + className.replace(".", "/") + ".class"));
+ mainClassFile);
// Preparation.
try {
@@ -92,12 +95,30 @@
Assert.fail("Cannot find inner class '" + resourcePath + "'");
}
}
+
+ // Lookup for all the other inner classes (not manipulated)
+ File[] files = mainClassFile.getParentFile().listFiles(new FilenameFilter() {
+ public boolean accept(File dir, String name) {
+ return name.startsWith(mainClassFile.getName().substring(0, mainClassFile.getName().length() -
+ ".class".length()) + "$");
+ }
+ });
+
+ for (File f : files) {
+ String name = className + f.getName().substring(f.getName().indexOf("$"));
+ name = name.substring(0, name.length() - ".class".length());
+ byte[] innerClassBytecode = ManipulatorTest.getBytesFromFile(f);
+ classloader.addInnerClassIfNotAlreadyDefined(name, innerClassBytecode);
+ }
+
return classloader;
}
public static ManipulatedClassLoader manipulate(String className, Manipulator manipulator,
ManipulatedClassLoader initial) throws IOException {
byte[] bytecode = initial.get(className);
+ final File mainClassFile = new File(baseClassDirectory + className.replace(".", "/") + ".class");
+ String mainClass = className;
// Preparation.
try {
@@ -142,6 +163,23 @@
Assert.fail("Cannot find inner class '" + resourcePath + "'");
}
}
+
+ // Lookup for all the other inner classes (not manipulated)
+ File[] files = mainClassFile.getParentFile().listFiles(new FilenameFilter() {
+ public boolean accept(File dir, String name) {
+ return name.startsWith(mainClassFile.getName().substring(0, mainClassFile.getName().length() -
+ ".class".length()) + "$");
+ }
+ });
+
+
+ for (File f : files) {
+ String name = className + f.getName().substring(f.getName().indexOf("$"));
+ name = name.substring(0, name.length() - ".class".length());
+ byte[] innerClassBytecode = ManipulatorTest.getBytesFromFile(f);
+ classloader.addInnerClassIfNotAlreadyDefined(name, innerClassBytecode);
+ }
+
return classloader;
}
@@ -238,7 +276,7 @@
InstanceManager im = Mockito.mock(InstanceManager.class);
Constructor constructor = clazz.getDeclaredConstructor(InstanceManager.class);
constructor.setAccessible(true);
- Object pojo = constructor.newInstance(new Object[]{im});
+ Object pojo = constructor.newInstance(im);
Assert.assertNotNull(pojo);
Assert.assertTrue(pojo instanceof Pojo);
Method method = clazz.getMethod("doSomething", new Class[0]);
@@ -267,7 +305,7 @@
InstanceManager im = Mockito.mock(InstanceManager.class);
Constructor constructor = clazz.getDeclaredConstructor(InstanceManager.class);
constructor.setAccessible(true);
- Object pojo = constructor.newInstance(new Object[]{im});
+ Object pojo = constructor.newInstance(im);
Assert.assertNotNull(pojo);
Assert.assertTrue(pojo instanceof Pojo);
Method method = clazz.getMethod("doSomething", new Class[0]);
@@ -337,17 +375,17 @@
assertThat((String) bar.invoke(o)).isEqualTo("bar");
}
-// @Test
-// public void testThatAnonymousClassDeclaredInStaticFieldsAreNotManipulated() throws Exception {
-// Manipulator manipulator = new Manipulator();
-// String className = "test.inner.ComponentWithInnerClasses";
-// ManipulatedClassLoader classLoader = manipulate(className, manipulator);
-//
-// Class clazz = classLoader.findClass(className);
-// Method method = clazz.getMethod("call");
-// assertThat(method).isNotNull();
-// assertThat(method.invoke(null)).isEqualTo(1);
-// }
+ @Test
+ public void testThatAnonymousClassDeclaredInStaticFieldsAreNotManipulated() throws Exception {
+ Manipulator manipulator = new Manipulator();
+ String className = "test.inner.ComponentWithInnerClasses";
+ ManipulatedClassLoader classLoader = manipulate(className, manipulator);
+
+ Class clazz = classLoader.findClass(className);
+ Method method = clazz.getMethod("call");
+ assertThat(method).isNotNull();
+ assertThat(method.invoke(null)).isEqualTo(1);
+ }
private Class findInnerClass(Class[] classes, String name) {
for (Class clazz : classes) {
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 6cf6329..1aa8ce4 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
@@ -88,4 +88,10 @@
public Map<String, byte[]> getAllInnerClasses() {
return inner;
}
+
+ public void addInnerClassIfNotAlreadyDefined(String name, byte[] innerClassBytecode) {
+ if (! inner.containsKey(name)) {
+ inner.put(name, innerClassBytecode);
+ }
+ }
}