From 77e0db50bcb18ee4865794f61fd30a4b9310fe32 Mon Sep 17 00:00:00 2001 From: Konrad Windszus Date: Thu, 29 Aug 2024 18:01:51 +0200 Subject: [PATCH] Call TypeAwareExpressionEvaluator.evaluate(String,Class) if available Only enforce types in some places (where conversion afterwards never happens) This closes #160 --- .../AbstractConfigurationConverter.java | 64 ++++++++++----- .../converters/basic/EnumConverter.java | 2 +- .../converters/composite/ArrayConverter.java | 2 +- .../composite/CollectionConverter.java | 2 +- .../converters/composite/MapConverter.java | 9 ++- .../composite/ObjectWithFieldsConverter.java | 3 +- .../composite/PropertiesConverter.java | 4 +- .../BasicComponentConfiguratorTest.java | 77 +++++++++++++++---- 8 files changed, 118 insertions(+), 45 deletions(-) diff --git a/org.eclipse.sisu.plexus/src/main/java/org/codehaus/plexus/component/configurator/converters/AbstractConfigurationConverter.java b/org.eclipse.sisu.plexus/src/main/java/org/codehaus/plexus/component/configurator/converters/AbstractConfigurationConverter.java index ffeaee2e..9fe6253e 100644 --- a/org.eclipse.sisu.plexus/src/main/java/org/codehaus/plexus/component/configurator/converters/AbstractConfigurationConverter.java +++ b/org.eclipse.sisu.plexus/src/main/java/org/codehaus/plexus/component/configurator/converters/AbstractConfigurationConverter.java @@ -18,6 +18,7 @@ import org.codehaus.plexus.component.configurator.converters.lookup.ConverterLookup; import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluationException; import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluator; +import org.codehaus.plexus.component.configurator.expression.TypeAwareExpressionEvaluator; import org.codehaus.plexus.configuration.PlexusConfiguration; import org.eclipse.sisu.plexus.Roles; @@ -37,45 +38,72 @@ public Object fromConfiguration( final ConverterLookup lookup, final PlexusConfi // Customizable methods // ---------------------------------------------------------------------- + /** + * + * @param configuration + * @param evaluator + * @return + * @throws ComponentConfigurationException + * @deprecated Use {@link #fromExpression(PlexusConfiguration, ExpressionEvaluator, Class)} instead + */ + @Deprecated protected Object fromExpression( final PlexusConfiguration configuration, final ExpressionEvaluator evaluator ) throws ComponentConfigurationException + { + return fromExpression( configuration, evaluator, null ); + } + + protected Object fromExpression( final PlexusConfiguration configuration, final ExpressionEvaluator evaluator, + final Class type ) + throws ComponentConfigurationException + { + return fromExpression( configuration, evaluator, type, true ); + } + + protected Object fromExpression( final PlexusConfiguration configuration, final ExpressionEvaluator evaluator, + final Class type, boolean enforceTypeCompatibility ) + throws ComponentConfigurationException + { String value = configuration.getValue(); try { Object result = null; - if ( null != value && value.length() > 0 ) - { - result = evaluator.evaluate( value ); - } - if ( null == result && configuration.getChildCount() == 0 ) + if ( null != value && !value.isEmpty() ) { - value = configuration.getAttribute( "default-value" ); - if ( null != value && value.length() > 0 ) + if ( evaluator instanceof TypeAwareExpressionEvaluator && type != null ) + { + result = ((TypeAwareExpressionEvaluator) evaluator).evaluate( value, type ); + } else { result = evaluator.evaluate( value ); } } + if ( null == result && configuration.getChildCount() == 0 ) { + value = configuration.getAttribute( "default-value" ); + if ( null != value && !value.isEmpty() ) { + if ( evaluator instanceof TypeAwareExpressionEvaluator && type != null ) { + result = ((TypeAwareExpressionEvaluator) evaluator).evaluate( value, type ); + } else { + result = evaluator.evaluate( value ); + } + } + } + if ( enforceTypeCompatibility && type != null ) { + failIfNotTypeCompatible( result, type, configuration ); + } return result; } catch ( final ExpressionEvaluationException e ) { - final String reason = String.format( "Cannot evaluate expression '%s' for configuration entry '%s'", value, - configuration.getName() ); + final String reason = + String.format( "Cannot evaluate expression '%s' for configuration entry '%s'", + value, configuration.getName() ); throw new ComponentConfigurationException( configuration, reason, e ); } } - protected Object fromExpression( final PlexusConfiguration configuration, final ExpressionEvaluator evaluator, - final Class type ) - throws ComponentConfigurationException - { - final Object result = fromExpression( configuration, evaluator ); - failIfNotTypeCompatible( result, type, configuration ); - return result; - } - // ---------------------------------------------------------------------- // Shared methods // ---------------------------------------------------------------------- diff --git a/org.eclipse.sisu.plexus/src/main/java/org/codehaus/plexus/component/configurator/converters/basic/EnumConverter.java b/org.eclipse.sisu.plexus/src/main/java/org/codehaus/plexus/component/configurator/converters/basic/EnumConverter.java index 763a7479..e9f01436 100644 --- a/org.eclipse.sisu.plexus/src/main/java/org/codehaus/plexus/component/configurator/converters/basic/EnumConverter.java +++ b/org.eclipse.sisu.plexus/src/main/java/org/codehaus/plexus/component/configurator/converters/basic/EnumConverter.java @@ -39,7 +39,7 @@ public Object fromConfiguration( final ConverterLookup lookup, final PlexusConfi + "' must not contain child elements" ); } - Object result = fromExpression( configuration, evaluator ); + Object result = fromExpression( configuration, evaluator, type, false ); if ( result instanceof String ) { try diff --git a/org.eclipse.sisu.plexus/src/main/java/org/codehaus/plexus/component/configurator/converters/composite/ArrayConverter.java b/org.eclipse.sisu.plexus/src/main/java/org/codehaus/plexus/component/configurator/converters/composite/ArrayConverter.java index 69e92ea0..e53ff063 100644 --- a/org.eclipse.sisu.plexus/src/main/java/org/codehaus/plexus/component/configurator/converters/composite/ArrayConverter.java +++ b/org.eclipse.sisu.plexus/src/main/java/org/codehaus/plexus/component/configurator/converters/composite/ArrayConverter.java @@ -49,7 +49,7 @@ public Object fromConfiguration( final ConverterLookup lookup, final PlexusConfi final ConfigurationListener listener ) throws ComponentConfigurationException { - final Object value = fromExpression( configuration, evaluator ); + final Object value = fromExpression( configuration, evaluator, type, false ); if ( type.isInstance( value ) ) { return value; diff --git a/org.eclipse.sisu.plexus/src/main/java/org/codehaus/plexus/component/configurator/converters/composite/CollectionConverter.java b/org.eclipse.sisu.plexus/src/main/java/org/codehaus/plexus/component/configurator/converters/composite/CollectionConverter.java index e0a010f5..f9302dd3 100644 --- a/org.eclipse.sisu.plexus/src/main/java/org/codehaus/plexus/component/configurator/converters/composite/CollectionConverter.java +++ b/org.eclipse.sisu.plexus/src/main/java/org/codehaus/plexus/component/configurator/converters/composite/CollectionConverter.java @@ -53,7 +53,7 @@ public Object fromConfiguration( final ConverterLookup lookup, final PlexusConfi final ConfigurationListener listener ) throws ComponentConfigurationException { - final Object value = fromExpression( configuration, evaluator ); + final Object value = fromExpression( configuration, evaluator, type, false ); if ( type.isInstance( value ) ) { return value; diff --git a/org.eclipse.sisu.plexus/src/main/java/org/codehaus/plexus/component/configurator/converters/composite/MapConverter.java b/org.eclipse.sisu.plexus/src/main/java/org/codehaus/plexus/component/configurator/converters/composite/MapConverter.java index 110727e2..d83c5074 100644 --- a/org.eclipse.sisu.plexus/src/main/java/org/codehaus/plexus/component/configurator/converters/composite/MapConverter.java +++ b/org.eclipse.sisu.plexus/src/main/java/org/codehaus/plexus/component/configurator/converters/composite/MapConverter.java @@ -60,13 +60,13 @@ public Object fromConfiguration( final ConverterLookup lookup, final PlexusConfi try { final Map map = instantiateMap( configuration, type, loader ); - final Type elementType = findElementType( typeArguments ); + final Type elementType = findElementClass( typeArguments ); if ( Object.class == elementType || String.class == elementType ) { for ( int i = 0, size = configuration.getChildCount(); i < size; i++ ) { final PlexusConfiguration element = configuration.getChild( i ); - map.put( element.getName(), fromExpression( element, evaluator ) ); + map.put( element.getName(), fromExpression( element, evaluator, (Class)elementType ) ); } return map; } @@ -96,7 +96,7 @@ public Object fromConfiguration( final ConverterLookup lookup, final PlexusConfi // TEMP: remove when http://jira.codehaus.org/browse/MSHADE-168 is fixed catch ( final ComponentConfigurationException e ) { - elementValue = fromExpression( element, evaluator ); + elementValue = fromExpression( element, evaluator, type ); Logs.warn( "Map in " + enclosingType + " declares value type as: {} but saw: {} at runtime", elementType, null != elementValue ? elementValue.getClass() : null ); @@ -132,7 +132,7 @@ private Map instantiateMap( final PlexusConfiguration configurat return (Map) impl; } - private static Type findElementType( final Type[] typeArguments ) + private static Type findElementClass( final Type[] typeArguments ) { if ( null != typeArguments && typeArguments.length > 1 ) { @@ -140,4 +140,5 @@ private static Type findElementType( final Type[] typeArguments ) } return Object.class; } + } diff --git a/org.eclipse.sisu.plexus/src/main/java/org/codehaus/plexus/component/configurator/converters/composite/ObjectWithFieldsConverter.java b/org.eclipse.sisu.plexus/src/main/java/org/codehaus/plexus/component/configurator/converters/composite/ObjectWithFieldsConverter.java index 2b054f87..29b8e060 100644 --- a/org.eclipse.sisu.plexus/src/main/java/org/codehaus/plexus/component/configurator/converters/composite/ObjectWithFieldsConverter.java +++ b/org.eclipse.sisu.plexus/src/main/java/org/codehaus/plexus/component/configurator/converters/composite/ObjectWithFieldsConverter.java @@ -40,7 +40,8 @@ public Object fromConfiguration( final ConverterLookup lookup, final PlexusConfi final ExpressionEvaluator evaluator, final ConfigurationListener listener ) throws ComponentConfigurationException { - final Object value = fromExpression( configuration, evaluator ); + // no type enforcement as also accepting string types used in the custom types constructor + final Object value = fromExpression( configuration, evaluator, type, false ); if ( type.isInstance( value ) ) { return value; diff --git a/org.eclipse.sisu.plexus/src/main/java/org/codehaus/plexus/component/configurator/converters/composite/PropertiesConverter.java b/org.eclipse.sisu.plexus/src/main/java/org/codehaus/plexus/component/configurator/converters/composite/PropertiesConverter.java index 8678acb9..3477aa46 100644 --- a/org.eclipse.sisu.plexus/src/main/java/org/codehaus/plexus/component/configurator/converters/composite/PropertiesConverter.java +++ b/org.eclipse.sisu.plexus/src/main/java/org/codehaus/plexus/component/configurator/converters/composite/PropertiesConverter.java @@ -48,7 +48,7 @@ public Object fromConfiguration( final ConverterLookup lookup, final PlexusConfi final PlexusConfiguration element = configuration.getChild( i ); if ( element.getChildCount() > 0 ) { - final Object name = fromExpression( element.getChild( "name" ), evaluator ); + final Object name = fromExpression( element.getChild( "name" ), evaluator, String.class, false ); setProperty( properties, name, element.getChild( "value" ), evaluator ); } else @@ -91,7 +91,7 @@ private void setProperty( final Properties properties, final Object name, final String key = null != name ? name.toString() : null; if ( null != key ) { - final Object value = fromExpression( valueConfiguration, evaluator ); + final Object value = fromExpression( valueConfiguration, evaluator, String.class, false ); properties.setProperty( key, null != value ? value.toString() : "" ); } else diff --git a/org.eclipse.sisu.plexus/src/test/java/org/eclipse/sisu/plexus/BasicComponentConfiguratorTest.java b/org.eclipse.sisu.plexus/src/test/java/org/eclipse/sisu/plexus/BasicComponentConfiguratorTest.java index 031f1202..f53247f7 100644 --- a/org.eclipse.sisu.plexus/src/test/java/org/eclipse/sisu/plexus/BasicComponentConfiguratorTest.java +++ b/org.eclipse.sisu.plexus/src/test/java/org/eclipse/sisu/plexus/BasicComponentConfiguratorTest.java @@ -31,7 +31,9 @@ import org.codehaus.plexus.component.configurator.ComponentConfigurationException; import org.codehaus.plexus.component.configurator.ComponentConfigurator; import org.codehaus.plexus.component.configurator.expression.DefaultExpressionEvaluator; +import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluationException; import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluator; +import org.codehaus.plexus.component.configurator.expression.TypeAwareExpressionEvaluator; import org.codehaus.plexus.configuration.DefaultPlexusConfiguration; import org.codehaus.plexus.configuration.PlexusConfiguration; import org.junit.Before; @@ -40,6 +42,7 @@ import org.junit.rules.TemporaryFolder; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; @@ -81,6 +84,38 @@ public void testTypeWithoutConverterButConstructorAcceptingString() assertEquals( "hello world", component.custom.toString() ); } + @Test + public void testTypePassedToExpressionEvaluator() + throws ComponentConfigurationException + { + CustomTypeComponent component = new CustomTypeComponent(); + ExpressionEvaluator evaluator = new TypeAwareExpressionEvaluator() + { + @Override + public Object evaluate( String expression, Class type ) + throws ExpressionEvaluationException + { + assertEquals( CustomType.class, type ); + return expression; + } + + @Override + public Object evaluate( String expression ) throws ExpressionEvaluationException + { + fail( "Wrong evaluate method being called (without type)" ); + return expression; // unreachable + } + + @Override + public File alignToBaseDirectory(File path) + { + return path; + } + }; + configure( evaluator, component, "custom", "hello world" ); + assertEquals( "hello world", component.custom.toString() ); + } + @Test public void testTemporalConvertersWithoutMillisecondsAndOffset() throws ComponentConfigurationException @@ -119,7 +154,6 @@ public void testTemporalConvertersWithISO8601StringWithOffset() @Test public void testTemporalConvertersWithInvalidString() - throws ComponentConfigurationException { TemporalComponent component = new TemporalComponent(); String dateString = "invalid"; @@ -143,7 +177,7 @@ public void testConfigureComplexBean() config.addChild( child ); - configure( complexBean, config, + configure( null, complexBean, config, new ClassWorld( "foo", Thread.currentThread().getContextClassLoader() ).getClassRealm( "foo" ) ); assertEquals( complexBean.resources.size(), 2 ); @@ -153,6 +187,12 @@ public void testConfigureComplexBean() private void configure( Object component, String... keysAndValues ) throws ComponentConfigurationException + { + configure( null, component, keysAndValues ); + } + + private void configure( ExpressionEvaluator evaluator, Object component, String... keysAndValues ) + throws ComponentConfigurationException { final DefaultPlexusConfiguration config = new DefaultPlexusConfiguration( "testConfig" ); if ( keysAndValues.length % 2 != 0 ) @@ -163,33 +203,36 @@ private void configure( Object component, String... keysAndValues ) { config.addChild( keysAndValues[i], keysAndValues[i + 1] ); } - configure( component, config ); + configure( evaluator, component, config ); } - private void configure( Object component, PlexusConfiguration config ) + private void configure( ExpressionEvaluator evaluator, Object component, PlexusConfiguration config ) throws ComponentConfigurationException { - configure( component, config, null ); + configure( evaluator, component, config, null ); } - private void configure( Object component, PlexusConfiguration config, ClassRealm loader ) + private void configure( ExpressionEvaluator evaluator, Object component, PlexusConfiguration config, ClassRealm loader ) throws ComponentConfigurationException { - final ExpressionEvaluator evaluator = new DefaultExpressionEvaluator() + if ( evaluator == null ) { - @Override - public File alignToBaseDirectory( File path ) + evaluator = new DefaultExpressionEvaluator() { - if ( !path.isAbsolute() ) - { - return new File( tmpDirectory.getRoot(), path.getPath() ); - } - else + @Override + public File alignToBaseDirectory( File path ) { - return path; + if ( !path.isAbsolute() ) + { + return new File( tmpDirectory.getRoot(), path.getPath() ); + } + else + { + return path; + } } - } - }; + }; + } configurator.configureComponent( component, config, evaluator, loader ); }