Christian Meder wrote:
On Thu, 2005-11-17 at 03:40 +0000, [EMAIL PROTECTED] wrote:
Author: laurieh
Date: Wed Nov 16 19:40:50 2005
New Revision: 345179

URL: http://svn.apache.org/viewcvs?rev=345179&view=rev
Log:
Reworking DynaActionForm enhancement:
- enhancement is now optional, and remains off by default
- this makes cglib an optional dependency (only required
  when enhancement is enabled)
- improved efficiency (thanks mainly to Niall's improvements)
- enhancer functionality is now exposed so it can be
  re-used elsewhere if appropriate
- enhancement is enabled by adding 'enhanced="true"' to
  the form-bean element in struts-config.xml

This should address most comments/concerns, with the
exception of whether this code should move to 'extras',
which will require a small amount of additional work.


Added:
   struts/core/trunk/src/java/org/apache/struts/util/DynaBeanInterceptor.java
Modified:
   struts/core/trunk/conf/java/struts-config_1_3.dtd
   struts/core/trunk/src/java/org/apache/struts/action/DynaActionFormClass.java
   struts/core/trunk/src/java/org/apache/struts/config/FormBeanConfig.java

Modified: struts/core/trunk/conf/java/struts-config_1_3.dtd
URL: 
http://svn.apache.org/viewcvs/struts/core/trunk/conf/java/struts-config_1_3.dtd?rev=345179&r1=345178&r2=345179&view=diff
==============================================================================
--- struts/core/trunk/conf/java/struts-config_1_3.dtd (original)
+++ struts/core/trunk/conf/java/struts-config_1_3.dtd Wed Nov 16 19:40:50 2005
@@ -125,11 +125,19 @@
type Fully qualified Java class name of the ActionForm subclass
                     to use with this form bean.
+ + enhanced Flag indicating if the form bean should be dynamically
+                     enhanced to include getters and setters for defined
+                     properties. This is only valid when 'type' is a
+ dynamic form bean (an instance of + "org.apache.struts.action.DynaActionForm" or a sub-class,
+                     and requires CGLIB when enabled.
-->
<!ELEMENT form-bean (icon?, display-name?, description?, set-property*, 
form-property*)>
<!ATTLIST form-bean      id             ID              #IMPLIED>
<!ATTLIST form-bean      className      %ClassName;     #IMPLIED>
<!ATTLIST form-bean      dynamic        %Boolean;       #IMPLIED>
+<!ATTLIST form-bean      enhanced       %Boolean;       #IMPLIED>
<!ATTLIST form-bean      extends        %BeanName;      #IMPLIED>
<!ATTLIST form-bean      name           %BeanName;      #REQUIRED>
<!ATTLIST form-bean      type           %ClassName;     #IMPLIED>

Modified: 
struts/core/trunk/src/java/org/apache/struts/action/DynaActionFormClass.java
URL: 
http://svn.apache.org/viewcvs/struts/core/trunk/src/java/org/apache/struts/action/DynaActionFormClass.java?rev=345179&r1=345178&r2=345179&view=diff
==============================================================================
--- 
struts/core/trunk/src/java/org/apache/struts/action/DynaActionFormClass.java 
(original)
+++ 
struts/core/trunk/src/java/org/apache/struts/action/DynaActionFormClass.java 
Wed Nov 16 19:40:50 2005
@@ -22,9 +22,8 @@

import java.io.Serializable;
import java.util.HashMap;
-import java.util.Map;
-import java.lang.reflect.Method;

+import net.sf.cglib.proxy.Enhancer;
import org.apache.commons.beanutils.DynaBean;
import org.apache.commons.beanutils.DynaClass;
import org.apache.commons.beanutils.DynaProperty;
@@ -33,13 +32,7 @@
import org.apache.struts.config.FormBeanConfig;
import org.apache.struts.config.FormPropertyConfig;
import org.apache.struts.util.RequestUtils;
-import net.sf.cglib.proxy.InterfaceMaker;
-import net.sf.cglib.proxy.Enhancer;
-import net.sf.cglib.proxy.MethodInterceptor;
-import net.sf.cglib.proxy.MethodProxy;
-import net.sf.cglib.asm.Type;
-import net.sf.cglib.core.Signature;
-import net.sf.cglib.core.Constants;
+import org.apache.struts.util.DynaBeanInterceptor;


/**
@@ -92,6 +85,12 @@


    /**
+     * <p>The CGLIB <code>Enhancer</code> which we will use to dynamically
+     * add getters/setters if 'enhanced' is true in the form config.
+     */
+    protected transient Enhancer enhancer = null;

Shouldn't we mark enhancer private as getBeanEnhancer will always do the
right thing ?

protected seems to be the norm in Struts code for members of classes likely to be subclassed. This is just consistent with other members in the class.

+
+    /**
     * <p>The form bean configuration information for this class.</p>
     */
    protected FormBeanConfig config = null;
@@ -193,9 +192,14 @@
    public DynaBean newInstance()
        throws IllegalAccessException, InstantiationException {

-        FormPropertyConfig[] props = config.findFormPropertyConfigs();
-        DynaActionForm dynaBean = (DynaActionForm) doCreate(props);
+        DynaActionForm dynaBean = null;
+        if (config.isEnhanced()) {
+            dynaBean = (DynaActionForm) getBeanEnhancer().create();
+        } else {
+            dynaBean = (DynaActionForm) getBeanClass().newInstance();
+        }
        dynaBean.setDynaActionFormClass(this);
+        FormPropertyConfig[] props = config.findFormPropertyConfigs();
        for (int i = 0; i < props.length; i++) {
            dynaBean.set(props[i].getName(), props[i].initial());
        }
@@ -273,6 +277,24 @@


    /**
+     * <p>Return the <code>Enhancer</code> we are using to construct new
+     * instances, re-introspecting our [EMAIL PROTECTED] FormBeanConfig} if 
necessary
+     * (that is, after being deserialized, since <code>enhancer</code> is
+     * marked transient).</p>
+     *
+     * @return The enhancer used to construct new instances.
+     */
+    protected Enhancer getBeanEnhancer() {
+
+        if (enhancer == null) {
+            introspect(config);
+        }
+        return (enhancer);

Idiom question. Why the brackets around enhancer ?

Local coding convention... They're redundant but, again, this is consistent with other code.

+
+    }
+
+
+    /**
     * <p>Introspect our form bean configuration to identify the supported
     * properties.</p>
     *
@@ -320,115 +342,14 @@
                              properties[i]);
        }

+        // Create CGLIB enhancer if enhancement is enabled
+        if (config.isEnhanced()) {
+             DynaBeanInterceptor interceptor = new DynaBeanInterceptor();
+             enhancer = interceptor.createEnhancer(this, getBeanClass());
+        }
    }


    // -------------------------------------------------------- Private Methods

-    private Object doCreate(FormPropertyConfig[] props) {
-        // Build an interface to implement consisting of getter/setter
-        // pairs for each property. Also create a lookup table so we
-        // can map method names back to the corresponding dynamic
-        // property on invocation. This allows us to correctly handle
-        // property names that don't comply with JavaBeans naming
-        // conventions.
-        Map properties = new HashMap(props.length * 2);
-        InterfaceMaker im = new InterfaceMaker();
-        for (int i = 0; i < props.length; i++) {
-            String name = props[i].getName();
-            Class type = props[i].getTypeClass();
-            Type ttype = Type.getType(type);
-
-            if (! name.matches("[\\w]+")) {
-                // Note: this allows leading digits, which is not legal
-                // for an identifier but is valid in a getter/setter
-                // method name. Since you can define such getter/setter
-                // directly, we support doing so dynamically too.
-                if (log.isWarnEnabled()) {
-                    log.warn(
-                        "Dyna property name '" + name +
-                        "' in form bean " + config.getName() +
-                        " is not a legal Java identifier. " +
-                        "No property access methods generated.");
-                }
-            } else {
-                // Capitalize property name appropriately
-                String property;
-                if ((name.length() <= 1) ||
-                    (  Character.isLowerCase(name.charAt(0)) &&
-                    (! Character.isLowerCase(name.charAt(1))))
-                ) {
-                    property = name;
-                } else {
-                    property =
-                        Character.toUpperCase(name.charAt(0)) +
-                        name.substring(1);
-                }
-
-                // Create the getter/setter method pair
-                Signature getter = new Signature("get"+property, ttype, 
Constants.TYPES_EMPTY);
-                Signature setter = new Signature("set"+property, 
Type.VOID_TYPE, new Type[] { ttype });
-                im.add(getter, Constants.TYPES_EMPTY);
-                im.add(setter, Constants.TYPES_EMPTY);
-                properties.put("get"+property, name);
-                properties.put("set"+property, name);
-            }
-        }
-        Class beanInterface = im.create();
-
-        // Now generate a proxy for the dyna bean that also implements
-        // the getter/setter methods defined above. We turn off the
-        // Factory interface to preven problems with BeanUtils.copyProperties
-        // when both source and target bean are enhanced (otherwise, the
-        // target bean's callbacks get overwritten with the source beans,
-        // leading to unexpected behaviour).
-        Enhancer e = new Enhancer();
-        e.setSuperclass(beanClass);
-        e.setInterfaces(new Class[] { beanInterface });
-        e.setCallback(new BeanInterceptor(properties));
-        e.setUseFactory(false);
-
-        // Return the generated/enhanced bean
-        return e.create();
-    }
-
-    private static class BeanInterceptor implements MethodInterceptor, 
Serializable {
-        private Map propertyLookup;
-
-        public BeanInterceptor(Map propertyLookup) {
-            this.propertyLookup = propertyLookup;
-        }
-
-        public Object intercept(Object obj, Method method, Object[] args,
-                                MethodProxy proxy) throws Throwable {
-
-            String methodNm = method.getName();
-            String propertyNm = (String) propertyLookup.get(methodNm);
-            String targetNm = methodNm.substring(0, 3); // get/set
-
-            if (propertyNm == null) {
-                // Not a dyna property access, just pass call along
-                return proxy.invokeSuper(obj, args);
-            }
-
-            // Handle the dyna property access:
-            //  - map getFoo(...) to get("foo", ...)
-            //  - map setFoo(bar, ...) to set("foo", bar, ...)
-            Class[] targetArgTypes = new Class[args.length + 1];
-            Object[] targetArgs = new Object[args.length + 1];
-            targetArgTypes[0] = String.class; // for property name
-            targetArgs[0] = propertyNm;
-
-            System.arraycopy(args, 0, targetArgs, 1, args.length);
-
-            for (int i = 0; i < args.length; i++) {
-                targetArgTypes[i + 1] = args[i].getClass();
-            }
-
-            Method target = obj.getClass().getMethod(targetNm, targetArgTypes);
-
-            // Return the result of mapped get/set
-            return target.invoke(obj, targetArgs);
-        }
-    }
}

Modified: 
struts/core/trunk/src/java/org/apache/struts/config/FormBeanConfig.java
URL: 
http://svn.apache.org/viewcvs/struts/core/trunk/src/java/org/apache/struts/config/FormBeanConfig.java?rev=345179&r1=345178&r2=345179&view=diff
==============================================================================
--- struts/core/trunk/src/java/org/apache/struts/config/FormBeanConfig.java 
(original)
+++ struts/core/trunk/src/java/org/apache/struts/config/FormBeanConfig.java Wed 
Nov 16 19:40:50 2005
@@ -105,6 +105,20 @@
        return (this.dynamic);
    }

+    /**
+     * Should the form bean class be dynamically enhanced to simplify
+     * property access in JSP and tag files?
+     */
+    protected boolean enhance = false;
+ + public boolean isEnhanced() {
+        return (this.enhance);
+    }
+ + public void setEnhanced(boolean enhance) {
+        throwIfConfigured();
+        this.enhance = enhance;
+    }

    /**
     * The name of the FormBeanConfig that this config inherits configuration

Added: 
struts/core/trunk/src/java/org/apache/struts/util/DynaBeanInterceptor.java
URL: 
http://svn.apache.org/viewcvs/struts/core/trunk/src/java/org/apache/struts/util/DynaBeanInterceptor.java?rev=345179&view=auto
==============================================================================
--- struts/core/trunk/src/java/org/apache/struts/util/DynaBeanInterceptor.java 
(added)
+++ struts/core/trunk/src/java/org/apache/struts/util/DynaBeanInterceptor.java 
Wed Nov 16 19:40:50 2005
@@ -0,0 +1,180 @@
+/*
+ * $Id$
+ * + * Copyright 2005 The Apache Software Foundation.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.struts.util;
+
+import java.io.Serializable;
+import java.lang.reflect.Method;
+import java.util.HashMap;
+import java.util.Map;
+
+import net.sf.cglib.asm.Type;
+import net.sf.cglib.core.Constants;
+import net.sf.cglib.core.Signature;
+import net.sf.cglib.proxy.Enhancer;
+import net.sf.cglib.proxy.InterfaceMaker;
+import net.sf.cglib.proxy.MethodInterceptor;
+import net.sf.cglib.proxy.MethodProxy;
+import org.apache.commons.beanutils.DynaBean;
+import org.apache.commons.beanutils.DynaClass;
+import org.apache.commons.beanutils.DynaProperty;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+
+/**
+ * DynaBeanInterceptor


creates dynamic proxies for DynaBeans with getter/setter pairs for each
property. The proxies intercept the corresponding methods and map them
back to the properties.


+ * + * @since Struts 1.3
+ * @version $Revision$ $Date$
+ */
+public class DynaBeanInterceptor implements MethodInterceptor, Serializable  {
+
+    private Log log = LogFactory.getLog(DynaBeanInterceptor.class);
+    private Map propertyLookup = new HashMap();


/**
* A lookup table to map method names to the corresponding properties.
*/


+
+    /**
+     * Default Constructor.
+     */
+    public DynaBeanInterceptor() {
+    }
+ + /**
+     * Creates an Enhancer for a DynaClass/DynaBean.

        *
        * @param dynaClass the dynamic properties to use for enhancement
        * @param beanClass the class to create the proxy for
        * @return an enhancer to generate proxies

+     */
+    public Enhancer createEnhancer(DynaClass dynaClass, Class beanClass) {
+        // Build an interface to implement consisting of getter/setter
+        // pairs for each property. Also create a lookup table so we
+        // can map method names back to the corresponding dynamic
+        // property on invocation. This allows us to correctly handle
+        // property names that don't comply with JavaBeans naming
+        // conventions.
+        DynaProperty[] dynaProperties = dynaClass.getDynaProperties();
+        Map properties = new HashMap(dynaProperties.length * 2);
+        InterfaceMaker im = new InterfaceMaker();
+        for (int i = 0; i < dynaProperties.length; i++) {
+            String name = dynaProperties[i].getName();
+            Class type = dynaProperties[i].getType();
+            Type ttype = Type.getType(type);
+
+            if (! name.matches("[\\w]+")) {
+                // Note: this allows leading digits, which is not legal
+                // for an identifier but is valid in a getter/setter
+                // method name. Since you can define such getter/setter
+                // directly, we support doing so dynamically too.
+                if (log.isWarnEnabled()) {
+                    log.warn(
+                        "Dyna property name '" + name +
+                        "' in DynaBean " + dynaClass.getName() +
+                        " is not a legal Java identifier. " +
+                        "No property access methods generated.");
+                }
+            } else {
+                // Capitalize property name appropriately
+                String property;
+                if ((name.length() <= 1) ||
+                    (  Character.isLowerCase(name.charAt(0)) &&
+                    (! Character.isLowerCase(name.charAt(1))))
+                ) {
+                    property = name;
+                } else {
+                    property =
+                        Character.toUpperCase(name.charAt(0)) +
+                        name.substring(1);
+                }
+ + // Method names
+                String getterName = "get"+property;
+                String setterName = "set"+property;
+
+                // Create the standard getter/setter method pair
+                Signature getter = new Signature(getterName, ttype, 
Constants.TYPES_EMPTY);
+                Signature setter = new Signature(setterName, Type.VOID_TYPE, 
new Type[] { ttype });
+                im.add(getter, Constants.TYPES_EMPTY);
+                im.add(setter, Constants.TYPES_EMPTY);
+
+                // Create the indexed getter/setter method pair
+                if (dynaProperties[i].isIndexed()) {
+                    Type itype = Type.getType(Integer.class);
+                    ttype = Type.getType((type.isArray() ? 
type.getComponentType() : Object.class));
+                    Signature indexGetter = new Signature(getterName, ttype, 
new Type[] { itype });
+                    Signature indexSetter = new Signature(setterName, 
Type.VOID_TYPE, new Type[] { ttype, itype });
+                    im.add(indexGetter, Constants.TYPES_EMPTY);
+                    im.add(indexSetter, Constants.TYPES_EMPTY);
+                }
+                propertyLookup.put(getterName, name);
+                propertyLookup.put(setterName, name);
+ + }
+        }
+        Class beanInterface = im.create();
+
+        // Now generate a proxy for the dyna bean that also implements
+        // the getter/setter methods defined above.  We turn off the
+        // Factory interface to prevent problems with BeanUtils.copyProperties
+        // when both source and target bean are enhanced (otherwise, the
+        // target bean's callbacks get overwritten with the source bean's,
+        // leading to unexpected behaviour).
+        Enhancer enhancer = new Enhancer();
+        enhancer.setSuperclass(beanClass);
+        enhancer.setInterfaces(new Class[] { beanInterface });
+        enhancer.setCallback(this);
+        enhancer.setUseFactory(false);
+        return enhancer;
+    }
+
+    /**
+     * Intercepts a method call on the enhanced DynaBean.

* * @param obj the enhanced <code>DynaBean</code>
        * @param method the method to invoke on the object
        * @param args the method parameters
        * @param proxy the method proxy
        * @return the return value of the intercepted method call

+     */
+    public Object intercept(Object obj, Method method, Object[] args,
+                            MethodProxy proxy) throws Throwable {
+
+        String methodNm = method.getName();
+        String property = (String)propertyLookup.get(methodNm);
+
+        if (property == null) {
+            // Not a dyna property access, just pass call along
+            return proxy.invokeSuper(obj, args);
+        }
+ + boolean getter = methodNm.startsWith("get");
+
+        DynaBean dynaBean = (DynaBean)obj;
+        if (getter) {
+            if (args.length == 0) {
+                return dynaBean.get(property);
+            } else if(args.length == 1 && args[0].getClass() == Integer.class) 
{
+                int index = ((Integer)args[0]).intValue();
+                return dynaBean.get(property, index);
+            } else {
+                return proxy.invokeSuper(obj, args);
+            }
+        } else {
+            if (args.length == 1) {
+                dynaBean.set(property, args[0]);
+                return null;
+            } else if(args.length == 2 && args[1].getClass() == Integer.class) 
{
+                int index = ((Integer)args[1]).intValue();
+                dynaBean.set(property, index, args[0]);
+                return null;
+            } else {
+                return proxy.invokeSuper(obj, args);
+            }
+        }
+
+    }
+ +}

Thanks, I'll add those Javadoc additions.

L.


---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to