impl and test for dec precedence in aop.xml without extends

9 files changed, 173 insertions, 19 deletions

diff --git a/docs/devGuideDB/ltw.xml b/docs/devGuideDB/ltw.xml
index 45c98d712..d3d9636b2 100644
--- a/docs/devGuideDB/ltw.xml
+++ b/docs/devGuideDB/ltw.xml
@@ -354,8 +354,37 @@
                 // here we are using a singleton aspect
                 AbstractAspect concreteInstance = Aspects.aspectOf(myConcreteAspectClass);
             ]]></programlisting>
-        </sect2>
+       </sect2>
 
+        <sect2 id="concrete-aspect-precedence" xreflabel="concrete-aspect-precedence">
+            <title>Using Concrete Aspects to define precedence</title>
+            <para>
+                As described in the previous section, the <literal>concrete-aspect</literal> element in
+                <literal>META-INF/aop.xml</literal> gives the option to declare the precedence, just as
+                <literal>@DeclarePrecedence</literal> or <literal>declare precedence</literal> do in
+                aspect source code.
+            </para>
+            <para>
+                Sometimes it is required to declare precedence without extending any abstract aspect
+                as well. For such a need, it is possible to use the <literal>concrete-aspect</literal>
+                element without the <literal>extends</literal> attribute and without any
+                <literal>pointcut</literal> nested element, but only with a <literal>precedence</literal>
+                attribute.
+                Consider the following:
+            </para>
+            <programlisting><![CDATA[
+                <aspectj>
+                    <concrete-aspect name="mypack.__MyDeclarePrecedence"
+                                     precedence="*..*Security*, Logging+, *"/>
+                </aspectj>
+            ]]></programlisting>
+            <para>
+                This deployment time declaration is only defining a precedence rule. You have to remember
+                that the <literal>name</literal> attribute must be a valid fully qualified class name
+                that will be then reserved for this concrete-aspect and must not conflict with other classes
+                you deploy.
+            </para>
+        </sect2>
         <!-- TODO someone implement that -->
         <!--
         <sect2 id="configuring-load-time-weaving-with-properties-files" xreflabel="configuring-load-time-weaving-with-properties-files">
@@ -524,4 +553,3 @@
         </sect2>
     </sect1>
 </chapter>
-
diff --git a/loadtime/src/org/aspectj/weaver/loadtime/ConcreteAspectCodeGen.java b/loadtime/src/org/aspectj/weaver/loadtime/ConcreteAspectCodeGen.java
index fb30edd6e..897e17f52 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/ConcreteAspectCodeGen.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/ConcreteAspectCodeGen.java
@@ -34,6 +34,7 @@ import org.aspectj.weaver.bcel.LazyClassGen;
 import org.aspectj.weaver.bcel.LazyMethodGen;
 import org.aspectj.weaver.loadtime.definition.Definition;
 import org.aspectj.weaver.patterns.PerClause;
+import org.aspectj.weaver.patterns.PerSingleton;
 
 import java.lang.reflect.Modifier;
 import java.util.ArrayList;
@@ -102,6 +103,26 @@ public class ConcreteAspectCodeGen {
             return false;
         }
 
+        // name must be undefined so far
+        ResolvedType current = m_world.resolve(m_concreteAspect.name, true);
+        if (!current.isMissing()) {
+            reportError("Attempt to concretize but choosen aspect name already defined: " + stringify());
+            return false;
+        }
+
+        // it can happen that extends is null, for precedence only declaration
+        if (m_concreteAspect.extend == null && m_concreteAspect.precedence != null) {
+            if (m_concreteAspect.pointcuts.isEmpty()) {
+                m_isValid = true;
+                m_perClause = new PerSingleton();
+                m_parent = null;
+                return true;// no need to checks more in that special case
+            } else {
+                reportError("Attempt to use nested pointcuts without extends clause: "+stringify());
+                return false;
+            }
+        }
+
         m_parent = m_world.resolve(m_concreteAspect.extend, true);
         // handle inner classes
         if (m_parent.equals(ResolvedType.MISSING)) {
@@ -136,13 +157,6 @@ public class ConcreteAspectCodeGen {
             return false;
         }
 
-        // must be undefined so far
-        ResolvedType current = m_world.resolve(m_concreteAspect.name, true);
-        if (!current.isMissing()) {
-            reportError("Attempt to concretize but choosen aspect name already defined:" + stringify());
-            return false;
-        }
-
         // must have all abstractions defined
         List elligibleAbstractions = new ArrayList();
         Iterator methods = m_parent.getMethods();
@@ -202,15 +216,15 @@ public class ConcreteAspectCodeGen {
         //TODO AV - abstract away from BCEL...
         // @Aspect //inherit clause from m_parent
         // @DeclarePrecedence("....") // if any
-        // public class xxxName extends xxxExtends {
-        //    @Pointcut(xxxExpression-n)
-        //    private void xxxName-n() {}
+        // public class xxxName [extends xxxExtends] {
+        //    [@Pointcut(xxxExpression-n)
+        //    public void xxxName-n() {}]
         // }
 
         // @Aspect public class ...
         LazyClassGen cg = new LazyClassGen(
                 m_concreteAspect.name.replace('.', '/'),
-                m_parent.getName(),
+                (m_parent==null)?"java/lang/Object":m_parent.getName().replace('.', '/'),
                 null,//TODO AV - we could point to the aop.xml that defines it and use JSR-45
                 Modifier.PUBLIC + Constants.ACC_SUPER,
                 EMPTY_STRINGS,
@@ -252,7 +266,7 @@ public class ConcreteAspectCodeGen {
         InstructionList cbody = init.getBody();
         cbody.append(InstructionConstants.ALOAD_0);
         cbody.append(cg.getFactory().createInvoke(
-                m_parent.getName().replace('.', '/'),
+                (m_parent==null)?"java/lang/Object":m_parent.getName().replace('.', '/'),
                 "<init>",
                 Type.VOID,
                 EMPTY_TYPES,
@@ -265,7 +279,7 @@ public class ConcreteAspectCodeGen {
             Definition.Pointcut abstractPc = (Definition.Pointcut) it.next();
 
             LazyMethodGen mg = new LazyMethodGen(
-                    Modifier.PUBLIC,
+                    Modifier.PUBLIC,//TODO AV - respect visibility instead of opening up?
                     Type.VOID,
                     abstractPc.name,
                     EMPTY_TYPES,
diff --git a/loadtime/src/org/aspectj/weaver/loadtime/definition/Definition.java b/loadtime/src/org/aspectj/weaver/loadtime/definition/Definition.java
index 87c2f82e2..9bea5cd2d 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/definition/Definition.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/definition/Definition.java
@@ -85,7 +85,15 @@ public class Definition {
 
         public ConcreteAspect(String name, String extend, String precedence) {
             this.name = name;
-            this.extend = extend;
+            // make sure extend set to null if ""
+            if (extend == null || extend.length() == 0) {
+                this.extend = null;
+                if (precedence == null || precedence.length() == 0) {
+                    throw new RuntimeException("Not allowed");
+                }
+            } else {
+                this.extend = extend;
+            }
             this.precedence = precedence;
             this.pointcuts = new ArrayList();
         }
diff --git a/loadtime/src/org/aspectj/weaver/loadtime/definition/DocumentParser.java b/loadtime/src/org/aspectj/weaver/loadtime/definition/DocumentParser.java
index 1aeccaf1d..ac80afd20 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/definition/DocumentParser.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/definition/DocumentParser.java
@@ -151,10 +151,11 @@ public class DocumentParser extends DefaultHandler {
             String name = attributes.getValue(NAME_ATTRIBUTE);
             String extend = attributes.getValue(EXTEND_ATTRIBUTE);
             String precedence = attributes.getValue(PRECEDENCE_ATTRIBUTE);
-            if (!isNull(name) && !isNull(extend)) {
-                if (isNull(precedence)) {
+            if (!isNull(name)) {
+                if (isNull(precedence) && !isNull(extend)) {//if no precedence, then extends must be there
                     m_lastConcreteAspect = new Definition.ConcreteAspect(name, extend);
-                } else {
+                } else if (!isNull(precedence)) {
+                    // wether a pure precedence def, or an extendsANDprecedence def.
                     m_lastConcreteAspect = new Definition.ConcreteAspect(name, extend, precedence);
                 }
                 m_definition.getConcreteAspects().add(m_lastConcreteAspect);
diff --git a/tests/java5/ataspectj/ataspectj/ConcreteAtAspectTest.java b/tests/java5/ataspectj/ataspectj/ConcreteAtAspectTest.java
index 2d673ca9f..3aa5cea7a 100644
--- a/tests/java5/ataspectj/ataspectj/ConcreteAtAspectTest.java
+++ b/tests/java5/ataspectj/ataspectj/ConcreteAtAspectTest.java
@@ -17,6 +17,7 @@ import junit.framework.TestSuite;
 import org.aspectj.lang.annotation.Aspect;
 import org.aspectj.lang.annotation.Before;
 import org.aspectj.lang.annotation.Pointcut;
+import org.aspectj.lang.Aspects;
 
 /**
  * @author <a href="mailto:alex AT gnilux DOT com">Alexandre Vasseur</a>
@@ -61,6 +62,15 @@ public class ConcreteAtAspectTest extends TestCase {
         assertEquals(3, I);
     }
 
+    public void tesCanLoad() {
+        try {
+            Class jitAspect = Class.forName("ataspectj.Foo");
+            Object aspect = Aspects.aspectOf(jitAspect);
+        } catch (Throwable t) {
+            fail(t.toString());
+        }
+    }
+
     public static void main(String[] args) {
         TestHelper.runAndThrowOnFailure(suite());
     }
diff --git a/tests/java5/ataspectj/ataspectj/ConcretePrecedenceAspectTest.java b/tests/java5/ataspectj/ataspectj/ConcretePrecedenceAspectTest.java
new file mode 100644
index 000000000..c873028b1
--- /dev/null
+++ b/tests/java5/ataspectj/ataspectj/ConcretePrecedenceAspectTest.java
@@ -0,0 +1,69 @@
+/*******************************************************************************
+ * 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;
+
+import junit.framework.TestCase;
+import junit.framework.Test;
+import junit.framework.TestSuite;
+import org.aspectj.lang.annotation.Aspect;
+import org.aspectj.lang.annotation.Before;
+
+/**
+ * @author <a href="mailto:alex AT gnilux DOT com">Alexandre Vasseur</a>
+ */
+public class ConcretePrecedenceAspectTest extends TestCase {
+
+    static String LOG = "";
+
+    void target() {
+        LOG = LOG + "target ";
+    }
+
+    @Aspect
+    static class TestAspect_1 {
+        @Before("execution(* ataspectj.ConcretePrecedenceAspectTest.target())")
+        public void before() {
+            LOG = LOG + "1 ";
+        }
+    }
+
+    @Aspect
+    static class TestAspect_2 {
+        @Before("execution(* ataspectj.ConcretePrecedenceAspectTest.target())")
+        public void before() {
+            LOG = LOG + "2 ";
+        }
+    }
+
+    @Aspect
+    static class TestAspect_3 {
+        @Before("execution(* ataspectj.ConcretePrecedenceAspectTest.target())")
+        public void before() {
+            LOG = LOG + "3 ";
+        }
+    }
+
+    public void testPrecedenceFromXML() {
+        LOG = "";
+        target();
+        assertEquals("2 3 1 target ", LOG);
+    }
+
+    public static void main(String[] args) {
+        TestHelper.runAndThrowOnFailure(suite());
+    }
+
+    public static Test suite() {
+        return new TestSuite(ConcretePrecedenceAspectTest.class);
+    }
+
+}
diff --git a/tests/java5/ataspectj/ataspectj/aop-concreteprecedenceaspect.xml b/tests/java5/ataspectj/ataspectj/aop-concreteprecedenceaspect.xml
new file mode 100644
index 000000000..a3dbc3927
--- /dev/null
+++ b/tests/java5/ataspectj/ataspectj/aop-concreteprecedenceaspect.xml
@@ -0,0 +1,12 @@
+<?xml version="1.0"?>
+<aspectj>
+    <weaver options="-XmessageHandlerClass:ataspectj.TestHelper -1.5"/>
+    <aspects>
+        <concrete-aspect
+                name="ataspectj.Foo"
+                precedence="*..*_2, *..*_3, *..*_1"/>
+        <aspect name="ataspectj.ConcretePrecedenceAspectTest.TestAspect_1"/>
+        <aspect name="ataspectj.ConcretePrecedenceAspectTest.TestAspect_2"/>
+        <aspect name="ataspectj.ConcretePrecedenceAspectTest.TestAspect_3"/>
+    </aspects>
+</aspectj>
diff --git a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java
index 49e9e7850..ae8f478f7 100644
--- a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java
+++ b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java
@@ -102,6 +102,10 @@ public class AtAjLTWTests extends XMLBasedAjcTestCase {
         runTest("ConcreteAspect");
     }
 
+    public void testConcretePrecedenceAspect() {
+        runTest("ConcretePrecedenceAspect");
+    }
+
     public void testAspectOfWhenAspectNotInInclude() {
         runTest("AspectOfWhenAspectNotInInclude");
     }
diff --git a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml
index 257d56805..dcb9f3918 100644
--- a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml
+++ b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml
@@ -147,6 +147,14 @@
         <run class="ataspectj.ConcreteAspectTest" ltw="ataspectj/aop-concreteaspect.xml"/>
     </ajc-test>
 
+    <ajc-test dir="java5/ataspectj" title="ConcretePrecedenceAspect">
+        <compile
+            files="ataspectj/ConcretePrecedenceAspectTest.java,ataspectj/TestHelper.java"
+            options="-1.5 -Xdev:NoAtAspectJProcessing -XnoWeave"
+            />
+        <run class="ataspectj.ConcretePrecedenceAspectTest" ltw="ataspectj/aop-concreteprecedenceaspect.xml"/>
+    </ajc-test>
+
     <ajc-test dir="java5/ataspectj" title="AspectOfWhenAspectNotInInclude">
         <compile
             files="ataspectj/bugs/AspectOfWhenAspectNotInIncludeTest.java,ataspectj/TestHelper.java"