Fix unneeded/wrong substring calls in expression evaluation and put back weak references in cache (#1258)

This commit is contained in:
Guillaume Nodet 2023-09-27 09:28:31 +02:00 committed by GitHub
parent 5c2e671a06
commit 14d16064f6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 24 additions and 21 deletions

View File

@ -172,11 +172,11 @@ public class PluginParameterExpressionEvaluator implements TypeAwareExpressionEv
int pathSeparator = expression.indexOf('/'); int pathSeparator = expression.indexOf('/');
if (pathSeparator > 0) { if (pathSeparator > 0) {
String pathExpression = expression.substring(1, pathSeparator); String pathExpression = expression.substring(0, pathSeparator);
value = ReflectionValueExtractor.evaluate(pathExpression, session); value = ReflectionValueExtractor.evaluate(pathExpression, session);
value = value + expression.substring(pathSeparator); value = value + expression.substring(pathSeparator);
} else { } else {
value = ReflectionValueExtractor.evaluate(expression.substring(1), session); value = ReflectionValueExtractor.evaluate(expression, session);
} }
} catch (Exception e) { } catch (Exception e) {
// TODO don't catch exception // TODO don't catch exception
@ -198,7 +198,7 @@ public class PluginParameterExpressionEvaluator implements TypeAwareExpressionEv
value = ReflectionValueExtractor.evaluate(pathExpression, project); value = ReflectionValueExtractor.evaluate(pathExpression, project);
value = value + expression.substring(pathSeparator); value = value + expression.substring(pathSeparator);
} else { } else {
value = ReflectionValueExtractor.evaluate(expression.substring(1), project); value = ReflectionValueExtractor.evaluate(expression, project);
} }
} catch (Exception e) { } catch (Exception e) {
// TODO don't catch exception // TODO don't catch exception
@ -214,11 +214,11 @@ public class PluginParameterExpressionEvaluator implements TypeAwareExpressionEv
int pathSeparator = expression.indexOf('/'); int pathSeparator = expression.indexOf('/');
if (pathSeparator > 0) { if (pathSeparator > 0) {
String pathExpression = expression.substring(1, pathSeparator); String pathExpression = expression.substring(0, pathSeparator);
value = ReflectionValueExtractor.evaluate(pathExpression, mojoExecution); value = ReflectionValueExtractor.evaluate(pathExpression, mojoExecution);
value = value + expression.substring(pathSeparator); value = value + expression.substring(pathSeparator);
} else { } else {
value = ReflectionValueExtractor.evaluate(expression.substring(1), mojoExecution); value = ReflectionValueExtractor.evaluate(expression, mojoExecution);
} }
} catch (Exception e) { } catch (Exception e) {
// TODO don't catch exception // TODO don't catch exception
@ -234,11 +234,11 @@ public class PluginParameterExpressionEvaluator implements TypeAwareExpressionEv
PluginDescriptor pluginDescriptor = mojoDescriptor.getPluginDescriptor(); PluginDescriptor pluginDescriptor = mojoDescriptor.getPluginDescriptor();
if (pathSeparator > 0) { if (pathSeparator > 0) {
String pathExpression = expression.substring(1, pathSeparator); String pathExpression = expression.substring(0, pathSeparator);
value = ReflectionValueExtractor.evaluate(pathExpression, pluginDescriptor); value = ReflectionValueExtractor.evaluate(pathExpression, pluginDescriptor);
value = value + expression.substring(pathSeparator); value = value + expression.substring(pathSeparator);
} else { } else {
value = ReflectionValueExtractor.evaluate(expression.substring(1), pluginDescriptor); value = ReflectionValueExtractor.evaluate(expression, pluginDescriptor);
} }
} catch (Exception e) { } catch (Exception e) {
throw new ExpressionEvaluationException( throw new ExpressionEvaluationException(
@ -251,11 +251,11 @@ public class PluginParameterExpressionEvaluator implements TypeAwareExpressionEv
int pathSeparator = expression.indexOf('/'); int pathSeparator = expression.indexOf('/');
if (pathSeparator > 0) { if (pathSeparator > 0) {
String pathExpression = expression.substring(1, pathSeparator); String pathExpression = expression.substring(0, pathSeparator);
value = ReflectionValueExtractor.evaluate(pathExpression, session.getSettings()); value = ReflectionValueExtractor.evaluate(pathExpression, session.getSettings());
value = value + expression.substring(pathSeparator); value = value + expression.substring(pathSeparator);
} else { } else {
value = ReflectionValueExtractor.evaluate(expression.substring(1), session.getSettings()); value = ReflectionValueExtractor.evaluate(expression, session.getSettings());
} }
} catch (Exception e) { } catch (Exception e) {
// TODO don't catch exception // TODO don't catch exception

View File

@ -175,11 +175,11 @@ public class PluginParameterExpressionEvaluatorV4 implements TypeAwareExpression
int pathSeparator = expression.indexOf('/'); int pathSeparator = expression.indexOf('/');
if (pathSeparator > 0) { if (pathSeparator > 0) {
String pathExpression = expression.substring(1, pathSeparator); String pathExpression = expression.substring(0, pathSeparator);
value = ReflectionValueExtractor.evaluate(pathExpression, session); value = ReflectionValueExtractor.evaluate(pathExpression, session);
value = value + expression.substring(pathSeparator); value = value + expression.substring(pathSeparator);
} else { } else {
value = ReflectionValueExtractor.evaluate(expression.substring(1), session); value = ReflectionValueExtractor.evaluate(expression, session);
} }
} catch (Exception e) { } catch (Exception e) {
// TODO don't catch exception // TODO don't catch exception
@ -206,7 +206,7 @@ public class PluginParameterExpressionEvaluatorV4 implements TypeAwareExpression
value = ReflectionValueExtractor.evaluate(pathExpression, project); value = ReflectionValueExtractor.evaluate(pathExpression, project);
value = value + expression.substring(pathSeparator); value = value + expression.substring(pathSeparator);
} else { } else {
value = ReflectionValueExtractor.evaluate(expression.substring(1), project); value = ReflectionValueExtractor.evaluate(expression, project);
} }
} catch (Exception e) { } catch (Exception e) {
// TODO don't catch exception // TODO don't catch exception
@ -223,11 +223,11 @@ public class PluginParameterExpressionEvaluatorV4 implements TypeAwareExpression
int pathSeparator = expression.indexOf('/'); int pathSeparator = expression.indexOf('/');
if (pathSeparator > 0) { if (pathSeparator > 0) {
String pathExpression = expression.substring(1, pathSeparator); String pathExpression = expression.substring(0, pathSeparator);
value = ReflectionValueExtractor.evaluate(pathExpression, mojoExecution); value = ReflectionValueExtractor.evaluate(pathExpression, mojoExecution);
value = value + expression.substring(pathSeparator); value = value + expression.substring(pathSeparator);
} else { } else {
value = ReflectionValueExtractor.evaluate(expression.substring(1), mojoExecution); value = ReflectionValueExtractor.evaluate(expression, mojoExecution);
} }
} catch (Exception e) { } catch (Exception e) {
// TODO don't catch exception // TODO don't catch exception
@ -246,11 +246,11 @@ public class PluginParameterExpressionEvaluatorV4 implements TypeAwareExpression
mojoExecution.getMojoDescriptor().getPluginDescriptor(); mojoExecution.getMojoDescriptor().getPluginDescriptor();
if (pathSeparator > 0) { if (pathSeparator > 0) {
String pathExpression = expression.substring(1, pathSeparator); String pathExpression = expression.substring(0, pathSeparator);
value = ReflectionValueExtractor.evaluate(pathExpression, pluginDescriptor); value = ReflectionValueExtractor.evaluate(pathExpression, pluginDescriptor);
value = value + expression.substring(pathSeparator); value = value + expression.substring(pathSeparator);
} else { } else {
value = ReflectionValueExtractor.evaluate(expression.substring(1), pluginDescriptor); value = ReflectionValueExtractor.evaluate(expression, pluginDescriptor);
} }
} catch (Exception e) { } catch (Exception e) {
throw new ExpressionEvaluationException( throw new ExpressionEvaluationException(
@ -263,11 +263,11 @@ public class PluginParameterExpressionEvaluatorV4 implements TypeAwareExpression
int pathSeparator = expression.indexOf('/'); int pathSeparator = expression.indexOf('/');
if (pathSeparator > 0) { if (pathSeparator > 0) {
String pathExpression = expression.substring(1, pathSeparator); String pathExpression = expression.substring(0, pathSeparator);
value = ReflectionValueExtractor.evaluate(pathExpression, session.getSettings()); value = ReflectionValueExtractor.evaluate(pathExpression, session.getSettings());
value = value + expression.substring(pathSeparator); value = value + expression.substring(pathSeparator);
} else { } else {
value = ReflectionValueExtractor.evaluate(expression.substring(1), session.getSettings()); value = ReflectionValueExtractor.evaluate(expression, session.getSettings());
} }
} catch (Exception e) { } catch (Exception e) {
// TODO don't catch exception // TODO don't catch exception

View File

@ -18,6 +18,8 @@
*/ */
package org.apache.maven.plugin; package org.apache.maven.plugin;
import java.lang.ref.Reference;
import java.lang.ref.WeakReference;
import java.lang.reflect.Array; import java.lang.reflect.Array;
import java.lang.reflect.InvocationTargetException; import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method; import java.lang.reflect.Method;
@ -41,7 +43,7 @@ class ReflectionValueExtractor {
* This approach prevents permgen space overflows due to retention of discarded * This approach prevents permgen space overflows due to retention of discarded
* classloaders. * classloaders.
*/ */
private static final Map<Class<?>, ClassMap> CLASS_MAPS = new WeakHashMap<>(); private static final Map<Class<?>, WeakReference<ClassMap>> CLASS_MAPS = new WeakHashMap<>();
static final int EOF = -1; static final int EOF = -1;
@ -285,12 +287,13 @@ class ReflectionValueExtractor {
} }
private static ClassMap getClassMap(Class<?> clazz) { private static ClassMap getClassMap(Class<?> clazz) {
ClassMap classMap = CLASS_MAPS.get(clazz); Reference<ClassMap> ref = CLASS_MAPS.get(clazz);
ClassMap classMap = ref != null ? ref.get() : null;
if (classMap == null) { if (classMap == null) {
classMap = new ClassMap(clazz); classMap = new ClassMap(clazz);
CLASS_MAPS.put(clazz, classMap); CLASS_MAPS.put(clazz, new WeakReference<>(classMap));
} }
return classMap; return classMap;