Fix for bug 45890 - made HSSFSheet.shiftRows also update conditional formats

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@700005 13f79535-47bb-0310-9956-ffa450edef68

src/documentation/content/xdocs/changes.xml

@@ -37,6 +37,7 @@
 
 		<!-- Don't forget to update status.xml too! -->
         <release version="3.2-alpha1" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="add">45890 - fixed HSSFSheet.shiftRows to also update conditional formats</action>
            <action dev="POI-DEVELOPERS" type="add">45865 modified Formula Parser/Evaluator to handle cross-worksheet formulas</action>
            <action dev="POI-DEVELOPERS" type="add">Optimised the FormulaEvaluator to take cell dependencies into account</action>
            <action dev="POI-DEVELOPERS" type="add">16936 - Initial support for whole-row cell styling</action>

src/documentation/content/xdocs/status.xml

@@ -34,6 +34,7 @@
 	<!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.2-alpha1" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="add">45890 - fixed HSSFSheet.shiftRows to also update conditional formats</action>
            <action dev="POI-DEVELOPERS" type="add">45865 modified Formula Parser/Evaluator to handle cross-worksheet formulas</action>
            <action dev="POI-DEVELOPERS" type="add">Optimised the FormulaEvaluator to take cell dependencies into account</action>
            <action dev="POI-DEVELOPERS" type="add">16936 - Initial support for whole-row cell styling</action>

src/java/org/apache/poi/hssf/model/Sheet.java

@@ -68,6 +68,7 @@ import org.apache.poi.hssf.record.aggregates.RecordAggregate;
 import org.apache.poi.hssf.record.aggregates.RowRecordsAggregate;
 import org.apache.poi.hssf.record.aggregates.RecordAggregate.PositionTrackingVisitor;
 import org.apache.poi.hssf.record.aggregates.RecordAggregate.RecordVisitor;
+import org.apache.poi.hssf.record.formula.FormulaShifter;
 import org.apache.poi.hssf.util.CellRangeAddress;
 import org.apache.poi.hssf.util.PaneInformation;
 import org.apache.poi.util.POILogFactory;
@@ -470,6 +471,15 @@ public final class Sheet implements Model {
         return _mergedCellsTable;
     }
 
+    /**
+     * Updates formulas in cells and conditional formats due to moving of cells
+     * @param externSheetIndex the externSheet index of this sheet 
+     */
+    public void updateFormulasAfterCellShift(FormulaShifter shifter, int externSheetIndex) {
+        getRowsAggregate().updateFormulasAfterRowShift(shifter, externSheetIndex);
+        getConditionalFormattingTable().updateFormulasAfterCellShift(shifter, externSheetIndex);
+        // TODO - adjust data validations 
+    }
 
     public int addMergedRegion(int rowFrom, int colFrom, int rowTo, int colTo) {
         // Validate input
@@ -509,7 +519,7 @@ public final class Sheet implements Model {
     public int getNumMergedRegions() {
         return getMergedRecords().getNumberOfMergedRegions();
     }
-    private ConditionalFormattingTable getConditionalFormattingTable() {
+    public ConditionalFormattingTable getConditionalFormattingTable() {
         if (condFormatting == null) {
             condFormatting = new ConditionalFormattingTable();
             RecordOrderer.addNewSheetRecord(records, condFormatting);
@@ -517,24 +527,6 @@ public final class Sheet implements Model {
         return condFormatting;
     }
 
-
-    public int addConditionalFormatting(CFRecordsAggregate cfAggregate) {
-        ConditionalFormattingTable cft = getConditionalFormattingTable();
-        return cft.add(cfAggregate);
-    }
-
-    public void removeConditionalFormatting(int index) {
-        getConditionalFormattingTable().remove(index);
-    }
-
-    public CFRecordsAggregate getCFRecordsAggregateAt(int index) {
-        return getConditionalFormattingTable().get(index);
-    }
-
-    public int getNumConditionalFormattings() {
-        return getConditionalFormattingTable().size();
-    }
-
     /**
      * Per an earlier reported bug in working with Andy Khan's excel read library.  This
      * sets the values in the sheet's DimensionsRecord object to be correct.  Excel doesn't

src/java/org/apache/poi/hssf/record/CFRuleRecord.java

@@ -420,6 +420,15 @@ public final class CFRuleRecord extends Record {
 	{
 		return field_17_formula1;
 	}
+	public void setParsedExpression1(Ptg[] ptgs) {
+		field_17_formula1 = safeClone(ptgs);
+	}
+	private static Ptg[] safeClone(Ptg[] ptgs) {
+		if (ptgs == null) {
+			return null;
+		}
+		return (Ptg[]) ptgs.clone();
+	}
 
 	/**
 	 * get the stack of the 2nd expression as a list
@@ -434,6 +443,9 @@ public final class CFRuleRecord extends Record {
 	{
 		return field_18_formula2;
 	}
+	public void setParsedExpression2(Ptg[] ptgs) {
+		field_18_formula2 = safeClone(ptgs);
+	}
 	
 	/**
 	 * called by constructor, should throw runtime exception in the event of a

src/java/org/apache/poi/hssf/record/aggregates/CFRecordsAggregate.java

@@ -24,6 +24,10 @@ import org.apache.poi.hssf.model.RecordStream;
 import org.apache.poi.hssf.record.CFHeaderRecord;
 import org.apache.poi.hssf.record.CFRuleRecord;
 import org.apache.poi.hssf.record.Record;
+import org.apache.poi.hssf.record.formula.AreaErrPtg;
+import org.apache.poi.hssf.record.formula.AreaPtg;
+import org.apache.poi.hssf.record.formula.FormulaShifter;
+import org.apache.poi.hssf.record.formula.Ptg;
 import org.apache.poi.hssf.util.CellRangeAddress;
 
 /**
@@ -174,4 +178,68 @@ public final class CFRecordsAggregate extends RecordAggregate {
 			rv.visitRecord(rule);
 		}
 	}
+
+	/**
+	 * @return <code>false</code> if this whole {@link CFHeaderRecord} / {@link CFRuleRecord}s should be deleted
+	 */
+	public boolean updateFormulasAfterCellShift(FormulaShifter shifter, int currentExternSheetIx) {
+		CellRangeAddress[] cellRanges = header.getCellRanges();
+		boolean changed = false;
+		List temp = new ArrayList();
+		for (int i = 0; i < cellRanges.length; i++) {
+			CellRangeAddress craOld = cellRanges[i];
+			CellRangeAddress craNew = shiftRange(shifter, craOld, currentExternSheetIx);
+			if (craNew == null) {
+				changed = true;
+				continue;
+			}
+			temp.add(craNew);
+			if (craNew != craOld) {
+				changed = true;
+			}
+		}
+
+		if (changed) {
+			int nRanges = temp.size();
+			if (nRanges == 0) {
+				return false;
+			}
+			CellRangeAddress[] newRanges = new CellRangeAddress[nRanges];
+			temp.toArray(newRanges);
+			header.setCellRanges(newRanges);
+		}
+		
+		for(int i=0; i<rules.size(); i++) {
+			CFRuleRecord rule = (CFRuleRecord)rules.get(i);
+			Ptg[] ptgs;
+			ptgs = rule.getParsedExpression1();
+			if (ptgs != null && shifter.adjustFormula(ptgs, currentExternSheetIx)) {
+				rule.setParsedExpression1(ptgs);
+			}
+			ptgs = rule.getParsedExpression2();
+			if (ptgs != null && shifter.adjustFormula(ptgs, currentExternSheetIx)) {
+				rule.setParsedExpression2(ptgs);
+			}
+		}
+		return true;
+	}
+
+	private static CellRangeAddress shiftRange(FormulaShifter shifter, CellRangeAddress cra, int currentExternSheetIx) {
+		// FormulaShifter works well in terms of Ptgs - so convert CellRangeAddress to AreaPtg (and back) here
+		AreaPtg aptg = new AreaPtg(cra.getFirstRow(), cra.getLastRow(), cra.getFirstColumn(), cra.getLastColumn(), false, false, false, false);
+		Ptg[] ptgs = { aptg, };
+		
+		if (!shifter.adjustFormula(ptgs, currentExternSheetIx)) {
+			return cra;
+		}
+		Ptg ptg0 = ptgs[0];
+		if (ptg0 instanceof AreaPtg) {
+			AreaPtg bptg = (AreaPtg) ptg0;
+			return new CellRangeAddress(bptg.getFirstRow(), bptg.getLastRow(), bptg.getFirstColumn(), bptg.getLastColumn());
+		}
+		if (ptg0 instanceof AreaErrPtg) {
+			return null;
+		}
+		throw new IllegalStateException("Unexpected shifted ptg class (" + ptg0.getClass().getName() + ")");
+	}
 }

src/java/org/apache/poi/hssf/record/aggregates/ConditionalFormattingTable.java

@@ -22,6 +22,7 @@ import java.util.List;
 
 import org.apache.poi.hssf.model.RecordStream;
 import org.apache.poi.hssf.record.CFHeaderRecord;
+import org.apache.poi.hssf.record.formula.FormulaShifter;
 
 /**
  * Holds all the conditional formatting for a workbook sheet.<p/>
@@ -85,4 +86,15 @@ public final class ConditionalFormattingTable extends RecordAggregate {
 					+ " is outside the allowable range (0.." + (_cfHeaders.size() - 1) + ")");
 		}
 	}
+
+	public void updateFormulasAfterCellShift(FormulaShifter shifter, int externSheetIndex) {
+		for (int i = 0; i < _cfHeaders.size(); i++) {
+			CFRecordsAggregate subAgg = (CFRecordsAggregate) _cfHeaders.get(i);
+			boolean shouldKeep = subAgg.updateFormulasAfterCellShift(shifter, externSheetIndex);
+			if (!shouldKeep) {
+				_cfHeaders.remove(i);
+				i--;
+			}
+		}
+	}
 }

src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java

@@ -1272,7 +1272,7 @@ public final class HSSFSheet {
         int sheetIndex = workbook.getSheetIndex(this);
         short externSheetIndex = book.checkExternSheet(sheetIndex);
         FormulaShifter shifter = FormulaShifter.createForRowShift(externSheetIndex, startRow, endRow, n);
-        sheet.getRowsAggregate().updateFormulasAfterRowShift(shifter, externSheetIndex);
+        sheet.updateFormulasAfterCellShift(shifter, externSheetIndex);
 
         int nSheets = workbook.getNumberOfSheets();
         for(int i=0; i<nSheets; i++) {
@@ -1281,7 +1281,7 @@ public final class HSSFSheet {
                 continue;
             }
             short otherExtSheetIx = book.checkExternSheet(i);
-            otherSheet.getRowsAggregate().updateFormulasAfterRowShift(shifter, otherExtSheetIx);
+            otherSheet.updateFormulasAfterCellShift(shifter, otherExtSheetIx);
         }
         // TODO - adjust formulas in named ranges
     }

src/java/org/apache/poi/hssf/usermodel/HSSFSheetConditionalFormatting.java

@@ -20,6 +20,7 @@ package org.apache.poi.hssf.usermodel;
 import org.apache.poi.hssf.model.Sheet;
 import org.apache.poi.hssf.record.CFRuleRecord;
 import org.apache.poi.hssf.record.aggregates.CFRecordsAggregate;
+import org.apache.poi.hssf.record.aggregates.ConditionalFormattingTable;
 import org.apache.poi.hssf.util.CellRangeAddress;
 import org.apache.poi.hssf.util.Region;
 
@@ -31,11 +32,11 @@ import org.apache.poi.hssf.util.Region;
 public final class HSSFSheetConditionalFormatting {
 	
 	private final HSSFWorkbook _workbook;
-	private final Sheet _sheet;
+	private final ConditionalFormattingTable _conditionalFormattingTable;
 
 	/* package */ HSSFSheetConditionalFormatting(HSSFWorkbook workbook, Sheet sheet) {
 		_workbook = workbook;
-		_sheet = sheet;
+		_conditionalFormattingTable = sheet.getConditionalFormattingTable();
 	}
 
 	/**
@@ -99,7 +100,7 @@ public final class HSSFSheetConditionalFormatting {
 	public int addConditionalFormatting( HSSFConditionalFormatting cf ) {
 		CFRecordsAggregate cfraClone = cf.getCFRecordsAggregate().cloneCFAggregate();
 
-		return _sheet.addConditionalFormatting(cfraClone);
+		return _conditionalFormattingTable.add(cfraClone);
 	}
 	/**
 	 * @deprecated use <tt>CellRangeAddress</tt> instead of <tt>Region</tt>
@@ -134,7 +135,7 @@ public final class HSSFSheetConditionalFormatting {
 			rules[i] = cfRules[i].getCfRuleRecord();
 		}
 		CFRecordsAggregate cfra = new CFRecordsAggregate(regions, rules);
-		return _sheet.addConditionalFormatting(cfra);
+		return _conditionalFormattingTable.add(cfra);
 	}
 
 	public int addConditionalFormatting(CellRangeAddress[] regions,
@@ -166,7 +167,7 @@ public final class HSSFSheetConditionalFormatting {
 	* @return Conditional Formatting object
 	*/
 	public HSSFConditionalFormatting getConditionalFormattingAt(int index) {
-		CFRecordsAggregate cf = _sheet.getCFRecordsAggregateAt(index);
+		CFRecordsAggregate cf = _conditionalFormattingTable.get(index);
 		if (cf == null) {
 			return null;
 		}
@@ -177,7 +178,7 @@ public final class HSSFSheetConditionalFormatting {
 	* @return number of Conditional Formatting objects of the sheet
 	*/
 	public int getNumConditionalFormattings() {
-		return _sheet.getNumConditionalFormattings();
+		return _conditionalFormattingTable.size();
 	}
 
 	/**
@@ -185,6 +186,6 @@ public final class HSSFSheetConditionalFormatting {
 	* @param index of a Conditional Formatting object to remove
 	*/
 	public void removeConditionalFormatting(int index) {
-		_sheet.removeConditionalFormatting(index);
+		_conditionalFormattingTable.remove(index);
 	}
 }

src/testcases/org/apache/poi/hssf/usermodel/TestHSSFConditionalFormatting.java

@@ -27,10 +27,8 @@ import org.apache.poi.hssf.util.HSSFColor;
  * 
  * @author Dmitriy Kumshayev
  */
-public final class TestHSSFConditionalFormatting extends TestCase
-{
-	public void testCreateCF() 
-	{
+public final class TestHSSFConditionalFormatting extends TestCase {
+	public void testCreateCF() {
 		HSSFWorkbook workbook = new HSSFWorkbook();
 		HSSFSheet sheet = workbook.createSheet();
 		String formula = "7";
@@ -148,4 +146,44 @@ public final class TestHSSFConditionalFormatting extends TestCase
 		}
 		assertEquals(2, wb.getNumberOfSheets());
 	}
+
+	public void testShiftRows() {
+
+		HSSFWorkbook wb = new HSSFWorkbook();
+		HSSFSheet sheet = wb.createSheet();
+
+		HSSFSheetConditionalFormatting sheetCF = sheet.getSheetConditionalFormatting();
+
+		HSSFConditionalFormattingRule rule1 = sheetCF.createConditionalFormattingRule(
+				ComparisonOperator.BETWEEN, "sum(A10:A15)", "1+sum(B16:B30)");
+		HSSFFontFormatting fontFmt = rule1.createFontFormatting();
+		fontFmt.setFontStyle(true, false);
+
+		HSSFPatternFormatting patternFmt = rule1.createPatternFormatting();
+		patternFmt.setFillBackgroundColor(HSSFColor.YELLOW.index);
+		HSSFConditionalFormattingRule [] cfRules = { rule1, };
+
+		CellRangeAddress [] regions = {
+			new CellRangeAddress(2, 4, 0, 0), // A3:A5
+		};
+		sheetCF.addConditionalFormatting(regions, cfRules);
+
+		// This row-shift should destroy the CF region
+		sheet.shiftRows(10, 20, -9);
+		assertEquals(0, sheetCF.getNumConditionalFormattings());
+
+		// re-add the CF
+		sheetCF.addConditionalFormatting(regions, cfRules);
+
+		// This row shift should only affect the formulas
+		sheet.shiftRows(14, 17, 8);
+		HSSFConditionalFormatting cf = sheetCF.getConditionalFormattingAt(0);
+		assertEquals("SUM(A10:A23)", cf.getRule(0).getFormula1());
+		assertEquals("1+SUM(B24:B30)", cf.getRule(0).getFormula2());
+
+		sheet.shiftRows(0, 8, 21);
+		cf = sheetCF.getConditionalFormattingAt(0);
+		assertEquals("SUM(A10:A21)", cf.getRule(0).getFormula1());
+		assertEquals("1+SUM(#REF!)", cf.getRule(0).getFormula2());
+	}
 }