]> source.dussan.org Git - poi.git/commitdiff
Fix for bug 46213 - FormulaRecordAggregate should gracefully ignore extra StringRecords
authorJosh Micich <josh@apache.org>
Sat, 15 Nov 2008 17:25:32 +0000 (17:25 +0000)
committerJosh Micich <josh@apache.org>
Sat, 15 Nov 2008 17:25:32 +0000 (17:25 +0000)
git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@717883 13f79535-47bb-0310-9956-ffa450edef68

src/documentation/content/xdocs/changes.xml
src/documentation/content/xdocs/status.xml
src/java/org/apache/poi/hssf/record/aggregates/FormulaRecordAggregate.java
src/testcases/org/apache/poi/hssf/record/aggregates/TestFormulaRecordAggregate.java
src/testcases/org/apache/poi/hssf/usermodel/RecordInspector.java

index 8257ad5ef10a226ee74db671e11f9ea230cf028f..bb45c636923aa44f7ce091b9c64744b6e45e2381 100644 (file)
@@ -37,6 +37,7 @@
 
                <!-- Don't forget to update status.xml too! -->
         <release version="3.5-beta4" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">46213 - Fixed FormulaRecordAggregate to gracefully ignore extra StringRecords</action>
            <action dev="POI-DEVELOPERS" type="fix">46174 - Fixed HSSFName to handle general formulas (not just area references)</action>
            <action dev="POI-DEVELOPERS" type="add">46189 - added chart records: CHARTFRTINFO, STARTBLOCK, ENDBLOCK, STARTOBJECT, ENDOBJECT, and CATLAB</action>
            <action dev="POI-DEVELOPERS" type="fix">46199 - More tweaks to EmbeddedObjectRefSubRecord</action>
index d3f1ed911be532e034b8bf684790486e359c52db..57a0616d78a45f8b2709cf962e946daa96cffd4e 100644 (file)
@@ -34,6 +34,7 @@
        <!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.5-beta4" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">46213 - Fixed FormulaRecordAggregate to gracefully ignore extra StringRecords</action>
            <action dev="POI-DEVELOPERS" type="fix">46174 - Fixed HSSFName to handle general formulas (not just area references)</action>
            <action dev="POI-DEVELOPERS" type="add">46189 - added chart records: CHARTFRTINFO, STARTBLOCK, ENDBLOCK, STARTOBJECT, ENDOBJECT, and CATLAB</action>
            <action dev="POI-DEVELOPERS" type="fix">46199 - More tweaks to EmbeddedObjectRefSubRecord</action>
index 18653e952ee81ee04f8cbba7359c61becafdba18..83a4ce7e0e7c6beac9b8723c4cbff5ab0c88cebd 100644 (file)
@@ -26,7 +26,6 @@ import org.apache.poi.hssf.record.StringRecord;
 import org.apache.poi.hssf.record.formula.ExpPtg;
 import org.apache.poi.hssf.record.formula.Ptg;
 import org.apache.poi.hssf.util.CellReference;
-import org.apache.poi.ss.formula.Formula;
 
 /**
  * The formula record aggregate is used to join together the formula record and it's
@@ -51,17 +50,19 @@ public final class FormulaRecordAggregate extends RecordAggregate implements Cel
                if (svm == null) {
                        throw new IllegalArgumentException("sfm must not be null");
                }
-               boolean hasStringRec = stringRec != null;
-               boolean hasCachedStringFlag = formulaRec.hasCachedResultString();
-               if (hasStringRec != hasCachedStringFlag) {
-                       throw new RecordFormatException("String record was "
-                                       + (hasStringRec ? "": "not ") + " supplied but formula record flag is "
-                                       + (hasCachedStringFlag ? "" : "not ") + " set");
+               if (formulaRec.hasCachedResultString()) {
+                       if (stringRec == null) {
+                               throw new RecordFormatException("Formula record flag is set but String record was not found");
+                       }
+                       _stringRecord = stringRec;
+               } else {
+                       // Usually stringRec is null here (in agreement with what the formula rec says).
+                       // In the case where an extra StringRecord is erroneously present, Excel (2007)
+                       // ignores it (see bug 46213). 
+                       _stringRecord = null;
                }
 
                if (formulaRec.isSharedFormula()) {
-                       CellReference cr = new CellReference(formulaRec.getRow(), formulaRec.getColumn());
-                       
                        CellReference firstCell = formulaRec.getFormula().getExpReference();
                        if (firstCell == null) {
                                handleMissingSharedFormulaRecord(formulaRec);
@@ -71,7 +72,6 @@ public final class FormulaRecordAggregate extends RecordAggregate implements Cel
                }
                _formulaRecord = formulaRec;
                _sharedValueManager = svm;
-               _stringRecord = stringRec;
        }
        /**
         * Sometimes the shared formula flag "seems" to be erroneously set (because the corresponding
index 978f400fcd0a87f6e69f56b1dd77adad43ff7290..a93bed20c21cff024afffbf74ef0a3b57ad0c65c 100644 (file)
 
 package org.apache.poi.hssf.record.aggregates;
 
+import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
 
 import org.apache.poi.hssf.record.FormulaRecord;
+import org.apache.poi.hssf.record.Record;
+import org.apache.poi.hssf.record.RecordFormatException;
 import org.apache.poi.hssf.record.StringRecord;
+import org.apache.poi.hssf.usermodel.RecordInspector.RecordCollector;
 
 /**
- *
- * @author  avik
+ * 
+ * @author avik
  */
 public final class TestFormulaRecordAggregate extends TestCase {
-    
-    public void testBasic() throws Exception {
-        FormulaRecord f = new FormulaRecord();
-        f.setCachedResultTypeString();
-        StringRecord s = new StringRecord();
-        s.setString("abc");
-        FormulaRecordAggregate fagg = new FormulaRecordAggregate(f, s, SharedValueManager.EMPTY);
-        assertEquals("abc", fagg.getStringValue());
-    }
+
+       public void testBasic() throws Exception {
+               FormulaRecord f = new FormulaRecord();
+               f.setCachedResultTypeString();
+               StringRecord s = new StringRecord();
+               s.setString("abc");
+               FormulaRecordAggregate fagg = new FormulaRecordAggregate(f, s, SharedValueManager.EMPTY);
+               assertEquals("abc", fagg.getStringValue());
+       }
+
+       /**
+        * Sometimes a {@link StringRecord} appears after a {@link FormulaRecord} even though the
+        * formula has evaluated to a text value.  This might be more likely to occur when the formula
+        * <i>can</i> evaluate to a text value.<br/>
+        * Bug 46213 attachment 22874 has such an extra {@link StringRecord} at stream offset 0x5765.
+        * This file seems to open in Excel (2007) with no trouble.  When it is re-saved, Excel omits
+        * the extra record.  POI should do the same.
+        */
+       public void testExtraStringRecord_bug46213() {
+               FormulaRecord fr = new FormulaRecord();
+               fr.setValue(2.0);
+               StringRecord sr = new StringRecord();
+               sr.setString("NA");
+               SharedValueManager svm = SharedValueManager.EMPTY;
+               FormulaRecordAggregate fra;
+               
+               try {
+                       fra = new FormulaRecordAggregate(fr, sr, svm);
+               } catch (RecordFormatException e) {
+                       if ("String record was  supplied but formula record flag is not  set".equals(e.getMessage())) {
+                               throw new AssertionFailedError("Identified bug 46213");
+                       }
+                       throw e;
+               }
+               RecordCollector rc = new RecordCollector();
+               fra.visitContainedRecords(rc);
+               Record[] vraRecs = rc.getRecords();
+               assertEquals(1, vraRecs.length);
+               assertEquals(fr, vraRecs[0]);
+       }
 }
index 867c13db4a3a46a5d23618e87595e23ffc412b46..c47fca762946c45990b2d697088401218d446367 100644 (file)
@@ -24,7 +24,7 @@ import org.apache.poi.hssf.record.Record;
 import org.apache.poi.hssf.record.aggregates.RecordAggregate.RecordVisitor;
 
 /**
- * Test utility class to get {@link Record}s out HSSF objects
+ * Test utility class to get {@link Record}s out of HSSF objects
  * 
  * @author Josh Micich
  */
@@ -34,12 +34,12 @@ public final class RecordInspector {
                // no instances of this class
        }
 
-       private static final class RecordCollector implements RecordVisitor {
+       public static final class RecordCollector implements RecordVisitor {
 
-               private List _list;
+               private List<Record> _list;
 
                public RecordCollector() {
-                       _list = new ArrayList(128);
+                       _list = new ArrayList<Record>(128);
                }
 
                public void visitRecord(Record r) {