Bug 59814

Summary:	WorkbookEvaluator.clearAllCachedResultValues() needs to propagate the action to EvaluationWorkbook and EvaluationSheet instances
Product:	POI	Reporter:	Greg Woolsey <greg.woolsey>
Component:	XSSF	Assignee:	POI Developers List <dev>
Status:	RESOLVED FIXED
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.