[jboss-cvs] JBossAS SVN: r58322 - projects/aop/trunk/aop/src/main/org/jboss/aop/proxy/container

jboss-cvs-commits at lists.jboss.org jboss-cvs-commits at lists.jboss.org
Mon Nov 13 19:37:52 EST 2006


Author: kabir.khan at jboss.com
Date: 2006-11-13 19:37:46 -0500 (Mon, 13 Nov 2006)
New Revision: 58322

Modified:
   projects/aop/trunk/aop/src/main/org/jboss/aop/proxy/container/AOPProxyFactoryParameters.java
   projects/aop/trunk/aop/src/main/org/jboss/aop/proxy/container/ContainerProxyFactory.java
   projects/aop/trunk/aop/src/main/org/jboss/aop/proxy/container/GeneratedAOPProxyFactory.java
   projects/aop/trunk/aop/src/main/org/jboss/aop/proxy/container/ProxyTemplate.java
Log:
[JBAOP-307] Solve NPE where mc instantiates a bean w/ ctor injection and the proxied class has no default ctors, and tries to use the params in its ctor by overriding all ctors in the proxy

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/proxy/container/AOPProxyFactoryParameters.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/proxy/container/AOPProxyFactoryParameters.java	2006-11-13 22:50:47 UTC (rev 58321)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/proxy/container/AOPProxyFactoryParameters.java	2006-11-14 00:37:46 UTC (rev 58322)
@@ -21,6 +21,8 @@
 */ 
 package org.jboss.aop.proxy.container;
 
+import java.lang.reflect.Constructor;
+
 import org.jboss.aop.metadata.SimpleMetaData;
 //import org.jboss.repository.spi.MetaDataContext;
 
@@ -41,6 +43,7 @@
    private boolean objectAsSuperClass;
    private SimpleMetaData simpleMetaData;
    private ContainerCache containerCache;
+   private Ctor ctor;
    
    public AOPProxyFactoryParameters()
    {
@@ -54,7 +57,9 @@
          Object context, 
          boolean objectAsSuperClass,
          SimpleMetaData simpleMetaData,
-         ContainerCache containerCache)
+         ContainerCache containerCache,
+         Class[] ctorSignature,
+         Object[] ctorArguments)
    {
       this.interfaces = interfaces;
       this.metaDataContext = context;
@@ -63,6 +68,7 @@
       this.target = target;
       this.simpleMetaData = simpleMetaData;
       this.containerCache = containerCache;
+      setCtor(ctorSignature, ctorArguments);
    }
 
    public Class[] getInterfaces()
@@ -144,6 +150,50 @@
    {
       this.containerCache = containerCache;
    }
+
+   public Ctor getCtor()
+   {
+      return ctor;
+   }
+
+   public void setCtor(Class[] ctorSignature, Object[] ctorArguments)
+   {
+      boolean haveSig = (ctorSignature != null && ctorSignature.length > 0);
+      boolean haveArgs = (ctorArguments != null && ctorArguments.length > 0);
+      
+      if (haveSig && haveArgs)
+      {
+         ctor = new Ctor(ctorSignature, ctorArguments);
+      }
+      else if (!haveSig && !haveArgs)
+      {
+         ctor = null;
+      }
+      else
+      {
+         throw new RuntimeException("If specifying either constructor arguments or signature, you must specify the other");
+      }
+   }
    
-   
+   public static class Ctor
+   {
+      Class[] signature;
+      Object[] arguments;
+      
+      public Ctor(Class[] signature, Object[] arguments)
+      {
+         this.signature = signature;
+         this.arguments = arguments;
+      }
+
+      public Object[] getArguments()
+      {
+         return arguments;
+      }
+
+      public Class[] getSignature()
+      {
+         return signature;
+      }
+   }
 }

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/proxy/container/ContainerProxyFactory.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/proxy/container/ContainerProxyFactory.java	2006-11-13 22:50:47 UTC (rev 58321)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/proxy/container/ContainerProxyFactory.java	2006-11-14 00:37:46 UTC (rev 58322)
@@ -21,7 +21,6 @@
   */
 package org.jboss.aop.proxy.container;
 
-import java.io.IOException;
 import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -31,6 +30,7 @@
 import java.util.Set;
 import java.util.WeakHashMap;
 
+import javassist.CannotCompileException;
 import javassist.ClassPool;
 import javassist.CtClass;
 import javassist.CtConstructor;
@@ -210,8 +210,7 @@
          proxy.addInterface(interfaze);
       }
       
-      addFieldFromTemplate(template, "_proxy_initialised");
-      ensureDefaultConstructor(superclass, proxy);
+      copyConstructors(superclass, proxy);
       addFieldFromTemplate(template, "mixins");
 
       //Add methods/fields needed for Delegate interface
@@ -328,53 +327,63 @@
       return method;
    }
    
-   private void ensureDefaultConstructor(CtClass superclass, CtClass proxy) throws Exception
+   private void copyConstructors(CtClass superclass, CtClass proxy) throws Exception
    {
-      
       CtConstructor[] ctors = superclass.getConstructors();
       int minParameters = Integer.MAX_VALUE;
       CtConstructor bestCtor = null;
       for (int i = 0 ; i < ctors.length ; i++)
       {
          CtClass[] params = ctors[i].getParameterTypes(); 
+         
+         CtConstructor ctor = CtNewConstructor.make(
+            ctors[i].getParameterTypes(),
+            ctors[i].getExceptionTypes(),
+            CtNewConstructor.PASS_PARAMS,
+            null,
+            null,
+            proxy);
+         ctor.setModifiers(ctors[i].getModifiers());
+         proxy.addConstructor(ctor);
+
          if (params.length < minParameters)
          {
-            bestCtor = ctors[i];
+            bestCtor = ctor;
             minParameters = params.length;
          }
+         if (params.length == 0)
+         {
+            defaultCtor = ctor;
+         }
       }
       
       if (minParameters > 0)
       {
          //We don't have a default constructor, we need to create one passing in null to the super ctor
-
-         //TODO We will get problems if the super class does some validation of the parameters, resulting in exceptions thrown
-         CtClass params[] = bestCtor.getParameterTypes();
-         
-         StringBuffer superCall = new StringBuffer("super(");
-         
-         for (int i = 0 ; i < params.length ; i++)
+         createDefaultConstructor(bestCtor);
+      }
+   }
+   
+   private void createDefaultConstructor(CtConstructor bestCtor) throws NotFoundException, CannotCompileException
+   {
+      CtClass params[] = bestCtor.getParameterTypes();
+      
+      StringBuffer superCall = new StringBuffer("super(");
+      
+      for (int i = 0 ; i < params.length ; i++)
+      {
+         if (i > 0)
          {
-            if (i > 0)
-            {
-               superCall.append(", ");
-            }
-            
-            superCall.append(getNullType(params[i]));
+            superCall.append(", ");
          }
          
-         superCall.append(");");
-         superCall.append("_proxy_initialised = true;");
-         
-         defaultCtor = CtNewConstructor.make(EMPTY_CTCLASS_ARRAY, EMPTY_CTCLASS_ARRAY, "{" + superCall.toString() + "}", proxy);
-         proxy.addConstructor(defaultCtor);
+         superCall.append(getNullType(params[i]));
       }
-      else
-      {
-         defaultCtor = CtNewConstructor.defaultConstructor(proxy);
-         defaultCtor.setBody("{_proxy_initialised = true;}");
-         proxy.addConstructor(defaultCtor);
-      }
+      
+      superCall.append(");");
+      
+      defaultCtor = CtNewConstructor.make(EMPTY_CTCLASS_ARRAY, EMPTY_CTCLASS_ARRAY, "{" + superCall.toString() + "}", proxy);
+      proxy.addConstructor(defaultCtor);
    }
 
    private String getNullType(CtClass clazz)
@@ -578,7 +587,7 @@
          
          if (m.getParameterTypes().length > 0) args = "$args";
          String code = "{   " +
-                       "    if (_proxy_initialised == false) {" +
+                       "    if (currentAdvisor == null) {" +
                        "       return " + getNullType(m.getReturnType()) + ";" +
                        "    }" +
                        "    try{" +

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/proxy/container/GeneratedAOPProxyFactory.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/proxy/container/GeneratedAOPProxyFactory.java	2006-11-13 22:50:47 UTC (rev 58321)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/proxy/container/GeneratedAOPProxyFactory.java	2006-11-14 00:37:46 UTC (rev 58322)
@@ -21,6 +21,8 @@
 */ 
 package org.jboss.aop.proxy.container;
 
+import java.lang.reflect.Constructor;
+
 import org.jboss.aop.Advised;
 import org.jboss.aop.AspectManager;
 import org.jboss.aop.instrument.Untransformable;
@@ -36,79 +38,88 @@
 {
    public Object createAdvisedProxy(AOPProxyFactoryParameters params)
    {
-      return createAdvisedProxy(
-            params.isObjectAsSuperClass(), 
-            params.getProxiedClass(), 
-            params.getInterfaces(), 
-            params.getMixins(),
-            params.getSimpleMetaData(), 
-            params.getTarget(),
-            params.getMetaDataContext(),
-            params.getContainerCache());
-   }
-   
-   //FIXME - make metaDataContext a MetaDataContext once MC 2.0 is released
-   private Object createAdvisedProxy(boolean objectAsSuper, Class proxiedClass, Class[] interfaces, 
-         AOPProxyFactoryMixin[] mixins, SimpleMetaData simpleMetaData, Object target, /*MetaDataContext*/Object metaDataContext, ContainerCache containerCache)
-   {
-      AspectManager manager = AspectManager.instance();
-      
-      if (target != null)
+//      return createAdvisedProxy(
+//            params.isObjectAsSuperClass(), 
+//            params.getProxiedClass(), 
+//            params.getInterfaces(), 
+//            params.getMixins(),
+//            params.getSimpleMetaData(), 
+//            params.getTarget(),
+//            params.getMetaDataContext(),
+//            params.getContainerCache(),
+//            params.getCtor(),
+//            params.getCtorArguments());
+//   }
+//   
+//   //FIXME - make metaDataContext a MetaDataContext once MC 2.0 is released
+//   private Object createAdvisedProxy(boolean objectAsSuper, Class proxiedClass, Class[] interfaces, 
+//         AOPProxyFactoryMixin[] mixins, SimpleMetaData simpleMetaData, Object target, /*MetaDataContext*/Object metaDataContext, ContainerCache containerCache, Constructor ctor, Object[] ctorArguments)
+//   {
+      if (params.getTarget() != null)
       {
-         if (proxiedClass != null)
+         if (params.getProxiedClass() != null)
          {
-            if (proxiedClass.isAssignableFrom(target.getClass()) == false)
+            if (params.getProxiedClass().isAssignableFrom(params.getTarget().getClass()) == false)
             {
-               throw new RuntimeException("Specified class type " + proxiedClass.getName() + " and target " + target.getClass().getName() + " are not compatible");
+               throw new RuntimeException("Specified class type " + params.getProxiedClass().getName() + " and target " + params.getTarget().getClass().getName() + " are not compatible");
             }
          }
          else
          {
-            proxiedClass = target.getClass();
+            params.setProxiedClass(params.getTarget().getClass());
          }
       }
-      else if (proxiedClass == null)
+      else if (params.getProxiedClass() == null)
       {
-         proxiedClass = Object.class;
+         params.setProxiedClass(Object.class);
       }
       
-      return getProxy(objectAsSuper, manager, proxiedClass, interfaces, mixins, simpleMetaData, target, metaDataContext, containerCache);
+//      return getProxy(objectAsSuper, manager, proxiedClass, interfaces, mixins, simpleMetaData, target, metaDataContext, containerCache, ctor, ctorArguments);
+      return getProxy(params);
    }
 
    //FIXME - make metaDataContext a MetaDataContext once MC 2.0 is released
-   private Object getProxy(boolean objectAsSuper, AspectManager manager, Class proxiedClass,  
-         Class[] interfaces, AOPProxyFactoryMixin[] mixins, SimpleMetaData simpleMetaData, Object target, /*MetaDataContext*/ Object metaDataContext, ContainerCache containerCache)
+//   private Object getProxy(boolean objectAsSuper, AspectManager manager, Class proxiedClass,  
+//         Class[] interfaces, AOPProxyFactoryMixin[] mixins, SimpleMetaData simpleMetaData, Object target, /*MetaDataContext*/ Object metaDataContext, ContainerCache containerCache, Constructor ctor, Object[] ctorArguments)
+   private Object getProxy(AOPProxyFactoryParameters params)
    {
       try
       {
          Class proxyClass = null;
          
-         boolean isAdvised = Advised.class.isAssignableFrom(proxiedClass);
+         boolean isAdvised = Advised.class.isAssignableFrom(params.getProxiedClass());
          
-         if (target instanceof Untransformable || (isAdvised && interfaces == null && mixins == null && metaDataContext == null && simpleMetaData == null))
+         if (params.getTarget() instanceof Untransformable || (isAdvised && params.getInterfaces() == null && params.getMixins() == null && params.getMetaDataContext() == null && params.getSimpleMetaData() == null))
          {
-            return target;
+            return params.getTarget();
          }
          
          
          synchronized (ContainerCache.mapLock)
          {
-            if (containerCache == null)
+            if (params.getContainerCache() == null)
             {
-               containerCache = ContainerCache.initialise(manager, proxiedClass, interfaces, mixins, metaDataContext, simpleMetaData);
+               params.setContainerCache(
+                     ContainerCache.initialise(
+                     AspectManager.instance(), 
+                     params.getProxiedClass(), 
+                     params.getInterfaces(), 
+                     params.getMixins(), 
+                     params.getMetaDataContext(), 
+                     params.getSimpleMetaData()));
             }
             
-            if (!containerCache.hasAspects() && !containerCache.requiresInstanceAdvisor())
+            if (!params.getContainerCache().hasAspects() && !params.getContainerCache().requiresInstanceAdvisor())
             {
-               return target;
+               return params.getTarget();
             }
             else
             {  
-               proxyClass = generateProxy(objectAsSuper, containerCache);
+               proxyClass = generateProxy(params);
             }
          }
          
-         return instantiateAndConfigureProxy(proxyClass, containerCache, simpleMetaData, target);
+         return instantiateAndConfigureProxy(proxyClass, params);
       }
       catch (Exception e)
       {
@@ -116,31 +127,42 @@
       }
    }
    
-   private Class generateProxy(boolean objectAsSuper, ContainerCache cache) throws Exception
+   private Class generateProxy(AOPProxyFactoryParameters params) throws Exception
    {
-      Class proxyClass = ContainerProxyFactory.getProxyClass(objectAsSuper, cache.getKey(), cache.getAdvisor());
+      Class proxyClass = ContainerProxyFactory.getProxyClass(params.isObjectAsSuperClass(), params.getContainerCache().getKey(), params.getContainerCache().getAdvisor());
       
       return proxyClass;
    }
    
-   private Object instantiateAndConfigureProxy(Class proxyClass, ContainerCache cache, SimpleMetaData metadata, Object target) throws Exception
+//   private Object instantiateAndConfigureProxy(Class proxyClass, ContainerCache cache, SimpleMetaData metadata, Object target, Constructor ctor, Object[] ctorArguments) throws Exception
+   private Object instantiateAndConfigureProxy(Class proxyClass, AOPProxyFactoryParameters params) throws Exception
    {
-      AspectManaged proxy = (AspectManaged)proxyClass.newInstance();
-      proxy.setAdvisor(cache.getClassAdvisor());
+      AspectManaged proxy;
+      if (params.getCtor() != null)
+      {
+         Constructor ctor = proxyClass.getConstructor(params.getCtor().getSignature());
+         proxy = (AspectManaged)ctor.newInstance(params.getCtor().getArguments());
+      }
+      else
+      {
+         proxy = (AspectManaged)proxyClass.newInstance();
+      }
+         
+      proxy.setAdvisor(params.getContainerCache().getClassAdvisor());
       
-      if (cache.getInstanceContainer() != null)
+      if (params.getContainerCache().getInstanceContainer() != null)
       {
-         proxy.setInstanceAdvisor(cache.getInstanceContainer());
+         proxy.setInstanceAdvisor(params.getContainerCache().getInstanceContainer());
       }
       
-      if (metadata != null)
+      if (params.getSimpleMetaData() != null)
       {
-         proxy.setMetadata(metadata);
+         proxy.setMetadata(params.getSimpleMetaData());
       }
       
-      if (target != null)
+      if (params.getTarget() != null)
       {
-         ((Delegate)proxy).setDelegate(target);
+         ((Delegate)proxy).setDelegate(params.getTarget());
       }
       else
       {

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/proxy/container/ProxyTemplate.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/proxy/container/ProxyTemplate.java	2006-11-13 22:50:47 UTC (rev 58321)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/proxy/container/ProxyTemplate.java	2006-11-14 00:37:46 UTC (rev 58322)
@@ -42,7 +42,6 @@
    private transient Advisor classAdvisor;
    private transient InstanceAdvisor instanceAdvisor;
    protected volatile transient Advisor currentAdvisor;
-   protected boolean _proxy_initialised;
    
    private Object delegate;
    private Object[] mixins; // Do not remove this




More information about the jboss-cvs-commits mailing list