Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spring AOP's implementation of AspectJ's JoinPoint.Static part does not comply with AspectJ spec (not 'static', id always 0) [SPR-13865] #18438

Closed
spring-issuemaster opened this issue Jan 12, 2016 · 6 comments

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Jan 12, 2016

Russell Allen opened SPR-13865 and commented

AspectJ's JoinPoint.StaticPart interface defines a separate object which is meant to be a singleton style reference to that join point, independent of the currently intercepted execution state (thus the 'static part' in name.) Within this interface is a method, int getId(), which should...

Return the id for this JoinPoint.StaticPart. All JoinPoint.StaticPart instances are assigned an id number upon creation. For each advised type the id numbers start at 0.
-javaDoc

The intent of the static part and the id method is to permit faster access to 'static' information about the location of the join point independent of the execution state (thread, parameters, etc). This static information is handy for logging and necessary for creating unique cache keys.

Take for example this use case: Create, using AOP, an aspect that counts and logs the execution of any request handler method on a per handler method basis.

The implementation will likely contain a Map<handler,Integer> to track the count per handler. However, as is, the static part can not be used as a key. Nor can the result of the id method be used as an intra-type method id.


Affects: 4.1.7

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jan 26, 2016

Juergen Hoeller commented

Any suggestion for what we should be returning instead of 0 here? An instance-specific count value doesn't seem to deliver any benefit either, since every individual method invocation would have its own value there...

Juergen

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jan 26, 2016

Russell Allen commented

I did quote the JavaDoc there, but I have to admit, that's not what triggered me to open the ticket. In my use case, I need to cache a bunch of introspected data about the join point. Data such as method name, return type, arguments, annotations on or up the hierarchy, etc. These are costly to compute and they don't change as the program executes. Thus the need to cache, and to have a readily available cache key. As is, I have to cast the join point and pull out the target method, being careful of proxies and interfaces, and use the java Method reference as a key.

In short, I'm more interested in the StaticPart meeting it's javaDoc semantics; that I can use a reference to the StaticPart as a cache key.

That being said, here's my thoughts on the id number: I'm not sure I understand exactly what the definition of that id is. Does "advised type" mean a Class, or is "type" being used more loosely to mean any advisable structure such as a method, field, etc.? I'm going to assume it means a java.lang.Class<?> because that makes some sense. Then each Class has an id sequence, starting at 0, from which it assigns id values to any JoinPoint within that Class's static structure. For example, if class Foo has a property bar with get and set methods, then their might be a join point (id=0) on the field bar, join point (id=1) on method getBar(), and a join point (id=2) on the method setBar(T bar).

Some key points here...

  • the sequence is 'rooted' to the "advised type", which I am inferring to be a java.util.Class<?>.
  • the id is associated to the static artifact (method, field, etc.) not an instance's artifact. This is an inherited trait, as this getId() method is defined on StaticPart, not its parent (the invocation part.)
  • I assume that the id of join point static part is shared by aspects, regardless of the point cut that associated them to the join point.

Clarification on that last point: Lets say I have a method called homepage() which is annotated with @Controller. I also have two point cut's defined, one for any method execution and another for the execution of methods annotated with @Controller. I have two around advice, one per point cut. The two advice implementations should see the same id value returned from JoinPoint.StaticPart.getId() when they are intercepting that @Controller homepage() method.

That's my thoughts, but I'm by no means an AOP expert. It'd be great if an AspectJ team member could lend us some, uh, advice. Pun intended.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jan 26, 2016

Juergen Hoeller commented

Andy Clement, what's your take on this one?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jan 26, 2016

Andy Clement commented

Not sure if you have seen it but some extra discussion on this feature is in the README for 1.6.4 ( https://eclipse.org/aspectj/doc/released/README-164.html ) and AspectJ bug 89009 ( https://bugs.eclipse.org/bugs/show_bug.cgi?id=89009 ). The README does include some sample code on usage which shows using getId() as an array index, using a pertypewithin aspect to have the array in place per advised type/class:

aspect X pertypewithin(*) {
  Timer[] timerArray = ...
  
  Object around(): execution(public * *(..)) {
    Timer timerToUse = timerArray[thisJoinPointStaticPart.getId()];
    timerToUse.start();
    Object o =  proceed();
    timerToUse.stop();
    return o;
  }
}

Much faster than having the timerArray as a map:

Timer timerToUse = timerMap.get(thisJoinPointStaticPart);

(Advised type is a class, as you were assuming).

Matching/Advising works in AspectJ by running over the byte code in potential classes and creating join points. Then attempting to match these join points against pointcuts, if they match then inserting calls to the advice. So for any join point (e.g. a method execution) if it matches any number of pointcuts, they will all be matches on the same join point object. So in your situation if you match a join point execution via wildcard in one pointcut and via annotation matching in another pointcut, it is the same join point and in both advices attached to the pointcuts getId() will return the same number.

For example, if class Foo has a property bar with get and set methods, then their might be a join point (id=0) on the field bar, join point (id=1) on method getBar(), and a join point (id=2) on the method setBar(T bar).

Basically yes, but the field itself is not a join point - only the access or modification of the field is a join point. Let's turn your sentence into a program Foo.java:

public class Foo {
  public static void main(String []argv) {
    Foo f = new Foo();
    f.getBar();
    f.setBar(null);
  }
  Bar bar;
  public Bar getBar() { return this.bar; }
  public void setBar(Bar newBar) { this.bar = newBar; }
}

class Bar {}

aspect X {
  before(): execution(* *Bar(..)) { // getBar or setBar
    System.out.println(thisJoinPointStaticPart.getId()+": "+thisJoinPoint);
  }
  before(): set(Bar bar) || get(Bar bar) { // set or get of the Bar field
    System.out.println(thisJoinPointStaticPart.getId()+": "+thisJoinPoint);
  }
}

If we compile that, at compile time the join points are computed, matched against pointcuts and ids allocated. Now if we run it:

java Foo
1: execution(Bar Foo.getBar())
0: get(Bar Foo.bar)
3: execution(void Foo.setBar(Bar))
2: set(Bar Foo.bar)

We can see that each one gets a different ID. Nothing should really be inferred that they are printed in the order 1,0,3,2 here. The numbers are allocated simply based on the order in which the byte code is processed, in this case we are processing method bodies (so computing join points for byte codes like getfield/putfield) before creating the join points representing method execution. What is important here is each distinct join point gets a different ID. Notice also that the second pointcut matches either the get or set of the field and those are different join points, so this same advice can run with different join point instances. I hope some of that helps.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jan 26, 2016

Russell Allen commented

Thank you Andy, that helped clarify a lot of assumptions on my part. Good stuff! :)

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jan 12, 2019

Bulk closing outdated, unresolved issues. Please, reopen if still relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.