* @author Jason Height (jheight at chariot dot net dot au)
*/
-public class ValueRecordsAggregate
+public final class ValueRecordsAggregate
extends Record
{
public final static short sid = -1000;
FormulaRecordAggregate lastFormulaAggregate = null;
- // First up, locate all the shared formulas
+ // First up, locate all the shared formulas for this sheet
List sharedFormulas = new java.util.ArrayList();
for (k = offset; k < records.size(); k++)
{
if (rec instanceof SharedFormulaRecord) {
sharedFormulas.add(rec);
}
+ if(rec instanceof EOFRecord) {
+ // End of current sheet. Ignore all subsequent shared formula records (Bugzilla 44449)
+ break;
+ }
}
// Now do the main processing sweep
// for us
boolean found = false;
for (int i=sharedFormulas.size()-1;i>=0;i--) {
+ // TODO - there is no junit test case to justify this reversed loop
+ // perhaps it could just run in the normal direction?
SharedFormulaRecord shrd = (SharedFormulaRecord)sharedFormulas.get(i);
if (shrd.isFormulaInShared(formula)) {
shrd.convertSharedFormulaRecord(formula);
}
}
if (!found) {
- //Sometimes the shared formula flag "seems" to be errornously set,
- //cant really do much about that.
- //throw new RecordFormatException("Could not find appropriate shared formula");
+ handleMissingSharedFormulaRecord(formula);
}
}
return k;
}
+ /**
+ * Sometimes the shared formula flag "seems" to be erroneously set, in which case there is no
+ * call to <tt>SharedFormulaRecord.convertSharedFormulaRecord</tt> and hence the
+ * <tt>parsedExpression</tt> field of this <tt>FormulaRecord</tt> will not get updated.<br/>
+ * As it turns out, this is not a problem, because in these circumstances, the existing value
+ * for <tt>parsedExpression</tt> is perfectly OK.<p/>
+ *
+ * This method may also be used for setting breakpoints to help diagnose issues regarding the
+ * abnormally-set 'shared formula' flags.
+ * (see TestValueRecordsAggregate.testSpuriousSharedFormulaFlag()).<p/>
+ *
+ * The method currently does nothing but do not delete it without finding a nice home for this
+ * comment.
+ */
+ private static void handleMissingSharedFormulaRecord(FormulaRecord formula) {
+ // could log an info message here since this is a fairly unusual occurrence.
+ }
+
/**
* called by the class that is responsible for writing this sucker.
* Subclasses should implement this so that their data is passed back in a
return rec;
}
- public class MyIterator implements Iterator {
+ private final class MyIterator implements Iterator {
short nextColumn=-1;
int nextRow,lastRow;
package org.apache.poi.hssf.record.aggregates;
-import junit.framework.TestCase;
-import org.apache.poi.hssf.record.*;
-
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
+import java.util.zip.CRC32;
+
+import junit.framework.AssertionFailedError;
+import junit.framework.TestCase;
+
+import org.apache.poi.hssf.record.BlankRecord;
+import org.apache.poi.hssf.record.FormulaRecord;
+import org.apache.poi.hssf.record.Record;
+import org.apache.poi.hssf.record.SharedFormulaRecord;
+import org.apache.poi.hssf.record.UnknownRecord;
+import org.apache.poi.hssf.record.WindowOneRecord;
+import org.apache.poi.hssf.usermodel.HSSFSheet;
+import org.apache.poi.hssf.usermodel.HSSFWorkbook;
public class TestValueRecordsAggregate extends TestCase
{
+ private static final String ABNORMAL_SHARED_FORMULA_FLAG_TEST_FILE = "AbnormalSharedFormulaFlag.xls";
ValueRecordsAggregate valueRecord = new ValueRecordsAggregate();
/**
assertEquals( 36, valueRecord.getRecordSize() );
}
+
+ /**
+ * Sometimes the 'shared formula' flag (<tt>FormulaRecord.isSharedFormula()</tt>) is set when
+ * there is no corresponding SharedFormulaRecord available. SharedFormulaRecord definitions do
+ * not span multiple sheets. They are are only defined within a sheet, and thus they do not
+ * have a sheet index field (only row and column range fields).<br/>
+ * So it is important that the code which locates the SharedFormulaRecord for each
+ * FormulaRecord does not allow matches across sheets.</br>
+ *
+ * Prior to bugzilla 44449 (Feb 2008), POI <tt>ValueRecordsAggregate.construct(int, List)</tt>
+ * allowed <tt>SharedFormulaRecord</tt>s to be erroneously used across sheets. That incorrect
+ * behaviour is shown by this test.<p/>
+ *
+ * <b>Notes on how to produce the test spreadsheet</b>:</p>
+ * The setup for this test (AbnormalSharedFormulaFlag.xls) is rather fragile, insomuchas
+ * re-saving the file (either with Excel or POI) clears the flag.<br/>
+ * <ol>
+ * <li>A new spreadsheet was created in Excel (File | New | Blank Workbook).</li>
+ * <li>Sheet3 was deleted.</li>
+ * <li>Sheet2!A1 formula was set to '="second formula"', and fill-dragged through A1:A8.</li>
+ * <li>Sheet1!A1 formula was set to '="first formula"', and also fill-dragged through A1:A8.</li>
+ * <li>Four rows on Sheet1 "5" through "8" were deleted ('delete rows' alt-E D, not 'clear' Del).</li>
+ * <li>The spreadsheet was saved as AbnormalSharedFormulaFlag.xls.</li>
+ * </ol>
+ * Prior to the row delete action the spreadsheet has two <tt>SharedFormulaRecord</tt>s. One
+ * for each sheet. To expose the bug, the shared formulas have been made to overlap.<br/>
+ * The row delete action (as described here) seems to to delete the
+ * <tt>SharedFormulaRecord</tt> from Sheet1 (but not clear the 'shared formula' flags.<br/>
+ * There are other variations on this theme to create the same effect.
+ *
+ */
+ public void testSpuriousSharedFormulaFlag() {
+ File dataDir = new File(System.getProperty("HSSF.testdata.path"));
+ File testFile = new File(dataDir, ABNORMAL_SHARED_FORMULA_FLAG_TEST_FILE);
+
+ long actualCRC = getFileCRC(testFile);
+ long expectedCRC = 2277445406L;
+ if(actualCRC != expectedCRC) {
+ System.err.println("Expected crc " + expectedCRC + " but got " + actualCRC);
+ throw failUnexpectedTestFileChange();
+ }
+ HSSFWorkbook wb;
+ try {
+ FileInputStream in = new FileInputStream(testFile);
+ wb = new HSSFWorkbook(in);
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+
+ HSSFSheet s = wb.getSheetAt(0); // Sheet1
+
+ String cellFormula;
+ cellFormula = getFormulaFromFirstCell(s, 0); // row "1"
+ // the problem is not observable in the first row of the shared formula
+ if(!cellFormula.equals("\"first formula\"")) {
+ throw new RuntimeException("Something else wrong with this test case");
+ }
+
+ // but the problem is observable in rows 2,3,4
+ cellFormula = getFormulaFromFirstCell(s, 1); // row "2"
+ if(cellFormula.equals("\"second formula\"")) {
+ throw new AssertionFailedError("found bug 44449 (Wrong SharedFormulaRecord was used).");
+ }
+ if(!cellFormula.equals("\"first formula\"")) {
+ throw new RuntimeException("Something else wrong with this test case");
+ }
+ }
+ private static String getFormulaFromFirstCell(HSSFSheet s, int rowIx) {
+ return s.getRow(rowIx).getCell((short)0).getCellFormula();
+ }
+
+ /**
+ * If someone opened this particular test file in Excel and saved it, the peculiar condition
+ * which causes the target bug would probably disappear. This test would then just succeed
+ * regardless of whether the fix was present. So a CRC check is performed to make it less easy
+ * for that to occur.
+ */
+ private static RuntimeException failUnexpectedTestFileChange() {
+ String msg = "Test file '" + ABNORMAL_SHARED_FORMULA_FLAG_TEST_FILE + "' has changed. "
+ + "This junit may not be properly testing for the target bug. "
+ + "Either revert the test file or ensure that the new version "
+ + "has the right characteristics to test the target bug.";
+ // A breakpoint in ValueRecordsAggregate.handleMissingSharedFormulaRecord(FormulaRecord)
+ // should get hit during parsing of Sheet1.
+ // If the test spreadsheet is created as directed, this condition should occur.
+ // It is easy to upset the test spreadsheet (for example re-saving will destroy the
+ // peculiar condition we are testing for).
+ throw new RuntimeException(msg);
+ }
+
+ /**
+ * gets a CRC checksum for the content of a file
+ */
+ private static long getFileCRC(File f) {
+ CRC32 crc = new CRC32();
+ byte[] buf = new byte[2048];
+ try {
+ InputStream is = new FileInputStream(f);
+ while(true) {
+ int bytesRead = is.read(buf);
+ if(bytesRead < 1) {
+ break;
+ }
+ crc.update(buf, 0, bytesRead);
+ }
+ is.close();
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+
+ return crc.getValue();
+ }
+
}
projects / poi.git / commitdiff