From 7edecc34bf07429e1759714b20edf0017dde7089 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 | 45 +++-- .../apache/poi/hwpf/usermodel/TableRow.java | 184 ++++++++++-------- .../poi/hwpf/usermodel/TestProblems.java | 73 ++++--- 5 files changed, 216 insertions(+), 123 deletions(-) diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index b87e29f6f6..6506898626 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 3ccb5b9379..68bdc127f3 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 c37f76ff80..246f01a5e1 100644 --- a/src/scratchpad/src/org/apache/poi/hwpf/usermodel/Table.java +++ b/src/scratchpad/src/org/apache/poi/hwpf/usermodel/Table.java @@ -23,14 +23,35 @@ 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; + initRows(); + } + + public TableRow getRow( int index ) + { + initRows(); + return _rows.get( index ); + } + public int getTableLevel() + { + return _tableLevel; + } + + private void initRows() + { + if ( _rowsFound ) + return; + + _rows = new ArrayList(); int rowStart = 0; int rowEnd = 0; @@ -39,27 +60,25 @@ public final class Table extends Range { Paragraph p = getParagraph( rowEnd ); rowEnd++; - if ( p.isTableRowEnd() && p.getTableLevel() == levelNum ) + if ( p.isTableRowEnd() && p.getTableLevel() == _tableLevel ) { - _rows.add( new TableRow( rowStart, rowEnd, this, levelNum ) ); + _rows.add( new TableRow( rowStart, rowEnd, this, _tableLevel ) ); rowStart = rowEnd; } } + _rowsFound = true; } - public TableRow getRow( int index ) - { - return _rows.get( index ); - } - - public int getTableLevel() + public int numRows() { - return _tableLevel; + initRows(); + return _rows.size(); } - public int numRows() + @Override + protected void reset() { - return _rows.size(); + _rowsFound = false; } public int type() 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 95d378480e..a7dc3b2fe6 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 static final POILogger logger = POILogFactory + .getLogger( TableRow.class ); - private final static short SPRM_TJC = 0x5400; 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_DYAROWHEIGHT = (short) 0x9407; + private final static short SPRM_TJC = 0x5400; - private static final POILogger logger = POILogFactory - .getLogger( TableRow.class ); + 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,33 +189,36 @@ 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(); + _cellsFound = false; } - public void setGapHalf( int dxaGapHalf ) + public void setCantSplit( boolean cantSplit ) { - _tprops.setDxaGapHalf( dxaGapHalf ); - _papx.updateSprm( SPRM_DXAGAPHALF, (short) dxaGapHalf ); + _tprops.setFCantSplit( cantSplit ); + _papx.updateSprm( SPRM_FCANTSPLIT, (byte) ( cantSplit ? 1 : 0 ) ); } - public int getRowHeight() + public void setGapHalf( int dxaGapHalf ) { - return _tprops.getDyaRowHeight(); + _tprops.setDxaGapHalf( dxaGapHalf ); + _papx.updateSprm( SPRM_DXAGAPHALF, (short) dxaGapHalf ); } public void setRowHeight( int dyaRowHeight ) @@ -154,20 +227,10 @@ public final class TableRow extends Paragraph _papx.updateSprm( SPRM_DYAROWHEIGHT, (short) dyaRowHeight ); } - public boolean cantSplit() - { - return _tprops.getFCantSplit(); - } - - public void setCantSplit( boolean cantSplit ) - { - _tprops.setFCantSplit( cantSplit ); - _papx.updateSprm( SPRM_FCANTSPLIT, (byte) ( cantSplit ? 1 : 0 ) ); - } - - public boolean isTableHeader() + public void setRowJustification( int jc ) { - return _tprops.getFTableHeader(); + _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 6a2fce237c..f0664b3b03 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); - - 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)); + 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 ); + 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 ); } /** -- 2.39.5