diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml index f9f29be39..c03b89e79 100644 --- a/src/documentation/content/xdocs/changes.xml +++ b/src/documentation/content/xdocs/changes.xml @@ -37,6 +37,7 @@ + 45338 - Fix HSSFWorkbook to give you the same HSSFFont every time, and then fix it to find newly added fonts 45336 - Fix HSSFColor.getTripletHash() 45334 - Fixed formula parser to handle dots in identifiers 45252 - Improvement for HWPF Range.replaceText() diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index d2c0907a8..f706bed1a 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + 45338 - Fix HSSFWorkbook to give you the same HSSFFont every time, and then fix it to find newly added fonts 45336 - Fix HSSFColor.getTripletHash() 45334 - Fixed formula parser to handle dots in identifiers 45252 - Improvement for HWPF Range.replaceText() diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java b/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java index 68409156d..5bb4a3433 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java +++ b/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; } diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java index 5fe3cf687..1f2911128 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java +++ b/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 + ) + ); + } } diff --git a/src/testcases/org/apache/poi/hssf/util/TestHSSFColor.java b/src/testcases/org/apache/poi/hssf/util/TestHSSFColor.java new file mode 100644 index 000000000..42571eb32 --- /dev/null +++ b/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() + ); + } +}