From dd2005da091a4d4996eae9de5dbf761ab5e2e343 Mon Sep 17 00:00:00 2001 From: Sergey Vladimirov Date: Thu, 7 Jul 2011 12:52:57 +0000 Subject: [PATCH] fix 47563 - Exception when working with table git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1143802 13f79535-47bb-0310-9956-ffa450edef68 --- src/documentation/content/xdocs/status.xml | 1 + .../org/apache/poi/hwpf/usermodel/Range.java | 36 +++- .../org/apache/poi/hwpf/usermodel/Table.java | 53 +++-- .../apache/poi/hwpf/usermodel/TableRow.java | 202 ++++++++++-------- .../poi/hwpf/usermodel/TestProblems.java | 71 +++--- 5 files changed, 228 insertions(+), 135 deletions(-) diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index b87e29f6f..650689862 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + 47563 - Exception when working with table 47287 - StringIndexOutOfBoundsException in CharacterRun.replaceText() 46817 - Regression: Text from some table cells missing Add getOverallRange() method to HWPFDocumentCore diff --git a/src/scratchpad/src/org/apache/poi/hwpf/usermodel/Range.java b/src/scratchpad/src/org/apache/poi/hwpf/usermodel/Range.java index 3ccb5b937..68bdc127f 100644 --- a/src/scratchpad/src/org/apache/poi/hwpf/usermodel/Range.java +++ b/src/scratchpad/src/org/apache/poi/hwpf/usermodel/Range.java @@ -1046,7 +1046,7 @@ public class Range { // TODO -instantiable superclass /** * resets the list indexes. */ - private void reset() { + protected void reset() { _textRangeFound = false; _charRangeFound = false; _parRangeFound = false; @@ -1141,4 +1141,38 @@ public class Range { // TODO -instantiable superclass return "Range from " + getStartOffset() + " to " + getEndOffset() + " (chars)"; } + + /** + * Method for debug purposes. Checks that all resolved elements are inside + * of current range. + */ + public void sanityCheck() + { + if ( _charRangeFound ) + { + for ( int c = _charStart; c < _charEnd; c++ ) + { + CHPX chpx = _characters.get( c ); + + int left = Math.max( this._start, chpx.getStart() ); + int right = Math.min( this._end, chpx.getEnd() ); + + if ( left >= right ) + throw new AssertionError(); + } + } + if ( _parRangeFound ) + { + for ( int p = _parStart; p < _parEnd; p++ ) + { + PAPX papx = _paragraphs.get( p ); + + int left = Math.max( this._start, papx.getStart() ); + int right = Math.min( this._end, papx.getEnd() ); + + if ( left >= right ) + throw new AssertionError(); + } + } + } } diff --git a/src/scratchpad/src/org/apache/poi/hwpf/usermodel/Table.java b/src/scratchpad/src/org/apache/poi/hwpf/usermodel/Table.java index c37f76ff8..246f01a5e 100644 --- a/src/scratchpad/src/org/apache/poi/hwpf/usermodel/Table.java +++ b/src/scratchpad/src/org/apache/poi/hwpf/usermodel/Table.java @@ -23,32 +23,21 @@ public final class Table extends Range { private ArrayList _rows; + private boolean _rowsFound = false; + private int _tableLevel; - Table( int startIdxInclusive, int endIdxExclusive, Range parent, int levelNum ) + Table( int startIdxInclusive, int endIdxExclusive, Range parent, + int levelNum ) { super( startIdxInclusive, endIdxExclusive, Range.TYPE_PARAGRAPH, parent ); - _rows = new ArrayList(); _tableLevel = levelNum; - - int rowStart = 0; - int rowEnd = 0; - - int numParagraphs = numParagraphs(); - while ( rowEnd < numParagraphs ) - { - Paragraph p = getParagraph( rowEnd ); - rowEnd++; - if ( p.isTableRowEnd() && p.getTableLevel() == levelNum ) - { - _rows.add( new TableRow( rowStart, rowEnd, this, levelNum ) ); - rowStart = rowEnd; - } - } + initRows(); } public TableRow getRow( int index ) { + initRows(); return _rows.get( index ); } @@ -57,11 +46,41 @@ public final class Table extends Range return _tableLevel; } + private void initRows() + { + if ( _rowsFound ) + return; + + _rows = new ArrayList(); + int rowStart = 0; + int rowEnd = 0; + + int numParagraphs = numParagraphs(); + while ( rowEnd < numParagraphs ) + { + Paragraph p = getParagraph( rowEnd ); + rowEnd++; + if ( p.isTableRowEnd() && p.getTableLevel() == _tableLevel ) + { + _rows.add( new TableRow( rowStart, rowEnd, this, _tableLevel ) ); + rowStart = rowEnd; + } + } + _rowsFound = true; + } + public int numRows() { + initRows(); return _rows.size(); } + @Override + protected void reset() + { + _rowsFound = false; + } + public int type() { return TYPE_TABLE; diff --git a/src/scratchpad/src/org/apache/poi/hwpf/usermodel/TableRow.java b/src/scratchpad/src/org/apache/poi/hwpf/usermodel/TableRow.java index 95d378480..a7dc3b2fe 100644 --- a/src/scratchpad/src/org/apache/poi/hwpf/usermodel/TableRow.java +++ b/src/scratchpad/src/org/apache/poi/hwpf/usermodel/TableRow.java @@ -26,20 +26,22 @@ import org.apache.poi.util.POILogger; public final class TableRow extends Paragraph { - private final static char TABLE_CELL_MARK = '\u0007'; - - private final static short SPRM_TJC = 0x5400; - private final static short SPRM_DXAGAPHALF = (short) 0x9602; - private final static short SPRM_FCANTSPLIT = 0x3403; - private final static short SPRM_FTABLEHEADER = 0x3404; - private final static short SPRM_DYAROWHEIGHT = (short) 0x9407; - private static final POILogger logger = POILogFactory .getLogger( TableRow.class ); + private final static short SPRM_DXAGAPHALF = (short) 0x9602; + private final static short SPRM_DYAROWHEIGHT = (short) 0x9407; + private final static short SPRM_FCANTSPLIT = 0x3403; + private final static short SPRM_FTABLEHEADER = 0x3404; + private final static short SPRM_TJC = 0x5400; + + private final static char TABLE_CELL_MARK = '\u0007'; + + private TableCell[] _cells; + private boolean _cellsFound = false; + int _levelNum; private TableProperties _tprops; - private TableCell[] _cells; public TableRow( int startIdxInclusive, int endIdxExclusive, Table parent, int levelNum ) @@ -48,19 +50,88 @@ public final class TableRow extends Paragraph _tprops = TableSprmUncompressor.uncompressTAP( _papx.toByteArray(), 2 ); _levelNum = levelNum; + initCells(); + } + + public boolean cantSplit() + { + return _tprops.getFCantSplit(); + } + + public BorderCode getBarBorder() + { + throw new UnsupportedOperationException( "not applicable for TableRow" ); + } + + public BorderCode getBottomBorder() + { + return _tprops.getBrcBottom(); + } + + public TableCell getCell( int index ) + { + initCells(); + return _cells[index]; + } + + public int getGapHalf() + { + return _tprops.getDxaGapHalf(); + } + + public BorderCode getHorizontalBorder() + { + return _tprops.getBrcHorizontal(); + } + + public BorderCode getLeftBorder() + { + return _tprops.getBrcLeft(); + } + + public BorderCode getRightBorder() + { + return _tprops.getBrcRight(); + } + + public int getRowHeight() + { + return _tprops.getDyaRowHeight(); + } + + public int getRowJustification() + { + return _tprops.getJc(); + } + + public BorderCode getTopBorder() + { + return _tprops.getBrcBottom(); + } + + public BorderCode getVerticalBorder() + { + return _tprops.getBrcVertical(); + } + + private void initCells() + { + if ( _cellsFound ) + return; + final short expectedCellsCount = _tprops.getItcMac(); int lastCellStart = 0; List cells = new ArrayList( expectedCellsCount + 1 ); - for ( int p = 0; p < ( endIdxExclusive - startIdxInclusive ); p++ ) + for ( int p = 0; p < numParagraphs(); p++ ) { Paragraph paragraph = getParagraph( p ); String s = paragraph.text(); if ( ( ( s.length() > 0 && s.charAt( s.length() - 1 ) == TABLE_CELL_MARK ) || paragraph .isEmbeddedCellMark() ) - && paragraph.getTableLevel() == levelNum ) + && paragraph.getTableLevel() == _levelNum ) { TableCellDescriptor tableCellDescriptor = _tprops.getRgtc() != null && _tprops.getRgtc().length > cells.size() ? _tprops @@ -73,14 +144,14 @@ public final class TableRow extends Paragraph .getRgdxaCenter()[cells.size() + 1] : 0; TableCell tableCell = new TableCell( lastCellStart, p + 1, - this, levelNum, tableCellDescriptor, leftEdge, + this, _levelNum, tableCellDescriptor, leftEdge, rightEdge - leftEdge ); cells.add( tableCell ); lastCellStart = p + 1; } } - if ( lastCellStart < ( endIdxExclusive - startIdxInclusive - 1 ) ) + if ( lastCellStart < ( numParagraphs() - 1 ) ) { TableCellDescriptor tableCellDescriptor = _tprops.getRgtc() != null && _tprops.getRgtc().length > cells.size() ? _tprops @@ -93,9 +164,8 @@ public final class TableRow extends Paragraph .getRgdxaCenter()[cells.size() + 1] : 0; TableCell tableCell = new TableCell( lastCellStart, - ( endIdxExclusive - startIdxInclusive - 1 ), this, - levelNum, tableCellDescriptor, leftEdge, rightEdge - - leftEdge ); + ( numParagraphs() - 1 ), this, _levelNum, + tableCellDescriptor, leftEdge, rightEdge - leftEdge ); cells.add( tableCell ); } @@ -119,44 +189,24 @@ public final class TableRow extends Paragraph } _cells = cells.toArray( new TableCell[cells.size()] ); + _cellsFound = true; } - public int getRowJustification() + public boolean isTableHeader() { - return _tprops.getJc(); + return _tprops.getFTableHeader(); } - public void setRowJustification( int jc ) + public int numCells() { - _tprops.setJc( (short) jc ); - _papx.updateSprm( SPRM_TJC, (short) jc ); + initCells(); + return _cells.length; } - public int getGapHalf() + @Override + protected void reset() { - return _tprops.getDxaGapHalf(); - } - - public void setGapHalf( int dxaGapHalf ) - { - _tprops.setDxaGapHalf( dxaGapHalf ); - _papx.updateSprm( SPRM_DXAGAPHALF, (short) dxaGapHalf ); - } - - public int getRowHeight() - { - return _tprops.getDyaRowHeight(); - } - - public void setRowHeight( int dyaRowHeight ) - { - _tprops.setDyaRowHeight( dyaRowHeight ); - _papx.updateSprm( SPRM_DYAROWHEIGHT, (short) dyaRowHeight ); - } - - public boolean cantSplit() - { - return _tprops.getFCantSplit(); + _cellsFound = false; } public void setCantSplit( boolean cantSplit ) @@ -165,9 +215,22 @@ public final class TableRow extends Paragraph _papx.updateSprm( SPRM_FCANTSPLIT, (byte) ( cantSplit ? 1 : 0 ) ); } - public boolean isTableHeader() + public void setGapHalf( int dxaGapHalf ) { - return _tprops.getFTableHeader(); + _tprops.setDxaGapHalf( dxaGapHalf ); + _papx.updateSprm( SPRM_DXAGAPHALF, (short) dxaGapHalf ); + } + + public void setRowHeight( int dyaRowHeight ) + { + _tprops.setDyaRowHeight( dyaRowHeight ); + _papx.updateSprm( SPRM_DYAROWHEIGHT, (short) dyaRowHeight ); + } + + public void setRowJustification( int jc ) + { + _tprops.setJc( (short) jc ); + _papx.updateSprm( SPRM_TJC, (short) jc ); } public void setTableHeader( boolean tableHeader ) @@ -176,49 +239,4 @@ public final class TableRow extends Paragraph _papx.updateSprm( SPRM_FTABLEHEADER, (byte) ( tableHeader ? 1 : 0 ) ); } - public int numCells() - { - return _cells.length; - } - - public TableCell getCell( int index ) - { - return _cells[index]; - } - - public BorderCode getTopBorder() - { - return _tprops.getBrcBottom(); - } - - public BorderCode getBottomBorder() - { - return _tprops.getBrcBottom(); - } - - public BorderCode getLeftBorder() - { - return _tprops.getBrcLeft(); - } - - public BorderCode getRightBorder() - { - return _tprops.getBrcRight(); - } - - public BorderCode getHorizontalBorder() - { - return _tprops.getBrcHorizontal(); - } - - public BorderCode getVerticalBorder() - { - return _tprops.getBrcVertical(); - } - - public BorderCode getBarBorder() - { - throw new UnsupportedOperationException( "not applicable for TableRow" ); - } - } diff --git a/src/scratchpad/testcases/org/apache/poi/hwpf/usermodel/TestProblems.java b/src/scratchpad/testcases/org/apache/poi/hwpf/usermodel/TestProblems.java index 6a2fce237..f0664b3b0 100644 --- a/src/scratchpad/testcases/org/apache/poi/hwpf/usermodel/TestProblems.java +++ b/src/scratchpad/testcases/org/apache/poi/hwpf/usermodel/TestProblems.java @@ -599,42 +599,63 @@ public final class TestProblems extends HWPFTestCase { assertFalse( docText.contains( "1-15" ) ); } - private static void insertTable(int rows, int columns) { + private static void insertTable( int rows, int columns ) + { // POI apparently can't create a document from scratch, // so we need an existing empty dummy document - HWPFDocument doc = HWPFTestDataSamples.openSampleFile("empty.doc"); + HWPFDocument doc = HWPFTestDataSamples.openSampleFile( "empty.doc" ); Range range = doc.getRange(); - Table table = range.insertBefore(new TableProperties((short) columns), rows); + Table table = range.insertBefore( + new TableProperties( (short) columns ), rows ); + table.sanityCheck(); + range.sanityCheck(); - for (int rowIdx = 0; rowIdx < table.numRows(); rowIdx++) { - TableRow row = table.getRow(rowIdx); - for (int colIdx = 0; colIdx < row.numCells(); colIdx++) { - TableCell cell = row.getCell(colIdx); - Paragraph par = cell.getParagraph(0); - par.insertBefore("" + (rowIdx * row.numCells() + colIdx)); + for ( int rowIdx = 0; rowIdx < table.numRows(); rowIdx++ ) + { + TableRow row = table.getRow( rowIdx ); + row.sanityCheck(); + for ( int colIdx = 0; colIdx < row.numCells(); colIdx++ ) + { + TableCell cell = row.getCell( colIdx ); + cell.sanityCheck(); + + Paragraph par = cell.getParagraph( 0 ); + par.sanityCheck(); + + par.insertBefore( "" + ( rowIdx * row.numCells() + colIdx ) ); + + par.sanityCheck(); + cell.sanityCheck(); + row.sanityCheck(); + table.sanityCheck(); + range.sanityCheck(); } } + + String text = range.text(); + int mustBeAfter = 0; + for ( int i = 0; i < rows * columns; i++ ) + { + int next = text.indexOf( Integer.toString( i ), mustBeAfter ); + assertFalse( next == -1 ); + mustBeAfter = next; + } } /** - * [FAILING] Bug 47563 - HWPF failing while creating tables, + * [RESOLVED FIXED] Bug 47563 - Exception when working with table */ - public void test47563() { - try { - insertTable(1, 5); - insertTable(1, 6); - insertTable(5, 1); - insertTable(6, 1); - insertTable(2, 2); - insertTable(3, 2); - insertTable(2, 3); - insertTable(3, 3); - - fixed("47563"); - } catch (Exception e) { - // expected exception - } + public void test47563() + { + insertTable( 1, 5 ); + insertTable( 1, 6 ); + insertTable( 5, 1 ); + insertTable( 6, 1 ); + insertTable( 2, 2 ); + insertTable( 3, 2 ); + insertTable( 2, 3 ); + insertTable( 3, 3 ); } /**