From: Josh Micich Date: Thu, 3 Apr 2008 20:25:53 +0000 (+0000) Subject: Fix for bug 44739 - Conditional formatting (regions with max row/col index) X-Git-Tag: REL_3_0_3_BETA1~44 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=fd84995b3f1d19fce6cb5254100ab147ef532954;p=poi.git Fix for bug 44739 - Conditional formatting (regions with max row/col index) git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@644473 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml index a30c523680..e2e6faef7e 100644 --- a/src/documentation/content/xdocs/changes.xml +++ b/src/documentation/content/xdocs/changes.xml @@ -37,6 +37,7 @@ + 44739 - Small fixes for conditional formatting (regions with max row/col index) Implement Sheet.removeShape(Shape shape) in HSLF 44694 - HPSF: Support for property sets without sections Various fixes: Recognising var-arg built-in functions #44675, ExternalNameRecord serialisation bug #44695, PMT() bug #44691 diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index 5e963e484c..7a1d2e9717 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + 44739 - Small fixes for conditional formatting (regions with max row/col index) 44694 - HPSF: Support for property sets without sections Implement Sheet.removeShape(Shape shape) in HSLF Various fixes: Recognising var-arg built-in functions #44675, ExternalNameRecord serialisation bug #44695, PMT() bug #44691 diff --git a/src/java/org/apache/poi/hssf/record/CFHeaderRecord.java b/src/java/org/apache/poi/hssf/record/CFHeaderRecord.java index 7f2d5c3197..d74d54ab17 100644 --- a/src/java/org/apache/poi/hssf/record/CFHeaderRecord.java +++ b/src/java/org/apache/poi/hssf/record/CFHeaderRecord.java @@ -58,12 +58,12 @@ public final class CFHeaderRecord extends Record { field_1_numcf = in.readShort(); field_2_need_recalculation = in.readShort(); - field_3_enclosing_cell_range = new CellRange(in.readShort(),in.readShort(),in.readShort(),in.readShort()); + field_3_enclosing_cell_range = new CellRange(in.readUShort(), in.readUShort(), in.readUShort(), in.readUShort()); int numCellRanges = in.readShort(); CellRange[] crs = new CellRange[numCellRanges]; for( int i=0; i - * - * Note - this value converts to -1 when cast to a short - */ - private static final int MAX_INDEX = Integer.MAX_VALUE; + /** max 65536 rows in BIFF8 */ + private static final int LAST_ROW_INDEX = 0x00FFFF; + /** max 256 columns in BIFF8 */ + private static final int LAST_COLUMN_INDEX = 0x00FF; private static final Region[] EMPTY_REGION_ARRAY = { }; @@ -57,54 +55,46 @@ public final class CellRange + ", " + firstColumn + ", " + lastColumn + ")"); } _firstRow = firstRow; - _lastRow = convertM1ToMax(lastRow); + _lastRow = convertM1ToMax(lastRow, LAST_ROW_INDEX); _firstColumn = firstColumn; - _lastColumn = convertM1ToMax(lastColumn); + _lastColumn = convertM1ToMax(lastColumn, LAST_COLUMN_INDEX); } - private static int convertM1ToMax(int lastIx) { + /** + * Range arithmetic is easier when using a large positive number for 'max row or column' + * instead of -1. + */ + private static int convertM1ToMax(int lastIx, int maxIndex) { if(lastIx < 0) { - return MAX_INDEX; - } - return lastIx; - } - private static int convertMaxToM1(int lastIx) { - if(lastIx == MAX_INDEX) { - return -1; + return maxIndex; } return lastIx; } public boolean isFullColumnRange() { - return _firstColumn == 0 && _lastColumn == MAX_INDEX; + return _firstRow == 0 && _lastRow == LAST_ROW_INDEX; } public boolean isFullRowRange() { - return _firstRow == 0 && _lastRow == MAX_INDEX; + return _firstColumn == 0 && _lastColumn == LAST_COLUMN_INDEX; } public CellRange(Region r) { this(r.getRowFrom(), r.getRowTo(), r.getColumnFrom(), r.getColumnTo()); } - - private static boolean isValid(int firstRow, int lastRow, int firstColumn, int lastColumn) { - if(lastRow == -1) { - if(firstRow !=0) { - return false; - } + if(lastRow < 0 || lastRow > LAST_ROW_INDEX) { + return false; } - if(firstRow < 0 || lastRow < -1) { + if(firstRow < 0 || firstRow > LAST_ROW_INDEX) { return false; } - if(lastColumn == -1) { - if(firstColumn !=0) { - return false; - } + if(lastColumn < 0 || lastColumn > LAST_COLUMN_INDEX) { + return false; } - if(firstColumn < 0 || lastColumn < -1) { + if(firstColumn < 0 || firstColumn > LAST_COLUMN_INDEX) { return false; } return true; @@ -114,23 +104,17 @@ public final class CellRange { return _firstRow; } - /** - * @return -1 for whole column ranges - */ public int getLastRow() { - return convertMaxToM1(_lastRow); + return _lastRow; } public int getFirstColumn() { return _firstColumn; } - /** - * @return -1 for whole row ranges - */ public int getLastColumn() { - return convertMaxToM1(_lastColumn); + return _lastColumn; } public static final int NO_INTERSECTION = 1; @@ -404,10 +388,8 @@ public final class CellRange private Region convertToRegion() { - int lastRow = convertMaxToM1(_lastRow); - int lastColumn = convertMaxToM1(_lastColumn); - return new Region(_firstRow, (short)_firstColumn, lastRow, (short)lastColumn); + return new Region(_firstRow, (short)_firstColumn, _lastRow, (short)_lastColumn); } diff --git a/src/testcases/org/apache/poi/hssf/record/AllRecordTests.java b/src/testcases/org/apache/poi/hssf/record/AllRecordTests.java index 32b16cf656..78128dbe19 100755 --- a/src/testcases/org/apache/poi/hssf/record/AllRecordTests.java +++ b/src/testcases/org/apache/poi/hssf/record/AllRecordTests.java @@ -17,13 +17,13 @@ package org.apache.poi.hssf.record; -import org.apache.poi.hssf.record.aggregates.AllRecordAggregateTests; -import org.apache.poi.hssf.record.formula.AllFormulaTests; -import org.apache.poi.hssf.record.formula.functions.AllIndividualFunctionEvaluationTests; - import junit.framework.Test; import junit.framework.TestSuite; +import org.apache.poi.hssf.record.aggregates.AllRecordAggregateTests; +import org.apache.poi.hssf.record.cf.TestCellRange; +import org.apache.poi.hssf.record.formula.AllFormulaTests; + /** * Collects all tests for package org.apache.poi.hssf.record. * @@ -104,6 +104,7 @@ public final class AllRecordTests { result.addTestSuite(TestUnicodeString.class); result.addTestSuite(TestUnitsRecord.class); result.addTestSuite(TestValueRangeRecord.class); + result.addTestSuite(TestCellRange.class); return result; } } diff --git a/src/testcases/org/apache/poi/hssf/record/TestCFHeaderRecord.java b/src/testcases/org/apache/poi/hssf/record/TestCFHeaderRecord.java index 2a6faaccdd..e883503674 100644 --- a/src/testcases/org/apache/poi/hssf/record/TestCFHeaderRecord.java +++ b/src/testcases/org/apache/poi/hssf/record/TestCFHeaderRecord.java @@ -17,6 +17,7 @@ package org.apache.poi.hssf.record; +import junit.framework.AssertionFailedError; import junit.framework.TestCase; import org.apache.poi.hssf.record.cf.CellRange; @@ -34,8 +35,8 @@ public final class TestCFHeaderRecord extends TestCase { CFHeaderRecord record = new CFHeaderRecord(); CellRange[] ranges = { - new CellRange(0,-1,5,5), - new CellRange(0,-1,6,6), + new CellRange(0,0xFFFF,5,5), + new CellRange(0,0xFFFF,6,6), new CellRange(0,1,0,1), new CellRange(0,1,2,3), new CellRange(2,3,0,1), @@ -46,7 +47,7 @@ public final class TestCFHeaderRecord extends TestCase assertEquals(6,ranges.length); CellRange enclosingCellRange = record.getEnclosingCellRange(); assertEquals(0, enclosingCellRange.getFirstRow()); - assertEquals(-1, enclosingCellRange.getLastRow()); + assertEquals(65535, enclosingCellRange.getLastRow()); assertEquals(0, enclosingCellRange.getFirstColumn()); assertEquals(6, enclosingCellRange.getLastColumn()); record.setNeedRecalculation(true); @@ -56,7 +57,7 @@ public final class TestCFHeaderRecord extends TestCase } public void testSerialization() { - byte[] recordData = new byte[] + byte[] recordData = { (byte)0x03, (byte)0x00, (byte)0x01, (byte)0x00, @@ -66,7 +67,7 @@ public final class TestCFHeaderRecord extends TestCase (byte)0x00, (byte)0x00, (byte)0x03, (byte)0x00, - (byte)0x04, (byte)0x00, + (byte)0x04, (byte)0x00, // nRegions (byte)0x00, (byte)0x00, (byte)0x01, (byte)0x00, @@ -93,44 +94,88 @@ public final class TestCFHeaderRecord extends TestCase assertEquals("#CFRULES", 3, record.getNumberOfConditionalFormats()); assertTrue(record.getNeedRecalculation()); - CellRange enclosingCellRange = record.getEnclosingCellRange(); - assertEquals(0, enclosingCellRange.getFirstRow()); - assertEquals(3, enclosingCellRange.getLastRow()); - assertEquals(0, enclosingCellRange.getFirstColumn()); - assertEquals(3, enclosingCellRange.getLastColumn()); + confirm(record.getEnclosingCellRange(), 0, 3, 0, 3); CellRange[] ranges = record.getCellRanges(); - CellRange range0 = ranges[0]; - assertEquals(0, range0.getFirstRow()); - assertEquals(1, range0.getLastRow()); - assertEquals(0, range0.getFirstColumn()); - assertEquals(1, range0.getLastColumn()); - CellRange range1 = ranges[1]; - assertEquals(0, range1.getFirstRow()); - assertEquals(1, range1.getLastRow()); - assertEquals(2, range1.getFirstColumn()); - assertEquals(3, range1.getLastColumn()); - CellRange range2 = ranges[2]; - assertEquals(2, range2.getFirstRow()); - assertEquals(3, range2.getLastRow()); - assertEquals(0, range2.getFirstColumn()); - assertEquals(1, range2.getLastColumn()); - CellRange range3 = ranges[3]; - assertEquals(2, range3.getFirstRow()); - assertEquals(3, range3.getLastRow()); - assertEquals(2, range3.getFirstColumn()); - assertEquals(3, range3.getLastColumn()); + assertEquals(4, ranges.length); + confirm(ranges[0], 0, 1, 0, 1); + confirm(ranges[1], 0, 1, 2, 3); + confirm(ranges[2], 2, 3, 0, 1); + confirm(ranges[3], 2, 3, 2, 3); assertEquals(recordData.length+4, record.getRecordSize()); byte[] output = record.serialize(); assertEquals("Output size", recordData.length+4, output.length); //includes sid+recordlength - for (int i = 0; i < recordData.length;i++) + for (int i = 0; i < recordData.length; i++) { assertEquals("CFHeaderRecord doesn't match", recordData[i], output[i+4]); } } + public void testExtremeRows() { + byte[] recordData = { + (byte)0x13, (byte)0x00, // nFormats + (byte)0x00, (byte)0x00, + + (byte)0x00, (byte)0x00, + (byte)0xFF, (byte)0xFF, + (byte)0x00, (byte)0x00, + (byte)0xFF, (byte)0x00, + + (byte)0x03, (byte)0x00, // nRegions + + (byte)0x40, (byte)0x9C, + (byte)0x50, (byte)0xC3, + (byte)0x02, (byte)0x00, + (byte)0x02, (byte)0x00, + + (byte)0x00, (byte)0x00, + (byte)0xFF, (byte)0xFF, + (byte)0x05, (byte)0x00, + (byte)0x05, (byte)0x00, + + (byte)0x07, (byte)0x00, + (byte)0x07, (byte)0x00, + (byte)0x00, (byte)0x00, + (byte)0xFF, (byte)0x00, + }; + + CFHeaderRecord record; + try { + record = new CFHeaderRecord(new TestcaseRecordInputStream(CFHeaderRecord.sid, (short)recordData.length, recordData)); + } catch (IllegalArgumentException e) { + if(e.getMessage().equals("invalid cell range (-25536, 2, -15536, 2)")) { + throw new AssertionFailedError("Identified bug 44739b"); + } + throw e; + } + + assertEquals("#CFRULES", 19, record.getNumberOfConditionalFormats()); + assertFalse(record.getNeedRecalculation()); + confirm(record.getEnclosingCellRange(), 0, 65535, 0, 255); + CellRange[] ranges = record.getCellRanges(); + assertEquals(3, ranges.length); + confirm(ranges[0], 40000, 50000, 2, 2); + confirm(ranges[1], 0, 65535, 5, 5); + confirm(ranges[2], 7, 7, 0, 255); + + byte[] output = record.serialize(); + + assertEquals("Output size", recordData.length+4, output.length); //includes sid+recordlength + + for (int i = 0; i < recordData.length;i++) { + assertEquals("CFHeaderRecord doesn't match", recordData[i], output[i+4]); + } + } + + + private static void confirm(CellRange cr, int expFirstRow, int expLastRow, int expFirstCol, int expLastColumn) { + assertEquals("first row", expFirstRow, cr.getFirstRow()); + assertEquals("last row", expLastRow, cr.getLastRow()); + assertEquals("first column", expFirstCol, cr.getFirstColumn()); + assertEquals("last column", expLastColumn, cr.getLastColumn()); + } public static void main(String[] ignored_args) { diff --git a/src/testcases/org/apache/poi/hssf/record/cf/TestCellRange.java b/src/testcases/org/apache/poi/hssf/record/cf/TestCellRange.java index ade097e697..2ba196d55a 100644 --- a/src/testcases/org/apache/poi/hssf/record/cf/TestCellRange.java +++ b/src/testcases/org/apache/poi/hssf/record/cf/TestCellRange.java @@ -17,62 +17,76 @@ limitations under the License. package org.apache.poi.hssf.record.cf; +import junit.framework.AssertionFailedError; import junit.framework.TestCase; /** * Tests CellRange operations. */ -public class TestCellRange extends TestCase +public final class TestCellRange extends TestCase { - private static final CellRange biggest = new CellRange( 0, -1, 0,-1); - private static final CellRange tenthColumn = new CellRange( 0, -1,10,10); - private static final CellRange tenthRow = new CellRange(10, 10, 0,-1); - private static final CellRange box10x10 = new CellRange( 0, 10, 0,10); - private static final CellRange box9x9 = new CellRange( 0, 9, 0, 9); - private static final CellRange box10to20c = new CellRange( 0, 10,10,20); - private static final CellRange oneCell = new CellRange(10, 10,10,10); + private static final CellRange biggest = createCR( 0, -1, 0,-1); + private static final CellRange tenthColumn = createCR( 0, -1,10,10); + private static final CellRange tenthRow = createCR(10, 10, 0,-1); + private static final CellRange box10x10 = createCR( 0, 10, 0,10); + private static final CellRange box9x9 = createCR( 0, 9, 0, 9); + private static final CellRange box10to20c = createCR( 0, 10,10,20); + private static final CellRange oneCell = createCR(10, 10,10,10); - boolean [][] contanis = new boolean[][] + private static final CellRange[] sampleRanges = { + biggest, tenthColumn, tenthRow, box10x10, box9x9, box10to20c, oneCell, + }; + + /** cross-reference of contains() operations for sampleRanges against itself */ + private static final boolean [][] containsExpectedResults = { - // biggest, tenthColumn, tenthRow, box10x10, box9x9, box10to20c, oneCell - /*biggest */ new boolean[]{true, true , true , true , true , true , true}, - /*tenthColumn*/ new boolean[]{false, true , false, false, false, false, true}, - /*tenthRow */ new boolean[]{false, false, true , false, false, false, true}, - /*box10x10 */ new boolean[]{false, false, false, true , true , false, true}, - /*box9x9 */ new boolean[]{false, false, false, false, true , false, false}, - /*box10to20c */ new boolean[]{false, false, false, false, false, true , true}, - /*oneCell */ new boolean[]{false, false, false, false, false, false, true}, + // biggest, tenthColumn, tenthRow, box10x10, box9x9, box10to20c, oneCell + /*biggest */ {true, true , true , true , true , true , true}, + /*tenthColumn*/ {false, true , false, false, false, false, true}, + /*tenthRow */ {false, false, true , false, false, false, true}, + /*box10x10 */ {false, false, false, true , true , false, true}, + /*box9x9 */ {false, false, false, false, true , false, false}, + /*box10to20c */ {false, false, false, false, false, true , true}, + /*oneCell */ {false, false, false, false, false, false, true}, } ; - - - public void testContainsMethod() - { - CellRange [] ranges = new CellRange[]{biggest,tenthColumn,tenthRow,box10x10,box9x9,box10to20c,oneCell}; - testContainsMethod(contanis,ranges); + + /** + * @param lastRow pass -1 for max row index + * @param lastCol pass -1 for max col index + */ + private static CellRange createCR(int firstRow, int lastRow, int firstCol, int lastCol) { + // max row & max col limit as per BIFF8 + return new CellRange( + firstRow, + lastRow == -1 ? 0xFFFF : lastRow, + firstCol, + lastCol == -1 ? 0x00FF : lastCol); } - private void testContainsMethod(boolean[][]contains,CellRange[] ranges) + public void testContainsMethod() { + CellRange [] ranges = sampleRanges; for(int i=0; i!=ranges.length;i++) { for(int j=0; j!=ranges.length;j++) { - assertEquals("("+i+","+j+"): ",contains[i][j],ranges[i].contains(ranges[j])); + boolean expectedResult = containsExpectedResults[i][j]; + assertEquals("("+i+","+j+"): ", expectedResult, ranges[i].contains(ranges[j])); } } } - private static final CellRange col1 = new CellRange( 0, -1, 1,1); - private static final CellRange col2 = new CellRange( 0, -1, 2,2); - private static final CellRange row1 = new CellRange( 1, 1, 0,-1); - private static final CellRange row2 = new CellRange( 2, 2, 0,-1); + private static final CellRange col1 = createCR( 0, -1, 1,1); + private static final CellRange col2 = createCR( 0, -1, 2,2); + private static final CellRange row1 = createCR( 1, 1, 0,-1); + private static final CellRange row2 = createCR( 2, 2, 0,-1); - private static final CellRange box0 = new CellRange( 0, 2, 0,2); - private static final CellRange box1 = new CellRange( 0, 1, 0,1); - private static final CellRange box2 = new CellRange( 0, 1, 2,3); - private static final CellRange box3 = new CellRange( 2, 3, 0,1); - private static final CellRange box4 = new CellRange( 2, 3, 2,3); - private static final CellRange box5 = new CellRange( 1, 3, 1,3); + private static final CellRange box0 = createCR( 0, 2, 0,2); + private static final CellRange box1 = createCR( 0, 1, 0,1); + private static final CellRange box2 = createCR( 0, 1, 2,3); + private static final CellRange box3 = createCR( 2, 3, 0,1); + private static final CellRange box4 = createCR( 2, 3, 2,3); + private static final CellRange box5 = createCR( 1, 3, 1,3); public void testHasSharedBorderMethod() { @@ -119,8 +133,8 @@ public class TestCellRange extends TestCase public void testIntersectMethod() { - assertEquals( CellRange.OVERLAP,box0.intersect(box5)); - assertEquals( CellRange.OVERLAP,box5.intersect(box0)); + assertEquals(CellRange.OVERLAP,box0.intersect(box5)); + assertEquals(CellRange.OVERLAP,box5.intersect(box0)); assertEquals(CellRange.NO_INTERSECTION,box1.intersect(box4)); assertEquals(CellRange.NO_INTERSECTION,box4.intersect(box1)); assertEquals(CellRange.NO_INTERSECTION,box2.intersect(box3)); @@ -135,4 +149,31 @@ public class TestCellRange extends TestCase assertEquals(CellRange.INSIDE,tenthColumn.intersect(tenthColumn)); assertEquals(CellRange.INSIDE,tenthRow.intersect(tenthRow)); } + + /** + * Cell ranges like the following are valid + * =$C:$IV,$B$1:$B$8,$B$10:$B$65536,$A:$A + */ + public void testCreate() { + CellRange cr; + + cr = createCR(0, -1, 2, 255); // $C:$IV + confirmRange(cr, false, true); + cr = createCR(0, 7, 1, 1); // $B$1:$B$8 + + try { + cr = createCR(9, -1, 1, 1); // $B$65536 + } catch (IllegalArgumentException e) { + if(e.getMessage().startsWith("invalid cell range")) { + throw new AssertionFailedError("Identified bug 44739"); + } + throw e; + } + cr = createCR(0, -1, 0, 0); // $A:$A + } + + private static void confirmRange(CellRange cr, boolean isFullRow, boolean isFullColumn) { + assertEquals("isFullRowRange", isFullRow, cr.isFullRowRange()); + assertEquals("isFullColumnRange", isFullColumn, cr.isFullColumnRange()); + } }