From: Javen O'Neal Date: Thu, 7 Jul 2016 22:22:10 +0000 (+0000) Subject: bug 59814: clear evaluation workbook and evaluation sheet caches. Patch from Greg... X-Git-Tag: REL_3_15_BETA3~167 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=3ed02a347913df756c14f3400aa2782a72ba94f7;p=poi.git bug 59814: clear evaluation workbook and evaluation sheet caches. Patch from Greg Woolsey. git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1751836 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationSheet.java b/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationSheet.java index 3497539bed..0af2c842e7 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationSheet.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationSheet.java @@ -45,4 +45,8 @@ final class HSSFEvaluationSheet implements EvaluationSheet { } return new HSSFEvaluationCell(cell, this); } + + public void clearAllCachedResultValues() { + // nothing to do + } } diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationWorkbook.java b/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationWorkbook.java index 81eef90050..61984497ec 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationWorkbook.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationWorkbook.java @@ -64,6 +64,10 @@ public final class HSSFEvaluationWorkbook implements FormulaRenderingWorkbook, E _iBook = book.getWorkbook(); } + public void clearAllCachedResultValues() { + // nothing to do + } + @Override public HSSFName createName() { return _uBook.createName(); diff --git a/src/java/org/apache/poi/ss/formula/EvaluationSheet.java b/src/java/org/apache/poi/ss/formula/EvaluationSheet.java index 5dec3aacda..3cc292dc54 100644 --- a/src/java/org/apache/poi/ss/formula/EvaluationSheet.java +++ b/src/java/org/apache/poi/ss/formula/EvaluationSheet.java @@ -30,4 +30,12 @@ public interface EvaluationSheet { * @return null if there is no cell at the specified coordinates */ EvaluationCell getCell(int rowIndex, int columnIndex); + + /** + * Propagated from {@link EvaluationWorkbook#clearAllCachedResultValues()} to clear locally cached data. + * + * @see WorkbookEvaluator#clearAllCachedResultValues() + * @see EvaluationWorkbook#clearAllCachedResultValues() + */ + public void clearAllCachedResultValues(); } diff --git a/src/java/org/apache/poi/ss/formula/EvaluationWorkbook.java b/src/java/org/apache/poi/ss/formula/EvaluationWorkbook.java index b0a3c76060..fc4cfb32b5 100644 --- a/src/java/org/apache/poi/ss/formula/EvaluationWorkbook.java +++ b/src/java/org/apache/poi/ss/formula/EvaluationWorkbook.java @@ -73,6 +73,13 @@ public interface EvaluationWorkbook { String resolveNameXText(NameXPtg ptg); Ptg[] getFormulaTokens(EvaluationCell cell); UDFFinder getUDFFinder(); + + /** + * Propagated from {@link WorkbookEvaluator#clearAllCachedResultValues()} to clear locally cached data. + * Implementations must call the same method on all referenced {@link EvaluationSheet} instances, as well as clearing local caches. + * @see WorkbookEvaluator#clearAllCachedResultValues() + */ + public void clearAllCachedResultValues(); class ExternalSheet { private final String _workbookName; diff --git a/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java b/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java index d1e7d8b226..4ed054467f 100644 --- a/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java +++ b/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java @@ -208,6 +208,7 @@ public final class WorkbookEvaluator { public void clearAllCachedResultValues() { _cache.clear(); _sheetIndexesBySheet.clear(); + _workbook.clearAllCachedResultValues(); } /** diff --git a/src/java/org/apache/poi/ss/formula/eval/forked/ForkedEvaluationSheet.java b/src/java/org/apache/poi/ss/formula/eval/forked/ForkedEvaluationSheet.java index 886919fe6e..fcf00a488b 100644 --- a/src/java/org/apache/poi/ss/formula/eval/forked/ForkedEvaluationSheet.java +++ b/src/java/org/apache/poi/ss/formula/eval/forked/ForkedEvaluationSheet.java @@ -101,6 +101,14 @@ final class ForkedEvaluationSheet implements EvaluationSheet { return mewb.getSheetIndex(_masterSheet); } + /* (non-Javadoc) + * leave the map alone, if it needs resetting, reusing this class is probably a bad idea. + * @see org.apache.poi.ss.formula.EvaluationSheet#clearAllCachedResultValues() + */ + public void clearAllCachedResultValues() { + _masterSheet.clearAllCachedResultValues(); + } + private static final class RowColKey implements Comparable{ private final int _rowIndex; private final int _columnIndex; diff --git a/src/java/org/apache/poi/ss/formula/eval/forked/ForkedEvaluationWorkbook.java b/src/java/org/apache/poi/ss/formula/eval/forked/ForkedEvaluationWorkbook.java index b936a54760..5e1da9bdc9 100644 --- a/src/java/org/apache/poi/ss/formula/eval/forked/ForkedEvaluationWorkbook.java +++ b/src/java/org/apache/poi/ss/formula/eval/forked/ForkedEvaluationWorkbook.java @@ -136,4 +136,12 @@ final class ForkedEvaluationWorkbook implements EvaluationWorkbook { public UDFFinder getUDFFinder(){ return _masterBook.getUDFFinder(); } + + /* (non-Javadoc) + * leave the map alone, if it needs resetting, reusing this class is probably a bad idea. + * @see org.apache.poi.ss.formula.EvaluationSheet#clearAllCachedResultValues() + */ + public void clearAllCachedResultValues() { + _masterBook.clearAllCachedResultValues(); + } } diff --git a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFEvaluationSheet.java b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFEvaluationSheet.java index c5d809cada..f2c11953f9 100644 --- a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFEvaluationSheet.java +++ b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFEvaluationSheet.java @@ -47,4 +47,8 @@ final class SXSSFEvaluationSheet implements EvaluationSheet { } return new SXSSFEvaluationCell(cell, this); } + + public void clearAllCachedResultValues() { + // nothing to do + } } diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/BaseXSSFEvaluationWorkbook.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/BaseXSSFEvaluationWorkbook.java index 44b182b4d7..8f0eac330e 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/BaseXSSFEvaluationWorkbook.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/BaseXSSFEvaluationWorkbook.java @@ -56,6 +56,10 @@ public abstract class BaseXSSFEvaluationWorkbook implements FormulaRenderingWork _uBook = book; } + public void clearAllCachedResultValues() { + _tableCache = null; + } + private int convertFromExternalSheetIndex(int externSheetIndex) { return externSheetIndex; } diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationSheet.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationSheet.java index 0286740e08..78aa8f86c6 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationSheet.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationSheet.java @@ -41,6 +41,10 @@ final class XSSFEvaluationSheet implements EvaluationSheet { return _xs; } + public void clearAllCachedResultValues() { + _cellCache = null; + } + public EvaluationCell getCell(int rowIndex, int columnIndex) { // cache for performance: ~30% speedup due to caching if (_cellCache == null) { diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationWorkbook.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationWorkbook.java index 1e849b673c..60f99676cf 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationWorkbook.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationWorkbook.java @@ -40,6 +40,11 @@ public final class XSSFEvaluationWorkbook extends BaseXSSFEvaluationWorkbook { super(book); } + public void clearAllCachedResultValues() { + super.clearAllCachedResultValues(); + _sheetCache = null; + } + @Override public int getSheetIndex(EvaluationSheet evalSheet) { XSSFSheet sheet = ((XSSFEvaluationSheet)evalSheet).getXSSFSheet(); diff --git a/src/ooxml/testcases/org/apache/poi/ss/formula/TestStructuredReferences.java b/src/ooxml/testcases/org/apache/poi/ss/formula/TestStructuredReferences.java index fd598a479b..4e176257f6 100644 --- a/src/ooxml/testcases/org/apache/poi/ss/formula/TestStructuredReferences.java +++ b/src/ooxml/testcases/org/apache/poi/ss/formula/TestStructuredReferences.java @@ -18,17 +18,22 @@ package org.apache.poi.ss.formula; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import org.apache.poi.ss.usermodel.Cell; import org.apache.poi.ss.usermodel.CellType; import org.apache.poi.ss.usermodel.CellValue; import org.apache.poi.ss.usermodel.FormulaEvaluator; +import org.apache.poi.ss.usermodel.Row; import org.apache.poi.ss.usermodel.Table; +import org.apache.poi.ss.util.AreaReference; +import org.apache.poi.ss.util.CellReference; import org.apache.poi.xssf.XSSFTestDataSamples; import org.apache.poi.xssf.usermodel.XSSFFormulaEvaluator; +import org.apache.poi.xssf.usermodel.XSSFSheet; +import org.apache.poi.xssf.usermodel.XSSFTable; import org.apache.poi.xssf.usermodel.XSSFWorkbook; import org.junit.Test; @@ -63,8 +68,40 @@ public class TestStructuredReferences { try { final FormulaEvaluator eval = new XSSFFormulaEvaluator(wb); - confirm(eval, wb.getSheet("Table").getRow(5).getCell(0), 49); - confirm(eval, wb.getSheet("Formulas").getRow(0).getCell(0), 209); + final XSSFSheet tableSheet = wb.getSheet("Table"); + final XSSFSheet formulaSheet = wb.getSheet("Formulas"); + + confirm(eval, tableSheet.getRow(5).getCell(0), 49); + confirm(eval, formulaSheet.getRow(0).getCell(0), 209); + confirm(eval, formulaSheet.getRow(1).getCell(0), "one"); + + // test changing a table value, to see if the caches are properly cleared + // Issue 59814 + + // this test passes before the fix for 59814 + tableSheet.getRow(1).getCell(1).setCellValue("ONEA"); + confirm(eval, formulaSheet.getRow(1).getCell(0), "ONEA"); + + // test adding a row to a table, issue 59814 + Row newRow = tableSheet.getRow(7); + if (newRow == null) newRow = tableSheet.createRow(7); + newRow.createCell(0, CellType.FORMULA).setCellFormula("\\_Prime.1[[#This Row],[@Number]]*\\_Prime.1[[#This Row],[@Number]]"); + newRow.createCell(1, CellType.STRING).setCellValue("thirteen"); + newRow.createCell(2, CellType.NUMERIC).setCellValue(13); + + // update Table + final XSSFTable table = wb.getTable("\\_Prime.1"); + final AreaReference newArea = new AreaReference(table.getStartCellReference(), new CellReference(table.getEndRowIndex() + 1, table.getEndColIndex())); + String newAreaStr = newArea.formatAsString(); + table.getCTTable().setRef(newAreaStr); + table.getCTTable().getAutoFilter().setRef(newAreaStr); + table.updateHeaders(); + table.updateReferences(); + + // these fail before the fix for 59814 + confirm(eval, tableSheet.getRow(7).getCell(0), 13*13); + confirm(eval, formulaSheet.getRow(0).getCell(0), 209 + 13*13); + } finally { wb.close(); } @@ -78,4 +115,13 @@ public class TestStructuredReferences { } assertEquals(expectedResult, cv.getNumberValue(), 0.0); } + + private static void confirm(FormulaEvaluator fe, Cell cell, String expectedResult) { + fe.clearAllCachedResultValues(); + CellValue cv = fe.evaluate(cell); + if (cv.getCellType() != CellType.STRING) { + fail("expected String cell type but got " + cv.formatAsString()); + } + assertEquals(expectedResult, cv.getStringValue()); + } } diff --git a/test-data/spreadsheet/StructuredReferences.xlsx b/test-data/spreadsheet/StructuredReferences.xlsx index 54cf75234e..90a470a93f 100644 Binary files a/test-data/spreadsheet/StructuredReferences.xlsx and b/test-data/spreadsheet/StructuredReferences.xlsx differ