You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@struts.apache.org by la...@apache.org on 2005/11/17 04:40:58 UTC
svn commit: r345179 - in /struts/core/trunk: conf/java/struts-config_1_3.dtd
src/java/org/apache/struts/action/DynaActionFormClass.java
src/java/org/apache/struts/config/FormBeanConfig.java
src/java/org/apache/struts/util/DynaBeanInterceptor.java
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;
+
+ /**
* <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 {@link 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);
+
+ }
+
+
+ /**
* <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
+ *
+ * @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();
+
+ /**
+ * Default Constructor.
+ */
+ public DynaBeanInterceptor() {
+ }
+
+ /**
+ * Creates an Enhancer for a DynaClass/DynaBean.
+ */
+ 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.
+ */
+ 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);
+ }
+ }
+
+ }
+
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: svn commit: r345179 - in /struts/core/trunk: conf/java/struts-config_1_3.dtd
src/java/org/apache/struts/action/DynaActionFormClass.java src/java/org/apache/struts/config/FormBeanConfig.java
src/java/org/apache/struts/util/DynaBeanInterceptor.java
Posted by Laurie Harper <la...@holoweb.net>.
Christian Meder wrote:
> On Thu, 2005-11-17 at 16:29 -0500, Laurie Harper wrote:
>
>>Christian Meder wrote:
>>
>>>On Thu, 2005-11-17 at 03:40 +0000, laurieh@apache.org wrote:
>>>
>>>> /**
>>>>+ * <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.
>
>
> Just after posting I saw that it's consistent with other members in the
> class ;-)
>
> But that still leaves the point that the re-introspection after
> deserialization isn't properly encapsulated and bug-prone if you can
> access the enhancer directly without the check. Consistently the same
> issue for the other transient members like beanClass ;-)
That's true. The others obviously can't change without breaking backward
compatibility, though. I guess we could make this one field private to
shut that one door...
L.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: svn commit: r345179 - in /struts/core/trunk: conf/java/struts-config_1_3.dtd src/java/org/apache/struts/action/DynaActionFormClass.java src/java/org/apache/struts/config/FormBeanConfig.java src/java/org/apache/struts/util/DynaBeanInterceptor.java
Posted by Martin Cooper <ma...@apache.org>.
On 11/17/05, Christian Meder <ch...@absolutegiganten.org> wrote:
>
> On Thu, 2005-11-17 at 16:29 -0500, Laurie Harper wrote:
> > Christian Meder wrote:
> > > On Thu, 2005-11-17 at 03:40 +0000, laurieh@apache.org wrote:
> > >>
> > >> /**
> > >>+ * <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.
>
> Just after posting I saw that it's consistent with other members in the
> class ;-)
>
> But that still leaves the point that the re-introspection after
> deserialization isn't properly encapsulated and bug-prone if you can
> access the enhancer directly without the check. Consistently the same
> issue for the other transient members like beanClass ;-)
>
> >
> > >>+
> > >>+ /**
> > >> * <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 {@link 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.
>
> Thanks. But is there any explanation for this coding convention ?
Some people like it, some people don't. There is no standard in the Struts
coding guidelines for whether to use this or not, so you'll find a mixture,
probably depending on who wrote the code. With almost 5,000 Checkstyle
warnings, not including this kind of thing, I think this is the least of our
problems. ;-) And at one point, the Struts code base had zero Checkstyle
errors. Sad.
--
Martin Cooper
Greetings,
>
>
> Christian
> --
> Christian Meder, email: chris@absolutegiganten.org
>
> The Way-Seeking Mind of a tenzo is actualized
> by rolling up your sleeves.
>
> (Eihei Dogen Zenji)
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
>
Re: svn commit: r345179 - in /struts/core/trunk:
conf/java/struts-config_1_3.dtd
src/java/org/apache/struts/action/DynaActionFormClass.java
src/java/org/apache/struts/config/FormBeanConfig.java
src/java/org/apache/struts/util/DynaBeanInterceptor.java
Posted by Christian Meder <ch...@absolutegiganten.org>.
On Thu, 2005-11-17 at 16:29 -0500, Laurie Harper wrote:
> Christian Meder wrote:
> > On Thu, 2005-11-17 at 03:40 +0000, laurieh@apache.org wrote:
> >>
> >> /**
> >>+ * <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.
Just after posting I saw that it's consistent with other members in the
class ;-)
But that still leaves the point that the re-introspection after
deserialization isn't properly encapsulated and bug-prone if you can
access the enhancer directly without the check. Consistently the same
issue for the other transient members like beanClass ;-)
>
> >>+
> >>+ /**
> >> * <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 {@link 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.
Thanks. But is there any explanation for this coding convention ?
Greetings,
Christian
--
Christian Meder, email: chris@absolutegiganten.org
The Way-Seeking Mind of a tenzo is actualized
by rolling up your sleeves.
(Eihei Dogen Zenji)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: svn commit: r345179 - in /struts/core/trunk: conf/java/struts-config_1_3.dtd
src/java/org/apache/struts/action/DynaActionFormClass.java src/java/org/apache/struts/config/FormBeanConfig.java
src/java/org/apache/struts/util/DynaBeanInterceptor.java
Posted by Laurie Harper <la...@holoweb.net>.
Christian Meder wrote:
> On Thu, 2005-11-17 at 03:40 +0000, laurieh@apache.org 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 {@link 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: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: svn commit: r345179 - in /struts/core/trunk:
conf/java/struts-config_1_3.dtd
src/java/org/apache/struts/action/DynaActionFormClass.java
src/java/org/apache/struts/config/FormBeanConfig.java
src/java/org/apache/struts/util/DynaBeanInterceptor.java
Posted by Christian Meder <ch...@absolutegiganten.org>.
On Thu, 2005-11-17 at 03:40 +0000, laurieh@apache.org 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 ?
> +
> + /**
> * <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 {@link 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 ?
> +
> + }
> +
> +
> + /**
> * <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);
> + }
> + }
> +
> + }
> +
> +}
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
--
Christian Meder, email: chris@absolutegiganten.org
The Way-Seeking Mind of a tenzo is actualized
by rolling up your sleeves.
(Eihei Dogen Zenji)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org