diff options
11 files changed, 345 insertions, 55 deletions
diff --git a/build.gradle b/build.gradle index 17f6b1bd26..5eccd98835 100644 --- a/build.gradle +++ b/build.gradle @@ -34,10 +34,10 @@ plugins { id 'com.dorongold.task-tree' version '4.0.1' id 'org.nosphere.apache.rat' version '0.8.1' id 'distribution' - id "com.github.spotbugs" version '6.1.10' + id 'com.github.spotbugs' version '6.1.13' id 'de.thetaphi.forbiddenapis' version '3.9' id 'org.sonarqube' version '4.0.0.2929' - id 'org.cyclonedx.bom' version '2.3.0' + id 'org.cyclonedx.bom' version '2.3.1' id 'com.adarshr.test-logger' version '3.2.0' } @@ -67,7 +67,7 @@ configurations { } dependencies { - antLibs("org.junit.jupiter:junit-jupiter:5.12.2") + antLibs("org.junit.jupiter:junit-jupiter:5.13.0") antLibs("org.apache.ant:ant-junitlauncher:1.10.15") } @@ -77,7 +77,7 @@ ant.taskdef(name: "junit", wrapper { - gradleVersion = '8.14' + gradleVersion = '8.14.1' } group = 'org.apache.poi' @@ -112,7 +112,7 @@ subprojects { commonsCompressVersion = '1.27.1' commonsIoVersion = '2.19.0' commonsMathVersion = '3.6.1' - junitVersion = '5.12.2' + junitVersion = '5.13.0' log4jVersion = '2.24.3' mockitoVersion = '4.11.0' hamcrestVersion = '3.0' @@ -143,7 +143,7 @@ subprojects { resolutionStrategy { force "commons-io:commons-io:${commonsIoVersion}" force 'org.slf4j:slf4j-api:2.0.17' - force 'com.fasterxml.woodstox:woodstox-core:7.1.0' + force 'com.fasterxml.woodstox:woodstox-core:7.1.1' } } } @@ -172,7 +172,7 @@ subprojects { dependencies { testImplementation "org.junit.jupiter:junit-jupiter:${junitVersion}" - testRuntimeOnly 'org.junit.platform:junit-platform-launcher:1.12.2' + testRuntimeOnly 'org.junit.platform:junit-platform-launcher:1.13.0' testImplementation "org.mockito:mockito-core:${mockitoVersion}" testImplementation "org.hamcrest:hamcrest:${hamcrestVersion}" testImplementation "org.apache.logging.log4j:log4j-core:${log4jVersion}" @@ -422,7 +422,7 @@ subprojects { jvmArgs += [ // see https://github.com/java9-modularity/gradle-modules-plugin/issues/97 // opposed to the recommendation there, it doesn't work to add ... to the dependencies - // testRuntimeOnly 'org.junit.platform:junit-platform-launcher:1.12.2' + // testRuntimeOnly 'org.junit.platform:junit-platform-launcher:1.13.0' // gradles gradle-worker.jar is still not a JPMS module and thus runs as unnamed module '--add-exports','org.junit.platform.commons/org.junit.platform.commons.util=org.apache.poi.poi', '--add-exports','org.junit.platform.commons/org.junit.platform.commons.util=ALL-UNNAMED', @@ -270,20 +270,20 @@ under the License. <dependency prefix="main.com.zaxxer" artifact="com.zaxxer:SparseBitSet:1.3" usage="main"/> <dependency prefix="main.log4j-api" artifact="org.apache.logging.log4j:log4j-api:2.24.3" usage="main"/> - <dependency prefix="main.junit-api" artifact="org.junit.jupiter:junit-jupiter-api:5.12.2" usage="main-tests"/> - <dependency prefix="main.junit-jengine" artifact="org.junit.jupiter:junit-jupiter-engine:5.12.2" usage="main-tests"/> - <dependency prefix="main.junit-params" artifact="org.junit.jupiter:junit-jupiter-params:5.12.2" usage="main-tests"/> + <dependency prefix="main.junit-api" artifact="org.junit.jupiter:junit-jupiter-api:5.13.0" usage="main-tests"/> + <dependency prefix="main.junit-jengine" artifact="org.junit.jupiter:junit-jupiter-engine:5.13.0" usage="main-tests"/> + <dependency prefix="main.junit-params" artifact="org.junit.jupiter:junit-jupiter-params:5.13.0" usage="main-tests"/> <dependency prefix="main.junit-opentest4j" artifact="org.opentest4j:opentest4j:1.2.0" usage="main-tests"/> <dependency prefix="main.junit-apiguardian" artifact="org.apiguardian:apiguardian-api:1.1.2" usage="main-tests"/> - <dependency prefix="main.junit-pcommons" artifact="org.junit.platform:junit-platform-commons:1.12.2" usage="main-tests"/> - <dependency prefix="main.junit-pengine" artifact="org.junit.platform:junit-platform-engine:1.12.2" usage="main-tests"/> - <dependency prefix="main.junit-plauncher" artifact="org.junit.platform:junit-platform-launcher:1.12.2" usage="main-tests"/> + <dependency prefix="main.junit-pcommons" artifact="org.junit.platform:junit-platform-commons:1.13.0" usage="main-tests"/> + <dependency prefix="main.junit-pengine" artifact="org.junit.platform:junit-platform-engine:1.13.0" usage="main-tests"/> + <dependency prefix="main.junit-plauncher" artifact="org.junit.platform:junit-platform-launcher:1.13.0" usage="main-tests"/> <dependency prefix="main.jmh" artifact="org.openjdk.jmh:jmh-core:1.35" usage="main-tests"/> <dependency prefix="main.jmhAnnotation" artifact="org.openjdk.jmh:jmh-generator-annprocess:1.35" usage="main-tests"/> <dependency prefix="main.hamcrest" artifact="org.hamcrest:hamcrest:3.0" usage="main-tests"/> - <dependency prefix="main.xmlunit" artifact="org.xmlunit:xmlunit-core:2.10.1" usage="main-tests"/> + <dependency prefix="main.xmlunit" artifact="org.xmlunit:xmlunit-core:2.10.2" usage="main-tests"/> <dependency prefix="main.mockito" artifact="org.mockito:mockito-core:4.11.0" usage="main-tests"/> <dependency prefix="main.byte-buddy" artifact="net.bytebuddy:byte-buddy:1.17.5" usage="main-tests"/> <dependency prefix="main.byte-buddy-agent" artifact="net.bytebuddy:byte-buddy-agent:1.17.5" usage="main-tests"/> diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index ca025c83a7..002b867c48 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,6 +1,6 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-8.14-bin.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-8.14.1-bin.zip networkTimeout=10000 validateDistributionUrl=true zipStoreBase=GRADLE_USER_HOME diff --git a/poi-ooxml/build.gradle b/poi-ooxml/build.gradle index 2f8a76de47..f90d6a5cb8 100644 --- a/poi-ooxml/build.gradle +++ b/poi-ooxml/build.gradle @@ -114,7 +114,7 @@ dependencies { testImplementation project(path:':poi', configuration:'tests') testImplementation project(path:':poi-ooxml-lite-agent', configuration: 'archives') testRuntimeOnly "org.apiguardian:apiguardian-api:${apiGuardianVersion}" - testImplementation 'org.xmlunit:xmlunit-core:2.10.1' + testImplementation 'org.xmlunit:xmlunit-core:2.10.2' testImplementation 'org.reflections:reflections:0.10.2' testImplementation 'org.openjdk.jmh:jmh-core:1.36' testImplementation 'org.openjdk.jmh:jmh-generator-annprocess:1.36' diff --git a/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFSheet.java b/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFSheet.java index b8166a880e..c851eda3ec 100644 --- a/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFSheet.java +++ b/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFSheet.java @@ -3926,7 +3926,15 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet, OoxmlSheetEx if(minCell != Integer.MAX_VALUE) { cellRangeAddress = new CellRangeAddress(getFirstRowNum(), getLastRowNum(), minCell, maxCell); } + } else { + // sort columns + for(Map.Entry<Integer, XSSFRow> entry : _rows.entrySet()) { + XSSFRow row = entry.getValue(); + // sorting happens in XSSFRow.fixupCTCells + row.onDocumentWrite(); + } } + if (cellRangeAddress != null) { if (worksheet.isSetDimension()) { worksheet.getDimension().setRef(cellRangeAddress.formatAsString()); diff --git a/poi-ooxml/src/test/java/org/apache/poi/xssf/streaming/TestOutOfOrderColumns.java b/poi-ooxml/src/test/java/org/apache/poi/xssf/streaming/TestOutOfOrderColumns.java new file mode 100644 index 0000000000..b7b6e67030 --- /dev/null +++ b/poi-ooxml/src/test/java/org/apache/poi/xssf/streaming/TestOutOfOrderColumns.java @@ -0,0 +1,122 @@ +/*
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * ====================================================================
+ */
+
+package org.apache.poi.xssf.streaming;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.IOException;
+import java.io.InputStream;
+
+import org.apache.commons.io.output.UnsynchronizedByteArrayOutputStream;
+import org.apache.poi.ss.usermodel.Row;
+import org.apache.poi.ss.usermodel.Sheet;
+import org.apache.poi.xssf.usermodel.XSSFSheet;
+import org.apache.poi.xssf.usermodel.XSSFWorkbook;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Test creates cells in reverse column order in XSSF and SXSSF and expects
+ * saved files to have fixed the order.
+ *
+ * This is necessary because if columns in the saved file are out of order
+ * Excel will show a repair dialog when opening the file and removing data.
+ */
+public final class TestOutOfOrderColumns {
+
+ @Test
+ void outOfOrderColumnsXSSF() throws IOException {
+ try (
+ XSSFWorkbook wb = new XSSFWorkbook();
+ UnsynchronizedByteArrayOutputStream bos = UnsynchronizedByteArrayOutputStream.builder().get()
+ ) {
+ XSSFSheet sheet = wb.createSheet();
+
+ Row row = sheet.createRow(0);
+ // create cells in reverse order
+ row.createCell(1).setCellValue("def");
+ row.createCell(0).setCellValue("abc");
+
+ wb.write(bos);
+
+ validateOrder(bos.toInputStream());
+ }
+ }
+
+ @Test
+ void outOfOrderColumnsSXSSF() throws IOException {
+ try (
+ SXSSFWorkbook wb = new SXSSFWorkbook();
+ UnsynchronizedByteArrayOutputStream bos = UnsynchronizedByteArrayOutputStream.builder().get()
+ ) {
+ Sheet sheet = wb.createSheet();
+
+ Row row = sheet.createRow(0);
+ // create cells in reverse order
+ row.createCell(1).setCellValue("xyz");
+ row.createCell(0).setCellValue("uvw");
+
+ wb.write(bos);
+
+ validateOrder(bos.toInputStream());
+ }
+ }
+
+ @Test
+ /** this is the problematic case, as XSSF column sorting is skipped when saving with SXSSF. */
+ void mixOfXSSFAndSXSSF() throws IOException {
+ try (
+ XSSFWorkbook wb = new XSSFWorkbook();
+ UnsynchronizedByteArrayOutputStream bos = UnsynchronizedByteArrayOutputStream.builder().get()
+ ) {
+ XSSFSheet sheet = wb.createSheet();
+
+ Row row1 = sheet.createRow(0);
+ // create cells in reverse order
+ row1.createCell(1).setCellValue("def");
+ row1.createCell(0).setCellValue("abc");
+
+ try (SXSSFWorkbook sxssfWorkbook = new SXSSFWorkbook(wb)) {
+ Sheet sSheet = sxssfWorkbook.getSheetAt(0);
+ Row row2 = sSheet.createRow(1);
+ // create cells in reverse order
+ row2.createCell(1).setCellValue("xyz");
+ row2.createCell(0).setCellValue("uvw");
+
+ sxssfWorkbook.write(bos);
+
+ validateOrder(bos.toInputStream());
+ }
+ }
+ }
+
+ private void validateOrder(InputStream is) throws IOException {
+ // test if saved cells are in order
+ try (XSSFWorkbook xssfWorkbook = new XSSFWorkbook(is)) {
+ XSSFSheet xssfSheet = xssfWorkbook.getSheetAt(0);
+
+ Row resultRow = xssfSheet.getRow(0);
+ // POI doesn't show stored order because _cells TreeMap sorts it automatically.
+ // The only way to test is to compare the xml.
+ String s = resultRow.toString();
+ assertTrue(s.matches("(?s).*A1.*B1.*"), "unexpected order: " + s);
+ }
+ }
+
+}
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFShapePlaceholderDetails.java b/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFShapePlaceholderDetails.java index 8a467dfedc..348d901235 100644 --- a/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFShapePlaceholderDetails.java +++ b/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFShapePlaceholderDetails.java @@ -47,6 +47,68 @@ import org.apache.poi.util.LocaleUtil; * @since POI 4.0.0 */ public class HSLFShapePlaceholderDetails extends HSLFPlaceholderDetails { + + static void updateSPRecord(final HSLFSimpleShape shape, final Placeholder placeholder) { + final EscherSpRecord spRecord = shape.getEscherChild(EscherSpRecord.RECORD_ID); + int flags = spRecord.getFlags(); + if (placeholder == null) { + flags ^= EscherSpRecord.FLAG_HAVEMASTER; + } else { + flags |= EscherSpRecord.FLAG_HAVEANCHOR | EscherSpRecord.FLAG_HAVEMASTER; + } + spRecord.setFlags(flags); + + // Placeholders can't be grouped + shape.setEscherProperty(EscherPropertyTypes.PROTECTION__LOCKAGAINSTGROUPING, (placeholder == null ? -1 : 262144)); + } + + static void removePlaceholder(final HSLFSimpleShape shape) { + final HSLFEscherClientDataRecord clientData = shape.getClientData(false); + if (clientData != null) { + clientData.removeChild(OEPlaceholderAtom.class); + clientData.removeChild(RoundTripHFPlaceholder12.class); + // remove client data if the placeholder was the only child to be carried + if (clientData.getChildRecords().isEmpty()) { + shape.getSpContainer().removeChildRecord(clientData); + } + } + } + + static OEPlaceholderAtom createOEPlaceholderAtom(final HSLFEscherClientDataRecord clientData) { + return createOEPlaceholderAtom(clientData, (byte) 0); + } + + static OEPlaceholderAtom createOEPlaceholderAtom(final HSLFEscherClientDataRecord clientData, + final byte placeholderId) { + OEPlaceholderAtom oePlaceholderAtom = new OEPlaceholderAtom(); + oePlaceholderAtom.setPlaceholderSize((byte)OEPlaceholderAtom.PLACEHOLDER_FULLSIZE); + // TODO: placement id only "SHOULD" be unique ... check other placeholders on sheet for unique id + oePlaceholderAtom.setPlacementId(-1); + oePlaceholderAtom.setPlaceholderId(placeholderId); + clientData.addChild(oePlaceholderAtom); + return oePlaceholderAtom; + } + + static byte getPlaceholderId(final HSLFSheet sheet, final Placeholder placeholder) { + if (placeholder == null) { + return 0; + } + byte phId; + // TODO: implement/switch NotesMaster + if (sheet instanceof HSLFSlideMaster) { + phId = (byte) placeholder.nativeSlideMasterId; + } else if (sheet instanceof HSLFNotes) { + phId = (byte) placeholder.nativeNotesId; + } else { + phId = (byte) placeholder.nativeSlideId; + } + + if (phId == -2) { + throw new HSLFException("Placeholder " + placeholder.name() + " not supported for this sheet type (" + sheet.getClass() + ")"); + } + return phId; + } + private enum PlaceholderContainer { slide, master, notes, notesMaster } @@ -78,7 +140,7 @@ public class HSLFShapePlaceholderDetails extends HSLFPlaceholderDetails { @Override public Placeholder getPlaceholder() { - updatePlaceholderAtom(false); + updatePlaceholderAtom(null, false); final int phId; if (oePlaceholderAtom != null) { phId = oePlaceholderAtom.getPlaceholderId(); @@ -105,17 +167,7 @@ public class HSLFShapePlaceholderDetails extends HSLFPlaceholderDetails { @Override public void setPlaceholder(final Placeholder placeholder) { - final EscherSpRecord spRecord = shape.getEscherChild(EscherSpRecord.RECORD_ID); - int flags = spRecord.getFlags(); - if (placeholder == null) { - flags ^= EscherSpRecord.FLAG_HAVEMASTER; - } else { - flags |= EscherSpRecord.FLAG_HAVEANCHOR | EscherSpRecord.FLAG_HAVEMASTER; - } - spRecord.setFlags(flags); - - // Placeholders can't be grouped - shape.setEscherProperty(EscherPropertyTypes.PROTECTION__LOCKAGAINSTGROUPING, (placeholder == null ? -1 : 262144)); + updateSPRecord(shape, placeholder); if (placeholder == null) { removePlaceholder(); @@ -123,7 +175,7 @@ public class HSLFShapePlaceholderDetails extends HSLFPlaceholderDetails { } // init client data - updatePlaceholderAtom(true); + updatePlaceholderAtom(placeholder, true); final byte phId = getPlaceholderId(placeholder); oePlaceholderAtom.setPlaceholderId(phId); @@ -158,7 +210,7 @@ public class HSLFShapePlaceholderDetails extends HSLFPlaceholderDetails { if (ph == null || size == null) { return; } - updatePlaceholderAtom(true); + updatePlaceholderAtom(ph, true); final byte ph_size; switch (size) { @@ -209,20 +261,12 @@ public class HSLFShapePlaceholderDetails extends HSLFPlaceholderDetails { } private void removePlaceholder() { - final HSLFEscherClientDataRecord clientData = shape.getClientData(false); - if (clientData != null) { - clientData.removeChild(OEPlaceholderAtom.class); - clientData.removeChild(RoundTripHFPlaceholder12.class); - // remove client data if the placeholder was the only child to be carried - if (clientData.getChildRecords().isEmpty()) { - shape.getSpContainer().removeChildRecord(clientData); - } - } + removePlaceholder(shape); oePlaceholderAtom = null; roundTripHFPlaceholder12 = null; } - private void updatePlaceholderAtom(final boolean create) { + private void updatePlaceholderAtom(final Placeholder placeholder, final boolean create) { localDateTime = null; if (shape instanceof HSLFTextBox) { EscherTextboxWrapper txtBox = ((HSLFTextBox)shape).getEscherTextboxWrapper(); @@ -255,11 +299,8 @@ public class HSLFShapePlaceholderDetails extends HSLFPlaceholderDetails { } if (oePlaceholderAtom == null) { - oePlaceholderAtom = new OEPlaceholderAtom(); - oePlaceholderAtom.setPlaceholderSize((byte)OEPlaceholderAtom.PLACEHOLDER_FULLSIZE); - // TODO: placement id only "SHOULD" be unique ... check other placeholders on sheet for unique id - oePlaceholderAtom.setPlacementId(-1); - clientData.addChild(oePlaceholderAtom); + final byte phId = getPlaceholderId(shape.getSheet(), placeholder); + oePlaceholderAtom = createOEPlaceholderAtom(clientData, phId); } if (roundTripHFPlaceholder12 == null) { roundTripHFPlaceholder12 = new RoundTripHFPlaceholder12(); diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFSimpleShape.java b/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFSimpleShape.java index da930b6899..aa85a65a92 100644 --- a/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFSimpleShape.java +++ b/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFSimpleShape.java @@ -20,6 +20,9 @@ package org.apache.poi.hslf.usermodel; import java.awt.Color; import org.apache.logging.log4j.Logger; +import org.apache.poi.hslf.record.HSLFEscherClientDataRecord; +import org.apache.poi.hslf.record.OEPlaceholderAtom; +import org.apache.poi.hslf.record.RoundTripHFPlaceholder12; import org.apache.poi.logging.PoiLogManager; import org.apache.poi.ddf.AbstractEscherOptRecord; import org.apache.poi.ddf.EscherChildAnchorRecord; @@ -53,6 +56,11 @@ import org.apache.poi.sl.usermodel.StrokeStyle.LineDash; import org.apache.poi.util.LittleEndian; import org.apache.poi.util.Units; +import static org.apache.poi.hslf.usermodel.HSLFShapePlaceholderDetails.createOEPlaceholderAtom; +import static org.apache.poi.hslf.usermodel.HSLFShapePlaceholderDetails.getPlaceholderId; +import static org.apache.poi.hslf.usermodel.HSLFShapePlaceholderDetails.removePlaceholder; +import static org.apache.poi.hslf.usermodel.HSLFShapePlaceholderDetails.updateSPRecord; + /** * An abstract simple (non-group) shape. * This is the parent class for all primitive shapes like Line, Rectangle, etc. @@ -80,6 +88,8 @@ public abstract class HSLFSimpleShape extends HSLFShape implements SimpleShape<H */ protected HSLFHyperlink _hyperlink; + protected HSLFShapePlaceholderDetails _placeholderDetails; + /** * Create a SimpleShape object and initialize it from the supplied Record container. * @@ -564,10 +574,12 @@ public abstract class HSLFSimpleShape extends HSLFShape implements SimpleShape<H @Override public HSLFShapePlaceholderDetails getPlaceholderDetails() { - return new HSLFShapePlaceholderDetails(this); + if (_placeholderDetails == null) { + _placeholderDetails = new HSLFShapePlaceholderDetails(this); + } + return _placeholderDetails; } - @Override public Placeholder getPlaceholder() { return getPlaceholderDetails().getPlaceholder(); @@ -575,9 +587,62 @@ public abstract class HSLFSimpleShape extends HSLFShape implements SimpleShape<H @Override public void setPlaceholder(Placeholder placeholder) { - getPlaceholderDetails().setPlaceholder(placeholder); - } + // reset the placeholder details so that the next call to getPlaceholderDetails() will reinitialize it + _placeholderDetails = null; + updateSPRecord(this, placeholder); + if (placeholder == null) { + removePlaceholder(this); + return; + } + + HSLFEscherClientDataRecord clientData = getClientData(true); + + // OEPlaceholderAtom tells powerpoint that this shape is a placeholder + OEPlaceholderAtom oep = null; + RoundTripHFPlaceholder12 rtp = null; + for (org.apache.poi.hslf.record.Record r : clientData.getHSLFChildRecords()) { + if (r instanceof OEPlaceholderAtom) { + oep = (OEPlaceholderAtom) r; + break; + } + if (r instanceof RoundTripHFPlaceholder12) { + rtp = (RoundTripHFPlaceholder12) r; + break; + } + } + + /** + * Extract from MSDN: + * + * There is a special case when the placeholder does not have a position in the layout. + * This occurs when the user has moved the placeholder from its original position. + * In this case the placeholder ID is -1. + */ + final byte phId = getPlaceholderId(getSheet(), placeholder); + + switch (placeholder) { + case HEADER: + case FOOTER: + if (rtp == null) { + rtp = new RoundTripHFPlaceholder12(); + rtp.setPlaceholderId(phId); + clientData.addChild(rtp); + } + if (oep != null) { + clientData.removeChild(OEPlaceholderAtom.class); + } + break; + default: + if (rtp != null) { + clientData.removeChild(RoundTripHFPlaceholder12.class); + } + if (oep == null) { + oep = createOEPlaceholderAtom(clientData, phId); + } + break; + } + } @Override public void setStrokeStyle(Object... styles) { diff --git a/poi-scratchpad/src/test/java/org/apache/poi/hslf/model/TestLine.java b/poi-scratchpad/src/test/java/org/apache/poi/hslf/model/TestLine.java index 0384b19043..e26793bde5 100644 --- a/poi-scratchpad/src/test/java/org/apache/poi/hslf/model/TestLine.java +++ b/poi-scratchpad/src/test/java/org/apache/poi/hslf/model/TestLine.java @@ -18,6 +18,8 @@ package org.apache.poi.hslf.model; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; import java.awt.Color; import java.awt.Rectangle; @@ -27,9 +29,11 @@ import java.util.Arrays; import org.apache.poi.hslf.HSLFTestDataSamples; import org.apache.poi.hslf.usermodel.HSLFLine; +import org.apache.poi.hslf.usermodel.HSLFPlaceholder; import org.apache.poi.hslf.usermodel.HSLFShape; import org.apache.poi.hslf.usermodel.HSLFSlide; import org.apache.poi.hslf.usermodel.HSLFSlideShow; +import org.apache.poi.hslf.usermodel.HSLFTextBox; import org.apache.poi.sl.usermodel.StrokeStyle.LineCompound; import org.apache.poi.sl.usermodel.StrokeStyle.LineDash; import org.junit.jupiter.api.Test; @@ -62,9 +66,14 @@ public final class TestLine { @Test void testCreateLines() throws IOException { + final String title = "Lines tester"; try (HSLFSlideShow ppt1 = new HSLFSlideShow()) { HSLFSlide slide1 = ppt1.createSlide(); - slide1.addTitle().setText("Lines tester"); + HSLFTextBox titleBox = slide1.addTitle(); + titleBox.setText(title); + assertInstanceOf(HSLFPlaceholder.class, titleBox); + HSLFPlaceholder pl = (HSLFPlaceholder) titleBox; + assertNotNull(pl.getPlaceholder()); for (Object[] line : lines) { HSLFLine hslfLine = new HSLFLine(); @@ -85,6 +94,7 @@ public final class TestLine { try (HSLFSlideShow ppt2 = HSLFTestDataSamples.writeOutAndReadBack(ppt1)) { HSLFSlide slide2 = ppt2.getSlides().get(0); + assertEquals(title, slide2.getTitle()); int idx = 0; for (HSLFShape shape : slide2.getShapes().subList(1,14)) { diff --git a/poi/src/main/java/org/apache/poi/ss/util/CellUtil.java b/poi/src/main/java/org/apache/poi/ss/util/CellUtil.java index d0ee0db55b..b2879fb7fd 100644 --- a/poi/src/main/java/org/apache/poi/ss/util/CellUtil.java +++ b/poi/src/main/java/org/apache/poi/ss/util/CellUtil.java @@ -280,6 +280,7 @@ public final class CellUtil { map.put(LEFT_BORDER_COLOR, CellPropertyType.LEFT_BORDER_COLOR); map.put(RIGHT_BORDER_COLOR, CellPropertyType.RIGHT_BORDER_COLOR); map.put(TOP_BORDER_COLOR, CellPropertyType.TOP_BORDER_COLOR); + map.put(DATA_FORMAT, CellPropertyType.DATA_FORMAT); map.put(FILL_BACKGROUND_COLOR, CellPropertyType.FILL_BACKGROUND_COLOR); map.put(FILL_FOREGROUND_COLOR, CellPropertyType.FILL_FOREGROUND_COLOR); map.put(FILL_BACKGROUND_COLOR_COLOR, CellPropertyType.FILL_BACKGROUND_COLOR_COLOR); @@ -289,10 +290,11 @@ public final class CellUtil { map.put(HIDDEN, CellPropertyType.HIDDEN); map.put(INDENTION, CellPropertyType.INDENTION); map.put(LOCKED, CellPropertyType.LOCKED); + map.put(QUOTE_PREFIXED, CellPropertyType.QUOTE_PREFIXED); map.put(ROTATION, CellPropertyType.ROTATION); - map.put(VERTICAL_ALIGNMENT, CellPropertyType.VERTICAL_ALIGNMENT); map.put(SHRINK_TO_FIT, CellPropertyType.SHRINK_TO_FIT); - map.put(QUOTE_PREFIXED, CellPropertyType.QUOTE_PREFIXED); + map.put(VERTICAL_ALIGNMENT, CellPropertyType.VERTICAL_ALIGNMENT); + map.put(WRAP_TEXT, CellPropertyType.WRAP_TEXT); namePropertyMap = Collections.unmodifiableMap(map); } @@ -572,7 +574,7 @@ public final class CellUtil { @Deprecated @Removal(version = "7.0.0") public static void setCellStyleProperties(Cell cell, Map<String, Object> properties) { - EnumMap<CellPropertyType, Object> strPropMap = new EnumMap<>(CellPropertyType.class); + final EnumMap<CellPropertyType, Object> strPropMap = new EnumMap<>(CellPropertyType.class); properties.forEach((k, v) -> strPropMap.put(namePropertyMap.get(k), v)); setCellStyleProperties(cell, strPropMap, false); } @@ -685,10 +687,16 @@ public final class CellUtil { * @param cell The cell that is to be changed. * @param property The name of the property that is to be changed. * @param propertyValue The value of the property that is to be changed. - * + * @throws NullPointerException if {@code cell} or {@code property} is null * @since POI 5.4.0 */ public static void setCellStyleProperty(Cell cell, CellPropertyType property, Object propertyValue) { + if (cell == null) { + throw new NullPointerException("Cell must not be null"); + } + if (property == null) { + throw new NullPointerException("CellPropertyType must not be null"); + } boolean disableNullColorCheck = false; final Map<CellPropertyType, Object> propMap; if (CellPropertyType.FILL_FOREGROUND_COLOR_COLOR.equals(property) && propertyValue == null) { diff --git a/poi/src/test/java/org/apache/poi/ss/util/TestCellUtil.java b/poi/src/test/java/org/apache/poi/ss/util/TestCellUtil.java new file mode 100644 index 0000000000..b7defbef99 --- /dev/null +++ b/poi/src/test/java/org/apache/poi/ss/util/TestCellUtil.java @@ -0,0 +1,36 @@ +/* ==================================================================== + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +==================================================================== */ +package org.apache.poi.ss.util; + +import org.apache.poi.ss.usermodel.CellPropertyType; +import org.junit.jupiter.api.Test; + +import java.util.Arrays; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Test for CellUtil constants + */ +class TestCellUtil { + @Test + void testNamePropertyMap() { + Arrays.stream(CellPropertyType.values()).forEach(cellPropertyType -> + assertTrue(CellUtil.namePropertyMap.containsValue(cellPropertyType), + "missing " + cellPropertyType)); + } +} |