diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCellStyle.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCellStyle.java index 6e66f99..1ffffe6 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCellStyle.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCellStyle.java @@ -188,7 +188,7 @@ src.getFont().getCTFont().toString(), DEFAULT_XML_OPTIONS ); XSSFFont font = new XSSFFont(ctFont); - font.registerTo(_stylesSource); + font.registerTo_Extended(_stylesSource, false); setFont(font); } catch(XmlException e) { throw new POIXMLException(e); diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFFont.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFFont.java index bdbbf74..2c278e2 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFFont.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFFont.java @@ -16,6 +16,8 @@ ==================================================================== */ package org.apache.poi.xssf.usermodel; +import java.util.Objects; + import org.apache.poi.POIXMLException; import org.apache.poi.util.Internal; import org.apache.poi.ss.usermodel.Font; @@ -553,11 +555,17 @@ * to the style table */ public long registerTo(StylesTable styles) { + return registerTo_Extended(styles, true); + } + + public long registerTo_Extended(StylesTable styles, boolean force) { this._themes = styles.getTheme(); - short idx = (short)styles.putFont(this, true); + short idx = (short)styles.putFont(this, force); this._index = idx; return idx; } + + /** * Records the Themes Table that is associated with * the current font, used when looking up theme @@ -644,7 +652,24 @@ if(!(o instanceof XSSFFont)) return false; XSSFFont cf = (XSSFFont)o; - return _ctFont.toString().equals(cf.getCTFont().toString()); + + // BUG 60845 + boolean equal = + Objects.equals(this.getItalic(), cf.getItalic()) + && Objects.equals(this.getBold(), cf.getBold()) + && Objects.equals(this.getStrikeout(), cf.getStrikeout()) + && Objects.equals(this.getCharSet(), cf.getCharSet()) + && Objects.equals(this.getBold(), cf.getBold()) + && Objects.equals(this.getColor(), cf.getColor()) + && Objects.equals(this.getFamily(), cf.getFamily()) + && Objects.equals(this.getFontHeight(), cf.getFontHeight()) + && Objects.equals(this.getFontName(), cf.getFontName()) + && Objects.equals(this.getScheme(), cf.getScheme()) + && Objects.equals(this.getThemeColor(), cf.getThemeColor()) + && Objects.equals(this.getTypeOffset(), cf.getTypeOffset()) + && Objects.equals(this.getUnderline(), cf.getUnderline()) + && Objects.equals(this.getXSSFColor(), cf.getXSSFColor()); + return equal; } } 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 b100bc2..acbdbf1 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 @@ -17,6 +17,8 @@ package org.apache.poi.xssf.usermodel.extensions; +import java.util.Objects; + import org.apache.poi.ss.usermodel.BorderStyle; import org.apache.poi.xssf.model.ThemesTable; import org.apache.poi.xssf.usermodel.XSSFColor; @@ -180,6 +182,17 @@ if (!(o instanceof XSSFCellBorder)) return false; XSSFCellBorder cf = (XSSFCellBorder) o; - return border.toString().equals(cf.getCTBorder().toString()); + + // bug 60845 + boolean equal = true; + for(BorderSide side : BorderSide.values()){ + if(!Objects.equals(this.getBorderColor(side), cf.getBorderColor(side)) + || !Objects.equals(this.getBorderStyle(side), cf.getBorderStyle(side))){ + equal = false; + break; + } + } + + return equal; } } \ No newline at end of file diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/extensions/XSSFCellFill.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/extensions/XSSFCellFill.java index fd6a70e..aef8182 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/extensions/XSSFCellFill.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/extensions/XSSFCellFill.java @@ -21,6 +21,9 @@ import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTPatternFill; import org.openxmlformats.schemas.spreadsheetml.x2006.main.STPatternType; import org.apache.poi.xssf.usermodel.XSSFColor; + +import java.util.Objects; + import org.apache.poi.util.Internal; /** @@ -122,7 +125,8 @@ */ public STPatternType.Enum getPatternType() { CTPatternFill ptrn = _fill.getPatternFill(); - return ptrn == null ? null : ptrn.getPatternType(); + + return ptrn == null ? STPatternType.NONE : ptrn.getPatternType(); } /** @@ -150,7 +154,7 @@ */ @Internal public CTFill getCTFill() { - return _fill; + return _fill; } @@ -162,6 +166,20 @@ if (!(o instanceof XSSFCellFill)) return false; XSSFCellFill cf = (XSSFCellFill) o; - return _fill.toString().equals(cf.getCTFill().toString()); + + // bug 60845 + // Do not compare the representing strings but the properties + // Reason: + // The strings are different if he XMLObject is a fragment (e.g. the ones from cloneStyle) + // even if they are in fact representing the same style + + + + boolean equal = + Objects.equals(this.getFillBackgroundColor(), cf.getFillBackgroundColor()) + && Objects.equals(this.getFillForegroundColor(), cf.getFillForegroundColor()) + && Objects.equals(this.getPatternType(), cf.getPatternType()); + + return equal; } } 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 c907d40..ae866c1 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCellStyle.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCellStyle.java @@ -67,15 +67,15 @@ ctBorderA = CTBorder.Factory.newInstance(); XSSFCellBorder borderA = new XSSFCellBorder(ctBorderA); long borderId = stylesTable.putBorder(borderA); - assertEquals(1, borderId); + assertEquals(0, borderId); XSSFCellBorder borderB = new XSSFCellBorder(); - assertEquals(1, stylesTable.putBorder(borderB)); + assertEquals(0, stylesTable.putBorder(borderB)); ctFill = CTFill.Factory.newInstance(); XSSFCellFill fill = new XSSFCellFill(ctFill); long fillId = stylesTable.putFill(fill); - assertEquals(2, fillId); + assertEquals(0, fillId); ctFont = CTFont.Factory.newInstance(); XSSFFont font = new XSSFFont(ctFont); @@ -133,7 +133,10 @@ assertEquals(num, stylesTable.getBorders().size()); borderId = (int)cellStyle.getCoreXf().getBorderId(); ctBorder = stylesTable.getBorderAt(borderId).getCTBorder(); - assertFalse(ctBorder.isSetBottom()); + //none is not the same as "not set", therefore the following doesn't work any more + //assertFalse(ctBorder.isSetBottom()); + //replacement: + assertEquals(ctBorder.getBottom().getStyle(), STBorderStyle.NONE); } @Test @@ -168,7 +171,10 @@ assertEquals(num, stylesTable.getBorders().size()); borderId = (int)cellStyle.getCoreXf().getBorderId(); ctBorder = stylesTable.getBorderAt(borderId).getCTBorder(); - assertFalse(ctBorder.isSetRight()); + //none is not the same as "not set", therefore the following doesn't work any more + //assertFalse(ctBorder.isSetRight()); + //replacement: + assertEquals(ctBorder.getRight().getStyle(), STBorderStyle.NONE); } @Test @@ -203,7 +209,10 @@ assertEquals(num, stylesTable.getBorders().size()); borderId = (int)cellStyle.getCoreXf().getBorderId(); ctBorder = stylesTable.getBorderAt(borderId).getCTBorder(); - assertFalse(ctBorder.isSetLeft()); + //none is not the same as "not set", therefore the following doesn't work any more + //assertFalse(ctBorder.isSetLeft()); + //replacement: + assertEquals(ctBorder.getLeft().getStyle(), STBorderStyle.NONE); } @Test @@ -238,7 +247,10 @@ assertEquals(num, stylesTable.getBorders().size()); borderId = (int)cellStyle.getCoreXf().getBorderId(); ctBorder = stylesTable.getBorderAt(borderId).getCTBorder(); - assertFalse(ctBorder.isSetTop()); + //none is not the same as "not set", therefore the following doesn't work any more + //assertFalse(ctBorder.isSetTop()); + //replacement: + assertEquals(ctBorder.getTop().getStyle(), STBorderStyle.NONE); } private void testGetSetBorderXMLBean(BorderStyle border, STBorderStyle.Enum expected) { @@ -258,10 +270,14 @@ cellStyle.setBorderTop(BorderStyle.NONE); assertEquals(BorderStyle.NONE, cellStyle.getBorderTopEnum()); int borderId = (int)cellStyle.getCoreXf().getBorderId(); - assertTrue(borderId > 0); + // The default Style is already "none" + // Therefore the new style already exists as Id=0 + //assertTrue(borderId > 0); + // replacement: + assertTrue(borderId == 0); //check changes in the underlying xml bean CTBorder ctBorder = stylesTable.getBorderAt(borderId).getCTBorder(); - assertNull(ctBorder.getTop()); + assertNotNull(ctBorder.getTop()); // no border style and STBorderStyle.NONE are equivalent // POI prefers to unset the border style than explicitly set it STBorderStyle.NONE } @@ -759,88 +775,96 @@ wb.close(); } - - /** - * Cloning one XSSFCellStyle onto Another, different XSSFWorkbooks - */ - @Test + + /** + * Cloning one XSSFCellStyle onto Another, different XSSFWorkbooks + */ + @Test public void testCloneStyleDiffWB() throws IOException { - XSSFWorkbook wbOrig = new XSSFWorkbook(); - assertEquals(1, wbOrig.getNumberOfFonts()); - assertEquals(0, wbOrig.getStylesSource().getNumberFormats().size()); - - XSSFFont fnt = wbOrig.createFont(); - fnt.setFontName("TestingFont"); - assertEquals(2, wbOrig.getNumberOfFonts()); - assertEquals(0, wbOrig.getStylesSource().getNumberFormats().size()); - - XSSFDataFormat fmt = wbOrig.createDataFormat(); - fmt.getFormat("MadeUpOne"); - fmt.getFormat("MadeUpTwo"); - - XSSFCellStyle orig = wbOrig.createCellStyle(); - orig.setAlignment(HSSFCellStyle.ALIGN_RIGHT); - orig.setFont(fnt); - orig.setDataFormat(fmt.getFormat("Test##")); - - assertTrue(XSSFCellStyle.ALIGN_RIGHT == orig.getAlignment()); - assertTrue(fnt == orig.getFont()); - assertTrue(fmt.getFormat("Test##") == orig.getDataFormat()); - - assertEquals(2, wbOrig.getNumberOfFonts()); - assertEquals(3, wbOrig.getStylesSource().getNumberFormats().size()); - - - // Now a style on another workbook - XSSFWorkbook wbClone = new XSSFWorkbook(); - assertEquals(1, wbClone.getNumberOfFonts()); - assertEquals(0, wbClone.getStylesSource().getNumberFormats().size()); - assertEquals(1, wbClone.getNumCellStyles()); - - XSSFDataFormat fmtClone = wbClone.createDataFormat(); - XSSFCellStyle clone = wbClone.createCellStyle(); - - assertEquals(1, wbClone.getNumberOfFonts()); - assertEquals(0, wbClone.getStylesSource().getNumberFormats().size()); - - assertFalse(HSSFCellStyle.ALIGN_RIGHT == clone.getAlignment()); - assertNotEquals("TestingFont", clone.getFont().getFontName()); - - clone.cloneStyleFrom(orig); - - assertEquals(2, wbClone.getNumberOfFonts()); - assertEquals(2, wbClone.getNumCellStyles()); - assertEquals(1, wbClone.getStylesSource().getNumberFormats().size()); - - assertEquals(HSSFCellStyle.ALIGN_RIGHT, clone.getAlignment()); - assertEquals("TestingFont", clone.getFont().getFontName()); - assertEquals(fmtClone.getFormat("Test##"), clone.getDataFormat()); - assertFalse(fmtClone.getFormat("Test##") == fmt.getFormat("Test##")); - - // Save it and re-check - XSSFWorkbook wbReload = XSSFTestDataSamples.writeOutAndReadBack(wbClone); - assertEquals(2, wbReload.getNumberOfFonts()); - assertEquals(2, wbReload.getNumCellStyles()); - assertEquals(1, wbReload.getStylesSource().getNumberFormats().size()); - - XSSFCellStyle reload = wbReload.getCellStyleAt((short)1); - assertEquals(HSSFCellStyle.ALIGN_RIGHT, reload.getAlignment()); - assertEquals("TestingFont", reload.getFont().getFontName()); - assertEquals(fmtClone.getFormat("Test##"), reload.getDataFormat()); - assertFalse(fmtClone.getFormat("Test##") == fmt.getFormat("Test##")); + XSSFWorkbook wbOrig = new XSSFWorkbook(); + assertEquals(1, wbOrig.getNumberOfFonts()); + assertEquals(0, wbOrig.getStylesSource().getNumberFormats().size()); + + XSSFFont fnt = wbOrig.createFont(); + fnt.setFontName("TestingFont"); + assertEquals(2, wbOrig.getNumberOfFonts()); + assertEquals(0, wbOrig.getStylesSource().getNumberFormats().size()); + + XSSFDataFormat fmt = wbOrig.createDataFormat(); + fmt.getFormat("MadeUpOne"); + fmt.getFormat("MadeUpTwo"); + + XSSFCellStyle orig = wbOrig.createCellStyle(); + orig.setAlignment(HSSFCellStyle.ALIGN_RIGHT); + orig.setFont(fnt); + orig.setDataFormat(fmt.getFormat("Test##")); + orig.setFillPattern(FillPatternType.SOLID_FOREGROUND); + orig.setFillForegroundColor(IndexedColors.BRIGHT_GREEN.getIndex()); + + XSSFCellStyle origEmpty = wbOrig.createCellStyle(); + + assertTrue(XSSFCellStyle.ALIGN_RIGHT == orig.getAlignment()); + assertTrue(fnt == orig.getFont()); + assertTrue(fmt.getFormat("Test##") == orig.getDataFormat()); + + assertEquals(2, wbOrig.getNumberOfFonts()); + assertEquals(3, wbOrig.getStylesSource().getNumberFormats().size()); + + + // Now a style on another workbook + XSSFWorkbook wbClone = new XSSFWorkbook(); + assertEquals(1, wbClone.getNumberOfFonts()); + assertEquals(0, wbClone.getStylesSource().getNumberFormats().size()); + assertEquals(1, wbClone.getNumCellStyles()); + + XSSFDataFormat fmtClone = wbClone.createDataFormat(); + XSSFCellStyle clone = wbClone.createCellStyle(); + + assertEquals(1, wbClone.getNumberOfFonts()); + assertEquals(0, wbClone.getStylesSource().getNumberFormats().size()); + + assertFalse(HSSFCellStyle.ALIGN_RIGHT == clone.getAlignment()); + assertNotEquals("TestingFont", clone.getFont().getFontName()); + + clone.cloneStyleFrom(orig); + + assertEquals(2, wbClone.getNumberOfFonts()); + assertEquals(2, wbClone.getNumCellStyles()); + assertEquals(1, wbClone.getStylesSource().getNumberFormats().size()); + + assertEquals(HSSFCellStyle.ALIGN_RIGHT, clone.getAlignment()); + assertEquals("TestingFont", clone.getFont().getFontName()); + assertEquals(fmtClone.getFormat("Test##"), clone.getDataFormat()); + assertFalse(fmtClone.getFormat("Test##") == fmt.getFormat("Test##")); + assertEquals(clone.getFillPatternEnum(), FillPatternType.SOLID_FOREGROUND); + assertEquals(clone.getFillForegroundColor(), IndexedColors.BRIGHT_GREEN.getIndex()); + + // Save it and re-check + XSSFWorkbook wbReload = XSSFTestDataSamples.writeOutAndReadBack(wbClone); + assertEquals(2, wbReload.getNumberOfFonts()); + assertEquals(2, wbReload.getNumCellStyles()); + assertEquals(1, wbReload.getStylesSource().getNumberFormats().size()); + + XSSFCellStyle reload = wbReload.getCellStyleAt((short)1); + assertEquals(HSSFCellStyle.ALIGN_RIGHT, reload.getAlignment()); + assertEquals("TestingFont", reload.getFont().getFontName()); + assertEquals(fmtClone.getFormat("Test##"), reload.getDataFormat()); + assertFalse(fmtClone.getFormat("Test##") == fmt.getFormat("Test##")); + assertEquals(clone.getFillPatternEnum(), FillPatternType.SOLID_FOREGROUND); + assertEquals(clone.getFillForegroundColor(), IndexedColors.BRIGHT_GREEN.getIndex()); - XSSFWorkbook wbOrig2 = XSSFTestDataSamples.writeOutAndReadBack(wbOrig); - assertNotNull(wbOrig2); - wbOrig2.close(); - - XSSFWorkbook wbClone2 = XSSFTestDataSamples.writeOutAndReadBack(wbClone); - assertNotNull(wbClone2); - wbClone2.close(); - - wbReload.close(); - wbClone.close(); - wbOrig.close(); - } + XSSFWorkbook wbOrig2 = XSSFTestDataSamples.writeOutAndReadBack(wbOrig); + assertNotNull(wbOrig2); + wbOrig2.close(); + + XSSFWorkbook wbClone2 = XSSFTestDataSamples.writeOutAndReadBack(wbClone); + assertNotNull(wbClone2); + wbClone2.close(); + + wbReload.close(); + wbClone.close(); + wbOrig.close(); + } /** * Avoid ArrayIndexOutOfBoundsException when creating cell style