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

Reply via email to