]> source.dussan.org Git - poi.git/commitdiff
bug 61474, github #81: add ShiftMode#ColumnCopy for FormulaShifter
authorJaven O'Neal <onealj@apache.org>
Sat, 4 Nov 2017 10:11:20 +0000 (10:11 +0000)
committerJaven O'Neal <onealj@apache.org>
Sat, 4 Nov 2017 10:11:20 +0000 (10:11 +0000)
git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1814268 13f79535-47bb-0310-9956-ffa450edef68

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

index 0f9625d6235fb932d0335ae2107dca544ef4fee1..d4d23135bc86e69e9f3fea04264ed01142a82368 100644 (file)
@@ -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,29 +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),
+     * @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 adjustPtgDueToRowMove(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 rowMoveRefPtg(rptg);
+            return isRowMove ? rowMoveRefPtg(rptg) : columnMoveRefPtg(rptg);
         }
         if(ptg instanceof Ref3DPtg) {
             Ref3DPtg rptg = (Ref3DPtg)ptg;
@@ -194,23 +208,24 @@ public final class FormulaShifter {
                 // (currentExternSheetIx is irrelevant)
                 return null;
             }
-            return rowMoveRefPtg(rptg);
+            return isRowMove ? rowMoveRefPtg(rptg) : columnMoveRefPtg(rptg);
         }
         if(ptg instanceof Ref3DPxg) {
             Ref3DPxg rpxg = (Ref3DPxg)ptg;
             if (rpxg.getExternalWorkbookNumber() > 0 ||
-                   ! _sheetName.equals(rpxg.getSheetName())) {
+                    ! _sheetName.equals(rpxg.getSheetName())) {
                 // only move 3D refs that refer to the sheet with cells being moved
                 return null;
             }
-            return rowMoveRefPtg(rpxg);
+            return isRowMove ? rowMoveRefPtg(rpxg) : columnMoveRefPtg(rpxg);
         }
         if(ptg instanceof Area2DPtgBase) {
             if (currentExternSheetIx != _externSheetIndex) {
                 // local refs on other sheets are unaffected
                 return ptg;
             }
-            return rowMoveAreaPtg((Area2DPtgBase)ptg);
+            Area2DPtgBase aptg = (Area2DPtgBase) ptg;
+            return isRowMove ? rowMoveAreaPtg(aptg) : columnMoveAreaPtg(aptg);
         }
         if(ptg instanceof Area3DPtg) {
             Area3DPtg aptg = (Area3DPtg)ptg;
@@ -219,7 +234,7 @@ public final class FormulaShifter {
                 // (currentExternSheetIx is irrelevant)
                 return null;
             }
-            return rowMoveAreaPtg(aptg);
+            return isRowMove ? rowMoveAreaPtg(aptg) : columnMoveAreaPtg(aptg);
         }
         if(ptg instanceof Area3DPxg) {
             Area3DPxg apxg = (Area3DPxg)ptg;
@@ -228,111 +243,96 @@ public final class FormulaShifter {
                 // only move 3D refs that refer to the sheet with cells being moved
                 return null;
             }
-            return rowMoveAreaPtg(apxg);
+            return isRowMove ? rowMoveAreaPtg(apxg) : columnMoveAreaPtg(apxg);
         }
         return null;
     }
-    
+
     /**
-     * Call this on any ptg reference contained in a row of cells that was copied.
+     * @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 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).
-     * 
+     * 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 of a sheet, return deleted ref
+     * 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 <code>null</code>
      */
-    private Ptg adjustPtgDueToRowCopy(Ptg ptg) {
+    private Ptg adjustPtgDueToCopy(Ptg ptg, boolean isRowCopy) {
         if(ptg instanceof RefPtg) {
             RefPtg rptg = (RefPtg)ptg;
-            return rowCopyRefPtg(rptg);
+            return isRowCopy ? rowCopyRefPtg(rptg) : columnCopyRefPtg(rptg);
         }
         if(ptg instanceof Ref3DPtg) {
             Ref3DPtg rptg = (Ref3DPtg)ptg;
-            return rowCopyRefPtg(rptg);
+            return isRowCopy ? rowCopyRefPtg(rptg) : columnCopyRefPtg(rptg);
         }
         if(ptg instanceof Ref3DPxg) {
             Ref3DPxg rpxg = (Ref3DPxg)ptg;
-            return rowCopyRefPtg(rpxg);
+            return isRowCopy ? rowCopyRefPtg(rpxg) : columnCopyRefPtg(rpxg);
         }
         if(ptg instanceof Area2DPtgBase) {
-            return rowCopyAreaPtg((Area2DPtgBase)ptg);
+            Area2DPtgBase aptg = (Area2DPtgBase) ptg;
+            return isRowCopy ? rowCopyAreaPtg(aptg) : columnCopyAreaPtg(aptg);
         }
         if(ptg instanceof Area3DPtg) {
             Area3DPtg aptg = (Area3DPtg)ptg;
-            return rowCopyAreaPtg(aptg);
+            return isRowCopy ? rowCopyAreaPtg(aptg) : columnCopyAreaPtg(aptg);
         }
         if(ptg instanceof Area3DPxg) {
             Area3DPxg apxg = (Area3DPxg)ptg;
-            return rowCopyAreaPtg(apxg);
+            return isRowCopy ? rowCopyAreaPtg(apxg) : columnCopyAreaPtg(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)
+     * 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 <code>null</code>
      */
-    private Ptg adjustPtgDueToColumnMove(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 columnMoveRefPtg(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 columnMoveRefPtg(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 columnMoveRefPtg(rpxg);
-        }
-        if(ptg instanceof Area2DPtgBase) {
-            if (currentExternSheetIx != _externSheetIndex) {
-                // local refs on other sheets are unaffected
-                return ptg;
-            }
-            return columnMoveAreaPtg((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 columnMoveAreaPtg(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 columnMoveAreaPtg(apxg);
-        }
-        return 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 <code>null</code>
+     */
+    private Ptg adjustPtgDueToColumnCopy(Ptg ptg) {
+        return adjustPtgDueToCopy(ptg, false);
     }
 
 
@@ -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 <code>null</code> 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 <code>null</code> 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) {
index 25ff2da8383232b247bc9c677728d1b159582c94..f4ceee0bcce95f838fa28e922e6275a4ca390bd9 100644 (file)
@@ -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);