[DIGESTER-153] Add Constructor support to ObjectCreateRule ; cooperative work between Simo Tripodi & Matt Benson
git-svn-id: https://svn.apache.org/repos/asf/commons/proper/digester/trunk@1200623 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/pom.xml b/pom.xml
index d3dc941..2dbe71f 100644
--- a/pom.xml
+++ b/pom.xml
@@ -89,6 +89,11 @@
<id>simonetripodi</id>
<email>simonetripodi AT apache DOT org</email>
</developer>
+ <developer>
+ <name>Matt Benson</name>
+ <id>mbenson</id>
+ <email>mbenson AT apache DOT org</email>
+ </developer>
</developers>
<contributors>
<contributor>
@@ -167,6 +172,11 @@
<dependencies>
<dependency>
+ <groupId>cglib</groupId>
+ <artifactId>cglib</artifactId>
+ <version>2.2.2</version>
+ </dependency>
+ <dependency>
<groupId>commons-beanutils</groupId>
<artifactId>commons-beanutils</artifactId>
<version>1.8.3</version>
diff --git a/src/main/java/org/apache/commons/digester3/ObjectCreateRule.java b/src/main/java/org/apache/commons/digester3/ObjectCreateRule.java
index 167da8f..dbf2309 100644
--- a/src/main/java/org/apache/commons/digester3/ObjectCreateRule.java
+++ b/src/main/java/org/apache/commons/digester3/ObjectCreateRule.java
@@ -19,19 +19,21 @@
* under the License.
*/
-import static java.util.Arrays.asList;
import static java.lang.String.format;
-
-import static org.apache.commons.beanutils.ConvertUtils.convert;
+import static java.util.Arrays.fill;
+import static net.sf.cglib.proxy.Enhancer.isEnhanced;
import static org.apache.commons.beanutils.ConstructorUtils.getAccessibleConstructor;
import java.lang.reflect.Constructor;
-import java.util.Formatter;
-import java.util.LinkedHashMap;
-import java.util.Map;
-import java.util.Map.Entry;
+import java.util.Arrays;
+
+import net.sf.cglib.proxy.Callback;
+import net.sf.cglib.proxy.Enhancer;
+import net.sf.cglib.proxy.Factory;
+import net.sf.cglib.proxy.LazyLoader;
import org.xml.sax.Attributes;
+import org.xml.sax.SAXException;
/**
* Rule implementation that creates a new object and pushes it onto the object stack. When the element is complete, the
@@ -111,27 +113,31 @@
*
* @since 3.2
*/
- private final Map<String, Class<?>> constructorArguments = new LinkedHashMap<String, Class<?>>();
+ private Class<?>[] constructorArgumentsTypes;
+
+ /**
+ * cglib Factory for lazily-loaded instances after the first.
+ * Only used in the presence of constructor args.
+ *
+ * @since 3.2
+ */
+ private Factory proxyFactory;
// --------------------------------------------------------- Public Methods
/**
- * Allows users specify constructor arguments <b>from attributes only</b>.
+ * Allows users specify constructor arguments.
*
* @since 3.2
*/
- public void addConstructorArgument( String attibuteName, Class<?> type )
+ public void setConstructorArguments( Class<?>...constructorArgumentsTypes )
{
- if ( attibuteName == null )
+ if ( constructorArgumentsTypes == null )
{
- throw new IllegalArgumentException( "Parameter 'attibuteName' must not be null" );
- }
- if ( type == null )
- {
- throw new IllegalArgumentException( "Parameter 'type' must not be null" );
+ throw new IllegalArgumentException( "Parameter 'constructorArgumentsTypes' must not be null" );
}
- constructorArguments.put( attibuteName, type );
+ this.constructorArgumentsTypes = constructorArgumentsTypes;
}
/**
@@ -166,7 +172,7 @@
clazz = getDigester().getClassLoader().loadClass( realClassName );
}
Object instance;
- if ( constructorArguments.isEmpty() )
+ if ( constructorArgumentsTypes == null || constructorArgumentsTypes.length == 0 )
{
if ( getDigester().getLogger().isDebugEnabled() )
{
@@ -179,52 +185,46 @@
}
else
{
- Class<?>[] parameterTypes = new Class<?>[constructorArguments.size()];
- Object[] initargs = new Object[constructorArguments.size()];
-
- int counter = 0;
-
- // prepare the arguments types with related values
- for ( Entry<String, Class<?>> argEntry : constructorArguments.entrySet() )
- {
- parameterTypes[counter] = argEntry.getValue();
-
- String argumentValueAsString = attributes.getValue( argEntry.getKey() );
- // ConvertUtils manages null values as well
- initargs[counter] = convert( argumentValueAsString, parameterTypes[counter] );
-
- counter++;
- }
-
- Constructor<?> constructor = getAccessibleConstructor( clazz, parameterTypes );
+ Constructor<?> constructor = getAccessibleConstructor( clazz, constructorArgumentsTypes );
if ( constructor == null )
{
- throw new IllegalArgumentException( format( "[ObjectCreateRule]{%s} class '%s' doesn't have a Contructor with params %s",
- getDigester().getMatch(),
- clazz.getName(),
- asList( parameterTypes ) ) );
+ throw new SAXException( format( "[ObjectCreateRule]{%s} Class '%s' does not have a construcor with types",
+ getDigester().getMatch(),
+ clazz.getName(),
+ Arrays.toString( constructorArgumentsTypes ) ) );
}
- // print out constructor debug
- if ( getDigester().getLogger().isDebugEnabled() )
- {
- Formatter formatter = new Formatter().format( "[ObjectCreateRule]{%s} New '%s' using constructor( ",
- getDigester().getMatch(),
- clazz.getName() );
- for ( int i = 0; i < initargs.length; i++ )
- {
- formatter.format( "%s%s/%s", ( i > 0 ? ", " : "" ), initargs[i], parameterTypes[i].getName() );
- }
- formatter.format( " )" );
- getDigester().getLogger().debug( formatter.toString() );
- }
-
- instance = constructor.newInstance( initargs );
+ instance = createLazyProxy( constructor );
}
getDigester().push( instance );
}
+ private Object createLazyProxy( Constructor<?> constructor ) {
+ Object[] constructorArguments = new Object[constructorArgumentsTypes.length];
+ fill( constructorArguments, null );
+ getDigester().pushParams( constructorArguments );
+
+ ObjectCreateRuleLazyLoader lazyLoader = new ObjectCreateRuleLazyLoader( constructor,
+ constructorArgumentsTypes,
+ constructorArguments );
+ if ( proxyFactory == null ) {
+ synchronized ( this ) {
+ // check again for null now that we're in the synchronized block:
+ if ( proxyFactory == null ) {
+ Enhancer enhancer = new Enhancer();
+ enhancer.setSuperclass( clazz );
+ enhancer.setCallback( lazyLoader );
+ enhancer.setClassLoader( getDigester().getClassLoader() );
+ Object result = enhancer.create();
+ proxyFactory = (Factory) result;
+ return result;
+ }
+ }
+ }
+ return proxyFactory.newInstance( lazyLoader );
+ }
+
/**
* {@inheritDoc}
*/
@@ -233,6 +233,13 @@
throws Exception
{
Object top = getDigester().pop();
+
+ if ( isEnhanced( top.getClass() ) )
+ {
+ // do lazy load?!?
+ getDigester().popParams();
+ }
+
if ( getDigester().getLogger().isDebugEnabled() )
{
getDigester().getLogger().debug( format( "[ObjectCreateRule]{%s} Pop '%s'",
diff --git a/src/main/java/org/apache/commons/digester3/ObjectCreateRuleLazyLoader.java b/src/main/java/org/apache/commons/digester3/ObjectCreateRuleLazyLoader.java
new file mode 100644
index 0000000..90de242
--- /dev/null
+++ b/src/main/java/org/apache/commons/digester3/ObjectCreateRuleLazyLoader.java
@@ -0,0 +1,73 @@
+package org.apache.commons.digester3;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+import static org.apache.commons.beanutils.ConvertUtils.convert;
+
+import java.lang.reflect.Constructor;
+
+import net.sf.cglib.proxy.LazyLoader;
+
+final class ObjectCreateRuleLazyLoader
+ implements LazyLoader
+{
+
+ private final Constructor<?> constructor;
+
+ private final Class<?>[] paramTypes;
+
+ private final Object[] parameters;
+
+ public ObjectCreateRuleLazyLoader( Constructor<?> constructor,
+ Class<?>[] paramTypes,
+ Object[] parameters )
+ {
+ this.constructor = constructor;
+ this.paramTypes = paramTypes;
+ this.parameters = parameters;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ public Object loadObject()
+ throws Exception
+ {
+ // this piece of code is borrowed from CallMethodRule
+ Object[] paramValues = new Object[paramTypes.length];
+ for ( int i = 0; i < paramTypes.length; i++ )
+ {
+ // convert nulls and convert stringy parameters
+ // for non-stringy param types
+ if ( parameters[i] == null
+ || ( parameters[i] instanceof String && !String.class.isAssignableFrom( paramTypes[i] ) ) )
+ {
+ paramValues[i] = convert( (String) parameters[i], paramTypes[i] );
+ }
+ else
+ {
+ paramValues[i] = parameters[i];
+ }
+ }
+
+ return constructor.newInstance( paramValues );
+ }
+
+}
diff --git a/src/main/java/org/apache/commons/digester3/annotations/handlers/ObjectCreateHandler.java b/src/main/java/org/apache/commons/digester3/annotations/handlers/ObjectCreateHandler.java
index dedd35e..6c4b24a 100644
--- a/src/main/java/org/apache/commons/digester3/annotations/handlers/ObjectCreateHandler.java
+++ b/src/main/java/org/apache/commons/digester3/annotations/handlers/ObjectCreateHandler.java
@@ -19,13 +19,10 @@
* under the License.
*/
-import java.lang.annotation.Annotation;
import java.lang.reflect.AnnotatedElement;
import java.lang.reflect.Constructor;
import org.apache.commons.digester3.annotations.AnnotationHandler;
-import org.apache.commons.digester3.annotations.reflect.MethodArgument;
-import org.apache.commons.digester3.annotations.rules.Attribute;
import org.apache.commons.digester3.annotations.rules.ObjectCreate;
import org.apache.commons.digester3.binder.ObjectCreateBuilder;
import org.apache.commons.digester3.binder.RulesBinder;
@@ -69,17 +66,7 @@
if ( element instanceof Constructor<?> )
{
Constructor<?> method = (Constructor<?>) element;
- Annotation[][] parameterAnnotations = method.getParameterAnnotations();
- Class<?>[] parameterTypes = method.getParameterTypes();
- for ( int i = 0; i < parameterTypes.length; i++ )
- {
- MethodArgument methodArgument = new MethodArgument( i, parameterTypes[i], parameterAnnotations[i] );
- if ( methodArgument.isAnnotationPresent( Attribute.class ) )
- {
- Attribute attribute = methodArgument.getAnnotation( Attribute.class );
- builder.addConstructorArgument( attribute.value() ).ofType( methodArgument.getParameterType() );
- }
- }
+ builder.usingConstructor( method.getParameterTypes() );
}
}
diff --git a/src/main/java/org/apache/commons/digester3/annotations/rules/Attribute.java b/src/main/java/org/apache/commons/digester3/annotations/rules/Attribute.java
deleted file mode 100644
index a385d8a..0000000
--- a/src/main/java/org/apache/commons/digester3/annotations/rules/Attribute.java
+++ /dev/null
@@ -1,40 +0,0 @@
-package org.apache.commons.digester3.annotations.rules;
-
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you 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.
- */
-
-import java.lang.annotation.Documented;
-import java.lang.annotation.ElementType;
-import java.lang.annotation.Retention;
-import java.lang.annotation.RetentionPolicy;
-import java.lang.annotation.Target;
-
-/**
- *
- * @since 3.2
- */
-@Documented
-@Retention( RetentionPolicy.RUNTIME )
-@Target( ElementType.PARAMETER )
-public @interface Attribute
-{
-
- String value();
-
-}
diff --git a/src/main/java/org/apache/commons/digester3/binder/ConstructorArgumentTypeBinder.java b/src/main/java/org/apache/commons/digester3/binder/ConstructorArgumentTypeBinder.java
deleted file mode 100644
index 85e5b88..0000000
--- a/src/main/java/org/apache/commons/digester3/binder/ConstructorArgumentTypeBinder.java
+++ /dev/null
@@ -1,88 +0,0 @@
-package org.apache.commons.digester3.binder;
-
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you 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.
- */
-
-import static java.lang.String.format;
-
-import java.util.Map;
-
-/**
- *
- *
- * @since 3.2
- */
-public final class ConstructorArgumentTypeBinder
-{
-
- private final ObjectCreateBuilder parent;
-
- private final Map<String, Class<?>> constructorArguments;
-
- private final ClassLoader classLoader;
-
- private final String attibuteName;
-
- ConstructorArgumentTypeBinder( ObjectCreateBuilder parent,
- Map<String, Class<?>> constructorArguments,
- String attibuteName,
- ClassLoader classLoader )
- {
- this.parent = parent;
- this.constructorArguments = constructorArguments;
- this.attibuteName = attibuteName;
- this.classLoader = classLoader;
- }
-
- public <T> ObjectCreateBuilder ofType( Class<T> type )
- {
- if ( type == null )
- {
- parent.reportError( format( "createObject().addConstructorArgument( %s ).ofType( Class<T> )", attibuteName ),
- "NULL attibute name not allowed" );
- return parent;
- }
-
- constructorArguments.put( attibuteName, type );
- return parent;
- }
-
- public <T> ObjectCreateBuilder ofType( String type )
- {
- if ( type == null )
- {
- parent.reportError( format( "createObject().addConstructorArgument( %s ).ofType( String )", attibuteName ),
- "NULL attibute name not allowed" );
- return parent;
- }
-
- try
- {
- constructorArguments.put( attibuteName, classLoader.loadClass( type ) );
- }
- catch ( ClassNotFoundException e )
- {
- parent.reportError( format( "createObject().addConstructorArgument( %s ).ofType( String )", attibuteName ),
- format( "class '%s' cannot be load", type ) );
- }
-
- return parent;
- }
-
-}
diff --git a/src/main/java/org/apache/commons/digester3/binder/ObjectCreateBuilder.java b/src/main/java/org/apache/commons/digester3/binder/ObjectCreateBuilder.java
index f60dd53..e9a0625 100644
--- a/src/main/java/org/apache/commons/digester3/binder/ObjectCreateBuilder.java
+++ b/src/main/java/org/apache/commons/digester3/binder/ObjectCreateBuilder.java
@@ -19,10 +19,6 @@
* under the License.
*/
-import java.util.LinkedHashMap;
-import java.util.Map;
-import java.util.Map.Entry;
-
import org.apache.commons.digester3.ObjectCreateRule;
/**
@@ -45,7 +41,7 @@
*
* @since 3.2
*/
- private final Map<String, Class<?>> constructorArguments = new LinkedHashMap<String, Class<?>>();
+ private Class<?>[] constructorArgumentsType;
ObjectCreateBuilder( String keyPattern, String namespaceURI, RulesBinder mainBinder, LinkedRuleBuilder mainBuilder,
ClassLoader classLoader )
@@ -113,18 +109,19 @@
/**
*
- * @param attibuteName
* @return
* @since 3.2
*/
- public ConstructorArgumentTypeBinder addConstructorArgument( String attibuteName )
+ public ObjectCreateBuilder usingConstructor( Class<?>...constructorArgumentsType )
{
- if ( attibuteName == null )
+ if ( constructorArgumentsType == null )
{
- reportError( "createObject().addConstructorArgument( String )", "NULL attibute name not allowed" );
+ reportError( "createObject().usingConstructor( Class<?>[] )", "NULL parametersTypes not allowed" );
}
- return new ConstructorArgumentTypeBinder( this, constructorArguments, attibuteName, classLoader );
+ this.constructorArgumentsType = constructorArgumentsType;
+
+ return this;
}
/**
@@ -135,9 +132,9 @@
{
ObjectCreateRule objectCreateRule = new ObjectCreateRule( attributeName, type );
- for ( Entry<String, Class<?>> argEntry : constructorArguments.entrySet() )
+ if ( constructorArgumentsType != null )
{
- objectCreateRule.addConstructorArgument( argEntry.getKey(), argEntry.getValue() );
+ objectCreateRule.setConstructorArguments( constructorArgumentsType );
}
return objectCreateRule;
diff --git a/src/main/java/org/apache/commons/digester3/xmlrules/ConstructorArgumentRule.java b/src/main/java/org/apache/commons/digester3/xmlrules/ConstructorArgumentRule.java
deleted file mode 100644
index 6ecc91f..0000000
--- a/src/main/java/org/apache/commons/digester3/xmlrules/ConstructorArgumentRule.java
+++ /dev/null
@@ -1,41 +0,0 @@
-package org.apache.commons.digester3.xmlrules;
-
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you 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.
- */
-
-import org.apache.commons.digester3.Rule;
-import org.apache.commons.digester3.binder.ObjectCreateBuilder;
-import org.xml.sax.Attributes;
-
-/**
- * @since 3.2
- */
-final class ConstructorArgumentRule
- extends Rule
-{
-
- @Override
- public void begin( String namespace, String name, Attributes attributes )
- throws Exception
- {
- ObjectCreateBuilder builder = getDigester().peek();
- builder.addConstructorArgument( attributes.getValue( "attrname" ) ).ofType( attributes.getValue( "type" ) );
- }
-
-}
diff --git a/src/main/java/org/apache/commons/digester3/xmlrules/XmlRulesModule.java b/src/main/java/org/apache/commons/digester3/xmlrules/XmlRulesModule.java
index 29b0f99..6a5ba25 100644
--- a/src/main/java/org/apache/commons/digester3/xmlrules/XmlRulesModule.java
+++ b/src/main/java/org/apache/commons/digester3/xmlrules/XmlRulesModule.java
@@ -90,7 +90,6 @@
forPattern( "*/factory-create-rule" ).addRule( new FactoryCreateRule( targetRulesBinder, patternStack ) );
forPattern( "*/node-create-rule" ).addRule( new NodeCreateRule( targetRulesBinder, patternStack ) );
forPattern( "*/object-create-rule" ).addRule( new ObjectCreateRule( targetRulesBinder, patternStack ) );
- forPattern( "*/object-create-rule/constructor-argument" ).addRule( new ConstructorArgumentRule() );
forPattern( "*/set-properties-rule" ).addRule( new SetPropertiesRule( targetRulesBinder, patternStack ) );
forPattern( "*/set-properties-rule/alias" )
diff --git a/src/test/java/org/apache/commons/digester3/Digester153TestCase.java b/src/test/java/org/apache/commons/digester3/Digester153TestCase.java
index 1e0ccb2..688644b 100644
--- a/src/test/java/org/apache/commons/digester3/Digester153TestCase.java
+++ b/src/test/java/org/apache/commons/digester3/Digester153TestCase.java
@@ -43,18 +43,26 @@
throws Exception
{
ObjectCreateRule createRule = new ObjectCreateRule( TestBean.class );
- createRule.addConstructorArgument( "boolean", boolean.class );
- createRule.addConstructorArgument( "double", double.class );
+ createRule.setConstructorArguments( boolean.class, double.class );
Digester digester = new Digester();
digester.addRule( "toplevel/bean", createRule );
+ digester.addCallParam( "toplevel/bean", 0, "boolean" );
+ digester.addCallParam( "toplevel/bean", 1, "double" );
TestBean bean = digester.parse( getClass().getResourceAsStream( "BasicConstructor.xml" ) );
assertTrue( bean.getBooleanProperty() );
assertEquals( 9.99D, bean.getDoubleProperty(), 0 );
+
+ // do it again to exercise the cglib Factory:
+ bean = digester.parse( getClass().getResourceAsStream( "BasicConstructor.xml" ) );
+
+ assertTrue( bean.getBooleanProperty() );
+ assertEquals( 9.99D, bean.getDoubleProperty(), 0 );
}
+ /*
@Test
public void basicConstructorViaBinder()
throws Exception
@@ -163,5 +171,6 @@
assertTrue( bean.getBooleanProperty() );
assertEquals( 9.99D, bean.getDoubleProperty(), 0 );
}
+ */
}
diff --git a/src/test/java/org/apache/commons/digester3/TestBean.java b/src/test/java/org/apache/commons/digester3/TestBean.java
index 5041f77..afe17ec 100644
--- a/src/test/java/org/apache/commons/digester3/TestBean.java
+++ b/src/test/java/org/apache/commons/digester3/TestBean.java
@@ -18,7 +18,7 @@
package org.apache.commons.digester3;
-import org.apache.commons.digester3.annotations.rules.Attribute;
+import org.apache.commons.digester3.annotations.rules.CallParam;
import org.apache.commons.digester3.annotations.rules.ObjectCreate;
/**
@@ -37,8 +37,8 @@
}
@ObjectCreate( pattern = "toplevel/bean" )
- public TestBean( @Attribute( "boolean" ) boolean booleanProperty,
- @Attribute( "double" ) double doubleProperty )
+ public TestBean( @CallParam( pattern = "toplevel/bean", attributeName = "boolean" ) boolean booleanProperty,
+ @CallParam( pattern = "toplevel/bean", attributeName = "double" ) double doubleProperty )
{
setBooleanProperty( booleanProperty );
setDoubleProperty( doubleProperty );
