11

I have two aspects each of which modify method arguments. When both aspects are applied to the same method, I would expect execution of the aspects to be chained and I would expect that the arguments modified in the first aspect to be available to the second aspect via joinPoint.getArgs(); However, it appears that each aspect gets only the original arguments; the second aspect never sees the modified values. I've contrived an example:

The test class:

public class AspectTest extends TestCase {
    @Moo
    private void foo(String boo, String foo) {
        System.out.println(boo + foo);
    }

    public void testAspect() {
        foo("You should", " never see this");
    }
}

The method foo() is advised by two aspects:

@Aspect
public class MooImpl {

    @Pointcut("execution(@Moo * *(..))")
    public void methodPointcut() {}

    @Around("methodPointcut()")
    public Object afterMethodInControllerClass(ProceedingJoinPoint joinPoint) throws Throwable {
        System.out.println("MooImpl is being called");
        Object[] args = joinPoint.getArgs();
        args[0] = "don't";
        return joinPoint.proceed(args);
    }
}

and...

@Aspect
public class DoubleMooImpl {

    @Pointcut("execution(@Moo * *(..))")
    public void methodPointcut() {}

    @Around("methodPointcut()")
    public Object afterMethodInControllerClass(ProceedingJoinPoint joinPoint) throws Throwable {
        System.out.println("DoubleMooImpl is being called");
        Object[] args = joinPoint.getArgs();
        args[1] = " run and hide";
        return joinPoint.proceed(args);
    }
}

I would expect the output to be:

MooImpl is being called
DoubleMooImpl is being called
don't run and hide

...but is:

MooImpl is being called
DoubleMooImpl is being called
You should run and hide

Am I using the correct approach to modify arguments via around advice?

rolve
  • 10,083
  • 4
  • 55
  • 75
Java Junky
  • 131
  • 1
  • 7
  • Please subscribe to the AspectJ users mailing list and ask your question there. You should get a competent answer there. I would be interested in the outcome too. – kriegaex Oct 22 '12 at 10:27

2 Answers2

2

This does not sound like a aspect ordering issue , it is more of how method arguments are handled in java - references to arguments are passed by value, since your first argument is a String, by modifying what the String reference is pointing to you are not really affecting the original String in any way and so gets passed as such.

You can try instead passing in a StringBuilder or some other mutable type and then modifying the state, the state change should get reflected correctly then.

Update:

I tested with a mutable type and it changes as expected:

@Moo
private void foo(StringBuilder boo, StringBuilder foo) {
    System.out.println(boo.toString() + foo.toString());
}

public void testAspect() {
    foo(new StringBuilder("You should"), new StringBuilder(" never see this"));
}

With MooImpl Aspect:

@Aspect
public class MooImpl {

    @Pointcut("execution(@Moo * *(..))")
    public void methodPointcut() {}

    @Around("methodPointcut()")
    public Object afterMethodInControllerClass(ProceedingJoinPoint joinPoint) throws Throwable {
        System.out.println("MooImpl is being called");
        Object[] args = joinPoint.getArgs();
        ((StringBuilder)args[0]).append("****");
        return joinPoint.proceed(args);
    }
}

and DoubleMooImpl:

@Aspect
public class DoubleMooImpl {

    @Pointcut("execution(@Moo * *(..))")
    public void methodPointcut() {}

    @Around("methodPointcut()")
    public Object afterMethodInControllerClass(ProceedingJoinPoint joinPoint) throws Throwable {
        System.out.println("DoubleMooImpl is being called");
        Object[] args = joinPoint.getArgs();
        ((StringBuilder)args[1]).append("****");
        return joinPoint.proceed(args);
    }
}

and getting this output:

MooImpl is being called
DoubleMooImpl is being called
You should**** never see this****
Biju Kunjummen
  • 49,138
  • 14
  • 112
  • 125
  • Mutable type or not, the behavior is the same. – Java Junky Oct 11 '12 at 19:42
  • It is, I just tested it, updating my answer with more details. In your case you are before you modify args[0] or args[1] it is pointing to the correct object, but when you say `args[0]="don't"` the reference is pointing to a brand different object altogether. The args array that you get is just a copy of the argument references, so changing the argument array does not really have any effect, whereas if you change it the way I have in my sample it does get reflected. – Biju Kunjummen Oct 11 '12 at 19:53
  • I understand that the array is a copy of the arguments. What I don't understand is why the second of the two aspects does not get a copy of the arguments passed by the first aspect's invocation of proceed(args). – Java Junky Oct 11 '12 at 20:40
  • 1
    I think it will be a little more clear if you look at the code for JoinPoint.getArgs - http://grepcode.com/file/repo1.maven.org/maven2/org.aspectj/aspectjrt/1.7.0/org/aspectj/runtime/reflect/JoinPointImpl.java?av=f - only a copy of the args is being passed and you are changing a reference of the copy so nothing really changes – Biju Kunjummen Oct 11 '12 at 20:51
  • If that is the case, then how is it the first advice is applied properly, but not the second? – Java Junky Oct 11 '12 at 20:57
  • Just to be clear, the fact that a copy of the arguments is available via the joinpoint is not in dispute. The question is about what copy appears on the joinpoint when there are multiple aspects applied to a method. Intuition might suggest it would be a copy of whatever previous advice passed to proceed(args). This doesn't seem to be the case. I'm wondering if I'm either doing something wrong, or if this is a possible bug in AspectJ. – Java Junky Oct 11 '12 at 21:06
2

After having been on the road for days, I have finally gotten around to thinking about this and answering your question. I hope it is okay if I use native AspectJ syntax because I do not feel comfortable with the @AspectJ notation.

import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;

@Retention(RetentionPolicy.RUNTIME)
public @interface Moo {}
public class AspectTest {
    @Moo
    private void foo(String boo, String foo) {
        System.out.println(boo + foo);
    }

    public static void main(String[] args) {
        new AspectTest().foo("You should", " never see this");
    }
}
public aspect MooImpl {
    pointcut methodPointcut() : execution(@Moo * *(String, ..));

    Object around(String firstArg) : methodPointcut() && args(firstArg, ..) {
        System.out.println("MooImpl is being called");
        return proceed("don't");
    }
}
public aspect DoubleMooImpl {
    pointcut methodPointcut() : execution(@Moo * *(*, String, ..));

    Object around(String secondArg) : methodPointcut() && args(*, secondArg, ..) {
        System.out.println("DoubleMooImpl is being called");
        return proceed(" run and hide");
    }
}

Your mistake was to assume that you could manipulate arguments retrieved via getArgs(), which is wrong. In order to pass arguments to proceed() you need to refer to them via args(), which I have demonstrated above. Please note the syntax for retrieving the first vs. second String argument in the two aspects.

This should solve your problem.

kriegaex
  • 63,017
  • 15
  • 111
  • 202
  • Using @AspectJ style, that means you have to use `ProceedingJointPoint.proceed(Object[] args)` to get the same functionality, but the semantics are a bit different and you should refer to the [javadoc](http://www.eclipse.org/aspectj/doc/released/runtime-api/org/aspectj/lang/ProceedingJoinPoint.html) for the precise parameters to pass. – Frank Pavageau Oct 26 '12 at 13:26