You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@struts.apache.org by Laurie Harper <la...@holoweb.net> on 2005/11/17 22:29:08 UTC
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
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 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