Hopefully fix bug #50846 - Improve how XSSFColor inherits from Themes, by pushing...

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

diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml
index b6944750258810fd018bb3bd13da7aff9446883e..1ef1f8335ee4846eaebafba9a66bb575d2ba2aff 100644 (file)
--- a/src/documentation/content/xdocs/status.xml
+++ b/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>

diff --git a/src/ooxml/java/org/apache/poi/xssf/model/StylesTable.java b/src/ooxml/java/org/apache/poi/xssf/model/StylesTable.java
index 6edb91c6ae8a545e394d64a960dc325f9acb5d92..31844afb8e97c2c1a6f63a514f551010872313fd 100644 (file)
--- a/src/ooxml/java/org/apache/poi/xssf/model/StylesTable.java
+++ b/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);

diff --git a/src/ooxml/java/org/apache/poi/xssf/model/ThemesTable.java b/src/ooxml/java/org/apache/poi/xssf/model/ThemesTable.java
index 5b8b2286914abf5cf35d43119d4694825d2bab03..562837157a93e8af91c4ce52b2adcdb3e47c6ef7 100644 (file)
--- a/src/ooxml/java/org/apache/poi/xssf/model/ThemesTable.java
+++ b/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
+    }
 }

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 93b4364d15a9afb9e93152b3dff632e705550153..55590a2b422e116e9e771e25064f9e39fa67672d 100644 (file)
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCellStyle.java
+++ b/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) {
-            extractColorFromTheme(fillBackgroundColor);
+        if (fillBackgroundColor != null && _theme != null) {
+            _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) {
-            extractColorFromTheme(fillForegroundColor);
+        if (fillForegroundColor != null && _theme != null) {
+            _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());
-    }
 }

diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFColor.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFColor.java
index af5928fbf667d6618c73ca824e1befec947ba4ad..46b2ef2361021982bb123b6a39cbc1a23b49175c 100644 (file)
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFColor.java
+++ b/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();
     }

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 54d584bc6a5d6510a98a63daf6346f29b0d04717..adc695b0956634cd25866991d7c2ed0ef14a9398 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
@@ -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
-     * given Styles Table
+     * Creates a new, empty Cell Border.
+     * 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());
     }
-
 }
\ No newline at end of file

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 3b17b9ac0d47452ab36a2f4eff63af38f94b7086..465d164c269a1ae4602d30f080c09825519ebb8a 100644 (file)
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java
+++ b/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

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 02b34ae85ef2dd4ba42414a94229aff710a9e1a2..83edc8f2f45a86e5849160e1dbd444264d15bf57 100644 (file)
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCellStyle.java
+++ b/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();

diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFColor.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFColor.java
index 17b1d0d54714432cb56d518f3fe43085bd382c39..60dfb4f97d87762aaf50a14ee01debe4b53e6ddc 100644 (file)
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFColor.java
+++ b/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 {

diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/extensions/TestXSSFBorder.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/extensions/TestXSSFBorder.java
index e7233c227e6b3d464703639ceaccd9958f8e45b6..46af27d59d440fac418409d8c1e08c0cf56579a9 100644 (file)
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/extensions/TestXSSFBorder.java
+++ b/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());