* Make on unit test not fail if Fonts are not available on the machine

* Refactor SheetUtil a bit to reduce code-duplication, adjust JavaDoc for -1 return values
* Add method to check if we can compute text-width
* Add unit-tests which verify some more of SheetUtil

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

src/java/org/apache/poi/ss/util/SheetUtil.java

@@ -93,10 +93,9 @@ public class SheetUtil {
      * @param defaultCharWidth the width of a single character
      * @param formatter formatter used to prepare the text to be measured
      * @param useMergedCells    whether to use merged cells
-     * @return  the width in pixels
+     * @return  the width in pixels or -1 if cell is empty
      */
     public static double getCellWidth(Cell cell, int defaultCharWidth, DataFormatter formatter, boolean useMergedCells) {
-
         Sheet sheet = cell.getSheet();
         Workbook wb = sheet.getWorkbook();
         Row row = cell.getRow();
@@ -123,9 +122,6 @@ public class SheetUtil {
 
         Font font = wb.getFontAt(style.getFontIndex());
 
-        AttributedString str;
-        TextLayout layout;
-
         double width = -1;
         if (cellType == Cell.CELL_TYPE_STRING) {
             RichTextString rt = cell.getRichStringCellValue();
@@ -133,30 +129,14 @@ public class SheetUtil {
             for (int i = 0; i < lines.length; i++) {
                 String txt = lines[i] + defaultChar;
 
-                str = new AttributedString(txt);
+                AttributedString str = new AttributedString(txt);
                 copyAttributes(font, str, 0, txt.length());
 
                 if (rt.numFormattingRuns() > 0) {
                     // TODO: support rich text fragments
                 }
 
-                layout = new TextLayout(str.getIterator(), fontRenderContext);
+                width = getCellWidth(defaultCharWidth, colspan, style, width, str);
-                if(style.getRotation() != 0){
-                    /*
-                     * Transform the text using a scale so that it's height is increased by a multiple of the leading,
-                     * and then rotate the text before computing the bounds. The scale results in some whitespace around
-                     * the unrotated top and bottom of the text that normally wouldn't be present if unscaled, but
-                     * is added by the standard Excel autosize.
-                     */
-                    AffineTransform trans = new AffineTransform();
-                    trans.concatenate(AffineTransform.getRotateInstance(style.getRotation()*2.0*Math.PI/360.0));
-                    trans.concatenate(
-                    AffineTransform.getScaleInstance(1, fontHeightMultiple)
-                    );
-                    width = Math.max(width, ((layout.getOutline(trans).getBounds().getWidth() / colspan) / defaultCharWidth) + cell.getCellStyle().getIndention());
-                } else {
-                    width = Math.max(width, ((layout.getBounds().getWidth() / colspan) / defaultCharWidth) + cell.getCellStyle().getIndention());
-                }
             }
         } else {
             String sval = null;
@@ -172,66 +152,49 @@ public class SheetUtil {
             }
             if(sval != null) {
                 String txt = sval + defaultChar;
-                str = new AttributedString(txt);
+                AttributedString str = new AttributedString(txt);
                 copyAttributes(font, str, 0, txt.length());
 
-                layout = new TextLayout(str.getIterator(), fontRenderContext);
+                width = getCellWidth(defaultCharWidth, colspan, style, width, str);
-                if(style.getRotation() != 0){
-                    /*
-                     * Transform the text using a scale so that it's height is increased by a multiple of the leading,
-                     * and then rotate the text before computing the bounds. The scale results in some whitespace around
-                     * the unrotated top and bottom of the text that normally wouldn't be present if unscaled, but
-                     * is added by the standard Excel autosize.
-                     */
-                    AffineTransform trans = new AffineTransform();
-                    trans.concatenate(AffineTransform.getRotateInstance(style.getRotation()*2.0*Math.PI/360.0));
-                    trans.concatenate(
-                    AffineTransform.getScaleInstance(1, fontHeightMultiple)
-                    );
-                    width = Math.max(width, ((layout.getOutline(trans).getBounds().getWidth() / colspan) / defaultCharWidth) + cell.getCellStyle().getIndention());
-                } else {
-                    width = Math.max(width, ((layout.getBounds().getWidth() / colspan) / defaultCharWidth) + cell.getCellStyle().getIndention());
-                }
             }
         }
         return width;
     }
 
+    private static double getCellWidth(int defaultCharWidth, int colspan,
+            CellStyle style, double width, AttributedString str) {
+        TextLayout layout = new TextLayout(str.getIterator(), fontRenderContext);
+        if(style.getRotation() != 0){
+            /*
+             * Transform the text using a scale so that it's height is increased by a multiple of the leading,
+             * and then rotate the text before computing the bounds. The scale results in some whitespace around
+             * the unrotated top and bottom of the text that normally wouldn't be present if unscaled, but
+             * is added by the standard Excel autosize.
+             */
+            AffineTransform trans = new AffineTransform();
+            trans.concatenate(AffineTransform.getRotateInstance(style.getRotation()*2.0*Math.PI/360.0));
+            trans.concatenate(
+            AffineTransform.getScaleInstance(1, fontHeightMultiple)
+            );
+            width = Math.max(width, ((layout.getOutline(trans).getBounds().getWidth() / colspan) / defaultCharWidth) + style.getIndention());
+        } else {
+            width = Math.max(width, ((layout.getBounds().getWidth() / colspan) / defaultCharWidth) + style.getIndention());
+        }
+        return width;
+    }
+
     /**
      * Compute width of a column and return the result
      *
      * @param sheet the sheet to calculate
      * @param column    0-based index of the column
      * @param useMergedCells    whether to use merged cells
-     * @return  the width in pixels
+     * @return  the width in pixels or -1 if all cells are empty
      */
-    public static double getColumnWidth(Sheet sheet, int column, boolean useMergedCells){
+    public static double getColumnWidth(Sheet sheet, int column, boolean useMergedCells) {
-        AttributedString str;
+        return getColumnWidth(sheet, column, useMergedCells, sheet.getFirstRowNum(), sheet.getLastRowNum());
-        TextLayout layout;
-
-        Workbook wb = sheet.getWorkbook();
-        DataFormatter formatter = new DataFormatter();
-        Font defaultFont = wb.getFontAt((short) 0);
-
-        str = new AttributedString(String.valueOf(defaultChar));
-        copyAttributes(defaultFont, str, 0, 1);
-        layout = new TextLayout(str.getIterator(), fontRenderContext);
-        int defaultCharWidth = (int)layout.getAdvance();
-
-        double width = -1;
-        for (Row row : sheet) {
-            Cell cell = row.getCell(column);
-
-            if (cell == null) {
-                continue;
-            }
-
-            double cellWidth = getCellWidth(cell, defaultCharWidth, formatter, useMergedCells);
-            width = Math.max(width, cellWidth);
-        }
-        return width;
     }
-
+    
     /**
      * Compute width of a column based on a subset of the rows and return the result
      *
@@ -240,19 +203,16 @@ public class SheetUtil {
      * @param useMergedCells    whether to use merged cells
      * @param firstRow  0-based index of the first row to consider (inclusive)
      * @param lastRow   0-based index of the last row to consider (inclusive)
-     * @return  the width in pixels
+     * @return  the width in pixels or -1 if cell is empty
      */
     public static double getColumnWidth(Sheet sheet, int column, boolean useMergedCells, int firstRow, int lastRow){
-        AttributedString str;
-        TextLayout layout;
-
         Workbook wb = sheet.getWorkbook();
         DataFormatter formatter = new DataFormatter();
         Font defaultFont = wb.getFontAt((short) 0);
 
-        str = new AttributedString(String.valueOf(defaultChar));
+        AttributedString str = new AttributedString(String.valueOf(defaultChar));
         copyAttributes(defaultFont, str, 0, 1);
-        layout = new TextLayout(str.getIterator(), fontRenderContext);
+        TextLayout layout = new TextLayout(str.getIterator(), fontRenderContext);
         int defaultCharWidth = (int)layout.getAdvance();
 
         double width = -1;
@@ -273,6 +233,30 @@ public class SheetUtil {
         return width;
     }
 
+    /**
+     * Check if the Fonts are installed correctly so that Java can compute the size of
+     * columns. 
+     * 
+     * If a Cell uses a Font which is not available on the operating system then Java may 
+     * fail to return useful Font metrics and thus lead to an auto-computed size of 0.
+     * 
+     *  This method allows to check if computing the sizes for a given Font will succeed or not.
+     *
+     * @param font The Font that is used in the Cell
+     * @return true if computing the size for this Font will succeed, false otherwise
+     */
+    public static boolean canComputeColumnWidht(Font font) {
+        AttributedString str = new AttributedString("1");
+        copyAttributes(font, str, 0, "1".length());
+
+        TextLayout layout = new TextLayout(str.getIterator(), fontRenderContext);
+        if(layout.getBounds().getWidth() > 0) {
+            return true;
+        }
+
+        return false;
+    }
+
     /**
      * Copy text attributes from the supplied Font to Java2D AttributedString
      */

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

@@ -17,6 +17,8 @@
 
 package org.apache.poi.xssf.usermodel;
 
+import java.io.IOException;
+
 import org.apache.poi.POIXMLException;
 import org.apache.poi.ss.usermodel.BaseTestFont;
 import org.apache.poi.ss.usermodel.Font;
@@ -25,6 +27,8 @@ import org.apache.poi.ss.usermodel.FontFamily;
 import org.apache.poi.ss.usermodel.FontScheme;
 import org.apache.poi.ss.usermodel.FontUnderline;
 import org.apache.poi.ss.usermodel.IndexedColors;
+import org.apache.poi.ss.usermodel.Workbook;
+import org.apache.poi.ss.util.SheetUtil;
 import org.apache.poi.xssf.XSSFITestDataProvider;
 import org.apache.poi.xssf.XSSFTestDataSamples;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTBooleanProperty;
@@ -88,6 +92,8 @@ public final class TestXSSFFont extends BaseTestFont{
       // And set with the Charset index
       xssfFont.setCharSet(FontCharset.ARABIC.getValue());
       assertEquals(FontCharset.ARABIC.getValue(), xssfFont.getCharSet());
+      xssfFont.setCharSet((byte)(FontCharset.ARABIC.getValue()));
+      assertEquals(FontCharset.ARABIC.getValue(), xssfFont.getCharSet());
       
       // This one isn't allowed
       assertEquals(null, FontCharset.valueOf(9999));
@@ -279,4 +285,23 @@ public final class TestXSSFFont extends BaseTestFont{
 		font.setTypeOffset(XSSFFont.SS_SUPER);
 		assertEquals(STVerticalAlignRun.SUPERSCRIPT,ctFont.getVertAlignArray(0).getVal());
 	}
+
+	// store test from TestSheetUtil here as it uses XSSF
+    public void testCanComputeWidthXSSF() throws IOException {
+        Workbook wb = new XSSFWorkbook();
+
+        // cannot check on result because on some machines we get back false here!
+        SheetUtil.canComputeColumnWidht(wb.getFontAt((short)0));
+
+        wb.close();        
+    }
+
+    // store test from TestSheetUtil here as it uses XSSF
+    public void testCanComputeWidthInvalidFont() throws IOException {
+        Font font = new XSSFFont(CTFont.Factory.newInstance());
+        font.setFontName("some non existing font name");
+        
+        // Even with invalid fonts we still get back useful data most of the time... 
+        SheetUtil.canComputeColumnWidht(font);
+    }
 }

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

@@ -32,6 +32,8 @@ import org.apache.poi.hssf.util.PaneInformation;
 import org.apache.poi.ss.ITestDataProvider;
 import org.apache.poi.ss.SpreadsheetVersion;
 import org.apache.poi.ss.util.CellRangeAddress;
+import org.apache.poi.ss.util.SheetUtil;
+import org.junit.Assume;
 import org.junit.Ignore;
 import org.junit.Test;
 
@@ -336,7 +338,7 @@ public abstract class BaseTestBugzillaIssues {
     }
 
     @Test
-    public final void bug506819_testAutoSize() {
+    public final void bug50681_testAutoSize() {
         Workbook wb = _testDataProvider.createWorkbook();
         BaseTestSheetAutosizeColumn.fixFonts(wb);
         Sheet sheet = wb.createSheet("Sheet1");
@@ -353,6 +355,12 @@ public abstract class BaseTestBugzillaIssues {
         cell0.setCellValue(longValue);
 
         sheet.autoSizeColumn(0);
+        
+        // autoSize will fail if required fonts are not installed, skip this test then
+        Font font = wb.getFontAt(cell0.getCellStyle().getFontIndex());
+        Assume.assumeTrue("Cannot verify auoSizeColumn() because the necessary Fonts are not installed on this machine: " + font, 
+                SheetUtil.canComputeColumnWidht(font));
+        
         assertEquals(255*256, sheet.getColumnWidth(0)); // maximum column width is 255 characters
         sheet.setColumnWidth(0, sheet.getColumnWidth(0)); // Bug 506819 reports exception at this point
     }

src/testcases/org/apache/poi/ss/util/TestSheetUtil.java

@@ -17,13 +17,16 @@
 
 package org.apache.poi.ss.util;
 
-import junit.framework.TestCase;
+import java.io.IOException;
 
 import org.apache.poi.hssf.usermodel.HSSFWorkbook;
+import org.apache.poi.ss.usermodel.Cell;
 import org.apache.poi.ss.usermodel.Row;
 import org.apache.poi.ss.usermodel.Sheet;
 import org.apache.poi.ss.usermodel.Workbook;
 
+import junit.framework.TestCase;
+
 /**
  * Tests SheetUtil.
  *
@@ -61,5 +64,86 @@ public final class TestSheetUtil extends TestCase {
         assertEquals(21.0, SheetUtil.getCellWithMerges(s, 2, 2).getNumericCellValue());
         assertEquals(21.0, SheetUtil.getCellWithMerges(s, 2, 3).getNumericCellValue());
         assertEquals(21.0, SheetUtil.getCellWithMerges(s, 2, 4).getNumericCellValue());
+        
+        wb.close();
     }
-}
+    
+    public void testCanComputeWidthHSSF() throws IOException {
+        Workbook wb = new HSSFWorkbook();
+        
+        // cannot check on result because on some machines we get back false here!
+        SheetUtil.canComputeColumnWidht(wb.getFontAt((short)0));
+
+        wb.close();        
+    }
+
+    public void testGetCellWidthEmpty() throws IOException {
+        Workbook wb = new HSSFWorkbook();
+        Sheet sheet = wb.createSheet("sheet");
+        Row row = sheet.createRow(0);
+        Cell cell = row.createCell(0);
+        
+        // no contents: cell.setCellValue("sometext");
+        
+        assertEquals(-1.0, SheetUtil.getCellWidth(cell, 1, null, true));
+        
+        wb.close();
+    }
+
+    public void testGetCellWidthString() throws IOException {
+        Workbook wb = new HSSFWorkbook();
+        Sheet sheet = wb.createSheet("sheet");
+        Row row = sheet.createRow(0);
+        Cell cell = row.createCell(0);
+        
+        cell.setCellValue("sometext");
+        
+        assertTrue(SheetUtil.getCellWidth(cell, 1, null, true) > 0);
+        
+        wb.close();
+    }
+
+    public void testGetCellWidthNumber() throws IOException {
+        Workbook wb = new HSSFWorkbook();
+        Sheet sheet = wb.createSheet("sheet");
+        Row row = sheet.createRow(0);
+        Cell cell = row.createCell(0);
+        
+        cell.setCellValue(88.234);
+        
+        assertTrue(SheetUtil.getCellWidth(cell, 1, null, true) > 0);
+        
+        wb.close();
+    }
+
+    public void testGetCellWidthBoolean() throws IOException {
+        Workbook wb = new HSSFWorkbook();
+        Sheet sheet = wb.createSheet("sheet");
+        Row row = sheet.createRow(0);
+        Cell cell = row.createCell(0);
+        
+        cell.setCellValue(false);
+        
+        assertTrue(SheetUtil.getCellWidth(cell, 1, null, false) > 0);
+        
+        wb.close();
+    }
+
+    public void testGetColumnWidthString() throws IOException {
+        Workbook wb = new HSSFWorkbook();
+        Sheet sheet = wb.createSheet("sheet");
+        Row row = sheet.createRow(0);
+        sheet.createRow(1);
+        sheet.createRow(2);
+        Cell cell = row.createCell(0);
+        
+        cell.setCellValue("sometext");
+        
+        assertTrue("Having some width for rows with actual cells", 
+                SheetUtil.getColumnWidth(sheet, 0, true) > 0);
+        assertEquals("Not having any widht for rows with all empty cells", 
+                -1.0, SheetUtil.getColumnWidth(sheet, 0, true, 1, 2));
+        
+        wb.close();
+    }
+}