Bug 64516 - XSSFSheet.shiftRows has a bug when shifting rows affect the order of the rows.
Summary: XSSFSheet.shiftRows has a bug when shifting rows affect the order of the rows.
Status: NEW
Alias: None
Product: POI
Classification: Unclassified
Component: XSSF (show other bugs)
Version: 4.1.2-FINAL
Hardware: PC All
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-06-11 05:12 UTC by Axel Richter
Modified: 2020-06-11 09:24 UTC (History)
0 users



Attachments
Sample Excel file (9.15 KB, application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
2020-06-11 05:12 UTC, Axel Richter
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Axel Richter 2020-06-11 05:12:47 UTC
Created attachment 37303 [details]
Sample Excel file

In current Apache POI 4.1.2 XSSFSheet.shiftRows has a bug when shifting rows affect the order of the rows. For example if one shift one row down to the end (after the last row). So before shifting the sheet data conain:

<sheetData>
 <row r = "1"...
 <row r = "2"...
 <row r = "3"...
 <row r = "4"...
 <row r = "5"...
</sheetData>

Now we do:

XSSFSheet sheet...
...
sheet.shiftRows(2, 2, sheet.getLastRowNum() - rowNum + 1);

After that we get:

<sheetData>
 <row r = "1"...
 <row r = "2"...
 <row r = "6"...
 <row r = "4"...
 <row r = "5"...
</sheetData>

As you see, the formerly row r = "3" becaomes r = "6". But the order of sheet data row list is not changed. That leads to a corrupt file in Excel since row numbering and row order does not match. LibreOffice Calc is able handling that properly.

That is because XSSFSheet.rebuildRows only rebuild the _rows map but does not rebuild the underlying CTSheetData CTRow order.

Following complete example shows this. It also provides a rebuildRow method which provides rebuilding the underlying CTSheetData CTRow order too.

import org.apache.poi.xssf.usermodel.*;
import org.apache.poi.ss.usermodel.*;
import org.openxmlformats.schemas.spreadsheetml.x2006.main.*;

import java.io.*;
import java.lang.reflect.*;
import java.util.*;

class XSSFShiftRowsBug {

 static void rebuildRows(XSSFSheet sheet) throws Exception {
  //rebuild the CTSheetData CTRow order
  SortedMap<Long, CTRow> ctRows = new TreeMap<Long, CTRow>();
  CTSheetData sheetData = sheet.getCTWorksheet().getSheetData();
  for (CTRow ctRow : sheetData.getRowList()) {
   Long rownumL = ctRow.getR();
   ctRows.put(rownumL, ctRow);
  }
  List<CTRow> ctRowList = new ArrayList<CTRow>(ctRows.values());
  CTRow[] ctRowArray = new CTRow[ctRowList.size()];
  ctRowArray = ctRowList.toArray(ctRowArray);
  sheetData.setRowArray(ctRowArray);
 
  //rebuild the _rows map
  Field rows = XSSFSheet.class.getDeclaredField("_rows");
  rows.setAccessible(true);
  @SuppressWarnings("unchecked")
  SortedMap<Integer, XSSFRow> _rows = (SortedMap<Integer, XSSFRow>)rows.get(sheet);
  _rows.clear();
  Constructor rowConstructor = XSSFRow.class.getDeclaredConstructor(CTRow.class, XSSFSheet.class);
  rowConstructor.setAccessible(true);
  for (CTRow ctRow : sheetData.getRowList()) {
   XSSFRow row = (XSSFRow)rowConstructor.newInstance(ctRow, sheet);
   Integer rownumI = Math.toIntExact(row.getRowNum());
   _rows.put(rownumI, row);
  }
 }

 static void shiftRowToEnd(Sheet sheet, int rowNum) throws Exception {
  if (rowNum >= sheet.getLastRowNum()) return;
  sheet.shiftRows(rowNum, rowNum, sheet.getLastRowNum() - rowNum + 1, true, false);
  //if (sheet instanceof XSSFSheet) rebuildRows((XSSFSheet)sheet);
 }

 public static void main(String[] args) throws Exception { 

  InputStream inp = new FileInputStream("Test.xlsx"); String filePath = "Test_1.xlsx";
  Workbook workbook = WorkbookFactory.create(inp);

  Sheet sheet = workbook.getSheetAt(0);

  shiftRowToEnd(sheet, 2);

  FileOutputStream out = new FileOutputStream(filePath);
  workbook.write(out);
  out.close();
  workbook.close();

 }
}
Comment 1 PJ Fanning 2020-06-11 09:24:28 UTC
Thanks Alex. I merged using https://github.com/apache/poi/commit/2a8dfb353e7022ad4802b6c131e919cf20e1fcd5

One test was broken and has been disabled temporarily. I will try to look at the issue later (could be a problem in the test).