From 7ef60aa130f733a8027627621d26d9de1c267f51 Mon Sep 17 00:00:00 2001 From: Andreas Beeker Date: Thu, 1 Dec 2016 23:46:27 +0000 Subject: [PATCH] SonarCube fix - make members private git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1772291 13f79535-47bb-0310-9956-ffa450edef68 --- src/java/org/apache/poi/POIDocument.java | 41 ++++++++++++++- .../org/apache/poi/POIOLE2TextExtractor.java | 2 +- .../poi/hpsf/HPSFPropertiesOnlyDocument.java | 4 +- .../poi/hssf/usermodel/HSSFWorkbook.java | 27 ++++++---- .../crypt/temp/AesZipFileZipEntrySource.java | 2 +- .../java/org/apache/poi/util/OOXMLLite.java | 2 +- .../poi/hslf/usermodel/HSLFSlideShowImpl.java | 26 ++++++---- .../src/org/apache/poi/hwpf/HWPFDocument.java | 10 ++-- .../apache/poi/TestPOIDocumentScratchpad.java | 52 ++++++++++++------- .../org/apache/poi/TestPOIDocumentMain.java | 26 ++++++---- 10 files changed, 129 insertions(+), 63 deletions(-) diff --git a/src/java/org/apache/poi/POIDocument.java b/src/java/org/apache/poi/POIDocument.java index d742c5572a..afd78a4678 100644 --- a/src/java/org/apache/poi/POIDocument.java +++ b/src/java/org/apache/poi/POIDocument.java @@ -53,7 +53,7 @@ public abstract class POIDocument implements Closeable { /** Holds further metadata on our document */ private DocumentSummaryInformation dsInf; /** The directory that our document lives in */ - protected DirectoryNode directory; + private DirectoryNode directory; /** For our own logging use */ private static final POILogger logger = POILogFactory.getLogger(POIDocument.class); @@ -396,7 +396,7 @@ public abstract class POIDocument implements Closeable { if (directory != null) { if (directory.getNFileSystem() != null) { directory.getNFileSystem().close(); - directory = null; + clearDirectory(); } } } @@ -405,4 +405,41 @@ public abstract class POIDocument implements Closeable { public DirectoryNode getDirectory() { return directory; } + + /** + * Clear/unlink the attached directory entry + */ + @Internal + protected void clearDirectory() { + directory = null; + } + + /** + * check if we were created by POIFS otherwise create a new dummy POIFS + * for storing the package data + * + * @return {@code true} if dummy directory was created, {@code false} otherwise + */ + @Internal + protected boolean initDirectory() { + if (directory == null) { + directory = new NPOIFSFileSystem().getRoot(); + return true; + } + return false; + } + + /** + * Replaces the attached directory, e.g. if this document is written + * to a new POIFSFileSystem + * + * @param newDirectory the new directory + * @return the old/previous directory + */ + @Internal + protected DirectoryNode replaceDirectory(DirectoryNode newDirectory) { + DirectoryNode dn = directory; + directory = newDirectory; + return dn; + } } diff --git a/src/java/org/apache/poi/POIOLE2TextExtractor.java b/src/java/org/apache/poi/POIOLE2TextExtractor.java index 24bbb9a65a..5884a9054e 100644 --- a/src/java/org/apache/poi/POIOLE2TextExtractor.java +++ b/src/java/org/apache/poi/POIOLE2TextExtractor.java @@ -98,6 +98,6 @@ public abstract class POIOLE2TextExtractor extends POITextExtractor { */ public DirectoryEntry getRoot() { - return document.directory; + return document.getDirectory(); } } diff --git a/src/java/org/apache/poi/hpsf/HPSFPropertiesOnlyDocument.java b/src/java/org/apache/poi/hpsf/HPSFPropertiesOnlyDocument.java index 79a91a4b68..1097b71e48 100644 --- a/src/java/org/apache/poi/hpsf/HPSFPropertiesOnlyDocument.java +++ b/src/java/org/apache/poi/hpsf/HPSFPropertiesOnlyDocument.java @@ -50,7 +50,7 @@ public class HPSFPropertiesOnlyDocument extends POIDocument { * Write out to the currently open file the properties changes, but nothing else */ public void write() throws IOException { - NPOIFSFileSystem fs = directory.getFileSystem(); + NPOIFSFileSystem fs = getDirectory().getFileSystem(); validateInPlaceWritePossible(); writeProperties(fs, null); @@ -89,7 +89,7 @@ public class HPSFPropertiesOnlyDocument extends POIDocument { writeProperties(fs, excepts); // Copy over everything else unchanged - FilteringDirectoryNode src = new FilteringDirectoryNode(directory, excepts); + FilteringDirectoryNode src = new FilteringDirectoryNode(getDirectory(), excepts); FilteringDirectoryNode dest = new FilteringDirectoryNode(fs.getRoot(), excepts); EntryUtils.copyNodes(src, dest); diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java b/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java index a09756c8b2..7c50b6643d 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java @@ -110,6 +110,7 @@ import org.apache.poi.util.LittleEndianByteArrayInputStream; import org.apache.poi.util.LittleEndianByteArrayOutputStream; import org.apache.poi.util.POILogFactory; import org.apache.poi.util.POILogger; +import org.apache.poi.util.Removal; /** * High level representation of a workbook. This is the first object most users @@ -334,7 +335,7 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss // If we're not preserving nodes, don't track the // POIFS any more if(! preserveNodes) { - this.directory = null; + clearDirectory(); } _sheets = new ArrayList(INITIAL_CAPACITY); @@ -1349,10 +1350,11 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss @Override public void write() throws IOException { validateInPlaceWritePossible(); + final DirectoryNode dir = getDirectory(); // Update the Workbook stream in the file - DocumentNode workbookNode = (DocumentNode)directory.getEntry( - getWorkbookDirEntryName(directory)); + DocumentNode workbookNode = (DocumentNode)dir.getEntry( + getWorkbookDirEntryName(dir)); NPOIFSDocument workbookDoc = new NPOIFSDocument(workbookNode); workbookDoc.replaceContents(new ByteArrayInputStream(getBytes())); @@ -1360,7 +1362,7 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss writeProperties(); // Sync with the File on disk - directory.getFileSystem().writeFilesystem(); + dir.getFileSystem().writeFilesystem(); } /** @@ -1435,13 +1437,13 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss // Copy over all the other nodes to our new poifs EntryUtils.copyNodes( - new FilteringDirectoryNode(this.directory, excepts) + new FilteringDirectoryNode(getDirectory(), excepts) , new FilteringDirectoryNode(fs.getRoot(), excepts) ); // YK: preserve StorageClsid, it is important for embedded workbooks, // see Bugzilla 47920 - fs.getRoot().setStorageClsid(this.directory.getStorageClsid()); + fs.getRoot().setStorageClsid(getDirectory().getStorageClsid()); } } @@ -2054,8 +2056,7 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss public int addOlePackage(byte[] oleData, String label, String fileName, String command) throws IOException { // check if we were created by POIFS otherwise create a new dummy POIFS for storing the package data - if (directory == null) { - directory = new NPOIFSFileSystem().getRoot(); + if (initDirectory()) { preserveNodes = true; } @@ -2064,8 +2065,8 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss DirectoryEntry oleDir = null; do { String storageStr = "MBD"+ HexDump.toHex(++storageId); - if (!directory.hasEntry(storageStr)) { - oleDir = directory.createDirectory(storageStr); + if (!getDirectory().hasEntry(storageStr)) { + oleDir = getDirectory().createDirectory(storageStr); oleDir.setStorageClsid(ClassID.OLE10_PACKAGE); } } while (oleDir == null); @@ -2242,8 +2243,12 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss return workbook.changeExternalReference(oldUrl, newUrl); } + /** + * @deprecated POI 3.16 beta 1. use {@link POIDocument#getDirectory()} instead + */ + @Removal(version="3.18") public DirectoryNode getRootDirectory(){ - return directory; + return getDirectory(); } @Internal diff --git a/src/ooxml/java/org/apache/poi/poifs/crypt/temp/AesZipFileZipEntrySource.java b/src/ooxml/java/org/apache/poi/poifs/crypt/temp/AesZipFileZipEntrySource.java index 5e1f4b383d..e9c47b418f 100644 --- a/src/ooxml/java/org/apache/poi/poifs/crypt/temp/AesZipFileZipEntrySource.java +++ b/src/ooxml/java/org/apache/poi/poifs/crypt/temp/AesZipFileZipEntrySource.java @@ -54,7 +54,7 @@ import org.apache.poi.util.TempFile; */ @Beta public class AesZipFileZipEntrySource implements ZipEntrySource { - private static POILogger LOG = POILogFactory.getLogger(AesZipFileZipEntrySource.class); + private static final POILogger LOG = POILogFactory.getLogger(AesZipFileZipEntrySource.class); private final File tmpFile; private final ZipFile zipFile; diff --git a/src/ooxml/java/org/apache/poi/util/OOXMLLite.java b/src/ooxml/java/org/apache/poi/util/OOXMLLite.java index 0b2c5d92a4..d537a9d3a6 100644 --- a/src/ooxml/java/org/apache/poi/util/OOXMLLite.java +++ b/src/ooxml/java/org/apache/poi/util/OOXMLLite.java @@ -233,7 +233,7 @@ public final class OOXMLLite { || checkForTestAnnotation(testclass)) { out.add(testclass); } - } catch (Throwable e) { + } catch (Throwable e) { // NOSONAR System.out.println("Class " + cls + " is not in classpath"); return; } diff --git a/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSlideShowImpl.java b/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSlideShowImpl.java index 493d461746..403db98ede 100644 --- a/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSlideShowImpl.java +++ b/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSlideShowImpl.java @@ -58,6 +58,7 @@ import org.apache.poi.util.IOUtils; import org.apache.poi.util.LittleEndian; import org.apache.poi.util.POILogFactory; import org.apache.poi.util.POILogger; +import org.apache.poi.util.Removal; /** * This class contains the main functionality for the Powerpoint file @@ -87,9 +88,12 @@ public final class HSLFSlideShowImpl extends POIDocument implements Closeable { /** * Returns the directory in the underlying POIFSFileSystem for the * document that is open. + * + * @deprecated POI 3.16 beta 1. use {@link POIDocument#getDirectory()} instead */ + @Removal(version="3.18") protected DirectoryNode getPOIFSDirectory() { - return directory; + return getDirectory(); } /** @@ -201,11 +205,11 @@ public final class HSLFSlideShowImpl extends POIDocument implements Closeable { private void readPowerPointStream() throws IOException { // Get the main document stream DocumentEntry docProps = - (DocumentEntry) directory.getEntry("PowerPoint Document"); + (DocumentEntry) getDirectory().getEntry("PowerPoint Document"); // Grab the document stream int len = docProps.getSize(); - InputStream is = directory.createDocumentInputStream("PowerPoint Document"); + InputStream is = getDirectory().createDocumentInputStream("PowerPoint Document"); try { _docstream = IOUtils.toByteArray(is, len); } finally { @@ -334,7 +338,7 @@ public final class HSLFSlideShowImpl extends POIDocument implements Closeable { */ private void readCurrentUserStream() { try { - currentUser = new CurrentUserAtom(directory); + currentUser = new CurrentUserAtom(getDirectory()); } catch (IOException ie) { logger.log(POILogger.ERROR, "Error finding Current User Atom:\n" + ie); currentUser = new CurrentUserAtom(); @@ -356,12 +360,12 @@ public final class HSLFSlideShowImpl extends POIDocument implements Closeable { _pictures = new ArrayList(); // if the presentation doesn't contain pictures - will use a null set instead - if (!directory.hasEntry("Pictures")) return; + if (!getDirectory().hasEntry("Pictures")) return; HSLFSlideShowEncrypted decryptData = new HSLFSlideShowEncrypted(getDocumentEncryptionAtom()); - DocumentEntry entry = (DocumentEntry) directory.getEntry("Pictures"); - DocumentInputStream is = directory.createDocumentInputStream(entry); + DocumentEntry entry = (DocumentEntry) getDirectory().getEntry("Pictures"); + DocumentInputStream is = getDirectory().createDocumentInputStream(entry); byte[] pictstream = IOUtils.toByteArray(is, entry.getSize()); is.close(); @@ -562,10 +566,10 @@ public final class HSLFSlideShowImpl extends POIDocument implements Closeable { // Write the PowerPoint streams to the current FileSystem // No need to do anything to other streams, already there! - write(directory.getFileSystem(), false); + write(getDirectory().getFileSystem(), false); // Sync with the File on disk - directory.getFileSystem().writeFilesystem(); + getDirectory().getFileSystem().writeFilesystem(); } /** @@ -707,7 +711,7 @@ public final class HSLFSlideShowImpl extends POIDocument implements Closeable { // If requested, copy over any other streams we spot, eg Macros if (copyAllOtherNodes) { - EntryUtils.copyNodes(directory.getFileSystem(), outFS, writtenEntries); + EntryUtils.copyNodes(getDirectory().getFileSystem(), outFS, writtenEntries); } } @@ -865,7 +869,7 @@ public final class HSLFSlideShowImpl extends POIDocument implements Closeable { @Override public void close() throws IOException { - NPOIFSFileSystem fs = directory.getFileSystem(); + NPOIFSFileSystem fs = getDirectory().getFileSystem(); if (fs != null) { fs.close(); } diff --git a/src/scratchpad/src/org/apache/poi/hwpf/HWPFDocument.java b/src/scratchpad/src/org/apache/poi/hwpf/HWPFDocument.java index ce9d58f038..7ec8fb6efc 100644 --- a/src/scratchpad/src/org/apache/poi/hwpf/HWPFDocument.java +++ b/src/scratchpad/src/org/apache/poi/hwpf/HWPFDocument.java @@ -567,10 +567,10 @@ public final class HWPFDocument extends HWPFDocumentCore { validateInPlaceWritePossible(); // Update the Document+Properties streams in the file - write(directory.getFileSystem(), false); + write(getDirectory().getFileSystem(), false); // Sync with the File on disk - directory.getFileSystem().writeFilesystem(); + getDirectory().getFileSystem().writeFilesystem(); } /** @@ -911,7 +911,7 @@ public final class HWPFDocument extends HWPFDocumentCore { boolean objectPoolWritten = false; boolean tableWritten = false; boolean propertiesWritten = false; - for (Entry entry : directory) { + for (Entry entry : getDirectory()) { if ( entry.getName().equals( STREAM_WORD_DOCUMENT ) ) { if ( !docWritten ) @@ -977,13 +977,11 @@ public final class HWPFDocument extends HWPFDocumentCore { if ( !objectPoolWritten && copyOtherEntries ) _objectPool.writeTo( pfs.getRoot() ); - this.directory = pfs.getRoot(); - /* * since we updated all references in FIB and etc, using new arrays to * access data */ - this.directory = pfs.getRoot(); + replaceDirectory(pfs.getRoot()); this._tableStream = tableStream.toByteArray(); this._dataStream = dataBuf; } diff --git a/src/scratchpad/testcases/org/apache/poi/TestPOIDocumentScratchpad.java b/src/scratchpad/testcases/org/apache/poi/TestPOIDocumentScratchpad.java index d19843a6f9..152fd0405c 100644 --- a/src/scratchpad/testcases/org/apache/poi/TestPOIDocumentScratchpad.java +++ b/src/scratchpad/testcases/org/apache/poi/TestPOIDocumentScratchpad.java @@ -21,13 +21,20 @@ package org.apache.poi; -import junit.framework.TestCase; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; -import java.io.*; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import org.apache.poi.hpsf.HPSFPropertiesOnlyDocument; import org.apache.poi.hslf.usermodel.HSLFSlideShowImpl; import org.apache.poi.hwpf.HWPFTestDataSamples; -import org.apache.poi.poifs.filesystem.*; +import org.apache.poi.poifs.filesystem.NPOIFSFileSystem; +import org.apache.poi.poifs.filesystem.POIFSFileSystem; +import org.junit.Before; +import org.junit.Test; /** * Tests that POIDocument correctly loads and saves the common @@ -35,10 +42,8 @@ import org.apache.poi.poifs.filesystem.*; * * This is part 2 of 2 of the tests - it only does the POIDocuments * which are part of the scratchpad (not main) - * - * @author Nick Burch (nick at torchbox dot com) */ -public final class TestPOIDocumentScratchpad extends TestCase { +public final class TestPOIDocumentScratchpad { // The POI Documents to work on private POIDocument doc; private POIDocument doc2; @@ -47,23 +52,28 @@ public final class TestPOIDocumentScratchpad extends TestCase { * Set things up, using a PowerPoint document and * a Word Document for our testing */ - @Override - public void setUp() throws Exception { + @Before + public void setUp() throws IOException { doc = new HSLFSlideShowImpl(POIDataSamples.getSlideShowInstance().openResourceAsStream("basic_test_ppt_file.ppt")); - doc2 = HWPFTestDataSamples.openSampleFile("test2.doc"); } + @Test public void testReadProperties() { + testReadPropertiesHelper(doc); + } + + private void testReadPropertiesHelper(POIDocument docPH) { // We should have both sets - assertNotNull(doc.getDocumentSummaryInformation()); - assertNotNull(doc.getSummaryInformation()); + assertNotNull(docPH.getDocumentSummaryInformation()); + assertNotNull(docPH.getSummaryInformation()); // Check they are as expected for the test doc - assertEquals("Hogwarts", doc.getSummaryInformation().getAuthor()); - assertEquals(10598, doc.getDocumentSummaryInformation().getByteCount()); + assertEquals("Hogwarts", docPH.getSummaryInformation().getAuthor()); + assertEquals(10598, docPH.getDocumentSummaryInformation().getByteCount()); } + @Test public void testReadProperties2() { // Check again on the word one assertNotNull(doc2.getDocumentSummaryInformation()); @@ -74,7 +84,8 @@ public final class TestPOIDocumentScratchpad extends TestCase { assertEquals(0, doc2.getDocumentSummaryInformation().getByteCount()); } - public void testWriteProperties() throws Exception { + @Test + public void testWriteProperties() throws IOException { // Just check we can write them back out into a filesystem NPOIFSFileSystem outFS = new NPOIFSFileSystem(); doc.writeProperties(outFS); @@ -82,9 +93,11 @@ public final class TestPOIDocumentScratchpad extends TestCase { // Should now hold them assertNotNull(outFS.createDocumentInputStream("\005SummaryInformation")); assertNotNull(outFS.createDocumentInputStream("\005DocumentSummaryInformation")); + outFS.close(); } - public void testWriteReadProperties() throws Exception { + @Test + public void testWriteReadProperties() throws IOException { ByteArrayOutputStream baos = new ByteArrayOutputStream(); // Write them out @@ -97,10 +110,13 @@ public final class TestPOIDocumentScratchpad extends TestCase { POIFSFileSystem inFS = new POIFSFileSystem(bais); // Check they're still there - doc.directory = inFS.getRoot(); - doc.readProperties(); + POIDocument ppt = new HPSFPropertiesOnlyDocument(inFS); + ppt.readProperties(); // Delegate test - testReadProperties(); + testReadPropertiesHelper(ppt); + + ppt.close(); + inFS.close(); } } diff --git a/src/testcases/org/apache/poi/TestPOIDocumentMain.java b/src/testcases/org/apache/poi/TestPOIDocumentMain.java index 7483d59960..3c06fd346b 100644 --- a/src/testcases/org/apache/poi/TestPOIDocumentMain.java +++ b/src/testcases/org/apache/poi/TestPOIDocumentMain.java @@ -25,6 +25,7 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; +import org.apache.poi.hpsf.HPSFPropertiesOnlyDocument; import org.apache.poi.hssf.HSSFTestDataSamples; import org.apache.poi.hssf.usermodel.HSSFWorkbook; import org.apache.poi.poifs.filesystem.NPOIFSFileSystem; @@ -55,13 +56,17 @@ public final class TestPOIDocumentMain { @Test public void readProperties() { + readPropertiesHelper(doc); + } + + private void readPropertiesHelper(POIDocument docWB) { // We should have both sets - assertNotNull(doc.getDocumentSummaryInformation()); - assertNotNull(doc.getSummaryInformation()); + assertNotNull(docWB.getDocumentSummaryInformation()); + assertNotNull(docWB.getSummaryInformation()); // Check they are as expected for the test doc - assertEquals("Administrator", doc.getSummaryInformation().getAuthor()); - assertEquals(0, doc.getDocumentSummaryInformation().getByteCount()); + assertEquals("Administrator", docWB.getSummaryInformation().getAuthor()); + assertEquals(0, docWB.getDocumentSummaryInformation().getByteCount()); } @Test @@ -76,7 +81,7 @@ public final class TestPOIDocumentMain { } @Test - public void writeProperties() throws Exception { + public void writeProperties() throws IOException { // Just check we can write them back out into a filesystem NPOIFSFileSystem outFS = new NPOIFSFileSystem(); doc.readProperties(); @@ -92,7 +97,7 @@ public final class TestPOIDocumentMain { } @Test - public void WriteReadProperties() throws Exception { + public void WriteReadProperties() throws IOException { ByteArrayOutputStream baos = new ByteArrayOutputStream(); // Write them out @@ -106,11 +111,12 @@ public final class TestPOIDocumentMain { OPOIFSFileSystem inFS = new OPOIFSFileSystem(bais); // Check they're still there - doc.directory = inFS.getRoot(); - doc.readProperties(); - + POIDocument doc3 = new HPSFPropertiesOnlyDocument(inFS); + doc3.readProperties(); + // Delegate test - readProperties(); + readPropertiesHelper(doc3); + doc3.close(); } @Test -- 2.39.5