From 9c9e919a7f4c7244a1b37398924e6c43ab726694 Mon Sep 17 00:00:00 2001 From: Avik Sengupta Date: Sat, 1 Mar 2003 17:56:55 +0000 Subject: [PATCH] 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 | 97 +++++++++++++++---- .../org/apache/poi/hssf/model/SheetTest.java | 79 +++++++++++++++ 2 files changed, 159 insertions(+), 17 deletions(-) create mode 100644 src/testcases/org/apache/poi/hssf/model/SheetTest.java diff --git a/src/java/org/apache/poi/hssf/model/Sheet.java b/src/java/org/apache/poi/hssf/model/Sheet.java index 475d2cfc3f..555232ae62 100644 --- a/src/java/org/apache/poi/hssf/model/Sheet.java +++ b/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.merged = ( MergeCellsRecord ) rec; - retval.mergedloc = k - offset; + retval.mergedRecords.add(rec); + retval.merged = ( MergeCellsRecord ) rec; + retval.mergedLocs.add(new Integer(k - offset)); + retval.numMergedRegions += retval.merged.getNumAreas(); } else if (rec.getSid() == ColumnInfoRecord.sid) { @@ -421,36 +425,95 @@ 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; + merged = ( MergeCellsRecord ) createMergedCells(); + 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); - if (merged.getNumAreas() == 0) + //safety checks + if (index >= numMergedRegions || mergedRecords.size() == 0) + return; + + int pos = 0; + int startNumRegions = 0; + + //optimisation for current record + if (numMergedRegions - index < merged.getNumAreas()) { - merged = null; - records.remove(mergedloc); - mergedloc = 0; + 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(); + } + } + + 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 diff --git a/src/testcases/org/apache/poi/hssf/model/SheetTest.java b/src/testcases/org/apache/poi/hssf/model/SheetTest.java new file mode 100644 index 0000000000..28d2d13b37 --- /dev/null +++ b/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 + } + +} + + +