diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/ConfigurationProperties.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/ConfigurationProperties.java index 2435a4477..ae6a38884 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/ConfigurationProperties.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/ConfigurationProperties.java @@ -42,83 +42,31 @@ import org.apache.log4j.Logger; public class ConfigurationProperties { private static final Logger LOG = Logger .getLogger(ConfigurationProperties.class); - + + /** + * The JNDI naming context where Tomcat stores environment attributes. + */ + static final String JNDI_BASE = "java:comp/env"; + /** * The name of the JNDI environment mapping for the path to the * configuration file (or resource). */ - private static final String PATH_CONFIGURATION = "path.configuration"; + static final String PATH_CONFIGURATION = "path.configuration"; + + /** + * The map of the configuration properties. + * + * This should not be accessed directly, but only through the synchronized + * method {@link #getTheMap() (and {@link #reset()} for unit tests). + */ private static volatile Map theMap; - static { - try { - // Obtain our environment naming context - Context initCtx = new InitialContext(); - Context envCtx = (Context) initCtx.lookup("java:comp/env"); - - // Get the name of the configuration properties file. - String configPath = (String) envCtx.lookup(PATH_CONFIGURATION); - if (configPath == null) { - throw new IllegalStateException( - "Could not find a JNDI Environment naming for '" - + PATH_CONFIGURATION + "'."); - } - - InputStream inStream = null; - // Try to find this as a file. - File file = new File(configPath); - try { - inStream = new FileInputStream(file); - } catch (FileNotFoundException e) { - inStream = null; - } - - // If no file, try to find it as a resource. - if (inStream == null) { - inStream = ConfigurationProperties.class.getClassLoader() - .getResourceAsStream(configPath); - } - - // If neither file nor resource, give up. - if (inStream == null) { - throw new IllegalArgumentException( - "Failed to find a configuration properties file at '" - + file.getAbsolutePath() - + "', or a resource at '" + configPath + "'"); - } - - // Load a properties object - it will handle the syntax of the file. - Properties props = new Properties(); - try { - props.load(inStream); - } catch (IOException e) { - throw new IllegalStateException("Problem while reading the " - + "configuration properties file at '" + configPath - + "'", e); - } - - // It's awkward to copy from Properties to a Map. - Map newMap = new HashMap(); - for (Enumeration keys = props.keys(); keys.hasMoreElements();) { - String key = (String) keys.nextElement(); - newMap.put(key, props.getProperty(key)); - } - - LOG.info("Configuration properties are: " + newMap); - - // Save an unmodifiable version of the Map - theMap = Collections.unmodifiableMap(newMap); - } catch (NamingException e) { - throw new IllegalStateException(e); - } - - } - /** * Get an unmodifiable copy of the map of configuration properties. */ public static Map getMap() { - return theMap; + return getTheMap(); } /** @@ -126,7 +74,7 @@ public class ConfigurationProperties { * property has not been assigned a value. */ public static String getProperty(String key) { - return theMap.get(key); + return getTheMap().get(key); } /** @@ -134,11 +82,144 @@ public class ConfigurationProperties { * property has not been assigned a value. */ public static String getProperty(String key, String defaultValue) { - String value = theMap.get(key); + String value = getTheMap().get(key); if (value == null) { return defaultValue; } else { return value; } } + + /** + * Force the map to be reloaded on the next attempt to access it. + * + * This and {@link #getTheMap()} should be the only access to + * {@link ConfigurationProperties#theMap}. + * + * NOTE: This should only be used in Unit Tests. + */ + static synchronized void reset() { + theMap = null; + } + + /** + * This and {@link #reset()} should be the only access to {@link #theMap}. + */ + private static synchronized Map getTheMap() { + if (theMap == null) { + theMap = loadTheMap(); + } + return theMap; + } + + /** + * The map is null, so find the properties file and load the map. + */ + private static synchronized Map loadTheMap() { + String configPath = getConfigurationFilePath(); + + InputStream inStream = getConfigurationInputStream(configPath); + + // Load a properties object - it will parse the file easily. + Properties props = new Properties(); + try { + props.load(inStream); + } catch (IOException e) { + throw new IllegalStateException("Problem while reading the " + + "configuration properties file at '" + configPath + "'", + e); + } finally { + try { + inStream.close(); + } catch (IOException e) { + LOG.error("Failed to close input stream", e); + } + } + + // It's awkward to copy from Properties to a Map. + Map newMap = new HashMap(); + for (Enumeration keys = props.keys(); keys.hasMoreElements();) { + String key = (String) keys.nextElement(); + newMap.put(key, props.getProperty(key)); + } + + LOG.info("Configuration properties are: " + newMap); + + // Save an unmodifiable version of the Map + return Collections.unmodifiableMap(newMap); + + } + + /** + * Find the path to the Configuration properties file. + * + * @throws IllegalStateException + * If we can't find the path. + */ + private static String getConfigurationFilePath() { + String message = ""; + try { + message = "JNDI Lookup on \"" + JNDI_BASE + + "\" failed. Is the context file missing?"; + Context envCtx = (Context) new InitialContext().lookup(JNDI_BASE); + if (envCtx == null) { + LOG.error(message); + throw new IllegalStateException(message); + } + + // Get the name of the configuration properties file. + message = "Could not find a JNDI Environment naming for '" + + PATH_CONFIGURATION + + "'. Is the context file set up correctly?"; + String configPath = (String) envCtx.lookup(PATH_CONFIGURATION); + if (configPath == null) { + LOG.error(message); + throw new IllegalStateException(message); + } + + return configPath; + } catch (NamingException e) { + throw new IllegalStateException(message, e); + } + } + + /** + * Find the Configuration properties file. + * + * First try to interpret the path as a file path (like + * /usr/local/config.props). + * + * If that doesn't work, try it as a resource path (relative to + * WEB-INF/classes). + * + * @throws IllegalArgumentException + * If the path fails to locate a file or a resource. + */ + private static InputStream getConfigurationInputStream(String configPath) { + InputStream inStream = null; + + // Try to find this as a file. + File file = new File(configPath); + try { + inStream = new FileInputStream(file); + } catch (FileNotFoundException e) { + inStream = null; + } + + // If no file, try to find it as a resource. + if (inStream == null) { + inStream = ConfigurationProperties.class.getClassLoader() + .getResourceAsStream(configPath); + } + + // If neither file nor resource, give up. + if (inStream == null) { + throw new IllegalArgumentException( + "Failed to find a configuration properties file at '" + + file.getAbsolutePath() + "', or a resource at '" + + configPath + "'"); + } + return inStream; + } + } diff --git a/webapp/test/edu/cornell/mannlib/vitro/webapp/ConfigurationPropertiesTest.java b/webapp/test/edu/cornell/mannlib/vitro/webapp/ConfigurationPropertiesTest.java new file mode 100644 index 000000000..1a0f70819 --- /dev/null +++ b/webapp/test/edu/cornell/mannlib/vitro/webapp/ConfigurationPropertiesTest.java @@ -0,0 +1,164 @@ +/* $This file is distributed under the terms of the license in /doc/license.txt$ */ + +package edu.cornell.mannlib.vitro.webapp; + +import static org.junit.Assert.assertEquals; + +import java.io.File; +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; + +import javax.naming.InitialContext; +import javax.naming.NamingException; + +import org.apache.log4j.Level; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; + +import stubs.javax.naming.InitialContextStub; +import stubs.javax.naming.spi.InitialContextFactoryStub; +import edu.cornell.mannlib.vitro.testing.AbstractTestClass; + +/** + * + * @author jeb228 + */ +public class ConfigurationPropertiesTest extends AbstractTestClass { + + /** + * The JNDI Mapping for the configuration properties path. + */ + private static final String CONFIGURATION_PROPERTIES_MAPPING = ConfigurationProperties.JNDI_BASE + + "/" + ConfigurationProperties.PATH_CONFIGURATION; + + private static final String NOT_THE_DESIRED_MAPPING = ConfigurationProperties.JNDI_BASE + + "/notTheDesiredMapping"; + + private static File tempDir; + private static File testFile; + private static File invalidFile; + + private InitialContext initial; + + /** + * Create a good test file and a bad test file. + * + * (a class-path-based resource should already exist.) + */ + @BeforeClass + public static void createTestFiles() throws IOException { + tempDir = createTempDirectory(ConfigurationPropertiesTest.class + .getSimpleName()); + testFile = createFile(tempDir, "testFile", "source = file\n"); + invalidFile = createFile(tempDir, "invalidFile", + "source = bad Unicode constant \\uu1045"); + } + + /** + * Clean up. + */ + @AfterClass + public static void removeTestFiles() { + purgeDirectoryRecursively(tempDir); + } + + /** + * I don't want to see the INFO messages while running the tests. + */ + @Before + public void setLogging() { + setLoggerLevel(ConfigurationProperties.class, Level.WARN); + } + + /** + * Use the context stub, and be sure that it is clean for each test. + */ + @Before + public void initializeContextStubs() throws NamingException { + System.setProperty(InitialContext.INITIAL_CONTEXT_FACTORY, + InitialContextFactoryStub.class.getName()); + InitialContextStub.reset(); + initial = new InitialContext(); + } + + /** + * {@link ConfigurationProperties} is a singleton, so we need to clean it + * before each test. + */ + @Before + public void resetProperties() { + ConfigurationProperties.reset(); + } + + // ---------------------------------------------------------------------- + // the tests + // ---------------------------------------------------------------------- + + @Test(expected = IllegalStateException.class) + public void topLevelContextIsMissing() { + ConfigurationProperties.getMap(); + } + + @Test(expected = IllegalStateException.class) + public void noEnvironmentMapping() throws NamingException { + // We map something in the same JNDI environment, + // but not the mapping we will be looking for. + initial.bind(NOT_THE_DESIRED_MAPPING, "doesn't matter"); + ConfigurationProperties.getMap(); + } + + @Test(expected = IllegalArgumentException.class) + public void fileNotFound() throws NamingException { + initial.bind(CONFIGURATION_PROPERTIES_MAPPING, "noSuchFileOrResource"); + ConfigurationProperties.getMap(); + } + + @Test(expected = IllegalArgumentException.class) + public void invalidFileFormat() throws NamingException { + initial.bind(CONFIGURATION_PROPERTIES_MAPPING, invalidFile.getPath()); + ConfigurationProperties.getMap(); + } + + @Test + public void readFromResource() throws NamingException { + initial.bind(CONFIGURATION_PROPERTIES_MAPPING, + "edu/cornell/mannlib/vitro/webapp/test_config.properties"); + assertExpectedMap(new String[][] { { "source", "resource" } }); + } + + @Test + public void readFromFile() throws NamingException { + initial.bind(CONFIGURATION_PROPERTIES_MAPPING, testFile.getPath()); + assertExpectedMap(new String[][] { { "source", "file" } }); + } + + @Test + public void checkOtherMethods() throws NamingException { + initial.bind(CONFIGURATION_PROPERTIES_MAPPING, testFile.getPath()); + assertEquals("file", ConfigurationProperties.getProperty("source")); + assertEquals(null, ConfigurationProperties.getProperty("notThere")); + assertEquals("default", ConfigurationProperties.getProperty("notThere", + "default")); + } + + // ---------------------------------------------------------------------- + // helper methods + // ---------------------------------------------------------------------- + + /** + * Does the configuration properties map look like this group of key/value + * pairs? + */ + private void assertExpectedMap(String[][] strings) { + Map expected = new HashMap(); + for (String[] pair : strings) { + expected.put(pair[0], pair[1]); + } + assertEquals("properties map", expected, ConfigurationProperties + .getMap()); + } + +} diff --git a/webapp/test/edu/cornell/mannlib/vitro/webapp/test_config.properties b/webapp/test/edu/cornell/mannlib/vitro/webapp/test_config.properties new file mode 100644 index 000000000..2cff85acf --- /dev/null +++ b/webapp/test/edu/cornell/mannlib/vitro/webapp/test_config.properties @@ -0,0 +1,4 @@ +# +# This is a data file for ConfigurationPropertiesTest. +# +source = resource \ No newline at end of file