NIHVIVO-927 Merge 5418 from the branch.

webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/backend/FileStorageImpl.java

@@ -17,11 +17,9 @@ import java.io.PrintWriter;
 import java.io.Reader;
 import java.util.Collection;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.Map;
-import java.util.Properties;
-import java.util.Set;
 import java.util.Map.Entry;
+import java.util.Properties;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -62,10 +60,15 @@ public class FileStorageImpl implements FileStorage {
 				FILE_STORAGE_NAMESPACES_PROPERTIES);
 
 		if (rootDir.exists() && namespaceFile.exists()) {
-			this.namespacesMap = confirmNamespaces(namespaces);
+			Map<Character, String> existingMap = readNamespaces();
+			this.namespacesMap = adjustNamespaces(existingMap, namespaces);
+			if (!namespacesMap.equals(existingMap)) {
+				initializeNamespacesFile();
+			}
 		} else if (!rootDir.exists() && !namespaceFile.exists()) {
 			this.namespacesMap = mapNamespaces(namespaces);
-			initializeStorage();
+			initializeRootDirectory();
+			initializeNamespacesFile();
 		} else if (rootDir.exists()) {
 			throw new IllegalStateException("Storage directory '"
 					+ baseDir.getPath() + "' has been partially initialized. '"
@@ -114,30 +117,16 @@ public class FileStorageImpl implements FileStorage {
 	 */
 	private Map<Character, String> mapNamespaces(Collection<String> namespaces) {
 		Map<Character, String> map = new HashMap<Character, String>();
-
-		char prefixChar = 'a';
 		for (String namespace : namespaces) {
-			map.put(prefixChar, namespace);
+			map.put(findAvailableKey(map), namespace);
-			prefixChar++;
-			if (prefixChar > 'z') {
-				throw new IllegalArgumentException(
-						"Can't handle more than 26 namespaces.");
-			}
 		}
 		return map;
 	}
 
 	/**
-	 * Create the root directory and the namespaces file. Write the namespaces
+	 * @throws FileNotFoundException
-	 * map to the namespaces file.
 	 */
-	private void initializeStorage() throws IOException {
+	private void initializeNamespacesFile() throws FileNotFoundException {
-		boolean created = this.rootDir.mkdir();
-		if (!created) {
-			throw new IOException("Failed to create root directory '"
-					+ this.rootDir + "'");
-		}
-
 		PrintWriter writer = null;
 		try {
 			writer = new PrintWriter(this.namespaceFile);
@@ -152,22 +141,34 @@ public class FileStorageImpl implements FileStorage {
 	}
 
 	/**
-	 * Confirm that the namespaces file contains mappings for these namespaces,
+	 * Create the root directory. Check for success.
-	 * and only these namespaces.
 	 */
-	private Map<Character, String> confirmNamespaces(
+	private void initializeRootDirectory() throws IOException {
-			Collection<String> namespaces) throws IOException {
+		boolean created = this.rootDir.mkdir();
-		Map<Character, String> map;
+		if (!created) {
+			throw new IOException("Failed to create root directory '"
+					+ this.rootDir + "'");
+		}
+	}
+
+	/**
+	 * Load the namespaces file from the disk. It's easy to load into a
+	 * {@link Properties}, but we need to convert it to a {@link Map}.
+	 */
+	private Map<Character, String> readNamespaces() throws IOException {
 		Reader reader = null;
 		try {
 			reader = new FileReader(this.namespaceFile);
 			Properties props = new Properties();
 			props.load(reader);
-			map = new HashMap<Character, String>();
+
+			Map<Character, String> map = new HashMap<Character, String>();
 			for (Object key : props.keySet()) {
 				char keyChar = key.toString().charAt(0);
 				map.put(keyChar, (String) props.get(key));
 			}
+
+			return map;
 		} catch (Exception e) {
 			throw new IOException("Problem loading the namespace file.");
 		} finally {
@@ -179,19 +180,40 @@ public class FileStorageImpl implements FileStorage {
 				}
 			}
 		}
+	}
 
-		Set<String> requestedNamespaces = new HashSet<String>(namespaces);
+	/**
-		Set<String> previousNamespaces = new HashSet<String>(map.values());
+	 * If any of the requested aren't in the existing map, add them.
-		if (!requestedNamespaces.equals(previousNamespaces)) {
+	 */
-			throw new IllegalStateException(
+	private Map<Character, String> adjustNamespaces(
-					"File storage was previously initialized with a "
+			Map<Character, String> existingMap, Collection<String> namespaces) {
-							+ "different set of namespaces than are found "
+		Map<Character, String> adjustedMap = new HashMap<Character, String>(
-							+ "in the current request. Previous: "
+				existingMap);
-							+ previousNamespaces + ", Requested: "
+
-							+ requestedNamespaces);
+		for (String namespace : namespaces) {
+			if (!existingMap.values().contains(namespace)) {
+				log.warn("Adding a new namespace to the file storage system: "
+						+ namespace);
+				Character key = findAvailableKey(adjustedMap);
+				adjustedMap.put(key, namespace);
+			}
 		}
 
-		return map;
+		return adjustedMap;
+	}
+
+	/**
+	 * Are there any characters that we're not using as prefixes? We only allow
+	 * a-z.
+	 */
+	private Character findAvailableKey(Map<Character, String> adjustedMap) {
+		for (char key = 'a'; key <= 'z'; key++) {
+			if (!adjustedMap.keySet().contains(key)) {
+				return key;
+			}
+		}
+		throw new IllegalArgumentException(
+				"Can't handle more than 26 namespaces.");
 	}
 
 	// ----------------------------------------------------------------------

webapp/test/edu/cornell/mannlib/vitro/webapp/filestorage/backend/FileStorageImplTest.java

@@ -20,6 +20,7 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 
+import org.apache.log4j.Level;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
@@ -108,10 +109,21 @@ public class FileStorageImplTest extends AbstractTestClass {
 		createFileStorage("initializeTwiceTheSame", "ns2", "ns1");
 	}
 
-	@Test(expected = IllegalStateException.class)
+	@Test
-	public void initializedNamespacesDontMatch() throws IOException {
+	public void namespaceDisappears() throws IOException {
-		createFileStorage("initializeTwiceDifferent", "ns1", "ns2");
+		createFileStorage("namespaceDisappears", "ns1", "ns2");
-		createFileStorage("initializeTwiceDifferent", "ns2");
+		FileStorageImpl fs = createFileStorage("namespaceDisappears", "ns2");
+		assertEqualSets("namespaces", new String[] { "ns1", "ns2" }, fs
+				.getNamespaces().values());
+	}
+
+	@Test
+	public void namespaceChanged() throws IOException {
+		setLoggerLevel(FileStorageImpl.class, Level.ERROR);
+		createFileStorage("namespaceChanges", "ns1", "ns2");
+		FileStorageImpl fs = createFileStorage("namespaceChanges", "ns3", "ns1");
+		assertEqualSets("namespaces", new String[] { "ns1", "ns2", "ns3" }, fs
+				.getNamespaces().values());
 	}
 
 	@Test
@@ -187,8 +199,8 @@ public class FileStorageImplTest extends AbstractTestClass {
 		generalFs.createFile(id, filename, bytes);
 
 		assertFileContents(generalFs, id, filename, contents);
-		assertEquals("getInputStream", contents, readAll(generalFs
+		assertEquals("getInputStream", contents,
-				.getInputStream(id, filename)));
+				readAll(generalFs.getInputStream(id, filename)));
 	}
 
 	@Test(expected = FileNotFoundException.class)
@@ -226,8 +238,8 @@ public class FileStorageImplTest extends AbstractTestClass {
 
 		assertFileContents(generalFs, id, filename, contents);
 		assertEquals("filename", filename, generalFs.getFilename(id));
-		assertEquals("getInputStream", contents, readAll(generalFs
+		assertEquals("getInputStream", contents,
-				.getInputStream(id, filename)));
+				readAll(generalFs.getInputStream(id, filename)));
 	}
 
 	// ----------------------------------------------------------------------
@@ -265,8 +277,8 @@ public class FileStorageImplTest extends AbstractTestClass {
 	private void assertFileContents(FileStorageImpl fs, String id,
 			String filename, String expectedContents) throws IOException {
 		File rootDir = new File(fs.getBaseDir(), FileStorage.FILE_STORAGE_ROOT);
-		File path = FileStorageHelper.getFullPath(rootDir, id, filename, fs
+		File path = FileStorageHelper.getFullPath(rootDir, id, filename,
-				.getNamespaces());
+				fs.getNamespaces());
 
 		assertTrue("file exists: " + path, path.exists());