From: Nick Burch Date: Wed, 4 Aug 2010 15:58:51 +0000 (+0000) Subject: Improve handling and warnings when closing OPCPackage objects X-Git-Tag: REL_3_7_BETA2~4 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=ddbddfafa41df397f0a2c670549b2898361312e0;p=poi.git Improve handling and warnings when closing OPCPackage objects git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@982311 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index d5e39d3918..040594cecf 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + Improve handling and warnings when closing OPCPackage objects 49702 - Correct XSSFWorkbook.getNumCellStyles to check the right styles list 49690 - Add WorkbookUtil, which provies a way of generating valid sheet names 49694 - Use DataFormatter when autosizing columns, to better match the real display width of formatted cells diff --git a/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java b/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java index fe2c3e2799..8be4fa9f14 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java @@ -338,15 +338,19 @@ public abstract class OPCPackage implements RelationshipSource { } /** - * Close the package and save its content. + * Close the open, writable package and save its content. + * + * If your package is open read only, then you should call {@link #revert()} + * when finished with the package. * * @throws IOException * If an IO exception occur during the saving process. */ public void close() throws IOException { if (this.packageAccess == PackageAccess.READ) { - logger - .log(POILogger.WARN, "The close() method is intended to SAVE a package. This package is open in READ ONLY mode, use the revert() method instead !"); + logger.log(POILogger.WARN, + "The close() method is intended to SAVE a package. This package is open in READ ONLY mode, use the revert() method instead !"); + revert(); return; } @@ -1298,6 +1302,17 @@ public abstract class OPCPackage implements RelationshipSource { throw new IllegalArgumentException("targetFile"); this.throwExceptionIfReadOnly(); + + // You shouldn't save the the same file, do a close instead + if(targetFile.exists() && + targetFile.getAbsolutePath().equals(this.originalPackagePath)) { + throw new InvalidOperationException( + "You can't call save(File) to save to the currently open " + + "file. To save to the current file, please just call close()" + ); + } + + // Do the save FileOutputStream fos = null; try { fos = new FileOutputStream(targetFile); diff --git a/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackage.java b/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackage.java index 114c75c06e..52a871411e 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackage.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackage.java @@ -295,8 +295,11 @@ public final class ZipPackage extends Package { // Save the final package to a temporary file try { save(tempFile); - this.zipArchive.close(); // Close the zip archive to be - // able to delete it + + // Close the current zip file, so we can + // overwrite it on all platforms + this.zipArchive.close(); + // Copy the new file over the old one FileHelper.copyFile(tempFile, targetFile); } finally { // Either the save operation succeed or not, we delete the @@ -312,7 +315,7 @@ public final class ZipPackage extends Package { throw new InvalidOperationException( "Can't close a package not previously open with the open() method !"); } - } + } } /** diff --git a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java index 9bb66c0a41..4224317f43 100644 --- a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java +++ b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java @@ -32,6 +32,7 @@ import junit.framework.TestCase; import org.apache.poi.openxml4j.OpenXML4JTestDataSamples; import org.apache.poi.openxml4j.exceptions.InvalidFormatException; +import org.apache.poi.openxml4j.exceptions.InvalidOperationException; import org.apache.poi.openxml4j.opc.internal.ContentTypeManager; import org.apache.poi.openxml4j.opc.internal.FileHelper; import org.apache.poi.util.TempFile; @@ -443,6 +444,64 @@ public final class TestPackage extends TestCase { // Don't save modifications p.revert(); } + + /** + * Test that we can open a file by path, and then + * write changes to it. + */ + public void testOpenFileThenOverwrite() throws Exception { + File tempFile = File.createTempFile("poiTesting","tmp"); + File origFile = OpenXML4JTestDataSamples.getSampleFile("TestPackageCommon.docx"); + FileHelper.copyFile(origFile, tempFile); + + // Open the temp file + OPCPackage p = OPCPackage.open(tempFile.toString(), PackageAccess.READ_WRITE); + // Close it + p.close(); + // Delete it + assertTrue(tempFile.delete()); + + // Reset + FileHelper.copyFile(origFile, tempFile); + p = OPCPackage.open(tempFile.toString(), PackageAccess.READ_WRITE); + + // Save it to the same file - not allowed + try { + p.save(tempFile); + fail("You shouldn't be able to call save(File) to overwrite the current file"); + } catch(InvalidOperationException e) {} + + // Delete it + assertTrue(tempFile.delete()); + + + // Open it read only, then close and delete - allowed + FileHelper.copyFile(origFile, tempFile); + p = OPCPackage.open(tempFile.toString(), PackageAccess.READ); + p.close(); + assertTrue(tempFile.delete()); + } + /** + * Test that we can open a file by path, save it + * to another file, then delete both + */ + public void testOpenFileThenSaveDelete() throws Exception { + File tempFile = File.createTempFile("poiTesting","tmp"); + File tempFile2 = File.createTempFile("poiTesting","tmp"); + File origFile = OpenXML4JTestDataSamples.getSampleFile("TestPackageCommon.docx"); + FileHelper.copyFile(origFile, tempFile); + + // Open the temp file + OPCPackage p = OPCPackage.open(tempFile.toString(), PackageAccess.READ_WRITE); + + // Save it to a different file + p.save(tempFile2); + p.close(); + + // Delete both the files + assertTrue(tempFile.delete()); + assertTrue(tempFile2.delete()); + } private static ContentTypeManager getContentTypeManager(OPCPackage pkg) throws Exception { Field f = OPCPackage.class.getDeclaredField("contentTypeManager");