]> source.dussan.org Git - poi.git/commitdiff
Finally fix bug #42464 - Expected ExpPtg to be converted from Shared to Non-Shared...
authorNick Burch <nick@apache.org>
Tue, 8 Jan 2008 23:01:12 +0000 (23:01 +0000)
committerNick Burch <nick@apache.org>
Tue, 8 Jan 2008 23:01:12 +0000 (23:01 +0000)
git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@610216 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/record/FormulaRecord.java
src/java/org/apache/poi/hssf/record/RecordInputStream.java
src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java
src/java/org/apache/poi/hssf/record/aggregates/FormulaRecordAggregate.java
src/java/org/apache/poi/hssf/record/aggregates/ValueRecordsAggregate.java
src/java/org/apache/poi/hssf/record/formula/ExpPtg.java
src/scratchpad/testcases/org/apache/poi/hssf/data/42464-ExpPtg-bad.xls [new file with mode: 0644]
src/scratchpad/testcases/org/apache/poi/hssf/data/42464-ExpPtg-ok.xls [new file with mode: 0644]
src/scratchpad/testcases/org/apache/poi/hssf/usermodel/TestBug42464.java [new file with mode: 0644]
src/testcases/org/apache/poi/hssf/data/42464-ExpPtg-bad.xls [new file with mode: 0644]
src/testcases/org/apache/poi/hssf/data/42464-ExpPtg-ok.xls [new file with mode: 0644]

index 59bd2eab7a3bc6b632a103c6a1230d04e38aed90..c691408478e6768f4f32ae956b5dd71cb216a55d 100644 (file)
@@ -36,6 +36,7 @@
 
                <!-- Don't forget to update status.xml too! -->
         <release version="3.0.2-FINAL" date="2008-??-??">
+            <action dev="POI-DEVELOPERS" type="fix">42464 - Avoid "Expected ExpPtg to be converted from Shared to Non-Shared Formula" on large, formula heavy worksheets</action>
             <action dev="POI-DEVELOPERS" type="add">42033 - Add support for named ranges with unicode names</action>
             <action dev="POI-DEVELOPERS" type="add">34023 - When shifting rows, update formulas on that sheet to point to the new location of those rows</action>
             <action dev="POI-DEVELOPERS" type="add">Support getting all the cells referenced by an AreaReference, not just the corner ones</action>
index 584482100837ff181e7b5e98b760e42f1f3a0a73..6cac38bf52bdbe9f5ae9e955c0251cf4a7a8630b 100644 (file)
@@ -33,6 +33,7 @@
        <!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.0.2-FINAL" date="2008-??-??">
+            <action dev="POI-DEVELOPERS" type="fix">42464 - Avoid "Expected ExpPtg to be converted from Shared to Non-Shared Formula" on large, formula heavy worksheets</action>
             <action dev="POI-DEVELOPERS" type="add">42033 - Add support for named ranges with unicode names</action>
             <action dev="POI-DEVELOPERS" type="add">34023 - When shifting rows, update formulas on that sheet to point to the new location of those rows</action>
             <action dev="POI-DEVELOPERS" type="add">Support getting all the cells referenced by an AreaReference, not just the corner ones</action>
index f864cc63ea940038c08f8ead2a66c4dc4faa3416..eba34a30731768f49ed6fc989c03e0c85fcfc68d 100644 (file)
@@ -199,11 +199,24 @@ public class FormulaRecord
     public boolean isSharedFormula() {
         return sharedFormula.isSet(field_5_options);
     }
-    
     public void setSharedFormula(boolean flag) {
        sharedFormula.setBoolean(field_5_options, flag);
     }
     
+    public boolean isAlwaysCalc() {
+       return alwaysCalc.isSet(field_5_options);
+    }
+    public void setAlwaysCalc(boolean flag) {
+       alwaysCalc.setBoolean(field_5_options, flag);
+    }
+    
+    public boolean isCalcOnLoad() {
+       return calcOnLoad.isSet(field_5_options);
+    }
+    public void setCalcOnLoad(boolean flag) {
+       calcOnLoad.setBoolean(field_5_options, flag);
+    }
+    
     /**
      * get the length (in number of tokens) of the expression
      * @return  expression length
index 399e0f566d52a17f725f60e24937a1e9f4294a3b..dd853f2463904ea36293d51124c2e8a73b22cb43 100755 (executable)
@@ -133,6 +133,9 @@ public class RecordInputStream extends InputStream
     }    
   }
   
+  /**
+   * Reads an 8 bit, signed value
+   */
   public byte readByte() {
     checkRecordPosition();
     
@@ -141,7 +144,10 @@ public class RecordInputStream extends InputStream
     pos += 1;
     return result;
   }
-
+  
+  /**
+   * Reads a 16 bit, signed value
+   */
   public short readShort() {
     checkRecordPosition();
     
@@ -169,6 +175,21 @@ public class RecordInputStream extends InputStream
     return result;
   }
 
+  /**
+   * Reads an 8 bit, unsigned value
+   */
+  public short readUByte() {
+         short s = readByte();
+         if(s < 0) {
+                 s += 256;
+         }
+         return s;
+  }
+
+  /**
+   * Reads a 16 bit,un- signed value.
+   * @return
+   */
   public int readUShort() {
     checkRecordPosition();    
     
index c0f77cb558bab2a93ed9a0207eed7f6761843d01..e2871ac5cff0da1c3ea55a96d721e9266861e629 100755 (executable)
 package org.apache.poi.hssf.record;
 
 import java.util.Stack;
-import java.util.List;
 
 import org.apache.poi.hssf.record.formula.*;
-import org.apache.poi.util.LittleEndian;
 
 /**
  * Title:        SharedFormulaRecord
@@ -156,15 +154,12 @@ public class SharedFormulaRecord
         return sid;
     }
 
-        /**
-         * Shared formulas are to treated like unknown records, and as a result d
-         */
     protected void fillFields(RecordInputStream in)
     {
       field_1_first_row       = in.readShort();
       field_2_last_row        = in.readShort();
-      field_3_first_column    = in.readByte();
-      field_4_last_column     = in.readByte();
+      field_3_first_column    = in.readUByte();
+      field_4_last_column     = in.readUByte();
       field_5_reserved        = in.readShort();
       field_6_expression_len = in.readShort();
       field_7_parsed_expr    = getParsedExpressionTokens(in);
@@ -181,6 +176,9 @@ public class SharedFormulaRecord
         return stack;
     }
 
+    /**
+     * Are we shared by the supplied formula record?
+     */
     public boolean isFormulaInShared(FormulaRecord formula) {
       final int formulaRow = formula.getRow();
       final int formulaColumn = formula.getColumn();
index be0f703e80a1f6ff68de776f8d2bd2fa26dad72b..7840d32562c4ac730e0cb86389f014076343434a 100644 (file)
@@ -99,7 +99,7 @@ public class FormulaRecordAggregate
     {
         this.formulaRecord = formulaRecord;
     }
-
+    
     public FormulaRecord getFormulaRecord()
     {
         return formulaRecord;
@@ -109,7 +109,7 @@ public class FormulaRecordAggregate
     {
         return stringRecord;
     }
-
+    
     public boolean isEqual(CellValueRecordInterface i)
     {
         return formulaRecord.isEqual( i );
index fe3af5aedede6d9fc3a58211bcda7fa5a06bc3e6..e48a0a902bf84f8bfbabb7b9645dab81c2172fda 100644 (file)
@@ -127,8 +127,17 @@ public class ValueRecordsAggregate
 
         FormulaRecordAggregate lastFormulaAggregate = null;
         
+        // First up, locate all the shared formulas
         List sharedFormulas = new java.util.ArrayList();
+        for (k = offset; k < records.size(); k++)
+        {
+            Record rec = ( Record ) records.get(k);
+            if (rec instanceof SharedFormulaRecord) {
+               sharedFormulas.add(rec);
+            }
+        }
 
+        // Now do the main processing sweep
         for (k = offset; k < records.size(); k++)
         {
             Record rec = ( Record ) records.get(k);
@@ -137,18 +146,14 @@ public class ValueRecordsAggregate
             {
                 break;
             } else if (rec instanceof SharedFormulaRecord) {
-               sharedFormulas.add(rec);
+               // Already handled, not to worry
             } else if (rec instanceof FormulaRecord)
             {
               FormulaRecord formula = (FormulaRecord)rec;
               if (formula.isSharedFormula()) {
-                Record nextRecord = (Record) records.get(k + 1);
-                if (nextRecord instanceof SharedFormulaRecord) {
-                       sharedFormulas.add(nextRecord);
-                       k++;
-                }
-                //traverse the list of shared formulas in reverse order, and try to find the correct one
-                //for us
+                // Traverse the list of shared formulas in
+               //  reverse order, and try to find the correct one
+                //  for us
                 boolean found = false;
                 for (int i=sharedFormulas.size()-1;i>=0;i--) {
                        SharedFormulaRecord shrd = (SharedFormulaRecord)sharedFormulas.get(i);
index 511ac44d7be2b744d325d51a37d82c256375288d..a7fc0274adb3c05aa1791cf699ec64c6476c0fb2 100644 (file)
@@ -75,7 +75,7 @@ public class ExpPtg
 
     public String toFormulaString(Workbook book)
     {
-        throw new RecordFormatException("Coding Error: Expected ExpPtg to be converted from Shared to Non-Shared Formula");
+        throw new RecordFormatException("Coding Error: Expected ExpPtg to be converted from Shared to Non-Shared Formula by ValueRecordsAggregate, but it wasn't");
     }
     
     public String toString()
diff --git a/src/scratchpad/testcases/org/apache/poi/hssf/data/42464-ExpPtg-bad.xls b/src/scratchpad/testcases/org/apache/poi/hssf/data/42464-ExpPtg-bad.xls
new file mode 100644 (file)
index 0000000..54a7edb
Binary files /dev/null and b/src/scratchpad/testcases/org/apache/poi/hssf/data/42464-ExpPtg-bad.xls differ
diff --git a/src/scratchpad/testcases/org/apache/poi/hssf/data/42464-ExpPtg-ok.xls b/src/scratchpad/testcases/org/apache/poi/hssf/data/42464-ExpPtg-ok.xls
new file mode 100644 (file)
index 0000000..5ae84bc
Binary files /dev/null and b/src/scratchpad/testcases/org/apache/poi/hssf/data/42464-ExpPtg-ok.xls differ
diff --git a/src/scratchpad/testcases/org/apache/poi/hssf/usermodel/TestBug42464.java b/src/scratchpad/testcases/org/apache/poi/hssf/usermodel/TestBug42464.java
new file mode 100644 (file)
index 0000000..3b31cc0
--- /dev/null
@@ -0,0 +1,93 @@
+/* ====================================================================
+   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.usermodel;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.util.Iterator;
+import java.util.List;
+
+import org.apache.poi.hssf.record.FormulaRecord;
+import org.apache.poi.hssf.record.aggregates.FormulaRecordAggregate;
+import org.apache.poi.hssf.record.formula.ExpPtg;
+import org.apache.poi.hssf.util.CellReference;
+
+import junit.framework.TestCase;
+
+public class TestBug42464 extends TestCase {
+       String dirname;
+
+       protected void setUp() throws Exception {
+               super.setUp();
+               dirname = System.getProperty("HSSF.testdata.path");
+       }
+
+       public void testOKFile() throws Exception {
+               HSSFWorkbook wb = new HSSFWorkbook(
+                               new FileInputStream(new File(dirname,"42464-ExpPtg-ok.xls"))
+               );
+               process(wb);
+       }
+       public void testExpSharedBadFile() throws Exception {
+               HSSFWorkbook wb = new HSSFWorkbook(
+                               new FileInputStream(new File(dirname,"42464-ExpPtg-bad.xls"))
+               );
+               process(wb);
+       }
+       
+       protected void process(HSSFWorkbook wb) {
+               for(int i=0; i<wb.getNumberOfSheets(); i++) {
+                       HSSFSheet s = wb.getSheetAt(i);
+                       HSSFFormulaEvaluator eval =
+                               new HSSFFormulaEvaluator(s, wb);
+                       
+                       Iterator it = s.rowIterator();
+                       while(it.hasNext()) {
+                               HSSFRow r = (HSSFRow)it.next();
+                               eval.setCurrentRow(r);
+                               process(r, eval);
+                       }
+               }
+       }
+       
+       protected void process(HSSFRow row, HSSFFormulaEvaluator eval) {
+               Iterator it = row.cellIterator();
+               while(it.hasNext()) {
+                       HSSFCell cell = (HSSFCell)it.next();
+                       if(cell.getCellType() == HSSFCell.CELL_TYPE_FORMULA) {
+                               FormulaRecordAggregate record = (FormulaRecordAggregate)
+                                       cell.getCellValueRecord();
+                               FormulaRecord r = record.getFormulaRecord();
+                               List ptgs = r.getParsedExpression();
+                               
+                               String cellRef = (new CellReference(row.getRowNum(), cell.getCellNum())).toString();
+                               if(cellRef.equals("BP24")) {
+                                       System.out.print(cellRef);
+                                       System.out.println(" - has " + r.getNumberOfExpressionTokens() + " ptgs over " + r.getExpressionLength()  + " tokens:");
+                                       for(int i=0; i<ptgs.size(); i++) {
+                                               String c = ptgs.get(i).getClass().toString();
+                                               System.out.println("\t" + c.substring(c.lastIndexOf('.')+1) );
+                                       }
+                                       System.out.println("-> " + cell.getCellFormula());
+                               }
+                               
+                               eval.evaluate(cell);
+                               
+                       }
+               }
+       }
+}
diff --git a/src/testcases/org/apache/poi/hssf/data/42464-ExpPtg-bad.xls b/src/testcases/org/apache/poi/hssf/data/42464-ExpPtg-bad.xls
new file mode 100644 (file)
index 0000000..54a7edb
Binary files /dev/null and b/src/testcases/org/apache/poi/hssf/data/42464-ExpPtg-bad.xls differ
diff --git a/src/testcases/org/apache/poi/hssf/data/42464-ExpPtg-ok.xls b/src/testcases/org/apache/poi/hssf/data/42464-ExpPtg-ok.xls
new file mode 100644 (file)
index 0000000..5ae84bc
Binary files /dev/null and b/src/testcases/org/apache/poi/hssf/data/42464-ExpPtg-ok.xls differ