Hopefully fix bug #50846 - Improve how XSSFColor inherits from Themes, by pushing the logic out of XSSFCellStyle and into ThemesTable + make it easier to call

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

src/documentation/content/xdocs/status.xml

@@ -34,6 +34,7 @@
 
     <changes>
         <release version="3.8-beta2" date="2011-??-??">
+           <action dev="poi-developers" type="fix">50846 - Improve how XSSFColor inherits from Themes</action>
            <action dev="poi-developers" type="fix">50847 - XSSFFont now accepts the full range of Charsets from FontChartset</action>
            <action dev="poi-developers" type="fix">50786 - Speed up calls to HSSFColor.getIndexHash() by returning a cached, unmodifiable Map. HSSFColor.getModifiableIndexHash() provides access to the old (slow but modifiable) functionality</action>
            <action dev="poi-developers" type="fix">47100 - Change related formulas and named ranges when XSSFWorkbook.setSheetName is called</action>

src/ooxml/java/org/apache/poi/xssf/model/StylesTable.java

@@ -140,7 +140,7 @@ public class StylesTable extends POIXMLDocumentPart {
             CTBorders ctborders = styleSheet.getBorders();
             if(ctborders != null) {
                 for (CTBorder border : ctborders.getBorderArray()) {
-                    borders.add(new XSSFCellBorder(border));
+                    borders.add(new XSSFCellBorder(border, theme));
                 }
             }
 
@@ -433,7 +433,7 @@ public class StylesTable extends POIXMLDocumentPart {
 		fills.add(new XSSFCellFill(ctFill[1]));
 
 		CTBorder ctBorder = createDefaultBorder();
-		borders.add(new XSSFCellBorder(ctBorder));
+		borders.add(new XSSFCellBorder(ctBorder, theme));
 
 		CTXf styleXf = createDefaultXf();
 		styleXfs.add(styleXf);

src/ooxml/java/org/apache/poi/xssf/model/ThemesTable.java

@@ -68,4 +68,27 @@ public class ThemesTable extends POIXMLDocumentPart {
         }
         return null;
     }
+    
+    /**
+     * If the colour is based on a theme, then inherit 
+     *  information (currently just colours) from it as
+     *  required.
+     */
+    public void inheritFromThemeAsRequired(XSSFColor color) {
+       if(color == null) {
+          // Nothing for us to do
+          return;
+       }
+       if(! color.getCTColor().isSetTheme()) {
+          // No theme set, nothing to do
+          return;
+       }
+       
+       // Get the theme colour
+       XSSFColor themeColor = getThemeColor(color.getTheme());
+       // Set the raw colour, not the adjusted one
+       color.setRgb(themeColor.getCTColor().getRgb());
+       
+       // All done
+    }
 }

src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCellStyle.java

@@ -441,8 +441,8 @@ public class XSSFCellStyle implements CellStyle {
         XSSFCellFill fg = _stylesSource.getFillAt(fillIndex);
 
         XSSFColor fillBackgroundColor = fg.getFillBackgroundColor();
-        if (fillBackgroundColor != null && fillBackgroundColor.getCTColor().isSetTheme() && _theme != null) {
+        if (fillBackgroundColor != null && _theme != null) {
-            extractColorFromTheme(fillBackgroundColor);
+            _theme.inheritFromThemeAsRequired(fillBackgroundColor);
         }
         return fillBackgroundColor;
     }
@@ -477,8 +477,8 @@ public class XSSFCellStyle implements CellStyle {
         XSSFCellFill fg = _stylesSource.getFillAt(fillIndex);
 
         XSSFColor fillForegroundColor = fg.getFillForegroundColor();
-        if (fillForegroundColor != null && fillForegroundColor.getCTColor().isSetTheme() && _theme != null) {
+        if (fillForegroundColor != null && _theme != null) {
-            extractColorFromTheme(fillForegroundColor);
+            _theme.inheritFromThemeAsRequired(fillForegroundColor);
         }
         return fillForegroundColor;
     }
@@ -766,7 +766,7 @@ public class XSSFCellStyle implements CellStyle {
         if(border == BORDER_NONE) ct.unsetBottom();
         else pr.setStyle(STBorderStyle.Enum.forInt(border + 1));
 
-        int idx = _stylesSource.putBorder(new XSSFCellBorder(ct));
+        int idx = _stylesSource.putBorder(new XSSFCellBorder(ct, _theme));
 
         _cellXf.setBorderId(idx);
         _cellXf.setApplyBorder(true);
@@ -806,7 +806,7 @@ public class XSSFCellStyle implements CellStyle {
         if(border == BORDER_NONE) ct.unsetLeft();
         else pr.setStyle(STBorderStyle.Enum.forInt(border + 1));
 
-        int idx = _stylesSource.putBorder(new XSSFCellBorder(ct));
+        int idx = _stylesSource.putBorder(new XSSFCellBorder(ct, _theme));
 
         _cellXf.setBorderId(idx);
         _cellXf.setApplyBorder(true);
@@ -846,7 +846,7 @@ public class XSSFCellStyle implements CellStyle {
         if(border == BORDER_NONE) ct.unsetRight();
         else pr.setStyle(STBorderStyle.Enum.forInt(border + 1));
 
-        int idx = _stylesSource.putBorder(new XSSFCellBorder(ct));
+        int idx = _stylesSource.putBorder(new XSSFCellBorder(ct, _theme));
 
         _cellXf.setBorderId(idx);
         _cellXf.setApplyBorder(true);
@@ -886,7 +886,7 @@ public class XSSFCellStyle implements CellStyle {
         if(border == BORDER_NONE) ct.unsetTop();
         else pr.setStyle(STBorderStyle.Enum.forInt(border + 1));
 
-        int idx = _stylesSource.putBorder(new XSSFCellBorder(ct));
+        int idx = _stylesSource.putBorder(new XSSFCellBorder(ct, _theme));
 
         _cellXf.setBorderId(idx);
         _cellXf.setApplyBorder(true);
@@ -925,7 +925,7 @@ public class XSSFCellStyle implements CellStyle {
         if(color != null)  pr.setColor(color.getCTColor());
         else pr.unsetColor();
 
-        int idx = _stylesSource.putBorder(new XSSFCellBorder(ct));
+        int idx = _stylesSource.putBorder(new XSSFCellBorder(ct, _theme));
 
         _cellXf.setBorderId(idx);
         _cellXf.setApplyBorder(true);
@@ -1194,7 +1194,7 @@ public class XSSFCellStyle implements CellStyle {
         if(color != null)  pr.setColor(color.getCTColor());
         else pr.unsetColor();
 
-        int idx = _stylesSource.putBorder(new XSSFCellBorder(ct));
+        int idx = _stylesSource.putBorder(new XSSFCellBorder(ct, _theme));
 
         _cellXf.setBorderId(idx);
         _cellXf.setApplyBorder(true);
@@ -1234,7 +1234,7 @@ public class XSSFCellStyle implements CellStyle {
         if(color != null)  pr.setColor(color.getCTColor());
         else pr.unsetColor();
 
-        int idx = _stylesSource.putBorder(new XSSFCellBorder(ct));
+        int idx = _stylesSource.putBorder(new XSSFCellBorder(ct, _theme));
 
         _cellXf.setBorderId(idx);
         _cellXf.setApplyBorder(true);
@@ -1284,7 +1284,7 @@ public class XSSFCellStyle implements CellStyle {
         if(color != null)  pr.setColor(color.getCTColor());
         else pr.unsetColor();
 
-        int idx = _stylesSource.putBorder(new XSSFCellBorder(ct));
+        int idx = _stylesSource.putBorder(new XSSFCellBorder(ct, _theme));
 
         _cellXf.setBorderId(idx);
         _cellXf.setApplyBorder(true);
@@ -1445,14 +1445,4 @@ public class XSSFCellStyle implements CellStyle {
         int indexXf = _stylesSource.putCellXf(xf);
         return new XSSFCellStyle(indexXf-1, xfSize-1, _stylesSource, _theme);
     }
-
-    /**
-     * Extracts RGB form theme color.
-     * @param originalColor Color that refers to theme.
-     */
-    private void extractColorFromTheme(XSSFColor originalColor){
-        XSSFColor themeColor = _theme.getThemeColor(originalColor.getTheme());
-        // Set the raw colour, not the adjusted one
-        originalColor.setRgb(themeColor.getCTColor().getRgb());
-    }
 }

src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFColor.java

@@ -16,10 +16,10 @@
 ==================================================================== */
 package org.apache.poi.xssf.usermodel;
 
-import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTColor;
 import org.apache.poi.hssf.util.HSSFColor;
 import org.apache.poi.ss.usermodel.Color;
 import org.apache.poi.util.Internal;
+import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTColor;
 
 /**
  * Represents a color in SpreadsheetML
@@ -329,7 +329,7 @@ public class XSSFColor implements Color {
     public CTColor getCTColor(){
         return ctColor;
     }
-
+    
     public int hashCode(){
         return ctColor.toString().hashCode();
     }

src/ooxml/java/org/apache/poi/xssf/usermodel/extensions/XSSFCellBorder.java

@@ -18,6 +18,7 @@ package org.apache.poi.xssf.usermodel.extensions;
 
 
 import org.apache.poi.ss.usermodel.BorderStyle;
+import org.apache.poi.xssf.model.ThemesTable;
 import org.apache.poi.xssf.usermodel.XSSFColor;
 import org.apache.poi.util.Internal;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTBorder;
@@ -30,22 +31,24 @@ import org.openxmlformats.schemas.spreadsheetml.x2006.main.STBorderStyle;
  * Color is optional.
  */
 public class XSSFCellBorder {
-
+    private ThemesTable _theme;
     private CTBorder border;
 
     /**
      * Creates a Cell Border from the supplied XML definition
      */
-    public XSSFCellBorder(CTBorder border) {
+    public XSSFCellBorder(CTBorder border, ThemesTable theme) {
         this.border = border;
+        this._theme = theme;
     }
 
     /**
-     * Creates a new, empty Cell Border, on the
+     * Creates a new, empty Cell Border.
-     * given Styles Table
+     * You need to attach this to the Styles Table
      */
-    public XSSFCellBorder() {
+    public XSSFCellBorder(ThemesTable theme) {
         border = CTBorder.Factory.newInstance();
+        this._theme = theme;
     }
 
     /**
@@ -97,8 +100,17 @@ public class XSSFCellBorder {
      */
     public XSSFColor getBorderColor(BorderSide side) {
         CTBorderPr borderPr = getBorder(side);
-        return borderPr != null && borderPr.isSetColor() ?
+        
-                new XSSFColor(borderPr.getColor()) : null;
+        if(borderPr != null && borderPr.isSetColor()) { 
+            XSSFColor clr = new XSSFColor(borderPr.getColor());
+            if(_theme != null) {
+               _theme.inheritFromThemeAsRequired(clr);
+            }
+            return clr;
+        } else {
+           // No border set
+           return null;
+        }
     }
 
     /**
@@ -155,5 +167,4 @@ public class XSSFCellBorder {
         XSSFCellBorder cf = (XSSFCellBorder) o;
         return border.toString().equals(cf.getCTBorder().toString());
     }
-
 }

src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java

@@ -725,6 +725,19 @@ public final class TestXSSFBugs extends BaseTestBugzillaIssues {
        assertEquals("FFCCFFCC", cs.getFillForegroundColorColor().getARGBHex());
     }
     
+    /**
+     * If the border colours are set with themes, then we 
+     *  should still be able to get colours
+     */
+    public void test50846() throws Exception {
+       // TODO Get file and test
+       //Workbook wb = XSSFTestDataSamples.openSampleWorkbook("50846.xlsx");
+       
+       // Check the style that is theme based
+       
+       // Check the one that isn't
+    }
+    
     /**
      * Fonts where their colours come from the theme rather
      *  then being set explicitly still should allow the

src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCellStyle.java

@@ -59,11 +59,11 @@ public class TestXSSFCellStyle extends TestCase {
 		ctStylesheet = stylesTable.getCTStylesheet();
 
 		ctBorderA = CTBorder.Factory.newInstance();
-		XSSFCellBorder borderA = new XSSFCellBorder(ctBorderA);
+		XSSFCellBorder borderA = new XSSFCellBorder(ctBorderA, null);
 		long borderId = stylesTable.putBorder(borderA);
 		assertEquals(1, borderId);
 
-		XSSFCellBorder borderB = new XSSFCellBorder();
+		XSSFCellBorder borderB = new XSSFCellBorder(null);
 		assertEquals(1, stylesTable.putBorder(borderB));
 
 		ctFill = CTFill.Factory.newInstance();

src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFColor.java

@@ -19,21 +19,7 @@ package org.apache.poi.xssf.usermodel;
 
 import junit.framework.TestCase;
 
-import org.apache.poi.ss.usermodel.*;
-import org.apache.poi.xssf.XSSFITestDataProvider;
 import org.apache.poi.xssf.XSSFTestDataSamples;
-import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTBooleanProperty;
-import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTColor;
-import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTFont;
-import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTFontName;
-import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTFontScheme;
-import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTFontSize;
-import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTIntProperty;
-import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTUnderlineProperty;
-import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTVerticalAlignFontProperty;
-import org.openxmlformats.schemas.spreadsheetml.x2006.main.STFontScheme;
-import org.openxmlformats.schemas.spreadsheetml.x2006.main.STUnderlineValues;
-import org.openxmlformats.schemas.spreadsheetml.x2006.main.STVerticalAlignRun;
 
 public final class TestXSSFColor extends TestCase {
    public void testIndexedColour() throws Exception {

src/ooxml/testcases/org/apache/poi/xssf/usermodel/extensions/TestXSSFBorder.java

@@ -40,7 +40,7 @@ public class TestXSSFBorder extends TestCase {
 		right.setStyle(STBorderStyle.NONE);
 		bottom.setStyle(STBorderStyle.THIN);
 		
-		XSSFCellBorder cellBorderStyle = new XSSFCellBorder(border);
+		XSSFCellBorder cellBorderStyle = new XSSFCellBorder(border, null);
 		assertEquals("DASH_DOT", cellBorderStyle.getBorderStyle(BorderSide.TOP).toString());
 		
 		assertEquals("NONE", cellBorderStyle.getBorderStyle(BorderSide.RIGHT).toString());