diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/freemarker/DelimitingTemplateLoader.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/freemarker/DelimitingTemplateLoader.java index a0650366e..9b103ad82 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/freemarker/DelimitingTemplateLoader.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/freemarker/DelimitingTemplateLoader.java @@ -38,6 +38,7 @@ public class DelimitingTemplateLoader implements TemplateLoader { @Override public Object findTemplateSource(String name) throws IOException { Object innerTS = innerLoader.findTemplateSource(name); + log.debug("template source for '" + name + "' is '" + innerTS + "'"); if (innerTS == null) { return null; } else { diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/freemarker/FlatteningTemplateLoader.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/freemarker/FlatteningTemplateLoader.java deleted file mode 100644 index 23f625291..000000000 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/freemarker/FlatteningTemplateLoader.java +++ /dev/null @@ -1,162 +0,0 @@ -/* $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; - -/** - *
- * A {@link TemplateLoader} that treats a directory and its sub-directories as a - * flat namespace. - *
- *
- * 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 myFile.ftl
might return a reference to a file at
- * base/myFile.ftl
or at base/this/myFile.ftl
- *
- * The order in which the sub-directories are searched is unspecified. The first - * matching file will be returned. - *
- *
- * 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
- * path/file.ftl
or /absolute/path/file.ftl
is
- * functionally identical to a request for file.ftl
- *
- *
- */ -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, ornull
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 {
- }
-
-}
diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/freemarker/config/FreemarkerConfiguration.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/freemarker/config/FreemarkerConfiguration.java
index 7872974bb..274da8d39 100644
--- a/webapp/src/edu/cornell/mannlib/vitro/webapp/freemarker/config/FreemarkerConfiguration.java
+++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/freemarker/config/FreemarkerConfiguration.java
@@ -3,7 +3,6 @@
package edu.cornell.mannlib.vitro.webapp.freemarker.config;
import java.io.File;
-import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
@@ -21,9 +20,9 @@ import org.apache.commons.logging.LogFactory;
import edu.cornell.mannlib.vitro.webapp.config.RevisionInfoBean;
import edu.cornell.mannlib.vitro.webapp.controller.VitroRequest;
import edu.cornell.mannlib.vitro.webapp.controller.freemarker.DelimitingTemplateLoader;
-import edu.cornell.mannlib.vitro.webapp.controller.freemarker.FlatteningTemplateLoader;
import edu.cornell.mannlib.vitro.webapp.controller.freemarker.UrlBuilder;
import edu.cornell.mannlib.vitro.webapp.edit.n3editing.configuration.EditConfigurationConstants;
+import edu.cornell.mannlib.vitro.webapp.freemarker.loader.FreemarkerTemplateLoader;
import edu.cornell.mannlib.vitro.webapp.i18n.freemarker.I18nMethodModel;
import edu.cornell.mannlib.vitro.webapp.startup.StartupStatus;
import edu.cornell.mannlib.vitro.webapp.utils.developer.DeveloperSettings;
@@ -35,7 +34,6 @@ import edu.cornell.mannlib.vitro.webapp.web.methods.IndividualLocalNameMethod;
import edu.cornell.mannlib.vitro.webapp.web.methods.IndividualPlaceholderImageUrlMethod;
import edu.cornell.mannlib.vitro.webapp.web.methods.IndividualProfileUrlMethod;
import freemarker.cache.ClassTemplateLoader;
-import freemarker.cache.FileTemplateLoader;
import freemarker.cache.MultiTemplateLoader;
import freemarker.cache.TemplateLoader;
import freemarker.ext.beans.BeansWrapper;
@@ -150,38 +148,31 @@ public abstract class FreemarkerConfiguration {
List+ * "this_es_MX.ftl" matches "this_es_MX.ftl" + * "this_es.ftl" matches "this_es.ftl" or "this_es_MX.ftl" + * "this.ftl" matches "this.ftl" or "this_es.ftl" or "this_es_MX.ftl" + *+ * + * This allows Freemarker to mimic the behavior of the language filtering RDF + * service, because if Freemarker does not find a match for "this_es_MX.ftl", it + * will try again with "this_es.ftl" and "this.ftl". So the net effect is that a + * search for "silly_es_MX.ftl" would eventually return any of these, in order + * of preference: + * + *
+ * silly_es_MX.ftl + * silly_es.ftl + * silly_es_*.ftl + * silly.ftl + * silly_*.ftl + *+ * + * If more than one template file qualifies, we choose by best fit, shortest + * path, and alphabetical order, to insure that identical requests produce + * identical results. + */ +public class FreemarkerTemplateLoader implements TemplateLoader { + private static final Log log = LogFactory + .getLog(FreemarkerTemplateLoader.class); + + private final File baseDir; + + public FreemarkerTemplateLoader(File baseDir) { + if (baseDir == null) { + throw new NullPointerException("baseDir may not be null."); + } + + String path = baseDir.getAbsolutePath(); + if (!baseDir.exists()) { + throw new IllegalArgumentException("Template directory '" + path + + "' does not exist"); + } + if (!baseDir.isDirectory()) { + throw new IllegalArgumentException("Template directory '" + path + + "' is not a directory"); + } + if (!baseDir.canRead()) { + throw new IllegalArgumentException( + "Can't read template directory '" + path + "'"); + } + + log.debug("Created template loader - baseDir is '" + path + "'"); + this.baseDir = baseDir; + } + + /** + * Get the best template for this name. Walk the tree finding all possible + * matches, then choose our favorite. + */ + @Override + public Object findTemplateSource(String name) throws IOException { + if (StringUtils.isBlank(name)) { + return null; + } + + SortedSet
- * 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. - * - *- */ - // ---------------------------------------------------------------------- - // 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 File tempDir; - private static File notADirectory; - private static File upperTemplate; - private static File lowerTemplate; - - private FlatteningTemplateLoader loader; - - @BeforeClass - public static void setUpFiles() throws IOException { - 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); - } - - /** - * Some systems only record last-modified times to the nearest second, so we - * can't rely on them changing during the course of the test. Force the - * change, and test for it. - */ - @Test - public void teplateLastModified() throws IOException { - Object source = loader.findTemplateSource(TEMPLATE_NAME_UPPER); - long modified = loader.getLastModified(source); - long now = System.currentTimeMillis(); - assertTrue("near to now: modified=" + formatTimeStamp(modified) - + ", now=" + formatTimeStamp(now), - 2000 > Math.abs(modified - now)); - - upperTemplate.setLastModified(5000); - assertEquals("modified modified", 5000, loader.getLastModified(source)); - } - - @Test - public void closeDoesntCrash() throws IOException { - Object source = loader.findTemplateSource(TEMPLATE_NAME_UPPER); - loader.closeTemplateSource(source); - } - - // ---------------------------------------------------------------------- - // helper methods - // ---------------------------------------------------------------------- - - private String formatTimeStamp(long time) { - SimpleDateFormat formatter = new SimpleDateFormat( - "yyyy-MM-dd HH:mm:ss.SSS"); - return formatter.format(time); - } - -} diff --git a/webapp/test/edu/cornell/mannlib/vitro/webapp/freemarker/loader/FreemarkerTemplateLoaderTest.java b/webapp/test/edu/cornell/mannlib/vitro/webapp/freemarker/loader/FreemarkerTemplateLoaderTest.java new file mode 100644 index 000000000..5b4d8b40d --- /dev/null +++ b/webapp/test/edu/cornell/mannlib/vitro/webapp/freemarker/loader/FreemarkerTemplateLoaderTest.java @@ -0,0 +1,359 @@ +/* $This file is distributed under the terms of the license in /doc/license.txt$ */ + +package edu.cornell.mannlib.vitro.webapp.freemarker.loader; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +import java.io.IOException; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Arrays; +import java.util.SortedSet; + +import org.apache.commons.lang.StringUtils; +import org.junit.Test; + +import edu.cornell.mannlib.vitro.webapp.freemarker.loader.FreemarkerTemplateLoader.PathPieces; +import edu.cornell.mannlib.vitro.webapp.freemarker.loader.FreemarkerTemplateLoader.PathPiecesFileVisitor; + +/** + * TODO + */ +public class FreemarkerTemplateLoaderTest { + private PathPiecesFileVisitor visitor; + private String[] paths; + + // ---------------------------------------------------------------------- + // PathPieces tests + // ---------------------------------------------------------------------- + + @Test + public void ppLanguageRegionExtension() { + assertPathPieces("this_en_US.ftl", "this", "_en", "_US", ".ftl"); + } + + @Test + public void ppLanguageRegion() { + assertPathPieces("this_en_US", "this", "_en", "_US", ""); + } + + @Test + public void ppLanguageExtension() { + assertPathPieces("this_en.ftl", "this", "_en", "", ".ftl"); + } + + @Test + public void ppLanguage() { + assertPathPieces("this_en", "this", "_en", "", ""); + } + + @Test + public void ppDefaultExtension() { + assertPathPieces("this.ftl", "this", "", "", ".ftl"); + } + + @Test + public void ppDefault() { + assertPathPieces("this", "this", "", "", ""); + } + + @Test + public void ppExtraUnderscoreExtension() { + assertPathPieces("woo_hoo_en_US.ftl", "woo_hoo", "_en", "_US", ".ftl"); + } + + @Test + public void ppExtraUnderscore() { + assertPathPieces("woo_hoo_en_US", "woo_hoo", "_en", "_US", ""); + } + + // ---------------------------------------------------------------------- + // Specific function tests + // ---------------------------------------------------------------------- + + @Test + public void baseAndExtensionMatch() { + paths("match-me.ftl"); + assertMatches("match-me.ftl", 1, "match-me.ftl"); + } + + @Test + public void baseAndExtensionDontMatch() { + paths("match-me.ftl"); + assertMatches("fail.ftl", 0, null); + assertMatches("match-me", 0, null); + assertMatches("match-me.FTL", 0, null); + } + + @Test + public void matchRegardlessOfDepth() { + paths("short-path.ftl", "long/long-path.ftl"); + assertMatches("long/short-path.ftl", 1, "short-path.ftl"); + assertMatches("long-path.ftl", 1, "long/long-path.ftl"); + } + + @Test + public void preferShorterPath() { + paths("shorter-is-better", "long/shorter-is-better"); + assertMatches("shorter-is-better", 2, "shorter-is-better"); + } + + @Test + public void preferShorterPathToExactPath() { + paths("shorter-is-better", "long/shorter-is-better"); + assertMatches("long/shorter-is-better", 2, "shorter-is-better"); + } + + @Test + public void languageAndRegionMustMatchExactly() { + paths("this_es_MX.ftl", "this_es_ES.ftl", "this_es.ftl"); + assertMatches("this_es_ES.ftl", 1, "this_es_ES.ftl"); + } + + @Test + public void languageAndRegionNoMatch() { + paths("this_es_MX.ftl", "this_es_ES.ftl", "this_es.ftl"); + assertMatches("this_es_GO.ftl", 0, null); + } + + @Test + public void languagePrefersExactMatch() { + paths("this_es_MX.ftl", "this_es.ftl", "this_es_ES.ftl"); + assertMatches("this_es.ftl", 3, "this_es.ftl"); + } + + @Test + public void languageAcceptsApproximateMatch() { + paths("this_es_MX.ftl"); + assertMatches("this_es.ftl", 1, "this_es_MX.ftl"); + } + + @Test + public void languagePrefersApproximateAlphabetical() { + paths("this_es_MX.ftl", "this_es_ES.ftl"); + assertMatches("this_es.ftl", 2, "this_es_ES.ftl"); + } + + @Test + public void defaultPrefersExactMatch() { + paths("this_fr.ftl", "this.ftl", "this_fr_BE.ftl"); + assertMatches("this.ftl", 3, "this.ftl"); + } + + @Test + public void defaultPrefersDefaultRegion() { + paths("this_fr_BE.ftl", "this_fr.ftl", "this_fr_CA.ftl"); + assertMatches("this.ftl", 3, "this_fr.ftl"); + } + + @Test + public void defaultPrefersLanguageAlphabetical() { + paths("this_es.ftl", "this_fr.ftl"); + assertMatches("this.ftl", 2, "this_es.ftl"); + } + + @Test + public void defaultPrefersRegionAlphabetical() { + paths("this_fr_BE.ftl", "this_fr_CA.ftl"); + assertMatches("this.ftl", 2, "this_fr_BE.ftl"); + } + + // ---------------------------------------------------------------------- + // Freemarker simulation tests + // ---------------------------------------------------------------------- + + public static final String[] FREEMARKER_TEST_PATHS = { + "long/this_fr_BE.ftl", "language_fr.ftl", "default.ftl", + "language-approx_en_US.ftl" }; + + @Test + public void freemarkerLangAndRegionExact() { + paths = FREEMARKER_TEST_PATHS; + assertFM("this_fr_BE.ftl", 1, "long/this_fr_BE.ftl"); + } + + @Test + public void freemarkerLangAndRegionMatchLang() { + paths = FREEMARKER_TEST_PATHS; + assertFM("language_fr_CA.ftl", 2, "language_fr.ftl"); + } + + @Test + public void freemarkerLangAndRegionMatchDefault() { + paths = FREEMARKER_TEST_PATHS; + assertFM("default_es_ES.ftl", 3, "default.ftl"); + } + + @Test + public void freemarkerLangAndRegionNoMatch() { + paths = FREEMARKER_TEST_PATHS; + assertFM("bogus_en_US.ftl", 3, null); + } + + @Test + public void freemarkerLangExact() { + paths = FREEMARKER_TEST_PATHS; + assertFM("language_fr.ftl", 1, "language_fr.ftl"); + } + + @Test + public void freemarkerLangMatchLangAndRegion() { + paths = FREEMARKER_TEST_PATHS; + assertFM("language-approx_en.ftl", 1, "language-approx_en_US.ftl"); + } + + @Test + public void freemarkerLangMatchDefault() { + paths = FREEMARKER_TEST_PATHS; + assertFM("default_en.ftl", 2, "default.ftl"); + } + + @Test + public void freemarkerLangNoMatch() { + paths = FREEMARKER_TEST_PATHS; + assertFM("bogus_it.ftl", 2, null); + } + + @Test + public void freemarkerDefaultExact() { + paths = FREEMARKER_TEST_PATHS; + assertFM("default.ftl", 1, "default.ftl"); + } + + @Test + public void freemarkerDefaultMatchLang() { + paths = FREEMARKER_TEST_PATHS; + assertFM("language.ftl", 1, "language_fr.ftl"); + } + + @Test + public void freemarkerDefaultMatchLangAndRegion() { + paths = FREEMARKER_TEST_PATHS; + assertFM("this.ftl", 1, "long/this_fr_BE.ftl"); + } + + @Test + public void freemarkerDefaultNoMatch() { + paths = FREEMARKER_TEST_PATHS; + assertFM("bogus.ftl", 1, null); + } + + // ---------------------------------------------------------------------- + // Helper methods + // ---------------------------------------------------------------------- + + private void paths(String... p) { + this.paths = p; + } + + private void assertPathPieces(String path, String base, String language, + String region, String extension) { + PathPieces pp = new PathPieces(path); + String[] expected = new String[] { base, language, region, extension }; + String[] actual = new String[] { pp.base, pp.language, pp.region, + pp.extension }; + assertEquals("pieces", Arrays.asList(expected), Arrays.asList(actual)); + } + + /** + * @param searchTerm + * template we are looking for + * @param expectedHowMany + * How many matches do we expect? + * @param expectedBestFit + * What should the best match turn out to be? + * @throws IOException + */ + private void assertMatches(String searchTerm, int expectedHowMany, + String expectedBestFit) { + SortedSet