From 673e558b7ace2d9725da9353bd2750cd949ab583 Mon Sep 17 00:00:00 2001 From: Javen O'Neal Date: Sat, 4 Nov 2017 10:11:20 +0000 Subject: [PATCH] bug 61474, github #81: add ShiftMode#ColumnCopy for FormulaShifter git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1814268 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/poi/ss/formula/FormulaShifter.java | 284 +++++++++++------- .../poi/ss/formula/TestFormulaShifter.java | 80 ++++- 2 files changed, 252 insertions(+), 112 deletions(-) diff --git a/src/java/org/apache/poi/ss/formula/FormulaShifter.java b/src/java/org/apache/poi/ss/formula/FormulaShifter.java index 0f9625d62..d4d23135b 100644 --- a/src/java/org/apache/poi/ss/formula/FormulaShifter.java +++ b/src/java/org/apache/poi/ss/formula/FormulaShifter.java @@ -40,10 +40,13 @@ import org.apache.poi.ss.formula.ptg.RefPtgBase; */ public final class FormulaShifter { - private static enum ShiftMode { + private enum ShiftMode { RowMove, RowCopy, + /** @since POI 4.0.0 */ ColumnMove, + /** @since POI 4.0.0 */ + ColumnCopy, SheetMove, } @@ -124,6 +127,14 @@ public final class FormulaShifter { SpreadsheetVersion version) { return new FormulaShifter(externSheetIndex, sheetName, firstMovedColumnIndex, lastMovedColumnIndex, numberOfColumnsToMove, ShiftMode.ColumnMove, version); } + + /** + * @since POI 4.0.0 + */ + public static FormulaShifter createForColumnCopy(int externSheetIndex, String sheetName, int firstMovedColumnIndex, int lastMovedColumnIndex, int numberOfColumnsToMove, + SpreadsheetVersion version) { + return new FormulaShifter(externSheetIndex, sheetName, firstMovedColumnIndex, lastMovedColumnIndex, numberOfColumnsToMove, ShiftMode.ColumnCopy, version); + } public static FormulaShifter createForSheetShift(int srcSheetIndex, int dstSheetIndex) { return new FormulaShifter(srcSheetIndex, dstSheetIndex); @@ -163,131 +174,32 @@ public final class FormulaShifter { case RowCopy: // Covered Scenarios: // * row copy on same sheet - // * row copy between different sheetsin the same workbook + // * row copy between different sheets in the same workbook return adjustPtgDueToRowCopy(ptg); case ColumnMove: return adjustPtgDueToColumnMove(ptg, currentExternSheetIx); + case ColumnCopy: + return adjustPtgDueToColumnCopy(ptg); case SheetMove: return adjustPtgDueToSheetMove(ptg); default: throw new IllegalStateException("Unsupported shift mode: " + _mode); } } - /** - * @return in-place modified ptg (if row move would cause Ptg to change), - * deleted ref ptg (if row move causes an error), - * or null (if no Ptg change is needed) - */ - private Ptg adjustPtgDueToRowMove(Ptg ptg, int currentExternSheetIx) { - if(ptg instanceof RefPtg) { - if (currentExternSheetIx != _externSheetIndex) { - // local refs on other sheets are unaffected - return null; - } - RefPtg rptg = (RefPtg)ptg; - return rowMoveRefPtg(rptg); - } - if(ptg instanceof Ref3DPtg) { - Ref3DPtg rptg = (Ref3DPtg)ptg; - if (_externSheetIndex != rptg.getExternSheetIndex()) { - // only move 3D refs that refer to the sheet with cells being moved - // (currentExternSheetIx is irrelevant) - return null; - } - return rowMoveRefPtg(rptg); - } - if(ptg instanceof Ref3DPxg) { - Ref3DPxg rpxg = (Ref3DPxg)ptg; - if (rpxg.getExternalWorkbookNumber() > 0 || - ! _sheetName.equals(rpxg.getSheetName())) { - // only move 3D refs that refer to the sheet with cells being moved - return null; - } - return rowMoveRefPtg(rpxg); - } - if(ptg instanceof Area2DPtgBase) { - if (currentExternSheetIx != _externSheetIndex) { - // local refs on other sheets are unaffected - return ptg; - } - return rowMoveAreaPtg((Area2DPtgBase)ptg); - } - if(ptg instanceof Area3DPtg) { - Area3DPtg aptg = (Area3DPtg)ptg; - if (_externSheetIndex != aptg.getExternSheetIndex()) { - // only move 3D refs that refer to the sheet with cells being moved - // (currentExternSheetIx is irrelevant) - return null; - } - return rowMoveAreaPtg(aptg); - } - if(ptg instanceof Area3DPxg) { - Area3DPxg apxg = (Area3DPxg)ptg; - if (apxg.getExternalWorkbookNumber() > 0 || - ! _sheetName.equals(apxg.getSheetName())) { - // only move 3D refs that refer to the sheet with cells being moved - return null; - } - return rowMoveAreaPtg(apxg); - } - 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; - } /** * @return in-place modified ptg (if column move would cause Ptg to change), * deleted ref ptg (if column move causes an error), * or null (if no Ptg change is needed) */ - private Ptg adjustPtgDueToColumnMove(Ptg ptg, int currentExternSheetIx) { + private Ptg adjustPtgDueToMove(Ptg ptg, int currentExternSheetIx, boolean isRowMove) { if(ptg instanceof RefPtg) { if (currentExternSheetIx != _externSheetIndex) { // local refs on other sheets are unaffected return null; } RefPtg rptg = (RefPtg)ptg; - return columnMoveRefPtg(rptg); + return isRowMove ? rowMoveRefPtg(rptg) : columnMoveRefPtg(rptg); } if(ptg instanceof Ref3DPtg) { Ref3DPtg rptg = (Ref3DPtg)ptg; @@ -296,7 +208,7 @@ public final class FormulaShifter { // (currentExternSheetIx is irrelevant) return null; } - return columnMoveRefPtg(rptg); + return isRowMove ? rowMoveRefPtg(rptg) : columnMoveRefPtg(rptg); } if(ptg instanceof Ref3DPxg) { Ref3DPxg rpxg = (Ref3DPxg)ptg; @@ -305,14 +217,15 @@ public final class FormulaShifter { // only move 3D refs that refer to the sheet with cells being moved return null; } - return columnMoveRefPtg(rpxg); + return isRowMove ? rowMoveRefPtg(rpxg) : columnMoveRefPtg(rpxg); } if(ptg instanceof Area2DPtgBase) { if (currentExternSheetIx != _externSheetIndex) { // local refs on other sheets are unaffected return ptg; } - return columnMoveAreaPtg((Area2DPtgBase)ptg); + Area2DPtgBase aptg = (Area2DPtgBase) ptg; + return isRowMove ? rowMoveAreaPtg(aptg) : columnMoveAreaPtg(aptg); } if(ptg instanceof Area3DPtg) { Area3DPtg aptg = (Area3DPtg)ptg; @@ -321,7 +234,7 @@ public final class FormulaShifter { // (currentExternSheetIx is irrelevant) return null; } - return columnMoveAreaPtg(aptg); + return isRowMove ? rowMoveAreaPtg(aptg) : columnMoveAreaPtg(aptg); } if(ptg instanceof Area3DPxg) { Area3DPxg apxg = (Area3DPxg)ptg; @@ -330,11 +243,98 @@ public final class FormulaShifter { // only move 3D refs that refer to the sheet with cells being moved return null; } - return columnMoveAreaPtg(apxg); + return isRowMove ? rowMoveAreaPtg(apxg) : columnMoveAreaPtg(apxg); } return null; } + /** + * @return in-place modified ptg (if row move would cause Ptg to change), + * deleted ref ptg (if row move causes an error), + * or null (if no Ptg change is needed) + */ + private Ptg adjustPtgDueToRowMove(Ptg ptg, int currentExternSheetIx) { + return adjustPtgDueToMove(ptg, currentExternSheetIx, true); + } + + /** + * @return in-place modified ptg (if column move would cause Ptg to change), + * deleted ref ptg (if column move causes an error), + * or null (if no Ptg change is needed) + */ + private Ptg adjustPtgDueToColumnMove(Ptg ptg, int currentExternSheetIx) { + return adjustPtgDueToMove(ptg, currentExternSheetIx, false); + } + + /** + * Call this on any ptg reference contained in a row or column of cells that was copied. + * If the ptg reference is relative, the references will be shifted by the distance + * that the rows or columns were copied. + * + * @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 or columns 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 adjustPtgDueToCopy(Ptg ptg, boolean isRowCopy) { + if(ptg instanceof RefPtg) { + RefPtg rptg = (RefPtg)ptg; + return isRowCopy ? rowCopyRefPtg(rptg) : columnCopyRefPtg(rptg); + } + if(ptg instanceof Ref3DPtg) { + Ref3DPtg rptg = (Ref3DPtg)ptg; + return isRowCopy ? rowCopyRefPtg(rptg) : columnCopyRefPtg(rptg); + } + if(ptg instanceof Ref3DPxg) { + Ref3DPxg rpxg = (Ref3DPxg)ptg; + return isRowCopy ? rowCopyRefPtg(rpxg) : columnCopyRefPtg(rpxg); + } + if(ptg instanceof Area2DPtgBase) { + Area2DPtgBase aptg = (Area2DPtgBase) ptg; + return isRowCopy ? rowCopyAreaPtg(aptg) : columnCopyAreaPtg(aptg); + } + if(ptg instanceof Area3DPtg) { + Area3DPtg aptg = (Area3DPtg)ptg; + return isRowCopy ? rowCopyAreaPtg(aptg) : columnCopyAreaPtg(aptg); + } + if(ptg instanceof Area3DPxg) { + Area3DPxg apxg = (Area3DPxg)ptg; + return isRowCopy ? rowCopyAreaPtg(apxg) : columnCopyAreaPtg(apxg); + } + 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. + * + * @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) { + return adjustPtgDueToCopy(ptg, true); + } + + /** + * Call this on any ptg reference contained in a column of cells that was copied. + * If the ptg reference is relative, the references will be shifted by the distance + * that the columns were copied. + * + * @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 column 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 adjustPtgDueToColumnCopy(Ptg ptg) { + return adjustPtgDueToCopy(ptg, false); + } + private Ptg adjustPtgDueToSheetMove(Ptg ptg) { if(ptg instanceof Ref3DPtg) { @@ -747,6 +747,70 @@ public final class FormulaShifter { throw new IllegalStateException("Situation not covered: (" + _firstMovedIndex + ", " + _lastMovedIndex + ", " + _amountToMove + ", " + aFirstColumn + ", " + aLastColumn + ")"); } + + /** + * Modifies rptg in-place and return a reference to rptg if the cell reference + * would move due to a column copy operation + * Returns null or {@link RefErrorPtg} if no change was made + * + * @param rptg The REF that is copied + * @return The Ptg reference if the cell would move due to copy, otherwise null + */ + private Ptg columnCopyRefPtg(RefPtgBase rptg) { + final int refColumn = rptg.getColumn(); + if (rptg.isColRelative()) { + // check new location where the ref is located + final int destColumnIndex = _firstMovedIndex + _amountToMove; + if (destColumnIndex < 0 || _version.getLastColumnIndex() < destColumnIndex) { + return createDeletedRef(rptg); + } + + // check new location where the ref points to + final int newColumnIndex = refColumn + _amountToMove; + if(newColumnIndex < 0 || _version.getLastColumnIndex() < newColumnIndex) { + return createDeletedRef(rptg); + } + + rptg.setColumn(newColumnIndex); + return rptg; + } + return null; + } + + /** + * Modifies aptg in-place and return a reference to aptg if the first or last column of + * of the Area reference would move due to a column copy operation + * Returns null or {@link AreaErrPtg} if no change was made + * + * @param aptg The Area that is copied + * @return null, AreaErrPtg, or modified aptg + */ + private Ptg columnCopyAreaPtg(AreaPtgBase aptg) { + boolean changed = false; + + final int aFirstColumn = aptg.getFirstColumn(); + final int aLastColumn = aptg.getLastColumn(); + + if (aptg.isFirstColRelative()) { + final int destFirstColumnIndex = aFirstColumn + _amountToMove; + if (destFirstColumnIndex < 0 || _version.getLastColumnIndex() < destFirstColumnIndex) + return createDeletedRef(aptg); + aptg.setFirstColumn(destFirstColumnIndex); + changed = true; + } + if (aptg.isLastColRelative()) { + final int destLastColumnIndex = aLastColumn + _amountToMove; + if (destLastColumnIndex < 0 || _version.getLastColumnIndex() < destLastColumnIndex) + return createDeletedRef(aptg); + aptg.setLastColumn(destLastColumnIndex); + 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 25ff2da83..f4ceee0bc 100644 --- a/src/testcases/org/apache/poi/ss/formula/TestFormulaShifter.java +++ b/src/testcases/org/apache/poi/ss/formula/TestFormulaShifter.java @@ -33,8 +33,6 @@ import org.junit.Test; /** * Tests for {@link FormulaShifter}. - * - * @author Josh Micich */ public final class TestFormulaShifter { // Note - the expected result row coordinates here were determined/verified @@ -176,6 +174,61 @@ public final class TestFormulaShifter { confirmAreaRowCopy(aptg, 0, 30, 20, 10, 20, false); confirmAreaRowCopy(aptg, 15, 25, -15, 10, 20, false); } + + @Test + public void testCopyAreasSourceColumnsRelRel() { + + // all these operations are on an area ref spanning columns 10 to 20 + final AreaPtg aptg = createAreaPtgColumn(10, 20, true, true); + + confirmAreaColumnCopy(aptg, 0, 30, 20, 30, 40, true); + confirmAreaColumnCopy(aptg, 15, 25, -15, -1, -1, true); //DeletedRef + } + + @Test + public void testCopyAreasSourceColumnsRelAbs() { + + // all these operations are on an area ref spanning columns 10 to 20 + final AreaPtg aptg = createAreaPtgColumn(10, 20, true, false); + + // Only first column should move + confirmAreaColumnCopy(aptg, 0, 30, 20, 20, 30, true); + confirmAreaColumnCopy(aptg, 15, 25, -15, -1, -1, true); //DeletedRef + } + + @Test + public void testCopyAreasSourceColumnsAbsRel() { + // aptg is part of a formula in a cell that was just copied to another column + // aptg column references should be updated by the difference in columns that the cell was copied + // No other references besides the cells that were involved in the copy need to be updated + // this makes the column copy significantly different from the column shift, where all references + // in the workbook need to track the column shift + + // all these operations are on an area ref spanning columns 10 to 20 + final AreaPtg aptg = createAreaPtgColumn(10, 20, false, true); + + // Only last column should move + confirmAreaColumnCopy(aptg, 0, 30, 20, 10, 40, true); + confirmAreaColumnCopy(aptg, 15, 25, -15, 5, 10, true); //sortTopLeftToBottomRight swapped firstColumn and lastColumn because firstColumn is absolute + } + + @Test + public void testCopyAreasSourceColumnsAbsAbs() { + // aptg is part of a formula in a cell that was just copied to another column + // aptg column references should be updated by the difference in columns that the cell was copied + // No other references besides the cells that were involved in the copy need to be updated + // this makes the column copy significantly different from the column shift, where all references + // in the workbook need to track the column shift + + // all these operations are on an area ref spanning columns 10 to 20 + final AreaPtg aptg = createAreaPtgColumn(10, 20, false, false); + + //AbsFirstColumn AbsLastColumn references should't change when copied to a different column + confirmAreaColumnCopy(aptg, 0, 30, 20, 10, 20, false); + confirmAreaColumnCopy(aptg, 15, 25, -15, 10, 20, false); + } + + /** * Tests what happens to an area ref when some outside rows are moved to overlap @@ -284,6 +337,29 @@ public final class TestFormulaShifter { assertEquals("AreaPtg last row", expectedLastRow, copyPtg.getLastRow()); } + + private static void confirmAreaColumnCopy(AreaPtg aptg, + int firstColumnCopied, int lastColumnCopied, int columnOffset, + int expectedFirstColumn, int expectedLastColumn, 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.createForColumnCopy(0, null, firstColumnCopied, lastColumnCopied, columnOffset, SpreadsheetVersion.EXCEL2007); + final boolean actualChanged = fs.adjustFormula(ptgs, 0); + + // DeletedAreaRef + if (expectedFirstColumn < 0 || expectedLastColumn < 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 column copy?", expectedChanged, actualChanged); + assertEquals("AreaPtgs should be modified in-place when a column containing the AreaPtg is copied", copyPtg, ptgs[0]); // expected to change in place (although this is not a strict requirement) + assertEquals("AreaPtg first column", expectedFirstColumn, copyPtg.getFirstColumn()); + assertEquals("AreaPtg last column", expectedLastColumn, copyPtg.getLastColumn()); + + } private static AreaPtg createAreaPtgRow(int initialAreaFirstRow, int initialAreaLastRow) { return createAreaPtgRow(initialAreaFirstRow, initialAreaLastRow, false, false);