bug 48294 - Fixed HSSFWorkbook.setSheetOrder() to respect inter-sheet references

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

src/documentation/content/xdocs/status.xml

@@ -34,6 +34,7 @@
 
     <changes>
         <release version="3.8-beta4" date="2011-??-??">
+           <action dev="poi-developers" type="fix">48294 - Fixed HSSFWorkbook.setSheetOrder() to respect inter-sheet references </action>
            <action dev="poi-developers" type="fix">51448 - Avoid exception when evaluating workbooks with more than 256 sheets </action>
            <action dev="poi-developers" type="fix">51458 - Correct BitField wrapping when setting large values</action>
            <action dev="poi-developers" type="add">51460 - Improve HSSF performance when loading very long rows, by switching the CellValue array to an iterator</action>

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

@@ -42,6 +42,7 @@ import org.apache.poi.hssf.model.InternalWorkbook;
 import org.apache.poi.hssf.record.*;
 import org.apache.poi.hssf.record.aggregates.RecordAggregate.RecordVisitor;
 import org.apache.poi.hssf.record.common.UnicodeString;
+import org.apache.poi.ss.formula.FormulaShifter;
 import org.apache.poi.ss.formula.ptg.Area3DPtg;
 import org.apache.poi.ss.formula.ptg.MemFuncPtg;
 import org.apache.poi.ss.formula.ptg.OperandPtg;
@@ -414,8 +415,17 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss
      */
 
     public void setSheetOrder(String sheetname, int pos ) {
-        _sheets.add(pos,_sheets.remove(getSheetIndex(sheetname)));
+        int oldSheetIndex = getSheetIndex(sheetname);
+        _sheets.add(pos,_sheets.remove(oldSheetIndex));
         workbook.setSheetOrder(sheetname, pos);
+
+        FormulaShifter shifter = FormulaShifter.createForSheetShift(oldSheetIndex, pos);
+        for (HSSFSheet sheet : _sheets) {
+            sheet.getSheet().updateFormulasAfterCellShift(shifter, /* not used */ -1 );
+        }
+
+        workbook.updateNamesAfterCellShift(shifter);
+
     }
 
     private void validateSheetIndex(int index) {

src/java/org/apache/poi/ss/formula/FormulaShifter.java

@@ -25,6 +25,11 @@ import org.apache.poi.ss.formula.ptg.*;
  */
 public final class FormulaShifter {
 
+    static enum ShiftMode {
+        Row,
+        Sheet
+    }
+
 	/**
 	 * Extern sheet index of sheet where moving is occurring
 	 */
@@ -33,6 +38,16 @@ public final class FormulaShifter {
 	private final int _lastMovedIndex;
 	private final int _amountToMove;
 
+    private final int _srcSheetIndex;
+    private final int _dstSheetIndex;
+
+    private final ShiftMode _mode;
+
+    /**
+     * Create an instance for shifting row.
+     *
+     * For example, this will be called on {@link org.apache.poi.hssf.usermodel.HSSFSheet#shiftRows(int, int, int)} }
+     */
 	private FormulaShifter(int externSheetIndex, int firstMovedIndex, int lastMovedIndex, int amountToMove) {
 		if (amountToMove == 0) {
 			throw new IllegalArgumentException("amountToMove must not be zero");
@@ -44,12 +59,32 @@ public final class FormulaShifter {
 		_firstMovedIndex = firstMovedIndex;
 		_lastMovedIndex = lastMovedIndex;
 		_amountToMove = amountToMove;
+        _mode = ShiftMode.Row;
+
+        _srcSheetIndex = _dstSheetIndex = -1;
 	}
 
+    /**
+     * Create an instance for shifting sheets.
+     *
+     * For example, this will be called on {@link org.apache.poi.hssf.usermodel.HSSFWorkbook#setSheetOrder(String, int)}  
+     */
+    private FormulaShifter(int srcSheetIndex, int dstSheetIndex) {
+        _externSheetIndex = _firstMovedIndex = _lastMovedIndex = _amountToMove = -1;
+
+        _srcSheetIndex = srcSheetIndex;
+        _dstSheetIndex = dstSheetIndex;
+        _mode = ShiftMode.Sheet;
+    }
+
 	public static FormulaShifter createForRowShift(int externSheetIndex, int firstMovedRowIndex, int lastMovedRowIndex, int numberOfRowsToMove) {
 		return new FormulaShifter(externSheetIndex, firstMovedRowIndex, lastMovedRowIndex, numberOfRowsToMove);
 	}
 
+    public static FormulaShifter createForSheetShift(int srcSheetIndex, int dstSheetIndex) {
+        return new FormulaShifter(srcSheetIndex, dstSheetIndex);
+    }
+
 	public String toString() {
 		StringBuffer sb = new StringBuffer();
 
@@ -79,7 +114,14 @@ public final class FormulaShifter {
 	}
 
 	private Ptg adjustPtg(Ptg ptg, int currentExternSheetIx) {
-		return adjustPtgDueToRowMove(ptg, currentExternSheetIx);
+		switch(_mode){
+            case Row:
+                return adjustPtgDueToRowMove(ptg, currentExternSheetIx);
+            case Sheet:
+                return adjustPtgDueToShiftMove(ptg);
+            default:
+                throw new IllegalStateException("Unsupported shift mode: " + _mode);
+        }
 	}
 	/**
 	 * @return <code>true</code> if this Ptg needed to be changed
@@ -121,6 +163,21 @@ public final class FormulaShifter {
 		return null;
 	}
 
+    private Ptg adjustPtgDueToShiftMove(Ptg ptg) {
+        Ptg updatedPtg = null;
+        if(ptg instanceof Ref3DPtg) {
+            Ref3DPtg ref = (Ref3DPtg)ptg;
+            if(ref.getExternSheetIndex() == _srcSheetIndex){
+                ref.setExternSheetIndex(_dstSheetIndex);
+                updatedPtg = ref;
+            } else if (ref.getExternSheetIndex() == _dstSheetIndex){
+                ref.setExternSheetIndex(_srcSheetIndex);
+                updatedPtg = ref;
+            }
+        }
+        return updatedPtg;
+    }
+
 	private Ptg rowMoveRefPtg(RefPtgBase rptg) {
 		int refRow = rptg.getRow();
 		if (_firstMovedIndex <= refRow && refRow <= _lastMovedIndex) {

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

@@ -27,15 +27,12 @@ import org.apache.poi.hssf.HSSFITestDataProvider;
 import org.apache.poi.hssf.model.HSSFFormulaParser;
 import org.apache.poi.hssf.model.InternalWorkbook;
 import org.apache.poi.hssf.model.InternalSheet;
-import org.apache.poi.hssf.record.NameRecord;
+import org.apache.poi.hssf.record.*;
-import org.apache.poi.hssf.record.Record;
-import org.apache.poi.hssf.record.RecordBase;
-import org.apache.poi.hssf.record.RecordFormatException;
-import org.apache.poi.hssf.record.WindowOneRecord;
 import org.apache.poi.ss.formula.ptg.Area3DPtg;
 import org.apache.poi.util.LittleEndian;
 import org.apache.poi.util.TempFile;
 import org.apache.poi.ss.usermodel.BaseTestWorkbook;
+import org.apache.poi.ss.util.CellRangeAddress;
 import org.apache.poi.poifs.filesystem.DirectoryNode;
 import org.apache.poi.poifs.filesystem.NPOIFSFileSystem;
 import org.apache.poi.poifs.filesystem.POIFSFileSystem;
@@ -578,4 +575,60 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
         }
         assertEquals(MAX_STYLES, wb.getNumCellStyles());
     }
+
+    public void testSetSheetOrderHSSF(){
+        HSSFWorkbook wb = new HSSFWorkbook();
+        HSSFSheet s1 = wb.createSheet("first sheet");
+        HSSFSheet s2 = wb.createSheet("other sheet");
+
+        HSSFName name1 = wb.createName();
+        name1.setNameName("name1");
+        name1.setRefersToFormula("'first sheet'!D1");
+
+        HSSFName name2 = wb.createName();
+        name2.setNameName("name2");
+        name2.setRefersToFormula("'other sheet'!C1");
+
+
+        HSSFRow s1r1 = s1.createRow(2);
+        HSSFCell c1 = s1r1.createCell(3);
+        c1.setCellValue(30);
+        HSSFCell c2 = s1r1.createCell(2);
+        c2.setCellFormula("SUM('other sheet'!C1,'first sheet'!C1)");
+
+        HSSFRow s2r1 = s2.createRow(0);
+        HSSFCell c3 = s2r1.createCell(1);
+        c3.setCellFormula("'first sheet'!D3");
+        HSSFCell c4 = s2r1.createCell(2);
+        c4.setCellFormula("'other sheet'!D3");
+
+        // conditional formatting
+        HSSFSheetConditionalFormatting sheetCF = s1.getSheetConditionalFormatting();
+
+        HSSFConditionalFormattingRule rule1 = sheetCF.createConditionalFormattingRule(
+                CFRuleRecord.ComparisonOperator.BETWEEN, "'first sheet'!D1", "'other sheet'!D1");
+
+        HSSFConditionalFormattingRule [] cfRules = { rule1 };
+
+        CellRangeAddress[] regions = {
+            new CellRangeAddress(2, 4, 0, 0), // A3:A5
+        };
+        sheetCF.addConditionalFormatting(regions, cfRules);
+
+        wb.setSheetOrder("other sheet", 0);
+
+        // names
+        assertEquals("'first sheet'!D1", wb.getName("name1").getRefersToFormula());
+        assertEquals("'other sheet'!C1", wb.getName("name2").getRefersToFormula());
+
+        // cells
+        assertEquals("SUM('other sheet'!C1,'first sheet'!C1)", c2.getCellFormula());
+        assertEquals("'first sheet'!D3", c3.getCellFormula());
+        assertEquals("'other sheet'!D3", c4.getCellFormula());
+
+        // conditional formatting
+        HSSFConditionalFormatting cf = sheetCF.getConditionalFormattingAt(0);
+        assertEquals("'first sheet'!D1", cf.getRule(0).getFormula1());
+        assertEquals("'other sheet'!D1", cf.getRule(0).getFormula2());
+    }
 }