Doc: AspectJ argument binding does not work with typed collections [SPR-7186]

[spring-projects-issues]

Oliver Drotbohm opened SPR-7186 and commented

Suppose you have a generic class with a method accepting a collection of the generic type:

public class FooImpl<T> extends Foo<T>{

  @Override
  public void bar(List<T> param) {
    ...
  }
}

Now want to intercept the method calls whenever a list of special T's is given:

@Before("execution(* Foo+.bar(..)) && args(param)")
public void method(List<MySpecialType> param) {
  ...
}

Unfortunately this gets also invoked if FooImpl.bar(...) is called with a list consisting of non-MySpecialType instances which results in a ClassCastException. The reason for this probably lies in the fact, that of course List<MySpecialType> gets erased to List and thus the pointcut arguments get a primitive type of List assigned and thus matches also lists of non-MySpecialType. I tried to add concrete type information to the args(...) directive as follows:

@Before("execution(* Foo+.bar(..)) && args(param<java.util.List<MySpecialType>>)")

This get's parsed by the PointcutParser successfully but the information is neglected during matching. The only way around seems to be making a collection member type check inside the advice manually which is a little ugly IMHO.

Regards, Ollie

---

Affects: 3.0.2

Referenced from: commits https://github.com/spring-projects/spring-framework/commit/680bfbe718bc47897bb9b2aac068dd764a2a7bcb

1 votes, 2 watchers

[spring-projects-issues]

Ramnivas Laddad commented

I don't think this can be implemented (in Spring AOP as well as AspectJ). With AspectJ weaver, if you use call() pointcut, you can have a match based on generic parameters, BTW.

Also, did your second pointcut:

@Before("execution(* Foo+.bar(..)) && args(param<java.util.List<MySpecialType>>)")

work? That is not a syntactically valid pointcut. If it worked, that is a (separate) bug. You probably meant:

@Before("execution(* Foo+.bar(..)) && args(java.util.List<MySpecialType>)")

then that won't work for the same erasure issue. There is no way to do check such as 'foo instanceof List\'

[spring-projects-issues]

Oliver Drotbohm commented

Thanks for your comment, Ramnivas. Yes, the strangely looking pointcut is being parsed without issues. I'll take another deep dive into that and open a ticket in the AspectJ bugtracker.

Regarding the "impossible" I see, that there's no general solution possible as the typechecks would have to be done possibly into unlimited depth. But in case of the simple "Collection of X" case I see two ways. In general if X can be found out one could peek into the provided argument collection at runtime and include that result into the overall matching decision. From the poincut expression you gave above MySpecialType is extractable (actually the "buggy" parser already does) as well as the fact that we deal wich a collection. Alternatively Spring's GenericCollectionTypeResolver could be used to narrow the types if a PointcutParameter's type is a collection.

[spring-projects-issues]

Andy Clement commented

Let me know (via an AspectJ bug) about the rogue parsing of that unusual expression - looks to be a bug.

I agree with Ramnivas. We might be able to do a bit more but I'm still concerned we can't always recover what is required to do the match - yes Springs helper (or to avoid a spring dependency, a new AspectJ one) can dig to a degree but sometimes you cannot recover that generic info as you don't know where to look (I'm not saying the info isn't there, I'm saying we don't know where it is). The List flowing into bar() could have come from anywhere (without flow analysis) and at the point of doing the match all I can ask at runtime is for the class of that list. Unless it is a distinct type that extends list:

class MySpecialTypeList extends List\ {}

then I just won't know where the original list was declared in order to discover the generic signature.

I could automate creation of the check of the elements (what you are currently coding in the advice) but turning this:

args(List\)

info a runtime check like this:

if (argument instanceof List && ((List)argument).size()>0 && ((List)argument).get(0) instanceof MyFoo)) {

doesn't feel right.

[spring-projects-issues]

Ramnivas Laddad commented

To add to Andy's comment: Even checking for the 0th element to guess/check collection's type doesn't feel adequate (or right). To be semantically right, AspectJ will need to examine every element in the collection. And even then, there are issues such as accounting for 'null' elements (does it match a logical 'instanceof' or not?).

I think this issue belongs to "blame it on erasure" category :-) with little room to do the right thing.

[spring-projects-issues]

Oliver Drotbohm commented

Okay, I get your points. Can we turn this into a documentation issue then? Spring AOP documentation is quite short on generics and arguments in general and AspectJ documentation on generics does not mention @Aspect configuration at all. I think we should at least leave a note at least in Spring documentation IMHO then that collections cann not be bound this way. Assign this ticket to me if you like me to propose a draft.

Regarding the invalid pointcut I've opened https://bugs.eclipse.org/bugs/show_bug.cgi?id=312730.

[spring-projects-issues]

Ramnivas Laddad commented

Assigning for draft document

[spring-projects-issues]

Oliver Drotbohm commented

What do you think about this?

<section id="aop-ataspectj-advice-params-generics">
          <title>Advice parameters and generics</title>

          <para>Spring AOP can handle generics used in class declarations and
          method parameters. Suppose you have a generic type like this:</para>

          <programlisting language="java">public interface Sample&lt;T&gt; {
  void sampleGenericMethod(T param);
  void sampleGenericCollectionMethod(Collection&gt;T&gt; param);
}</programlisting>

          <para>You can restrict interception of method types to certain
          parameter types by simply typing the advice parameter to the
          parameter type you want to intercept the method for:</para>

          <programlisting language="java">@Before("execution(* ..Sample+.sampleGenericMethod(*)) &amp;&amp; args(param)")
public void beforeSampleMethod(MyType param) {
  // Advice implementation
}</programlisting>

          <para>That this works is pretty obvious as we already discussed
          above. However, it's worth pointing out that this won't work for
          generic collections. So you cannot define a pointcut like
          this:</para>

          <programlisting language="java">@Before("execution(* ..Sample+.sampleGenericCollectionMethod(*)) &amp;&amp; args(param)")
public void beforeSampleMethod(Collection&lt;MyType&gt; param) {
  // Advice implementation
}</programlisting>

          <para>To make this work we would have to inspect every element of
          the collection, which is not reasonable as we also cannot decide how
          to treat <literal>null</literal> values in general. To achieve
          something similar to this you have to type the parameter to
          <interfacename>Collection&lt;?&gt;</interfacename> and manually
          check the type of the elements.</para>
        </section>

[spring-projects-issues]

Oliver Drotbohm commented

Reassigned for feedback.

[spring-projects-issues]

Ramnivas Laddad commented

Looks good to me. Do you want to commit this change yourself? Otherwise, let me know and I can do it.

[spring-projects-issues]

Oliver Drotbohm commented

See https://fisheye.springsource.org/changelog/spring-framework/?cs=3351. Close it if you like.

[spring-projects-issues]

Oliver Drotbohm commented

Updated ticket properties according to latest development.