Finally fix bug #42464 - Expected ExpPtg to be converted from Shared to Non-Shared Formula - tracked down to a signed vs unsigned byte issue!

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@610216 13f79535-47bb-0310-9956-ffa450edef68

src/documentation/content/xdocs/changes.xml

@@ -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>

src/documentation/content/xdocs/status.xml

@@ -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>

src/java/org/apache/poi/hssf/record/FormulaRecord.java

@@ -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

src/java/org/apache/poi/hssf/record/RecordInputStream.java

@@ -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();    
     

src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java

@@ -20,10 +20,8 @@
 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();

src/java/org/apache/poi/hssf/record/aggregates/FormulaRecordAggregate.java

@@ -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 );

src/java/org/apache/poi/hssf/record/aggregates/ValueRecordsAggregate.java

@@ -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);

src/java/org/apache/poi/hssf/record/formula/ExpPtg.java

@@ -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()

src/scratchpad/testcases/org/apache/poi/hssf/usermodel/TestBug42464.java

@@ -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);
+				
+			}
+		}
+	}
+}