diff options
author | Andreas Beeker <kiwiwings@apache.org> | 2021-08-28 23:48:48 +0000 |
---|---|---|
committer | Andreas Beeker <kiwiwings@apache.org> | 2021-08-28 23:48:48 +0000 |
commit | f71cebcce5ed809ee15cd69524f8cb0b0b2ea47c (patch) | |
tree | 4f9f798ae3acd7a3e50a675becbe85055d1beff6 | |
parent | 6d9b450ce31209014d945a13c65fc23cb29a351c (diff) | |
download | poi-f71cebcce5ed809ee15cd69524f8cb0b0b2ea47c.tar.gz poi-f71cebcce5ed809ee15cd69524f8cb0b0b2ea47c.zip |
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
50 files changed, 1677 insertions, 1748 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 <associated parameters> 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<Node> 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<cfvos.length; i++) { t[i] = new XSSFConditionalFormattingThreshold(cfvos[i]); } return t; } + @Override public void setThresholds(ConditionalFormattingThreshold[] thresholds) { CTCfvo[] cfvos = new CTCfvo[thresholds.length]; for (int i=0; i<thresholds.length; i++) { @@ -78,6 +86,7 @@ public class XSSFIconMultiStateFormatting implements IconMultiStateFormatting { } _iconset.setCfvoArray(cfvos); } + @Override public XSSFConditionalFormattingThreshold createThreshold() { return new XSSFConditionalFormattingThreshold(_iconset.addNewCfvo()); } diff --git a/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFWorkbookFactory.java b/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFWorkbookFactory.java index 3ac689f372..8d49b1eaf9 100644 --- a/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFWorkbookFactory.java +++ b/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFWorkbookFactory.java @@ -139,6 +139,7 @@ public class XSSFWorkbookFactory implements WorkbookProvider { * @throws IOException if an error occurs while reading the data * @throws EncryptedDocumentException If the wrong password is given for a protected file */ + @Override @SuppressWarnings("resource") public XSSFWorkbook create(File file, String password, boolean readOnly) throws IOException { FileMagic fm = FileMagic.valueOf(file); diff --git a/poi-ooxml/src/test/java/org/apache/poi/xssf/extractor/TestXSSFExportToXML.java b/poi-ooxml/src/test/java/org/apache/poi/xssf/extractor/TestXSSFExportToXML.java index 5672564fd0..0afb927551 100644 --- a/poi-ooxml/src/test/java/org/apache/poi/xssf/extractor/TestXSSFExportToXML.java +++ b/poi-ooxml/src/test/java/org/apache/poi/xssf/extractor/TestXSSFExportToXML.java @@ -45,7 +45,6 @@ import org.apache.poi.xssf.XSSFTestDataSamples; import org.apache.poi.xssf.model.MapInfo; import org.apache.poi.xssf.usermodel.XSSFMap; import org.apache.poi.xssf.usermodel.XSSFWorkbook; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.xml.sax.SAXException; import org.xml.sax.SAXParseException; @@ -216,15 +215,13 @@ public final class TestXSSFExportToXML { } @Test - @Disabled(value="Fails, but I don't know if it is ok or not...") void testExportToXMLSingleAttributeNamespace() throws Exception { try (XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("CustomXMLMapping-singleattributenamespace.xlsx")) { - for (XSSFMap map : wb.getCustomXMLMappings()) { XSSFExportToXml exporter = new XSSFExportToXml(map); - UnsynchronizedByteArrayOutputStream os = new UnsynchronizedByteArrayOutputStream(); - exporter.exportToXML(os, true); + SAXParseException ex = assertThrows(SAXParseException.class, () -> 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()); - - 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 (XSSFWorkbook xssfWb1 = new XSSFWorkbook()) { + xssfWb1.createSheet("S1"); + try (DeferredSXSSFWorkbook wb1 = new DeferredSXSSFWorkbook(xssfWb1); + XSSFWorkbook xssfWb2 = DeferredSXSSFITestDataProvider.instance.writeOutAndReadBack(wb1)) { + assertTrue(wb1.dispose()); + + 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(); - - 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"); - }); - - // 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"); - }); - - XSSFWorkbook xssfWb3 = DeferredSXSSFITestDataProvider.instance.writeOutAndReadBack(wb2); - wb2.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()); - - xssfWb2.close(); - xssfWb3.close(); - wb1.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 (DeferredSXSSFWorkbook wb1 = new DeferredSXSSFWorkbook(xssfWb1); + XSSFWorkbook xssfWb2 = DeferredSXSSFITestDataProvider.instance.writeOutAndReadBack(wb1)) { + assertTrue(wb1.dispose()); + + 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"); + }); + + // 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 (XSSFWorkbook xssfWb3 = DeferredSXSSFITestDataProvider.instance.writeOutAndReadBack(wb2)) { + + 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(); - - final int rowNum = 1000; - final int sheetNum = 5; - populateData(wb, 1000, 5); - - 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 cell3 = row.getCell(2); - assertEquals(j, (int) cell3.getNumericCellValue()); + try (DeferredSXSSFWorkbook wb = new DeferredSXSSFWorkbook()) { + + final int rowNum = 1000; + final int sheetNum = 5; + populateData(wb); + + 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 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(); - - DeferredSXSSFWorkbook wb2 = new DeferredSXSSFWorkbook(); - wb2.setCompressTempFiles(true); - // the underlying writer is GZIPSheetDataWriter - assertWorkbookDispose(wb2); - wb2.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); - private static void assertWorkbookDispose(DeferredSXSSFWorkbook wb) { - populateData(wb, 1000, 5); + populateData(wb); - 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()); - - 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 (XSSFWorkbook xssfWb1 = new XSSFWorkbook()) { + xssfWb1.createSheet("S1"); + try (SXSSFWorkbook wb1 = new SXSSFWorkbook(xssfWb1); + XSSFWorkbook xssfWb2 = SXSSFITestDataProvider.instance.writeOutAndReadBack(wb1)) { + assertTrue(wb1.dispose()); + + 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); - - SharedStringsTable sss = wb.getSharedStringSource(); - - assertNotNull(sss); - - Row row = wb.createSheet("S1").createRow(0); - - 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()); - - 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(); + try (SXSSFWorkbook wb = new SXSSFWorkbook(null, 10, false, true)) { + + SharedStringsTable sss = wb.getSharedStringSource(); + + assertNotNull(sss); + + Row row = wb.createSheet("S1").createRow(0); + + row.createCell(0).setCellValue("A"); + row.createCell(1).setCellValue("B"); + row.createCell(2).setCellValue("A"); + + 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()); + } + } } @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(); - - 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 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 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()); + + 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 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"); + + 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(); - - 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()) { + 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()); + } - //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.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 - void gzipSheetdataWriter() throws IOException { - SXSSFWorkbook wb = new SXSSFWorkbook(); - wb.setCompressTempFiles(true); - - final int rowNum = 1000; - final int sheetNum = 5; - populateData(wb, 1000, 5); - - 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 cell3 = row.getCell(2); - assertEquals(j, (int)cell3.getNumericCellValue()); + //Test escaping of Unicode control characters + 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()); } } - - 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()); + @Test + void gzipSheetdataWriter() throws IOException { + try (SXSSFWorkbook wb = new SXSSFWorkbook()) { + wb.setCompressTempFiles(true); + + final int rowNum = 1000; + final int sheetNum = 5; + populateData(wb); + + 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 cell3 = row.getCell(2); + assertEquals(j, (int) cell3.getNumericCellValue()); + } + } - for (Sheet sheet : wb) { - SXSSFSheet sxSheet = (SXSSFSheet) sheet; - assertFalse(sxSheet.getSheetDataWriter().getTempFile().exists()); + assertTrue(wb.dispose()); + } } } - 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(); - - SXSSFWorkbook wb2 = new SXSSFWorkbook(); - wb2.setCompressTempFiles(true); - // the underlying writer is GZIPSheetDataWriter - assertWorkbookDispose(wb2); - wb2.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); + + 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(); - - Sheet sheet1 = wb.createSheet(); - Cell cell1 = sheet1.createRow(0).createCell(0); - Comment comment1 = sheet1.createDrawingPatriarch().createCellComment(creationHelper.createClientAnchor()); - cell1.setCellComment(comment1); - - Sheet sheet2 = wb.createSheet(); - Cell cell2 = sheet2.createRow(0).createCell(0); - Comment comment2 = sheet2.createDrawingPatriarch().createCellComment(creationHelper.createClientAnchor()); - cell2.setCellComment(comment2); - - 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(); + try (XSSFWorkbook wb = new XSSFWorkbook()) { + XSSFCreationHelper creationHelper = wb.getCreationHelper(); + + 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()); + + 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); + + 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<? extends EmfPlusObjectData> 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<? extends EmfPlusObjectData> 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<HwmfEmbedded> { @Override public boolean hasNext() { + return moveNext(); + } + + private boolean moveNext() { if (iterStack.isEmpty()) { return false; } @@ -290,36 +295,33 @@ public class HemfEmbeddedIterator implements Iterator<HwmfEmbedded> { final int objectId = epo.getObjectId(); - HwmfEmbedded emb = new HwmfEmbedded(); - - EmfPlusImage img = epo.getObjectData(); - assert(img.getImageDataType() != null); + final HwmfEmbedded emb = new HwmfEmbedded(); - 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); + } + } catch (Exception e) { + int lastLen = chunk.length - 1; + while (lastLen >= 0 && chunk[lastLen] == 0) { + lastLen--; } - out.write(chunk, 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); - - 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); + 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); + + 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(); + int size = bout.size(); - // Update the size (header bytes 5-8) - LittleEndian.putInt(_header,4,size+8); + // Update the size (header bytes 5-8) + LittleEndian.putInt(_header, 4, size + 8); - // Write out our header - out.write(_header); + // 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); + 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); + // 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(); - - // 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); - } + 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 children + for (Record aChildren : children) { + aChildren.writeOut(baos); + } - // Grab the bytes back - byte[] toWrite = baos.toByteArray(); + // 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)); + // 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); + // 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 - - UnsynchronizedByteArrayOutputStream baos = new UnsynchronizedByteArrayOutputStream(); - - // First up, we need to serialise the paragraph properties - for(TextPropCollection tpc : paragraphStyles) { - tpc.writeOut(baos); + // 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); + } + + // Now, we do the character ones + for (TextPropCollection tpc : charStyles) { + 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<runs.length && covered < size; i++) { - TextSpecInfoRun run = runs[i]; - if (covered + run.getLength() > size || i == runs.length-1) { - run.setLength(size-covered); - } - covered += run.getLength(); - try { + + 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); - } catch (IOException e) { - throw new HSLFException(e); + idx++; } - } - _data = bos.toByteArray(); - // Update the size (header bytes 5-8) - LittleEndian.putInt(_header, 4, _data.length); + _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<HwmfEmbedded> { 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); - - /* 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); - - assert(bos.size() == OFFSET_HEADER); - - 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(); + 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 property set's header. */ + leos.writeShort(getByteOrder()); + leos.writeShort(getFormat()); + leos.writeInt(getOSVersion()); + putClassId(bos, getClassID()); + leos.writeInt(nrSections); + + assert (bos.size() == OFFSET_HEADER); + + 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(); + } + 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 length - dummy value, fixed later */ + leos.writeInt(-1); - /* Write the section's number of properties: */ - leos.writeInt(properties.size()); + /* 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(); - leos.writeInt(-1); - } + 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); + /* 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()); + byte[] result = bos.toByteArray(); + LittleEndian.putInt(result, 0, bos.size()); - for (int[] off : offsets) { - LittleEndian.putUInt(result, off[0], off[1]); - } + for (int[] off : offsets) { + LittleEndian.putUInt(result, off[0], off[1]); + } - out.write(result); + out.write(result); - return bos.size(); + 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 <i>current</i> 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, <code>null</code> 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 <code>null</code>. + * @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, <code>null</code> 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 <code>null</code>. + * @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, <code>null</code> 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 <code>null</code>. + * @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, <code>null</code> 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, <code>null</code> 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, <code>null</code> 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<t.length; i++) { @@ -76,6 +82,7 @@ public final class HSSFIconMultiStateFormatting implements org.apache.poi.ss.use iconFormatting.setThresholds(t); } + @Override public HSSFConditionalFormattingThreshold createThreshold() { return new HSSFConditionalFormattingThreshold(new IconMultiStateThreshold(), sheet); } diff --git a/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFWorkbookFactory.java b/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFWorkbookFactory.java index b43a45de6a..b9fdbb9727 100644 --- a/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFWorkbookFactory.java +++ b/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFWorkbookFactory.java @@ -48,6 +48,7 @@ public class HSSFWorkbookFactory implements WorkbookProvider { * * @return The created workbook */ + @Override public HSSFWorkbook create() { return new HSSFWorkbook(); } @@ -66,6 +67,7 @@ public class HSSFWorkbookFactory implements WorkbookProvider { * Note that in order to properly release resources the * Workbook should be closed after use. */ + @Override public HSSFWorkbook create(final DirectoryNode root, String password) throws IOException { boolean passwordSet = false; if (password != null) { @@ -93,6 +95,7 @@ public class HSSFWorkbookFactory implements WorkbookProvider { } @Override + @SuppressWarnings("java:S2095") public Workbook create(File file, String password, boolean readOnly) throws IOException { boolean passwordSet = false; if (password != null) { @@ -104,11 +107,8 @@ public class HSSFWorkbookFactory implements WorkbookProvider { try { return new HSSFWorkbook(fs, true); } catch (RuntimeException e) { - // we need to close the filesystem - // if we encounter an exception to - // not leak file handles + // we need to close the filesystem if we encounter an exception to not leak file handles fs.close(); - throw e; } } finally { diff --git a/poi/src/main/java/org/apache/poi/poifs/crypt/cryptoapi/CryptoAPIDecryptor.java b/poi/src/main/java/org/apache/poi/poifs/crypt/cryptoapi/CryptoAPIDecryptor.java index 36b44efbcc..4f229e9a18 100644 --- a/poi/src/main/java/org/apache/poi/poifs/crypt/cryptoapi/CryptoAPIDecryptor.java +++ b/poi/src/main/java/org/apache/poi/poifs/crypt/cryptoapi/CryptoAPIDecryptor.java @@ -28,7 +28,7 @@ import javax.crypto.Cipher; import javax.crypto.SecretKey; import javax.crypto.spec.SecretKeySpec; -import org.apache.commons.io.output.UnsynchronizedByteArrayOutputStream; +import org.apache.commons.io.input.BoundedInputStream; import org.apache.poi.EncryptedDocumentException; import org.apache.poi.poifs.crypt.ChunkedCipherInputStream; import org.apache.poi.poifs.crypt.CryptoFunctions; @@ -39,11 +39,9 @@ import org.apache.poi.poifs.crypt.EncryptionVerifier; import org.apache.poi.poifs.crypt.HashAlgorithm; import org.apache.poi.poifs.filesystem.DirectoryNode; import org.apache.poi.poifs.filesystem.DocumentInputStream; -import org.apache.poi.poifs.filesystem.DocumentNode; import org.apache.poi.poifs.filesystem.POIFSFileSystem; import org.apache.poi.util.BitField; import org.apache.poi.util.BitFieldFactory; -import org.apache.commons.io.input.BoundedInputStream; import org.apache.poi.util.IOUtils; import org.apache.poi.util.LittleEndian; import org.apache.poi.util.LittleEndianInputStream; @@ -170,14 +168,10 @@ public class CryptoAPIDecryptor extends Decryptor { */ public POIFSFileSystem getSummaryEntries(DirectoryNode root, String encryptedStream) throws IOException, GeneralSecurityException { - DocumentNode es = (DocumentNode) root.getEntry(encryptedStream); - UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream(); - try (DocumentInputStream dis = root.createDocumentInputStream(es)) { - IOUtils.copy(dis, bos); - } POIFSFileSystem fsOut = null; try ( - CryptoAPIDocumentInputStream sbis = new CryptoAPIDocumentInputStream(this, bos.toByteArray()); + DocumentInputStream dis = root.createDocumentInputStream(root.getEntry(encryptedStream)); + CryptoAPIDocumentInputStream sbis = new CryptoAPIDocumentInputStream(this, IOUtils.toByteArray(dis)); LittleEndianInputStream leis = new LittleEndianInputStream(sbis) ) { int streamDescriptorArrayOffset = (int) leis.readUInt(); diff --git a/poi/src/main/java/org/apache/poi/poifs/macros/VBAMacroReader.java b/poi/src/main/java/org/apache/poi/poifs/macros/VBAMacroReader.java index 71d6a4b405..789bfbb702 100644 --- a/poi/src/main/java/org/apache/poi/poifs/macros/VBAMacroReader.java +++ b/poi/src/main/java/org/apache/poi/poifs/macros/VBAMacroReader.java @@ -68,6 +68,7 @@ import org.apache.poi.util.StringUtil; * * @since 3.15-beta2 */ +@SuppressWarnings("unused") public class VBAMacroReader implements Closeable { private static final Logger LOGGER = LogManager.getLogger(VBAMacroReader.class); @@ -667,35 +668,36 @@ public class VBAMacroReader implements Closeable { private static String readUnicode(InputStream is) throws IOException { //reads null-terminated unicode string - UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream(); - int b0 = IOUtils.readByte(is); - int b1 = IOUtils.readByte(is); - - int read = 2; - while ((b0 + b1) != 0 && read < MAX_STRING_LENGTH) { - - bos.write(b0); - bos.write(b1); - b0 = IOUtils.readByte(is); - b1 = IOUtils.readByte(is); - read += 2; - } - if (read >= MAX_STRING_LENGTH) { - LOGGER.atWarn().log("stopped reading unicode name after {} bytes", box(read)); + try (UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream()) { + int b0 = IOUtils.readByte(is); + int b1 = IOUtils.readByte(is); + + int read = 2; + while ((b0 + b1) != 0 && read < MAX_STRING_LENGTH) { + bos.write(b0); + bos.write(b1); + b0 = IOUtils.readByte(is); + b1 = IOUtils.readByte(is); + read += 2; + } + if (read >= MAX_STRING_LENGTH) { + LOGGER.atWarn().log("stopped reading unicode name after {} bytes", box(read)); + } + return bos.toString(StandardCharsets.UTF_16LE); } - 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); - } - return bos.toString(charset); + 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); + } } /** @@ -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<EmbeddedExtractor> { 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.</p> */ 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 - * + * * <p>If you change the range type, you need to * ensure that the Formula and Value parameters * are compatible with it before saving</p> */ void setRangeType(RangeType type); - + /** * Formula to use to calculate the threshold, - * or <code>null</code> if no formula + * or {@code null} if no formula */ String getFormula(); /** * Sets the formula used to calculate the threshold, - * or unsets it if <code>null</code> is given. + * or unsets it if {@code null} is given. */ void setFormula(String formula); - + /** - * Gets the value used for the threshold, or - * <code>null</code> 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. - * <p>If the type is {@link RangeType#PERCENT} or + * Sets the value used for the threshold. + * <p>If the type is {@link RangeType#PERCENT} or * {@link RangeType#PERCENTILE} it must be between 0 and 100. * <p>If the type is {@link RangeType#MIN} or {@link RangeType#MAX} * or {@link RangeType#FORMULA} it shouldn't be set. - * <p>Use <code>null</code> to unset + * <p>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); - 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."); + } - if (maxLength != Integer.MAX_VALUE && totalBytes == maxLength) { - throw new IOException("MaxLength ("+maxLength+") reached - stream seems to be invalid."); - } + if (len != Integer.MAX_VALUE && totalBytes < len) { + throw new EOFException("unexpected EOF - expected len: " + len + " - actual len: " + totalBytes); + } - if (len != Integer.MAX_VALUE && totalBytes < len) { - throw new EOFException("unexpected EOF - expected len: "+len+" - actual len: "+totalBytes); + return baos.toByteArray(); } - - 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: - * <code><pre> - * 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<Integer> 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 ); - - // Shift the second row down 1 and write to temp file - s.shiftRows( 1, 1, 1 ); - - Workbook wb2 = _testDataProvider.writeOutAndReadBack(wb1); - wb1.close(); - - // 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); - - // 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(); + 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); + + 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); + + 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); + 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 ); - - // 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(); + 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); + 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()); - - 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()); + try (Workbook wb1 = _testDataProvider.openSampleWorkbook("comments." + _testDataProvider.getStandardFileNameExtension())) { + + 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); - - 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"); - - //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); - - 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()); - - //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(); + 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 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"); + + //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()); + + 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()); + + 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()); - - assertEquals(0, sheet.addMergedRegion(region)); - - sheet.shiftRows(0, 1, 2); - region = sheet.getMergedRegion(0); - assertEquals("A3:C3", region.formatAsString()); - wb.close(); + 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)); + + 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); - - for (int j = 0; j < 10; j++) { - Cell cell = row.createCell(j, CellType.STRING); - cell.setCellValue(i + "x" + j); + 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); + } } - } - CellRangeAddress A4_B7 = CellRangeAddress.valueOf("A4:B7"); - CellRangeAddress C4_D7 = CellRangeAddress.valueOf("C4:D7"); + 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)); + assertEquals(0, sheet.addMergedRegion(A4_B7)); + assertEquals(1, sheet.addMergedRegion(C4_D7)); - assumeTrue(sheet.getLastRowNum() > 8); + assumeTrue(sheet.getLastRowNum() > 8); - // Insert a row in the middle of both merged regions. - sheet.shiftRows(4, sheet.getLastRowNum(), 1); + // 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<CellRangeAddress> 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); + // all regions should still start at row 3, and elongate by 1 row + List<CellRangeAddress> 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); + // 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()); } - 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()); - - 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()); - - // 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()); + 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()); + + 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()); + } } - } - 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()); + 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); + // 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(); + assertEquals("SUM(G29:I29)", sheet.getRow(28).getCell(9).getCellFormula()); + assertEquals("SUM(G30:I30)", sheet.getRow(29).getCell(9).getCellFormula()); + } } @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); - - // 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"); - - // 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"); - - // 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/"); - - int startRow = 0; - int endRow = 0; - int n = 3; - sheet.shiftRows(startRow, endRow, n); - - Workbook read = _testDataProvider.writeOutAndReadBack(wb); - wb.close(); - - Sheet sh = read.getSheet("test"); - - Row shiftedRow = sh.getRow(3); - - // 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"); - - // 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 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 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."); + 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); + + // 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/"); + + // 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"); + + 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); + + try (Workbook read = _testDataProvider.writeOutAndReadBack(wb)) { + Sheet sh = read.getSheet("test"); + Row shiftedRow = sh.getRow(3); + + // 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"); + + // 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 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 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 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/"); + } } - - // 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"); - - // 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); + 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); + } } - } - 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); - assertEquals(0, sheet.addMergedRegion(A1_E1)); - assertEquals(1, sheet.addMergedRegion(A2_C2)); + CellRangeAddress A1_E1 = new CellRangeAddress(0, 0, 0, 4); + CellRangeAddress A2_C2 = new CellRangeAddress(1, 1, 0, 2); - // A1:E1 should be removed - // A2:C2 will be A1:C1 - sheet.shiftRows(1, sheet.getLastRowNum(), -1); + assertEquals(0, sheet.addMergedRegion(A1_E1)); + assertEquals(1, sheet.addMergedRegion(A2_C2)); - assertEquals(1, sheet.getNumMergedRegions()); - assertEquals(CellRangeAddress.valueOf("A1:C1"), sheet.getMergedRegion(0)); + // A1:E1 should be removed + // A2:C2 will be A1:C1 + sheet.shiftRows(1, sheet.getLastRowNum(), -1); - 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); - - CellRangeAddress A1_E1 = new CellRangeAddress(0, 0, 0, 4); - CellRangeAddress A2_C2 = new CellRangeAddress(1, 7, 0, 2); + try (Workbook wb = _testDataProvider.createWorkbook()) { + Sheet sheet = wb.createSheet("test"); + populateSheetCells(sheet, 10); - assertEquals(0, sheet.addMergedRegion(A1_E1)); - assertEquals(1, sheet.addMergedRegion(A2_C2)); + CellRangeAddress A1_E1 = new CellRangeAddress(0, 0, 0, 4); + CellRangeAddress A2_C2 = new CellRangeAddress(1, 7, 0, 2); - // A1:E1 should move to A5:E5 - // A2:C2 should be removed - sheet.shiftRows(0, 0, 4); + assertEquals(0, sheet.addMergedRegion(A1_E1)); + assertEquals(1, sheet.addMergedRegion(A2_C2)); - assertEquals(1, sheet.getNumMergedRegions()); - assertEquals(CellRangeAddress.valueOf("A5:E5"), sheet.getMergedRegion(0)); + // A1:E1 should move to A5:E5 + // A2:C2 should be removed + sheet.shiftRows(0, 0, 4); - 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); - assertEquals(0, sheet.addMergedRegion(A8_E8)); - assertEquals(1, sheet.addMergedRegion(A9_C9)); + CellRangeAddress A8_E8 = new CellRangeAddress(7, 7, 0, 4); + CellRangeAddress A9_C9 = new CellRangeAddress(8, 8, 0, 2); - // A1:E1 should be removed - // A2:C2 will be A1:C1 - sheet.shiftRows(3, sheet.getLastRowNum(), 1); + assertEquals(0, sheet.addMergedRegion(A8_E8)); + assertEquals(1, sheet.addMergedRegion(A9_C9)); - assertEquals(2, sheet.getNumMergedRegions()); - assertEquals(CellRangeAddress.valueOf("A9:E9"), sheet.getMergedRegion(0)); - assertEquals(CellRangeAddress.valueOf("A10:C10"), sheet.getMergedRegion(1)); + // A1:E1 should be removed + // A2:C2 will be A1:C1 + sheet.shiftRows(3, sheet.getLastRowNum(), 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"); - - // populate sheet cells - populateSheetCells(sheet, 2); + try (Workbook wb = _testDataProvider.createWorkbook()) { + Sheet sheet = wb.createSheet("test"); - CellRangeAddress A1_E1 = new CellRangeAddress(0, 0, 0, 4); - CellRangeAddress A2_C2 = new CellRangeAddress(1, 1, 0, 2); + // populate sheet cells + populateSheetCells(sheet, 2); - assertEquals(0, sheet.addMergedRegion(A1_E1)); - assertEquals(1, sheet.addMergedRegion(A2_C2)); + CellRangeAddress A1_E1 = new CellRangeAddress(0, 0, 0, 4); + CellRangeAddress A2_C2 = new CellRangeAddress(1, 1, 0, 2); - // A1:E1 should be moved to A2:E2 - // A2:C2 will be removed - sheet.shiftRows(0, 0, 1); + assertEquals(0, sheet.addMergedRegion(A1_E1)); + assertEquals(1, sheet.addMergedRegion(A2_C2)); - assertEquals(1, sheet.getNumMergedRegions()); - assertEquals(CellRangeAddress.valueOf("A2:E2"), sheet.getMergedRegion(0)); + // A1:E1 should be moved to A2:E2 + // A2:C2 will be removed + sheet.shiftRows(0, 0, 1); - 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); - - cell.setCellFormula("(B5-C5)/B5"); - sheet.shiftRows(4, 4, -1); + try (Workbook wb = _testDataProvider.createWorkbook()) { + Sheet sheet = wb.createSheet(); + Cell cell = sheet.createRow(4).createCell(0); - // 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.setCellFormula("(B5-C5)/B5"); + sheet.shiftRows(4, 4, -1); - // New cell references are also valid. - Cell shiftedCell = sheet.getRow(3).getCell(0); - assertNotNull(shiftedCell); - assertEquals("(B4-C4)/B4", shiftedCell.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()); - 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)) { |