From f71cebcce5ed809ee15cd69524f8cb0b0b2ea47c Mon Sep 17 00:00:00 2001 From: Andreas Beeker Date: Sat, 28 Aug 2021 23:48:48 +0000 Subject: [PATCH] sonar fixes close resources in tests fix gradle warnings git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1892683 13f79535-47bb-0310-9956-ffa450edef68 --- build.gradle | 9 +- poi-examples/build.gradle | 2 + poi-excelant/build.gradle | 4 +- poi-integration/build.gradle | 2 +- .../poi/stress/BaseIntegrationTest.java | 1 + .../apache/poi/stress/XSSFFileHandler.java | 12 +- poi-ooxml-lite/build.gradle | 2 + .../poifs/crypt/dsig/DigestOutputStream.java | 16 +- .../crypt/dsig/OOXMLURIDereferencer.java | 11 +- .../dsig/facets/XAdESXLSignatureFacet.java | 5 +- .../XSSFConditionalFormattingThreshold.java | 12 +- .../XSSFIconMultiStateFormatting.java | 13 +- .../xssf/usermodel/XSSFWorkbookFactory.java | 1 + .../xssf/extractor/TestXSSFExportToXML.java | 7 +- .../streaming/TestDeferredSXSSFWorkbook.java | 276 +++-- .../poi/xssf/streaming/TestSXSSFWorkbook.java | 396 ++++---- .../poi/xssf/usermodel/TestXSSFBugs.java | 11 +- .../poi/xssf/usermodel/TestXSSFFont.java | 14 +- .../poi/xssf/usermodel/TestXSSFSheet.java | 711 +++++++------ .../hemf/record/emfplus/HemfPlusBrush.java | 5 +- .../hemf/record/emfplus/HemfPlusImage.java | 5 +- .../hemf/usermodel/HemfEmbeddedIterator.java | 42 +- .../java/org/apache/poi/hslf/blip/PICT.java | 48 +- .../poi/hslf/record/EscherTextboxWrapper.java | 13 +- .../apache/poi/hslf/record/ExOleObjStg.java | 25 +- .../poi/hslf/record/PPDrawingGroup.java | 72 +- .../poi/hslf/record/RecordContainer.java | 42 +- .../poi/hslf/record/StyleTextPropAtom.java | 24 +- .../poi/hslf/record/TextSpecInfoAtom.java | 40 +- .../hwmf/usermodel/HwmfEmbeddedIterator.java | 32 +- poi/build.gradle | 4 +- .../java/org/apache/poi/hpsf/ClassID.java | 17 +- .../java/org/apache/poi/hpsf/PropertySet.java | 73 +- .../java/org/apache/poi/hpsf/Section.java | 83 +- .../poi/hssf/record/RecordInputStream.java | 19 +- .../HSSFConditionalFormattingRule.java | 40 +- .../HSSFConditionalFormattingThreshold.java | 12 +- .../HSSFIconMultiStateFormatting.java | 17 +- .../hssf/usermodel/HSSFWorkbookFactory.java | 8 +- .../crypt/cryptoapi/CryptoAPIDecryptor.java | 12 +- .../poi/poifs/macros/VBAMacroReader.java | 58 +- .../poi/ss/extractor/EmbeddedExtractor.java | 7 +- .../ConditionalFormattingThreshold.java | 38 +- .../poi/util/GenericRecordJsonWriter.java | 1 + .../java/org/apache/poi/util/IOUtils.java | 36 +- .../test/java/org/apache/poi/POITestCase.java | 65 +- .../ss/usermodel/BaseTestBugzillaIssues.java | 8 +- .../poi/ss/usermodel/BaseTestSheet.java | 11 +- .../ss/usermodel/BaseTestSheetShiftRows.java | 941 +++++++++--------- .../poi/ss/usermodel/BaseTestWorkbook.java | 2 +- 50 files changed, 1617 insertions(+), 1688 deletions(-) diff --git a/build.gradle b/build.gradle index 1522a04637..38f45d8684 100644 --- a/build.gradle +++ b/build.gradle @@ -209,7 +209,7 @@ subprojects { // make XML test-results available for Jenkins CI useJUnitPlatform() reports { - junitXml.enabled = true + junitXml.required = true } // Exclude some tests that are not actually tests or do not run cleanly on purpose @@ -238,7 +238,7 @@ subprojects { "-Dversion.id=${project.version}", '-ea', '-Djunit.jupiter.execution.parallel.config.strategy=fixed', - '-Djunit.jupiter.execution.parallel.config.fixed.parallelism=3' + '-Djunit.jupiter.execution.parallel.config.fixed.parallelism=2' // -Xjit:verbose={compileStart|compileEnd},vlog=build/jit.log${no.jit.sherlock} ... if ${isIBMVM} ] @@ -278,7 +278,7 @@ subprojects { jacocoTestReport { reports { - xml.enabled true + xml.required = true } } @@ -396,6 +396,7 @@ subprojects { spotbugs { ignoreFailures = true + showStackTraces = false } } @@ -536,7 +537,7 @@ task replaceVersion() { task zipJavadocs(type: Zip, dependsOn: allJavaDoc) { from('build/docs/javadoc/') - destinationDir = file('build/dist') + destinationDirectory = file('build/dist') archiveBaseName = 'poi' archiveVersion = subprojects[0].version archiveAppendix = 'javadoc' diff --git a/poi-examples/build.gradle b/poi-examples/build.gradle index 2f704cd184..fe8feea87f 100644 --- a/poi-examples/build.gradle +++ b/poi-examples/build.gradle @@ -65,6 +65,8 @@ task cacheJava9(type: Copy) { } jar { + dependsOn cacheJava9 + destinationDirectory = file("../build/dist/maven/${project.archivesBaseName}") if (JavaVersion.current() == JavaVersion.VERSION_1_8) { diff --git a/poi-excelant/build.gradle b/poi-excelant/build.gradle index d7562e554d..ee7cd664c0 100644 --- a/poi-excelant/build.gradle +++ b/poi-excelant/build.gradle @@ -105,6 +105,8 @@ task cacheTest9(type: Copy) { } jar { + dependsOn cacheJava9 + destinationDirectory = file("../build/dist/maven/${project.archivesBaseName}") if (JavaVersion.current() == JavaVersion.VERSION_1_8) { @@ -120,7 +122,7 @@ jar { // Create a separate jar for test-code to depend on it in other projects // See http://stackoverflow.com/questions/5144325/gradle-test-dependency -task testJar(type: Jar, dependsOn: testClasses) { +task testJar(type: Jar, dependsOn: [ testClasses, cacheTest9 ] ) { destinationDirectory = file("../build/dist/maven/${project.archivesBaseName}-tests") classifier 'tests' diff --git a/poi-integration/build.gradle b/poi-integration/build.gradle index c8556f3a50..fa1e0aa3ca 100644 --- a/poi-integration/build.gradle +++ b/poi-integration/build.gradle @@ -96,7 +96,7 @@ jar { // Create a separate jar for test-code to depend on it in other projects // See http://stackoverflow.com/questions/5144325/gradle-test-dependency -task testJar(type: Jar, dependsOn: testClasses) { +task testJar(type: Jar, dependsOn: [ testClasses, cacheTest9 ] ) { destinationDirectory = file("../build/dist/maven/${project.archivesBaseName}-tests") classifier 'tests' diff --git a/poi-integration/src/test/java/org/apache/poi/stress/BaseIntegrationTest.java b/poi-integration/src/test/java/org/apache/poi/stress/BaseIntegrationTest.java index 47a80d198b..2ae90a930d 100644 --- a/poi-integration/src/test/java/org/apache/poi/stress/BaseIntegrationTest.java +++ b/poi-integration/src/test/java/org/apache/poi/stress/BaseIntegrationTest.java @@ -37,6 +37,7 @@ import org.apache.poi.poifs.filesystem.OfficeXmlFileException; * types of files/exceptions, e.g. old file formats. * */ +@SuppressWarnings({"java:S2187", "unused"}) public class BaseIntegrationTest { private final File rootDir; private final String file; diff --git a/poi-integration/src/test/java/org/apache/poi/stress/XSSFFileHandler.java b/poi-integration/src/test/java/org/apache/poi/stress/XSSFFileHandler.java index 6f9571564f..ba60188040 100644 --- a/poi-integration/src/test/java/org/apache/poi/stress/XSSFFileHandler.java +++ b/poi-integration/src/test/java/org/apache/poi/stress/XSSFFileHandler.java @@ -17,6 +17,8 @@ package org.apache.poi.stress; import static org.apache.commons.io.output.NullOutputStream.NULL_OUTPUT_STREAM; +import static org.apache.poi.xssf.XSSFTestDataSamples.getSampleFile; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -219,12 +221,14 @@ class XSSFFileHandler extends SpreadsheetHandler { } @Test - void testExtracting() throws Exception { - handleExtracting(new File("test-data/spreadsheet/ref-56737.xlsx")); + void testExtracting() { + File testFile = getSampleFile("ref-56737.xlsx"); + assertDoesNotThrow(() -> handleExtracting(testFile)); } @Test - void testAdditional() throws Exception { - handleAdditional(new File("test-data/spreadsheet/poc-xmlbomb.xlsx")); + void testAdditional() { + File testFile = getSampleFile("poc-xmlbomb.xlsx"); + assertDoesNotThrow(() -> handleAdditional(testFile)); } } diff --git a/poi-ooxml-lite/build.gradle b/poi-ooxml-lite/build.gradle index 96d58f3ec3..4984516aa3 100644 --- a/poi-ooxml-lite/build.gradle +++ b/poi-ooxml-lite/build.gradle @@ -141,3 +141,5 @@ jar { } } +spotbugsTest.enabled = false +spotbugsMain.enabled = false \ No newline at end of file diff --git a/poi-ooxml/src/main/java/org/apache/poi/poifs/crypt/dsig/DigestOutputStream.java b/poi-ooxml/src/main/java/org/apache/poi/poifs/crypt/dsig/DigestOutputStream.java index 35e0f4924a..077d487bcc 100644 --- a/poi-ooxml/src/main/java/org/apache/poi/poifs/crypt/dsig/DigestOutputStream.java +++ b/poi-ooxml/src/main/java/org/apache/poi/poifs/crypt/dsig/DigestOutputStream.java @@ -66,13 +66,14 @@ import org.ietf.jgss.Oid; } public byte[] sign() throws IOException, GeneralSecurityException { - UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream(); - bos.write(getHashMagic()); - bos.write(md.digest()); + try (UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream()) { + bos.write(getHashMagic()); + bos.write(md.digest()); - final Cipher cipher = CryptoFunctions.getCipher(key, CipherAlgorithm.rsa - , ChainingMode.ecb, null, Cipher.ENCRYPT_MODE, "PKCS1Padding"); - return cipher.doFinal(bos.toByteArray()); + final Cipher cipher = CryptoFunctions.getCipher(key, CipherAlgorithm.rsa + , ChainingMode.ecb, null, Cipher.ENCRYPT_MODE, "PKCS1Padding"); + return cipher.doFinal(bos.toByteArray()); + } } static boolean isMSCapi(final PrivateKey key) { @@ -91,10 +92,9 @@ import org.ietf.jgss.Oid; // in an earlier release the hashMagic (aka DigestAlgorithmIdentifier) contained only // an object identifier, but to conform with the header generated by the // javax-signature API, the empty are also included - try { + try (UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream()) { final byte[] oidBytes = new Oid(algo.rsaOid).getDER(); - final UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream(); bos.write(0x30); bos.write(algo.hashSize+oidBytes.length+6); bos.write(0x30); diff --git a/poi-ooxml/src/main/java/org/apache/poi/poifs/crypt/dsig/OOXMLURIDereferencer.java b/poi-ooxml/src/main/java/org/apache/poi/poifs/crypt/dsig/OOXMLURIDereferencer.java index ff977fa45a..d01724a923 100644 --- a/poi-ooxml/src/main/java/org/apache/poi/poifs/crypt/dsig/OOXMLURIDereferencer.java +++ b/poi-ooxml/src/main/java/org/apache/poi/poifs/crypt/dsig/OOXMLURIDereferencer.java @@ -90,12 +90,13 @@ public class OOXMLURIDereferencer implements URIDereferencer { // although xmlsec has an option to ignore line breaks, currently this // only affects .rels files, so we only modify these // http://stackoverflow.com/questions/4728300 - UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream(); - for (int ch; (ch = dataStream.read()) != -1; ) { - if (ch == 10 || ch == 13) continue; - bos.write(ch); + try (UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream()) { + for (int ch; (ch = dataStream.read()) != -1; ) { + if (ch == 10 || ch == 13) continue; + bos.write(ch); + } + dataStream = bos.toInputStream(); } - dataStream = bos.toInputStream(); } } catch (IOException e) { throw new URIReferenceException("I/O error: " + e.getMessage(), e); diff --git a/poi-ooxml/src/main/java/org/apache/poi/poifs/crypt/dsig/facets/XAdESXLSignatureFacet.java b/poi-ooxml/src/main/java/org/apache/poi/poifs/crypt/dsig/facets/XAdESXLSignatureFacet.java index 2a1a32c5a5..d500c6a8c7 100644 --- a/poi-ooxml/src/main/java/org/apache/poi/poifs/crypt/dsig/facets/XAdESXLSignatureFacet.java +++ b/poi-ooxml/src/main/java/org/apache/poi/poifs/crypt/dsig/facets/XAdESXLSignatureFacet.java @@ -289,8 +289,7 @@ public class XAdESXLSignatureFacet implements SignatureFacet { } public static byte[] getC14nValue(List nodeList, String c14nAlgoId) { - UnsynchronizedByteArrayOutputStream c14nValue = new UnsynchronizedByteArrayOutputStream(); - try { + try (UnsynchronizedByteArrayOutputStream c14nValue = new UnsynchronizedByteArrayOutputStream()) { for (Node node : nodeList) { /* * Re-initialize the c14n else the namespaces will get cached @@ -299,12 +298,12 @@ public class XAdESXLSignatureFacet implements SignatureFacet { Canonicalizer c14n = Canonicalizer.getInstance(c14nAlgoId); c14n.canonicalizeSubtree(node, c14nValue); } + return c14nValue.toByteArray(); } catch (RuntimeException e) { throw e; } catch (Exception e) { throw new RuntimeException("c14n error: " + e.getMessage(), e); } - return c14nValue.toByteArray(); } private BigInteger getCrlNumber(X509CRL crl) { diff --git a/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFConditionalFormattingThreshold.java b/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFConditionalFormattingThreshold.java index f868f14a2b..b155a79654 100644 --- a/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFConditionalFormattingThreshold.java +++ b/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFConditionalFormattingThreshold.java @@ -27,34 +27,39 @@ import org.openxmlformats.schemas.spreadsheetml.x2006.main.STCfvoType; * Colour Scale change thresholds */ public class XSSFConditionalFormattingThreshold implements org.apache.poi.ss.usermodel.ConditionalFormattingThreshold { - private CTCfvo cfvo; - + private final CTCfvo cfvo; + protected XSSFConditionalFormattingThreshold(CTCfvo cfvo) { this.cfvo = cfvo; } - + protected CTCfvo getCTCfvo() { return cfvo; } + @Override public RangeType getRangeType() { return RangeType.byName(cfvo.getType().toString()); } + @Override public void setRangeType(RangeType type) { STCfvoType.Enum xtype = STCfvoType.Enum.forString(type.name); cfvo.setType(xtype); } + @Override public String getFormula() { if (cfvo.getType() == STCfvoType.FORMULA) { return cfvo.getVal(); } return null; } + @Override public void setFormula(String formula) { cfvo.setVal(formula); } + @Override public Double getValue() { if (cfvo.getType() == STCfvoType.FORMULA || cfvo.getType() == STCfvoType.MIN || @@ -67,6 +72,7 @@ public class XSSFConditionalFormattingThreshold implements org.apache.poi.ss.use return null; } } + @Override public void setValue(Double value) { if (value == null) { cfvo.unsetVal(); diff --git a/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFIconMultiStateFormatting.java b/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFIconMultiStateFormatting.java index 29062ee06f..50bf90fd8b 100644 --- a/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFIconMultiStateFormatting.java +++ b/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFIconMultiStateFormatting.java @@ -25,7 +25,7 @@ import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTIconSet; import org.openxmlformats.schemas.spreadsheetml.x2006.main.STIconSetType; /** - * High level representation for Icon / Multi-State Formatting + * High level representation for Icon / Multi-State Formatting * component of Conditional Formatting settings */ public class XSSFIconMultiStateFormatting implements IconMultiStateFormatting { @@ -35,42 +35,50 @@ public class XSSFIconMultiStateFormatting implements IconMultiStateFormatting { _iconset = iconset; } + @Override public IconSet getIconSet() { String set = _iconset.getIconSet().toString(); return IconSet.byName(set); } + @Override public void setIconSet(IconSet set) { STIconSetType.Enum xIconSet = STIconSetType.Enum.forString(set.name); _iconset.setIconSet(xIconSet); } + @Override public boolean isIconOnly() { if (_iconset.isSetShowValue()) return !_iconset.getShowValue(); return false; } + @Override public void setIconOnly(boolean only) { _iconset.setShowValue(!only); } + @Override public boolean isReversed() { if (_iconset.isSetReverse()) return _iconset.getReverse(); return false; } + @Override public void setReversed(boolean reversed) { _iconset.setReverse(reversed); } + @Override public XSSFConditionalFormattingThreshold[] getThresholds() { CTCfvo[] cfvos = _iconset.getCfvoArray(); - XSSFConditionalFormattingThreshold[] t = + XSSFConditionalFormattingThreshold[] t = new XSSFConditionalFormattingThreshold[cfvos.length]; for (int i=0; i exporter.exportToXML(os, true)); + assertEquals("schema_reference: Failed to read schema document 'Schema11', because 'file' access is not allowed due to restriction set by the accessExternalSchema property.", ex.getMessage().trim()); } } } diff --git a/poi-ooxml/src/test/java/org/apache/poi/xssf/streaming/TestDeferredSXSSFWorkbook.java b/poi-ooxml/src/test/java/org/apache/poi/xssf/streaming/TestDeferredSXSSFWorkbook.java index 92cbd37ef2..c07b59ca13 100644 --- a/poi-ooxml/src/test/java/org/apache/poi/xssf/streaming/TestDeferredSXSSFWorkbook.java +++ b/poi-ooxml/src/test/java/org/apache/poi/xssf/streaming/TestDeferredSXSSFWorkbook.java @@ -41,6 +41,8 @@ import org.apache.poi.xssf.usermodel.XSSFWorkbook; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; public final class TestDeferredSXSSFWorkbook extends BaseTestXWorkbook { @@ -90,112 +92,109 @@ public final class TestDeferredSXSSFWorkbook extends BaseTestXWorkbook { @Test void existingWorkbook() throws IOException { - XSSFWorkbook xssfWb1 = new XSSFWorkbook(); - xssfWb1.createSheet("S1"); - DeferredSXSSFWorkbook wb1 = new DeferredSXSSFWorkbook(xssfWb1); - XSSFWorkbook xssfWb2 = DeferredSXSSFITestDataProvider.instance.writeOutAndReadBack(wb1); - assertTrue(wb1.dispose()); + try (XSSFWorkbook xssfWb1 = new XSSFWorkbook()) { + xssfWb1.createSheet("S1"); + try (DeferredSXSSFWorkbook wb1 = new DeferredSXSSFWorkbook(xssfWb1); + XSSFWorkbook xssfWb2 = DeferredSXSSFITestDataProvider.instance.writeOutAndReadBack(wb1)) { + assertTrue(wb1.dispose()); - DeferredSXSSFWorkbook wb2 = new DeferredSXSSFWorkbook(xssfWb2); - assertEquals(1, wb2.getNumberOfSheets()); - Sheet sheet = wb2.getStreamingSheetAt(0); - assertNotNull(sheet); - assertEquals("S1", sheet.getSheetName()); - assertTrue(wb2.dispose()); - xssfWb2.close(); - xssfWb1.close(); - - wb2.close(); - wb1.close(); + try (DeferredSXSSFWorkbook wb2 = new DeferredSXSSFWorkbook(xssfWb2)) { + assertEquals(1, wb2.getNumberOfSheets()); + Sheet sheet = wb2.getStreamingSheetAt(0); + assertNotNull(sheet); + assertEquals("S1", sheet.getSheetName()); + assertTrue(wb2.dispose()); + } + } + } } @Test void addToExistingWorkbook() throws IOException { - XSSFWorkbook xssfWb1 = new XSSFWorkbook(); - xssfWb1.createSheet("S1"); - Sheet sheet = xssfWb1.createSheet("S2"); - Row row = sheet.createRow(1); - Cell cell = row.createCell(1); - cell.setCellValue("value 2_1_1"); - DeferredSXSSFWorkbook wb1 = new DeferredSXSSFWorkbook(xssfWb1); - XSSFWorkbook xssfWb2 = DeferredSXSSFITestDataProvider.instance.writeOutAndReadBack(wb1); - assertTrue(wb1.dispose()); - xssfWb1.close(); + try (XSSFWorkbook xssfWb1 = new XSSFWorkbook()) { + xssfWb1.createSheet("S1"); + Sheet sheet = xssfWb1.createSheet("S2"); + Row row = sheet.createRow(1); + Cell cell = row.createCell(1); + cell.setCellValue("value 2_1_1"); - DeferredSXSSFWorkbook wb2 = new DeferredSXSSFWorkbook(xssfWb2); - // Add a row to the existing empty sheet - DeferredSXSSFSheet ssheet1 = wb2.getStreamingSheetAt(0); - ssheet1.setRowGenerator((ssxSheet) -> { - Row row1_1 = ssxSheet.createRow(1); - Cell cell1_1_1 = row1_1.createCell(1); - cell1_1_1.setCellValue("value 1_1_1"); - }); + try (DeferredSXSSFWorkbook wb1 = new DeferredSXSSFWorkbook(xssfWb1); + XSSFWorkbook xssfWb2 = DeferredSXSSFITestDataProvider.instance.writeOutAndReadBack(wb1)) { + assertTrue(wb1.dispose()); - // Add a row to the existing non-empty sheet - DeferredSXSSFSheet ssheet2 = wb2.getStreamingSheetAt(1); - ssheet2.setRowGenerator((ssxSheet) -> { - Row row2_2 = ssxSheet.createRow(2); - Cell cell2_2_1 = row2_2.createCell(1); - cell2_2_1.setCellValue("value 2_2_1"); - }); - // Add a sheet with one row - DeferredSXSSFSheet ssheet3 = wb2.createSheet("S3"); - ssheet3.setRowGenerator((ssxSheet) -> { - Row row3_1 = ssxSheet.createRow(1); - Cell cell3_1_1 = row3_1.createCell(1); - cell3_1_1.setCellValue("value 3_1_1"); - }); + try (DeferredSXSSFWorkbook wb2 = new DeferredSXSSFWorkbook(xssfWb2)) { + // Add a row to the existing empty sheet + DeferredSXSSFSheet ssheet1 = wb2.getStreamingSheetAt(0); + ssheet1.setRowGenerator((ssxSheet) -> { + Row row1_1 = ssxSheet.createRow(1); + Cell cell1_1_1 = row1_1.createCell(1); + cell1_1_1.setCellValue("value 1_1_1"); + }); - XSSFWorkbook xssfWb3 = DeferredSXSSFITestDataProvider.instance.writeOutAndReadBack(wb2); - wb2.close(); + // Add a row to the existing non-empty sheet + DeferredSXSSFSheet ssheet2 = wb2.getStreamingSheetAt(1); + ssheet2.setRowGenerator((ssxSheet) -> { + Row row2_2 = ssxSheet.createRow(2); + Cell cell2_2_1 = row2_2.createCell(1); + cell2_2_1.setCellValue("value 2_2_1"); + }); + // Add a sheet with one row + DeferredSXSSFSheet ssheet3 = wb2.createSheet("S3"); + ssheet3.setRowGenerator((ssxSheet) -> { + Row row3_1 = ssxSheet.createRow(1); + Cell cell3_1_1 = row3_1.createCell(1); + cell3_1_1.setCellValue("value 3_1_1"); + }); - assertEquals(3, xssfWb3.getNumberOfSheets()); - // Verify sheet 1 - XSSFSheet sheet1 = xssfWb3.getSheetAt(0); - assertEquals("S1", sheet1.getSheetName()); - assertEquals(1, sheet1.getPhysicalNumberOfRows()); - XSSFRow row1_1 = sheet1.getRow(1); - assertNotNull(row1_1); - XSSFCell cell1_1_1 = row1_1.getCell(1); - assertNotNull(cell1_1_1); - assertEquals("value 1_1_1", cell1_1_1.getStringCellValue()); - // Verify sheet 2 - XSSFSheet sheet2 = xssfWb3.getSheetAt(1); - assertEquals("S2", sheet2.getSheetName()); - assertEquals(2, sheet2.getPhysicalNumberOfRows()); - Row row2_1 = sheet2.getRow(1); - assertNotNull(row2_1); - Cell cell2_1_1 = row2_1.getCell(1); - assertNotNull(cell2_1_1); - assertEquals("value 2_1_1", cell2_1_1.getStringCellValue()); - XSSFRow row2_2 = sheet2.getRow(2); - assertNotNull(row2_2); - XSSFCell cell2_2_1 = row2_2.getCell(1); - assertNotNull(cell2_2_1); - assertEquals("value 2_2_1", cell2_2_1.getStringCellValue()); - // Verify sheet 3 - XSSFSheet sheet3 = xssfWb3.getSheetAt(2); - assertEquals("S3", sheet3.getSheetName()); - assertEquals(1, sheet3.getPhysicalNumberOfRows()); - XSSFRow row3_1 = sheet3.getRow(1); - assertNotNull(row3_1); - XSSFCell cell3_1_1 = row3_1.getCell(1); - assertNotNull(cell3_1_1); - assertEquals("value 3_1_1", cell3_1_1.getStringCellValue()); + try (XSSFWorkbook xssfWb3 = DeferredSXSSFITestDataProvider.instance.writeOutAndReadBack(wb2)) { - xssfWb2.close(); - xssfWb3.close(); - wb1.close(); + assertEquals(3, xssfWb3.getNumberOfSheets()); + // Verify sheet 1 + XSSFSheet sheet1 = xssfWb3.getSheetAt(0); + assertEquals("S1", sheet1.getSheetName()); + assertEquals(1, sheet1.getPhysicalNumberOfRows()); + XSSFRow row1_1 = sheet1.getRow(1); + assertNotNull(row1_1); + XSSFCell cell1_1_1 = row1_1.getCell(1); + assertNotNull(cell1_1_1); + assertEquals("value 1_1_1", cell1_1_1.getStringCellValue()); + // Verify sheet 2 + XSSFSheet sheet2 = xssfWb3.getSheetAt(1); + assertEquals("S2", sheet2.getSheetName()); + assertEquals(2, sheet2.getPhysicalNumberOfRows()); + Row row2_1 = sheet2.getRow(1); + assertNotNull(row2_1); + Cell cell2_1_1 = row2_1.getCell(1); + assertNotNull(cell2_1_1); + assertEquals("value 2_1_1", cell2_1_1.getStringCellValue()); + XSSFRow row2_2 = sheet2.getRow(2); + assertNotNull(row2_2); + XSSFCell cell2_2_1 = row2_2.getCell(1); + assertNotNull(cell2_2_1); + assertEquals("value 2_2_1", cell2_2_1.getStringCellValue()); + // Verify sheet 3 + XSSFSheet sheet3 = xssfWb3.getSheetAt(2); + assertEquals("S3", sheet3.getSheetName()); + assertEquals(1, sheet3.getPhysicalNumberOfRows()); + XSSFRow row3_1 = sheet3.getRow(1); + assertNotNull(row3_1); + XSSFCell cell3_1_1 = row3_1.getCell(1); + assertNotNull(cell3_1_1); + assertEquals("value 3_1_1", cell3_1_1.getStringCellValue()); + } + } + } + } } @Test void sheetdataWriter() throws IOException { - DeferredSXSSFWorkbook wb = new DeferredSXSSFWorkbook(); - SXSSFSheet sh = wb.createSheet(); - assertSame(sh.getClass(), DeferredSXSSFSheet.class); - SheetDataWriter wr = sh.getSheetDataWriter(); - assertNull(wr); - wb.close(); + try (DeferredSXSSFWorkbook wb = new DeferredSXSSFWorkbook()) { + SXSSFSheet sh = wb.createSheet(); + assertSame(sh.getClass(), DeferredSXSSFSheet.class); + SheetDataWriter wr = sh.getSheetDataWriter(); + assertNull(wr); + } } @Test @@ -225,71 +224,65 @@ public final class TestDeferredSXSSFWorkbook extends BaseTestXWorkbook { @Test void gzipSheetdataWriter() throws IOException { - DeferredSXSSFWorkbook wb = new DeferredSXSSFWorkbook(); + try (DeferredSXSSFWorkbook wb = new DeferredSXSSFWorkbook()) { - final int rowNum = 1000; - final int sheetNum = 5; - populateData(wb, 1000, 5); + final int rowNum = 1000; + final int sheetNum = 5; + populateData(wb); - XSSFWorkbook xwb = DeferredSXSSFITestDataProvider.instance.writeOutAndReadBack(wb); - for (int i = 0; i < sheetNum; i++) { - Sheet sh = xwb.getSheetAt(i); - assertEquals("sheet" + i, sh.getSheetName()); - for (int j = 0; j < rowNum; j++) { - Row row = sh.getRow(j); - assertNotNull(row, "row[" + j + "]"); - Cell cell1 = row.getCell(0); - assertEquals(new CellReference(cell1).formatAsString(), cell1.getStringCellValue()); + try (XSSFWorkbook xwb = DeferredSXSSFITestDataProvider.instance.writeOutAndReadBack(wb)) { + for (int i = 0; i < sheetNum; i++) { + Sheet sh = xwb.getSheetAt(i); + assertEquals("sheet" + i, sh.getSheetName()); + for (int j = 0; j < rowNum; j++) { + Row row = sh.getRow(j); + assertNotNull(row, "row[" + j + "]"); + Cell cell1 = row.getCell(0); + assertEquals(new CellReference(cell1).formatAsString(), cell1.getStringCellValue()); - Cell cell2 = row.getCell(1); - assertEquals(i, (int) cell2.getNumericCellValue()); + Cell cell2 = row.getCell(1); + assertEquals(i, (int) cell2.getNumericCellValue()); - Cell cell3 = row.getCell(2); - assertEquals(j, (int) cell3.getNumericCellValue()); + Cell cell3 = row.getCell(2); + assertEquals(j, (int) cell3.getNumericCellValue()); + } + } + + assertTrue(wb.dispose()); } } - - assertTrue(wb.dispose()); - xwb.close(); - wb.close(); } - @Test - void workbookDispose() throws IOException { - DeferredSXSSFWorkbook wb1 = new DeferredSXSSFWorkbook(); - // the underlying writer is SheetDataWriter - assertWorkbookDispose(wb1); - wb1.close(); + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void workbookDispose(boolean compressTempFiles) throws IOException { + try (DeferredSXSSFWorkbook wb = new DeferredSXSSFWorkbook()) { + // compressTempFiles == false: the underlying writer is SheetDataWriter + // compressTempFiles == true: the underlying writer is GZIPSheetDataWriter + wb.setCompressTempFiles(compressTempFiles); - DeferredSXSSFWorkbook wb2 = new DeferredSXSSFWorkbook(); - wb2.setCompressTempFiles(true); - // the underlying writer is GZIPSheetDataWriter - assertWorkbookDispose(wb2); - wb2.close(); - } + populateData(wb); - private static void assertWorkbookDispose(DeferredSXSSFWorkbook wb) { - populateData(wb, 1000, 5); + for (Sheet sheet : wb) { + DeferredSXSSFSheet sxSheet = (DeferredSXSSFSheet) sheet; + assertNull(sxSheet.getSheetDataWriter()); + } - for (Sheet sheet : wb) { - DeferredSXSSFSheet sxSheet = (DeferredSXSSFSheet) sheet; - assertNull(sxSheet.getSheetDataWriter()); - } + assertTrue(wb.dispose()); - assertTrue(wb.dispose()); - - for (Sheet sheet : wb) { - DeferredSXSSFSheet sxSheet = (DeferredSXSSFSheet) sheet; - assertNull(sxSheet.getSheetDataWriter()); + for (Sheet sheet : wb) { + DeferredSXSSFSheet sxSheet = (DeferredSXSSFSheet) sheet; + assertNull(sxSheet.getSheetDataWriter()); + } } } - private static void populateData(DeferredSXSSFWorkbook wb, final int rowNum, final int sheetNum) { - for (int i = 0; i < sheetNum; i++) { + private static void populateData(DeferredSXSSFWorkbook wb) { + for (int i = 0; i < 5; i++) { DeferredSXSSFSheet sheet = wb.createSheet("sheet" + i); int index = i; sheet.setRowGenerator((sh) -> { - for (int j = 0; j < rowNum; j++) { + for (int j = 0; j < 1000; j++) { Row row = sh.createRow(j); Cell cell1 = row.createCell(0); cell1.setCellValue(new CellReference(cell1).formatAsString()); @@ -304,7 +297,8 @@ public final class TestDeferredSXSSFWorkbook extends BaseTestXWorkbook { } } - void changeSheetNameWithSharedFormulas() { - /* not implemented */ + @Override + @Disabled("not implemented") + protected void changeSheetNameWithSharedFormulas() { } } diff --git a/poi-ooxml/src/test/java/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java b/poi-ooxml/src/test/java/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java index 982c5e7373..f4eb9e0990 100644 --- a/poi-ooxml/src/test/java/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java +++ b/poi-ooxml/src/test/java/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java @@ -55,6 +55,8 @@ import org.apache.poi.xssf.usermodel.XSSFWorkbook; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; public final class TestSXSSFWorkbook extends BaseTestXWorkbook { @@ -98,225 +100,201 @@ public final class TestSXSSFWorkbook extends BaseTestXWorkbook { @Test void existingWorkbook() throws IOException { - XSSFWorkbook xssfWb1 = new XSSFWorkbook(); - xssfWb1.createSheet("S1"); - SXSSFWorkbook wb1 = new SXSSFWorkbook(xssfWb1); - XSSFWorkbook xssfWb2 = SXSSFITestDataProvider.instance.writeOutAndReadBack(wb1); - assertTrue(wb1.dispose()); + try (XSSFWorkbook xssfWb1 = new XSSFWorkbook()) { + xssfWb1.createSheet("S1"); + try (SXSSFWorkbook wb1 = new SXSSFWorkbook(xssfWb1); + XSSFWorkbook xssfWb2 = SXSSFITestDataProvider.instance.writeOutAndReadBack(wb1)) { + assertTrue(wb1.dispose()); - SXSSFWorkbook wb2 = new SXSSFWorkbook(xssfWb2); - assertEquals(1, wb2.getNumberOfSheets()); - Sheet sheet = wb2.getSheetAt(0); - assertNotNull(sheet); - assertEquals("S1", sheet.getSheetName()); - assertTrue(wb2.dispose()); - xssfWb2.close(); - xssfWb1.close(); - - wb2.close(); - wb1.close(); + try (SXSSFWorkbook wb2 = new SXSSFWorkbook(xssfWb2)) { + assertEquals(1, wb2.getNumberOfSheets()); + Sheet sheet = wb2.getSheetAt(0); + assertNotNull(sheet); + assertEquals("S1", sheet.getSheetName()); + assertTrue(wb2.dispose()); + } + } + } } @Test void useSharedStringsTable() throws Exception { - SXSSFWorkbook wb = new SXSSFWorkbook(null, 10, false, true); + try (SXSSFWorkbook wb = new SXSSFWorkbook(null, 10, false, true)) { - SharedStringsTable sss = wb.getSharedStringSource(); + SharedStringsTable sss = wb.getSharedStringSource(); - assertNotNull(sss); + assertNotNull(sss); - Row row = wb.createSheet("S1").createRow(0); + Row row = wb.createSheet("S1").createRow(0); - row.createCell(0).setCellValue("A"); - row.createCell(1).setCellValue("B"); - row.createCell(2).setCellValue("A"); + row.createCell(0).setCellValue("A"); + row.createCell(1).setCellValue("B"); + row.createCell(2).setCellValue("A"); - XSSFWorkbook xssfWorkbook = SXSSFITestDataProvider.instance.writeOutAndReadBack(wb); - sss = wb.getSharedStringSource(); - assertEquals(2, sss.getUniqueCount()); - assertTrue(wb.dispose()); + try (XSSFWorkbook xssfWorkbook = SXSSFITestDataProvider.instance.writeOutAndReadBack(wb)) { + sss = wb.getSharedStringSource(); + assertEquals(2, sss.getUniqueCount()); + assertTrue(wb.dispose()); - Sheet sheet1 = xssfWorkbook.getSheetAt(0); - assertEquals("S1", sheet1.getSheetName()); - assertEquals(1, sheet1.getPhysicalNumberOfRows()); - row = sheet1.getRow(0); - assertNotNull(row); - Cell cell = row.getCell(0); - assertNotNull(cell); - assertEquals("A", cell.getStringCellValue()); - cell = row.getCell(1); - assertNotNull(cell); - assertEquals("B", cell.getStringCellValue()); - cell = row.getCell(2); - assertNotNull(cell); - assertEquals("A", cell.getStringCellValue()); - - xssfWorkbook.close(); - wb.close(); + Sheet sheet1 = xssfWorkbook.getSheetAt(0); + assertEquals("S1", sheet1.getSheetName()); + assertEquals(1, sheet1.getPhysicalNumberOfRows()); + row = sheet1.getRow(0); + assertNotNull(row); + Cell cell = row.getCell(0); + assertNotNull(cell); + assertEquals("A", cell.getStringCellValue()); + cell = row.getCell(1); + assertNotNull(cell); + assertEquals("B", cell.getStringCellValue()); + cell = row.getCell(2); + assertNotNull(cell); + assertEquals("A", cell.getStringCellValue()); + } + } } @Test void addToExistingWorkbook() throws IOException { - XSSFWorkbook xssfWb1 = new XSSFWorkbook(); - xssfWb1.createSheet("S1"); - Sheet sheet = xssfWb1.createSheet("S2"); - Row row = sheet.createRow(1); - Cell cell = row.createCell(1); - cell.setCellValue("value 2_1_1"); - SXSSFWorkbook wb1 = new SXSSFWorkbook(xssfWb1); - XSSFWorkbook xssfWb2 = SXSSFITestDataProvider.instance.writeOutAndReadBack(wb1); - assertTrue(wb1.dispose()); - xssfWb1.close(); + try (XSSFWorkbook xssfWb1 = new XSSFWorkbook()) { + xssfWb1.createSheet("S1"); + Sheet sheet = xssfWb1.createSheet("S2"); + Row row = sheet.createRow(1); + Cell cell = row.createCell(1); + cell.setCellValue("value 2_1_1"); + try (SXSSFWorkbook wb1 = new SXSSFWorkbook(xssfWb1); + XSSFWorkbook xssfWb2 = SXSSFITestDataProvider.instance.writeOutAndReadBack(wb1)) { + assertTrue(wb1.dispose()); - SXSSFWorkbook wb2 = new SXSSFWorkbook(xssfWb2); - // Add a row to the existing empty sheet - Sheet sheet1 = wb2.getSheetAt(0); - Row row1_1 = sheet1.createRow(1); - Cell cell1_1_1 = row1_1.createCell(1); - cell1_1_1.setCellValue("value 1_1_1"); + try (SXSSFWorkbook wb2 = new SXSSFWorkbook(xssfWb2)) { + // Add a row to the existing empty sheet + Sheet sheet1 = wb2.getSheetAt(0); + Row row1_1 = sheet1.createRow(1); + Cell cell1_1_1 = row1_1.createCell(1); + cell1_1_1.setCellValue("value 1_1_1"); - // Add a row to the existing non-empty sheet - Sheet sheet2 = wb2.getSheetAt(1); - Row row2_2 = sheet2.createRow(2); - Cell cell2_2_1 = row2_2.createCell(1); - cell2_2_1.setCellValue("value 2_2_1"); + // Add a row to the existing non-empty sheet + Sheet sheet2 = wb2.getSheetAt(1); + Row row2_2 = sheet2.createRow(2); + Cell cell2_2_1 = row2_2.createCell(1); + cell2_2_1.setCellValue("value 2_2_1"); - // Add a sheet with one row - Sheet sheet3 = wb2.createSheet("S3"); - Row row3_1 = sheet3.createRow(1); - Cell cell3_1_1 = row3_1.createCell(1); - cell3_1_1.setCellValue("value 3_1_1"); + // Add a sheet with one row + Sheet sheet3 = wb2.createSheet("S3"); + Row row3_1 = sheet3.createRow(1); + Cell cell3_1_1 = row3_1.createCell(1); + cell3_1_1.setCellValue("value 3_1_1"); - XSSFWorkbook xssfWb3 = SXSSFITestDataProvider.instance.writeOutAndReadBack(wb2); - wb2.close(); - - assertEquals(3, xssfWb3.getNumberOfSheets()); - // Verify sheet 1 - sheet1 = xssfWb3.getSheetAt(0); - assertEquals("S1", sheet1.getSheetName()); - assertEquals(1, sheet1.getPhysicalNumberOfRows()); - row1_1 = sheet1.getRow(1); - assertNotNull(row1_1); - cell1_1_1 = row1_1.getCell(1); - assertNotNull(cell1_1_1); - assertEquals("value 1_1_1", cell1_1_1.getStringCellValue()); - // Verify sheet 2 - sheet2 = xssfWb3.getSheetAt(1); - assertEquals("S2", sheet2.getSheetName()); - assertEquals(2, sheet2.getPhysicalNumberOfRows()); - Row row2_1 = sheet2.getRow(1); - assertNotNull(row2_1); - Cell cell2_1_1 = row2_1.getCell(1); - assertNotNull(cell2_1_1); - assertEquals("value 2_1_1", cell2_1_1.getStringCellValue()); - row2_2 = sheet2.getRow(2); - assertNotNull(row2_2); - cell2_2_1 = row2_2.getCell(1); - assertNotNull(cell2_2_1); - assertEquals("value 2_2_1", cell2_2_1.getStringCellValue()); - // Verify sheet 3 - sheet3 = xssfWb3.getSheetAt(2); - assertEquals("S3", sheet3.getSheetName()); - assertEquals(1, sheet3.getPhysicalNumberOfRows()); - row3_1 = sheet3.getRow(1); - assertNotNull(row3_1); - cell3_1_1 = row3_1.getCell(1); - assertNotNull(cell3_1_1); - assertEquals("value 3_1_1", cell3_1_1.getStringCellValue()); - - xssfWb2.close(); - xssfWb3.close(); - wb1.close(); + try (XSSFWorkbook xssfWb3 = SXSSFITestDataProvider.instance.writeOutAndReadBack(wb2)) { + assertEquals(3, xssfWb3.getNumberOfSheets()); + // Verify sheet 1 + sheet1 = xssfWb3.getSheetAt(0); + assertEquals("S1", sheet1.getSheetName()); + assertEquals(1, sheet1.getPhysicalNumberOfRows()); + row1_1 = sheet1.getRow(1); + assertNotNull(row1_1); + cell1_1_1 = row1_1.getCell(1); + assertNotNull(cell1_1_1); + assertEquals("value 1_1_1", cell1_1_1.getStringCellValue()); + // Verify sheet 2 + sheet2 = xssfWb3.getSheetAt(1); + assertEquals("S2", sheet2.getSheetName()); + assertEquals(2, sheet2.getPhysicalNumberOfRows()); + Row row2_1 = sheet2.getRow(1); + assertNotNull(row2_1); + Cell cell2_1_1 = row2_1.getCell(1); + assertNotNull(cell2_1_1); + assertEquals("value 2_1_1", cell2_1_1.getStringCellValue()); + row2_2 = sheet2.getRow(2); + assertNotNull(row2_2); + cell2_2_1 = row2_2.getCell(1); + assertNotNull(cell2_2_1); + assertEquals("value 2_2_1", cell2_2_1.getStringCellValue()); + // Verify sheet 3 + sheet3 = xssfWb3.getSheetAt(2); + assertEquals("S3", sheet3.getSheetName()); + assertEquals(1, sheet3.getPhysicalNumberOfRows()); + row3_1 = sheet3.getRow(1); + assertNotNull(row3_1); + cell3_1_1 = row3_1.getCell(1); + assertNotNull(cell3_1_1); + assertEquals("value 3_1_1", cell3_1_1.getStringCellValue()); + } + } + } + } } @Test void sheetdataWriter() throws IOException{ - SXSSFWorkbook wb = new SXSSFWorkbook(); - SXSSFSheet sh = wb.createSheet(); - SheetDataWriter wr = sh.getSheetDataWriter(); - assertSame(wr.getClass(), SheetDataWriter.class); - File tmp = wr.getTempFile(); - assertStartsWith(tmp.getName(), "poi-sxssf-sheet"); - assertEndsWith(tmp.getName(), ".xml"); - assertTrue(wb.dispose()); - wb.close(); + try (SXSSFWorkbook wb = new SXSSFWorkbook()) { + SXSSFSheet sh = wb.createSheet(); + SheetDataWriter wr = sh.getSheetDataWriter(); + assertSame(wr.getClass(), SheetDataWriter.class); + File tmp = wr.getTempFile(); + assertStartsWith(tmp.getName(), "poi-sxssf-sheet"); + assertEndsWith(tmp.getName(), ".xml"); + assertTrue(wb.dispose()); + } - wb = new SXSSFWorkbook(); - wb.setCompressTempFiles(true); - sh = wb.createSheet(); - wr = sh.getSheetDataWriter(); - assertSame(wr.getClass(), GZIPSheetDataWriter.class); - tmp = wr.getTempFile(); - assertStartsWith(tmp.getName(), "poi-sxssf-sheet-xml"); - assertEndsWith(tmp.getName(), ".gz"); - assertTrue(wb.dispose()); - wb.close(); + try (SXSSFWorkbook wb = new SXSSFWorkbook()) { + wb.setCompressTempFiles(true); + SXSSFSheet sh = wb.createSheet(); + SheetDataWriter wr = sh.getSheetDataWriter(); + assertSame(wr.getClass(), GZIPSheetDataWriter.class); + File tmp = wr.getTempFile(); + assertStartsWith(tmp.getName(), "poi-sxssf-sheet-xml"); + assertEndsWith(tmp.getName(), ".gz"); + assertTrue(wb.dispose()); + } //Test escaping of Unicode control characters - wb = new SXSSFWorkbook(); - wb.createSheet("S1").createRow(0).createCell(0).setCellValue("value\u0019"); - XSSFWorkbook xssfWorkbook = SXSSFITestDataProvider.instance.writeOutAndReadBack(wb); - Cell cell = xssfWorkbook.getSheet("S1").getRow(0).getCell(0); - assertEquals("value?", cell.getStringCellValue()); - - assertTrue(wb.dispose()); - wb.close(); - xssfWorkbook.close(); + try (SXSSFWorkbook wb = new SXSSFWorkbook()) { + wb.createSheet("S1").createRow(0).createCell(0).setCellValue("value\u0019"); + try (XSSFWorkbook xssfWorkbook = SXSSFITestDataProvider.instance.writeOutAndReadBack(wb)) { + Cell cell = xssfWorkbook.getSheet("S1").getRow(0).getCell(0); + assertEquals("value?", cell.getStringCellValue()); + assertTrue(wb.dispose()); + } + } } @Test void gzipSheetdataWriter() throws IOException { - SXSSFWorkbook wb = new SXSSFWorkbook(); - wb.setCompressTempFiles(true); + try (SXSSFWorkbook wb = new SXSSFWorkbook()) { + wb.setCompressTempFiles(true); - final int rowNum = 1000; - final int sheetNum = 5; - populateData(wb, 1000, 5); + final int rowNum = 1000; + final int sheetNum = 5; + populateData(wb); - XSSFWorkbook xwb = SXSSFITestDataProvider.instance.writeOutAndReadBack(wb); - for(int i = 0; i < sheetNum; i++){ - Sheet sh = xwb.getSheetAt(i); - assertEquals("sheet" + i, sh.getSheetName()); - for(int j = 0; j < rowNum; j++){ - Row row = sh.getRow(j); - assertNotNull(row, "row[" + j + "]"); - Cell cell1 = row.getCell(0); - assertEquals(new CellReference(cell1).formatAsString(), cell1.getStringCellValue()); + try (XSSFWorkbook xwb = SXSSFITestDataProvider.instance.writeOutAndReadBack(wb)) { + for (int i = 0; i < sheetNum; i++) { + Sheet sh = xwb.getSheetAt(i); + assertEquals("sheet" + i, sh.getSheetName()); + for (int j = 0; j < rowNum; j++) { + Row row = sh.getRow(j); + assertNotNull(row, "row[" + j + "]"); + Cell cell1 = row.getCell(0); + assertEquals(new CellReference(cell1).formatAsString(), cell1.getStringCellValue()); - Cell cell2 = row.getCell(1); - assertEquals(i, (int)cell2.getNumericCellValue()); + Cell cell2 = row.getCell(1); + assertEquals(i, (int) cell2.getNumericCellValue()); - Cell cell3 = row.getCell(2); - assertEquals(j, (int)cell3.getNumericCellValue()); + Cell cell3 = row.getCell(2); + assertEquals(j, (int) cell3.getNumericCellValue()); + } + } + + assertTrue(wb.dispose()); } } - - assertTrue(wb.dispose()); - xwb.close(); - wb.close(); } - private static void assertWorkbookDispose(SXSSFWorkbook wb) - { - populateData(wb, 1000, 5); - - for (Sheet sheet : wb) { - SXSSFSheet sxSheet = (SXSSFSheet) sheet; - assertTrue(sxSheet.getSheetDataWriter().getTempFile().exists()); - } - - assertTrue(wb.dispose()); - - for (Sheet sheet : wb) { - SXSSFSheet sxSheet = (SXSSFSheet) sheet; - assertFalse(sxSheet.getSheetDataWriter().getTempFile().exists()); - } - } - - private static void populateData(Workbook wb, final int rowNum, final int sheetNum) { - for(int i = 0; i < sheetNum; i++){ + private static void populateData(Workbook wb) { + for(int i = 0; i < 5; i++){ Sheet sh = wb.createSheet("sheet" + i); - for(int j = 0; j < rowNum; j++){ + for(int j = 0; j < 1000; j++){ Row row = sh.createRow(j); Cell cell1 = row.createCell(0); cell1.setCellValue(new CellReference(cell1).formatAsString()); @@ -330,28 +308,40 @@ public final class TestSXSSFWorkbook extends BaseTestXWorkbook { } } - @Test - void workbookDispose() throws IOException { - SXSSFWorkbook wb1 = new SXSSFWorkbook(); - // the underlying writer is SheetDataWriter - assertWorkbookDispose(wb1); - wb1.close(); + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void workbookDispose(boolean compressTempFiles) throws IOException { + try (SXSSFWorkbook wb = new SXSSFWorkbook()) { + // compressTempFiles == false: the underlying writer is SheetDataWriter + // compressTempFiles == true: the underlying writer is GZIPSheetDataWriter + wb.setCompressTempFiles(compressTempFiles); - SXSSFWorkbook wb2 = new SXSSFWorkbook(); - wb2.setCompressTempFiles(true); - // the underlying writer is GZIPSheetDataWriter - assertWorkbookDispose(wb2); - wb2.close(); + populateData(wb); + + for (Sheet sheet : wb) { + SXSSFSheet sxSheet = (SXSSFSheet) sheet; + assertTrue(sxSheet.getSheetDataWriter().getTempFile().exists()); + } + + assertTrue(wb.dispose()); + + for (Sheet sheet : wb) { + SXSSFSheet sxSheet = (SXSSFSheet) sheet; + assertFalse(sxSheet.getSheetDataWriter().getTempFile().exists()); + } + } } @Test void bug53515() throws Exception { try (Workbook wb1 = new SXSSFWorkbook(10)) { populateWorkbook(wb1); - saveTwice(wb1); + assertDoesNotThrow(() -> wb1.write(NULL_OUTPUT_STREAM)); + assertDoesNotThrow(() -> wb1.write(NULL_OUTPUT_STREAM)); try (Workbook wb2 = new XSSFWorkbook()) { populateWorkbook(wb2); - saveTwice(wb2); + assertDoesNotThrow(() -> wb2.write(NULL_OUTPUT_STREAM)); + assertDoesNotThrow(() -> wb2.write(NULL_OUTPUT_STREAM)); } } } @@ -405,18 +395,6 @@ public final class TestSXSSFWorkbook extends BaseTestXWorkbook { } } - private static void saveTwice(Workbook wb) throws Exception { - for (int i = 0; i < 2; i++) { - try { - wb.write(NULL_OUTPUT_STREAM); - } catch (Exception e) { - throw new Exception("ERROR: failed on " + (i + 1) - + "th time calling " + wb.getClass().getName() - + ".write() with exception " + e.getMessage(), e); - } - } - } - @Test void closeDoesNotModifyWorkbook() throws IOException { final String filename = "SampleSS.xlsx"; @@ -460,10 +438,10 @@ public final class TestSXSSFWorkbook extends BaseTestXWorkbook { char[] useless = new char[32767]; Arrays.fill(useless, ' '); - for (int row = 0; row < 1; row++) { + for (int row = 0; row < 10; row++) { Row r = s.createRow(row); for (int col = 0; col < 10; col++) { - char[] prefix = Integer.toHexString(row * 1000 + col).toCharArray(); + char[] prefix = Integer.toHexString(row * 10 + col).toCharArray(); Arrays.fill(useless, 0, 10, ' '); System.arraycopy(prefix, 0, useless, 0, prefix.length); String ul = new String(useless); @@ -528,8 +506,8 @@ public final class TestSXSSFWorkbook extends BaseTestXWorkbook { } } + @Override @Disabled("not implemented") - @Test - void changeSheetNameWithSharedFormulas() { + protected void changeSheetNameWithSharedFormulas() { } } diff --git a/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFBugs.java b/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFBugs.java index febe557ad0..25a16e0f69 100644 --- a/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFBugs.java +++ b/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFBugs.java @@ -100,7 +100,6 @@ import org.apache.poi.ss.util.CellRangeAddress; import org.apache.poi.ss.util.CellReference; import org.apache.poi.ss.util.CellUtil; import org.apache.poi.util.LocaleUtil; -import org.apache.commons.io.output.NullOutputStream; import org.apache.poi.util.TempFile; import org.apache.poi.util.XMLHelper; import org.apache.poi.xssf.SXSSFITestDataProvider; @@ -117,7 +116,14 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.EnumSource; import org.junit.jupiter.params.provider.ValueSource; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.*; +import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTCalcCell; +import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTCols; +import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTDefinedName; +import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTDefinedNames; +import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTMergeCell; +import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTMergeCells; +import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTWorksheet; +import org.openxmlformats.schemas.spreadsheetml.x2006.main.STCellFormulaType; import org.openxmlformats.schemas.spreadsheetml.x2006.main.impl.CTFontImpl; import org.xml.sax.InputSource; import org.xml.sax.SAXParseException; @@ -2714,6 +2720,7 @@ public final class TestXSSFBugs extends BaseTestBugzillaIssues { CellStyle style = wb.createCellStyle(); style.setRotation((short) -90); cell.setCellStyle(style); + assertEquals(180, style.getRotation()); XSSFTestDataSamples.writeOut(wb, fileName); } diff --git a/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFFont.java b/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFFont.java index f5e81dbdb9..a3878dcc31 100644 --- a/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFFont.java +++ b/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFFont.java @@ -259,11 +259,11 @@ public final class TestXSSFFont extends BaseTestFont{ xssfFont.setUnderline(Font.U_DOUBLE); assertEquals(ctFont.sizeOfUArray(),1); - assertEquals(STUnderlineValues.DOUBLE,ctFont.getUArray(0).getVal()); + assertSame(STUnderlineValues.DOUBLE,ctFont.getUArray(0).getVal()); xssfFont.setUnderline(FontUnderline.DOUBLE_ACCOUNTING); assertEquals(ctFont.sizeOfUArray(),1); - assertEquals(STUnderlineValues.DOUBLE_ACCOUNTING,ctFont.getUArray(0).getVal()); + assertSame(STUnderlineValues.DOUBLE_ACCOUNTING,ctFont.getUArray(0).getVal()); } @Test @@ -342,7 +342,7 @@ public final class TestXSSFFont extends BaseTestFont{ assertEquals(FontScheme.MAJOR,font.getScheme()); font.setScheme(FontScheme.NONE); - assertEquals(STFontScheme.NONE,ctFont.getSchemeArray(0).getVal()); + assertSame(STFontScheme.NONE,ctFont.getSchemeArray(0).getVal()); } @Test @@ -356,17 +356,15 @@ public final class TestXSSFFont extends BaseTestFont{ assertEquals(Font.SS_NONE,font.getTypeOffset()); font.setTypeOffset(XSSFFont.SS_SUPER); - assertEquals(STVerticalAlignRun.SUPERSCRIPT,ctFont.getVertAlignArray(0).getVal()); + assertSame(STVerticalAlignRun.SUPERSCRIPT,ctFont.getVertAlignArray(0).getVal()); } // store test from TestSheetUtil here as it uses XSSF @Test void testCanComputeWidthXSSF() throws IOException { try (Workbook wb = new XSSFWorkbook()) { - // cannot check on result because on some machines we get back false here! - SheetUtil.canComputeColumnWidth(wb.getFontAt(0)); - + assertDoesNotThrow(() -> SheetUtil.canComputeColumnWidth(wb.getFontAt(0))); } } @@ -377,7 +375,7 @@ public final class TestXSSFFont extends BaseTestFont{ font.setFontName("some non existing font name"); // Even with invalid fonts we still get back useful data most of the time... - SheetUtil.canComputeColumnWidth(font); + assertDoesNotThrow(() -> SheetUtil.canComputeColumnWidth(font)); } /** diff --git a/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFSheet.java b/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFSheet.java index 1ca51d3dcc..54229938e4 100644 --- a/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFSheet.java +++ b/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFSheet.java @@ -252,14 +252,14 @@ public final class TestXSSFSheet extends BaseTestXSheet { sheet.createFreezePane(2, 4); assertEquals(2.0, ctWorksheet.getSheetViews().getSheetViewArray(0).getPane().getXSplit(), 0.0); - assertEquals(STPane.BOTTOM_RIGHT, ctWorksheet.getSheetViews().getSheetViewArray(0).getPane().getActivePane()); + assertSame(STPane.BOTTOM_RIGHT, ctWorksheet.getSheetViews().getSheetViewArray(0).getPane().getActivePane()); sheet.createFreezePane(3, 6, 10, 10); assertEquals(3.0, ctWorksheet.getSheetViews().getSheetViewArray(0).getPane().getXSplit(), 0.0); // assertEquals(10, sheet.getTopRow()); // assertEquals(10, sheet.getLeftCol()); sheet.createSplitPane(4, 8, 12, 12, 1); assertEquals(8.0, ctWorksheet.getSheetViews().getSheetViewArray(0).getPane().getYSplit(), 0.0); - assertEquals(STPane.BOTTOM_RIGHT, ctWorksheet.getSheetViews().getSheetViewArray(0).getPane().getActivePane()); + assertSame(STPane.BOTTOM_RIGHT, ctWorksheet.getSheetViews().getSheetViewArray(0).getPane().getActivePane()); workbook.close(); } @@ -1152,7 +1152,7 @@ public final class TestXSSFSheet extends BaseTestXSheet { CTCalcPr calcPr = wb1.getCTWorkbook().addNewCalcPr(); calcPr.setCalcMode(STCalcMode.MANUAL); sheet.setForceFormulaRecalculation(true); - assertEquals(STCalcMode.AUTO, calcPr.getCalcMode()); + assertSame(STCalcMode.AUTO, calcPr.getCalcMode()); // Check sheet.setForceFormulaRecalculation(false); @@ -1422,350 +1422,337 @@ public final class TestXSSFSheet extends BaseTestXSheet { wb.close(); } - private void testCopyOneRow(String copyRowsTestWorkbook) throws IOException { - final double FLOAT_PRECISION = 1e-9; - final XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook(copyRowsTestWorkbook); - final XSSFSheet sheet = wb.getSheetAt(0); - final CellCopyPolicy defaultCopyPolicy = new CellCopyPolicy(); - sheet.copyRows(1, 1, 6, defaultCopyPolicy); - - final Row srcRow = sheet.getRow(1); - final Row destRow = sheet.getRow(6); - int col = 0; - Cell cell; - - cell = CellUtil.getCell(destRow, col++); - assertEquals("Source row ->", cell.getStringCellValue()); - - // Style - cell = CellUtil.getCell(destRow, col++); - assertEquals("Red", cell.getStringCellValue(), "[Style] B7 cell value"); - assertEquals(CellUtil.getCell(srcRow, 1).getCellStyle(), cell.getCellStyle(), "[Style] B7 cell style"); - - // Blank - cell = CellUtil.getCell(destRow, col++); - assertEquals(CellType.BLANK, cell.getCellType(), "[Blank] C7 cell type"); - - // Error - cell = CellUtil.getCell(destRow, col++); - assertEquals(CellType.ERROR, cell.getCellType(), "[Error] D7 cell type"); - final FormulaError error = FormulaError.forInt(cell.getErrorCellValue()); - //FIXME: XSSFCell and HSSFCell expose different interfaces. getErrorCellString would be helpful here - assertEquals(FormulaError.NA, error, "[Error] D7 cell value"); - - // Date - cell = CellUtil.getCell(destRow, col++); - assertEquals(CellType.NUMERIC, cell.getCellType(), "[Date] E7 cell type"); - final Date date = LocaleUtil.getLocaleCalendar(2000, Calendar.JANUARY, 1).getTime(); - assertEquals(date, cell.getDateCellValue(), "[Date] E7 cell value"); - - // Boolean - cell = CellUtil.getCell(destRow, col++); - assertEquals(CellType.BOOLEAN, cell.getCellType(), "[Boolean] F7 cell type"); - assertTrue(cell.getBooleanCellValue(), "[Boolean] F7 cell value"); - - // String - cell = CellUtil.getCell(destRow, col++); - assertEquals(CellType.STRING, cell.getCellType(), "[String] G7 cell type"); - assertEquals("Hello", cell.getStringCellValue(), "[String] G7 cell value"); - - // Int - cell = CellUtil.getCell(destRow, col++); - assertEquals(CellType.NUMERIC, cell.getCellType(), "[Int] H7 cell type"); - assertEquals(15, (int) cell.getNumericCellValue(), "[Int] H7 cell value"); - - // Float - cell = CellUtil.getCell(destRow, col++); - assertEquals(CellType.NUMERIC, cell.getCellType(), "[Float] I7 cell type"); - assertEquals(12.5, cell.getNumericCellValue(), FLOAT_PRECISION, "[Float] I7 cell value"); - - // Cell Formula - cell = CellUtil.getCell(destRow, col++); - assertEquals("Sheet1!J7", new CellReference(cell).formatAsString()); - assertEquals(CellType.FORMULA, cell.getCellType(), "[Cell Formula] J7 cell type"); - assertEquals("5+2", cell.getCellFormula(), "[Cell Formula] J7 cell formula"); - //System.out.println("Cell formula evaluation currently unsupported"); - - // Cell Formula with Reference - // Formula row references should be adjusted by destRowNum-srcRowNum - cell = CellUtil.getCell(destRow, col++); - assertEquals("Sheet1!K7", new CellReference(cell).formatAsString()); - assertEquals(CellType.FORMULA, cell.getCellType(), "[Cell Formula with Reference] K7 cell type"); - assertEquals("J7+H$2", cell.getCellFormula(), "[Cell Formula with Reference] K7 cell formula"); - - // Cell Formula with Reference spanning multiple rows - cell = CellUtil.getCell(destRow, col++); - assertEquals(CellType.FORMULA, cell.getCellType(), "[Cell Formula with Reference spanning multiple rows] L7 cell type"); - assertEquals("G7&\" \"&G8", cell.getCellFormula(), "[Cell Formula with Reference spanning multiple rows] L7 cell formula"); - - // Cell Formula with Reference spanning multiple rows - cell = CellUtil.getCell(destRow, col++); - assertEquals(CellType.FORMULA, cell.getCellType(), "[Cell Formula with Area Reference] M7 cell type"); - assertEquals("SUM(H7:I8)", cell.getCellFormula(), "[Cell Formula with Area Reference] M7 cell formula"); - - // Array Formula - cell = CellUtil.getCell(destRow, col++); - //System.out.println("Array formulas currently unsupported"); - // FIXME: Array Formula set with Sheet.setArrayFormula() instead of cell.setFormula() - /* - assertEquals(CellType.FORMULA, cell.getCellType(), "[Array Formula] N7 cell type"); - assertEquals("{SUM(H7:J7*{1,2,3})}", cell.getCellFormula(), "[Array Formula] N7 cell formula"); - */ - - // Data Format - cell = CellUtil.getCell(destRow, col++); - assertEquals(CellType.NUMERIC, cell.getCellType(), "[Data Format] O7 cell type"); - assertEquals(100.20, cell.getNumericCellValue(), FLOAT_PRECISION, "[Data Format] O7 cell value"); - //FIXME: currently fails - final String moneyFormat = "\"$\"#,##0.00_);[Red]\\(\"$\"#,##0.00\\)"; - assertEquals(moneyFormat, cell.getCellStyle().getDataFormatString(), "[Data Format] O7 data format"); - - // Merged - cell = CellUtil.getCell(destRow, col); - assertEquals("Merged cells", cell.getStringCellValue(), "[Merged] P7:Q7 cell value"); - assertTrue(sheet.getMergedRegions().contains(CellRangeAddress.valueOf("P7:Q7")), "[Merged] P7:Q7 merged region"); - - // Merged across multiple rows - // Microsoft Excel 2013 does not copy a merged region unless all rows of - // the source merged region are selected - // POI's behavior should match this behavior - col += 2; - cell = CellUtil.getCell(destRow, col); - // Note: this behavior deviates from Microsoft Excel, - // which will not overwrite a cell in destination row if merged region extends beyond the copied row. - // The Excel way would require: - //assertEquals("[Merged across multiple rows] R7:S8 merged region", "Should NOT be overwritten", cell.getStringCellValue()); - //assertFalse("[Merged across multiple rows] R7:S8 merged region", - // sheet.getMergedRegions().contains(CellRangeAddress.valueOf("R7:S8"))); - // As currently implemented, cell value is copied but merged region is not copied - assertEquals("Merged cells across multiple rows", cell.getStringCellValue(), "[Merged across multiple rows] R7:S8 cell value"); - // shouldn't do 1-row merge - assertFalse(sheet.getMergedRegions().contains(CellRangeAddress.valueOf("R7:S7")), "[Merged across multiple rows] R7:S7 merged region (one row)"); - //shouldn't do 2-row merge - assertFalse(sheet.getMergedRegions().contains(CellRangeAddress.valueOf("R7:S8")), "[Merged across multiple rows] R7:S8 merged region"); - - // Make sure other rows are blank (off-by-one errors) - assertNull(sheet.getRow(5)); - assertNull(sheet.getRow(7)); - - wb.close(); - } - - private void testCopyMultipleRows(String copyRowsTestWorkbook) throws IOException { - final double FLOAT_PRECISION = 1e-9; - final XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook(copyRowsTestWorkbook); - final XSSFSheet sheet = wb.getSheetAt(0); - final CellCopyPolicy defaultCopyPolicy = new CellCopyPolicy(); - sheet.copyRows(0, 3, 8, defaultCopyPolicy); - - sheet.getRow(0); - final Row srcRow1 = sheet.getRow(1); - final Row srcRow2 = sheet.getRow(2); - final Row srcRow3 = sheet.getRow(3); - final Row destHeaderRow = sheet.getRow(8); - final Row destRow1 = sheet.getRow(9); - final Row destRow2 = sheet.getRow(10); - final Row destRow3 = sheet.getRow(11); - int col = 0; - Cell cell; - - // Header row should be copied - assertNotNull(destHeaderRow); - - // Data rows - cell = CellUtil.getCell(destRow1, col); - assertEquals("Source row ->", cell.getStringCellValue()); - - // Style - col++; - cell = CellUtil.getCell(destRow1, col); - assertEquals("Red", cell.getStringCellValue(), "[Style] B10 cell value"); - assertEquals(CellUtil.getCell(srcRow1, 1).getCellStyle(), cell.getCellStyle(), "[Style] B10 cell style"); - - cell = CellUtil.getCell(destRow2, col); - assertEquals("Blue", cell.getStringCellValue(), "[Style] B11 cell value"); - assertEquals(CellUtil.getCell(srcRow2, 1).getCellStyle(), cell.getCellStyle(), "[Style] B11 cell style"); - - // Blank - col++; - cell = CellUtil.getCell(destRow1, col); - assertEquals(CellType.BLANK, cell.getCellType(), "[Blank] C10 cell type"); - - cell = CellUtil.getCell(destRow2, col); - assertEquals(CellType.BLANK, cell.getCellType(), "[Blank] C11 cell type"); - - // Error - col++; - cell = CellUtil.getCell(destRow1, col); - FormulaError error = FormulaError.forInt(cell.getErrorCellValue()); - //FIXME: XSSFCell and HSSFCell expose different interfaces. getErrorCellString would be helpful here - assertEquals(CellType.ERROR, cell.getCellType(), "[Error] D10 cell type"); - assertEquals(FormulaError.NA, error, "[Error] D10 cell value"); - - cell = CellUtil.getCell(destRow2, col); - error = FormulaError.forInt(cell.getErrorCellValue()); - //FIXME: XSSFCell and HSSFCell expose different interfaces. getErrorCellString would be helpful here - assertEquals(CellType.ERROR, cell.getCellType(), "[Error] D11 cell type"); - assertEquals(FormulaError.NAME, error, "[Error] D11 cell value"); - - // Date - col++; - cell = CellUtil.getCell(destRow1, col); - Date date = LocaleUtil.getLocaleCalendar(2000, Calendar.JANUARY, 1).getTime(); - assertEquals(CellType.NUMERIC, cell.getCellType(), "[Date] E10 cell type"); - assertEquals(date, cell.getDateCellValue(), "[Date] E10 cell value"); - - cell = CellUtil.getCell(destRow2, col); - date = LocaleUtil.getLocaleCalendar(2000, Calendar.JANUARY, 2).getTime(); - assertEquals(CellType.NUMERIC, cell.getCellType(), "[Date] E11 cell type"); - assertEquals(date, cell.getDateCellValue(), "[Date] E11 cell value"); - - // Boolean - col++; - cell = CellUtil.getCell(destRow1, col); - assertEquals(CellType.BOOLEAN, cell.getCellType(), "[Boolean] F10 cell type"); - assertTrue(cell.getBooleanCellValue(), "[Boolean] F10 cell value"); - - cell = CellUtil.getCell(destRow2, col); - assertEquals(CellType.BOOLEAN, cell.getCellType(), "[Boolean] F11 cell type"); - assertFalse(cell.getBooleanCellValue(), "[Boolean] F11 cell value"); - - // String - col++; - cell = CellUtil.getCell(destRow1, col); - assertEquals(CellType.STRING, cell.getCellType(), "[String] G10 cell type"); - assertEquals("Hello", cell.getStringCellValue(), "[String] G10 cell value"); - - cell = CellUtil.getCell(destRow2, col); - assertEquals(CellType.STRING, cell.getCellType(), "[String] G11 cell type"); - assertEquals("World", cell.getStringCellValue(), "[String] G11 cell value"); - - // Int - col++; - cell = CellUtil.getCell(destRow1, col); - assertEquals(CellType.NUMERIC, cell.getCellType(), "[Int] H10 cell type"); - assertEquals(15, (int) cell.getNumericCellValue(), "[Int] H10 cell value"); - - cell = CellUtil.getCell(destRow2, col); - assertEquals(CellType.NUMERIC, cell.getCellType(), "[Int] H11 cell type"); - assertEquals(42, (int) cell.getNumericCellValue(), "[Int] H11 cell value"); - - // Float - col++; - cell = CellUtil.getCell(destRow1, col); - assertEquals(CellType.NUMERIC, cell.getCellType(), "[Float] I10 cell type"); - assertEquals(12.5, cell.getNumericCellValue(), FLOAT_PRECISION, "[Float] I10 cell value"); - - cell = CellUtil.getCell(destRow2, col); - assertEquals(CellType.NUMERIC, cell.getCellType(), "[Float] I11 cell type"); - assertEquals(5.5, cell.getNumericCellValue(), FLOAT_PRECISION, "[Float] I11 cell value"); - - // Cell Formula - col++; - cell = CellUtil.getCell(destRow1, col); - assertEquals(CellType.FORMULA, cell.getCellType(), "[Cell Formula] J10 cell type"); - assertEquals("5+2", cell.getCellFormula(), "[Cell Formula] J10 cell formula"); - - cell = CellUtil.getCell(destRow2, col); - assertEquals(CellType.FORMULA, cell.getCellType(), "[Cell Formula] J11 cell type"); - assertEquals("6+18", cell.getCellFormula(), "[Cell Formula] J11 cell formula"); - - // Cell Formula with Reference - col++; - // Formula row references should be adjusted by destRowNum-srcRowNum - cell = CellUtil.getCell(destRow1, col); - assertEquals(CellType.FORMULA, cell.getCellType(), "[Cell Formula with Reference] K10 cell type"); - assertEquals("J10+H$2", cell.getCellFormula(), "[Cell Formula with Reference] K10 cell formula"); - - cell = CellUtil.getCell(destRow2, col); - assertEquals(CellType.FORMULA, cell.getCellType(), "[Cell Formula with Reference] K11 cell type"); - assertEquals("J11+H$2", cell.getCellFormula(), "[Cell Formula with Reference] K11 cell formula"); - - // Cell Formula with Reference spanning multiple rows - col++; - cell = CellUtil.getCell(destRow1, col); - assertEquals(CellType.FORMULA, cell.getCellType(), "[Cell Formula with Reference spanning multiple rows] L10 cell type"); - assertEquals("G10&\" \"&G11", cell.getCellFormula(), "[Cell Formula with Reference spanning multiple rows] L10 cell formula"); - - cell = CellUtil.getCell(destRow2, col); - assertEquals(CellType.FORMULA, cell.getCellType(), "[Cell Formula with Reference spanning multiple rows] L11 cell type"); - assertEquals("G11&\" \"&G12", cell.getCellFormula(), "[Cell Formula with Reference spanning multiple rows] L11 cell formula"); - - // Cell Formula with Area Reference - col++; - cell = CellUtil.getCell(destRow1, col); - assertEquals(CellType.FORMULA, cell.getCellType(), "[Cell Formula with Area Reference] M10 cell type"); - assertEquals("SUM(H10:I11)", cell.getCellFormula(), "[Cell Formula with Area Reference] M10 cell formula"); - - cell = CellUtil.getCell(destRow2, col); - // Also acceptable: SUM($H10:I$3), but this AreaReference isn't in ascending order - assertEquals(CellType.FORMULA, cell.getCellType(), "[Cell Formula with Area Reference] M11 cell type"); - assertEquals("SUM($H$3:I10)", cell.getCellFormula(), "[Cell Formula with Area Reference] M11 cell formula"); - - // Array Formula - col++; - cell = CellUtil.getCell(destRow1, col); - // System.out.println("Array formulas currently unsupported"); - /* - // FIXME: Array Formula set with Sheet.setArrayFormula() instead of cell.setFormula() - assertEquals(CellType.FORMULA, cell.getCellType(), "[Array Formula] N10 cell type"); - assertEquals("{SUM(H10:J10*{1,2,3})}", cell.getCellFormula(), "[Array Formula] N10 cell formula"); - - cell = CellUtil.getCell(destRow2, col); - // FIXME: Array Formula set with Sheet.setArrayFormula() instead of cell.setFormula() - assertEquals(CellType.FORMULA, cell.getCellType(). "[Array Formula] N11 cell type"); - assertEquals("{SUM(H11:J11*{1,2,3})}", cell.getCellFormula(). "[Array Formula] N11 cell formula"); - */ - - // Data Format - col++; - cell = CellUtil.getCell(destRow2, col); - assertEquals(CellType.NUMERIC, cell.getCellType(), "[Data Format] O10 cell type"); - assertEquals(100.20, cell.getNumericCellValue(), FLOAT_PRECISION, "[Data Format] O10 cell value"); - final String moneyFormat = "\"$\"#,##0.00_);[Red]\\(\"$\"#,##0.00\\)"; - assertEquals(moneyFormat, cell.getCellStyle().getDataFormatString(), "[Data Format] O10 cell data format"); - - // Merged - col++; - cell = CellUtil.getCell(destRow1, col); - assertEquals("Merged cells", cell.getStringCellValue(), "[Merged] P10:Q10 cell value"); - assertTrue(sheet.getMergedRegions().contains(CellRangeAddress.valueOf("P10:Q10")), "[Merged] P10:Q10 merged region"); - - cell = CellUtil.getCell(destRow2, col); - assertEquals("Merged cells", cell.getStringCellValue(), "[Merged] P11:Q11 cell value"); - assertTrue(sheet.getMergedRegions().contains(CellRangeAddress.valueOf("P11:Q11")), "[Merged] P11:Q11 merged region"); - - // Should Q10/Q11 be checked? - - // Merged across multiple rows - // Microsoft Excel 2013 does not copy a merged region unless all rows of - // the source merged region are selected - // POI's behavior should match this behavior - col += 2; - cell = CellUtil.getCell(destRow1, col); - assertEquals("Merged cells across multiple rows", cell.getStringCellValue(), "[Merged across multiple rows] R10:S11 cell value"); - assertTrue(sheet.getMergedRegions().contains(CellRangeAddress.valueOf("R10:S11")), "[Merged across multiple rows] R10:S11 merged region"); - - // Row 3 (zero-based) was empty, so Row 11 (zero-based) should be empty too. - if (srcRow3 == null) { - assertNull(destRow3, "Row 3 was empty, so Row 11 should be empty"); - } - - // Make sure other rows are blank (off-by-one errors) - assertNull(sheet.getRow(7), "Off-by-one lower edge case"); //one row above destHeaderRow - assertNull(sheet.getRow(12), "Off-by-one upper edge case"); //one row below destRow3 - - wb.close(); - } - @Test void testCopyOneRow() throws IOException { - testCopyOneRow("XSSFSheet.copyRows.xlsx"); + final double FLOAT_PRECISION = 1e-9; + try (final XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("XSSFSheet.copyRows.xlsx")) { + final XSSFSheet sheet = wb.getSheetAt(0); + final CellCopyPolicy defaultCopyPolicy = new CellCopyPolicy(); + sheet.copyRows(1, 1, 6, defaultCopyPolicy); + + final Row srcRow = sheet.getRow(1); + final Row destRow = sheet.getRow(6); + int col = 0; + Cell cell; + + cell = CellUtil.getCell(destRow, col++); + assertEquals("Source row ->", cell.getStringCellValue()); + + // Style + cell = CellUtil.getCell(destRow, col++); + assertEquals("Red", cell.getStringCellValue(), "[Style] B7 cell value"); + assertEquals(CellUtil.getCell(srcRow, 1).getCellStyle(), cell.getCellStyle(), "[Style] B7 cell style"); + + // Blank + cell = CellUtil.getCell(destRow, col++); + assertEquals(CellType.BLANK, cell.getCellType(), "[Blank] C7 cell type"); + + // Error + cell = CellUtil.getCell(destRow, col++); + assertEquals(CellType.ERROR, cell.getCellType(), "[Error] D7 cell type"); + final FormulaError error = FormulaError.forInt(cell.getErrorCellValue()); + //FIXME: XSSFCell and HSSFCell expose different interfaces. getErrorCellString would be helpful here + assertEquals(FormulaError.NA, error, "[Error] D7 cell value"); + + // Date + cell = CellUtil.getCell(destRow, col++); + assertEquals(CellType.NUMERIC, cell.getCellType(), "[Date] E7 cell type"); + final Date date = LocaleUtil.getLocaleCalendar(2000, Calendar.JANUARY, 1).getTime(); + assertEquals(date, cell.getDateCellValue(), "[Date] E7 cell value"); + + // Boolean + cell = CellUtil.getCell(destRow, col++); + assertEquals(CellType.BOOLEAN, cell.getCellType(), "[Boolean] F7 cell type"); + assertTrue(cell.getBooleanCellValue(), "[Boolean] F7 cell value"); + + // String + cell = CellUtil.getCell(destRow, col++); + assertEquals(CellType.STRING, cell.getCellType(), "[String] G7 cell type"); + assertEquals("Hello", cell.getStringCellValue(), "[String] G7 cell value"); + + // Int + cell = CellUtil.getCell(destRow, col++); + assertEquals(CellType.NUMERIC, cell.getCellType(), "[Int] H7 cell type"); + assertEquals(15, (int) cell.getNumericCellValue(), "[Int] H7 cell value"); + + // Float + cell = CellUtil.getCell(destRow, col++); + assertEquals(CellType.NUMERIC, cell.getCellType(), "[Float] I7 cell type"); + assertEquals(12.5, cell.getNumericCellValue(), FLOAT_PRECISION, "[Float] I7 cell value"); + + // Cell Formula + cell = CellUtil.getCell(destRow, col++); + assertEquals("Sheet1!J7", new CellReference(cell).formatAsString()); + assertEquals(CellType.FORMULA, cell.getCellType(), "[Cell Formula] J7 cell type"); + assertEquals("5+2", cell.getCellFormula(), "[Cell Formula] J7 cell formula"); + //System.out.println("Cell formula evaluation currently unsupported"); + + // Cell Formula with Reference + // Formula row references should be adjusted by destRowNum-srcRowNum + cell = CellUtil.getCell(destRow, col++); + assertEquals("Sheet1!K7", new CellReference(cell).formatAsString()); + assertEquals(CellType.FORMULA, cell.getCellType(), "[Cell Formula with Reference] K7 cell type"); + assertEquals("J7+H$2", cell.getCellFormula(), "[Cell Formula with Reference] K7 cell formula"); + + // Cell Formula with Reference spanning multiple rows + cell = CellUtil.getCell(destRow, col++); + assertEquals(CellType.FORMULA, cell.getCellType(), "[Cell Formula with Reference spanning multiple rows] L7 cell type"); + assertEquals("G7&\" \"&G8", cell.getCellFormula(), "[Cell Formula with Reference spanning multiple rows] L7 cell formula"); + + // Cell Formula with Reference spanning multiple rows + cell = CellUtil.getCell(destRow, col++); + assertEquals(CellType.FORMULA, cell.getCellType(), "[Cell Formula with Area Reference] M7 cell type"); + assertEquals("SUM(H7:I8)", cell.getCellFormula(), "[Cell Formula with Area Reference] M7 cell formula"); + + // Array Formula + col++; + // cell = CellUtil.getCell(destRow, col++); + // Array formulas currently unsupported" + // FIXME: Array Formula set with Sheet.setArrayFormula() instead of cell.setFormula() + // assertEquals(CellType.FORMULA, cell.getCellType(), "[Array Formula] N7 cell type"); + // assertEquals("{SUM(H7:J7*{1,2,3})}", cell.getCellFormula(), "[Array Formula] N7 cell formula"); + + // Data Format + cell = CellUtil.getCell(destRow, col++); + assertEquals(CellType.NUMERIC, cell.getCellType(), "[Data Format] O7 cell type"); + assertEquals(100.20, cell.getNumericCellValue(), FLOAT_PRECISION, "[Data Format] O7 cell value"); + //FIXME: currently fails + final String moneyFormat = "\"$\"#,##0.00_);[Red]\\(\"$\"#,##0.00\\)"; + assertEquals(moneyFormat, cell.getCellStyle().getDataFormatString(), "[Data Format] O7 data format"); + + // Merged + cell = CellUtil.getCell(destRow, col); + assertEquals("Merged cells", cell.getStringCellValue(), "[Merged] P7:Q7 cell value"); + assertTrue(sheet.getMergedRegions().contains(CellRangeAddress.valueOf("P7:Q7")), "[Merged] P7:Q7 merged region"); + + // Merged across multiple rows + // Microsoft Excel 2013 does not copy a merged region unless all rows of + // the source merged region are selected + // POI's behavior should match this behavior + col += 2; + cell = CellUtil.getCell(destRow, col); + // Note: this behavior deviates from Microsoft Excel, + // which will not overwrite a cell in destination row if merged region extends beyond the copied row. + // The Excel way would require: + //assertEquals("[Merged across multiple rows] R7:S8 merged region", "Should NOT be overwritten", cell.getStringCellValue()); + //assertFalse("[Merged across multiple rows] R7:S8 merged region", + // sheet.getMergedRegions().contains(CellRangeAddress.valueOf("R7:S8"))); + // As currently implemented, cell value is copied but merged region is not copied + assertEquals("Merged cells across multiple rows", cell.getStringCellValue(), "[Merged across multiple rows] R7:S8 cell value"); + // shouldn't do 1-row merge + assertFalse(sheet.getMergedRegions().contains(CellRangeAddress.valueOf("R7:S7")), "[Merged across multiple rows] R7:S7 merged region (one row)"); + //shouldn't do 2-row merge + assertFalse(sheet.getMergedRegions().contains(CellRangeAddress.valueOf("R7:S8")), "[Merged across multiple rows] R7:S8 merged region"); + + // Make sure other rows are blank (off-by-one errors) + assertNull(sheet.getRow(5)); + assertNull(sheet.getRow(7)); + } } @Test void testCopyMultipleRows() throws IOException { - testCopyMultipleRows("XSSFSheet.copyRows.xlsx"); + final double FLOAT_PRECISION = 1e-9; + try (XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("XSSFSheet.copyRows.xlsx")) { + final XSSFSheet sheet = wb.getSheetAt(0); + final CellCopyPolicy defaultCopyPolicy = new CellCopyPolicy(); + sheet.copyRows(0, 3, 8, defaultCopyPolicy); + + sheet.getRow(0); + final Row srcRow1 = sheet.getRow(1); + final Row srcRow2 = sheet.getRow(2); + final Row srcRow3 = sheet.getRow(3); + final Row destHeaderRow = sheet.getRow(8); + final Row destRow1 = sheet.getRow(9); + final Row destRow2 = sheet.getRow(10); + final Row destRow3 = sheet.getRow(11); + int col = 0; + Cell cell; + + // Header row should be copied + assertNotNull(destHeaderRow); + + // Data rows + cell = CellUtil.getCell(destRow1, col); + assertEquals("Source row ->", cell.getStringCellValue()); + + // Style + col++; + cell = CellUtil.getCell(destRow1, col); + assertEquals("Red", cell.getStringCellValue(), "[Style] B10 cell value"); + assertEquals(CellUtil.getCell(srcRow1, 1).getCellStyle(), cell.getCellStyle(), "[Style] B10 cell style"); + + cell = CellUtil.getCell(destRow2, col); + assertEquals("Blue", cell.getStringCellValue(), "[Style] B11 cell value"); + assertEquals(CellUtil.getCell(srcRow2, 1).getCellStyle(), cell.getCellStyle(), "[Style] B11 cell style"); + + // Blank + col++; + cell = CellUtil.getCell(destRow1, col); + assertEquals(CellType.BLANK, cell.getCellType(), "[Blank] C10 cell type"); + + cell = CellUtil.getCell(destRow2, col); + assertEquals(CellType.BLANK, cell.getCellType(), "[Blank] C11 cell type"); + + // Error + col++; + cell = CellUtil.getCell(destRow1, col); + FormulaError error = FormulaError.forInt(cell.getErrorCellValue()); + //FIXME: XSSFCell and HSSFCell expose different interfaces. getErrorCellString would be helpful here + assertEquals(CellType.ERROR, cell.getCellType(), "[Error] D10 cell type"); + assertEquals(FormulaError.NA, error, "[Error] D10 cell value"); + + cell = CellUtil.getCell(destRow2, col); + error = FormulaError.forInt(cell.getErrorCellValue()); + //FIXME: XSSFCell and HSSFCell expose different interfaces. getErrorCellString would be helpful here + assertEquals(CellType.ERROR, cell.getCellType(), "[Error] D11 cell type"); + assertEquals(FormulaError.NAME, error, "[Error] D11 cell value"); + + // Date + col++; + cell = CellUtil.getCell(destRow1, col); + Date date = LocaleUtil.getLocaleCalendar(2000, Calendar.JANUARY, 1).getTime(); + assertEquals(CellType.NUMERIC, cell.getCellType(), "[Date] E10 cell type"); + assertEquals(date, cell.getDateCellValue(), "[Date] E10 cell value"); + + cell = CellUtil.getCell(destRow2, col); + date = LocaleUtil.getLocaleCalendar(2000, Calendar.JANUARY, 2).getTime(); + assertEquals(CellType.NUMERIC, cell.getCellType(), "[Date] E11 cell type"); + assertEquals(date, cell.getDateCellValue(), "[Date] E11 cell value"); + + // Boolean + col++; + cell = CellUtil.getCell(destRow1, col); + assertEquals(CellType.BOOLEAN, cell.getCellType(), "[Boolean] F10 cell type"); + assertTrue(cell.getBooleanCellValue(), "[Boolean] F10 cell value"); + + cell = CellUtil.getCell(destRow2, col); + assertEquals(CellType.BOOLEAN, cell.getCellType(), "[Boolean] F11 cell type"); + assertFalse(cell.getBooleanCellValue(), "[Boolean] F11 cell value"); + + // String + col++; + cell = CellUtil.getCell(destRow1, col); + assertEquals(CellType.STRING, cell.getCellType(), "[String] G10 cell type"); + assertEquals("Hello", cell.getStringCellValue(), "[String] G10 cell value"); + + cell = CellUtil.getCell(destRow2, col); + assertEquals(CellType.STRING, cell.getCellType(), "[String] G11 cell type"); + assertEquals("World", cell.getStringCellValue(), "[String] G11 cell value"); + + // Int + col++; + cell = CellUtil.getCell(destRow1, col); + assertEquals(CellType.NUMERIC, cell.getCellType(), "[Int] H10 cell type"); + assertEquals(15, (int) cell.getNumericCellValue(), "[Int] H10 cell value"); + + cell = CellUtil.getCell(destRow2, col); + assertEquals(CellType.NUMERIC, cell.getCellType(), "[Int] H11 cell type"); + assertEquals(42, (int) cell.getNumericCellValue(), "[Int] H11 cell value"); + + // Float + col++; + cell = CellUtil.getCell(destRow1, col); + assertEquals(CellType.NUMERIC, cell.getCellType(), "[Float] I10 cell type"); + assertEquals(12.5, cell.getNumericCellValue(), FLOAT_PRECISION, "[Float] I10 cell value"); + + cell = CellUtil.getCell(destRow2, col); + assertEquals(CellType.NUMERIC, cell.getCellType(), "[Float] I11 cell type"); + assertEquals(5.5, cell.getNumericCellValue(), FLOAT_PRECISION, "[Float] I11 cell value"); + + // Cell Formula + col++; + cell = CellUtil.getCell(destRow1, col); + assertEquals(CellType.FORMULA, cell.getCellType(), "[Cell Formula] J10 cell type"); + assertEquals("5+2", cell.getCellFormula(), "[Cell Formula] J10 cell formula"); + + cell = CellUtil.getCell(destRow2, col); + assertEquals(CellType.FORMULA, cell.getCellType(), "[Cell Formula] J11 cell type"); + assertEquals("6+18", cell.getCellFormula(), "[Cell Formula] J11 cell formula"); + + // Cell Formula with Reference + col++; + // Formula row references should be adjusted by destRowNum-srcRowNum + cell = CellUtil.getCell(destRow1, col); + assertEquals(CellType.FORMULA, cell.getCellType(), "[Cell Formula with Reference] K10 cell type"); + assertEquals("J10+H$2", cell.getCellFormula(), "[Cell Formula with Reference] K10 cell formula"); + + cell = CellUtil.getCell(destRow2, col); + assertEquals(CellType.FORMULA, cell.getCellType(), "[Cell Formula with Reference] K11 cell type"); + assertEquals("J11+H$2", cell.getCellFormula(), "[Cell Formula with Reference] K11 cell formula"); + + // Cell Formula with Reference spanning multiple rows + col++; + cell = CellUtil.getCell(destRow1, col); + assertEquals(CellType.FORMULA, cell.getCellType(), "[Cell Formula with Reference spanning multiple rows] L10 cell type"); + assertEquals("G10&\" \"&G11", cell.getCellFormula(), "[Cell Formula with Reference spanning multiple rows] L10 cell formula"); + + cell = CellUtil.getCell(destRow2, col); + assertEquals(CellType.FORMULA, cell.getCellType(), "[Cell Formula with Reference spanning multiple rows] L11 cell type"); + assertEquals("G11&\" \"&G12", cell.getCellFormula(), "[Cell Formula with Reference spanning multiple rows] L11 cell formula"); + + // Cell Formula with Area Reference + col++; + cell = CellUtil.getCell(destRow1, col); + assertEquals(CellType.FORMULA, cell.getCellType(), "[Cell Formula with Area Reference] M10 cell type"); + assertEquals("SUM(H10:I11)", cell.getCellFormula(), "[Cell Formula with Area Reference] M10 cell formula"); + + cell = CellUtil.getCell(destRow2, col); + // Also acceptable: SUM($H10:I$3), but this AreaReference isn't in ascending order + assertEquals(CellType.FORMULA, cell.getCellType(), "[Cell Formula with Area Reference] M11 cell type"); + assertEquals("SUM($H$3:I10)", cell.getCellFormula(), "[Cell Formula with Area Reference] M11 cell formula"); + + // Array Formula + col++; + // cell = CellUtil.getCell(destRow1, col); + // Array formulas currently unsupported + // FIXME: Array Formula set with Sheet.setArrayFormula() instead of cell.setFormula() + // assertEquals(CellType.FORMULA, cell.getCellType(), "[Array Formula] N10 cell type"); + // assertEquals("{SUM(H10:J10*{1,2,3})}", cell.getCellFormula(), "[Array Formula] N10 cell formula"); + + // cell = CellUtil.getCell(destRow2, col); + // FIXME: Array Formula set with Sheet.setArrayFormula() instead of cell.setFormula() + // assertEquals(CellType.FORMULA, cell.getCellType(). "[Array Formula] N11 cell type"); + // assertEquals("{SUM(H11:J11*{1,2,3})}", cell.getCellFormula(). "[Array Formula] N11 cell formula"); + + // Data Format + col++; + cell = CellUtil.getCell(destRow2, col); + assertEquals(CellType.NUMERIC, cell.getCellType(), "[Data Format] O10 cell type"); + assertEquals(100.20, cell.getNumericCellValue(), FLOAT_PRECISION, "[Data Format] O10 cell value"); + final String moneyFormat = "\"$\"#,##0.00_);[Red]\\(\"$\"#,##0.00\\)"; + assertEquals(moneyFormat, cell.getCellStyle().getDataFormatString(), "[Data Format] O10 cell data format"); + + // Merged + col++; + cell = CellUtil.getCell(destRow1, col); + assertEquals("Merged cells", cell.getStringCellValue(), "[Merged] P10:Q10 cell value"); + assertTrue(sheet.getMergedRegions().contains(CellRangeAddress.valueOf("P10:Q10")), "[Merged] P10:Q10 merged region"); + + cell = CellUtil.getCell(destRow2, col); + assertEquals("Merged cells", cell.getStringCellValue(), "[Merged] P11:Q11 cell value"); + assertTrue(sheet.getMergedRegions().contains(CellRangeAddress.valueOf("P11:Q11")), "[Merged] P11:Q11 merged region"); + + // Should Q10/Q11 be checked? + + // Merged across multiple rows + // Microsoft Excel 2013 does not copy a merged region unless all rows of + // the source merged region are selected + // POI's behavior should match this behavior + col += 2; + cell = CellUtil.getCell(destRow1, col); + assertEquals("Merged cells across multiple rows", cell.getStringCellValue(), "[Merged across multiple rows] R10:S11 cell value"); + assertTrue(sheet.getMergedRegions().contains(CellRangeAddress.valueOf("R10:S11")), "[Merged across multiple rows] R10:S11 merged region"); + + // Row 3 (zero-based) was empty, so Row 11 (zero-based) should be empty too. + if (srcRow3 == null) { + assertNull(destRow3, "Row 3 was empty, so Row 11 should be empty"); + } + + // Make sure other rows are blank (off-by-one errors) + assertNull(sheet.getRow(7), "Off-by-one lower edge case"); //one row above destHeaderRow + assertNull(sheet.getRow(12), "Off-by-one upper edge case"); //one row below destRow3 + } } @Test @@ -1998,26 +1985,34 @@ public final class TestXSSFSheet extends BaseTestXSheet { @Test public void bug65120() throws IOException { - XSSFWorkbook wb = new XSSFWorkbook(); - CreationHelper creationHelper = wb.getCreationHelper(); + try (XSSFWorkbook wb = new XSSFWorkbook()) { + XSSFCreationHelper creationHelper = wb.getCreationHelper(); - Sheet sheet1 = wb.createSheet(); - Cell cell1 = sheet1.createRow(0).createCell(0); - Comment comment1 = sheet1.createDrawingPatriarch().createCellComment(creationHelper.createClientAnchor()); - cell1.setCellComment(comment1); + XSSFSheet sheet1 = wb.createSheet(); + Cell cell1 = sheet1.createRow(0).createCell(0); + XSSFDrawing dwg1 = sheet1.createDrawingPatriarch(); + XSSFComment comment1 = dwg1.createCellComment(creationHelper.createClientAnchor()); + cell1.setCellComment(comment1); + XSSFVMLDrawing vml0 = sheet1.getVMLDrawing(false); + assertEquals("/xl/drawings/vmlDrawing0.vml", vml0.getPackagePart().getPartName().getName()); - Sheet sheet2 = wb.createSheet(); - Cell cell2 = sheet2.createRow(0).createCell(0); - Comment comment2 = sheet2.createDrawingPatriarch().createCellComment(creationHelper.createClientAnchor()); - cell2.setCellComment(comment2); + XSSFSheet sheet2 = wb.createSheet(); + Cell cell2 = sheet2.createRow(0).createCell(0); + XSSFDrawing dwg2 = sheet2.createDrawingPatriarch(); + Comment comment2 = dwg2.createCellComment(creationHelper.createClientAnchor()); + cell2.setCellComment(comment2); + XSSFVMLDrawing vml1 = sheet2.getVMLDrawing(false); + assertEquals("/xl/drawings/vmlDrawing1.vml", vml1.getPackagePart().getPartName().getName()); - wb.removeSheetAt(0); + wb.removeSheetAt(0); - Sheet sheet3 = wb.createSheet(); - Cell cell3 = sheet3.createRow(0).createCell(0); - Comment comment3 = sheet3.createDrawingPatriarch().createCellComment(creationHelper.createClientAnchor()); - cell3.setCellComment(comment3); - - wb.close(); + XSSFSheet sheet3 = wb.createSheet(); + Cell cell3 = sheet3.createRow(0).createCell(0); + XSSFDrawing dwg3 = sheet3.createDrawingPatriarch(); + Comment comment3 = dwg3.createCellComment(creationHelper.createClientAnchor()); + cell3.setCellComment(comment3); + XSSFVMLDrawing vml2 = sheet3.getVMLDrawing(false); + assertEquals("/xl/drawings/vmlDrawing2.vml", vml2.getPackagePart().getPartName().getName()); + } } } diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hemf/record/emfplus/HemfPlusBrush.java b/poi-scratchpad/src/main/java/org/apache/poi/hemf/record/emfplus/HemfPlusBrush.java index 9739635c21..0e02d12e8e 100644 --- a/poi-scratchpad/src/main/java/org/apache/poi/hemf/record/emfplus/HemfPlusBrush.java +++ b/poi-scratchpad/src/main/java/org/apache/poi/hemf/record/emfplus/HemfPlusBrush.java @@ -399,18 +399,17 @@ public class HemfPlusBrush { } public byte[] getRawData(List continuedObjectData) { - UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream(); - try { + try (UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream()) { bos.write(getBrushBytes()); if (continuedObjectData != null) { for (EmfPlusObjectData od : continuedObjectData) { bos.write(((EmfPlusBrush)od).getBrushBytes()); } } + return bos.toByteArray(); } catch (IOException e) { throw new RuntimeException(e); } - return bos.toByteArray(); } diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hemf/record/emfplus/HemfPlusImage.java b/poi-scratchpad/src/main/java/org/apache/poi/hemf/record/emfplus/HemfPlusImage.java index 1ea137dc1a..4b0a402e13 100644 --- a/poi-scratchpad/src/main/java/org/apache/poi/hemf/record/emfplus/HemfPlusImage.java +++ b/poi-scratchpad/src/main/java/org/apache/poi/hemf/record/emfplus/HemfPlusImage.java @@ -445,18 +445,17 @@ public class HemfPlusImage { } public byte[] getRawData(List continuedObjectData) { - UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream(); - try { + try (UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream()) { bos.write(getImageData()); if (continuedObjectData != null) { for (EmfPlusObjectData od : continuedObjectData) { bos.write(((EmfPlusImage)od).getImageData()); } } + return bos.toByteArray(); } catch (IOException e) { throw new RuntimeException(e); } - return bos.toByteArray(); } @Override diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hemf/usermodel/HemfEmbeddedIterator.java b/poi-scratchpad/src/main/java/org/apache/poi/hemf/usermodel/HemfEmbeddedIterator.java index f94c05a598..da67f62fa5 100644 --- a/poi-scratchpad/src/main/java/org/apache/poi/hemf/usermodel/HemfEmbeddedIterator.java +++ b/poi-scratchpad/src/main/java/org/apache/poi/hemf/usermodel/HemfEmbeddedIterator.java @@ -35,6 +35,7 @@ import org.apache.poi.hemf.record.emf.HemfComment.EmfCommentDataMultiformats; import org.apache.poi.hemf.record.emf.HemfComment.EmfCommentDataPlus; import org.apache.poi.hemf.record.emf.HemfComment.EmfCommentDataWMF; import org.apache.poi.hemf.record.emf.HemfRecord; +import org.apache.poi.hemf.record.emfplus.HemfPlusImage; import org.apache.poi.hemf.record.emfplus.HemfPlusImage.EmfPlusBitmapDataType; import org.apache.poi.hemf.record.emfplus.HemfPlusImage.EmfPlusImage; import org.apache.poi.hemf.record.emfplus.HemfPlusObject.EmfPlusObject; @@ -63,6 +64,10 @@ public class HemfEmbeddedIterator implements Iterator { @Override public boolean hasNext() { + return moveNext(); + } + + private boolean moveNext() { if (iterStack.isEmpty()) { return false; } @@ -290,36 +295,33 @@ public class HemfEmbeddedIterator implements Iterator { final int objectId = epo.getObjectId(); - HwmfEmbedded emb = new HwmfEmbedded(); + final HwmfEmbedded emb = new HwmfEmbedded(); - EmfPlusImage img = epo.getObjectData(); - assert(img.getImageDataType() != null); - - int totalSize = epo.getTotalObjectSize(); + // totalSize is only set, if there are multiple chunks + final int totalSize = epo.getTotalObjectSize() == 0 + ? ((EmfPlusImage)epo.getObjectData()).getImageData().length + : epo.getTotalObjectSize(); IOUtils.safelyAllocateCheck(totalSize, MAX_RECORD_LENGTH); - UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream(epo.getTotalObjectSize()); - try { - for (;;) { + try (UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream(totalSize)) { + boolean hasNext = false; + do { + EmfPlusImage img = epo.getObjectData(); + assert(img.getImageDataType() != null); + assert(!hasNext || img.getImageDataType() == HemfPlusImage.EmfPlusImageDataType.CONTINUED); bos.write(img.getImageData()); - current = null; - //noinspection ConstantConditions - if (hasNext() && + hasNext = moveNext() && (current instanceof EmfPlusObject) && ((epo = (EmfPlusObject) current).getObjectId() == objectId) && - bos.size() < totalSize-16 - ) { - img = epo.getObjectData(); - } else { - return emb; - } - } + bos.size() < totalSize-16; + } while (hasNext); + + emb.setData(bos.toByteArray()); + return emb; } catch (IOException ignored) { // UnsynchronizedByteArrayOutputStream doesn't throw IOException return null; - } finally { - emb.setData(bos.toByteArray()); } } } diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hslf/blip/PICT.java b/poi-scratchpad/src/main/java/org/apache/poi/hslf/blip/PICT.java index dc1e93cc67..5bed94a34d 100644 --- a/poi-scratchpad/src/main/java/org/apache/poi/hslf/blip/PICT.java +++ b/poi-scratchpad/src/main/java/org/apache/poi/hslf/blip/PICT.java @@ -70,9 +70,8 @@ public final class PICT extends Metafile { @Override public byte[] getData(){ byte[] rawdata = getRawData(); - try { + try (UnsynchronizedByteArrayOutputStream out = new UnsynchronizedByteArrayOutputStream()) { byte[] macheader = new byte[512]; - UnsynchronizedByteArrayOutputStream out = new UnsynchronizedByteArrayOutputStream(); out.write(macheader); int pos = CHECKSUM_SIZE*getUIDInstanceCount(); byte[] pict = read(rawdata, pos); @@ -93,30 +92,33 @@ public final class PICT extends Metafile { throw new EOFException(); } byte[] chunk = new byte[4096]; - UnsynchronizedByteArrayOutputStream out = new UnsynchronizedByteArrayOutputStream(header.getWmfSize()); - try (InflaterInputStream inflater = new InflaterInputStream(bis)) { - int count; - while ((count = inflater.read(chunk)) >= 0) { - out.write(chunk, 0, count); - // PICT zip-stream can be erroneous, so we clear the array to determine - // the maximum of read bytes, after the inflater crashed - bytefill(chunk, (byte) 0); - } - } catch (Exception e) { - int lastLen; - for (lastLen = chunk.length - 1; lastLen >= 0 && chunk[lastLen] == 0; lastLen--) ; - if (++lastLen > 0) { - if (header.getWmfSize() > out.size()) { - // sometimes the wmfsize is smaller than the amount of already successfully read bytes - // in this case we take the lastLen as-is, otherwise we truncate it to the given size - lastLen = Math.min(lastLen, header.getWmfSize() - out.size()); + try (UnsynchronizedByteArrayOutputStream out = new UnsynchronizedByteArrayOutputStream(header.getWmfSize())) { + try (InflaterInputStream inflater = new InflaterInputStream(bis)) { + int count; + while ((count = inflater.read(chunk)) >= 0) { + out.write(chunk, 0, count); + // PICT zip-stream can be erroneous, so we clear the array to determine + // the maximum of read bytes, after the inflater crashed + bytefill(chunk, (byte) 0); } - out.write(chunk, 0, lastLen); + } catch (Exception e) { + int lastLen = chunk.length - 1; + while (lastLen >= 0 && chunk[lastLen] == 0) { + lastLen--; + } + if (++lastLen > 0) { + if (header.getWmfSize() > out.size()) { + // sometimes the wmfsize is smaller than the amount of already successfully read bytes + // in this case we take the lastLen as-is, otherwise we truncate it to the given size + lastLen = Math.min(lastLen, header.getWmfSize() - out.size()); + } + out.write(chunk, 0, lastLen); + } + // End of picture marker for PICT is 0x00 0xFF + LOG.atError().withThrowable(e).log("PICT zip-stream is invalid, read as much as possible. Uncompressed length of header: {} / Read bytes: {}", box(header.getWmfSize()), box(out.size())); } - // End of picture marker for PICT is 0x00 0xFF - LOG.atError().withThrowable(e).log("PICT zip-stream is invalid, read as much as possible. Uncompressed length of header: {} / Read bytes: {}", box(header.getWmfSize()),box(out.size())); + return out.toByteArray(); } - return out.toByteArray(); } @Override diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/EscherTextboxWrapper.java b/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/EscherTextboxWrapper.java index 7f9b70bcfc..4164158e5f 100644 --- a/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/EscherTextboxWrapper.java +++ b/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/EscherTextboxWrapper.java @@ -89,12 +89,13 @@ public final class EscherTextboxWrapper extends RecordContainer { // Write out our children, and stuff them into the Escher layer // Grab the children's data - UnsynchronizedByteArrayOutputStream baos = new UnsynchronizedByteArrayOutputStream(); - for (org.apache.poi.hslf.record.Record r : _children) r.writeOut(baos); - byte[] data = baos.toByteArray(); - - // Save in the escher layer - _escherRecord.setData(data); + try (UnsynchronizedByteArrayOutputStream baos = new UnsynchronizedByteArrayOutputStream()) { + for (org.apache.poi.hslf.record.Record r : _children) { + r.writeOut(baos); + } + // Save in the escher layer + _escherRecord.setData(baos.toByteArray()); + } } /** diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/ExOleObjStg.java b/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/ExOleObjStg.java index 8aa091fdb2..d4366fa436 100644 --- a/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/ExOleObjStg.java +++ b/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/ExOleObjStg.java @@ -27,8 +27,8 @@ import java.util.function.Supplier; import java.util.zip.DeflaterOutputStream; import java.util.zip.InflaterInputStream; -import org.apache.commons.io.output.UnsynchronizedByteArrayOutputStream; import org.apache.commons.io.input.BoundedInputStream; +import org.apache.commons.io.output.UnsynchronizedByteArrayOutputStream; import org.apache.poi.util.GenericRecordUtil; import org.apache.poi.util.IOUtils; import org.apache.poi.util.LittleEndian; @@ -124,18 +124,19 @@ public class ExOleObjStg extends PositionDependentRecordAtom implements PersistR * @param data the embedded data. */ public void setData(byte[] data) throws IOException { - UnsynchronizedByteArrayOutputStream out = new UnsynchronizedByteArrayOutputStream(); - //first four bytes is the length of the raw data - byte[] b = new byte[4]; - LittleEndian.putInt(b, 0, data.length); - out.write(b); + try (UnsynchronizedByteArrayOutputStream out = new UnsynchronizedByteArrayOutputStream(); + DeflaterOutputStream def = new DeflaterOutputStream(out)) { + //first four bytes is the length of the raw data + byte[] b = new byte[4]; + LittleEndian.putInt(b, 0, data.length); + out.write(b); - DeflaterOutputStream def = new DeflaterOutputStream(out); - def.write(data, 0, data.length); - def.finish(); - // TODO: CHECK if it's correct that DeflaterOutputStream is only finished and not closed? - _data = out.toByteArray(); - LittleEndian.putInt(_header, 4, _data.length); + def.write(data, 0, data.length); + def.finish(); + // TODO: CHECK if it's correct that DeflaterOutputStream is only finished and not closed? + _data = out.toByteArray(); + LittleEndian.putInt(_header, 4, _data.length); + } } /** diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/PPDrawingGroup.java b/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/PPDrawingGroup.java index 3bc4f0fe57..957edcb945 100644 --- a/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/PPDrawingGroup.java +++ b/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/PPDrawingGroup.java @@ -49,7 +49,7 @@ public final class PPDrawingGroup extends RecordAtom { //cached dgg private EscherDggRecord dgg; - protected PPDrawingGroup(byte[] source, int start, int len) { + PPDrawingGroup(byte[] source, int start, int len) { // Get the header _header = Arrays.copyOfRange(source, start, start+8); @@ -80,44 +80,44 @@ public final class PPDrawingGroup extends RecordAtom { @Override public void writeOut(OutputStream out) throws IOException { - UnsynchronizedByteArrayOutputStream bout = new UnsynchronizedByteArrayOutputStream(); - for (EscherRecord r : dggContainer) { - if (r.getRecordId() == EscherContainerRecord.BSTORE_CONTAINER){ - EscherContainerRecord bstore = (EscherContainerRecord)r; - - UnsynchronizedByteArrayOutputStream b2 = new UnsynchronizedByteArrayOutputStream(); - for (EscherRecord br : bstore) { - byte[] b = new byte[36+8]; - br.serialize(0, b); - b2.write(b); + byte[] bstorehead = new byte[8]; + byte[] recordBytes = new byte[36 + 8]; + try (UnsynchronizedByteArrayOutputStream bout = new UnsynchronizedByteArrayOutputStream(); + UnsynchronizedByteArrayOutputStream recordBuf = new UnsynchronizedByteArrayOutputStream()) { + for (EscherRecord r : dggContainer) { + if (r.getRecordId() == EscherContainerRecord.BSTORE_CONTAINER) { + EscherContainerRecord bstore = (EscherContainerRecord) r; + recordBuf.reset(); + for (EscherRecord br : bstore) { + br.serialize(0, recordBytes); + recordBuf.write(recordBytes); + } + LittleEndian.putShort(bstorehead, 0, bstore.getOptions()); + LittleEndian.putShort(bstorehead, 2, bstore.getRecordId()); + LittleEndian.putInt(bstorehead, 4, recordBuf.size()); + bout.write(bstorehead); + recordBuf.writeTo(bout); + } else { + bout.write(r.serialize()); } - byte[] bstorehead = new byte[8]; - LittleEndian.putShort(bstorehead, 0, bstore.getOptions()); - LittleEndian.putShort(bstorehead, 2, bstore.getRecordId()); - LittleEndian.putInt(bstorehead, 4, b2.size()); - bout.write(bstorehead); - bout.write(b2.toByteArray()); - - } else { - bout.write(r.serialize()); } + int size = bout.size(); + + // Update the size (header bytes 5-8) + LittleEndian.putInt(_header, 4, size + 8); + + // Write out our header + out.write(_header); + + byte[] dgghead = new byte[8]; + LittleEndian.putShort(dgghead, 0, dggContainer.getOptions()); + LittleEndian.putShort(dgghead, 2, dggContainer.getRecordId()); + LittleEndian.putInt(dgghead, 4, size); + out.write(dgghead); + + // Finally, write out the children + bout.writeTo(out); } - int size = bout.size(); - - // Update the size (header bytes 5-8) - LittleEndian.putInt(_header,4,size+8); - - // Write out our header - out.write(_header); - - byte[] dgghead = new byte[8]; - LittleEndian.putShort(dgghead, 0, dggContainer.getOptions()); - LittleEndian.putShort(dgghead, 2, dggContainer.getRecordId()); - LittleEndian.putInt(dgghead, 4, size); - out.write(dgghead); - - // Finally, write out the children - bout.writeTo(out); } public EscherContainerRecord getDggContainer(){ diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/RecordContainer.java b/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/RecordContainer.java index 4815b42b74..abd3a22ced 100644 --- a/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/RecordContainer.java +++ b/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/RecordContainer.java @@ -227,30 +227,30 @@ public abstract class RecordContainer extends Record */ public void writeOut(byte headerA, byte headerB, long type, Record[] children, OutputStream out) throws IOException { // Create a UnsynchronizedByteArrayOutputStream to hold everything in - UnsynchronizedByteArrayOutputStream baos = new UnsynchronizedByteArrayOutputStream(); + try (UnsynchronizedByteArrayOutputStream baos = new UnsynchronizedByteArrayOutputStream()) { - // Write out our header, less the size - baos.write(new byte[] {headerA,headerB}); - byte[] typeB = new byte[2]; - LittleEndian.putShort(typeB,0,(short)type); - baos.write(typeB); - baos.write(new byte[] {0,0,0,0}); + // Write out our header, less the size + baos.write(new byte[]{headerA, headerB}); + byte[] typeB = new byte[2]; + LittleEndian.putShort(typeB, 0, (short) type); + baos.write(typeB); + baos.write(new byte[]{0, 0, 0, 0}); - // Write out our children - for (Record aChildren : children) { - aChildren.writeOut(baos); + // Write out our children + for (Record aChildren : children) { + aChildren.writeOut(baos); + } + + // Grab the bytes back + byte[] toWrite = baos.toByteArray(); + + // Update our header with the size + // Don't forget to knock 8 more off, since we don't include the header in the size + LittleEndian.putInt(toWrite, 4, (toWrite.length - 8)); + + // Write out the bytes + out.write(toWrite); } - - // Grab the bytes back - byte[] toWrite = baos.toByteArray(); - - // Update our header with the size - // Don't forget to knock 8 more off, since we don't include the - // header in the size - LittleEndian.putInt(toWrite,4,(toWrite.length-8)); - - // Write out the bytes - out.write(toWrite); } /** diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/StyleTextPropAtom.java b/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/StyleTextPropAtom.java index e758d57fce..d7e30b3532 100644 --- a/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/StyleTextPropAtom.java +++ b/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/StyleTextPropAtom.java @@ -309,22 +309,20 @@ public final class StyleTextPropAtom extends RecordAtom { */ private void updateRawContents() throws IOException { if (initialised) { - // Only update the style bytes, if the styles have been potentially - // changed + // Only update the style bytes, if the styles have been potentially changed + try (UnsynchronizedByteArrayOutputStream baos = new UnsynchronizedByteArrayOutputStream()) { + // First up, we need to serialise the paragraph properties + for (TextPropCollection tpc : paragraphStyles) { + tpc.writeOut(baos); + } - UnsynchronizedByteArrayOutputStream baos = new UnsynchronizedByteArrayOutputStream(); + // Now, we do the character ones + for (TextPropCollection tpc : charStyles) { + tpc.writeOut(baos); + } - // First up, we need to serialise the paragraph properties - for(TextPropCollection tpc : paragraphStyles) { - tpc.writeOut(baos); + rawContents = baos.toByteArray(); } - - // Now, we do the character ones - for(TextPropCollection tpc : charStyles) { - tpc.writeOut(baos); - } - - rawContents = baos.toByteArray(); } // Now ensure that the header size is correct diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/TextSpecInfoAtom.java b/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/TextSpecInfoAtom.java index 1071422e3b..da2a40ffba 100644 --- a/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/TextSpecInfoAtom.java +++ b/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/TextSpecInfoAtom.java @@ -130,26 +130,28 @@ public final class TextSpecInfoAtom extends RecordAtom { */ public void setParentSize(int size) { assert(size > 0); - int covered = 0; - UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream(); - TextSpecInfoRun[] runs = getTextSpecInfoRuns(); - assert(runs.length > 0); - for (int i=0; i size || i == runs.length-1) { - run.setLength(size-covered); - } - covered += run.getLength(); - try { - run.writeOut(bos); - } catch (IOException e) { - throw new HSLFException(e); - } - } - _data = bos.toByteArray(); - // Update the size (header bytes 5-8) - LittleEndian.putInt(_header, 4, _data.length); + try (UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream()) { + TextSpecInfoRun[] runs = getTextSpecInfoRuns(); + int remaining = size; + int idx = 0; + for (TextSpecInfoRun run : runs) { + int len = run.getLength(); + if (len > remaining || idx == runs.length - 1) { + run.setLength(len = remaining); + } + remaining -= len; + run.writeOut(bos); + idx++; + } + + _data = bos.toByteArray(); + + // Update the size (header bytes 5-8) + LittleEndian.putInt(_header, 4, _data.length); + } catch (IOException e) { + throw new HSLFException(e); + } } /** diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hwmf/usermodel/HwmfEmbeddedIterator.java b/poi-scratchpad/src/main/java/org/apache/poi/hwmf/usermodel/HwmfEmbeddedIterator.java index fb24af7cb4..03dfdec402 100644 --- a/poi-scratchpad/src/main/java/org/apache/poi/hwmf/usermodel/HwmfEmbeddedIterator.java +++ b/poi-scratchpad/src/main/java/org/apache/poi/hwmf/usermodel/HwmfEmbeddedIterator.java @@ -110,33 +110,29 @@ public class HwmfEmbeddedIterator implements Iterator { if (!(current instanceof HwmfEscape)) { return null; } - final HwmfEscape esc = (HwmfEscape)current; - assert(esc.getEscapeFunction() == EscapeFunction.META_ESCAPE_ENHANCED_METAFILE); - - WmfEscapeEMF img = esc.getEscapeData(); - assert(img.isValid()); - current = null; final HwmfEmbedded emb = new HwmfEmbedded(); emb.setEmbeddedType(HwmfEmbeddedType.EMF); - UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream(); - try { - for (;;) { - bos.write(img.getEmfData()); + try (UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream()) { + WmfEscapeEMF img; + do { + final HwmfEscape esc = (HwmfEscape)current; + assert(esc.getEscapeFunction() == EscapeFunction.META_ESCAPE_ENHANCED_METAFILE); + img = esc.getEscapeData(); + assert(img.isValid()); + + bos.write(img.getEmfData()); current = null; - if (img.getRemainingBytes() > 0 && hasNext() && (current instanceof HwmfEscape)) { - img = ((HwmfEscape)current).getEscapeData(); - } else { - return emb; - } - } + } while (img.getRemainingBytes() > 0 && hasNext() && (current instanceof HwmfEscape)); + + emb.setData(bos.toByteArray()); + return emb; + } catch (IOException ignored) { // UnsynchronizedByteArrayOutputStream doesn't throw IOException return null; - } finally { - emb.setData(bos.toByteArray()); } } } diff --git a/poi/build.gradle b/poi/build.gradle index ea061ff73f..4e4507d51f 100644 --- a/poi/build.gradle +++ b/poi/build.gradle @@ -104,6 +104,8 @@ task cacheTest9(type: Copy) { } jar { + dependsOn cacheJava9 + if (JavaVersion.current() == JavaVersion.VERSION_1_8) { into('META-INF/versions/9') { from JAVA9_SRC include '*.class' @@ -117,7 +119,7 @@ jar { // Create a separate jar for test-code to depend on it in other projects // See http://stackoverflow.com/questions/5144325/gradle-test-dependency -task testJar(type: Jar, dependsOn: testClasses) { +task testJar(type: Jar, dependsOn: [ testClasses, cacheTest9 ]) { destinationDirectory = file("../build/dist/maven/${project.archivesBaseName}-tests") classifier 'tests' diff --git a/poi/src/main/java/org/apache/poi/hpsf/ClassID.java b/poi/src/main/java/org/apache/poi/hpsf/ClassID.java index aaf45554ae..21aba376b0 100644 --- a/poi/src/main/java/org/apache/poi/hpsf/ClassID.java +++ b/poi/src/main/java/org/apache/poi/hpsf/ClassID.java @@ -49,7 +49,7 @@ public class ClassID implements Duplicatable, GenericRecord { private final byte[] bytes = new byte[LENGTH]; /** - * Creates a {@link ClassID} and reads its value from a byte array. + * Creates a ClassID and reads its value from a byte array. * * @param src The byte array to read from. * @param offset The offset of the first byte to read. @@ -60,7 +60,7 @@ public class ClassID implements Duplicatable, GenericRecord { /** - * Creates a {@link ClassID} and initializes its value with 0x00 bytes. + * Creates a ClassID and initializes its value with 0x00 bytes. */ public ClassID() { Arrays.fill(bytes, (byte)0); @@ -77,7 +77,7 @@ public class ClassID implements Duplicatable, GenericRecord { /** - * Creates a {@link ClassID} from a human-readable representation of the Class ID in standard + * Creates a ClassID from a human-readable representation of the Class ID in standard * format {@code "{xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx}"}. * * @param externalForm representation of the Class ID represented by this object. @@ -102,12 +102,11 @@ public class ClassID implements Duplicatable, GenericRecord { /** * @return The number of bytes occupied by this object in the byte stream. */ + @SuppressWarnings("java:S1845") public int length() { return LENGTH; } - - /** * Gets the bytes making out the class ID. They are returned in correct order, i.e. big-endian. * @@ -117,8 +116,6 @@ public class ClassID implements Duplicatable, GenericRecord { return bytes; } - - /** * Sets the bytes making out the class ID. * @@ -129,8 +126,6 @@ public class ClassID implements Duplicatable, GenericRecord { System.arraycopy(bytes, 0, this.bytes, 0, LENGTH); } - - /** * Reads the class ID's value from a byte array by turning little-endian into big-endian. * @@ -249,10 +244,6 @@ public class ClassID implements Duplicatable, GenericRecord { ; } - - /** - * @see Object#hashCode() - */ @Override public int hashCode() { return toString().hashCode(); diff --git a/poi/src/main/java/org/apache/poi/hpsf/PropertySet.java b/poi/src/main/java/org/apache/poi/hpsf/PropertySet.java index 5237309c9c..eed2a7d4c8 100644 --- a/poi/src/main/java/org/apache/poi/hpsf/PropertySet.java +++ b/poi/src/main/java/org/apache/poi/hpsf/PropertySet.java @@ -514,51 +514,52 @@ public class PropertySet { } private byte[] toBytes() throws WritingNotSupportedException, IOException { - UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream(); - LittleEndianOutputStream leos = new LittleEndianOutputStream(bos); + try (UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream(); + LittleEndianOutputStream leos = new LittleEndianOutputStream(bos)) { - /* Write the number of sections in this property set stream. */ - final int nrSections = getSectionCount(); + /* Write the number of sections in this property set stream. */ + final int nrSections = getSectionCount(); - /* Write the property set's header. */ - leos.writeShort(getByteOrder()); - leos.writeShort(getFormat()); - leos.writeInt(getOSVersion()); - putClassId(bos, getClassID()); - leos.writeInt(nrSections); + /* Write the property set's header. */ + leos.writeShort(getByteOrder()); + leos.writeShort(getFormat()); + leos.writeInt(getOSVersion()); + putClassId(bos, getClassID()); + leos.writeInt(nrSections); - assert(bos.size() == OFFSET_HEADER); + assert (bos.size() == OFFSET_HEADER); - final int[][] offsets = new int[getSectionCount()][2]; + final int[][] offsets = new int[getSectionCount()][2]; - /* Write the section list, i.e. the references to the sections. Each - * entry in the section list consist of the section's class ID and the - * section's offset relative to the beginning of the stream. */ - int secCnt = 0; - for (final Section section : getSections()) { - final ClassID formatID = section.getFormatID(); - if (formatID == null) { - throw new NoFormatIDException(); + /* Write the section list, i.e. the references to the sections. Each + * entry in the section list consist of the section's class ID and the + * section's offset relative to the beginning of the stream. */ + int secCnt = 0; + for (final Section section : getSections()) { + final ClassID formatID = section.getFormatID(); + if (formatID == null) { + throw new NoFormatIDException(); + } + putClassId(bos, formatID); + offsets[secCnt++][0] = bos.size(); + // offset dummy - filled later + leos.writeInt(-1); } - putClassId(bos, formatID); - offsets[secCnt++][0] = bos.size(); - // offset dummy - filled later - leos.writeInt(-1); - } - /* Write the sections themselves. */ - secCnt = 0; - for (final Section section : getSections()) { - offsets[secCnt++][1] = bos.size(); - section.write(bos); - } + /* Write the sections themselves. */ + secCnt = 0; + for (final Section section : getSections()) { + offsets[secCnt++][1] = bos.size(); + section.write(bos); + } - byte[] result = bos.toByteArray(); - for (int[] off : offsets) { - LittleEndian.putInt(result, off[0], off[1]); - } + byte[] result = bos.toByteArray(); + for (int[] off : offsets) { + LittleEndian.putInt(result, off[0], off[1]); + } - return result; + return result; + } } /** diff --git a/poi/src/main/java/org/apache/poi/hpsf/Section.java b/poi/src/main/java/org/apache/poi/hpsf/Section.java index 90d20cf448..13ae9af603 100644 --- a/poi/src/main/java/org/apache/poi/hpsf/Section.java +++ b/poi/src/main/java/org/apache/poi/hpsf/Section.java @@ -735,53 +735,54 @@ public class Section { } final int[][] offsets = new int[properties.size()][2]; - final UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream(); - final LittleEndianOutputStream leos = new LittleEndianOutputStream(bos); + try (UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream(); + LittleEndianOutputStream leos = new LittleEndianOutputStream(bos)) { - /* Write the section's length - dummy value, fixed later */ - leos.writeInt(-1); - - /* Write the section's number of properties: */ - leos.writeInt(properties.size()); - - int propCnt = 0; - for (Property p : properties.values()) { - /* Write the property list entry. */ - leos.writeUInt(p.getID()); - // dummy offset to be fixed later - offsets[propCnt++][0] = bos.size(); + /* Write the section's length - dummy value, fixed later */ leos.writeInt(-1); - } + /* Write the section's number of properties: */ + leos.writeInt(properties.size()); - /* Write the properties and the property list into their respective - * streams: */ - propCnt = 0; - for (Property p : properties.values()) { - offsets[propCnt++][1] = bos.size(); - /* If the property ID is not equal 0 we write the property and all - * is fine. However, if it equals 0 we have to write the section's - * dictionary which has an implicit type only and an explicit - * value. */ - if (p.getID() != 0) { - /* Write the property and update the position to the next - * property. */ - p.write(bos, codepage); - } else { - writeDictionary(bos, codepage); + int propCnt = 0; + for (Property p : properties.values()) { + /* Write the property list entry. */ + leos.writeUInt(p.getID()); + // dummy offset to be fixed later + offsets[propCnt++][0] = bos.size(); + leos.writeInt(-1); } + + + /* Write the properties and the property list into their respective + * streams: */ + propCnt = 0; + for (Property p : properties.values()) { + offsets[propCnt++][1] = bos.size(); + /* If the property ID is not equal 0 we write the property and all + * is fine. However, if it equals 0 we have to write the section's + * dictionary which has an implicit type only and an explicit + * value. */ + if (p.getID() != 0) { + /* Write the property and update the position to the next + * property. */ + p.write(bos, codepage); + } else { + writeDictionary(bos, codepage); + } + } + + byte[] result = bos.toByteArray(); + LittleEndian.putInt(result, 0, bos.size()); + + for (int[] off : offsets) { + LittleEndian.putUInt(result, off[0], off[1]); + } + + out.write(result); + + return bos.size(); } - - byte[] result = bos.toByteArray(); - LittleEndian.putInt(result, 0, bos.size()); - - for (int[] off : offsets) { - LittleEndian.putUInt(result, off[0], off[1]); - } - - out.write(result); - - return bos.size(); } /** diff --git a/poi/src/main/java/org/apache/poi/hssf/record/RecordInputStream.java b/poi/src/main/java/org/apache/poi/hssf/record/RecordInputStream.java index 94fc4070c7..5b7b114cdb 100644 --- a/poi/src/main/java/org/apache/poi/hssf/record/RecordInputStream.java +++ b/poi/src/main/java/org/apache/poi/hssf/record/RecordInputStream.java @@ -459,17 +459,20 @@ public final class RecordInputStream implements LittleEndianInput { */ @Deprecated public byte[] readAllContinuedRemainder() { - UnsynchronizedByteArrayOutputStream out = new UnsynchronizedByteArrayOutputStream(2 * MAX_RECORD_DATA_SIZE); + try (UnsynchronizedByteArrayOutputStream out = new UnsynchronizedByteArrayOutputStream(2 * MAX_RECORD_DATA_SIZE)) { - while (true) { - byte[] b = readRemainder(); - out.write(b, 0, b.length); - if (!isContinueNext()) { - break; + while (true) { + byte[] b = readRemainder(); + out.write(b, 0, b.length); + if (!isContinueNext()) { + break; + } + nextRecord(); } - nextRecord(); + return out.toByteArray(); + } catch (IOException ex) { + throw new RecordFormatException(ex); } - return out.toByteArray(); } /** The remaining number of bytes in the current record. diff --git a/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFConditionalFormattingRule.java b/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFConditionalFormattingRule.java index 29506743f6..af688471fd 100644 --- a/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFConditionalFormattingRule.java +++ b/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFConditionalFormattingRule.java @@ -64,8 +64,8 @@ public final class HSSFConditionalFormattingRule implements ConditionalFormattin * Only newer style formatting rules have priorities. For older ones, * we don't know priority for these, other than definition/model order, * which appears to be what Excel uses. - * @see org.apache.poi.ss.usermodel.ConditionalFormattingRule#getPriority() */ + @Override public int getPriority() { CFRule12Record rule12 = getCFRule12Record(false); if (rule12 == null) return 0; @@ -74,8 +74,8 @@ public final class HSSFConditionalFormattingRule implements ConditionalFormattin /** * Always true for HSSF files, per Microsoft Excel documentation - * @see org.apache.poi.ss.usermodel.ConditionalFormattingRule#getStopIfTrue() */ + @Override public boolean getStopIfTrue() { return true; } @@ -95,8 +95,8 @@ public final class HSSFConditionalFormattingRule implements ConditionalFormattin /** * Always null for HSSF records, until someone figures out where to find it - * @see org.apache.poi.ss.usermodel.ConditionalFormattingRule#getNumberFormat() */ + @Override public ExcelNumberFormat getNumberFormat() { return null; } @@ -112,16 +112,18 @@ public final class HSSFConditionalFormattingRule implements ConditionalFormattin } /** - * @return - font formatting object if defined, null otherwise + * @return - font formatting object if defined, {@code null} otherwise */ + @Override public HSSFFontFormatting getFontFormatting() { return getFontFormatting(false); } /** * create a new font formatting structure if it does not exist, * otherwise just return existing object. - * @return - font formatting object, never returns null. + * @return - font formatting object, never returns {@code null}. */ + @Override public HSSFFontFormatting createFontFormatting() { return getFontFormatting(true); } @@ -137,16 +139,18 @@ public final class HSSFConditionalFormattingRule implements ConditionalFormattin } /** - * @return - border formatting object if defined, null otherwise + * @return - border formatting object if defined, {@code null} otherwise */ + @Override public HSSFBorderFormatting getBorderFormatting() { return getBorderFormatting(false); } /** * create a new border formatting structure if it does not exist, * otherwise just return existing object. - * @return - border formatting object, never returns null. + * @return - border formatting object, never returns {@code null}. */ + @Override public HSSFBorderFormatting createBorderFormatting() { return getBorderFormatting(true); } @@ -162,8 +166,9 @@ public final class HSSFConditionalFormattingRule implements ConditionalFormattin } /** - * @return - pattern formatting object if defined, null otherwise + * @return - pattern formatting object if defined, {@code null} otherwise */ + @Override public HSSFPatternFormatting getPatternFormatting() { return getPatternFormatting(false); @@ -171,8 +176,9 @@ public final class HSSFConditionalFormattingRule implements ConditionalFormattin /** * create a new pattern formatting structure if it does not exist, * otherwise just return existing object. - * @return - pattern formatting object, never returns null. + * @return - pattern formatting object, never returns {@code null}. */ + @Override public HSSFPatternFormatting createPatternFormatting() { return getPatternFormatting(true); @@ -192,8 +198,9 @@ public final class HSSFConditionalFormattingRule implements ConditionalFormattin } /** - * @return databar / data-bar formatting object if defined, null otherwise + * @return databar / data-bar formatting object if defined, {@code null} otherwise */ + @Override public HSSFDataBarFormatting getDataBarFormatting() { return getDataBarFormatting(false); } @@ -218,8 +225,9 @@ public final class HSSFConditionalFormattingRule implements ConditionalFormattin } /** - * @return icon / multi-state formatting object if defined, null otherwise + * @return icon / multi-state formatting object if defined, {@code null} otherwise */ + @Override public HSSFIconMultiStateFormatting getMultiStateFormatting() { return getMultiStateFormatting(false); } @@ -245,8 +253,9 @@ public final class HSSFConditionalFormattingRule implements ConditionalFormattin } /** - * @return color scale / gradient formatting object if defined, null otherwise + * @return color scale / gradient formatting object if defined, {@code null} otherwise */ + @Override public HSSFColorScaleFormatting getColorScaleFormatting() { return getColorScaleFormatting(false); } @@ -270,12 +279,13 @@ public final class HSSFConditionalFormattingRule implements ConditionalFormattin /** * always null (not a filter condition) or {@link ConditionFilterType#FILTER} if it is. - * @see org.apache.poi.ss.usermodel.ConditionalFormattingRule#getConditionFilterType() */ + @Override public ConditionFilterType getConditionFilterType() { return getConditionType() == ConditionType.FILTER ? ConditionFilterType.FILTER : null; } + @Override public ConditionFilterData getFilterConfiguration() { return null; } @@ -288,11 +298,13 @@ public final class HSSFConditionalFormattingRule implements ConditionalFormattin return cfRuleRecord.getComparisonOperation(); } + @Override public String getFormula1() { return toFormulaString(cfRuleRecord.getParsedExpression1()); } + @Override public String getFormula2() { byte conditionType = cfRuleRecord.getConditionType(); if (conditionType == CELL_COMPARISON) { @@ -306,6 +318,7 @@ public final class HSSFConditionalFormattingRule implements ConditionalFormattin return null; } + @Override public String getText() { return null; // not available here, unless it exists and is unimplemented in cfRuleRecord } @@ -325,6 +338,7 @@ public final class HSSFConditionalFormattingRule implements ConditionalFormattin * Conditional format rules don't define stripes, so always 0 * @see org.apache.poi.ss.usermodel.DifferentialStyleProvider#getStripeSize() */ + @Override public int getStripeSize() { return 0; } diff --git a/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFConditionalFormattingThreshold.java b/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFConditionalFormattingThreshold.java index 4ab124c05c..987d9f47f3 100644 --- a/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFConditionalFormattingThreshold.java +++ b/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFConditionalFormattingThreshold.java @@ -31,32 +31,40 @@ public final class HSSFConditionalFormattingThreshold implements org.apache.poi. private final HSSFSheet sheet; private final HSSFWorkbook workbook; - protected HSSFConditionalFormattingThreshold(Threshold threshold, HSSFSheet sheet) { + HSSFConditionalFormattingThreshold(Threshold threshold, HSSFSheet sheet) { this.threshold = threshold; this.sheet = sheet; this.workbook = sheet.getWorkbook(); } - protected Threshold getThreshold() { + + Threshold getThreshold() { return threshold; } + @Override public RangeType getRangeType() { return RangeType.byId(threshold.getType()); } + @Override public void setRangeType(RangeType type) { threshold.setType((byte)type.id); } + @Override public String getFormula() { return toFormulaString(threshold.getParsedExpression(), workbook); } + @Override public void setFormula(String formula) { threshold.setParsedExpression(parseFormula(formula, sheet)); } + @Override public Double getValue() { return threshold.getValue(); } + + @Override public void setValue(Double value) { threshold.setValue(value); } diff --git a/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFIconMultiStateFormatting.java b/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFIconMultiStateFormatting.java index cabfad2f80..110612e8bb 100644 --- a/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFIconMultiStateFormatting.java +++ b/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFIconMultiStateFormatting.java @@ -24,41 +24,46 @@ import org.apache.poi.hssf.record.cf.Threshold; import org.apache.poi.ss.usermodel.ConditionalFormattingThreshold; /** - * High level representation for Icon / Multi-State Formatting + * High level representation for Icon / Multi-State Formatting * component of Conditional Formatting settings */ public final class HSSFIconMultiStateFormatting implements org.apache.poi.ss.usermodel.IconMultiStateFormatting { private final HSSFSheet sheet; - private final CFRule12Record cfRule12Record; private final IconMultiStateFormatting iconFormatting; - protected HSSFIconMultiStateFormatting(CFRule12Record cfRule12Record, HSSFSheet sheet) { + HSSFIconMultiStateFormatting(CFRule12Record cfRule12Record, HSSFSheet sheet) { this.sheet = sheet; - this.cfRule12Record = cfRule12Record; - this.iconFormatting = this.cfRule12Record.getMultiStateFormatting(); + this.iconFormatting = cfRule12Record.getMultiStateFormatting(); } + @Override public IconSet getIconSet() { return iconFormatting.getIconSet(); } + @Override public void setIconSet(IconSet set) { iconFormatting.setIconSet(set); } + @Override public boolean isIconOnly() { return iconFormatting.isIconOnly(); } + @Override public void setIconOnly(boolean only) { iconFormatting.setIconOnly(only); } + @Override public boolean isReversed() { return iconFormatting.isReversed(); } + @Override public void setReversed(boolean reversed) { iconFormatting.setReversed(reversed); } + @Override public HSSFConditionalFormattingThreshold[] getThresholds() { Threshold[] t = iconFormatting.getThresholds(); HSSFConditionalFormattingThreshold[] ht = new HSSFConditionalFormattingThreshold[t.length]; @@ -68,6 +73,7 @@ public final class HSSFIconMultiStateFormatting implements org.apache.poi.ss.use return ht; } + @Override public void setThresholds(ConditionalFormattingThreshold[] thresholds) { Threshold[] t = new Threshold[thresholds.length]; for (int i=0; i= MAX_STRING_LENGTH) { + LOGGER.atWarn().log("stopped reading unicode name after {} bytes", box(read)); + } + return bos.toString(StandardCharsets.UTF_16LE); } - if (read >= MAX_STRING_LENGTH) { - LOGGER.atWarn().log("stopped reading unicode name after {} bytes", box(read)); - } - return bos.toString(StandardCharsets.UTF_16LE); } private static String readMBCS(int firstByte, InputStream is, Charset charset) throws IOException { - UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream(); - int len = 0; - int b = firstByte; - while (b > 0 && len < MAX_STRING_LENGTH) { - ++len; - bos.write(b); - b = IOUtils.readByte(is); + try (UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream()) { + int len = 0; + int b = firstByte; + while (b > 0 && len < MAX_STRING_LENGTH) { + ++len; + bos.write(b); + b = IOUtils.readByte(is); + } + return bos.toString(charset); } - return bos.toString(charset); } /** @@ -792,9 +794,7 @@ public class VBAMacroReader implements Closeable { */ private static byte[] findCompressedStreamWBruteForce(InputStream is) throws IOException { //buffer to memory for multiple tries - UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream(); - IOUtils.copy(is, bos); - byte[] compressed = bos.toByteArray(); + byte[] compressed = IOUtils.toByteArray(is); byte[] decompressed = null; for (int i = 0; i < compressed.length; i++) { if (compressed[i] == 0x01 && i < compressed.length-1) { @@ -821,12 +821,10 @@ public class VBAMacroReader implements Closeable { } private static byte[] tryToDecompress(InputStream is) { - UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream(); - try { - IOUtils.copy(new RLEDecompressingInputStream(is), bos); + try (RLEDecompressingInputStream ris = new RLEDecompressingInputStream(is)) { + return IOUtils.toByteArray(ris); } catch (IllegalArgumentException | IOException | IllegalStateException e){ return null; } - return bos.toByteArray(); } } diff --git a/poi/src/main/java/org/apache/poi/ss/extractor/EmbeddedExtractor.java b/poi/src/main/java/org/apache/poi/ss/extractor/EmbeddedExtractor.java index 18eef73108..47793a14c1 100644 --- a/poi/src/main/java/org/apache/poi/ss/extractor/EmbeddedExtractor.java +++ b/poi/src/main/java/org/apache/poi/ss/extractor/EmbeddedExtractor.java @@ -162,14 +162,13 @@ public class EmbeddedExtractor implements Iterable { protected EmbeddedData extract(DirectoryNode dn) throws IOException { assert(canExtract(dn)); - UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream(20000); - try (POIFSFileSystem dest = new POIFSFileSystem()) { + try (UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream(20000); + POIFSFileSystem dest = new POIFSFileSystem()) { copyNodes(dn, dest.getRoot()); // start with a reasonable big size dest.writeFilesystem(bos); + return new EmbeddedData(dn.getName(), bos.toByteArray(), CONTENT_TYPE_BYTES); } - - return new EmbeddedData(dn.getName(), bos.toByteArray(), CONTENT_TYPE_BYTES); } protected EmbeddedData extract(Picture source) throws IOException { diff --git a/poi/src/main/java/org/apache/poi/ss/usermodel/ConditionalFormattingThreshold.java b/poi/src/main/java/org/apache/poi/ss/usermodel/ConditionalFormattingThreshold.java index c70e3d1d24..69cc1887c5 100644 --- a/poi/src/main/java/org/apache/poi/ss/usermodel/ConditionalFormattingThreshold.java +++ b/poi/src/main/java/org/apache/poi/ss/usermodel/ConditionalFormattingThreshold.java @@ -27,7 +27,7 @@ package org.apache.poi.ss.usermodel; * icon and which Yellow or Red.

*/ public interface ConditionalFormattingThreshold { - public enum RangeType { + enum RangeType { /** Number / Parameter */ NUMBER(1, "num"), /** The minimum value from the range */ @@ -41,16 +41,16 @@ public interface ConditionalFormattingThreshold { UNALLOCATED(6, null), /** Formula result */ FORMULA(7, "formula"); - + /** Numeric ID of the type */ public final int id; /** Name (system) of the type */ public final String name; - + public String toString() { return id + " - " + name; } - + public static RangeType byId(int id) { return values()[id-1]; // 1-based IDs } @@ -60,51 +60,51 @@ public interface ConditionalFormattingThreshold { } return null; } - - private RangeType(int id, String name) { + + RangeType(int id, String name) { this.id = id; this.name = name; } } - + /** * Get the Range Type used */ RangeType getRangeType(); - + /** * Changes the Range Type used - * + * *

If you change the range type, you need to * ensure that the Formula and Value parameters * are compatible with it before saving

*/ void setRangeType(RangeType type); - + /** * Formula to use to calculate the threshold, - * or null if no formula + * or {@code null} if no formula */ String getFormula(); /** * Sets the formula used to calculate the threshold, - * or unsets it if null is given. + * or unsets it if {@code null} is given. */ void setFormula(String formula); - + /** - * Gets the value used for the threshold, or - * null if there isn't one. + * Gets the value used for the threshold, or + * {@code null} if there isn't one. */ Double getValue(); - + /** - * Sets the value used for the threshold. - *

If the type is {@link RangeType#PERCENT} or + * Sets the value used for the threshold. + *

If the type is {@link RangeType#PERCENT} or * {@link RangeType#PERCENTILE} it must be between 0 and 100. *

If the type is {@link RangeType#MIN} or {@link RangeType#MAX} * or {@link RangeType#FORMULA} it shouldn't be set. - *

Use null to unset + *

Use {@code null} to unset */ void setValue(Double value); } diff --git a/poi/src/main/java/org/apache/poi/util/GenericRecordJsonWriter.java b/poi/src/main/java/org/apache/poi/util/GenericRecordJsonWriter.java index 281b8180b0..f282907c94 100644 --- a/poi/src/main/java/org/apache/poi/util/GenericRecordJsonWriter.java +++ b/poi/src/main/java/org/apache/poi/util/GenericRecordJsonWriter.java @@ -265,6 +265,7 @@ public class GenericRecordJsonWriter implements Closeable { return true; } + @SuppressWarnings("java:S3516") protected boolean printNumber(String name, Object o) { Number n = (Number)o; printName(name); diff --git a/poi/src/main/java/org/apache/poi/util/IOUtils.java b/poi/src/main/java/org/apache/poi/util/IOUtils.java index 279b56974c..09bb25a9cb 100644 --- a/poi/src/main/java/org/apache/poi/util/IOUtils.java +++ b/poi/src/main/java/org/apache/poi/util/IOUtils.java @@ -178,29 +178,29 @@ public final class IOUtils { } final int len = Math.min(length, maxLength); - UnsynchronizedByteArrayOutputStream baos = new UnsynchronizedByteArrayOutputStream(len == Integer.MAX_VALUE ? 4096 : len); + try (UnsynchronizedByteArrayOutputStream baos = new UnsynchronizedByteArrayOutputStream(len == Integer.MAX_VALUE ? 4096 : len)) { + byte[] buffer = new byte[4096]; + int totalBytes = 0, readBytes; + do { + readBytes = stream.read(buffer, 0, Math.min(buffer.length, len - totalBytes)); + totalBytes += Math.max(readBytes, 0); + if (readBytes > 0) { + baos.write(buffer, 0, readBytes); + } - byte[] buffer = new byte[4096]; - int totalBytes = 0, readBytes; - do { - readBytes = stream.read(buffer, 0, Math.min(buffer.length, len-totalBytes)); - totalBytes += Math.max(readBytes,0); - if (readBytes > 0) { - baos.write(buffer, 0, readBytes); + checkByteSizeLimit(totalBytes); + } while (totalBytes < len && readBytes > -1); + + if (maxLength != Integer.MAX_VALUE && totalBytes == maxLength) { + throw new IOException("MaxLength (" + maxLength + ") reached - stream seems to be invalid."); } - checkByteSizeLimit(totalBytes); - } while (totalBytes < len && readBytes > -1); + if (len != Integer.MAX_VALUE && totalBytes < len) { + throw new EOFException("unexpected EOF - expected len: " + len + " - actual len: " + totalBytes); + } - if (maxLength != Integer.MAX_VALUE && totalBytes == maxLength) { - throw new IOException("MaxLength ("+maxLength+") reached - stream seems to be invalid."); + return baos.toByteArray(); } - - if (len != Integer.MAX_VALUE && totalBytes < len) { - throw new EOFException("unexpected EOF - expected len: "+len+" - actual len: "+totalBytes); - } - - return baos.toByteArray(); } private static void checkLength(long length, int maxLength) { diff --git a/poi/src/test/java/org/apache/poi/POITestCase.java b/poi/src/test/java/org/apache/poi/POITestCase.java index 7dbed7bd0d..4f43cb216c 100644 --- a/poi/src/test/java/org/apache/poi/POITestCase.java +++ b/poi/src/test/java/org/apache/poi/POITestCase.java @@ -25,8 +25,6 @@ import static org.hamcrest.CoreMatchers.startsWith; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.fail; -import static org.junit.jupiter.api.Assumptions.assumeTrue; import java.io.File; import java.lang.reflect.Field; @@ -47,6 +45,7 @@ import org.mockito.internal.matchers.apachecommons.ReflectionEquals; * Util class for POI JUnit TestCases, which provide additional features */ @Internal +@SuppressWarnings("java:S2187") public final class POITestCase { private POITestCase() { @@ -141,68 +140,6 @@ public final class POITestCase { assertTrue(new ReflectionEquals(expected, "$jacocoData").matches(actual)); } - /** - * Rather than adding {@literal @}Ignore to known-failing tests, - * write the test so that it notifies us if it starts passing. - * This is useful for closing related or forgotten bugs. - * - * An Example: - * 

-     * public static int add(int a, int b) {
-     *     // a known bug in behavior that has not been fixed yet
-     *     raise UnsupportedOperationException("add");
-     * }
-     *
-     * {@literal @}Test
-     * void knownFailingUnitTest() {
-     *     try {
-     *         assertEquals(2, add(1,1));
-     *         // this test fails because the assumption that this bug had not been fixed is false
-     *         testPassesNow(12345);
-     *     } catch (UnsupportedOperationException e) {
-     *         // test is skipped because the assumption that this bug had not been fixed is true
-     *         skipTest(e);
-     *     }
-     * }
-     *
-     * Once passing, this unit test can be rewritten as:
-     * {@literal @}Test
-     * void knownPassingUnitTest() {
-     *     assertEquals(2, add(1,1));
-     * }
-     *
-     * If you have a better idea how to simplify test code while still notifying
-     * us when a previous known-failing test now passes, please improve these.
-     * As a bonus, a known-failing test that fails should not be counted as a
-     * passing test.
-     *
-     * One possible alternative is to expect the known exception, but without
-     * a clear message that it is a good thing to no longer get the expected
-     * exception once the test passes.
-     * {@literal @}Test(expected=UnsupportedOperationException.class)
-     * void knownFailingUnitTest() {
-     *     assertEquals(2, add(1,1));
-     * }
-     *
-     * @param e  the exception that was caught that will no longer
-     * be raised when the bug is fixed
-     */
-    public static void skipTest(Throwable e) {
-        assumeTrue(e != null, "This test currently fails with");
-    }
-    /**
-     * @see #skipTest(Throwable)
-     *
-     * @param bug  the bug number corresponding to a known bug in bugzilla
-     */
-    public static void testPassesNow(int bug) {
-        fail("This test passes now. Please update the unit test and bug " + bug + ".");
-    }
-
-    public static void assertBetween(String message, int value, int min, int max) {
-        assertTrue(min <= value, message + ": " + value + " is less than the minimum value of " + min);
-        assertTrue(value <= max, message + ": " + value + " is greater than the maximum value of " + max);
-    }
 
     /**
      * Ensures that the temporary directory is defined and exists and
diff --git a/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java b/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java
index 5e7b5d87f7..3edce0e0e4 100644
--- a/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java
+++ b/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java
@@ -1382,6 +1382,7 @@ public abstract class BaseTestBugzillaIssues {
     }
 
     @Test
+    @SuppressWarnings("java:S2699")
     void test58896() throws IOException {
         final int nrows = 160;
         final int ncols = 139;
@@ -1493,8 +1494,8 @@ public abstract class BaseTestBugzillaIssues {
 
             // *******************************
             // First cell of array formula, OK
-            int rowId = 0;
-            int cellId = 1;
+            final int rowId = 0;
+            final int cellId = 1;
 
             Row row = sheet.getRow(rowId);
             Cell cell = row.getCell(cellId);
@@ -1507,9 +1508,6 @@ public abstract class BaseTestBugzillaIssues {
 
             // *******************************
             // Second cell of array formula, NOT OK for xlsx files
-            rowId = 1;
-            cellId = 1;
-
             row = sheet.getRow(rowId);
             cell = row.getCell(cellId);
             assertEquals("A1", cell.getCellFormula());
diff --git a/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestSheet.java b/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestSheet.java
index 947bd71a9f..8ed16c996b 100644
--- a/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestSheet.java
+++ b/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestSheet.java
@@ -17,7 +17,6 @@
 
 package org.apache.poi.ss.usermodel;
 
-import static org.apache.poi.POITestCase.assertBetween;
 import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -50,8 +49,7 @@ import org.junit.jupiter.api.Test;
 
 /**
  * Common superclass for testing {@link org.apache.poi.hssf.usermodel.HSSFCell},
- * {@link org.apache.poi.xssf.usermodel.XSSFCell} and
- * {@link org.apache.poi.xssf.streaming.SXSSFCell}
+ * XSSFCell and SXSSFCell
  */
 public abstract class BaseTestSheet {
     private static final int ROW_COUNT = 40000;
@@ -430,7 +428,7 @@ public abstract class BaseTestSheet {
             assertCollectionEquals(mergedRegions.values(), sheet.getMergedRegions());
 
             Collection removed = Arrays.asList(0, 2, 3, 6, 8);
-            mergedRegions.keySet().removeAll(removed);
+            removed.forEach(mergedRegions.keySet()::remove);
             sheet.removeMergedRegions(removed);
             assertCollectionEquals(mergedRegions.values(), sheet.getMergedRegions());
         }
@@ -1364,4 +1362,9 @@ public abstract class BaseTestSheet {
             assertBetween("Date column width", s.getColumnWidth(1), 4750, 7300);
         }
     }
+
+    private static void assertBetween(String message, int value, int min, int max) {
+        assertTrue(min <= value, message + ": " + value + " is less than the minimum value of " + min);
+        assertTrue(value <= max, message + ": " + value + " is greater than the maximum value of " + max);
+    }
 }
diff --git a/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java b/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java
index 6c8e2f2a93..869b591eff 100644
--- a/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java
+++ b/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java
@@ -17,8 +17,6 @@
 
 package org.apache.poi.ss.usermodel;
 
-import static org.apache.poi.POITestCase.skipTest;
-import static org.apache.poi.POITestCase.testPassesNow;
 import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
@@ -60,57 +58,55 @@ public abstract class BaseTestSheetShiftRows {
     public final void testShiftRows() throws IOException {
         // Read initial file in
         String sampleName = "SimpleMultiCell." + _testDataProvider.getStandardFileNameExtension();
-        Workbook wb1 = _testDataProvider.openSampleWorkbook(sampleName);
-        Sheet s = wb1.getSheetAt( 0 );
+        try (Workbook wb1 = _testDataProvider.openSampleWorkbook(sampleName)) {
+            Sheet s = wb1.getSheetAt(0);
 
-        // Shift the second row down 1 and write to temp file
-        s.shiftRows( 1, 1, 1 );
+            // Shift the second row down 1 and write to temp file
+            s.shiftRows(1, 1, 1);
 
-        Workbook wb2 = _testDataProvider.writeOutAndReadBack(wb1);
-        wb1.close();
+            try (Workbook wb2 = _testDataProvider.writeOutAndReadBack(wb1)) {
 
-        // Read from temp file and check the number of cells in each
-        // row (in original file each row was unique)
-        s = wb2.getSheetAt( 0 );
+                // Read from temp file and check the number of cells in each
+                // row (in original file each row was unique)
+                s = wb2.getSheetAt(0);
 
-        assertEquals(s.getRow(0).getPhysicalNumberOfCells(), 1);
-        confirmEmptyRow(s, 1);
-        assertEquals(s.getRow(2).getPhysicalNumberOfCells(), 2);
-        assertEquals(s.getRow(3).getPhysicalNumberOfCells(), 4);
-        assertEquals(s.getRow(4).getPhysicalNumberOfCells(), 5);
+                assertEquals(s.getRow(0).getPhysicalNumberOfCells(), 1);
+                confirmEmptyRow(s, 1);
+                assertEquals(s.getRow(2).getPhysicalNumberOfCells(), 2);
+                assertEquals(s.getRow(3).getPhysicalNumberOfCells(), 4);
+                assertEquals(s.getRow(4).getPhysicalNumberOfCells(), 5);
 
-        // Shift rows 1-3 down 3 in the current one.  This tests when
-        // 1 row is blank.  Write to a another temp file
-        s.shiftRows( 0, 2, 3 );
-        Workbook wb3 = _testDataProvider.writeOutAndReadBack(wb2);
-        wb2.close();
-
-        // Read and ensure things are where they should be
-        s = wb3.getSheetAt(0);
-        confirmEmptyRow(s, 0);
-        confirmEmptyRow(s, 1);
-        confirmEmptyRow(s, 2);
-        assertEquals(s.getRow(3).getPhysicalNumberOfCells(), 1);
-        confirmEmptyRow(s, 4);
-        assertEquals(s.getRow(5).getPhysicalNumberOfCells(), 2);
-
-        wb3.close();
+                // Shift rows 1-3 down 3 in the current one.  This tests when
+                // 1 row is blank.  Write to a another temp file
+                s.shiftRows(0, 2, 3);
+                try (Workbook wb3 = _testDataProvider.writeOutAndReadBack(wb2)) {
+                    // Read and ensure things are where they should be
+                    s = wb3.getSheetAt(0);
+                    confirmEmptyRow(s, 0);
+                    confirmEmptyRow(s, 1);
+                    confirmEmptyRow(s, 2);
+                    assertEquals(s.getRow(3).getPhysicalNumberOfCells(), 1);
+                    confirmEmptyRow(s, 4);
+                    assertEquals(s.getRow(5).getPhysicalNumberOfCells(), 2);
+                }
+            }
+        }
 
         // Read the first file again
-        Workbook wb4 = _testDataProvider.openSampleWorkbook(sampleName);
-        s = wb4.getSheetAt( 0 );
+        try (Workbook wb4 = _testDataProvider.openSampleWorkbook(sampleName)) {
+            Sheet s = wb4.getSheetAt(0);
 
-        // Shift rows 3 and 4 up and write to temp file
-        s.shiftRows( 2, 3, -2 );
-        Workbook wb5 = _testDataProvider.writeOutAndReadBack(wb4);
-        wb4.close();
-        s = wb5.getSheetAt( 0 );
-        assertEquals(s.getRow(0).getPhysicalNumberOfCells(), 3);
-        assertEquals(s.getRow(1).getPhysicalNumberOfCells(), 4);
-        confirmEmptyRow(s, 2);
-        confirmEmptyRow(s, 3);
-        assertEquals(s.getRow(4).getPhysicalNumberOfCells(), 5);
-        wb5.close();
+            // Shift rows 3 and 4 up and write to temp file
+            s.shiftRows(2, 3, -2);
+            try (Workbook wb5 = _testDataProvider.writeOutAndReadBack(wb4)) {
+                s = wb5.getSheetAt(0);
+                assertEquals(s.getRow(0).getPhysicalNumberOfCells(), 3);
+                assertEquals(s.getRow(1).getPhysicalNumberOfCells(), 4);
+                confirmEmptyRow(s, 2);
+                confirmEmptyRow(s, 3);
+                assertEquals(s.getRow(4).getPhysicalNumberOfCells(), 5);
+            }
+        }
     }
     private static void confirmEmptyRow(Sheet s, int rowIx) {
         Row row = s.getRow(rowIx);
@@ -149,195 +145,188 @@ public abstract class BaseTestSheetShiftRows {
 
     @Test
     void testShiftWithComments() throws IOException {
-        Workbook wb1 = _testDataProvider.openSampleWorkbook("comments." + _testDataProvider.getStandardFileNameExtension());
+        try (Workbook wb1 = _testDataProvider.openSampleWorkbook("comments." + _testDataProvider.getStandardFileNameExtension())) {
 
-        Sheet sheet = wb1.getSheet("Sheet1");
-        assertEquals(3, sheet.getLastRowNum());
-
-        // Verify comments are in the position expected
-        assertNotNull(sheet.getCellComment(new CellAddress(0,0)));
-        assertNull(sheet.getCellComment(new CellAddress(1,0)));
-        assertNotNull(sheet.getCellComment(new CellAddress(2,0)));
-        assertNotNull(sheet.getCellComment(new CellAddress(3,0)));
-
-        String comment1 = sheet.getCellComment(new CellAddress(0,0)).getString().getString();
-        assertEquals(comment1,"comment top row1 (index0)\n");
-        String comment3 = sheet.getCellComment(new CellAddress(2,0)).getString().getString();
-        assertEquals(comment3,"comment top row3 (index2)\n");
-        String comment4 = sheet.getCellComment(new CellAddress(3,0)).getString().getString();
-        assertEquals(comment4,"comment top row4 (index3)\n");
-
-        //Workbook wbBack = _testDataProvider.writeOutAndReadBack(wb);
-
-        // Shifting all but first line down to test comments shifting
-        sheet.shiftRows(1, sheet.getLastRowNum(), 1, true, true);
-
-        // Test that comments were shifted as expected
-        assertEquals(4, sheet.getLastRowNum());
-        assertNotNull(sheet.getCellComment(new CellAddress(0,0)));
-        assertNull(sheet.getCellComment(new CellAddress(1,0)));
-        assertNull(sheet.getCellComment(new CellAddress(2,0)));
-        assertNotNull(sheet.getCellComment(new CellAddress(3,0)));
-        assertNotNull(sheet.getCellComment(new CellAddress(4,0)));
-
-        String comment1_shifted = sheet.getCellComment(new CellAddress(0,0)).getString().getString();
-        assertEquals(comment1,comment1_shifted);
-        String comment3_shifted = sheet.getCellComment(new CellAddress(3,0)).getString().getString();
-        assertEquals(comment3,comment3_shifted);
-        String comment4_shifted = sheet.getCellComment(new CellAddress(4,0)).getString().getString();
-        assertEquals(comment4,comment4_shifted);
-
-        // Write out and read back in again
-        // Ensure that the changes were persisted
-        Workbook wb2 = _testDataProvider.writeOutAndReadBack(wb1);
-        wb1.close();
-
-        sheet = wb2.getSheet("Sheet1");
-        assertEquals(4, sheet.getLastRowNum());
-
-        // Verify comments are in the position expected after the shift
-        assertNotNull(sheet.getCellComment(new CellAddress(0,0)));
-        assertNull(sheet.getCellComment(new CellAddress(1,0)));
-        assertNull(sheet.getCellComment(new CellAddress(2,0)));
-        assertNotNull(sheet.getCellComment(new CellAddress(3,0)));
-        assertNotNull(sheet.getCellComment(new CellAddress(4,0)));
-
-        comment1_shifted = sheet.getCellComment(new CellAddress(0,0)).getString().getString();
-        assertEquals(comment1,comment1_shifted);
-        comment3_shifted = sheet.getCellComment(new CellAddress(3,0)).getString().getString();
-        assertEquals(comment3,comment3_shifted);
-        comment4_shifted = sheet.getCellComment(new CellAddress(4,0)).getString().getString();
-        assertEquals(comment4,comment4_shifted);
-
-        // Shifting back up again, now two rows
-        sheet.shiftRows(2, sheet.getLastRowNum(), -2, true, true);
-
-        // TODO: it seems HSSFSheet does not correctly remove comments from rows that are overwritten
-        // by shifting rows...
-        if(!(wb2 instanceof HSSFWorkbook)) {
-            assertEquals(2, sheet.getLastRowNum());
+            Sheet sheet = wb1.getSheet("Sheet1");
+            assertEquals(3, sheet.getLastRowNum());
 
             // Verify comments are in the position expected
-            assertNull(sheet.getCellComment(new CellAddress(0,0)),
-                "Had: " + (sheet.getCellComment(new CellAddress(0,0)) == null ? "null" : sheet.getCellComment(new CellAddress(0,0)).getString()));
-            assertNotNull(sheet.getCellComment(new CellAddress(1,0)));
-            assertNotNull(sheet.getCellComment(new CellAddress(2,0)));
+            assertNotNull(sheet.getCellComment(new CellAddress(0, 0)));
+            assertNull(sheet.getCellComment(new CellAddress(1, 0)));
+            assertNotNull(sheet.getCellComment(new CellAddress(2, 0)));
+            assertNotNull(sheet.getCellComment(new CellAddress(3, 0)));
+
+            String comment1 = sheet.getCellComment(new CellAddress(0, 0)).getString().getString();
+            assertEquals(comment1, "comment top row1 (index0)\n");
+            String comment3 = sheet.getCellComment(new CellAddress(2, 0)).getString().getString();
+            assertEquals(comment3, "comment top row3 (index2)\n");
+            String comment4 = sheet.getCellComment(new CellAddress(3, 0)).getString().getString();
+            assertEquals(comment4, "comment top row4 (index3)\n");
+
+            //Workbook wbBack = _testDataProvider.writeOutAndReadBack(wb);
+
+            // Shifting all but first line down to test comments shifting
+            sheet.shiftRows(1, sheet.getLastRowNum(), 1, true, true);
+
+            // Test that comments were shifted as expected
+            assertEquals(4, sheet.getLastRowNum());
+            assertNotNull(sheet.getCellComment(new CellAddress(0, 0)));
+            assertNull(sheet.getCellComment(new CellAddress(1, 0)));
+            assertNull(sheet.getCellComment(new CellAddress(2, 0)));
+            assertNotNull(sheet.getCellComment(new CellAddress(3, 0)));
+            assertNotNull(sheet.getCellComment(new CellAddress(4, 0)));
+
+            String comment1_shifted = sheet.getCellComment(new CellAddress(0, 0)).getString().getString();
+            assertEquals(comment1, comment1_shifted);
+            String comment3_shifted = sheet.getCellComment(new CellAddress(3, 0)).getString().getString();
+            assertEquals(comment3, comment3_shifted);
+            String comment4_shifted = sheet.getCellComment(new CellAddress(4, 0)).getString().getString();
+            assertEquals(comment4, comment4_shifted);
+
+            // Write out and read back in again
+            // Ensure that the changes were persisted
+            try (Workbook wb2 = _testDataProvider.writeOutAndReadBack(wb1)) {
+
+                sheet = wb2.getSheet("Sheet1");
+                assertEquals(4, sheet.getLastRowNum());
+
+                // Verify comments are in the position expected after the shift
+                assertNotNull(sheet.getCellComment(new CellAddress(0, 0)));
+                assertNull(sheet.getCellComment(new CellAddress(1, 0)));
+                assertNull(sheet.getCellComment(new CellAddress(2, 0)));
+                assertNotNull(sheet.getCellComment(new CellAddress(3, 0)));
+                assertNotNull(sheet.getCellComment(new CellAddress(4, 0)));
+
+                comment1_shifted = sheet.getCellComment(new CellAddress(0, 0)).getString().getString();
+                assertEquals(comment1, comment1_shifted);
+                comment3_shifted = sheet.getCellComment(new CellAddress(3, 0)).getString().getString();
+                assertEquals(comment3, comment3_shifted);
+                comment4_shifted = sheet.getCellComment(new CellAddress(4, 0)).getString().getString();
+                assertEquals(comment4, comment4_shifted);
+
+                // Shifting back up again, now two rows
+                sheet.shiftRows(2, sheet.getLastRowNum(), -2, true, true);
+
+                // TODO: it seems HSSFSheet does not correctly remove comments from rows that are overwritten
+                // by shifting rows...
+                if (!(wb2 instanceof HSSFWorkbook)) {
+                    assertEquals(2, sheet.getLastRowNum());
+
+                    // Verify comments are in the position expected
+                    assertNull(sheet.getCellComment(new CellAddress(0, 0)),
+                        "Had: " + (sheet.getCellComment(new CellAddress(0, 0)) == null ? "null" : sheet.getCellComment(new CellAddress(0, 0)).getString()));
+                    assertNotNull(sheet.getCellComment(new CellAddress(1, 0)));
+                    assertNotNull(sheet.getCellComment(new CellAddress(2, 0)));
+                }
+
+                comment1 = sheet.getCellComment(new CellAddress(1, 0)).getString().getString();
+                assertEquals(comment1, "comment top row3 (index2)\n");
+                String comment2 = sheet.getCellComment(new CellAddress(2, 0)).getString().getString();
+                assertEquals(comment2, "comment top row4 (index3)\n");
+            }
         }
-
-        comment1 = sheet.getCellComment(new CellAddress(1,0)).getString().getString();
-        assertEquals(comment1,"comment top row3 (index2)\n");
-        String comment2 = sheet.getCellComment(new CellAddress(2,0)).getString().getString();
-        assertEquals(comment2,"comment top row4 (index3)\n");
-
-        wb2.close();
     }
 
     @Test
     public final void testShiftWithNames() throws IOException {
-        Workbook wb = _testDataProvider.createWorkbook();
-        Sheet sheet1 = wb.createSheet("Sheet1");
-        wb.createSheet("Sheet2");
-        Row row = sheet1.createRow(0);
-        row.createCell(0).setCellValue(1.1);
-        row.createCell(1).setCellValue(2.2);
+        try (Workbook wb = _testDataProvider.createWorkbook()) {
+            Sheet sheet1 = wb.createSheet("Sheet1");
+            wb.createSheet("Sheet2");
+            Row row = sheet1.createRow(0);
+            row.createCell(0).setCellValue(1.1);
+            row.createCell(1).setCellValue(2.2);
 
-        Name name1 = wb.createName();
-        name1.setNameName("name1");
-        name1.setRefersToFormula("Sheet1!$A$1+Sheet1!$B$1");
+            Name name1 = wb.createName();
+            name1.setNameName("name1");
+            name1.setRefersToFormula("Sheet1!$A$1+Sheet1!$B$1");
 
-        Name name2 = wb.createName();
-        name2.setNameName("name2");
-        name2.setRefersToFormula("Sheet1!$A$1");
+            Name name2 = wb.createName();
+            name2.setNameName("name2");
+            name2.setRefersToFormula("Sheet1!$A$1");
 
-        //refers to A1 but on Sheet2. Should stay unaffected.
-        Name name3 = wb.createName();
-        name3.setNameName("name3");
-        name3.setRefersToFormula("Sheet2!$A$1");
+            //refers to A1 but on Sheet2. Should stay unaffected.
+            Name name3 = wb.createName();
+            name3.setNameName("name3");
+            name3.setRefersToFormula("Sheet2!$A$1");
 
-        //The scope of this one is Sheet2. Should stay unaffected.
-        Name name4 = wb.createName();
-        name4.setNameName("name4");
-        name4.setRefersToFormula("A1");
-        name4.setSheetIndex(1);
+            //The scope of this one is Sheet2. Should stay unaffected.
+            Name name4 = wb.createName();
+            name4.setNameName("name4");
+            name4.setRefersToFormula("A1");
+            name4.setSheetIndex(1);
 
-        sheet1.shiftRows(0, 1, 2);  //shift down the top row on Sheet1.
-        name1 = wb.getName("name1");
-        assertEquals("Sheet1!$A$3+Sheet1!$B$3", name1.getRefersToFormula());
+            sheet1.shiftRows(0, 1, 2);  //shift down the top row on Sheet1.
+            name1 = wb.getName("name1");
+            assertEquals("Sheet1!$A$3+Sheet1!$B$3", name1.getRefersToFormula());
 
-        name2 = wb.getName("name2");
-        assertEquals("Sheet1!$A$3", name2.getRefersToFormula());
+            name2 = wb.getName("name2");
+            assertEquals("Sheet1!$A$3", name2.getRefersToFormula());
 
-        //name3 and name4 refer to Sheet2 and should not be affected
-        name3 = wb.getName("name3");
-        assertEquals("Sheet2!$A$1", name3.getRefersToFormula());
+            //name3 and name4 refer to Sheet2 and should not be affected
+            name3 = wb.getName("name3");
+            assertEquals("Sheet2!$A$1", name3.getRefersToFormula());
 
-        name4 = wb.getName("name4");
-        assertEquals("A1", name4.getRefersToFormula());
-
-        wb.close();
+            name4 = wb.getName("name4");
+            assertEquals("A1", name4.getRefersToFormula());
+        }
     }
 
     @Test
     public final void testShiftWithMergedRegions() throws IOException {
-        Workbook wb = _testDataProvider.createWorkbook();
-        Sheet sheet = wb.createSheet();
-        Row row = sheet.createRow(0);
-        row.createCell(0).setCellValue(1.1);
-        row.createCell(1).setCellValue(2.2);
-        CellRangeAddress region = new CellRangeAddress(0, 0, 0, 2);
-        assertEquals("A1:C1", region.formatAsString());
+        try (Workbook wb = _testDataProvider.createWorkbook()) {
+            Sheet sheet = wb.createSheet();
+            Row row = sheet.createRow(0);
+            row.createCell(0).setCellValue(1.1);
+            row.createCell(1).setCellValue(2.2);
+            CellRangeAddress region = new CellRangeAddress(0, 0, 0, 2);
+            assertEquals("A1:C1", region.formatAsString());
 
-        assertEquals(0, sheet.addMergedRegion(region));
+            assertEquals(0, sheet.addMergedRegion(region));
 
-        sheet.shiftRows(0, 1, 2);
-        region = sheet.getMergedRegion(0);
-        assertEquals("A3:C3", region.formatAsString());
-        wb.close();
+            sheet.shiftRows(0, 1, 2);
+            region = sheet.getMergedRegion(0);
+            assertEquals("A3:C3", region.formatAsString());
+        }
     }
 
     //@Disabled("bug 56454: Incorrectly handles merged regions that do not contain column 0")
     @Test
     public final void shiftWithMergedRegions_bug56454() throws IOException {
-        Workbook wb = _testDataProvider.createWorkbook();
-        Sheet sheet = wb.createSheet();
-        // populate sheet cells
-        for (int i = 0; i < 10; i++) {
-            Row row = sheet.createRow(i);
+        try (Workbook wb = _testDataProvider.createWorkbook()) {
+            Sheet sheet = wb.createSheet();
+            // populate sheet cells
+            for (int i = 0; i < 10; i++) {
+                Row row = sheet.createRow(i);
 
-            for (int j = 0; j < 10; j++) {
-                Cell cell = row.createCell(j, CellType.STRING);
-                cell.setCellValue(i + "x" + j);
+                for (int j = 0; j < 10; j++) {
+                    Cell cell = row.createCell(j, CellType.STRING);
+                    cell.setCellValue(i + "x" + j);
+                }
             }
+
+            CellRangeAddress A4_B7 = CellRangeAddress.valueOf("A4:B7");
+            CellRangeAddress C4_D7 = CellRangeAddress.valueOf("C4:D7");
+
+            assertEquals(0, sheet.addMergedRegion(A4_B7));
+            assertEquals(1, sheet.addMergedRegion(C4_D7));
+
+            assumeTrue(sheet.getLastRowNum() > 8);
+
+            // Insert a row in the middle of both merged regions.
+            sheet.shiftRows(4, sheet.getLastRowNum(), 1);
+
+            // all regions should still start at row 3, and elongate by 1 row
+            List expectedMergedRegions = new ArrayList<>();
+            CellRangeAddress A4_B8 = CellRangeAddress.valueOf("A4:B8"); //A4:B7 should be elongated by 1 row
+            CellRangeAddress C4_D8 = CellRangeAddress.valueOf("C4:D8"); //C4:B7 should be elongated by 1 row
+            expectedMergedRegions.add(A4_B8);
+            expectedMergedRegions.add(C4_D8);
+
+            // This test is written as expected-to-fail and should be rewritten
+            // as expected-to-pass when the bug is fixed.
+            // FIXME: remove try, catch, and testPassesNow, skipTest when test passes
+            assertNotEquals(expectedMergedRegions, sheet.getMergedRegions());
         }
-
-        CellRangeAddress A4_B7 = CellRangeAddress.valueOf("A4:B7");
-        CellRangeAddress C4_D7 = CellRangeAddress.valueOf("C4:D7");
-
-        assertEquals(0, sheet.addMergedRegion(A4_B7));
-        assertEquals(1, sheet.addMergedRegion(C4_D7));
-
-        assumeTrue(sheet.getLastRowNum() > 8);
-
-        // Insert a row in the middle of both merged regions.
-        sheet.shiftRows(4, sheet.getLastRowNum(), 1);
-
-        // all regions should still start at row 3, and elongate by 1 row
-        List expectedMergedRegions = new ArrayList<>();
-        CellRangeAddress A4_B8 = CellRangeAddress.valueOf("A4:B8"); //A4:B7 should be elongated by 1 row
-        CellRangeAddress C4_D8 = CellRangeAddress.valueOf("C4:D8"); //C4:B7 should be elongated by 1 row
-        expectedMergedRegions.add(A4_B8);
-        expectedMergedRegions.add(C4_D8);
-
-        // This test is written as expected-to-fail and should be rewritten
-        // as expected-to-pass when the bug is fixed.
-        // FIXME: remove try, catch, and testPassesNow, skipTest when test passes
-        try {
-            assertEquals(expectedMergedRegions, sheet.getMergedRegions());
-            testPassesNow(56454);
-        } catch (AssertionError e) {
-            skipTest(e);
-        }
-        wb.close();
     }
 
 
@@ -347,60 +336,59 @@ public abstract class BaseTestSheetShiftRows {
      */
     @Test
     public final void testShiftWithFormulas() throws IOException {
-        Workbook wb = _testDataProvider.openSampleWorkbook("ForShifting." + _testDataProvider.getStandardFileNameExtension());
+        try (Workbook wb = _testDataProvider.openSampleWorkbook("ForShifting." + _testDataProvider.getStandardFileNameExtension())) {
 
-        Sheet sheet = wb.getSheet("Sheet1");
-        assertEquals(20, sheet.getLastRowNum());
+            Sheet sheet = wb.getSheet("Sheet1");
+            assertEquals(20, sheet.getLastRowNum());
 
-        confirmRow(sheet, 0, 1, 171, 1, "ROW(D1)", "100+B1", "COUNT(D1:E1)");
-        confirmRow(sheet, 1, 2, 172, 1, "ROW(D2)", "100+B2", "COUNT(D2:E2)");
-        confirmRow(sheet, 2, 3, 173, 1, "ROW(D3)", "100+B3", "COUNT(D3:E3)");
+            confirmRow(sheet, 0, 1, 171, 1, "ROW(D1)", "100+B1", "COUNT(D1:E1)");
+            confirmRow(sheet, 1, 2, 172, 1, "ROW(D2)", "100+B2", "COUNT(D2:E2)");
+            confirmRow(sheet, 2, 3, 173, 1, "ROW(D3)", "100+B3", "COUNT(D3:E3)");
 
-        confirmCell(sheet, 6, 1, 271, "200+B1");
-        confirmCell(sheet, 7, 1, 272, "200+B2");
-        confirmCell(sheet, 8, 1, 273, "200+B3");
+            confirmCell(sheet, 6, 1, 271, "200+B1");
+            confirmCell(sheet, 7, 1, 272, "200+B2");
+            confirmCell(sheet, 8, 1, 273, "200+B3");
 
-        confirmCell(sheet, 14, 0, 0.0, "A12"); // the cell referred to by this formula will be replaced
+            confirmCell(sheet, 14, 0, 0.0, "A12"); // the cell referred to by this formula will be replaced
 
-        // -----------
-        // Row index 1 -> 11 (row "2" -> row "12")
-        sheet.shiftRows(1, 1, 10);
+            // -----------
+            // Row index 1 -> 11 (row "2" -> row "12")
+            sheet.shiftRows(1, 1, 10);
 
-        // Now check what sheet looks like after move
+            // Now check what sheet looks like after move
 
-        // no changes on row "1"
-        confirmRow(sheet, 0, 1, 171, 1, "ROW(D1)", "100+B1", "COUNT(D1:E1)");
+            // no changes on row "1"
+            confirmRow(sheet, 0, 1, 171, 1, "ROW(D1)", "100+B1", "COUNT(D1:E1)");
 
-        // row "2" is now empty
-        confirmEmptyRow(sheet, 1);
+            // row "2" is now empty
+            confirmEmptyRow(sheet, 1);
 
-        // Row "2" moved to row "12", and the formula has been updated.
-        // note however that the cached formula result (2) has not been updated. (POI differs from Excel here)
-        confirmRow(sheet, 11, 2, 172, 1, "ROW(D12)", "100+B12", "COUNT(D12:E12)");
+            // Row "2" moved to row "12", and the formula has been updated.
+            // note however that the cached formula result (2) has not been updated. (POI differs from Excel here)
+            confirmRow(sheet, 11, 2, 172, 1, "ROW(D12)", "100+B12", "COUNT(D12:E12)");
 
-        // no changes on row "3"
-        confirmRow(sheet, 2, 3, 173, 1, "ROW(D3)", "100+B3", "COUNT(D3:E3)");
+            // no changes on row "3"
+            confirmRow(sheet, 2, 3, 173, 1, "ROW(D3)", "100+B3", "COUNT(D3:E3)");
 
 
-        confirmCell(sheet, 14, 0, 0.0, "#REF!");
+            confirmCell(sheet, 14, 0, 0.0, "#REF!");
 
 
-        // Formulas on rows that weren't shifted:
-        confirmCell(sheet, 6, 1, 271, "200+B1");
-        confirmCell(sheet, 7, 1, 272, "200+B12"); // this one moved
-        confirmCell(sheet, 8, 1, 273, "200+B3");
+            // Formulas on rows that weren't shifted:
+            confirmCell(sheet, 6, 1, 271, "200+B1");
+            confirmCell(sheet, 7, 1, 272, "200+B12"); // this one moved
+            confirmCell(sheet, 8, 1, 273, "200+B3");
 
-        // check formulas on other sheets
-        Sheet sheet2 = wb.getSheet("Sheet2");
-        confirmCell(sheet2,  0, 0, 371, "300+Sheet1!B1");
-        confirmCell(sheet2,  1, 0, 372, "300+Sheet1!B12");
-        confirmCell(sheet2,  2, 0, 373, "300+Sheet1!B3");
+            // check formulas on other sheets
+            Sheet sheet2 = wb.getSheet("Sheet2");
+            confirmCell(sheet2, 0, 0, 371, "300+Sheet1!B1");
+            confirmCell(sheet2, 1, 0, 372, "300+Sheet1!B12");
+            confirmCell(sheet2, 2, 0, 373, "300+Sheet1!B3");
 
-        confirmCell(sheet2, 11, 0, 300, "300+Sheet1!#REF!");
+            confirmCell(sheet2, 11, 0, 300, "300+Sheet1!#REF!");
 
-
-        // Note - named ranges formulas have not been updated
-        wb.close();
+            // Note - named ranges formulas have not been updated
+        }
     }
 
     private static void confirmRow(Sheet sheet, int rowIx, double valA, double valB, double valC,
@@ -419,72 +407,72 @@ public abstract class BaseTestSheetShiftRows {
 
     @Test
     public final void testShiftSharedFormulasBug54206() throws IOException {
-        Workbook wb = _testDataProvider.openSampleWorkbook("54206." + _testDataProvider.getStandardFileNameExtension());
+        try (Workbook wb = _testDataProvider.openSampleWorkbook("54206." + _testDataProvider.getStandardFileNameExtension())) {
 
-        Sheet sheet = wb.getSheetAt(0);
-        assertEquals("SUMIF($B$19:$B$82,$B4,G$19:G$82)", sheet.getRow(3).getCell(6).getCellFormula());
-        assertEquals("SUMIF($B$19:$B$82,$B4,H$19:H$82)", sheet.getRow(3).getCell(7).getCellFormula());
-        assertEquals("SUMIF($B$19:$B$82,$B4,I$19:I$82)", sheet.getRow(3).getCell(8).getCellFormula());
+            Sheet sheet = wb.getSheetAt(0);
+            assertEquals("SUMIF($B$19:$B$82,$B4,G$19:G$82)", sheet.getRow(3).getCell(6).getCellFormula());
+            assertEquals("SUMIF($B$19:$B$82,$B4,H$19:H$82)", sheet.getRow(3).getCell(7).getCellFormula());
+            assertEquals("SUMIF($B$19:$B$82,$B4,I$19:I$82)", sheet.getRow(3).getCell(8).getCellFormula());
 
-        assertEquals("SUMIF($B$19:$B$82,$B15,G$19:G$82)", sheet.getRow(14).getCell(6).getCellFormula());
-        assertEquals("SUMIF($B$19:$B$82,$B15,H$19:H$82)", sheet.getRow(14).getCell(7).getCellFormula());
-        assertEquals("SUMIF($B$19:$B$82,$B15,I$19:I$82)", sheet.getRow(14).getCell(8).getCellFormula());
+            assertEquals("SUMIF($B$19:$B$82,$B15,G$19:G$82)", sheet.getRow(14).getCell(6).getCellFormula());
+            assertEquals("SUMIF($B$19:$B$82,$B15,H$19:H$82)", sheet.getRow(14).getCell(7).getCellFormula());
+            assertEquals("SUMIF($B$19:$B$82,$B15,I$19:I$82)", sheet.getRow(14).getCell(8).getCellFormula());
 
-        // now the whole block G4L:15
-        for(int i = 3; i <= 14; i++){
-            for(int j = 6; j <= 8; j++){
-                String col = CellReference.convertNumToColString(j);
-                String expectedFormula = "SUMIF($B$19:$B$82,$B"+(i+1)+","+col+"$19:"+col+"$82)";
-                assertEquals(expectedFormula, sheet.getRow(i).getCell(j).getCellFormula());
+            // now the whole block G4L:15
+            for (int i = 3; i <= 14; i++) {
+                for (int j = 6; j <= 8; j++) {
+                    String col = CellReference.convertNumToColString(j);
+                    String expectedFormula = "SUMIF($B$19:$B$82,$B" + (i + 1) + "," + col + "$19:" + col + "$82)";
+                    assertEquals(expectedFormula, sheet.getRow(i).getCell(j).getCellFormula());
+                }
             }
-        }
 
-        assertEquals("SUM(G24:I24)", sheet.getRow(23).getCell(9).getCellFormula());
-        assertEquals("SUM(G25:I25)", sheet.getRow(24).getCell(9).getCellFormula());
-        assertEquals("SUM(G26:I26)", sheet.getRow(25).getCell(9).getCellFormula());
+            assertEquals("SUM(G24:I24)", sheet.getRow(23).getCell(9).getCellFormula());
+            assertEquals("SUM(G25:I25)", sheet.getRow(24).getCell(9).getCellFormula());
+            assertEquals("SUM(G26:I26)", sheet.getRow(25).getCell(9).getCellFormula());
 
-        sheet.shiftRows(24, sheet.getLastRowNum(), 4, true, false);
+            sheet.shiftRows(24, sheet.getLastRowNum(), 4, true, false);
 
-        assertEquals("SUMIF($B$19:$B$86,$B4,G$19:G$86)", sheet.getRow(3).getCell(6).getCellFormula());
-        assertEquals("SUMIF($B$19:$B$86,$B4,H$19:H$86)", sheet.getRow(3).getCell(7).getCellFormula());
-        assertEquals("SUMIF($B$19:$B$86,$B4,I$19:I$86)", sheet.getRow(3).getCell(8).getCellFormula());
+            assertEquals("SUMIF($B$19:$B$86,$B4,G$19:G$86)", sheet.getRow(3).getCell(6).getCellFormula());
+            assertEquals("SUMIF($B$19:$B$86,$B4,H$19:H$86)", sheet.getRow(3).getCell(7).getCellFormula());
+            assertEquals("SUMIF($B$19:$B$86,$B4,I$19:I$86)", sheet.getRow(3).getCell(8).getCellFormula());
 
-        assertEquals("SUMIF($B$19:$B$86,$B15,G$19:G$86)", sheet.getRow(14).getCell(6).getCellFormula());
-        assertEquals("SUMIF($B$19:$B$86,$B15,H$19:H$86)", sheet.getRow(14).getCell(7).getCellFormula());
-        assertEquals("SUMIF($B$19:$B$86,$B15,I$19:I$86)", sheet.getRow(14).getCell(8).getCellFormula());
+            assertEquals("SUMIF($B$19:$B$86,$B15,G$19:G$86)", sheet.getRow(14).getCell(6).getCellFormula());
+            assertEquals("SUMIF($B$19:$B$86,$B15,H$19:H$86)", sheet.getRow(14).getCell(7).getCellFormula());
+            assertEquals("SUMIF($B$19:$B$86,$B15,I$19:I$86)", sheet.getRow(14).getCell(8).getCellFormula());
 
-        // now the whole block G4L:15
-        for(int i = 3; i <= 14; i++){
-            for(int j = 6; j <= 8; j++){
-                String col = CellReference.convertNumToColString(j);
-                String expectedFormula = "SUMIF($B$19:$B$86,$B"+(i+1)+","+col+"$19:"+col+"$86)";
-                assertEquals(expectedFormula, sheet.getRow(i).getCell(j).getCellFormula());
+            // now the whole block G4L:15
+            for (int i = 3; i <= 14; i++) {
+                for (int j = 6; j <= 8; j++) {
+                    String col = CellReference.convertNumToColString(j);
+                    String expectedFormula = "SUMIF($B$19:$B$86,$B" + (i + 1) + "," + col + "$19:" + col + "$86)";
+                    assertEquals(expectedFormula, sheet.getRow(i).getCell(j).getCellFormula());
+                }
             }
+
+            assertEquals("SUM(G24:I24)", sheet.getRow(23).getCell(9).getCellFormula());
+
+            // shifted rows
+            assertTrue(sheet.getRow(24) == null || sheet.getRow(24).getCell(9) == null);
+            assertTrue(sheet.getRow(25) == null || sheet.getRow(25).getCell(9) == null);
+            assertTrue(sheet.getRow(26) == null || sheet.getRow(26).getCell(9) == null);
+            assertTrue(sheet.getRow(27) == null || sheet.getRow(27).getCell(9) == null);
+
+            assertEquals("SUM(G29:I29)", sheet.getRow(28).getCell(9).getCellFormula());
+            assertEquals("SUM(G30:I30)", sheet.getRow(29).getCell(9).getCellFormula());
         }
-
-        assertEquals("SUM(G24:I24)", sheet.getRow(23).getCell(9).getCellFormula());
-
-        // shifted rows
-        assertTrue( sheet.getRow(24) == null || sheet.getRow(24).getCell(9) == null);
-        assertTrue( sheet.getRow(25) == null || sheet.getRow(25).getCell(9) == null);
-        assertTrue( sheet.getRow(26) == null || sheet.getRow(26).getCell(9) == null);
-        assertTrue( sheet.getRow(27) == null || sheet.getRow(27).getCell(9) == null);
-
-        assertEquals("SUM(G29:I29)", sheet.getRow(28).getCell(9).getCellFormula());
-        assertEquals("SUM(G30:I30)", sheet.getRow(29).getCell(9).getCellFormula());
-        wb.close();
     }
 
     @Test
     void testBug55280() throws IOException {
-        Workbook w = _testDataProvider.createWorkbook();
-        Sheet s = w.createSheet();
-        for (int row = 0; row < 5000; ++row) {
-            assertEquals(row, s.addMergedRegion(new CellRangeAddress(row, row, 0, 3)));
-        }
+        try (Workbook w = _testDataProvider.createWorkbook()) {
+            Sheet s = w.createSheet();
+            for (int row = 0; row < 5000; ++row) {
+                assertEquals(row, s.addMergedRegion(new CellRangeAddress(row, row, 0, 3)));
+            }
 
-        s.shiftRows(0, 4999, 1);        // takes a long time...
-        w.close();
+            s.shiftRows(0, 4999, 1);        // takes a long time...
+        }
     }
 
     @Test
@@ -504,203 +492,191 @@ public abstract class BaseTestSheetShiftRows {
      */
     @Test
     void testBug46742_52903_shiftHyperlinks() throws IOException {
-        Workbook wb = _testDataProvider.createWorkbook();
-        Sheet sheet = wb.createSheet("test");
-        Row row = sheet.createRow(0);
+        try (Workbook wb = _testDataProvider.createWorkbook()) {
+            Sheet sheet = wb.createSheet("test");
+            Row row = sheet.createRow(0);
 
-        // How to create hyperlinks
-        // https://poi.apache.org/spreadsheet/quick-guide.html#Hyperlinks
-        CreationHelper helper = wb.getCreationHelper();
-        CellStyle hlinkStyle = wb.createCellStyle();
-        Font hlinkFont = wb.createFont();
-        hlinkFont.setUnderline(Font.U_SINGLE);
-        hlinkFont.setColor(IndexedColors.BLUE.getIndex());
-        hlinkStyle.setFont(hlinkFont);
+            // How to create hyperlinks
+            // https://poi.apache.org/spreadsheet/quick-guide.html#Hyperlinks
+            CreationHelper helper = wb.getCreationHelper();
+            CellStyle hlinkStyle = wb.createCellStyle();
+            Font hlinkFont = wb.createFont();
+            hlinkFont.setUnderline(Font.U_SINGLE);
+            hlinkFont.setColor(IndexedColors.BLUE.getIndex());
+            hlinkStyle.setFont(hlinkFont);
 
-        // 3D relative document link
-        // CellAddress=A1, shifted to A4
-        Cell cell = row.createCell(0);
-        cell.setCellStyle(hlinkStyle);
-        createHyperlink(helper, cell, HyperlinkType.DOCUMENT, "test!E1");
+            // 3D relative document link
+            // CellAddress=A1, shifted to A4
+            Cell cell = row.createCell(0);
+            cell.setCellStyle(hlinkStyle);
+            createHyperlink(helper, cell, HyperlinkType.DOCUMENT, "test!E1");
 
-        // URL
-        cell = row.createCell(1);
-        // CellAddress=B1, shifted to B4
-        cell.setCellStyle(hlinkStyle);
-        createHyperlink(helper, cell, HyperlinkType.URL, "https://poi.apache.org/");
+            // URL
+            cell = row.createCell(1);
+            // CellAddress=B1, shifted to B4
+            cell.setCellStyle(hlinkStyle);
+            createHyperlink(helper, cell, HyperlinkType.URL, "https://poi.apache.org/");
 
-        // row0 will be shifted on top of row1, so this URL should be removed from the workbook
-        Row overwrittenRow = sheet.createRow(3);
-        cell = overwrittenRow.createCell(2);
-        // CellAddress=C4, will be overwritten (deleted)
-        cell.setCellStyle(hlinkStyle);
-        createHyperlink(helper, cell, HyperlinkType.EMAIL, "mailto:poi@apache.org");
+            // row0 will be shifted on top of row1, so this URL should be removed from the workbook
+            Row overwrittenRow = sheet.createRow(3);
+            cell = overwrittenRow.createCell(2);
+            // CellAddress=C4, will be overwritten (deleted)
+            cell.setCellStyle(hlinkStyle);
+            createHyperlink(helper, cell, HyperlinkType.EMAIL, "mailto:poi@apache.org");
 
-        // hyperlinks on this row are unaffected by the row shifting, so the hyperlinks should not move
-        Row unaffectedRow = sheet.createRow(20);
-        cell = unaffectedRow.createCell(3);
-        // CellAddress=D21, will be unaffected
-        cell.setCellStyle(hlinkStyle);
-        createHyperlink(helper, cell, HyperlinkType.FILE, "54524.xlsx");
+            // hyperlinks on this row are unaffected by the row shifting, so the hyperlinks should not move
+            Row unaffectedRow = sheet.createRow(20);
+            cell = unaffectedRow.createCell(3);
+            // CellAddress=D21, will be unaffected
+            cell.setCellStyle(hlinkStyle);
+            createHyperlink(helper, cell, HyperlinkType.FILE, "54524.xlsx");
 
-        cell = wb.createSheet("other").createRow(0).createCell(0);
-        // CellAddress=Other!A1, will be unaffected
-        cell.setCellStyle(hlinkStyle);
-        createHyperlink(helper, cell, HyperlinkType.URL, "http://apache.org/");
+            cell = wb.createSheet("other").createRow(0).createCell(0);
+            // CellAddress=Other!A1, will be unaffected
+            cell.setCellStyle(hlinkStyle);
+            createHyperlink(helper, cell, HyperlinkType.URL, "http://apache.org/");
 
-        int startRow = 0;
-        int endRow = 0;
-        int n = 3;
-        sheet.shiftRows(startRow, endRow, n);
+            int startRow = 0;
+            int endRow = 0;
+            int n = 3;
+            sheet.shiftRows(startRow, endRow, n);
 
-        Workbook read = _testDataProvider.writeOutAndReadBack(wb);
-        wb.close();
+            try (Workbook read = _testDataProvider.writeOutAndReadBack(wb)) {
+                Sheet sh = read.getSheet("test");
+                Row shiftedRow = sh.getRow(3);
 
-        Sheet sh = read.getSheet("test");
+                // document link anchored on a shifted cell should be moved
+                // Note that hyperlinks do not track what they point to, so this hyperlink should still refer to test!E1
+                verifyHyperlink(shiftedRow.getCell(0), HyperlinkType.DOCUMENT, "test!E1");
 
-        Row shiftedRow = sh.getRow(3);
+                // URL, EMAIL, and FILE links anchored on a shifted cell should be moved
+                verifyHyperlink(shiftedRow.getCell(1), HyperlinkType.URL, "https://poi.apache.org/");
 
-        // document link anchored on a shifted cell should be moved
-        // Note that hyperlinks do not track what they point to, so this hyperlink should still refer to test!E1
-        verifyHyperlink(shiftedRow.getCell(0), HyperlinkType.DOCUMENT, "test!E1");
+                // Make sure hyperlinks were moved and not copied
+                assertNull(sh.getHyperlink(0, 0), "Document hyperlink should be moved, not copied");
+                assertNull(sh.getHyperlink(0, 1), "URL hyperlink should be moved, not copied");
 
-        // URL, EMAIL, and FILE links anchored on a shifted cell should be moved
-        verifyHyperlink(shiftedRow.getCell(1), HyperlinkType.URL, "https://poi.apache.org/");
+                // Make sure hyperlink in overwritten row is deleted
+                assertEquals(3, sh.getHyperlinkList().size());
+                CellAddress unexpectedLinkAddress = new CellAddress("C4");
+                for (Hyperlink link : sh.getHyperlinkList()) {
+                    final CellAddress linkAddress = new CellAddress(link.getFirstRow(), link.getFirstColumn());
+                    assertNotEquals(linkAddress, unexpectedLinkAddress,
+                        "Row 4, including the hyperlink at C4, should have " +
+                            "been deleted when Row 1 was shifted on top of it.");
+                }
 
-        // Make sure hyperlinks were moved and not copied
-        assertNull(sh.getHyperlink(0, 0), "Document hyperlink should be moved, not copied");
-        assertNull(sh.getHyperlink(0, 1), "URL hyperlink should be moved, not copied");
+                // Make sure unaffected rows are not shifted
+                Cell unaffectedCell = sh.getRow(20).getCell(3);
+                assertTrue(cellHasHyperlink(unaffectedCell));
+                verifyHyperlink(unaffectedCell, HyperlinkType.FILE, "54524.xlsx");
 
-        // Make sure hyperlink in overwritten row is deleted
-        assertEquals(3, sh.getHyperlinkList().size());
-        CellAddress unexpectedLinkAddress = new CellAddress("C4");
-        for (Hyperlink link : sh.getHyperlinkList()) {
-            final CellAddress linkAddress = new CellAddress(link.getFirstRow(), link.getFirstColumn());
-            assertNotEquals(linkAddress, unexpectedLinkAddress,
-                "Row 4, including the hyperlink at C4, should have " +
-                    "been deleted when Row 1 was shifted on top of it.");
+                // Make sure cells on other sheets are not affected
+                unaffectedCell = read.getSheet("other").getRow(0).getCell(0);
+                assertTrue(cellHasHyperlink(unaffectedCell));
+                verifyHyperlink(unaffectedCell, HyperlinkType.URL, "http://apache.org/");
+            }
         }
-
-        // Make sure unaffected rows are not shifted
-        Cell unaffectedCell = sh.getRow(20).getCell(3);
-        assertTrue(cellHasHyperlink(unaffectedCell));
-        verifyHyperlink(unaffectedCell, HyperlinkType.FILE, "54524.xlsx");
-
-        // Make sure cells on other sheets are not affected
-        unaffectedCell = read.getSheet("other").getRow(0).getCell(0);
-        assertTrue(cellHasHyperlink(unaffectedCell));
-        verifyHyperlink(unaffectedCell, HyperlinkType.URL, "http://apache.org/");
-
-        read.close();
     }
 
     //@Disabled("bug 56454: Incorrectly handles merged regions that do not contain column 0")
     @Test
     void shiftRowsWithMergedRegionsThatDoNotContainColumnZero() throws IOException {
-        Workbook wb = _testDataProvider.createWorkbook();
-        Sheet sheet = wb.createSheet("test");
+        try (Workbook wb = _testDataProvider.createWorkbook()) {
+            Sheet sheet = wb.createSheet("test");
 
-        // populate sheet cells
-        for (int i = 0; i < 10; i++) {
-            Row row = sheet.createRow(i);
-            for (int j = 0; j < 12; j++) {
-                Cell cell = row.createCell(j);
-                cell.setCellValue(i + "x" + j);
+            // populate sheet cells
+            for (int i = 0; i < 10; i++) {
+                Row row = sheet.createRow(i);
+                for (int j = 0; j < 12; j++) {
+                    Cell cell = row.createCell(j);
+                    cell.setCellValue(i + "x" + j);
+                }
             }
-        }
 
-        CellRangeAddress A4_B7 = new CellRangeAddress(3, 6, 0, 1);
-        CellRangeAddress C5_D7 = new CellRangeAddress(4, 6, 2, 3);
+            CellRangeAddress A4_B7 = new CellRangeAddress(3, 6, 0, 1);
+            CellRangeAddress C5_D7 = new CellRangeAddress(4, 6, 2, 3);
 
-        assertEquals(0, sheet.addMergedRegion(A4_B7));
-        assertEquals(1, sheet.addMergedRegion(C5_D7));
+            assertEquals(0, sheet.addMergedRegion(A4_B7));
+            assertEquals(1, sheet.addMergedRegion(C5_D7));
 
-        // A4:B7 will elongate vertically
-        // C5:D7 will be shifted down with same size
-        sheet.shiftRows(4, sheet.getLastRowNum(), 1);
+            // A4:B7 will elongate vertically
+            // C5:D7 will be shifted down with same size
+            sheet.shiftRows(4, sheet.getLastRowNum(), 1);
 
-        // This test is written as expected-to-fail and should be rewritten
-        // as expected-to-pass when the bug is fixed.
-        // FIXME: remove try, catch, and testPassesNow, skipTest when test passes
-        try {
+            // This test is written as expected-to-fail and should be rewritten
+            // as expected-to-pass when the bug is fixed.
+            // FIXME: remove try, catch, and testPassesNow, skipTest when test passes
             assertEquals(2, sheet.getNumMergedRegions());
-            assertEquals(CellRangeAddress.valueOf("A4:B8"), sheet.getMergedRegion(0));
-            assertEquals(CellRangeAddress.valueOf("C5:D8"), sheet.getMergedRegion(1));
-            testPassesNow(56454);
-        } catch (AssertionError e) {
-            skipTest(e);
+            assertNotEquals(CellRangeAddress.valueOf("A4:B8"), sheet.getMergedRegion(0));
+            assertNotEquals(CellRangeAddress.valueOf("C5:D8"), sheet.getMergedRegion(1));
         }
-
-        wb.close();
     }
 
     @Test
     void shiftMergedRowsToMergedRowsUp() throws IOException {
-        Workbook wb = _testDataProvider.createWorkbook();
-        Sheet sheet = wb.createSheet("test");
-        populateSheetCells(sheet, 2);
+        try (Workbook wb = _testDataProvider.createWorkbook()) {
+            Sheet sheet = wb.createSheet("test");
+            populateSheetCells(sheet, 2);
 
 
-        CellRangeAddress A1_E1 = new CellRangeAddress(0, 0, 0, 4);
-        CellRangeAddress A2_C2 = new CellRangeAddress(1, 1, 0, 2);
+            CellRangeAddress A1_E1 = new CellRangeAddress(0, 0, 0, 4);
+            CellRangeAddress A2_C2 = new CellRangeAddress(1, 1, 0, 2);
 
-        assertEquals(0, sheet.addMergedRegion(A1_E1));
-        assertEquals(1, sheet.addMergedRegion(A2_C2));
+            assertEquals(0, sheet.addMergedRegion(A1_E1));
+            assertEquals(1, sheet.addMergedRegion(A2_C2));
 
-        // A1:E1 should be removed
-        // A2:C2 will be A1:C1
-        sheet.shiftRows(1, sheet.getLastRowNum(), -1);
+            // A1:E1 should be removed
+            // A2:C2 will be A1:C1
+            sheet.shiftRows(1, sheet.getLastRowNum(), -1);
 
-        assertEquals(1, sheet.getNumMergedRegions());
-        assertEquals(CellRangeAddress.valueOf("A1:C1"), sheet.getMergedRegion(0));
-
-        wb.close();
+            assertEquals(1, sheet.getNumMergedRegions());
+            assertEquals(CellRangeAddress.valueOf("A1:C1"), sheet.getMergedRegion(0));
+        }
     }
 
     @Test
     void shiftMergedRowsToMergedRowsOverlappingMergedRegion() throws IOException {
-        Workbook wb = _testDataProvider.createWorkbook();
-        Sheet sheet = wb.createSheet("test");
-        populateSheetCells(sheet, 10);
+        try (Workbook wb = _testDataProvider.createWorkbook()) {
+            Sheet sheet = wb.createSheet("test");
+            populateSheetCells(sheet, 10);
 
-        CellRangeAddress A1_E1 = new CellRangeAddress(0, 0, 0, 4);
-        CellRangeAddress A2_C2 = new CellRangeAddress(1, 7, 0, 2);
+            CellRangeAddress A1_E1 = new CellRangeAddress(0, 0, 0, 4);
+            CellRangeAddress A2_C2 = new CellRangeAddress(1, 7, 0, 2);
 
-        assertEquals(0, sheet.addMergedRegion(A1_E1));
-        assertEquals(1, sheet.addMergedRegion(A2_C2));
+            assertEquals(0, sheet.addMergedRegion(A1_E1));
+            assertEquals(1, sheet.addMergedRegion(A2_C2));
 
-        // A1:E1 should move to A5:E5
-        // A2:C2 should be removed
-        sheet.shiftRows(0, 0, 4);
+            // A1:E1 should move to A5:E5
+            // A2:C2 should be removed
+            sheet.shiftRows(0, 0, 4);
 
-        assertEquals(1, sheet.getNumMergedRegions());
-        assertEquals(CellRangeAddress.valueOf("A5:E5"), sheet.getMergedRegion(0));
-
-        wb.close();
+            assertEquals(1, sheet.getNumMergedRegions());
+            assertEquals(CellRangeAddress.valueOf("A5:E5"), sheet.getMergedRegion(0));
+        }
     }
 
     @Test
     void bug60384ShiftMergedRegion() throws IOException {
-        Workbook wb = _testDataProvider.createWorkbook();
-        Sheet sheet = wb.createSheet("test");
-        populateSheetCells(sheet, 9);
+        try (Workbook wb = _testDataProvider.createWorkbook()) {
+            Sheet sheet = wb.createSheet("test");
+            populateSheetCells(sheet, 9);
 
 
-        CellRangeAddress A8_E8 = new CellRangeAddress(7, 7, 0, 4);
-        CellRangeAddress A9_C9 = new CellRangeAddress(8, 8, 0, 2);
+            CellRangeAddress A8_E8 = new CellRangeAddress(7, 7, 0, 4);
+            CellRangeAddress A9_C9 = new CellRangeAddress(8, 8, 0, 2);
 
-        assertEquals(0, sheet.addMergedRegion(A8_E8));
-        assertEquals(1, sheet.addMergedRegion(A9_C9));
+            assertEquals(0, sheet.addMergedRegion(A8_E8));
+            assertEquals(1, sheet.addMergedRegion(A9_C9));
 
-        // A1:E1 should be removed
-        // A2:C2 will be A1:C1
-        sheet.shiftRows(3, sheet.getLastRowNum(), 1);
+            // A1:E1 should be removed
+            // A2:C2 will be A1:C1
+            sheet.shiftRows(3, sheet.getLastRowNum(), 1);
 
-        assertEquals(2, sheet.getNumMergedRegions());
-        assertEquals(CellRangeAddress.valueOf("A9:E9"), sheet.getMergedRegion(0));
-        assertEquals(CellRangeAddress.valueOf("A10:C10"), sheet.getMergedRegion(1));
-
-        wb.close();
+            assertEquals(2, sheet.getNumMergedRegions());
+            assertEquals(CellRangeAddress.valueOf("A9:E9"), sheet.getMergedRegion(0));
+            assertEquals(CellRangeAddress.valueOf("A10:C10"), sheet.getMergedRegion(1));
+        }
     }
 
     private void populateSheetCells(Sheet sheet, int rowCount) {
@@ -716,80 +692,79 @@ public abstract class BaseTestSheetShiftRows {
 
     @Test
     void shiftMergedRowsToMergedRowsDown() throws IOException {
-        Workbook wb = _testDataProvider.createWorkbook();
-        Sheet sheet = wb.createSheet("test");
+        try (Workbook wb = _testDataProvider.createWorkbook()) {
+            Sheet sheet = wb.createSheet("test");
 
-        // populate sheet cells
-        populateSheetCells(sheet, 2);
+            // populate sheet cells
+            populateSheetCells(sheet, 2);
 
-        CellRangeAddress A1_E1 = new CellRangeAddress(0, 0, 0, 4);
-        CellRangeAddress A2_C2 = new CellRangeAddress(1, 1, 0, 2);
+            CellRangeAddress A1_E1 = new CellRangeAddress(0, 0, 0, 4);
+            CellRangeAddress A2_C2 = new CellRangeAddress(1, 1, 0, 2);
 
-        assertEquals(0, sheet.addMergedRegion(A1_E1));
-        assertEquals(1, sheet.addMergedRegion(A2_C2));
+            assertEquals(0, sheet.addMergedRegion(A1_E1));
+            assertEquals(1, sheet.addMergedRegion(A2_C2));
 
-        // A1:E1 should be moved to A2:E2
-        // A2:C2 will be removed
-        sheet.shiftRows(0, 0, 1);
+            // A1:E1 should be moved to A2:E2
+            // A2:C2 will be removed
+            sheet.shiftRows(0, 0, 1);
 
-        assertEquals(1, sheet.getNumMergedRegions());
-        assertEquals(CellRangeAddress.valueOf("A2:E2"), sheet.getMergedRegion(0));
-
-        wb.close();
+            assertEquals(1, sheet.getNumMergedRegions());
+            assertEquals(CellRangeAddress.valueOf("A2:E2"), sheet.getMergedRegion(0));
+        }
     }
 
     @Test
     void test61840_shifting_rows_up_does_not_produce_REF_errors() throws IOException {
-        Workbook wb = _testDataProvider.createWorkbook();
-        Sheet sheet = wb.createSheet();
-        Cell cell = sheet.createRow(4).createCell(0);
+        try (Workbook wb = _testDataProvider.createWorkbook()) {
+            Sheet sheet = wb.createSheet();
+            Cell cell = sheet.createRow(4).createCell(0);
 
-        cell.setCellFormula("(B5-C5)/B5");
-        sheet.shiftRows(4, 4, -1);
+            cell.setCellFormula("(B5-C5)/B5");
+            sheet.shiftRows(4, 4, -1);
 
-        // Cell objects created before a row shift are still valid.
-        // The row number of those cell references will be shifted if
-        // the cell is within the shift range.
-        assertEquals("(B4-C4)/B4", cell.getCellFormula());
+            // Cell objects created before a row shift are still valid.
+            // The row number of those cell references will be shifted if
+            // the cell is within the shift range.
+            assertEquals("(B4-C4)/B4", cell.getCellFormula());
 
-        // New cell references are also valid.
-        Cell shiftedCell = sheet.getRow(3).getCell(0);
-        assertNotNull(shiftedCell);
-        assertEquals("(B4-C4)/B4", shiftedCell.getCellFormula());
-
-        wb.close();
+            // New cell references are also valid.
+            Cell shiftedCell = sheet.getRow(3).getCell(0);
+            assertNotNull(shiftedCell);
+            assertEquals("(B4-C4)/B4", shiftedCell.getCellFormula());
+        }
     }
 
 
     @Test
-    void checkMergedRegions56454() {
-        Workbook wb = _testDataProvider.createWorkbook();
-        Sheet sheet = wb.createSheet();
+    void checkMergedRegions56454() throws IOException {
+        try (Workbook wb = _testDataProvider.createWorkbook()) {
+            Sheet sheet = wb.createSheet();
 
-        // populate sheet cells
-        for (int i = 0; i < 10; i++) {
-            Row row = sheet.createRow(i);
+            // populate sheet cells
+            for (int i = 0; i < 10; i++) {
+                Row row = sheet.createRow(i);
 
-            for (int j = 0; j < 10; j++) {
-                Cell cell = row.createCell(j, CellType.STRING);
+                for (int j = 0; j < 10; j++) {
+                    Cell cell = row.createCell(j, CellType.STRING);
 
-                cell.setCellValue(i + "x" + j);
+                    cell.setCellValue(i + "x" + j);
+                }
             }
-        }
 
-        CellRangeAddress region1 = new CellRangeAddress(3, 6, 0, 1);
-        CellRangeAddress region2 = new CellRangeAddress(3, 6, 2, 3);
+            CellRangeAddress region1 = new CellRangeAddress(3, 6, 0, 1);
+            CellRangeAddress region2 = new CellRangeAddress(3, 6, 2, 3);
 
-        sheet.addMergedRegion(region1);
-        sheet.addMergedRegion(region2);
+            sheet.addMergedRegion(region1);
+            sheet.addMergedRegion(region2);
 
-        sheet.shiftRows(4, sheet.getLastRowNum(), 1);
+            sheet.shiftRows(4, sheet.getLastRowNum(), 1);
 
-        // check, if all regions still start at row 3
-        for (int i = 0; i < sheet.getNumMergedRegions(); i++) {
-            CellRangeAddress cr = sheet.getMergedRegion(i);
+            // check, if all regions still start at row 3
+            for (int i = 0; i < sheet.getNumMergedRegions(); i++) {
+                CellRangeAddress cr = sheet.getMergedRegion(i);
 
-            assertEquals(cr.getFirstRow(), 3);
+                assertEquals(cr.getFirstRow(), 3);
+            }
         }
     }
 
diff --git a/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestWorkbook.java b/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestWorkbook.java
index 22b8dff2ab..150622eccc 100644
--- a/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestWorkbook.java
+++ b/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestWorkbook.java
@@ -724,7 +724,7 @@ public abstract class BaseTestWorkbook {
     }
 
     @Test
-    void changeSheetNameWithSharedFormulas() throws IOException {
+    protected void changeSheetNameWithSharedFormulas() throws IOException {
         String sampleFile = "shared_formulas.xls" + (getClass().getName().contains("xssf") ? "x" : "");
 
         try (Workbook wb = _testDataProvider.openSampleWorkbook(sampleFile)) {