SonarCube fix - make members private

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1772291 13f79535-47bb-0310-9956-ffa450edef68

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<HSSFSheet>(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<HSLFPictureData>();
 
         // 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