Author: scolebourne
Date: Sun Mar 12 07:11:19 2006
New Revision: 385293
URL: http://svn.apache.org/viewcvs?rev=385293&view=rev
Log:
LockableFileWriter locking mechanism was broken and only provided limited
protection
File deletion and locking in case of constructor error was broken
bug 38942
Modified:
jakarta/commons/proper/io/trunk/RELEASE-NOTES.txt
jakarta/commons/proper/io/trunk/src/java/org/apache/commons/io/output/LockableFileWriter.java
jakarta/commons/proper/io/trunk/src/test/org/apache/commons/io/output/LockableFileWriterTest.java
jakarta/commons/proper/io/trunk/xdocs/upgradeto1_2.xml
Modified: jakarta/commons/proper/io/trunk/RELEASE-NOTES.txt
URL:
http://svn.apache.org/viewcvs/jakarta/commons/proper/io/trunk/RELEASE-NOTES.txt?rev=385293&r1=385292&r2=385293&view=diff
==============================================================================
--- jakarta/commons/proper/io/trunk/RELEASE-NOTES.txt (original)
+++ jakarta/commons/proper/io/trunk/RELEASE-NOTES.txt Sun Mar 12 07:11:19 2006
@@ -36,6 +36,10 @@
- FileUtils.read*
Increase certainty that files are closed in case of error
+- LockableFileWriter
+ Locking mechanism was broken and only provided limited protection [38942]
+ File deletion and locking in case of constructor error was broken
+
Enhancements from 1.1
---------------------
Modified:
jakarta/commons/proper/io/trunk/src/java/org/apache/commons/io/output/LockableFileWriter.java
URL:
http://svn.apache.org/viewcvs/jakarta/commons/proper/io/trunk/src/java/org/apache/commons/io/output/LockableFileWriter.java?rev=385293&r1=385292&r2=385293&view=diff
==============================================================================
---
jakarta/commons/proper/io/trunk/src/java/org/apache/commons/io/output/LockableFileWriter.java
(original)
+++
jakarta/commons/proper/io/trunk/src/java/org/apache/commons/io/output/LockableFileWriter.java
Sun Mar 12 07:11:19 2006
@@ -19,10 +19,12 @@
import java.io.FileOutputStream;
import java.io.FileWriter;
import java.io.IOException;
+import java.io.OutputStream;
import java.io.OutputStreamWriter;
import java.io.Writer;
import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.IOUtils;
/**
* FileWriter that will create and honor lock files to allow simple
@@ -52,9 +54,9 @@
private static final String LCK = ".lck";
/** The writer to decorate. */
- private Writer out;
+ private final Writer out;
/** The lock file. */
- private File lockFile;
+ private final File lockFile;
/**
* Constructs a LockableFileWriter.
@@ -171,22 +173,13 @@
File lockDirFile = new File(lockDir);
FileUtils.forceMkdir(lockDirFile);
testLockDir(lockDirFile);
- this.lockFile = new File(lockDirFile, file.getName() + LCK);
+ lockFile = new File(lockDirFile, file.getName() + LCK);
+
+ // check if locked
+ createLock();
// init wrapped writer
- try {
- createLock();
- if (encoding == null) {
- out = new FileWriter(file.getAbsolutePath(), append);
- } else {
- FileOutputStream fos = new
FileOutputStream(file.getAbsolutePath(), append);
- out = new OutputStreamWriter(fos, encoding);
- }
- } catch (IOException ioe) {
- this.lockFile.delete();
- throw ioe;
- }
- this.out = new FileWriter(file.getAbsolutePath(), append);
+ out = initWriter(file, encoding, append);
}
//-----------------------------------------------------------------------
@@ -223,6 +216,47 @@
}
}
+ /**
+ * Initialise the wrapped file writer.
+ * Ensure that a cleanup occurs if the writer creation fails.
+ *
+ * @param file the file to be accessed
+ * @param encoding the encoding to use
+ * @param append true to append
+ * @throws IOException if an error occurs
+ */
+ private Writer initWriter(File file, String encoding, boolean append)
throws IOException {
+ boolean fileExistedAlready = file.exists();
+ OutputStream stream = null;
+ Writer writer = null;
+ try {
+ if (encoding == null) {
+ writer = new FileWriter(file.getAbsolutePath(), append);
+ } else {
+ stream = new FileOutputStream(file.getAbsolutePath(), append);
+ writer = new OutputStreamWriter(stream, encoding);
+ }
+ } catch (IOException ex) {
+ IOUtils.closeQuietly(writer);
+ IOUtils.closeQuietly(stream);
+ lockFile.delete();
+ if (fileExistedAlready == false) {
+ file.delete();
+ }
+ throw ex;
+ } catch (RuntimeException ex) {
+ IOUtils.closeQuietly(writer);
+ IOUtils.closeQuietly(stream);
+ lockFile.delete();
+ if (fileExistedAlready == false) {
+ file.delete();
+ }
+ throw ex;
+ }
+ return writer;
+ }
+
+ //-----------------------------------------------------------------------
/**
* Closes the file writer.
*
Modified:
jakarta/commons/proper/io/trunk/src/test/org/apache/commons/io/output/LockableFileWriterTest.java
URL:
http://svn.apache.org/viewcvs/jakarta/commons/proper/io/trunk/src/test/org/apache/commons/io/output/LockableFileWriterTest.java?rev=385293&r1=385292&r2=385293&view=diff
==============================================================================
---
jakarta/commons/proper/io/trunk/src/test/org/apache/commons/io/output/LockableFileWriterTest.java
(original)
+++
jakarta/commons/proper/io/trunk/src/test/org/apache/commons/io/output/LockableFileWriterTest.java
Sun Mar 12 07:11:19 2006
@@ -17,8 +17,10 @@
import java.io.File;
import java.io.IOException;
+import java.io.Writer;
-import junit.framework.TestCase;
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.io.testtools.FileBasedTestCase;
/**
* Tests that files really lock, although no writing is done as
@@ -27,80 +29,198 @@
* @author Henri Yandell (bayard at apache dot org)
* @version $Revision$ $Date$
*/
-
-public class LockableFileWriterTest extends TestCase {
+public class LockableFileWriterTest extends FileBasedTestCase {
private File file;
private File lockDir;
private File lockFile;
+ private File altLockDir;
+ private File altLockFile;
public LockableFileWriterTest(String name) {
super(name);
}
public void setUp() {
- file = new File("testlockfile");
+ file = new File(getTestDirectory(), "testlockfile");
lockDir = new File(System.getProperty("java.io.tmpdir"));
lockFile = new File(lockDir, file.getName() + ".lck");
+ altLockDir = getTestDirectory();
+ altLockFile = new File(altLockDir, file.getName() + ".lck");
}
public void tearDown() {
file.delete();
lockFile.delete();
+ altLockFile.delete();
}
//-----------------------------------------------------------------------
public void testFileLocked() throws IOException {
- LockableFileWriter lfw = new LockableFileWriter(this.file);
+ LockableFileWriter lfw1 = null;
+ LockableFileWriter lfw2 = null;
+ LockableFileWriter lfw3 = null;
try {
- new LockableFileWriter(this.file);
- fail("Somehow able to open a locked file. ");
- } catch(IOException ioe) {
- String msg = ioe.getMessage();
- assertTrue( "Exception message does not start correctly. ",
- msg.startsWith("Can't write file, lock ") );
+ // open a valid locakable writer
+ lfw1 = new LockableFileWriter(file);
+ assertEquals(true, file.exists());
+ assertEquals(true, lockFile.exists());
+
+ // try to open a second writer
+ try {
+ lfw2 = new LockableFileWriter(file);
+ fail("Somehow able to open a locked file. ");
+ } catch(IOException ioe) {
+ String msg = ioe.getMessage();
+ assertTrue( "Exception message does not start correctly. ",
+ msg.startsWith("Can't write file, lock ") );
+ assertEquals(true, file.exists());
+ assertEquals(true, lockFile.exists());
+ }
+
+ // try to open a third writer
+ try {
+ lfw3 = new LockableFileWriter(file);
+ fail("Somehow able to open a locked file. ");
+ } catch(IOException ioe) {
+ String msg = ioe.getMessage();
+ assertTrue( "Exception message does not start correctly. ",
+ msg.startsWith("Can't write file, lock ") );
+ assertEquals(true, file.exists());
+ assertEquals(true, lockFile.exists());
+ }
+
} finally {
- lfw.close();
+ IOUtils.closeQuietly(lfw1);
+ IOUtils.closeQuietly(lfw2);
+ IOUtils.closeQuietly(lfw3);
}
+ assertEquals(true, file.exists());
assertEquals(false, lockFile.exists());
}
- public void testFileNotLocked() throws IOException {
- LockableFileWriter lfw = new LockableFileWriter(this.file);
- lfw.close();
+ //-----------------------------------------------------------------------
+ public void testAlternateLockDir() throws IOException {
+ LockableFileWriter lfw1 = null;
+ LockableFileWriter lfw2 = null;
try {
- LockableFileWriter lfw2 = new LockableFileWriter(this.file);
- lfw2.close();
- } catch(IOException ioe) {
- String msg = ioe.getMessage();
- if( msg.startsWith("Can't write file, lock ") ) {
- fail("Somehow unable to open a unlocked file. ");
+ // open a valid locakable writer
+ lfw1 = new LockableFileWriter(file, true,
altLockDir.getAbsolutePath());
+ assertEquals(true, file.exists());
+ assertEquals(true, altLockFile.exists());
+
+ // try to open a second writer
+ try {
+ lfw2 = new LockableFileWriter(file, true,
altLockDir.getAbsolutePath());
+ fail("Somehow able to open a locked file. ");
+ } catch(IOException ioe) {
+ String msg = ioe.getMessage();
+ assertTrue( "Exception message does not start correctly. ",
+ msg.startsWith("Can't write file, lock ") );
+ assertEquals(true, file.exists());
+ assertEquals(true, altLockFile.exists());
}
+
+ } finally {
+ IOUtils.closeQuietly(lfw1);
+ IOUtils.closeQuietly(lfw2);
+ }
+ assertEquals(true, file.exists());
+ assertEquals(false, altLockFile.exists());
+ }
+
+ //-----------------------------------------------------------------------
+ public void testFileNotLocked() throws IOException {
+ // open a valid locakable writer
+ LockableFileWriter lfw1 = null;
+ try {
+ lfw1 = new LockableFileWriter(file);
+ assertEquals(true, file.exists());
+ assertEquals(true, lockFile.exists());
+ } finally {
+ IOUtils.closeQuietly(lfw1);
+ }
+ assertEquals(true, file.exists());
+ assertEquals(false, lockFile.exists());
+
+ // open a second valid writer on the same file
+ LockableFileWriter lfw2 = null;
+ try {
+ lfw2 = new LockableFileWriter(file);
+ assertEquals(true, file.exists());
+ assertEquals(true, lockFile.exists());
+ } finally {
+ IOUtils.closeQuietly(lfw2);
}
+ assertEquals(true, file.exists());
assertEquals(false, lockFile.exists());
}
+ //-----------------------------------------------------------------------
public void testConstructor_File_encoding_badEncoding() throws IOException
{
+ Writer writer = null;
try {
- new LockableFileWriter(file, "BAD-ENCODE");
+ writer = new LockableFileWriter(file, "BAD-ENCODE");
fail();
- } catch (IOException ex) {}
+ } catch (IOException ex) {
+ // expected
+ assertEquals(false, file.exists());
+ assertEquals(false, lockFile.exists());
+ } finally {
+ IOUtils.closeQuietly(writer);
+ }
+ assertEquals(false, file.exists());
assertEquals(false, lockFile.exists());
}
+ //-----------------------------------------------------------------------
+ public void testConstructor_File_directory() throws IOException {
+ Writer writer = null;
+ try {
+ writer = new LockableFileWriter(getTestDirectory());
+ fail();
+ } catch (IOException ex) {
+ // expected
+ assertEquals(false, file.exists());
+ assertEquals(false, lockFile.exists());
+ } finally {
+ IOUtils.closeQuietly(writer);
+ }
+ assertEquals(false, file.exists());
+ assertEquals(false, lockFile.exists());
+ }
+
+ //-----------------------------------------------------------------------
public void testConstructor_File_nullFile() throws IOException {
+ Writer writer = null;
try {
- new LockableFileWriter((File) null);
+ writer = new LockableFileWriter((File) null);
fail();
- } catch (NullPointerException ex) {}
+ } catch (NullPointerException ex) {
+ // expected
+ assertEquals(false, file.exists());
+ assertEquals(false, lockFile.exists());
+ } finally {
+ IOUtils.closeQuietly(writer);
+ }
+ assertEquals(false, file.exists());
assertEquals(false, lockFile.exists());
}
+ //-----------------------------------------------------------------------
public void testConstructor_fileName_nullFile() throws IOException {
+ Writer writer = null;
try {
- new LockableFileWriter((String) null);
+ writer = new LockableFileWriter((String) null);
fail();
- } catch (NullPointerException ex) {}
+ } catch (NullPointerException ex) {
+ // expected
+ assertEquals(false, file.exists());
+ assertEquals(false, lockFile.exists());
+ } finally {
+ IOUtils.closeQuietly(writer);
+ }
+ assertEquals(false, file.exists());
assertEquals(false, lockFile.exists());
}
Modified: jakarta/commons/proper/io/trunk/xdocs/upgradeto1_2.xml
URL:
http://svn.apache.org/viewcvs/jakarta/commons/proper/io/trunk/xdocs/upgradeto1_2.xml?rev=385293&r1=385292&r2=385293&view=diff
==============================================================================
--- jakarta/commons/proper/io/trunk/xdocs/upgradeto1_2.xml (original)
+++ jakarta/commons/proper/io/trunk/xdocs/upgradeto1_2.xml Sun Mar 12 07:11:19
2006
@@ -55,6 +55,10 @@
- FileUtils.read*
Increase certainty that files are closed in case of error
+- LockableFileWriter
+ Locking mechanism was broken and only provided limited protection [38942]
+ File deletion and locking in case of constructor error was broken
+
Enhancements from 1.1
---------------------
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
