Skip to content

Commit

Permalink
ZSS-687: Formula that refer to a NameRange does not update contents c…
Browse files Browse the repository at this point in the history
…orrectly
  • Loading branch information
henrichen committed Jun 10, 2014
1 parent facdc96 commit c5469cf
Show file tree
Hide file tree
Showing 3 changed files with 155 additions and 0 deletions.
1 change: 1 addition & 0 deletions zkdoc/release-note
Expand Up @@ -31,6 +31,7 @@ ZK Spreadsheet 3.5.0
ZSS-683 Enter formula with bad syntax, would cause infinite error message window
ZSS-661 Formula is incorrect after rename Name or it's refersTo region.
ZSS-649 After delete row, NameRange doesn't update
ZSS-687 Formula that refer to a NameRange does not update contents correctly

** Upgrade Notes
* Following api are deprecated
Expand Down
38 changes: 38 additions & 0 deletions zss.test/src/test/java/org/zkoss/zss/api/impl/Issue600Test.java
Expand Up @@ -342,6 +342,44 @@ public void testZSS685ImportStyle(){
}
}

@Test
public void testZSS687ReferName() {
SBook book = SBooks.createBook("book1");
book.getBookSeries().setAutoFormulaCacheClean(true);
SSheet sheet1 = book.createSheet("SheetX");

SName name = book.createName("FOO");
name.setRefersToFormula("SheetX!A1:B2");
sheet1.getCell("D2").setValue("=SUM(FOO)");

// test insert/delete cells
sheet1.getCell("A1").setValue(1);
sheet1.getCell("B1").setValue(2);
sheet1.getCell("A2").setValue(3);
sheet1.getCell("B2").setValue(4);

Assert.assertEquals("SheetX!A1:B2", name.getRefersToFormula());
Assert.assertEquals(10D, sheet1.getCell("D2").getValue());

// insert B1:B1
sheet1.insertCell(0, 1, 0, 1, true);
Assert.assertEquals(null, sheet1.getCell("B1").getValue());
Assert.assertEquals("SheetX!A1:B2", name.getRefersToFormula());
Assert.assertEquals(8D, sheet1.getCell("D2").getValue());

// delete B1:B1
sheet1.deleteCell(0, 1, 0, 1, true);
Assert.assertEquals(2D, sheet1.getCell("B1").getValue());
Assert.assertEquals("SheetX!A1:B2", name.getRefersToFormula());
Assert.assertEquals(10D, sheet1.getCell("D2").getValue());

// move B1:B1
sheet1.moveCell(0, 1, 0, 1, 1, 1);
Assert.assertEquals(2D, sheet1.getCell("C2").getValue());
Assert.assertEquals("SheetX!A1:B2", name.getRefersToFormula());
Assert.assertEquals(8D, sheet1.getCell("D2").getValue());
}

private static Object[] _loadBooks(Object base,String respath) {
if(base==null){
base = Util.class;
Expand Down
116 changes: 116 additions & 0 deletions zssmodel/src/org/zkoss/zss/model/impl/FormulaTunerHelper.java
Expand Up @@ -16,6 +16,8 @@
*/
package org.zkoss.zss.model.impl;

import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.Map;
Expand All @@ -33,6 +35,7 @@
import org.zkoss.zss.model.chart.SGeneralChartData;
import org.zkoss.zss.model.chart.SSeries;
import org.zkoss.zss.model.sys.EngineFactory;
import org.zkoss.zss.model.sys.dependency.DependencyTable;
import org.zkoss.zss.model.sys.dependency.NameRef;
import org.zkoss.zss.model.sys.dependency.ObjectRef;
import org.zkoss.zss.model.sys.dependency.Ref;
Expand Down Expand Up @@ -206,6 +209,9 @@ private void moveNameRef(SheetRegion sheetRegion,NameRef dependent,int rowOffset
name.setRefersToFormula(exprAfter.getFormulaString());
//don't need to notify name change, name will do
}else{
//zss-687, clear dependents's cache of NameRef
clearFormulaCache(dependent);

//zss-626, has to clear cache and notify ref update
ModelUpdateUtil.addRefUpdate(dependent);
}
Expand Down Expand Up @@ -379,6 +385,8 @@ private void extendNameRef(SheetRegion sheetRegion, NameRef dependent, boolean h
name.setRefersToFormula(exprAfter.getFormulaString());
//don't need to notify cell change, cell will do
}else{
//zss-687, clear dependents's cache of NameRef
clearFormulaCache(dependent);

//zss-626, has to clear cache and notify ref update
ModelUpdateUtil.addRefUpdate(dependent);
Expand Down Expand Up @@ -545,6 +553,9 @@ private void shrinkNameRef(SheetRegion sheetRegion, NameRef dependent, boolean h
name.setRefersToFormula(exprAfter.getFormulaString());
//don't need to notify name change, setRefersToFormula will do
}else{
//zss-687, clear dependents's cache of NameRef
clearFormulaCache(dependent);

ModelUpdateUtil.addRefUpdate(dependent);
}
}
Expand Down Expand Up @@ -771,4 +782,109 @@ private void splitDependents(final Set<Ref> dependents,
}
}
}

//ZSS-687
private void clearFormulaCache(NameRef precedent) {
Map<String,Ref> chartDependents = new HashMap<String, Ref>();
Map<String,Ref> validationDependents = new HashMap<String, Ref>();
Set<Ref> nameDependents = new HashSet<Ref>();
AbstractBookSeriesAdv bs = (AbstractBookSeriesAdv) _bookSeries;
DependencyTable dt = bs.getDependencyTable();

clearFormulaCache(precedent, dt, chartDependents, validationDependents, nameDependents);

for (Ref dependent : chartDependents.values()) {
clearFormulaCacheChartRef((ObjectRef)dependent);
}
for (Ref dependent : validationDependents.values()) {
clearFormulaCacheDataValidationRef((ObjectRef)dependent);
}
}

// ZSS-687
private void clearFormulaCache(NameRef precedent,
DependencyTable dt,
Map<String,Ref> chartDependents,
Map<String,Ref> validationDependents,
Set<Ref> nameDependents) {

Set<Ref> dependents = dt.getDependents(precedent);

for (Ref dependent : dependents) {
RefType type = dependent.getType();
if (type == RefType.CELL) {
clearFormulaCacheCellRef(dependent);
} else if (type == RefType.OBJECT) {
if(((ObjectRef)dependent).getObjectType()==ObjectType.CHART){
chartDependents.put(((ObjectRef)dependent).getObjectIdPath()[0], dependent);
}else if(((ObjectRef)dependent).getObjectType()==ObjectType.DATA_VALIDATION){
validationDependents.put(((ObjectRef)dependent).getObjectIdPath()[0], dependent);
}
} else if (type == RefType.NAME) {
if (!nameDependents.contains(dependent)) {
nameDependents.add(dependent);
// recursive back when NameRef depends on NameRef
clearFormulaCache((NameRef)dependent, dt,
chartDependents, validationDependents, nameDependents);
}
} else {// TODO another

}
}
}

// ZSS-687
private void clearFormulaCacheDataValidationRef(ObjectRef dependent) {
//TODO zss 3.5
// NBook book = bookSeries.getBook(dependent.getBookName());
// if(book==null) return;
// SSheet sheet = book.getSheetByName(dependent.getSheetName());
// if(sheet==null) return;
// String[] ids = dependent.getObjectIdPath();
// NDataValidation validation = sheet.getDataValidation(ids[0]);
// if(validation!=null){
// validation.clearFormulaResultCache();
// ModelUpdateUtil.addRefUpdate(dependent);
// }
}

// ZSS-687
private void clearFormulaCacheChartRef(ObjectRef dependent) {
SBook book = _bookSeries.getBook(dependent.getBookName());
if(book == null) {
return;
}
SSheet sheet = book.getSheetByName(dependent.getSheetName());
if(sheet == null) {
return;
}
SChart chart = sheet.getChart(dependent.getObjectIdPath()[0]);
if(chart == null) {
return;
}
SChartData d = chart.getData();
if(!(d instanceof SGeneralChartData)) {
return;
}
SGeneralChartData data = (SGeneralChartData)d;

data.clearFormulaResultCache();
ModelUpdateUtil.addRefUpdate(dependent);
}

// ZSS-687
private void clearFormulaCacheCellRef(Ref dependent) {
SBook book = _bookSeries.getBook(dependent.getBookName());
if(book==null) return;
SSheet sheet = book.getSheetByName(dependent.getSheetName());
if(sheet==null) return;
SCell cell = sheet.getCell(dependent.getRow(),
dependent.getColumn());
if(cell.getType()!=CellType.FORMULA)
return;//impossible

//zss-626, has to clear cache and notify ref update
cell.clearFormulaResultCache();
ModelUpdateUtil.addRefUpdate(dependent);
}
}

0 comments on commit c5469cf

Please sign in to comment.