You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2021/09/16 16:16:22 UTC
[tomcat] branch main updated: Refactor ScopedAttributeELResolver
into three.
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
The following commit(s) were added to refs/heads/main by this push:
new 6519345 Refactor ScopedAttributeELResolver into three.
6519345 is described below
commit 65193458a48af7c966dd10f02d95941a52836a4d
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Sep 16 17:15:03 2021 +0100
Refactor ScopedAttributeELResolver into three.
ScopedAttributeELResolver - handles scoped attributes
ImportELResolver - handles imports
NotFoundELResolver - handles anything that isn't resolved
Separating them out makes the role of each clearer and makes some
planned (minor) functional enhancements simpler.
This is implemented in Tomcat first to confirm that it works before
making the same changes in the JSP API.
---
java/jakarta/servlet/jsp/el/ImportELResolver.java | 141 +++++++++++++++++++++
.../jakarta/servlet/jsp/el/NotFoundELResolver.java | 104 +++++++++++++++
.../servlet/jsp/el/ScopedAttributeELResolver.java | 58 +--------
java/org/apache/jasper/el/JasperELResolver.java | 7 +-
.../org/apache/jasper/el/TestJasperELResolver.java | 6 +-
webapps/docs/changelog.xml | 7 +
6 files changed, 264 insertions(+), 59 deletions(-)
diff --git a/java/jakarta/servlet/jsp/el/ImportELResolver.java b/java/jakarta/servlet/jsp/el/ImportELResolver.java
new file mode 100644
index 0000000..535e877
--- /dev/null
+++ b/java/jakarta/servlet/jsp/el/ImportELResolver.java
@@ -0,0 +1,141 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package jakarta.servlet.jsp.el;
+
+import java.beans.FeatureDescriptor;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.Objects;
+
+import jakarta.el.ELClass;
+import jakarta.el.ELContext;
+import jakarta.el.ELResolver;
+import jakarta.el.ImportHandler;
+
+/**
+ * Providers resolution of imports and static imports in the Jakarta Server
+ * Pages ELResolver chain.
+ *
+ * @since JSP 3.1
+ */
+public class ImportELResolver extends ELResolver {
+
+ // Indicates if a performance short-cut is available
+ private static final Class<?> AST_IDENTIFIER_KEY;
+
+ static {
+ Class<?> key = null;
+ try {
+ key = Class.forName("org.apache.el.parser.AstIdentifier");
+ } catch (Exception e) {
+ // Ignore: Expected if not running on Tomcat. Not a problem since
+ // this just allows a short-cut.
+ }
+ AST_IDENTIFIER_KEY = key;
+ }
+
+ @Override
+ public Object getValue(ELContext context, Object base, Object property) {
+ Objects.requireNonNull(context);
+
+ Object result = null;
+
+ if (base == null) {
+ if (property != null) {
+ boolean resolveClass = true;
+ // Performance short-cut available when running on Tomcat
+ if (AST_IDENTIFIER_KEY != null) {
+ // Tomcat will set this key to Boolean.TRUE if the
+ // identifier is a stand-alone identifier (i.e.
+ // identifier) rather than part of an AstValue (i.e.
+ // identifier.something). Imports do not need to be
+ // checked if this is a stand-alone identifier
+ Boolean value = (Boolean) context.getContext(AST_IDENTIFIER_KEY);
+ if (value != null && value.booleanValue()) {
+ resolveClass = false;
+ }
+ }
+
+ ImportHandler importHandler = context.getImportHandler();
+ if (importHandler != null) {
+ String key = property.toString();
+ Class<?> clazz = null;
+ if (resolveClass) {
+ clazz = importHandler.resolveClass(key);
+ if (clazz != null) {
+ result = new ELClass(clazz);
+ }
+ }
+ if (result == null) {
+ // This might be the name of an imported static field
+ clazz = importHandler.resolveStatic(key);
+ if (clazz != null) {
+ try {
+ result = clazz.getField(key).get(null);
+ } catch (IllegalArgumentException | IllegalAccessException |
+ NoSuchFieldException | SecurityException e) {
+ // Most (all?) of these should have been
+ // prevented by the checks when the import
+ // was defined.
+ }
+ }
+ }
+ }
+ }
+ }
+
+ if (result != null) {
+ context.setPropertyResolved(base, property);
+ }
+
+ return result;
+ }
+
+ @Override
+ public Class<Object> getType(ELContext context, Object base, Object property) {
+ Objects.requireNonNull(context);
+
+ // In normal usage, ScopedAttributeELResolver will have responded.
+ return null;
+ }
+
+ @Override
+ public void setValue(ELContext context, Object base, Object property, Object value) {
+ Objects.requireNonNull(context);
+ // In normal usage, ScopedAttributeELResolver will have responded.
+ }
+
+ @Override
+ public boolean isReadOnly(ELContext context, Object base, Object property) {
+ Objects.requireNonNull(context);
+
+ // In normal usage, ScopedAttributeELResolver will have responded.
+ return false;
+ }
+
+ @Deprecated(forRemoval = true, since = "JSP 3.1")
+ @Override
+ public Iterator<FeatureDescriptor> getFeatureDescriptors(ELContext context, Object base) {
+ return Collections.emptyIterator();
+ }
+
+ @Override
+ public Class<String> getCommonPropertyType(ELContext context, Object base) {
+ // In normal usage, ScopedAttributeELResolver will have responded.
+ return null;
+ }
+}
diff --git a/java/jakarta/servlet/jsp/el/NotFoundELResolver.java b/java/jakarta/servlet/jsp/el/NotFoundELResolver.java
new file mode 100644
index 0000000..90fd930
--- /dev/null
+++ b/java/jakarta/servlet/jsp/el/NotFoundELResolver.java
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package jakarta.servlet.jsp.el;
+
+import java.beans.FeatureDescriptor;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.Objects;
+
+import jakarta.el.ELContext;
+import jakarta.el.ELResolver;
+
+/**
+ * The final resolver of the Jakarta Server Pages ELResolver chain. It always
+ * resolves the requested value, returning {@code null} when it does so.
+ *
+ * @since JSP 3.1
+ */
+public class NotFoundELResolver extends ELResolver {
+
+ /**
+ * {@inheritDoc}
+ * <p>
+ * Always resolves the property and always returns {@code null}.
+ *
+ * @return Always {@code null}
+ */
+ @Override
+ public Object getValue(ELContext context, Object base, Object property) {
+ Objects.requireNonNull(context);
+ context.setPropertyResolved(base, property);
+ return null;
+ }
+
+ /**
+ * {@inheritDoc}
+ * <p>
+ * In normal usage, {@link ScopedAttributeELResolver} will have responded.
+ *
+ * @return Always {@code null}
+ */
+ @Override
+ public Class<Object> getType(ELContext context, Object base, Object property) {
+ Objects.requireNonNull(context);
+ return null;
+ }
+
+ /**
+ * {@inheritDoc}
+ * <p>
+ * No-op. In normal usage, {@link ScopedAttributeELResolver} will have
+ * responded.
+ */
+ @Override
+ public void setValue(ELContext context, Object base, Object property, Object value) {
+ Objects.requireNonNull(context);
+ }
+
+
+ /**
+ * {@inheritDoc}
+ * <p>
+ * In normal usage, {@link ScopedAttributeELResolver} will have responded.
+ *
+ * @return Always {@code false}
+ */
+ @Override
+ public boolean isReadOnly(ELContext context, Object base, Object property) {
+ Objects.requireNonNull(context);
+ return false;
+ }
+
+ @Deprecated(forRemoval = true, since = "JSP 3.1")
+ @Override
+ public Iterator<FeatureDescriptor> getFeatureDescriptors(ELContext context, Object base) {
+ return Collections.emptyIterator();
+ }
+
+ /**
+ * {@inheritDoc}
+ * <p>
+ * In normal usage, {@link ScopedAttributeELResolver} will have responded.
+ *
+ * @return Always {@code null}
+ */
+ @Override
+ public Class<String> getCommonPropertyType(ELContext context, Object base) {
+ return null;
+ }
+}
diff --git a/java/jakarta/servlet/jsp/el/ScopedAttributeELResolver.java b/java/jakarta/servlet/jsp/el/ScopedAttributeELResolver.java
index 25c14c2..4e3b145 100644
--- a/java/jakarta/servlet/jsp/el/ScopedAttributeELResolver.java
+++ b/java/jakarta/servlet/jsp/el/ScopedAttributeELResolver.java
@@ -23,10 +23,8 @@ import java.util.Iterator;
import java.util.List;
import java.util.Objects;
-import jakarta.el.ELClass;
import jakarta.el.ELContext;
import jakarta.el.ELResolver;
-import jakarta.el.ImportHandler;
import jakarta.servlet.jsp.JspContext;
import jakarta.servlet.jsp.PageContext;
@@ -36,20 +34,6 @@ import jakarta.servlet.jsp.PageContext;
*/
public class ScopedAttributeELResolver extends ELResolver {
- // Indicates if a performance short-cut is available
- private static final Class<?> AST_IDENTIFIER_KEY;
-
- static {
- Class<?> key = null;
- try {
- key = Class.forName("org.apache.el.parser.AstIdentifier");
- } catch (Exception e) {
- // Ignore: Expected if not running on Tomcat. Not a problem since
- // this just allows a short-cut.
- }
- AST_IDENTIFIER_KEY = key;
- }
-
@Override
public Object getValue(ELContext context, Object base, Object property) {
Objects.requireNonNull(context);
@@ -57,51 +41,13 @@ public class ScopedAttributeELResolver extends ELResolver {
Object result = null;
if (base == null) {
- context.setPropertyResolved(base, property);
if (property != null) {
String key = property.toString();
PageContext page = (PageContext) context.getContext(JspContext.class);
result = page.findAttribute(key);
- if (result == null) {
- boolean resolveClass = true;
- // Performance short-cut available when running on Tomcat
- if (AST_IDENTIFIER_KEY != null) {
- // Tomcat will set this key to Boolean.TRUE if the
- // identifier is a stand-alone identifier (i.e.
- // identifier) rather than part of an AstValue (i.e.
- // identifier.something). Imports do not need to be
- // checked if this is a stand-alone identifier
- Boolean value = (Boolean) context.getContext(AST_IDENTIFIER_KEY);
- if (value != null && value.booleanValue()) {
- resolveClass = false;
- }
- }
- // This might be the name of an imported class
- ImportHandler importHandler = context.getImportHandler();
- if (importHandler != null) {
- Class<?> clazz = null;
- if (resolveClass) {
- clazz = importHandler.resolveClass(key);
- }
- if (clazz != null) {
- result = new ELClass(clazz);
- }
- if (result == null) {
- // This might be the name of an imported static field
- clazz = importHandler.resolveStatic(key);
- if (clazz != null) {
- try {
- result = clazz.getField(key).get(null);
- } catch (IllegalArgumentException | IllegalAccessException |
- NoSuchFieldException | SecurityException e) {
- // Most (all?) of these should have been
- // prevented by the checks when the import
- // was defined.
- }
- }
- }
- }
+ if (result != null) {
+ context.setPropertyResolved(base, property);
}
}
}
diff --git a/java/org/apache/jasper/el/JasperELResolver.java b/java/org/apache/jasper/el/JasperELResolver.java
index 7c02cea..a2e4e12 100644
--- a/java/org/apache/jasper/el/JasperELResolver.java
+++ b/java/org/apache/jasper/el/JasperELResolver.java
@@ -34,6 +34,8 @@ import jakarta.el.PropertyNotFoundException;
import jakarta.el.ResourceBundleELResolver;
import jakarta.el.StaticFieldELResolver;
import jakarta.servlet.jsp.el.ImplicitObjectELResolver;
+import jakarta.servlet.jsp.el.ImportELResolver;
+import jakarta.servlet.jsp.el.NotFoundELResolver;
import jakarta.servlet.jsp.el.ScopedAttributeELResolver;
import org.apache.jasper.runtime.ExceptionUtils;
@@ -45,7 +47,8 @@ import org.apache.jasper.runtime.JspRuntimeLibrary;
*/
public class JasperELResolver extends CompositeELResolver {
- private static final int STANDARD_RESOLVERS_COUNT = 9;
+ // Keep aligned with class under test
+ private static final int STANDARD_RESOLVERS_COUNT = 11;
private AtomicInteger resolversSize = new AtomicInteger(0);
private volatile ELResolver[] resolvers;
@@ -71,6 +74,8 @@ public class JasperELResolver extends CompositeELResolver {
}
add(new BeanELResolver());
add(new ScopedAttributeELResolver());
+ add(new ImportELResolver());
+ add(new NotFoundELResolver());
}
@Override
diff --git a/test/org/apache/jasper/el/TestJasperELResolver.java b/test/org/apache/jasper/el/TestJasperELResolver.java
index d58391e..31f9f48 100644
--- a/test/org/apache/jasper/el/TestJasperELResolver.java
+++ b/test/org/apache/jasper/el/TestJasperELResolver.java
@@ -32,6 +32,8 @@ import org.apache.jasper.runtime.JspRuntimeLibrary;
public class TestJasperELResolver {
+ private static final int STANDARD_RESOLVERS_COUNT = 11;
+
@Test
public void testConstructorNone() throws Exception {
doTestConstructor(0);
@@ -61,9 +63,9 @@ public class TestJasperELResolver {
Assert.assertEquals(Integer.valueOf(count),
getField("appResolversSize", resolver));
- Assert.assertEquals(9 + adjustedForGraalCount,
+ Assert.assertEquals(STANDARD_RESOLVERS_COUNT + adjustedForGraalCount,
((ELResolver[])getField("resolvers", resolver)).length);
- Assert.assertEquals(Integer.valueOf(9 + adjustedForGraalCount),
+ Assert.assertEquals(Integer.valueOf(STANDARD_RESOLVERS_COUNT + adjustedForGraalCount),
Integer.valueOf(((AtomicInteger) getField("resolversSize", resolver)).get()));
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index ab5b181..0f3f4bb 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -149,6 +149,13 @@
on <code>MehtodExpression</code> to align Tomcat with recent updates in
the Jakarta EL specification project. (markt)
</add>
+ <add>
+ Refactor <code>ScopedAttributeELResolver</code> to separate out the
+ functionality that is unrelated to scoped attributes into two new
+ resolvers: <code>ImportELResolver</code> and
+ <code>NotFoundELResolver</code>. This aligns Tomcat with recent updates
+ to the Jakarta Server Pages specification. (markt)
+ </add>
</changelog>
</subsection>
</section>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org