From efb588bbd1270993504ed87f1170481b877f8b88 Mon Sep 17 00:00:00 2001 From: Max Date: Sat, 29 Mar 2025 17:10:34 +0800 Subject: [PATCH] feat: Add sheet name validation to Excel module - Implement validateSheetName function to check for invalid characters, empty names, and length restrictions for sheet names. - Integrate validation into CreateSheet, UpdateSheet, and CopySheet methods to ensure compliance before proceeding with operations. - Enhance WriteAll method to create a new sheet if it doesn't exist, improving robustness in sheet handling. --- excel/sheet.go | 33 +++++++++ excel/write.go | 28 ++++++-- excel/write_test.go | 164 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 221 insertions(+), 4 deletions(-) create mode 100644 excel/write_test.go diff --git a/excel/sheet.go b/excel/sheet.go index 0e94449c..23ebd70e 100644 --- a/excel/sheet.go +++ b/excel/sheet.go @@ -2,6 +2,7 @@ package excel import ( "fmt" + "strings" "github.com/xuri/excelize/v2" ) @@ -18,9 +19,31 @@ func New() (*Excel, error) { }, nil } +// validateSheetName checks if the sheet name contains invalid characters +func (excel *Excel) validateSheetName(name string) error { + invalidChars := []string{":", "\\", "/", "?", "*", "[", "]"} + for _, char := range invalidChars { + if strings.Contains(name, char) { + return fmt.Errorf("sheet name cannot contain any of these characters: :/?*[\\]") + } + } + if len(name) == 0 { + return fmt.Errorf("sheet name cannot be empty") + } + if len(name) > 31 { + return fmt.Errorf("sheet name cannot be longer than 31 characters") + } + return nil +} + // CreateSheet creates a new sheet with the given name // Returns the index of the new sheet and any error encountered func (excel *Excel) CreateSheet(name string) (int, error) { + // Validate sheet name + if err := excel.validateSheetName(name); err != nil { + return 0, err + } + // Check if sheet already exists if idx, _ := excel.GetSheetIndex(name); idx != -1 { return 0, fmt.Errorf("sheet %s already exists", name) @@ -56,6 +79,11 @@ func (excel *Excel) ReadSheet(name string) ([][]interface{}, error) { // UpdateSheet updates an existing sheet with new data // If the sheet doesn't exist, it will be created func (excel *Excel) UpdateSheet(name string, data [][]interface{}) error { + // Validate sheet name + if err := excel.validateSheetName(name); err != nil { + return err + } + // Ensure sheet exists _, err := excel.SetSheet(name) if err != nil { @@ -95,6 +123,11 @@ func (excel *Excel) ListSheets() []string { // CopySheet copies a sheet to a new name func (excel *Excel) CopySheet(source, destination string) error { + // Validate destination sheet name + if err := excel.validateSheetName(destination); err != nil { + return err + } + // Check if source exists if idx, _ := excel.GetSheetIndex(source); idx == -1 { return fmt.Errorf("source sheet %s does not exist", source) diff --git a/excel/write.go b/excel/write.go index 09dd34e6..ec3adfd2 100644 --- a/excel/write.go +++ b/excel/write.go @@ -38,18 +38,38 @@ func (excel *Excel) WriteColumn(sheet string, cell string, value []interface{}) // WriteAll write all the sheet func (excel *Excel) WriteAll(sheet string, cell string, rows [][]interface{}) error { - _, err := excel.SetSheet(sheet) + // Check if sheet exists + idx, err := excel.GetSheetIndex(sheet) if err != nil { return err } + if idx == -1 { + // Create new sheet if it doesn't exist + idx, err = excel.NewSheet(sheet) + if err != nil { + return err + } + } + + // If no data to write, return + if len(rows) == 0 { + return nil + } + + // Write each row + currentCell := cell for _, row := range rows { - if err := excel.SetSheetRow(sheet, cell, &row); err != nil { + if err := excel.SetSheetRow(sheet, currentCell, &row); err != nil { return err } - colIndex, rowIndex, err := excelize.CellNameToCoordinates(cell) - cell, err = excelize.CoordinatesToCellName(colIndex, rowIndex+1) + // Move to next row + colIndex, rowIndex, err := excelize.CellNameToCoordinates(currentCell) + if err != nil { + return err + } + currentCell, err = excelize.CoordinatesToCellName(colIndex, rowIndex+1) if err != nil { return err } diff --git a/excel/write_test.go b/excel/write_test.go new file mode 100644 index 00000000..cb9aaf98 --- /dev/null +++ b/excel/write_test.go @@ -0,0 +1,164 @@ +package excel + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/xuri/excelize/v2" + "github.com/yaoapp/yao/config" + "github.com/yaoapp/yao/test" +) + +func TestWriteAll(t *testing.T) { + test.Prepare(t, config.Conf) + defer test.Clean() + + // Create a new Excel file + xls := excelize.NewFile() + defer func() { + if err := xls.Close(); err != nil { + t.Error(err) + } + }() + + // Create Excel instance + excel := &Excel{ + File: xls, + abs: "test.xlsx", + } + + t.Run("Write to default sheet", func(t *testing.T) { + data := [][]interface{}{ + {"Header1", "Header2", "Header3"}, + {1, "Data1", true}, + {2, "Data2", false}, + } + + err := excel.WriteAll("Sheet1", "A1", data) + assert.NoError(t, err) + + // Verify data was written + rows, err := excel.GetRows("Sheet1") + assert.NoError(t, err) + assert.GreaterOrEqual(t, len(rows), 3) + assert.Equal(t, "Header1", rows[0][0]) + assert.Equal(t, "Header2", rows[0][1]) + assert.Equal(t, "Header3", rows[0][2]) + }) + + t.Run("Write to new sheet", func(t *testing.T) { + data := [][]interface{}{ + {"Name", "Age", "Active"}, + {"John", 30, true}, + {"Jane", 25, false}, + } + + // Verify sheet doesn't exist before writing + sheets := excel.ListSheets() + assert.NotContains(t, sheets, "NewSheet") + + err := excel.WriteAll("NewSheet", "B2", data) + assert.NoError(t, err) + + // Verify sheet was created + sheets = excel.ListSheets() + assert.Contains(t, sheets, "NewSheet") + + // Verify data was written + rows, err := excel.GetRows("NewSheet") + assert.NoError(t, err) + assert.GreaterOrEqual(t, len(rows), 4) // Account for B2 start position + assert.Equal(t, "Name", rows[1][1]) // B2 position + assert.Equal(t, "Age", rows[1][2]) + assert.Equal(t, "Active", rows[1][3]) + }) + + t.Run("Write empty data to new sheet", func(t *testing.T) { + var data [][]interface{} + + // Verify sheet doesn't exist before writing + sheets := excel.ListSheets() + assert.NotContains(t, sheets, "EmptySheet") + + err := excel.WriteAll("EmptySheet", "A1", data) + assert.NoError(t, err) + + // Verify sheet was created but is empty + sheets = excel.ListSheets() + assert.Contains(t, sheets, "EmptySheet") + + rows, err := excel.GetRows("EmptySheet") + assert.NoError(t, err) + assert.Empty(t, rows) + }) + + t.Run("Write empty data to existing sheet", func(t *testing.T) { + // First write some data + data := [][]interface{}{ + {"Test"}, + } + err := excel.WriteAll("ExistingSheet", "A1", data) + assert.NoError(t, err) + + // Then write empty data + var emptyData [][]interface{} + err = excel.WriteAll("ExistingSheet", "A1", emptyData) + assert.NoError(t, err) + + // Verify original data remains + rows, err := excel.GetRows("ExistingSheet") + assert.NoError(t, err) + assert.NotEmpty(t, rows) + assert.Equal(t, "Test", rows[0][0]) + }) + + t.Run("Write with invalid cell reference", func(t *testing.T) { + data := [][]interface{}{ + {"Test"}, + } + err := excel.WriteAll("InvalidCell", "INVALID", data) + assert.Error(t, err) + + // Verify sheet was still created despite error + sheets := excel.ListSheets() + assert.Contains(t, sheets, "InvalidCell") + }) + + t.Run("Write to sheet with special characters", func(t *testing.T) { + // Valid sheet name with allowed special characters + data := [][]interface{}{ + {"Special"}, + } + err := excel.WriteAll("Sheet-123_中文", "A1", data) + assert.NoError(t, err) + + // Verify sheet was created and data written + sheets := excel.ListSheets() + assert.Contains(t, sheets, "Sheet-123_中文") + + rows, err := excel.GetRows("Sheet-123_中文") + assert.NoError(t, err) + assert.Equal(t, "Special", rows[0][0]) + + // Invalid sheet names + invalidNames := []string{ + "Sheet:1", + "Sheet/2", + "Sheet\\3", + "Sheet?4", + "Sheet*5", + "Sheet[6]", + "", // Empty name + "ThisSheetNameIsWayTooLongAndShouldFailBecauseExcelHasALimitOf31Characters", // Too long + } + + for _, name := range invalidNames { + err := excel.WriteAll(name, "A1", data) + assert.Error(t, err, "Should fail for invalid sheet name: %s", name) + } + }) + + // Optional: Save the file for manual inspection + // err := excel.SaveAs("test_output.xlsx") + // assert.NoError(t, err) +}