You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@felix.apache.org by cl...@apache.org on 2013/12/11 15:15:27 UTC
svn commit: r1550146 - in /felix/trunk/ipojo/manipulator/manipulator/src:
main/java/org/apache/felix/ipojo/manipulation/
test/java/org/apache/felix/ipojo/manipulation/
Author: clement
Date: Wed Dec 11 14:15:27 2013
New Revision: 1550146
URL: http://svn.apache.org/r1550146
Log:
Fix https://issues.apache.org/jira/browse/FELIX-4347
Detect inner classes that should have been marked static but are not.
Modified:
felix/trunk/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/ClassChecker.java
felix/trunk/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/InnerClassChecker.java
felix/trunk/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/InnerClassAdapterTest.java
felix/trunk/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/ManipulatedClassLoader.java
Modified: felix/trunk/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/ClassChecker.java
URL: http://svn.apache.org/viewvc/felix/trunk/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/ClassChecker.java?rev=1550146&r1=1550145&r2=1550146&view=diff
==============================================================================
--- felix/trunk/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/ClassChecker.java (original)
+++ felix/trunk/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/ClassChecker.java Wed Dec 11 14:15:27 2013
@@ -97,7 +97,6 @@ public class ClassChecker extends EmptyV
*/
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 @@ public class ClassChecker extends EmptyV
}
+ if (name.equals("<clinit>")) {
+ return new InnerClassAssignedToStaticFieldDetector();
+ }
+
return null;
}
@@ -367,7 +370,7 @@ public class ClassChecker extends EmptyV
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 @@ public class ClassChecker extends EmptyV
}
+ /**
+ * 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);
+ }
+ }
+ }
}
Modified: felix/trunk/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/InnerClassChecker.java
URL: http://svn.apache.org/viewvc/felix/trunk/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/InnerClassChecker.java?rev=1550146&r1=1550145&r2=1550146&view=diff
==============================================================================
--- felix/trunk/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/InnerClassChecker.java (original)
+++ felix/trunk/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/InnerClassChecker.java Wed Dec 11 14:15:27 2013
@@ -42,7 +42,6 @@ public class InnerClassChecker extends E
@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;
}
Modified: felix/trunk/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/InnerClassAdapterTest.java
URL: http://svn.apache.org/viewvc/felix/trunk/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/InnerClassAdapterTest.java?rev=1550146&r1=1550145&r2=1550146&view=diff
==============================================================================
--- felix/trunk/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/InnerClassAdapterTest.java (original)
+++ felix/trunk/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/InnerClassAdapterTest.java Wed Dec 11 14:15:27 2013
@@ -23,10 +23,12 @@ import junit.framework.Assert;
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 class InnerClassAdapterTest {
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 @@ public class InnerClassAdapterTest {
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 @@ public class InnerClassAdapterTest {
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 @@ public class InnerClassAdapterTest {
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 @@ public class InnerClassAdapterTest {
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 @@ public class InnerClassAdapterTest {
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) {
Modified: felix/trunk/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/ManipulatedClassLoader.java
URL: http://svn.apache.org/viewvc/felix/trunk/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/ManipulatedClassLoader.java?rev=1550146&r1=1550145&r2=1550146&view=diff
==============================================================================
--- felix/trunk/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/ManipulatedClassLoader.java (original)
+++ felix/trunk/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/ManipulatedClassLoader.java Wed Dec 11 14:15:27 2013
@@ -88,4 +88,10 @@ public class ManipulatedClassLoader exte
public Map<String, byte[]> getAllInnerClasses() {
return inner;
}
+
+ public void addInnerClassIfNotAlreadyDefined(String name, byte[] innerClassBytecode) {
+ if (! inner.containsKey(name)) {
+ inner.put(name, innerClassBytecode);
+ }
+ }
}