VIVO-125 Improvements to I18n

Do not clear the cache more than once per request. Improve the hooks for unit tests.

webapp/src/edu/cornell/mannlib/vitro/webapp/i18n/I18n.java

@@ -7,7 +7,6 @@ import java.util.List;
 import java.util.Locale;
 import java.util.MissingResourceException;
 import java.util.ResourceBundle;
-import java.util.ResourceBundle.Control;
 import java.util.concurrent.atomic.AtomicReference;
 
 import javax.servlet.ServletContext;
@@ -34,6 +33,13 @@ public class I18n {
 	public static final String DEFAULT_BUNDLE_NAME = "all";
 	private static final String PROPERTY_DEVELOPER_DEFEAT_CACHE = "developer.defeatI18nCache";
 
+	/**
+	 * If this attribute is present on the request, then the cache has already
+	 * been cleared.
+	 */
+	private static final String ATTRIBUTE_CACHE_CLEARED = I18n.class.getName()
+			+ "-cacheCleared";
+
 	/**
 	 * This is where the work gets done. Not declared final, so it can be
 	 * modified in unit tests.
@@ -91,8 +97,10 @@ public class I18n {
 	 * cache is cleared on each request.
 	 * 
 	 * If the theme directory has changed, the cache is cleared.
+	 * 
+	 * Declared 'protected' so it can be overridden in unit tests.
 	 */
-	private I18nBundle getBundle(String bundleName, HttpServletRequest req) {
+	protected I18nBundle getBundle(String bundleName, HttpServletRequest req) {
 		log.debug("Getting bundle '" + bundleName + "'");
 
 		try {
@@ -102,7 +110,7 @@ public class I18n {
 			String dir = themeDirectory.get();
 			ServletContext ctx = req.getSession().getServletContext();
 
-			ResourceBundle.Control control = getControl(ctx, dir);
+			ResourceBundle.Control control = new ThemeBasedControl(ctx, dir);
 			ResourceBundle rb = ResourceBundle.getBundle(bundleName,
 					req.getLocale(), control);
 			return new I18nBundle(bundleName, rb);
@@ -125,7 +133,7 @@ public class I18n {
 				.getProperty(PROPERTY_DEVELOPER_DEFEAT_CACHE, "false");
 		if (Boolean.valueOf(flag.trim())) {
 			log.debug("In development mode - clearing the cache.");
-			ResourceBundle.clearCache();
+			clearCacheOnRequest(req);
 		}
 	}
 
@@ -139,16 +147,19 @@ public class I18n {
 		if (!currentDir.equals(previousDir)) {
 			log.debug("Theme directory changed from '" + previousDir + "' to '"
 					+ currentDir + "' - clearing the cache.");
-			ResourceBundle.clearCache();
+			clearCacheOnRequest(req);
 		}
 	}
 
-	/**
+	/** Only clear the cache one time per request. */
-	 * Override this method in the unit tests, to return a more testable Control
+	private void clearCacheOnRequest(HttpServletRequest req) {
-	 * instance.
+		if (req.getAttribute(ATTRIBUTE_CACHE_CLEARED) != null) {
-	 */
+			log.debug("Cache was already cleared on this request.");
-	protected Control getControl(ServletContext ctx, String dir) {
+		} else {
-		return new ThemeBasedControl(ctx, dir);
+			ResourceBundle.clearCache();
+			log.debug("Cache cleared.");
+			req.setAttribute(ATTRIBUTE_CACHE_CLEARED, Boolean.TRUE);
+		}
 	}
 
 	// ----------------------------------------------------------------------

webapp/test/stubs/edu/cornell/mannlib/vitro/webapp/i18n/I18nStub.java

@@ -0,0 +1,78 @@
+/* $This file is distributed under the terms of the license in /doc/license.txt$ */
+
+package stubs.edu.cornell.mannlib.vitro.webapp.i18n;
+
+import java.lang.reflect.Field;
+import java.util.Collections;
+import java.util.Enumeration;
+import java.util.ResourceBundle;
+
+import javax.servlet.http.HttpServletRequest;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+
+import edu.cornell.mannlib.vitro.webapp.i18n.I18n;
+import edu.cornell.mannlib.vitro.webapp.i18n.I18nBundle;
+
+/**
+ * An implementation of I18n for unit tests. Construct a new instance and it
+ * replaces the instance of I18n.
+ * 
+ * Each bundle that you get from it is the same, returning the key itself as the
+ * value of that key.
+ */
+public class I18nStub extends I18n {
+	private static final Log log = LogFactory.getLog(I18nStub.class);
+
+	// ----------------------------------------------------------------------
+	// Stub infrastructure
+	// ----------------------------------------------------------------------
+
+	public I18nStub() {
+		try {
+			Field instanceField = I18n.class.getDeclaredField("instance");
+			log.debug("Field is " + instanceField);
+			instanceField.setAccessible(true);
+			log.debug("Instance is " + instanceField.get(null));
+			instanceField.set(null, this);
+			log.debug("Instance is " + instanceField.get(null));
+			log.debug("Created and inserted.");
+		} catch (Exception e) {
+			throw new RuntimeException(e);
+		}
+
+	}
+
+	@Override
+	protected I18nBundle getBundle(String bundleName, HttpServletRequest req) {
+		return new I18nBundleStub(bundleName);
+	}
+
+	private class I18nBundleStub extends I18nBundle {
+		public I18nBundleStub(String bundleName) {
+			super(bundleName, new DummyResourceBundle());
+		}
+
+		@Override
+		public String text(String key, Object... parameters) {
+			return key;
+		}
+	}
+
+	/**
+	 * Not actually used, but the constructor of I18nBundle requires a non-null
+	 * ResourceBundle.
+	 */
+	private class DummyResourceBundle extends ResourceBundle {
+		@Override
+		protected Object handleGetObject(String key) {
+			return key;
+		}
+
+		@Override
+		public Enumeration<String> getKeys() {
+			return Collections.emptyEnumeration();
+		}
+	}
+}