From 808a511b8f00f1ea3785796410b5f7c273e7af8e Mon Sep 17 00:00:00 2001 From: avasseur Date: Tue, 25 Oct 2005 15:36:26 +0000 Subject: [PATCH] fix for #113587: when aop.xml include/exclude is used, javac @Aspect are not passed thru the aspectof munger. Added custom logic for that --- .../context/CompilationAndWeavingContext.java | 12 +-- .../loadtime/ConcreteAspectCodeGen.java | 2 +- .../ajdt/internal/compiler/WeaverAdapter.java | 8 +- .../AspectOfWhenAspectNotInIncludeTest.java | 86 +++++++++++++++++++ ...aop-aspectofwhenaspectnotinincludetest.xml | 9 ++ .../ajc150/ataspectj/AtAjLTWTests.java | 5 ++ .../systemtest/ajc150/ataspectj/ltw.xml | 6 ++ .../aspectj/weaver/IClassFileProvider.java | 7 ++ .../weaver/bcel/BcelPerClauseAspectAdder.java | 46 ++++++++-- .../org/aspectj/weaver/bcel/BcelWeaver.java | 38 +++++++- .../aspectj/weaver/tools/WeavingAdaptor.java | 71 ++++++++++----- 11 files changed, 251 insertions(+), 39 deletions(-) create mode 100644 tests/java5/ataspectj/ataspectj/bugs/AspectOfWhenAspectNotInIncludeTest.java create mode 100644 tests/java5/ataspectj/ataspectj/bugs/aop-aspectofwhenaspectnotinincludetest.xml diff --git a/bridge/src/org/aspectj/bridge/context/CompilationAndWeavingContext.java b/bridge/src/org/aspectj/bridge/context/CompilationAndWeavingContext.java index f99146864..66da84a1b 100644 --- a/bridge/src/org/aspectj/bridge/context/CompilationAndWeavingContext.java +++ b/bridge/src/org/aspectj/bridge/context/CompilationAndWeavingContext.java @@ -73,9 +73,10 @@ public class CompilationAndWeavingContext { public static final int IMPLEMENTING_ON_SHADOW = 29; public static final int MATCHING_POINTCUT = 30; public static final int MUNGING_WITH = 31; - - - // phase names + public static final int PROCESSING_ATASPECTJTYPE_MUNGERS_ONLY = 32; + + + // phase names public static final String[] PHASE_NAMES = new String[] { "batch building", "incrementally building", @@ -111,8 +112,9 @@ public class CompilationAndWeavingContext { "matching shadow", "implementing on shadow", "matching pointcut", - "type munging with" - }; + "type munging with", + "type munging for @AspectJ aspectOf" + }; // context stacks, one per thread private static Map contextMap = new HashMap(); diff --git a/loadtime/src/org/aspectj/weaver/loadtime/ConcreteAspectCodeGen.java b/loadtime/src/org/aspectj/weaver/loadtime/ConcreteAspectCodeGen.java index 85b718ac5..fb30edd6e 100644 --- a/loadtime/src/org/aspectj/weaver/loadtime/ConcreteAspectCodeGen.java +++ b/loadtime/src/org/aspectj/weaver/loadtime/ConcreteAspectCodeGen.java @@ -299,7 +299,7 @@ public class ConcreteAspectCodeGen { ResolvedType.forName(m_concreteAspect.name).resolve(m_world), m_perClause.getKind() ); - perClauseMunger.forceMunge(cg); + perClauseMunger.forceMunge(cg, false); //TODO AV - unsafe cast // register the fresh new class into the world repository as it does not exist on the classpath anywhere diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/WeaverAdapter.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/WeaverAdapter.java index 0511929fa..36f926360 100644 --- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/WeaverAdapter.java +++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/WeaverAdapter.java @@ -71,8 +71,12 @@ public class WeaverAdapter implements IClassFileProvider, IWeaveRequestor, Itera public IWeaveRequestor getRequestor() { return this; } - - // Iteration + + public boolean isApplyAtAspectJMungersOnly() { + return false; + } + + // Iteration // ================================================================ /* (non-Javadoc) diff --git a/tests/java5/ataspectj/ataspectj/bugs/AspectOfWhenAspectNotInIncludeTest.java b/tests/java5/ataspectj/ataspectj/bugs/AspectOfWhenAspectNotInIncludeTest.java new file mode 100644 index 000000000..5d4437eef --- /dev/null +++ b/tests/java5/ataspectj/ataspectj/bugs/AspectOfWhenAspectNotInIncludeTest.java @@ -0,0 +1,86 @@ +/******************************************************************************* + * 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: + * Alexandre Vasseur initial implementation + *******************************************************************************/ +package ataspectj.bugs; + +import junit.framework.TestCase; +import junit.framework.Test; +import junit.framework.TestSuite; +import ataspectj.TestHelper; +import org.aspectj.lang.annotation.Before; +import org.aspectj.lang.annotation.Aspect; +import org.aspectj.lang.annotation.DeclareImplements; +import org.aspectj.lang.Aspects; + +import java.io.Serializable; +import java.lang.reflect.Method; + +/** + * @author Alexandre Vasseur + */ +public class AspectOfWhenAspectNotInIncludeTest extends TestCase { + + static int I; + + void target() { + I++; + } + + @Aspect + static class TestAspectForAspect { + @DeclareImplements("ataspectj.bugs.AspectOfWhenAspectNotInIncludeTest.TestAspect") + Serializable shouldNotHappenDueToInclude; + } + + @Aspect("perthis(execution(* ataspectj.bugs.AspectOfWhenAspectNotInIncludeTest.target()))") + static class TestAspect { + + public TestAspect() { + int i = 0; + i++; + } + + @Before("execution(* ataspectj.bugs.AspectOfWhenAspectNotInIncludeTest.target())") + public void before() { + I++; + } + } + + public void testInclude() { + I = 0; + target(); + assertEquals(1, I);//aspect not applied as per aop.xml include + } + + public void testAspectOf() { + // aspectOf method has been added to the aspect, no matter the aop.xml include/exclude + try { + Method m = TestAspect.class.getDeclaredMethod("aspectOf", new Class[]{Object.class}); + } catch (Throwable t) { + fail(t.toString()); + } + } + + public void testAspectUntouched() { + if (TestAspect.class.getInterfaces().length > 0) { + fail("Aspect was touched by some aspect while NOT in aop.xml include"); + } + } + + public static void main(String[] args) { + TestHelper.runAndThrowOnFailure(suite()); + } + + public static Test suite() { + return new TestSuite(AspectOfWhenAspectNotInIncludeTest.class); + } + +} diff --git a/tests/java5/ataspectj/ataspectj/bugs/aop-aspectofwhenaspectnotinincludetest.xml b/tests/java5/ataspectj/ataspectj/bugs/aop-aspectofwhenaspectnotinincludetest.xml new file mode 100644 index 000000000..d45adb36a --- /dev/null +++ b/tests/java5/ataspectj/ataspectj/bugs/aop-aspectofwhenaspectnotinincludetest.xml @@ -0,0 +1,9 @@ + + + + + + + + + \ No newline at end of file diff --git a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java index d0a88a27e..49e9e7850 100644 --- a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java +++ b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java @@ -101,4 +101,9 @@ public class AtAjLTWTests extends XMLBasedAjcTestCase { public void testConcreteAspect() { runTest("ConcreteAspect"); } + + public void testAspectOfWhenAspectNotInInclude() { + runTest("AspectOfWhenAspectNotInInclude"); + } + } diff --git a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml index d3bf28f77..257d56805 100644 --- a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml +++ b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml @@ -147,4 +147,10 @@ + + + + \ No newline at end of file diff --git a/weaver/src/org/aspectj/weaver/IClassFileProvider.java b/weaver/src/org/aspectj/weaver/IClassFileProvider.java index 73700e0bb..e0074c3d8 100644 --- a/weaver/src/org/aspectj/weaver/IClassFileProvider.java +++ b/weaver/src/org/aspectj/weaver/IClassFileProvider.java @@ -33,4 +33,11 @@ public interface IClassFileProvider { * The client to which the woven results should be returned. */ IWeaveRequestor getRequestor(); + + /** + * @return true if weaver should only do some internal munging as the one needed + * for @AspectJ aspectOf methods creation + */ + boolean isApplyAtAspectJMungersOnly(); + } diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelPerClauseAspectAdder.java b/weaver/src/org/aspectj/weaver/bcel/BcelPerClauseAspectAdder.java index 79427eb4c..e1ba9529e 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelPerClauseAspectAdder.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelPerClauseAspectAdder.java @@ -56,6 +56,22 @@ public class BcelPerClauseAspectAdder extends BcelTypeMunger { public boolean munge(BcelClassWeaver weaver) { LazyClassGen gen = weaver.getLazyClassGen(); + doAggressiveInner(gen); + + // Only munge the aspect type + if (!gen.getType().equals(aspectType)) { + return false; + } + + return doMunge(gen, true); + } + + public boolean forceMunge(LazyClassGen gen, boolean checkAlreadyThere) { + doAggressiveInner(gen); + return doMunge(gen, checkAlreadyThere); + } + + private void doAggressiveInner(LazyClassGen gen) { // agressively generate the inner interface if any // Note: we do so because of the bug #75442 that leads to have this interface implemented by all classes and not // only those matched by the per clause, which fails under LTW since the very first class @@ -79,16 +95,13 @@ public class BcelPerClauseAspectAdder extends BcelTypeMunger { } hasGeneratedInner = true; } - - // Only munge the aspect type - if (!gen.getType().equals(aspectType)) { - return false; - } - - return forceMunge(gen); } - public boolean forceMunge(LazyClassGen gen) { + private boolean doMunge(LazyClassGen gen, boolean checkAlreadyThere) { + if (checkAlreadyThere && hasPerClauseMembersAlready(gen)) { + return false; + } + generatePerClauseMembers(gen); if (kind == PerClause.SINGLETON) { @@ -132,6 +145,23 @@ public class BcelPerClauseAspectAdder extends BcelTypeMunger { //return aspectType.equals(onType); } + private boolean hasPerClauseMembersAlready(LazyClassGen classGen) { + ResolvedMember[] methods = classGen.getBcelObjectType().getDeclaredMethods(); + for (int i = 0; i < methods.length; i++) { + ResolvedMember method = methods[i]; + if ("aspectOf".equals(method.getName())) { + if ("()".equals(method.getParameterSignature()) && (kind == PerClause.SINGLETON || kind == PerClause.PERCFLOW)) { + return true; + } else if ("(Ljava/lang/Object;)".equals(method.getParameterSignature()) && kind == PerClause.PEROBJECT) { + return true; + } else if ("(Ljava/lang/Class;)".equals(method.getParameterSignature()) && kind == PerClause.PERTYPEWITHIN) { + return true; + } + } + } + return false; + } + private void generatePerClauseMembers(LazyClassGen classGen) { //FIXME Alex handle when field already there - or handle it with / similar to isAnnotationDefinedAspect() // for that use aspectType and iterate on the fields. diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java index 6ab999ec3..fc35718cb 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java @@ -858,7 +858,11 @@ public class BcelWeaver implements IWeaver { prepareForWeave(); Collection c = weave( new IClassFileProvider() { - public Iterator getClassFileIterator() { + public boolean isApplyAtAspectJMungersOnly() { + return false; + } + + public Iterator getClassFileIterator() { return addedClasses.iterator(); } @@ -961,7 +965,37 @@ public class BcelWeaver implements IWeaver { Collection wovenClassNames = new ArrayList(); IWeaveRequestor requestor = input.getRequestor(); - requestor.processingReweavableState(); + // special case for AtAspectJMungerOnly - see #113587 + if (input.isApplyAtAspectJMungersOnly()) { + ContextToken atAspectJMungersOnly = CompilationAndWeavingContext.enteringPhase(CompilationAndWeavingContext.PROCESSING_ATASPECTJTYPE_MUNGERS_ONLY, ""); + requestor.weavingAspects(); + ContextToken aspectToken = CompilationAndWeavingContext.enteringPhase(CompilationAndWeavingContext.WEAVING_ASPECTS, ""); + for (Iterator i = input.getClassFileIterator(); i.hasNext(); ) { + UnwovenClassFile classFile = (UnwovenClassFile)i.next(); + String className = classFile.getClassName(); + ResolvedType theType = world.resolve(className); + if (theType.isAnnotationStyleAspect()) { + BcelObjectType classType = BcelWorld.getBcelObjectType(theType); + if (classType==null) { + throw new BCException("Can't find bcel delegate for "+className+" type="+theType.getClass()); + } + LazyClassGen clazz = classType.getLazyClassGen(); + BcelPerClauseAspectAdder selfMunger = new BcelPerClauseAspectAdder(theType, theType.getPerClause().getKind()); + selfMunger.forceMunge(clazz, true); + classType.finishedWith(); + UnwovenClassFile[] newClasses = getClassFilesFor(clazz); + for (int news = 0; news < newClasses.length; news++) { + requestor.acceptResult(newClasses[news]); + } + wovenClassNames.add(classFile.getClassName()); + } + } + requestor.weaveCompleted(); + CompilationAndWeavingContext.leavingPhase(atAspectJMungersOnly); + return wovenClassNames; + } + + requestor.processingReweavableState(); ContextToken reweaveToken = CompilationAndWeavingContext.enteringPhase(CompilationAndWeavingContext.PROCESSING_REWEAVABLE_STATE,""); prepareToProcessReweavableState(); // clear all state from files we'll be reweaving diff --git a/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java b/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java index d0d46db81..c98773e3b 100644 --- a/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java +++ b/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java @@ -173,14 +173,28 @@ public class WeavingAdaptor { //System.out.println("WeavingAdaptor.weaveClass " + name); info("weaving '" + name + "'"); bytes = getWovenBytes(name, bytes); - } - return bytes; + } else if (shouldWeaveAtAspect(name)) { + // an @AspectJ aspect needs to be at least munged by the aspectOf munger + info("weaving '" + name + "'"); + bytes = getAtAspectJAspectBytes(name, bytes); + } + + return bytes; } - private boolean shouldWeave (String name) { + /** + * @param name + * @return true if should weave (but maybe we still need to munge it for @AspectJ aspectof support) + */ + private boolean shouldWeave (String name) { name = name.replace('/','.'); - boolean b = (enabled && !generatedClasses.containsKey(name) && shouldWeaveName(name) && shouldWeaveAspect(name)); - return b && accept(name); + boolean b = enabled && !generatedClasses.containsKey(name) && shouldWeaveName(name); + return b && accept(name); +// && shouldWeaveAtAspect(name); +// // we recall shouldWeaveAtAspect as we need to add aspectOf methods for @Aspect anyway +// //FIXME AV - this is half ok as the aspect will be weaved by others. In theory if the aspect +// // is excluded from include/exclude config we should only weave late type mungers for aspectof +// return b && (accept(name) || shouldWeaveAtAspect(name)); } //ATAJ @@ -206,11 +220,11 @@ public class WeavingAdaptor { * (and not part of the source compilation) * * @param name - * @return + * @return true if @Aspect */ - private boolean shouldWeaveAspect(String name) { + private boolean shouldWeaveAtAspect(String name) { ResolvedType type = bcelWorld.resolve(UnresolvedType.forName(name), true); - return (type == null || !type.isAspect() || type.isAnnotationStyleAspect()); + return (type == null || type.isAnnotationStyleAspect()); } /** @@ -224,18 +238,24 @@ public class WeavingAdaptor { WeavingClassFileProvider wcp = new WeavingClassFileProvider(name,bytes); weaver.weave(wcp); return wcp.getBytes(); - -// UnwovenClassFile unwoven = new UnwovenClassFile(name,bytes); -// -// // weave -// BcelObjectType bcelType = bcelWorld.addSourceObjectType(unwoven.getJavaClass()); -// LazyClassGen woven = weaver.weaveWithoutDump(unwoven,bcelType); -// -// byte[] wovenBytes = woven != null ? woven.getJavaClass(bcelWorld).getBytes() : bytes; -// return wovenBytes; } - - private void registerAspectLibraries(List aspectPath) { + + /** + * Weave a set of bytes defining a class for only what is needed to turn @AspectJ aspect + * in a usefull form ie with aspectOf method - see #113587 + * @param name the name of the class being woven + * @param bytes the bytes that define the class + * @return byte[] the woven bytes for the class + * @throws IOException + */ + private byte[] getAtAspectJAspectBytes(String name, byte[] bytes) throws IOException { + WeavingClassFileProvider wcp = new WeavingClassFileProvider(name,bytes); + wcp.setApplyAtAspectJMungersOnly(); + weaver.weave(wcp); + return wcp.getBytes(); + } + + private void registerAspectLibraries(List aspectPath) { // System.err.println("? WeavingAdaptor.registerAspectLibraries(" + aspectPath + ")"); for (Iterator i = aspectPath.iterator(); i.hasNext();) { String libName = (String)i.next(); @@ -346,14 +366,23 @@ public class WeavingAdaptor { private List unwovenClasses = new ArrayList(); /* List */ private UnwovenClassFile wovenClass; + private boolean isApplyAtAspectJMungersOnly = false; - public WeavingClassFileProvider (String name, byte[] bytes) { + public WeavingClassFileProvider (String name, byte[] bytes) { UnwovenClassFile unwoven = new UnwovenClassFile(name,bytes); unwovenClasses.add(unwoven); bcelWorld.addSourceObjectType(unwoven.getJavaClass()); } - public byte[] getBytes () { + public void setApplyAtAspectJMungersOnly() { + isApplyAtAspectJMungersOnly = true; + } + + public boolean isApplyAtAspectJMungersOnly() { + return isApplyAtAspectJMungersOnly; + } + + public byte[] getBytes () { return wovenClass.getBytes(); } -- 2.39.5