Mailing-List: contact commits-help@poi.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@poi.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1711866 - in /poi/trunk/src:
 java/org/apache/poi/ss/formula/FormulaShifter.java
 testcases/org/apache/poi/ss/formula/TestFormulaShifter.java
Date: Mon, 02 Nov 2015 00:25:09 -0000
To: commits@poi.apache.org
From: onealj@apache.org
Message-Id: <20151102002509.3F81B3A019A@svn01-us-west.apache.org>

Author: onealj
Date: Mon Nov  2 00:25:08 2015
New Revision: 1711866

URL: http://svn.apache.org/viewvc?rev=1711866&view=rev
Log:
bug 58384: add FormulaShifter.createForRowCopy

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

Modified: poi/trunk/src/java/org/apache/poi/ss/formula/FormulaShifter.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/FormulaShifter.java?rev=1711866&r1=1711865&r2=1711866&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/formula/FormulaShifter.java (original)
+++ poi/trunk/src/java/org/apache/poi/ss/formula/FormulaShifter.java Mon Nov  2 00:25:08 2015
@@ -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 <code>null</code>
+     */
+    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 <code>null</code> 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 <code>null</code> 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) {

Modified: poi/trunk/src/testcases/org/apache/poi/ss/formula/TestFormulaShifter.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/ss/formula/TestFormulaShifter.java?rev=1711866&r1=1711865&r2=1711866&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/ss/formula/TestFormulaShifter.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/ss/formula/TestFormulaShifter.java Mon Nov  2 00:25:08 2015
@@ -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 ex
 
 		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 ex
 			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 ex
 		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);
 	}
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org