]> source.dussan.org Git - poi.git/commitdiff
Github-177: Avoid NullPointerException if RangeCopier encounters empty/missing rows
authorDominik Stadler <centic@apache.org>
Sun, 26 Apr 2020 08:23:04 +0000 (08:23 +0000)
committerDominik Stadler <centic@apache.org>
Sun, 26 Apr 2020 08:23:04 +0000 (08:23 +0000)
Also expose one-parameter constructor and verify it in tests.

Closes #177

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

src/java/org/apache/poi/hssf/usermodel/HSSFRangeCopier.java
src/java/org/apache/poi/ss/usermodel/RangeCopier.java
src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFRangeCopier.java
src/ooxml/testcases/org/apache/poi/ss/usermodel/TestXSSFRangeCopier.java
src/testcases/org/apache/poi/hssf/usermodel/TestHSSFRangeCopier.java
src/testcases/org/apache/poi/ss/usermodel/TestRangeCopier.java

index f6d3fb381939b8e451d65d484f41d9c66dfd91ed..60b1a23417eb6767889b7123e79f7e50714c5c3d 100644 (file)
@@ -32,6 +32,10 @@ public class HSSFRangeCopier extends RangeCopier {
         super(sourceSheet, destSheet);
     }
 
+    public HSSFRangeCopier(Sheet sheet) {
+        super(sheet);
+    }
+
     protected void adjustCellReferencesInsideFormula(Cell cell, Sheet destSheet, int deltaX, int deltaY) {
         FormulaRecordAggregate fra = (FormulaRecordAggregate)((HSSFCell)cell).getCellValueRecord();
         int destSheetIndex = destSheet.getWorkbook().getSheetIndex(destSheet);
index 6c73530b5c7d2c22cd046def137f69d63cf7a8da..8409accc721c0438256eaa78d260c89a917ad531 100644 (file)
@@ -36,13 +36,15 @@ public abstract class RangeCopier {
         this.sourceSheet = sourceSheet;
         this.destSheet = destSheet;
     }
+
     public RangeCopier(Sheet sheet) {
         this(sheet, sheet);
     }
-    /** Uses input pattern to tile destination region, overwriting existing content. Works in following manner : 
+
+    /** Uses input pattern to tile destination region, overwriting existing content. Works in following manner :
      * 1.Start from top-left of destination.
      * 2.Paste source but only inside of destination borders.
-     * 3.If there is space left on right or bottom side of copy, process it as in step 2. 
+     * 3.If there is space left on right or bottom side of copy, process it as in step 2.
      * @param tilePatternRange source range which should be copied in tiled manner
      * @param tileDestRange     destination range, which should be overridden
      */
@@ -50,49 +52,54 @@ public abstract class RangeCopier {
         Sheet sourceCopy = sourceSheet.getWorkbook().cloneSheet(sourceSheet.getWorkbook().getSheetIndex(sourceSheet));
         int sourceWidthMinus1 = tilePatternRange.getLastColumn() - tilePatternRange.getFirstColumn();
         int sourceHeightMinus1 = tilePatternRange.getLastRow() - tilePatternRange.getFirstRow();
-        int rightLimitToCopy; 
+        int rightLimitToCopy;
         int bottomLimitToCopy;
 
         int nextRowIndexToCopy = tileDestRange.getFirstRow();
-        do { 
+        do {
             int nextCellIndexInRowToCopy = tileDestRange.getFirstColumn();
             int heightToCopyMinus1 = Math.min(sourceHeightMinus1, tileDestRange.getLastRow() - nextRowIndexToCopy);
             bottomLimitToCopy = tilePatternRange.getFirstRow() + heightToCopyMinus1;
-            do { 
+            do {
                 int widthToCopyMinus1 = Math.min(sourceWidthMinus1, tileDestRange.getLastColumn() - nextCellIndexInRowToCopy);
                 rightLimitToCopy = tilePatternRange.getFirstColumn() + widthToCopyMinus1;
                 CellRangeAddress rangeToCopy = new CellRangeAddress(
                         tilePatternRange.getFirstRow(),     bottomLimitToCopy,
-                        tilePatternRange.getFirstColumn(),  rightLimitToCopy 
+                        tilePatternRange.getFirstColumn(),  rightLimitToCopy
                        );
                 copyRange(rangeToCopy, nextCellIndexInRowToCopy - rangeToCopy.getFirstColumn(), nextRowIndexToCopy - rangeToCopy.getFirstRow(), sourceCopy);
-                nextCellIndexInRowToCopy += widthToCopyMinus1 + 1; 
+                nextCellIndexInRowToCopy += widthToCopyMinus1 + 1;
             } while (nextCellIndexInRowToCopy <= tileDestRange.getLastColumn());
             nextRowIndexToCopy += heightToCopyMinus1 + 1;
         } while (nextRowIndexToCopy <= tileDestRange.getLastRow());
-        
+
         int tempCopyIndex = sourceSheet.getWorkbook().getSheetIndex(sourceCopy);
-        sourceSheet.getWorkbook().removeSheetAt(tempCopyIndex); 
+        sourceSheet.getWorkbook().removeSheetAt(tempCopyIndex);
     }
 
     private void copyRange(CellRangeAddress sourceRange, int deltaX, int deltaY, Sheet sourceClone) { //NOSONAR, it's a bit complex but monolith method, does not make much sense to divide it
         if(deltaX != 0)
-            horizontalFormulaShifter = FormulaShifter.createForColumnCopy(sourceSheet.getWorkbook().getSheetIndex(sourceSheet), 
+            horizontalFormulaShifter = FormulaShifter.createForColumnCopy(sourceSheet.getWorkbook().getSheetIndex(sourceSheet),
                     sourceSheet.getSheetName(), sourceRange.getFirstColumn(), sourceRange.getLastColumn(), deltaX, sourceSheet.getWorkbook().getSpreadsheetVersion());
         if(deltaY != 0)
-            verticalFormulaShifter = FormulaShifter.createForRowCopy(sourceSheet.getWorkbook().getSheetIndex(sourceSheet), 
+            verticalFormulaShifter = FormulaShifter.createForRowCopy(sourceSheet.getWorkbook().getSheetIndex(sourceSheet),
                     sourceSheet.getSheetName(), sourceRange.getFirstRow(), sourceRange.getLastRow(), deltaY, sourceSheet.getWorkbook().getSpreadsheetVersion());
-        
-        for(int rowNo = sourceRange.getFirstRow(); rowNo <= sourceRange.getLastRow(); rowNo++) {   
-            Row sourceRow = sourceClone.getRow(rowNo); // copy from source copy, original source might be overridden in process!
-            for (int columnIndex = sourceRange.getFirstColumn(); columnIndex <= sourceRange.getLastColumn(); columnIndex++) {  
+
+        for(int rowNo = sourceRange.getFirstRow(); rowNo <= sourceRange.getLastRow(); rowNo++) {
+            // copy from source copy, original source might be overridden in process!
+            Row sourceRow = sourceClone.getRow(rowNo);
+            if(sourceRow == null) {
+                continue;
+            }
+
+            for (int columnIndex = sourceRange.getFirstColumn(); columnIndex <= sourceRange.getLastColumn(); columnIndex++) {
                 Cell sourceCell = sourceRow.getCell(columnIndex);
                 if(sourceCell == null)
                     continue;
                 Row destRow = destSheet.getRow(rowNo + deltaY);
                 if(destRow == null)
                     destRow = destSheet.createRow(rowNo + deltaY);
-                
+
                 Cell newCell = destRow.getCell(columnIndex + deltaX);
                 if(newCell == null) {
                     newCell = destRow.createCell(columnIndex + deltaX);
@@ -104,56 +111,56 @@ public abstract class RangeCopier {
             }
         }
     }
-    
+
     protected abstract void adjustCellReferencesInsideFormula(Cell cell, Sheet destSheet, int deltaX, int deltaY); // this part is different for HSSF and XSSF
-    
+
     protected boolean adjustInBothDirections(Ptg[] ptgs, int sheetIndex, int deltaX, int deltaY) {
         boolean adjustSucceeded = true;
         if(deltaY != 0)
-            adjustSucceeded = verticalFormulaShifter.adjustFormula(ptgs, sheetIndex); 
+            adjustSucceeded = verticalFormulaShifter.adjustFormula(ptgs, sheetIndex);
         if(deltaX != 0)
             adjustSucceeded = adjustSucceeded && horizontalFormulaShifter.adjustFormula(ptgs, sheetIndex);
         return adjustSucceeded;
     }
-    
-    // TODO clone some more properties ? 
-    public static void cloneCellContent(Cell srcCell, Cell destCell, Map<Integer, CellStyle> styleMap) {   
-         if(styleMap != null) {   
-             if(srcCell.getSheet().getWorkbook() == destCell.getSheet().getWorkbook()){   
-                 destCell.setCellStyle(srcCell.getCellStyle());   
+
+    // TODO clone some more properties ?
+    public static void cloneCellContent(Cell srcCell, Cell destCell, Map<Integer, CellStyle> styleMap) {
+         if(styleMap != null) {
+             if(srcCell.getSheet().getWorkbook() == destCell.getSheet().getWorkbook()){
+                 destCell.setCellStyle(srcCell.getCellStyle());
              } else {
-                 int stHashCode = srcCell.getCellStyle().hashCode();   
-                 CellStyle newCellStyle = styleMap.get(stHashCode);   
-                 if(newCellStyle == null){   
-                     newCellStyle = destCell.getSheet().getWorkbook().createCellStyle();   
-                     newCellStyle.cloneStyleFrom(srcCell.getCellStyle());   
-                     styleMap.put(stHashCode, newCellStyle);   
-                 }   
-                 destCell.setCellStyle(newCellStyle);   
-             }   
-         }   
-         switch(srcCell.getCellType()) {   
-             case STRING:   
-                 destCell.setCellValue(srcCell.getStringCellValue());   
-                 break;   
+                 int stHashCode = srcCell.getCellStyle().hashCode();
+                 CellStyle newCellStyle = styleMap.get(stHashCode);
+                 if(newCellStyle == null){
+                     newCellStyle = destCell.getSheet().getWorkbook().createCellStyle();
+                     newCellStyle.cloneStyleFrom(srcCell.getCellStyle());
+                     styleMap.put(stHashCode, newCellStyle);
+                 }
+                 destCell.setCellStyle(newCellStyle);
+             }
+         }
+         switch(srcCell.getCellType()) {
+             case STRING:
+                 destCell.setCellValue(srcCell.getStringCellValue());
+                 break;
              case NUMERIC:
-                 destCell.setCellValue(srcCell.getNumericCellValue());   
-                 break;   
-             case BLANK:   
+                 destCell.setCellValue(srcCell.getNumericCellValue());
+                 break;
+             case BLANK:
                  destCell.setBlank();
-                 break;   
-             case BOOLEAN:   
-                 destCell.setCellValue(srcCell.getBooleanCellValue());   
-                 break;   
-             case ERROR:   
-                 destCell.setCellErrorValue(srcCell.getErrorCellValue());   
-                 break;   
-             case FORMULA: 
+                 break;
+             case BOOLEAN:
+                 destCell.setCellValue(srcCell.getBooleanCellValue());
+                 break;
+             case ERROR:
+                 destCell.setCellErrorValue(srcCell.getErrorCellValue());
+                 break;
+             case FORMULA:
                  String oldFormula = srcCell.getCellFormula();
-                 destCell.setCellFormula(oldFormula);   
-                 break;   
-             default:   
-                 break;   
-         }   
+                 destCell.setCellFormula(oldFormula);
+                 break;
+             default:
+                 break;
+         }
      }
 }
index 9269843828cfc0b0e893f79b1242e614752b0dc2..50248fae813a8bd4c5fb759d6a2d16a90034404f 100644 (file)
@@ -31,13 +31,17 @@ import org.apache.poi.util.Beta;
 @Beta
 public class XSSFRangeCopier extends RangeCopier {
 
-    public XSSFRangeCopier(Sheet sourceSheet, Sheet destSheet){
+    public XSSFRangeCopier(Sheet sourceSheet, Sheet destSheet) {
         super(sourceSheet, destSheet);
     }
 
+    public XSSFRangeCopier(Sheet sheet) {
+        super(sheet);
+    }
+
     protected void adjustCellReferencesInsideFormula(Cell cell, Sheet destSheet, int deltaX, int deltaY){
         XSSFWorkbook hostWorkbook = (XSSFWorkbook) destSheet.getWorkbook();
-        XSSFEvaluationWorkbook fpb = XSSFEvaluationWorkbook.create(hostWorkbook); 
+        XSSFEvaluationWorkbook fpb = XSSFEvaluationWorkbook.create(hostWorkbook);
         Ptg[] ptgs = FormulaParser.parse(cell.getCellFormula(), fpb, FormulaType.CELL, 0);
         int destSheetIndex = hostWorkbook.getSheetIndex(destSheet);
         if(adjustInBothDirections(ptgs, destSheetIndex, deltaX, deltaY))
index 25b0d9402299c19b2f4bad11a7f9df8ff32ebb05..35f891129349eea4bfc6e4e694abc0747a34a215 100644 (file)
@@ -34,9 +34,9 @@ import java.io.IOException;
 
 public class TestXSSFRangeCopier extends TestRangeCopier {
     public TestXSSFRangeCopier() {
-        super(); 
+        super();
         workbook = new XSSFWorkbook();
-        testDataProvider = XSSFITestDataProvider.instance; 
+        testDataProvider = XSSFITestDataProvider.instance;
     }
 
     @Before
@@ -60,5 +60,5 @@ public class TestXSSFRangeCopier extends TestRangeCopier {
         newRow.copyRowFrom(existingRow, policy);
         assertEquals("$C2+B$2", newRow.getCell(1).getCellFormula());
     }
-    
+
 }
index 8f6b678513b2365dc686e8ae844b2eea053084f9..8d685fc68df80ad4855db1fcfbfa2ba60ae8fdc7 100644 (file)
@@ -27,9 +27,9 @@ import org.junit.Before;
 public class TestHSSFRangeCopier extends TestRangeCopier {
 
     public TestHSSFRangeCopier() {
-        super(); 
+        super();
         workbook = new HSSFWorkbook();
-        testDataProvider = HSSFITestDataProvider.instance; 
+        testDataProvider = HSSFITestDataProvider.instance;
     }
 
     @Before
index def64a32c980acc5349a6722a150ea48fe38e704..d00a820e3ca54dca83fbcad165309e96b8ba4c5b 100644 (file)
@@ -32,15 +32,15 @@ public abstract class TestRangeCopier {
     protected Sheet sheet1;
     protected Sheet sheet2;
     protected Workbook workbook;
-    protected RangeCopier rangeCopier; 
-    protected RangeCopier transSheetRangeCopier; 
+    protected RangeCopier rangeCopier;
+    protected RangeCopier transSheetRangeCopier;
     protected ITestDataProvider testDataProvider;
 
     protected void initSheets() {
         sheet1 = workbook.getSheet("sheet1");
         sheet2 = workbook.getSheet("sheet2");
     }
-    
+
     @Test
     public void copySheetRangeWithoutFormulas() {
         CellRangeAddress rangeToCopy = CellRangeAddress.valueOf("B1:C2");   //2x2
@@ -53,21 +53,21 @@ public abstract class TestRangeCopier {
     @Test
     public void tileTheRangeAway() {
         CellRangeAddress tileRange = CellRangeAddress.valueOf("C4:D5");
-        CellRangeAddress destRange = CellRangeAddress.valueOf("F4:K5"); 
+        CellRangeAddress destRange = CellRangeAddress.valueOf("F4:K5");
         rangeCopier.copyRange(tileRange, destRange);
-        assertEquals("1.3", getCellContent(sheet1, "H4"));  
-        assertEquals("1.3", getCellContent(sheet1, "J4"));  
-        assertEquals("$C1+G$2", getCellContent(sheet1, "G5"));  
-        assertEquals("SUM(G3:I3)", getCellContent(sheet1, "H5"));   
-        assertEquals("$C1+I$2", getCellContent(sheet1, "I5"));  
+        assertEquals("1.3", getCellContent(sheet1, "H4"));
+        assertEquals("1.3", getCellContent(sheet1, "J4"));
+        assertEquals("$C1+G$2", getCellContent(sheet1, "G5"));
+        assertEquals("SUM(G3:I3)", getCellContent(sheet1, "H5"));
+        assertEquals("$C1+I$2", getCellContent(sheet1, "I5"));
         assertEquals("", getCellContent(sheet1, "L5"));  //out of borders
         assertEquals("", getCellContent(sheet1, "G7")); //out of borders
     }
-    
+
     @Test
     public void tileTheRangeOver() {
         CellRangeAddress tileRange = CellRangeAddress.valueOf("C4:D5");
-        CellRangeAddress destRange = CellRangeAddress.valueOf("A4:C5"); 
+        CellRangeAddress destRange = CellRangeAddress.valueOf("A4:C5");
         rangeCopier.copyRange(tileRange, destRange);
         assertEquals("1.3", getCellContent(sheet1, "A4"));
         assertEquals("$C1+B$2", getCellContent(sheet1, "B5"));
@@ -78,7 +78,7 @@ public abstract class TestRangeCopier {
     public void copyRangeToOtherSheet() {
         Sheet destSheet = sheet2;
         CellRangeAddress tileRange = CellRangeAddress.valueOf("C4:D5"); // on sheet1
-        CellRangeAddress destRange = CellRangeAddress.valueOf("F4:J6"); // on sheet2 
+        CellRangeAddress destRange = CellRangeAddress.valueOf("F4:J6"); // on sheet2
         transSheetRangeCopier.copyRange(tileRange, destRange);
         assertEquals("1.3", getCellContent(destSheet, "H4"));
         assertEquals("1.3", getCellContent(destSheet, "J4"));
@@ -86,7 +86,36 @@ public abstract class TestRangeCopier {
         assertEquals("SUM(G3:I3)", getCellContent(destSheet, "H5"));
         assertEquals("$C1+I$2", getCellContent(destSheet, "I5"));
     }
-    
+
+    @Test
+    public void testEmptyRow() {
+        // leave some rows empty in-between
+        Row row = sheet1.createRow(23);
+        row.createCell(0).setCellValue(1.2);
+
+        Sheet destSheet = sheet2;
+        CellRangeAddress tileRange = CellRangeAddress.valueOf("A1:A100"); // on sheet1
+        CellRangeAddress destRange = CellRangeAddress.valueOf("G1:G100"); // on sheet2
+        transSheetRangeCopier.copyRange(tileRange, destRange);
+
+        assertEquals("1.2", getCellContent(destSheet, "G24"));
+    }
+
+    @Test
+    public void testSameSheet() {
+        // leave some rows empty in-between
+        Row row = sheet1.createRow(23);
+        row.createCell(0).setCellValue(1.2);
+
+        CellRangeAddress tileRange = CellRangeAddress.valueOf("A1:A100"); // on sheet1
+        CellRangeAddress destRange = CellRangeAddress.valueOf("G1:G100"); // on sheet2
+
+        // use the a RangeCopier with the same Sheet for source and dest
+        rangeCopier.copyRange(tileRange, destRange);
+
+        assertEquals("1.2", getCellContent(sheet1, "G24"));
+    }
+
     protected static String getCellContent(Sheet sheet, String coordinates) {
         try {
             CellReference p = new CellReference(coordinates);