Bug 60845: Apply patch and adjust tests

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1849969 13f79535-47bb-0310-9956-ffa450edef68

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 3933215ae8c99e6c25bb9a3ff017b213bfa4679f..6deafc4bec0e76c3997dcc0643f1c7b6bef7e88a 100644 (file)
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFFont.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFFont.java
@@ -39,6 +39,8 @@ import org.openxmlformats.schemas.spreadsheetml.x2006.main.STFontScheme;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.STUnderlineValues;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.STVerticalAlignRun;
 
+import java.util.Objects;
+
 /**
  * Represents a font used in a workbook.
  *
@@ -542,10 +544,16 @@ public class XSSFFont implements Font {
      *  to the style table
      */
     public long registerTo(StylesTable styles) {
+        return registerTo(styles, true);
+    }
+
+    public long registerTo(StylesTable styles, boolean force) {
         this._themes = styles.getTheme();
-        this._index = styles.putFont(this, true);
+        this._index = styles.putFont(this, force);
         return this._index;
     }
+
+
     /**
      * Records the Themes Table that is associated with
      *  the current font, used when looking up theme
@@ -635,7 +643,22 @@ public class XSSFFont implements Font {
         if(!(o instanceof XSSFFont)) return false;
 
         XSSFFont cf = (XSSFFont)o;
-        return _ctFont.toString().equals(cf.getCTFont().toString());
+
+        // BUG 60845
+        return 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());
     }
 
 }

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 54afd1cd220262f561a6a03da67371472bb7b7d8..da67280f52d1c4222b982576969805c8582a7e89 100644 (file)
--- 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.IndexedColorMap;
@@ -196,6 +198,21 @@ public class XSSFCellBorder {
         if (!(o instanceof XSSFCellBorder)) return false;
 
         XSSFCellBorder cf = (XSSFCellBorder) o;
-        return border.toString().equals(cf.getCTBorder().toString());
+
+        // bug 60845
+        // Do not compare the representing strings but the properties
+        // Reason:
+        //   The strings are different if the XMLObject is a fragment (e.g. the ones from cloneStyle)
+        //   even if they are in fact representing the same style
+        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 e619065b2f9fce454841aee1072b8fe5aa558311..dd9dca7d162b2bf5acf04665ba88b64cade2e158 100644 (file)
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/extensions/XSSFCellFill.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/extensions/XSSFCellFill.java
@@ -22,6 +22,9 @@ import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTPatternFill;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.STPatternType;
 import org.apache.poi.xssf.usermodel.IndexedColorMap;
 import org.apache.poi.xssf.usermodel.XSSFColor;
+
+import java.util.Objects;
+
 import org.apache.poi.util.Internal;
 
 /**
@@ -173,6 +176,14 @@ public final class XSSFCellFill {
         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 the XMLObject is a fragment (e.g. the ones from cloneStyle)
+        //   even if they are in fact representing the same style
+        return Objects.equals(this.getFillBackgroundColor(), cf.getFillBackgroundColor())
+                && Objects.equals(this.getFillForegroundColor(), cf.getFillForegroundColor())
+                && Objects.equals(this.getPatternType(), cf.getPatternType());
     }
 }

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 cee1578278b6e7fe4a0de3b17ff35e7c55916462..9030d2eb18bc7066d390836ea98af3f5bd61a7ec 100644 (file)
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCellStyle.java
+++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCellStyle.java
@@ -21,6 +21,7 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
@@ -67,10 +68,10 @@ public class TestXSSFCellStyle {
                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, null);
@@ -133,7 +134,10 @@ public class TestXSSFCellStyle {
         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 +172,10 @@ public class TestXSSFCellStyle {
         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 +210,10 @@ public class TestXSSFCellStyle {
         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 +248,10 @@ public class TestXSSFCellStyle {
         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 +271,14 @@ public class TestXSSFCellStyle {
         cellStyle.setBorderTop(BorderStyle.NONE);
         assertEquals(BorderStyle.NONE, cellStyle.getBorderTop());
         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:
+        assertEquals(0, borderId);
         //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
     }
@@ -537,7 +554,6 @@ public class TestXSSFCellStyle {
         assertEquals(IndexedColors.AUTOMATIC.getIndex(), cellStyle.getFillBackgroundColor());
        }
 
-       @SuppressWarnings("deprecation")
     @Test
     public void testDefaultStyles() throws IOException {
 
@@ -571,7 +587,6 @@ public class TestXSSFCellStyle {
        }
 
     @Test
-    @SuppressWarnings("deprecation")
     public void testGetFillForegroundColor() throws IOException {
         XSSFWorkbook wb = new XSSFWorkbook();
         StylesTable styles = wb.getStylesSource();
@@ -580,7 +595,7 @@ public class TestXSSFCellStyle {
 
         XSSFCellStyle defaultStyle = wb.getCellStyleAt((short)0);
         assertEquals(IndexedColors.AUTOMATIC.getIndex(), defaultStyle.getFillForegroundColor());
-        assertEquals(null, defaultStyle.getFillForegroundXSSFColor());
+        assertNull(defaultStyle.getFillForegroundXSSFColor());
         assertEquals(FillPatternType.NO_FILL, defaultStyle.getFillPattern());
         assertEquals(FillPatternType.NO_FILL, defaultStyle.getFillPatternEnum());
 
@@ -636,8 +651,7 @@ public class TestXSSFCellStyle {
         assertEquals(FillPatternType.NO_FILL, cellStyle.getFillPattern());
         fillId = (int)cellStyle.getCoreXf().getFillId();
         ctFill2 = stylesTable.getFillAt(fillId).getCTFill();
-        assertFalse(ctFill2.getPatternFill().isSetPatternType());
-
+        assertNull(ctFill2.getPatternFill());
        }
 
        @Test
@@ -753,9 +767,9 @@ public class TestXSSFCellStyle {
       assertEquals(18, orig.getDataFormat());
       
       XSSFCellStyle clone = wb.createCellStyle();
-      assertFalse(HorizontalAlignment.RIGHT == clone.getAlignment());
-      assertFalse(fnt == clone.getFont());
-      assertFalse(18 == clone.getDataFormat());
+        assertNotSame(HorizontalAlignment.RIGHT, clone.getAlignment());
+        assertNotSame(fnt, clone.getFont());
+        assertNotEquals(18, clone.getDataFormat());
       
       clone.cloneStyleFrom(orig);
       assertEquals(HorizontalAlignment.RIGHT, clone.getAlignment());
@@ -770,87 +784,95 @@ public class TestXSSFCellStyle {
       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(HorizontalAlignment.RIGHT);
-       orig.setFont(fnt);
-       orig.setDataFormat(fmt.getFormat("Test##"));
-       
-       assertTrue(HorizontalAlignment.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(HorizontalAlignment.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(HorizontalAlignment.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(HorizontalAlignment.RIGHT, reload.getAlignment());
-       assertEquals("TestingFont", reload.getFont().getFontName());
-       assertEquals(fmtClone.getFormat("Test##"), reload.getDataFormat());
-       assertFalse(fmtClone.getFormat("Test##") == fmt.getFormat("Test##"));
-
-       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 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(HorizontalAlignment.RIGHT);
+        orig.setFont(fnt);
+        orig.setDataFormat(fmt.getFormat("Test##"));
+        orig.setFillPattern(FillPatternType.SOLID_FOREGROUND);
+        orig.setFillForegroundColor(IndexedColors.BRIGHT_GREEN.getIndex());
+
+        XSSFCellStyle origEmpty = wbOrig.createCellStyle();
+
+        assertSame(HorizontalAlignment.RIGHT, orig.getAlignment());
+        assertSame(fnt, orig.getFont());
+        assertEquals(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());
+
+        assertNotSame(HorizontalAlignment.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(HorizontalAlignment.RIGHT, clone.getAlignment());
+        assertEquals("TestingFont", clone.getFont().getFontName());
+        assertEquals(fmtClone.getFormat("Test##"), clone.getDataFormat());
+        assertNotEquals(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(HorizontalAlignment.RIGHT, reload.getAlignment());
+        assertEquals("TestingFont", reload.getFont().getFontName());
+        assertEquals(fmtClone.getFormat("Test##"), reload.getDataFormat());
+        assertNotEquals(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();
+    }
 
     /**
      * Avoid ArrayIndexOutOfBoundsException  when creating cell style
@@ -900,7 +922,7 @@ public class TestXSSFCellStyle {
        Row r = s.getRow(0);
        CellStyle cs = r.getCell(0).getCellStyle();
 
-       assertEquals(true, cs.getShrinkToFit());
+        assertTrue(cs.getShrinkToFit());
 
        // New file
        XSSFWorkbook wb2 = new XSSFWorkbook();
@@ -919,8 +941,8 @@ public class TestXSSFCellStyle {
        XSSFWorkbook wb3 = XSSFTestDataSamples.writeOutAndReadBack(wb2);
        s = wb3.getSheetAt(0);
        r = s.getRow(0);
-       assertEquals(false, r.getCell(0).getCellStyle().getShrinkToFit());
-       assertEquals(true,  r.getCell(1).getCellStyle().getShrinkToFit());
+        assertFalse(r.getCell(0).getCellStyle().getShrinkToFit());
+        assertTrue(r.getCell(1).getCellStyle().getShrinkToFit());
 
        XSSFWorkbook wb4 = XSSFTestDataSamples.writeOutAndReadBack(wb2);
        assertNotNull(wb4);