From 90781ad846be29400e9015c562caf8f67062f6a7 Mon Sep 17 00:00:00 2001 From: chenwang Date: Tue, 27 Feb 2024 17:01:44 +0800 Subject: [PATCH 1/5] fix: data validation is unsafe when used in concurrent condition(#1825) --- datavalidation.go | 50 +++++++++++++++++++++++++----------------- datavalidation_test.go | 38 ++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 20 deletions(-) diff --git a/datavalidation.go b/datavalidation.go index 8e6e5945a1..d63ce1518c 100644 --- a/datavalidation.go +++ b/datavalidation.go @@ -114,6 +114,31 @@ func NewDataValidation(allowBlank bool) *DataValidation { } } +// newXlsxDataValidation is a internal function to create xlsxDataValidation by given data validation object. +func newXlsxDataValidation(dv *DataValidation) *xlsxDataValidation { + dataValidation := &xlsxDataValidation{ + AllowBlank: dv.AllowBlank, + Error: dv.Error, + ErrorStyle: dv.ErrorStyle, + ErrorTitle: dv.ErrorTitle, + Operator: dv.Operator, + Prompt: dv.Prompt, + PromptTitle: dv.PromptTitle, + ShowDropDown: dv.ShowDropDown, + ShowErrorMessage: dv.ShowErrorMessage, + ShowInputMessage: dv.ShowInputMessage, + Sqref: dv.Sqref, + Type: dv.Type, + } + if dv.Formula1 != "" { + dataValidation.Formula1 = &xlsxInnerXML{Content: dv.Formula1} + } + if dv.Formula2 != "" { + dataValidation.Formula2 = &xlsxInnerXML{Content: dv.Formula2} + } + return dataValidation +} + // SetError set error notice. func (dv *DataValidation) SetError(style DataValidationErrorStyle, title, msg string) { dv.Error = &msg @@ -256,29 +281,12 @@ func (f *File) AddDataValidation(sheet string, dv *DataValidation) error { if err != nil { return err } + f.mu.Lock() + defer f.mu.Unlock() if nil == ws.DataValidations { ws.DataValidations = new(xlsxDataValidations) } - dataValidation := &xlsxDataValidation{ - AllowBlank: dv.AllowBlank, - Error: dv.Error, - ErrorStyle: dv.ErrorStyle, - ErrorTitle: dv.ErrorTitle, - Operator: dv.Operator, - Prompt: dv.Prompt, - PromptTitle: dv.PromptTitle, - ShowDropDown: dv.ShowDropDown, - ShowErrorMessage: dv.ShowErrorMessage, - ShowInputMessage: dv.ShowInputMessage, - Sqref: dv.Sqref, - Type: dv.Type, - } - if dv.Formula1 != "" { - dataValidation.Formula1 = &xlsxInnerXML{Content: dv.Formula1} - } - if dv.Formula2 != "" { - dataValidation.Formula2 = &xlsxInnerXML{Content: dv.Formula2} - } + dataValidation := newXlsxDataValidation(dv) ws.DataValidations.DataValidation = append(ws.DataValidations.DataValidation, dataValidation) ws.DataValidations.Count = len(ws.DataValidations.DataValidation) return err @@ -330,6 +338,8 @@ func (f *File) DeleteDataValidation(sheet string, sqref ...string) error { if err != nil { return err } + f.mu.Lock() + defer f.mu.Unlock() if ws.DataValidations == nil { return nil } diff --git a/datavalidation_test.go b/datavalidation_test.go index 8816df0664..2b26e5a1b8 100644 --- a/datavalidation_test.go +++ b/datavalidation_test.go @@ -12,9 +12,12 @@ package excelize import ( + "fmt" "math" "path/filepath" + "strconv" "strings" + "sync" "testing" "github.com/stretchr/testify/assert" @@ -106,6 +109,41 @@ func TestDataValidation(t *testing.T) { assert.Equal(t, []*DataValidation(nil), dataValidations) } +func TestConcurrentAddDataValidation(t *testing.T) { + var ( + resultFile = filepath.Join("test", "TestConcurrentAddDataValidation.xlsx") + f = NewFile() + sheet1 = "Sheet1" + dataValidationLen = 1000 + ) + + // data validation list + dvs := make([]*DataValidation, dataValidationLen) + for i := 0; i < dataValidationLen; i++ { + dvi := NewDataValidation(true) + dvi.Sqref = fmt.Sprintf("A%d:B%d", i+1, i+1) + dvi.SetRange(10, 20, DataValidationTypeWhole, DataValidationOperatorGreaterThan) + dvi.SetInput(fmt.Sprintf("title:%d", i+1), strconv.Itoa(i+1)) + dvs[i] = dvi + } + assert.Len(t, dvs, dataValidationLen) + // simulated concurrency + var wg sync.WaitGroup + wg.Add(dataValidationLen) + for i := 0; i < dataValidationLen; i++ { + go func(i int) { + f.AddDataValidation(sheet1, dvs[i]) + wg.Done() + }(i) + } + wg.Wait() + // Test the length of data validation after concurrent + dataValidations, err := f.GetDataValidations(sheet1) + assert.NoError(t, err) + assert.Len(t, dataValidations, dataValidationLen) + assert.NoError(t, f.SaveAs(resultFile)) +} + func TestDataValidationError(t *testing.T) { resultFile := filepath.Join("test", "TestDataValidationError.xlsx") From 5296439644383d61f8f9b8987cb9537adc65217c Mon Sep 17 00:00:00 2001 From: chenwang Date: Tue, 27 Feb 2024 17:12:28 +0800 Subject: [PATCH 2/5] feat: supports batch append data validation to the worksheet(#1826) --- datavalidation.go | 20 ++++++++++ datavalidation_test.go | 83 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+) diff --git a/datavalidation.go b/datavalidation.go index d63ce1518c..c018d4127a 100644 --- a/datavalidation.go +++ b/datavalidation.go @@ -292,6 +292,26 @@ func (f *File) AddDataValidation(sheet string, dv *DataValidation) error { return err } +// BatchAddDataValidation is a function supports batch append data validation to the worksheet, reference to AddDataValidation(). +func (f *File) BatchAddDataValidation(sheet string, dvs []*DataValidation) error { + ws, err := f.workSheetReader(sheet) + if err != nil { + return err + } + f.mu.Lock() + defer f.mu.Unlock() + if nil == ws.DataValidations { + ws.DataValidations = new(xlsxDataValidations) + } + dataValidations := make([]*xlsxDataValidation, len(dvs)) + for i := 0; i < len(dvs); i++ { + dataValidations[i] = newXlsxDataValidation(dvs[i]) + } + ws.DataValidations.DataValidation = append(ws.DataValidations.DataValidation, dataValidations...) + ws.DataValidations.Count = len(ws.DataValidations.DataValidation) + return err +} + // GetDataValidations returns data validations list by given worksheet name. func (f *File) GetDataValidations(sheet string) ([]*DataValidation, error) { ws, err := f.workSheetReader(sheet) diff --git a/datavalidation_test.go b/datavalidation_test.go index 2b26e5a1b8..7ec150e530 100644 --- a/datavalidation_test.go +++ b/datavalidation_test.go @@ -142,6 +142,89 @@ func TestConcurrentAddDataValidation(t *testing.T) { assert.NoError(t, err) assert.Len(t, dataValidations, dataValidationLen) assert.NoError(t, f.SaveAs(resultFile)) + + // Test the length of data validation after calling DeleteDataValidation() + f.DeleteDataValidation(sheet1) + dataValidations, err = f.GetDataValidations(sheet1) + assert.NoError(t, err) + assert.Len(t, dataValidations, 0) + // Test the length of data validation after calling BatchAddDataValidation() + f.BatchAddDataValidation(sheet1, dvs) + dataValidations, err = f.GetDataValidations(sheet1) + assert.NoError(t, err) + assert.Len(t, dataValidations, dataValidationLen) + assert.NoError(t, f.SaveAs(resultFile)) +} + +func TestBatchAddDataValidation(t *testing.T) { + resultFile := filepath.Join("test", "TestBatchAddDataValidation.xlsx") + f := NewFile() + // data validation 1 + dv1 := NewDataValidation(true) + dv1.Sqref = "A1:B2" + assert.NoError(t, dv1.SetRange(10, 20, DataValidationTypeWhole, DataValidationOperatorBetween)) + dv1.SetError(DataValidationErrorStyleStop, "error title", "error body") + dv1.SetError(DataValidationErrorStyleWarning, "error title", "error body") + dv1.SetError(DataValidationErrorStyleInformation, "error title", "error body") + // data validation 2 + dv2 := NewDataValidation(true) + dv2.Sqref = "A3:B4" + assert.NoError(t, dv2.SetRange(10, 20, DataValidationTypeWhole, DataValidationOperatorGreaterThan)) + dv2.SetInput("input title", "input body") + // data validation 3 + dv3 := NewDataValidation(true) + dv3.Sqref = "A1:B1" + assert.NoError(t, dv3.SetRange("INDIRECT($A$2)", "INDIRECT($A$3)", DataValidationTypeWhole, DataValidationOperatorBetween)) + dv3.SetError(DataValidationErrorStyleStop, "error title", "error body") + // data validation 4 + dv4 := NewDataValidation(true) + dv4.Sqref = "A5:B6" + _, err := f.NewSheet("Sheet2") + assert.NoError(t, err) + assert.NoError(t, f.SetSheetRow("Sheet2", "A2", &[]interface{}{"B2", 1})) + assert.NoError(t, f.SetSheetRow("Sheet2", "A3", &[]interface{}{"B3", 3})) + for _, listValid := range [][]string{ + {"1", "2", "3"}, + {"=A1"}, + {strings.Repeat("&", MaxFieldLength)}, + {strings.Repeat("\u4E00", MaxFieldLength)}, + {strings.Repeat("\U0001F600", 100), strings.Repeat("\u4E01", 50), "<&>"}, + {`A<`, `B>`, `C"`, "D\t", `E'`, `F`}, + } { + dv4.Formula1 = "" + assert.NoError(t, dv4.SetDropList(listValid), + "SetDropList failed for valid input %v", listValid) + assert.NotEqual(t, "", dv4.Formula1, + "Formula1 should not be empty for valid input %v", listValid) + } + assert.Equal(t, `"A<,B>,C"",D ,E',F"`, dv4.Formula1) + + // Test batch append data to the worksheet + assert.NoError(t, f.BatchAddDataValidation("Sheet1", []*DataValidation{dv1, dv2, dv3})) + assert.NoError(t, f.BatchAddDataValidation("Sheet2", []*DataValidation{dv4})) + + // Test the length of data validation of above worksheet. + dataValidations, err := f.GetDataValidations("Sheet1") + assert.NoError(t, err) + assert.Len(t, dataValidations, 3) + assert.NoError(t, f.SaveAs(resultFile)) + dataValidations, err = f.GetDataValidations("Sheet2") + assert.NoError(t, err) + assert.Len(t, dataValidations, 1) + + // Test get data validation on no exists worksheet + _, err = f.GetDataValidations("SheetN") + assert.EqualError(t, err, "sheet SheetN does not exist") + // Test get data validation with invalid sheet name + _, err = f.GetDataValidations("Sheet:1") + assert.EqualError(t, err, ErrSheetNameInvalid.Error()) + + assert.NoError(t, f.SaveAs(resultFile)) + // Test get data validation on a worksheet without data validation settings + f = NewFile() + dataValidations, err = f.GetDataValidations("Sheet1") + assert.NoError(t, err) + assert.Equal(t, []*DataValidation(nil), dataValidations) } func TestDataValidationError(t *testing.T) { From 4a205fae5c09a12f474d4c35e5fcc355848dcc41 Mon Sep 17 00:00:00 2001 From: chenwang Date: Thu, 29 Feb 2024 15:52:14 +0800 Subject: [PATCH 3/5] using the embed mutex in worksheet struct for data validation concurrency safe --- cell_test.go | 10 ++++ datavalidation.go | 33 +++-------- datavalidation_test.go | 121 ----------------------------------------- 3 files changed, 17 insertions(+), 147 deletions(-) diff --git a/cell_test.go b/cell_test.go index 0ed6e87fbc..78c1ee7219 100644 --- a/cell_test.go +++ b/cell_test.go @@ -88,6 +88,12 @@ func TestConcurrency(t *testing.T) { visible, err := f.GetColVisible("Sheet1", "A") assert.NoError(t, err) assert.Equal(t, true, visible) + // Concurrency add data validation + dv := NewDataValidation(true) + dv.Sqref = fmt.Sprintf("A%d:B%d", val, val) + dv.SetRange(10, 20, DataValidationTypeWhole, DataValidationOperatorGreaterThan) + dv.SetInput(fmt.Sprintf("title:%d", val), strconv.Itoa(val)) + f.AddDataValidation("Sheet1", dv) wg.Done() }(i, t) } @@ -97,6 +103,10 @@ func TestConcurrency(t *testing.T) { t.Error(err) } assert.Equal(t, "1", val) + // Test the length of data validation + dataValidations, err := f.GetDataValidations("Sheet1") + assert.NoError(t, err) + assert.Len(t, dataValidations, 5) assert.NoError(t, f.SaveAs(filepath.Join("test", "TestConcurrency.xlsx"))) assert.NoError(t, f.Close()) } diff --git a/datavalidation.go b/datavalidation.go index c018d4127a..0a52b6805d 100644 --- a/datavalidation.go +++ b/datavalidation.go @@ -247,8 +247,9 @@ func (dv *DataValidation) SetSqref(sqref string) { } // AddDataValidation provides set data validation on a range of the worksheet -// by given data validation object and worksheet name. The data validation -// object can be created by NewDataValidation function. +// by given data validation object and worksheet name. This function is +// concurrency safe. The data validation object can be created by +// NewDataValidation function. // // Example 1, set data validation on Sheet1!A1:B2 with validation criteria // settings, show error alert after invalid data is entered with "Stop" style @@ -281,8 +282,8 @@ func (f *File) AddDataValidation(sheet string, dv *DataValidation) error { if err != nil { return err } - f.mu.Lock() - defer f.mu.Unlock() + ws.mu.Lock() + defer ws.mu.Unlock() if nil == ws.DataValidations { ws.DataValidations = new(xlsxDataValidations) } @@ -292,26 +293,6 @@ func (f *File) AddDataValidation(sheet string, dv *DataValidation) error { return err } -// BatchAddDataValidation is a function supports batch append data validation to the worksheet, reference to AddDataValidation(). -func (f *File) BatchAddDataValidation(sheet string, dvs []*DataValidation) error { - ws, err := f.workSheetReader(sheet) - if err != nil { - return err - } - f.mu.Lock() - defer f.mu.Unlock() - if nil == ws.DataValidations { - ws.DataValidations = new(xlsxDataValidations) - } - dataValidations := make([]*xlsxDataValidation, len(dvs)) - for i := 0; i < len(dvs); i++ { - dataValidations[i] = newXlsxDataValidation(dvs[i]) - } - ws.DataValidations.DataValidation = append(ws.DataValidations.DataValidation, dataValidations...) - ws.DataValidations.Count = len(ws.DataValidations.DataValidation) - return err -} - // GetDataValidations returns data validations list by given worksheet name. func (f *File) GetDataValidations(sheet string) ([]*DataValidation, error) { ws, err := f.workSheetReader(sheet) @@ -358,8 +339,8 @@ func (f *File) DeleteDataValidation(sheet string, sqref ...string) error { if err != nil { return err } - f.mu.Lock() - defer f.mu.Unlock() + ws.mu.Lock() + defer ws.mu.Unlock() if ws.DataValidations == nil { return nil } diff --git a/datavalidation_test.go b/datavalidation_test.go index 7ec150e530..8816df0664 100644 --- a/datavalidation_test.go +++ b/datavalidation_test.go @@ -12,12 +12,9 @@ package excelize import ( - "fmt" "math" "path/filepath" - "strconv" "strings" - "sync" "testing" "github.com/stretchr/testify/assert" @@ -109,124 +106,6 @@ func TestDataValidation(t *testing.T) { assert.Equal(t, []*DataValidation(nil), dataValidations) } -func TestConcurrentAddDataValidation(t *testing.T) { - var ( - resultFile = filepath.Join("test", "TestConcurrentAddDataValidation.xlsx") - f = NewFile() - sheet1 = "Sheet1" - dataValidationLen = 1000 - ) - - // data validation list - dvs := make([]*DataValidation, dataValidationLen) - for i := 0; i < dataValidationLen; i++ { - dvi := NewDataValidation(true) - dvi.Sqref = fmt.Sprintf("A%d:B%d", i+1, i+1) - dvi.SetRange(10, 20, DataValidationTypeWhole, DataValidationOperatorGreaterThan) - dvi.SetInput(fmt.Sprintf("title:%d", i+1), strconv.Itoa(i+1)) - dvs[i] = dvi - } - assert.Len(t, dvs, dataValidationLen) - // simulated concurrency - var wg sync.WaitGroup - wg.Add(dataValidationLen) - for i := 0; i < dataValidationLen; i++ { - go func(i int) { - f.AddDataValidation(sheet1, dvs[i]) - wg.Done() - }(i) - } - wg.Wait() - // Test the length of data validation after concurrent - dataValidations, err := f.GetDataValidations(sheet1) - assert.NoError(t, err) - assert.Len(t, dataValidations, dataValidationLen) - assert.NoError(t, f.SaveAs(resultFile)) - - // Test the length of data validation after calling DeleteDataValidation() - f.DeleteDataValidation(sheet1) - dataValidations, err = f.GetDataValidations(sheet1) - assert.NoError(t, err) - assert.Len(t, dataValidations, 0) - // Test the length of data validation after calling BatchAddDataValidation() - f.BatchAddDataValidation(sheet1, dvs) - dataValidations, err = f.GetDataValidations(sheet1) - assert.NoError(t, err) - assert.Len(t, dataValidations, dataValidationLen) - assert.NoError(t, f.SaveAs(resultFile)) -} - -func TestBatchAddDataValidation(t *testing.T) { - resultFile := filepath.Join("test", "TestBatchAddDataValidation.xlsx") - f := NewFile() - // data validation 1 - dv1 := NewDataValidation(true) - dv1.Sqref = "A1:B2" - assert.NoError(t, dv1.SetRange(10, 20, DataValidationTypeWhole, DataValidationOperatorBetween)) - dv1.SetError(DataValidationErrorStyleStop, "error title", "error body") - dv1.SetError(DataValidationErrorStyleWarning, "error title", "error body") - dv1.SetError(DataValidationErrorStyleInformation, "error title", "error body") - // data validation 2 - dv2 := NewDataValidation(true) - dv2.Sqref = "A3:B4" - assert.NoError(t, dv2.SetRange(10, 20, DataValidationTypeWhole, DataValidationOperatorGreaterThan)) - dv2.SetInput("input title", "input body") - // data validation 3 - dv3 := NewDataValidation(true) - dv3.Sqref = "A1:B1" - assert.NoError(t, dv3.SetRange("INDIRECT($A$2)", "INDIRECT($A$3)", DataValidationTypeWhole, DataValidationOperatorBetween)) - dv3.SetError(DataValidationErrorStyleStop, "error title", "error body") - // data validation 4 - dv4 := NewDataValidation(true) - dv4.Sqref = "A5:B6" - _, err := f.NewSheet("Sheet2") - assert.NoError(t, err) - assert.NoError(t, f.SetSheetRow("Sheet2", "A2", &[]interface{}{"B2", 1})) - assert.NoError(t, f.SetSheetRow("Sheet2", "A3", &[]interface{}{"B3", 3})) - for _, listValid := range [][]string{ - {"1", "2", "3"}, - {"=A1"}, - {strings.Repeat("&", MaxFieldLength)}, - {strings.Repeat("\u4E00", MaxFieldLength)}, - {strings.Repeat("\U0001F600", 100), strings.Repeat("\u4E01", 50), "<&>"}, - {`A<`, `B>`, `C"`, "D\t", `E'`, `F`}, - } { - dv4.Formula1 = "" - assert.NoError(t, dv4.SetDropList(listValid), - "SetDropList failed for valid input %v", listValid) - assert.NotEqual(t, "", dv4.Formula1, - "Formula1 should not be empty for valid input %v", listValid) - } - assert.Equal(t, `"A<,B>,C"",D ,E',F"`, dv4.Formula1) - - // Test batch append data to the worksheet - assert.NoError(t, f.BatchAddDataValidation("Sheet1", []*DataValidation{dv1, dv2, dv3})) - assert.NoError(t, f.BatchAddDataValidation("Sheet2", []*DataValidation{dv4})) - - // Test the length of data validation of above worksheet. - dataValidations, err := f.GetDataValidations("Sheet1") - assert.NoError(t, err) - assert.Len(t, dataValidations, 3) - assert.NoError(t, f.SaveAs(resultFile)) - dataValidations, err = f.GetDataValidations("Sheet2") - assert.NoError(t, err) - assert.Len(t, dataValidations, 1) - - // Test get data validation on no exists worksheet - _, err = f.GetDataValidations("SheetN") - assert.EqualError(t, err, "sheet SheetN does not exist") - // Test get data validation with invalid sheet name - _, err = f.GetDataValidations("Sheet:1") - assert.EqualError(t, err, ErrSheetNameInvalid.Error()) - - assert.NoError(t, f.SaveAs(resultFile)) - // Test get data validation on a worksheet without data validation settings - f = NewFile() - dataValidations, err = f.GetDataValidations("Sheet1") - assert.NoError(t, err) - assert.Equal(t, []*DataValidation(nil), dataValidations) -} - func TestDataValidationError(t *testing.T) { resultFile := filepath.Join("test", "TestDataValidationError.xlsx") From 2a5481874359f817e81c8a4326c1c489ae6cc86f Mon Sep 17 00:00:00 2001 From: chenwang Date: Thu, 29 Feb 2024 17:49:21 +0800 Subject: [PATCH 4/5] add comment test for DeleteDataValidation and remove the receiver of coordinatesToRangeRef,squashSqref,flatSqref function. --- adjust.go | 8 ++++---- cell_test.go | 4 +++- datavalidation.go | 14 +++++++------- lib.go | 4 ++-- lib_test.go | 9 ++++----- rows.go | 4 ++-- sheet.go | 2 +- stream.go | 2 +- table.go | 4 ++-- 9 files changed, 26 insertions(+), 25 deletions(-) diff --git a/adjust.go b/adjust.go index 75faf16ad6..b4f81f6ddd 100644 --- a/adjust.go +++ b/adjust.go @@ -267,7 +267,7 @@ func (f *File) adjustCellRef(ref string, dir adjustDirection, num, offset int) ( coordinates[3] += offset } } - ref, err = f.coordinatesToRangeRef(coordinates) + ref, err = coordinatesToRangeRef(coordinates) return ref, delete, err } @@ -666,7 +666,7 @@ func (f *File) adjustTable(ws *xlsxWorksheet, sheet string, dir adjustDirection, idx-- continue } - t.Ref, _ = f.coordinatesToRangeRef([]int{x1, y1, x2, y2}) + t.Ref, _ = coordinatesToRangeRef([]int{x1, y1, x2, y2}) if t.AutoFilter != nil { t.AutoFilter.Ref = t.Ref } @@ -706,7 +706,7 @@ func (f *File) adjustAutoFilter(ws *xlsxWorksheet, sheet string, dir adjustDirec coordinates = f.adjustAutoFilterHelper(dir, coordinates, num, offset) x1, y1, x2, y2 = coordinates[0], coordinates[1], coordinates[2], coordinates[3] - ws.AutoFilter.Ref, err = f.coordinatesToRangeRef([]int{x1, y1, x2, y2}) + ws.AutoFilter.Ref, err = coordinatesToRangeRef([]int{x1, y1, x2, y2}) return err } @@ -773,7 +773,7 @@ func (f *File) adjustMergeCells(ws *xlsxWorksheet, sheet string, dir adjustDirec continue } mergedCells.rect = []int{x1, y1, x2, y2} - if mergedCells.Ref, err = f.coordinatesToRangeRef([]int{x1, y1, x2, y2}); err != nil { + if mergedCells.Ref, err = coordinatesToRangeRef([]int{x1, y1, x2, y2}); err != nil { return err } } diff --git a/cell_test.go b/cell_test.go index 78c1ee7219..4f6cd77a89 100644 --- a/cell_test.go +++ b/cell_test.go @@ -94,6 +94,8 @@ func TestConcurrency(t *testing.T) { dv.SetRange(10, 20, DataValidationTypeWhole, DataValidationOperatorGreaterThan) dv.SetInput(fmt.Sprintf("title:%d", val), strconv.Itoa(val)) f.AddDataValidation("Sheet1", dv) + // Concurrency delete data validation with reference sequence + f.DeleteDataValidation("Sheet1", dv.Sqref) wg.Done() }(i, t) } @@ -106,7 +108,7 @@ func TestConcurrency(t *testing.T) { // Test the length of data validation dataValidations, err := f.GetDataValidations("Sheet1") assert.NoError(t, err) - assert.Len(t, dataValidations, 5) + assert.Len(t, dataValidations, 0) assert.NoError(t, f.SaveAs(filepath.Join("test", "TestConcurrency.xlsx"))) assert.NoError(t, f.Close()) } diff --git a/datavalidation.go b/datavalidation.go index 0a52b6805d..964f4361f2 100644 --- a/datavalidation.go +++ b/datavalidation.go @@ -332,7 +332,7 @@ func (f *File) GetDataValidations(sheet string) ([]*DataValidation, error) { } // DeleteDataValidation delete data validation by given worksheet name and -// reference sequence. All data validations in the worksheet will be deleted +// reference sequence. This function is concurrency safe. All data validations in the worksheet will be deleted // if not specify reference sequence parameter. func (f *File) DeleteDataValidation(sheet string, sqref ...string) error { ws, err := f.workSheetReader(sheet) @@ -348,14 +348,14 @@ func (f *File) DeleteDataValidation(sheet string, sqref ...string) error { ws.DataValidations = nil return nil } - delCells, err := f.flatSqref(sqref[0]) + delCells, err := flatSqref(sqref[0]) if err != nil { return err } dv := ws.DataValidations for i := 0; i < len(dv.DataValidation); i++ { var applySqref []string - colCells, err := f.flatSqref(dv.DataValidation[i].Sqref) + colCells, err := flatSqref(dv.DataValidation[i].Sqref) if err != nil { return err } @@ -368,7 +368,7 @@ func (f *File) DeleteDataValidation(sheet string, sqref ...string) error { } } for _, col := range colCells { - applySqref = append(applySqref, f.squashSqref(col)...) + applySqref = append(applySqref, squashSqref(col)...) } dv.DataValidation[i].Sqref = strings.Join(applySqref, " ") if len(applySqref) == 0 { @@ -384,7 +384,7 @@ func (f *File) DeleteDataValidation(sheet string, sqref ...string) error { } // squashSqref generates cell reference sequence by given cells coordinates list. -func (f *File) squashSqref(cells [][]int) []string { +func squashSqref(cells [][]int) []string { if len(cells) == 1 { cell, _ := CoordinatesToCellName(cells[0][0], cells[0][1]) return []string{cell} @@ -395,7 +395,7 @@ func (f *File) squashSqref(cells [][]int) []string { l, r := 0, 0 for i := 1; i < len(cells); i++ { if cells[i][0] == cells[r][0] && cells[i][1]-cells[r][1] > 1 { - ref, _ := f.coordinatesToRangeRef(append(cells[l], cells[r]...)) + ref, _ := coordinatesToRangeRef(append(cells[l], cells[r]...)) if l == r { ref, _ = CoordinatesToCellName(cells[l][0], cells[l][1]) } @@ -405,7 +405,7 @@ func (f *File) squashSqref(cells [][]int) []string { r++ } } - ref, _ := f.coordinatesToRangeRef(append(cells[l], cells[r]...)) + ref, _ := coordinatesToRangeRef(append(cells[l], cells[r]...)) if l == r { ref, _ = CoordinatesToCellName(cells[l][0], cells[l][1]) } diff --git a/lib.go b/lib.go index a1e340c880..bfa992e587 100644 --- a/lib.go +++ b/lib.go @@ -323,7 +323,7 @@ func sortCoordinates(coordinates []int) error { // coordinatesToRangeRef provides a function to convert a pair of coordinates // to range reference. -func (f *File) coordinatesToRangeRef(coordinates []int, abs ...bool) (string, error) { +func coordinatesToRangeRef(coordinates []int, abs ...bool) (string, error) { if len(coordinates) != 4 { return "", ErrCoordinates } @@ -360,7 +360,7 @@ func (f *File) getDefinedNameRefTo(definedNameName, currentSheet string) (refTo } // flatSqref convert reference sequence to cell reference list. -func (f *File) flatSqref(sqref string) (cells map[int][][]int, err error) { +func flatSqref(sqref string) (cells map[int][][]int, err error) { var coordinates []int cells = make(map[int][][]int) for _, ref := range strings.Fields(sqref) { diff --git a/lib_test.go b/lib_test.go index 48e730d0f1..7500f4951a 100644 --- a/lib_test.go +++ b/lib_test.go @@ -218,14 +218,13 @@ func TestCoordinatesToCellName_Error(t *testing.T) { } func TestCoordinatesToRangeRef(t *testing.T) { - f := NewFile() - _, err := f.coordinatesToRangeRef([]int{}) + _, err := coordinatesToRangeRef([]int{}) assert.EqualError(t, err, ErrCoordinates.Error()) - _, err = f.coordinatesToRangeRef([]int{1, -1, 1, 1}) + _, err = coordinatesToRangeRef([]int{1, -1, 1, 1}) assert.Equal(t, newCoordinatesToCellNameError(1, -1), err) - _, err = f.coordinatesToRangeRef([]int{1, 1, 1, -1}) + _, err = coordinatesToRangeRef([]int{1, 1, 1, -1}) assert.Equal(t, newCoordinatesToCellNameError(1, -1), err) - ref, err := f.coordinatesToRangeRef([]int{1, 1, 1, 1}) + ref, err := coordinatesToRangeRef([]int{1, 1, 1, 1}) assert.NoError(t, err) assert.EqualValues(t, ref, "A1:A1") } diff --git a/rows.go b/rows.go index 878c8d857c..814ff9c353 100644 --- a/rows.go +++ b/rows.go @@ -705,7 +705,7 @@ func (f *File) duplicateConditionalFormat(ws *xlsxWorksheet, sheet string, row, x1, y1, x2, y2 := coordinates[0], coordinates[1], coordinates[2], coordinates[3] if y1 == y2 && y1 == row { cfCopy := deepcopy.Copy(*cf).(xlsxConditionalFormatting) - if cfCopy.SQRef, err = f.coordinatesToRangeRef([]int{x1, row2, x2, row2}, abs); err != nil { + if cfCopy.SQRef, err = coordinatesToRangeRef([]int{x1, row2, x2, row2}, abs); err != nil { return err } cfs = append(cfs, &cfCopy) @@ -736,7 +736,7 @@ func (f *File) duplicateDataValidations(ws *xlsxWorksheet, sheet string, row, ro x1, y1, x2, y2 := coordinates[0], coordinates[1], coordinates[2], coordinates[3] if y1 == y2 && y1 == row { dvCopy := deepcopy.Copy(*dv).(xlsxDataValidation) - if dvCopy.Sqref, err = f.coordinatesToRangeRef([]int{x1, row2, x2, row2}, abs); err != nil { + if dvCopy.Sqref, err = coordinatesToRangeRef([]int{x1, row2, x2, row2}, abs); err != nil { return err } dvs = append(dvs, &dvCopy) diff --git a/sheet.go b/sheet.go index 315507dbfa..e089c41e3c 100644 --- a/sheet.go +++ b/sheet.go @@ -2014,7 +2014,7 @@ func (f *File) SetSheetDimension(sheet string, rangeRef string) error { return err } _ = sortCoordinates(coordinates) - ref, err := f.coordinatesToRangeRef(coordinates) + ref, err := coordinatesToRangeRef(coordinates) ws.Dimension = &xlsxDimension{Ref: ref} return err } diff --git a/stream.go b/stream.go index ef7fa1311b..e8d9ea649c 100644 --- a/stream.go +++ b/stream.go @@ -184,7 +184,7 @@ func (sw *StreamWriter) AddTable(table *Table) error { } // Correct table reference range, such correct C1:B3 to B1:C3. - ref, err := sw.file.coordinatesToRangeRef(coordinates) + ref, err := coordinatesToRangeRef(coordinates) if err != nil { return err } diff --git a/table.go b/table.go index c0c1594483..611bc560c8 100644 --- a/table.go +++ b/table.go @@ -350,7 +350,7 @@ func (f *File) addTable(sheet, tableXML string, x1, y1, x2, y2, i int, opts *Tab y1++ } // Correct table range reference, such correct C1:B3 to B1:C3. - ref, err := f.coordinatesToRangeRef([]int{x1, y1, x2, y2}) + ref, err := coordinatesToRangeRef([]int{x1, y1, x2, y2}) if err != nil { return err } @@ -463,7 +463,7 @@ func (f *File) AutoFilter(sheet, rangeRef string, opts []AutoFilterOptions) erro } _ = sortCoordinates(coordinates) // Correct reference range, such correct C1:B3 to B1:C3. - ref, _ := f.coordinatesToRangeRef(coordinates, true) + ref, _ := coordinatesToRangeRef(coordinates, true) wb, err := f.workbookReader() if err != nil { return err From 277df9ff6a55c4b1fb3d2b9b95921f59be055c1a Mon Sep 17 00:00:00 2001 From: chenwang Date: Fri, 1 Mar 2024 09:07:40 +0800 Subject: [PATCH 5/5] add assert for AddDataValidation and DeleteDataValidation in test file --- cell_test.go | 4 ++-- datavalidation.go | 49 +++++++++++++++++++++-------------------------- 2 files changed, 24 insertions(+), 29 deletions(-) diff --git a/cell_test.go b/cell_test.go index 4f6cd77a89..42472cfd72 100644 --- a/cell_test.go +++ b/cell_test.go @@ -93,9 +93,9 @@ func TestConcurrency(t *testing.T) { dv.Sqref = fmt.Sprintf("A%d:B%d", val, val) dv.SetRange(10, 20, DataValidationTypeWhole, DataValidationOperatorGreaterThan) dv.SetInput(fmt.Sprintf("title:%d", val), strconv.Itoa(val)) - f.AddDataValidation("Sheet1", dv) + assert.NoError(t, f.AddDataValidation("Sheet1", dv)) // Concurrency delete data validation with reference sequence - f.DeleteDataValidation("Sheet1", dv.Sqref) + assert.NoError(t, f.DeleteDataValidation("Sheet1", dv.Sqref)) wg.Done() }(i, t) } diff --git a/datavalidation.go b/datavalidation.go index 964f4361f2..c2ec1f1f8e 100644 --- a/datavalidation.go +++ b/datavalidation.go @@ -114,31 +114,6 @@ func NewDataValidation(allowBlank bool) *DataValidation { } } -// newXlsxDataValidation is a internal function to create xlsxDataValidation by given data validation object. -func newXlsxDataValidation(dv *DataValidation) *xlsxDataValidation { - dataValidation := &xlsxDataValidation{ - AllowBlank: dv.AllowBlank, - Error: dv.Error, - ErrorStyle: dv.ErrorStyle, - ErrorTitle: dv.ErrorTitle, - Operator: dv.Operator, - Prompt: dv.Prompt, - PromptTitle: dv.PromptTitle, - ShowDropDown: dv.ShowDropDown, - ShowErrorMessage: dv.ShowErrorMessage, - ShowInputMessage: dv.ShowInputMessage, - Sqref: dv.Sqref, - Type: dv.Type, - } - if dv.Formula1 != "" { - dataValidation.Formula1 = &xlsxInnerXML{Content: dv.Formula1} - } - if dv.Formula2 != "" { - dataValidation.Formula2 = &xlsxInnerXML{Content: dv.Formula2} - } - return dataValidation -} - // SetError set error notice. func (dv *DataValidation) SetError(style DataValidationErrorStyle, title, msg string) { dv.Error = &msg @@ -287,7 +262,26 @@ func (f *File) AddDataValidation(sheet string, dv *DataValidation) error { if nil == ws.DataValidations { ws.DataValidations = new(xlsxDataValidations) } - dataValidation := newXlsxDataValidation(dv) + dataValidation := &xlsxDataValidation{ + AllowBlank: dv.AllowBlank, + Error: dv.Error, + ErrorStyle: dv.ErrorStyle, + ErrorTitle: dv.ErrorTitle, + Operator: dv.Operator, + Prompt: dv.Prompt, + PromptTitle: dv.PromptTitle, + ShowDropDown: dv.ShowDropDown, + ShowErrorMessage: dv.ShowErrorMessage, + ShowInputMessage: dv.ShowInputMessage, + Sqref: dv.Sqref, + Type: dv.Type, + } + if dv.Formula1 != "" { + dataValidation.Formula1 = &xlsxInnerXML{Content: dv.Formula1} + } + if dv.Formula2 != "" { + dataValidation.Formula2 = &xlsxInnerXML{Content: dv.Formula2} + } ws.DataValidations.DataValidation = append(ws.DataValidations.DataValidation, dataValidation) ws.DataValidations.Count = len(ws.DataValidations.DataValidation) return err @@ -332,7 +326,8 @@ func (f *File) GetDataValidations(sheet string) ([]*DataValidation, error) { } // DeleteDataValidation delete data validation by given worksheet name and -// reference sequence. This function is concurrency safe. All data validations in the worksheet will be deleted +// reference sequence. This function is concurrency safe. +// All data validations in the worksheet will be deleted // if not specify reference sequence parameter. func (f *File) DeleteDataValidation(sheet string, sqref ...string) error { ws, err := f.workSheetReader(sheet)