From: aclement Date: Fri, 11 Nov 2005 13:51:57 +0000 (+0000) Subject: Matthews patches from 95529 X-Git-Tag: V1_5_0RC1~200 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=3226d27a82e968da0f04e6b4fb5a29a03fbdb0f5;p=aspectj.git Matthews patches from 95529 --- diff --git a/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java b/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java index dca1baab9..3ca6d69cf 100644 --- a/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java +++ b/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java @@ -66,7 +66,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor { private IWeavingContext weavingContext; public ClassLoaderWeavingAdaptor(final ClassLoader loader, IWeavingContext wContext) { - super(null); + this.weavingContext = wContext; } void initialize(final ClassLoader loader, IWeavingContext wContext) { @@ -132,7 +132,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor { */ private void registerDefinitions(final BcelWeaver weaver, final ClassLoader loader) { try { - MessageUtil.info(messageHandler, "register classloader " + ((loader!=null)?loader.getClass().getName()+"@"+loader.hashCode():"null")); + MessageUtil.info(messageHandler, "register classloader " + getClassLoaderName(loader)); //TODO av underoptimized: we will parse each XML once per CL that see it List definitions = new ArrayList(); @@ -172,6 +172,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor { registerDump(weaver, loader, definitions); } else { enabled = false;// will allow very fast skip in shouldWeave() + info("no configuration found. Disabling weaver for class loader " + getClassLoaderName(loader)); } } catch (Exception e) { weaver.getWorld().getMessageHandler().handleMessage( @@ -180,6 +181,10 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor { } } + private String getClassLoaderName (ClassLoader loader) { + return weavingContext.getClassLoaderName(); + } + /** * Configure the weaver according to the option directives * TODO av - don't know if it is that good to reuse, since we only allow a small subset of options in LTW @@ -195,7 +200,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor { allOptions.append(definition.getWeaverOptions()).append(' '); } - Options.WeaverOption weaverOption = Options.parse(allOptions.toString(), loader); + Options.WeaverOption weaverOption = Options.parse(allOptions.toString(), loader, messageHandler); // configure the weaver and world // AV - code duplicates AspectJBuilder.initWorldAndWeaver() @@ -297,13 +302,9 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor { for (Iterator aspects = definition.getAspectClassNames().iterator(); aspects.hasNext();) { String aspectClassName = (String) aspects.next(); if (acceptAspect(aspectClassName)) { + info("register aspect " + aspectClassName); ResolvedType aspect = weaver.addLibraryAspect(aspectClassName); - if (aspect.isAbstract()) { - // this is a warning - weaver.getWorld().getMessageHandler().handleMessage( - new Message("Abstract aspect registered in aop.xml, use a element instead", IMessage.WARNING, null, null) - ); - } + //generate key for SC if(namespace==null){ namespace=new StringBuffer(aspectClassName); @@ -323,9 +324,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor { if (acceptAspect(concreteAspect.name)) { ConcreteAspectCodeGen gen = new ConcreteAspectCodeGen(concreteAspect, weaver.getWorld()); if (!gen.validate()) { - weaver.getWorld().getMessageHandler().handleMessage( - new Message("Concrete-aspect '"+concreteAspect.name+"' could not be registered", IMessage.ERROR, null, null) - ); + error("Concrete-aspect '"+concreteAspect.name+"' could not be registered"); break; } this.generatedClassHandler.acceptClass( diff --git a/loadtime/src/org/aspectj/weaver/loadtime/DefaultWeavingContext.java b/loadtime/src/org/aspectj/weaver/loadtime/DefaultWeavingContext.java index 7110e0c54..092eddb7e 100644 --- a/loadtime/src/org/aspectj/weaver/loadtime/DefaultWeavingContext.java +++ b/loadtime/src/org/aspectj/weaver/loadtime/DefaultWeavingContext.java @@ -22,7 +22,7 @@ import java.util.Enumeration; */ public class DefaultWeavingContext implements IWeavingContext { - private ClassLoader loader; + protected ClassLoader loader; public DefaultWeavingContext(){ loader = getClass().getClassLoader(); @@ -51,4 +51,11 @@ public class DefaultWeavingContext implements IWeavingContext { return null; } + /** + * @return classname@hashcode + */ + public String getClassLoaderName() { + return ((loader!=null)?loader.getClass().getName()+"@"+loader.hashCode():"null"); + } + } diff --git a/loadtime/src/org/aspectj/weaver/loadtime/IWeavingContext.java b/loadtime/src/org/aspectj/weaver/loadtime/IWeavingContext.java index 285fc81dc..f634cabc0 100644 --- a/loadtime/src/org/aspectj/weaver/loadtime/IWeavingContext.java +++ b/loadtime/src/org/aspectj/weaver/loadtime/IWeavingContext.java @@ -41,5 +41,12 @@ public interface IWeavingContext { * @return */ public String getBundleIdFromURL(URL url); + + /** + * In an environment with multiple class loaders allows each to be + * identified using something safer and than toString + * @return name of the associated class loader + */ + public String getClassLoaderName (); } diff --git a/loadtime/src/org/aspectj/weaver/loadtime/Options.java b/loadtime/src/org/aspectj/weaver/loadtime/Options.java index 5bc469470..d96f45cb1 100644 --- a/loadtime/src/org/aspectj/weaver/loadtime/Options.java +++ b/loadtime/src/org/aspectj/weaver/loadtime/Options.java @@ -45,16 +45,16 @@ public class Options { private static final String OPTIONVALUED_Xlint = "-Xlint:"; - public static WeaverOption parse(String options, ClassLoader laoder) { + public static WeaverOption parse(String options, ClassLoader laoder, IMessageHandler imh) { + WeaverOption weaverOption = new WeaverOption(imh); + if (LangUtil.isEmpty(options)) { - return new WeaverOption(); + return weaverOption; } // the first option wins List flags = LangUtil.anySplit(options, " "); Collections.reverse(flags); - WeaverOption weaverOption = new WeaverOption(); - // do a first round on the message handler since it will report the options themselves for (Iterator iterator = flags.iterator(); iterator.hasNext();) { String arg = (String) iterator.next(); @@ -152,8 +152,9 @@ public class Options { String lint; String lintFile; - public WeaverOption() { - messageHandler = new DefaultMessageHandler();//default + public WeaverOption(IMessageHandler imh) { +// messageHandler = new DefaultMessageHandler();//default + this.messageHandler = imh; } } } diff --git a/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java b/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java index 8c7035b5e..4fe476eb1 100644 --- a/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java +++ b/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java @@ -34,7 +34,8 @@ public class WeavingURLClassLoader extends ExtensibleURLClassLoader implements W public static final String WEAVING_ASPECT_PATH = "aj.aspect.path"; private URL[] aspectURLs; - private WeavingAdaptor adaptor; + private WeavingAdaptor adaptor; + private boolean initializingAdaptor; private Map generatedClasses = new HashMap(); /* String -> byte[] */ /* @@ -99,15 +100,30 @@ public class WeavingURLClassLoader extends ExtensibleURLClassLoader implements W */ protected Class defineClass(String name, byte[] b, CodeSource cs) throws IOException { // System.err.println("? WeavingURLClassLoader.defineClass(" + name + ", [" + b.length + "])"); - - /* Need to defer creation because of possible recursion during constructor execution */ - if (adaptor == null) { - ClassLoaderWeavingAdaptor clwAdaptor = new ClassLoaderWeavingAdaptor(this,null); - clwAdaptor.initialize(this,null); - adaptor = clwAdaptor; + + /* Avoid recursion during adaptor initialization */ + if (!initializingAdaptor) { + + /* Need to defer creation because of possible recursion during constructor execution */ + if (adaptor == null && !initializingAdaptor) { + DefaultWeavingContext weavingContext = new DefaultWeavingContext (this) { + + /* Ensures consistent LTW messages for testing */ + public String getClassLoaderName() { + return loader.getClass().getName(); + } + + }; + + ClassLoaderWeavingAdaptor clwAdaptor = new ClassLoaderWeavingAdaptor(this,weavingContext); + initializingAdaptor = true; + clwAdaptor.initialize(this,weavingContext); + initializingAdaptor = false; + adaptor = clwAdaptor; + } + + b = adaptor.weaveClass(name,b); } - - b = adaptor.weaveClass(name,b); return super.defineClass(name, b, cs); } diff --git a/loadtime/testsrc/org/aspectj/weaver/loadtime/WeavingURLClassLoaderTest.java b/loadtime/testsrc/org/aspectj/weaver/loadtime/WeavingURLClassLoaderTest.java index fe3139314..093a0125a 100644 --- a/loadtime/testsrc/org/aspectj/weaver/loadtime/WeavingURLClassLoaderTest.java +++ b/loadtime/testsrc/org/aspectj/weaver/loadtime/WeavingURLClassLoaderTest.java @@ -69,6 +69,11 @@ public class WeavingURLClassLoaderTest extends TestCase { } } + /* + * We won't get an exception because the aspect path is empty and there is + * no aop.xml file so the weaver will be disabled and no reweaving will + * take place + */ public void testLoadWovenClass () { setSystemProperty(WeavingURLClassLoader.WEAVING_ASPECT_PATH,""); setSystemProperty(WeavingURLClassLoader.WEAVING_CLASS_PATH,WOVEN_JAR); @@ -83,6 +88,9 @@ public class WeavingURLClassLoaderTest extends TestCase { } } + /* + * We get an exception because the class was not built reweavable + */ public void testWeaveWovenClass () { setSystemProperty(WeavingURLClassLoader.WEAVING_ASPECT_PATH,ADVICE_ASPECTS); setSystemProperty(WeavingURLClassLoader.WEAVING_CLASS_PATH,ADVICE_ASPECTS + File.pathSeparator + WOVEN_JAR); @@ -345,6 +353,7 @@ public class WeavingURLClassLoaderTest extends TestCase { fail("Expecting java.lang.NoClassDefFoundError"); } catch (Exception ex) { + assertTrue("Expecting java.lang.NoClassDefFoundError but caught " + ex,ex.getMessage().contains("java.lang.NoClassDefFoundError")); } } @@ -463,7 +472,9 @@ public class WeavingURLClassLoaderTest extends TestCase { method.invoke(null,params); } catch (InvocationTargetException ex) { - throw new RuntimeException(ex.getTargetException().toString()); + Throwable targetException = ex.getTargetException(); + if (targetException instanceof RuntimeException) throw (RuntimeException)ex.getTargetException(); + else throw new RuntimeException(ex.getTargetException().toString()); } catch (Exception ex) { throw new RuntimeException(ex.toString()); diff --git a/org.aspectj.ajdt.core/testsrc/org/aspectj/tools/ajc/AjcTestCase.java b/org.aspectj.ajdt.core/testsrc/org/aspectj/tools/ajc/AjcTestCase.java index a6b109301..961079f85 100644 --- a/org.aspectj.ajdt.core/testsrc/org/aspectj/tools/ajc/AjcTestCase.java +++ b/org.aspectj.ajdt.core/testsrc/org/aspectj/tools/ajc/AjcTestCase.java @@ -578,7 +578,8 @@ public class AjcTestCase extends TestCase { URLClassLoader cLoader; if (useLTW) { - cLoader = new WeavingURLClassLoader(urls,null); + ClassLoader parent = getClass().getClassLoader(); + cLoader = new WeavingURLClassLoader(urls,parent); } else { cLoader = new URLClassLoader(urls,null); @@ -592,10 +593,10 @@ public class AjcTestCase extends TestCase { } catch (Exception ex) { fail ("Unable to prepare org.aspectj.testing.Tester for test run: " + ex); } - Class toRun = cLoader.loadClass(className); - Method mainMethod = toRun.getMethod("main",new Class[] {String[].class}); System.setOut(new PrintStream(baosOut)); System.setErr(new PrintStream(baosErr)); + Class toRun = cLoader.loadClass(className); + Method mainMethod = toRun.getMethod("main",new Class[] {String[].class}); mainMethod.invoke(null,new Object[] {args}); lastRunResult = new RunResult(command.toString(),new String(baosOut.toByteArray()),new String(baosErr.toByteArray())); } catch(ClassNotFoundException cnf) { diff --git a/testing/newsrc/org/aspectj/testing/RunSpec.java b/testing/newsrc/org/aspectj/testing/RunSpec.java index 891600dbf..166c2d89e 100644 --- a/testing/newsrc/org/aspectj/testing/RunSpec.java +++ b/testing/newsrc/org/aspectj/testing/RunSpec.java @@ -141,7 +141,7 @@ public class RunSpec implements ITestStep { useLtw = true; } catch (IOException ex) { - ex.printStackTrace(); + AjcTestCase.fail(ex.toString()); } } diff --git a/tests/ltw/AbstractAspect.aj b/tests/ltw/AbstractAspect.aj new file mode 100644 index 000000000..6a735f268 --- /dev/null +++ b/tests/ltw/AbstractAspect.aj @@ -0,0 +1,32 @@ +/******************************************************************************* + * 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 + *******************************************************************************/ +import java.lang.reflect.*; + +public abstract aspect AbstractAspect { + + /* + * These should not take effect unless a concrete sub-aspect is defined + */ + declare parents : TestITDMethod implements Runnable; + + declare soft : InvocationTargetException : execution(public void TestITDMethod.*()); + + declare warning : execution(public void main(..)) : + "AbstractAspect_main"; + + /* + * This should always take effect + */ + public void TestITDMethod.test () { + System.err.println("AbstractAspect_TestITDMethod.test"); + } +} diff --git a/tests/ltw/AbstractSuperAspect.aj b/tests/ltw/AbstractSuperAspect.aj new file mode 100644 index 000000000..705085848 --- /dev/null +++ b/tests/ltw/AbstractSuperAspect.aj @@ -0,0 +1,20 @@ +/******************************************************************************* + * 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 + *******************************************************************************/ + +public abstract aspect AbstractSuperAspect { + + protected abstract pointcut scope (); + + before () : execution(void test1()) && scope() { + System.err.println("AbstractSuperAspect.before_" + thisJoinPoint.getSignature().getName()); + } +} diff --git a/tests/ltw/Main.java b/tests/ltw/Main.java index fca018ac9..9e53bfb4d 100644 --- a/tests/ltw/Main.java +++ b/tests/ltw/Main.java @@ -9,6 +9,8 @@ * Contributors: * Matthew Webster initial implementation *******************************************************************************/ +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; public class Main { @@ -20,7 +22,18 @@ public class Main { System.out.println("Main.test2"); } - public static void main (String[] args) { + public void invokeDeclaredMethods () throws Exception { + Method[] methods = getClass().getDeclaredMethods(); + for (int i = 0; i < methods.length; i++) { + Method method = methods[i]; + int modifiers = method.getModifiers(); + if (!Modifier.isStatic(modifiers) && !method.getName().equals("invokeDeclaredMethods")) { + method.invoke(this,new Object[] {}); + } + } + } + + public static void main (String[] args) throws Exception { System.out.println("Main.main"); new Main().test1(); new Main().test2(); diff --git a/tests/ltw/TestITDMethod.java b/tests/ltw/TestITDMethod.java new file mode 100644 index 000000000..8f8d62268 --- /dev/null +++ b/tests/ltw/TestITDMethod.java @@ -0,0 +1,28 @@ +/******************************************************************************* + * 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 + *******************************************************************************/ +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; + +public class TestITDMethod { + + public void invokeDeclaredMethods (String[] names) throws Exception { + for (int i = 0; i < names.length; i++) { + Method method = getClass().getDeclaredMethod(names[i],new Class[] {}); + method.invoke(this,new Object[] {}); + } + } + + public static void main (String[] args) throws Exception { + System.out.println("TestITDMethod.main"); + new TestITDMethod().invokeDeclaredMethods(args); + } +} diff --git a/tests/ltw/TestMain.java b/tests/ltw/TestMain.java new file mode 100644 index 000000000..b55e94332 --- /dev/null +++ b/tests/ltw/TestMain.java @@ -0,0 +1,20 @@ +/******************************************************************************* + * 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 + *******************************************************************************/ +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; + +public class TestMain { + + public static void main (String[] args) throws Exception { + Main.main(args); + } +} diff --git a/tests/ltw/aop-abstractaspect.xml b/tests/ltw/aop-abstractaspect.xml new file mode 100644 index 000000000..abd54cc96 --- /dev/null +++ b/tests/ltw/aop-abstractaspect.xml @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/tests/ltw/aop-defineaspect.xml b/tests/ltw/aop-defineaspect.xml new file mode 100644 index 000000000..5fdc064b0 --- /dev/null +++ b/tests/ltw/aop-defineaspect.xml @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/tests/src/org/aspectj/systemtest/ajc150/ltw/LTWTests.java b/tests/src/org/aspectj/systemtest/ajc150/ltw/LTWTests.java index 3b489cf07..66eb92c68 100644 --- a/tests/src/org/aspectj/systemtest/ajc150/ltw/LTWTests.java +++ b/tests/src/org/aspectj/systemtest/ajc150/ltw/LTWTests.java @@ -12,10 +12,13 @@ package org.aspectj.systemtest.ajc150.ltw; import java.io.File; +import java.util.Enumeration; +import java.util.Properties; import junit.framework.Test; import org.aspectj.testing.XMLBasedAjcTestCase; +import org.aspectj.weaver.tools.WeavingAdaptor; public class LTWTests extends org.aspectj.testing.XMLBasedAjcTestCase { @@ -38,5 +41,57 @@ public class LTWTests extends org.aspectj.testing.XMLBasedAjcTestCase { public void testOutxmlJar (){ runTest("Ensure valid aop.xml is generated for -outjar"); } + + public void testNoAopxml(){ + setSystemProperty(WeavingAdaptor.WEAVING_ADAPTOR_VERBOSE,"true"); + runTest("Ensure no weaving without visible aop.xml"); + } + + public void testDefineConcreteAspect(){ + runTest("Define concrete sub-aspect using aop.xml"); + } + + public void testDeclareAbstractAspect(){ +// setSystemProperty(WeavingAdaptor.WEAVING_ADAPTOR_VERBOSE,"true"); +// setSystemProperty(WeavingAdaptor.SHOW_WEAVE_INFO_PROPERTY,"true"); + runTest("Use abstract aspect for ITD using aop.xml"); + } + + /* + * Allow system properties to be set and restored + * TODO maw move to XMLBasedAjcTestCase or RunSpec + */ + private final static String NULL = "null"; + + private Properties savedProperties; + + protected void setSystemProperty (String key, String value) { + Properties systemProperties = System.getProperties(); + copyProperty(key,systemProperties,savedProperties); + systemProperties.setProperty(key,value); + } + + private static void copyProperty (String key, Properties from, Properties to) { + String value = from.getProperty(key,NULL); + to.setProperty(key,value); + } + + protected void setUp() throws Exception { + super.setUp(); + savedProperties = new Properties(); + } + + protected void tearDown() throws Exception { + super.tearDown(); + + /* Restore system properties */ + Properties systemProperties = System.getProperties(); + for (Enumeration enu = savedProperties.keys(); enu.hasMoreElements(); ) { + String key = (String)enu.nextElement(); + String value = savedProperties.getProperty(key); + if (value == NULL) systemProperties.remove(key); + else systemProperties.setProperty(key,value); + } + } } diff --git a/tests/src/org/aspectj/systemtest/ajc150/ltw/ltw-tests.xml b/tests/src/org/aspectj/systemtest/ajc150/ltw/ltw-tests.xml index 34f2f3dab..6a8fe1881 100644 --- a/tests/src/org/aspectj/systemtest/ajc150/ltw/ltw-tests.xml +++ b/tests/src/org/aspectj/systemtest/ajc150/ltw/ltw-tests.xml @@ -8,7 +8,7 @@ outjar="main1.jar" options="-showWeaveInfo" > - + + + @@ -87,4 +89,73 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java b/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java index 1019ca617..b59ec5564 100644 --- a/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java +++ b/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java @@ -72,6 +72,11 @@ public class WeavingAdaptor { protected GeneratedClassHandler generatedClassHandler; protected Map generatedClasses = new HashMap(); /* String -> UnwovenClassFile */ + protected WeavingAdaptor () { + createMessageHandler(); + } + + /** * Construct a WeavingAdaptor with a reference to a weaving class loader. The * adaptor will automatically search the class loader hierarchy to resolve @@ -135,9 +140,7 @@ public class WeavingAdaptor { } private void init(List classPath, List aspectPath) { - messageHandler = new WeavingAdaptorMessageHandler(new PrintWriter(System.err)); - if (verbose) messageHandler.dontIgnore(IMessage.INFO); - if (Boolean.getBoolean(SHOW_WEAVE_INFO_PROPERTY)) messageHandler.dontIgnore(IMessage.WEAVEINFO); + createMessageHandler(); info("using classpath: " + classPath); info("using aspectpath: " + aspectPath); @@ -152,6 +155,13 @@ public class WeavingAdaptor { weaver = new BcelWeaver(bcelWorld); registerAspectLibraries(aspectPath); } + + + private void createMessageHandler() { + messageHandler = new WeavingAdaptorMessageHandler(new PrintWriter(System.err)); + if (verbose) messageHandler.dontIgnore(IMessage.INFO); + if (Boolean.getBoolean(SHOW_WEAVE_INFO_PROPERTY)) messageHandler.dontIgnore(IMessage.WEAVEINFO); + } /** * Appends URL to path used by the WeavingAdptor to resolve classes @@ -309,15 +319,15 @@ public class WeavingAdaptor { return ret; } - private boolean info (String message) { + protected boolean info (String message) { return MessageUtil.info(messageHandler,message); } - private boolean warn (String message) { + protected boolean warn (String message) { return MessageUtil.warn(messageHandler,message); } - private boolean error (String message) { + protected boolean error (String message) { return MessageUtil.error(messageHandler,message); }