fix for bug 14729 - cant merge more than 1027 times.

fix submitted by tony poppleton


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

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

@@ -109,8 +109,10 @@ public class Sheet implements Model
     protected FooterRecord              footer           = null;
     protected PrintGridlinesRecord      printGridlines   = null;
     protected MergeCellsRecord          merged           = null;
+    protected ArrayList                 mergedRecords    = new ArrayList();
+    protected ArrayList                 mergedLocs       = new ArrayList();
+    protected int                       numMergedRegions = 0;
     protected SelectionRecord           selection        = null;
-    protected int                       mergedloc        = 0;
     private static POILogger            log              = POILogFactory.getLogger(Sheet.class);
     private ArrayList                   columnSizes      = null;  // holds column info
     protected ValueRecordsAggregate     cells            = null;
@@ -191,8 +193,10 @@ public class Sheet implements Model
             }
             else if (rec.getSid() == MergeCellsRecord.sid)
             {
+                retval.mergedRecords.add(rec);
                 retval.merged = ( MergeCellsRecord ) rec;
-                retval.mergedloc = k - offset;
+                retval.mergedLocs.add(new Integer(k - offset));
+                retval.numMergedRegions += retval.merged.getNumAreas();
             }
             else if (rec.getSid() == ColumnInfoRecord.sid)
             {
@@ -421,37 +425,96 @@ public class Sheet implements Model
     public int addMergedRegion(int rowFrom, short colFrom, int rowTo,
                                short colTo)
     {
-        if (merged == null)
+        if (merged == null || merged.getNumAreas() == 1027)
         {
             merged = ( MergeCellsRecord ) createMergedCells();
-            mergedloc = records.size() - 1;
+            mergedRecords.add(merged);
+            mergedLocs.add(new Integer(records.size() - 1));
             records.add(records.size() - 1, merged);
         }
-        return merged.addArea(rowFrom, colFrom, rowTo, colTo);
+        merged.addArea(rowFrom, colFrom, rowTo, colTo);
+        return numMergedRegions++; 
     }
 
     public void removeMergedRegion(int index)
     {
-        merged.removeAreaAt(index);
+        //safety checks
-        if (merged.getNumAreas() == 0)
+        if (index >= numMergedRegions || mergedRecords.size() == 0)
+           return;
+            
+        int pos = 0;
+        int startNumRegions = 0;
+        
+        //optimisation for current record
+        if (numMergedRegions - index < merged.getNumAreas())
         {
-            merged = null;
+            pos = mergedRecords.size() - 1;
-            records.remove(mergedloc);
+            startNumRegions = numMergedRegions - merged.getNumAreas(); 
-            mergedloc = 0;
+        }
+        else
+        {
+            for (int n = 0; n < mergedRecords.size(); n++)
+            {
+                MergeCellsRecord record = (MergeCellsRecord) mergedRecords.get(n);
+                if (startNumRegions + record.getNumAreas() > index)
+                {
+                    pos = n;
+                    break;
+                }
+                startNumRegions += record.getNumAreas(); 
+            }
+        }
+
+        MergeCellsRecord rec = (MergeCellsRecord) mergedRecords.get(pos);
+        rec.removeAreaAt(index - startNumRegions);
+        numMergedRegions--;
+        if (rec.getNumAreas() == 0)
+        {
+            mergedRecords.remove(pos);
+            if (merged == rec)
+            	merged = (MergeCellsRecord) mergedRecords.get(mergedRecords.size() - 1);
+            int removePos = ((Integer) mergedLocs.get(pos)).intValue();
+            records.remove(removePos);
+            mergedLocs.remove(pos);
         }
     }
 
     public MergeCellsRecord.MergedRegion getMergedRegionAt(int index)
     {
-        return merged.getAreaAt(index);
+        //safety checks
+        if (index < numMergedRegions || mergedRecords.size() == 0)
+            return null;
+            
+        int pos = 0;
+        int startNumRegions = 0;
+        
+        //optimisation for current record
+        if (numMergedRegions - index < merged.getNumAreas())
+        {
+            pos = mergedRecords.size() - 1;
+            startNumRegions = numMergedRegions - merged.getNumAreas();
+        }
+        else
+        {
+            for (int n = 0; n < mergedRecords.size(); n++)
+            {
+                MergeCellsRecord record = (MergeCellsRecord) mergedRecords.get(n);
+                if (startNumRegions + record.getNumAreas() > index)
+                {
+                    pos = n;
+                    break;
+                }
+                startNumRegions += record.getNumAreas(); 
+            }
+        }
+        return ((MergeCellsRecord) mergedRecords.get(pos)).getAreaAt(index - startNumRegions);
     }
 
     public int getNumMergedRegions()
     {
-	    return merged!=null ? merged.getNumAreas() : 0;
+        return numMergedRegions;
     }
 
-
     /**
      * This is basically a kludge to deal with the now obsolete Label records.  If
      * you have to read in a sheet that contains Label records, be aware that the rest

src/testcases/org/apache/poi/hssf/model/SheetTest.java

@@ -0,0 +1,79 @@
+package org.apache.poi.hssf.model;
+
+import junit.framework.TestCase;
+
+/**
+ * @author Tony Poppleton
+ */
+public class SheetTest extends TestCase
+{
+	/**
+	 * Constructor for SheetTest.
+	 * @param arg0
+	 */
+	public SheetTest(String arg0)
+	{
+		super(arg0);
+	}
+	
+	public void testAddMergedRegion()
+	{
+		Sheet sheet = Sheet.createSheet();
+		int regionsToAdd = 4096;
+		int startRecords = sheet.getRecords().size();
+		
+		//simple test that adds a load of regions
+		for (int n = 0; n < regionsToAdd; n++)
+		{
+			int index = sheet.addMergedRegion(0, (short) 0, 1, (short) 1);
+			assertTrue("Merged region index expected to be " + n + " got " + index, index == n);
+		}
+		
+		//test all the regions were indeed added 
+		assertTrue(sheet.getNumMergedRegions() == regionsToAdd);
+		
+		//test that the regions were spread out over the appropriate number of records
+		int recordsAdded    = sheet.getRecords().size() - startRecords;
+		int recordsExpected = regionsToAdd/1027;
+		if ((regionsToAdd % 1027) != 0)
+			recordsExpected++;
+		assertTrue("The " + regionsToAdd + " merged regions should have been spread out over " + recordsExpected + " records, not " + recordsAdded, recordsAdded == recordsExpected);
+	}
+
+	public void testRemoveMergedRegion()
+	{
+		Sheet sheet = Sheet.createSheet();
+		int regionsToAdd = 4096;
+		
+		for (int n = 0; n < regionsToAdd; n++)
+			sheet.addMergedRegion(0, (short) 0, 1, (short) 1);
+			
+		int records = sheet.getRecords().size();
+		
+		//remove a third from the beginning
+		for (int n = 0; n < regionsToAdd/3; n++)
+		{
+			sheet.removeMergedRegion(0); 
+			//assert they have been deleted
+			assertTrue("Num of regions should be " + (regionsToAdd - n - 1) + " not " + sheet.getNumMergedRegions(), sheet.getNumMergedRegions() == regionsToAdd - n - 1);
+		}
+		
+		//assert any record removing was done
+		int recordsRemoved = (regionsToAdd/3)/1027; //doesn't work for particular values of regionsToAdd
+		assertTrue("Expected " + recordsRemoved + " record to be removed from the starting " + records + ".  Currently there are " + sheet.getRecords().size() + " records", records - sheet.getRecords().size() == recordsRemoved);
+	}
+
+	public void testGetMergedRegionAt()
+	{
+		//TODO
+	}
+
+	public void testGetNumMergedRegions()
+	{
+		//TODO
+	}
+
+}
+
+
+