diff options
15 files changed, 203 insertions, 59 deletions
diff --git a/loadtime/src/org/aspectj/weaver/loadtime/Aj.java b/loadtime/src/org/aspectj/weaver/loadtime/Aj.java index 079b5439a..c70545301 100644 --- a/loadtime/src/org/aspectj/weaver/loadtime/Aj.java +++ b/loadtime/src/org/aspectj/weaver/loadtime/Aj.java @@ -11,8 +11,6 @@ *******************************************************************************/ package org.aspectj.weaver.loadtime; -import java.io.File; -import java.io.FileOutputStream; import java.lang.reflect.Method; import java.lang.reflect.InvocationTargetException; import java.util.Map; @@ -63,11 +61,7 @@ public class Aj implements ClassPreProcessor { try { WeavingAdaptor weavingAdaptor = WeaverContainer.getWeaver(loader, weavingContext); - byte[] weaved = weavingAdaptor.weaveClass(className, bytes); - if (weavingAdaptor.shouldDump(className.replace('/', '.'))) { - dump(className, weaved); - } - return weaved; + return weavingAdaptor.weaveClass(className, bytes); } catch (Throwable t) { //FIXME AV wondering if we should have the option to fail (throw runtime exception) here // would make sense at least in test f.e. see TestHelper.handleMessage() @@ -165,32 +159,6 @@ public class Aj implements ClassPreProcessor { } /** - * Dump the given bytcode in _dump/... (dev mode) - * - * @param name - * @param b - * @throws Throwable - */ - static void dump(String name, byte[] b) throws Throwable { - String className = name.replace('.', '/'); - final File dir; - if (className.indexOf('/') > 0) { - dir = new File("_ajdump" + File.separator + className.substring(0, className.lastIndexOf('/'))); - } else { - dir = new File("_ajdump"); - } - dir.mkdirs(); - String fileName = "_ajdump" + File.separator + className + ".class"; - FileOutputStream os = new FileOutputStream(fileName); - os.write(b); - os.close(); - } - - /* - * Shared classes methods - */ - - /** * Returns a namespace based on the contest of the aspects available */ public String getNamespace (ClassLoader loader) { diff --git a/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java b/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java index 09d11f162..b17392d05 100644 --- a/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java +++ b/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java @@ -54,6 +54,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor { private final static String AOP_XML = "META-INF/aop.xml"; private List m_dumpTypePattern = new ArrayList(); + private boolean m_dumpBefore = false; private List m_includeTypePattern = new ArrayList(); private List m_excludeTypePattern = new ArrayList(); private List m_includeStartsWith = new ArrayList(); @@ -81,8 +82,8 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor { */ public void acceptClass(String name, byte[] bytes) { try { - if (shouldDump(name.replace('/', '.'))) { - Aj.dump(name, bytes); + if (shouldDump(name.replace('/', '.'), false)) { + dump(name, bytes, false); } } catch (Throwable throwable) { throwable.printStackTrace(); @@ -438,6 +439,9 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor { TypePattern pattern = new PatternParser(dump).parseTypePattern(); m_dumpTypePattern.add(pattern); } + if (definition.shouldDumpBefore()) { + m_dumpBefore = true; + } } } @@ -543,11 +547,17 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor { return accept; } - public boolean shouldDump(String className) { + protected boolean shouldDump(String className, boolean before) { + // Don't dump before weaving unless asked to + if (before && !m_dumpBefore) { + return false; + } + // avoid ResolvedType if not needed if (m_dumpTypePattern.isEmpty()) { return false; } + //TODO AV - optimize for className.startWith only ResolvedType classInfo = weaver.getWorld().resolve(UnresolvedType.forName(className), true); //dump diff --git a/loadtime/src/org/aspectj/weaver/loadtime/definition/Definition.java b/loadtime/src/org/aspectj/weaver/loadtime/definition/Definition.java index efb7d3259..2730428d2 100644 --- a/loadtime/src/org/aspectj/weaver/loadtime/definition/Definition.java +++ b/loadtime/src/org/aspectj/weaver/loadtime/definition/Definition.java @@ -24,6 +24,7 @@ public class Definition { private StringBuffer m_weaverOptions; private List m_dumpPatterns; + private boolean m_dumpBefore; private List m_includePatterns; @@ -39,6 +40,7 @@ public class Definition { public Definition() { m_weaverOptions = new StringBuffer(); + m_dumpBefore = false; m_dumpPatterns = new ArrayList(0); m_includePatterns = new ArrayList(0); m_excludePatterns = new ArrayList(0); @@ -56,6 +58,14 @@ public class Definition { return m_dumpPatterns; } + public void setDumpBefore(boolean b) { + m_dumpBefore = b; + } + + public boolean shouldDumpBefore() { + return m_dumpBefore; + } + public List getIncludePatterns() { return m_includePatterns; } diff --git a/loadtime/src/org/aspectj/weaver/loadtime/definition/DocumentParser.java b/loadtime/src/org/aspectj/weaver/loadtime/definition/DocumentParser.java index 931e26d17..2800ebece 100644 --- a/loadtime/src/org/aspectj/weaver/loadtime/definition/DocumentParser.java +++ b/loadtime/src/org/aspectj/weaver/loadtime/definition/DocumentParser.java @@ -51,6 +51,7 @@ public class DocumentParser extends DefaultHandler { private final static String ASPECTJ_ELEMENT = "aspectj"; private final static String WEAVER_ELEMENT = "weaver"; private final static String DUMP_ELEMENT = "dump"; + private final static String DUMP_BEFOREANDAFTER_ATTRIBUTE = "beforeandafter"; private final static String INCLUDE_ELEMENT = "include"; private final static String EXCLUDE_ELEMENT = "exclude"; private final static String OPTIONS_ATTRIBUTE = "options"; @@ -206,6 +207,10 @@ public class DocumentParser extends DefaultHandler { if (!isNull(typePattern)) { m_definition.getDumpPatterns().add(typePattern); } + String beforeAndAfter = attributes.getValue(DUMP_BEFOREANDAFTER_ATTRIBUTE); + if (isTrue(beforeAndAfter)) { + m_definition.setDumpBefore(true); + } } else if (EXCLUDE_ELEMENT.equals(qName) && m_inAspects) { String typePattern = attributes.getValue(WITHIN_ATTRIBUTE); if (!isNull(typePattern)) { @@ -258,5 +263,9 @@ public class DocumentParser extends DefaultHandler { return (s == null || s.length() <= 0); } + private boolean isTrue(String s) { + return (s != null && s.equals("true")); + } + } diff --git a/loadtime/testsrc/org/aspectj/weaver/loadtime/test/DocumentParserTest.java b/loadtime/testsrc/org/aspectj/weaver/loadtime/test/DocumentParserTest.java index fddbc2794..92d35983f 100644 --- a/loadtime/testsrc/org/aspectj/weaver/loadtime/test/DocumentParserTest.java +++ b/loadtime/testsrc/org/aspectj/weaver/loadtime/test/DocumentParserTest.java @@ -39,6 +39,7 @@ public class DocumentParserTest extends TestCase { assertEquals("@Baz", def.getAspectExcludePatterns().get(0)); assertEquals("@Whoo", def.getAspectIncludePatterns().get(0)); assertEquals("foo..*", def.getDumpPatterns().get(0)); + assertEquals(true,def.shouldDumpBefore()); } } diff --git a/loadtime/testsrc/org/aspectj/weaver/loadtime/test/simpleWithDtd.xml b/loadtime/testsrc/org/aspectj/weaver/loadtime/test/simpleWithDtd.xml index f565e6ef1..252bc0136 100644 --- a/loadtime/testsrc/org/aspectj/weaver/loadtime/test/simpleWithDtd.xml +++ b/loadtime/testsrc/org/aspectj/weaver/loadtime/test/simpleWithDtd.xml @@ -3,7 +3,7 @@ <aspectj> <weaver options="-showWeaveInfo"> <include within="foo..bar.Goo+"/> - <dump within="foo..*"/> + <dump within="foo..*" beforeandafter="true"/> </weaver> <aspects> <exclude within="@Baz"/> diff --git a/tests/java5/ataspectj/ataspectj/DumpTest.java b/tests/java5/ataspectj/ataspectj/DumpTest.java index 6039f766a..4fb2b7487 100644 --- a/tests/java5/ataspectj/ataspectj/DumpTest.java +++ b/tests/java5/ataspectj/ataspectj/DumpTest.java @@ -21,25 +21,7 @@ import java.io.File; public class DumpTest extends TestCase { public static void main(String[] args) { - TestHelper.runAndThrowOnFailure(suite()); - } - - public static junit.framework.Test suite() { - return new junit.framework.TestSuite(DumpTest.class); - } - - public void testDump() { - File f = new File("_ajdump/ataspectj/DumpTest.class"); - assertFalse(f.exists()); - - DumpTestTheDump forceLoad = new DumpTestTheDump(); - f = new File("_ajdump/ataspectj/DumpTestTheDump.class"); - assertTrue(f.exists()); - - // tidy up... - f.delete(); - new File("_ajdump/ataspectj").delete(); - new File("_ajdump").delete(); + new DumpTestTheDump().aroundMethod("DumpTest"); } } diff --git a/tests/java5/ataspectj/ataspectj/DumpTestTheDump.java b/tests/java5/ataspectj/ataspectj/DumpTestTheDump.java index d2f65ff34..f33ffa85a 100644 --- a/tests/java5/ataspectj/ataspectj/DumpTestTheDump.java +++ b/tests/java5/ataspectj/ataspectj/DumpTestTheDump.java @@ -15,4 +15,7 @@ package ataspectj; * @author <a href="mailto:alex AT gnilux DOT com">Alexandre Vasseur</a> */ public class DumpTestTheDump { + public void aroundMethod (String s) { + + } } diff --git a/tests/java5/ataspectj/ataspectj/TestAroundAspect.aj b/tests/java5/ataspectj/ataspectj/TestAroundAspect.aj new file mode 100644 index 000000000..8ae2954c3 --- /dev/null +++ b/tests/java5/ataspectj/ataspectj/TestAroundAspect.aj @@ -0,0 +1,18 @@ +/******************************************************************************* + * Copyright (c) 2005 Contributors. + * All rights reserved. + * This program and the accompanying materials are made available + * under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution and is available at + * http://eclipse.org/legal/epl-v10.html + * + * Contributors: + * Matthew Webster initial implementation + *******************************************************************************/ +package ataspectj; + +public aspect TestAroundAspect { + Object around () : execution(public void aroundMethod(..)) { + return proceed(); + } +} diff --git a/tests/java5/ataspectj/ataspectj/aop-dumpbeforeandafter.xml b/tests/java5/ataspectj/ataspectj/aop-dumpbeforeandafter.xml new file mode 100644 index 000000000..c2824e2be --- /dev/null +++ b/tests/java5/ataspectj/ataspectj/aop-dumpbeforeandafter.xml @@ -0,0 +1,6 @@ +<?xml version="1.0"?> +<aspectj> + <weaver options="-XmessageHandlerClass:ataspectj.TestHelper"> + <dump within="ataspectj.DumpTestThe*" beforeandafter="true"/> + </weaver> +</aspectj> diff --git a/tests/java5/ataspectj/ataspectj/aop-dumpclosure.xml b/tests/java5/ataspectj/ataspectj/aop-dumpclosure.xml new file mode 100644 index 000000000..d73c1c23b --- /dev/null +++ b/tests/java5/ataspectj/ataspectj/aop-dumpclosure.xml @@ -0,0 +1,9 @@ +<?xml version="1.0"?> +<aspectj> + <aspects> + <aspect name="ataspectj.TestAroundAspect"/> + </aspects> + <weaver options="-Xnoinline"> + <dump within="ataspectj.DumpTestThe*" beforeandafter="true"/> + </weaver> +</aspectj> diff --git a/tests/java5/ataspectj/ataspectj/aop-dumpnone.xml b/tests/java5/ataspectj/ataspectj/aop-dumpnone.xml new file mode 100644 index 000000000..425f9ad23 --- /dev/null +++ b/tests/java5/ataspectj/ataspectj/aop-dumpnone.xml @@ -0,0 +1,5 @@ +<?xml version="1.0"?> +<aspectj> + <weaver options="-XmessageHandlerClass:ataspectj.TestHelper"> + </weaver> +</aspectj> diff --git a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java index 27d485d46..5db988b07 100644 --- a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java +++ b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java @@ -12,6 +12,8 @@ package org.aspectj.systemtest.ajc150.ataspectj; import org.aspectj.testing.XMLBasedAjcTestCase; +import org.aspectj.util.FileUtil; + import junit.framework.Test; import java.io.File; @@ -65,9 +67,66 @@ public class AtAjLTWTests extends XMLBasedAjcTestCase { runTest("AjcLTW AroundInlineMungerTest2"); } + public void testLTWDumpNone() { + runTest("LTW DumpTest none"); + + File f = new File("_ajdump/ataspectj/DumpTest.class"); + assertFalse(f.exists()); + f = new File("_ajdump/_before/ataspectj/DumpTestTheDump.class"); + assertFalse(f.exists()); + f = new File("_ajdump/ataspectj/DumpTestTheDump.class"); + assertFalse(f.exists()); + } + public void testLTWDump() { runTest("LTW DumpTest"); - } + + File f = new File("_ajdump/ataspectj/DumpTest.class"); + assertFalse(f.exists()); + f = new File("_ajdump/_before/ataspectj/DumpTestTheDump.class"); + assertFalse(f.exists()); + f = new File("_ajdump/ataspectj/DumpTestTheDump.class"); + assertTrue(f.exists()); + + // tidy up... + f = new File("_ajdump"); + FileUtil.deleteContents(f); + f.delete(); + } + + public void testLTWDumpBeforeAndAfter() { + runTest("LTW DumpTest before and after"); + + File f = new File("_ajdump/ataspectj/DumpTest.class"); + assertFalse(f.exists()); + f = new File("_ajdump/_before/ataspectj/DumpTestTheDump.class"); + assertTrue(f.exists()); + f = new File("_ajdump/ataspectj/DumpTestTheDump.class"); + assertTrue(f.exists()); + + // tidy up... + f = new File("_ajdump"); + FileUtil.deleteContents(f); + f.delete(); + } + + /* FIXME maw currently can't dump closures because the logic in + * ClassLoaderWeavingAdaptor.shouldDump() relies on the World being + * able to resolve the name which it can't for closures. + */ +// public void testLTWDumpClosure() { +// runTest("LTW DumpTest closure"); +// +// File f = new File("_ajdump/_before/ataspectj/DumpTestTheDump$AjcClosure1.class"); +// assertTrue(f.exists()); +// f = new File("_ajdump/ataspectj/DumpTestTheDump$AjcClosure1.class"); +// assertTrue(f.exists()); +// +// // tidy up... +// f = new File("_ajdump"); +// FileUtil.deleteContents(f); +// f.delete(); +// } public void testAjcAspect1LTWAspect2_Xreweavable() { runTest("Ajc Aspect1 LTW Aspect2 -Xreweavable"); diff --git a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml index c3d0045d8..1510598fb 100644 --- a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml +++ b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml @@ -79,11 +79,32 @@ <ant file="ajc-ant.xml" target="ltw.AroundInlineMungerTest2" verbose="true"/> </ajc-test> + <ajc-test dir="java5/ataspectj" title="LTW DumpTest none"> + <compile + files="ataspectj/DumpTest.java,ataspectj/DumpTestTheDump.java,ataspectj/TestHelper.java" + options="-1.5"/> + <run class="ataspectj.DumpTest" ltw="ataspectj/aop-dumpnone.xml"/> + </ajc-test> + <ajc-test dir="java5/ataspectj" title="LTW DumpTest"> <compile files="ataspectj/DumpTest.java,ataspectj/DumpTestTheDump.java,ataspectj/TestHelper.java" options="-1.5"/> - <ant file="ajc-ant.xml" target="ltw.DumpTest" verbose="true"/> + <run class="ataspectj.DumpTest" ltw="ataspectj/aop-dump.xml"/> + </ajc-test> + + <ajc-test dir="java5/ataspectj" title="LTW DumpTest before and after"> + <compile + files="ataspectj/DumpTest.java,ataspectj/DumpTestTheDump.java,ataspectj/TestHelper.java" + options="-1.5"/> + <run class="ataspectj.DumpTest" ltw="ataspectj/aop-dumpbeforeandafter.xml"/> + </ajc-test> + + <ajc-test dir="java5/ataspectj" title="LTW DumpTest closure"> + <compile + files="ataspectj/DumpTest.java,ataspectj/DumpTestTheDump.java,ataspectj/TestAroundAspect.aj" + options="-1.5"/> + <run class="ataspectj.DumpTest" ltw="ataspectj/aop-dumpclosure.xml"/> </ajc-test> <ajc-test dir="java5/ataspectj" title="Ajc Aspect1 LTW Aspect2 -Xreweavable"> diff --git a/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java b/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java index f70215328..398a0b4f3 100644 --- a/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java +++ b/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java @@ -14,6 +14,7 @@ package org.aspectj.weaver.tools; import java.io.File; +import java.io.FileOutputStream; import java.io.IOException; import java.io.PrintWriter; import java.net.URL; @@ -187,6 +188,7 @@ public class WeavingAdaptor { */ public byte[] weaveClass (String name, byte[] bytes) throws IOException { if (enabled) { + if (shouldWeave(name, bytes)) { info("weaving '" + name + "'"); bytes = getWovenBytes(name, bytes); @@ -220,7 +222,7 @@ public class WeavingAdaptor { return true; } - public boolean shouldDump(String name) { + protected boolean shouldDump(String name, boolean before) { return false; } @@ -338,6 +340,37 @@ public class WeavingAdaptor { } /** + * Dump the given bytcode in _dump/... (dev mode) + * + * @param name + * @param b + * @param before whether we are dumping before weaving + * @throws Throwable + */ + protected void dump(String name, byte[] b, boolean before) { + String dirName = "_ajdump"; + if (before) dirName = dirName + File.separator + "_before"; + + String className = name.replace('.', '/'); + final File dir; + if (className.indexOf('/') > 0) { + dir = new File(dirName + File.separator + className.substring(0, className.lastIndexOf('/'))); + } else { + dir = new File(dirName); + } + dir.mkdirs(); + String fileName = dirName + File.separator + className + ".class"; + try { + FileOutputStream os = new FileOutputStream(fileName); + os.write(b); + os.close(); + } + catch (IOException ex) { + warn("unable to dump class " + name + " in directory " + dirName,ex); + } + } + + /** * Processes messages arising from weaver operations. * Tell weaver to abort on any message more severe than warning. */ @@ -396,6 +429,11 @@ public class WeavingAdaptor { public WeavingClassFileProvider (String name, byte[] bytes) { this.unwovenClass = new UnwovenClassFile(name,bytes); this.unwovenClasses.add(unwovenClass); + + if (shouldDump(name.replace('/', '.'),true)) { + dump(name, bytes, true); + } + bcelWorld.addSourceObjectType(unwovenClass.getJavaClass()); } @@ -422,6 +460,11 @@ public class WeavingAdaptor { public void acceptResult(UnwovenClassFile result) { if (wovenClass == null) { wovenClass = result; + + String name = result.getClassName(); + if (shouldDump(name.replace('/', '.'), false)) { + dump(name, result.getBytes(), false); + } } /* Classes generated by weaver e.g. around closure advice */ |