[jboss-cvs] JBossAS SVN: r102848 - in projects/aop/branches/Branch_2_2/aop: src/main/java/org/jboss/aop/instrument and 1 other directories.
jboss-cvs-commits at lists.jboss.org
jboss-cvs-commits at lists.jboss.org
Tue Mar 23 22:49:50 EDT 2010
Author: flavia.rainone at jboss.com
Date: 2010-03-23 22:49:50 -0400 (Tue, 23 Mar 2010)
New Revision: 102848
Added:
projects/aop/branches/Branch_2_2/aop/src/test/java/org/jboss/test/aop/field/SuperClassFieldTestCase.java
Modified:
projects/aop/branches/Branch_2_2/aop/1.5.x-compliance-tests.xml
projects/aop/branches/Branch_2_2/aop/2.0.x-compliance-tests.xml
projects/aop/branches/Branch_2_2/aop/src/main/java/org/jboss/aop/instrument/Instrumentor.java
projects/aop/branches/Branch_2_2/aop/src/test/java/org/jboss/test/aop/field/FieldTestCase.java
Log:
[JBAOP-779] Bug fixed: now Instrumentor.convertReferences does look at all superclasses and interfaces, regardless of whether a referenced class is marked to be skipped at InterceptionMarkers.
FieldTestCase still won't pass in compliance tests; for that reason the failing test has been extracted to a new SuperCalssFieldTestCase, which is excluded from compliance precompiled tests.
Modified: projects/aop/branches/Branch_2_2/aop/1.5.x-compliance-tests.xml
===================================================================
--- projects/aop/branches/Branch_2_2/aop/1.5.x-compliance-tests.xml 2010-03-24 02:48:12 UTC (rev 102847)
+++ projects/aop/branches/Branch_2_2/aop/1.5.x-compliance-tests.xml 2010-03-24 02:49:50 UTC (rev 102848)
@@ -415,6 +415,7 @@
<!-- These are not test cases, and so they will fail when junit tries to run them. Should really rename all tests to *TestCase -->
<exclude name="org/jboss/test/aop/reflection/ReflectionAspectTester.class"/>
<exclude name="org/jboss/test/aop/basic/POJOAspectTester.class"/>
+ <exclude name="org/jboss/test/aop/field/SuperClassFieldTestCase.class"/>
</fileset>
</batchtest>
</junit>
Modified: projects/aop/branches/Branch_2_2/aop/2.0.x-compliance-tests.xml
===================================================================
--- projects/aop/branches/Branch_2_2/aop/2.0.x-compliance-tests.xml 2010-03-24 02:48:12 UTC (rev 102847)
+++ projects/aop/branches/Branch_2_2/aop/2.0.x-compliance-tests.xml 2010-03-24 02:49:50 UTC (rev 102848)
@@ -721,7 +721,7 @@
<exclude name="org/jboss/test/aop/constructortarget/*.class"/>
<exclude name="org/jboss/test/aop/reflection/ReflectionAspectTester.class"/>
<exclude name="org/jboss/test/aop/basic/POJOAspectTester.class"/>
-
+ <exclude name="org/jboss/test/aop/field/SuperClassFieldTestCase.class"/>
</fileset>
</batchtest>
</junit>
Modified: projects/aop/branches/Branch_2_2/aop/src/main/java/org/jboss/aop/instrument/Instrumentor.java
===================================================================
--- projects/aop/branches/Branch_2_2/aop/src/main/java/org/jboss/aop/instrument/Instrumentor.java 2010-03-24 02:48:12 UTC (rev 102847)
+++ projects/aop/branches/Branch_2_2/aop/src/main/java/org/jboss/aop/instrument/Instrumentor.java 2010-03-24 02:49:50 UTC (rev 102848)
@@ -51,6 +51,7 @@
import org.jboss.aop.Advisor;
import org.jboss.aop.AspectManager;
import org.jboss.aop.ClassAdvisor;
+import org.jboss.aop.InterceptionMarkers;
import org.jboss.aop.annotation.compiler.AnnotationInfoCreator;
import org.jboss.aop.array.ArrayAdvisor;
import org.jboss.aop.array.ArrayReplacement;
@@ -615,100 +616,123 @@
protected boolean convertReferences(CtClass clazz, ClassAdvisor clazzAdvisor) throws Exception
{
boolean converted = false;
- String ref = null;
- try
+ for (CtClass reference: getAllReferences(clazz))
{
- ClassPool pool = ClassPoolRepository.getInstance().getClassPoolFactory().create(clazz.getClassPool(), ClassPoolRepository.getInstance());
+ String referenceName = reference.getName();
+ ClassAdvisor advisor = reference == clazz? clazzAdvisor:
+ manager.getTempClassAdvisor(reference);
+ final ClassLoader refCl = reference.getClassPool().getClassLoader();
+ InterceptionMarkers markers = manager.getInterceptionMarkers(refCl);
- //Class.getRefClasses() only gets classes explicitly referenced in the class. We need to check the super classes and do some extra handling
- for (ReferenceClassIterator it = new ReferenceClassIterator(clazz.getRefClasses()) ; it.hasNext() ; )
+ if (!markers.shouldSkipFieldAccess(referenceName) &&
+ !referenceName.equals(clazz.getName()))
{
- ref = it.next();
- if (!manager.getInterceptionMarkers(clazz.getClassPool().getClassLoader()).convertReference(ref)
- || manager.isNonAdvisableClassName(ref)
- || ref.startsWith("java.")
- || ref.startsWith("javax.")
- || ref.startsWith("["))
+ List<CtField> fields = getAdvisableFields(reference);
+ if (fieldAccessTransformer.replaceFieldAccess(fields, reference, advisor))
{
- continue;
+ markers.addFieldInterceptionMarker(referenceName);
+ converted = true;
}
- // Only need a temporary advisor for resolving metadata
- CtClass ctRef = null;
- ClassAdvisor advisor = null;
- if (ref.equals(clazz.getName()))
- {
- ctRef = clazz;
- advisor = clazzAdvisor;
- }
else
{
- try
- {
- ctRef = pool.get(ref);
- }
- catch (NotFoundException e)
- {
- if (AspectManager.suppressReferenceErrors)
- {
- System.err.println("[warn] Could not find class " + ref + " (or one of its implemented interfaces) that " + clazz.getName() + " references. It may not be in your classpath and you may not be getting field and constructor weaving for this class.");
- if (AspectManager.verbose) e.printStackTrace();
- continue;
- }
- else
- {
- throw e;
- }
- }
- if (!isTransformable(ctRef)) continue;
- advisor = manager.getTempClassAdvisor(ctRef);
+ markers.skipFieldAccess(referenceName);
}
- it.addSuperClass(ctRef);
- //converted = false;
-
- final ClassLoader refCl = ctRef.getClassPool().getClassLoader();
-
- if (!manager.getInterceptionMarkers(refCl).shouldSkipFieldAccess(ref) && !ref.equals(clazz.getName()))
+ }
+ if (!markers.shouldSkipConstruction(referenceName))
+ {
+ if (constructorExecutionTransformer.replaceConstructorAccess(advisor, reference))
{
- List<CtField> fields = getAdvisableFields(ctRef);
- if (fieldAccessTransformer.replaceFieldAccess(fields, ctRef, advisor))
- {
- manager.getInterceptionMarkers(refCl).addFieldInterceptionMarker(ref);
- converted = true;
- }
- else
- {
- manager.getInterceptionMarkers(refCl).skipFieldAccess(ref);
- }
+ markers.addConstructionInterceptionMarker(referenceName);
+ converted = true;
}
- if (!manager.getInterceptionMarkers(refCl).shouldSkipConstruction(ref))
+ else
{
- if (constructorExecutionTransformer.replaceConstructorAccess(advisor, ctRef))
- {
- manager.getInterceptionMarkers(refCl).addConstructionInterceptionMarker(ref);
- converted = true;
- }
- else
- {
- manager.getInterceptionMarkers(refCl).skipConstruction(ref);
- }
+ markers.skipConstruction(referenceName);
}
+ }
- if (!converted)
+ if (!converted)
+ {
+ markers.skipReference(referenceName);
+ }
+ }
+ return converted;
+ }
+
+ @SuppressWarnings("unchecked")
+ private Collection<CtClass> getAllReferences(CtClass clazz)
+ {
+ ClassPool pool = ClassPoolRepository.getInstance().getClassPoolFactory().
+ create(clazz.getClassPool(), ClassPoolRepository.getInstance());
+
+ Collection<String> refClassNames = new HashSet<String>();
+ refClassNames.addAll((Collection<String>) clazz.getRefClasses());
+ Map<String, CtClass> refClasses = new HashMap<String, CtClass>();
+ for (String refClassName: refClassNames)
+ {
+ addRefClass(refClassName, null, pool, clazz, refClasses);
+ }
+ return refClasses.values();
+ }
+
+ private void addRefClass(String refClassName, CtClass refClass,
+ ClassPool pool, CtClass clazz, Map<String, CtClass> refClasses)
+ {
+ if (manager.isNonAdvisableClassName(refClassName) || refClassName.startsWith("[") ||
+ refClasses.containsKey(refClassName))
+ {
+ return;
+ }
+ try
+ {
+ if (refClass == null)
+ {
+ if (refClassName.equals(clazz.getName()))
{
- manager.getInterceptionMarkers(refCl).skipReference(ref);
+ refClass = clazz;
}
- ref = null;
+ else
+ {
+ refClass = pool.get(refClassName);
+ }
}
+ ClassLoader classLoader = clazz.getClassPool().getClassLoader();
+ if ((refClass == clazz || isTransformable(refClass)) &&
+ manager.getInterceptionMarkers(classLoader).convertReference(refClassName))
+ {
+ refClasses.put(refClassName, refClass);
+ }
+ CtClass superRefClass = refClass.getSuperclass();
+ if (superRefClass != null)
+ {
+ addRefClass(superRefClass.getName(), superRefClass, pool, clazz, refClasses);
+ }
+ for (CtClass refInterface: refClass.getInterfaces())
+ {
+ addRefClass(refInterface.getName(), refInterface, pool, clazz, refClasses);
+ }
}
- catch (Exception ex)
+ catch (NotFoundException e)
{
- if (ref != null)
+ if (AspectManager.suppressReferenceErrors)
{
- throw new TransformationException("Failed to aspectize class " + clazz.getName() + ". Could not find class it references " + ref + " (or one of its implemented interfaces). It may not be in your classpath and you may not be getting field and constructor weaving for this class.");
+ System.err.println("[warn] Could not find class " +
+ refClassName +
+ " (or one of its implemented interfaces) that " +
+ clazz.getName() + " references. It may not be in your " +
+ "classpath and you may not be getting field and" +
+ " constructor weaving for this class.");
+ if (AspectManager.verbose) e.printStackTrace();
}
- throw ex;
+ else
+ {
+ throw new TransformationException("Failed to aspectize class " +
+ clazz.getName() + ". Could not find class it references " +
+ refClassName + " (or one of its implemented interfaces). " +
+ "It may not be in your classpath and you may not be " +
+ "getting field and constructor weaving for this class.");
+ }
}
- return converted;
}
protected boolean shouldNotTransform(CtClass clazz)throws NotFoundException
@@ -1176,60 +1200,6 @@
protected abstract CtMethod createMixinInvokeMethod(CtClass clazz, CtClass mixinClass, String initializer, CtMethod method, long hash)
throws CannotCompileException, NotFoundException, Exception;
- private static class ReferenceClassIterator
- {
- int size;
- int current;
- ArrayList<String> classes;
- HashSet<String> handledClasses;
- String currentEntry;
-
- public ReferenceClassIterator(Collection<String> refClasses)
- {
- size = refClasses.size();
- classes = new ArrayList<String>(refClasses.size());
- classes.addAll(refClasses);
- handledClasses = new HashSet<String>(refClasses.size());
- }
-
- boolean hasNext()
- {
- while (current < size)
- {
- String s = classes.get(current++);
- if (!handledClasses.contains(s))
- {
- handledClasses.add(s);
- currentEntry = s;
- return true;
- }
- }
- return false;
- }
-
- String next()
- {
- return currentEntry;
- }
-
- void addSuperClass(CtClass clazz)throws NotFoundException
- {
- if (clazz != null)
- {
- CtClass superClass = clazz.getSuperclass();
- if (superClass != null)
- {
- String name = superClass.getName();
- if (!handledClasses.contains(name))
- {
- classes.add(name);
- size++;
- }
- }
- }
- }
- }
-
AspectManager getManager()
{
return manager;
Modified: projects/aop/branches/Branch_2_2/aop/src/test/java/org/jboss/test/aop/field/FieldTestCase.java
===================================================================
--- projects/aop/branches/Branch_2_2/aop/src/test/java/org/jboss/test/aop/field/FieldTestCase.java 2010-03-24 02:48:12 UTC (rev 102847)
+++ projects/aop/branches/Branch_2_2/aop/src/test/java/org/jboss/test/aop/field/FieldTestCase.java 2010-03-24 02:49:50 UTC (rev 102848)
@@ -1,24 +1,24 @@
/*
- * JBoss, Home of Professional Open Source
- * Copyright 2005, JBoss Inc., and individual contributors as indicated
- * by the @authors tag. See the copyright.txt in the distribution for a
- * full listing of individual contributors.
- *
- * This is free software; you can redistribute it and/or modify it
- * under the terms of the GNU Lesser General Public License as
- * published by the Free Software Foundation; either version 2.1 of
- * the License, or (at your option) any later version.
- *
- * This software is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this software; if not, write to the Free
- * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
- * 02110-1301 USA, or see the FSF site: http://www.fsf.org.
- */
+ * JBoss, Home of Professional Open Source
+ * Copyright 2005, JBoss Inc., and individual contributors as indicated
+ * by the @authors tag. See the copyright.txt in the distribution for a
+ * full listing of individual contributors.
+ *
+ * This is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as
+ * published by the Free Software Foundation; either version 2.1 of
+ * the License, or (at your option) any later version.
+ *
+ * This software is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this software; if not, write to the Free
+ * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA, or see the FSF site: http://www.fsf.org.
+ */
package org.jboss.test.aop.field;
import java.lang.reflect.Field;
@@ -252,15 +252,4 @@
assertEquals(10, pojo.getOnly);
assertTrue(TraceInterceptor.intercepted);
}
-
- public void testFieldsReplacedInSubClass()
- {
- C c = new C();
- //Sanity
- assertEquals("intercepted", c.inheritedFieldInSubClassFieldA);
- //These are the real purpose of this test
- assertEquals("intercepted", c.useField());
- assertEquals("intercepted", AccessFieldViaB.accessField(c));
- assertEquals("intercepted", AccessFieldViaC.accessField(c));
- }
}
Added: projects/aop/branches/Branch_2_2/aop/src/test/java/org/jboss/test/aop/field/SuperClassFieldTestCase.java
===================================================================
--- projects/aop/branches/Branch_2_2/aop/src/test/java/org/jboss/test/aop/field/SuperClassFieldTestCase.java (rev 0)
+++ projects/aop/branches/Branch_2_2/aop/src/test/java/org/jboss/test/aop/field/SuperClassFieldTestCase.java 2010-03-24 02:49:50 UTC (rev 102848)
@@ -0,0 +1,72 @@
+/*
+ * JBoss, Home of Professional Open Source
+ * Copyright 2005, JBoss Inc., and individual contributors as indicated
+ * by the @authors tag. See the copyright.txt in the distribution for a
+ * full listing of individual contributors.
+ *
+ * This is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as
+ * published by the Free Software Foundation; either version 2.1 of
+ * the License, or (at your option) any later version.
+ *
+ * This software is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this software; if not, write to the Free
+ * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA, or see the FSF site: http://www.fsf.org.
+ */
+package org.jboss.test.aop.field;
+
+import junit.framework.Test;
+import junit.framework.TestSuite;
+import junit.textui.TestRunner;
+
+import org.jboss.test.aop.AOPTestWithSetup;
+
+/**
+ *
+ * @author <a href="mailto:stalep at conduct.no">Stale W. Pedersen</a>
+ * @author <a href="mailto:flavia.rainone at jboss.com">Flavia Rainone</a>
+ * @version $Revision 1.1 $
+ */
+public class SuperClassFieldTestCase extends AOPTestWithSetup
+{
+
+ public static void main(String[] args)
+ {
+ TestRunner.run(suite());
+ }
+
+ public static Test suite()
+ {
+ TestSuite suite = new TestSuite("SuperClassFieldTestCase");
+ suite.addTestSuite(SuperClassFieldTestCase.class);
+ return suite;
+ }
+
+ public SuperClassFieldTestCase(String name)
+ {
+ super(name);
+ }
+
+ protected void setUp() throws Exception
+ {
+ super.setUp();
+ }
+
+ // JBAOP-779 this test fails with versions prior to 2.2.0.GA
+ public void testFieldsReplacedInSubClass()
+ {
+ C c = new C();
+ //Sanity
+ assertEquals("intercepted", c.inheritedFieldInSubClassFieldA);
+ //These are the real purpose of this test
+ assertEquals("intercepted", c.useField());
+ assertEquals("intercepted", AccessFieldViaB.accessField(c));
+ assertEquals("intercepted", AccessFieldViaC.accessField(c));
+ }
+}
Property changes on: projects/aop/branches/Branch_2_2/aop/src/test/java/org/jboss/test/aop/field/SuperClassFieldTestCase.java
___________________________________________________________________
Name: svn:keywords
+ Author Date Id Revision
Name: svn:eol-style
+ native
More information about the jboss-cvs-commits
mailing list