applied patches #45894 and #45892, also major cleanup of XSSFSheet and related classes

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

src/examples/src/org/apache/poi/xssf/usermodel/examples/CreateNewSpreadsheet.java

@@ -25,11 +25,11 @@ import org.apache.poi.xssf.usermodel.*;
 
 public class CreateNewSpreadsheet {
 	public static void main(String[] args) throws Exception {
-		Workbook wb = new XSSFWorkbook();
+		XSSFWorkbook wb = new XSSFWorkbook();
 		CreationHelper createHelper = wb.getCreationHelper();
 		
-		Sheet s1 = wb.createSheet("Sheet One");
-		Sheet s2 = wb.createSheet("2nd Sheet");
+		XSSFSheet s1 = wb.createSheet("Sheet One");
+		XSSFSheet s2 = wb.createSheet("Sheet One");
 		
 		// Create a few cells
 		s1.createRow(0);
@@ -46,24 +46,13 @@ public class CreateNewSpreadsheet {
 		
 		s2.getRow(2).createCell(1).setCellValue(createHelper.createRichTextString("Sheet 2"));
 
-/*
-		// Comment
-		Comment comment = ((XSSFSheet)s1).createComment();
-//		HSSFPatriarch patriach = (HSSFPatriarch)s1.createDrawingPatriarch();
-//		Comment comment = patriach.createComment(new HSSFClientAnchor(0, 0, 0, 0, (short)4, 2, (short) 6, 5));
-		
-		comment.setAuthor("Apache POI");
-		comment.setString(createHelper.createRichTextString("I am a comment"));
-		s1.getRow(0).getCell(0).setCellComment(comment);
-		
-		// Hyperlink
-		Hyperlink hyperlink = createHelper.createHyperlink(Hyperlink.LINK_URL);
-		hyperlink.setAddress("http://poi.apache.org/");
-		hyperlink.setLabel("Link to POI");
-		s1.getRow(1).createCell(1).setHyperlink(hyperlink);
-		s1.getRow(1).getCell(1).setCellValue(createHelper.createRichTextString("Link to POI"));
-*/
-		// Save
+        s1.groupRow(0, 3);
+
+        s1.getRow(1).setHeight(10.4);
+        //s1.setActiveCell("A2");
+        //s2.setSelected(true);
+
+        // Save
 		FileOutputStream fout = new FileOutputStream("NewFile.xlsx");
 		wb.write(fout);
 		fout.close();

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

@@ -659,8 +659,6 @@ public class HSSFWorkbook extends POIDocument implements org.apache.poi.ss.userm
 
 
     /**
-=======
->>>>>>> .merge-right.r696898
      * create an HSSFSheet for this HSSFWorkbook, adds it to the sheets and returns
      * the high level representation.  Use this to create new sheets.
      *

src/ooxml/interfaces-jdk15/org/apache/poi/ss/usermodel/Sheet.java

@@ -232,12 +232,6 @@ public interface Sheet extends Iterable<Row> {
 
     void setVerticallyCenter(boolean value);
 
-    /**
-     * Determine whether printed output for this sheet will be vertically centered.
-     */
-
-    boolean getVerticallyCenter(boolean value);
-
     /**
      * determines whether the output is horizontally centered on the page.
      * @param value true to horizontally center, false otherwise.
@@ -672,12 +666,6 @@ public interface Sheet extends Iterable<Row> {
      */
     void removeColumnBreak(short column);
 
-    /**
-     * Aggregates the drawing records and dumps the escher record hierarchy
-     * to the standard output.
-     */
-    void dumpDrawingRecords(boolean fat);
-
     /**
      * Creates the toplevel drawing patriarch.  This will have the effect of
      * removing any existing drawings on this sheet.

src/ooxml/interfaces-jdk15/org/apache/poi/ss/usermodel/Workbook.java

@@ -283,20 +283,6 @@ public interface Workbook {
 
     void write(OutputStream stream) throws IOException;
 
-    /**
-     * Method getBytes - get the bytes of just the HSSF portions of the XLS file.
-     * Use this to construct a POI POIFSFileSystem yourself.
-     *
-     *
-     * @return byte[] array containing the binary representation of this workbook and all contained
-     *         sheets, rows, cells, etc.
-     *
-     * @see org.apache.poi.hssf.model.Workbook
-     * @see org.apache.poi.hssf.model.Sheet
-     */
-
-    byte[] getBytes();
-
     /** gets the total number of named ranges in the workboko
      * @return number of named ranges
      */
@@ -400,11 +386,6 @@ public interface Workbook {
     /** Test only. Do not use */
     void insertChartRecord();
 
-    /**
-     * Spits out a list of all the drawing records in the workbook.
-     */
-    void dumpDrawingGroupRecords(boolean fat);
-
     /**
      * Adds a picture to the workbook.
      *

src/ooxml/java/org/apache/poi/xssf/model/CommentsTable.java

@@ -61,7 +61,7 @@ public class CommentsTable implements CommentsSource, XSSFModel {
         options.setUseDefaultNamespace();
         
         // Requests use of whitespace for easier reading
-        options.setSavePrettyPrint();
+        //options.setSavePrettyPrint();
         
         CommentsDocument doc = CommentsDocument.Factory.newInstance(options);
         doc.setComments(comments);

src/ooxml/java/org/apache/poi/xssf/model/Drawing.java

@@ -56,7 +56,7 @@ public class Drawing implements XSSFChildContainingModel {
         options.setSaveOuter();
         options.setUseDefaultNamespace();        
         // Requests use of whitespace for easier reading
-        options.setSavePrettyPrint();
+        //options.setSavePrettyPrint();
         drawing.save(out, options);
 	}
 	

src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java

@@ -297,10 +297,6 @@ public final class XSSFCell implements Cell {
 
 
     public void setCellFormula(String formula) {
-        if (this.cell.getT() != STCellType.STR)
-        {
-            this.cell.setT(STCellType.STR);
-        }
         CTCellFormula f =  CTCellFormula.Factory.newInstance();
         f.setStringValue(formula);
         this.cell.setF(f);

src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFRow.java

@@ -186,7 +186,7 @@ public class XSSFRow implements Row {
 
     public short getHeight() {
     	if (this.row.getHt() > 0) {
-    		return (short) (this.row.getHt() * 20);
+    		return (short) (this.row.getHt());
     	}
         return -1;
     }
@@ -198,12 +198,31 @@ public class XSSFRow implements Row {
         return -1;
     }
 
+    /**
+     * Gets the index of the last cell contained in this row <b>PLUS ONE</b>. The result also
+     * happens to be the 1-based column number of the last cell.  This value can be used as a
+     * standard upper bound when iterating over cells:
+     * <pre>
+     * short minColIx = row.getFirstCellNum();
+     * short maxColIx = row.getLastCellNum();
+     * for(short colIx=minColIx; colIx&lt;maxColIx; colIx++) {
+     *   XSSFCell cell = row.getCell(colIx);
+     *   if(cell == null) {
+     *     continue;
+     *   }
+     *   //... do something with cell
+     * }
+     * </pre>
+     *
+     * @return short representing the last logical cell in the row <b>PLUS ONE</b>, or -1 if the
+     *  row does not contain any cells.
+     */
     public short getLastCellNum() {
     	short lastCellNum = -1;
     	for (Iterator<Cell> it = cellIterator() ; it.hasNext() ; ) {
     		Cell cell = it.next();
     		if (cell != null) {
-    			lastCellNum = cell.getCellNum();
+    			lastCellNum = (short)(cell.getCellNum() + 1);
     		}
     	}
     	return lastCellNum;
@@ -241,11 +260,17 @@ public class XSSFRow implements Row {
     }
 
     public void setHeight(short height) {
-    	this.row.setHt((double) height / 20);
+    	this.row.setHt((double) height);
+    	this.row.setCustomHeight(true);
     }
 
-    public void setHeightInPoints(float height) {
+    public void setHeight(double height) {
     	this.row.setHt((double) height);
+    	this.row.setCustomHeight(true);
+    }
+
+    public void setHeightInPoints(float height) {
+	    setHeight((short)height);
     }
 
     public void setRowNum(int rowNum) {

src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java

@@ -19,10 +19,7 @@ package org.apache.poi.xssf.usermodel;
 
 import java.io.IOException;
 import java.io.OutputStream;
-import java.util.ArrayList;
-import java.util.Iterator;
-import java.util.LinkedList;
-import java.util.List;
+import java.util.*;
 
 import org.apache.poi.hssf.util.PaneInformation;
 import org.apache.poi.ss.usermodel.CellStyle;
@@ -80,6 +77,15 @@ import org.openxmlformats.schemas.spreadsheetml.x2006.main.STPane;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.STSheetState;
 
 
+/**
+ * High level representation of a worksheet.
+ *
+ * <p>
+ * Sheets are the central structures within a workbook, and are where a user does most of his spreadsheet work.
+ * The most common type of sheet is the worksheet, which is represented as a grid of cells. Worksheet cells can
+ * contain text, numbers, dates, and formulas. Cells can also be formatted.  A workbook usually contains more than one sheet.
+ * </p>
+ */
 public class XSSFSheet implements Sheet {
     protected CTSheet sheet;
     protected CTWorksheet worksheet;
@@ -139,12 +145,17 @@ public class XSSFSheet implements Sheet {
     	hyperlinks = new ArrayList<XSSFHyperlink>();
 	}
 
-    public XSSFSheet(XSSFWorkbook workbook) {
+    protected XSSFSheet(XSSFWorkbook workbook) {
         this.workbook = workbook;
 
         hyperlinks = new ArrayList<XSSFHyperlink>();
     }
 
+    /**
+     * Returns the parent XSSFWorkbook
+     *
+     * @return the parent XSSFWorkbook
+     */
     public XSSFWorkbook getWorkbook() {
         return this.workbook;
     }
@@ -154,7 +165,7 @@ public class XSSFSheet implements Sheet {
      *
      * @return a new instance
      */
-    protected static CTWorksheet newSheetInstance(){
+    protected static CTWorksheet newInstance(){
         CTWorksheet worksheet = CTWorksheet.Factory.newInstance();
         CTSheetFormatPr ctFormat = worksheet.addNewSheetFormatPr();
         ctFormat.setDefaultRowHeight(15.0);
@@ -214,8 +225,13 @@ public class XSSFSheet implements Sheet {
         worksheet.save(out, xmlOptions);
         out.close();
     }
-    
-    protected CTWorksheet getWorksheet() {
+
+    /**
+     * Provide access to the underlying XML bean
+     *
+     * @return the underlying CTWorksheet bean
+     */
+    public CTWorksheet getWorksheet() {
         return this.worksheet;
     }
     
@@ -265,8 +281,35 @@ public class XSSFSheet implements Sheet {
     	return ctMergeCells.sizeOfMergeCellArray();
     }
 
-    public void autoSizeColumn(short column) {
-    	columnHelper.setColBestFit(column, true);
+    /**
+     * Adjusts the column width to fit the contents.
+     *
+     * This process can be relatively slow on large sheets, so this should
+     *  normally only be called once per column, at the end of your
+     *  processing.
+     *
+     * @param column the column index
+     */
+   public void autoSizeColumn(short column) {
+    	autoSizeColumn(column, false);
+    }
+
+    /**
+     * Adjusts the column width to fit the contents.
+     *
+     * This process can be relatively slow on large sheets, so this should
+     *  normally only be called once per column, at the end of your
+     *  processing.
+     *
+     * You can specify whether the content of merged cells should be considered or ignored.
+     *  Default is to ignore merged cells.
+     *
+     * @param column the column index
+     * @param useMergedCells whether to use the contents of merged cells when calculating the width of the column
+     */
+    public void autoSizeColumn(short column, boolean useMergedCells) {
+        //TODO:
+        columnHelper.setColBestFit(column, true);
     }
 
     public Patriarch createDrawingPatriarch() {
@@ -274,11 +317,23 @@ public class XSSFSheet implements Sheet {
         return null;
     }
 
+    /**
+     * Creates a split (freezepane). Any existing freezepane or split pane is overwritten.
+     * @param colSplit      Horizonatal position of split.
+     * @param rowSplit      Vertical position of split.
+     * @param topRow        Top row visible in bottom pane
+     * @param leftmostColumn   Left column visible in right pane.
+     */
     public void createFreezePane(int colSplit, int rowSplit, int leftmostColumn, int topRow) {
     	this.createFreezePane(colSplit, rowSplit);
     	this.showInPane((short)topRow, (short)leftmostColumn);
     }
 
+    /**
+     * Creates a split (freezepane). Any existing freezepane or split pane is overwritten.
+     * @param colSplit      Horizonatal position of split.
+     * @param rowSplit      Vertical position of split.
+     */
     public void createFreezePane(int colSplit, int rowSplit) {
     	getPane().setXSplit(colSplit);
     	getPane().setYSplit(rowSplit);
@@ -290,8 +345,8 @@ public class XSSFSheet implements Sheet {
      * Creates a new comment for this sheet. You still
      *  need to assign it to a cell though
      */
-    public Comment createComment() {
-    	return getComments().addComment();
+    public XSSFComment createComment() {
+    	return (XSSFComment)getComments().addComment();
     }
 
     protected XSSFRow addRow(int index, int rownum) {
@@ -301,37 +356,52 @@ public class XSSFSheet implements Sheet {
         return xrow;
     }
 
+    /**
+     * Create a new row within the sheet and return the high level representation
+     *
+     * @param rownum  row number
+     * @return High level {@link XSSFRow} object representing a row in the sheet
+     * @see #removeRow(org.apache.poi.ss.usermodel.Row)
+     */
     public XSSFRow createRow(int rownum) {
         int index = 0;
         for (Row r : this.rows) {
-                if (r.getRowNum() == rownum) {
-                        // Replace r with new row
+            if (r.getRowNum() == rownum) {
+                // Replace r with new row
+                XSSFRow xrow = addRow(index, rownum);
+                rows.set(index, xrow);
+                return xrow;
+            }
+            if (r.getRowNum() > rownum) {
                 XSSFRow xrow = addRow(index, rownum);
-                        rows.set(index, xrow);
-                        return xrow;
-                }
-                if (r.getRowNum() > rownum) {
-                        XSSFRow xrow = addRow(index, rownum);
-                        rows.add(index, xrow);
-                        return xrow;
-                }
-                ++index;
+                rows.add(index, xrow);
+                return xrow;
+            }
+            ++index;
         }
         XSSFRow xrow = addRow(index, rownum);
         rows.add(xrow);
         return xrow;
     }
 
+    /**
+     * Creates a split pane. Any existing freezepane or split pane is overwritten.
+     * @param xSplitPos      Horizonatal position of split (in 1/20th of a point).
+     * @param ySplitPos      Vertical position of split (in 1/20th of a point).
+     * @param topRow        Top row visible in bottom pane
+     * @param leftmostColumn   Left column visible in right pane.
+     * @param activePane    Active pane.  One of: PANE_LOWER_RIGHT,
+     *                      PANE_UPPER_RIGHT, PANE_LOWER_LEFT, PANE_UPPER_LEFT
+     * @see #PANE_LOWER_LEFT
+     * @see #PANE_LOWER_RIGHT
+     * @see #PANE_UPPER_LEFT
+     * @see #PANE_UPPER_RIGHT
+     */
     public void createSplitPane(int xSplitPos, int ySplitPos, int leftmostColumn, int topRow, int activePane) {
     	createFreezePane(xSplitPos, ySplitPos, leftmostColumn, topRow);
     	getPane().setActivePane(STPane.Enum.forInt(activePane));
     }
 
-    public void dumpDrawingRecords(boolean fat) {
-        // TODO Auto-generated method stub
-
-    }
-
     public boolean getAlternateExpression() {
         // TODO Auto-generated method stub
         return false;
@@ -342,15 +412,11 @@ public class XSSFSheet implements Sheet {
         return false;
     }
 
-    public boolean getAutobreaks() {
-        return getSheetTypePageSetUpPr().getAutoPageBreaks();
-    }
-
-    public Comment getCellComment(int row, int column) {
-    	return getComments().findCellComment(row, column);
+    public XSSFComment getCellComment(int row, int column) {
+    	return (XSSFComment)getComments().findCellComment(row, column);
     }
     
-    public Hyperlink getHyperlink(int row, int column) {
+    public XSSFHyperlink getHyperlink(int row, int column) {
     	String ref = new CellReference(row, column).formatAsString();
     	for(XSSFHyperlink hyperlink : hyperlinks) {
     		if(hyperlink.getCellRef().equals(ref)) {
@@ -360,11 +426,18 @@ public class XSSFSheet implements Sheet {
     	return null;
     }
 
+    /**
+     * Vertical page break information used for print layout view, page layout view, drawing print breaks
+     * in normal view, and for printing the worksheet.
+     *
+     * @return column indexes of all the vertical page breaks, never <code>null</code>
+     */
     public int[] getColumnBreaks() {
-        CTBreak[] brkArray = getSheetTypeColumnBreaks().getBrkArray();
-        if (brkArray.length == 0) {
-            return null;
+        if (!worksheet.isSetColBreaks() || worksheet.getColBreaks().sizeOfBrkArray() == 0) {
+            return new int[0];
         }
+
+        CTBreak[] brkArray = worksheet.getColBreaks().getBrkArray();
         int[] breaks = new int[brkArray.length];
         for (int i = 0 ; i < brkArray.length ; i++) {
             CTBreak brk = brkArray[i];
@@ -418,6 +491,11 @@ public class XSSFSheet implements Sheet {
         return false;
     }
 
+    /**
+     * Gets the first row on the sheet
+     *
+     * @return the number of the first logical row on the sheet, zero based
+     */
     public int getFirstRowNum() {
         for (Iterator<Row> it = rowIterator() ; it.hasNext() ; ) {
             Row row = it.next();
@@ -428,18 +506,34 @@ public class XSSFSheet implements Sheet {
         return -1;
     }
 
+    /**
+     * Flag indicating whether the Fit to Page print option is enabled.
+     *
+     * @return <code>true</code>
+     */
     public boolean getFitToPage() {
-        return getSheetTypePageSetUpPr().getFitToPage();
+        CTSheetPr sheetPr = getSheetTypeSheetPr();
+        CTPageSetUpPr psSetup = (sheetPr == null || !sheetPr.isSetPageSetUpPr()) ?
+                CTPageSetUpPr.Factory.newInstance() : sheetPr.getPageSetUpPr();
+        return psSetup.getFitToPage();
+    }
+
+    protected CTSheetPr getSheetTypeSheetPr() {
+        if (worksheet.getSheetPr() == null) {
+            worksheet.setSheetPr(CTSheetPr.Factory.newInstance());
+        }
+        return worksheet.getSheetPr();
     }
 
-    
 	protected CTHeaderFooter getSheetTypeHeaderFooter() {
 		if (worksheet.getHeaderFooter() == null) {
 			worksheet.setHeaderFooter(CTHeaderFooter.Factory.newInstance());
 		}
 		return worksheet.getHeaderFooter();
 	}
-    
+
+
+
     /**
      * Returns the default footer for the sheet,
      *  creating one as needed.
@@ -617,49 +711,133 @@ public class XSSFSheet implements Sheet {
         return false;
     }
 
+    /**
+     * Returns the logical row ( 0-based).  If you ask for a row that is not
+     * defined you get a null.  This is to say row 4 represents the fifth row on a sheet.
+     *
+     * @param rownum  row to get
+     * @return <code>XSSFRow</code> representing the rownumber or <code>null</code> if its not defined on the sheet
+     */
     public XSSFRow getRow(int rownum) {
+        //TODO current implemenation is expensive, it should take O(1), not O(N)
         for (Iterator<Row> it = rowIterator() ; it.hasNext() ; ) {
-                Row row = it.next();
-                if (row.getRowNum() == rownum) {
-                        return (XSSFRow)row;
-                }
+            Row row = it.next();
+            if (row.getRowNum() == rownum) {
+                 return (XSSFRow)row;
+            }
         }
         return null;
     }
 
+    /**
+     * Horizontal page break information used for print layout view, page layout view, drawing print breaks in normal
+     *  view, and for printing the worksheet.
+     *
+     * @return row indexes of all the horizontal page breaks, never <code>null</code>
+     */
     public int[] getRowBreaks() {
-        CTPageBreak rowBreaks = getSheetTypeRowBreaks();
-        int breaksCount = rowBreaks.getBrkArray().length;
-        if (breaksCount == 0) {
-            return null;
+        if (!worksheet.isSetRowBreaks() || worksheet.getRowBreaks().sizeOfBrkArray() == 0) {
+            return new int[0];
         }
-        int[] breaks = new int[breaksCount];
-        for (int i = 0 ; i < breaksCount ; i++) {
-            CTBreak brk = rowBreaks.getBrkArray(i);
-            breaks[i] = (int) brk.getId();
+
+        CTBreak[] brkArray = worksheet.getRowBreaks().getBrkArray();
+        int[] breaks = new int[brkArray.length];
+        for (int i = 0 ; i < brkArray.length ; i++) {
+            CTBreak brk = brkArray[i];
+            breaks[i] = (int)brk.getId();
         }
         return breaks;
     }
 
-	protected CTPageBreak getSheetTypeRowBreaks() {
-		if (worksheet.getRowBreaks() == null) {
-			worksheet.setRowBreaks(CTPageBreak.Factory.newInstance());
-		}
-		return worksheet.getRowBreaks();
-	}
-
+    /**
+     * Flag indicating whether summary rows appear below detail in an outline, when applying an outline.
+     *
+     * <p>
+     * When true a summary row is inserted below the detailed data being summarized and a
+     * new outline level is established on that row.
+     * </p>
+     * <p>
+     * When false a summary row is inserted above the detailed data being summarized and a new outline level
+     * is established on that row.
+     * </p>
+     * @return <code>true</code> if row summaries appear below detail in the outline
+     */
     public boolean getRowSumsBelow() {
-        CTSheetPr sheetPr = getSheetTypeSheetPr();
-        CTOutlinePr outLinePr = sheetPr.getOutlinePr();
-        return outLinePr.getSummaryBelow();
+        CTSheetPr sheetPr = worksheet.getSheetPr();
+        CTOutlinePr outlinePr = (sheetPr != null && sheetPr.isSetOutlinePr())
+                ? sheetPr.getOutlinePr() : CTOutlinePr.Factory.newInstance();
+        return outlinePr.getSummaryBelow();
     }
 
+    /**
+     * Flag indicating whether summary rows appear below detail in an outline, when applying an outline.
+     *
+     * <p>
+     * When true a summary row is inserted below the detailed data being summarized and a
+     * new outline level is established on that row.
+     * </p>
+     * <p>
+     * When false a summary row is inserted above the detailed data being summarized and a new outline level
+     * is established on that row.
+     * </p>
+     * @param value <code>true</code> if row summaries appear below detail in the outline
+     */
+    public void setRowSumsBelow(boolean value) {
+        ensureOutlinePr().setSummaryBelow(value);
+    }
+
+    /**
+     * Flag indicating whether summary columns appear to the right of detail in an outline, when applying an outline.
+     *
+     * <p>
+     * When true a summary column is inserted to the right of the detailed data being summarized
+     * and a new outline level is established on that column.
+     * </p>
+     * <p>
+     * When false a summary column is inserted to the left of the detailed data being
+     * summarized and a new outline level is established on that column.
+     * </p>
+     * @return <code>true</code> if col summaries appear right of the detail in the outline
+     */
     public boolean getRowSumsRight() {
-        CTSheetPr sheetPr = getSheetTypeSheetPr();
-        CTOutlinePr outLinePr = sheetPr.getOutlinePr();
-        return outLinePr.getSummaryRight();
+        CTSheetPr sheetPr = worksheet.getSheetPr();
+        CTOutlinePr outlinePr = (sheetPr != null && sheetPr.isSetOutlinePr())
+                ? sheetPr.getOutlinePr() : CTOutlinePr.Factory.newInstance();
+        return outlinePr.getSummaryRight();
+    }
+
+    /**
+     * Flag indicating whether summary columns appear to the right of detail in an outline, when applying an outline.
+     *
+     * <p>
+     * When true a summary column is inserted to the right of the detailed data being summarized
+     * and a new outline level is established on that column.
+     * </p>
+     * <p>
+     * When false a summary column is inserted to the left of the detailed data being
+     * summarized and a new outline level is established on that column.
+     * </p>
+     * @param value <code>true</code> if col summaries appear right of the detail in the outline
+     */
+    public void setRowSumsRight(boolean value) {
+        ensureOutlinePr().setSummaryRight(value);
     }
 
+
+    /**
+     * Ensure CTWorksheet.CTSheetPr.CTOutlinePr
+     */
+    private CTOutlinePr ensureOutlinePr(){
+        CTSheetPr sheetPr = worksheet.isSetSheetPr() ? worksheet.getSheetPr() : worksheet.addNewSheetPr();
+        CTOutlinePr outlinePr = sheetPr.isSetOutlinePr() ? sheetPr.getOutlinePr() : sheetPr.addNewOutlinePr();
+        return outlinePr;
+    }
+
+    /**
+     * A flag indicating whether scenarios are locked when the sheet is protected.
+     *
+     * @return true => protection enabled; false => protection disabled
+     */
     public boolean getScenarioProtect() {
     	return getSheetTypeProtection().getScenarios();
     }
@@ -671,19 +849,25 @@ public class XSSFSheet implements Sheet {
 		return worksheet.getSheetProtection();
 	}
 
+    /**
+     * The top row in the visible view when the sheet is
+     * first viewed after opening it in a viewer
+     *
+     * @return integer indicating the rownum (0 based) of the top row
+     */
     public short getTopRow() {
     	String cellRef = getSheetTypeSheetView().getTopLeftCell();
     	CellReference cellReference = new CellReference(cellRef);
         return (short) cellReference.getRow();
     }
 
-    // Right signature method. Remove the wrong one when it will be removed in HSSFSheet (and interface)
+    /**
+     * Determine whether printed output for this sheet will be vertically centered.
+     *
+     * @return whether printed output for this sheet will be vertically centered.
+     */
     public boolean getVerticallyCenter() {
-    	return getVerticallyCenter(true);
-    }
-
-    public boolean getVerticallyCenter(boolean value) {
-    	return getSheetTypePrintOptions().getVerticalCentered();
+        return getSheetTypePrintOptions().getVerticalCentered();
     }
 
     /**
@@ -711,9 +895,9 @@ public class XSSFSheet implements Sheet {
 
     public void groupRow(int fromRow, int toRow) {
       	for(int i=fromRow;i<=toRow;i++){
-    		XSSFRow xrow=(XSSFRow)getRow(i-1);
-    		if(xrow==null){//create a new Row
-    			 xrow=(XSSFRow)createRow(i-1);
+    		XSSFRow xrow = getRow(i-1);
+    		if(xrow == null){//create a new Row
+    			 xrow = createRow(i-1);
     		}    		
     		CTRow ctrow=xrow.getCTRow();
     		short outlineLevel=ctrow.getOutlineLevel();
@@ -742,10 +926,13 @@ public class XSSFSheet implements Sheet {
     	return outlineLevel;
     }    
 
+    /**
+     * Determines if there is a page break at the indicated column
+     */
     public boolean isColumnBroken(short column) {
-        CTBreak[] brkArray = getSheetTypeColumnBreaks().getBrkArray();
-        for (int i = 0 ; i < brkArray.length ; i++) {
-            if (brkArray[i].getId() == column) {
+        int[] colBreaks = getColumnBreaks();
+        for (int i = 0 ; i < colBreaks.length ; i++) {
+            if (colBreaks[i] == column) {
                 return true;
             }
         }
@@ -779,11 +966,14 @@ public class XSSFSheet implements Sheet {
     	return getSheetTypePrintOptions().getGridLines();
     }
 
+    /**
+     * Tests if there is a page break at the indicated row
+     *
+     * @param row index of the row to test
+     * @return <code>true</code> if there is a page break at the indicated row
+     */
     public boolean isRowBroken(int row) {
         int[] rowBreaks = getRowBreaks();
-        if (rowBreaks == null) {
-            return false;
-        }
         for (int i = 0 ; i < rowBreaks.length ; i++) {
             if (rowBreaks[i] == row) {
                 return true;
@@ -792,21 +982,34 @@ public class XSSFSheet implements Sheet {
         return false;
     }
 
-    public void protectSheet(String password) {
-        // TODO Auto-generated method stub
-
+    /**
+     * Sets a page break at the indicated row
+     */
+    public void setRowBreak(int row) {
+        CTPageBreak pgBreak = worksheet.isSetRowBreaks() ? worksheet.getRowBreaks() : worksheet.addNewRowBreaks();
+        if (! isRowBroken(row)) {
+            CTBreak brk = pgBreak.addNewBrk();
+            brk.setId(row);
+        }
     }
 
+    /**
+     * Removes a page break at the indicated column
+     */
     public void removeColumnBreak(short column) {
         CTBreak[] brkArray = getSheetTypeColumnBreaks().getBrkArray();
         for (int i = 0 ; i < brkArray.length ; i++) {
             if (brkArray[i].getId() == column) {
                 getSheetTypeColumnBreaks().removeBrk(i);
-                continue;
             }
         }
     }
 
+    public void protectSheet(String password) {
+        // TODO Auto-generated method stub
+
+    }
+
     public void removeMergedRegion(int index) {
     	CTMergeCell[] mergeCellsArray = new CTMergeCell[getMergedCells().sizeOfMergeCellArray() - 1];
     	for (int i = 0 ; i < getMergedCells().sizeOfMergeCellArray() ; i++) {
@@ -833,12 +1036,15 @@ public class XSSFSheet implements Sheet {
         }
     }
 
+    /**
+     * Removes the page break at the indicated row
+     */
     public void removeRowBreak(int row) {
-        CTBreak[] brkArray = getSheetTypeRowBreaks().getBrkArray();
+        CTPageBreak pgBreak = worksheet.isSetRowBreaks() ? worksheet.getRowBreaks() : worksheet.addNewRowBreaks();
+        CTBreak[] brkArray = pgBreak.getBrkArray();
         for (int i = 0 ; i < brkArray.length ; i++) {
             if (brkArray[i].getId() == row) {
-                getSheetTypeRowBreaks().removeBrk(i);
-                continue;
+                pgBreak.removeBrk(i);
             }
         }
     }
@@ -846,6 +1052,7 @@ public class XSSFSheet implements Sheet {
     public Iterator<Row> rowIterator() {
         return rows.iterator();
     }
+
     /**
      * Alias for {@link #rowIterator()} to
      *  allow foreach loops
@@ -864,8 +1071,27 @@ public class XSSFSheet implements Sheet {
 
     }
 
-    public void setAutobreaks(boolean b) {
-        getSheetTypePageSetUpPr().setAutoPageBreaks(b);
+    /**
+     * Flag indicating whether the sheet displays Automatic Page Breaks.
+     *
+     * @return <code>true</code> if the sheet displays Automatic Page Breaks.
+     */
+     public boolean getAutobreaks() {
+        CTSheetPr sheetPr = getSheetTypeSheetPr();
+        CTPageSetUpPr psSetup = (sheetPr == null || !sheetPr.isSetPageSetUpPr()) ?
+                CTPageSetUpPr.Factory.newInstance() : sheetPr.getPageSetUpPr();
+        return psSetup.getAutoPageBreaks();
+    }
+
+    /**
+     * Flag indicating whether the sheet displays Automatic Page Breaks.
+     *
+     * @param value <code>true</code> if the sheet displays Automatic Page Breaks.
+     */
+    public void setAutobreaks(boolean value) {
+        CTSheetPr sheetPr = getSheetTypeSheetPr();
+        CTPageSetUpPr psSetup = sheetPr.isSetPageSetUpPr() ? sheetPr.getPageSetUpPr() : sheetPr.addNewPageSetUpPr();
+        psSetup.setAutoPageBreaks(value);
     }
 
     public void setColumnBreak(short column) {
@@ -982,36 +1208,15 @@ public class XSSFSheet implements Sheet {
     	getSheetTypePrintOptions().setGridLines(newPrintGridlines);
     }
 
-    public void setRowBreak(int row) {
-        CTPageBreak pageBreak = getSheetTypeRowBreaks();
-        if (! isRowBroken(row)) {
-            CTBreak brk = pageBreak.addNewBrk();
-            brk.setId(row);
-        }
-    }
-
     public void setRowGroupCollapsed(int row, boolean collapse) {
         // TODO Auto-generated method stub
 
     }
 
-    public void setRowSumsBelow(boolean b) {
-        CTSheetPr sheetPr = getSheetTypeSheetPr();
-        CTOutlinePr outLinePr = sheetPr.getOutlinePr();
-        outLinePr.setSummaryBelow(b);
-    }
-
-    public void setRowSumsRight(boolean b) {
-        CTSheetPr sheetPr = getSheetTypeSheetPr();
-        CTOutlinePr outLinePr = sheetPr.getOutlinePr();
-        outLinePr.setSummaryRight(b);
-    }
-
     public void setVerticallyCenter(boolean value) {
     	getSheetTypePrintOptions().setVerticalCentered(value);
     }
 
-    // HSSFSheet compatibility methods. See also the following zoom related methods
     /**
      * Sets the zoom magnication for the sheet.  The zoom is expressed as a
      * fraction.  For example to express a zoom of 75% use 3 for the numerator
@@ -1019,25 +1224,98 @@ public class XSSFSheet implements Sheet {
      *
      * @param numerator     The numerator for the zoom magnification.
      * @param denominator   The denominator for the zoom magnification.
+     * @see #setZoom(int)
      */    
     public void setZoom(int numerator, int denominator) {
         Float result = new Float(numerator)/new Float(denominator)*100;
     	setZoom(result.intValue());
     }
 
-    public void setZoom(long scale) {
+    /**
+     * Window zoom magnification for current view representing percent values.
+     * Valid values range from 10 to 400. Horizontal & Vertical scale together.
+     *
+     * For example:
+     * <pre>
+     * 10 - 10%
+     * 20 - 20%
+     * …
+     * 100 - 100%
+     * …
+     * 400 - 400%
+     * </pre>
+     *
+     * Current view can be Normal, Page Layout, or Page Break Preview.
+     *
+     * @param scale window zoom magnification
+     */
+    public void setZoom(int scale) {
     	getSheetTypeSheetView().setZoomScale(scale);
     }
 
-    public void setZoomNormal(long scale) {
+    /**
+     * Zoom magnification to use when in normal view, representing percent values.
+     * Valid values range from 10 to 400. Horizontal & Vertical scale together.
+     *
+     * For example:
+     * <pre>
+     * 10 - 10%
+     * 20 - 20%
+     * …
+     * 100 - 100%
+     * …
+     * 400 - 400%
+     * </pre>
+     *
+     * Applies for worksheet sheet type only; zero implies the automatic setting.
+     *
+     * @param scale window zoom magnification
+     */
+    public void setZoomNormal(int scale) {
     	getSheetTypeSheetView().setZoomScaleNormal(scale);
     }
 
-    public void setZoomPageLayoutView(long scale) {
+    /**
+     * Zoom magnification to use when in page layout view, representing percent values.
+     * Valid values range from 10 to 400. Horizontal & Vertical scale together.
+     *
+     * For example:
+     * <pre>
+     * 10 - 10%
+     * 20 - 20%
+     * …
+     * 100 - 100%
+     * …
+     * 400 - 400%
+     * </pre>
+     *
+     * Applies for worksheet sheet type only; zero implies the automatic setting.
+     *
+     * @param scale
+     */
+    public void setZoomPageLayoutView(int scale) {
     	getSheetTypeSheetView().setZoomScalePageLayoutView(scale);
     }
 
-    public void setZoomSheetLayoutView(long scale) {
+    /**
+     * Zoom magnification to use when in page break preview, representing percent values.
+     * Valid values range from 10 to 400. Horizontal & Vertical scale together.
+     *
+     * For example:
+     * <pre>
+     * 10 - 10%
+     * 20 - 20%
+     * …
+     * 100 - 100%
+     * …
+     * 400 - 400%
+     * </pre>
+     *
+     * Applies for worksheet only; zero implies the automatic setting.
+     *
+     * @param scale
+     */
+    public void setZoomSheetLayoutView(int scale) {
     	getSheetTypeSheetView().setZoomScaleSheetLayoutView(scale);
     }
 
@@ -1063,13 +1341,20 @@ public class XSSFSheet implements Sheet {
         }
     }
 
+    /**
+     * Location of the top left visible cell Location of the top left visible cell in the bottom right
+     * pane (when in Left-to-Right mode).
+     *
+     * @param toprow the top row to show in desktop window pane
+     * @param leftcol the left column to show in desktop window pane
+     */
     public void showInPane(short toprow, short leftcol) {
     	CellReference cellReference = new CellReference(toprow, leftcol);
     	String cellRef = cellReference.formatAsString();
     	getSheetTypeSheetView().setTopLeftCell(cellRef);
     }
 
-public void ungroupColumn(short fromColumn, short toColumn) {
+    public void ungroupColumn(short fromColumn, short toColumn) {
     	CTCols cols=worksheet.getColsArray(0);
     	for(int index=fromColumn;index<=toColumn;index++){
     		CTCol col=columnHelper.getColumn(index, false);
@@ -1114,13 +1399,6 @@ public void ungroupColumn(short fromColumn, short toColumn) {
     	getSheetTypeSheetFormatPr().setOutlineLevelCol((short)(maxLevelCol));
     }
     
-    public void setSelected(boolean flag) {
-        CTSheetViews views = getSheetTypeSheetViews();
-        for (CTSheetView view : views.getSheetViewArray()) {
-            view.setTabSelected(flag);
-        }
-    }
-
 	protected CTSheetViews getSheetTypeSheetViews() {
 		if (worksheet.getSheetViews() == null) {
 			worksheet.setSheetViews(CTSheetViews.Factory.newInstance());
@@ -1128,30 +1406,76 @@ public void ungroupColumn(short fromColumn, short toColumn) {
 		}
 		return worksheet.getSheetViews();
 	}
-    
+
+    /**
+     * Returns a flag indicating whether this sheet is selected.
+     * <p>
+     * When only 1 sheet is selected and active, this value should be in synch with the activeTab value.
+     * In case of a conflict, the Start Part setting wins and sets the active sheet tab.
+     * </p>
+     * Note: multiple sheets can be selected, but only one sheet can be active at one time.
+     *
+     * @return <code>true</code> if this sheet is selected
+     */
     public boolean isSelected() {
         CTSheetView view = getDefaultSheetView();
         return view != null && view.getTabSelected();
     }
     
+    /**
+     * Sets a flag indicating whether this sheet is selected.
+     *
+     * <p>
+     * When only 1 sheet is selected and active, this value should be in synch with the activeTab value.
+     * In case of a conflict, the Start Part setting wins and sets the active sheet tab.
+     * </p>
+     * Note: multiple sheets can be selected, but only one sheet can be active at one time.
+     *
+     * @param value <code>true</code> if this sheet is selected
+     */
+    public void setSelected(boolean value) {
+        CTSheetViews views = getSheetTypeSheetViews();
+        for (CTSheetView view : views.getSheetViewArray()) {
+            view.setTabSelected(value);
+        }
+    }
+
+    /**
+     * Assign a cell comment to a cell region in this worksheet
+     *
+     * @param cellRef cell region
+     * @param comment the comment to assign
+     */
     public void setCellComment(String cellRef, XSSFComment comment) {
 		CellReference cellReference = new CellReference(cellRef);
     	
 		comment.setRow(cellReference.getRow());
-		comment.setColumn((short)cellReference.getCol());
+		comment.setColumn(cellReference.getCol());
     }
     
-    public void setCellHyperlink(XSSFHyperlink hyperlink) {
+    protected void setCellHyperlink(XSSFHyperlink hyperlink) {
     	hyperlinks.add(hyperlink);
     }
-    
+
+    /**
+     * Return location of the active cell, e.g. <code>A1</code>.
+     *
+     * @return the location of the active cell.
+     */
     public String getActiveCell() {
     	return getSheetTypeSelection().getActiveCell();
     }
 
+    /**
+     * Sets location of the active cell
+     *
+     * @param cellRef the location of the active cell, e.g. <code>A1</code>..
+     */
 	public void setActiveCell(String cellRef) {
-		getSheetTypeSelection().setActiveCell(cellRef);
-	}
+		CTSelection ctsel = getSheetTypeSelection();
+        ctsel.setActiveCell(cellRef);
+        ctsel.setSqref(Arrays.asList(cellRef));
+    }
 	
 	/**
 	 * Does this sheet have any comments on it? We need to know,
@@ -1228,19 +1552,10 @@ public void ungroupColumn(short fromColumn, short toColumn) {
 	}
 
 	private CTPageSetUpPr getSheetTypePageSetUpPr() {
-    	if (getSheetTypeSheetPr().getPageSetUpPr() == null) {
-    		getSheetTypeSheetPr().setPageSetUpPr(CTPageSetUpPr.Factory.newInstance());
-    	}
-		return getSheetTypeSheetPr().getPageSetUpPr();
+        CTSheetPr sheetPr = getSheetTypeSheetPr();
+        return sheetPr.isSetPageSetUpPr() ? sheetPr.getPageSetUpPr() : sheetPr.addNewPageSetUpPr();
 	}
 
-	protected CTSheetPr getSheetTypeSheetPr() {
-    	if (worksheet.getSheetPr() == null) {
-    		worksheet.setSheetPr(CTSheetPr.Factory.newInstance());
-    	}
-		return worksheet.getSheetPr();
-	}
-    
     private boolean removeRow(int startRow, int endRow, int n, int rownum) {
     	if (rownum >= (startRow + n) && rownum <= (endRow + n)) {
     		if (n > 0 && rownum > endRow) {

src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java

@@ -129,7 +129,7 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook {
             	throw new IOException("Unable to load styles - " + e.toString());
             }
             try {
-	            // Load shared strings
+	            // Load themes
 	            this.themes = XSSFRelation.THEME.loadAll(getCorePart());
             } catch(Exception e) {
             	throw new IOException("Unable to load shared strings - " + e.toString());
@@ -195,7 +195,12 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook {
         }
     }
 
-    protected CTWorkbook getWorkbook() {
+    /**
+     * Return the underlying XML bean
+     *
+     * @return the underlying CTWorkbook bean
+     */
+    public CTWorkbook getWorkbook() {
         return this.workbook;
     }
 
@@ -265,8 +270,8 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook {
         return style;
     }
 
-    public DataFormat createDataFormat() {
-    	return getCreationHelper().createDataFormat();
+    public XSSFDataFormat createDataFormat() {
+    	return (XSSFDataFormat)getCreationHelper().createDataFormat();
     }
 
     public XSSFFont createFont() {
@@ -287,7 +292,9 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook {
     }
 
     public XSSFSheet createSheet(String sheetname) {
-        return createSheet(sheetname, XSSFSheet.newSheetInstance());
+	    if (doesContainsSheetName( sheetname, sheets.size() ))
+	   	    throw new IllegalArgumentException( "The workbook already contains a sheet of this name" );
+        return createSheet(sheetname, XSSFSheet.newInstance());
     }
 
     public XSSFSheet createSheet(String sheetname, CTWorksheet worksheet) {
@@ -298,25 +305,18 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook {
     }
 
     public XSSFSheet createDialogsheet(String sheetname, CTDialogsheet dialogsheet) {
-    	  CTSheet sheet = addSheet(sheetname);
-    	  XSSFDialogsheet wrapper = new XSSFDialogsheet(sheet, dialogsheet, this);
-    	  this.sheets.add(wrapper);
-    	  return wrapper;
+    CTSheet sheet = addSheet(sheetname);
+    XSSFDialogsheet wrapper = new XSSFDialogsheet(sheet, dialogsheet, this);
+    this.sheets.add(wrapper);
+    return wrapper;
     }
 
 	private CTSheet addSheet(String sheetname) {
 		CTSheet sheet = workbook.getSheets().addNewSheet();
-        if (sheetname != null) {
-            sheet.setName(sheetname);
-        }
+        sheet.setName(sheetname);
 		return sheet;
 	}
 
-    public void dumpDrawingGroupRecords(boolean fat) {
-        // TODO Auto-generated method stub
-
-    }
-
     public XSSFFont findFont(short boldWeight, short color, short fontHeight, String name, boolean italic, boolean strikeout, short typeOffset, byte underline) {
     	short fontNum=getNumberOfFonts();
         for (short i = 0; i < fontNum; i++) {
@@ -373,13 +373,8 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook {
         return false;
     }
 
-    public byte[] getBytes() {
-        // TODO Auto-generated method stub
-        return null;
-    }
-
-    public CellStyle getCellStyleAt(short idx) {
-        return stylesSource.getStyleAt(idx);
+    public XSSFCellStyle getCellStyleAt(short idx) {
+        return (XSSFCellStyle)stylesSource.getStyleAt(idx);
     }
 
     public Palette getCustomPalette() {
@@ -387,22 +382,6 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook {
         return null;
     }
 
-    /**
-     * get the first tab that is displayed in the list of tabs in excel.
-     */
-    public int getFirstVisibleTab() {
-        CTBookViews bookViews = workbook.getBookViews();
-        CTBookView bookView = bookViews.getWorkbookViewArray(0);
-        return (short) bookView.getActiveTab();
-    }
-    /**
-     * deprecated Aug 2008
-     * @deprecated - Misleading name - use getFirstVisibleTab()
-     */
-    public short getDisplayedTab() {
-        return (short) getFirstVisibleTab();
-    }
-
     public XSSFFont getFontAt(short idx) {
         return (XSSFFont)stylesSource.getFontAt(idx);
     }
@@ -545,18 +524,44 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook {
     }
 
     /**
-     * sets the first tab that is displayed in the list of tabs
-     * in excel.
-     * @param index
+     * Gets the first tab that is displayed in the list of tabs in excel.
+     *
+     * @return integer that contains the index to the active sheet in this book view.
+     */
+    public int getFirstVisibleTab() {
+        CTBookViews bookViews = workbook.getBookViews();
+        CTBookView bookView = bookViews.getWorkbookViewArray(0);
+        return (short) bookView.getActiveTab();
+    }
+
+    /**
+     * Sets the first tab that is displayed in the list of tabs in excel.
+     *
+     * @param index integer that contains the index to the active sheet in this book view.
      */
     public void setFirstVisibleTab(short index) {
         CTBookViews bookViews = workbook.getBookViews();
         CTBookView bookView= bookViews.getWorkbookViewArray(0);
         bookView.setActiveTab(index);
     }
+
     /**
-     * deprecated Aug 2008
-     * @deprecated - Misleading name - use setFirstVisibleTab()
+     * Gets the first tab that is displayed in the list of tabs
+     * in excel.
+     * @return an integer that contains the index to the active sheet in this book view.
+     *
+     * @deprecated Aug 2008 - Misleading name - use #getFirstVisibleTab()
+     */
+    public short getDisplayedTab() {
+        return (short) getFirstVisibleTab();
+    }
+
+    /**
+     * sets the first tab that is displayed in the list of tabs
+     * in excel.
+     * @param index integer that contains the index to the active sheet in this book view.
+     *
+     * @deprecated Aug 2008 - Misleading name - use #setFirstVisibleTab()
      */
     public void setDisplayedTab(short index) {
         setFirstVisibleTab(index);
@@ -588,6 +593,8 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook {
     }
 
     public void setSheetName(int sheet, String name) {
+	    if (doesContainsSheetName(name, sheet ))
+	        throw new IllegalArgumentException( "The workbook already contains a sheet of this name" );
         this.workbook.getSheets().getSheetArray(sheet).setName(name);
     }
 
@@ -765,4 +772,14 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook {
     public CreationHelper getCreationHelper() {
     	return new XSSFCreationHelper(this);
     }
+
+    private boolean doesContainsSheetName(String name, int excludeSheetIdx) {
+        CTSheet[] ctSheetArray = workbook.getSheets().getSheetArray();
+        for (int i = 0; i < ctSheetArray.length; i++) {
+            if (excludeSheetIdx != i && name.equalsIgnoreCase(ctSheetArray[i].getName()))
+                return true;
+        }
+        return false;
+    }
+
 }

src/ooxml/testcases/org/apache/poi/xssf/model/TestCommentsTable.java

@@ -233,8 +233,7 @@ public class TestCommentsTable extends TestCase {
 		assertEquals("Also POI", 
 				sheet2.getRow(2).getCell(1).getCellComment().getAuthor());
 		
-		// TODO: fix xmlbeans so it doesn't eat newlines
-		assertEquals("Nick Burch:This is a comment",
+		assertEquals("Nick Burch:\nThis is a comment",
 				sheet1.getRow(4).getCell(2).getCellComment().getString().getString());
 	}
 	

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

@@ -17,14 +17,20 @@
 
 package org.apache.poi.xssf.usermodel;
 
+import java.io.FileNotFoundException;
+import java.io.FileOutputStream;
 import java.util.Iterator;
 
 import junit.framework.TestCase;
 
 import org.apache.poi.ss.usermodel.Cell;
 import org.apache.poi.ss.usermodel.Row;
-import org.apache.poi.xssf.usermodel.TestXSSFCell.DummySharedStringSource;
 import org.apache.poi.xssf.model.SharedStringsTable;
+import org.apache.poi.xssf.XSSFTestDataSamples;
+import org.apache.poi.hssf.usermodel.HSSFSheet;
+import org.apache.poi.hssf.usermodel.HSSFRow;
+import org.apache.poi.hssf.usermodel.HSSFWorkbook;
+import org.apache.poi.hssf.usermodel.HSSFCell;
 
 /**
  * Tests for XSSFRow
@@ -121,45 +127,19 @@ public final class TestXSSFRow extends TestCase {
         assertEquals(-1, emptyRow.getFirstCellNum());
     }
 
-    public void testLastCellNum() {
-        XSSFRow row = getSampleRow();
-        assertEquals(100, row.getLastCellNum());
-
-        Cell cell = row.getCell((short) 100);
-        row.removeCell(cell);
-        assertFalse(row.getLastCellNum() == (short) 100);
-    }
-
-    public void testRemoveCell() {
-        XSSFRow row = getSampleRow();
-
-        // Test removing the first cell
-        Cell firstCell = row.getCell((short) 2);
-        assertNotNull(firstCell);
-        assertEquals(7, row.getPhysicalNumberOfCells());
-        row.removeCell(firstCell);
-        assertEquals(6, row.getPhysicalNumberOfCells());
-        firstCell = row.getCell((short) 2);
-        assertNull(firstCell);
-
-        // Test removing the last cell
-        Cell lastCell = row.getCell((short) 100);
-        row.removeCell(lastCell);
-    }
-
     public void testGetSetHeight() {
         XSSFRow row = getSampleRow();
         // I assume that "ht" attribute value is in 'points', please verify that
         // Test that no rowHeight is set
         assertEquals((short) -1, row.getHeight());
-        // Set a rowHeight in twips (1/20th of a point) and test the new value
+        // Set a rowHeight and test the new value
         row.setHeight((short) 240);
-        assertEquals((short) 240, row.getHeight());
-        assertEquals(12F, row.getHeightInPoints());
+        assertEquals((short) 240.0, row.getHeight());
+        assertEquals((float)240.0, row.getHeightInPoints());
         // Set a new rowHeight in points and test the new value
-        row.setHeightInPoints(13F);
-        assertEquals((float) 13, row.getHeightInPoints());
-        assertEquals((short) 260, row.getHeight());
+        row.setHeightInPoints(13);
+        assertEquals((float) 13.0, row.getHeightInPoints());
+        assertEquals((short) 13.0, row.getHeight());
     }
 
     public void testGetSetZeroHeight() throws Exception {
@@ -248,4 +228,74 @@ public final class TestXSSFRow extends TestCase {
         wb.setSharedStringSource(new SharedStringsTable());
         return new XSSFSheet(wb);
     }
+
+    /**
+     * Test that XSSFRow.getLastCellNum is consistent with HSSFRow.getLastCellNum
+     */
+    public void testLastCellNum() {
+        HSSFWorkbook wb1 = new HSSFWorkbook();
+        XSSFWorkbook wb2 = new XSSFWorkbook();
+
+        HSSFSheet sheet1 = wb1.createSheet();
+        XSSFSheet sheet2 = wb2.createSheet();
+
+        for (int i = 0; i < 10; i++) {
+            HSSFRow row1 = sheet1.createRow(i);
+            XSSFRow row2 = sheet2.createRow(i);
+
+            for (int j = 0; j < 5; j++) {
+                //before adding a cell
+                assertEquals(row1.getLastCellNum(), row2.getLastCellNum());
+
+                HSSFCell cell1 = row1.createCell(j);
+                XSSFCell cell2 = row2.createCell(j);
+
+                //after adding a cell
+                assertEquals(row1.getLastCellNum(), row2.getLastCellNum());
+            }
+        }
+    }
+
+    public void testRemoveCell() {
+        XSSFRow row = getSampleRow();
+
+        // Test removing the first cell
+        Cell firstCell = row.getCell((short) 2);
+        assertNotNull(firstCell);
+        assertEquals(7, row.getPhysicalNumberOfCells());
+        row.removeCell(firstCell);
+        assertEquals(6, row.getPhysicalNumberOfCells());
+        firstCell = row.getCell((short) 2);
+        assertNull(firstCell);
+
+        // Test removing the last cell
+        Cell lastCell = row.getCell((short) 100);
+        row.removeCell(lastCell);
+    }
+
+    public void testFirstLastCellNum() {
+        XSSFWorkbook workbook = new XSSFWorkbook();
+        XSSFSheet sheet = workbook.createSheet();
+        XSSFRow row = sheet.createRow(0);
+        assertEquals(-1, row.getLastCellNum());
+        assertEquals(-1, row.getFirstCellNum());
+        row.createCell(1);
+        assertEquals(2, row.getLastCellNum());
+        assertEquals(1, row.getFirstCellNum());
+        row.createCell(3);
+        assertEquals(4, row.getLastCellNum());
+        assertEquals(1, row.getFirstCellNum());
+        row.removeCell(row.getCell(3));
+        assertEquals(2, row.getLastCellNum());
+        assertEquals(1, row.getFirstCellNum());
+        row.removeCell(row.getCell(1));
+        assertEquals(-1, row.getLastCellNum());
+        assertEquals(-1, row.getFirstCellNum());
+
+        workbook = XSSFTestDataSamples.writeOutAndReadBack(workbook);
+        sheet = workbook.getSheetAt(0);
+
+        assertEquals(-1, sheet.getRow(0).getLastCellNum());
+        assertEquals(-1, sheet.getRow(0).getFirstCellNum());
+    }
 }

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

@@ -30,6 +30,10 @@ import org.apache.poi.ss.util.Region;
 import org.apache.poi.xssf.model.CommentsTable;
 import org.apache.poi.xssf.model.StylesTable;
 import org.apache.poi.xssf.usermodel.helpers.ColumnHelper;
+import org.apache.poi.hssf.usermodel.HSSFSheet;
+import org.apache.poi.hssf.usermodel.HSSFWorkbook;
+import org.apache.poi.hssf.usermodel.HSSFRow;
+import org.apache.poi.hssf.usermodel.HSSFCell;
 import org.openxml4j.opc.Package;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTCol;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTCols;
@@ -166,7 +170,7 @@ public class TestXSSFSheet extends TestCase {
     public void testGetSetRowBreaks() {
         XSSFWorkbook workbook = new XSSFWorkbook();
         Sheet sheet = workbook.createSheet("Sheet 1");
-        assertNull(sheet.getRowBreaks());
+        assertEquals(0, sheet.getRowBreaks().length);
         sheet.setRowBreak(1);
         sheet.setRowBreak(15);
         assertNotNull(sheet.getRowBreaks());
@@ -190,7 +194,7 @@ public class TestXSSFSheet extends TestCase {
     public void testGetSetColumnBreaks() {
         XSSFWorkbook workbook = new XSSFWorkbook();
         Sheet sheet = workbook.createSheet("Sheet 1");
-        assertNull(sheet.getColumnBreaks());
+        assertEquals(0, sheet.getColumnBreaks().length);
         sheet.setColumnBreak((short) 11);
         assertNotNull(sheet.getColumnBreaks());
         assertEquals(11, sheet.getColumnBreaks()[0]);
@@ -201,7 +205,7 @@ public class TestXSSFSheet extends TestCase {
     public void testRemoveColumnBreak() {
         XSSFWorkbook workbook = new XSSFWorkbook();
         Sheet sheet = workbook.createSheet("Sheet 1");
-        assertNull(sheet.getColumnBreaks());
+        assertEquals(0, sheet.getColumnBreaks().length);
         sheet.setColumnBreak((short) 11);
         assertNotNull(sheet.getColumnBreaks());
         sheet.setColumnBreak((short) 12);
@@ -858,4 +862,19 @@ public class TestXSSFSheet extends TestCase {
                 assertEquals(zoom, 75);
             }
 
+    public void testOutlineProperties() {
+        XSSFWorkbook wb = new XSSFWorkbook();
+
+        XSSFSheet sheet = wb.createSheet();
+
+        assertTrue(sheet.getRowSumsBelow());
+        assertTrue(sheet.getRowSumsRight());
+
+        sheet.setRowSumsBelow(false);
+        sheet.setRowSumsRight(false);
+
+        assertFalse(sheet.getRowSumsBelow());
+        assertFalse(sheet.getRowSumsRight());
+    }
+
 }

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

@@ -240,10 +240,10 @@ public class TestXSSFWorkbook extends TestCase {
     }
     
     public void testFindFont(){
-        //get dafault font and check against default value
+        //get default font and check against default value
     	XSSFWorkbook workbook = new XSSFWorkbook();
     	Font fontFind=workbook.findFont(Font.BOLDWEIGHT_NORMAL, IndexedColors.BLACK.getIndex(), (short)11, "Calibri", false, false, Font.SS_NONE, Font.U_NONE);
-    	assertNotNull(fontFind);    	
+    	assertNotNull(fontFind);
     	
     	//get default font, then change 2 values and check against different values (height changes)
     	Font font=workbook.createFont();
@@ -251,6 +251,7 @@ public class TestXSSFWorkbook extends TestCase {
     	font.setUnderline(Font.U_DOUBLE);
     	StylesSource styleSource=new StylesTable();
     	long index=styleSource.putFont(font);
+    	System.out.println("index="+index);
     	workbook.setStylesSource(styleSource);
     	fontFind=workbook.findFont(Font.BOLDWEIGHT_BOLD, IndexedColors.BLACK.getIndex(), (short)15, "Calibri", false, false, Font.SS_NONE, Font.U_DOUBLE);
         assertNull(fontFind);
@@ -290,7 +291,6 @@ public class TestXSSFWorkbook extends TestCase {
         assertNotNull(fontAt);
     }
     
-
     public void testGetNumberOfFonts(){
      	XSSFWorkbook wb = new XSSFWorkbook();