Fix bug #45338 - Fix HSSFWorkbook to give you the same HSSFFont every time, and then fix it to find newly added fonts

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

src/documentation/content/xdocs/changes.xml

@@ -37,6 +37,7 @@
 
 		<!-- Don't forget to update status.xml too! -->
         <release version="3.1.1-alpha1" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">45338 - Fix HSSFWorkbook to give you the same HSSFFont every time, and then fix it to find newly added fonts</action>
            <action dev="POI-DEVELOPERS" type="fix">45336 - Fix HSSFColor.getTripletHash()</action>
            <action dev="POI-DEVELOPERS" type="fix">45334 - Fixed formula parser to handle dots in identifiers</action>
            <action dev="POI-DEVELOPERS" type="fix">45252 - Improvement for HWPF Range.replaceText()</action>

src/documentation/content/xdocs/status.xml

@@ -34,6 +34,7 @@
 	<!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.1.1-alpha1" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">45338 - Fix HSSFWorkbook to give you the same HSSFFont every time, and then fix it to find newly added fonts</action>
            <action dev="POI-DEVELOPERS" type="fix">45336 - Fix HSSFColor.getTripletHash()</action>
            <action dev="POI-DEVELOPERS" type="fix">45334 - Fixed formula parser to handle dots in identifiers</action>
            <action dev="POI-DEVELOPERS" type="fix">45252 - Improvement for HWPF Range.replaceText()</action>

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

@@ -43,6 +43,7 @@ import java.io.InputStream;
 import java.io.OutputStream;
 import java.io.PrintWriter;
 import java.util.ArrayList;
+import java.util.Hashtable;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Stack;
@@ -89,6 +90,12 @@ public class HSSFWorkbook extends POIDocument
      */
 
     private ArrayList names;
+    
+    /**
+     * this holds the HSSFFont objects attached to this workbook.
+     * We only create these from the low level records as required.
+     */
+    private Hashtable fonts;
 
     /**
      * holds whether or not to preserve other nodes in the POIFS.  Used
@@ -1000,9 +1007,10 @@ public class HSSFWorkbook extends POIDocument
         if(fontindex == Short.MAX_VALUE){
             throw new IllegalArgumentException("Maximum number of fonts was exceeded");
         }
-        HSSFFont retval = new HSSFFont(fontindex, font);
-
-        return retval;
+        
+        // Ask getFontAt() to build it for us,
+        //  so it gets properly cached
+        return getFontAt(fontindex);
     }
 
     /**
@@ -1012,15 +1020,11 @@ public class HSSFWorkbook extends POIDocument
                              String name, boolean italic, boolean strikeout,
                              short typeOffset, byte underline)
     {
-//        System.out.println( boldWeight + ", " + color + ", " + fontHeight + ", " + name + ", " + italic + ", " + strikeout + ", " + typeOffset + ", " + underline );
-        for (short i = 0; i < workbook.getNumberOfFontRecords(); i++)
-        {
-            if (i == 4)
-                continue;
-
-            FontRecord font = workbook.getFontRecordAt(i);
-            HSSFFont hssfFont = new HSSFFont(i, font);
-//            System.out.println( hssfFont.getBoldweight() + ", " + hssfFont.getColor() + ", " + hssfFont.getFontHeight() + ", " + hssfFont.getFontName() + ", " + hssfFont.getItalic() + ", " + hssfFont.getStrikeout() + ", " + hssfFont.getTypeOffset() + ", " + hssfFont.getUnderline() );
+    	for (short i=0; i<=getNumberOfFonts(); i++) {
+    		// Remember - there is no 4!
+    		if(i == 4) continue;
+    		
+    		HSSFFont hssfFont = getFontAt(i);
             if (hssfFont.getBoldweight() == boldWeight
                     && hssfFont.getColor() == color
                     && hssfFont.getFontHeight() == fontHeight
@@ -1030,12 +1034,10 @@ public class HSSFWorkbook extends POIDocument
                     && hssfFont.getTypeOffset() == typeOffset
                     && hssfFont.getUnderline() == underline)
             {
-//                System.out.println( "Found font" );
                 return hssfFont;
             }
         }
 
-//        System.out.println( "No font found" );
         return null;
     }
 
@@ -1050,15 +1052,26 @@ public class HSSFWorkbook extends POIDocument
     }
 
     /**
-     * get the font at the given index number
+     * Get the font at the given index number
      * @param idx  index number
      * @return HSSFFont at the index
      */
 
     public HSSFFont getFontAt(short idx)
     {
+    	if(fonts == null) fonts = new Hashtable();
+    	
+    	// So we don't confuse users, give them back
+    	//  the same object every time, but create
+    	//  them lazily
+    	Short sIdx = Short.valueOf(idx);
+    	if(fonts.containsKey(sIdx)) {
+    		return (HSSFFont)fonts.get(sIdx);
+    	}
+    	
         FontRecord font = workbook.getFontRecordAt(idx);
         HSSFFont retval = new HSSFFont(idx, font);
+        fonts.put(sIdx, retval);
 
         return retval;
     }

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

@@ -1051,4 +1051,88 @@ public final class TestBugs extends TestCase {
         	assertTrue(nd.get(0) instanceof DeletedArea3DPtg);
         }
     }
+    
+    /**
+     * Test that fonts get added properly
+     */
+    public void test45338() throws Exception {
+    	HSSFWorkbook wb = new HSSFWorkbook();
+    	assertEquals(4, wb.getNumberOfFonts());
+    	
+    	HSSFSheet s = wb.createSheet();
+    	s.createRow(0);
+    	s.createRow(1);
+    	HSSFCell c1 = s.getRow(0).createCell((short)0);
+    	HSSFCell c2 = s.getRow(1).createCell((short)0);
+    	
+    	assertEquals(4, wb.getNumberOfFonts());
+    	
+    	HSSFFont f1 = wb.getFontAt((short)0);
+    	assertEquals(400, f1.getBoldweight());
+    	
+    	// Check that asking for the same font
+    	//  multiple times gives you the same thing.
+    	// Otherwise, our tests wouldn't work!
+    	assertEquals(
+    			wb.getFontAt((short)0),
+    			wb.getFontAt((short)0)
+    	);
+    	assertEquals(
+    			wb.getFontAt((short)2),
+    			wb.getFontAt((short)2)
+    	);
+    	assertTrue(
+    			wb.getFontAt((short)0)
+    			!=
+    			wb.getFontAt((short)2)
+    	);
+    	
+    	// Look for a new font we have
+    	//  yet to add
+    	assertNull(
+    		wb.findFont(
+    			(short)11, (short)123, (short)22, 
+    			"Thingy", false, true, (short)2, (byte)2
+    		)
+    	);
+    	
+    	HSSFFont nf = wb.createFont();
+    	assertEquals(5, wb.getNumberOfFonts());
+    	
+    	assertEquals(5, nf.getIndex());
+    	assertEquals(nf, wb.getFontAt((short)5));
+    	
+    	nf.setBoldweight((short)11);
+    	nf.setColor((short)123);
+    	nf.setFontHeight((short)22);
+    	nf.setFontName("Thingy");
+    	nf.setItalic(false);
+    	nf.setStrikeout(true);
+    	nf.setTypeOffset((short)2);
+    	nf.setUnderline((byte)2);
+    	
+    	assertEquals(5, wb.getNumberOfFonts());
+    	assertEquals(nf, wb.getFontAt((short)5));
+    	
+    	// Find it now
+    	assertNotNull(
+    		wb.findFont(
+    			(short)11, (short)123, (short)22, 
+    			"Thingy", false, true, (short)2, (byte)2
+    		)
+    	);
+    	assertEquals(
+    		5,
+    		wb.findFont(
+       			(short)11, (short)123, (short)22, 
+       			"Thingy", false, true, (short)2, (byte)2
+       		).getIndex()
+    	);
+    	assertEquals(nf,
+       		wb.findFont(
+       			(short)11, (short)123, (short)22, 
+       			"Thingy", false, true, (short)2, (byte)2
+       		)
+    	);
+    }
 }

src/testcases/org/apache/poi/hssf/util/TestHSSFColor.java

@@ -0,0 +1,52 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+        
+package org.apache.poi.hssf.util;
+
+import java.util.Hashtable;
+
+import junit.framework.TestCase;
+
+public final class TestHSSFColor extends TestCase {
+	public void testBasics() {
+		assertNotNull(HSSFColor.YELLOW.class);
+		assertTrue(HSSFColor.YELLOW.index > 0);
+		assertTrue(HSSFColor.YELLOW.index2 > 0);
+	}
+	
+	public void testContents() {
+		assertEquals(3, HSSFColor.YELLOW.triplet.length);
+		assertEquals(255, HSSFColor.YELLOW.triplet[0]);
+		assertEquals(255, HSSFColor.YELLOW.triplet[1]);
+		assertEquals(0, HSSFColor.YELLOW.triplet[2]);
+		
+		assertEquals("FFFF:FFFF:0", HSSFColor.YELLOW.hexString);
+	}
+	
+	public void testTrippletHash() {
+		Hashtable tripplets = HSSFColor.getTripletHash();
+		
+		assertEquals(
+				HSSFColor.MAROON.class,
+				tripplets.get(HSSFColor.MAROON.hexString).getClass()
+		);
+		assertEquals(
+				HSSFColor.YELLOW.class,
+				tripplets.get(HSSFColor.YELLOW.hexString).getClass()
+		);
+	}
+}