You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@freemarker.apache.org by dd...@apache.org on 2017/03/01 14:21:15 UTC

[17/50] [abbrv] incubator-freemarker git commit: (Some source code cleanup.)

(Some source code cleanup.)


Project: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/commit/6a3b9266
Tree: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/tree/6a3b9266
Diff: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/diff/6a3b9266

Branch: refs/heads/2.3
Commit: 6a3b926611132035970c1b326b3629d6826f64e7
Parents: dce7f38
Author: ddekany <dd...@apache.org>
Authored: Sat Feb 11 17:32:45 2017 +0100
Committer: ddekany <dd...@apache.org>
Committed: Sat Feb 11 18:39:31 2017 +0100

----------------------------------------------------------------------
 .../freemarker/cache/FileTemplateLoader.java    | 46 ++++++----------
 .../java/freemarker/cache/TemplateCache.java    |  3 +-
 .../freemarker/cache/URLTemplateLoader.java     | 57 ++++++++++----------
 src/main/java/freemarker/template/Template.java |  9 +++-
 .../cache/MultiTemplateLoaderTest.java          | 18 +++----
 .../CopyrightCommentRemoverTemplateLoader.java  |  5 ++
 6 files changed, 68 insertions(+), 70 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/6a3b9266/src/main/java/freemarker/cache/FileTemplateLoader.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/cache/FileTemplateLoader.java b/src/main/java/freemarker/cache/FileTemplateLoader.java
index d14368f..a769146 100644
--- a/src/main/java/freemarker/cache/FileTemplateLoader.java
+++ b/src/main/java/freemarker/cache/FileTemplateLoader.java
@@ -83,8 +83,7 @@ public class FileTemplateLoader implements TemplateLoader {
      *             {@link FileTemplateLoader#FileTemplateLoader(File)} instead.
      */
     @Deprecated
-    public FileTemplateLoader()
-    throws IOException {
+    public FileTemplateLoader() throws IOException {
         this(new File(SecurityUtilities.getSystemProperty("user.dir")));
     }
 
@@ -95,8 +94,7 @@ public class FileTemplateLoader implements TemplateLoader {
      * the base directory.
      * @param baseDir the base directory for loading templates
      */
-    public FileTemplateLoader(final File baseDir)
-    throws IOException {
+    public FileTemplateLoader(final File baseDir) throws IOException {
         this(baseDir, false);
     }
 
@@ -116,11 +114,10 @@ public class FileTemplateLoader implements TemplateLoader {
      *            outside the {@code baseDir}, set this parameter to {@code true}, but then be very careful with
      *            template paths that are supplied by the visitor or an external system.
      */
-    public FileTemplateLoader(final File baseDir, final boolean disableCanonicalPathCheck)
-    throws IOException {
+    public FileTemplateLoader(final File baseDir, final boolean disableCanonicalPathCheck) throws IOException {
         try {
-            Object[] retval = (Object[]) AccessController.doPrivileged(new PrivilegedExceptionAction() {
-                public Object run() throws IOException {
+            Object[] retval = (Object[]) AccessController.doPrivileged(new PrivilegedExceptionAction<Object[]>() {
+                public Object[] run() throws IOException {
                     if (!baseDir.exists()) {
                         throw new FileNotFoundException(baseDir + " does not exist.");
                     }
@@ -153,11 +150,10 @@ public class FileTemplateLoader implements TemplateLoader {
         }
     }
     
-    public Object findTemplateSource(final String name)
-    throws IOException {
+    public Object findTemplateSource(final String name) throws IOException {
         try {
-            return AccessController.doPrivileged(new PrivilegedExceptionAction() {
-                public Object run() throws IOException {
+            return AccessController.doPrivileged(new PrivilegedExceptionAction<File>() {
+                public File run() throws IOException {
                     File source = new File(baseDir, SEP_IS_SLASH ? name : 
                         name.replace('/', File.separatorChar));
                     if (!source.isFile()) {
@@ -170,8 +166,8 @@ public class FileTemplateLoader implements TemplateLoader {
                         String normalized = source.getCanonicalPath();
                         if (!normalized.startsWith(canonicalBasePath)) {
                             throw new SecurityException(source.getAbsolutePath() 
-                                    + " resolves to " + normalized + " which " + 
-                                    " doesn't start with " + canonicalBasePath);
+                                    + " resolves to " + normalized + " which "
+                                    + " doesn't start with " + canonicalBasePath);
                         }
                     }
                     
@@ -188,23 +184,17 @@ public class FileTemplateLoader implements TemplateLoader {
     }
     
     public long getLastModified(final Object templateSource) {
-        return ((Long) (AccessController.doPrivileged(new PrivilegedAction()
-        {
-            public Object run() {
+        return (AccessController.doPrivileged(new PrivilegedAction<Long>() {
+            public Long run() {
                 return Long.valueOf(((File) templateSource).lastModified());
             }
-        }))).longValue();
-        
-        
+        })).longValue();
     }
     
-    public Reader getReader(final Object templateSource, final String encoding)
-    throws IOException {
+    public Reader getReader(final Object templateSource, final String encoding) throws IOException {
         try {
-            return (Reader) AccessController.doPrivileged(new PrivilegedExceptionAction()
-            {
-                public Object run()
-                throws IOException {
+            return AccessController.doPrivileged(new PrivilegedExceptionAction<Reader>() {
+                public Reader run() throws IOException {
                     if (!(templateSource instanceof File)) {
                         throw new IllegalArgumentException(
                                 "templateSource wasn't a File, but a: " + 
@@ -219,9 +209,7 @@ public class FileTemplateLoader implements TemplateLoader {
     }
     
     /**
-     * Called by {@link #findTemplateSource(String)} when {@link #getEmulateCaseSensitiveFileSystem()} is {@code true}. Should throw
-     * {@link FileNotFoundException} if there's a mismatch; the error message should contain both the requested and the
-     * correct file name.
+     * Called by {@link #findTemplateSource(String)} when {@link #getEmulateCaseSensitiveFileSystem()} is {@code true}.
      */
     private boolean isNameCaseCorrect(File source) throws IOException {
         final String sourcePath = source.getPath();

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/6a3b9266/src/main/java/freemarker/cache/TemplateCache.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/cache/TemplateCache.java b/src/main/java/freemarker/cache/TemplateCache.java
index 469c7f9..ba37c57 100644
--- a/src/main/java/freemarker/cache/TemplateCache.java
+++ b/src/main/java/freemarker/cache/TemplateCache.java
@@ -155,7 +155,8 @@ public class TemplateCache {
 
     /**
      * @param templateLoader
-     *            The {@link TemplateLoader} to use. Can't be {@code null}.
+     *            The {@link TemplateLoader} to use. Can be {@code null}, though then every request will result in
+     *            {@link TemplateNotFoundException}.
      * @param cacheStorage
      *            The {@link CacheStorage} to use. Can't be {@code null}.
      * @param templateLookupStrategy

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/6a3b9266/src/main/java/freemarker/cache/URLTemplateLoader.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/cache/URLTemplateLoader.java b/src/main/java/freemarker/cache/URLTemplateLoader.java
index e975869..d8474dd 100644
--- a/src/main/java/freemarker/cache/URLTemplateLoader.java
+++ b/src/main/java/freemarker/cache/URLTemplateLoader.java
@@ -31,8 +31,7 @@ import freemarker.template.Configuration;
 /**
  * This is an abstract template loader that can load templates whose
  * location can be described by an URL. Subclasses only need to override
- * the {@link #getURL(String)} method. Both {@link ClassTemplateLoader} and
- * {@link WebappTemplateLoader} are (quite trivial) subclasses of this class.
+ * the {@link #getURL(String)} method.
  */
 public abstract class URLTemplateLoader implements TemplateLoader {
     
@@ -44,16 +43,6 @@ public abstract class URLTemplateLoader implements TemplateLoader {
         return url == null ? null : new URLTemplateSource(url, getURLConnectionUsesCaches());
     }
     
-    /**
-     * Given a template name (plus potential locale decorations) retrieves
-     * an URL that points the template source.
-     * @param name the name of the sought template, including the locale
-     * decorations.
-     * @return an URL that points to the template source, or null if it can
-     * determine that the template source does not exist.
-     */
-    protected abstract URL getURL(String name);
-    
     public long getLastModified(Object templateSource) {
         return ((URLTemplateSource) templateSource).lastModified();
     }
@@ -71,23 +60,6 @@ public abstract class URLTemplateLoader implements TemplateLoader {
     }
 
     /**
-     * Can be used by subclasses to canonicalize URL path prefixes.
-     * @param prefix the path prefix to canonicalize
-     * @return the canonicalized prefix. All backslashes are replaced with
-     * forward slashes, and a trailing slash is appended if the original
-     * prefix wasn't empty and didn't already end with a slash.
-     */
-    protected static String canonicalizePrefix(String prefix) {
-        // make it foolproof
-        prefix = prefix.replace('\\', '/');
-        // ensure there's a trailing slash
-        if (prefix.length() > 0 && !prefix.endsWith("/")) {
-            prefix += "/";
-        }
-        return prefix;
-    }
-
-    /**
      * Getter pair of {@link #setURLConnectionUsesCaches(Boolean)}.
      * 
      * @since 2.3.21
@@ -115,5 +87,32 @@ public abstract class URLTemplateLoader implements TemplateLoader {
     public void setURLConnectionUsesCaches(Boolean urlConnectionUsesCaches) {
         this.urlConnectionUsesCaches = urlConnectionUsesCaches;
     }
+
+    /**
+     * Given a template name (plus potential locale decorations) retrieves
+     * an URL that points the template source.
+     * @param name the name of the sought template, including the locale
+     * decorations.
+     * @return an URL that points to the template source, or null if it can
+     * determine that the template source does not exist.
+     */
+    protected abstract URL getURL(String name);
+    
+    /**
+     * Can be used by subclasses to canonicalize URL path prefixes.
+     * @param prefix the path prefix to canonicalize
+     * @return the canonicalized prefix. All backslashes are replaced with
+     * forward slashes, and a trailing slash is appended if the original
+     * prefix wasn't empty and didn't already end with a slash.
+     */
+    protected static String canonicalizePrefix(String prefix) {
+        // make it foolproof
+        prefix = prefix.replace('\\', '/');
+        // ensure there's a trailing slash
+        if (prefix.length() > 0 && !prefix.endsWith("/")) {
+            prefix += "/";
+        }
+        return prefix;
+    }
     
 }

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/6a3b9266/src/main/java/freemarker/template/Template.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/template/Template.java b/src/main/java/freemarker/template/Template.java
index 5b19a33..ffef603 100644
--- a/src/main/java/freemarker/template/Template.java
+++ b/src/main/java/freemarker/template/Template.java
@@ -78,6 +78,8 @@ import freemarker.debug.impl.DebuggerService;
 public class Template extends Configurable {
     public static final String DEFAULT_NAMESPACE_PREFIX = "D";
     public static final String NO_NS_PREFIX = "N";
+
+    private static final int READER_BUFFER_SIZE = 4096;
     
     /** This is only non-null during parsing. It's used internally to make some information available through the
      *  Template API-s earlier than the parsing was finished. */
@@ -164,7 +166,10 @@ public class Template extends Configurable {
      *            See {@link #getSourceName()} for the meaning. Can be {@code null}, in which case
      *            {@link #getSourceName()} will return the same as {@link #getName()}.
      * @param reader
-     *            The character stream to read from. It will always be closed ({@link Reader#close()}) by this method.
+     *            The character stream to read from. It will always be closed ({@link Reader#close()}) by
+     *            this method (this is for backward compatibility; later major versions may discontinue this behavior).
+     *            The {@link Reader} need not be buffered, because this method ensures that it will be read in few
+     *            kilobyte chunks, not byte by byte.
      * @param cfg
      *            The Configuration object that this Template is associated with. If this is {@code null}, the "default"
      *            {@link Configuration} object is used, which is highly discouraged, because it can easily lead to
@@ -234,7 +239,7 @@ public class Template extends Configurable {
             ParserConfiguration actualParserConfiguration = getParserConfiguration();
             
             if (!(reader instanceof BufferedReader) && !(reader instanceof StringReader)) {
-                reader = new BufferedReader(reader, 0x1000);
+                reader = new BufferedReader(reader, READER_BUFFER_SIZE);
             }
             ltbReader = new LineTableBuilder(reader, actualParserConfiguration);
             reader = ltbReader;

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/6a3b9266/src/test/java/freemarker/cache/MultiTemplateLoaderTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/freemarker/cache/MultiTemplateLoaderTest.java b/src/test/java/freemarker/cache/MultiTemplateLoaderTest.java
index e4b14f3..9286185 100644
--- a/src/test/java/freemarker/cache/MultiTemplateLoaderTest.java
+++ b/src/test/java/freemarker/cache/MultiTemplateLoaderTest.java
@@ -38,10 +38,10 @@ public class MultiTemplateLoaderTest {
         stl2.putTemplate("both.ftl", "both 2");
         
         MultiTemplateLoader mtl = new MultiTemplateLoader(new TemplateLoader[] { stl1, stl2 });
-        assertEquals("1", getTemplate(mtl, "1.ftl"));
-        assertEquals("2", getTemplate(mtl, "2.ftl"));
-        assertEquals("both 1", getTemplate(mtl, "both.ftl"));
-        assertNull(getTemplate(mtl, "neither.ftl"));
+        assertEquals("1", getTemplateContent(mtl, "1.ftl"));
+        assertEquals("2", getTemplateContent(mtl, "2.ftl"));
+        assertEquals("both 1", getTemplateContent(mtl, "both.ftl"));
+        assertNull(getTemplateContent(mtl, "neither.ftl"));
     }
 
     @Test
@@ -64,16 +64,16 @@ public class MultiTemplateLoaderTest {
         MultiTemplateLoader mtl = new MultiTemplateLoader(new TemplateLoader[] { stl1, stl2 });
         mtl.setSticky(sticky);
         
-        assertEquals("both 1", getTemplate(mtl, "both.ftl"));
+        assertEquals("both 1", getTemplateContent(mtl, "both.ftl"));
         assertTrue(stl1.removeTemplate("both.ftl"));
-        assertEquals("both 2", getTemplate(mtl, "both.ftl"));
+        assertEquals("both 2", getTemplateContent(mtl, "both.ftl"));
         stl1.putTemplate("both.ftl", "both 1");
-        assertEquals(sticky ? "both 2" : "both 1", getTemplate(mtl, "both.ftl"));
+        assertEquals(sticky ? "both 2" : "both 1", getTemplateContent(mtl, "both.ftl"));
         assertTrue(stl2.removeTemplate("both.ftl"));
-        assertEquals("both 1", getTemplate(mtl, "both.ftl"));
+        assertEquals("both 1", getTemplateContent(mtl, "both.ftl"));
     }
     
-    private String getTemplate(TemplateLoader tl, String name) throws IOException {
+    private String getTemplateContent(TemplateLoader tl, String name) throws IOException {
         Object tSrc = tl.findTemplateSource(name);
         if (tSrc == null) {
             return null;

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/6a3b9266/src/test/java/freemarker/test/CopyrightCommentRemoverTemplateLoader.java
----------------------------------------------------------------------
diff --git a/src/test/java/freemarker/test/CopyrightCommentRemoverTemplateLoader.java b/src/test/java/freemarker/test/CopyrightCommentRemoverTemplateLoader.java
index 8c4a48d..b6b058e 100644
--- a/src/test/java/freemarker/test/CopyrightCommentRemoverTemplateLoader.java
+++ b/src/test/java/freemarker/test/CopyrightCommentRemoverTemplateLoader.java
@@ -27,6 +27,11 @@ import org.apache.commons.io.IOUtils;
 import freemarker.cache.TemplateLoader;
 import freemarker.test.utility.TestUtil;
 
+/**
+ * Removes the Apache copyright boiler plate from the beginning of the template, so that they don't mess up the expected
+ * template output. This can interfere with tests that try to test I/O errors and such low level things, so use with
+ * care. 
+ */
 public class CopyrightCommentRemoverTemplateLoader implements TemplateLoader {
 
     private final TemplateLoader innerTemplateLoader;