Skip to content

Commit

Permalink
This closes #1528, closes #1533
Browse files Browse the repository at this point in the history
- Avoid format text cell value as a numeric
- Fix race conditions for concurrency safety functions
  • Loading branch information
xuri committed Apr 25, 2023
1 parent 93c72b4 commit 612f6f1
Show file tree
Hide file tree
Showing 9 changed files with 86 additions and 35 deletions.
39 changes: 25 additions & 14 deletions cell.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,16 +288,19 @@ func setCellDuration(value time.Duration) (t string, v string) {
// SetCellInt provides a function to set int type value of a cell by given
// worksheet name, cell reference and cell value.
func (f *File) SetCellInt(sheet, cell string, value int) error {
f.mu.Lock()
ws, err := f.workSheetReader(sheet)
if err != nil {
f.mu.Unlock()
return err
}
f.mu.Unlock()
ws.mu.Lock()
defer ws.mu.Unlock()
c, col, row, err := ws.prepareCell(cell)
if err != nil {
return err
}
ws.mu.Lock()
defer ws.mu.Unlock()
c.S = ws.prepareCellStyle(col, row, c.S)
c.T, c.V = setCellInt(value)
c.IS = nil
Expand All @@ -314,16 +317,19 @@ func setCellInt(value int) (t string, v string) {
// SetCellBool provides a function to set bool type value of a cell by given
// worksheet name, cell reference and cell value.
func (f *File) SetCellBool(sheet, cell string, value bool) error {
f.mu.Lock()
ws, err := f.workSheetReader(sheet)
if err != nil {
f.mu.Unlock()
return err
}
f.mu.Unlock()
ws.mu.Lock()
defer ws.mu.Unlock()
c, col, row, err := ws.prepareCell(cell)
if err != nil {
return err
}
ws.mu.Lock()
defer ws.mu.Unlock()
c.S = ws.prepareCellStyle(col, row, c.S)
c.T, c.V = setCellBool(value)
c.IS = nil
Expand Down Expand Up @@ -351,16 +357,19 @@ func setCellBool(value bool) (t string, v string) {
// var x float32 = 1.325
// f.SetCellFloat("Sheet1", "A1", float64(x), 2, 32)
func (f *File) SetCellFloat(sheet, cell string, value float64, precision, bitSize int) error {
f.mu.Lock()
ws, err := f.workSheetReader(sheet)
if err != nil {
f.mu.Unlock()
return err
}
f.mu.Unlock()
ws.mu.Lock()
defer ws.mu.Unlock()
c, col, row, err := ws.prepareCell(cell)
if err != nil {
return err
}
ws.mu.Lock()
defer ws.mu.Unlock()
c.S = ws.prepareCellStyle(col, row, c.S)
c.T, c.V = setCellFloat(value, precision, bitSize)
c.IS = nil
Expand All @@ -377,16 +386,19 @@ func setCellFloat(value float64, precision, bitSize int) (t string, v string) {
// SetCellStr provides a function to set string type value of a cell. Total
// number of characters that a cell can contain 32767 characters.
func (f *File) SetCellStr(sheet, cell, value string) error {
f.mu.Lock()
ws, err := f.workSheetReader(sheet)
if err != nil {
f.mu.Unlock()
return err
}
f.mu.Unlock()
ws.mu.Lock()
defer ws.mu.Unlock()
c, col, row, err := ws.prepareCell(cell)
if err != nil {
return err
}
ws.mu.Lock()
defer ws.mu.Unlock()
c.S = ws.prepareCellStyle(col, row, c.S)
if c.T, c.V, err = f.setCellString(value); err != nil {
return err
Expand Down Expand Up @@ -558,8 +570,6 @@ func (c *xlsxC) getCellDate(f *File, raw bool) (string, error) {
// intended to be used with for range on rows an argument with the spreadsheet
// opened file.
func (c *xlsxC) getValueFrom(f *File, d *xlsxSST, raw bool) (string, error) {
f.mu.Lock()
defer f.mu.Unlock()
switch c.T {
case "b":
return c.getCellBool(f, raw)
Expand Down Expand Up @@ -596,16 +606,19 @@ func (c *xlsxC) getValueFrom(f *File, d *xlsxSST, raw bool) (string, error) {
// SetCellDefault provides a function to set string type value of a cell as
// default format without escaping the cell.
func (f *File) SetCellDefault(sheet, cell, value string) error {
f.mu.Lock()
ws, err := f.workSheetReader(sheet)
if err != nil {
f.mu.Unlock()
return err
}
f.mu.Unlock()
ws.mu.Lock()
defer ws.mu.Unlock()
c, col, row, err := ws.prepareCell(cell)
if err != nil {
return err
}
ws.mu.Lock()
defer ws.mu.Unlock()
c.S = ws.prepareCellStyle(col, row, c.S)
c.setCellDefault(value)
return f.removeFormula(c, ws, sheet)
Expand Down Expand Up @@ -1264,8 +1277,6 @@ func (ws *xlsxWorksheet) prepareCell(cell string) (*xlsxC, int, int, error) {
}

ws.prepareSheetXML(col, row)
ws.mu.Lock()
defer ws.mu.Unlock()
return &ws.SheetData.Row[row-1].C[col-1], col, row, err
}

Expand Down
2 changes: 1 addition & 1 deletion cell_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func TestConcurrency(t *testing.T) {
// Concurrency iterate columns
cols, err := f.Cols("Sheet1")
assert.NoError(t, err)
for rows.Next() {
for cols.Next() {
_, err := cols.Rows()
assert.NoError(t, err)
}
Expand Down
24 changes: 20 additions & 4 deletions col.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,10 +257,13 @@ func (f *File) GetColVisible(sheet, col string) (bool, error) {
if err != nil {
return true, err
}
f.mu.Lock()
ws, err := f.workSheetReader(sheet)
if err != nil {
f.mu.Unlock()
return false, err
}
f.mu.Unlock()
ws.mu.Lock()
defer ws.mu.Unlock()
if ws.Cols == nil {
Expand Down Expand Up @@ -428,20 +431,24 @@ func (f *File) SetColStyle(sheet, columns string, styleID int) error {
if err != nil {
return err
}
f.mu.Lock()
s, err := f.stylesReader()
if err != nil {
f.mu.Unlock()
return err
}
ws, err := f.workSheetReader(sheet)
if err != nil {
f.mu.Unlock()
return err
}
f.mu.Unlock()
s.mu.Lock()
if styleID < 0 || s.CellXfs == nil || len(s.CellXfs.Xf) <= styleID {
s.mu.Unlock()
return newInvalidStyleID(styleID)
}
s.mu.Unlock()
ws, err := f.workSheetReader(sheet)
if err != nil {
return err
}
ws.mu.Lock()
if ws.Cols == nil {
ws.Cols = &xlsxCols{}
Expand Down Expand Up @@ -484,10 +491,13 @@ func (f *File) SetColWidth(sheet, startCol, endCol string, width float64) error
if width > MaxColumnWidth {
return ErrColumnWidth
}
f.mu.Lock()
ws, err := f.workSheetReader(sheet)
if err != nil {
f.mu.Unlock()
return err
}
f.mu.Unlock()
ws.mu.Lock()
defer ws.mu.Unlock()
col := xlsxCol{
Expand Down Expand Up @@ -659,10 +669,13 @@ func (f *File) GetColStyle(sheet, col string) (int, error) {
if err != nil {
return styleID, err
}
f.mu.Lock()
ws, err := f.workSheetReader(sheet)
if err != nil {
f.mu.Unlock()
return styleID, err
}
f.mu.Unlock()
ws.mu.Lock()
defer ws.mu.Unlock()
if ws.Cols != nil {
Expand All @@ -682,10 +695,13 @@ func (f *File) GetColWidth(sheet, col string) (float64, error) {
if err != nil {
return defaultColWidth, err
}
f.mu.Lock()
ws, err := f.workSheetReader(sheet)
if err != nil {
f.mu.Unlock()
return defaultColWidth, err
}
f.mu.Unlock()
ws.mu.Lock()
defer ws.mu.Unlock()
if ws.Cols != nil {
Expand Down
2 changes: 0 additions & 2 deletions excelize.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,6 @@ func (f *File) setDefaultTimeStyle(sheet, cell string, format int) error {
// workSheetReader provides a function to get the pointer to the structure
// after deserialization by given worksheet name.
func (f *File) workSheetReader(sheet string) (ws *xlsxWorksheet, err error) {
f.mu.Lock()
defer f.mu.Unlock()
var (
name string
ok bool
Expand Down
5 changes: 4 additions & 1 deletion numfmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -948,10 +948,13 @@ func (nf *numberFormat) zeroHandler() string {
// textHandler will be handling text selection for a number format expression.
func (nf *numberFormat) textHandler() (result string) {
for _, token := range nf.section[nf.sectionIdx].Items {
if inStrSlice([]string{nfp.TokenTypeDateTimes, nfp.TokenTypeElapsedDateTimes}, token.TType, false) != -1 {
return nf.value
}
if token.TType == nfp.TokenTypeLiteral {
result += token.TValue
}
if token.TType == nfp.TokenTypeTextPlaceHolder || token.TType == nfp.TokenTypeZeroPlaceHolder {
if token.TType == nfp.TokenTypeGeneral || token.TType == nfp.TokenTypeTextPlaceHolder || token.TType == nfp.TokenTypeZeroPlaceHolder {
result += nf.value
}
}
Expand Down
8 changes: 8 additions & 0 deletions numfmt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1008,4 +1008,12 @@ func TestNumFmt(t *testing.T) {
result := format(item[0], item[1], false, CellTypeNumber)
assert.Equal(t, item[2], result, item)
}
for _, item := range [][]string{
{"1234.5678", "General", "1234.5678"},
{"1234.5678", "yyyy\"\"m\"\"d\"\";@", "1234.5678"},
{"1234.5678", "0_);[Red]\\(0\\)", "1234.5678"},
} {
result := format(item[0], item[1], false, CellTypeSharedString)
assert.Equal(t, item[2], result, item)
}
}
6 changes: 6 additions & 0 deletions picture.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,10 +212,13 @@ func (f *File) AddPictureFromBytes(sheet, cell string, pic *Picture) error {
return err
}
// Read sheet data.
f.mu.Lock()
ws, err := f.workSheetReader(sheet)
if err != nil {
f.mu.Unlock()
return err
}
f.mu.Unlock()
ws.mu.Lock()
// Add first picture for given sheet, create xl/drawings/ and xl/drawings/_rels/ folder.
drawingID := f.countDrawings() + 1
Expand Down Expand Up @@ -591,10 +594,13 @@ func (f *File) GetPictures(sheet, cell string) ([]Picture, error) {
}
col--
row--
f.mu.Lock()
ws, err := f.workSheetReader(sheet)
if err != nil {
f.mu.Unlock()
return nil, err
}
f.mu.Unlock()
if ws.Drawing == nil {
return nil, err
}
Expand Down
6 changes: 1 addition & 5 deletions sheet.go
Original file line number Diff line number Diff line change
Expand Up @@ -1843,8 +1843,6 @@ func (f *File) relsReader(path string) (*xlsxRelationships, error) {
// row to accept data. Missing rows are backfilled and given their row number
// Uses the last populated row as a hint for the size of the next row to add
func (ws *xlsxWorksheet) prepareSheetXML(col int, row int) {
ws.mu.Lock()
defer ws.mu.Unlock()
rowCount := len(ws.SheetData.Row)
sizeHint := 0
var ht *float64
Expand Down Expand Up @@ -1878,9 +1876,7 @@ func fillColumns(rowData *xlsxRow, col, row int) {
}

// makeContiguousColumns make columns in specific rows as contiguous.
func makeContiguousColumns(ws *xlsxWorksheet, fromRow, toRow, colCount int) {
ws.mu.Lock()
defer ws.mu.Unlock()
func (ws *xlsxWorksheet) makeContiguousColumns(fromRow, toRow, colCount int) {
for ; fromRow < toRow; fromRow++ {
rowData := &ws.SheetData.Row[fromRow-1]
fillColumns(rowData, colCount, fromRow)
Expand Down
29 changes: 21 additions & 8 deletions styles.go
Original file line number Diff line number Diff line change
Expand Up @@ -2004,10 +2004,13 @@ func (f *File) NewStyle(style *Style) (int, error) {
if fs.DecimalPlaces == 0 {
fs.DecimalPlaces = 2
}
f.mu.Lock()
s, err := f.stylesReader()
if err != nil {
f.mu.Unlock()
return cellXfsID, err
}
f.mu.Unlock()
s.mu.Lock()
defer s.mu.Unlock()
// check given style already exist.
Expand Down Expand Up @@ -2131,10 +2134,13 @@ func (f *File) getStyleID(ss *xlsxStyleSheet, style *Style) (int, error) {
// format by given style format. The parameters are the same with the NewStyle
// function.
func (f *File) NewConditionalStyle(style *Style) (int, error) {
f.mu.Lock()
s, err := f.stylesReader()
if err != nil {
f.mu.Unlock()
return 0, err
}
f.mu.Unlock()
fs, err := parseFormatStyleSet(style)
if err != nil {
return 0, err
Expand Down Expand Up @@ -2179,7 +2185,9 @@ func (f *File) SetDefaultFont(fontName string) error {
return err
}
font.Name.Val = stringPtr(fontName)
f.mu.Lock()
s, _ := f.stylesReader()
f.mu.Unlock()
s.Fonts.Font[0] = font
custom := true
s.CellStyles.CellStyle[0].CustomBuiltIn = &custom
Expand All @@ -2188,6 +2196,8 @@ func (f *File) SetDefaultFont(fontName string) error {

// readDefaultFont provides an un-marshalled font value.
func (f *File) readDefaultFont() (*xlsxFont, error) {
f.mu.Lock()
defer f.mu.Unlock()
s, err := f.stylesReader()
if err != nil {
return nil, err
Expand Down Expand Up @@ -2803,22 +2813,25 @@ func (f *File) SetCellStyle(sheet, hCell, vCell string, styleID int) error {

vColIdx := vCol - 1
vRowIdx := vRow - 1

f.mu.Lock()
ws, err := f.workSheetReader(sheet)
if err != nil {
f.mu.Unlock()
return err
}
ws.prepareSheetXML(vCol, vRow)
makeContiguousColumns(ws, hRow, vRow, vCol)
ws.mu.Lock()
defer ws.mu.Unlock()

s, err := f.stylesReader()
if err != nil {
f.mu.Unlock()
return err
}
s.mu.Lock()
defer s.mu.Unlock()
f.mu.Unlock()

ws.mu.Lock()
defer ws.mu.Unlock()

ws.prepareSheetXML(vCol, vRow)
ws.makeContiguousColumns(hRow, vRow, vCol)

if styleID < 0 || s.CellXfs == nil || len(s.CellXfs.Xf) <= styleID {
return newInvalidStyleID(styleID)
}
Expand Down

0 comments on commit 612f6f1

Please sign in to comment.