Issue Details (XML | Word | Printable)

Key: GROOVY-1979
Type: Improvement Improvement
Status: Closed Closed
Resolution: Fixed
Priority: Minor Minor
Assignee: Paul King
Reporter: Jon Travis
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
groovy

SimpleTemplateEngine (and poentially other TemplateEngines) should allow caller to specify classloader

Created: 13/Jul/07 09:51 AM   Updated: 10/Oct/07 06:18 AM   Resolved: 26/Sep/07 11:50 PM
Return to search
Component/s: Groovlet / GSP
Affects Version/s: 1.1-beta-2
Fix Version/s: 1.1-rc-1

Time Tracking:
Not Specified

Patch Submitted: Yes


 Description  « Hide

SimpleTemplateEngine doesn't allow the caller to specify a parent classloader. When it creates a GroovyShell, it just uses the loader for that class.

Index: src/main/groovy/text/SimpleTemplateEngine.java
===================================================================
--- src/main/groovy/text/SimpleTemplateEngine.java      (revision 6601)
+++ src/main/groovy/text/SimpleTemplateEngine.java      (working copy)
@@ -69,8 +69,14 @@
     }
     public Template createTemplate(Reader reader) throws CompilationFailedException, IOException {
+        return createTemplate(GroovyShell.class.getClassLoader(), reader);
+    }
+
+    public Template createTemplate(ClassLoader parentLoader, Reader reader)
+        throws CompilationFailedException, IOException
+    {
         SimpleTemplate template = new SimpleTemplate();
-        GroovyShell shell = new GroovyShell();
+        GroovyShell shell = new GroovyShell(parentLoader);
         String script = template.parse(reader);
         if (verbose) {
             System.out.println("\n-- script source --");

Other template engines should have consistent features with this enhancement.



Paul King made changes - 13/Jul/07 10:05 PM
Field Original Value New Value
Fix Version/s 2.0 [ 13489 ]
Fix Version/s 1.1 [ 13166 ]
Description SimpleTemplateEngine doesn't allow the caller to specify a parent classloader. When it creates a GroovyShell, it just uses the loader for that class.


Index: src/main/groovy/text/SimpleTemplateEngine.java
===================================================================
--- src/main/groovy/text/SimpleTemplateEngine.java (revision 6601)
+++ src/main/groovy/text/SimpleTemplateEngine.java (working copy)
@@ -69,8 +69,14 @@
     }
     public Template createTemplate(Reader reader) throws CompilationFailedException, IOException {
+ return createTemplate(GroovyShell.class.getClassLoader(), reader);
+ }
+
+ public Template createTemplate(ClassLoader parentLoader, Reader reader)
+ throws CompilationFailedException, IOException
+ {
         SimpleTemplate template = new SimpleTemplate();
- GroovyShell shell = new GroovyShell();
+ GroovyShell shell = new GroovyShell(parentLoader);
         String script = template.parse(reader);
         if (verbose) {
             System.out.println("\n-- script source --");

SimpleTemplateEngine doesn't allow the caller to specify a parent classloader. When it creates a GroovyShell, it just uses the loader for that class.

{code}
Index: src/main/groovy/text/SimpleTemplateEngine.java
===================================================================
--- src/main/groovy/text/SimpleTemplateEngine.java (revision 6601)
+++ src/main/groovy/text/SimpleTemplateEngine.java (working copy)
@@ -69,8 +69,14 @@
     }
     public Template createTemplate(Reader reader) throws CompilationFailedException, IOException {
+ return createTemplate(GroovyShell.class.getClassLoader(), reader);
+ }
+
+ public Template createTemplate(ClassLoader parentLoader, Reader reader)
+ throws CompilationFailedException, IOException
+ {
         SimpleTemplate template = new SimpleTemplate();
- GroovyShell shell = new GroovyShell();
+ GroovyShell shell = new GroovyShell(parentLoader);
         String script = template.parse(reader);
         if (verbose) {
             System.out.println("\n-- script source --");
{code}
Fix Version/s 1.1-rc-1 [ 13165 ]
Paul King made changes - 13/Jul/07 10:13 PM
Assignee Paul King [ paulk ]
Paul King added a comment - 13/Jul/07 10:19 PM

added to HEAD


Paul King made changes - 13/Jul/07 10:19 PM
Status Open [ 1 ] Resolved [ 5 ]
Resolution Fixed [ 1 ]
Paul King made changes - 14/Jul/07 11:39 PM
Description SimpleTemplateEngine doesn't allow the caller to specify a parent classloader. When it creates a GroovyShell, it just uses the loader for that class.

{code}
Index: src/main/groovy/text/SimpleTemplateEngine.java
===================================================================
--- src/main/groovy/text/SimpleTemplateEngine.java (revision 6601)
+++ src/main/groovy/text/SimpleTemplateEngine.java (working copy)
@@ -69,8 +69,14 @@
     }
     public Template createTemplate(Reader reader) throws CompilationFailedException, IOException {
+ return createTemplate(GroovyShell.class.getClassLoader(), reader);
+ }
+
+ public Template createTemplate(ClassLoader parentLoader, Reader reader)
+ throws CompilationFailedException, IOException
+ {
         SimpleTemplate template = new SimpleTemplate();
- GroovyShell shell = new GroovyShell();
+ GroovyShell shell = new GroovyShell(parentLoader);
         String script = template.parse(reader);
         if (verbose) {
             System.out.println("\n-- script source --");
{code}
SimpleTemplateEngine doesn't allow the caller to specify a parent classloader. When it creates a GroovyShell, it just uses the loader for that class.

{code}
Index: src/main/groovy/text/SimpleTemplateEngine.java
===================================================================
--- src/main/groovy/text/SimpleTemplateEngine.java (revision 6601)
+++ src/main/groovy/text/SimpleTemplateEngine.java (working copy)
@@ -69,8 +69,14 @@
     }
     public Template createTemplate(Reader reader) throws CompilationFailedException, IOException {
+ return createTemplate(GroovyShell.class.getClassLoader(), reader);
+ }
+
+ public Template createTemplate(ClassLoader parentLoader, Reader reader)
+ throws CompilationFailedException, IOException
+ {
         SimpleTemplate template = new SimpleTemplate();
- GroovyShell shell = new GroovyShell();
+ GroovyShell shell = new GroovyShell(parentLoader);
         String script = template.parse(reader);
         if (verbose) {
             System.out.println("\n-- script source --");
{code}

Other template engines should have consistent features with this enhancement.
Summary SimpleTemplateEngine should allow caller to specify classloader SimpleTemplateEngine (and poentially other TemplateEngines) should allow caller to specify classloader
Paul King added a comment - 14/Jul/07 11:40 PM

Re-opened to track changes to other TemplateEngines.


Paul King made changes - 14/Jul/07 11:40 PM
Resolution Fixed [ 1 ]
Status Resolved [ 5 ] Reopened [ 4 ]
Guillaume Laforge made changes - 20/Sep/07 03:11 PM
Fix Version/s 1.1-beta-3 [ 13590 ]
Fix Version/s 1.1-rc-1 [ 13165 ]
Paul King added a comment - 23/Sep/07 04:54 AM

Jon, I am looking at providing consistent options for all of the template engines. One option is to remove this added functionality but instead allow a parentLoader to be provided in the SimpleTemplateEngine constructor (for use by all templates created with that engine). This may be the easiest way to supply this functionality across all the template. I am not sure which way to go yet but if we did go that way, would that still meet your need? I am trying to get this resolved before 1.1 goes final - after that it will be much harder to remove anything we have provided for now.


Paul King added a comment - 26/Sep/07 11:50 PM - edited

Jon, no feedback given, assuming OK to refactor slightly.

Now parentLoader can be specified for Simple, Xml and GString TemplateEngines (in the constructor).
GroovyShell can be specified for Simple and Xml variations (again via a construction option).


Paul King made changes - 26/Sep/07 11:50 PM
Status Reopened [ 4 ] Resolved [ 5 ]
Resolution Fixed [ 1 ]
Paul King added a comment - 10/Oct/07 06:18 AM

No further feedback, assuming fixed


Paul King made changes - 10/Oct/07 06:18 AM
Status Resolved [ 5 ] Closed [ 6 ]