more improvements in shiftRows: 1. shift named ranges and merged regions, JUnit added. 2. fixed shiftRows in XSSF to preserve row heights, see bugzilla 46719.

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@747894 13f79535-47bb-0310-9956-ffa450edef68

src/documentation/content/xdocs/index.xml

@@ -38,7 +38,7 @@
       <p>Development for this is in a svn branch, but we are please to
        announce our first preview release containing this support.
        Users interested in the OOXML support should download the
-       <link href="http://www.apache.org/dyn/closer.cgi/poi/dev/">POI 3.5 beta 4</link>
+       <link href="http://www.apache.org/dyn/closer.cgi/poi/dev/">POI 3.5 beta 5</link>
        the source and binaries from their
        <link href="http://www.apache.org/dyn/closer.cgi/poi/dev/">local mirror</link>.
        People interested should also follow the

src/java/org/apache/poi/hssf/model/Workbook.java

@@ -79,6 +79,8 @@ import org.apache.poi.hssf.record.WindowProtectRecord;
 import org.apache.poi.hssf.record.WriteAccessRecord;
 import org.apache.poi.hssf.record.WriteProtectRecord;
 import org.apache.poi.hssf.record.formula.NameXPtg;
+import org.apache.poi.hssf.record.formula.FormulaShifter;
+import org.apache.poi.hssf.record.formula.Ptg;
 import org.apache.poi.hssf.util.HSSFColor;
 import org.apache.poi.ss.formula.EvaluationWorkbook.ExternalSheet;
 import org.apache.poi.util.POILogFactory;
@@ -2310,4 +2312,18 @@ public final class Workbook implements Model {
 
         }
     }
+
+    /**
+     * Updates named ranges due to moving of cells
+     */
+    public void updateNamesAfterCellShift(FormulaShifter shifter) {
+        for (int i = 0 ; i < getNumNames() ; ++i){
+            NameRecord nr = getNameRecord(i);
+            Ptg[] ptgs = nr.getNameDefinition();
+            if (shifter.adjustFormula(ptgs, nr.getExternSheetNumber())) {
+                nr.setNameDefinition(ptgs);
+            }
+        }
+    }
+
 }

src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java

@@ -1285,7 +1285,7 @@ public final class HSSFSheet implements org.apache.poi.ss.usermodel.Sheet {
             short otherExtSheetIx = _book.checkExternSheet(i);
             otherSheet.updateFormulasAfterCellShift(shifter, otherExtSheetIx);
         }
-        // TODO - adjust formulas in named ranges
+        _workbook.getWorkbook().updateNamesAfterCellShift(shifter);
     }
 
     protected void insertChartRecords(List<Record> records) {

src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java

@@ -37,6 +37,7 @@ import org.apache.poi.ss.formula.FormulaRenderer;
 import org.apache.poi.xssf.model.CommentsTable;
 import org.apache.poi.xssf.model.CalculationChain;
 import org.apache.poi.xssf.usermodel.helpers.ColumnHelper;
+import org.apache.poi.xssf.usermodel.helpers.XSSFRowShifter;
 import org.apache.poi.POIXMLDocumentPart;
 import org.apache.poi.POIXMLException;
 import org.apache.poi.util.POILogger;
@@ -74,7 +75,7 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
 
     protected CTSheet sheet;
     protected CTWorksheet worksheet;
-    private TreeMap<Integer, Row> rows;
+    private TreeMap<Integer, XSSFRow> rows;
     private List<XSSFHyperlink> hyperlinks;
     private ColumnHelper columnHelper;
     private CommentsTable sheetComments;
@@ -151,7 +152,7 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
     }
 
     private void initRows(CTWorksheet worksheet) {
-        rows = new TreeMap<Integer, Row>();
+        rows = new TreeMap<Integer, XSSFRow>();
         sharedFormulas = new HashMap<Integer, XSSFCell>();
         for (CTRow row : worksheet.getSheetData().getRowArray()) {
             XSSFRow r = new XSSFRow(row, this);
@@ -831,7 +832,7 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
      * @return <code>XSSFRow</code> representing the rownumber or <code>null</code> if its not defined on the sheet
      */
     public XSSFRow getRow(int rownum) {
-        return (XSSFRow)rows.get(rownum);
+        return rows.get(rownum);
     }
 
     /**
@@ -1012,8 +1013,7 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
 
     private short getMaxOutlineLevelRows(){
         short outlineLevel=0;
-        for(Row r : rows.values()){
-            XSSFRow xrow=(XSSFRow)r;
+        for(XSSFRow xrow : rows.values()){
             outlineLevel=xrow.getCTRow().getOutlineLevel()>outlineLevel? xrow.getCTRow().getOutlineLevel(): outlineLevel;
         }
         return outlineLevel;
@@ -1224,7 +1224,7 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
      * Call getRowNum() on each row if you care which one it is.
      */
     public Iterator<Row> rowIterator() {
-        return rows.values().iterator();
+        return (Iterator<Row>)(Iterator<? extends Row>)rows.values().iterator();
     }
 
     /**
@@ -1466,18 +1466,16 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
         for (Iterator<Row> it = rowIterator() ; it.hasNext() ; ) {
             XSSFRow row = (XSSFRow)it.next();
             int rownum = row.getRowNum();
+            if(rownum < startRow) continue;
 
             if (!copyRowHeight) {
                 row.setHeight((short)-1);
             }
 
-            if (resetOriginalRowHeight && getDefaultRowHeight() >= 0) {
-                row.setHeight(getDefaultRowHeight());
-            }
-            if (removeRow(startRow, endRow, n, row.getRowNum())) {
+            if (removeRow(startRow, endRow, n, rownum)) {
                 it.remove();
             }
-            else if (row.getRowNum() >= startRow && row.getRowNum() <= endRow) {
+            else if (rownum >= startRow && rownum <= endRow) {
                 row.shift(n);
             }
 
@@ -1493,26 +1491,21 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
                 }
             }
         }
-        //rebuild the rows map
+        XSSFRowShifter rowShifter = new XSSFRowShifter(this);
+
         int sheetIndex = getWorkbook().getSheetIndex(this);
         FormulaShifter shifter = FormulaShifter.createForRowShift(sheetIndex, startRow, endRow, n);
-        TreeMap<Integer, Row> map = new TreeMap<Integer, Row>();
-        for(Row r : this) {
-            XSSFRow row = (XSSFRow)r;
-            row.updateFormulasAfterCellShift(shifter);
+
+        rowShifter.updateNamedRanges(shifter);
+        rowShifter.updateFormulas(shifter);
+        rowShifter.shiftMerged(startRow, endRow, n);
+
+        //rebuild the rows map
+        TreeMap<Integer, XSSFRow> map = new TreeMap<Integer, XSSFRow>();
+        for(XSSFRow r : rows.values()) {
             map.put(r.getRowNum(), r);
         }
         rows = map;
-
-        //update formulas on other sheets
-        for(XSSFSheet sheet : getWorkbook()) {
-            if (sheet == this) continue;
-            for(Row r : sheet) {
-                XSSFRow row = (XSSFRow)r;
-                row.updateFormulasAfterCellShift(shifter);
-            }
-        }
-
     }
 
     /**
@@ -1783,10 +1776,9 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
 
         CTSheetData sheetData = worksheet.getSheetData();
         ArrayList<CTRow> rArray = new ArrayList<CTRow>(rows.size());
-        for(Row row : rows.values()){
-            XSSFRow r = (XSSFRow)row;
-            r.onDocumentWrite();
-            rArray.add(r.getCTRow());
+        for(XSSFRow row : rows.values()){
+            row.onDocumentWrite();
+            rArray.add(row.getCTRow());
         }
         sheetData.setRowArray(rArray.toArray(new CTRow[rArray.size()]));
 

src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/XSSFRowShifter.java

@@ -0,0 +1,189 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+package org.apache.poi.xssf.usermodel.helpers;
+
+import org.apache.poi.xssf.usermodel.*;
+import org.apache.poi.ss.util.CellRangeAddress;
+import org.apache.poi.ss.formula.FormulaParser;
+import org.apache.poi.ss.formula.FormulaType;
+import org.apache.poi.ss.formula.FormulaRenderer;
+import org.apache.poi.ss.usermodel.Row;
+import org.apache.poi.ss.usermodel.Cell;
+import org.apache.poi.hssf.record.formula.FormulaShifter;
+import org.apache.poi.hssf.record.formula.Ptg;
+import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTCell;
+import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTCellFormula;
+
+import java.util.List;
+import java.util.ArrayList;
+
+/**
+ * @author Yegor Kozlov
+ */
+public class XSSFRowShifter {
+    private final XSSFSheet sheet;
+
+    public XSSFRowShifter(XSSFSheet sh) {
+        sheet = sh;
+    }
+
+    /**
+     * Shift merged regions
+     *
+     * @param startRow the row to start shifting
+     * @param endRow   the row to end shifting
+     * @param n        the number of rows to shift
+     * @return an array of affected cell regions
+     */
+    public List<CellRangeAddress> shiftMerged(int startRow, int endRow, int n) {
+        List<CellRangeAddress> shiftedRegions = new ArrayList<CellRangeAddress>();
+        //move merged regions completely if they fall within the new region boundaries when they are shifted
+        for (int i = 0; i < sheet.getNumMergedRegions(); i++) {
+            CellRangeAddress merged = sheet.getMergedRegion(i);
+
+            boolean inStart = (merged.getFirstRow() >= startRow || merged.getLastRow() >= startRow);
+            boolean inEnd = (merged.getFirstRow() <= endRow || merged.getLastRow() <= endRow);
+
+            //don't check if it's not within the shifted area
+            if (!inStart || !inEnd) {
+                continue;
+            }
+
+            //only shift if the region outside the shifted rows is not merged too
+            if (!containsCell(merged, startRow - 1, 0) && !containsCell(merged, endRow + 1, 0)) {
+                merged.setFirstRow(merged.getFirstRow() + n);
+                merged.setLastRow(merged.getLastRow() + n);
+                //have to remove/add it back
+                shiftedRegions.add(merged);
+                sheet.removeMergedRegion(i);
+                i = i - 1; // we have to back up now since we removed one
+            }
+        }
+
+        //read so it doesn't get shifted again
+        for (CellRangeAddress region : shiftedRegions) {
+            sheet.addMergedRegion(region);
+        }
+        return shiftedRegions;
+    }
+
+    /**
+     * Check if the  row and column are in the specified cell range
+     *
+     * @param cr    the cell range to check in
+     * @param rowIx the row to check
+     * @param colIx the column to check
+     * @return true if the range contains the cell [rowIx,colIx]
+     */
+    private static boolean containsCell(CellRangeAddress cr, int rowIx, int colIx) {
+        if (cr.getFirstRow() <= rowIx && cr.getLastRow() >= rowIx
+                && cr.getFirstColumn() <= colIx && cr.getLastColumn() >= colIx) {
+            return true;
+        }
+        return false;
+    }
+
+    /**
+     * Updated named ranges
+     */
+    public void updateNamedRanges(FormulaShifter shifter) {
+        XSSFWorkbook wb = sheet.getWorkbook();
+        int sheetIndex = wb.getSheetIndex(sheet);
+        XSSFEvaluationWorkbook fpb = XSSFEvaluationWorkbook.create(wb);
+        for (int i = 0; i < wb.getNumberOfNames(); i++) {
+            XSSFName name = wb.getNameAt(i);
+            String formula = name.getRefersToFormula();
+
+            Ptg[] ptgs = FormulaParser.parse(formula, fpb, FormulaType.NAMEDRANGE, sheetIndex);
+            if (shifter.adjustFormula(ptgs, sheetIndex)) {
+                String shiftedFmla = FormulaRenderer.toFormulaString(fpb, ptgs);
+                name.setRefersToFormula(shiftedFmla);
+            }
+
+        }
+    }
+
+    /**
+     * Update formulas.
+     */
+    public void updateFormulas(FormulaShifter shifter) {
+        //update formulas on the parent sheet
+        updateSheetFormulas(sheet, shifter);
+
+        //update formulas on other sheets
+        XSSFWorkbook wb = sheet.getWorkbook();
+        for (XSSFSheet sh : wb) {
+            if (sheet == sh) continue;
+            updateSheetFormulas(sh, shifter);
+        }
+    }
+
+    private void updateSheetFormulas(XSSFSheet sh, FormulaShifter shifter) {
+        for (Row r : sh) {
+            XSSFRow row = (XSSFRow) r;
+            updateRowFormulas(row, shifter);
+        }
+    }
+
+    private void updateRowFormulas(XSSFRow row, FormulaShifter shifter) {
+        for (Cell c : row) {
+            XSSFCell cell = (XSSFCell) c;
+
+            CTCell ctCell = cell.getCTCell();
+            if (ctCell.isSetF()) {
+                CTCellFormula f = ctCell.getF();
+                String formula = f.getStringValue();
+                if (formula.length() > 0) {
+                    String shiftedFormula = shiftFormula(row, formula, shifter);
+                    if (shiftedFormula != null) {
+                        f.setStringValue(shiftedFormula);
+                    }
+                }
+
+                if (f.isSetRef()) { //Range of cells which the formula applies to.
+                    String ref = f.getRef();
+                    String shiftedRef = shiftFormula(row, ref, shifter);
+                    if (shiftedRef != null) f.setRef(shiftedRef);
+                }
+            }
+
+        }
+    }
+
+    /**
+     * Shift a formula using the supplied FormulaShifter
+     *
+     * @param row     the row of the cell this formula belongs to. Used to get a reference to the parent workbook.
+     * @param formula the formula to shift
+     * @param shifter the FormulaShifter object that operates on the parsed formula tokens
+     * @return the shifted formula if the formula was changed,
+     *         <code>null</code> if the formula wasn't modified
+     */
+    private static String shiftFormula(XSSFRow row, String formula, FormulaShifter shifter) {
+        XSSFSheet sheet = row.getSheet();
+        XSSFWorkbook wb = sheet.getWorkbook();
+        int sheetIndex = wb.getSheetIndex(sheet);
+        XSSFEvaluationWorkbook fpb = XSSFEvaluationWorkbook.create(wb);
+        Ptg[] ptgs = FormulaParser.parse(formula, fpb, FormulaType.CELL, sheetIndex);
+        String shiftedFmla = null;
+        if (shifter.adjustFormula(ptgs, sheetIndex)) {
+            shiftedFmla = FormulaRenderer.toFormulaString(fpb, ptgs);
+        }
+        return shiftedFmla;
+    }
+
+}

src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestSheetShiftRows.java

@@ -38,8 +38,8 @@ public class TestSheetShiftRows  extends BaseTestSheetShiftRows {
         baseTestShiftRow();
     }
 
-    public void testShiftRow0() {
-        baseTestShiftRow0();
+    public void testShiftNames() {
+        baseTestShiftWithNames();
     }
 
     //TODO support shifting of page breaks
@@ -55,4 +55,8 @@ public class TestSheetShiftRows  extends BaseTestSheetShiftRows {
     public void testShiftWithFormulas() {
         baseTestShiftWithFormulas("ForShifting.xlsx");
     }
+
+    public void testShiftWithMergedRegions() {
+        baseTestShiftWithMergedRegions();
+    }
 }

src/testcases/org/apache/poi/hssf/usermodel/TestSheetShiftRows.java

@@ -43,8 +43,8 @@ public final class TestSheetShiftRows extends BaseTestSheetShiftRows {
         baseTestShiftRow();
     }
 
-    public void testShiftRow0() {
-        baseTestShiftRow0();
+    public void testShiftNames() {
+        baseTestShiftWithNames();
     }
 
     public void testShiftRowBreaks() {
@@ -59,4 +59,7 @@ public final class TestSheetShiftRows extends BaseTestSheetShiftRows {
         baseTestShiftWithFormulas("ForShifting.xls");
     }
 
+    public void testShiftWithMergedRegions() {
+        baseTestShiftWithMergedRegions();
+    }
 }

src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java

@@ -19,6 +19,7 @@ package org.apache.poi.ss.usermodel;
 
 import junit.framework.TestCase;
 import org.apache.poi.ss.ITestDataProvider;
+import org.apache.poi.ss.util.CellRangeAddress;
 
 /**
  * Tests row shifting capabilities.
@@ -108,9 +109,10 @@ public abstract class BaseTestSheetShiftRows  extends TestCase {
     /**
      * Tests when shifting the first row.
      */
-    public final void baseTestShiftRow0() {
+    public final void baseTestActiveCell() {
         Workbook b = getTestDataProvider().createWorkbook();
         Sheet s	= b.createSheet();
+
         s.createRow(0).createCell(0).setCellValue("TEST1");
         s.createRow(3).createCell(0).setCellValue("TEST2");
         s.shiftRows(0,4,1);
@@ -190,6 +192,45 @@ public abstract class BaseTestSheetShiftRows  extends TestCase {
         assertEquals(comment4,comment4_shifted);
     }
 
+    public final void baseTestShiftWithNames() {
+        Workbook wb = getTestDataProvider().createWorkbook();
+        Sheet sheet	= wb.createSheet();
+        Row row = sheet.createRow(0);
+        row.createCell(0).setCellValue(1.1);
+        row.createCell(1).setCellValue(2.2);
+
+        Name name1 = wb.createName();
+        name1.setNameName("name1");
+        name1.setRefersToFormula("A1+B1");
+
+        Name name2 = wb.createName();
+        name2.setNameName("name2");
+        name2.setRefersToFormula("A1");
+
+        sheet.shiftRows(0, 1, 2);
+        name1 = wb.getNameAt(0);
+        assertEquals("A3+B3", name1.getRefersToFormula());
+
+        name2 = wb.getNameAt(1);
+        assertEquals("A3", name2.getRefersToFormula());    
+    }
+
+    public final void baseTestShiftWithMergedRegions() {
+        Workbook wb = getTestDataProvider().createWorkbook();
+        Sheet sheet	= wb.createSheet();
+        Row row = sheet.createRow(0);
+        row.createCell(0).setCellValue(1.1);
+        row.createCell(1).setCellValue(2.2);
+        CellRangeAddress region = new CellRangeAddress(0, 0, 0, 2);
+        assertEquals("A1:C1", region.formatAsString());
+
+        sheet.addMergedRegion(region);
+
+        sheet.shiftRows(0, 1, 2);
+        region = sheet.getMergedRegion(0);
+        assertEquals("A3:C3", region.formatAsString());
+   }
+
     /**
      * See bug #34023
      *