Fix for Bug 150254 "Provide context for LTW verbose messages" (tidy up ClassLoaderWeavingAdaptor.initialize())

5 files changed, 41 insertions, 22 deletions

diff --git a/loadtime/src/org/aspectj/weaver/loadtime/Aj.java b/loadtime/src/org/aspectj/weaver/loadtime/Aj.java
index 015a245b3..33398f0c5 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/Aj.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/Aj.java
@@ -103,7 +103,7 @@ public class Aj implements ClassPreProcessor {
                 	} else {
 	                    // create it and put it back in the weavingAdaptors map but avoid any kind of instantiation
 	                    // within the synchronized block
-	                    ClassLoaderWeavingAdaptor weavingAdaptor = new ClassLoaderWeavingAdaptor(loader, weavingContext);
+	                    ClassLoaderWeavingAdaptor weavingAdaptor = new ClassLoaderWeavingAdaptor();
 	                    adaptor = new ExplicitlyInitializedClassLoaderWeavingAdaptor(weavingAdaptor);
 	                    weavingAdaptors.put(loader, adaptor);
                 	}
diff --git a/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java b/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java
index 7f6502361..1ea1397ca 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java
@@ -71,23 +71,35 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
     private List m_aspectIncludeStartsWith = new ArrayList();
 
     private StringBuffer namespace;
-    private ClassLoader classLoader;
     private IWeavingContext weavingContext;
 
 	private static Trace trace = TraceFactory.getTraceFactory().getTrace(ClassLoaderWeavingAdaptor.class);
     
-    public ClassLoaderWeavingAdaptor(final ClassLoader loader, IWeavingContext wContext) {
+    public ClassLoaderWeavingAdaptor() {
     	super();
     	if (trace.isTraceEnabled()) trace.enter("<init>",this);
-    	this.classLoader = loader;
-    	this.weavingContext = wContext;
+    	if (trace.isTraceEnabled()) trace.exit("<init>");
+    }
+    
+    /**
+     * We don't need a reference to the class loader and using it during 
+     * construction can cause problems with recursion. It also makes sense
+     * to supply the weaving context during initialization to. 
+     * @deprecated
+     */
+    public ClassLoaderWeavingAdaptor(final ClassLoader deprecatedLoader, final IWeavingContext deprecatedContext) {
+    	super();
+    	if (trace.isTraceEnabled()) trace.enter("<init>",this,new Object[] { deprecatedLoader, deprecatedContext });
     	if (trace.isTraceEnabled()) trace.exit("<init>");
     }
 
-    protected void initialize (final ClassLoader deprecatedLoader, IWeavingContext deprecatedContext) {
+    protected void initialize (final ClassLoader classLoader, IWeavingContext context) {
         //super(null);// at this stage we don't have yet a generatedClassHandler to define to the VM the closures
     	if (initialized) return;
 
+    	if (trace.isTraceEnabled()) trace.enter("initialize",this,new Object[] { classLoader, context });
+
+    	this.weavingContext = context;
         if (weavingContext == null) {
         	weavingContext = new DefaultWeavingContext(classLoader);
         }
@@ -116,6 +128,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
 
         List definitions = parseDefinitions(classLoader);
         if (!enabled) {
+        	if (trace.isTraceEnabled()) trace.exit("initialize",enabled);
         	return;
         }
         
@@ -149,6 +162,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
         }
         
         initialized = true;
+    	if (trace.isTraceEnabled()) trace.exit("initialize",enabled);
     }
 
     /**
diff --git a/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java b/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java
index 671b8e17a..2a3d19381 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java
@@ -115,7 +115,7 @@ public class WeavingURLClassLoader extends ExtensibleURLClassLoader implements W
 					
 				};
 				
-				ClassLoaderWeavingAdaptor clwAdaptor = new ClassLoaderWeavingAdaptor(this,weavingContext);
+				ClassLoaderWeavingAdaptor clwAdaptor = new ClassLoaderWeavingAdaptor();
 				initializingAdaptor = true;
 				clwAdaptor.initialize(this,weavingContext);
 				initializingAdaptor = false;
diff --git a/loadtime/testsrc/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptorTest.java b/loadtime/testsrc/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptorTest.java
index a2a07c89a..805b989a6 100644
--- a/loadtime/testsrc/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptorTest.java
+++ b/loadtime/testsrc/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptorTest.java
@@ -19,26 +19,30 @@ public class ClassLoaderWeavingAdaptorTest extends TestCase {
 
 	public void testClassLoaderWeavingAdaptor() {
 		ClassLoader loader = new URLClassLoader(new URL[] {}, null);
-		ClassLoaderWeavingAdaptor adaptor = new ClassLoaderWeavingAdaptor(loader,null);
+		ClassLoaderWeavingAdaptor adaptor = new ClassLoaderWeavingAdaptor();
+		adaptor.initialize(loader,null);
 	}
 
 	public void testGetNamespace() {
 		ClassLoader loader = new URLClassLoader(new URL[] {}, null);
-		ClassLoaderWeavingAdaptor adaptor = new ClassLoaderWeavingAdaptor(loader,null);
+		ClassLoaderWeavingAdaptor adaptor = new ClassLoaderWeavingAdaptor();
+		adaptor.initialize(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);
+		ClassLoaderWeavingAdaptor adaptor = new ClassLoaderWeavingAdaptor();
+		adaptor.initialize(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);
+		ClassLoaderWeavingAdaptor adaptor = new ClassLoaderWeavingAdaptor();
+		adaptor.initialize(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/WeavingContextTest.java b/loadtime/testsrc/org/aspectj/weaver/loadtime/WeavingContextTest.java
index 7d5318219..af2e49182 100644
--- a/loadtime/testsrc/org/aspectj/weaver/loadtime/WeavingContextTest.java
+++ b/loadtime/testsrc/org/aspectj/weaver/loadtime/WeavingContextTest.java
@@ -25,7 +25,8 @@ public class WeavingContextTest extends TestCase {
 	public void testWeavingContext() {
 		URLClassLoader loader = new URLClassLoader(new URL[] {},null);
 		IWeavingContext context = new TestWeavingContext(loader);
-		ClassLoaderWeavingAdaptor adaptor = new ClassLoaderWeavingAdaptor(loader,context);
+		ClassLoaderWeavingAdaptor adaptor = new ClassLoaderWeavingAdaptor();
+		adaptor.initialize(loader,context);
 	}
 
 	public void testGetResources() {
@@ -38,8 +39,8 @@ public class WeavingContextTest extends TestCase {
 			}
 			
 		};
-		ClassLoaderWeavingAdaptor adaptor = new ClassLoaderWeavingAdaptor(loader,context);
-		adaptor.initialize(null,null);
+		ClassLoaderWeavingAdaptor adaptor = new ClassLoaderWeavingAdaptor();
+		adaptor.initialize(loader,context);
 		
 		assertTrue("IWeavingContext not called",called);
 	}
@@ -53,9 +54,9 @@ public class WeavingContextTest extends TestCase {
 			}
 			
 		};
-		ClassLoaderWeavingAdaptor adaptor = new ClassLoaderWeavingAdaptor(loader,context);
+		ClassLoaderWeavingAdaptor adaptor = new ClassLoaderWeavingAdaptor();
 		try {
-			adaptor.initialize(null,null);
+			adaptor.initialize(loader,context);
 		}
 		catch (UnsupportedOperationException ex) {
 			fail("IWeavingContect.getBundleIdFromURL() is deprecated");
@@ -72,8 +73,8 @@ public class WeavingContextTest extends TestCase {
 			}
 			
 		};
-		ClassLoaderWeavingAdaptor adaptor = new ClassLoaderWeavingAdaptor(loader,context);
-		adaptor.initialize(null,null);
+		ClassLoaderWeavingAdaptor adaptor = new ClassLoaderWeavingAdaptor();
+		adaptor.initialize(loader,context);
 		
 		assertTrue("IWeavingContext not called",called);
 	}
@@ -90,8 +91,8 @@ public class WeavingContextTest extends TestCase {
 			}
 			
 		};
-		ClassLoaderWeavingAdaptor adaptor = new ClassLoaderWeavingAdaptor(loader,context);
-		adaptor.initialize(null,null);
+		ClassLoaderWeavingAdaptor adaptor = new ClassLoaderWeavingAdaptor();
+		adaptor.initialize(loader,context);
 		
 		assertTrue("IWeavingContext not called",called);
 	}
@@ -108,8 +109,8 @@ public class WeavingContextTest extends TestCase {
 			}
 			
 		};
-		ClassLoaderWeavingAdaptor adaptor = new ClassLoaderWeavingAdaptor(loader,context);
-		adaptor.initialize(null,null);
+		ClassLoaderWeavingAdaptor adaptor = new ClassLoaderWeavingAdaptor();
+		adaptor.initialize(loader,context);
 		
 		assertTrue("IWeavingContext not called",called);
 	}