Sonar fixes

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

3 files changed, 78 insertions, 109 deletions

diff --git a/src/ooxml/testcases/org/apache/poi/ooxml/TestDetectAsOOXML.java b/src/ooxml/testcases/org/apache/poi/ooxml/TestDetectAsOOXML.java
index 32bd2df2f9..3aebdd1bce 100644
--- a/src/ooxml/testcases/org/apache/poi/ooxml/TestDetectAsOOXML.java
+++ b/src/ooxml/testcases/org/apache/poi/ooxml/TestDetectAsOOXML.java
@@ -16,21 +16,24 @@
 ==================================================================== */
 package org.apache.poi.ooxml;
 
+import static org.apache.poi.hssf.HSSFTestDataSamples.openSampleFileStream;
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 
-import org.apache.poi.hssf.HSSFTestDataSamples;
 import org.apache.poi.openxml4j.exceptions.InvalidFormatException;
 import org.apache.poi.openxml4j.opc.OPCPackage;
 import org.apache.poi.poifs.filesystem.DocumentFactoryHelper;
 import org.apache.poi.poifs.filesystem.FileMagic;
 import org.apache.poi.util.IOUtils;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
 
 /**
  * Class to test that HXF correctly detects OOXML
@@ -40,27 +43,23 @@ import org.junit.jupiter.api.Test;
 public class TestDetectAsOOXML {
     @Test
 	public void testOpensProperly() throws IOException, InvalidFormatException {
-		OPCPackage.open(HSSFTestDataSamples.openSampleFileStream("sample.xlsx")).close();
+    	try (InputStream is = openSampleFileStream("sample.xlsx");
+			 OPCPackage pkg = OPCPackage.open(is)) {
+    		assertNotNull(pkg);
+		}
 	}
 
-    @Test
-	public void testDetectAsPOIFS() throws IOException {
-        Object[][] fileAndMagic = {
-                {"SampleSS.xlsx", FileMagic.OOXML},
-                {"SampleSS.xls", FileMagic.OLE2},
-                {"SampleSS.txt", FileMagic.UNKNOWN}
-        };
-
-	    for (Object[] fm : fileAndMagic) {
-	        try (InputStream is = FileMagic.prepareToCheckMagic(HSSFTestDataSamples.openSampleFileStream((String)fm[0]))) {
-				FileMagic act = FileMagic.valueOf(is);
+    @ParameterizedTest
+	@CsvSource({"SampleSS.xlsx, OOXML", "SampleSS.xls, OLE2", "SampleSS.txt, UNKNOWN"})
+	public void testDetectAsPOIFS(String file, FileMagic fm) throws IOException {
+		try (InputStream is = FileMagic.prepareToCheckMagic(openSampleFileStream(file))) {
+			FileMagic act = FileMagic.valueOf(is);
 
-				assertEquals(act == FileMagic.OOXML, DocumentFactoryHelper.hasOOXMLHeader(is),
-					"OOXML files should be detected, others not");
+			assertEquals(act == FileMagic.OOXML, DocumentFactoryHelper.hasOOXMLHeader(is),
+				"OOXML files should be detected, others not");
 
-				assertEquals(fm[1], act, "file magic failed for " + fm[0]);
-			}
-	    }
+			assertEquals(fm, act, "file magic failed for " + file);
+		}
 	}
 
     @Test
diff --git a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestContentType.java b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestContentType.java
index 6a725b3d47..3831e51ad9 100644
--- a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestContentType.java
+++ b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestContentType.java
@@ -17,6 +17,7 @@
 
 package org.apache.poi.openxml4j.opc;
 
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -31,6 +32,8 @@ import org.apache.poi.openxml4j.OpenXML4JTestDataSamples;
 import org.apache.poi.openxml4j.exceptions.InvalidFormatException;
 import org.apache.poi.openxml4j.opc.internal.ContentType;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 
 /**
  * Tests for content type (ContentType class).
@@ -46,14 +49,10 @@ public final class TestContentType {
      * package parts shall fit the definition and syntax for media types as
      * specified in RFC 2616, \u00A73.7.
      */
-    @Test
-    public void testContentTypeValidation() throws InvalidFormatException {
-        String[] contentTypesToTest = new String[]{"text/xml",
-                "application/pgp-key", "application/vnd.hp-PCLXL",
-                "application/vnd.lotus-1-2-3"};
-        for (String contentType : contentTypesToTest) {
-            new ContentType(contentType);
-        }
+    @ParameterizedTest
+    @ValueSource(strings = {"text/xml", "application/pgp-key", "application/vnd.hp-PCLXL", "application/vnd.lotus-1-2-3"})
+    public void testContentTypeValidation(String contentType) throws InvalidFormatException {
+        assertDoesNotThrow(() -> new ContentType(contentType));
     }
 
     /**
@@ -71,19 +70,17 @@ public final class TestContentType {
      * designers shall specify only such content types for inclusion in the
      * format.
      */
-    @Test
-    public void testContentTypeValidationFailure() {
-        String[] contentTypesToTest = new String[]{"text/xml/app", "",
-                "test", "text(xml/xml", "text)xml/xml", "text<xml/xml",
-                "text>/xml", "text@/xml", "text,/xml", "text;/xml",
-                "text:/xml", "text\\/xml", "t/ext/xml", "t\"ext/xml",
-                "text[/xml", "text]/xml", "text?/xml", "tex=t/xml",
-                "te{xt/xml", "tex}t/xml", "te xt/xml",
-                "text" + (char) 9 + "/xml", "text xml", " text/xml "};
-        for (String contentType : contentTypesToTest) {
-            assertThrows(InvalidFormatException.class, () -> new ContentType(contentType),
-                "Must have fail for content type: '" + contentType + "' !");
-        }
+    @ParameterizedTest
+    @ValueSource(strings = {"text/xml/app", "",
+        "test", "text(xml/xml", "text)xml/xml", "text<xml/xml",
+        "text>/xml", "text@/xml", "text,/xml", "text;/xml",
+        "text:/xml", "text\\/xml", "t/ext/xml", "t\"ext/xml",
+        "text[/xml", "text]/xml", "text?/xml", "tex=t/xml",
+        "te{xt/xml", "tex}t/xml", "te xt/xml",
+        "text\u0009/xml", "text xml", " text/xml "})
+    public void testContentTypeValidationFailure(String contentType) {
+        assertThrows(InvalidFormatException.class, () -> new ContentType(contentType),
+            "Must have fail for content type: '" + contentType + "' !");
     }
 
     /**
@@ -92,33 +89,28 @@ public final class TestContentType {
      * Invalid parameters are verified as incorrect in
      * {@link #testContentTypeParameterFailure()}
      */
-    @Test
-    public void testContentTypeParam() throws InvalidFormatException {
-        String[] contentTypesToTest = new String[]{"mail/toto;titi=tata",
-                "text/xml;a=b;c=d", "text/xml;key1=param1;key2=param2",
-                "application/pgp-key;version=\"2\"",
-                "application/x-resqml+xml;version=2.0;type=obj_global2dCrs"
-        };
-        for (String contentType : contentTypesToTest) {
-            new ContentType(contentType);
-        }
+    @ParameterizedTest
+    @ValueSource(strings = {"mail/toto;titi=tata",
+        "text/xml;a=b;c=d", "text/xml;key1=param1;key2=param2",
+        "application/pgp-key;version=\"2\"",
+        "application/x-resqml+xml;version=2.0;type=obj_global2dCrs"})
+    public void testContentTypeParam(String contentType) {
+        assertDoesNotThrow(() -> new ContentType(contentType));
     }
 
     /**
      * Check rule [O1.2]: Format designers might restrict the usage of
      * parameters for content types.
      */
-    @Test
-    public void testContentTypeParameterFailure() {
-        String[] contentTypesToTest = new String[]{
-                "mail/toto;\"titi=tata\"", // quotes not allowed like that
-                "mail/toto;titi = tata", // spaces not allowed
-                "text/\u0080" // characters above ASCII are not allowed
-        };
-        for (String contentType : contentTypesToTest) {
-            assertThrows(InvalidFormatException.class, () -> new ContentType(contentType),
-                "Must have fail for content type: '" + contentType + "' !");
-        }
+    @ParameterizedTest
+    @ValueSource(strings = {
+        "mail/toto;\"titi=tata\"", // quotes not allowed like that
+        "mail/toto;titi = tata", // spaces not allowed
+        "text/\u0080" // characters above ASCII are not allowed
+    })
+    public void testContentTypeParameterFailure(String contentType) {
+        assertThrows(InvalidFormatException.class, () -> new ContentType(contentType),
+            "Must have fail for content type: '" + contentType + "' !");
     }
 
     /**
@@ -126,13 +118,11 @@ public final class TestContentType {
      * that does not include comments and the format designer shall specify such
      * a content type.
      */
-    @Test
-    public void testContentTypeCommentFailure() {
-        String[] contentTypesToTest = new String[]{"text/xml(comment)"};
-        for (String contentType : contentTypesToTest) {
-            assertThrows(InvalidFormatException.class, () -> new ContentType(contentType),
-                "Must have fail for content type: '" + contentType + "' !");
-        }
+    @ParameterizedTest
+    @ValueSource(strings = {"text/xml(comment)"})
+    public void testContentTypeCommentFailure(String contentType) {
+        assertThrows(InvalidFormatException.class, () -> new ContentType(contentType),
+            "Must have fail for content type: '" + contentType + "' !");
     }
 
     /**
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 aaff77ecbb..f467b4fa07 100644
--- a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java
+++ b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java
@@ -61,7 +61,6 @@ import org.apache.commons.compress.archivers.zip.ZipFile;
 import org.apache.poi.EncryptedDocumentException;
 import org.apache.poi.POIDataSamples;
 import org.apache.poi.POITestCase;
-import org.apache.poi.UnsupportedFileFormatException;
 import org.apache.poi.extractor.ExtractorFactory;
 import org.apache.poi.extractor.POITextExtractor;
 import org.apache.poi.ooxml.POIXMLException;
@@ -69,8 +68,6 @@ import org.apache.poi.ooxml.util.DocumentHelper;
 import org.apache.poi.openxml4j.exceptions.InvalidFormatException;
 import org.apache.poi.openxml4j.exceptions.InvalidOperationException;
 import org.apache.poi.openxml4j.exceptions.NotOfficeXmlFileException;
-import org.apache.poi.openxml4j.exceptions.ODFNotOfficeXmlFileException;
-import org.apache.poi.openxml4j.exceptions.OLE2NotOfficeXmlFileException;
 import org.apache.poi.openxml4j.opc.internal.ContentTypeManager;
 import org.apache.poi.openxml4j.opc.internal.FileHelper;
 import org.apache.poi.openxml4j.opc.internal.PackagePropertiesPart;
@@ -92,6 +89,8 @@ import org.apache.poi.xwpf.usermodel.XWPFRelation;
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.function.Executable;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
 import org.w3c.dom.NodeList;
@@ -631,47 +630,29 @@ public final class TestPackage {
 		}
     }
 
-	@Test
-	public void NonOOXML_File() throws Exception {
-    	handleNonOOXML(
-			"SampleSS.xls", OLE2NotOfficeXmlFileException.class,
-			"The supplied data appears to be in the OLE2 Format",
-			"You are calling the part of POI that deals with OOXML"
-		);
-
-    	handleNonOOXML(
-			"SampleSS.xml", NotOfficeXmlFileException.class,
-			"The supplied data appears to be a raw XML file",
-			"Formats such as Office 2003 XML"
-		);
-
-    	handleNonOOXML(
-			"SampleSS.ods", ODFNotOfficeXmlFileException.class,
-			"The supplied data appears to be in ODF",
-			"Formats like these (eg ODS"
-		);
+	@SuppressWarnings("unchecked")
+	@ParameterizedTest
+	@CsvSource({
+		"SampleSS.xls, org.apache.poi.openxml4j.exceptions.OLE2NotOfficeXmlFileException, The supplied data appears to be in the OLE2 Format, You are calling the part of POI that deals with OOXML",
+		"SampleSS.xml, org.apache.poi.openxml4j.exceptions.NotOfficeXmlFileException, The supplied data appears to be a raw XML file, Formats such as Office 2003 XML",
+		"SampleSS.ods, org.apache.poi.openxml4j.exceptions.ODFNotOfficeXmlFileException, The supplied data appears to be in ODF, Formats like these (eg ODS",
+		"SampleSS.txt, org.apache.poi.openxml4j.exceptions.NotOfficeXmlFileException, No valid entries or contents found, not a valid OOXML"
+	})
+	public void NonOOXML_File(String file, String exClazzStr, String msg1, String msg2) throws Exception {
+    	Class<? extends Exception> exClazz = (Class<? extends Exception>)Class.forName(exClazzStr);
 
-    	handleNonOOXML(
-			"SampleSS.txt", NotOfficeXmlFileException.class,
-			"No valid entries or contents found",
-			"not a valid OOXML"
-		);
-	}
-
-	private void handleNonOOXML(String file, Class<? extends UnsupportedFileFormatException> exception, String... messageParts) throws IOException {
 		try (InputStream stream = xlsSamples.openResourceAsStream(file)) {
 			Executable[] trs = {
 				() -> OPCPackage.open(stream),
 				() -> OPCPackage.open(xlsSamples.getFile(file))
 			};
 			for (Executable tr : trs) {
-				Exception ex = assertThrows(exception, tr, "Shouldn't be able to open "+file);
-				Stream.of(messageParts).forEach(mp -> assertTrue(ex.getMessage().contains(mp)));
+				Exception ex = assertThrows(exClazz, tr, "Shouldn't be able to open "+file);
+				Stream.of(msg1, msg2).forEach(mp -> assertTrue(ex.getMessage().contains(mp)));
 			}
 		}
 	}
 
-
 	/**
 	 * Zip bomb handling test
 	 *
@@ -1009,8 +990,8 @@ public final class TestPackage {
 		//make absolutely certain that sequential calls don't throw InvalidFormatExceptions
 		String originalFile = getSampleFileName("TestPackageCommon.docx");
 		try (OPCPackage p2 = OPCPackage.open(originalFile, PackageAccess.READ)) {
-			p2.getParts();
-			p2.getParts();
+			assertDoesNotThrow(p2::getParts);
+			assertDoesNotThrow(p2::getParts);
 		}
 	}
 
@@ -1026,7 +1007,7 @@ public final class TestPackage {
 
 			@Override
 			public void close() {
-				throw new IllegalStateException("close should not be called here");
+				fail("close should not be called here");
 			}
 		};
 
@@ -1074,7 +1055,6 @@ public final class TestPackage {
 		int numPartsBefore = 0;
 		String md5Before = Files.asByteSource(tmpFile).hash(Hashing.sha256()).toString();
 
-		RuntimeException ex = null;
 		try(OPCPackage pkg = OPCPackage.open(tmpFile, PackageAccess.READ_WRITE))
 		{
 			numPartsBefore = pkg.getParts().size();
@@ -1083,13 +1063,13 @@ public final class TestPackage {
 			pkg.addMarshaller("poi/junit", (part, out) -> {
 				throw new RuntimeException("Bugzilla 63029");
 			});
+
 			pkg.createPart(createPartName("/poi/test.xml"), "poi/junit");
-		} catch (RuntimeException e){
-			ex = e;
+
+			RuntimeException ex = assertThrows(RuntimeException.class, pkg::close);
+			// verify there was an exception while closing the file
+			assertEquals("Fail to save: an error occurs while saving the package : Bugzilla 63029", ex.getMessage());
 		}
-		// verify there was an exception while closing the file
-		assertNotNull(ex, "Fail to save: an error occurs while saving the package : Bugzilla 63029");
-		assertEquals("Fail to save: an error occurs while saving the package : Bugzilla 63029", ex.getMessage());
 
 		// assert that md5 after closing is the same, i.e. the source is left intact
 		String md5After = Files.asByteSource(tmpFile).hash(Hashing.sha256()).toString();