Fix bug #45518 - Fix up ColumnHelper to output valid col tags, by making 1 based and 0 based bits clearer, and using the right ones

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

src/documentation/content/xdocs/changes.xml

@@ -41,6 +41,7 @@
         </release>
 -->
         <release version="3.5.1-beta2" date="2008-08-20">
+           <action dev="POI-DEVELOPERS" type="fix">45518 - Fix up ColumnHelper to output valid col tags, by making 1 based and 0 based bits clearer, and using the right ones</action>
            <action dev="POI-DEVELOPERS" type="fix">45676 - Handle very long cells in the XSSF EventUserModel example</action>
            <action dev="POI-DEVELOPERS" type="add">Initial ExtractorFactory support for building TextExtractors for embeded documents</action>
            <action dev="POI-DEVELOPERS" type="add">Support stripping XSSF header and footer fields (eg page number) out of header and footer text if required</action>

src/documentation/content/xdocs/status.xml

@@ -38,6 +38,7 @@
         </release>
 -->
         <release version="3.5.1-beta2" date="2008-08-20">
+           <action dev="POI-DEVELOPERS" type="fix">45518 - Fix up ColumnHelper to output valid col tags, by making 1 based and 0 based bits clearer, and using the right ones</action>
            <action dev="POI-DEVELOPERS" type="fix">45676 - Handle very long cells in the XSSF EventUserModel example</action>
            <action dev="POI-DEVELOPERS" type="add">Initial ExtractorFactory support for building TextExtractors for embeded documents</action>
            <action dev="POI-DEVELOPERS" type="add">Support stripping XSSF header and footer fields (eg page number) out of header and footer text if required</action>

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

@@ -537,7 +537,7 @@ public final class HSSFRow implements Comparable, Row {
       return new CellIterator();
     }
     /**
-     * Alias for {@link CellIterator} to allow
+     * Alias for {@link #cellIterator} to allow
      *  foreach loops
      */
     public Iterator iterator() { 

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

@@ -648,15 +648,20 @@ public class XSSFSheet implements Sheet {
     	return getSheetTypePrintOptions().getVerticalCentered();
     }
 
-    
+    /**
+     * Group between (0 based) columns 
+     */
     public void groupColumn(short fromColumn, short toColumn) {
+    	groupColumn1Based(fromColumn+1, toColumn+1);
+    }
+    private void groupColumn1Based(int fromColumn, int toColumn) {
     	CTCols ctCols=worksheet.getColsArray(0);
     	CTCol ctCol=CTCol.Factory.newInstance();
     	ctCol.setMin(fromColumn);
     	ctCol.setMax(toColumn);
     	this.columnHelper.addCleanColIntoCols(ctCols, ctCol);    	
     	for(int index=fromColumn;index<=toColumn;index++){
-    		CTCol col=columnHelper.getColumn(index, false);
+    		CTCol col=columnHelper.getColumn1Based(index, false);
     		//col must exist
     		short outlineLevel=col.getOutlineLevel();
     		col.setOutlineLevel((short)(outlineLevel+1));

src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/ColumnHelper.java

@@ -26,6 +26,12 @@ import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTCol;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTCols;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTWorksheet;
 
+/**
+ * Helper class for dealing with the Column settings on
+ *  a CTWorksheet (the data part of a sheet).
+ * Note - within POI, we use 0 based column indexes, but
+ *  the column definitions in the XML are 1 based!
+ */
 public class ColumnHelper {
 
     private CTWorksheet worksheet;
@@ -70,20 +76,31 @@ public class ColumnHelper {
         return newCol;
     }
 
+    /**
+     * Returns the Column at the given 0 based index
+     */
     public CTCol getColumn(long index, boolean splitColumns) {
+    	return getColumn1Based(index+1, splitColumns);
+    }
+    /**
+     * Returns the Column at the given 1 based index.
+     * POI default is 0 based, but the file stores
+     *  as 1 based.
+     */
+    public CTCol getColumn1Based(long index1, boolean splitColumns) {
         CTCols colsArray = worksheet.getColsArray(0);
 		for (int i = 0; i < colsArray.sizeOfColArray(); i++) {
             CTCol colArray = colsArray.getColArray(i);
-			if (colArray.getMin() <= index && colArray.getMax() >= index) {
+			if (colArray.getMin() <= index1 && colArray.getMax() >= index1) {
 				if (splitColumns) {
-					if (colArray.getMin() < index) {
+					if (colArray.getMin() < index1) {
-						insertCol(colsArray, colArray.getMin(), (index - 1), new CTCol[]{colArray});
+						insertCol(colsArray, colArray.getMin(), (index1 - 1), new CTCol[]{colArray});
 					}
-					if (colArray.getMax() > index) {
+					if (colArray.getMax() > index1) {
-						insertCol(colsArray, (index + 1), colArray.getMax(), new CTCol[]{colArray});
+						insertCol(colsArray, (index1 + 1), colArray.getMax(), new CTCol[]{colArray});
 					}
-					colArray.setMin(index);
+					colArray.setMin(index1);
-					colArray.setMax(index);
+					colArray.setMax(index1);
 				}
                 return colArray;
             }
@@ -175,9 +192,16 @@ public class ColumnHelper {
         return null;
     }
 
+    /**
+     * Does the column at the given 0 based index exist
+     *  in the supplied list of column definitions?
+     */
     public boolean columnExists(CTCols cols, long index) {
+    	return columnExists1Based(cols, index+1);
+    }
+    private boolean columnExists1Based(CTCols cols, long index1) {
         for (int i = 0; i < cols.sizeOfColArray(); i++) {
-            if (cols.getColArray(i).getMin() == index) {
+            if (cols.getColArray(i).getMin() == index1) {
                 return true;
             }
         }
@@ -195,26 +219,30 @@ public class ColumnHelper {
     }
 
     public void setColBestFit(long index, boolean bestFit) {
-        CTCol col = getOrCreateColumn(index, false);
+        CTCol col = getOrCreateColumn1Based(index+1, false);
         col.setBestFit(bestFit);
     }
 
     public void setColWidth(long index, double width) {
-        CTCol col = getOrCreateColumn(index, false);
+        CTCol col = getOrCreateColumn1Based(index+1, false);
         col.setWidth(width);
     }
 
     public void setColHidden(long index, boolean hidden) {
-        CTCol col = getOrCreateColumn(index, false);
+        CTCol col = getOrCreateColumn1Based(index+1, false);
         col.setHidden(hidden);
     }
 
-    protected CTCol getOrCreateColumn(long index, boolean splitColumns) {
+    /**
-        CTCol col = getColumn(index, splitColumns);
+     * Return the CTCol at the given (0 based) column index,
+     *  creating it if required.
+     */
+    protected CTCol getOrCreateColumn1Based(long index1, boolean splitColumns) {
+        CTCol col = getColumn1Based(index1, splitColumns);
         if (col == null) {
             col = worksheet.getColsArray(0).addNewCol();
-            col.setMin(index);
+            col.setMin(index1);
-            col.setMax(index);
+            col.setMax(index1);
         }
         return col;
     }
@@ -224,7 +252,7 @@ public class ColumnHelper {
 	}
 	
 	public void setColDefaultStyle(long index, int styleId) {
-		CTCol col = getOrCreateColumn(index, true);
+		CTCol col = getOrCreateColumn1Based(index+1, true);
 		col.setStyle(styleId);
 	}
 	
@@ -235,22 +263,22 @@ public class ColumnHelper {
 		}
 		return -1;
 	}
-        
+
-        private boolean columnExists(CTCols cols, long min, long max) {
+	private boolean columnExists(CTCols cols, long min, long max) {
-            for (int i = 0; i < cols.sizeOfColArray(); i++) {
+	    for (int i = 0; i < cols.sizeOfColArray(); i++) {
-                if (cols.getColArray(i).getMin() == min && cols.getColArray(i).getMax() == max) {
+	        if (cols.getColArray(i).getMin() == min && cols.getColArray(i).getMax() == max) {
-                    return true;
+	            return true;
-                }
+	        }
-            }
+	    }
-            return false;
+	    return false;
-        }
+	}
-        
+	
-        public int getIndexOfColumn(CTCols cols, CTCol col) {
+	public int getIndexOfColumn(CTCols cols, CTCol col) {
-            for (int i = 0; i < cols.sizeOfColArray(); i++) {
+	    for (int i = 0; i < cols.sizeOfColArray(); i++) {
-                if (cols.getColArray(i).getMin() == col.getMin() && cols.getColArray(i).getMax() == col.getMax()) {
+	        if (cols.getColArray(i).getMin() == col.getMin() && cols.getColArray(i).getMax() == col.getMax()) {
-                    return i;
+	            return i;
-                }
+	        }
-            }
+	    }
-            return -1;
+	    return -1;
-        }
+	}
 }

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

@@ -407,6 +407,41 @@ public class TestXSSFSheet extends TestCase {
         Sheet sheet = workbook.createSheet("Sheet 1");
         sheet.setColumnWidth((short) 1,(short)  22);
         assertEquals(22, sheet.getColumnWidth((short) 1));
+        
+        // Now check the low level stuff, and check that's all
+        //  been set correctly
+        XSSFSheet xs = (XSSFSheet)sheet;
+        CTWorksheet cts = xs.getWorksheet();
+        
+        CTCols[] cols_s = cts.getColsArray();
+        assertEquals(1, cols_s.length);
+        CTCols cols = cols_s[0];
+        assertEquals(1, cols.sizeOfColArray());
+        CTCol col = cols.getColArray(0);
+        
+        // XML is 1 based, POI is 0 based
+        assertEquals(2, col.getMin());
+        assertEquals(2, col.getMax());
+        assertEquals(22.0, col.getWidth());
+        
+        
+        // Now set another
+        sheet.setColumnWidth((short) 3,(short)  33);
+        
+        cols_s = cts.getColsArray();
+        assertEquals(1, cols_s.length);
+        cols = cols_s[0];
+        assertEquals(2, cols.sizeOfColArray());
+        
+        col = cols.getColArray(0);
+        assertEquals(2, col.getMin()); // POI 1
+        assertEquals(2, col.getMax());
+        assertEquals(22.0, col.getWidth());
+        
+        col = cols.getColArray(1);
+        assertEquals(4, col.getMin()); // POI 3
+        assertEquals(4, col.getMax());
+        assertEquals(33.0, col.getWidth());
     }
     
     public void testGetSetColumnHidden() {
@@ -744,8 +779,8 @@ public class TestXSSFSheet extends TestCase {
 	    	assertEquals(2,cols.sizeOfColArray());
 	    	CTCol[]colArray=cols.getColArray();
 	    	assertNotNull(colArray);
-	    	assertEquals(2,colArray[0].getMin());
+	    	assertEquals(2+1,colArray[0].getMin()); // 1 based
-	    	assertEquals(7,colArray[0].getMax());
+	    	assertEquals(7+1,colArray[0].getMax()); // 1 based
 	    	assertEquals(1, colArray[0].getOutlineLevel());
 
 	    	//two level  

src/ooxml/testcases/org/apache/poi/xssf/usermodel/helpers/TestColumnHelper.java

@@ -57,8 +57,11 @@ public class TestColumnHelper extends TestCase {
         assertEquals(1, worksheet.sizeOfColsArray());
         count = countColumns(worksheet);
         assertEquals(16375, count);
-        assertEquals((double) 88, helper.getColumn(1, false).getWidth());
+        // Remember - POI column 0 == OOXML column 1
-        assertTrue(helper.getColumn(1, false).getHidden());
+        assertEquals((double) 88, helper.getColumn(0, false).getWidth());
+        assertTrue(helper.getColumn(0, false).getHidden());
+        assertEquals((double) 00, helper.getColumn(1, false).getWidth());
+        assertFalse(helper.getColumn(1, false).getHidden());
     }
 
     public void testSortColumns() {
@@ -199,11 +202,14 @@ public class TestColumnHelper extends TestCase {
         col4.setMin(3);
         col4.setMax(6);
 
+        // Remember - POI column 0 == OOXML column 1
         ColumnHelper helper = new ColumnHelper(worksheet);
+        assertNotNull(helper.getColumn(0, false));
         assertNotNull(helper.getColumn(1, false));
-        assertEquals((double) 88, helper.getColumn(1, false).getWidth());
+        assertEquals((double) 88, helper.getColumn(0, false).getWidth());
-        assertTrue(helper.getColumn(1, false).getHidden());
+        assertEquals((double) 0, helper.getColumn(1, false).getWidth());
-        assertFalse(helper.getColumn(2, false).getHidden());
+        assertTrue(helper.getColumn(0, false).getHidden());
+        assertFalse(helper.getColumn(1, false).getHidden());
         assertNull(helper.getColumn(99, false));
         assertNotNull(helper.getColumn(5, false));
     }
@@ -226,13 +232,21 @@ public class TestColumnHelper extends TestCase {
         XSSFWorkbook workbook = new XSSFWorkbook();
         XSSFSheet sheet = (XSSFSheet) workbook.createSheet("Sheet 1");
         ColumnHelper columnHelper = sheet.getColumnHelper();
-        CTCol col = columnHelper.getOrCreateColumn(3, false);
+        
+        // Check POI 0 based, OOXML 1 based
+        CTCol col = columnHelper.getOrCreateColumn1Based(3, false);
         assertNotNull(col);
-        assertNotNull(columnHelper.getColumn(3, false));
+        assertNull(columnHelper.getColumn(1, false));
+        assertNotNull(columnHelper.getColumn(2, false));
+        assertNotNull(columnHelper.getColumn1Based(3, false));
+        assertNull(columnHelper.getColumn(3, false));
 
-        CTCol col2 = columnHelper.getOrCreateColumn(30, false);
+        CTCol col2 = columnHelper.getOrCreateColumn1Based(30, false);
         assertNotNull(col2);
-        assertNotNull(columnHelper.getColumn(30, false));
+        assertNull(columnHelper.getColumn(28, false));
+        assertNotNull(columnHelper.getColumn(29, false));
+        assertNotNull(columnHelper.getColumn1Based(30, false));
+        assertNull(columnHelper.getColumn(30, false));
     }
     
     public void testGetSetColDefaultStyle() {
@@ -241,7 +255,10 @@ public class TestColumnHelper extends TestCase {
     	CTWorksheet ctWorksheet = CTWorksheet.Factory.newInstance();
     	XSSFSheet sheet = new XSSFSheet(ctSheet, ctWorksheet, (XSSFWorkbook) workbook);
         ColumnHelper columnHelper = sheet.getColumnHelper();
-        CTCol col = columnHelper.getOrCreateColumn(3, false);
+        
+        // POI column 3, OOXML column 4
+        CTCol col = columnHelper.getOrCreateColumn1Based(4, false);
+        
         assertNotNull(col);
         assertNotNull(columnHelper.getColumn(3, false));
         columnHelper.setColDefaultStyle(3, 2);