From fdbae3fda3e572ea35ac4a73373896b9b5cff8f7 Mon Sep 17 00:00:00 2001 From: mwebster Date: Thu, 15 Jun 2006 16:32:39 +0000 Subject: [PATCH] Fix for 139936 "[ltw] WeavingAdaptor.generatedClassesExist() cannot cope with additional class weaving" --- .../src/org/aspectj/weaver/loadtime/Aj.java | 4 +- .../loadtime/ClassLoaderWeavingAdaptor.java | 18 +++--- .../loadtime/DefaultWeavingContext.java | 8 ++- .../weaver/loadtime/IWeavingContext.java | 6 ++ .../loadtime/WeavingURLClassLoader.java | 7 +++ .../org/aspectj/weaver/loadtime/AjTest.java | 63 +++++++++++++++++++ .../ClassLoaderWeavingAdaptorTest.java | 47 ++++++++++++++ .../weaver/loadtime/JRockitAgentTest.java | 21 ++++++- tests/ltw/EmptyAspect.aj | 3 + tests/ltw/EmptyTest1.java | 8 +++ tests/ltw/EmptyTest2.java | 7 +++ tests/ltw/aop-emptytests.xml | 8 +++ .../systemtest/ajc150/ataspectj/ltw.xml | 1 + .../systemtest/ajc152/Ajc152Tests.java | 4 ++ .../org/aspectj/systemtest/ajc152/ajc152.xml | 19 ++++++ .../aspectj/weaver/tools/WeavingAdaptor.java | 11 +++- 16 files changed, 221 insertions(+), 14 deletions(-) create mode 100644 loadtime/testsrc/org/aspectj/weaver/loadtime/AjTest.java create mode 100644 loadtime/testsrc/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptorTest.java create mode 100644 tests/ltw/EmptyAspect.aj create mode 100644 tests/ltw/EmptyTest1.java create mode 100644 tests/ltw/EmptyTest2.java create mode 100644 tests/ltw/aop-emptytests.xml diff --git a/loadtime/src/org/aspectj/weaver/loadtime/Aj.java b/loadtime/src/org/aspectj/weaver/loadtime/Aj.java index 5c8336135..b0552a040 100644 --- a/loadtime/src/org/aspectj/weaver/loadtime/Aj.java +++ b/loadtime/src/org/aspectj/weaver/loadtime/Aj.java @@ -152,11 +152,11 @@ public class Aj implements ClassPreProcessor { * @return true if classes have been generated. */ public boolean generatedClassesExist(ClassLoader loader){ - return ((ClassLoaderWeavingAdaptor)WeaverContainer.getWeaver(loader, weavingContext)).generatedClassesExist(); + return ((ClassLoaderWeavingAdaptor)WeaverContainer.getWeaver(loader, weavingContext)).generatedClassesExistFor(null); } public void flushGeneratedClasses(ClassLoader loader){ ((ClassLoaderWeavingAdaptor)WeaverContainer.getWeaver(loader, weavingContext)).flushGeneratedClasses(); } -} +} \ No newline at end of file diff --git a/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java b/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java index 6f41e6aeb..2b1eef064 100644 --- a/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java +++ b/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java @@ -166,7 +166,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor { while (xmls.hasMoreElements()) { URL xml = (URL) xmls.nextElement(); - info("using " + xml.getFile()); + info("using configuration " + weavingContext.getFile(xml)); definitions.add(DocumentParser.parse(xml)); } } @@ -332,6 +332,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor { String aspectClassName = (String) aspects.next(); if (acceptAspect(aspectClassName)) { info("register aspect " + aspectClassName); +// System.err.println("? ClassLoaderWeavingAdaptor.registerAspects() aspectName=" + aspectClassName + ", loader=" + loader + ", bundle=" + weavingContext.getClassLoaderName()); /*ResolvedType aspect = */weaver.addLibraryAspect(aspectClassName); //generate key for SC @@ -375,6 +376,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor { } } } +// System.out.println("ClassLoaderWeavingAdaptor.registerAspects() classloader=" + weavingContext.getClassLoaderName() + ", namespace=" + namespace); /* We didn't register any aspects so disable the adaptor */ if (namespace == null) { @@ -606,6 +608,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor { * @return Returns the key. */ public String getNamespace() { +// System.out.println("ClassLoaderWeavingAdaptor.getNamespace() classloader=" + weavingContext.getClassLoaderName() + ", namespace=" + namespace); if(namespace==null) return ""; else return new String(namespace); } @@ -613,19 +616,20 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor { /** * Check to see if any classes are stored in the generated classes cache. * Then flush the cache if it is not empty + * @param className TODO * @return true if a class has been generated and is stored in the cache */ - public boolean generatedClassesExist(){ - if(generatedClasses.size()>0) { - return true; - } - return false; + public boolean generatedClassesExistFor (String className) { +// System.err.println("? ClassLoaderWeavingAdaptor.generatedClassesExist() classname=" + className + ", size=" + generatedClasses); + if (className == null) return !generatedClasses.isEmpty(); + else return generatedClasses.containsKey(className); } /** * Flush the generated classes cache */ - public void flushGeneratedClasses(){ + public void flushGeneratedClasses() { +// System.err.println("? ClassLoaderWeavingAdaptor.flushGeneratedClasses() generatedClasses=" + generatedClasses); generatedClasses = new HashMap(); } diff --git a/loadtime/src/org/aspectj/weaver/loadtime/DefaultWeavingContext.java b/loadtime/src/org/aspectj/weaver/loadtime/DefaultWeavingContext.java index f54d917b0..3817ae5b0 100644 --- a/loadtime/src/org/aspectj/weaver/loadtime/DefaultWeavingContext.java +++ b/loadtime/src/org/aspectj/weaver/loadtime/DefaultWeavingContext.java @@ -44,7 +44,7 @@ public class DefaultWeavingContext implements IWeavingContext { * @return null as we are not in an OSGi environment (therefore no bundles) */ public String getBundleIdFromURL(URL url) { - return null; + return ""; } /** @@ -54,4 +54,10 @@ public class DefaultWeavingContext implements IWeavingContext { return ((loader!=null)?loader.getClass().getName()+"@"+loader.hashCode():"null"); } + /** + * @return filename + */ + public String getFile(URL url) { + return url.getFile(); + } } diff --git a/loadtime/src/org/aspectj/weaver/loadtime/IWeavingContext.java b/loadtime/src/org/aspectj/weaver/loadtime/IWeavingContext.java index f634cabc0..5b4a0c21c 100644 --- a/loadtime/src/org/aspectj/weaver/loadtime/IWeavingContext.java +++ b/loadtime/src/org/aspectj/weaver/loadtime/IWeavingContext.java @@ -49,4 +49,10 @@ public interface IWeavingContext { */ public String getClassLoaderName (); + /** + * Format a URL + * @return filename + */ + public String getFile(URL url); + } diff --git a/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java b/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java index 8cff0d992..671b8e17a 100644 --- a/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java +++ b/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java @@ -151,6 +151,13 @@ public class WeavingURLClassLoader extends ExtensibleURLClassLoader implements W public void acceptClass (String name, byte[] bytes) { generatedClasses.put(name,bytes); } + +// protected synchronized Class loadClass(String name, boolean resolve) throws ClassNotFoundException { +// System.err.println("> WeavingURLClassLoader.loadClass() name=" + name); +// Class clazz= super.loadClass(name, resolve); +// System.err.println("< WeavingURLClassLoader.loadClass() clazz=" + clazz + ", loader=" + clazz.getClassLoader()); +// return clazz; +// } // private interface ClassPreProcessorAdaptor extends ClassPreProcessor { // public void addURL(URL url); diff --git a/loadtime/testsrc/org/aspectj/weaver/loadtime/AjTest.java b/loadtime/testsrc/org/aspectj/weaver/loadtime/AjTest.java new file mode 100644 index 000000000..e55ce2ced --- /dev/null +++ b/loadtime/testsrc/org/aspectj/weaver/loadtime/AjTest.java @@ -0,0 +1,63 @@ +/******************************************************************************* + * Copyright (c) 2006 IBM Corporation and others. + * 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://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Matthew Webster - initial implementation + *******************************************************************************/ +package org.aspectj.weaver.loadtime; + +import java.net.URL; +import java.net.URLClassLoader; + +import junit.framework.TestCase; + +public class AjTest extends TestCase { + + public void testAj() { + Aj aj = new Aj(); + } + + public void testAjIWeavingContext() { + ClassLoader loader = new URLClassLoader(new URL[] {}, null); + IWeavingContext weavingContext = new DefaultWeavingContext(loader); + Aj aj = new Aj(weavingContext); + } + + public void testInitialize() { + Aj aj = new Aj(); + aj.initialize(); + } + + public void testPreProcess() { + ClassLoader loader = new URLClassLoader(new URL[] {}, null); + Aj aj = new Aj(); + aj.preProcess("Junk", new byte[] {}, loader); + } + + public void testGetNamespace() { + ClassLoader loader = new URLClassLoader(new URL[] {}, null); + Aj aj = new Aj(); + String namespace = aj.getNamespace(loader); + assertEquals("Namespace should be empty","",namespace); + } + + public void testGeneratedClassesExist() { + ClassLoader loader = new URLClassLoader(new URL[] {}, null); + Aj aj = new Aj(); + boolean exist = aj.generatedClassesExist(loader); + assertFalse("There should be no generated classes",exist); + } + + public void testFlushGeneratedClasses() { + ClassLoader loader = new URLClassLoader(new URL[] {}, null); + Aj aj = new Aj(); + aj.flushGeneratedClasses(loader); + boolean exist = aj.generatedClassesExist(loader); + assertFalse("There should be no generated classes",exist); + } + +} diff --git a/loadtime/testsrc/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptorTest.java b/loadtime/testsrc/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptorTest.java new file mode 100644 index 000000000..a2a07c89a --- /dev/null +++ b/loadtime/testsrc/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptorTest.java @@ -0,0 +1,47 @@ +/******************************************************************************* + * Copyright (c) 2006 IBM Corporation and others. + * 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://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Matthew Webster - initial implementation + *******************************************************************************/ +package org.aspectj.weaver.loadtime; + +import java.net.URL; +import java.net.URLClassLoader; + +import junit.framework.TestCase; + +public class ClassLoaderWeavingAdaptorTest extends TestCase { + + public void testClassLoaderWeavingAdaptor() { + ClassLoader loader = new URLClassLoader(new URL[] {}, null); + ClassLoaderWeavingAdaptor adaptor = new ClassLoaderWeavingAdaptor(loader,null); + } + + public void testGetNamespace() { + ClassLoader loader = new URLClassLoader(new URL[] {}, null); + ClassLoaderWeavingAdaptor adaptor = new ClassLoaderWeavingAdaptor(loader,null); + String namespace = adaptor.getNamespace(); + assertEquals("Namespace should be empty","",namespace); + } + + public void testGeneratedClassesExistFor() { + ClassLoader loader = new URLClassLoader(new URL[] {}, null); + ClassLoaderWeavingAdaptor adaptor = new ClassLoaderWeavingAdaptor(loader,null); + boolean exist = adaptor.generatedClassesExistFor("Junk"); + assertFalse("There should be no generated classes",exist); + } + + public void testFlushGeneratedClasses() { + ClassLoader loader = new URLClassLoader(new URL[] {}, null); + ClassLoaderWeavingAdaptor adaptor = new ClassLoaderWeavingAdaptor(loader,null); + adaptor.flushGeneratedClasses(); + boolean exist = adaptor.generatedClassesExistFor("Junk"); + assertFalse("There should be no generated classes",exist); + } + +} diff --git a/loadtime/testsrc/org/aspectj/weaver/loadtime/JRockitAgentTest.java b/loadtime/testsrc/org/aspectj/weaver/loadtime/JRockitAgentTest.java index a3fd2cd10..1bc41b3cc 100644 --- a/loadtime/testsrc/org/aspectj/weaver/loadtime/JRockitAgentTest.java +++ b/loadtime/testsrc/org/aspectj/weaver/loadtime/JRockitAgentTest.java @@ -82,10 +82,11 @@ public class JRockitAgentTest extends TestCase { for (int i = 0; i < urls.length; i++) { Object pathElement; URL url = urls[i]; - File file = new File(url.getFile()); + if (debug) System.out.println("JRockitClassLoader.JRockitClassLoader() url=" + url.getPath()); + File file = new File(encode(url.getFile())); if (debug) System.out.println("JRockitClassLoader.JRockitClassLoader() file" + file); if (file.isDirectory()) pathElement = file; - else if (file.getName().endsWith(".jar")) pathElement = new JarFile(file); + else if (file.exists() && file.getName().endsWith(".jar")) pathElement = new JarFile(file); else throw new RuntimeException(file.getAbsolutePath().toString()); path.add(pathElement); } @@ -99,6 +100,22 @@ public class JRockitAgentTest extends TestCase { preProcess = agentClazz.getMethod("preProcess",parameterTypes); } + /* Get rid of escaped characters */ + private String encode (String s) { + StringBuffer result = new StringBuffer(); + int i = s.indexOf("%"); + while (i != -1) { + result.append(s.substring(0,i)); + String escaped = s.substring(i+1,i+3); + s = s.substring(i+3); + Integer value = Integer.valueOf(escaped,16); + result.append(new Character((char)value.intValue())); + i = s.indexOf("%"); + } + result.append(s); + return result.toString(); + } + protected Class findClass(String name) throws ClassNotFoundException { if (debug) System.out.println("> JRockitClassLoader.findClass() name=" + name); Class clazz = null; diff --git a/tests/ltw/EmptyAspect.aj b/tests/ltw/EmptyAspect.aj new file mode 100644 index 000000000..e24bad3e8 --- /dev/null +++ b/tests/ltw/EmptyAspect.aj @@ -0,0 +1,3 @@ +public aspect EmptyAspect { + +} \ No newline at end of file diff --git a/tests/ltw/EmptyTest1.java b/tests/ltw/EmptyTest1.java new file mode 100644 index 000000000..42e579d78 --- /dev/null +++ b/tests/ltw/EmptyTest1.java @@ -0,0 +1,8 @@ +public class EmptyTest1 { + + public static void main (String[] args) { + System.out.println("EmptyTest1.main()"); + EmptyTest2.main(args); + } + +} \ No newline at end of file diff --git a/tests/ltw/EmptyTest2.java b/tests/ltw/EmptyTest2.java new file mode 100644 index 000000000..6b51489d7 --- /dev/null +++ b/tests/ltw/EmptyTest2.java @@ -0,0 +1,7 @@ +public class EmptyTest2 { + + public static void main (String[] args) { + System.out.println("EmptyTest2.main()"); + } + +} \ No newline at end of file diff --git a/tests/ltw/aop-emptytests.xml b/tests/ltw/aop-emptytests.xml new file mode 100644 index 000000000..da7bd5e2a --- /dev/null +++ b/tests/ltw/aop-emptytests.xml @@ -0,0 +1,8 @@ + + + + + + + + \ No newline at end of file diff --git a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml index 23f0bfac1..ebccda9a0 100644 --- a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml +++ b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml @@ -113,6 +113,7 @@ + diff --git a/tests/src/org/aspectj/systemtest/ajc152/Ajc152Tests.java b/tests/src/org/aspectj/systemtest/ajc152/Ajc152Tests.java index bc572fc48..3b23af93a 100644 --- a/tests/src/org/aspectj/systemtest/ajc152/Ajc152Tests.java +++ b/tests/src/org/aspectj/systemtest/ajc152/Ajc152Tests.java @@ -199,6 +199,10 @@ public class Ajc152Tests extends org.aspectj.testing.XMLBasedAjcTestCase { runTest("Ensure no weaving without included aspects"); } + public void testWeaveinfoMessages (){ + runTest("weaveinfo messages with include and exclude"); + } + // tests that can't be included for some reason // Not valid whilst the ajc compiler forces debug on (ignores -g:none) - it will be green but is invalid, trust me diff --git a/tests/src/org/aspectj/systemtest/ajc152/ajc152.xml b/tests/src/org/aspectj/systemtest/ajc152/ajc152.xml index 948741547..b91caf7ea 100644 --- a/tests/src/org/aspectj/systemtest/ajc152/ajc152.xml +++ b/tests/src/org/aspectj/systemtest/ajc152/ajc152.xml @@ -668,4 +668,23 @@ + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java b/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java index 29f6f0e37..67fb699b0 100644 --- a/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java +++ b/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java @@ -215,6 +215,9 @@ public class WeavingAdaptor { info("weaving '" + name + "'"); bytes = getAtAspectJAspectBytes(name, bytes); } + else { + info("not weaving '" + name + "'"); + } } return bytes; @@ -245,12 +248,14 @@ public class WeavingAdaptor { } private boolean shouldWeaveName (String name) { - return !((/*(name.startsWith("org.apache.bcel.")//FIXME AV why ? bcel is wrapped in org.aspectj. + boolean should = + !((/*(name.startsWith("org.apache.bcel.")//FIXME AV why ? bcel is wrapped in org.aspectj. ||*/ name.startsWith("org.aspectj.") || name.startsWith("java.") || name.startsWith("javax.")) //|| name.startsWith("$Proxy")//JDK proxies//FIXME AV is that 1.3 proxy ? fe. ataspect.$Proxy0 is a java5 proxy... || name.startsWith("sun.reflect."));//JDK reflect + return should; } /** @@ -507,7 +512,9 @@ public class WeavingAdaptor { /* Classes generated by weaver e.g. around closure advice */ else { String className = result.getClassName(); + System.err.println("? WeavingClassFileProvider.acceptResult() " + wovenClass.getClassName() + "->" + className); generatedClasses.put(className,result); + generatedClasses.put(wovenClass.getClassName(),result); generatedClassHandler.acceptClass(className,result.getBytes()); } } @@ -527,4 +534,4 @@ public class WeavingAdaptor { }; } } -} +} \ No newline at end of file -- 2.39.5