Fix bug #49928 for HSSF too - refactor HSSFDataFormat to allow overriding of built in formats, and tweak the format unit tests to do the same check for HSSF and XSSF. Also corrects some builtin formats for new HSSFWorkbooks which were slightly off

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

src/documentation/content/xdocs/status.xml

@@ -34,6 +34,7 @@
 
     <changes>
         <release version="3.8-beta1" date="2010-??-??">
+           <action dev="poi-developers" type="add">49928 - allow overridden built-in formats in HSSFCellStyle</action>
            <action dev="POI-DEVELOPERS" type="add">50607 - Added implementation for CLEAN(), CHAR() and ADDRESS()</action>
            <action dev="poi-developers" type="add">50587 - Improved documentation on user-defined functions</action>
            <action dev="poi-developers" type="add">Inside ExtractorFactory, support finding embedded OOXML documents and providing extractors for them</action>

src/java/org/apache/poi/hssf/model/InternalWorkbook.java

@@ -88,6 +88,7 @@ import org.apache.poi.ss.formula.ptg.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.usermodel.BuiltinFormats;
 import org.apache.poi.util.Internal;
 import org.apache.poi.util.POILogFactory;
 import org.apache.poi.util.POILogger;
@@ -1284,15 +1285,16 @@ public final class InternalWorkbook {
         // we'll need multiple editions for
         // the different formats
 
+        
         switch (id) {
-            case 0: return new FormatRecord(5, "\"$\"#,##0_);\\(\"$\"#,##0\\)");
-            case 1: return new FormatRecord(6, "\"$\"#,##0_);[Red]\\(\"$\"#,##0\\)");
-            case 2: return new FormatRecord(7, "\"$\"#,##0.00_);\\(\"$\"#,##0.00\\)");
-            case 3: return new FormatRecord(8, "\"$\"#,##0.00_);[Red]\\(\"$\"#,##0.00\\)");
-            case 4: return new FormatRecord(0x2a, "_(\"$\"* #,##0_);_(\"$\"* \\(#,##0\\);_(\"$\"* \"-\"_);_(@_)");
-            case 5: return new FormatRecord(0x29, "_(* #,##0_);_(* \\(#,##0\\);_(* \"-\"_);_(@_)");
-            case 6: return new FormatRecord(0x2c, "_(\"$\"* #,##0.00_);_(\"$\"* \\(#,##0.00\\);_(\"$\"* \"-\"??_);_(@_)");
-            case 7: return new FormatRecord(0x2b, "_(* #,##0.00_);_(* \\(#,##0.00\\);_(* \"-\"??_);_(@_)");
+            case 0: return new FormatRecord(5, BuiltinFormats.getBuiltinFormat(5)); 
+            case 1: return new FormatRecord(6, BuiltinFormats.getBuiltinFormat(6)); 
+            case 2: return new FormatRecord(7, BuiltinFormats.getBuiltinFormat(7)); 
+            case 3: return new FormatRecord(8, BuiltinFormats.getBuiltinFormat(8)); 
+            case 4: return new FormatRecord(0x2a, BuiltinFormats.getBuiltinFormat(0x2a)); 
+            case 5: return new FormatRecord(0x29, BuiltinFormats.getBuiltinFormat(0x29)); 
+            case 6: return new FormatRecord(0x2c, BuiltinFormats.getBuiltinFormat(0x2c)); 
+            case 7: return new FormatRecord(0x2b, BuiltinFormats.getBuiltinFormat(0x2b)); 
         }
         throw new  IllegalArgumentException("Unexpected id " + id);
     }

src/java/org/apache/poi/hssf/usermodel/HSSFDataFormat.java

@@ -71,9 +71,7 @@ public final class HSSFDataFormat implements DataFormat {
 		Iterator<FormatRecord> i = workbook.getFormats().iterator();
 		while (i.hasNext()) {
 			FormatRecord r = i.next();
-			if (_formats.size() < r.getIndexCode() + 1) {
-				_formats.setSize(r.getIndexCode() + 1);
-			}
+			ensureFormatsSize(r.getIndexCode());
 			_formats.set(r.getIndexCode(), r.getFormatString());
 		}
 	}
@@ -100,7 +98,7 @@ public final class HSSFDataFormat implements DataFormat {
 	 * @return index of format.
 	 */
 	public short getFormat(String pFormat) {
-
+	   // Normalise the format string
 		String format;
 		if (pFormat.toUpperCase().equals("TEXT")) {
 			format = "@";
@@ -108,31 +106,31 @@ public final class HSSFDataFormat implements DataFormat {
 			format = pFormat;
 		}
 
+		// Merge in the built in formats if we haven't already
 		if (!_movedBuiltins) {
 			for (int i=0; i<_builtinFormats.length; i++) {
-				if (_formats.size() < i + 1) {
-					_formats.setSize(i + 1);
+			   ensureFormatsSize(i);
+				if (_formats.get(i) == null) {
+				   _formats.set(i, _builtinFormats[i]);
+				} else {
+				   // The workbook overrides this default format
 				}
-				_formats.set(i, _builtinFormats[i]);
 			}
 			_movedBuiltins = true;
 		}
-		ListIterator<String> i;
-		i = _formats.listIterator();
-		int ind;
-		while (i.hasNext()) {
-			ind = i.nextIndex();
-			if (format.equals(i.next())) {
-				return (short) ind;
-			}
+		
+		// See if we can find it
+		for(int i=0; i<_formats.size(); i++) {
+		   if(format.equals(_formats.get(i))) {
+		      return (short)i;
+		   }
 		}
 
-		ind = _workbook.getFormat(format, true);
-		if (_formats.size() <= ind)
-			_formats.setSize(ind + 1);
-		_formats.set(ind, format);
-
-		return (short) ind;
+		// We can't find it, so add it as a new one
+		short index = _workbook.getFormat(format, true);
+		ensureFormatsSize(index);
+		_formats.set(index, format);
+		return index;
 	}
 
 	/**
@@ -144,10 +142,19 @@ public final class HSSFDataFormat implements DataFormat {
 		if (_movedBuiltins) {
 			return _formats.get(index);
 		}
+		
+		String fmt = _formats.get(index);
 		if (_builtinFormats.length > index && _builtinFormats[index] != null) {
-			return _builtinFormats[index];
+		   // It's in the built in range
+		   if (fmt != null) {
+		      // It's been overriden, use that value
+		      return fmt;
+		   } else {
+		      // Standard built in format
+	         return _builtinFormats[index];
+		   }
 		}
-		return _formats.get(index);
+		return fmt;
 	}
 
 	/**
@@ -166,4 +173,14 @@ public final class HSSFDataFormat implements DataFormat {
 	public static int getNumberOfBuiltinBuiltinFormats() {
 		return _builtinFormats.length;
 	}
+	
+	/**
+	 * Ensures that the formats list can hold entries
+	 *  up to and including the entry with this index
+	 */
+	private void ensureFormatsSize(int index) {
+	   if(_formats.size() <= index) {
+	      _formats.setSize(index+1);
+	   }
+	}
 }

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

@@ -19,7 +19,7 @@ package org.apache.poi.xssf.usermodel;
 
 import org.apache.poi.ss.usermodel.BaseTestDataFormat;
 import org.apache.poi.ss.usermodel.BuiltinFormats;
-import org.apache.poi.ss.usermodel.DataFormatter;
+import org.apache.poi.ss.usermodel.DataFormat;
 import org.apache.poi.xssf.XSSFITestDataProvider;
 import org.apache.poi.xssf.XSSFTestDataSamples;
 
@@ -37,33 +37,16 @@ public final class TestXSSFDataFormat extends BaseTestDataFormat {
      */
     public void test49928(){
         XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("49928.xlsx");
-        DataFormatter df = new DataFormatter();
-
-        XSSFSheet sheet = wb.getSheetAt(0);
-        XSSFCell cell = sheet.getRow(0).getCell(0);
-        XSSFCellStyle style = cell.getCellStyle();
-
-        String poundFmt = "\"\u00a3\"#,##0;[Red]\\-\"\u00a3\"#,##0";
-        // not expected normally, id of a custom format should be gerater 
-        // than BuiltinFormats.FIRST_USER_DEFINED_FORMAT_INDEX
-        short  poundFmtIdx = 6;
-
-        assertEquals(poundFmt, style.getDataFormatString());
-        assertEquals(poundFmtIdx, style.getDataFormat());
-        assertEquals("\u00a31", df.formatCellValue(cell));
-
-
-        XSSFDataFormat dataFormat = wb.createDataFormat();
-        assertEquals(poundFmtIdx, dataFormat.getFormat(poundFmt));
-        assertEquals(poundFmt, dataFormat.getFormat(poundFmtIdx));
+        doTest49928Core(wb);
 
         // an attempt to register an existing format returns its index
+        int poundFmtIdx = wb.getSheetAt(0).getRow(0).getCell(0).getCellStyle().getDataFormat();
         assertEquals(poundFmtIdx, wb.getStylesSource().putNumberFormat(poundFmt));
 
         // now create a custom format with Pound (\u00a3)
+        DataFormat dataFormat = wb.createDataFormat();
         short customFmtIdx = dataFormat.getFormat("\u00a3##.00[Yellow]");
         assertTrue(customFmtIdx > BuiltinFormats.FIRST_USER_DEFINED_FORMAT_INDEX );
         assertEquals("\u00a3##.00[Yellow]", dataFormat.getFormat(customFmtIdx));
-
     }
 }

src/testcases/org/apache/poi/hssf/usermodel/TestHSSFDataFormat.java

@@ -18,7 +18,10 @@
 package org.apache.poi.hssf.usermodel;
 
 import org.apache.poi.hssf.HSSFITestDataProvider;
+import org.apache.poi.hssf.HSSFTestDataSamples;
 import org.apache.poi.ss.usermodel.BaseTestDataFormat;
+import org.apache.poi.ss.usermodel.BuiltinFormats;
+import org.apache.poi.ss.usermodel.DataFormat;
 
 /**
  * Tests for {@link HSSFDataFormat}
@@ -28,4 +31,22 @@ public final class TestHSSFDataFormat extends BaseTestDataFormat {
     public TestHSSFDataFormat() {
         super(HSSFITestDataProvider.instance);
     }
+
+    /**
+     * [Bug 49928] formatCellValue returns incorrect value for \u00a3 formatted cells
+     */
+    public void test49928(){
+        HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("49928.xls");
+        doTest49928Core(wb);
+
+        // an attempt to register an existing format returns its index
+        int poundFmtIdx = wb.getSheetAt(0).getRow(0).getCell(0).getCellStyle().getDataFormat();
+        assertEquals(poundFmtIdx, wb.createDataFormat().getFormat(poundFmt));
+
+        // now create a custom format with Pound (\u00a3)
+        DataFormat dataFormat = wb.createDataFormat();
+        short customFmtIdx = dataFormat.getFormat("\u00a3##.00[Yellow]");
+        assertTrue(customFmtIdx >= BuiltinFormats.FIRST_USER_DEFINED_FORMAT_INDEX );
+        assertEquals("\u00a3##.00[Yellow]", dataFormat.getFormat(customFmtIdx));
+    }
 }

src/testcases/org/apache/poi/ss/usermodel/BaseTestDataFormat.java

@@ -20,6 +20,12 @@ package org.apache.poi.ss.usermodel;
 import junit.framework.TestCase;
 
 import org.apache.poi.ss.ITestDataProvider;
+import org.apache.poi.xssf.XSSFTestDataSamples;
+import org.apache.poi.xssf.usermodel.XSSFCell;
+import org.apache.poi.xssf.usermodel.XSSFCellStyle;
+import org.apache.poi.xssf.usermodel.XSSFDataFormat;
+import org.apache.poi.xssf.usermodel.XSSFSheet;
+import org.apache.poi.xssf.usermodel.XSSFWorkbook;
 
 /**
  * Tests of implementation of {@link DataFormat}
@@ -60,4 +66,31 @@ public abstract class BaseTestDataFormat extends TestCase {
         //read and verify the string representation
         assertEquals(customFmt, df.getFormat((short)customIdx));
     }
+    
+    /**
+     * [Bug 49928] formatCellValue returns incorrect value for \u00a3 formatted cells
+     */
+    public abstract void test49928();
+    protected final String poundFmt = "\"\u00a3\"#,##0;[Red]\\-\"\u00a3\"#,##0";
+    public void doTest49928Core(Workbook wb){
+        DataFormatter df = new DataFormatter();
+
+        Sheet sheet = wb.getSheetAt(0);
+        Cell cell = sheet.getRow(0).getCell(0);
+        CellStyle style = cell.getCellStyle();
+
+        String poundFmt = "\"\u00a3\"#,##0;[Red]\\-\"\u00a3\"#,##0";
+        // not expected normally, id of a custom format should be greater 
+        // than BuiltinFormats.FIRST_USER_DEFINED_FORMAT_INDEX
+        short  poundFmtIdx = 6;
+
+        assertEquals(poundFmt, style.getDataFormatString());
+        assertEquals(poundFmtIdx, style.getDataFormat());
+        assertEquals("\u00a31", df.formatCellValue(cell));
+
+
+        DataFormat dataFormat = wb.createDataFormat();
+        assertEquals(poundFmtIdx, dataFormat.getFormat(poundFmt));
+        assertEquals(poundFmt, dataFormat.getFormat(poundFmtIdx));
+    }
 }