slub.team team mailing list archive

[Ralf Claussnitzer <ralf.claussnitzer@xxxxxxxxxxxxxxx>]

Ralf Claussnitzer has proposed merging lp:~slub.team/goobi-production/bug-988813 into lp:goobi-production.

Requested reviews:
  Henning Gerhardt (henning-gerhardt)
Related bugs:
  Bug #988813 in Goobi.Production: "Renaming of files sometimes fails"
  https://bugs.launchpad.net/goobi-production/+bug/988813

For more details, see:
https://code.launchpad.net/~slub.team/goobi-production/bug-988813/+merge/105330

Reconciled with master branch. Uses FilesystemHelper class for file renaming.
-- 
https://code.launchpad.net/~slub.team/goobi-production/bug-988813/+merge/105330
Your team Saxon State Library Team is subscribed to branch lp:goobi-production.

=== modified file 'src/de/sub/goobi/beans/Prozess.java'
--- src/de/sub/goobi/beans/Prozess.java	2012-05-09 15:19:19 +0000
+++ src/de/sub/goobi/beans/Prozess.java	2012-05-10 14:00:29 +0000
@@ -58,6 +58,7 @@
 import de.sub.goobi.persistence.BenutzerDAO;
 import de.sub.goobi.persistence.ProzessDAO;
 import de.sub.goobi.config.ConfigMain;
+import de.sub.goobi.helper.FilesystemHelper;
 import de.sub.goobi.helper.Helper;
 import de.sub.goobi.helper.Messages;
 import de.sub.goobi.helper.enums.MetadataFormat;
@@ -728,17 +729,6 @@
 		return result;
 	}
 
-	private void renameMetadataFile(String oldFileName, String newFileName) {
-		File oldFile;
-		File newFile;
-
-		if (oldFileName != null && newFileName != null) {
-			oldFile = new File(oldFileName);
-			newFile = new File(newFileName);
-			oldFile.renameTo(newFile);
-		}
-	}
-
 	private String getTemporaryMetadataFileName(String fileName) {
 		File temporaryFile = new File(fileName);
 		String directoryPath = temporaryFile.getParentFile().getPath();
@@ -747,7 +737,8 @@
 		return directoryPath + File.separator + temporaryFileName;
 	}
 
-	private void removePrefixFromRelatedMetsAnchorFileFor(String temporaryMetadataFilename) {
+	private void removePrefixFromRelatedMetsAnchorFileFor(String temporaryMetadataFilename)
+		throws IOException {
 		File temporaryFile = new File(temporaryMetadataFilename);
 		File temporaryAnchorFile;
 
@@ -762,7 +753,7 @@
 			temporaryAnchorFileName = directoryPath + File.separator + temporaryAnchorFileName;
 			anchorFileName = directoryPath + File.separator + anchorFileName;
 
-			renameMetadataFile(temporaryAnchorFileName, anchorFileName);
+			FilesystemHelper.renameFile(temporaryAnchorFileName, anchorFileName);
 		}
 	}
 
@@ -794,7 +785,7 @@
 		writeResult = ff.write(temporaryMetadataFileName);
 		if (writeResult) {
 			createBackupFile();
-			renameMetadataFile(temporaryMetadataFileName, metadataFileName);
+			FilesystemHelper.renameFile(temporaryMetadataFileName, metadataFileName);
 			removePrefixFromRelatedMetsAnchorFileFor(temporaryMetadataFileName);
 		}
 	}

=== added file 'src/de/sub/goobi/helper/FilesystemHelper.java'
--- src/de/sub/goobi/helper/FilesystemHelper.java	1970-01-01 00:00:00 +0000
+++ src/de/sub/goobi/helper/FilesystemHelper.java	2012-05-10 14:00:29 +0000
@@ -0,0 +1,101 @@
+/*
+ * This file is part of the Goobi Application - a Workflow tool for the support of
+ * mass digitization.
+ *
+ * Visit the websites for more information.
+ *     - http://gdz.sub.uni-goettingen.de
+ *     - http://www.goobi.org
+ *     - http://launchpad.net/goobi-production
+ *
+ * This program is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License as published by the Free Software
+ * Foundation; either version 2 of the License, or (at your option) any later
+ * version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT ANY
+ * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A
+ * PARTICULAR PURPOSE. See the GNU General Public License for more details. You
+ * should have received a copy of the GNU General Public License along with this
+ * program; if not, write to the Free Software Foundation, Inc., 59 Temple Place,
+ * Suite 330, Boston, MA 02111-1307 USA
+ */
+
+package de.sub.goobi.helper;
+
+import org.apache.commons.lang.SystemUtils;
+import org.apache.log4j.Logger;
+
+import java.io.File;
+import java.io.FileNotFoundException;
+import java.io.IOException;
+
+/**
+ * Helper class for file system operations.
+ * 
+ * @author Matthias Ronge <matthias.ronge@xxxxxxxxxxxx>
+ */
+public class FilesystemHelper {
+	private static final Logger logger = Logger
+			.getLogger(FilesystemHelper.class);
+
+	/**
+	 * This function implements file renaming. Renaming of files is full of
+	 * mischief under Windows which unaccountably holds locks on files.
+	 * Sometimes running the JVM’s garbage collector puts things right.
+	 * 
+	 * @param oldFileName
+	 *            File to move or rename
+	 * @param newFileName
+	 *            New file name / destination
+	 * @throws IOException
+	 *             is thrown if the rename fails permanently
+ 	 * @throws FileNotFoundException
+	 *             is thrown if old file (source file of renaming) does not exists
+	 */
+	public static void renameFile(String oldFileName, String newFileName)
+			throws IOException {
+		final int SLEEP_INTERVAL_MILLIS = 20;
+		final int MAX_WAIT_MILLIS = 150000; // 2½ minutes
+		File oldFile;
+		File newFile;
+		boolean success;
+		int millisWaited = 0;
+
+		if ((oldFileName == null) || (newFileName == null)) {
+			return;
+		}
+
+		oldFile = new File(oldFileName);
+		newFile = new File(newFileName);
+
+		if (! oldFile.exists()) {
+			logger.debug("File " + oldFileName + " does not exist for renaming.");
+			throw new FileNotFoundException(oldFileName + " does not exist for renaming.");
+		}
+
+		do {
+			if (SystemUtils.IS_OS_WINDOWS
+					&& millisWaited == SLEEP_INTERVAL_MILLIS) {
+				logger.warn("Renaming " + oldFileName  + " failed. This is Windows. Running the garbage collector may yield good results. Forcing immediate garbage collection now!");
+				System.gc();
+			}
+			success = oldFile.renameTo(newFile);
+			if (!success) {
+				if (millisWaited == 0)
+					logger.info("Renaming " + oldFileName + " failed. File may be locked. Retrying...");
+				try {
+					Thread.sleep(SLEEP_INTERVAL_MILLIS);
+				} catch (InterruptedException e) {
+				}
+				millisWaited += SLEEP_INTERVAL_MILLIS;
+			}
+		} while (!success && millisWaited < MAX_WAIT_MILLIS);
+
+		if (!success) {
+			logger.error("Rename " + oldFileName + " failed. This is a permanent error. Giving up.");
+			throw new IOException("Renaming of " + oldFileName + " into "
+					+ newFileName + " failed.");
+		} else if (millisWaited > 0)
+			logger.info("Rename finally succeeded after" + Integer.toString(millisWaited) + " milliseconds.");
+	}
+}

=== modified file 'src/org/goobi/io/BackupFileRotation.java'
--- src/org/goobi/io/BackupFileRotation.java	2012-04-13 15:32:20 +0000
+++ src/org/goobi/io/BackupFileRotation.java	2012-05-10 14:00:29 +0000
@@ -22,10 +22,13 @@
 
 package org.goobi.io;
 
+import de.sub.goobi.helper.FilesystemHelper;
 import org.apache.log4j.Logger;
 
 import java.io.File;
+import java.io.FileNotFoundException;
 import java.io.FilenameFilter;
+import java.io.IOException;
 
 /**
  * Creates backup for files in a given directory that match a regular expression.
@@ -101,15 +104,10 @@
 	}
 
 	private void rename(String oldFileName, String newFileName) {
-		File oldFile = new File(oldFileName);
-		File newFile = new File(newFileName);
-
-		if (oldFile.exists()) {
-			boolean renameSuccessful = oldFile.renameTo(newFile);
-
-			if (!renameSuccessful) {
-				myLogger.warn("Renaming file from " + oldFileName + " to " +  newFileName + " failed.");
-			}
+		try {
+			FilesystemHelper.renameFile(oldFileName, newFileName);
+		} catch (IOException ioe) {
+			myLogger.warn("Renaming file from " + oldFileName + " to " + newFileName + " failed. Reason: " + ioe.getMessage());
 		}
 	}
 

=== added file 'test/src/de/sub/goobi/helper/FilesystemHelperTest.java'
--- test/src/de/sub/goobi/helper/FilesystemHelperTest.java	1970-01-01 00:00:00 +0000
+++ test/src/de/sub/goobi/helper/FilesystemHelperTest.java	2012-05-10 14:00:29 +0000
@@ -0,0 +1,113 @@
+/*
+ * This file is part of the Goobi Application - a Workflow tool for the support of
+ * mass digitization.
+ *
+ * Visit the websites for more information.
+ *     - http://gdz.sub.uni-goettingen.de
+ *     - http://www.goobi.org
+ *     - http://launchpad.net/goobi-production
+ *
+ * This program is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License as published by the Free Software
+ * Foundation; either version 2 of the License, or (at your option) any later
+ * version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT ANY
+ * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A
+ * PARTICULAR PURPOSE. See the GNU General Public License for more details. You
+ * should have received a copy of the GNU General Public License along with this
+ * program; if not, write to the Free Software Foundation, Inc., 59 Temple Place,
+ * Suite 330, Boston, MA 02111-1307 USA
+ */
+
+package de.sub.goobi.helper;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.BeforeClass;
+
+import org.junit.Test;
+
+import static junit.framework.Assert.assertEquals;
+import static junit.framework.Assert.fail;
+
+import org.apache.log4j.BasicConfigurator;
+
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+
+public class FilesystemHelperTest {
+
+	@BeforeClass
+	public static void oneTimeSetUp() {
+		BasicConfigurator.configure();
+	}
+
+	@Before
+	public void setUp() throws Exception {
+	}
+
+	@After
+	public void tearDown() throws Exception {
+		deleteFile("old.xml");
+		deleteFile("new.xml");
+	}
+
+	@Test(expected = java.io.FileNotFoundException.class)
+	public void RenamingOfNonExistingFileShouldThrowFileNotFoundException () throws IOException {
+		String oldFileName = "old.xml";
+		String newFileName = "new.xml";
+
+		FilesystemHelper.renameFile(oldFileName, newFileName);
+	}
+
+	@Test
+	public void shouldRenameAFile()
+		throws IOException {
+		createFile("old.xml");
+		FilesystemHelper.renameFile("old.xml", "new.xml");
+		assertFileExists("new.xml");
+		assertFileNotExists("old.xml");
+	}
+
+	@Test
+	public void nothingHappensIfSourceFilenameIsNotSet()
+		throws IOException {
+		FilesystemHelper.renameFile(null, "new.xml");
+		assertFileNotExists("new.xml");
+	}
+	
+	@Test
+	public void nothingHappensIfTargetFilenameIsNotSet()
+		throws IOException {
+		createFile("old.xml");
+		FilesystemHelper.renameFile("old.xml", null);
+		assertFileNotExists("new.xml");
+	}
+
+	private void assertFileExists(String fileName) {
+		File newFile = new File(fileName);
+		if (!newFile.exists()) {
+			fail("File " + fileName + " does not exist.");
+		}
+	}
+
+	private void assertFileNotExists(String fileName) {
+		File newFile = new File(fileName);
+		if (newFile.exists()) {
+			fail("File " + fileName + " should not exist.");
+		}
+	}
+
+	private void createFile(String fileName) throws IOException {
+		File testFile = new File(fileName);
+		FileWriter writer = new FileWriter(testFile);
+		writer.close();
+	}
+
+	private void deleteFile(String fileName) {
+		File testFile = new File(fileName);
+		testFile.delete();
+	}
+}