#108892 cannot reproduce: hierarchy of aop.xml accross classloader test is fine

9 files changed, 223 insertions, 7 deletions

diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/ValidateAtAspectJAnnotationsVisitor.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/ValidateAtAspectJAnnotationsVisitor.java
index 3a7fe62e3..f5855ba63 100644
--- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/ValidateAtAspectJAnnotationsVisitor.java
+++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/ValidateAtAspectJAnnotationsVisitor.java
@@ -209,6 +209,7 @@ public class ValidateAtAspectJAnnotationsVisitor extends ASTVisitor {
 	}
 	
 	/**
+     * aspect must be public
 	 * nested aspect must be static
 	 * cannot extend a concrete aspect
 	 * pointcut in perclause must be good.
@@ -228,8 +229,13 @@ public class ValidateAtAspectJAnnotationsVisitor extends ASTVisitor {
 				typeDecl.scope.problemReporter().signalError(typeDecl.sourceStart,typeDecl.sourceEnd,"only classes can have an @Aspect annotation");
 			}
 		}
-		
-		TypeReference parentRef = typeDecl.superclass;
+
+        //FIXME AV - do we really want that
+//        if (!Modifier.isPublic(typeDecl.modifiers)) {
+//            typeDecl.scope.problemReporter().signalError(typeDecl.sourceStart,typeDecl.sourceEnd,"@Aspect class must be public");
+//        }
+
+        TypeReference parentRef = typeDecl.superclass;
 		if (parentRef != null) {
 			TypeBinding parentBinding = parentRef.resolvedType;
 			if (parentBinding instanceof SourceTypeBinding) {
diff --git a/tests/java5/ataspectj/ajc-ant.xml b/tests/java5/ataspectj/ajc-ant.xml
index 077fc2667..5c4495be4 100644
--- a/tests/java5/ataspectj/ajc-ant.xml
+++ b/tests/java5/ataspectj/ajc-ant.xml
@@ -186,4 +186,31 @@
         </java>
     </target>
 
+    <target name="ltw.AppContainer">
+        <!-- mkdir the 2 sub app root folder -->
+        <mkdir dir="${aj.sandbox}/META-INF"/>
+        <mkdir dir="${aj.sandbox}/app_1"/>
+        <mkdir dir="${aj.sandbox}/app_1/META-INF"/>
+        <!-- install sub app -->
+        <copy todir="${aj.sandbox}/app_1">
+            <fileset dir="${aj.sandbox}" includes="ataspectj/hierarchy/app/*"/>
+        </copy>
+        <copy todir="${aj.sandbox}/app_2">
+            <fileset dir="${aj.sandbox}" includes="ataspectj/hierarchy/app/*"/>
+        </copy>
+        <delete dir="${aj.sandbox}/ataspectj/hierarchy/app"/>
+        <!-- install the aop.xml DD -->
+        <copy file="${aj.root}/tests/java5/ataspectj/ataspectj/hierarchy/aop-global.xml"
+              tofile="${aj.sandbox}/META-INF/aop.xml"/>
+        <!-- only app1 gets local aspect -->
+        <copy file="${aj.root}/tests/java5/ataspectj/ataspectj/hierarchy/aop-local.xml"
+              tofile="${aj.sandbox}/app_1/META-INF/aop.xml"/>
+        <!-- run -->
+        <java fork="yes" classname="ataspectj.hierarchy.AppContainerTest" failonerror="yes">
+            <classpath refid="aj.path"/>
+            <jvmarg value="-javaagent:${aj.root}/lib/test/loadtime5.jar"/>
+            <jvmarg value="-Daj.weaving.verbose=true"/>
+        </java>
+    </target>
+
 </project>
\ No newline at end of file
diff --git a/tests/java5/ataspectj/ataspectj/hierarchy/AppContainerTest.java b/tests/java5/ataspectj/ataspectj/hierarchy/AppContainerTest.java
new file mode 100644
index 000000000..5bd2330f4
--- /dev/null
+++ b/tests/java5/ataspectj/ataspectj/hierarchy/AppContainerTest.java
@@ -0,0 +1,101 @@
+/*******************************************************************************
+ * 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.hierarchy;
+
+import junit.framework.TestCase;
+import junit.framework.Test;
+import junit.framework.TestSuite;
+
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.io.File;
+
+import org.aspectj.lang.annotation.Aspect;
+import org.aspectj.lang.annotation.Around;
+import org.aspectj.lang.ProceedingJoinPoint;
+import ataspectj.TestHelper;
+
+/**
+ * Assumes ataspect.hierarchy.app DOES NOT EXISTS in system classpath
+ * but exists in 2 folder located at "/app_1" and /app_2
+ * ie "/app_1/ataspect.hierarchy.app.*.class"
+ *
+ * @author <a href="mailto:alex AT gnilux DOT com">Alexandre Vasseur</a>
+ */
+public class AppContainerTest extends TestCase {
+
+    public static interface IApp {
+
+        String invoke(String input);
+    }
+
+
+    IApp app1;
+    IApp app2;
+
+    public void setUp() throws Exception {
+        try {
+            Class k = Class.forName("ataspectj.hierarchy.app.SubApp");
+            throw new Exception("config error, app must not be in system classpath");
+        } catch (ClassNotFoundException e) {
+            ;//fine
+        }
+
+        //build path and app
+        URL path = AppContainerTest.class.getProtectionDomain().getCodeSource().getLocation();
+        String path1 = path.toString() + "app_1/";
+        String path2 = path.toString() + "app_2/";
+
+        URLClassLoader app1CL = new URLClassLoader(
+                new URL[]{new URL(path1)},
+                AppContainerTest.class.getClassLoader()
+        );
+        URLClassLoader app2CL = new URLClassLoader(
+                new URL[]{new URL(path2)},
+                AppContainerTest.class.getClassLoader()
+        );
+
+        app1 = (IApp)Class.forName("ataspectj.hierarchy.app.SubApp", false, app1CL).newInstance();
+        app2 = (IApp)Class.forName("ataspectj.hierarchy.app.SubApp", false, app2CL).newInstance();
+    }
+
+    public void testApp1LocalAspect() {
+        String res = app1.invoke("app1");
+        assertEquals("globalAspect[localAspect[app1]]", res);
+    }
+
+    public void testApp2NoLocalAspect() {
+        String res = app2.invoke("app2");
+        assertEquals("globalAspect[app2]", res);
+    }
+
+
+    @Aspect
+    public static class BaseAspect {
+
+        @Around("execution(* ataspectj.hierarchy.app.SubApp.invoke(..))")//TODO IApp
+        public Object around(ProceedingJoinPoint jp) throws Throwable {
+            String out = (String) jp.proceed();
+            return "globalAspect[" + out + "]";
+        }
+    }
+
+    public static void main(String[] args) {
+        TestHelper.runAndThrowOnFailure(suite());
+    }
+
+    public static Test suite() {
+        return new TestSuite(AppContainerTest.class);
+    }
+
+
+}
diff --git a/tests/java5/ataspectj/ataspectj/hierarchy/aop-global.xml b/tests/java5/ataspectj/ataspectj/hierarchy/aop-global.xml
new file mode 100644
index 000000000..f17696441
--- /dev/null
+++ b/tests/java5/ataspectj/ataspectj/hierarchy/aop-global.xml
@@ -0,0 +1,6 @@
+<aspectj>
+    <weaver options="-1.5 -XmessageHandlerClass:ataspectj.TestHelper -Xlint:ignore"/>
+    <aspects>
+        <aspect name="ataspectj.hierarchy.AppContainerTest.BaseAspect"/>
+    </aspects>
+</aspectj>
\ No newline at end of file
diff --git a/tests/java5/ataspectj/ataspectj/hierarchy/aop-local.xml b/tests/java5/ataspectj/ataspectj/hierarchy/aop-local.xml
new file mode 100644
index 000000000..384b77416
--- /dev/null
+++ b/tests/java5/ataspectj/ataspectj/hierarchy/aop-local.xml
@@ -0,0 +1,6 @@
+<aspectj>
+    <weaver options="-1.5 -XmessageHandlerClass:ataspectj.TestHelper -Xlint:ignore"/>
+    <aspects>
+        <aspect name="ataspectj.hierarchy.app.SubApp.SubAspect"/>
+    </aspects>
+</aspectj>
\ No newline at end of file
diff --git a/tests/java5/ataspectj/ataspectj/hierarchy/app/SubApp.java b/tests/java5/ataspectj/ataspectj/hierarchy/app/SubApp.java
new file mode 100644
index 000000000..e724c486d
--- /dev/null
+++ b/tests/java5/ataspectj/ataspectj/hierarchy/app/SubApp.java
@@ -0,0 +1,42 @@
+/*******************************************************************************
+ * 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.hierarchy.app;
+
+import ataspectj.hierarchy.AppContainerTest;
+import org.aspectj.lang.ProceedingJoinPoint;
+import org.aspectj.lang.annotation.Around;
+import org.aspectj.lang.annotation.Aspect;
+
+/**
+ * Leaves in child classloader in two forms, like 2 webapp
+ *
+ * @author <a href="mailto:alex AT gnilux DOT com">Alexandre Vasseur</a>
+ */
+public class SubApp implements AppContainerTest.IApp {
+
+    // simple echo. May be advised or not depending on the aspect deployed there
+    public String invoke(String input) {
+        return input;
+    }
+
+    // this child aspect will be LTW for only one variation of the SubApp
+    @Aspect
+    public static class SubAspect {
+
+        @Around("execution(* ataspectj.hierarchy.app.SubApp.invoke(..))")
+        public Object around(ProceedingJoinPoint jp) throws Throwable {
+            String out = (String) jp.proceed();
+            return "localAspect[" + out + "]";
+        }
+    }
+
+}
diff --git a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java
index ae8f478f7..5b0d24e70 100644
--- a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java
+++ b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java
@@ -110,4 +110,8 @@ public class AtAjLTWTests extends XMLBasedAjcTestCase {
         runTest("AspectOfWhenAspectNotInInclude");
     }
 
+    public void testAppContainer() {
+        runTest("AppContainer");
+    }
+
 }
diff --git a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml
index dcb9f3918..7122a2a70 100644
--- a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml
+++ b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml
@@ -161,4 +161,12 @@
             options="-1.5 -XnoWeave"/>
         <run class="ataspectj.bugs.AspectOfWhenAspectNotInIncludeTest" ltw="ataspectj/bugs/aop-aspectofwhenaspectnotinincludetest.xml"/>
     </ajc-test>
+
+    <ajc-test dir="java5/ataspectj" title="AppContainer">
+        <compile
+            files="ataspectj/hierarchy/AppContainerTest.java,ataspectj/hierarchy/app/SubApp.java,ataspectj/TestHelper.java"
+            options="-1.5 -XnoWeave"
+            />
+        <ant file="ajc-ant.xml" target="ltw.AppContainer" verbose="true"/>
+    </ajc-test>
 </suite>
\ No newline at end of file
diff --git a/weaver/src/org/aspectj/weaver/bcel/AtAjAttributes.java b/weaver/src/org/aspectj/weaver/bcel/AtAjAttributes.java
index f3846c53a..66a21b84b 100644
--- a/weaver/src/org/aspectj/weaver/bcel/AtAjAttributes.java
+++ b/weaver/src/org/aspectj/weaver/bcel/AtAjAttributes.java
@@ -243,7 +243,14 @@ public class AtAjAttributes {
             // bypass what we have read
             return EMPTY_LIST;
         }
-        //FIXME turn on when ajcMightHaveAspect
+
+        // the following block will not detect @Pointcut in non @Aspect types for optimization purpose
+        if (!hasAtAspectAnnotation) {
+            return EMPTY_LIST;
+        }
+
+
+        //FIXME AV - turn on when ajcMightHaveAspect
 //        if (hasAtAspectAnnotation && type.isInterface()) {
 //            msgHandler.handleMessage(
 //                    new Message(
@@ -257,10 +264,19 @@ public class AtAjAttributes {
 //            return EMPTY_LIST;
 //        }
 
-        // the following block will not detect @Pointcut in non @Aspect types for optimization purpose
-        if (!hasAtAspectAnnotation) {
-            return EMPTY_LIST;
-        }
+        // semantic check: @Aspect must be public
+        // FIXME AV - do we really want to enforce that?
+//        if (hasAtAspectAnnotation && !javaClass.isPublic()) {
+//            msgHandler.handleMessage(
+//                    new Message(
+//                            "Found @Aspect annotation on a non public class '" + javaClass.getClassName() + "'",
+//                            IMessage.ERROR,
+//                            null,
+//                            type.getSourceLocation()
+//                    )
+//            );
+//            return EMPTY_LIST;
+//        }
 
         // code style pointcuts are class attributes
         // we need to gather the @AJ pointcut right now and not at method level annotation extraction time