From 4ed42be22f1c0ca73cdd02f0fea938c6c602ffd9 Mon Sep 17 00:00:00 2001 From: Nick Burch Date: Mon, 15 Sep 2008 20:54:49 +0000 Subject: [PATCH] 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 | 1 + src/documentation/content/xdocs/status.xml | 1 + .../apache/poi/hssf/usermodel/HSSFRow.java | 2 +- .../apache/poi/xssf/usermodel/XSSFSheet.java | 9 +- .../xssf/usermodel/helpers/ColumnHelper.java | 96 ++++++++++++------- .../poi/xssf/usermodel/TestXSSFSheet.java | 39 +++++++- .../usermodel/helpers/TestColumnHelper.java | 37 +++++-- 7 files changed, 136 insertions(+), 49 deletions(-) diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml index 9bb4ec1c7..aa53ad8ad 100644 --- a/src/documentation/content/xdocs/changes.xml +++ b/src/documentation/content/xdocs/changes.xml @@ -41,6 +41,7 @@ --> + 45518 - Fix up ColumnHelper to output valid col tags, by making 1 based and 0 based bits clearer, and using the right ones 45676 - Handle very long cells in the XSSF EventUserModel example Initial ExtractorFactory support for building TextExtractors for embeded documents Support stripping XSSF header and footer fields (eg page number) out of header and footer text if required diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index 9c6d8d266..c60af7d94 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -38,6 +38,7 @@ --> + 45518 - Fix up ColumnHelper to output valid col tags, by making 1 based and 0 based bits clearer, and using the right ones 45676 - Handle very long cells in the XSSF EventUserModel example Initial ExtractorFactory support for building TextExtractors for embeded documents Support stripping XSSF header and footer fields (eg page number) out of header and footer text if required diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFRow.java b/src/java/org/apache/poi/hssf/usermodel/HSSFRow.java index 641c07467..44922014f 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFRow.java +++ b/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() { diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java index 6f2b1da3b..94da7ac01 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java +++ b/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)); diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/ColumnHelper.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/ColumnHelper.java index 79ebb774d..ddd594918 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/ColumnHelper.java +++ b/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) { - insertCol(colsArray, colArray.getMin(), (index - 1), new CTCol[]{colArray}); + if (colArray.getMin() < index1) { + insertCol(colsArray, colArray.getMin(), (index1 - 1), new CTCol[]{colArray}); } - if (colArray.getMax() > index) { - insertCol(colsArray, (index + 1), colArray.getMax(), new CTCol[]{colArray}); + if (colArray.getMax() > index1) { + insertCol(colsArray, (index1 + 1), colArray.getMax(), new CTCol[]{colArray}); } - colArray.setMin(index); - colArray.setMax(index); + colArray.setMin(index1); + 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.setMax(index); + col.setMin(index1); + 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) { - for (int i = 0; i < cols.sizeOfColArray(); i++) { - if (cols.getColArray(i).getMin() == min && cols.getColArray(i).getMax() == max) { - return true; - } - } - return false; - } - - public int getIndexOfColumn(CTCols cols, CTCol col) { - for (int i = 0; i < cols.sizeOfColArray(); i++) { - if (cols.getColArray(i).getMin() == col.getMin() && cols.getColArray(i).getMax() == col.getMax()) { - return i; - } - } - return -1; - } + + private boolean columnExists(CTCols cols, long min, long max) { + for (int i = 0; i < cols.sizeOfColArray(); i++) { + if (cols.getColArray(i).getMin() == min && cols.getColArray(i).getMax() == max) { + return true; + } + } + return false; + } + + public int getIndexOfColumn(CTCols cols, CTCol col) { + for (int i = 0; i < cols.sizeOfColArray(); i++) { + if (cols.getColArray(i).getMin() == col.getMin() && cols.getColArray(i).getMax() == col.getMax()) { + return i; + } + } + return -1; + } } diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java index 628ffc027..d5c892c58 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java +++ b/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(7,colArray[0].getMax()); + assertEquals(2+1,colArray[0].getMin()); // 1 based + assertEquals(7+1,colArray[0].getMax()); // 1 based assertEquals(1, colArray[0].getOutlineLevel()); //two level diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/helpers/TestColumnHelper.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/helpers/TestColumnHelper.java index 6e964b795..a0ae950f7 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/helpers/TestColumnHelper.java +++ b/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()); - assertTrue(helper.getColumn(1, false).getHidden()); + // Remember - POI column 0 == OOXML column 1 + 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()); - assertTrue(helper.getColumn(1, false).getHidden()); - assertFalse(helper.getColumn(2, false).getHidden()); + assertEquals((double) 88, helper.getColumn(0, false).getWidth()); + assertEquals((double) 0, helper.getColumn(1, false).getWidth()); + 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);