]> source.dussan.org Git - poi.git/commitdiff
Merged revisions 638786-638802,638805-638811,638813-638814,638816-639230,639233-63924...
authorNick Burch <nick@apache.org>
Fri, 6 Jun 2008 11:36:03 +0000 (11:36 +0000)
committerNick Burch <nick@apache.org>
Fri, 6 Jun 2008 11:36:03 +0000 (11:36 +0000)
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

13 files changed:
src/documentation/content/xdocs/changes.xml
src/documentation/content/xdocs/status.xml
src/java/org/apache/poi/hssf/dev/BiffViewer.java
src/java/org/apache/poi/hssf/model/Sheet.java
src/java/org/apache/poi/hssf/record/ObjRecord.java
src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java
src/java/org/apache/poi/hssf/record/aggregates/ValueRecordsAggregate.java
src/java/org/apache/poi/ss/usermodel/FormulaEvaluator.java
src/testcases/org/apache/poi/hssf/model/TestSheet.java
src/testcases/org/apache/poi/hssf/record/AllRecordTests.java
src/testcases/org/apache/poi/hssf/record/TestObjRecord.java
src/testcases/org/apache/poi/hssf/record/TestSharedFormulaRecord.java [new file with mode: 0644]
src/testcases/org/apache/poi/hssf/usermodel/TestNamedRange.java

index 5d7dce206f3f74521d8f43971be26f25bf9cb07e..267811d78aacffe4544a8687c0261ff8b00f6502 100644 (file)
@@ -46,6 +46,9 @@
            <action dev="POI-DEVELOPERS" type="add">Created a common interface for handling Excel files, irrespective of if they are .xls or .xlsx</action>
         </release>
         <release version="3.1-final" date="2008-06-??">
+           <action dev="POI-DEVELOPERS" type="fix">45133 - Fixed OBJ Record (5Dh) to pad the sub-record data to a 4-byte boundary</action>
+           <action dev="POI-DEVELOPERS" type="fix">45145 - Fixed Sheet to always enforce RowRecordsAggregate before ValueRecordsAggregate</action>
+           <action dev="POI-DEVELOPERS" type="fix">45123 - Fixed SharedFormulaRecord.convertSharedFormulas() to propagate token operand classes</action>
            <action dev="POI-DEVELOPERS" type="fix">45087 - Correctly detect date formats like [Black]YYYY as being date based</action>
            <action dev="POI-DEVELOPERS" type="add">45060 - Improved token class transformation during formula parsing</action>
            <action dev="POI-DEVELOPERS" type="add">44840 - Improved handling of HSSFObjectData, especially for entries with data held not in POIFS</action>
index 68028d539dfd79010da219294822daacac830e4d..115db4e1037cc03a38dab1d46b193c93314a6196 100644 (file)
@@ -43,6 +43,9 @@
            <action dev="POI-DEVELOPERS" type="add">Created a common interface for handling Excel files, irrespective of if they are .xls or .xlsx</action>
         </release>
         <release version="3.1-final" date="2008-06-??">
+           <action dev="POI-DEVELOPERS" type="fix">45133 - Fixed OBJ Record (5Dh) to pad the sub-record data to a 4-byte boundary</action>
+           <action dev="POI-DEVELOPERS" type="fix">45145 - Fixed Sheet to always enforce RowRecordsAggregate before ValueRecordsAggregate</action>
+           <action dev="POI-DEVELOPERS" type="fix">45123 - Fixed SharedFormulaRecord.convertSharedFormulas() to propagate token operand classes</action>
            <action dev="POI-DEVELOPERS" type="fix">45087 - Correctly detect date formats like [Black]YYYY as being date based</action>
            <action dev="POI-DEVELOPERS" type="add">45060 - Improved token class transformation during formula parsing</action>
            <action dev="POI-DEVELOPERS" type="add">44840 - Improved handling of HSSFObjectData, especially for entries with data held not in POIFS</action>
index cd43651912805b3ab86803b5d3d6bdc005e0c5c3..9396e679de6ba36c5e9e01194141424f84b05e51 100644 (file)
@@ -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);
index 871ecc100f859c14ce0dbf7407fafde49bac66e9..b95ad6bba38ea9139e22ff22f7f8fd0f0f24a460 100644 (file)
@@ -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;
+    /**
+     * @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...
+
+        // 'initial sheet records' are between INDEX and first ROW record.
+        int sizeOfInitialSheetRecords = 0;
+        // start just after BOF record (INDEX is not present in this list)
+        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;
+            }
+            sizeOfInitialSheetRecords += tmpRec.getRecordSize();
         }
-        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);
-
-      int rowBlockOffset = 0;
-      int cellBlockOffset = 0;
-      int dbCellOffset = 0;
-      for (int block=0;block<blockCount;block++) {
-        rowBlockOffset += rows.getRowBlockSize(block);
-        cellBlockOffset += null == cells ? 0 : cells.getRowCellBlockSize(rows.getStartRowNumberForBlock(block),
-                                                     rows.getEndRowNumberForBlock(block));
-        //Note: The offsets are relative to the Workbook BOF. Assume that this is
-        //0 for now.....
-        index.addDbcell(offset + indexRecSize + sheetRecSize + dbCellOffset + rowBlockOffset + cellBlockOffset);
-        //Add space required to write the dbcell record(s) (whose references were just added).
-        dbCellOffset += (8 + (rows.getRowCountForBlock(block) * 2));
-      }
-      return index.serialize(offset, data);
+        if (_isUncalced) {
+            sizeOfInitialSheetRecords += UncalcedRecord.getStaticRecordSize();
+        }
+
+        // Add the references to the DBCells in the IndexRecord (one for each block)
+        // Note: The offsets are relative to the Workbook BOF. Assume that this is
+        // 0 for now.....
+
+        int blockCount = rows.getRowBlockCount();
+        // Calculate the size of this IndexRecord
+        int indexRecSize = IndexRecord.getRecordSizeForBlockCount(blockCount);
+
+        int currentOffset = indexRecordOffset + indexRecSize + sizeOfInitialSheetRecords;
+
+        for (int block = 0; block < blockCount; block++) {
+            // each row-block has a DBCELL record.
+            // The offset of each DBCELL record needs to be updated in the INDEX record
+
+            // account for row records in this row-block
+            currentOffset += rows.getRowBlockSize(block);
+            // account for cell value records after those
+            currentOffset += null == cells ? 0 : cells.getRowCellBlockSize(rows
+                    .getStartRowNumberForBlock(block), rows.getEndRowNumberForBlock(block));
+
+            // currentOffset is now the location of the DBCELL record for this row-block
+            index.addDbcell(currentOffset);
+            // Add space required to write the DBCELL record (whose reference was just added).
+            currentOffset += (8 + (rows.getRowCountForBlock(block) * 2));
+        }
+        return index.serialize(indexRecordOffset, data);
     }
 
 
index 6583bd6777db360b99295966a83008183de8d9ea..7d29654cd85779f9185d0ed24d6bd22e87367985 100644 (file)
@@ -1,4 +1,3 @@
-
 /* ====================================================================
    Licensed to the Apache Software Foundation (ASF) under one or more
    contributor license agreements.  See the NOTICE file distributed with
    limitations under the License.
 ==================================================================== */
         
-
-
 package org.apache.poi.hssf.record;
 
-
-
-import org.apache.poi.util.*;
-
 import java.io.ByteArrayInputStream;
-import java.util.List;
-import java.util.Iterator;
 import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
+
+import org.apache.poi.util.LittleEndian;
 
 /**
  * The obj record is used to hold various graphic objects and controls.
  *
  * @author Glen Stampoultzis (glens at apache.org)
  */
-public class ObjRecord
-    extends Record
-{
+public final class ObjRecord extends Record {
     public final static short      sid                             = 0x5D;
     private List subrecords;
 
@@ -47,6 +40,7 @@ public class ObjRecord
     public ObjRecord()
     {
         subrecords = new ArrayList(2);
+        // TODO - ensure 2 sub-records (ftCmo  15h, and ftEnd  00h) are always created
     }
 
     /**
@@ -80,6 +74,7 @@ public class ObjRecord
         //following wont work properly
         int subSize = 0;
         byte[] subRecordData = in.readRemainder();
+
         RecordInputStream subRecStream = new RecordInputStream(new ByteArrayInputStream(subRecordData));
         while(subRecStream.hasNextRecord()) {
           subRecStream.nextRecord();
@@ -89,28 +84,19 @@ public class ObjRecord
         }
 
         /**
-         * Check if the RecordInputStream skipped EndSubRecord,
-         * if it did then append it explicitly.
-         * See Bug 41242 for details.
+         * Add the EndSubRecord explicitly.
+         * 
+         * TODO - the reason the EndSubRecord is always skipped is because its 'sid' is zero and
+         * that causes subRecStream.hasNextRecord() to return false.
+         * There may be more than the size of EndSubRecord left-over, if there is any padding 
+         * after that record.  The content of the EndSubRecord and the padding is all zeros.
+         * So there's not much to look at past the last substantial record.
+         * 
+         * See Bugs 41242/45133 for details.
          */
-        if (subRecordData.length - subSize == 4){
+        if (subRecordData.length - subSize >= 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
-
-
-
-
+}
index 2b0c50d122675828157fe158fd89486acbaa55bc..e4c0f28ea48ba047abce5957787d2a6f5d683765 100755 (executable)
@@ -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;
index 068896952906387710d6b9892750f20da27085fe..fb4bfd59a4bb2f87fe333487347845d77ae44dc2 100644 (file)
@@ -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;
index 0ecde630493a6f7d3f8fc1f01a8598ae9e4a2d10..88d8dcecddf96d5406da344c5206df8d3bcb90f8 100644 (file)
@@ -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) { 
index 3bdcae57cb7b65f23a49ad7faf4ff8b1c17b3ad9..ef21cc9b362f3a524966e161b0c6930be905555a 100644 (file)
@@ -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 <tt>UncalcedRecord</tt> was present.<p/>
      */
     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 <tt>RowRecordsAggregate</tt> and <tt>ValueRecordsAggregate</tt> could
+     * sometimes occur in reverse order.  This test reproduces one of those situations and makes
+     * sure that RRA comes before VRA.<br/>
+     *
+     * 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;
+        }
+    }
 }
 
index 2b8e3c3ab557f2c6dd95a3ce4d63f778517c1348..988be1dac5ef7303ff03314764819ba6c4910a68 100755 (executable)
@@ -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);
index 5a792ba7dfd8413db68c99cc2a046bb752277115..e8a6596e556c1ae0905951d9f529497f113888b8 100644 (file)
@@ -1,4 +1,3 @@
-
 /* ====================================================================
    Licensed to the Apache Software Foundation (ASF) under one or more
    contributor license agreements.  See the NOTICE file distributed with
 
 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 (file)
index 0000000..cb08ede
--- /dev/null
@@ -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 <tt>SharedFormulaRecord.convertSharedFormulas()</tt> 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;
+       }
+}
index b3d08bf7244fbfefd6253522033413ed0f26dc28..591a7ba24fe86d28ed9a26c243fde47f5062c11e 100644 (file)
 
 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)
         * <p>
         * Addresses Bug <a href="http://issues.apache.org/bugzilla/show_bug.cgi?id=13775" target="_bug">#13775</a>
-        * @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");
+               //Check created print areas
+               String retrievedPrintArea;
+               
+               retrievedPrintArea = workbook.getPrintArea(0);
+               assertNotNull("Print Area Not Found (Sheet 1)", retrievedPrintArea);
+               assertEquals(reference1, retrievedPrintArea);
 
-               FileOutputStream fileOut = new FileOutputStream(file);
-               workbook.write(fileOut);
-               fileOut.close();
+               retrievedPrintArea = workbook.getPrintArea(1);
+               assertNotNull("Print Area Not Found (Sheet 2)", retrievedPrintArea);
+               assertEquals(reference2, retrievedPrintArea);
 
-               assertTrue("file exists",file.exists());
+               retrievedPrintArea = workbook.getPrintArea(2);
+               assertNotNull("Print Area Not Found (Sheet 3)", retrievedPrintArea);
+               assertEquals(reference3, retrievedPrintArea);
 
-               FileInputStream in = new FileInputStream(file);
-               workbook = new HSSFWorkbook(in);
+               // Check print areas after re-reading workbook
+               workbook = HSSFTestDataSamples.writeOutAndReadBack(workbook);
 
-               String retrievedPrintArea = workbook.getPrintArea(0);
+               retrievedPrintArea = workbook.getPrintArea(0);
                assertNotNull("Print Area Not Found (Sheet 1)", retrievedPrintArea);
-               assertEquals(reference, retrievedPrintArea);
-
-               String retrievedPrintArea2 = workbook.getPrintArea(1);
-               assertNotNull("Print Area Not Found (Sheet 2)", retrievedPrintArea2);
-               assertEquals(reference2, retrievedPrintArea2);
-
-               String retrievedPrintArea3 = workbook.getPrintArea(2);
-               assertNotNull("Print Area Not Found (Sheet 3)", retrievedPrintArea3);
-               assertEquals(reference3, retrievedPrintArea3);
+               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());
-
-        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){
-            ;
-        }
-
-    }
-
+       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 (StringIndexOutOfBoundsException e) { // TODO - use a different exception for this condition
+                       // expected during successful test
+               }
+   }
 }