Skip to content
This repository has been archived by the owner on Sep 13, 2024. It is now read-only.

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 #52

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand Down Expand Up @@ -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 );
}

// ----------------------------------------------------------------------
Expand Down Expand Up @@ -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,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this was converted from class method to instance method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not really intentional. but could it be a problem for a private method?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's undo it, created #64

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 )
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
{
Expand Down Expand Up @@ -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
{
Expand All @@ -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()
{
Expand All @@ -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
Expand Down Expand Up @@ -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 );
}
}
}
Loading