Bug 59814

Summary: WorkbookEvaluator.clearAllCachedResultValues() needs to propagate the action to EvaluationWorkbook and EvaluationSheet instances
Product: POI Reporter: Greg Woolsey <greg.woolsey>
Component: XSSFAssignee: POI Developers List <dev>
Severity: normal    
Priority: P2    
Version: 3.15-dev   
Target Milestone: ---   
Hardware: All   
OS: All   
Attachments: Fix and updated unit test resource

Description Greg Woolsey 2016-07-07 02:42:17 UTC
Now that I've added more cached values to support Structured Reference syntax and performance improvements, I see that this method needs augmentation to clear the new caches down at the workbook and sheet level.

XSSFEvaluationWorkbook has _sheetCache, which needs to be set to null
BaseXSSFEvaluationWorkbook has _tableCache, which needs to be set to null
XSSFEvaluationSheet has _cellCache, which needs to be set to null

I'll work on a patch tomorrow, with some tests.  I have some other temp changes to strip out, though, to make current trunk work with another project that doesn't like some of the 3.15 API changes yet.

Motivation for this change is that in that library (Vaadin Spreadsheet) there isn't a way currently to reset the formula evaluator in use, so I need to make sure clear really clears state.
Comment 1 Greg Woolsey 2016-07-07 05:19:18 UTC
This may only be an issue for cells inside a Table, referenced by Structured Reference syntax in a formula - very narrow.  It appears in this new use case the cell values are calculated from the XSSFEvaluationSheet._cellCache and not from the WorkbookEvaluator cell cache, thus the need to push the clear down.  

Still a good idea though, to make it more obvious what needs changing when adding future cached data.

Building a test case will be more work than usual, as it requires a Table and a formula cell that references something in the table that can change.
Comment 2 Greg Woolsey 2016-07-07 19:07:49 UTC
Created attachment 34022 [details]
Fix and updated unit test resource

Attached a patch to propagate the cache clearing through the evaluator object tree.

Includes an updated Structured References unit test and source file.  The updated test fails before the fix and passes with the fix.
Comment 3 Javen O'Neal 2016-07-07 23:18:36 UTC
Applied in r1751836 without modification. Added @Overrides in r1751837.
Thanks for the patch.
Comment 4 Dominik Stadler 2016-07-15 18:51:46 UTC
This seems to be fixed based on the previous comment. Please reopen if not.