Hi,

This is a patch to myrmidon's task configurer, to cache reflection info.
The goal was to start playing with some new configuration features, but
unfortunately I didn't find time to do much with it.  I'd like to put this
on hold temporarily, so I'm submitting the changes as they currently stand.
Configuration should be heaps faster now.

Here's what I'd like to try sometime soon-ish:

* Allow an object (task or data type) to override parts of its configuration
process.

This would be an alternative to implementing Configurable (which requires
the object to deal with configuring itself entirely).

The plan is to allow a class to provide a customised introspector (an
ObjectConfigurer) via a static method - getConfigurer(), say.  This would
allow a class to do simple things like:

  - Explicitly choose an overloaded setter/adder/creator method to use when
there's more than one.
  - Ignore certain setter/adder/creator methods.
  - Map text content to a particular attribute (e.g. "message" in the <log>
task).
  - Deal with attributes or elements whose names are only known at runtime
(e.g. compiler adaptors).
  - etc.

* Deal with references.

Have the configurer resolve (and type check) references to data types,
rather than forcing the object do it.  An adder method would be passed an
object, and whether that object was obtained by reference, or created and
configured inline, the parent object doesn't know or care.

This raises the issue of ownership, and immutability - maybe a clone gets
passed to the adder methods.

* Polymorphic types.

This was raised a few months ago and it is truely an excellent idea.  This
would allow the configurer to pass an adder method, an object of any class
that is assignable to the declared type.  As Peter pointed out, this would
neatly solve the adaptor problem that is being discussed.

* Lifecycle management.

It would be handy if the lifecycle of nested objects was handled by the
container, rather than forcing the parent object to do it.  Not the
configurer's job, necessarily.

* Put some tests together, and maybe some docs as well (yeah sure).


Adam
Index: 
proposal/myrmidon/src/java/org/apache/myrmidon/components/configurer/DefaultConfigurer.java
===================================================================
RCS file: 
/home/cvspublic/jakarta-ant/proposal/myrmidon/src/java/org/apache/myrmidon/components/configurer/DefaultConfigurer.java,v
retrieving revision 1.15
diff -u -r1.15 DefaultConfigurer.java
--- 
proposal/myrmidon/src/java/org/apache/myrmidon/components/configurer/DefaultConfigurer.java
 29 Dec 2001 21:55:07 -0000      1.15
+++ 
proposal/myrmidon/src/java/org/apache/myrmidon/components/configurer/DefaultConfigurer.java
 30 Dec 2001 03:17:45 -0000
@@ -9,7 +9,8 @@
 
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
-import java.util.ArrayList;
+import java.util.*;
+
 import org.apache.avalon.excalibur.i18n.ResourceManager;
 import org.apache.avalon.excalibur.i18n.Resources;
 import org.apache.avalon.excalibur.property.PropertyException;
@@ -25,7 +26,11 @@
 import org.apache.avalon.framework.logger.LogEnabled;
 import org.apache.myrmidon.converter.ConverterException;
 import org.apache.myrmidon.interfaces.configurer.Configurer;
+import org.apache.myrmidon.interfaces.configurer.ObjectConfigurer;
+import org.apache.myrmidon.interfaces.configurer.ElementConfigurer;
+import org.apache.myrmidon.interfaces.configurer.AttributeSetter;
 import org.apache.myrmidon.interfaces.converter.MasterConverter;
+import org.apache.myrmidon.framework.configurer.DefaultObjectConfigurer;
 
 /**
  * Class used to configure tasks.
@@ -42,18 +47,13 @@
     ///Compile time constant to turn on extreme debugging
     private final static boolean DEBUG = false;
 
-    /*
-     * TODO: Should reserved names be "configurable" ?
-     */
-    ///Element names that are reserved
-    private final static String[] RESERVED_ELEMENTS =
-        {
-            "content"
-        };
-
     ///Converter to use for converting between values
     private MasterConverter m_converter;
 
+    ///Cached object configurers.  This is a map from Class to the
+    ///ObjectConfigurer for that class.
+    private Map m_configurerCache = new HashMap();
+
     public void compose( final ComponentManager componentManager )
         throws ComponentException
     {
@@ -81,7 +81,7 @@
         if( DEBUG )
         {
             final String message = REZ.getString( "configuring-object.notice", 
object );
-            getLogger().debug( "Configuring " + object );
+            getLogger().debug( message );
         }
 
         if( object instanceof Configurable )
@@ -89,9 +89,10 @@
             if( DEBUG )
             {
                 final String message = REZ.getString( "configurable.notice" );
-                getLogger().debug( "Configuring object via Configurable 
interface" );
+                getLogger().debug( message );
             }
 
+            // Let the object configure itself
             ( (Configurable)object ).configure( configuration );
         }
         else
@@ -102,52 +103,105 @@
                 getLogger().debug( message );
             }
 
+            // Locate the configurer for this object
+            ObjectConfigurer configurer = getConfigurer( object.getClass() );
+
+            // Set each of the attributes
             final String[] attributes = configuration.getAttributeNames();
             for( int i = 0; i < attributes.length; i++ )
             {
                 final String name = attributes[ i ];
                 final String value = configuration.getAttribute( name );
 
-                if( DEBUG )
-                {
-                    final String message = REZ.getString( 
"configure-attribute.notice", name, value );
-                    getLogger().debug( message );
-                }
+                // Set the attribute
+                setAttribute(configurer, object, name, value, context);
+            }
 
-                configureAttribute( object, name, value, context );
+            // Set the text content
+            final String content = configuration.getValue( null );
+            if( null != content && content.length() > 0 ) {
+                setContent(configurer, object, content, context);
             }
 
+            // Create and configure each of the child elements
             final Configuration[] children = configuration.getChildren();
-
             for( int i = 0; i < children.length; i++ )
             {
-                final Configuration child = children[ i ];
+                final Configuration childConfig = children[ i ];
+                configureElement(configurer, object, childConfig, context);
+            }
+        }
+    }
 
-                if( DEBUG )
-                {
-                    final String message =
-                        REZ.getString( "configure-subelement.notice", 
child.getName() );
-                    getLogger().debug( message );
-                }
+    /**
+     * Sets the text content of an object.
+     */
+    private void setContent( final ObjectConfigurer configurer,
+                             final Object object,
+                             final String content,
+                             final Context context)
+            throws ConfigurationException
+    {
+        if( DEBUG )
+        {
+            final String message =
+                REZ.getString( "configure-content.notice", content );
+            getLogger().debug( message );
+        }
 
-                configureElement( object, child, context );
-            }
+        // Set the content
+        AttributeSetter setter = configurer.getContentSetter();
+        if ( setter == null ) {
+            String msg = REZ.getString("content-not-supported.error");
+            throw new ConfigurationException(msg);
+        }
+        try {
+            setValue(setter, object, content, context);
+        }
+        catch ( Exception exc ) {
+            String msg = REZ.getString("bad-set-content.error");
+            throw new ConfigurationException(msg, exc);
+        }
+    }
 
-            final String content = configuration.getValue( null );
-            if( null != content )
-            {
-                if( !content.trim().equals( "" ) )
-                {
-                    if( DEBUG )
-                    {
-                        final String message =
-                            REZ.getString( "configure-content.notice", content 
);
-                        getLogger().debug( message );
-                    }
+    /**
+     * Creates and configures a nested element
+     */
+    private void configureElement( final ObjectConfigurer configurer,
+                                   final Object object,
+                                   final Configuration childConfig,
+                                   final Context context)
+            throws ConfigurationException
+    {
+        final String childName = childConfig.getName();
 
-                    configureContent( object, content, context );
-                }
-            }
+        if( DEBUG )
+        {
+            final String message =
+                REZ.getString( "configure-subelement.notice", childName );
+            getLogger().debug( message );
+        }
+
+        // Locate the configurer for the child element
+        ElementConfigurer childConfigurer = configurer.getElement(childName);
+        if ( childConfigurer == null ) {
+            String msg = REZ.getString("unknown-subelement.error", childName);
+            throw new ConfigurationException(msg);
+        }
+
+        try {
+            // Create the child element
+            Object child = childConfigurer.createElement(object);
+
+            // Configure the child element
+            configure(child, childConfig, context);
+
+            // Set the child element
+            childConfigurer.addElement(object, child);
+        }
+        catch ( ConfigurationException e ) {
+            String msg = REZ.getString("bad-configure-subelement.error", 
childName);
+            throw new ConfigurationException(msg, e);
         }
     }
 
@@ -168,156 +222,83 @@
                            final Context context )
         throws ConfigurationException
     {
-        configureAttribute( object, name, value, context );
+        // Locate the configurer for this object
+        ObjectConfigurer configurer = getConfigurer(object.getClass());
+
+        // Set the attribute value
+        setAttribute(configurer, object, name, value, context);
     }
 
     /**
-     * Try to configure content of an object.
-     *
-     * @param object the object
-     * @param content the content value to be set
-     * @param context the Context
-     * @exception ConfigurationException if an error occurs
+     * Sets an attribute value.
      */
-    private void configureContent( final Object object,
-                                   final String content,
-                                   final Context context )
-        throws ConfigurationException
-    {
-        setValue( object, "addContent", content, context );
-    }
-
-    private void configureAttribute( final Object object,
-                                     final String name,
-                                     final String value,
-                                     final Context context )
+    private void setAttribute( final ObjectConfigurer configurer,
+                               final Object object,
+                               final String name,
+                               final String value,
+                               final Context context )
         throws ConfigurationException
     {
-        final String methodName = getMethodNameFor( name );
-        setValue( object, methodName, value, context );
-    }
-
-    private void setValue( final Object object,
-                           final String methodName,
-                           final String value,
-                           final Context context )
-        throws ConfigurationException
-    {
-        // OMFG the rest of this is soooooooooooooooooooooooooooooooo
-        // slow. Need to cache results per class etc.
-
-        final Class clazz = object.getClass();
-        final Method[] methods = getMethodsFor( clazz, methodName );
-        if( 0 == methods.length )
+        if( DEBUG )
         {
-            final String message =
-                REZ.getString( "no-attribute-method.error", methodName );
-            throw new ConfigurationException( message );
+            final String message = REZ.getString( "configure-attribute.notice",
+                                                  name,
+                                                  value );
+            getLogger().debug( message );
+        }
+
+        // Locate the setter for this attribute
+        AttributeSetter setter = configurer.getAttributeSetter(name);
+        if ( setter == null ) {
+            String msg = REZ.getString("unknown-attribute.error", name);
+            throw new ConfigurationException(msg);
+        }
+
+        // Set the value
+        try {
+            setValue(setter, object, value, context);
+        }
+        catch ( Exception exc ) {
+            String msg = REZ.getString("bad-set-attribute.error", name);
+            throw new ConfigurationException(msg, exc);
         }
-
-        setValue( object, value, context, methods );
     }
 
-    private void setValue( final Object object,
+    /**
+     * Sets an attribute value, or an element's text content.
+     */
+    private void setValue( final AttributeSetter setter,
+                           final Object object,
                            final String value,
-                           final Context context,
-                           final Method methods[] )
-        throws ConfigurationException
-    {
-        try
-        {
-            final Object objectValue =
-                PropertyUtil.resolveProperty( value, context, false );
-
-            setValue( object, objectValue, methods, context );
-        }
-        catch( final PropertyException pe )
-        {
-            final String message =
-                REZ.getString( "bad-property-resolve.error", value );
-            throw new ConfigurationException( message, pe );
-        }
-    }
-
-    private void setValue( final Object object,
-                           Object value,
-                           final Method methods[],
                            final Context context )
-        throws ConfigurationException
+        throws Exception
     {
-        final Class sourceClass = value.getClass();
-        final String source = sourceClass.getName();
+        // Resolve property references in the attribute value
+        Object objValue = PropertyUtil.resolveProperty( value, context, false 
);
 
-        for( int i = 0; i < methods.length; i++ )
+        // Convert the value to the appropriate type
+        Class cl = setter.getType();
+        if( cl.isPrimitive() )
         {
-            if( setValue( object, value, methods[ i ], sourceClass, source, 
context ) )
-            {
-                return;
-            }
+            cl = getComplexTypeFor( cl );
         }
+        objValue = m_converter.convert( cl, objValue, context );
 
-        final String message =
-            REZ.getString( "no-can-convert.error", methods[ 0 ].getName(), 
source );
-        throw new ConfigurationException( message );
+        // Set the value
+        setter.setAttribute(object, objValue);
     }
 
-    private boolean setValue( final Object object,
-                              Object value,
-                              final Method method,
-                              final Class sourceClass,
-                              final String source,
-                              final Context context )
-        throws ConfigurationException
+    /**
+     * Locates the configurer for a particular class.
+     */
+    private ObjectConfigurer getConfigurer(Class cl) throws 
ConfigurationException
     {
-        Class parameterType = method.getParameterTypes()[ 0 ];
-        if( parameterType.isPrimitive() )
-        {
-            parameterType = getComplexTypeFor( parameterType );
-        }
-
-        try
-        {
-            value = m_converter.convert( parameterType, value, context );
-        }
-        catch( final ConverterException ce )
-        {
-            if( DEBUG )
-            {
-                final String message = REZ.getString( "no-converter.error" );
-                getLogger().debug( message, ce );
-            }
-
-            throw new ConfigurationException( ce.getMessage(), ce );
-        }
-        catch( final Exception e )
-        {
-            final String message =
-                REZ.getString( "bad-convert-for-attribute.error", 
method.getName() );
-            throw new ConfigurationException( message, e );
+        ObjectConfigurer configurer = 
(ObjectConfigurer)m_configurerCache.get(cl);
+        if ( configurer == null ) {
+            configurer = DefaultObjectConfigurer.getConfigurer(cl);
+            m_configurerCache.put(cl, configurer);
         }
-
-        if( null == value )
-        {
-            return false;
-        }
-
-        try
-        {
-            method.invoke( object, new Object[]{value} );
-        }
-        catch( final IllegalAccessException iae )
-        {
-            //should never happen ....
-            final String message = REZ.getString( "illegal-access.error" );
-            throw new ConfigurationException( message, iae );
-        }
-        catch( final InvocationTargetException ite )
-        {
-            final String message = REZ.getString( "invoke-target.error", 
method.getName() );
-            throw new ConfigurationException( message, ite );
-        }
-
-        return true;
+        return configurer;
     }
 
     private Class getComplexTypeFor( final Class clazz )
@@ -342,168 +323,6 @@
         {
             final String message = REZ.getString( "no-complex-type.error", 
clazz.getName() );
             throw new IllegalArgumentException( message );
-        }
-    }
-
-    private Method[] getMethodsFor( final Class clazz, final String methodName 
)
-    {
-        final Method methods[] = clazz.getMethods();
-        final ArrayList matches = new ArrayList();
-
-        for( int i = 0; i < methods.length; i++ )
-        {
-            final Method method = methods[ i ];
-            if( methodName.equals( method.getName() ) &&
-                Method.PUBLIC == ( method.getModifiers() & Method.PUBLIC ) )
-            {
-                if( method.getReturnType().equals( Void.TYPE ) )
-                {
-                    final Class parameters[] = method.getParameterTypes();
-                    if( 1 == parameters.length )
-                    {
-                        matches.add( method );
-                    }
-                }
-            }
-        }
-
-        return (Method[])matches.toArray( new Method[ 0 ] );
-    }
-
-    private Method[] getCreateMethodsFor( final Class clazz, final String 
methodName )
-    {
-        final Method methods[] = clazz.getMethods();
-        final ArrayList matches = new ArrayList();
-
-        for( int i = 0; i < methods.length; i++ )
-        {
-            final Method method = methods[ i ];
-            if( methodName.equals( method.getName() ) &&
-                Method.PUBLIC == ( method.getModifiers() & Method.PUBLIC ) )
-            {
-                final Class returnType = method.getReturnType();
-                if( !returnType.equals( Void.TYPE ) &&
-                    !returnType.isPrimitive() )
-                {
-                    final Class parameters[] = method.getParameterTypes();
-                    if( 0 == parameters.length )
-                    {
-                        matches.add( method );
-                    }
-                }
-            }
-        }
-
-        return (Method[])matches.toArray( new Method[ 0 ] );
-    }
-
-    private String getMethodNameFor( final String attribute )
-    {
-        return "set" + getJavaNameFor( attribute.toLowerCase() );
-    }
-
-    private String getJavaNameFor( final String name )
-    {
-        final StringBuffer sb = new StringBuffer();
-
-        int index = name.indexOf( '-' );
-        int last = 0;
-
-        while( -1 != index )
-        {
-            final String word = name.substring( last, index ).toLowerCase();
-            sb.append( Character.toUpperCase( word.charAt( 0 ) ) );
-            sb.append( word.substring( 1, word.length() ) );
-            last = index + 1;
-            index = name.indexOf( '-', last );
-        }
-
-        index = name.length();
-        final String word = name.substring( last, index ).toLowerCase();
-        sb.append( Character.toUpperCase( word.charAt( 0 ) ) );
-        sb.append( word.substring( 1, word.length() ) );
-
-        return sb.toString();
-    }
-
-    private void configureElement( final Object object,
-                                   final Configuration configuration,
-                                   final Context context )
-        throws ConfigurationException
-    {
-        final String name = configuration.getName();
-        final String javaName = getJavaNameFor( name );
-
-        // OMFG the rest of this is soooooooooooooooooooooooooooooooo
-        // slow. Need to cache results per class etc.
-        final Class clazz = object.getClass();
-        Method methods[] = getMethodsFor( clazz, "add" + javaName );
-
-        if( 0 != methods.length )
-        {
-            //guess it is first method ????
-            addElement( object, methods[ 0 ], configuration, context );
-        }
-        else
-        {
-            methods = getCreateMethodsFor( clazz, "create" + javaName );
-
-            if( 0 == methods.length )
-            {
-                final String message =
-                    REZ.getString( "no-element-method.error", javaName );
-                throw new ConfigurationException( message );
-            }
-
-            //guess it is first method ????
-            createElement( object, methods[ 0 ], configuration, context );
-        }
-    }
-
-    private void createElement( final Object object,
-                                final Method method,
-                                final Configuration configuration,
-                                final Context context )
-        throws ConfigurationException
-    {
-        try
-        {
-            final Object created = method.invoke( object, new Object[ 0 ] );
-            configure( created, configuration, context );
-        }
-        catch( final ConfigurationException ce )
-        {
-            throw ce;
-        }
-        catch( final Exception e )
-        {
-            final String message = REZ.getString( "subelement-create.error" );
-            throw new ConfigurationException( message, e );
-        }
-    }
-
-    private void addElement( final Object object,
-                             final Method method,
-                             final Configuration configuration,
-                             final Context context )
-        throws ConfigurationException
-    {
-        try
-        {
-            final Class clazz = method.getParameterTypes()[ 0 ];
-            final Object created = clazz.newInstance();
-
-            configure( created, configuration, context );
-            method.invoke( object, new Object[]{created} );
-        }
-        catch( final ConfigurationException ce )
-        {
-            throw ce;
-        }
-        catch( final Exception e )
-        {
-            final String message = REZ.getString( "subelement-create.error" );
-            throw new ConfigurationException( message, e );
         }
     }
 }
Index: 
proposal/myrmidon/src/java/org/apache/myrmidon/components/configurer/Resources.properties
===================================================================
RCS file: 
/home/cvspublic/jakarta-ant/proposal/myrmidon/src/java/org/apache/myrmidon/components/configurer/Resources.properties,v
retrieving revision 1.2
diff -u -r1.2 Resources.properties
--- 
proposal/myrmidon/src/java/org/apache/myrmidon/components/configurer/Resources.properties
   19 Nov 2001 12:37:25 -0000      1.2
+++ 
proposal/myrmidon/src/java/org/apache/myrmidon/components/configurer/Resources.properties
   30 Dec 2001 03:17:45 -0000
@@ -1,18 +1,14 @@
 configuring-object.notice=Configuring {0}.
 configurable.notice=Configuring object via Configurable interface.
-reflection.notice=Configuring object via Configurable reflection.
-configure-attribute.notice=Configuring attribute name={0} value={1}.
-configure-subelement.notice=Configuring subelement name={0}.
-configure-content.notice=Configuring content {0}.
+reflection.notice=Configuring object via ObjectConfigurer.
+configure-content.notice=Configuring content with "{0}".
+configure-subelement.notice=Configuring subelement "{0}".
+configure-attribute.notice=Configuring attribute name="{0}" value="{1}".
 
-reserved-attribute.error=Can not specify reserved attribute {0}.
-no-attribute-method.error=Unable to set attribute via {0} due to not finding 
any appropriate mutator method.
-bad-property-resolve.error=Error resolving property {0}.
-no-can-convert.error=Unable to set attribute via {0} as could not convert {1} 
to a matching type.
-no-converter.error=Failed to find converter.
-bad-convert-for-attribute.error=Error converting attribute for {0}.
-no-element-method.error=Unable to set element {0} due to not finding any 
appropriate mutator method.
-illegal-access.error=Error retrieving methods with correct access specifiers.
-invoke-target.error=Error calling method attribute {0}.
+content-not-supported.error=Text content is not supported for this element.
+bad-set-content.error=Could not set text content.
+unknown-subelement.error=Unknown element "{0}".
+bad-configure-subelement.error=Could not configure element "{0}".
+unknown-attribute.error=Unknown attribute "{0}".
+bad-set-attribute.error=Could not set attribute "{0}".
 no-complex-type.error=Can not get complex type for non-primitive type {0}.
-subelement-create.error=Error creating sub-element.

<<attachment: newfiles.zip>>

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

Reply via email to