[patch] Progress towards a fix

bug56454.patch (text/plain), 15.56 KB, created by Javen O'Neal on 2015-12-06 21:29:06 UTC

(hide)

Description: Progress towards a fix

Filename:

MIME Type:

Creator: Javen O'Neal

Created: 2015-12-06 21:29:06 UTC

Size: 15.56 KB

patch

obsolete

>Index: src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/XSSFRowShifter.java >=================================================================== >--- src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/XSSFRowShifter.java (revision 1718223) >+++ src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/XSSFRowShifter.java (working copy) >@@ -18,9 +18,12 @@ > package org.apache.poi.xssf.usermodel.helpers; > > import java.util.ArrayList; >-import java.util.HashSet; >+import java.util.Collection; >+import java.util.Collections; > import java.util.List; >-import java.util.Set; >+import java.util.Map; >+import java.util.Map.Entry; >+import java.util.TreeMap; > > import org.apache.poi.ss.formula.FormulaParseException; > import org.apache.poi.ss.formula.FormulaParser; >@@ -71,59 +74,118 @@ > * @return an array of affected cell regions > */ > public List<CellRangeAddress> shiftMerged(int startRow, int endRow, int n) { >- List<CellRangeAddress> shiftedRegions = new ArrayList<CellRangeAddress>(); >- Set<Integer> removedIndices = new HashSet<Integer>(); >+ Map<Integer, CellRangeAddress> modifiedRegions = new TreeMap<Integer, CellRangeAddress>(); >+ List<CellRangeAddress> removedRegions = new ArrayList<CellRangeAddress>(); >+ Collection<Integer> modifiedIndices = new ArrayList<Integer>(); //overwritten >+ >+ if (n == 0) { >+ return Collections.emptyList(); >+ } >+ >+ int destStartRow = startRow + n; >+ int destEndRow = endRow + n; >+ > //move merged regions completely if they fall within the new region boundaries when they are shifted >- int size = sheet.getNumMergedRegions(); >- for (int i = 0; i < size; i++) { >+ final int numMergedRegions = sheet.getNumMergedRegions(); >+ for (int i = 0; i < numMergedRegions; 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) { >+ >+ // Behavior: Unaffected >+ // Condition: if shift source and destination rows do not overlap with merged region >+ boolean mergedRegionNotInSourceRows = (endRow < merged.getFirstRow() || merged.getLastRow() < startRow); >+ boolean mergedRegionNotInDestRows = (destEndRow < merged.getFirstRow() || merged.getLastRow() < destStartRow); >+ if (mergedRegionNotInSourceRows && mergedRegionNotInDestRows) { > 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)) { >+ // Condition: or if source and destination rows fully inside merged region >+ boolean sourceRowsInMergedRegion = (merged.getFirstRow() < startRow && endRow < merged.getLastRow()); >+ boolean destRowsInMergedRegion = (merged.getFirstRow() < destStartRow && destEndRow < merged.getLastRow()); >+ if (sourceRowsInMergedRegion && destRowsInMergedRegion) { >+ continue; >+ } >+ >+ // CellRangeAddresses cannot be modified in place. >+ // At this point, any CellRangeAddress will be modified or removed. >+ modifiedIndices.add(i); >+ >+ // Behavior: Merged region gets deleted >+ // Condition: if merged region is outside shift source rows but fully or partially contained within shift destination rows >+ // Condition: if destination rows contain merged regions, merged regions would be shrunk or broken up. Remove them if there's any conflict. >+ boolean mergedRegionInDestRows = (destStartRow <= merged.getLastRow() && merged.getFirstRow() <= destEndRow); >+ if (mergedRegionInDestRows) { >+ removedRegions.add(i, merged); >+ continue; >+ } >+ >+ // At this point, any CellRangeAddress will be modified. >+ modifiedRegions.put(i, merged); >+ >+ // Behavior: Entire merged region gets shifted down >+ // Condition: if shift source rows fully contain the merged region >+ if (startRow <= merged.getFirstRow() && merged.getLastRow() <= endRow) { > merged.setFirstRow(merged.getFirstRow() + n); > merged.setLastRow(merged.getLastRow() + n); >- //have to remove/add it back >- shiftedRegions.add(merged); >- removedIndices.add(i); >+ continue; > } >+ >+ // Behavior: Merged region gets elongated >+ // Condition: if startRow is inside the merged region and not the first row of the region (which would result in a shift) and n>0 >+ if (n > 0) { >+ boolean startRowInMergedRegion = merged.getFirstRow() < startRow && startRow <= merged.getLastRow(); >+ boolean endRowAfterMergedRegion = merged.getLastRow() <= endRow; >+ if (startRowInMergedRegion && endRowAfterMergedRegion) { >+ merged.setLastRow(merged.getLastRow() + n); >+ continue; >+ } >+ } >+ else { >+ // Condition: if endRow is inside the merged region and not the last row of the region (which would result in a shift) and n>0 >+ boolean endRowInMergedRegion = merged.getFirstRow() <= endRow && endRow < merged.getLastRow(); >+ boolean startRowBeforeMergedRegion = startRow <= merged.getFirstRow(); >+ if (endRowInMergedRegion && startRowBeforeMergedRegion) { >+ merged.setFirstRow(merged.getFirstRow() + n); >+ continue; >+ } >+ } >+ >+ // Behavior: Merged region gets shrunk >+ // Condition: startRow is moved down or endRow is moved up >+ boolean mergedRegionShrunkFromTop = (startRow == merged.getFirstRow() && merged.getLastRow() <= destEndRow); >+ if (mergedRegionShrunkFromTop) { >+ merged.setFirstRow(destStartRow); >+ continue; >+ } >+ boolean mergedRegionShrunkFromBottom = (destStartRow <= merged.getLastRow() && merged.getLastRow() == endRow); >+ if (mergedRegionShrunkFromBottom) { >+ merged.setLastRow(destEndRow); >+ continue; >+ } > } > >- if(!removedIndices.isEmpty()) { >- sheet.removeMergedRegions(removedIndices); >+ // probably need to replace all merged regions atomically and only verify if there are >+ // overlapping merged regions after all replacements and removals are done, then either >+ // roll back all changes and throw an exception if there are overlapping merged regions >+ // or delete the merged regions that are invalidated due to the shift >+ for (Entry<Integer, CellRangeAddress> entry : modifiedRegions.entrySet()) { >+ sheet.replaceMergedRegion(entry.getKey(), entry.getValue()); > } >+ >+ // Remove all merged regions that were modified or deleted >+ if(!modifiedIndices.isEmpty()) { >+ sheet.removeMergedRegions(modifiedIndices); >+ } > >- //read so it doesn't get shifted again >- for (CellRangeAddress region : shiftedRegions) { >+ /*// Add back merged regions that were modified >+ for (CellRangeAddress region : modifiedRegions) { > sheet.addMergedRegion(region); >- } >- return shiftedRegions; >+ }*/ >+ // Include deleted regions in the return value >+ List<CellRangeAddress> affectedRegions = new ArrayList<CellRangeAddress>(modifiedRegions.values()); >+ affectedRegions.addAll(removedRegions); >+ return affectedRegions; > } > > /** >- * 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) { >Index: src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java >=================================================================== >--- src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java (revision 1718223) >+++ src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java (working copy) >@@ -396,6 +396,24 @@ > } > } > } >+ >+ /** >+ * Verify that candidate region does not intersect with an existing, non-ignored merged region in this sheet >+ * >+ * @param candidateRegion >+ * @param indexToIgnore the index of the merged region to ignore >+ * @throws IllegalStateException if candidate region intersects an existing, non-ignored merged region in this sheet >+ */ >+ private void validateMergedRegions(CellRangeAddress candidateRegion, int indexToIgnore) { >+ List<CellRangeAddress> mergedRegions = getMergedRegions(); >+ mergedRegions.remove(indexToIgnore); >+ for (final CellRangeAddress existingRegion : getMergedRegions()) { >+ if (existingRegion.intersects(candidateRegion)) { >+ throw new IllegalStateException("Cannot add merged region " + candidateRegion.formatAsString() + >+ " to sheet because it overlaps with an existing merged region (" + existingRegion.formatAsString() + ")."); >+ } >+ } >+ } > > /** > * Adjusts the column width to fit the contents. >@@ -1154,6 +1172,8 @@ > * Returns the merged region at the specified index. If you want multiple > * regions, it is faster to call {@link #getMergedRegions()} than to call > * this each time. >+ * Modifying the returned merged region will not affect the merged regions in the sheet. >+ * FIXME: Perhaps this should return an UnmodifiableCellRangeAddress to eliminate confusion? > * > * @return the merged region at the specified index > */ >@@ -1751,7 +1771,7 @@ > worksheet.unsetMergeCells(); > } > } >- >+ > /** > * Removes a number of merged regions of cells (hence letting them free) > * >@@ -1761,7 +1781,6 @@ > * > * @param indices A set of the regions to unmerge > */ >- @SuppressWarnings("deprecation") > public void removeMergedRegions(Collection<Integer> indices) { > if (!worksheet.isSetMergeCells()) return; > >@@ -1769,7 +1788,7 @@ > List<CTMergeCell> newMergeCells = new ArrayList<CTMergeCell>(ctMergeCells.sizeOfMergeCellArray()); > > int idx = 0; >- for (CTMergeCell mc : ctMergeCells.getMergeCellArray()) { >+ for (CTMergeCell mc : ctMergeCells.getMergeCellList()) { > if (!indices.contains(idx++)) newMergeCells.add(mc); > } > >@@ -1780,6 +1799,39 @@ > ctMergeCells.setMergeCellArray(newMergeCells.toArray(newMergeCellsArray)); > } > } >+ >+ /** >+ * Replace a merged region with a different merged region. >+ * This is faster than calling <code>removeMergedRegion(int)</code> >+ * and <code>addMergedRegion(CellRangeAddress)</code> >+ * >+ * @param index of the region to unmerge >+ * @param >+ */ >+ //@Override >+ public void replaceMergedRegion(int index, CellRangeAddress region) { >+ region.validate(SpreadsheetVersion.EXCEL2007); >+ >+ // throw IllegalStateException if the argument CellRangeAddress intersects with >+ // a multi-cell array formula defined in this sheet >+ validateArrayFormulas(region); >+ >+ if (!worksheet.isSetMergeCells()) { >+ throw new IllegalStateException("Worksheet has no merged cells to replace."); >+ } >+ >+ CTMergeCells ctMergeCells = worksheet.getMergeCells(); >+ int size = ctMergeCells.sizeOfMergeCellArray(); >+ assert(0 <= index && index < size); >+ CTMergeCell ctMergeCell = ctMergeCells.getMergeCellArray(index); >+ >+ // Throw IllegalStateException if the argument CellRangeAddress intersects with >+ // a merged region already in this sheet >+ validateMergedRegions(region, index); >+ >+ String ref = region.formatAsString(); >+ ctMergeCell.setRef(ref); >+ } > > /** > * Remove a row from this sheet. All cells contained in the row are removed as well >Index: src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java >=================================================================== >--- src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java (revision 1718223) >+++ src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java (working copy) >@@ -1385,8 +1385,8 @@ > } > > //only shift if the region outside the shifted rows is not merged too >- if (!SheetUtil.containsCell(merged, startRow - 1, 0) && >- !SheetUtil.containsCell(merged, endRow + 1, 0)) { >+ if (!merged.containsRow(startRow - 1) && >+ !merged.containsRow(endRow + 1)) { > merged.setFirstRow(merged.getFirstRow() + n); > merged.setLastRow(merged.getLastRow() + n); > //have to remove/add it back >Index: src/java/org/apache/poi/ss/util/SheetUtil.java >=================================================================== >--- src/java/org/apache/poi/ss/util/SheetUtil.java (revision 1718223) >+++ src/java/org/apache/poi/ss/util/SheetUtil.java (working copy) >@@ -325,6 +325,7 @@ > * @param rowIx the row to check > * @param colIx the column to check > * @return true if the range contains the cell [rowIx, colIx] >+ * @deprecated 3.14beta2. Use {@link CellRangeAddressBase#isInRange(int, int)} instead. > */ > public static boolean containsCell(CellRangeAddress cr, int rowIx, int colIx) { > return cr.isInRange(rowIx, colIx); >Index: src/java/org/apache/poi/ss/util/CellRangeAddressBase.java >=================================================================== >--- src/java/org/apache/poi/ss/util/CellRangeAddressBase.java (revision 1718223) >+++ src/java/org/apache/poi/ss/util/CellRangeAddressBase.java (working copy) >@@ -122,11 +122,30 @@ > * @return True if the coordinates lie within the bounds, false otherwise. > */ > public boolean isInRange(int rowInd, int colInd) { >- return _firstRow <= rowInd && rowInd <= _lastRow && >- _firstCol <= colInd && colInd <= _lastCol; >+ return containsRow(rowInd) && containsColumn(colInd); > } > > /** >+ * Check if the row is in the specified cell range >+ * >+ * @param rowInd the row to check >+ * @return true if the range contains the row [rowInd] >+ */ >+ public boolean containsRow(int rowInd) { >+ return _firstRow <= rowInd && rowInd <= _lastRow; >+ } >+ >+ /** >+ * Check if the column is in the specified cell range >+ * >+ * @param colInd the column to check >+ * @return true if the range contains the column [colInd] >+ */ >+ public boolean containsColumn(int colInd) { >+ return _firstCol <= colInd && colInd <= _lastCol; >+ } >+ >+ /** > * Determines whether or not this CellRangeAddress and the specified CellRangeAddress intersect. > * > * @param other a candidate cell range address to check for intersection with this range View Attachment As Diff View Attachment As Raw

This is ASF Bugzilla: the Apache Software Foundation bug system. In case of problems with the functioning of ASF Bugzilla, please contact bugzilla-admin@apache.org. Please Note: this e-mail address is only for reporting problems with ASF Bugzilla. Mail about any other subject will be silently ignored.

• • Home
  • | New
  • | Browse
  • | Search
  • |
    [?]
  • | Reports
  • | Help
  • | New Account
  • | Log In
    Remember [x]
  • | Forgot Password
    Login: [x]