Bug 58746: Fix missing adjustment of formulas when sheet-ordering is changed.

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

4 files changed, 112 insertions, 9 deletions

diff --git a/src/java/org/apache/poi/ss/formula/FormulaShifter.java b/src/java/org/apache/poi/ss/formula/FormulaShifter.java
index a74cc51e49..1e79b1dfc9 100644
--- a/src/java/org/apache/poi/ss/formula/FormulaShifter.java
+++ b/src/java/org/apache/poi/ss/formula/FormulaShifter.java
@@ -283,18 +283,42 @@ public final class FormulaShifter {
 
 
     private Ptg adjustPtgDueToSheetMove(Ptg ptg) {
-        Ptg updatedPtg = null;
         if(ptg instanceof Ref3DPtg) {
             Ref3DPtg ref = (Ref3DPtg)ptg;
-            if(ref.getExternSheetIndex() == _srcSheetIndex){
+            int oldSheetIndex = ref.getExternSheetIndex();
+            
+            // we have to handle a few cases here
+            
+            // 1. sheet is outside moved sheets, no change necessary
+            if(oldSheetIndex < _srcSheetIndex &&
+                    oldSheetIndex < _dstSheetIndex) {
+                return null;
+            }
+            if(oldSheetIndex > _srcSheetIndex &&
+                    oldSheetIndex > _dstSheetIndex) {
+                return null;
+            }
+            
+            // 2. ptg refers to the moved sheet
+            if(oldSheetIndex == _srcSheetIndex) {
                 ref.setExternSheetIndex(_dstSheetIndex);
-                updatedPtg = ref;
-            } else if (ref.getExternSheetIndex() == _dstSheetIndex){
-                ref.setExternSheetIndex(_srcSheetIndex);
-                updatedPtg = ref;
+                return ref;
+            }
+
+            // 3. new index is lower than old one => sheets get moved up
+            if (_dstSheetIndex < _srcSheetIndex) {
+                ref.setExternSheetIndex(oldSheetIndex+1);
+                return ref;
+            }
+
+            // 4. new index is higher than old one => sheets get moved down
+            if (_dstSheetIndex > _srcSheetIndex) {
+                ref.setExternSheetIndex(oldSheetIndex-1);
+                return ref;
             }
         }
-        return updatedPtg;
+
+        return null;
     }
 
     private Ptg rowMoveRefPtg(RefPtgBase rptg) {
diff --git a/src/ooxml/testcases/org/apache/poi/xssf/eventusermodel/TestXSSFReader.java b/src/ooxml/testcases/org/apache/poi/xssf/eventusermodel/TestXSSFReader.java
index 0eebf89ffb..c67447377c 100644
--- a/src/ooxml/testcases/org/apache/poi/xssf/eventusermodel/TestXSSFReader.java
+++ b/src/ooxml/testcases/org/apache/poi/xssf/eventusermodel/TestXSSFReader.java
@@ -205,8 +205,10 @@ public final class TestXSSFReader extends TestCase {
    public void test58747() throws Exception {
        OPCPackage pkg =  XSSFTestDataSamples.openSamplePackage("58747.xlsx");
        ReadOnlySharedStringsTable strings = new ReadOnlySharedStringsTable(pkg);
+       assertNotNull(strings);
        XSSFReader reader = new XSSFReader(pkg);
        StylesTable styles = reader.getStylesTable();
+       assertNotNull(styles);
        
        XSSFReader.SheetIterator iter = (XSSFReader.SheetIterator) reader.getSheetsData();
        assertEquals(true, iter.hasNext());
diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFSheet.java b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFSheet.java
index 8d64bf14f9..71436195a8 100644
--- a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFSheet.java
+++ b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFSheet.java
@@ -1193,4 +1193,33 @@ public final class TestHSSFSheet extends BaseTestSheet {
         assertNotNull(record);
         wb.close();
     }
+    
+    @Test
+    public void test58746() throws IOException {
+        HSSFWorkbook wb = new HSSFWorkbook();
+        
+        HSSFSheet first = wb.createSheet("first");
+        first.createRow(0).createCell(0).setCellValue(1);
+        
+        HSSFSheet second = wb.createSheet("second");
+        second.createRow(0).createCell(0).setCellValue(2);
+        
+        HSSFSheet third = wb.createSheet("third");
+        HSSFRow row = third.createRow(0);
+        row.createCell(0).setCellFormula("first!A1");
+        row.createCell(1).setCellFormula("second!A1");
+
+        // re-order for sheet "third"
+        wb.setSheetOrder("third", 0);
+        
+        // verify results
+        assertEquals("third", wb.getSheetAt(0).getSheetName());
+        assertEquals("first", wb.getSheetAt(1).getSheetName());
+        assertEquals("second", wb.getSheetAt(2).getSheetName());
+        
+        assertEquals("first!A1", wb.getSheetAt(0).getRow(0).getCell(0).getCellFormula());
+        assertEquals("second!A1", wb.getSheetAt(0).getRow(0).getCell(1).getCellFormula());
+        
+        wb.close();
+    }
 }
diff --git a/src/testcases/org/apache/poi/ss/formula/TestFormulaShifter.java b/src/testcases/org/apache/poi/ss/formula/TestFormulaShifter.java
index c7e017ae09..501df6a347 100644
--- a/src/testcases/org/apache/poi/ss/formula/TestFormulaShifter.java
+++ b/src/testcases/org/apache/poi/ss/formula/TestFormulaShifter.java
@@ -17,12 +17,14 @@
 
 package org.apache.poi.ss.formula;
 
-import junit.framework.TestCase;
-
 import org.apache.poi.ss.SpreadsheetVersion;
 import org.apache.poi.ss.formula.ptg.AreaErrPtg;
 import org.apache.poi.ss.formula.ptg.AreaPtg;
 import org.apache.poi.ss.formula.ptg.Ptg;
+import org.apache.poi.ss.formula.ptg.Ref3DPtg;
+import org.apache.poi.ss.util.CellReference;
+
+import junit.framework.TestCase;
 
 /**
  * Tests for {@link FormulaShifter}.
@@ -196,4 +198,50 @@ public final class TestFormulaShifter extends TestCase {
 	private static AreaPtg createAreaPtg(int initialAreaFirstRow, int initialAreaLastRow, boolean firstRowRelative, boolean lastRowRelative) {
 		return new AreaPtg(initialAreaFirstRow, initialAreaLastRow, 2, 5, firstRowRelative, lastRowRelative, false, false);
 	}
+
+	public void testShiftSheet() {
+	    // 4 sheets, move a sheet from pos 2 to pos 0, i.e. current 0 becomes 1, current 1 becomes pos 2 
+        FormulaShifter shifter = FormulaShifter.createForSheetShift(2, 0);
+        
+        Ptg[] ptgs = new Ptg[] {
+          new Ref3DPtg(new CellReference("first", 0, 0, true, true), 0),
+          new Ref3DPtg(new CellReference("second", 0, 0, true, true), 1),
+          new Ref3DPtg(new CellReference("third", 0, 0, true, true), 2),
+          new Ref3DPtg(new CellReference("fourth", 0, 0, true, true), 3),
+        };
+        
+        shifter.adjustFormula(ptgs, -1);
+        
+        assertEquals("formula previously pointing to sheet 0 should now point to sheet 1", 
+                1, ((Ref3DPtg)ptgs[0]).getExternSheetIndex());
+        assertEquals("formula previously pointing to sheet 1 should now point to sheet 2", 
+                2, ((Ref3DPtg)ptgs[1]).getExternSheetIndex());
+        assertEquals("formula previously pointing to sheet 2 should now point to sheet 0", 
+                0, ((Ref3DPtg)ptgs[2]).getExternSheetIndex());
+        assertEquals("formula previously pointing to sheet 3 should be unchanged", 
+                3, ((Ref3DPtg)ptgs[3]).getExternSheetIndex());
+	}
+
+    public void testShiftSheet2() {
+        // 4 sheets, move a sheet from pos 1 to pos 2, i.e. current 2 becomes 1, current 1 becomes pos 2 
+        FormulaShifter shifter = FormulaShifter.createForSheetShift(1, 2);
+        
+        Ptg[] ptgs = new Ptg[] {
+          new Ref3DPtg(new CellReference("first", 0, 0, true, true), 0),
+          new Ref3DPtg(new CellReference("second", 0, 0, true, true), 1),
+          new Ref3DPtg(new CellReference("third", 0, 0, true, true), 2),
+          new Ref3DPtg(new CellReference("fourth", 0, 0, true, true), 3),
+        };
+
+        shifter.adjustFormula(ptgs, -1);
+        
+        assertEquals("formula previously pointing to sheet 0 should be unchanged", 
+                0, ((Ref3DPtg)ptgs[0]).getExternSheetIndex());
+        assertEquals("formula previously pointing to sheet 1 should now point to sheet 2", 
+                2, ((Ref3DPtg)ptgs[1]).getExternSheetIndex());
+        assertEquals("formula previously pointing to sheet 2 should now point to sheet 1", 
+                1, ((Ref3DPtg)ptgs[2]).getExternSheetIndex());
+        assertEquals("formula previously pointing to sheet 3 should be unchanged", 
+                3, ((Ref3DPtg)ptgs[3]).getExternSheetIndex());
+    }
 }