if a bean have multiple methods with same name corresponding to a con…

…figuration field, the methods can be returned randomly by the jvm and the search was not taken into account the implementation atttribute of a field to match the corresponding set method (#52)

* if a bean have multiple methods with same name corresponding to a configuration field, the methods can be returned randomly by the jvm and the search was not taken into account the implementation atttribute of a field to match the corresponding set method

Signed-off-by: Olivier Lamy <olamy@apache.org>

org.eclipse.sisu.plexus/src/main/java/org/eclipse/sisu/plexus/CompositeBeanHelper.java

@@ -79,7 +79,7 @@ public void setDefault( final Object bean, final Object defaultValue, final Plex
 
  // ----------------------------------------------------------------------
 
- final Method setter = findMethod( beanType, paramTypeHolder, "set" );
+ final Method setter = findMethod( beanType, paramTypeHolder, "set", null );
  if ( null == setter )
  {
  throw new ComponentConfigurationException( configuration, "Cannot find default setter in " + beanType );
@@ -140,10 +140,10 @@ public void setProperty( final Object bean, final String propertyName, final Cla
  // ----------------------------------------------------------------------
 
  final String title = Character.toTitleCase( propertyName.charAt( 0 ) ) + propertyName.substring( 1 );
- Method setter = findMethod( beanType, paramTypeHolder, "set" + title );
+ Method setter = findMethod( beanType, paramTypeHolder, "set" + title, valueType );
  if ( null == setter )
  {
- setter = findMethod( beanType, paramTypeHolder, "add" + title );
+ setter = findMethod( beanType, paramTypeHolder, "add" + title, valueType );
  }
 
  // ----------------------------------------------------------------------
@@ -264,21 +264,35 @@ private Object convertProperty( final Class<?> beanType, final Class<?> rawPrope
  listener );
  }
 
- private static Method findMethod( final Class<?> beanType, final Type[] paramTypeHolder, final String methodName )
+ private Method findMethod( final Class<?> beanType, final Type[] paramTypeHolder, final String methodName,
+ final Class<?> valueType )
  {
+ Method candidate = null;
  for ( final Method m : beanType.getMethods() )
  {
  if ( methodName.equals( m.getName() ) && !Modifier.isStatic( m.getModifiers() ) )
  {
  final Type[] paramTypes = m.getGenericParameterTypes();
  if ( paramTypes.length == 1 )
  {
- paramTypeHolder[0] = paramTypes[0];
- return m;
+ if ( valueType != null )
+ {
+ if ( m.getParameters()[0].getType().isAssignableFrom( valueType ) )
+ {
+ paramTypeHolder[0] = paramTypes[0];
+ return m;
+ }
+ }
+ // backward compat we keep returning the first method found
+ if ( candidate == null )
+ {
+ paramTypeHolder[0] = paramTypes[0];
+ candidate = m;
+ }
  }
  }
  }
- return null;
+ return candidate;
  }
 
  private static Field findField( final Class<?> beanType, final String fieldName )

org.eclipse.sisu.plexus/src/test/java/org/eclipse/sisu/plexus/BasicComponentConfiguratorTest.java

@@ -20,7 +20,13 @@
 import java.time.ZoneId;
 import java.time.ZoneOffset;
 import java.time.ZonedDateTime;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.stream.Collectors;
 
+import org.codehaus.plexus.classworlds.ClassWorld;
+import org.codehaus.plexus.classworlds.realm.ClassRealm;
 import org.codehaus.plexus.component.configurator.BasicComponentConfigurator;
 import org.codehaus.plexus.component.configurator.ComponentConfigurationException;
 import org.codehaus.plexus.component.configurator.ComponentConfigurator;
@@ -35,6 +41,7 @@
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
 
 public class BasicComponentConfiguratorTest
 {
@@ -122,6 +129,28 @@ public void testTemporalConvertersWithInvalidString()
  dateString, "zonedDateTime", dateString ) );
  }
 
+ @Test
+ public void testConfigureComplexBean()
+ throws Exception
+ {
+ ComplexBean complexBean = new ComplexBean();
+
+ // configure( complexBean, "resources", "foo;bar" );
+ DefaultPlexusConfiguration config = new DefaultPlexusConfiguration( "testConfig" );
+
+ DefaultPlexusConfiguration child = new DefaultPlexusConfiguration( "resources", "foo;bar" );
+ child.setAttribute( "implementation", "java.lang.String" );
+
+ config.addChild( child );
+
+ configure( complexBean, config,
+ new ClassWorld( "foo", Thread.currentThread().getContextClassLoader() ).getClassRealm( "foo" ) );
+
+ assertEquals( complexBean.resources.size(), 2 );
+ assertTrue( complexBean.resources.toString(), complexBean.resources.contains( Resource.newResource( "foo" ) ) );
+ assertTrue( complexBean.resources.toString(), complexBean.resources.contains( Resource.newResource( "bar" ) ) );
+ }
+
  private void configure( Object component, String... keysAndValues )
  throws ComponentConfigurationException
  {
@@ -139,6 +168,12 @@ private void configure( Object component, String... keysAndValues )
 
  private void configure( Object component, PlexusConfiguration config )
  throws ComponentConfigurationException
+ {
+ configure( component, config, null );
+ }
+
+ private void configure( Object component, PlexusConfiguration config, ClassRealm loader )
+ throws ComponentConfigurationException
  {
  final ExpressionEvaluator evaluator = new DefaultExpressionEvaluator()
  {
@@ -155,7 +190,7 @@ public File alignToBaseDirectory( File path )
  }
  }
  };
- configurator.configureComponent( component, config, evaluator, null );
+ configurator.configureComponent( component, config, evaluator, loader );
  }
 
  static final class PathTestComponent
@@ -211,4 +246,57 @@ static final class TemporalComponent
 
  ZonedDateTime zonedDateTime;
  }
+
+ static final class ComplexBean
+ {
+ private List<Resource> resources;
+
+ public void setResources( List<Resource> resources )
+ {
+ this.resources = resources;
+ }
+
+ public void setResources( String resources )
+ {
+ this.resources =
+ Arrays.stream( resources.split( ";" ) ).map( Resource::newResource ).collect( Collectors.toList() );
+ }
+
+ }
+
+ static abstract class Resource
+ {
+ String path;
+
+ static Resource newResource( String path )
+ {
+ return new BaseResource( path );
+ }
+
+ static class BaseResource
+ extends Resource
+ {
+ public BaseResource( String path )
+ {
+ this.path = path;
+ }
+ }
+
+ @Override
+ public boolean equals( Object o )
+ {
+ if ( this == o )
+ return true;
+ if ( o == null || getClass() != o.getClass() )
+ return false;
+ Resource resource = (Resource) o;
+ return Objects.equals( path, resource.path );
+ }
+
+ @Override
+ public int hashCode()
+ {
+ return Objects.hashCode( path );
+ }
+ }
 }