[jboss-cvs] JBossAS SVN: r73173 - in projects/aop/trunk/aop/src: main/org/jboss/aop/advice and 4 other directories.

jboss-cvs-commits at lists.jboss.org jboss-cvs-commits at lists.jboss.org
Thu May 8 16:13:57 EDT 2008


Author: flavia.rainone at jboss.com
Date: 2008-05-08 16:13:57 -0400 (Thu, 08 May 2008)
New Revision: 73173

Modified:
   projects/aop/trunk/aop/src/main/org/jboss/aop/Advisor.java
   projects/aop/trunk/aop/src/main/org/jboss/aop/AspectManager.java
   projects/aop/trunk/aop/src/main/org/jboss/aop/ClassInstanceAdvisor.java
   projects/aop/trunk/aop/src/main/org/jboss/aop/GeneratedClassAdvisor.java
   projects/aop/trunk/aop/src/main/org/jboss/aop/GeneratedInstanceAdvisorMixin.java
   projects/aop/trunk/aop/src/main/org/jboss/aop/InstanceAdvisorDelegate.java
   projects/aop/trunk/aop/src/main/org/jboss/aop/advice/AdviceStack.java
   projects/aop/trunk/aop/src/main/org/jboss/aop/advice/AspectFactory.java
   projects/aop/trunk/aop/src/main/org/jboss/aop/advice/GeneratedAdvisorInterceptor.java
   projects/aop/trunk/aop/src/main/org/jboss/aop/advice/GenericAspectFactory.java
   projects/aop/trunk/aop/src/main/org/jboss/aop/advice/PerInstanceAdvice.java
   projects/aop/trunk/aop/src/main/org/jboss/aop/advice/PerInstanceInterceptor.java
   projects/aop/trunk/aop/src/main/org/jboss/aop/advice/PerJoinpointAdvice.java
   projects/aop/trunk/aop/src/main/org/jboss/aop/advice/PerJoinpointInterceptor.java
   projects/aop/trunk/aop/src/main/org/jboss/aop/advice/PerVmAdvice.java
   projects/aop/trunk/aop/src/main/org/jboss/aop/instrument/JoinPointGenerator.java
   projects/aop/trunk/aop/src/main/org/jboss/aop/proxy/container/MarshalledContainerProxy.java
   projects/aop/trunk/aop/src/resources/test/scope/jboss-aop.xml
   projects/aop/trunk/aop/src/test/org/jboss/test/aop/scope/ScopeTestCase.java
Log:
[JBAOP-560] Now AspectFactories can return null. Tests for this feature have been added to scope test suite.

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/Advisor.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/Advisor.java	2008-05-08 20:13:03 UTC (rev 73172)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/Advisor.java	2008-05-08 20:13:57 UTC (rev 73173)
@@ -839,6 +839,10 @@
    {
       if (aspects.containsKey(def.getName())) return;
       Object aspect = def.getFactory().createPerClass(this);
+      if (aspect == null)
+      {
+         return;
+      }
       aspects.put(def.getName(), aspect);
       def.registerAdvisor(this);
    }
@@ -886,7 +890,14 @@
          {
             throw new RuntimeException("Before/After/Throwing is only supported for Generated Advisors");
          }
-         if (factories[i].isDeployed()) newinterceptors.add(factories[i].create(this, joinpoint));
+         if (factories[i].isDeployed())
+         {
+            Interceptor interceptor = factories[i].create(this, joinpoint);
+            if (interceptor != null)
+            {
+               newinterceptors.add(interceptor);
+            }
+         }
       }
    }
 

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/AspectManager.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/AspectManager.java	2008-05-08 20:13:03 UTC (rev 73172)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/AspectManager.java	2008-05-08 20:13:57 UTC (rev 73173)
@@ -1886,7 +1886,10 @@
             }
             instance = adef.getFactory().createPerVM();
             initPerVMAspectsMap();
-            perVMAspects.put(def, instance);
+            if (instance != null)
+            {
+               perVMAspects.put(def, instance);
+            }
          }
          finally
          {
@@ -2523,5 +2526,4 @@
          }
       }
    }
-
 }

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/ClassInstanceAdvisor.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/ClassInstanceAdvisor.java	2008-05-08 20:13:03 UTC (rev 73172)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/ClassInstanceAdvisor.java	2008-05-08 20:13:57 UTC (rev 73173)
@@ -273,6 +273,10 @@
       {
          if (!factory.isDeployed()) continue;
          Interceptor interceptor = factory.create(classAdvisor, null);
+         if (interceptor == null)
+         {
+            continue;
+         }
          insertInterceptor(interceptor);
          interceptorsAdded ++;
       }
@@ -298,6 +302,10 @@
       {
          if (!factory.isDeployed()) continue;
          Interceptor interceptor = factory.create(classAdvisor, null);
+         if (interceptor == null)
+         {
+            continue;
+         }
          appendInterceptor(interceptor);
          interceptorsAdded ++;
       }
@@ -323,7 +331,10 @@
       {
          if (!factory.isDeployed()) continue;
          Interceptor interceptor = factory.create(classAdvisor, null);
-         interceptorsRemoved += internalRemoveInterceptor(interceptor.getName());
+         if (interceptor != null)
+         {
+            interceptorsRemoved += internalRemoveInterceptor(interceptor.getName());
+         }
       }
       if (interceptorChainObserver != null)
       {

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/GeneratedClassAdvisor.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/GeneratedClassAdvisor.java	2008-05-08 20:13:03 UTC (rev 73172)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/GeneratedClassAdvisor.java	2008-05-08 20:13:57 UTC (rev 73173)
@@ -891,7 +891,11 @@
 
       if (joinpoints.get(joinpoint) == null)
       {
-         joinpoints.put(joinpoint, def.getFactory().createPerJoinpoint(this, joinpoint));
+         Object aspect = def.getFactory().createPerJoinpoint(this, joinpoint);
+         if (aspect != null)
+         {
+            joinpoints.put(joinpoint, aspect);
+         }
       }
       def.registerAdvisor(this);
    }

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/GeneratedInstanceAdvisorMixin.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/GeneratedInstanceAdvisorMixin.java	2008-05-08 20:13:03 UTC (rev 73172)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/GeneratedInstanceAdvisorMixin.java	2008-05-08 20:13:57 UTC (rev 73173)
@@ -96,7 +96,12 @@
       {
          for (Interceptor icptr : insertedInterceptors)
          {
-            newlist.add(((GeneratedAdvisorInterceptor)icptr).create(null, null));
+            Interceptor interceptor = ((GeneratedAdvisorInterceptor)icptr).
+               create(null, null);
+            if (interceptor != null)
+            {
+               newlist.add(interceptor);
+            }
          }
       }
       if (appendedInterceptors != null && appendedInterceptors.size() > 0) 
@@ -120,7 +125,12 @@
       {
          for (Interceptor icptr : insertedInterceptors)
          {
-            newlist.add(((GeneratedAdvisorInterceptor)icptr).create(null, null));
+            Interceptor interceptor = ((GeneratedAdvisorInterceptor)icptr).
+               create(null, null);
+            if (interceptor != null)
+            {
+               newlist.add(interceptor);
+            }
          }
       }
       if (advisorChain != null)
@@ -305,6 +315,10 @@
       {
          if (!factory.isDeployed()) continue;
          Interceptor interceptor = factory.create(classAdvisor, null);
+         if (interceptor == null)
+         {
+            continue;
+         }
          insertInterceptor(interceptor);
          interceptorsAdded ++;
       }
@@ -331,6 +345,10 @@
       {
          if (!factory.isDeployed()) continue;
          Interceptor interceptor = factory.create(classAdvisor, null);
+         if (interceptor == null)
+         {
+            continue;
+         }
          appendInterceptor(interceptor);
          interceptorsAdded ++;
       }
@@ -357,7 +375,10 @@
       {
          if (!factory.isDeployed()) continue;
          Interceptor interceptor = factory.create(classAdvisor, null);
-         interceptorsRemoved += internalRemoveInterceptor(interceptor.getName());
+         if (interceptor != null)
+         {
+            interceptorsRemoved += internalRemoveInterceptor(interceptor.getName());
+         }
       }
       if (interceptorChainObserver != null)
       {

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/InstanceAdvisorDelegate.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/InstanceAdvisorDelegate.java	2008-05-08 20:13:03 UTC (rev 73172)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/InstanceAdvisorDelegate.java	2008-05-08 20:13:57 UTC (rev 73173)
@@ -150,7 +150,12 @@
       Set<Joinpoint> joinpoints = jpAspects.get(def);
       for (Joinpoint joinpoint : joinpoints)
       {
-         joins.put(joinpoint, def.getFactory().createPerJoinpoint(getClassAdvisor(), instanceAdvisor, joinpoint));
+         Object aspect = def.getFactory().createPerJoinpoint(getClassAdvisor(),
+               instanceAdvisor, joinpoint);
+         if (aspect != null)
+         {
+            joins.put(joinpoint, aspect);
+         }
       }
    }
    
@@ -174,25 +179,24 @@
             return aspects.get(def);
          }
       }
-      Object aspect = aspects.get(def);
-      if (aspect == null)
+      if (!aspects.containsKey(def))
       {
          synchronized (this) // doublecheck, but I don't want to synchronize everywhere and dynamic aspects are rare
          {
-            aspect = aspects.get(def);
-            if (aspect != null) return aspect;
+            if (aspects.containsKey(def)) return aspects.get(def);
             if (classAdvisor != null && getClassAdvisor() instanceof ClassAdvisor)
             {
                ClassAdvisor cadvisor = (ClassAdvisor) getClassAdvisor();
                cadvisor.getPerInstanceAspectDefinitions().add(def);
-               aspect = def.getFactory().createPerInstance(null, null);
+               Object aspect = def.getFactory().createPerInstance(null, null);
                WeakHashMap<AspectDefinition, Object> copy = new WeakHashMap<AspectDefinition, Object>(aspects);
                copy.put(def, aspect);
                aspects = copy;
+               return aspect;
             }
          }
       }
-      return aspect;
+      return aspects.get(def);
    }
 
    public Object getPerInstanceJoinpointAspect(Joinpoint joinpoint, AspectDefinition def)
@@ -215,6 +219,10 @@
                ClassAdvisor cadvisor = (ClassAdvisor) getClassAdvisor();
                cadvisor.addPerInstanceJoinpointAspect(joinpoint, def);
                aspect = def.getFactory().createPerJoinpoint(getClassAdvisor(), instanceAdvisor, joinpoint);
+               if (aspect == null)
+               {
+                  return null;
+               }
                WeakHashMap<AspectDefinition, ConcurrentHashMap<Joinpoint, Object>> copy = new WeakHashMap<AspectDefinition, ConcurrentHashMap<Joinpoint, Object>>(joinpointAspects);
                Map<Joinpoint, Object> map = copy.get(def);
                if (map == null)

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/advice/AdviceStack.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/advice/AdviceStack.java	2008-05-08 20:13:03 UTC (rev 73172)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/advice/AdviceStack.java	2008-05-08 20:13:57 UTC (rev 73173)
@@ -52,7 +52,14 @@
       for (int i = 0; i < interceptorFactories.size(); i++)
       {
          InterceptorFactory factory = interceptorFactories.get(i);
-         if (factory.isDeployed()) interceptors.add(factory.create(advisor, jp));
+         if (factory.isDeployed())
+         {
+            Interceptor interceptor = factory.create(advisor, jp);
+            if (interceptor != null)
+            {
+               interceptors.add(interceptor);
+            }
+         }
       }
       return interceptors.toArray(new Interceptor[interceptors.size()]);
    }

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/advice/AspectFactory.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/advice/AspectFactory.java	2008-05-08 20:13:03 UTC (rev 73172)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/advice/AspectFactory.java	2008-05-08 20:13:57 UTC (rev 73173)
@@ -57,7 +57,8 @@
     * Creates an aspect with scope value {@link Scope#PER_VM}.
     * 
     * @return the single aspect instance that will be invoked for all applicable
-    *         joinpoints during  Java VM execution
+    *         joinpoints during  Java VM execution. If {@code null}, the aspect
+    *         represented by this factory is ignored, resulting in no interception.
     */
    Object createPerVM();
    
@@ -67,7 +68,10 @@
     * @param advisor manages all the interceptions that should occur during execution
     *                of a specific class
     * @return        the aspect instance that will be invoked for all applicable
-    *                joinpoints contained in the class managed by {@code advisor}
+    *                joinpoints contained in the class managed by {@code advisor}. If
+    *                {@code null}, the aspect represented by this factory is ignored,
+    *                resulting in no interception of the joinpoints contained in the
+    *                referred class.
     * @see Advisor#getClazz() 
     */
    Object createPerClass(Advisor advisor);
@@ -83,7 +87,10 @@
     *                        {@code advisor}
     * @return                the aspect instance that will be invoked for all
     *                        applicable joinpoints contained in the instance managed
-    *                        by {@code instanceAdvisor}
+    *                        by {@code instanceAdvisor}. If {@code null}, the aspect
+    *                        represented by this factory is ignored, resulting in no
+    *                        interception of the joinpoints contained in the referred
+    *                        instance.
     * @see Advisor#getClazz()
     * @see InstanceAdvisor#getInstance()
     */
@@ -105,7 +112,8 @@
     *                This joinpoint is contained in the class managed by
     *                {@code advisor}
     * @return        the aspect instance that will be invoked only to intercept
-    *                {@code jp}
+    *                {@code jp}. If {@code null}, the aspect represented by this
+    *                factory is ignored, resulting in no interception of {@code jp}.
     * @see Advisor#getClazz()
     * @see Joinpoint
     */
@@ -128,7 +136,10 @@
     *                        {@code advisor}
     * @return                the aspect instance that will be invoked only to
     *                        intercept {@code jp} when it happens on the instance
-    *                        managed by {@code instanceAdvisor}
+    *                        managed by {@code instanceAdvisor}. If {@code null}, the
+    *                        aspect represented by this factory is ignored, resulting
+    *                        in no interception of {@code jp} when it is executed in
+    *                        the context of the referred instance.
     * @see Advisor#getClazz()
     * @see InstanceAdvisor#getInstance()
     * @see Joinpoint

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/advice/GeneratedAdvisorInterceptor.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/advice/GeneratedAdvisorInterceptor.java	2008-05-08 20:13:03 UTC (rev 73172)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/advice/GeneratedAdvisorInterceptor.java	2008-05-08 20:13:57 UTC (rev 73173)
@@ -423,6 +423,10 @@
             }
          }
       }
+      if (lazyInterceptor == null)
+      {
+         return invocation.invokeNext();
+      }
       return lazyInterceptor.invoke(invocation);
    }
 

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/advice/GenericAspectFactory.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/advice/GenericAspectFactory.java	2008-05-08 20:13:03 UTC (rev 73172)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/advice/GenericAspectFactory.java	2008-05-08 20:13:57 UTC (rev 73173)
@@ -45,7 +45,7 @@
  * @version $Revision$
  */
 public class GenericAspectFactory extends AspectFactoryWithClassLoaderSupport
-{
+{   
    private static final Logger logger = AOPLogger.getLogger(GenericAspectFactory.class);
    
    final static Class<?>[] ADVISOR_INJECTOR_SIGNATURE = new Class[]{Advisor.class};

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/advice/PerInstanceAdvice.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/advice/PerInstanceAdvice.java	2008-05-08 20:13:03 UTC (rev 73172)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/advice/PerInstanceAdvice.java	2008-05-08 20:13:57 UTC (rev 73173)
@@ -105,7 +105,10 @@
           }
           aspect = instanceAdvisor.getPerInstanceAspect(aspectDefinition);
       }
-      
+      if (aspect == null)
+      {
+         return invocation.invokeNext();
+      }
       if (!initialized)
       {
          init(adviceName, aspect.getClass());

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/advice/PerInstanceInterceptor.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/advice/PerInstanceInterceptor.java	2008-05-08 20:13:03 UTC (rev 73172)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/advice/PerInstanceInterceptor.java	2008-05-08 20:13:57 UTC (rev 73173)
@@ -58,54 +58,61 @@
          //to make sure that there is only one instance per target rather than caller
          Object callingObject = ((CallerInvocation) invocation).getCallingObject();
 
-         if (callingObject == null) return invocation.invokeNext(); // called from static method
-         
-         Advised advised = (Advised) callingObject;
-         InstanceAdvisor advisor = advised._getInstanceAdvisor();
-         Interceptor interceptor = (Interceptor) advisor.getPerInstanceAspect(aspectDefinition);
-         return interceptor.invoke(invocation);         
-         
+         if (callingObject != null) // called from static method
+         {
+            Advised advised = (Advised) callingObject;
+            InstanceAdvisor advisor = advised._getInstanceAdvisor();
+            Interceptor interceptor = (Interceptor) advisor.getPerInstanceAspect(aspectDefinition);
+            if (interceptor != null)
+            {
+               return interceptor.invoke(invocation);
+            }
+         }
       }
       else
       {
          Object targetObject = invocation.getTargetObject();
-         if (targetObject == null) return invocation.invokeNext(); // static method call or static field call
-
          InstanceAdvisor instanceAdvisor = null;
-         if (targetObject instanceof Advised)
+         // non-static method call or non-static field call
+         if (targetObject != null) 
          {
-            Advised advised = (Advised) targetObject;
-            instanceAdvisor = advised._getInstanceAdvisor();
+            instanceAdvisor = getInstanceAdvisor(invocation, targetObject);
          }
-         else
+         if (instanceAdvisor != null)
          {
-            Advisor advisor = invocation.getAdvisor();
-            if (advisor == null)
+            Interceptor interceptor = getAspectInstance(instanceAdvisor);
+            if (interceptor != null)
             {
-               return invocation.invokeNext();
+               return interceptor.invoke(invocation);
             }
-            
-            if (advisor instanceof InstanceAdvisor)
-            {
-               instanceAdvisor = (InstanceAdvisor) advisor;
-            }
-            else 
-            {
-               if (advisor instanceof ClassProxyContainer && invocation instanceof ContainerProxyMethodInvocation)
-               {
-                  ContainerProxyMethodInvocation pi = (ContainerProxyMethodInvocation)invocation;
-                  instanceAdvisor = pi.getProxy().getInstanceAdvisor();
-               }
-               else
-               {
-                  return invocation.invokeNext();
-               }
-            }
          }
-         Interceptor interceptor = getAspectInstance(instanceAdvisor);
-         return interceptor.invoke(invocation);
       }
+      return invocation.invokeNext(); 
    }
+
+   private InstanceAdvisor getInstanceAdvisor(Invocation invocation, Object targetObject)
+   {
+      if (targetObject instanceof Advised)
+      {
+         Advised advised = (Advised) targetObject;
+         return advised._getInstanceAdvisor();
+      }
+      Advisor advisor = invocation.getAdvisor();
+      if (advisor != null)
+      {
+         if (advisor instanceof InstanceAdvisor)
+         {
+            return (InstanceAdvisor) advisor;
+         }
+         if (advisor instanceof ClassProxyContainer && 
+               invocation instanceof ContainerProxyMethodInvocation)
+         {
+            ContainerProxyMethodInvocation pi = (ContainerProxyMethodInvocation)invocation;
+            return pi.getProxy().getInstanceAdvisor();
+         }
+      }
+      return null;
+   }
    
    public Interceptor getAspectInstance(InstanceAdvisor ia)
    {

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/advice/PerJoinpointAdvice.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/advice/PerJoinpointAdvice.java	2008-05-08 20:13:03 UTC (rev 73172)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/advice/PerJoinpointAdvice.java	2008-05-08 20:13:57 UTC (rev 73173)
@@ -156,8 +156,11 @@
             }
          }
          aspect = instanceAdvisor.getPerInstanceJoinpointAspect(joinpoint, aspectDefinition);
-      }  
-      
+      }
+      if (aspect == null)
+      {
+         return invocation.invokeNext();
+      }
       if (!initialized)
       {
          init(adviceName, aspect.getClass());

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/advice/PerJoinpointInterceptor.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/advice/PerJoinpointInterceptor.java	2008-05-08 20:13:03 UTC (rev 73172)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/advice/PerJoinpointInterceptor.java	2008-05-08 20:13:57 UTC (rev 73173)
@@ -111,51 +111,62 @@
          //TODO: Naive implementation. Ideally callers should be able to look up the aspect by target instance
          //to make sure that there is only one instance per target rather than caller
          Object callingObject = ((CallerInvocation) invocation).getCallingObject();
-
-         if (callingObject == null) return invocation.invokeNext(); // called from static method
+         // called from non-static method
+         if (callingObject != null)
+         {
+            Advised advised = (Advised) callingObject;
+            InstanceAdvisor advisor = advised._getInstanceAdvisor();
+            Interceptor interceptor = (Interceptor) advisor.getPerInstanceJoinpointAspect(joinpoint, aspectDefinition);
+            if (interceptor != null)
+            {
+               return interceptor.invoke(invocation);
+            }
+         }
          
-         Advised advised = (Advised) callingObject;
-         InstanceAdvisor advisor = advised._getInstanceAdvisor();
-         Interceptor interceptor = (Interceptor) advisor.getPerInstanceJoinpointAspect(joinpoint, aspectDefinition);
-         return interceptor.invoke(invocation);         
-         
       }
       else
       {
          Object targetObject = invocation.getTargetObject();
-         if (targetObject == null) return invocation.invokeNext(); // static method call or static field call
-
-         InstanceAdvisor instanceAdvisor = null;
-         if (targetObject instanceof Advised)
+         // non-static method call or non-static field call
+         if (targetObject != null)
          {
-            Advised advised = (Advised) targetObject;
-            instanceAdvisor = advised._getInstanceAdvisor();
-         }
-         else
-         {
-            Advisor advisor = invocation.getAdvisor();
-            if (advisor == null)
+            InstanceAdvisor instanceAdvisor = getInstanceAdvisor(invocation, targetObject);
+            if (instanceAdvisor != null)
             {
-               return invocation.invokeNext();
+               Interceptor interceptor = getAspectInstance(instanceAdvisor);
+               if (interceptor != null)
+               {
+                  return interceptor.invoke(invocation);
+               }
             }
-            else if (advisor instanceof InstanceAdvisor)
-            {
-               instanceAdvisor = (InstanceAdvisor) advisor;
-            }
-            else if (advisor instanceof ClassProxyContainer && invocation instanceof ContainerProxyMethodInvocation)
-            {
-               ContainerProxyMethodInvocation pi = (ContainerProxyMethodInvocation)invocation;
-               instanceAdvisor = pi.getProxy().getInstanceAdvisor();
-            }
-            else
-            {
-               return invocation.invokeNext();
-            }
          }
-         Interceptor interceptor = getAspectInstance(instanceAdvisor);
-         return interceptor.invoke(invocation);         
       }
+      return invocation.invokeNext();
    }
+
+   private InstanceAdvisor getInstanceAdvisor(Invocation invocation, Object targetObject)
+   {
+      if (targetObject instanceof Advised)
+      {
+         Advised advised = (Advised) targetObject;
+         return advised._getInstanceAdvisor();
+      }
+      Advisor advisor = invocation.getAdvisor();
+      if (advisor == null)
+      {
+         return null;
+      }
+      if (advisor instanceof InstanceAdvisor)
+      {
+         return (InstanceAdvisor) advisor;
+      }
+      if (advisor instanceof ClassProxyContainer && invocation instanceof ContainerProxyMethodInvocation)
+      {
+         ContainerProxyMethodInvocation pi = (ContainerProxyMethodInvocation)invocation;
+         return pi.getProxy().getInstanceAdvisor();
+      }
+      return null;
+   }
    
    public Interceptor getAspectInstance(InstanceAdvisor ia)
    {

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/advice/PerVmAdvice.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/advice/PerVmAdvice.java	2008-05-08 20:13:03 UTC (rev 73172)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/advice/PerVmAdvice.java	2008-05-08 20:13:57 UTC (rev 73173)
@@ -49,12 +49,20 @@
    public static synchronized Interceptor generateOptimized(Joinpoint joinpoint, AspectManager manager, String adviceName, AspectDefinition a) throws Exception
    {
       Object aspect = manager.getPerVMAspect(a);
+      if (aspect == null)
+      {
+         return null;
+      }
       return generateInterceptor(joinpoint, aspect, adviceName);
 
    }
 
    public static Interceptor generateInterceptor(Joinpoint joinpoint, Object aspect, String adviceName) throws Exception
    {
+      if (aspect == null)
+      {
+         return null;
+      }
       ClassLoader cl = aspect.getClass().getClassLoader();
       String name = "org.jboss.aop.advice." + aspect.getClass().getName() + "_z_" + adviceName + "_" + System.identityHashCode(cl);
       Class<?> iclass = null;

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/instrument/JoinPointGenerator.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/instrument/JoinPointGenerator.java	2008-05-08 20:13:03 UTC (rev 73172)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/instrument/JoinPointGenerator.java	2008-05-08 20:13:57 UTC (rev 73173)
@@ -28,6 +28,7 @@
 import java.security.PrivilegedExceptionAction;
 import java.security.ProtectionDomain;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -511,7 +512,7 @@
    private AdviceSetups initialiseAdviceInfosAndAddFields(ClassPool pool, CtClass clazz, JoinPointInfo info) throws ClassNotFoundException, NotFoundException, CannotCompileException
    {
       HashMap<String, Integer> cflows = new HashMap<String, Integer>();
-      AdviceSetup[] setups = new AdviceSetup[info.getInterceptors().length];
+      Collection<AdviceSetup> setups = new ArrayList<AdviceSetup>(info.getInterceptors().length);
       
       ClassLoader classLoader = pool.getClassLoader();
       if (classLoader == null)
@@ -522,23 +523,33 @@
 
       for (int i = 0 ; i < info.getInterceptors().length ; i++)
       {
-         setups[i] = new AdviceSetup(i, (GeneratedAdvisorInterceptor)info.getInterceptors()[i], info, classLoader);
-         addAspectFieldAndGetter(pool, clazz, setups[i]);
-         addCFlowFieldsAndGetters(pool, setups[i], clazz, cflows);
+         AdviceSetup setup = new AdviceSetup(i, (GeneratedAdvisorInterceptor)info.getInterceptors()[i], info, classLoader);
+         if (!setup.shouldInvokeAspect())
+         {
+            continue;
+         }
+         setups.add(setup);
+         addAspectFieldAndGetter(pool, clazz, setup);
+         addCFlowFieldsAndGetters(pool, setup, clazz, cflows);
       }
       addLightweightInstanceAspectsTrackerFields(clazz);
-      return new AdviceSetups(info, setups);
+      AdviceSetup[] setupArray = setups.toArray(new AdviceSetup[setups.size()]);
+      return new AdviceSetups(info, setupArray);
    }
    
+   /**
+    * 
+    * @param pool
+    * @param clazz
+    * @param setup represents an advice that should be invoked (i.e.,
+    *              {@code setup.shouldBeInvoked()} is {@code true})
+    * @throws NotFoundException
+    * @throws CannotCompileException
+    */
    private void addAspectFieldAndGetter(ClassPool pool, CtClass clazz, AdviceSetup setup) throws NotFoundException, CannotCompileException
    {
       CtClass aspectClass = setup.getAspectCtClass();
       
-      if (!setup.shouldInvokeAspect())
-      {
-         return;
-      }
-
       CtField field = new CtField(aspectClass, setup.getAspectFieldName(), clazz);
       field.setModifiers(Modifier.PRIVATE | Modifier.TRANSIENT);
       clazz.addField(field);
@@ -1125,11 +1136,6 @@
       AdviceSetup[] allSetups = setups.getAllSetups();
       for (int i = 0 ; i < allSetups.length ; i++)
       {
-         if (!allSetups[i].shouldInvokeAspect())
-         {
-            continue;
-         }
-         
          if (allSetups[i].requiresInstanceAdvisor())
          {
             adviceInit.append(allSetups[i].getAspectFieldName());
@@ -1389,14 +1395,19 @@
          if (ifw.isAspectFactory())
          {
             Object aspectInstance = ((GeneratedAdvisorInterceptor)info.getInterceptors()[index]).getAspect(info.getAdvisor(), info.getJoinpoint(), true);
-            aspectClass = aspectInstance.getClass();
+            if (aspectInstance != null)
+            {
+               aspectClass = aspectInstance.getClass();
+            }
          }
          else
          {
             aspectClass = classLoader.loadClass(ifw.getAspectClassName());
          }
-         aspectCtClass = ReflectToJavassist.classToJavassist(aspectClass);
-
+         if (aspectClass != null)
+         {
+            aspectCtClass = ReflectToJavassist.classToJavassist(aspectClass);
+         }
          type = ifw.getType();
       }
       
@@ -1459,7 +1470,7 @@
       
       boolean shouldInvokeAspect()
       {
-         return !(isPerInstance() && isStaticCall());
+         return !((isPerInstance() && isStaticCall()) || aspectClass == null);
       }
       
       boolean requiresInstanceAdvisor()
@@ -1572,7 +1583,14 @@
       List<AdviceSetup> lightweightAdvicesRequiringInstanceAdvisor;
       boolean hasAroundAdvices;
       boolean hasArgsAroundAdvices;
-      
+
+      /**
+       * 
+       * @param info
+       * @param allSetups contains only setups that represent advices to be invoked
+       *                  (i.e., allSetups[i].shouldInvoke() equals true for
+       *                  0 <= i <= allSetups.length)
+       */
       AdviceSetups(JoinPointInfo info, AdviceSetup[] allSetups)
       {
          this.allSetups = allSetups;
@@ -1581,10 +1599,6 @@
          
          for (int i = 0 ; i < allSetups.length ; i++)
          {
-            if (!allSetups[i].shouldInvokeAspect())
-            {
-               continue;
-            }
             AdviceMethodProperties properties = getAdviceMethodProperties(info, allSetups[i]);
             AdviceType type = allSetups[i].getType();
             int index = type.ordinal();
@@ -1742,6 +1756,18 @@
       }
       
       protected abstract String generateKey(JoinPointGenerator generator);
+      /**
+       * 
+       * @param setup represents an advice that should be invoked (i.e.,
+       *              {@code setup.shouldBeInvoked()} is {@code true})
+       * @param key
+       * @param beforeCall
+       * @param call
+       * @param generator
+       * @param info
+       * @return
+       * @throws NotFoundException
+       */
       protected abstract boolean appendAdviceCall(AdviceSetup setup, String key,
            StringBuffer beforeCall, StringBuffer call, JoinPointGenerator generator, JoinPointInfo info) throws NotFoundException;
       
@@ -1889,16 +1915,10 @@
          }
          return "return ($w)";
       }
-      
+
       public boolean appendAdviceCall(AdviceSetup setup, String key,
             StringBuffer beforeCall, StringBuffer call, JoinPointGenerator generator, JoinPointInfo info)
       {
-         if (!setup.shouldInvokeAspect())
-         {
-            //We are invoking a static method/ctor, do not include advice in chain
-            return false;
-         }
-      
          boolean result = false;
          
          AdviceMethodProperties properties = setup.getAdviceMethodProperties();

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/proxy/container/MarshalledContainerProxy.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/proxy/container/MarshalledContainerProxy.java	2008-05-08 20:13:03 UTC (rev 73172)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/proxy/container/MarshalledContainerProxy.java	2008-05-08 20:13:57 UTC (rev 73173)
@@ -406,7 +406,10 @@
                   {
                      icptr = icptrs[i];
                   }
-                  allIcptrs.add(icptr);
+                  if (icptr != null)
+                  {
+                     allIcptrs.add(icptr);
+                  }
                }
                else
                {

Modified: projects/aop/trunk/aop/src/resources/test/scope/jboss-aop.xml
===================================================================
--- projects/aop/trunk/aop/src/resources/test/scope/jboss-aop.xml	2008-05-08 20:13:03 UTC (rev 73172)
+++ projects/aop/trunk/aop/src/resources/test/scope/jboss-aop.xml	2008-05-08 20:13:57 UTC (rev 73173)
@@ -21,7 +21,7 @@
    <bind pointcut="execution(* org.jboss.test.aop.scope.POJO*WithAspectFactory->perVm*(..))">
       <advice aspect="vmAFactory" name="advice"/>
    </bind>
-
+   
    <!-- PER CLASS -->
    <interceptor name="classInterceptor" class="org.jboss.test.aop.scope.PerClassInterceptor" scope="PER_CLASS"/>
    <aspect name="classAspect" class="org.jboss.test.aop.scope.PerClassAspect" scope="PER_CLASS"/>
@@ -110,13 +110,55 @@
       <advice aspect="joinpointAFactory" name="advice"/>
    </bind>
 
+   <!-- NULL ASPECT/INTERCEPTOR FACTORY -->
+
+   <interceptor name="vmNullIFactory" factory="org.jboss.test.aop.scope.NullAspectFactory"/>
+   <aspect name="vmNullFactory" factory="org.jboss.test.aop.scope.NullAspectFactory"/>
+   <interceptor name="classNullIFactory" factory="org.jboss.test.aop.scope.NullAspectFactory" scope="PER_CLASS"/>
+   <aspect name="classNullFactory" factory="org.jboss.test.aop.scope.NullAspectFactory" scope="PER_CLASS"/>
+   <interceptor name="classJoinpointNullIFactory" factory="org.jboss.test.aop.scope.NullAspectFactory" scope="PER_CLASS_JOINPOINT"/>
+   <aspect name="classJoinpointNullFactory" factory="org.jboss.test.aop.scope.NullAspectFactory" scope="PER_CLASS_JOINPOINT"/>
+   <interceptor name="instanceNullIFactory" factory="org.jboss.test.aop.scope.NullAspectFactory" scope="PER_INSTANCE"/>
+   <aspect name="instanceNullFactory" factory="org.jboss.test.aop.scope.NullAspectFactory" scope="PER_INSTANCE"/>
+   <interceptor name="joinpointNullIFactory" factory="org.jboss.test.aop.scope.NullAspectFactory" scope="PER_JOINPOINT"/>
+   <aspect name="joinpointNullFactory" factory="org.jboss.test.aop.scope.NullAspectFactory" scope="PER_JOINPOINT"/>
+   
+   <stack name="nullFactoryStack">
+      <interceptor-ref name="vmNullIFactory"/>
+      <advice aspect="vmNullFactory" name="anyAdvice"/>
+      <interceptor-ref name="classNullIFactory"/>
+      <advice aspect="classNullFactory" name="anyAdvice"/>
+      <interceptor-ref name="classJoinpointNullIFactory"/>
+      <advice aspect="classJoinpointNullFactory" name="anyAdvice"/>
+      <interceptor-ref name="instanceNullIFactory"/>
+      <advice aspect="instanceNullFactory" name="anyAdvice"/>
+      <interceptor-ref name="joinpointNullIFactory"/>
+      <advice aspect="joinpointNullFactory" name="anyAdvice"/>
+   </stack>
+   
+   <bind pointcut="field(int org.jboss.test.aop.scope.POJOWithNullAspect->*)">
+      <stack-ref name="nullFactoryStack"/>
+   </bind>
+   
+   <bind pointcut="construction(org.jboss.test.aop.scope.POJOWithNullAspect->new(..))">
+      <stack-ref name="nullFactoryStack"/>
+   </bind>
+   
+   <bind pointcut="execution(org.jboss.test.aop.scope.POJOWithNullAspect->new(..))">
+      <stack-ref name="nullFactoryStack"/>
+   </bind>
+   
+   <bind pointcut="execution(* org.jboss.test.aop.scope.POJOWithNullAspect->*(..))">
+      <stack-ref name="nullFactoryStack"/>
+   </bind>
+   
    <!-- PER_INSTANCE AND PER_CLASS -->
    <bind pointcut="execution(* org.jboss.test.aop.scope.POJOWithInstanceAndClass->method(..))">
       <interceptor-ref name="joinpointInterceptor"/>
       <advice aspect="classJoinpointAFactory" name="advice"/>
       <advice aspect="instanceAFactory" name="advice"/>
    </bind>
-
+   
    <!-- MIXED CHAINS -->   
    <stack name="mixed-stack">
       <interceptor-ref name="joinpointInterceptor"/>
@@ -142,6 +184,11 @@
    <bind pointcut="call(* org.jboss.test.aop.scope.CalledPOJO->*(..)) AND within(org.jboss.test.aop.scope.CallingPOJO)">
       <stack-ref name="mixed-stack"/>
    </bind>
+   
+   <bind pointcut="execution(* org.jboss.test.aop.scope.POJOWithNullAspect->mixedMethod(..))">
+      <stack-ref name="nullFactoryStack"/>
+      <interceptor-ref name="vmIFactory"/>
+   </bind>
 
    <!-- MIXED CHAINS WITH COPY INVOCATION -->
    <interceptor class="org.jboss.test.aop.scope.CopyInterceptor"/>

Modified: projects/aop/trunk/aop/src/test/org/jboss/test/aop/scope/ScopeTestCase.java
===================================================================
--- projects/aop/trunk/aop/src/test/org/jboss/test/aop/scope/ScopeTestCase.java	2008-05-08 20:13:03 UTC (rev 73172)
+++ projects/aop/trunk/aop/src/test/org/jboss/test/aop/scope/ScopeTestCase.java	2008-05-08 20:13:57 UTC (rev 73173)
@@ -552,6 +552,27 @@
    }
 
    
+   public void testNullAspectFactory()
+   {
+      System.out.println("RUNNING TEST NULL ASPECT FACTORY");
+      Interceptions.clear();
+      assertEquals(0, Interceptions.size());
+      int value = POJOWithNullAspect.staticField;
+      POJOWithNullAspect.staticField = value + 1;
+      POJOWithNullAspect.staticMethod();
+      
+      POJOWithNullAspect pojo = new POJOWithNullAspect();
+      value += pojo.field;
+      pojo.field = value - 10;
+      pojo.method();
+      
+      assertEquals(0, Interceptions.size());
+      pojo.mixedMethod();
+      assertEquals(1, Interceptions.size());
+      System.out.println("FINISHED TEST NULL ASPECT FACTORY");
+      System.out.println();
+   }
+   
    final static String[] staticNames = {
          PerJoinpointInterceptor.class.getName(),
          PerVmInterceptor.class.getName(), 




More information about the jboss-cvs-commits mailing list