NIHVIVO-705 NIHVIVO-789 Use a Flattening Template Loader for templates from Vitro core, instead of flattening the template directory during the build process.

webapp/build.xml

@@ -188,17 +188,8 @@ deploy   - Deploy the application directly into the Tomcat webapps directory.
 					set this property and they will be skipped. 
 				-->
 				<exclude name="themes/**/*" if="skip.core.themes" />
-				<!--
-				    Don't copy the freemarker templates; we need to flatten them. 
-				-->
-				<exclude name="templates/freemarker/" />
 			</fileset>
 		</copy>
-		<!-- Flatten the freemarker templates and copy them. -->
-		<copy todir="${build.dir}/war/templates/freemarker" includeemptydirs="false">
-			<fileset dir="${webapp.dir}/web/templates/freemarker" />
-			<flattenmapper />
-		</copy>
 
 		<copy todir="${build.dir}/war/WEB-INF">
 			<fileset file="${webapp.dir}/config/web.xml" />

webapp/src/edu/cornell/mannlib/vitro/webapp/controller/freemarker/FlatteningTemplateLoader.java

@@ -0,0 +1,162 @@
+/* $This file is distributed under the terms of the license in /doc/license.txt$ */
+
+package edu.cornell.mannlib.vitro.webapp.controller.freemarker;
+
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.io.Reader;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+
+import freemarker.cache.TemplateLoader;
+
+/**
+ * <p>
+ * A {@link TemplateLoader} that treats a directory and its sub-directories as a
+ * flat namespace.
+ * </p>
+ * <p>
+ * When a request is made to find a template source, the loader will search its
+ * base directory and any sub-directories for a file with a matching name. So a
+ * request for <code>myFile.ftl</code> might return a reference to a file at
+ * <code>base/myFile.ftl</code> or at <code>base/this/myFile.ftl</code>
+ * </p>
+ * <p>
+ * The order in which the sub-directories are searched is unspecified. The first
+ * matching file will be returned.
+ * </p>
+ * <p>
+ * A path (absolute or relative) on the source name would be meaningless, so any
+ * such path will be stripped before the search is made. That is, a request for
+ * <code>path/file.ftl</code> or <code>/absolute/path/file.ftl</code>is
+ * functionally identical to a request for <code>file.ftl</code>
+ * </p>
+ * <p>
+ * </p>
+ */
+public class FlatteningTemplateLoader implements TemplateLoader {
+	private static final Log log = LogFactory
+			.getLog(FlatteningTemplateLoader.class);
+
+	private final File baseDir;
+
+	public FlatteningTemplateLoader(File baseDir) {
+		if (baseDir == null) {
+			throw new NullPointerException("baseDir may not be null.");
+		}
+		if (!baseDir.exists()) {
+			throw new IllegalArgumentException("Template directory '"
+					+ baseDir.getAbsolutePath() + "' does not exist");
+		}
+		if (!baseDir.isDirectory()) {
+			throw new IllegalArgumentException("Template directory '"
+					+ baseDir.getAbsolutePath() + "' is not a directory");
+		}
+		if (!baseDir.canRead()) {
+			throw new IllegalArgumentException("Can't read template "
+					+ "directory '" + baseDir.getAbsolutePath() + "'");
+		}
+
+		log.debug("Created template loader - baseDir is '"
+				+ baseDir.getAbsolutePath() + "'");
+		this.baseDir = baseDir;
+	}
+
+	/**
+	 * Look for a file by this name in the base directory, or its
+	 * subdirectories, disregarding any path information.
+	 * 
+	 * @return a {@link File} that can be used in subsequent calls the template
+	 *         loader methods, or <code>null</code> if no template is found.
+	 */
+	@Override
+	public Object findTemplateSource(String name) throws IOException {
+		if (name == null) {
+			return null;
+		}
+
+		int lastSlashHere = name.indexOf('/');
+		String trimmedName = (lastSlashHere == -1) ? name : name
+				.substring(lastSlashHere + 1);
+
+		// start the recursive search.
+		File source = findFile(trimmedName, baseDir);
+		if (source == null) {
+			log.debug("For template name '" + name
+					+ "', found no template file.");
+		} else {
+			log.debug("For template name '" + name + "', template file is "
+					+ source.getAbsolutePath());
+		}
+		return source;
+	}
+
+	/**
+	 * Recursively search for a file of this name.
+	 */
+	private File findFile(String name, File dir) {
+		for (File child : dir.listFiles()) {
+			if (child.isDirectory()) {
+				File file = findFile(name, child);
+				if (file != null) {
+					return file;
+				}
+			} else {
+				if (child.getName().equals(name)) {
+					return child;
+				}
+			}
+		}
+		return null;
+	}
+
+	/**
+	 * Ask the file when it was last modified.
+	 * 
+	 * @param templateSource
+	 *            a {@link File} that was obtained earlier from
+	 *            {@link #findTemplateSource(String)}.
+	 */
+	@Override
+	public long getLastModified(Object templateSource) {
+		if (!(templateSource instanceof File)) {
+			throw new IllegalArgumentException("templateSource is not a File: "
+					+ templateSource);
+		}
+
+		return ((File) templateSource).lastModified();
+	}
+
+	/**
+	 * Get a {@link Reader} on this {@link File}. The framework will see that
+	 * the {@link Reader} is closed when it has been read.
+	 * 
+	 * @param templateSource
+	 *            a {@link File} that was obtained earlier from
+	 *            {@link #findTemplateSource(String)}.
+	 */
+	@Override
+	public Reader getReader(Object templateSource, String encoding)
+			throws IOException {
+		if (!(templateSource instanceof File)) {
+			throw new IllegalArgumentException("templateSource is not a File: "
+					+ templateSource);
+		}
+
+		return new FileReader(((File) templateSource));
+	}
+
+	/**
+	 * Nothing to do here. No resources to free up.
+	 * 
+	 * @param templateSource
+	 *            a {@link File} that was obtained earlier from
+	 *            {@link #findTemplateSource(String)}.
+	 */
+	@Override
+	public void closeTemplateSource(Object templateSource) throws IOException {
+	}
+
+}

webapp/src/edu/cornell/mannlib/vitro/webapp/controller/freemarker/FreemarkerHttpServlet.java

@@ -169,7 +169,7 @@ public class FreemarkerHttpServlet extends VitroHttpServlet {
         
         try {
             TemplateLoader[] loaders;
-            FileTemplateLoader vitroFtl = new FileTemplateLoader(new File(vitroTemplatePath));
+            FlatteningTemplateLoader vitroFtl = new FlatteningTemplateLoader(new File(vitroTemplatePath));
             ClassTemplateLoader ctl = new ClassTemplateLoader(getClass(), "");
             
             File themeTemplateDir = new File(themeTemplatePath);

webapp/test/edu/cornell/mannlib/vitro/webapp/controller/freemarker/FlatteningTemplateLoaderTest.java

@@ -0,0 +1,222 @@
+/* $This file is distributed under the terms of the license in /doc/license.txt$ */
+
+package edu.cornell.mannlib.vitro.webapp.controller.freemarker;
+
+import static org.junit.Assert.*;
+
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.io.Reader;
+import java.io.Writer;
+import java.text.SimpleDateFormat;
+
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import edu.cornell.mannlib.vitro.testing.AbstractTestClass;
+
+/**
+ * Test the methods of {@link FlatteningTemplateLoader}.
+ */
+public class FlatteningTemplateLoaderTest extends AbstractTestClass {
+	/**
+	 * TODO test plan
+	 * 
+	 * <pre>
+	 * findTemplateSource
+	 *   null arg
+	 *   not found
+	 *   found in top level
+	 *   found in lower level
+	 *   with path
+	 *   
+	 * getReader
+	 *   get it, read it, check it, close it.
+	 *   
+	 * getLastModified
+	 * 	 check the create date within a range
+	 *   modify it and check again.
+	 * 
+	 * </pre>
+	 */
+	// ----------------------------------------------------------------------
+	// setup and teardown
+	// ----------------------------------------------------------------------
+
+	private static final String SUBDIRECTORY_NAME = "sub";
+
+	private static final String TEMPLATE_NAME_UPPER = "template.ftl";
+	private static final String TEMPLATE_NAME_UPPER_WITH_PATH = "path/template.ftl";
+	private static final String TEMPLATE_UPPER_CONTENTS = "The contents of the file.";
+
+	private static final String TEMPLATE_NAME_LOWER = "another.ftl";
+	private static final String TEMPLATE_LOWER_CONTENTS = "Another template file.";
+
+	private static long setupTime;
+	private static File tempDir;
+	private static File notADirectory;
+	private static File upperTemplate;
+	private static File lowerTemplate;
+
+	private FlatteningTemplateLoader loader;
+
+	@BeforeClass
+	public static void setUpFiles() throws IOException {
+		setupTime = System.currentTimeMillis();
+
+		notADirectory = File.createTempFile(
+				FlatteningTemplateLoader.class.getSimpleName(), "");
+
+		tempDir = createTempDirectory(FlatteningTemplateLoader.class
+				.getSimpleName());
+		upperTemplate = createFile(tempDir, TEMPLATE_NAME_UPPER,
+				TEMPLATE_UPPER_CONTENTS);
+
+		File subdirectory = new File(tempDir, SUBDIRECTORY_NAME);
+		subdirectory.mkdir();
+		lowerTemplate = createFile(subdirectory, TEMPLATE_NAME_LOWER,
+				TEMPLATE_LOWER_CONTENTS);
+	}
+
+	@Before
+	public void initializeLoader() {
+		loader = new FlatteningTemplateLoader(tempDir);
+	}
+
+	@AfterClass
+	public static void cleanUpFiles() throws IOException {
+		purgeDirectoryRecursively(tempDir);
+	}
+
+	// ----------------------------------------------------------------------
+	// the tests
+	// ----------------------------------------------------------------------
+
+	@Test(expected = NullPointerException.class)
+	public void constructorNull() {
+		new FlatteningTemplateLoader(null);
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void constructorNonExistent() {
+		new FlatteningTemplateLoader(new File("bogusDirName"));
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void constructorNotADirectory() {
+		new FlatteningTemplateLoader(notADirectory);
+	}
+
+	@Test
+	public void findNull() throws IOException {
+		Object source = loader.findTemplateSource(null);
+		assertNull("find null", source);
+	}
+
+	@Test
+	public void findNotFound() throws IOException {
+		Object source = loader.findTemplateSource("bogus");
+		assertNull("not found", source);
+	}
+
+	@Test
+	public void findInTopLevel() throws IOException {
+		Object source = loader.findTemplateSource(TEMPLATE_NAME_UPPER);
+		assertEquals("top level", upperTemplate, source);
+	}
+
+	@Test
+	public void findInLowerLevel() throws IOException {
+		Object source = loader.findTemplateSource(TEMPLATE_NAME_LOWER);
+		assertEquals("lower level", lowerTemplate, source);
+	}
+
+	@Test
+	public void findIgnoringPath() throws IOException {
+		Object source = loader
+				.findTemplateSource(TEMPLATE_NAME_UPPER_WITH_PATH);
+		assertEquals("top level", upperTemplate, source);
+	}
+
+	@Test
+	public void checkTheReader() throws IOException {
+		Object source = loader.findTemplateSource(TEMPLATE_NAME_UPPER);
+		Reader reader = loader.getReader(source, "UTF-8");
+		String contents = readAll(reader);
+		assertEquals("read the contents", contents, TEMPLATE_UPPER_CONTENTS);
+	}
+
+	/**
+	 * We may not know exactly when the file was last modified, but it should
+	 * fall into a known range.
+	 */
+	@Test
+	public void lastModified() throws IOException {
+		Object source = loader.findTemplateSource(TEMPLATE_NAME_UPPER);
+		long modified = loader.getLastModified(source);
+		long firstBoundary = System.currentTimeMillis();
+		assertInRange("created", setupTime, firstBoundary, modified);
+
+		rewriteFile(upperTemplate, TEMPLATE_UPPER_CONTENTS);
+		long secondBoundary = System.currentTimeMillis();
+		modified = loader.getLastModified(source);
+		assertInRange("modified", firstBoundary, secondBoundary, modified);
+	}
+
+	@Test
+	public void closeDoesntCrash() throws IOException {
+		Object source = loader.findTemplateSource(TEMPLATE_NAME_UPPER);
+		loader.closeTemplateSource(source);
+	}
+
+	// ----------------------------------------------------------------------
+	// helper methods
+	// ----------------------------------------------------------------------
+
+	/**
+	 * Fill an existing file with new contents.
+	 */
+	private void rewriteFile(File file, String contents) throws IOException {
+		Writer writer = null;
+		try {
+			writer = new FileWriter(file);
+			writer.write(contents);
+		} finally {
+			if (writer != null) {
+				try {
+					writer.close();
+				} catch (IOException e) {
+					e.printStackTrace();
+				}
+			}
+		}
+	}
+
+	/**
+	 * Assert that the modified time falls between (or on) the two boundary
+	 * times.
+	 */
+	private void assertInRange(String message, long lowerBound,
+			long upperBound, long modified) {
+		if (modified < lowerBound) {
+			fail(message + ": " + formatTimeStamp(modified)
+					+ " is less than the lower bound "
+					+ formatTimeStamp(lowerBound));
+		}
+		if (modified > upperBound) {
+			fail(message + ": " + formatTimeStamp(modified)
+					+ " is greater than the upper bound "
+					+ formatTimeStamp(upperBound));
+		}
+	}
+
+	private String formatTimeStamp(long time) {
+		SimpleDateFormat formatter = new SimpleDateFormat(
+				"yyyy-MM-dd HH:mm:ss.SSS");
+		return formatter.format(time);
+	}
+
+}