From 4af62d8bbe300bb2a7662073013b603112ac9baf Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Wed, 30 Dec 2015 20:31:44 +0000 Subject: [PATCH] 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 --- .../apache/poi/ss/formula/FormulaShifter.java | 38 +++++++++++--- .../xssf/eventusermodel/TestXSSFReader.java | 2 + .../poi/hssf/usermodel/TestHSSFSheet.java | 29 +++++++++++ .../poi/ss/formula/TestFormulaShifter.java | 52 ++++++++++++++++++- 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()); + } }