From a153249c24c32d3ab81850aaf7a263a78ca23f99 Mon Sep 17 00:00:00 2001 From: Igor Fedorenko Date: Wed, 18 Dec 2013 09:32:44 -0500 Subject: [PATCH] MNG-5553 reworked dotted expressions interpreter Introduced state machine to correctly parse mapped property keys and likely other cases where original code misbehaved. Added more unit tests to cover more legal and illegal expressions. Signed-off-by: Igor Fedorenko --- .../ReflectionValueExtractor.java | 378 +++++++++++------- .../ReflectionValueExtractorTest.java | 165 +++++++- 2 files changed, 387 insertions(+), 156 deletions(-) diff --git a/src/main/java/org/codehaus/plexus/util/introspection/ReflectionValueExtractor.java b/src/main/java/org/codehaus/plexus/util/introspection/ReflectionValueExtractor.java index 96cdfbab..53a124f2 100644 --- a/src/main/java/org/codehaus/plexus/util/introspection/ReflectionValueExtractor.java +++ b/src/main/java/org/codehaus/plexus/util/introspection/ReflectionValueExtractor.java @@ -16,63 +16,142 @@ * limitations under the License. */ -import org.codehaus.plexus.util.StringUtils; - import java.lang.ref.WeakReference; +import java.lang.reflect.Array; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.util.*; -import java.util.regex.Matcher; -import java.util.regex.Pattern; +import java.util.List; +import java.util.Map; +import java.util.WeakHashMap; + +import org.codehaus.plexus.util.StringUtils; /** - *

Using simple dotted expressions to extract the values from an Object instance, - * For example we might want to extract a value like: project.build.sourceDirectory

- * - *

The implementation supports indexed, nested and mapped properties similar to the JSP way.

- * + *

+ * Using simple dotted expressions to extract the values from an Object instance, For example we might want to extract a + * value like: project.build.sourceDirectory + *

+ *

+ * The implementation supports indexed, nested and mapped properties similar to the JSP way. + *

+ * * @author Jason van Zyl * @author Vincent Siveton * @version $Id$ - * @see http://struts.apache.org/1.x/struts-taglib/indexedprops.html + * @see http://struts.apache.org/1.x/struts-taglib/indexedprops.html */ public class ReflectionValueExtractor { - private static final Class[] CLASS_ARGS = new Class[0]; + private static final Class[] CLASS_ARGS = new Class[0]; private static final Object[] OBJECT_ARGS = new Object[0]; /** - * Use a WeakHashMap here, so the keys (Class objects) can be garbage collected. - * This approach prevents permgen space overflows due to retention of discarded - * classloaders. + * Use a WeakHashMap here, so the keys (Class objects) can be garbage collected. This approach prevents permgen + * space overflows due to retention of discarded classloaders. */ - private static final Map> classMaps = new WeakHashMap>(); + private static final Map, WeakReference> classMaps = + new WeakHashMap, WeakReference>(); - /** - * Indexed properties pattern, ie (\\w+)\\[(\\d+)\\] - */ - private static final Pattern INDEXED_PROPS = Pattern.compile( "(\\w+)\\[(\\d+)\\]" ); + static final int EOF = -1; - /** - * Indexed properties pattern, ie (\\w+)\\((.+)\\) - */ - private static final Pattern MAPPED_PROPS = Pattern.compile( "(\\w+)\\((.+)\\)" ); + static final char PROPERTY_START = '.'; + + static final char INDEXED_START = '['; + + static final char INDEXED_END = ']'; + + static final char MAPPED_START = '('; + + static final char MAPPED_END = ')'; + + static class Tokenizer + { + final String expression; + + int idx; + + public Tokenizer( String expression ) + { + this.expression = expression; + } + + public int peekChar() + { + return idx < expression.length() ? expression.charAt( idx ) : EOF; + } + + public int skipChar() + { + return idx < expression.length() ? expression.charAt( idx++ ) : EOF; + } + + public String nextToken( char delimiter ) + { + int start = idx; + + while ( idx < expression.length() && delimiter != expression.charAt( idx ) ) + { + idx++; + } + + // delimiter MUST be present + if ( idx <= start || idx >= expression.length() ) + { + return null; + } + + return expression.substring( start, idx++ ); + } + + public String nextPropertyName() + { + final int start = idx; + + while ( idx < expression.length() && Character.isJavaIdentifierPart( expression.charAt( idx ) ) ) + { + idx++; + } + + // property name does not require delimiter + if ( idx <= start || idx > expression.length() ) + { + return null; + } + + return expression.substring( start, idx ); + } + + public int getPosition() + { + return idx < expression.length() ? idx : EOF; + } + + // to make tokenizer look pretty in debugger + @Override + public String toString() + { + return idx < expression.length() ? expression.substring( idx ) : ""; + } + } private ReflectionValueExtractor() { } /** - *

The implementation supports indexed, nested and mapped properties.

- * + *

+ * The implementation supports indexed, nested and mapped properties. + *

*
    *
  • nested properties should be defined by a dot, i.e. "user.address.street"
    • *
  • indexed properties (java.util.List or array instance) should be contains (\\w+)\\[(\\d+)\\] * pattern, i.e. "user.addresses[1].street"
    • - *
  • mapped properties should be contains (\\w+)\\((.+)\\) pattern, i.e. "user.addresses(myAddress).street"
    • + *
  • mapped properties should be contains (\\w+)\\((.+)\\) pattern, i.e. + * "user.addresses(myAddress).street"
    • *
      - * + * * @param expression not null expression * @param root not null object * @return the object defined by the expression @@ -85,30 +164,26 @@ public static Object evaluate( String expression, Object root ) } /** - *

      The implementation supports indexed, nested and mapped properties.

      - * + *

      + * The implementation supports indexed, nested and mapped properties. + *

      *
        *
      • nested properties should be defined by a dot, i.e. "user.address.street"
        • *
      • indexed properties (java.util.List or array instance) should be contains (\\w+)\\[(\\d+)\\] * pattern, i.e. "user.addresses[1].street"
        • - *
      • mapped properties should be contains (\\w+)\\((.+)\\) pattern, i.e. "user.addresses(myAddress).street"
        • + *
      • mapped properties should be contains (\\w+)\\((.+)\\) pattern, i.e. + * "user.addresses(myAddress).street"
        • *
          - * + * * @param expression not null expression * @param root not null object * @return the object defined by the expression * @throws Exception if any */ // TODO: don't throw Exception - public static Object evaluate( String expression, Object root, boolean trimRootToken ) + public static Object evaluate( final String expression, final Object root, final boolean trimRootToken ) throws Exception { - // if the root token refers to the supplied root object parameter, remove it. - if ( trimRootToken ) - { - expression = expression.substring( expression.indexOf( '.' ) + 1 ); - } - Object value = root; // ---------------------------------------------------------------------- @@ -116,131 +191,166 @@ public static Object evaluate( String expression, Object root, boolean trimRootT // MavenProject instance. // ---------------------------------------------------------------------- - StringTokenizer parser = new StringTokenizer( expression, "." ); + if ( StringUtils.isEmpty( expression ) || !Character.isJavaIdentifierStart( expression.charAt( 0 ) ) ) + { + return null; + } - while ( parser.hasMoreTokens() ) + final Tokenizer tokenizer; + if ( trimRootToken ) { - // if we have nothing, stop now - if ( value == null ) + tokenizer = new Tokenizer( expression ); + tokenizer.nextPropertyName(); + } + else + { + tokenizer = new Tokenizer( "." + expression ); + } + + int propertyPosition = tokenizer.getPosition(); + while ( value != null && tokenizer.peekChar() != EOF ) + { + switch ( tokenizer.skipChar() ) { - return null; + case INDEXED_START: + value = + getIndexedValue( expression, propertyPosition, tokenizer.getPosition(), value, + tokenizer.nextToken( INDEXED_END ) ); + break; + case MAPPED_START: + value = + getMappedValue( expression, propertyPosition, tokenizer.getPosition(), value, + tokenizer.nextToken( MAPPED_END ) ); + break; + case PROPERTY_START: + propertyPosition = tokenizer.getPosition(); + value = getPropertyValue( value, tokenizer.nextPropertyName() ); + break; + default: + // could not parse expression + return null; } + } - String token = parser.nextToken(); + return value; + } + private static Object getMappedValue( final String expression, final int from, final int to, final Object value, + final String key ) + throws Exception + { + if ( value == null || key == null ) + { + return null; + } + + if ( value instanceof Map ) + { + Object[] localParams = new Object[] { key }; ClassMap classMap = getClassMap( value.getClass() ); + Method method = classMap.findMethod( "get", localParams ); + return method.invoke( value, localParams ); + } + + final String message = + String.format( "The token '%s' at position '%d' refers to a java.util.Map, but the value seems is an instance of '%s'", + expression.subSequence( from, to ), from, value.getClass() ); - Method method; - Object[] localParams = OBJECT_ARGS; + throw new Exception( message ); + } - // do we have an indexed property? - Matcher matcher = INDEXED_PROPS.matcher( token ); - if ( matcher.find() ) + private static Object getIndexedValue( final String expression, final int from, final int to, final Object value, + final String indexStr ) + throws Exception + { + try + { + int index = Integer.parseInt( indexStr ); + + if ( value.getClass().isArray() ) { - String methodBase = StringUtils.capitalizeFirstLetter( matcher.group( 1 ) ); - String methodName = "get" + methodBase; - method = classMap.findMethod( methodName, CLASS_ARGS ); - value = method.invoke( value, OBJECT_ARGS ); - classMap = getClassMap( value.getClass() ); - - if ( classMap.getCachedClass().isArray() ) - { - value = Arrays.asList( (Object[]) value ); - classMap = getClassMap( value.getClass() ); - } - - if ( value instanceof List ) - { - // use get method on List interface - localParams = new Object[1]; - localParams[0] = Integer.valueOf( matcher.group( 2 ) ); - method = classMap.findMethod( "get", localParams ); - } - else - { - throw new Exception( "The token '" + token - + "' refers to a java.util.List or an array, but the value seems is an instance of '" - + value.getClass() + "'." ); - } + return Array.get( value, index ); } - else + + if ( value instanceof List ) { - // do we have a mapped property? - matcher = MAPPED_PROPS.matcher( token ); - if ( matcher.find() ) - { - String methodBase = StringUtils.capitalizeFirstLetter( matcher.group( 1 ) ); - String methodName = "get" + methodBase; - method = classMap.findMethod( methodName, CLASS_ARGS ); - value = method.invoke( value, OBJECT_ARGS ); - classMap = getClassMap( value.getClass() ); - - if ( value instanceof Map ) - { - // use get method on List interface - localParams = new Object[1]; - localParams[0] = matcher.group( 2 ); - method = classMap.findMethod( "get", localParams ); - } - else - { - throw new Exception( "The token '" + token - + "' refers to a java.util.Map, but the value seems is an instance of '" - + value.getClass() + "'." ); - } - } - else - { - String methodBase = StringUtils.capitalizeFirstLetter( token ); - String methodName = "get" + methodBase; - method = classMap.findMethod( methodName, CLASS_ARGS ); - - if ( method == null ) - { - // perhaps this is a boolean property?? - methodName = "is" + methodBase; - - method = classMap.findMethod( methodName, CLASS_ARGS ); - } - } + ClassMap classMap = getClassMap( value.getClass() ); + // use get method on List interface + Object[] localParams = new Object[] { index }; + Method method = classMap.findMethod( "get", localParams ); + return method.invoke( value, localParams ); } - - if ( method == null ) + } + catch ( NumberFormatException e ) + { + return null; + } + catch ( InvocationTargetException e ) + { + // catch array index issues gracefully, otherwise release + if ( e.getCause() instanceof IndexOutOfBoundsException ) { return null; } - try - { - value = method.invoke( value, localParams ); - } - catch ( InvocationTargetException e ) - { - // catch array index issues gracefully, otherwise release - if ( e.getCause() instanceof IndexOutOfBoundsException ) - { - return null; - } + throw e; + } - throw e; - } + final String message = + String.format( "The token '%s' at position '%d' refers to a java.util.List or an array, but the value seems is an instance of '%s'", + expression.subSequence( from, to ), from, value.getClass() ); + + throw new Exception( message ); + } + + private static Object getPropertyValue( Object value, String property ) + throws Exception + { + if ( value == null || property == null ) + { + return null; } - return value; + ClassMap classMap = getClassMap( value.getClass() ); + String methodBase = StringUtils.capitalizeFirstLetter( property ); + String methodName = "get" + methodBase; + Method method = classMap.findMethod( methodName, CLASS_ARGS ); + + if ( method == null ) + { + // perhaps this is a boolean property?? + methodName = "is" + methodBase; + + method = classMap.findMethod( methodName, CLASS_ARGS ); + } + + if ( method == null ) + { + return null; + } + + try + { + return method.invoke( value, OBJECT_ARGS ); + } + catch ( InvocationTargetException e ) + { + throw e; + } } - private static ClassMap getClassMap( Class clazz ) + private static ClassMap getClassMap( Class clazz ) { - WeakReference softRef = classMaps.get( clazz); + WeakReference softRef = classMaps.get( clazz ); ClassMap classMap; - if ( softRef == null || (classMap = softRef.get() ) == null) + if ( softRef == null || ( classMap = softRef.get() ) == null ) { classMap = new ClassMap( clazz ); - classMaps.put( clazz, new WeakReference(classMap) ); + classMaps.put( clazz, new WeakReference( classMap ) ); } return classMap; diff --git a/src/test/java/org/codehaus/plexus/util/introspection/ReflectionValueExtractorTest.java b/src/test/java/org/codehaus/plexus/util/introspection/ReflectionValueExtractorTest.java index d09bce95..f620176a 100644 --- a/src/test/java/org/codehaus/plexus/util/introspection/ReflectionValueExtractorTest.java +++ b/src/test/java/org/codehaus/plexus/util/introspection/ReflectionValueExtractorTest.java @@ -16,15 +16,15 @@ * limitations under the License. */ -import junit.framework.Assert; -import junit.framework.TestCase; - +import java.util.ArrayList; import java.util.HashMap; import java.util.Iterator; import java.util.List; -import java.util.ArrayList; import java.util.Map; +import junit.framework.Assert; +import junit.framework.TestCase; + /** * @author Jason van Zyl * @version $Id$ @@ -40,9 +40,9 @@ protected void setUp() super.setUp(); Dependency dependency1 = new Dependency(); - dependency1.setArtifactId("dep1"); + dependency1.setArtifactId( "dep1" ); Dependency dependency2 = new Dependency(); - dependency2.setArtifactId("dep2"); + dependency2.setArtifactId( "dep2" ); project = new Project(); project.setModelVersion( "4.0.0" ); @@ -95,39 +95,39 @@ public void testValueExtraction() // ---------------------------------------------------------------------- // List - Dependency dependency = (Dependency)ReflectionValueExtractor.evaluate( "project.dependencies[0]", project ); + Dependency dependency = (Dependency) ReflectionValueExtractor.evaluate( "project.dependencies[0]", project ); Assert.assertNotNull( dependency ); - Assert.assertTrue( "dep1".equals(dependency.getArtifactId()) ); + Assert.assertTrue( "dep1".equals( dependency.getArtifactId() ) ); - String artifactId = (String)ReflectionValueExtractor.evaluate( "project.dependencies[1].artifactId", project ); + String artifactId = (String) ReflectionValueExtractor.evaluate( "project.dependencies[1].artifactId", project ); - Assert.assertTrue( "dep2".equals(artifactId) ); + Assert.assertTrue( "dep2".equals( artifactId ) ); // Array - dependency = (Dependency)ReflectionValueExtractor.evaluate( "project.dependenciesAsArray[0]", project ); + dependency = (Dependency) ReflectionValueExtractor.evaluate( "project.dependenciesAsArray[0]", project ); Assert.assertNotNull( dependency ); - Assert.assertTrue( "dep1".equals(dependency.getArtifactId()) ); + Assert.assertTrue( "dep1".equals( dependency.getArtifactId() ) ); - artifactId = (String)ReflectionValueExtractor.evaluate( "project.dependenciesAsArray[1].artifactId", project ); + artifactId = (String) ReflectionValueExtractor.evaluate( "project.dependenciesAsArray[1].artifactId", project ); - Assert.assertTrue( "dep2".equals(artifactId) ); + Assert.assertTrue( "dep2".equals( artifactId ) ); // Map - dependency = (Dependency)ReflectionValueExtractor.evaluate( "project.dependenciesAsMap(dep1)", project ); + dependency = (Dependency) ReflectionValueExtractor.evaluate( "project.dependenciesAsMap(dep1)", project ); Assert.assertNotNull( dependency ); - Assert.assertTrue( "dep1".equals(dependency.getArtifactId()) ); + Assert.assertTrue( "dep1".equals( dependency.getArtifactId() ) ); - artifactId = (String)ReflectionValueExtractor.evaluate( "project.dependenciesAsMap(dep2).artifactId", project ); + artifactId = (String) ReflectionValueExtractor.evaluate( "project.dependenciesAsMap(dep2).artifactId", project ); - Assert.assertTrue( "dep2".equals(artifactId) ); + Assert.assertTrue( "dep2".equals( artifactId ) ); // ---------------------------------------------------------------------- // Build @@ -146,6 +146,112 @@ public void testValueExtractorWithAInvalidExpression() Assert.assertNull( ReflectionValueExtractor.evaluate( "project.dependencies[0].foo", project ) ); } + public void testMappedDottedKey() + throws Exception + { + Map map = new HashMap(); + map.put( "a.b", "a.b-value" ); + + Assert.assertEquals( "a.b-value", ReflectionValueExtractor.evaluate( "h.value(a.b)", new ValueHolder( map ) ) ); + } + + public void testIndexedMapped() + throws Exception + { + Map map = new HashMap(); + map.put( "a", "a-value" ); + List list = new ArrayList(); + list.add( map ); + + Assert.assertEquals( "a-value", ReflectionValueExtractor.evaluate( "h.value[0](a)", new ValueHolder( list ) ) ); + } + + public void testMappedIndexed() + throws Exception + { + List list = new ArrayList(); + list.add( "a-value" ); + Map map = new HashMap(); + map.put( "a", list ); + Assert.assertEquals( "a-value", ReflectionValueExtractor.evaluate( "h.value(a)[0]", new ValueHolder( map ) ) ); + } + + public void testMappedMissingDot() + throws Exception + { + Map map = new HashMap(); + map.put( "a", new ValueHolder( "a-value" ) ); + Assert.assertNull( ReflectionValueExtractor.evaluate( "h.value(a)value", new ValueHolder( map ) ) ); + } + + public void testIndexedMissingDot() + throws Exception + { + List list = new ArrayList(); + list.add( new ValueHolder( "a-value" ) ); + Assert.assertNull( ReflectionValueExtractor.evaluate( "h.value[0]value", new ValueHolder( list ) ) ); + } + + public void testDotDot() + throws Exception + { + Assert.assertNull( ReflectionValueExtractor.evaluate( "h..value", new ValueHolder( "value" ) ) ); + } + + public void testBadIndexedSyntax() + throws Exception + { + List list = new ArrayList(); + list.add( "a-value" ); + Object value = new ValueHolder( list ); + + Assert.assertNull( ReflectionValueExtractor.evaluate( "h.value[", value ) ); + Assert.assertNull( ReflectionValueExtractor.evaluate( "h.value[]", value ) ); + Assert.assertNull( ReflectionValueExtractor.evaluate( "h.value[a]", value ) ); + Assert.assertNull( ReflectionValueExtractor.evaluate( "h.value[0", value ) ); + Assert.assertNull( ReflectionValueExtractor.evaluate( "h.value[0)", value ) ); + Assert.assertNull( ReflectionValueExtractor.evaluate( "h.value[-1]", value ) ); + } + + public void testBadMappedSyntax() + throws Exception + { + Map map = new HashMap(); + map.put( "a", "a-value" ); + Object value = new ValueHolder( map ); + + Assert.assertNull( ReflectionValueExtractor.evaluate( "h.value(", value ) ); + Assert.assertNull( ReflectionValueExtractor.evaluate( "h.value()", value ) ); + Assert.assertNull( ReflectionValueExtractor.evaluate( "h.value(a", value ) ); + Assert.assertNull( ReflectionValueExtractor.evaluate( "h.value(a]", value ) ); + } + + public void testIllegalIndexedType() + throws Exception + { + try + { + ReflectionValueExtractor.evaluate( "h.value[1]", new ValueHolder( "string" ) ); + } + catch ( Exception e ) + { + // TODO assert exception message + } + } + + public void testIllegalMappedType() + throws Exception + { + try + { + ReflectionValueExtractor.evaluate( "h.value(key)", new ValueHolder( "string" ) ); + } + catch ( Exception e ) + { + // TODO assert exception message + } + } + public static class Project { private String modelVersion; @@ -246,15 +352,15 @@ public String getVersion() public Dependency[] getDependenciesAsArray() { - return (Dependency[]) getDependencies().toArray( new Dependency[0]); + return (Dependency[]) getDependencies().toArray( new Dependency[0] ); } public Map getDependenciesAsMap() { Map ret = new HashMap(); - for ( Iterator it = getDependencies().iterator(); it.hasNext();) + for ( Iterator it = getDependencies().iterator(); it.hasNext(); ) { - Dependency dep = (Dependency)it.next(); + Dependency dep = (Dependency) it.next(); ret.put( dep.getArtifactId(), dep ); } return ret; @@ -275,7 +381,7 @@ public String getArtifactId() return artifactId; } - public void setArtifactId(String id) + public void setArtifactId( String id ) { artifactId = id; } @@ -295,4 +401,19 @@ public String getConnection() return connection; } } + + public static class ValueHolder + { + private final Object value; + + public ValueHolder( Object value ) + { + this.value = value; + } + + public Object getValue() + { + return value; + } + } } \ No newline at end of file