From c7c5fc23bed47c613afa0bc980d1b19102d9de09 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Fri, 24 Apr 2020 20:58:44 +0000 Subject: [PATCH] Fix regression introduced via Bug 60845: There are more items in CTBorder that need to be handled in equals() git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1876949 13f79535-47bb-0310-9956-ffa450edef68 --- .../usermodel/extensions/XSSFCellBorder.java | 59 +++++- .../poi/xssf/usermodel/TestXSSFCellStyle.java | 110 +++++------ .../extensions/XSSFCellBorderTest.java | 177 ++++++++++++++++++ 3 files changed, 279 insertions(+), 67 deletions(-) create mode 100644 src/ooxml/testcases/org/apache/poi/xssf/usermodel/extensions/XSSFCellBorderTest.java diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/extensions/XSSFCellBorder.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/extensions/XSSFCellBorder.java index da67280f52..31ed2a2203 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/extensions/XSSFCellBorder.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/extensions/XSSFCellBorder.java @@ -57,7 +57,7 @@ public class XSSFCellBorder { public XSSFCellBorder(CTBorder border) { this(border, null, null); } - + /** * * @param border The ooxml object for the border @@ -83,12 +83,12 @@ public class XSSFCellBorder { public void setThemesTable(ThemesTable themes) { this._theme = themes; } - + /** * The enumeration value indicating the side being used for a cell border. */ - public static enum BorderSide { - TOP, RIGHT, BOTTOM, LEFT + public enum BorderSide { + TOP, RIGHT, BOTTOM, LEFT, DIAGONAL, VERTICAL, HORIZONTAL } /** @@ -133,8 +133,8 @@ public class XSSFCellBorder { */ public XSSFColor getBorderColor(BorderSide side) { CTBorderPr borderPr = getBorder(side); - - if(borderPr != null && borderPr.isSetColor()) { + + if(borderPr != null && borderPr.isSetColor()) { XSSFColor clr = XSSFColor.from(borderPr.getColor(), _indexedColorMap); if(_theme != null) { _theme.inheritFromThemeAsRequired(clr); @@ -183,8 +183,20 @@ public class XSSFCellBorder { borderPr = border.getLeft(); if (ensure && borderPr == null) borderPr = border.addNewLeft(); break; + case DIAGONAL: + borderPr = border.getDiagonal(); + if (ensure && borderPr == null) borderPr = border.addNewDiagonal(); + break; + case VERTICAL: + borderPr = border.getVertical(); + if (ensure && borderPr == null) borderPr = border.addNewVertical(); + break; + case HORIZONTAL: + borderPr = border.getHorizontal(); + if (ensure && borderPr == null) borderPr = border.addNewHorizontal(); + break; default: - throw new IllegalArgumentException("No suitable side specified for the border"); + throw new IllegalArgumentException("No suitable side specified for the border, had " + side); } return borderPr; } @@ -212,7 +224,36 @@ public class XSSFCellBorder { break; } } - - return equal; + if(!equal) { + return false; + } + + // we also need to compare some more boolean-values + + // first all booleans need to have the same state of "defined" or "undefined" + if(this.border.isSetDiagonalUp() != cf.border.isSetDiagonalUp() || + this.border.isSetDiagonalDown() != cf.border.isSetDiagonalDown() || + this.border.isSetOutline() != cf.border.isSetOutline()) { + return false; + } + + // then compare each value if necessary + if(this.border.isSetDiagonalUp() && + this.border.getDiagonalUp() != cf.border.getDiagonalUp()) { + return false; + } + + if(this.border.isSetDiagonalDown() && + this.border.getDiagonalDown() != cf.border.getDiagonalDown()) { + return false; + } + + //noinspection RedundantIfStatement + if(this.border.isSetOutline() && + this.border.getOutline() != cf.border.getOutline()) { + return false; + } + + return true; } } \ No newline at end of file diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCellStyle.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCellStyle.java index 9030d2eb18..870a9f36b9 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCellStyle.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCellStyle.java @@ -50,22 +50,15 @@ import org.openxmlformats.schemas.spreadsheetml.x2006.main.STVerticalAlignment; public class TestXSSFCellStyle { private StylesTable stylesTable; - private CTBorder ctBorderA; - private CTFill ctFill; - private CTFont ctFont; - private CTXf cellStyleXf; - private CTXf cellXf; - private CTCellXfs cellXfs; - private XSSFCellStyle cellStyle; - private CTStylesheet ctStylesheet; - - @Before + private XSSFCellStyle cellStyle; + + @Before public void setUp() { stylesTable = new StylesTable(); - ctStylesheet = stylesTable.getCTStylesheet(); + CTStylesheet ctStylesheet = stylesTable.getCTStylesheet(); - ctBorderA = CTBorder.Factory.newInstance(); + CTBorder ctBorderA = CTBorder.Factory.newInstance(); XSSFCellBorder borderA = new XSSFCellBorder(ctBorderA); long borderId = stylesTable.putBorder(borderA); assertEquals(0, borderId); @@ -73,23 +66,23 @@ public class TestXSSFCellStyle { XSSFCellBorder borderB = new XSSFCellBorder(); assertEquals(0, stylesTable.putBorder(borderB)); - ctFill = CTFill.Factory.newInstance(); + CTFill ctFill = CTFill.Factory.newInstance(); XSSFCellFill fill = new XSSFCellFill(ctFill, null); long fillId = stylesTable.putFill(fill); assertEquals(2, fillId); - ctFont = CTFont.Factory.newInstance(); + CTFont ctFont = CTFont.Factory.newInstance(); XSSFFont font = new XSSFFont(ctFont); long fontId = stylesTable.putFont(font); assertEquals(1, fontId); - cellStyleXf = ctStylesheet.addNewCellStyleXfs().addNewXf(); + CTXf cellStyleXf = ctStylesheet.addNewCellStyleXfs().addNewXf(); cellStyleXf.setBorderId(1); cellStyleXf.setFillId(1); cellStyleXf.setFontId(1); - cellXfs = ctStylesheet.addNewCellXfs(); - cellXf = cellXfs.addNewXf(); + CTCellXfs cellXfs = ctStylesheet.addNewCellXfs(); + CTXf cellXf = cellXfs.addNewXf(); cellXf.setXfId(1); cellXf.setBorderId(1); cellXf.setFillId(1); @@ -253,7 +246,7 @@ public class TestXSSFCellStyle { //replacement: assertEquals(ctBorder.getTop().getStyle(), STBorderStyle.NONE); } - + private void testGetSetBorderXMLBean(BorderStyle border, STBorderStyle.Enum expected) { cellStyle.setBorderTop(border); assertEquals(border, cellStyle.getBorderTop()); @@ -263,8 +256,8 @@ public class TestXSSFCellStyle { CTBorder ctBorder = stylesTable.getBorderAt(borderId).getCTBorder(); assertEquals(expected, ctBorder.getTop().getStyle()); } - - + + // Border Styles, in BorderStyle/STBorderStyle enum order @Test public void testGetSetBorderNone() { @@ -287,67 +280,67 @@ public class TestXSSFCellStyle { public void testGetSetBorderThin() { testGetSetBorderXMLBean(BorderStyle.THIN, STBorderStyle.THIN); } - + @Test public void testGetSetBorderMedium() { testGetSetBorderXMLBean(BorderStyle.MEDIUM, STBorderStyle.MEDIUM); } - + @Test public void testGetSetBorderDashed() { testGetSetBorderXMLBean(BorderStyle.DASHED, STBorderStyle.DASHED); } - + @Test public void testGetSetBorderDotted() { testGetSetBorderXMLBean(BorderStyle.DOTTED, STBorderStyle.DOTTED); } - + @Test public void testGetSetBorderThick() { testGetSetBorderXMLBean(BorderStyle.THICK, STBorderStyle.THICK); } - + @Test public void testGetSetBorderDouble() { testGetSetBorderXMLBean(BorderStyle.DOUBLE, STBorderStyle.DOUBLE); } - + @Test public void testGetSetBorderHair() { testGetSetBorderXMLBean(BorderStyle.HAIR, STBorderStyle.HAIR); } - + @Test public void testGetSetBorderMediumDashed() { testGetSetBorderXMLBean(BorderStyle.MEDIUM_DASHED, STBorderStyle.MEDIUM_DASHED); } - + @Test public void testGetSetBorderDashDot() { testGetSetBorderXMLBean(BorderStyle.DASH_DOT, STBorderStyle.DASH_DOT); } - + @Test public void testGetSetBorderMediumDashDot() { testGetSetBorderXMLBean(BorderStyle.MEDIUM_DASH_DOT, STBorderStyle.MEDIUM_DASH_DOT); } - + @Test public void testGetSetBorderDashDotDot() { testGetSetBorderXMLBean(BorderStyle.DASH_DOT_DOT, STBorderStyle.DASH_DOT_DOT); } - + @Test public void testGetSetBorderMediumDashDotDot() { testGetSetBorderXMLBean(BorderStyle.MEDIUM_DASH_DOT_DOT, STBorderStyle.MEDIUM_DASH_DOT_DOT); } - + @Test public void testGetSetBorderSlantDashDot() { testGetSetBorderXMLBean(BorderStyle.SLANTED_DASH_DOT, STBorderStyle.SLANT_DASH_DOT); } - + @Test public void testGetSetBottomBorderColor() { //defaults @@ -703,7 +696,7 @@ public class TestXSSFCellStyle { assertEquals(HorizontalAlignment.CENTER, cellStyle.getAlignment()); assertEquals(STHorizontalAlignment.CENTER, cellStyle.getCellAlignment().getCTCellAlignment().getHorizontal()); } - + @Test public void testGetSetReadingOrder() { assertEquals(ReadingOrder.CONTEXT, cellStyle.getReadingOrder()); @@ -716,7 +709,7 @@ public class TestXSSFCellStyle { cellStyle.setReadingOrder(ReadingOrder.RIGHT_TO_LEFT); assertEquals(ReadingOrder.RIGHT_TO_LEFT, cellStyle.getReadingOrder()); assertEquals(ReadingOrder.RIGHT_TO_LEFT.getCode(), cellStyle.getCellAlignment().getCTCellAlignment().getReadingOrder()); - + cellStyle.setReadingOrder(ReadingOrder.CONTEXT); assertEquals(ReadingOrder.CONTEXT, cellStyle.getReadingOrder()); assertEquals(ReadingOrder.CONTEXT.getCode(), cellStyle.getCellAlignment().getCTCellAlignment().getReadingOrder()); @@ -752,25 +745,25 @@ public class TestXSSFCellStyle { public void testCloneStyleSameWB() throws IOException { XSSFWorkbook wb = new XSSFWorkbook(); assertEquals(1, wb.getNumberOfFonts()); - + XSSFFont fnt = wb.createFont(); fnt.setFontName("TestingFont"); assertEquals(2, wb.getNumberOfFonts()); - + XSSFCellStyle orig = wb.createCellStyle(); orig.setAlignment(HorizontalAlignment.RIGHT); orig.setFont(fnt); orig.setDataFormat((short)18); - + assertEquals(HorizontalAlignment.RIGHT, orig.getAlignment()); assertEquals(fnt, orig.getFont()); assertEquals(18, orig.getDataFormat()); - + XSSFCellStyle clone = wb.createCellStyle(); assertNotSame(HorizontalAlignment.RIGHT, clone.getAlignment()); assertNotSame(fnt, clone.getFont()); assertNotEquals(18, clone.getDataFormat()); - + clone.cloneStyleFrom(orig); assertEquals(HorizontalAlignment.RIGHT, clone.getAlignment()); assertEquals(fnt, clone.getFont()); @@ -780,7 +773,7 @@ public class TestXSSFCellStyle { XSSFWorkbook wb2 = XSSFTestDataSamples.writeOutAndReadBack(wb); assertNotNull(wb2); wb2.close(); - + wb.close(); } @@ -810,6 +803,7 @@ public class TestXSSFCellStyle { orig.setFillForegroundColor(IndexedColors.BRIGHT_GREEN.getIndex()); XSSFCellStyle origEmpty = wbOrig.createCellStyle(); + assertNotNull(origEmpty); assertSame(HorizontalAlignment.RIGHT, orig.getAlignment()); assertSame(fnt, orig.getFont()); @@ -883,7 +877,7 @@ public class TestXSSFCellStyle { XSSFWorkbook workbook = XSSFTestDataSamples.openSampleWorkbook("52348.xlsx"); StylesTable st = workbook.getStylesSource(); assertEquals(0, st._getStyleXfsSize()); - + XSSFCellStyle style = workbook.createCellStyle(); // no exception at this point assertNull(style.getStyleXf()); @@ -910,7 +904,7 @@ public class TestXSSFCellStyle { XSSFWorkbook wb2 = XSSFTestDataSamples.writeOutAndReadBack(workbook); assertNotNull(wb2); wb2.close(); - + workbook.close(); } @@ -947,17 +941,17 @@ public class TestXSSFCellStyle { XSSFWorkbook wb4 = XSSFTestDataSamples.writeOutAndReadBack(wb2); assertNotNull(wb4); wb4.close(); - + XSSFWorkbook wb5 = XSSFTestDataSamples.writeOutAndReadBack(wb3); assertNotNull(wb5); wb5.close(); - + wb3.close(); wb2.close(); wb1.close(); - + } - + @Test public void testSetColor() throws IOException { try(Workbook wb = new XSSFWorkbook()) { @@ -1011,7 +1005,7 @@ public class TestXSSFCellStyle { Workbook reference = XSSFTestDataSamples.openSampleWorkbook("template.xlsx"); Workbook target = new XSSFWorkbook(); copyStyles(reference, target); - + assertEquals(reference.getNumCellStyles(), target.getNumCellStyles()); final Sheet sheet = target.createSheet(); final Row row = sheet.createRow(0); @@ -1027,11 +1021,11 @@ public class TestXSSFCellStyle { Workbook copy = XSSFTestDataSamples.writeOutAndReadBack(target); - // previously this failed because the border-element was not copied over + // previously this failed because the border-element was not copied over copy.getCellStyleAt((short)1).getBorderBottom(); - + copy.close(); - + target.close(); reference.close(); } @@ -1042,23 +1036,23 @@ public class TestXSSFCellStyle { cellStyle.setRotation((short)89); assertEquals(89, cellStyle.getRotation()); - + cellStyle.setRotation((short)90); assertEquals(90, cellStyle.getRotation()); - + cellStyle.setRotation((short)179); assertEquals(179, cellStyle.getRotation()); - + cellStyle.setRotation((short)180); assertEquals(180, cellStyle.getRotation()); - + // negative values are mapped to the correct values for compatibility between HSSF and XSSF cellStyle.setRotation((short)-1); assertEquals(91, cellStyle.getRotation()); - + cellStyle.setRotation((short)-89); assertEquals(179, cellStyle.getRotation()); - + cellStyle.setRotation((short)-90); assertEquals(180, cellStyle.getRotation()); } @@ -1088,7 +1082,7 @@ public class TestXSSFCellStyle { cellStyle.setRightBorderColor(null); assertNull(cellStyle.getRightBorderXSSFColor()); - + workbook.close(); } } diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/extensions/XSSFCellBorderTest.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/extensions/XSSFCellBorderTest.java new file mode 100644 index 0000000000..3911d32462 --- /dev/null +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/extensions/XSSFCellBorderTest.java @@ -0,0 +1,177 @@ +/* ==================================================================== + 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.usermodel.extensions; + +import org.apache.poi.ss.usermodel.BorderStyle; +import org.apache.poi.ss.usermodel.Cell; +import org.apache.poi.ss.usermodel.Sheet; +import org.apache.poi.xssf.XSSFTestDataSamples; +import org.apache.poi.xssf.model.StylesTable; +import org.apache.poi.xssf.model.ThemesTable; +import org.apache.poi.xssf.usermodel.XSSFCellStyle; +import org.apache.poi.xssf.usermodel.XSSFColor; +import org.apache.poi.xssf.usermodel.XSSFWorkbook; +import org.junit.Before; +import org.junit.Test; +import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTBorder; +import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTBorderPr; +import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTXf; +import org.openxmlformats.schemas.spreadsheetml.x2006.main.STBorderStyle; + +import static org.junit.Assert.*; + +public class XSSFCellBorderTest { + private final XSSFWorkbook wb = new XSSFWorkbook(); + private final StylesTable stylesSource = wb.getStylesSource(); + private final XSSFCellBorder empty = new XSSFCellBorder(); + + @Before + public void setUp() { + assertNotNull(stylesSource); + assertEquals(1, stylesSource.getBorders().size()); + } + + @Test + public void testEquals() { + for (XSSFCellBorder.BorderSide side : XSSFCellBorder.BorderSide.values()) { + XSSFCellBorder border = new XSSFCellBorder(); + assertEquals(empty, border); + assertEquals(empty.hashCode(), border.hashCode()); + + border.setBorderStyle(side, BorderStyle.THICK); + assertNotEquals(empty, border); + + border = new XSSFCellBorder(); + assertEquals(empty, border); + assertEquals(empty.hashCode(), border.hashCode()); + + border.setBorderColor(side, new XSSFColor(stylesSource.getIndexedColors())); + assertNotEquals(empty, border); + } + + // also verify diagonal_up, diagonal_down and outline + XSSFCellBorder border = new XSSFCellBorder(); + border.getCTBorder().setDiagonalUp(true); + assertNotEquals(empty, border); + + border = new XSSFCellBorder(); + border.getCTBorder().setDiagonalDown(true); + assertNotEquals(empty, border); + + border = new XSSFCellBorder(); + border.getCTBorder().setOutline(true); + assertNotEquals(empty, border); + } + + @Test + public void testConstruct() { + XSSFCellBorder border = new XSSFCellBorder((CTBorder) empty.getCTBorder().copy()); + assertEquals(empty, border); + border.getCTBorder().setOutline(true); + assertNotEquals(empty, border); + + border = new XSSFCellBorder((CTBorder) empty.getCTBorder().copy(), stylesSource.getIndexedColors()); + assertEquals(empty, border); + border.getCTBorder().setOutline(true); + assertNotEquals(empty, border); + + border = new XSSFCellBorder((CTBorder) empty.getCTBorder().copy(), stylesSource.getTheme(), + stylesSource.getIndexedColors()); + assertEquals(empty, border); + border.getCTBorder().setOutline(true); + assertNotEquals(empty, border); + } + + @Test + public void testGettersSetters() { + assertNotNull(empty.getCTBorder()); + + XSSFCellBorder border = new XSSFCellBorder((CTBorder) empty.getCTBorder().copy()); + border.setThemesTable(stylesSource.getTheme()); + assertNotNull(border.getCTBorder()); + } + + @Test + public void testSetBorderStyle() { + XSSFCellBorder border = new XSSFCellBorder(); + for (XSSFCellBorder.BorderSide side : XSSFCellBorder.BorderSide.values()) { + assertEquals(BorderStyle.NONE, border.getBorderStyle(side)); + + border.setBorderStyle(side, BorderStyle.THIN); + assertEquals(BorderStyle.THIN, border.getBorderStyle(side)); + } + } + + @Test + public void testSetBorderColor() { + XSSFCellBorder border = new XSSFCellBorder(); + XSSFColor color = new XSSFColor(stylesSource.getIndexedColors()); + + for (XSSFCellBorder.BorderSide side : XSSFCellBorder.BorderSide.values()) { + assertNull(border.getBorderColor(side)); + + border.setBorderColor(side, color); + assertEquals(color, border.getBorderColor(side)); + } + } + + @Test + public void testRegression() throws Exception { + XSSFCellStyle style = wb.createCellStyle(); + style.setBorderTop(BorderStyle.THICK); + style.setBorderBottom(BorderStyle.THICK); + + assertEquals(3, stylesSource.getBorders().size()); + + ThemesTable _theme = stylesSource.getTheme(); + CTXf _cellXf = style.getCoreXf(); + assertTrue(_cellXf.getApplyBorder()); + + int idx = (int) _cellXf.getBorderId(); + XSSFCellBorder cf = stylesSource.getBorderAt(idx); + CTBorder ct = (CTBorder) cf.getCTBorder().copy(); + + assertFalse(ct.isSetDiagonal()); + CTBorderPr pr = ct.addNewDiagonal(); + ct.setDiagonalUp(true); + pr.setStyle(STBorderStyle.Enum.forInt(BorderStyle.THICK.getCode() + 1)); + idx = stylesSource.putBorder(new XSSFCellBorder(ct, _theme, + stylesSource.getIndexedColors())); + _cellXf.setBorderId(idx); + _cellXf.setApplyBorder(true); + + assertEquals(4, stylesSource.getBorders().size()); + + style.setBorderLeft(BorderStyle.THICK); + style.setBorderRight(BorderStyle.THICK); + + Sheet sheet = wb.createSheet("Sheet1"); + + assertEquals(6, stylesSource.getBorders().size()); + + Cell cell = sheet.createRow(1).createCell(2); + cell.setCellStyle(style); + + assertEquals(6, stylesSource.getBorders().size()); + + XSSFWorkbook wbBack = XSSFTestDataSamples.writeOutAndReadBack(wb); + + assertEquals(6, wbBack.getStylesSource().getBorders().size()); + + wb.close(); + } +} \ No newline at end of file -- 2.39.5