Bug 57163 - Cannot delete an arbitrary sheet in an XLS workbook (only the last one)
Summary: Cannot delete an arbitrary sheet in an XLS workbook (only the last one)
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: HSSF (show other bugs)
Version: 3.11-dev
Hardware: PC All
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-10-29 11:57 UTC by Philippe Lhoste
Modified: 2015-10-27 09:13 UTC (History)
1 user (show)



Attachments
The Excel file used in the test (31.59 KB, application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
2014-10-31 11:28 UTC, Philippe Lhoste
Details
Show bugs (4.32 KB, text/x-java)
2014-10-31 11:29 UTC, Philippe Lhoste
Details
Proposed fix which adjusts the active sheet in setSheetOrder() and removeSheet() (15.58 KB, patch)
2014-11-09 22:20 UTC, Dominik Stadler
Details | Diff
Java file to test (448 bytes, text/x-java-source)
2015-04-06 13:28 UTC, Vladimir
Details
Excel sheet to test (6.50 KB, application/vnd.ms-excel)
2015-04-06 13:28 UTC, Vladimir
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Philippe Lhoste 2014-10-29 11:57:14 UTC
When I want to delete a sheet that's not the last one in a workbook with several sheets, I get an exception.

Exception in thread "main" java.lang.NullPointerException
	at org.apache.poi.hssf.model.InternalWorkbook.removeSheet(InternalWorkbook.java:780)
	at org.apache.poi.hssf.usermodel.HSSFWorkbook.removeSheetAt(HSSFWorkbook.java:924)

I open the file with:

try (InputStream in = new BufferedInputStream(new FileInputStream(new File(fileName))))
{
  return WorkbookFactory.create(in);
}

then I call workbook.removeSheetAt(0);

Apparently, the linkTable field is null.

My use case is to load a template with several sheets, delete them all but one to clone it to the number of sheets needed.
As a workaround, I move the wanted sheet at first position with:

workbook.setSheetOrder(workbook.getSheetName(sheetIndex), 0);

then I delete all the others in reverse order...
Comment 1 Nick Burch 2014-10-29 12:07:42 UTC
Can you attach a small file that shows this problem?
Comment 2 Philippe Lhoste 2014-10-31 11:28:27 UTC
Created attachment 32172 [details]
The Excel file used in the test
Comment 3 Philippe Lhoste 2014-10-31 11:29:28 UTC
Created attachment 32173 [details]
Show bugs
Comment 4 Philippe Lhoste 2014-10-31 11:33:37 UTC
Added a standalone class that loads the sample Excel file and reproduce the bugs (three bugs are shown, I numbered them with the Bugzilla id).
You might need to adjust the path to the sample.
Comment 5 Dominik Stadler 2014-11-09 22:20:33 UTC
Created attachment 32198 [details]
Proposed fix which adjusts the active sheet in setSheetOrder() and removeSheet()

FYI, I have a proposed fix for this and bug 57171, but would like to postpone checkin until after release 3.11 as it runs a slight risk of breaking stuff.
Comment 6 Dominik Stadler 2014-12-22 09:01:02 UTC
Fixed this and Bug 57163 via r1647264
Comment 7 Vladimir 2015-04-06 13:27:32 UTC
This bug is still occurs for XLS (HSSFWorkbook) in release POI 3.12 beta 1.
Problem in org.apache.poi.hssf.model.InternalWorkbook:780

public void removeSheet(int sheetIndex) {
    ....

    // also tell the LinkTable about the removed sheet
    // +1 because we already removed it from the count of sheets!
    for(int i = sheetIndex+1;i < getNumSheets()+1;i++) {
        // also update the link-table as otherwise references might point at invalid sheets
        linkTable.removeSheet(i);
    }
}

It is allowed linkTable field to be null!!!
Comment 8 Vladimir 2015-04-06 13:28:17 UTC
Created attachment 32633 [details]
Java file to test
Comment 9 Vladimir 2015-04-06 13:28:41 UTC
Created attachment 32634 [details]
Excel sheet to test
Comment 10 Vladimir 2015-04-06 13:32:36 UTC
Proposed fix (add check for linkTable):

public void removeSheet(int sheetIndex) {
    ....
    

    if (linkTable != null) {
        // also tell the LinkTable about the removed sheet
        // +1 because we already removed it from the count of sheets!
        for(int i = sheetIndex+1;i < getNumSheets()+1;i++) {
            // also update the link-table as otherwise references might point at invalid sheets
            linkTable.removeSheet(i);
        }
    }
}
Comment 11 Dominik Stadler 2015-10-27 09:13:42 UTC
This is actually fixed already since some time via r1673168, it seems we just missed to close this issue.