(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 <[email protected]> Authored: Sat Feb 11 17:32:45 2017 +0100 Committer: ddekany <[email protected]> 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;
