Bug 56295: Fix cloning of styles across workbooks and handling of default value of attribute applyFill

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

4 files changed, 90 insertions, 18 deletions

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 d1f0759078..760026be11 100644
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCellStyle.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCellStyle.java
@@ -154,6 +154,13 @@ public class XSSFCellStyle implements CellStyle {
                   _cellXf = CTXf.Factory.parse(
                         src.getCoreXf().toString()
                   );
+                  
+                  // bug 56295: ensure that the fills is available and set correctly
+                  CTFill fill = CTFill.Factory.parse(
+                		  src.getCTFill().toString()
+                		  );
+                  addFill(fill);
+                  
                   // Swap it over
                   _stylesSource.replaceCellXfAt(_cellXfId, _cellXf);
                } catch(XmlException e) {
@@ -187,6 +194,13 @@ public class XSSFCellStyle implements CellStyle {
         }
     }
 
+	private void addFill(CTFill fill) {
+		int idx = _stylesSource.putFill(new XSSFCellFill(fill));
+
+		_cellXf.setFillId(idx);
+		_cellXf.setApplyFill(true);
+	}
+
     /**
      * Get the type of horizontal alignment for the cell
      *
@@ -444,7 +458,8 @@ public class XSSFCellStyle implements CellStyle {
      * @return XSSFColor - fill color or <code>null</code> if not set
      */
     public XSSFColor getFillBackgroundXSSFColor() {
-        if(!_cellXf.getApplyFill()) return null;
+    	// bug 56295: handle missing applyFill attribute as "true" because Excel does as well
+        if(_cellXf.isSetApplyFill() && !_cellXf.getApplyFill()) return null;
 
         int fillIndex = (int)_cellXf.getFillId();
         XSSFCellFill fg = _stylesSource.getFillAt(fillIndex);
@@ -480,7 +495,8 @@ public class XSSFCellStyle implements CellStyle {
      * @return XSSFColor - fill color or <code>null</code> if not set
      */
     public XSSFColor getFillForegroundXSSFColor() {
-        if(!_cellXf.getApplyFill()) return null;
+    	// bug 56295: handle missing applyFill attribute as "true" because Excel does as well
+        if(_cellXf.isSetApplyFill() && !_cellXf.getApplyFill()) return null;
 
         int fillIndex = (int)_cellXf.getFillId();
         XSSFCellFill fg = _stylesSource.getFillAt(fillIndex);
@@ -515,7 +531,8 @@ public class XSSFCellStyle implements CellStyle {
      * @see org.apache.poi.ss.usermodel.CellStyle#DIAMONDS
      */
     public short getFillPattern() {
-        if(!_cellXf.getApplyFill()) return 0;
+    	// bug 56295: handle missing applyFill attribute as "true" because Excel does as well
+        if(_cellXf.isSetApplyFill() && !_cellXf.getApplyFill()) return 0;
 
         int fillIndex = (int)_cellXf.getFillId();
         XSSFCellFill fill = _stylesSource.getFillAt(fillIndex);
@@ -996,10 +1013,7 @@ public class XSSFCellStyle implements CellStyle {
             ptrn.setBgColor(color.getCTColor());
         }
 
-        int idx = _stylesSource.putFill(new XSSFCellFill(ct));
-
-        _cellXf.setFillId(idx);
-        _cellXf.setApplyFill(true);
+        addFill(ct);
     }
 
     /**
@@ -1052,10 +1066,7 @@ public class XSSFCellStyle implements CellStyle {
             ptrn.setFgColor(color.getCTColor());
         }
 
-        int idx = _stylesSource.putFill(new XSSFCellFill(ct));
-
-        _cellXf.setFillId(idx);
-        _cellXf.setApplyFill(true);
+        addFill(ct);
     }
 
     /**
@@ -1076,7 +1087,8 @@ public class XSSFCellStyle implements CellStyle {
      */
     private CTFill getCTFill(){
         CTFill ct;
-        if(_cellXf.getApplyFill()) {
+    	// bug 56295: handle missing applyFill attribute as "true" because Excel does as well
+        if(!_cellXf.isSetApplyFill() || _cellXf.getApplyFill()) {
             int fillIndex = (int)_cellXf.getFillId();
             XSSFCellFill cf = _stylesSource.getFillAt(fillIndex);
 
@@ -1135,10 +1147,7 @@ public class XSSFCellStyle implements CellStyle {
         if(fp == NO_FILL && ptrn.isSetPatternType()) ptrn.unsetPatternType();
         else ptrn.setPatternType(STPatternType.Enum.forInt(fp + 1));
 
-        int idx = _stylesSource.putFill(new XSSFCellFill(ct));
-
-        _cellXf.setFillId(idx);
-        _cellXf.setApplyFill(true);
+        addFill(ct);
     }
 
     /**
diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java
index bb9efa4073..5caf72988f 100644
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java
+++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java
@@ -31,6 +31,7 @@ import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.OutputStream;
 import java.util.Calendar;
 import java.util.List;
@@ -2308,4 +2309,60 @@ public final class TestXSSFBugs extends BaseTestBugzillaIssues {
         // TODO Re-check sheet contents
         // TODO Re-check formula evaluation
     }
+
+    @Test
+    public void testBug56295_MergeXlslsWithStyles() throws IOException {
+        XSSFWorkbook xlsToAppendWorkbook = XSSFTestDataSamples.openSampleWorkbook("56295.xlsx");
+        XSSFSheet sheet = xlsToAppendWorkbook.getSheetAt(0);
+        XSSFRow srcRow = sheet.getRow(0);
+        XSSFCell oldCell = srcRow.getCell(0);
+        XSSFCellStyle cellStyle = oldCell.getCellStyle();
+        
+        checkStyle(cellStyle);
+        
+//        StylesTable table = xlsToAppendWorkbook.getStylesSource();
+//        List<XSSFCellFill> fills = table.getFills();
+//        System.out.println("Having " + fills.size() + " fills");
+//        for(XSSFCellFill fill : fills) {
+//        	System.out.println("Fill: " + fill.getFillBackgroundColor() + "/" + fill.getFillForegroundColor());
+//        }        
+        
+        XSSFWorkbook targetWorkbook = new XSSFWorkbook();
+        XSSFSheet newSheet = targetWorkbook.createSheet(sheet.getSheetName());
+        XSSFRow destRow = newSheet.createRow(0);
+        XSSFCell newCell = destRow.createCell(0);
+
+		//newCell.getCellStyle().cloneStyleFrom(cellStyle);
+        CellStyle newCellStyle = targetWorkbook.createCellStyle();
+        newCellStyle.cloneStyleFrom(cellStyle);
+        newCell.setCellStyle(newCellStyle);
+		checkStyle(newCell.getCellStyle());
+        newCell.setCellValue(oldCell.getStringCellValue());
+
+//        OutputStream os = new FileOutputStream("output.xlsm");
+//        try {
+//        	targetWorkbook.write(os);
+//        } finally {
+//        	os.close();
+//        }
+        
+        XSSFWorkbook wbBack = XSSFTestDataSamples.writeOutAndReadBack(targetWorkbook);
+        XSSFCellStyle styleBack = wbBack.getSheetAt(0).getRow(0).getCell(0).getCellStyle();
+        checkStyle(styleBack);
+    }
+
+	private void checkStyle(XSSFCellStyle cellStyle) {
+		assertNotNull(cellStyle);
+        assertEquals(0, cellStyle.getFillForegroundColor());
+        assertNotNull(cellStyle.getFillForegroundXSSFColor());
+        XSSFColor fgColor = cellStyle.getFillForegroundColorColor();
+		assertNotNull(fgColor);
+		assertEquals("FF00FFFF", fgColor.getARGBHex());
+
+        assertEquals(0, cellStyle.getFillBackgroundColor());
+        assertNotNull(cellStyle.getFillBackgroundXSSFColor());
+        XSSFColor bgColor = cellStyle.getFillBackgroundColorColor();
+		assertNotNull(bgColor);
+		assertEquals("FF00FFFF", fgColor.getARGBHex());
+	}
 }
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 8d427be552..4ec812d306 100644
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCellStyle.java
+++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCellStyle.java
@@ -17,6 +17,8 @@
 
 package org.apache.poi.xssf.usermodel;
 
+import java.io.IOException;
+
 import junit.framework.TestCase;
 
 import org.apache.poi.hssf.usermodel.HSSFCellStyle;
@@ -82,6 +84,9 @@ public class TestXSSFCellStyle extends TestCase {
 		stylesTable.putCellStyleXf(cellStyleXf);
 		stylesTable.putCellXf(cellXf);
 		cellStyle = new XSSFCellStyle(1, 1, stylesTable, null);
+
+		assertNotNull(stylesTable.getFillAt(1).getCTFill().getPatternFill());
+		assertEquals(STPatternType.INT_DARK_GRAY, stylesTable.getFillAt(1).getCTFill().getPatternFill().getPatternType().intValue());
 	}
 
 	public void testGetSetBorderBottom() {
@@ -551,7 +556,7 @@ public class TestXSSFCellStyle extends TestCase {
         assertEquals(IndexedColors.AUTOMATIC.getIndex(), cellStyle.getFillBackgroundColor());
 	}
 
-	public void testDefaultStyles() {
+	public void testDefaultStyles() throws IOException {
 
 		XSSFWorkbook wb1 = new XSSFWorkbook();
 
@@ -577,6 +582,7 @@ public class TestXSSFCellStyle extends TestCase {
         assertEquals(style2.getBorderLeft(), style1.getBorderLeft());
         assertEquals(style2.getBorderRight(), style1.getBorderRight());
         assertEquals(style2.getBorderTop(), style1.getBorderTop());
+        wb2.close();
 	}
 
 
@@ -618,7 +624,7 @@ public class TestXSSFCellStyle extends TestCase {
 
 	public void testGetFillPattern() {
 
-        assertEquals(CellStyle.NO_FILL, cellStyle.getFillPattern());
+        assertEquals(STPatternType.INT_DARK_GRAY-1, cellStyle.getFillPattern());
 
         int num = stylesTable.getFills().size();
         cellStyle.setFillPattern(CellStyle.SOLID_FOREGROUND);
diff --git a/test-data/spreadsheet/56295.xlsx b/test-data/spreadsheet/56295.xlsx
new file mode 100644
index 0000000000..d929d99a18
--- /dev/null
+++ b/test-data/spreadsheet/56295.xlsx