1 2 Previous Next 16 Replies Latest reply on Oct 14, 2004 7:59 AM by kevinconner

    Why lack of 'before' and 'after' advice types is bad..

      Hi!

      It's in my mind from some time. Now I share it with you.

      Imagine situation of say, 3D concern which does sth before invoking core method. It's applied directly on core classes (assuming OO is 2D)

      Let's suppose someone wants to add 4th dimension concern directly on 3D concern. Its aim would be to provide fault tolerance in a way, that if 3D concern fails, then we should still keep running our core system, but without 3D concern applied.

      To implement it in JBossAOP one must intercept all execution of advices of 3D concern and surround it with some try()..catch() block.

      3D concern (actual state):

      public Object 3DAdvice(...)
      {
       if (doSomething());
       invocation.invokeNext(); <- this is core method (2D)
       }
      }


      Fault tolerant 4D concern (actual state):
      public Object failTolerance4DAdvice(...)
      {
       try
       {
       invocation.invokeNext();
       }catch (SomeNastyException ...)
       {
       //continue running system... means proceed to the "core" method
       //which may be impossible with current JBossAOP
       ??? return invocation.invokeNextAfter3D(); ???
      
       }
      }


      In AspectJ style and maybe with JBossAOP future style, one would use "before" advice style for 3D concern..
      3D concern (after/before state):
      public Object before3DAdvice(...)
      {
       doSomething();
      }


      Fault tolerant 4D concern (after/before state) then would be simple:
      public Object failTolerance4DBeforeAdvice(...)
      {
       try
       {
       invocation.invokeNext();
       }catch (SomeNastyException ...)
       {
       //continue running system... means proceed to the "core" method,
       //means: "do nothing because we are "on" before advice"
       //and next invocation in chain is "core"
       }
      }



      What do you say on that?

      Or maybe there is some nice looking way for invoking invocation after next invocation (which could solve lack "before" problem)?

      Regards, Tomasz


      PS. Anyway, emulation of "after" and "before" with "around only" is ugly.

        • 1. Re: Why lack of 'before' and 'after' advice types is bad..
          bill.burke

          what are the exact semantics of before/after?

          You get the same thing with around. You want before?

          try
          {
           doBefore();
           invocation.invokeNext();
          }
          


          You want after?

          try
          {
           Object rtn = invocation.invokeNext();
           doAfter();
           return rtn;
          }
          


          You want after throwing?
          try
          {
           return invocation.invokeNext();
          }
          catch (Exception ex)
          {
           afterThrowing();
           throw ex;
          }
          


          IMO, I'd rather have the aspect developer thinking in java call stacks and using try/catch/finally blocks.

          Less is better, IMO.

          Bill

          • 2. Re: Why lack of 'before' and 'after' advice types is bad..


            Thanks for explanation..

            t.

            • 3. Re: Why lack of 'before' and 'after' advice types is bad..

              Hi Bill
              Sorry, but I'm not satisfied yet..

              Try to read my first post once more, and try to look at concerns in samples: core system concern, 3D concern (might be anything you want; persistence, authorization; ..) and 4D fault tolerant concern (for handling exceptions of 3D concerns) which must be applied to one of 3D concerns in my case.

              I've tried to imlpement it with "emulated before", but I can't. It's impossible(?)


              So, would you please show me, the code for JBossAOP for some 3D advice (say: "persistence advice") that does the following:
              -executes some persistenceOperations() method which always throws exceptions
              -at the end (no matter what happened before) invokes 'invocation.invokeNext()' for executing "core" logic

              And for advice (say: "exception handling advice") that is applied to above advice, and does the following:
              -handles exceptions of "persistence concern"

              Because I know how to implement above with real 'before', and I don't know how to implement it with 'emulated before'.. (of course AspectJ and AspectWerkz both have 'before' and 'after' advice)


              And btw; here is the code. Where does marked exception goes?! Look for strange output..

              (This code for 3D advice is awful, ugly, hard to understand. 3DAspect's advice should consist of ONE statement only if we had 'before' advice type)

              public class Pojo
              {
               public void method1()
               {
               log.debug("Pojo here...");
               }
              ...}
              
              public 3DAspect
              {
               public Object invoke(Invocation invocation) throws Throwable
               {
               try
               {
               log.debug("2-executing 3D operations");
               databaseOperations();
               }
               catch (Exception e)
               {
               log.debug("DB ops exception catched");
               throw e; <- where is this exception thrown ???? It isn't in output
               }
               finally
               {
               log.debug("3D-finally");
               return invocation.invokeNext(); <-- execution of core logic here
               }
               }
              
               private void databaseOperations()
               {
               throw new RuntimeException("X");
               }
              ...}
              
              public ExceptionHandler4DAspect
              {
               public Object invoke(Invocation invocation) throws Throwable
               {
               Object result = null;
               log.debug("1-Begin of 4D-Exception handler concern");
              
               try
               {
               result = invocation.invokeNext(); <-- execution of 3D concern here
               log.debug("After trown exception - You shouldn't see this!!!");
               }
               catch (Exception e)
               {
               //EXCEPTION HANDLING LOGIC
               log.debug("Handle 3D exceptions here...");
               }
              
               log.debug("4-end of 4D");
               return result;
               }
              ...}
              
              <aop>
               <bind pointcut="execution(* Pojo->method1(..))">
               <interceptor class="ThirdDimension" />
               </bind>
              
               <bind pointcut="execution(* ThirdDimension->invoke(..))">
               <interceptor class="FourthDimension" />
               </bind>
              </aop>
              
              Output:
               [java] ------START
               [java] ==O==[main] FourthDimension: 25: 1-Begin of 4D-Exception handler concern
               [java] ==O==[main] ThirdDimension: ?: 2-executing 3D operations
               [java] ==O==[main] ThirdDimension: ?: DB ops exception catched
               [java] ==O==[main] ThirdDimension: ?: 3D-finally
               [java] ==O==[main] Pojo: ?: Pojo here...
               [java] ==O==[main] FourthDimension: 30: After trown exception - You shouldn't see this!!!
               [java] ==O==[main] FourthDimension: 38: 4-end of 4D
               [java] ------END
              
              (I'm sending you the code on private, for not wasting too much time for it)


              Regards,
              Tomasz

              PS1. Is it simpler, to write 30 try()..catch()..finally() statements, instead 30 'before' words? IMHO: No.

              PS2. If I think about some comparable example for this situation, there is System.out.println() comming to my mind. What if we would have only 'void System.out.println(char character)' ? ;p

              PS3. I don't know where to find definition of 'before'/'after'. But difference is that with around emulation, you must always think about executing invocation.invokeNext() at the end/beginning.


              • 4. Re: Why lack of 'before' and 'after' advice types is bad..
                kevinconner

                I think your code is wrong from your initial description.

                Firstly the pointcut should be on databaseOperations as this is the part you wish to hide.

                Secondly your try/catch/finally block causes the exception to be ignored as the finally issues a return. (cf. the question in comments)

                As for your PS1 and PS3 try something like this.

                package knrc.beforeAfter;
                
                import org.jboss.aop.joinpoint.Invocation;
                import org.jboss.aop.advice.Interceptor;
                
                public abstract class AbstractInterceptor implements Interceptor
                {
                 public final Object invoke(Invocation invocation) throws Throwable
                 {
                 final Object beforeResult = invokeBefore(invocation) ;
                 if (beforeResult != null)
                 {
                 return invokeAfter(invocation, beforeResult) ;
                 }
                
                 final Object result ;
                 try
                 {
                 result = invocation.invokeNext();
                 return invokeAfter(invocation, result) ;
                 }
                 catch (final Throwable th)
                 {
                 return invokeAfterThrowable(invocation, th) ;
                 }
                 }
                
                 public Object invokeBefore(final Invocation invocation)
                 {
                 return null ;
                 }
                
                 public Object invokeAfter(final Invocation invocation, final Object result)
                 {
                 return result ;
                 }
                
                 public Object invokeAfterThrowable(final Invocation invocation, final Throwable th)
                 throws Throwable
                 {
                 throw th ;
                 }
                }
                


                The rest of your example, using the above is as follows

                /*
                 * Created on 27-Sep-2004
                 */
                package knrc.beforeAfter;
                
                import org.jboss.aop.joinpoint.Invocation;
                
                /**
                 * @author kevin
                 */
                public class Aspect3D extends AbstractInterceptor
                {
                 public String getName()
                 {
                 return "Aspect3D" ;
                 }
                
                 public Object invokeBefore(final Invocation invocation)
                 {
                 System.out.println("2-executing 3D operations") ;
                 databaseOperations() ;
                 return null ;
                 }
                
                 private void databaseOperations()
                 {
                 throw new RuntimeException("X") ;
                 }
                }
                


                /*
                 * Created on 27-Sep-2004
                 */
                package knrc.beforeAfter;
                
                import org.jboss.aop.joinpoint.Invocation;
                
                /**
                 * @author kevin
                 */
                public class ExceptionHandlerAspect4D extends AbstractInterceptor
                {
                 public String getName()
                 {
                 return "ExceptionHandlerAspect4D" ;
                 }
                
                 public Object invokeBefore(final Invocation invocation)
                 {
                 System.out.println("1-Begin of 4D-Exception handler concern") ;
                 return null ;
                 }
                
                 public Object invokeAfter(final Invocation invocation, final Object result)
                 {
                 System.out.println("After thrown exception - You shouldn't see this!!!") ;
                 return result ;
                 }
                
                 public Object invokeAfterThrowable(final Invocation invocation, final Throwable th)
                 throws Throwable
                 {
                 System.out.println("Handle 3D exceptions here") ;
                 return null ;
                 }
                }
                


                <?xml version="1.0" encoding="UTF-8"?>
                <aop>
                 <bind pointcut="execution(* knrc.beforeAfter.Pojo1->method1())">
                 <interceptor class="knrc.beforeAfter.Aspect3D"/>
                 </bind>
                 <bind pointcut="execution(* knrc.beforeAfter.Aspect3D->invokeBefore(..))">
                 <interceptor class="knrc.beforeAfter.ExceptionHandlerAspect4D"/>
                 </bind>
                </aop>
                


                Is this what you want?

                Kev

                • 5. Re: Why lack of 'before' and 'after' advice types is bad..

                  Thanks for your time Kevin.

                  Yes it works. I'm impressed. With your solution my 3DAspect looks simple. It's simple to configure it in way that you've provided. And my ExceptionHandlerAspect4D is a simple old JBossAOP aspect, with plain invoke(..). That's enough for me.

                  Ok. But still it is a workaround, although smart and works for this example.

                  But what about a whole aspect of before advice methods? To put, say persistance logic in one Aspect class. Not one separate interceptor for every core method (these 30 methods ;) ) .
                  You know: beforeGetCustomers(), beforeRentAHouse(), beforeAnotherCoreMethod(), etc..

                  It isn't so simple now. You'll end up with 30 interceptors (extend AbstractInterceptor) or one aspect extending MyCustomAspect where you'll have 30 methods like this one... ouh my.. I'm scared of the code I see.. I won't write it...

                  Wouldn't be it simpler to have:

                  <bind pointcut="execution(* Pojo->method1())">
                   <interceptor class="knrc.beforeAfter.Aspect3D" type="before" />
                   ^^^^^^^^^^^^
                   </bind>
                  
                   <aspect class="Aspect3D" scope="PER_VM" />
                   <bind pointcut="execution(* Pojo->method1())">
                   <advice name="beforeMethod1" type="before" aspect="Aspect3D" />
                   ^^^^^^^^^^^^
                   </bind>


                  Thanks for explaining me this try()..catch()..finally() issues.

                  Best regards,
                  Tomasz

                  • 6. Re: Why lack of 'before' and 'after' advice types is bad..
                    kevinconner

                    Hiya Tomasz.

                    Yes it works. I'm impressed.

                    Thanks, I aim to please ;-)

                    Ok. But still it is a workaround, although smart and works for this example.

                    I wouldn't call it a workaround, just a clean design that refactors common code and is based on the JBoss API :-)

                    But what about a whole aspect of before advice methods? To put, say persistance logic in one Aspect class. Not one separate interceptor for every core method (these 30 methods ;) ) .
                    You know: beforeGetCustomers(), beforeRentAHouse(), beforeAnotherCoreMethod(), etc..

                    It isn't so simple now. You'll end up with 30 interceptors (extend AbstractInterceptor) or one aspect extending MyCustomAspect where you'll have 30 methods like this one... ouh my.. I'm scared of the code I see.. I won't write it...


                    I certainly wouldn't want to write it that way but I can think of two alternatives that are cleaner, depending on your use case.

                    I must admit that if I was faced with an aspect containing 30 advices then I would be wondering whether the design was correct. Is this connected with your pervayler library? If so, we can take this offline.

                    Wouldn't be it simpler to have:


                    It would certainly be simpler for your 30 advices aspect. Personally I like the current implementation ;-)

                    Kev


                    • 7. Re: Why lack of 'before' and 'after' advice types is bad..

                       

                      "KevinConner" wrote:

                      ...I must admit that if I was faced with an aspect containing 30 advices then I would be wondering whether the design was correct. Is this connected with your pervayler library? If so, we can take this offline.

                      Kev

                      Hello,

                      Oh my, no way! :) It's for my current employer. PAT works fine with no problems :)
                      But I would be _very_ interested in your opinion about PAT (private)..

                      Tomasz

                      PS. I will leave this topic now. I've said what I wanted. It's up to JBossAOP authors to consider all the facts. Anyway, thanks guys :)

                      • 8. Re: Why lack of 'before' and 'after' advice types is bad..
                        kevinconner

                        Hiya Tomasz.

                        I eventually found some time to put together one of my ideas, here's an example using it (uses the annotation compiler).

                        The annotation interfaces.

                        package knrc.beforeAfter2;
                        
                        /**
                         * @author kevin
                         */
                        public interface after
                        {
                        }


                        package knrc.beforeAfter2;
                        
                        /**
                         * @author kevin
                         */
                        public interface afterThrowable
                        {
                        }


                        package knrc.beforeAfter2;
                        
                        /**
                         * @author kevin
                         */
                        public interface before
                        {
                        }


                        The before/after/afterThrowable aspect.
                        package knrc.beforeAfter2;
                        
                        import org.jboss.aop.joinpoint.Invocation;
                        import org.jboss.aop.joinpoint.MethodInvocation;
                        
                        public class AspectBeforeAfter
                        {
                         public final static String RESULT_KEY = AspectBeforeAfter.class.getName() + ".result" ;
                         public final static String THROWABLE_KEY = AspectBeforeAfter.class.getName() + ".throwable" ;
                        
                         public Object invokeBefore(final MethodInvocation methodInvocation)
                         throws Throwable
                         {
                         final Invocation invocation = getInvocation(methodInvocation) ;
                         methodInvocation.invokeNext() ;
                         return invocation.invokeNext() ;
                         }
                        
                         public Object invokeAfter(final MethodInvocation methodInvocation)
                         throws Throwable
                         {
                         final Invocation invocation = getInvocation(methodInvocation) ;
                         final Object result = invocation.invokeNext() ;
                         invocation.addResponseAttachment(RESULT_KEY, result) ;
                         try
                         {
                         return methodInvocation.invokeNext() ;
                         }
                         finally
                         {
                         invocation.getResponseContextInfo().remove(RESULT_KEY) ;
                         }
                         }
                        
                         public Object invokeAfterThrowable(final MethodInvocation methodInvocation)
                         throws Throwable
                         {
                         final Invocation invocation = getInvocation(methodInvocation) ;
                         try
                         {
                         return invocation.invokeNext() ;
                         }
                         catch (final Throwable th)
                         {
                         invocation.addResponseAttachment(THROWABLE_KEY, th) ;
                         try
                         {
                         return methodInvocation.invokeNext() ;
                         }
                         finally
                         {
                         invocation.getResponseContextInfo().remove(THROWABLE_KEY) ;
                         }
                         }
                         }
                        
                         private static Invocation getInvocation(final MethodInvocation methodInvocation)
                         {
                         return (Invocation)methodInvocation.getArguments()[0] ;
                         }
                        }


                        The POJO code
                        package knrc.beforeAfter2;
                        
                        /**
                         * @author kevin
                         */
                        public class Pojo
                        {
                         public void method1()
                         {
                         System.out.println("Pojo method1 here...") ;
                         }
                        
                         public static void main(final String args[])
                         {
                         new Pojo().method1() ;
                         new Aspect3D();
                         }
                        }


                        The POJO aspects (3D and 4D)
                        package knrc.beforeAfter2;
                        
                        import org.jboss.aop.joinpoint.Invocation;
                        
                        public class Aspect3D
                        {
                         /**
                         * @@knrc.beforeAfter2.before
                         */
                         public Object before(final Invocation invocation)
                         {
                         System.out.println("2-executing 3D operations") ;
                         databaseOperations() ;
                         return null ;
                         }
                        
                         private void databaseOperations()
                         {
                         throw new RuntimeException("X") ;
                         }
                        }


                        package knrc.beforeAfter2;
                        
                        import org.jboss.aop.joinpoint.MethodInvocation;
                        
                        public class AspectExceptionHandler4D
                        {
                         /**
                         * @@knrc.beforeAfter2.before
                         */
                         public Object before(final MethodInvocation methodInvocation)
                         {
                         System.out.println("1-Begin of 4D-Exception handler concern") ;
                         return null ;
                         }
                        
                         /**
                         * @@knrc.beforeAfter2.after
                         */
                         public Object after(final MethodInvocation methodInvocation)
                         {
                         System.out.println("After thrown exception - You shouldn't see this!!!") ;
                         return methodInvocation.getResponseAttachment(AspectBeforeAfter.RESULT_KEY) ;
                         }
                        
                         /**
                         * @@knrc.beforeAfter2.afterThrowable
                         */
                         public Object afterThrowable(final MethodInvocation methodInvocation)
                         throws Throwable
                         {
                         System.out.println("Handle 3D exceptions here") ;
                         return null ;
                         //throw (Throwable)methodInvocation.getResponseAttachment(AspectBeforeAfter.THROWABLE_KEY) ;
                         }
                        }


                        The aop.xml
                        <?xml version="1.0" encoding="UTF-8"?>
                        <aop>
                         <aspect name="aspect3D" class="knrc.beforeAfter2.Aspect3D"/>
                         <aspect name="aspectExceptionHandler4D" class="knrc.beforeAfter2.AspectExceptionHandler4D"/>
                         <aspect name="aspectBeforeAfter" class="knrc.beforeAfter2.AspectBeforeAfter"/>
                        
                         <bind pointcut="execution(* *->@knrc.beforeAfter2.before(..))">
                         <advice name="invokeBefore" aspect="aspectBeforeAfter"/>
                         </bind>
                         <bind pointcut="execution(* *->@knrc.beforeAfter2.after(..))">
                         <advice name="invokeAfter" aspect="aspectBeforeAfter"/>
                         </bind>
                         <bind pointcut="execution(* *->@knrc.beforeAfter2.afterThrowable(..))">
                         <advice name="invokeAfterThrowable" aspect="aspectBeforeAfter"/>
                         </bind>
                        
                         <bind pointcut="execution(* knrc.beforeAfter2.Pojo->method1())">
                         <advice name="before" aspect="aspect3D"/>
                         </bind>
                        
                         <bind pointcut="execution(* knrc.beforeAfter2.Aspect3D->before(..))">
                         <advice name="afterThrowable" aspect="aspectExceptionHandler4D"/>
                         <advice name="after" aspect="aspectExceptionHandler4D"/>
                         <advice name="before" aspect="aspectExceptionHandler4D"/>
                         </bind>
                        </aop>


                        This is still not as neat as the solution you were after but it is close. The only part to watch out for is the ordering of the 4D advices, the afterThrowable advice should be the first one so that the thrown throwable falls throught the after/before advices.

                        I'm sure there are other improvements that can be made ;-).

                        Kev


                        • 9. Re: Why lack of 'before' and 'after' advice types is bad..

                          Hi Kevin!

                          I was on short vacations, so forgive delay..

                          I'm impressed your patience for this topic. I've lost my energy for trying to explain why one needs 'before-after' types of advice.

                          When you get fully functional code for "emulation" of before-after, send it to JBossAOP CVS, so other app developers can use it transparently - this is suggestion for JBossAOP authors, if you haven't noticed ;)


                          Best regards,
                          Tomasz Nazar

                          PS. Thanks for the code..

                          • 10. Re: Why lack of 'before' and 'after' advice types is bad..
                            bill.burke

                            before/after returing/after throwing could easily be added to JBoss AOP with a day of work,

                            But....

                            Again, I still see this as nothing more than you wanting to avoid having a try/catch/finally block, or to avoid calling invocation.invokeNext(). IMNSHO, ordering of aspects and how the aspect is executed in the Java call stack is very important to understand as an aspect developer.

                            A smaller API is always better IMO. IMO, you're just talking about syntax sugar here and I'm still not convinced that the semantics of around don't give you what you need.

                            Bill

                            • 11. Re: Why lack of 'before' and 'after' advice types is bad..


                              Bill, will you agree with me that code duplication is something bad?


                              • 12. Re: Why lack of 'before' and 'after' advice types is bad..
                                kevinconner

                                Hiya guys.

                                I'm busy tracking down JacORB problems at the moment so missed this.

                                The code that I have given you is 'fully functional' in so far as it provides the advices that remove the code duplication. Give it a try, you may like it :-)

                                I tend to agree with Bill on the matter of simplicity. It is generally easier to keep a simpler framework fast and correct than an expanded one, it is usually easier to learn as well :-).

                                The tradeoff is in ease of development but, as I've tried to demonstrate, this can be achieved through other means. Both subclassing and aspects can be used to good effect.

                                The other advantage is that you can tailor this to fit your own needs. You are not constrained by a particular implementation of before/after as you have the code and can change it.

                                Kev

                                • 13. Re: Why lack of 'before' and 'after' advice types is bad..

                                   

                                  "KevinConner" wrote:
                                  Hiya guys.

                                  ... as it provides the advices that remove the code duplication....


                                  With full respect to your code Kevin (I use it):
                                  I will send you my custom code for using writting Strings to System.out. Why should we use System.out.println(String) method if we can use custom Utils.writeMyString(String) method which uses System.out.println(char) internally...


                                  I tend to agree with Bill on the matter of simplicity...

                                  I think we're on different sides of a mirror..

                                  What type of simplicity are you talking about?
                                  Simplicity of JBossAOP implementation (which I don't care in this case), simplicity of JBossAOP API (come on...JBossAOP has four ways of specifying joinpoints: AspectJ style, annotations in separate XML or within Java source and two additional methods/words won't blow it) or simplicity of using JBossAOP which is the one I care and is most important and it fails in some cases without after/before


                                  It is generally easier to keep a simpler framework fast and correct than an expanded one, it is usually easier to learn as well :-).


                                  The case we're discussing (types of advices) is simple to imagine. Come on..


                                  The tradeoff is in ease of development....


                                  Yeah, and I thought for a moment aspects and AOP frameworks were for ease of development..


                                  .. Both subclassing and aspects can be used to good effect...


                                  Yes, yes.. so let's use pure OO then. What for do we need AOP advantages if we can implement the same functionallity without AOP..


                                  The other advantage is that you can tailor this to fit your own needs. You are not constrained by a particular implementation of before/after as you have the code and can change it.


                                  I'm not sure if I understood this ..


                                  Kevin, thanks for your code. I've designed concerns and implemented them without having to use try-catch-finally in many places. My aim is to separate exception hangling which is a separate concern ("separation of concerns"). For this I need before/after functionallity.
                                  What about other people who will need similar functionallity as I? Will we tell them: use try-catch-finally or adopt Kev's sollution?


                                  This disscussion starts to be silly...


                                  Regards,
                                  Tomasz Nazar



                                  • 14. Re: Why lack of 'before' and 'after' advice types is bad..
                                    kevinconner

                                     

                                    What type of simplicity are you talking about?

                                    I was talking about the simplicity at runtime, i.e. calling the aspects, and not during bytecode annotation.

                                    The case we're discussing (types of advices) is simple to imagine. Come on..

                                    It is very simple to imagine and to implement but it is a syntactic modification to the interceptors, not a necessary one (which I think was Bill's point). There is no 'missing functionality' as the results can be achieved with the current code.

                                    Yeah, and I thought for a moment aspects and AOP frameworks were for ease of development..

                                    They are but you still have to write the aspect before it can be of use. Hopefully I have done that, or at least started, with the code I supplied.

                                    What about other people who will need similar functionallity as I? Will we tell them: use try-catch-finally or adopt Kev's sollution?

                                    I would hope that they would look at my solution, think about improving it and then use what they feel fits best.

                                    Think of my solution as the PrintWriter that surrounds the Writer ;-)

                                    Kev

                                    1 2 Previous Next