Bugzilla 53763: avoid style mess when using HSSFOptimiser

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

5 files changed, 74 insertions, 11 deletions

diff --git a/patch.xml b/patch.xml
index 61b3cb7a95..2ad888675d 100755
--- a/patch.xml
+++ b/patch.xml
@@ -65,7 +65,6 @@
     <target name="patchpackage" depends="createpatch,newfiles">

        <delete file="${patch.package}"/> 

        <tar includes="${tar.file.list}"

-            excludes="${patch.file}"  

             destfile="${patch.package}"   

             basedir="." 

             compression="gzip" >          

diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml
index a94ba77bfa..a953da8f68 100644
--- a/src/documentation/content/xdocs/status.xml
+++ b/src/documentation/content/xdocs/status.xml
@@ -34,6 +34,7 @@
 
     <changes>
         <release version="3.9-beta1" date="2012-??-??">
+          <action dev="poi-developers" type="fix">53763 - avoid style mess when using HSSFOptimiser  </action>
           <action dev="poi-developers" type="fix">52972 - preserve leading / trailing spaces in SXSSF </action>
           <action dev="poi-developers" type="fix">53965 - Fixed XmlValueOutOfRangeExceptio calling getDataValidations for custom validations with XSSFSheet </action>
           <action dev="poi-developers" type="fix">53974 - Avoid NPE when constructing HSSFWorbook on Google App Engine</action>
diff --git a/src/java/org/apache/poi/hssf/model/InternalWorkbook.java b/src/java/org/apache/poi/hssf/model/InternalWorkbook.java
index 7161e48f14..88b9d63ef5 100644
--- a/src/java/org/apache/poi/hssf/model/InternalWorkbook.java
+++ b/src/java/org/apache/poi/hssf/model/InternalWorkbook.java
@@ -82,12 +82,16 @@ import org.apache.poi.hssf.record.WindowProtectRecord;
 import org.apache.poi.hssf.record.WriteAccessRecord;
 import org.apache.poi.hssf.record.WriteProtectRecord;
 import org.apache.poi.hssf.record.common.UnicodeString;
-import org.apache.poi.ss.formula.FormulaShifter;
-import org.apache.poi.ss.formula.udf.UDFFinder;
-import org.apache.poi.ss.formula.ptg.*;
 import org.apache.poi.hssf.util.HSSFColor;
 import org.apache.poi.ss.formula.EvaluationWorkbook.ExternalName;
 import org.apache.poi.ss.formula.EvaluationWorkbook.ExternalSheet;
+import org.apache.poi.ss.formula.FormulaShifter;
+import org.apache.poi.ss.formula.ptg.Area3DPtg;
+import org.apache.poi.ss.formula.ptg.NameXPtg;
+import org.apache.poi.ss.formula.ptg.OperandPtg;
+import org.apache.poi.ss.formula.ptg.Ptg;
+import org.apache.poi.ss.formula.ptg.Ref3DPtg;
+import org.apache.poi.ss.formula.udf.UDFFinder;
 import org.apache.poi.ss.usermodel.BuiltinFormats;
 import org.apache.poi.util.Internal;
 import org.apache.poi.util.POILogFactory;
@@ -845,6 +849,19 @@ public final class InternalWorkbook {
         records.remove(rec); // this updates XfPos for us
         numxfs--;
     }
+    
+    /**
+     * Removes ExtendedFormatRecord record with given index from the
+     *  file's list. This will make all
+     *  subsequent font indicies drop by one,
+     *  so you'll need to update those yourself!
+     *  @param index of the Extended format record (0-based)
+     */
+    public void removeExFormatRecord(int index) {
+        int xfptr = records.getXfpos() - (numxfs - 1) + index;
+        records.remove(xfptr); // this updates XfPos for us
+        numxfs--;
+    }
 
 
     /**
diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFOptimiser.java b/src/java/org/apache/poi/hssf/usermodel/HSSFOptimiser.java
index e8768dc2ac..7c0851a42e 100644
--- a/src/java/org/apache/poi/hssf/usermodel/HSSFOptimiser.java
+++ b/src/java/org/apache/poi/hssf/usermodel/HSSFOptimiser.java
@@ -252,13 +252,18 @@ public class HSSFOptimiser {
 		}
 		
 		// Zap the un-needed user style records
-		for(int i=21; i<newPos.length; i++) {
-			if(zapRecords[i]) {
-				workbook.getWorkbook().removeExFormatRecord(
-						xfrs[i]
-				);
-			}
-		}
+        // removing by index, because removing by object may delete
+        // styles we did not intend to (the ones that _were_ duplicated and not the duplicates)
+        int max = newPos.length;
+        int removed = 0; // to adjust index after deletion
+        for(int i=21; i<max; i++) {
+            if(zapRecords[i + removed]) {
+                workbook.getWorkbook().removeExFormatRecord(i);
+                i--;
+                max--;
+                removed++;
+            }
+        }
 		
 		// Finally, update the cells to point at their new extended format records
 		for(int sheetNum=0; sheetNum<workbook.getNumberOfSheets(); sheetNum++) {
diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFOptimiser.java b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFOptimiser.java
index 6dc5f98e5e..d230185f73 100644
--- a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFOptimiser.java
+++ b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFOptimiser.java
@@ -224,6 +224,7 @@ public final class TestHSSFOptimiser extends TestCase {
 		assertEquals(21, r.getCell(0).getCellValueRecord().getXFIndex());
 		// cs2 -> 22
 		assertEquals(22, r.getCell(1).getCellValueRecord().getXFIndex());
+		assertEquals(22, r.getCell(1).getCellStyle().getFont(wb).getFontHeight());
 		// cs3 = cs1 -> 21
 		assertEquals(21, r.getCell(2).getCellValueRecord().getXFIndex());
 		// cs4 --> 24 -> 23
@@ -269,4 +270,44 @@ public final class TestHSSFOptimiser extends TestCase {
       // csD -> cs1 -> 21
       assertEquals(21, r.getCell(8).getCellValueRecord().getXFIndex());
 	}
+
+	public void testOptimiseStylesCheckActualStyles() {
+	    HSSFWorkbook wb = new HSSFWorkbook();
+	    
+	    // Several styles
+	    assertEquals(21, wb.getNumCellStyles());
+	    
+	    HSSFCellStyle cs1 = wb.createCellStyle();
+	    cs1.setBorderBottom(HSSFCellStyle.BORDER_THICK);
+	    
+	    HSSFCellStyle cs2 = wb.createCellStyle();
+	    cs2.setBorderBottom(HSSFCellStyle.BORDER_DASH_DOT);
+	    
+	    HSSFCellStyle cs3 = wb.createCellStyle(); // = cs1
+        cs3.setBorderBottom(HSSFCellStyle.BORDER_THICK);
+	    
+	    assertEquals(24, wb.getNumCellStyles());
+	    
+	    // Use them
+	    HSSFSheet s = wb.createSheet();
+	    HSSFRow r = s.createRow(0);
+	    
+	    r.createCell(0).setCellStyle(cs1);
+	    r.createCell(1).setCellStyle(cs2);
+	    r.createCell(2).setCellStyle(cs3);
+	    
+	    assertEquals(21, r.getCell(0).getCellValueRecord().getXFIndex());
+	    assertEquals(22, r.getCell(1).getCellValueRecord().getXFIndex());
+	    assertEquals(23, r.getCell(2).getCellValueRecord().getXFIndex());
+	    
+	    // Optimise
+	    HSSFOptimiser.optimiseCellStyles(wb);
+	    
+	    // Check
+	    assertEquals(23, wb.getNumCellStyles());
+	    
+	    assertEquals(HSSFCellStyle.BORDER_THICK, r.getCell(0).getCellStyle().getBorderBottom());
+	    assertEquals(HSSFCellStyle.BORDER_DASH_DOT, r.getCell(1).getCellStyle().getBorderBottom());
+	    assertEquals(HSSFCellStyle.BORDER_THICK, r.getCell(2).getCellStyle().getBorderBottom());
+	}
 }