big refactoring

6 files changed, 82 insertions, 31 deletions

diff --git a/loadtime/src/org/aspectj/weaver/loadtime/Aj.java b/loadtime/src/org/aspectj/weaver/loadtime/Aj.java
index 4df029d2f..4884d9eaa 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/Aj.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/Aj.java
@@ -14,6 +14,7 @@ package org.aspectj.weaver.loadtime;
 import java.util.Map;
 import java.util.WeakHashMap;
 
+import org.aspectj.bridge.context.CompilationAndWeavingContext;
 import org.aspectj.weaver.Dump;
 import org.aspectj.weaver.tools.Trace;
 import org.aspectj.weaver.tools.TraceFactory;
@@ -75,7 +76,7 @@ public class Aj implements ClassPreProcessor {
             		if (trace.isTraceEnabled()) trace.exit("preProcess");
                 	return bytes;
                 }
-                byte[] newBytes = weavingAdaptor.weaveClass(className, bytes);
+                byte[] newBytes = weavingAdaptor.weaveClass(className, bytes,false);
                 Dump.dumpOnExit(weavingAdaptor.getMessageHolder(), true);
         		if (trace.isTraceEnabled()) trace.exit("preProcess",newBytes);
                 return newBytes;
@@ -89,6 +90,8 @@ public class Aj implements ClassPreProcessor {
             // would make sense at least in test f.e. see TestHelper.handleMessage()
     		if (trace.isTraceEnabled()) trace.exit("preProcess",th);
             return bytes;
+        } finally {
+            CompilationAndWeavingContext.resetForThread();
         }
     }
 
@@ -98,7 +101,7 @@ public class Aj implements ClassPreProcessor {
      */
     static class WeaverContainer {
 
-        private static Map weavingAdaptors = new WeakHashMap();
+        private final static Map weavingAdaptors = new WeakHashMap();
 
         static WeavingAdaptor getWeaver(ClassLoader loader, IWeavingContext weavingContext) {
             ExplicitlyInitializedClassLoaderWeavingAdaptor adaptor = null;
diff --git a/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java b/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java
index 40497e103..17659c175 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java
@@ -31,6 +31,7 @@ import java.util.StringTokenizer;
 import org.aspectj.asm.IRelationship;
 import org.aspectj.bridge.AbortException;
 import org.aspectj.bridge.Constants;
+import org.aspectj.bridge.IMessage;
 import org.aspectj.bridge.ISourceLocation;
 import org.aspectj.util.LangUtil;
 import org.aspectj.weaver.ICrossReferenceHandler;
@@ -130,7 +131,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
             }
         };
 
-        List definitions = parseDefinitions(classLoader);
+        List definitions = weavingContext.getDefinitions(classLoader,this);
         if (definitions.isEmpty()) {
         	disable(); // TODO maw Needed to ensure messages are flushed
         	if (trace.isTraceEnabled()) trace.exit("initialize",definitions);
@@ -139,12 +140,13 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
         
         bcelWorld = new LTWWorld(
         		classLoader, weavingContext, // TODO when the world works in terms of the context, we can remove the loader...
-        		getMessageHandler(), new ICrossReferenceHandler() {
-                    public void addCrossReference(ISourceLocation from, ISourceLocation to, IRelationship.Kind kind, boolean runtimeTest) {
-                        ;// for tools only
-                    }
-                }
-        );
+        		getMessageHandler(), null);
+//        new ICrossReferenceHandler() {
+//                    public void addCrossReference(ISourceLocation from, ISourceLocation to, IRelationship.Kind kind, boolean runtimeTest) {
+//                        ;// for tools only
+//                    }
+//                }
+//        );
 
         weaver = new BcelWeaver(bcelWorld);
 
@@ -178,19 +180,19 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
      * @param weaver
      * @param loader
      */
-    private List parseDefinitions(final ClassLoader loader) {
+    List parseDefinitions(final ClassLoader loader) {
         if (trace.isTraceEnabled()) trace.enter("parseDefinitions",this,loader);
 
         List definitions = new ArrayList();
     	try {
-            info("register classloader " + getClassLoaderName(loader));
-            //TODO av underoptimized: we will parse each XML once per CL that see it
+    		info("register classloader " + getClassLoaderName(loader));
+    		//TODO av underoptimized: we will parse each XML once per CL that see it
 
             //TODO av dev mode needed ? TBD -Daj5.def=...
             if (loader.equals(ClassLoader.getSystemClassLoader())) {
                 String file = System.getProperty("aj5.def", null);
                 if (file != null) {
-                    info("using (-Daj5.def) " + file);
+                	info("using (-Daj5.def) " + file);
                     definitions.add(DocumentParser.parse((new File(file)).toURL()));
                 }
             }
@@ -213,16 +215,16 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
         			    seenBefore.add(xml);
     			    }
     			    else {
-    			    	warn("ignoring duplicate definition: " + xml);
+   			    		warn("ignoring duplicate definition: " + xml);
     			    }
     			}
     		}
     		if (definitions.isEmpty()) {
-        		info("no configuration found. Disabling weaver for class loader " + getClassLoaderName(loader));
+    			info("no configuration found. Disabling weaver for class loader " + getClassLoaderName(loader));
     		}
         } catch (Exception e) {
         	definitions.clear();
-            warn("parse definitions failed",e);
+    		warn("parse definitions failed",e);
         }
         
         if (trace.isTraceEnabled()) trace.exit("parseDefinitions",definitions);
@@ -243,7 +245,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
         } catch (Exception ex) {
             trace.error("register definition failed",ex);
             success = false;
-            warn("register definition failed",(ex instanceof AbortException)?null:ex);
+    		warn("register definition failed",(ex instanceof AbortException)?null:ex);
         }
         
         if (trace.isTraceEnabled()) trace.exit("registerDefinitions",success);
@@ -308,7 +310,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
                     }
                 }
                 if (failure != null || resource == null) {
-                	warn("Cannot access resource for -Xlintfile:"+weaverOption.lintFile,failure);
+		    		warn("Cannot access resource for -Xlintfile:"+weaverOption.lintFile,failure);
 //                    world.getMessageHandler().handleMessage(new Message(
 //                            "Cannot access resource for -Xlintfile:"+weaverOption.lintFile,
 //                            IMessage.WARNING,
@@ -422,7 +424,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
                 	info("define aspect " + concreteAspect.name);
                     ConcreteAspectCodeGen gen = new ConcreteAspectCodeGen(concreteAspect, weaver.getWorld());
                     if (!gen.validate()) {
-                        error("Concrete-aspect '"+concreteAspect.name+"' could not be registered");
+   			    		error("Concrete-aspect '"+concreteAspect.name+"' could not be registered");
                         success = false;
                         break;
                     }
@@ -451,7 +453,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
         /* We didn't register any aspects so disable the adaptor */
         else if (namespace == null) {
         	success = false;
-    		info("no aspects registered. Disabling weaver for class loader " + getClassLoaderName(loader));
+        	info("no aspects registered. Disabling weaver for class loader " + getClassLoaderName(loader));
         }
 
         if (trace.isTraceEnabled()) trace.exit("registerAspects",success);
@@ -468,12 +470,12 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
         	byte[] bytes = gen.getBytes();
 
         	try {
-        		byte[] newBytes = weaveClass(name, bytes);
+        		byte[] newBytes = weaveClass(name, bytes,true);
                 this.generatedClassHandler.acceptClass(name,newBytes);
         	}
         	catch (IOException ex) {
         		trace.error("weaveAndDefineConceteAspects",ex);
-				error("exception weaving aspect '" + name + "'",ex);
+		    	error("exception weaving aspect '" + name + "'",ex);
         	}
         }
         
@@ -734,7 +736,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
 	private void defineClass(ClassLoader loader, String name, byte[] bytes) {
     	if (trace.isTraceEnabled()) trace.enter("defineClass",this,new Object[] {loader,name,bytes});
     	Object clazz = null;
-		debug("generating class '" + name + "'");
+    	debug("generating class '" + name + "'");
 		
 		try {
 			//TODO av protection domain, and optimize
@@ -746,14 +748,14 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
 					new Integer(0), new Integer(bytes.length) });
 		} catch (InvocationTargetException e) {
 			if (e.getTargetException() instanceof LinkageError) {
-				warn("define generated class failed",e.getTargetException());
+	    		warn("define generated class failed",e.getTargetException());
 				//is already defined (happens for X$ajcMightHaveAspect interfaces since aspects are reweaved)
 				// TODO maw I don't think this is OK and
 			} else {
-				warn("define generated class failed",e.getTargetException());
+	    		warn("define generated class failed",e.getTargetException());
 			}
 		} catch (Exception e) {
-			warn("define generated class failed",e);
+    		warn("define generated class failed",e);
 		}
 
 		if (trace.isTraceEnabled()) trace.exit("defineClass",clazz);
diff --git a/loadtime/src/org/aspectj/weaver/loadtime/DefaultWeavingContext.java b/loadtime/src/org/aspectj/weaver/loadtime/DefaultWeavingContext.java
index 02c8a091e..6ffd2cb28 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/DefaultWeavingContext.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/DefaultWeavingContext.java
@@ -14,6 +14,11 @@ package org.aspectj.weaver.loadtime;
 import java.io.IOException;
 import java.net.URL;
 import java.util.Enumeration;
+import java.util.List;
+
+import org.aspectj.weaver.tools.Trace;
+import org.aspectj.weaver.tools.TraceFactory;
+import org.aspectj.weaver.tools.WeavingAdaptor;
 
 /**
  * Use in non-OSGi environment
@@ -21,17 +26,19 @@ import java.util.Enumeration;
  * @author David Knibb
  */
 public class DefaultWeavingContext implements IWeavingContext {
-	
+
 	protected ClassLoader loader;
-	
 	private String shortName;
 
+	private static Trace trace = TraceFactory.getTraceFactory().getTrace(DefaultWeavingContext.class);
+
 	/**
 	 * Construct a new WeavingContext to use the specifed ClassLoader
 	 * This is the constructor which should be used.
 	 * @param loader
 	 */
 	public DefaultWeavingContext(ClassLoader loader) {
+		super();
 		this.loader = loader;
 	}
 
@@ -93,4 +100,19 @@ public class DefaultWeavingContext implements IWeavingContext {
         } 
         return isLocallyDefined;
 	}
+
+	/**
+	 * Simply call weaving adaptor back to parse aop.xml
+	 *
+	 * @param weaver
+	 * @param loader
+	 */
+	public List getDefinitions(final ClassLoader loader, final WeavingAdaptor adaptor) {
+	        if (trace.isTraceEnabled()) trace.enter("getDefinitions",this,new Object[] { loader, adaptor });
+	
+	        List definitions = ((ClassLoaderWeavingAdaptor)adaptor).parseDefinitions(loader);
+	        
+	        if (trace.isTraceEnabled()) trace.exit("getDefinitions",definitions);
+			return definitions;
+	    }
 }
diff --git a/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java b/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java
index fca4a23a5..01d3c6d90 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java
@@ -125,7 +125,7 @@ public class WeavingURLClassLoader extends ExtensibleURLClassLoader implements W
 			}
 			
 			try {
-				b = adaptor.weaveClass(name,b);
+				b = adaptor.weaveClass(name,b,false);
 			}
 			catch (AbortException ex) {
 	    		trace.error("defineClass",ex);
diff --git a/loadtime/testsrc/org/aspectj/loadtime/LoadtimeModuleTests.java b/loadtime/testsrc/org/aspectj/loadtime/LoadtimeModuleTests.java
index 5683e5a8b..f1e6a3790 100644
--- a/loadtime/testsrc/org/aspectj/loadtime/LoadtimeModuleTests.java
+++ b/loadtime/testsrc/org/aspectj/loadtime/LoadtimeModuleTests.java
@@ -18,7 +18,6 @@ import junit.textui.TestRunner;
 
 import org.aspectj.weaver.loadtime.AjTest;
 import org.aspectj.weaver.loadtime.ClassLoaderWeavingAdaptorTest;
-import org.aspectj.weaver.loadtime.JRockitAgentTest;
 import org.aspectj.weaver.loadtime.WeavingContextTest;
 import org.aspectj.weaver.loadtime.WeavingURLClassLoaderTest;
 import org.aspectj.weaver.loadtime.test.DocumentParserTest;
@@ -34,7 +33,7 @@ public class LoadtimeModuleTests extends TestCase {
         suite.addTestSuite(DocumentParserTest.class);
         suite.addTestSuite(AjTest.class);
         suite.addTestSuite(ClassLoaderWeavingAdaptorTest.class);
-        suite.addTestSuite(JRockitAgentTest.class);
+        //        suite.addTestSuite(JRockitAgentTest.class);
         suite.addTestSuite(WeavingContextTest.class);
         suite.addTestSuite(WeavingURLClassLoaderTest.class);
         return suite;
diff --git a/loadtime/testsrc/org/aspectj/weaver/loadtime/WeavingContextTest.java b/loadtime/testsrc/org/aspectj/weaver/loadtime/WeavingContextTest.java
index c056b9ee6..97e037317 100644
--- a/loadtime/testsrc/org/aspectj/weaver/loadtime/WeavingContextTest.java
+++ b/loadtime/testsrc/org/aspectj/weaver/loadtime/WeavingContextTest.java
@@ -15,9 +15,12 @@ import java.io.IOException;
 import java.net.URL;
 import java.net.URLClassLoader;
 import java.util.Enumeration;
+import java.util.List;
 
 import junit.framework.TestCase;
 
+import org.aspectj.weaver.tools.WeavingAdaptor;
+
 public class WeavingContextTest extends TestCase {
 
 	private boolean called;
@@ -115,6 +118,24 @@ public class WeavingContextTest extends TestCase {
 		assertTrue("IWeavingContext not called",called);
 	}
 
+	public void testGetDefinitions () throws Exception {
+		File file = new File("../loadtime/testdata");
+		URL fileURL = file.getCanonicalFile().toURL();
+		URLClassLoader loader = new URLClassLoader(new URL[] { fileURL },null);
+		IWeavingContext context = new TestWeavingContext(loader) {
+
+			public List getDefinitions(ClassLoader loader, WeavingAdaptor adaptor) {
+				called = true;
+		        return super.getDefinitions(loader,adaptor);
+			}
+			
+		};
+		ClassLoaderWeavingAdaptor adaptor = new ClassLoaderWeavingAdaptor();
+		adaptor.initialize(loader,context);
+		
+		assertTrue("getDefinitions not called",called);
+	}
+	
 	private static class TestWeavingContext implements IWeavingContext {
 
 		private ClassLoader loader;
@@ -157,6 +178,10 @@ public class WeavingContextTest extends TestCase {
 	        } 
 	        return isLocallyDefined;
 		}
+
+		public List getDefinitions(ClassLoader loader, WeavingAdaptor adaptor) {
+	        return ((ClassLoaderWeavingAdaptor)adaptor).parseDefinitions(loader);
+		}
 	}
 
 	protected void setUp() throws Exception {