This closes qax-os#1825, made AddDataValidation and DeleteDataValidation functions concurrency safe (qax-os#1828)

- Remove the receiver of internal coordinatesToRangeRef, squashSqref and flatSqref functions
- Update unit tests

Co-authored-by: chenwang <chenwang@shinsson.com>

Author: JackMin1314

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
 		}
 	}

cell_test.go

@@ -88,6 +88,14 @@ 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))
+			assert.NoError(t, f.AddDataValidation("Sheet1", dv))
+			// Concurrency delete data validation with reference sequence
+			assert.NoError(t, f.DeleteDataValidation("Sheet1", dv.Sqref))
 			wg.Done()
 		}(i, t)
 	}
@@ -97,6 +105,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, 0)
 	assert.NoError(t, f.SaveAs(filepath.Join("test", "TestConcurrency.xlsx")))
 	assert.NoError(t, f.Close())
 }

datavalidation.go

@@ -222,8 +222,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
@@ -256,6 +257,8 @@ func (f *File) AddDataValidation(sheet string, dv *DataValidation) error {
 	if err != nil {
 		return err
 	}
+	ws.mu.Lock()
+	defer ws.mu.Unlock()
 	if nil == ws.DataValidations {
 		ws.DataValidations = new(xlsxDataValidations)
 	}
@@ -323,28 +326,31 @@ 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)
 	if err != nil {
 		return err
 	}
+	ws.mu.Lock()
+	defer ws.mu.Unlock()
 	if ws.DataValidations == nil {
 		return nil
 	}
 	if sqref == nil {
 		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
 		}
@@ -357,7 +363,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 {
@@ -373,7 +379,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}
@@ -384,7 +390,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])
 			}
@@ -394,7 +400,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])
 	}

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) {

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")
 }

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)

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
 }

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
 	}

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