Browse Source

Fix 60384 and 60709: When shifting with merged regions we should correctly check if the region is moved along or needs to be removed because it is not fully kept via the shifting. This was broken by the fix for bug 59740, now additional unit tests ensure that it works better.

pull/1600/head
Antony Liu 3 weeks ago
parent
commit
e4d58dc7ed
  1. 70
      main/SS/UserModel/Helpers/RowShifter.cs
  2. 57
      testcases/main/SS/UserModel/BaseTestSheetShiftRows.cs
  3. 25
      testcases/ooxml/XSSF/UserModel/TestXSSFSheet.cs
  4. 60
      testcases/ooxml/XSSF/UserModel/TestXSSFSheetShiftRows.cs
  5. BIN
      testcases/test-data/spreadsheet/60384.xlsx
  6. BIN
      testcases/test-data/spreadsheet/60709.xlsx

70
main/SS/UserModel/Helpers/RowShifter.cs

@ -20,6 +20,7 @@ using NPOI.SS.Util;
using System;
using System.Collections.Generic;
using System.Linq;
using static ICSharpCode.SharpZipLib.Zip.FastZip;
namespace NPOI.SS.UserModel.Helpers
{
@ -47,7 +48,7 @@ namespace NPOI.SS.UserModel.Helpers
/// <returns>an array of affected merged regions, doesn't contain deleted ones</returns>
public List<CellRangeAddress> ShiftMergedRegions(int startRow, int endRow, int n)
{
List<CellRangeAddress> ShiftedRegions = [];
List<CellRangeAddress> shiftedRegions = [];
HashSet<int> removedIndices = [];
var size = sheet.NumMergedRegions;
@ -56,43 +57,28 @@ namespace NPOI.SS.UserModel.Helpers
var merged = sheet.GetMergedRegion(i);
//Shift if the merged region inside the Shifting rows
if (merged.FirstRow >= startRow && merged.LastRow <= endRow)
if (RemovalNeeded(merged, startRow, endRow, n))
{
merged.FirstRow += n;
merged.LastRow += n;
//have to Remove/add it back
ShiftedRegions.Add(merged);
removedIndices.Add(i);
continue;
}
//don't check if it's not within the Shifted area
if (n > 0)
{
// area is moved down
if (merged.LastRow < startRow
|| merged.FirstRow > endRow + n
|| merged.FirstRow > endRow && merged.LastRow < startRow + n
)
{
continue;
}
}
else
{
// area is moved up
if (merged.LastRow < startRow + n
|| merged.FirstRow > endRow
|| merged.FirstRow > endRow + n && merged.LastRow < startRow
)
{
continue;
}
bool inStart = (merged.FirstRow >= startRow || merged.LastRow >= startRow);
bool inEnd = (merged.FirstRow <= endRow || merged.LastRow <= endRow);
//don't check if it's not within the shifted area
if (!inStart || !inEnd) {
continue;
}
//remove merged region that overlaps Shifting
removedIndices.Add(i);
//only shift if the region outside the shifted rows is not merged too
if (!merged.ContainsRow(startRow - 1) && !merged.ContainsRow(endRow + 1)) {
merged.FirstRow = merged.FirstRow + n;
merged.LastRow = merged.LastRow + n;
//have to remove/add it back
shiftedRegions.Add(merged);
removedIndices.Add(i);
}
}
if (removedIndices.Count != 0)
@ -101,14 +87,32 @@ namespace NPOI.SS.UserModel.Helpers
}
//add it which is within the shifted area back
foreach (var region in ShiftedRegions)
foreach (var region in shiftedRegions)
{
sheet.AddMergedRegion(region);
}
return ShiftedRegions;
return shiftedRegions;
}
private static bool RemovalNeeded(CellRangeAddress merged, int startRow, int endRow, int n)
{
int movedRows = endRow - startRow + 1;
// build a range of the rows that are overwritten, i.e. the target-area, but without
// rows that are moved along
CellRangeAddress overwrite;
if(n > 0) {
// area is moved down => overwritten area is [endRow + n - movedRows, endRow + n]
overwrite = new CellRangeAddress(Math.Max(endRow + 1, endRow + n - movedRows), endRow + n, 0, 0);
} else {
// area is moved up => overwritten area is [startRow + n, startRow + n + movedRows]
overwrite = new CellRangeAddress(startRow + n, Math.Min(startRow - 1, startRow + n + movedRows), 0, 0);
}
// if the merged-region and the overwritten area intersect, we need to remove it
return merged.Intersects(overwrite);
}
/// <summary>
/// Verify that the given column indices and step denote a valid range of columns to shift
/// </summary>

57
testcases/main/SS/UserModel/BaseTestSheetShiftRows.cs

@ -700,7 +700,7 @@ namespace TestCases.SS.UserModel
{
IWorkbook wb = _testDataProvider.CreateWorkbook();
ISheet sheet = wb.CreateSheet("test");
PopulateSheetCells(sheet);
PopulateSheetCells(sheet, 2);
CellRangeAddress A1_E1 = new CellRangeAddress(0, 0, 0, 4);
CellRangeAddress A2_C2 = new CellRangeAddress(1, 1, 0, 2);
sheet.AddMergedRegion(A1_E1);
@ -712,10 +712,59 @@ namespace TestCases.SS.UserModel
ClassicAssert.AreEqual(CellRangeAddress.ValueOf("A1:C1"), sheet.GetMergedRegion(0));
wb.Close();
}
private void PopulateSheetCells(ISheet sheet)
[Test]
public void ShiftMergedRowsToMergedRowsOverlappingMergedRegion()
{
IWorkbook wb = _testDataProvider.CreateWorkbook();
ISheet sheet = wb.CreateSheet("test");
PopulateSheetCells(sheet, 10);
CellRangeAddress A1_E1 = new CellRangeAddress(0, 0, 0, 4);
CellRangeAddress A2_C2 = new CellRangeAddress(1, 7, 0, 2);
sheet.AddMergedRegion(A1_E1);
sheet.AddMergedRegion(A2_C2);
// A1:E1 should Move to A5:E5
// A2:C2 should be removed
sheet.ShiftRows(0, 0, 4);
ClassicAssert.AreEqual(1, sheet.NumMergedRegions);
ClassicAssert.AreEqual(CellRangeAddress.ValueOf("A5:E5"), sheet.GetMergedRegion(0));
wb.Close();
}
[Test]
public void Bug60384ShiftMergedRegion()
{
IWorkbook wb = _testDataProvider.CreateWorkbook();
ISheet sheet = wb.CreateSheet("test");
PopulateSheetCells(sheet, 9);
CellRangeAddress A8_E8 = new CellRangeAddress(7, 7, 0, 4);
CellRangeAddress A9_C9 = new CellRangeAddress(8, 8, 0, 2);
sheet.AddMergedRegion(A8_E8);
sheet.AddMergedRegion(A9_C9);
// A1:E1 should be removed
// A2:C2 will be A1:C1
sheet.ShiftRows(3, sheet.LastRowNum, 1);
ClassicAssert.AreEqual(2, sheet.NumMergedRegions);
ClassicAssert.AreEqual(CellRangeAddress.ValueOf("A9:E9"), sheet.GetMergedRegion(0));
ClassicAssert.AreEqual(CellRangeAddress.ValueOf("A10:C10"), sheet.GetMergedRegion(1));
wb.Close();
}
private void PopulateSheetCells(ISheet sheet, int rowCount)
{
// populate sheet cells
for (int i = 0; i < 2; i++)
for (int i = 0; i < rowCount; i++)
{
IRow row = sheet.CreateRow(i);
for (int j = 0; j < 5; j++)
@ -731,7 +780,7 @@ namespace TestCases.SS.UserModel
IWorkbook wb = _testDataProvider.CreateWorkbook();
ISheet sheet = wb.CreateSheet("test");
// populate sheet cells
PopulateSheetCells(sheet);
PopulateSheetCells(sheet, 2);
CellRangeAddress A1_E1 = new CellRangeAddress(0, 0, 0, 4);
CellRangeAddress A2_C2 = new CellRangeAddress(1, 1, 0, 2);
sheet.AddMergedRegion(A1_E1);

25
testcases/ooxml/XSSF/UserModel/TestXSSFSheet.cs

@ -140,22 +140,25 @@ namespace TestCases.XSSF.UserModel
ClassicAssert.True(sheet.MergedRegions.Any(r => r.FormatAsString().Equals("F5:G6")));
ClassicAssert.IsNull(sheet.GetRow(6).GetCell(7));
ClassicAssert.False(sheet.MergedRegions.Any(r => r.FormatAsString().Equals("H7:I8")));
//TODO: Cell Range H7:I8 was replaced by H4:I4,not merged, this assert shoud be false
bool merged = sheet.MergedRegions.Any(r => r.FormatAsString().Equals("H7:I8"));
Assume.That(merged, Is.False);
ClassicAssert.True(merged);
ClassicAssert.AreEqual("regionOutsideShiftedRowsBelow", sheet.GetRow(10).GetCell(9).StringCellValue);
ClassicAssert.True(sheet.MergedRegions.Any(r => r.FormatAsString().Equals("J11:K12")));
ClassicAssert.AreEqual("regionThatEndsWithinShiftedRows", sheet.GetRow(1).GetCell(11).StringCellValue);
ClassicAssert.False(sheet.MergedRegions.Any(r => r.FormatAsString().Equals("L2:M3")));
ClassicAssert.True(sheet.MergedRegions.Any(r => r.FormatAsString().Equals("L2:M3")));
ClassicAssert.AreEqual("regionThatEndsOnLastShiftedRow", sheet.GetRow(1).GetCell(13).StringCellValue);
ClassicAssert.False(sheet.MergedRegions.Any(r => r.FormatAsString().Equals("N2:O4")));
ClassicAssert.True(sheet.MergedRegions.Any(r => r.FormatAsString().Equals("N2:O4")));
ClassicAssert.AreEqual("regionThatEndsOutsideShiftedRows", sheet.GetRow(1).GetCell(15).StringCellValue);
ClassicAssert.False(sheet.MergedRegions.Any(r => r.FormatAsString().Equals("P2:Q5")));
ClassicAssert.True(sheet.MergedRegions.Any(r => r.FormatAsString().Equals("P2:Q5")));
ClassicAssert.AreEqual("reallyLongRegion", sheet.GetRow(1).GetCell(17).StringCellValue);
ClassicAssert.False(sheet.MergedRegions.Any(r => r.FormatAsString().Equals("R2:S12")));
ClassicAssert.True(sheet.MergedRegions.Any(r => r.FormatAsString().Equals("R2:S12")));
FileInfo file = TempFile.CreateTempFile("ShiftRows-", ".xlsx");
Stream output = File.OpenWrite(file.FullName);
@ -175,22 +178,24 @@ namespace TestCases.XSSF.UserModel
ClassicAssert.True(sheetLoaded.MergedRegions.Any(r => r.FormatAsString().Equals("F5:G6")));
ClassicAssert.IsNull(sheetLoaded.GetRow(6).GetCell(7));
ClassicAssert.False(sheetLoaded.MergedRegions.Any(r => r.FormatAsString().Equals("H7:I8")));
merged = sheetLoaded.MergedRegions.Any(r => r.FormatAsString().Equals("H7:I8"));
Assume.That(merged, Is.False);
ClassicAssert.True(merged);
ClassicAssert.AreEqual("regionOutsideShiftedRowsBelow", sheetLoaded.GetRow(10).GetCell(9).StringCellValue);
ClassicAssert.True(sheetLoaded.MergedRegions.Any(r => r.FormatAsString().Equals("J11:K12")));
ClassicAssert.AreEqual("regionThatEndsWithinShiftedRows", sheetLoaded.GetRow(1).GetCell(11).StringCellValue);
ClassicAssert.False(sheetLoaded.MergedRegions.Any(r => r.FormatAsString().Equals("L2:M3")));
ClassicAssert.True(sheetLoaded.MergedRegions.Any(r => r.FormatAsString().Equals("L2:M3")));
ClassicAssert.AreEqual("regionThatEndsOnLastShiftedRow", sheetLoaded.GetRow(1).GetCell(13).StringCellValue);
ClassicAssert.False(sheetLoaded.MergedRegions.Any(r => r.FormatAsString().Equals("N2:O4")));
ClassicAssert.True(sheetLoaded.MergedRegions.Any(r => r.FormatAsString().Equals("N2:O4")));
ClassicAssert.AreEqual("regionThatEndsOutsideShiftedRows", sheetLoaded.GetRow(1).GetCell(15).StringCellValue);
ClassicAssert.False(sheetLoaded.MergedRegions.Any(r => r.FormatAsString().Equals("P2:Q5")));
ClassicAssert.True(sheetLoaded.MergedRegions.Any(r => r.FormatAsString().Equals("P2:Q5")));
ClassicAssert.AreEqual("reallyLongRegion", sheetLoaded.GetRow(1).GetCell(17).StringCellValue);
ClassicAssert.False(sheetLoaded.MergedRegions.Any(r => r.FormatAsString().Equals("R2:S12")));
ClassicAssert.True(sheetLoaded.MergedRegions.Any(r => r.FormatAsString().Equals("R2:S12")));
}
[Test]

60
testcases/ooxml/XSSF/UserModel/TestXSSFSheetShiftRows.cs

@ -473,5 +473,65 @@ namespace TestCases.XSSF.UserModel
sheet.ShiftRows(1, 2, 3);
IOUtils.CloseQuietly(wb);
}
[Test]
public void Test60384()
{
XSSFWorkbook wb = XSSFTestDataSamples.OpenSampleWorkbook("60384.xlsx");
XSSFSheet sheet = wb.GetSheetAt(0) as XSSFSheet;
ClassicAssert.AreEqual(2, sheet.MergedRegions.Count);
ClassicAssert.AreEqual(7, sheet.GetMergedRegion(0).FirstRow);
ClassicAssert.AreEqual(7, sheet.GetMergedRegion(0).LastRow);
ClassicAssert.AreEqual(8, sheet.GetMergedRegion(1).FirstRow);
ClassicAssert.AreEqual(8, sheet.GetMergedRegion(1).LastRow);
sheet.ShiftRows(3, 8, 1);
// After shifting, the two named regions should still be there as they
// are fully inside the shifted area
ClassicAssert.AreEqual(2, sheet.MergedRegions.Count);
ClassicAssert.AreEqual(8, sheet.GetMergedRegion(0).FirstRow);
ClassicAssert.AreEqual(8, sheet.GetMergedRegion(0).LastRow);
ClassicAssert.AreEqual(9, sheet.GetMergedRegion(1).FirstRow);
ClassicAssert.AreEqual(9, sheet.GetMergedRegion(1).LastRow);
/*OutputStream out = new FileOutputStream("/tmp/60384.xlsx");
try {
wb.write(out);
} finally {
out.Close();
}*/
wb.Close();
}
[Test]
public void Test60709()
{
XSSFWorkbook wb = XSSFTestDataSamples.OpenSampleWorkbook("60709.xlsx");
XSSFSheet sheet = wb.GetSheetAt(0) as XSSFSheet;
ClassicAssert.AreEqual(1, sheet.MergedRegions.Count);
ClassicAssert.AreEqual(2, sheet.GetMergedRegion(0).FirstRow);
ClassicAssert.AreEqual(2, sheet.GetMergedRegion(0).LastRow);
sheet.ShiftRows(1, sheet.LastRowNum+1, -1, true, false);
// After shifting, the two named regions should still be there as they
// are fully inside the shifted area
ClassicAssert.AreEqual(1, sheet.MergedRegions.Count);
ClassicAssert.AreEqual(1, sheet.GetMergedRegion(0).FirstRow);
ClassicAssert.AreEqual(1, sheet.GetMergedRegion(0).LastRow);
/*OutputStream out = new FileOutputStream("/tmp/60709.xlsx");
try {
wb.write(out);
} finally {
out.Close();
}*/
wb.Close();
}
}
}

BIN
testcases/test-data/spreadsheet/60384.xlsx

BIN
testcases/test-data/spreadsheet/60709.xlsx

Loading…
Cancel
Save