Author: markt Date: Wed Dec 10 20:20:08 2014 New Revision: 1644516 URL: http://svn.apache.org/r1644516 Log: Revert original fix for BZ 57142 that attempted to: - determine if the page might use EL - if it did, create the ELContext (that would be required anyway) and set the necessary imports on the ImportHandler when the page loads
This alternative implementation: - Generates a list of package and class imports at compilation time - Which are then used when the ELContext is created The main advantage of this approach is that the ELContext is only generated if it is definitely going to be used rather than if it might be used. There are also some minor optimisations regarding the timing of the String processing to differentiate between package and class imports. Added: tomcat/trunk/java/org/apache/jasper/runtime/JspSourceImports.java (with props) Modified: tomcat/trunk/java/org/apache/jasper/compiler/Generator.java tomcat/trunk/java/org/apache/jasper/compiler/PageInfo.java tomcat/trunk/java/org/apache/jasper/compiler/Validator.java tomcat/trunk/java/org/apache/jasper/runtime/PageContextImpl.java Modified: tomcat/trunk/java/org/apache/jasper/compiler/Generator.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/Generator.java?rev=1644516&r1=1644515&r2=1644516&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/jasper/compiler/Generator.java (original) +++ tomcat/trunk/java/org/apache/jasper/compiler/Generator.java Wed Dec 10 20:20:08 2014 @@ -31,6 +31,7 @@ import java.util.Collections; import java.util.Date; import java.util.Enumeration; import java.util.HashMap; +import java.util.HashSet; import java.util.Hashtable; import java.util.Iterator; import java.util.List; @@ -532,13 +533,13 @@ class Generator { out.printil("private static java.util.Map<java.lang.String,java.lang.Long> _jspx_dependants;"); out.println(); Map<String,Long> dependants = pageInfo.getDependants(); - Iterator<Entry<String,Long>> iter = dependants.entrySet().iterator(); if (!dependants.isEmpty()) { out.printil("static {"); out.pushIndent(); out.printin("_jspx_dependants = new java.util.HashMap<java.lang.String,java.lang.Long>("); out.print("" + dependants.size()); out.println(");"); + Iterator<Entry<String,Long>> iter = dependants.entrySet().iterator(); while (iter.hasNext()) { Entry<String,Long> entry = iter.next(); out.printin("_jspx_dependants.put(\""); @@ -551,6 +552,55 @@ class Generator { out.printil("}"); out.println(); } + + // Static data for getImports() + List<String> imports = pageInfo.getImports(); + Set<String> packages = new HashSet<>(); + Set<String> classes = new HashSet<>(); + for (String importName : imports) { + if (importName == null) { + continue; + } + String trimmed = importName.trim(); + if (trimmed.endsWith(".*")) { + packages.add(trimmed.substring(0, trimmed.length() - 2)); + } else { + classes.add(trimmed); + } + } + out.printil("private static final java.util.Set<java.lang.String> _jspx_imports_packages;"); + out.println(); + out.printil("private static final java.util.Set<java.lang.String> _jspx_imports_classes;"); + out.println(); + out.printil("static {"); + out.pushIndent(); + if (packages.size() == 0) { + out.printin("_jspx_imports_packages = null;"); + out.println(); + } else { + out.printin("_jspx_imports_packages = new java.util.HashSet<>();"); + out.println(); + for (String packageName : packages) { + out.printin("_jspx_imports_packages.add(\""); + out.print(packageName); + out.println("\");"); + } + } + if (packages.size() == 0) { + out.printin("_jspx_imports_classes = null;"); + out.println(); + } else { + out.printin("_jspx_imports_classes = new java.util.HashSet<>();"); + out.println(); + for (String className : classes) { + out.printin("_jspx_imports_classes.add(\""); + out.print(className); + out.println("\");"); + } + } + out.popIndent(); + out.printil("}"); + out.println(); } /** @@ -582,7 +632,7 @@ class Generator { * preamble generation) */ private void genPreambleMethods() { - // Method used to get compile time file dependencies + // Implement JspSourceDependent out.printil("public java.util.Map<java.lang.String,java.lang.Long> getDependants() {"); out.pushIndent(); out.printil("return _jspx_dependants;"); @@ -590,6 +640,21 @@ class Generator { out.printil("}"); out.println(); + // Implement JspSourceImports + out.printil("public java.util.Set<java.lang.String> getPackageImports() {"); + out.pushIndent(); + out.printil("return _jspx_imports_packages;"); + out.popIndent(); + out.printil("}"); + out.println(); + out.printil("public java.util.Set<java.lang.String> getClassImports() {"); + out.pushIndent(); + out.printil("return _jspx_imports_classes;"); + out.popIndent(); + out.printil("}"); + out.println(); + + generateInit(); generateDestroy(); } @@ -614,7 +679,9 @@ class Generator { out.print(servletClassName); out.print(" extends "); out.println(pageInfo.getExtends()); - out.printin(" implements org.apache.jasper.runtime.JspSourceDependent"); + out.printin(" implements org.apache.jasper.runtime.JspSourceDependent,"); + out.println(); + out.printin(" org.apache.jasper.runtime.JspSourceImports"); if (!pageInfo.isThreadSafe()) { out.println(","); out.printin(" javax.servlet.SingleThreadModel"); @@ -712,34 +779,6 @@ class Generator { out.printil("out = pageContext.getOut();"); out.printil("_jspx_out = out;"); out.println(); - - if (pageInfo.isELUsed()) { - // If EL is going to be used on this page then make sure that the - // EL Context is properly configured with the imports. - // The clarification provided in https://java.net/jira/browse/JSP-44 - // is the the page import directive applies both to the scripting - // environment and to the EL environment. - out.printin("javax.el.ImportHandler _jspx_handler = pageContext.getELContext().getImportHandler();"); - out.println(); - for (String importName : pageInfo.getImports()) { - if (importName == null) { - continue; - } - String trimmed = importName.trim(); - if (trimmed.length() == 0) { - continue; - } - if (trimmed.endsWith(".*")) { - out.printin("_jspx_handler.importPackage(\""); - out.print(trimmed.substring(0, trimmed.length() - 2)); - out.println("\");"); - } else { - out.printin("_jspx_handler.importClass(\""); - out.print(trimmed); - out.println("\");"); - } - } - } } /** Modified: tomcat/trunk/java/org/apache/jasper/compiler/PageInfo.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/PageInfo.java?rev=1644516&r1=1644515&r2=1644516&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/jasper/compiler/PageInfo.java (original) +++ tomcat/trunk/java/org/apache/jasper/compiler/PageInfo.java Wed Dec 10 20:20:08 2014 @@ -72,7 +72,6 @@ class PageInfo { private String isELIgnoredValue; private boolean isELIgnored = false; - private boolean isELUsed = false; // JSP 2.1 private String deferredSyntaxAllowedAsLiteralValue; @@ -678,27 +677,6 @@ class PageInfo { return isELIgnored; } - /** - * Marks the current page as using EL. This allows an optimisation when - * generating the page. The imports need to be added to the EL Context but - * this is wasteful if the EL Context is never going to be used. The - * associated field allows the Generator to determine whether or not to - * configure the imports. - */ - public void setELUsed() { - isELUsed = true; - } - - /** - * Is expression language used on this page. - * - * @return <code>true</code> if expression language is used, otherwise - * <code>false</code> - */ - public boolean isELUsed() { - return isELUsed; - } - public void putNonCustomTagPrefix(String prefix, Mark where) { nonCustomTagPrefixMap.put(prefix, where); } Modified: tomcat/trunk/java/org/apache/jasper/compiler/Validator.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/Validator.java?rev=1644516&r1=1644515&r2=1644516&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/jasper/compiler/Validator.java (original) +++ tomcat/trunk/java/org/apache/jasper/compiler/Validator.java Wed Dec 10 20:20:08 2014 @@ -730,10 +730,6 @@ class Validator { if (pageInfo.isELIgnored()) return; - // EL is known to be used on this page. Mark the PageInfo - // accordingly. - pageInfo.setELUsed(); - // JSP.2.2 - '#{' not allowed in template text if (n.getType() == '#') { if (!pageInfo.isDeferredSyntaxAllowedAsLiteral()) { @@ -1124,12 +1120,6 @@ class Validator { } } - if (elExpression) { - // EL is known to be used on this page. Mark the PageInfo - // accordingly. - pageInfo.setELUsed(); - } - boolean expression = runtimeExpression || elExpression; // When attribute is not an expression, @@ -1386,9 +1376,6 @@ class Validator { if (el.containsEL()) { validateFunctions(el, n); - // EL is known to be used on this page. Mark the - // PageInfo accordingly. - pageInfo.setELUsed(); } else { // Get text with \$ and \# escaping removed. // Should be a single Text node Added: tomcat/trunk/java/org/apache/jasper/runtime/JspSourceImports.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/runtime/JspSourceImports.java?rev=1644516&view=auto ============================================================================== --- tomcat/trunk/java/org/apache/jasper/runtime/JspSourceImports.java (added) +++ tomcat/trunk/java/org/apache/jasper/runtime/JspSourceImports.java Wed Dec 10 20:20:08 2014 @@ -0,0 +1,30 @@ +/* + * 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 org.apache.jasper.runtime; + +import java.util.Set; + +/** + * The EL engine needs access to the imports used in the JSP page to configure + * the ELContext. The imports are available at compile time but the ELContext + * is created lazily per page. This interface exposes the imports at runtime so + * that they may be added to the ELContext when it is created. + */ +public interface JspSourceImports { + Set<String> getPackageImports(); + Set<String> getClassImports(); +} Propchange: tomcat/trunk/java/org/apache/jasper/runtime/JspSourceImports.java ------------------------------------------------------------------------------ svn:eol-style = native Modified: tomcat/trunk/java/org/apache/jasper/runtime/PageContextImpl.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/runtime/PageContextImpl.java?rev=1644516&r1=1644515&r2=1644516&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/jasper/runtime/PageContextImpl.java (original) +++ tomcat/trunk/java/org/apache/jasper/runtime/PageContextImpl.java Wed Dec 10 20:20:08 2014 @@ -26,10 +26,12 @@ import java.security.PrivilegedException import java.util.Collections; import java.util.Enumeration; import java.util.HashMap; +import java.util.Set; import javax.el.ELContext; import javax.el.ELException; import javax.el.ExpressionFactory; +import javax.el.ImportHandler; import javax.el.ValueExpression; import javax.servlet.RequestDispatcher; import javax.servlet.Servlet; @@ -962,10 +964,24 @@ public class PageContextImpl extends Pag @Override public ELContext getELContext() { - if (this.elContext == null) { - this.elContext = this.applicationContext.createELContext(this); + if (elContext == null) { + elContext = applicationContext.createELContext(this); + if (servlet instanceof JspSourceImports) { + ImportHandler ih = elContext.getImportHandler(); + Set<String> packageImports = ((JspSourceImports) servlet).getPackageImports(); + if (packageImports != null) { + for (String packageImport : packageImports) { + ih.importPackage(packageImport); + } + } + Set<String> classImports = ((JspSourceImports) servlet).getClassImports(); + if (classImports != null) { + for (String classImport : classImports) { + ih.importClass(classImport); + } + } + } } return this.elContext; } - } --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org