This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 9784bafa940938ebfca20a0cb914a64e3dc28a62
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Feb 20 15:07:20 2024 +0000
Add method invocation support to OptionalELREsolver
Ensure consistent behaviour and ensure that the OptionalELResolver never
returns an Optional instance.
Ensure coerceToType coerces empty Optional instances based on a value of
null rather than just returning null.
Improve Javadoc
Add/update tests for new/changed behaviour
---
java/jakarta/el/OptionalELResolver.java | 135 +++++++++++++++--------
test/jakarta/el/TestOptionalELResolver.java | 76 ++++++++++++-
test/jakarta/el/TestOptionalELResolverInJsp.java | 18 +--
test/jakarta/el/TesterBeanB.java | 4 +
test/webapp/el-optional.jsp | 8 ++
webapps/docs/changelog.xml | 11 ++
6 files changed, 196 insertions(+), 56 deletions(-)
diff --git a/java/jakarta/el/OptionalELResolver.java
b/java/jakarta/el/OptionalELResolver.java
index c16c8f781b..f8ff316305 100644
--- a/java/jakarta/el/OptionalELResolver.java
+++ b/java/jakarta/el/OptionalELResolver.java
@@ -20,32 +20,36 @@ import java.util.Objects;
import java.util.Optional;
/**
- * Defines property resolution behaviour on {@link Optional}s.
+ * Defines property resolution, method invocation and type conversion
behaviour on {@link Optional}s.
* <p>
* This resolver handles base objects that are instances of {@link Optional}.
* <p>
- * If the {@link Optional#isEmpty()} is {@code true} for the base object and
the property is {@code null} then the
- * resulting value is {@code null}.
- * <p>
- * If the {@link Optional#isEmpty()} is {@code true} for the base object and
the property is not {@code null} then the
- * resulting value is the base object (an empty {@link Optional}).
- * <p>
- * If the {@link Optional#isPresent()} is {@code true} for the base object and
the property is {@code null} then the
- * resulting value is the result of calling {@link Optional#get()} on the base
object.
- * <p>
- * If the {@link Optional#isPresent()} is {@code true} for the base object and
the property is not {@code null} then the
- * resulting value is the result of calling {@link
ELResolver#getValue(ELContext, Object, Object)} using the
- * {@link ELResolver} obtained from {@link ELContext#getELResolver()} with the
following parameters:
- * <ul>
- * <li>The {@link ELContext} is the current context</li>
- * <li>The base object is the result of calling {@link Optional#get()} on the
current base object</li>
- * <li>The property object is the current property object</li>
- * </ul>
- * <p>
- * This resolver is always a read-only resolver.
+ * This resolver is always a read-only resolver since {@link Optional}
instances are immutable.
*/
public class OptionalELResolver extends ELResolver {
+ /**
+ * {@inheritDoc}
+ *
+ * @return If the base object is an {@link Optional} and {@link
Optional#isEmpty()} returns {@code true} then the
+ * resulting value is {@code null}.
+ * <p>
+ * If the base object is an {@link Optional}, {@link
Optional#isPresent()} returns {@code true} and the
+ * property is {@code null} then the resulting value is the
result of calling {@link Optional#get()} on
+ * the base object.
+ * <p>
+ * If the base object is an {@link Optional}, {@link
Optional#isPresent()} returns {@code true} and the
+ * property is not {@code null} then the resulting value is
the result of calling
+ * {@link ELResolver#getValue(ELContext, Object, Object)}
using the {@link ELResolver} obtained from
+ * {@link ELContext#getELResolver()} with the following
parameters:
+ * <ul>
+ * <li>The {@link ELContext} is the current context</li>
+ * <li>The base object is the result of calling {@link
Optional#get()} on the current base object</li>
+ * <li>The property object is the current property object</li>
+ * </ul>
+ * <p>
+ * If the base object is not an {@link Optional} then the
return value is undefined.
+ */
@Override
public Object getValue(ELContext context, Object base, Object property) {
Objects.requireNonNull(context);
@@ -55,8 +59,6 @@ public class OptionalELResolver extends ELResolver {
if (((Optional<?>) base).isEmpty()) {
if (property == null) {
return null;
- } else {
- return base;
}
} else {
if (property == null) {
@@ -74,9 +76,11 @@ public class OptionalELResolver extends ELResolver {
/**
* {@inheritDoc}
- * <p>
- * If the base object is an {@link Optional} this method always returns
{@code null} since instances of this
- * resolver are always read-only.
+ *
+ * @return If the base object is an {@link Optional} this method always
returns {@code null} since instances of this
+ * resolver are always read-only.
+ * <p>
+ * If the base object is not an {@link Optional} then the
return value is undefined.
*/
@Override
public Class<?> getType(ELContext context, Object base, Object property) {
@@ -109,9 +113,11 @@ public class OptionalELResolver extends ELResolver {
/**
* {@inheritDoc}
- * <p>
- * If the base object is an {@link Optional} this method always returns
{@code true} since instances of this
- * resolver are always read-only.
+ *
+ * @return If the base object is an {@link Optional} this method always
returns {@code true} since instances of this
+ * resolver are always read-only.
+ * <p>
+ * If the base object is not an {@link Optional} then the
return value is undefined.
*/
@Override
public boolean isReadOnly(ELContext context, Object base, Object property)
{
@@ -128,8 +134,10 @@ public class OptionalELResolver extends ELResolver {
/**
* {@inheritDoc}
- * <p>
- * If the base object is an {@link Optional} this method always returns
{@code Object.class}.
+ *
+ * @return If the base object is an {@link Optional} this method always
returns {@code Object.class}.
+ * <p>
+ * If the base object is not an {@link Optional} then the
return value is undefined.
*/
@Override
public Class<?> getCommonPropertyType(ELContext context, Object base) {
@@ -141,12 +149,24 @@ public class OptionalELResolver extends ELResolver {
}
+ /**
+ * {@inheritDoc}
+ *
+ * @return If the base object is an {@link Optional} and {@link
Optional#isEmpty()} returns {@code true} then this
+ * method returns the result of coercing {@code null} to the
requested {@code type}.
+ * <p>
+ * If the base object is an {@link Optional} and {@link
Optional#isPresent()} returns {@code true} then
+ * this method returns the result of coercing {@code
Optional#get()} to the requested {@code type}.
+ * <p>
+ * If the base object is not an {@link Optional} then the
return value is undefined.
+ */
@Override
public <T> T convertToType(ELContext context, Object obj, Class<T> type) {
Objects.requireNonNull(context);
if (obj instanceof Optional) {
+ Object value = null;
if (((Optional<?>) obj).isPresent()) {
- Object value = ((Optional<?>) obj).get();
+ value = ((Optional<?>) obj).get();
// If the value is assignable to the required type, do so.
if (type.isAssignableFrom(value.getClass())) {
context.setPropertyResolved(true);
@@ -154,24 +174,51 @@ public class OptionalELResolver extends ELResolver {
T result = (T) value;
return result;
}
+ }
- try {
- Object convertedValue = context.convertToType(value, type);
- context.setPropertyResolved(true);
- @SuppressWarnings("unchecked")
- T result = (T) convertedValue;
- return result;
- } catch (ELException e) {
- /*
- * TODO: This isn't pretty but it works. Significant
refactoring would be required to avoid the
- * exception. See also Util.isCoercibleFrom().
- */
- }
- } else {
+ try {
+ Object convertedValue = context.convertToType(value, type);
context.setPropertyResolved(true);
+ @SuppressWarnings("unchecked")
+ T result = (T) convertedValue;
+ return result;
+ } catch (ELException e) {
+ /*
+ * TODO: This isn't pretty but it works. Significant
refactoring would be required to avoid the
+ * exception. See also Util.isCoercibleFrom().
+ */
+ }
+ }
+ return null;
+ }
+
+
+ /**
+ * {@inheritDoc}
+ *
+ * @return If the base object is an {@link Optional} and {@link
Optional#isEmpty()} returns {@code true} then this
+ * method returns {@code null}.
+ * <p>
+ * If the base object is an {@link Optional} and {@link
Optional#isPresent()} returns {@code true} then
+ * this method returns the result of invoking the specified
method on the object obtained by calling
+ * {@link Optional#get()} with the specified parameters.
+ * <p>
+ * If the base object is not an {@link Optional} then the
return value is undefined.
+ */
+ @Override
+ public Object invoke(ELContext context, Object base, Object method,
Class<?>[] paramTypes, Object[] params) {
+ Objects.requireNonNull(context);
+
+ if (base instanceof Optional && method != null) {
+ context.setPropertyResolved(base, method);
+ if (((Optional<?>) base).isEmpty()) {
return null;
+ } else {
+ Object resolvedBase = ((Optional<?>) base).get();
+ return context.getELResolver().invoke(context, resolvedBase,
method, paramTypes, params);
}
}
+
return null;
}
}
diff --git a/test/jakarta/el/TestOptionalELResolver.java
b/test/jakarta/el/TestOptionalELResolver.java
index 38b171d3f2..e6dbb866ed 100644
--- a/test/jakarta/el/TestOptionalELResolver.java
+++ b/test/jakarta/el/TestOptionalELResolver.java
@@ -69,7 +69,7 @@ public class TestOptionalELResolver {
ValueExpression ve = factory.createValueExpression(context,
"${beanA.beanBOpt.name}", String.class);
Object result = ve.getValue(context);
- Assert.assertNull(result);
+ Assert.assertEquals("", result);
}
@@ -185,11 +185,11 @@ public class TestOptionalELResolver {
ValueExpression ve = factory.createValueExpression(context,
"${beanA.beanBOpt.map(b -> b.name)}", String.class);
Object result = ve.getValue(context);
- Assert.assertNull(result);
+ Assert.assertEquals("", result);
}
- @Test
+ @Test(expected = MethodNotFoundException.class)
public void testIssue176WithOptionalResolverOptionalPresentWithMap() {
ExpressionFactory factory = ExpressionFactory.newInstance();
StandardELContext context = new StandardELContext(factory);
@@ -203,8 +203,76 @@ public class TestOptionalELResolver {
context.getVariableMapper().setVariable("beanA", varBeanA);
ValueExpression ve = factory.createValueExpression(context,
"${beanA.beanBOpt.map(b -> b.name)}", String.class);
+ ve.getValue(context);
+ }
+
+
+ @Test(expected = MethodNotFoundException.class)
+ public void testWithoutOptionalResolverInvokeOnEmpty() {
+ ExpressionFactory factory = ExpressionFactory.newInstance();
+ StandardELContext context = new StandardELContext(factory);
+
+ TesterBeanA beanA = new TesterBeanA();
+
+ ValueExpression varBeanA = factory.createValueExpression(beanA,
TesterBeanA.class);
+ context.getVariableMapper().setVariable("beanA", varBeanA);
+
+ ValueExpression ve = factory.createValueExpression(context,
"${beanA.beanBOpt.doSomething()}", String.class);
+ ve.getValue(context);
+ }
+
+
+ @Test
+ public void testWithOptionalResolverInvokeOnEmpty() {
+ ExpressionFactory factory = ExpressionFactory.newInstance();
+ StandardELContext context = new StandardELContext(factory);
+ context.addELResolver(new OptionalELResolver());
+
+ TesterBeanA beanA = new TesterBeanA();
+
+ ValueExpression varBeanA = factory.createValueExpression(beanA,
TesterBeanA.class);
+ context.getVariableMapper().setVariable("beanA", varBeanA);
+
+ ValueExpression ve = factory.createValueExpression(context,
"${beanA.beanBOpt.doSomething()}", String.class);
Object result = ve.getValue(context);
- Assert.assertEquals("test", result);
+ Assert.assertEquals("", result);
+ }
+
+
+ @Test(expected = MethodNotFoundException.class)
+ public void testWithoutOptionalResolverInvokeOnPresent() {
+ ExpressionFactory factory = ExpressionFactory.newInstance();
+ StandardELContext context = new StandardELContext(factory);
+
+ TesterBeanA beanA = new TesterBeanA();
+ TesterBeanB beanB = new TesterBeanB("test");
+ beanA.setBeanB(beanB);
+
+ ValueExpression varBeanA = factory.createValueExpression(beanA,
TesterBeanA.class);
+ context.getVariableMapper().setVariable("beanA", varBeanA);
+
+ ValueExpression ve = factory.createValueExpression(context,
"${beanA.beanBOpt.doSomething()}", String.class);
+ ve.getValue(context);
+ }
+
+
+ @Test
+ public void testWithOptionalResolverInvokeOnPresent() {
+ ExpressionFactory factory = ExpressionFactory.newInstance();
+ StandardELContext context = new StandardELContext(factory);
+ context.addELResolver(new OptionalELResolver());
+
+ TesterBeanA beanA = new TesterBeanA();
+ TesterBeanB beanB = new TesterBeanB("test");
+ beanA.setBeanB(beanB);
+
+ ValueExpression varBeanA = factory.createValueExpression(beanA,
TesterBeanA.class);
+ context.getVariableMapper().setVariable("beanA", varBeanA);
+
+ ValueExpression ve = factory.createValueExpression(context,
"${beanA.beanBOpt.doSomething()}", String.class);
+ Object result = ve.getValue(context);
+
+ Assert.assertEquals(beanB.doSomething(), result);
}
}
diff --git a/test/jakarta/el/TestOptionalELResolverInJsp.java
b/test/jakarta/el/TestOptionalELResolverInJsp.java
index 0b81fa3db0..e0ae7f83f8 100644
--- a/test/jakarta/el/TestOptionalELResolverInJsp.java
+++ b/test/jakarta/el/TestOptionalELResolverInJsp.java
@@ -53,18 +53,20 @@ public class TestOptionalELResolverInJsp extends
TomcatBaseTest {
Assert.assertEquals(HttpServletResponse.SC_OK, rc);
String result = responseBody.toString();
- assertEcho(result, "00-null");
- assertEcho(result, "01-null");
- assertEcho(result, "02-null");
+ assertEcho(result, "00-");
+ assertEcho(result, "01-");
+ assertEcho(result, "02-");
+ assertEcho(result, "03-");
assertEcho(result, "10-This is an instance of TesterBeanB");
assertEcho(result, "11-test");
- assertEcho(result, "12-test");
- assertEcho(result, "20-null");
- assertEcho(result, "21-null");
- assertEcho(result, "22-null");
+ assertEcho(result, "13-Returned from the doSomething() method");
+ assertEcho(result, "20-");
+ assertEcho(result, "21-");
+ assertEcho(result, "22-");
+ assertEcho(result, "23-");
assertEcho(result, "30-This is an instance of TesterBeanB");
assertEcho(result, "31-test");
- assertEcho(result, "32-test");
+ assertEcho(result, "33-Returned from the doSomething() method");
}
diff --git a/test/jakarta/el/TesterBeanB.java b/test/jakarta/el/TesterBeanB.java
index 66f9de0d4c..c081965e26 100644
--- a/test/jakarta/el/TesterBeanB.java
+++ b/test/jakarta/el/TesterBeanB.java
@@ -32,4 +32,8 @@ public class TesterBeanB {
public String toString() {
return "This is an instance of TesterBeanB";
}
+
+ public String doSomething() {
+ return "Returned from the doSomething() method";
+ }
}
diff --git a/test/webapp/el-optional.jsp b/test/webapp/el-optional.jsp
index 8fcf3b919d..58fe5d10b7 100644
--- a/test/webapp/el-optional.jsp
+++ b/test/webapp/el-optional.jsp
@@ -34,15 +34,23 @@
<tags:echo echo="00-${testBeanA1.beanBOpt}" />
<tags:echo echo="01-${testBeanA1.beanBOpt.name}" />
<tags:echo echo="02-${testBeanA1.beanBOpt.map(b -> b.name)}" />
+ <tags:echo echo="03-${testBeanA1.beanBOpt.doSomething()}" />
<tags:echo echo="10-${testBeanA2.beanBOpt}" />
<tags:echo echo="11-${testBeanA2.beanBOpt.name}" />
+ <%-- Triggers MethodNotFoundException
<tags:echo echo="12-${testBeanA2.beanBOpt.map(b -> b.name)}" />
+ --%>
+ <tags:echo echo="13-${testBeanA2.beanBOpt.doSomething()}" />
<tags:echo-deferred echo="20-#{testBeanA1.beanBOpt}" />
<tags:echo-deferred echo="21-#{testBeanA1.beanBOpt.name}" />
<tags:echo-deferred echo="22-#{testBeanA1.beanBOpt.map(b -> b.name)}" />
+ <tags:echo-deferred echo="23-#{testBeanA1.beanBOpt.doSomething()}" />
<tags:echo-deferred echo="30-#{testBeanA2.beanBOpt}" />
<tags:echo-deferred echo="31-#{testBeanA2.beanBOpt.name}" />
+ <%-- Triggers MethodNotFoundException
<tags:echo-deferred echo="32-#{testBeanA2.beanBOpt.map(b -> b.name)}" />
+ --%>
+ <tags:echo-deferred echo="33-#{testBeanA2.beanBOpt.doSomething()}" />
</body>
</html>
\ No newline at end of file
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index bcc3491e88..32de8976c1 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -105,6 +105,17 @@
issues do not "pop up" wrt. others).
-->
<section name="Tomcat 11.0.0-M18 (markt)" rtext="in development">
+ <subsection name="Jasper">
+ <changelog>
+ <add>
+ Add method invocation support for <code>java.util.Optional</code> via
+ the <code>jakarta.el.OptionalELResolver</code> to Tomcat's
+ implementation of the Jakarta EL API to align with the latest proposals
+ for the Jakarta EL 6.0 API. The property support has also been refined
+ for greater consistency. (markt)
+ </add>
+ </changelog>
+ </subsection>
</section>
<section name="Tomcat 11.0.0-M17 (markt)" rtext="2024-02-19">
<subsection name="Catalina">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
