Skip to content

Commit d9a0da7

Browse files
xurixuri
committed
This closes qax-os#1687 and closes qax-os#1688
- Using sync map internally to get cell value concurrency safe - Support set the height and width for the comment box - Update the unit test
1 parent d133dc1 commit d9a0da7

17 files changed

+99
-84
lines changed

‎cell.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -1317,10 +1317,15 @@ func (ws *xlsxWorksheet) prepareCell(cell string) (*xlsxC, int, int, error) {
13171317
// value function. Passed function implements specific part of required
13181318
// logic.
13191319
func (f *File) getCellStringFunc(sheet, cell string, fn func(x *xlsxWorksheet, c *xlsxC) (string, bool, error)) (string, error) {
1320+
f.mu.Lock()
13201321
ws, err := f.workSheetReader(sheet)
13211322
if err != nil {
1323+
f.mu.Unlock()
13221324
return "", err
13231325
}
1326+
f.mu.Unlock()
1327+
ws.mu.Lock()
1328+
defer ws.mu.Unlock()
13241329
cell, err = ws.mergeCellsParser(cell)
13251330
if err != nil {
13261331
return "", err
@@ -1329,10 +1334,6 @@ func (f *File) getCellStringFunc(sheet, cell string, fn func(x *xlsxWorksheet, c
13291334
if err != nil {
13301335
return "", err
13311336
}
1332-
1333-
ws.mu.Lock()
1334-
defer ws.mu.Unlock()
1335-
13361337
lastRowNum := 0
13371338
if l := len(ws.SheetData.Row); l > 0 {
13381339
lastRowNum = ws.SheetData.Row[l-1].R

‎cell_test.go

+7-7
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ func TestGetCellValue(t *testing.T) {
313313

314314
f.Sheet.Delete("xl/worksheets/sheet1.xml")
315315
f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(fmt.Sprintf(sheetData, `<row r="3"><c t="inlineStr"><is><t>A3</t></is></c></row><row><c t="inlineStr"><is><t>A4</t></is></c><c t="inlineStr"><is><t>B4</t></is></c></row><row r="7"><c t="inlineStr"><is><t>A7</t></is></c><c t="inlineStr"><is><t>B7</t></is></c></row><row><c t="inlineStr"><is><t>A8</t></is></c><c t="inlineStr"><is><t>B8</t></is></c></row>`)))
316-
f.checked = nil
316+
f.checked = sync.Map{}
317317
cells := []string{"A3", "A4", "B4", "A7", "B7"}
318318
rows, err := f.GetRows("Sheet1")
319319
assert.Equal(t, [][]string{nil, nil, {"A3"}, {"A4", "B4"}, nil, nil, {"A7", "B7"}, {"A8", "B8"}}, rows)
@@ -329,35 +329,35 @@ func TestGetCellValue(t *testing.T) {
329329

330330
f.Sheet.Delete("xl/worksheets/sheet1.xml")
331331
f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(fmt.Sprintf(sheetData, `<row r="2"><c r="A2" t="inlineStr"><is><t>A2</t></is></c></row><row r="2"><c r="B2" t="inlineStr"><is><t>B2</t></is></c></row>`)))
332-
f.checked = nil
332+
f.checked = sync.Map{}
333333
cell, err := f.GetCellValue("Sheet1", "A2")
334334
assert.Equal(t, "A2", cell)
335335
assert.NoError(t, err)
336336

337337
f.Sheet.Delete("xl/worksheets/sheet1.xml")
338338
f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(fmt.Sprintf(sheetData, `<row r="2"><c r="A2" t="inlineStr"><is><t>A2</t></is></c></row><row r="2"><c r="B2" t="inlineStr"><is><t>B2</t></is></c></row>`)))
339-
f.checked = nil
339+
f.checked = sync.Map{}
340340
rows, err = f.GetRows("Sheet1")
341341
assert.Equal(t, [][]string{nil, {"A2", "B2"}}, rows)
342342
assert.NoError(t, err)
343343

344344
f.Sheet.Delete("xl/worksheets/sheet1.xml")
345345
f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(fmt.Sprintf(sheetData, `<row r="1"><c r="A1" t="inlineStr"><is><t>A1</t></is></c></row><row r="1"><c r="B1" t="inlineStr"><is><t>B1</t></is></c></row>`)))
346-
f.checked = nil
346+
f.checked = sync.Map{}
347347
rows, err = f.GetRows("Sheet1")
348348
assert.Equal(t, [][]string{{"A1", "B1"}}, rows)
349349
assert.NoError(t, err)
350350

351351
f.Sheet.Delete("xl/worksheets/sheet1.xml")
352352
f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(fmt.Sprintf(sheetData, `<row><c t="inlineStr"><is><t>A3</t></is></c></row><row><c t="inlineStr"><is><t>A4</t></is></c><c t="inlineStr"><is><t>B4</t></is></c></row><row r="7"><c t="inlineStr"><is><t>A7</t></is></c><c t="inlineStr"><is><t>B7</t></is></c></row><row><c t="inlineStr"><is><t>A8</t></is></c><c t="inlineStr"><is><t>B8</t></is></c></row>`)))
353-
f.checked = nil
353+
f.checked = sync.Map{}
354354
rows, err = f.GetRows("Sheet1")
355355
assert.Equal(t, [][]string{{"A3"}, {"A4", "B4"}, nil, nil, nil, nil, {"A7", "B7"}, {"A8", "B8"}}, rows)
356356
assert.NoError(t, err)
357357

358358
f.Sheet.Delete("xl/worksheets/sheet1.xml")
359359
f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(fmt.Sprintf(sheetData, `<row r="0"><c r="H6" t="inlineStr"><is><t>H6</t></is></c><c r="A1" t="inlineStr"><is><t>r0A6</t></is></c><c r="F4" t="inlineStr"><is><t>F4</t></is></c></row><row><c r="A1" t="inlineStr"><is><t>A6</t></is></c><c r="B1" t="inlineStr"><is><t>B6</t></is></c><c r="C1" t="inlineStr"><is><t>C6</t></is></c></row><row r="3"><c r="A3"><v>100</v></c><c r="B3" t="inlineStr"><is><t>B3</t></is></c></row>`)))
360-
f.checked = nil
360+
f.checked = sync.Map{}
361361
cell, err = f.GetCellValue("Sheet1", "H6")
362362
assert.Equal(t, "H6", cell)
363363
assert.NoError(t, err)
@@ -410,7 +410,7 @@ func TestGetCellValue(t *testing.T) {
410410
<row r="34"><c r="A34" t="d"><v>20221022T150529Z</v></c></row>
411411
<row r="35"><c r="A35" t="d"><v>2022-10-22T15:05:29Z</v></c></row>
412412
<row r="36"><c r="A36" t="d"><v>2020-07-10 15:00:00.000</v></c></row>`)))
413-
f.checked = nil
413+
f.checked = sync.Map{}
414414
rows, err = f.GetCols("Sheet1")
415415
assert.Equal(t, []string{
416416
"2422.3",

‎col_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package excelize
22

33
import (
44
"path/filepath"
5+
"sync"
56
"testing"
67

78
"github.com/stretchr/testify/assert"
@@ -125,7 +126,7 @@ func TestGetColsError(t *testing.T) {
125126
f = NewFile()
126127
f.Sheet.Delete("xl/worksheets/sheet1.xml")
127128
f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(`<worksheet><sheetData><row r="A"><c r="2" t="inlineStr"><is><t>B</t></is></c></row></sheetData></worksheet>`))
128-
f.checked = nil
129+
f.checked = sync.Map{}
129130
_, err = f.GetCols("Sheet1")
130131
assert.EqualError(t, err, `strconv.Atoi: parsing "A": invalid syntax`)
131132

‎excelize.go

+12-11
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ import (
3030
type File struct {
3131
mu sync.Mutex
3232
options *Options
33-
xmlAttr map[string][]xml.Attr
34-
checked map[string]bool
33+
xmlAttr sync.Map
34+
checked sync.Map
3535
sheetMap map[string]string
3636
streams map[string]*StreamWriter
3737
tempFiles sync.Map
@@ -133,8 +133,8 @@ func OpenFile(filename string, opts ...Options) (*File, error) {
133133
func newFile() *File {
134134
return &File{
135135
options: &Options{UnzipSizeLimit: UnzipSizeLimit, UnzipXMLSizeLimit: StreamChunkSize},
136-
xmlAttr: make(map[string][]xml.Attr),
137-
checked: make(map[string]bool),
136+
xmlAttr: sync.Map{},
137+
checked: sync.Map{},
138138
sheetMap: make(map[string]string),
139139
tempFiles: sync.Map{},
140140
Comments: make(map[string]*xlsxComments),
@@ -275,24 +275,25 @@ func (f *File) workSheetReader(sheet string) (ws *xlsxWorksheet, err error) {
275275
}
276276
}
277277
ws = new(xlsxWorksheet)
278-
if _, ok := f.xmlAttr[name]; !ok {
278+
if attrs, ok := f.xmlAttr.Load(name); !ok {
279279
d := f.xmlNewDecoder(bytes.NewReader(namespaceStrictToTransitional(f.readBytes(name))))
280-
f.xmlAttr[name] = append(f.xmlAttr[name], getRootElement(d)...)
280+
if attrs == nil {
281+
attrs = []xml.Attr{}
282+
}
283+
attrs = append(attrs.([]xml.Attr), getRootElement(d)...)
284+
f.xmlAttr.Store(name, attrs)
281285
}
282286
if err = f.xmlNewDecoder(bytes.NewReader(namespaceStrictToTransitional(f.readBytes(name)))).
283287
Decode(ws); err != nil && err != io.EOF {
284288
return
285289
}
286290
err = nil
287-
if f.checked == nil {
288-
f.checked = make(map[string]bool)
289-
}
290-
if ok = f.checked[name]; !ok {
291+
if _, ok = f.checked.Load(name); !ok {
291292
ws.checkSheet()
292293
if err = ws.checkRow(); err != nil {
293294
return
294295
}
295-
f.checked[name] = true
296+
f.checked.Store(name, true)
296297
}
297298
f.Sheet.Store(name, ws)
298299
return

‎excelize_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"path/filepath"
1717
"strconv"
1818
"strings"
19+
"sync"
1920
"testing"
2021
"time"
2122

@@ -1531,7 +1532,7 @@ func TestWorkSheetReader(t *testing.T) {
15311532
f = NewFile()
15321533
f.Sheet.Delete("xl/worksheets/sheet1.xml")
15331534
f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(`<worksheet xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main"><sheetData/></worksheet>`))
1534-
f.checked = nil
1535+
f.checked = sync.Map{}
15351536
_, err = f.workSheetReader("Sheet1")
15361537
assert.NoError(t, err)
15371538
}

‎lib.go

+21-12
Original file line numberDiff line numberDiff line change
@@ -682,8 +682,8 @@ func getXMLNamespace(space string, attr []xml.Attr) string {
682682
func (f *File) replaceNameSpaceBytes(path string, contentMarshal []byte) []byte {
683683
sourceXmlns := []byte(`xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main">`)
684684
targetXmlns := []byte(templateNamespaceIDMap)
685-
if attr, ok := f.xmlAttr[path]; ok {
686-
targetXmlns = []byte(genXMLNamespace(attr))
685+
if attrs, ok := f.xmlAttr.Load(path); ok {
686+
targetXmlns = []byte(genXMLNamespace(attrs.([]xml.Attr)))
687687
}
688688
return bytesReplace(contentMarshal, sourceXmlns, bytes.ReplaceAll(targetXmlns, []byte(" mc:Ignorable=\"r\""), []byte{}), -1)
689689
}
@@ -694,29 +694,36 @@ func (f *File) addNameSpaces(path string, ns xml.Attr) {
694694
exist := false
695695
mc := false
696696
ignore := -1
697-
if attr, ok := f.xmlAttr[path]; ok {
698-
for i, attribute := range attr {
699-
if attribute.Name.Local == ns.Name.Local && attribute.Name.Space == ns.Name.Space {
697+
if attrs, ok := f.xmlAttr.Load(path); ok {
698+
for i, attr := range attrs.([]xml.Attr) {
699+
if attr.Name.Local == ns.Name.Local && attr.Name.Space == ns.Name.Space {
700700
exist = true
701701
}
702-
if attribute.Name.Local == "Ignorable" && getXMLNamespace(attribute.Name.Space, attr) == "mc" {
702+
if attr.Name.Local == "Ignorable" && getXMLNamespace(attr.Name.Space, attrs.([]xml.Attr)) == "mc" {
703703
ignore = i
704704
}
705-
if attribute.Name.Local == "mc" && attribute.Name.Space == "xmlns" {
705+
if attr.Name.Local == "mc" && attr.Name.Space == "xmlns" {
706706
mc = true
707707
}
708708
}
709709
}
710710
if !exist {
711-
f.xmlAttr[path] = append(f.xmlAttr[path], ns)
711+
attrs, _ := f.xmlAttr.Load(path)
712+
if attrs == nil {
713+
attrs = []xml.Attr{}
714+
}
715+
attrs = append(attrs.([]xml.Attr), ns)
716+
f.xmlAttr.Store(path, attrs)
712717
if !mc {
713-
f.xmlAttr[path] = append(f.xmlAttr[path], SourceRelationshipCompatibility)
718+
attrs = append(attrs.([]xml.Attr), SourceRelationshipCompatibility)
719+
f.xmlAttr.Store(path, attrs)
714720
}
715721
if ignore == -1 {
716-
f.xmlAttr[path] = append(f.xmlAttr[path], xml.Attr{
722+
attrs = append(attrs.([]xml.Attr), xml.Attr{
717723
Name: xml.Name{Local: "Ignorable", Space: "mc"},
718724
Value: ns.Name.Local,
719725
})
726+
f.xmlAttr.Store(path, attrs)
720727
return
721728
}
722729
f.setIgnorableNameSpace(path, ignore, ns)
@@ -727,8 +734,10 @@ func (f *File) addNameSpaces(path string, ns xml.Attr) {
727734
// by the given attribute.
728735
func (f *File) setIgnorableNameSpace(path string, index int, ns xml.Attr) {
729736
ignorableNS := []string{"c14", "cdr14", "a14", "pic14", "x14", "xdr14", "x14ac", "dsp", "mso14", "dgm14", "x15", "x12ac", "x15ac", "xr", "xr2", "xr3", "xr4", "xr5", "xr6", "xr7", "xr8", "xr9", "xr10", "xr11", "xr12", "xr13", "xr14", "xr15", "x15", "x16", "x16r2", "mo", "mx", "mv", "o", "v"}
730-
if inStrSlice(strings.Fields(f.xmlAttr[path][index].Value), ns.Name.Local, true) == -1 && inStrSlice(ignorableNS, ns.Name.Local, true) != -1 {
731-
f.xmlAttr[path][index].Value = strings.TrimSpace(fmt.Sprintf("%s %s", f.xmlAttr[path][index].Value, ns.Name.Local))
737+
xmlAttrs, _ := f.xmlAttr.Load(path)
738+
if inStrSlice(strings.Fields(xmlAttrs.([]xml.Attr)[index].Value), ns.Name.Local, true) == -1 && inStrSlice(ignorableNS, ns.Name.Local, true) != -1 {
739+
xmlAttrs.([]xml.Attr)[index].Value = strings.TrimSpace(fmt.Sprintf("%s %s", xmlAttrs.([]xml.Attr)[index].Value, ns.Name.Local))
740+
f.xmlAttr.Store(path, xmlAttrs)
732741
}
733742
}
734743

‎lib_test.go

+4-2
Original file line numberDiff line numberDiff line change
@@ -294,9 +294,11 @@ func TestGetRootElement(t *testing.T) {
294294

295295
func TestSetIgnorableNameSpace(t *testing.T) {
296296
f := NewFile()
297-
f.xmlAttr["xml_path"] = []xml.Attr{{}}
297+
f.xmlAttr.Store("xml_path", []xml.Attr{{}})
298298
f.setIgnorableNameSpace("xml_path", 0, xml.Attr{Name: xml.Name{Local: "c14"}})
299-
assert.EqualValues(t, "c14", f.xmlAttr["xml_path"][0].Value)
299+
attrs, ok := f.xmlAttr.Load("xml_path")
300+
assert.EqualValues(t, "c14", attrs.([]xml.Attr)[0].Value)
301+
assert.True(t, ok)
300302
}
301303

302304
func TestStack(t *testing.T) {

‎rows_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -944,7 +944,7 @@ func TestCheckRow(t *testing.T) {
944944
f = NewFile()
945945
f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(xml.Header+`<worksheet xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main" ><sheetData><row r="2"><c><v>1</v></c><c r="-"><v>2</v></c><c><v>3</v></c><c><v>4</v></c><c r="M2"><v>5</v></c></row></sheetData></worksheet>`))
946946
f.Sheet.Delete("xl/worksheets/sheet1.xml")
947-
delete(f.checked, "xl/worksheets/sheet1.xml")
947+
f.checked.Delete("xl/worksheets/sheet1.xml")
948948
assert.EqualError(t, f.SetCellValue("Sheet1", "A1", false), newCellNameToCoordinatesError("-", newInvalidCellNameError("-")).Error())
949949
}
950950

‎sheet.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -164,10 +164,10 @@ func (f *File) workSheetWriter() {
164164
// reusing buffer
165165
_ = encoder.Encode(sheet)
166166
f.saveFileList(p.(string), replaceRelationshipsBytes(f.replaceNameSpaceBytes(p.(string), buffer.Bytes())))
167-
ok := f.checked[p.(string)]
167+
_, ok := f.checked.Load(p.(string))
168168
if ok {
169169
f.Sheet.Delete(p.(string))
170-
f.checked[p.(string)] = false
170+
f.checked.Store(p.(string), false)
171171
}
172172
buffer.Reset()
173173
}
@@ -237,7 +237,7 @@ func (f *File) setSheet(index int, name string) {
237237
sheetXMLPath := "xl/worksheets/sheet" + strconv.Itoa(index) + ".xml"
238238
f.sheetMap[name] = sheetXMLPath
239239
f.Sheet.Store(sheetXMLPath, &ws)
240-
f.xmlAttr[sheetXMLPath] = []xml.Attr{NameSpaceSpreadSheet}
240+
f.xmlAttr.Store(sheetXMLPath, []xml.Attr{NameSpaceSpreadSheet})
241241
}
242242

243243
// relsWriter provides a function to save relationships after
@@ -583,7 +583,7 @@ func (f *File) DeleteSheet(sheet string) error {
583583
f.Pkg.Delete(rels)
584584
f.Relationships.Delete(rels)
585585
f.Sheet.Delete(sheetXML)
586-
delete(f.xmlAttr, sheetXML)
586+
f.xmlAttr.Delete(sheetXML)
587587
f.SheetCount--
588588
}
589589
index, err := f.GetSheetIndex(activeSheetName)
@@ -714,8 +714,8 @@ func (f *File) copySheet(from, to int) error {
714714
f.Pkg.Store(toRels, rels.([]byte))
715715
}
716716
fromSheetXMLPath, _ := f.getSheetXMLPath(fromSheet)
717-
fromSheetAttr := f.xmlAttr[fromSheetXMLPath]
718-
f.xmlAttr[sheetXMLPath] = fromSheetAttr
717+
fromSheetAttr, _ := f.xmlAttr.Load(fromSheetXMLPath)
718+
f.xmlAttr.Store(sheetXMLPath, fromSheetAttr)
719719
return err
720720
}
721721

‎sheet_test.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"path/filepath"
99
"strconv"
1010
"strings"
11+
"sync"
1112
"testing"
1213

1314
"github.com/stretchr/testify/assert"
@@ -120,7 +121,7 @@ func TestPanes(t *testing.T) {
120121

121122
// Test add pane on empty sheet views worksheet
122123
f = NewFile()
123-
f.checked = nil
124+
f.checked = sync.Map{}
124125
f.Sheet.Delete("xl/worksheets/sheet1.xml")
125126
f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(`<worksheet xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main"><sheetData/></worksheet>`))
126127
assert.NoError(t, f.SetPanes("Sheet1",
@@ -173,7 +174,7 @@ func TestSearchSheet(t *testing.T) {
173174
f = NewFile()
174175
f.Sheet.Delete("xl/worksheets/sheet1.xml")
175176
f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(`<worksheet><sheetData><row r="A"><c r="2" t="inlineStr"><is><t>A</t></is></c></row></sheetData></worksheet>`))
176-
f.checked = nil
177+
f.checked = sync.Map{}
177178
result, err = f.SearchSheet("Sheet1", "A")
178179
assert.EqualError(t, err, "strconv.Atoi: parsing \"A\": invalid syntax")
179180
assert.Equal(t, []string(nil), result)
@@ -462,7 +463,7 @@ func TestWorksheetWriter(t *testing.T) {
462463
f.Sheet.Delete("xl/worksheets/sheet1.xml")
463464
worksheet := xml.Header + `<worksheet xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main" xmlns:r="http://schemas.openxmlformats.org/officeDocument/2006/relationships"><sheetData><row r="1"><c r="A1"><v>%d</v></c></row></sheetData><mc:AlternateContent xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"><mc:Choice xmlns:a14="http://schemas.microsoft.com/office/drawing/2010/main" Requires="a14"><xdr:twoCellAnchor editAs="oneCell"></xdr:twoCellAnchor></mc:Choice><mc:Fallback/></mc:AlternateContent></worksheet>`
464465
f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(fmt.Sprintf(worksheet, 1)))
465-
f.checked = nil
466+
f.checked = sync.Map{}
466467
assert.NoError(t, f.SetCellValue("Sheet1", "A1", 2))
467468
f.workSheetWriter()
468469
value, ok := f.Pkg.Load("xl/worksheets/sheet1.xml")

‎stream.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -678,7 +678,7 @@ func (sw *StreamWriter) Flush() error {
678678

679679
sheetPath := sw.file.sheetMap[sw.Sheet]
680680
sw.file.Sheet.Delete(sheetPath)
681-
delete(sw.file.checked, sheetPath)
681+
sw.file.checked.Delete(sheetPath)
682682
sw.file.Pkg.Delete(sheetPath)
683683

684684
return nil

‎styles.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -2161,7 +2161,7 @@ func (f *File) SetCellStyle(sheet, hCell, vCell string, styleID int) error {
21612161
//
21622162
// The 'Criteria' parameter is used to set the criteria by which the cell data
21632163
// will be evaluated. It has no default value. The most common criteria as
2164-
// applied to {"type"："cell"} are:
2164+
// applied to {Type: "cell"} are:
21652165
//
21662166
// between |
21672167
// not between |

0 commit comments

Comments
 (0)