Spring Framework
  1. Spring Framework
  2. SPR-9438

Mixed ordering of @Before and @After advices does not work

    Details

      Description

      Ordering is not applied when there are in place both @After and @Before advices.

      In this case the order number is silently ignored because of this extra code:
      https://github.com/SpringSource/spring-framework/blob/master/spring-aop/src/main/java/org/springframework/aop/aspectj/autoproxy/AspectJPrecedenceComparator.java#L93

      Is this error or intention?

      Such limitation is not mentioned in the documentation:
      http://static.springsource.org/spring/docs/3.0.x/spring-framework-reference/html/aop.html#aop-ataspectj-advice-ordering

      I have a real situation (simplified in attached example) when I have a @Before advice, which is able to throw an Exception, preventing the target method to be called. I also have one @After advice which needs to be called with no matter if the Exception was thrown by @Before advice or not. So I need to set ordering of aspects: higher precedence for the @After advice and lower precedence for the @Before advice. But here comes the problem - ordering is ignored for mixed @After and @Before advices.

      Please see the attached demo project which demonstrates the problem.

        Activity

        Show
        Michal Moravcik added a comment - Discussed on Spring forum: http://forum.springsource.org/showthread.php?126500-Mixed-ordering-of-Before-and-After-advices-does-not-work
        Hide
        Michal Moravcik added a comment -

        attached sequence diagram illustrating the problem

        Show
        Michal Moravcik added a comment - attached sequence diagram illustrating the problem
        Hide
        Chris Beams added a comment -

        Andrei has added a repro project at https://github.com/SpringSource/spring-framework-issues/pull/25 (pending merge at time of writing)

        Show
        Chris Beams added a comment - Andrei has added a repro project at https://github.com/SpringSource/spring-framework-issues/pull/25 (pending merge at time of writing)
        Hide
        Juergen Hoeller added a comment -

        Removing that explicit exclusion code from AspectJPrecedenceComparator doesn't seem to have any effect on our test suite except for a very specific test that artifically checks exactly for that exclusion, so I'm removing it for 3.2.2. I don't see anything wrong with allowing such cross-advice ordering, after all.

        Juergen

        Show
        Juergen Hoeller added a comment - Removing that explicit exclusion code from AspectJPrecedenceComparator doesn't seem to have any effect on our test suite except for a very specific test that artifically checks exactly for that exclusion, so I'm removing it for 3.2.2. I don't see anything wrong with allowing such cross-advice ordering, after all. Juergen
        Hide
        Michal Moravcik added a comment -

        Thank you for fix

        Show
        Michal Moravcik added a comment - Thank you for fix

          People

          • Assignee:
            Juergen Hoeller
            Reporter:
            Michal Moravcik
            Last updater:
            Michal Moravcik
          • Votes:
            1 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:
              Days since last comment:
              1 year, 4 weeks, 3 days ago