From c0c5b69473cda3937fdc949c385f80fd309cfa15 Mon Sep 17 00:00:00 2001 From: Nick Burch Date: Fri, 6 Jun 2008 11:36:03 +0000 Subject: [PATCH] Merged revisions 638786-638802,638805-638811,638813-638814,638816-639230,639233-639241,639243-639253,639255-639486,639488-639601,639603-639835,639837-639917,639919-640056,640058-640710,640712-641156,641158-641184,641186-641795,641797-641798,641800-641933,641935-641963,641965-641966,641968-641995,641997-642230,642232-642562,642564-642565,642568-642570,642572-642573,642576-642736,642739-642877,642879,642881-642890,642892-642903,642905-642945,642947-643624,643626-643653,643655-643669,643671,643673-643830,643832-643833,643835-644342,644344-644472,644474-644508,644510-645347,645349-645351,645353-645559,645561-645565,645568-645951,645953-646193,646195-646311,646313-646404,646406-646665,646667-646853,646855-646869,646871-647151,647153-647185,647187-647277,647279-647566,647568-647573,647575,647578-647711,647714-647737,647739-647823,647825-648155,648157-648202,648204-648273,648275,648277-648302,648304-648333,648335-648588,648590-648622,648625-648673,648675-649141,649144,649146-649556,649558-649795,649799,649801-649910,649912-649913,649915-650128,650131-650132,650134-650137,650140-650914,650916-651991,651993-652284,652286-652287,652289,652291,652293-652297,652299-652328,652330-652425,652427-652445,652447-652560,652562-652933,652935,652937-652993,652995-653116,653118-653124,653126-653483,653487-653519,653522-653550,653552-653607,653609-653667,653669-653674,653676-653814,653817-653830,653832-653891,653893-653944,653946-654055,654057-654355,654357-654365,654367-654648,654651-655215,655217-655277,655279-655281,655283-655911,655913-656212,656214,656216-656251,656253-656698,656700-656756,656758-656892,656894-657135,657137-657165,657168-657179,657181-657354,657356-657357,657359-657701,657703-657874,657876-658032,658034-658284,658286,658288-658301,658303-658307,658309-658321,658323-658335,658337-658348,658351,658353-658832,658834-658983,658985,658987-659066,659068-659402,659404-659428,659430-659451,659453-659454,659456-659461,659463-659477,659479-659524,659526-659571,659574-663870 via svnmerge from https://svn.apache.org:443/repos/asf/poi/trunk ........ r659575 | nick | 2008-05-23 16:55:08 +0100 (Fri, 23 May 2008) | 1 line Help for bug #44840 - Improved handling of HSSFObjectData, especially for entries with data held not in POIFS ........ r660256 | josh | 2008-05-26 19:02:23 +0100 (Mon, 26 May 2008) | 1 line Follow-on fix for bug 42564 (r653668). Array elements are stored internally column by column. ........ r660263 | josh | 2008-05-26 19:25:02 +0100 (Mon, 26 May 2008) | 1 line Small fix for FormulaParser. Need case-insentive match for IF function name ........ r660280 | josh | 2008-05-26 20:36:56 +0100 (Mon, 26 May 2008) | 1 line Added test cases for parsing IF expressions. Segregated IF test cases into a new class ........ r660344 | josh | 2008-05-27 01:57:23 +0100 (Tue, 27 May 2008) | 1 line Changed class hierarchy of Ptg to improve 'operand class' transformation. ........ r660474 | nick | 2008-05-27 12:44:49 +0100 (Tue, 27 May 2008) | 1 line X, Y, Width and Height getters/setters on HSSFChart ........ r660828 | josh | 2008-05-28 07:19:31 +0100 (Wed, 28 May 2008) | 1 line Fix for 45060 (and 45041) - Improved token class transformation during formula parsing ........ r660834 | yegor | 2008-05-28 07:50:35 +0100 (Wed, 28 May 2008) | 1 line bump 3.1-beta2 announcement ........ r660889 | nick | 2008-05-28 11:03:00 +0100 (Wed, 28 May 2008) | 1 line Fix bug #45087 - Correctly detect date formats like [Black]YYYY as being date based ........ r663322 | josh | 2008-06-04 18:37:18 +0100 (Wed, 04 Jun 2008) | 1 line Test code clean-up (prior to bug 45126) ........ r663436 | josh | 2008-06-05 04:12:35 +0100 (Thu, 05 Jun 2008) | 1 line Fix for bug 45123 - SharedFormulaRecord.convertSharedFormulas was ignoring token operand classes ........ r663765 | josh | 2008-06-05 23:24:05 +0100 (Thu, 05 Jun 2008) | 1 line Fix for bug 45145 - made sure RowRecordsAggregate comes before ValueRecordsAggregate. Also fixed BiffViewer to show correct record offsets ........ r663855 | josh | 2008-06-06 09:32:54 +0100 (Fri, 06 Jun 2008) | 1 line Fix for 45133 - OBJ Record (5Dh) needs to pad the sub-record data to a 4-byte boundary ........ git-svn-id: https://svn.apache.org/repos/asf/poi/branches/ooxml@663901 13f79535-47bb-0310-9956-ffa450edef68 --- src/documentation/content/xdocs/changes.xml | 3 + src/documentation/content/xdocs/status.xml | 3 + .../org/apache/poi/hssf/dev/BiffViewer.java | 3 +- src/java/org/apache/poi/hssf/model/Sheet.java | 109 +++++---- .../org/apache/poi/hssf/record/ObjRecord.java | 61 ++---- .../poi/hssf/record/SharedFormulaRecord.java | 10 +- .../aggregates/ValueRecordsAggregate.java | 2 +- .../poi/ss/usermodel/FormulaEvaluator.java | 7 + .../org/apache/poi/hssf/model/TestSheet.java | 110 ++++++++-- .../poi/hssf/record/AllRecordTests.java | 1 + .../apache/poi/hssf/record/TestObjRecord.java | 45 +++- .../hssf/record/TestSharedFormulaRecord.java | 97 +++++++++ .../poi/hssf/usermodel/TestNamedRange.java | 206 ++++++------------ 13 files changed, 401 insertions(+), 256 deletions(-) create mode 100644 src/testcases/org/apache/poi/hssf/record/TestSharedFormulaRecord.java diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml index 5d7dce206..267811d78 100644 --- a/src/documentation/content/xdocs/changes.xml +++ b/src/documentation/content/xdocs/changes.xml @@ -46,6 +46,9 @@ Created a common interface for handling Excel files, irrespective of if they are .xls or .xlsx + 45133 - Fixed OBJ Record (5Dh) to pad the sub-record data to a 4-byte boundary + 45145 - Fixed Sheet to always enforce RowRecordsAggregate before ValueRecordsAggregate + 45123 - Fixed SharedFormulaRecord.convertSharedFormulas() to propagate token operand classes 45087 - Correctly detect date formats like [Black]YYYY as being date based 45060 - Improved token class transformation during formula parsing 44840 - Improved handling of HSSFObjectData, especially for entries with data held not in POIFS diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index 68028d539..115db4e10 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -43,6 +43,9 @@ Created a common interface for handling Excel files, irrespective of if they are .xls or .xlsx + 45133 - Fixed OBJ Record (5Dh) to pad the sub-record data to a 4-byte boundary + 45145 - Fixed Sheet to always enforce RowRecordsAggregate before ValueRecordsAggregate + 45123 - Fixed SharedFormulaRecord.convertSharedFormulas() to propagate token operand classes 45087 - Correctly detect date formats like [Black]YYYY as being date based 45060 - Improved token class transformation during formula parsing 44840 - Improved handling of HSSFObjectData, especially for entries with data held not in POIFS diff --git a/src/java/org/apache/poi/hssf/dev/BiffViewer.java b/src/java/org/apache/poi/hssf/dev/BiffViewer.java index cd4365191..9396e679d 100644 --- a/src/java/org/apache/poi/hssf/dev/BiffViewer.java +++ b/src/java/org/apache/poi/hssf/dev/BiffViewer.java @@ -87,7 +87,8 @@ public final class BiffViewer { records.add(record); if (activeRecord != null) activeRecord.dump(ps); - activeRecord = new RecordDetails(recStream.getSid(), recStream.getLength(), (int)recStream.getPos(), record); + int startPos = (int)(recStream.getPos()-recStream.getLength() - 4); + activeRecord = new RecordDetails(recStream.getSid(), recStream.getLength(), startPos, record); } if (dump) { recStream.dumpBytes(ps); diff --git a/src/java/org/apache/poi/hssf/model/Sheet.java b/src/java/org/apache/poi/hssf/model/Sheet.java index 871ecc100..b95ad6bba 100644 --- a/src/java/org/apache/poi/hssf/model/Sheet.java +++ b/src/java/org/apache/poi/hssf/model/Sheet.java @@ -66,7 +66,7 @@ public final class Sheet implements Model { protected ArrayList records = null; int preoffset = 0; // offset of the sheet in a new file int loc = 0; - protected int dimsloc = 0; + protected int dimsloc = -1; // TODO - is it legal for dims record to be missing? protected DimensionsRecord dims; protected DefaultColWidthRecord defaultcolwidth = null; protected DefaultRowHeightRecord defaultrowheight = null; @@ -295,6 +295,8 @@ public final class Sheet implements Model { } else if ( rec.getSid() == IndexRecord.sid ) { + // ignore INDEX record because it is only needed by Excel, + // and POI always re-calculates its contents rec = null; } @@ -329,8 +331,8 @@ public final class Sheet implements Model { } } retval.records = records; - retval.checkCells(); retval.checkRows(); + retval.checkCells(); if (log.check( POILogger.DEBUG )) log.log(POILogger.DEBUG, "sheet createSheet (existing file) exited"); return retval; @@ -486,7 +488,15 @@ public final class Sheet implements Model { if (cells == null) { cells = new ValueRecordsAggregate(); - records.add(getDimsLoc() + 1, cells); + // In the worksheet stream, the row records always occur before the cell (value) + // records. Therefore POI's aggregates (RowRecordsAggregate, ValueRecordsAggregate) + // should follow suit. Some methods in this class tolerate either order, while + // others have been found to fail (see bug 45145). + int rraIndex = getDimsLoc() + 1; + if (records.get(rraIndex).getClass() != RowRecordsAggregate.class) { + throw new IllegalStateException("Cannot create value records before row records exist"); + } + records.add(rraIndex+1, cells); } } @@ -836,46 +846,61 @@ public final class Sheet implements Model { return pos-offset; } - private int serializeIndexRecord(final int BOFRecordIndex, final int offset, byte[] data) { - IndexRecord index = new IndexRecord(); - index.setFirstRow(rows.getFirstRowNum()); - index.setLastRowAdd1(rows.getLastRowNum()+1); - //Calculate the size of the records from the end of the BOF - //and up to the RowRecordsAggregate... - int sheetRecSize = 0; - for (int j = BOFRecordIndex+1; j < records.size(); j++) - { - Record tmpRec = (( Record ) records.get(j)); - if (tmpRec instanceof UncalcedRecord) { - continue; - } - if (tmpRec instanceof RowRecordsAggregate) { - break; - } - sheetRecSize+= tmpRec.getRecordSize(); - } - if (_isUncalced) { - sheetRecSize += UncalcedRecord.getStaticRecordSize(); - } - //Add the references to the DBCells in the IndexRecord (one for each block) - int blockCount = rows.getRowBlockCount(); - //Calculate the size of this IndexRecord - int indexRecSize = IndexRecord.getRecordSizeForBlockCount(blockCount); + /** + * @param indexRecordOffset also happens to be the end of the BOF record + * @return the size of the serialized INDEX record + */ + private int serializeIndexRecord(final int bofRecordIndex, final int indexRecordOffset, + byte[] data) { + IndexRecord index = new IndexRecord(); + index.setFirstRow(rows.getFirstRowNum()); + index.setLastRowAdd1(rows.getLastRowNum() + 1); + // Calculate the size of the records from the end of the BOF + // and up to the RowRecordsAggregate... - int rowBlockOffset = 0; - int cellBlockOffset = 0; - int dbCellOffset = 0; - for (int block=0;block= 4) { subrecords.add(new EndSubRecord()); } - - /* JMH the size present/not present in the code below - needs to be considered in the RecordInputStream?? - int pos = offset; - while (pos - offset <= size-2) // atleast one "short" must be present - { - short subRecordSid = LittleEndian.getShort(data, pos); - short subRecordSize = -1; // set default to "< 0" - if (pos-offset <= size-4) { // see if size info is present, else default to -1 - subRecordSize = LittleEndian.getShort(data, pos + 2); - } - Record subRecord = SubRecord.createSubRecord(subRecordSid, subRecordSize, data, pos + 4); - subrecords.add(subRecord); - pos += subRecord.getRecordSize(); - }*/ } public String toString() @@ -140,6 +126,8 @@ public class ObjRecord Record record = (Record) iterator.next(); pos += record.serialize(pos, data); } + // assume padding (if present) does not need to be written. + // it is probably zero already, and it probably doesn't matter anyway return getRecordSize(); } @@ -155,7 +143,9 @@ public class ObjRecord Record record = (Record) iterator.next(); size += record.getRecordSize(); } - return 4 + size; + int oddBytes = size & 0x03; + int padding = oddBytes == 0 ? 0 : 4 - oddBytes; + return 4 + size + padding; } public short getSid() @@ -192,9 +182,4 @@ public class ObjRecord return rec; } - -} // END OF CLASS - - - - +} diff --git a/src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java b/src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java index 2b0c50d12..e4c0f28ea 100755 --- a/src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java +++ b/src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java @@ -201,6 +201,10 @@ public final class SharedFormulaRecord extends Record { if (ptgs != null) for (int k = 0; k < ptgs.size(); k++) { Ptg ptg = (Ptg) ptgs.get(k); + byte originalOperandClass = -1; + if (!ptg.isBaseToken()) { + originalOperandClass = ptg.getPtgClass(); + } if (ptg instanceof RefNPtg) { RefNPtg refNPtg = (RefNPtg)ptg; ptg = new ReferencePtg(fixupRelativeRow(formulaRow,refNPtg.getRow(),refNPtg.isRowRelative()), @@ -249,7 +253,11 @@ public final class SharedFormulaRecord extends Record { areaNAPtg.isLastRowRelative(), areaNAPtg.isFirstColRelative(), areaNAPtg.isLastColRelative()); - } + } + if (!ptg.isBaseToken()) { + ptg.setClass(originalOperandClass); + } + newPtgStack.add(ptg); } return newPtgStack; diff --git a/src/java/org/apache/poi/hssf/record/aggregates/ValueRecordsAggregate.java b/src/java/org/apache/poi/hssf/record/aggregates/ValueRecordsAggregate.java index 068896952..fb4bfd59a 100644 --- a/src/java/org/apache/poi/hssf/record/aggregates/ValueRecordsAggregate.java +++ b/src/java/org/apache/poi/hssf/record/aggregates/ValueRecordsAggregate.java @@ -37,7 +37,7 @@ import java.util.List; public final class ValueRecordsAggregate extends Record { - public final static short sid = -1000; + public final static short sid = -1001; // 1000 clashes with RowRecordsAggregate int firstcell = -1; int lastcell = -1; CellValueRecordInterface[][] records; diff --git a/src/java/org/apache/poi/ss/usermodel/FormulaEvaluator.java b/src/java/org/apache/poi/ss/usermodel/FormulaEvaluator.java index 0ecde6304..88d8dcecd 100644 --- a/src/java/org/apache/poi/ss/usermodel/FormulaEvaluator.java +++ b/src/java/org/apache/poi/ss/usermodel/FormulaEvaluator.java @@ -337,10 +337,17 @@ public class FormulaEvaluator { // since we don't know how to handle these yet :( Ptg ptg = ptgs[i]; +<<<<<<< .mine + if (ptg instanceof ControlPtg) { + // skip Parentheses, Attr, etc + continue; + } +======= if (ptg instanceof ControlPtg) { // skip Parentheses, Attr, etc continue; } +>>>>>>> .r663896 if (ptg instanceof MemErrPtg) { continue; } if (ptg instanceof MissingArgPtg) { continue; } if (ptg instanceof NamePtg) { diff --git a/src/testcases/org/apache/poi/hssf/model/TestSheet.java b/src/testcases/org/apache/poi/hssf/model/TestSheet.java index 3bdcae57c..ef21cc9b3 100644 --- a/src/testcases/org/apache/poi/hssf/model/TestSheet.java +++ b/src/testcases/org/apache/poi/hssf/model/TestSheet.java @@ -19,11 +19,15 @@ package org.apache.poi.hssf.model; import junit.framework.AssertionFailedError; import junit.framework.TestCase; + +import org.apache.poi.hssf.eventmodel.ERFListener; +import org.apache.poi.hssf.eventmodel.EventRecordFactory; import org.apache.poi.hssf.record.*; import org.apache.poi.hssf.record.aggregates.ColumnInfoRecordsAggregate; import org.apache.poi.hssf.record.aggregates.RowRecordsAggregate; import org.apache.poi.hssf.record.aggregates.ValueRecordsAggregate; +import java.io.ByteArrayInputStream; import java.util.ArrayList; import java.util.Iterator; import java.util.List; @@ -34,8 +38,7 @@ import java.util.List; * @author Glen Stampoultzis (glens at apache.org) */ public final class TestSheet extends TestCase { - public void testCreateSheet() throws Exception - { + public void testCreateSheet() { // Check we're adding row and cell aggregates List records = new ArrayList(); records.add( new BOFRecord() ); @@ -52,8 +55,7 @@ public final class TestSheet extends TestCase { assertTrue( sheet.records.get(pos++) instanceof EOFRecord ); } - public void testAddMergedRegion() - { + public void testAddMergedRegion() { Sheet sheet = Sheet.createSheet(); int regionsToAdd = 4096; int startRecords = sheet.getRecords().size(); @@ -91,8 +93,7 @@ public final class TestSheet extends TestCase { } } - public void testRemoveMergedRegion() - { + public void testRemoveMergedRegion() { Sheet sheet = Sheet.createSheet(); int regionsToAdd = 4096; @@ -139,13 +140,11 @@ public final class TestSheet extends TestCase { assertEquals("Should be no more merged regions", 0, sheet.getNumMergedRegions()); } - public void testGetMergedRegionAt() - { + public void testGetMergedRegionAt() { //TODO } - public void testGetNumMergedRegions() - { + public void testGetNumMergedRegions() { //TODO } @@ -163,14 +162,13 @@ public final class TestSheet extends TestCase { Sheet sheet = Sheet.createSheet(records, 0); assertNotNull("Row [2] was skipped", sheet.getRow(2)); - } /** * Make sure page break functionality works (in memory) * */ - public void testRowPageBreaks(){ + public void testRowPageBreaks() { short colFrom = 0; short colTo = 255; @@ -226,7 +224,7 @@ public final class TestSheet extends TestCase { * Make sure column pag breaks works properly (in-memory) * */ - public void testColPageBreaks(){ + public void testColPageBreaks() { short rowFrom = 0; short rowTo = (short)65535; @@ -292,20 +290,20 @@ public final class TestSheet extends TestCase { final short DEFAULT_IDX = 0xF; // 15 short xfindex = Short.MIN_VALUE; Sheet sheet = Sheet.createSheet(); - + // without ColumnInfoRecord xfindex = sheet.getXFIndexForColAt((short) 0); assertEquals(DEFAULT_IDX, xfindex); xfindex = sheet.getXFIndexForColAt((short) 1); assertEquals(DEFAULT_IDX, xfindex); - + ColumnInfoRecord nci = ( ColumnInfoRecord ) sheet.createColInfo(); sheet.columns.insertColumn(nci); - + // single column ColumnInfoRecord nci.setFirstColumn((short) 2); nci.setLastColumn((short) 2); - nci.setXFIndex(TEST_IDX); + nci.setXFIndex(TEST_IDX); xfindex = sheet.getXFIndexForColAt((short) 0); assertEquals(DEFAULT_IDX, xfindex); xfindex = sheet.getXFIndexForColAt((short) 1); @@ -318,7 +316,7 @@ public final class TestSheet extends TestCase { // ten column ColumnInfoRecord nci.setFirstColumn((short) 2); nci.setLastColumn((short) 11); - nci.setXFIndex(TEST_IDX); + nci.setXFIndex(TEST_IDX); xfindex = sheet.getXFIndexForColAt((short) 1); assertEquals(DEFAULT_IDX, xfindex); xfindex = sheet.getXFIndexForColAt((short) 2); @@ -333,7 +331,7 @@ public final class TestSheet extends TestCase { // single column ColumnInfoRecord starting at index 0 nci.setFirstColumn((short) 0); nci.setLastColumn((short) 0); - nci.setXFIndex(TEST_IDX); + nci.setXFIndex(TEST_IDX); xfindex = sheet.getXFIndexForColAt((short) 0); assertEquals(TEST_IDX, xfindex); xfindex = sheet.getXFIndexForColAt((short) 1); @@ -342,7 +340,7 @@ public final class TestSheet extends TestCase { // ten column ColumnInfoRecord starting at index 0 nci.setFirstColumn((short) 0); nci.setLastColumn((short) 9); - nci.setXFIndex(TEST_IDX); + nci.setXFIndex(TEST_IDX); xfindex = sheet.getXFIndexForColAt((short) 0); assertEquals(TEST_IDX, xfindex); xfindex = sheet.getXFIndexForColAt((short) 7); @@ -354,7 +352,7 @@ public final class TestSheet extends TestCase { } /** - * Prior to bug 45066, POI would get the estimated sheet size wrong + * Prior to bug 45066, POI would get the estimated sheet size wrong * when an UncalcedRecord was present.

*/ public void testUncalcSize_bug45066() { @@ -363,7 +361,7 @@ public final class TestSheet extends TestCase { records.add(new BOFRecord()); records.add(new UncalcedRecord()); records.add(new EOFRecord()); - Sheet sheet = Sheet.createSheet( records, 0, 0 ); + Sheet sheet = Sheet.createSheet(records, 0, 0); int estimatedSize = sheet.getSize(); int serializedSize = sheet.serialize(0, new byte[estimatedSize]); @@ -372,5 +370,73 @@ public final class TestSheet extends TestCase { } assertEquals(50, serializedSize); } + + /** + * Prior to bug 45145 RowRecordsAggregate and ValueRecordsAggregate could + * sometimes occur in reverse order. This test reproduces one of those situations and makes + * sure that RRA comes before VRA.
+ * + * The code here represents a normal POI use case where a spreadsheet is created from scratch. + */ + public void testRowValueAggregatesOrder_bug45145() { + + Sheet sheet = Sheet.createSheet(); + + RowRecord rr = new RowRecord(5); + sheet.addRow(rr); + + CellValueRecordInterface cvr = new BlankRecord(); + cvr.setColumn((short)0); + cvr.setRow(5); + sheet.addValueRecord(5, cvr); + + + int dbCellRecordPos = getDbCellRecordPos(sheet); + if (dbCellRecordPos == 264) { + // The overt symptom of the bug + // DBCELL record pos is calculated wrong if VRA comes before RRA + throw new AssertionFailedError("Identified bug 45145"); + } + + // make sure that RRA and VRA are in the right place + int rraIx = sheet.getDimsLoc()+1; + List recs = sheet.getRecords(); + assertEquals(RowRecordsAggregate.class, recs.get(rraIx).getClass()); + assertEquals(ValueRecordsAggregate.class, recs.get(rraIx+1).getClass()); + + assertEquals(254, dbCellRecordPos); + } + + /** + * @return the value calculated for the position of the first DBCELL record for this sheet. + * That value is found on the IndexRecord. + */ + private static int getDbCellRecordPos(Sheet sheet) { + int size = sheet.getSize(); + byte[] data = new byte[size]; + sheet.serialize(0, data); + EventRecordFactory erf = new EventRecordFactory(); + MyIndexRecordListener myIndexListener = new MyIndexRecordListener(); + erf.registerListener(myIndexListener, new short[] { IndexRecord.sid, }); + erf.processRecords(new ByteArrayInputStream(data)); + IndexRecord indexRecord = myIndexListener.getIndexRecord(); + int dbCellRecordPos = indexRecord.getDbcellAt(0); + return dbCellRecordPos; + } + + private static final class MyIndexRecordListener implements ERFListener { + + private IndexRecord _indexRecord; + public MyIndexRecordListener() { + // no-arg constructor + } + public boolean processRecord(Record rec) { + _indexRecord = (IndexRecord)rec; + return true; + } + public IndexRecord getIndexRecord() { + return _indexRecord; + } + } } diff --git a/src/testcases/org/apache/poi/hssf/record/AllRecordTests.java b/src/testcases/org/apache/poi/hssf/record/AllRecordTests.java index 2b8e3c3ab..988be1dac 100755 --- a/src/testcases/org/apache/poi/hssf/record/AllRecordTests.java +++ b/src/testcases/org/apache/poi/hssf/record/AllRecordTests.java @@ -95,6 +95,7 @@ public final class AllRecordTests { result.addTestSuite(TestSeriesTextRecord.class); result.addTestSuite(TestSeriesToChartGroupRecord.class); result.addTestSuite(TestSheetPropertiesRecord.class); + result.addTestSuite(TestSharedFormulaRecord.class); result.addTestSuite(TestStringRecord.class); result.addTestSuite(TestSubRecord.class); result.addTestSuite(TestSupBookRecord.class); diff --git a/src/testcases/org/apache/poi/hssf/record/TestObjRecord.java b/src/testcases/org/apache/poi/hssf/record/TestObjRecord.java index 5a792ba7d..e8a6596e5 100644 --- a/src/testcases/org/apache/poi/hssf/record/TestObjRecord.java +++ b/src/testcases/org/apache/poi/hssf/record/TestObjRecord.java @@ -1,4 +1,3 @@ - /* ==================================================================== Licensed to the Apache Software Foundation (ASF) under one or more contributor license agreements. See the NOTICE file distributed with @@ -18,18 +17,19 @@ package org.apache.poi.hssf.record; -import junit.framework.*; - import java.util.Arrays; import java.util.List; +import junit.framework.AssertionFailedError; +import junit.framework.TestCase; + /** * Tests the serialization and deserialization of the ObjRecord class works correctly. * Test data taken directly from a real Excel file. * * @author Yegor Kozlov */ -public class TestObjRecord extends TestCase { +public final class TestObjRecord extends TestCase { /** * OBJ record data containing two sub-records. * The data taken directly from a real Excel file. @@ -38,22 +38,27 @@ public class TestObjRecord extends TestCase { * [ftCmo] * [ftEnd] */ - public static byte[] recdata = { + private static final byte[] recdata = { 0x15, 0x00, 0x12, 0x00, 0x06, 0x00, 0x01, 0x00, 0x11, 0x60, (byte)0xF4, 0x02, 0x41, 0x01, 0x14, 0x10, 0x1F, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 + // TODO - this data seems to require two extra bytes padding. not sure where original file is. + // it's not bug 38607 attachment 17639 }; + private static final byte[] recdataNeedingPadding = { + 21, 0, 18, 0, 0, 0, 1, 0, 17, 96, 0, 0, 0, 0, 56, 111, -52, 3, 0, 0, 0, 0, 6, 0, 2, 0, 0, 0, 0, 0, 0, 0 + }; - public void testLoad() throws Exception { + public void testLoad() { ObjRecord record = new ObjRecord(new TestcaseRecordInputStream(ObjRecord.sid, (short)recdata.length, recdata)); - assertEquals( recdata.length, record.getRecordSize() - 4); + assertEquals(28, record.getRecordSize() - 4); List subrecords = record.getSubRecords(); - assertEquals( 2, subrecords.size() ); - assertTrue( subrecords.get(0) instanceof CommonObjectDataSubRecord); - assertTrue( subrecords.get(1) instanceof EndSubRecord ); + assertEquals(2, subrecords.size() ); + assertTrue(subrecords.get(0) instanceof CommonObjectDataSubRecord); + assertTrue(subrecords.get(1) instanceof EndSubRecord ); } @@ -61,8 +66,8 @@ public class TestObjRecord extends TestCase { ObjRecord record = new ObjRecord(new TestcaseRecordInputStream(ObjRecord.sid, (short)recdata.length, recdata)); byte [] recordBytes = record.serialize(); - assertEquals(recdata.length, recordBytes.length - 4); - byte[] subData = new byte[recordBytes.length - 4]; + assertEquals(28, recordBytes.length - 4); + byte[] subData = new byte[recdata.length]; System.arraycopy(recordBytes, 4, subData, 0, subData.length); assertTrue(Arrays.equals(recdata, subData)); } @@ -92,4 +97,20 @@ public class TestObjRecord extends TestCase { assertTrue( subrecords.get(0) instanceof CommonObjectDataSubRecord); assertTrue( subrecords.get(1) instanceof EndSubRecord ); } + + public void testReadWriteWithPadding_bug45133() { + ObjRecord record = new ObjRecord(new TestcaseRecordInputStream(ObjRecord.sid, (short)recdataNeedingPadding.length, recdataNeedingPadding)); + + if (record.getRecordSize() == 34) { + throw new AssertionFailedError("Identified bug 45133"); + } + + assertEquals(36, record.getRecordSize()); + + List subrecords = record.getSubRecords(); + assertEquals(3, subrecords.size() ); + assertEquals(CommonObjectDataSubRecord.class, subrecords.get(0).getClass()); + assertEquals(GroupMarkerSubRecord.class, subrecords.get(1).getClass()); + assertEquals(EndSubRecord.class, subrecords.get(2).getClass()); + } } diff --git a/src/testcases/org/apache/poi/hssf/record/TestSharedFormulaRecord.java b/src/testcases/org/apache/poi/hssf/record/TestSharedFormulaRecord.java new file mode 100644 index 000000000..cb08edec2 --- /dev/null +++ b/src/testcases/org/apache/poi/hssf/record/TestSharedFormulaRecord.java @@ -0,0 +1,97 @@ +/* ==================================================================== + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +==================================================================== */ + +package org.apache.poi.hssf.record; + +import java.util.List; +import java.util.Stack; + +import junit.framework.AssertionFailedError; +import junit.framework.ComparisonFailure; +import junit.framework.TestCase; + +import org.apache.poi.hssf.record.formula.Ptg; +import org.apache.poi.hssf.record.formula.RefAPtg; + +/** + * @author Josh Micich + */ +public final class TestSharedFormulaRecord extends TestCase { + + /** + * Binary data for an encoded formula. Taken from attachment 22062 (bugzilla 45123/45421). + * The shared formula is in Sheet1!C6:C21, with text "SUMPRODUCT(--(End_Acct=$C6),--(End_Bal))" + * This data is found at offset 0x1A4A (within the shared formula record). + * The critical thing about this formula is that it contains shared formula tokens (tRefN*, + * tAreaN*) with operand class 'array'. + */ + private static final byte[] SHARED_FORMULA_WITH_REF_ARRAYS_DATA = { + 0x1A, 0x00, + 0x63, 0x02, 0x00, 0x00, 0x00, + 0x6C, 0x00, 0x00, 0x02, (byte)0x80, // tRefNA + 0x0B, + 0x15, + 0x13, + 0x13, + 0x63, 0x03, 0x00, 0x00, 0x00, + 0x15, + 0x13, + 0x13, + 0x42, 0x02, (byte)0xE4, 0x00, + }; + + /** + * The method SharedFormulaRecord.convertSharedFormulas() converts formulas from + * 'shared formula' to 'single cell formula' format. It is important that token operand + * classes are preserved during this transformation, because Excel may not tolerate the + * incorrect encoding. The formula here is one such example (Excel displays #VALUE!). + */ + public void testConvertSharedFormulasOperandClasses_bug45123() { + + TestcaseRecordInputStream in = new TestcaseRecordInputStream(0, SHARED_FORMULA_WITH_REF_ARRAYS_DATA); + short encodedLen = in.readShort(); + Stack sharedFormula = Ptg.createParsedExpressionTokens(encodedLen, in); + + Stack convertedFormula = SharedFormulaRecord.convertSharedFormulas(sharedFormula, 100, 200); + + RefAPtg refPtg = (RefAPtg) convertedFormula.get(1); + assertEquals("$C101", refPtg.toFormulaString(null)); + if (refPtg.getPtgClass() == Ptg.CLASS_REF) { + throw new AssertionFailedError("Identified bug 45123"); + } + + confirmOperandClasses(toPtgArray(sharedFormula), toPtgArray(convertedFormula)); + } + + private static void confirmOperandClasses(Ptg[] originalPtgs, Ptg[] convertedPtgs) { + assertEquals(originalPtgs.length, convertedPtgs.length); + for (int i = 0; i < convertedPtgs.length; i++) { + Ptg originalPtg = originalPtgs[i]; + Ptg convertedPtg = convertedPtgs[i]; + if (originalPtg.getPtgClass() != convertedPtg.getPtgClass()) { + throw new ComparisonFailure("Different operand class for token[" + i + "]", + String.valueOf(originalPtg.getPtgClass()), String.valueOf(convertedPtg.getPtgClass())); + } + } + } + + private static Ptg[] toPtgArray(List list) { + Ptg[] result = new Ptg[list.size()]; + list.toArray(result); + return result; + } +} diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestNamedRange.java b/src/testcases/org/apache/poi/hssf/usermodel/TestNamedRange.java index b3d08bf72..591a7ba24 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestNamedRange.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestNamedRange.java @@ -17,19 +17,11 @@ package org.apache.poi.hssf.usermodel; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.File; -import java.io.FileInputStream; -import java.io.FileOutputStream; -import java.io.IOException; - import junit.framework.TestCase; import org.apache.poi.hssf.HSSFTestDataSamples; import org.apache.poi.hssf.util.AreaReference; import org.apache.poi.hssf.util.CellReference; -import org.apache.poi.util.TempFile; /** * @@ -49,9 +41,7 @@ public final class TestNamedRange extends TestCase { } /** Test of TestCase method, of class test.RangeTest. */ - public void testNamedRange() - throws IOException - { + public void testNamedRange() { HSSFWorkbook wb = openSample("Simple.xls"); //Creating new Named Range @@ -64,7 +54,7 @@ public final class TestNamedRange extends TestCase { newNamedRange.setNameName("RangeTest"); //Setting its reference newNamedRange.setReference(sheetName + "!$D$4:$E$8"); - + //Getting NAmed Range HSSFName namedRange1 = wb.getNameAt(0); //Getting it sheet name @@ -76,22 +66,10 @@ public final class TestNamedRange extends TestCase { SanityChecker c = new SanityChecker(); c.checkHSSFWorkbook(wb); - File file = TempFile.createTempFile("testNamedRange", ".xls"); - - FileOutputStream fileOut = new FileOutputStream(file); - wb.write(fileOut); - - fileOut.close(); - - assertTrue("file exists",file.exists()); - - FileInputStream in = new FileInputStream(file); - wb = new HSSFWorkbook(in); + wb = HSSFTestDataSamples.writeOutAndReadBack(wb); HSSFName nm =wb.getNameAt(wb.getNameIndex("RangeTest")); assertTrue("Name is "+nm.getNameName(),"RangeTest".equals(nm.getNameName())); assertEquals(wb.getSheetName(0)+"!$D$4:$E$8", nm.getReference()); - - } /** @@ -143,13 +121,11 @@ public final class TestNamedRange extends TestCase { /** * Test that multiple named ranges can be added written and read */ - public void testMultipleNamedWrite() - throws IOException - { + public void testMultipleNamedWrite() { HSSFWorkbook wb = new HSSFWorkbook(); - HSSFSheet sheet = wb.createSheet("testSheet1"); + wb.createSheet("testSheet1"); String sheetName = wb.getSheetName(0); assertEquals("testSheet1", sheetName); @@ -170,18 +146,7 @@ public final class TestNamedRange extends TestCase { HSSFName namedRange1 = wb.getNameAt(0); String referece = namedRange1.getReference(); - File file = TempFile.createTempFile("testMultiNamedRange", ".xls"); - - FileOutputStream fileOut = new FileOutputStream(file); - wb.write(fileOut); - fileOut.close(); - - - assertTrue("file exists",file.exists()); - - - FileInputStream in = new FileInputStream(file); - wb = new HSSFWorkbook(in); + wb = HSSFTestDataSamples.writeOutAndReadBack(wb); HSSFName nm =wb.getNameAt(wb.getNameIndex("RangeTest")); assertTrue("Name is "+nm.getNameName(),"RangeTest".equals(nm.getNameName())); assertTrue("Reference is "+nm.getReference(),(wb.getSheetName(0)+"!$D$4:$E$8").equals(nm.getReference())); @@ -189,19 +154,14 @@ public final class TestNamedRange extends TestCase { nm = wb.getNameAt(wb.getNameIndex("AnotherTest")); assertTrue("Name is "+nm.getNameName(),"AnotherTest".equals(nm.getNameName())); assertTrue("Reference is "+nm.getReference(),newNamedRange2.getReference().equals(nm.getReference())); - - } /** * Test case provided by czhang@cambian.com (Chun Zhang) *

* Addresses Bug #13775 - * @throws IOException */ - public void testMultiNamedRange() - throws IOException - { + public void testMultiNamedRange() { // Create a new workbook HSSFWorkbook wb = new HSSFWorkbook (); @@ -234,16 +194,8 @@ public final class TestNamedRange extends TestCase { namedRange2.setReference("sheet2" + "!$A$1:$O$21"); // Write the workbook to a file - File file = TempFile.createTempFile("testMuiltipletNamedRanges", ".xls"); - FileOutputStream fileOut = new FileOutputStream(file); - wb.write(fileOut); - fileOut.close(); - - assertTrue("file exists",file.exists()); - // Read the Excel file and verify its content - FileInputStream in = new FileInputStream(file); - wb = new HSSFWorkbook(in); + wb = HSSFTestDataSamples.writeOutAndReadBack(wb); HSSFName nm1 =wb.getNameAt(wb.getNameIndex("RangeTest1")); assertTrue("Name is "+nm1.getNameName(),"RangeTest1".equals(nm1.getNameName())); assertTrue("Reference is "+nm1.getReference(),(wb.getSheetName(0)+"!$A$1:$L$41").equals(nm1.getReference())); @@ -253,17 +205,15 @@ public final class TestNamedRange extends TestCase { assertTrue("Reference is "+nm2.getReference(),(wb.getSheetName(1)+"!$A$1:$O$21").equals(nm2.getReference())); } - public void testUnicodeNamedRange() throws Exception { + public void testUnicodeNamedRange() { HSSFWorkbook workBook = new HSSFWorkbook(); - HSSFSheet sheet = workBook.createSheet("Test"); + workBook.createSheet("Test"); HSSFName name = workBook.createName(); name.setNameName("\u03B1"); name.setReference("Test!$D$3:$E$8"); - ByteArrayOutputStream out = new ByteArrayOutputStream(); - workBook.write(out); - HSSFWorkbook workBook2 = new HSSFWorkbook(new ByteArrayInputStream(out.toByteArray())); + HSSFWorkbook workBook2 = HSSFTestDataSamples.writeOutAndReadBack(workBook); HSSFName name2 = workBook2.getNameAt(0); assertEquals("\u03B1", name2.getNameName()); @@ -286,7 +236,6 @@ public final class TestNamedRange extends TestCase { assertNotNull("Print Area not defined for first sheet", retrievedPrintArea); assertEquals("'" + sheetName + "'!$A$1:$B$1", retrievedPrintArea); - } /** @@ -305,10 +254,8 @@ public final class TestNamedRange extends TestCase { assertNotNull("Print Area not defined for first sheet", retrievedPrintArea); assertEquals("'" + sheetName + "'!" + reference, retrievedPrintArea); - } - /** * Test to see if the print area can be retrieved from an excel created file */ @@ -321,88 +268,71 @@ public final class TestNamedRange extends TestCase { assertEquals(reference, workbook.getPrintArea(0)); } - /** * Test to see if the print area made it to the file */ - public void testPrintAreaFile() - throws IOException - { + public void testPrintAreaFile() { HSSFWorkbook workbook = new HSSFWorkbook(); - HSSFSheet sheet = workbook.createSheet("Test Print Area"); + workbook.createSheet("Test Print Area"); String sheetName = workbook.getSheetName(0); String reference = sheetName+"!$A$1:$B$1"; workbook.setPrintArea(0, reference); - File file = TempFile.createTempFile("testPrintArea",".xls"); - - FileOutputStream fileOut = new FileOutputStream(file); - workbook.write(fileOut); - fileOut.close(); - - assertTrue("file exists",file.exists()); - - FileInputStream in = new FileInputStream(file); - workbook = new HSSFWorkbook(in); + workbook = HSSFTestDataSamples.writeOutAndReadBack(workbook); String retrievedPrintArea = workbook.getPrintArea(0); assertNotNull("Print Area not defined for first sheet", retrievedPrintArea); assertEquals("References Match", "'" + sheetName + "'!$A$1:$B$1", retrievedPrintArea); - } /** * Test to see if multiple print areas made it to the file */ - public void testMultiplePrintAreaFile() - throws IOException - { + public void testMultiplePrintAreaFile() { HSSFWorkbook workbook = new HSSFWorkbook(); - HSSFSheet sheet = workbook.createSheet("Sheet1"); - sheet = workbook.createSheet("Sheet2"); - sheet = workbook.createSheet("Sheet3"); - - String sheetName = workbook.getSheetName(0); - String reference = null; - - reference = sheetName+"!$A$1:$B$1"; - workbook.setPrintArea(0, reference); - - sheetName = workbook.getSheetName(1); - String reference2 = sheetName+"!$B$2:$D$5"; + workbook.createSheet("Sheet1"); + workbook.createSheet("Sheet2"); + workbook.createSheet("Sheet3"); + String reference1 = "Sheet1!$A$1:$B$1"; + String reference2 = "Sheet2!$B$2:$D$5"; + String reference3 = "Sheet3!$D$2:$F$5"; + + workbook.setPrintArea(0, reference1); workbook.setPrintArea(1, reference2); - - sheetName = workbook.getSheetName(2); - String reference3 = sheetName+"!$D$2:$F$5"; workbook.setPrintArea(2, reference3); - File file = TempFile.createTempFile("testMultiPrintArea",".xls"); - - FileOutputStream fileOut = new FileOutputStream(file); - workbook.write(fileOut); - fileOut.close(); - - assertTrue("file exists",file.exists()); - - FileInputStream in = new FileInputStream(file); - workbook = new HSSFWorkbook(in); - - String retrievedPrintArea = workbook.getPrintArea(0); + //Check created print areas + String retrievedPrintArea; + + retrievedPrintArea = workbook.getPrintArea(0); assertNotNull("Print Area Not Found (Sheet 1)", retrievedPrintArea); - assertEquals(reference, retrievedPrintArea); + assertEquals(reference1, retrievedPrintArea); - String retrievedPrintArea2 = workbook.getPrintArea(1); - assertNotNull("Print Area Not Found (Sheet 2)", retrievedPrintArea2); - assertEquals(reference2, retrievedPrintArea2); + retrievedPrintArea = workbook.getPrintArea(1); + assertNotNull("Print Area Not Found (Sheet 2)", retrievedPrintArea); + assertEquals(reference2, retrievedPrintArea); - String retrievedPrintArea3 = workbook.getPrintArea(2); - assertNotNull("Print Area Not Found (Sheet 3)", retrievedPrintArea3); - assertEquals(reference3, retrievedPrintArea3); + retrievedPrintArea = workbook.getPrintArea(2); + assertNotNull("Print Area Not Found (Sheet 3)", retrievedPrintArea); + assertEquals(reference3, retrievedPrintArea); + // Check print areas after re-reading workbook + workbook = HSSFTestDataSamples.writeOutAndReadBack(workbook); + retrievedPrintArea = workbook.getPrintArea(0); + assertNotNull("Print Area Not Found (Sheet 1)", retrievedPrintArea); + assertEquals(reference1, retrievedPrintArea); + + retrievedPrintArea = workbook.getPrintArea(1); + assertNotNull("Print Area Not Found (Sheet 2)", retrievedPrintArea); + assertEquals(reference2, retrievedPrintArea); + + retrievedPrintArea = workbook.getPrintArea(2); + assertNotNull("Print Area Not Found (Sheet 3)", retrievedPrintArea); + assertEquals(reference3, retrievedPrintArea); } /** @@ -530,31 +460,29 @@ public final class TestNamedRange extends TestCase { HSSFSheet s = wb.getSheet(cref.getSheetName()); HSSFRow r = sheet.getRow(cref.getRow()); HSSFCell c = r.getCell(cref.getCol()); - String contents = c.getStringCellValue(); + String contents = c.getRichStringCellValue().getString(); assertEquals("Contents of cell retrieved by its named reference", contents, cvalue); } - public void testDeletedReference() throws Exception { - HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("24207.xls"); - assertEquals(2, wb.getNumberOfNames()); + public void testDeletedReference() throws Exception { + HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("24207.xls"); + assertEquals(2, wb.getNumberOfNames()); - HSSFName name1 = wb.getNameAt(0); - assertEquals("a", name1.getNameName()); - assertEquals("Sheet1!$A$1", name1.getReference()); - AreaReference ref1 = new AreaReference(name1.getReference()); - assertTrue("Successfully constructed first reference", true); - - HSSFName name2 = wb.getNameAt(1); - assertEquals("b", name2.getNameName()); - assertEquals("#REF!", name2.getReference()); - assertTrue(name2.isDeleted()); - try { - AreaReference ref2 = new AreaReference(name2.getReference()); - fail("attempt to supply an invalid reference to AreaReference constructor results in exception"); - } catch (Exception e){ - ; - } - - } + HSSFName name1 = wb.getNameAt(0); + assertEquals("a", name1.getNameName()); + assertEquals("Sheet1!$A$1", name1.getReference()); + AreaReference ref1 = new AreaReference(name1.getReference()); + assertTrue("Successfully constructed first reference", true); + HSSFName name2 = wb.getNameAt(1); + assertEquals("b", name2.getNameName()); + assertEquals("#REF!", name2.getReference()); + assertTrue(name2.isDeleted()); + try { + AreaReference ref2 = new AreaReference(name2.getReference()); + fail("attempt to supply an invalid reference to AreaReference constructor results in exception"); + } catch (StringIndexOutOfBoundsException e) { // TODO - use a different exception for this condition + // expected during successful test + } + } }