From af4a96f467360e0a0fc28378ee7494a2a764e648 Mon Sep 17 00:00:00 2001 From: Javen O'Neal Date: Mon, 2 Nov 2015 00:25:08 +0000 Subject: [PATCH] bug 58384: add FormulaShifter.createForRowCopy git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1711866 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/poi/ss/formula/FormulaShifter.java | 136 +++++++++++++++++- .../poi/ss/formula/TestFormulaShifter.java | 84 ++++++++++- 2 files changed, 215 insertions(+), 5 deletions(-) diff --git a/src/java/org/apache/poi/ss/formula/FormulaShifter.java b/src/java/org/apache/poi/ss/formula/FormulaShifter.java index ec194b55a..a1708776c 100644 --- a/src/java/org/apache/poi/ss/formula/FormulaShifter.java +++ b/src/java/org/apache/poi/ss/formula/FormulaShifter.java @@ -17,6 +17,7 @@ package org.apache.poi.ss.formula; +import org.apache.poi.ss.SpreadsheetVersion; import org.apache.poi.ss.formula.ptg.Area2DPtgBase; import org.apache.poi.ss.formula.ptg.Area3DPtg; import org.apache.poi.ss.formula.ptg.Area3DPxg; @@ -41,6 +42,7 @@ public final class FormulaShifter { private static enum ShiftMode { RowMove, + RowCopy, SheetMove, } @@ -61,6 +63,7 @@ public final class FormulaShifter { private final int _srcSheetIndex; private final int _dstSheetIndex; + private final SpreadsheetVersion _version; private final ShiftMode _mode; @@ -69,7 +72,8 @@ public final class FormulaShifter { * * For example, this will be called on {@link org.apache.poi.hssf.usermodel.HSSFSheet#shiftRows(int, int, int)} } */ - private FormulaShifter(int externSheetIndex, String sheetName, int firstMovedIndex, int lastMovedIndex, int amountToMove) { + private FormulaShifter(int externSheetIndex, String sheetName, int firstMovedIndex, int lastMovedIndex, int amountToMove, + ShiftMode mode, SpreadsheetVersion version) { if (amountToMove == 0) { throw new IllegalArgumentException("amountToMove must not be zero"); } @@ -81,7 +85,8 @@ public final class FormulaShifter { _firstMovedIndex = firstMovedIndex; _lastMovedIndex = lastMovedIndex; _amountToMove = amountToMove; - _mode = ShiftMode.RowMove; + _mode = mode; + _version = version; _srcSheetIndex = _dstSheetIndex = -1; } @@ -94,14 +99,36 @@ public final class FormulaShifter { private FormulaShifter(int srcSheetIndex, int dstSheetIndex) { _externSheetIndex = _firstMovedIndex = _lastMovedIndex = _amountToMove = -1; _sheetName = null; + _version = null; _srcSheetIndex = srcSheetIndex; _dstSheetIndex = dstSheetIndex; _mode = ShiftMode.SheetMove; } + /** + * @deprecated As of 3.14 beta 1 (November 2015), replaced by {@link #createForRowShift(int, String, int, int, int, SpreadsheetVersion)} + * + * @param externSheetIndex + * @param sheetName + * @param firstMovedRowIndex + * @param lastMovedRowIndex + * @param numberOfRowsToMove + * @return FormulaShifter object that can be passed to a RowShifter to modify formulas. + */ + @Deprecated public static FormulaShifter createForRowShift(int externSheetIndex, String sheetName, int firstMovedRowIndex, int lastMovedRowIndex, int numberOfRowsToMove) { - return new FormulaShifter(externSheetIndex, sheetName, firstMovedRowIndex, lastMovedRowIndex, numberOfRowsToMove); + return createForRowShift(externSheetIndex, sheetName, firstMovedRowIndex, lastMovedRowIndex, numberOfRowsToMove, SpreadsheetVersion.EXCEL97); + } + + public static FormulaShifter createForRowShift(int externSheetIndex, String sheetName, int firstMovedRowIndex, int lastMovedRowIndex, int numberOfRowsToMove, + SpreadsheetVersion version) { + return new FormulaShifter(externSheetIndex, sheetName, firstMovedRowIndex, lastMovedRowIndex, numberOfRowsToMove, ShiftMode.RowMove, version); + } + + public static FormulaShifter createForRowCopy(int externSheetIndex, String sheetName, int firstMovedRowIndex, int lastMovedRowIndex, int numberOfRowsToMove, + SpreadsheetVersion version) { + return new FormulaShifter(externSheetIndex, sheetName, firstMovedRowIndex, lastMovedRowIndex, numberOfRowsToMove, ShiftMode.RowCopy, version); } public static FormulaShifter createForSheetShift(int srcSheetIndex, int dstSheetIndex) { @@ -141,6 +168,11 @@ public final class FormulaShifter { switch(_mode){ case RowMove: return adjustPtgDueToRowMove(ptg, currentExternSheetIx); + case RowCopy: + // Covered Scenarios: + // * row copy on same sheet + // * row copy between different sheetsin the same workbook + return adjustPtgDueToRowCopy(ptg); case SheetMove: return adjustPtgDueToSheetMove(ptg); default: @@ -206,6 +238,49 @@ public final class FormulaShifter { } return null; } + + /** + * Call this on any ptg reference contained in a row of cells that was copied. + * If the ptg reference is relative, the references will be shifted by the distance + * that the rows were copied. + * In the future similar functions could be written due to column copying or + * individual cell copying. Just make sure to only call adjustPtgDueToRowCopy on + * formula cells that are copied (unless row shifting, where references outside + * of the shifted region need to be updated to reflect the shift, a copy is self-contained). + * + * @param ptg the ptg to shift + * @return deleted ref ptg, in-place modified ptg, or null + * If Ptg would be shifted off the first or last row of a sheet, return deleted ref + * If Ptg needs to be changed, modifies Ptg in-place + * If Ptg doesn't need to be changed, returns null + */ + private Ptg adjustPtgDueToRowCopy(Ptg ptg) { + if(ptg instanceof RefPtg) { + RefPtg rptg = (RefPtg)ptg; + return rowCopyRefPtg(rptg); + } + if(ptg instanceof Ref3DPtg) { + Ref3DPtg rptg = (Ref3DPtg)ptg; + return rowCopyRefPtg(rptg); + } + if(ptg instanceof Ref3DPxg) { + Ref3DPxg rpxg = (Ref3DPxg)ptg; + return rowCopyRefPtg(rpxg); + } + if(ptg instanceof Area2DPtgBase) { + return rowCopyAreaPtg((Area2DPtgBase)ptg); + } + if(ptg instanceof Area3DPtg) { + Area3DPtg aptg = (Area3DPtg)ptg; + return rowCopyAreaPtg(aptg); + } + if(ptg instanceof Area3DPxg) { + Area3DPxg apxg = (Area3DPxg)ptg; + return rowCopyAreaPtg(apxg); + } + return null; + } + private Ptg adjustPtgDueToSheetMove(Ptg ptg) { Ptg updatedPtg = null; @@ -375,6 +450,61 @@ public final class FormulaShifter { throw new IllegalStateException("Situation not covered: (" + _firstMovedIndex + ", " + _lastMovedIndex + ", " + _amountToMove + ", " + aFirstRow + ", " + aLastRow + ")"); } + + /** + * Modifies rptg in-place and return a reference to rptg if the cell reference + * would move due to a row copy operation + * Returns null or {@link #RefErrorPtg} if no change was made + * + * @param aptg + * @return + */ + private Ptg rowCopyRefPtg(RefPtgBase rptg) { + final int refRow = rptg.getRow(); + if (rptg.isRowRelative()) { + final int destRowIndex = _firstMovedIndex + _amountToMove; + if (destRowIndex < 0 || _version.getLastRowIndex() < destRowIndex) + return createDeletedRef(rptg); + rptg.setRow(refRow + _amountToMove); + return rptg; + } + return null; + } + + /** + * Modifies aptg in-place and return a reference to aptg if the first or last row of + * of the Area reference would move due to a row copy operation + * Returns null or {@link #AreaErrPtg} if no change was made + * + * @param aptg + * @return null, AreaErrPtg, or modified aptg + */ + private Ptg rowCopyAreaPtg(AreaPtgBase aptg) { + boolean changed = false; + + final int aFirstRow = aptg.getFirstRow(); + final int aLastRow = aptg.getLastRow(); + + if (aptg.isFirstRowRelative()) { + final int destFirstRowIndex = aFirstRow + _amountToMove; + if (destFirstRowIndex < 0 || _version.getLastRowIndex() < destFirstRowIndex) + return createDeletedRef(aptg); + aptg.setFirstRow(destFirstRowIndex); + changed = true; + } + if (aptg.isLastRowRelative()) { + final int destLastRowIndex = aLastRow + _amountToMove; + if (destLastRowIndex < 0 || _version.getLastRowIndex() < destLastRowIndex) + return createDeletedRef(aptg); + aptg.setLastRow(destLastRowIndex); + changed = true; + } + if (changed) { + aptg.sortTopLeftToBottomRight(); + } + + return changed ? aptg : null; + } private static Ptg createDeletedRef(Ptg ptg) { if (ptg instanceof RefPtg) { diff --git a/src/testcases/org/apache/poi/ss/formula/TestFormulaShifter.java b/src/testcases/org/apache/poi/ss/formula/TestFormulaShifter.java index d1848a100..c7e017ae0 100644 --- a/src/testcases/org/apache/poi/ss/formula/TestFormulaShifter.java +++ b/src/testcases/org/apache/poi/ss/formula/TestFormulaShifter.java @@ -19,6 +19,7 @@ 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; @@ -74,6 +75,56 @@ public final class TestFormulaShifter extends TestCase { confirmAreaShift(aptg, 18, 22, 5, 10, 25); // simple expansion at bottom } + + public void testCopyAreasSourceRowsRelRel() { + + // all these operations are on an area ref spanning rows 10 to 20 + final AreaPtg aptg = createAreaPtg(10, 20, true, true); + + confirmAreaCopy(aptg, 0, 30, 20, 30, 40, true); + confirmAreaCopy(aptg, 15, 25, -15, -1, -1, true); //DeletedRef + } + + public void testCopyAreasSourceRowsRelAbs() { + + // all these operations are on an area ref spanning rows 10 to 20 + final AreaPtg aptg = createAreaPtg(10, 20, true, false); + + // Only first row should move + confirmAreaCopy(aptg, 0, 30, 20, 20, 30, true); + confirmAreaCopy(aptg, 15, 25, -15, -1, -1, true); //DeletedRef + } + + public void testCopyAreasSourceRowsAbsRel() { + // aptg is part of a formula in a cell that was just copied to another row + // aptg row references should be updated by the difference in rows that the cell was copied + // No other references besides the cells that were involved in the copy need to be updated + // this makes the row copy significantly different from the row shift, where all references + // in the workbook need to track the row shift + + // all these operations are on an area ref spanning rows 10 to 20 + final AreaPtg aptg = createAreaPtg(10, 20, false, true); + + // Only last row should move + confirmAreaCopy(aptg, 0, 30, 20, 10, 40, true); + confirmAreaCopy(aptg, 15, 25, -15, 5, 10, true); //sortTopLeftToBottomRight swapped firstRow and lastRow because firstRow is absolute + } + + public void testCopyAreasSourceRowsAbsAbs() { + // aptg is part of a formula in a cell that was just copied to another row + // aptg row references should be updated by the difference in rows that the cell was copied + // No other references besides the cells that were involved in the copy need to be updated + // this makes the row copy significantly different from the row shift, where all references + // in the workbook need to track the row shift + + // all these operations are on an area ref spanning rows 10 to 20 + final AreaPtg aptg = createAreaPtg(10, 20, false, false); + + //AbsFirstRow AbsLastRow references should't change when copied to a different row + confirmAreaCopy(aptg, 0, 30, 20, 10, 20, false); + confirmAreaCopy(aptg, 15, 25, -15, 10, 20, false); + } + /** * Tests what happens to an area ref when some outside rows are moved to overlap * that area ref @@ -97,7 +148,7 @@ public final class TestFormulaShifter extends TestCase { int firstRowMoved, int lastRowMoved, int numberRowsMoved, int expectedAreaFirstRow, int expectedAreaLastRow) { - FormulaShifter fs = FormulaShifter.createForRowShift(0, "", firstRowMoved, lastRowMoved, numberRowsMoved); + FormulaShifter fs = FormulaShifter.createForRowShift(0, "", firstRowMoved, lastRowMoved, numberRowsMoved, SpreadsheetVersion.EXCEL2007); boolean expectedChanged = aptg.getFirstRow() != expectedAreaFirstRow || aptg.getLastRow() != expectedAreaLastRow; AreaPtg copyPtg = (AreaPtg) aptg.copy(); // clone so we can re-use aptg in calling method @@ -113,7 +164,36 @@ public final class TestFormulaShifter extends TestCase { assertEquals(expectedAreaLastRow, copyPtg.getLastRow()); } + + + private static void confirmAreaCopy(AreaPtg aptg, + int firstRowCopied, int lastRowCopied, int rowOffset, + int expectedFirstRow, int expectedLastRow, boolean expectedChanged) { + + final AreaPtg copyPtg = (AreaPtg) aptg.copy(); // clone so we can re-use aptg in calling method + final Ptg[] ptgs = { copyPtg, }; + final FormulaShifter fs = FormulaShifter.createForRowCopy(0, null, firstRowCopied, lastRowCopied, rowOffset, SpreadsheetVersion.EXCEL2007); + final boolean actualChanged = fs.adjustFormula(ptgs, 0); + + // DeletedAreaRef + if (expectedFirstRow < 0 || expectedLastRow < 0) { + assertEquals("Reference should have shifted off worksheet, producing #REF! error: " + ptgs[0], + AreaErrPtg.class, ptgs[0].getClass()); + return; + } + + assertEquals("Should this AreaPtg change due to row copy?", expectedChanged, actualChanged); + assertEquals("AreaPtgs should be modified in-place when a row containing the AreaPtg is copied", copyPtg, ptgs[0]); // expected to change in place (although this is not a strict requirement) + assertEquals("AreaPtg first row", expectedFirstRow, copyPtg.getFirstRow()); + assertEquals("AreaPtg last row", expectedLastRow, copyPtg.getLastRow()); + + } + private static AreaPtg createAreaPtg(int initialAreaFirstRow, int initialAreaLastRow) { - return new AreaPtg(initialAreaFirstRow, initialAreaLastRow, 2, 5, false, false, false, false); + return createAreaPtg(initialAreaFirstRow, initialAreaLastRow, false, false); + } + + private static AreaPtg createAreaPtg(int initialAreaFirstRow, int initialAreaLastRow, boolean firstRowRelative, boolean lastRowRelative) { + return new AreaPtg(initialAreaFirstRow, initialAreaLastRow, 2, 5, firstRowRelative, lastRowRelative, false, false); } }