poi-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From one...@apache.org
Subject svn commit: r1736155 - in /poi/trunk/src: java/org/apache/poi/hssf/usermodel/ java/org/apache/poi/ss/usermodel/ ooxml/java/org/apache/poi/xssf/streaming/ ooxml/java/org/apache/poi/xssf/usermodel/ ooxml/testcases/org/apache/poi/xssf/usermodel/
Date Tue, 22 Mar 2016 09:02:09 GMT
Author: onealj
Date: Tue Mar 22 09:02:08 2016
New Revision: 1736155

URL: http://svn.apache.org/viewvc?rev=1736155&view=rev
Log:
bug 59212: Do not check for overlapping regions when adding merged regions to a sheet

Modified:
    poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java
    poi/trunk/src/java/org/apache/poi/ss/usermodel/Sheet.java
    poi/trunk/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFSheet.java
    poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java
    poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java

Modified: poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java?rev=1736155&r1=1736154&r2=1736155&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java (original)
+++ poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java Tue Mar 22 09:02:08 2016
@@ -660,28 +660,75 @@ public final class HSSFSheet implements
     }
 
     /**
+     * Adds a merged region of cells on a sheet.
+     *
+     * @param region to merge
+     * @return index of this region
+     * @throws IllegalArgumentException if region contains fewer than 2 cells
+     * @throws IllegalStateException if region intersects with a multi-cell array formula
+     * @throws IllegalStateException if region intersects with an existing region on this
sheet
+     */
+    @Override
+    public int addMergedRegion(CellRangeAddress region) {
+        return addMergedRegion(region, true);
+    }
+
+    /**
+     * Adds a merged region of cells (hence those cells form one).
+     * Skips validation. It is possible to create overlapping merged regions
+     * or create a merged region that intersects a multi-cell array formula
+     * with this formula, which may result in a corrupt workbook.
+     *
+     * To check for merged regions overlapping array formulas or other merged regions
+     * after addMergedRegionUnsafe has been called, call {@link #validateMergedRegions()},
which runs in O(n^2) time.
+     *
+     * @param region to merge
+     * @return index of this region
+     * @throws IllegalArgumentException if region contains fewer than 2 cells
+     */
+    @Override
+    public int addMergedRegionUnsafe(CellRangeAddress region) {
+        return addMergedRegion(region, false);
+    }
+
+    /**
+     * Verify that merged regions do not intersect multi-cell array formulas and
+     * no merged regions intersect another merged region in this sheet.
+     *
+     * @throws IllegalStateException if region intersects with a multi-cell array formula
+     * @throws IllegalStateException if at least one region intersects with another merged
region in this sheet
+     */
+    @Override
+    public void validateMergedRegions() {
+        checkForMergedRegionsIntersectingArrayFormulas();
+        checkForIntersectingMergedRegions();
+    }
+
+    /**
      * adds a merged region of cells (hence those cells form one)
      *
      * @param region (rowfrom/colfrom-rowto/colto) to merge
+     * @param validate whether to validate merged region
      * @return index of this region
      * @throws IllegalArgumentException if region contains fewer than 2 cells
      * @throws IllegalStateException if region intersects with an existing merged region
      * or multi-cell array formula on this sheet
      */
-    @Override
-    public int addMergedRegion(CellRangeAddress region) {
+    private int addMergedRegion(CellRangeAddress region, boolean validate) {
         if (region.getNumberOfCells() < 2) {
             throw new IllegalArgumentException("Merged region " + region.formatAsString()
+ " must contain 2 or more cells");
         }
         region.validate(SpreadsheetVersion.EXCEL97);
 
-        // throw IllegalStateException if the argument CellRangeAddress intersects with
-        // a multi-cell array formula defined in this sheet
-        validateArrayFormulas(region);
+        if (validate) {
+            // throw IllegalStateException if the argument CellRangeAddress intersects with
+            // a multi-cell array formula defined in this sheet
+            validateArrayFormulas(region);
         
-        // Throw IllegalStateException if the argument CellRangeAddress intersects with
-        // a merged region already in this sheet
-        validateMergedRegions(region);
+            // Throw IllegalStateException if the argument CellRangeAddress intersects with
+            // a merged region already in this sheet
+            validateMergedRegions(region);
+        }
 
         return _sheet.addMergedRegion(region.getFirstRow(),
                 region.getFirstColumn(),
@@ -716,7 +763,19 @@ public final class HSSFSheet implements
         }
 
     }
-    
+
+    /**
+     * Verify that none of the merged regions intersect a multi-cell array formula in this
sheet
+     *
+     * @param region
+     * @throws IllegalStateException if candidate region intersects an existing array formula
in this sheet
+     */
+    private void checkForMergedRegionsIntersectingArrayFormulas() {
+        for (CellRangeAddress region : getMergedRegions()) {
+            validateArrayFormulas(region);
+        }
+    }
+
     private void validateMergedRegions(CellRangeAddress candidateRegion) {
         for (final CellRangeAddress existingRegion : getMergedRegions()) {
             if (existingRegion.intersects(candidateRegion)) {
@@ -725,6 +784,27 @@ public final class HSSFSheet implements
             }
         }
     }
+
+    /**
+     * Verify that no merged regions intersect another merged region in this sheet.
+     *
+     * @throws IllegalStateException if at least one region intersects with another merged
region in this sheet
+     */
+    private void checkForIntersectingMergedRegions() {
+        final List<CellRangeAddress> regions = getMergedRegions();
+        final int size = regions.size();
+        for (int i=0; i < size; i++) {
+            final CellRangeAddress region = regions.get(i);
+            for (final CellRangeAddress other : regions.subList(i+1, regions.size())) {
+                if (region.intersects(other)) {
+                    String msg = "The range " + region.formatAsString() +
+                                " intersects with another merged region " +
+                                other.formatAsString() + " in this sheet";
+                    throw new IllegalStateException(msg);
+                }
+            }
+        }
+    }
 
     /**
      * Control if Excel should be asked to recalculate all formulas on this sheet

Modified: poi/trunk/src/java/org/apache/poi/ss/usermodel/Sheet.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/usermodel/Sheet.java?rev=1736155&r1=1736154&r2=1736155&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/usermodel/Sheet.java (original)
+++ poi/trunk/src/java/org/apache/poi/ss/usermodel/Sheet.java Tue Mar 22 09:02:08 2016
@@ -277,6 +277,30 @@ public interface Sheet extends Iterable<
     int addMergedRegion(CellRangeAddress region);
 
     /**
+     * Adds a merged region of cells (hence those cells form one).
+     * Skips validation. It is possible to create overlapping merged regions
+     * or create a merged region that intersects a multi-cell array formula
+     * with this formula, which may result in a corrupt workbook.
+     *
+     * To check for merged regions overlapping array formulas or other merged regions
+     * after addMergedRegionUnsafe has been called, call {@link #validateMergedRegions()},
which runs in O(n^2) time.
+     *
+     * @param region to merge
+     * @return index of this region
+     * @throws IllegalArgumentException if region contains fewer than 2 cells
+     */
+    int addMergedRegionUnsafe(CellRangeAddress region);
+
+    /**
+     * Verify that merged regions do not intersect multi-cell array formulas and
+     * no merged regions intersect another merged region in this sheet.
+     *
+     * @throws IllegalStateException if region intersects with a multi-cell array formula
+     * @throws IllegalStateException if at least one region intersects with another merged
region in this sheet
+     */
+    void validateMergedRegions();
+
+    /**
      * Determines whether the output is vertically centered on the page.
      *
      * @param value true to vertically center, false otherwise.

Modified: poi/trunk/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFSheet.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFSheet.java?rev=1736155&r1=1736154&r2=1736155&view=diff
==============================================================================
--- poi/trunk/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFSheet.java (original)
+++ poi/trunk/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFSheet.java Tue Mar 22 09:02:08
2016
@@ -389,6 +389,30 @@ public class SXSSFSheet implements Sheet
     }
 
     /**
+     * Adds a merged region of cells (hence those cells form one)
+     *
+     * @param region (rowfrom/colfrom-rowto/colto) to merge
+     * @return index of this region
+     */
+    @Override
+    public int addMergedRegionUnsafe(CellRangeAddress region)
+    {
+        return _sh.addMergedRegionUnsafe(region);
+    }
+
+    /**
+     * Verify that merged regions do not intersect multi-cell array formulas and
+     * no merged regions intersect another merged region in this sheet.
+     *
+     * @throws IllegalStateException if region intersects with a multi-cell array formula
+     * @throws IllegalStateException if at least one region intersects with another merged
region in this sheet
+     */
+    @Override
+    public void validateMergedRegions() {
+        _sh.validateMergedRegions();
+    }
+
+    /**
      * Determines whether the output is vertically centered on the page.
      *
      * @param value true to vertically center, false otherwise.

Modified: poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java?rev=1736155&r1=1736154&r2=1736155&view=diff
==============================================================================
--- poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java (original)
+++ poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java Tue Mar 22 09:02:08
2016
@@ -323,6 +323,7 @@ public class XSSFSheet extends POIXMLDoc
      * @return index of this region
      * @throws IllegalArgumentException if region contains fewer than 2 cells
      */
+    @Override
     public int addMergedRegionUnsafe(CellRangeAddress region) {
         return addMergedRegion(region, false);
     }
@@ -408,7 +409,6 @@ public class XSSFSheet extends POIXMLDoc
         }
     }
 
-
     /**
      * Verify that candidate region does not intersect with an existing merged region in
this sheet
      *
@@ -452,6 +452,7 @@ public class XSSFSheet extends POIXMLDoc
      * @throws IllegalStateException if region intersects with a multi-cell array formula
      * @throws IllegalStateException if at least one region intersects with another merged
region in this sheet
      */
+    @Override
     public void validateMergedRegions() {
         checkForMergedRegionsIntersectingArrayFormulas();
         checkForIntersectingMergedRegions();

Modified: poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java?rev=1736155&r1=1736154&r2=1736155&view=diff
==============================================================================
--- poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java (original)
+++ poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java Tue Mar
22 09:02:08 2016
@@ -27,7 +27,6 @@ import static org.junit.Assert.assertNul
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
-import static org.junit.Assume.assumeTrue;
 
 import java.io.IOException;
 import java.util.Arrays;
@@ -303,54 +302,6 @@ public final class TestXSSFSheet extends
         workbook.close();
     }
 
-    /**
-     * bug 58885: checking for overlapping merged regions when
-     * adding a merged region is safe, but runs in O(n).
-     * the check for merged regions when adding a merged region
-     * can be skipped (unsafe) and run in O(1).
-     */
-    @Test
-    public void addMergedRegionUnsafe() throws IOException {
-        XSSFWorkbook wb = new XSSFWorkbook();
-        XSSFSheet sh = wb.createSheet();
-        CellRangeAddress region1 = CellRangeAddress.valueOf("A1:B2");
-        CellRangeAddress region2 = CellRangeAddress.valueOf("B2:C3");
-        CellRangeAddress region3 = CellRangeAddress.valueOf("C3:D4");
-        CellRangeAddress region4 = CellRangeAddress.valueOf("J10:K11");
-        assumeTrue(region1.intersects(region2));
-        assumeTrue(region2.intersects(region3));
-
-        sh.addMergedRegionUnsafe(region1);
-        assertTrue(sh.getMergedRegions().contains(region1));        
-
-        // adding a duplicate or overlapping merged region should not
-        // raise an exception with the unsafe version of addMergedRegion.
-           
-        sh.addMergedRegionUnsafe(region2);
-
-        // the safe version of addMergedRegion should throw when trying to add a merged region
that overlaps an existing region
-        assertTrue(sh.getMergedRegions().contains(region2));
-        try {
-            sh.addMergedRegion(region3);
-            fail("Expected IllegalStateException. region3 overlaps already added merged region2.");
-        } catch (final IllegalStateException e) {
-            // expected
-            assertFalse(sh.getMergedRegions().contains(region3));
-        }
-        // addMergedRegion should not re-validate previously-added merged regions
-        sh.addMergedRegion(region4);
-
-        // validation methods should detect a problem with previously added merged regions
(runs in O(n^2) time)
-        try {
-            sh.validateMergedRegions();
-            fail("Expected validation to fail. Sheet contains merged regions A1:B2 and B2:C3,
which overlap at B2.");
-        } catch (final IllegalStateException e) {
-            // expected
-        }
-
-        wb.close();
-    }
-
     @Test
     public void setDefaultColumnStyle() throws IOException {
         XSSFWorkbook workbook = new XSSFWorkbook();



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


Mime
View raw message