fix: fixed NPE exception in the WriteSheetWorkbookWriteHandler class (#566)

* fix: ensure sheetNo is set for writeSheetHolder and improve sheet indexing

* test: add test cases for WriteSheet with sheet name

---------

Co-authored-by: ian zhang <[email protected]>
Co-authored-by: Shuxin Pan <[email protected]>

Author: 3 people

fastexcel/src/main/java/cn/idev/excel/context/WriteContextImpl.java

@@ -233,6 +233,11 @@ private void initSheet(WriteTypeEnum writeType) {
         if (currentSheet == null) {
             currentSheet = createSheet();
         }
+
+        if (currentSheet != null && writeSheetHolder.getSheetNo() == null) {
+            writeSheetHolder.setSheetNo(writeWorkbookHolder.getWorkbook().getSheetIndex(currentSheet));
+        }
+
         writeSheetHolder.setSheet(currentSheet);
         WriteHandlerUtils.afterSheetCreate(sheetWriteHandlerContext);
         if (WriteTypeEnum.ADD.equals(writeType)) {

fastexcel/src/main/java/cn/idev/excel/write/handler/impl/WriteSheetWorkbookWriteHandler.java

@@ -7,6 +7,7 @@
 import cn.idev.excel.write.metadata.holder.WriteWorkbookHolder;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.stream.Collectors;
 import org.apache.commons.collections4.MapUtils;
 import org.apache.poi.ss.usermodel.Workbook;
@@ -32,8 +33,10 @@ public void afterWorkbookDispose(WriteWorkbookHolder writeWorkbookHolder) {
         }
         Workbook workbook = writeWorkbookHolder.getWorkbook();
         // sort by sheetNo.
-        List<Integer> sheetNoSortList =
-                writeSheetHolderMap.keySet().stream().sorted().collect(Collectors.toList());
+        List<Integer> sheetNoSortList = writeSheetHolderMap.keySet().stream()
+                .filter(Objects::nonNull)
+                .sorted()
+                .collect(Collectors.toList());
 
         int pos = 0;
         for (Integer key : sheetNoSortList) {

fastexcel/src/test/java/cn/idev/excel/writesheet/WriteSheetTest.java

@@ -4,6 +4,7 @@
 import cn.idev.excel.FastExcel;
 import cn.idev.excel.support.ExcelTypeEnum;
 import cn.idev.excel.util.TestFileUtil;
+import cn.idev.excel.write.metadata.WriteSheet;
 import java.io.File;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -28,6 +29,8 @@ public void testSheetOrder03() {
         testSheetOrderInternal(ExcelTypeEnum.XLS, Arrays.asList(-1));
         testSheetOrderInternal(ExcelTypeEnum.XLS, Arrays.asList(-8, -10, -6));
         testSheetOrderInternal(ExcelTypeEnum.XLS, Arrays.asList(-8, 6));
+        // build a WriteSheet using the sheet name
+        testSheetOrderWithSheetName(ExcelTypeEnum.XLS);
     }
 
     @Test
@@ -42,6 +45,8 @@ public void testSheetOrder07() {
         testSheetOrderInternal(ExcelTypeEnum.XLSX, Arrays.asList(-1));
         testSheetOrderInternal(ExcelTypeEnum.XLSX, Arrays.asList(-8, -10, -6));
         testSheetOrderInternal(ExcelTypeEnum.XLSX, Arrays.asList(-8, 6));
+        // build a WriteSheet using the sheet name
+        testSheetOrderWithSheetName(ExcelTypeEnum.XLSX);
     }
 
     private void testSheetOrderInternal(ExcelTypeEnum excelTypeEnum, List<Integer> sheetNoList) {
@@ -81,6 +86,49 @@ private Map<Integer, Integer> initSheetDataSizeList(List<Integer> sheetNoList) {
         return dataMap;
     }
 
+    private void testSheetOrderWithSheetName(ExcelTypeEnum excelTypeEnum) {
+        List<String> sheetNameList = Arrays.asList("Sheet1", "Sheet2", "Sheet3");
+        List<Integer> sheetNoList = Arrays.asList(0, 1, 2);
+
+        Map<Integer, Integer> dataMap = initSheetDataSizeList(sheetNoList);
+        File testFile = TestFileUtil.createNewFile("writesheet/write-sheet-order-name" + excelTypeEnum.getValue());
+
+        try (ExcelWriter excelWriter = FastExcel.write(testFile, WriteSheetData.class)
+                .excelType(excelTypeEnum)
+                .build()) {
+
+            // sheetName is empty
+            int sheetNo = 0;
+            WriteSheet writeSheet = FastExcel.writerSheet(sheetNo).build();
+            excelWriter.write(dataList(dataMap.get(sheetNo)), writeSheet);
+            Assertions.assertEquals(
+                    sheetNo, excelWriter.writeContext().writeSheetHolder().getSheetNo());
+
+            // sheetNo is empty
+            sheetNo = 1;
+            writeSheet = FastExcel.writerSheet(sheetNameList.get(sheetNo)).build();
+            excelWriter.write(dataList(dataMap.get(sheetNo)), writeSheet);
+            Assertions.assertEquals(
+                    sheetNo, excelWriter.writeContext().writeSheetHolder().getSheetNo());
+
+            sheetNo = 2;
+            writeSheet =
+                    FastExcel.writerSheet(sheetNo, sheetNameList.get(sheetNo)).build();
+            excelWriter.write(dataList(dataMap.get(sheetNo)), writeSheet);
+            Assertions.assertEquals(
+                    sheetNo, excelWriter.writeContext().writeSheetHolder().getSheetNo());
+        }
+
+        for (int i = 0; i < sheetNoList.size(); i++) {
+            List<WriteSheetData> sheetDataList = FastExcel.read(testFile)
+                    .excelType(excelTypeEnum)
+                    .head(WriteSheetData.class)
+                    .sheet(i)
+                    .doReadSync();
+            Assertions.assertEquals(dataMap.get(sheetNoList.get(i)), sheetDataList.size());
+        }
+    }
+
     private static List<WriteSheetData> dataList(int size) {
         List<WriteSheetData> dataList = new ArrayList<>();
         for (int i = 0; i < size; i++) {