Skip to content

Commit

Permalink
This closes qax-os#1687 and closes qax-os#1688
Browse files Browse the repository at this point in the history
- Using sync map internally to get cell value concurrency safe
- Support set the height and width for the comment box
- Update the unit test
  • Loading branch information
xuri committed Oct 10, 2023
1 parent d133dc1 commit d9a0da7
Show file tree
Hide file tree
Showing 17 changed files with 99 additions and 84 deletions.
9 changes: 5 additions & 4 deletions cell.go
Original file line number Diff line number Diff line change
Expand Up @@ -1317,10 +1317,15 @@ func (ws *xlsxWorksheet) prepareCell(cell string) (*xlsxC, int, int, error) {
// value function. Passed function implements specific part of required
// logic.
func (f *File) getCellStringFunc(sheet, cell string, fn func(x *xlsxWorksheet, c *xlsxC) (string, bool, error)) (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()
cell, err = ws.mergeCellsParser(cell)
if err != nil {
return "", err
Expand All @@ -1329,10 +1334,6 @@ func (f *File) getCellStringFunc(sheet, cell string, fn func(x *xlsxWorksheet, c
if err != nil {
return "", err
}

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

lastRowNum := 0
if l := len(ws.SheetData.Row); l > 0 {
lastRowNum = ws.SheetData.Row[l-1].R
Expand Down
14 changes: 7 additions & 7 deletions cell_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ func TestGetCellValue(t *testing.T) {

f.Sheet.Delete("xl/worksheets/sheet1.xml")
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>`)))
f.checked = nil
f.checked = sync.Map{}
cells := []string{"A3", "A4", "B4", "A7", "B7"}
rows, err := f.GetRows("Sheet1")
assert.Equal(t, [][]string{nil, nil, {"A3"}, {"A4", "B4"}, nil, nil, {"A7", "B7"}, {"A8", "B8"}}, rows)
Expand All @@ -329,35 +329,35 @@ func TestGetCellValue(t *testing.T) {

f.Sheet.Delete("xl/worksheets/sheet1.xml")
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>`)))
f.checked = nil
f.checked = sync.Map{}
cell, err := f.GetCellValue("Sheet1", "A2")
assert.Equal(t, "A2", cell)
assert.NoError(t, err)

f.Sheet.Delete("xl/worksheets/sheet1.xml")
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>`)))
f.checked = nil
f.checked = sync.Map{}
rows, err = f.GetRows("Sheet1")
assert.Equal(t, [][]string{nil, {"A2", "B2"}}, rows)
assert.NoError(t, err)

f.Sheet.Delete("xl/worksheets/sheet1.xml")
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>`)))
f.checked = nil
f.checked = sync.Map{}
rows, err = f.GetRows("Sheet1")
assert.Equal(t, [][]string{{"A1", "B1"}}, rows)
assert.NoError(t, err)

f.Sheet.Delete("xl/worksheets/sheet1.xml")
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>`)))
f.checked = nil
f.checked = sync.Map{}
rows, err = f.GetRows("Sheet1")
assert.Equal(t, [][]string{{"A3"}, {"A4", "B4"}, nil, nil, nil, nil, {"A7", "B7"}, {"A8", "B8"}}, rows)
assert.NoError(t, err)

f.Sheet.Delete("xl/worksheets/sheet1.xml")
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>`)))
f.checked = nil
f.checked = sync.Map{}
cell, err = f.GetCellValue("Sheet1", "H6")
assert.Equal(t, "H6", cell)
assert.NoError(t, err)
Expand Down Expand Up @@ -410,7 +410,7 @@ func TestGetCellValue(t *testing.T) {
<row r="34"><c r="A34" t="d"><v>20221022T150529Z</v></c></row>
<row r="35"><c r="A35" t="d"><v>2022-10-22T15:05:29Z</v></c></row>
<row r="36"><c r="A36" t="d"><v>2020-07-10 15:00:00.000</v></c></row>`)))
f.checked = nil
f.checked = sync.Map{}
rows, err = f.GetCols("Sheet1")
assert.Equal(t, []string{
"2422.3",
Expand Down
3 changes: 2 additions & 1 deletion col_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package excelize

import (
"path/filepath"
"sync"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -125,7 +126,7 @@ func TestGetColsError(t *testing.T) {
f = NewFile()
f.Sheet.Delete("xl/worksheets/sheet1.xml")
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>`))
f.checked = nil
f.checked = sync.Map{}
_, err = f.GetCols("Sheet1")
assert.EqualError(t, err, `strconv.Atoi: parsing "A": invalid syntax`)

Expand Down
23 changes: 12 additions & 11 deletions excelize.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ import (
type File struct {
mu sync.Mutex
options *Options
xmlAttr map[string][]xml.Attr
checked map[string]bool
xmlAttr sync.Map
checked sync.Map
sheetMap map[string]string
streams map[string]*StreamWriter
tempFiles sync.Map
Expand Down Expand Up @@ -133,8 +133,8 @@ func OpenFile(filename string, opts ...Options) (*File, error) {
func newFile() *File {
return &File{
options: &Options{UnzipSizeLimit: UnzipSizeLimit, UnzipXMLSizeLimit: StreamChunkSize},
xmlAttr: make(map[string][]xml.Attr),
checked: make(map[string]bool),
xmlAttr: sync.Map{},
checked: sync.Map{},
sheetMap: make(map[string]string),
tempFiles: sync.Map{},
Comments: make(map[string]*xlsxComments),
Expand Down Expand Up @@ -275,24 +275,25 @@ func (f *File) workSheetReader(sheet string) (ws *xlsxWorksheet, err error) {
}
}
ws = new(xlsxWorksheet)
if _, ok := f.xmlAttr[name]; !ok {
if attrs, ok := f.xmlAttr.Load(name); !ok {
d := f.xmlNewDecoder(bytes.NewReader(namespaceStrictToTransitional(f.readBytes(name))))
f.xmlAttr[name] = append(f.xmlAttr[name], getRootElement(d)...)
if attrs == nil {
attrs = []xml.Attr{}
}
attrs = append(attrs.([]xml.Attr), getRootElement(d)...)
f.xmlAttr.Store(name, attrs)
}
if err = f.xmlNewDecoder(bytes.NewReader(namespaceStrictToTransitional(f.readBytes(name)))).
Decode(ws); err != nil && err != io.EOF {
return
}
err = nil
if f.checked == nil {
f.checked = make(map[string]bool)
}
if ok = f.checked[name]; !ok {
if _, ok = f.checked.Load(name); !ok {
ws.checkSheet()
if err = ws.checkRow(); err != nil {
return
}
f.checked[name] = true
f.checked.Store(name, true)
}
f.Sheet.Store(name, ws)
return
Expand Down
3 changes: 2 additions & 1 deletion excelize_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"path/filepath"
"strconv"
"strings"
"sync"
"testing"
"time"

Expand Down Expand Up @@ -1531,7 +1532,7 @@ func TestWorkSheetReader(t *testing.T) {
f = NewFile()
f.Sheet.Delete("xl/worksheets/sheet1.xml")
f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(`<worksheet xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main"><sheetData/></worksheet>`))
f.checked = nil
f.checked = sync.Map{}
_, err = f.workSheetReader("Sheet1")
assert.NoError(t, err)
}
Expand Down
33 changes: 21 additions & 12 deletions lib.go
Original file line number Diff line number Diff line change
Expand Up @@ -682,8 +682,8 @@ func getXMLNamespace(space string, attr []xml.Attr) string {
func (f *File) replaceNameSpaceBytes(path string, contentMarshal []byte) []byte {
sourceXmlns := []byte(`xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main">`)
targetXmlns := []byte(templateNamespaceIDMap)
if attr, ok := f.xmlAttr[path]; ok {
targetXmlns = []byte(genXMLNamespace(attr))
if attrs, ok := f.xmlAttr.Load(path); ok {
targetXmlns = []byte(genXMLNamespace(attrs.([]xml.Attr)))
}
return bytesReplace(contentMarshal, sourceXmlns, bytes.ReplaceAll(targetXmlns, []byte(" mc:Ignorable=\"r\""), []byte{}), -1)
}
Expand All @@ -694,29 +694,36 @@ func (f *File) addNameSpaces(path string, ns xml.Attr) {
exist := false
mc := false
ignore := -1
if attr, ok := f.xmlAttr[path]; ok {
for i, attribute := range attr {
if attribute.Name.Local == ns.Name.Local && attribute.Name.Space == ns.Name.Space {
if attrs, ok := f.xmlAttr.Load(path); ok {
for i, attr := range attrs.([]xml.Attr) {
if attr.Name.Local == ns.Name.Local && attr.Name.Space == ns.Name.Space {
exist = true
}
if attribute.Name.Local == "Ignorable" && getXMLNamespace(attribute.Name.Space, attr) == "mc" {
if attr.Name.Local == "Ignorable" && getXMLNamespace(attr.Name.Space, attrs.([]xml.Attr)) == "mc" {
ignore = i
}
if attribute.Name.Local == "mc" && attribute.Name.Space == "xmlns" {
if attr.Name.Local == "mc" && attr.Name.Space == "xmlns" {
mc = true
}
}
}
if !exist {
f.xmlAttr[path] = append(f.xmlAttr[path], ns)
attrs, _ := f.xmlAttr.Load(path)
if attrs == nil {
attrs = []xml.Attr{}
}
attrs = append(attrs.([]xml.Attr), ns)
f.xmlAttr.Store(path, attrs)
if !mc {
f.xmlAttr[path] = append(f.xmlAttr[path], SourceRelationshipCompatibility)
attrs = append(attrs.([]xml.Attr), SourceRelationshipCompatibility)
f.xmlAttr.Store(path, attrs)
}
if ignore == -1 {
f.xmlAttr[path] = append(f.xmlAttr[path], xml.Attr{
attrs = append(attrs.([]xml.Attr), xml.Attr{
Name: xml.Name{Local: "Ignorable", Space: "mc"},
Value: ns.Name.Local,
})
f.xmlAttr.Store(path, attrs)
return
}
f.setIgnorableNameSpace(path, ignore, ns)
Expand All @@ -727,8 +734,10 @@ func (f *File) addNameSpaces(path string, ns xml.Attr) {
// by the given attribute.
func (f *File) setIgnorableNameSpace(path string, index int, ns xml.Attr) {
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"}
if inStrSlice(strings.Fields(f.xmlAttr[path][index].Value), ns.Name.Local, true) == -1 && inStrSlice(ignorableNS, ns.Name.Local, true) != -1 {
f.xmlAttr[path][index].Value = strings.TrimSpace(fmt.Sprintf("%s %s", f.xmlAttr[path][index].Value, ns.Name.Local))
xmlAttrs, _ := f.xmlAttr.Load(path)
if inStrSlice(strings.Fields(xmlAttrs.([]xml.Attr)[index].Value), ns.Name.Local, true) == -1 && inStrSlice(ignorableNS, ns.Name.Local, true) != -1 {
xmlAttrs.([]xml.Attr)[index].Value = strings.TrimSpace(fmt.Sprintf("%s %s", xmlAttrs.([]xml.Attr)[index].Value, ns.Name.Local))
f.xmlAttr.Store(path, xmlAttrs)
}
}

Expand Down
6 changes: 4 additions & 2 deletions lib_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,9 +294,11 @@ func TestGetRootElement(t *testing.T) {

func TestSetIgnorableNameSpace(t *testing.T) {
f := NewFile()
f.xmlAttr["xml_path"] = []xml.Attr{{}}
f.xmlAttr.Store("xml_path", []xml.Attr{{}})
f.setIgnorableNameSpace("xml_path", 0, xml.Attr{Name: xml.Name{Local: "c14"}})
assert.EqualValues(t, "c14", f.xmlAttr["xml_path"][0].Value)
attrs, ok := f.xmlAttr.Load("xml_path")
assert.EqualValues(t, "c14", attrs.([]xml.Attr)[0].Value)
assert.True(t, ok)
}

func TestStack(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion rows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -944,7 +944,7 @@ func TestCheckRow(t *testing.T) {
f = NewFile()
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>`))
f.Sheet.Delete("xl/worksheets/sheet1.xml")
delete(f.checked, "xl/worksheets/sheet1.xml")
f.checked.Delete("xl/worksheets/sheet1.xml")
assert.EqualError(t, f.SetCellValue("Sheet1", "A1", false), newCellNameToCoordinatesError("-", newInvalidCellNameError("-")).Error())
}

Expand Down
12 changes: 6 additions & 6 deletions sheet.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,10 @@ func (f *File) workSheetWriter() {
// reusing buffer
_ = encoder.Encode(sheet)
f.saveFileList(p.(string), replaceRelationshipsBytes(f.replaceNameSpaceBytes(p.(string), buffer.Bytes())))
ok := f.checked[p.(string)]
_, ok := f.checked.Load(p.(string))
if ok {
f.Sheet.Delete(p.(string))
f.checked[p.(string)] = false
f.checked.Store(p.(string), false)
}
buffer.Reset()
}
Expand Down Expand Up @@ -237,7 +237,7 @@ func (f *File) setSheet(index int, name string) {
sheetXMLPath := "xl/worksheets/sheet" + strconv.Itoa(index) + ".xml"
f.sheetMap[name] = sheetXMLPath
f.Sheet.Store(sheetXMLPath, &ws)
f.xmlAttr[sheetXMLPath] = []xml.Attr{NameSpaceSpreadSheet}
f.xmlAttr.Store(sheetXMLPath, []xml.Attr{NameSpaceSpreadSheet})
}

// relsWriter provides a function to save relationships after
Expand Down Expand Up @@ -583,7 +583,7 @@ func (f *File) DeleteSheet(sheet string) error {
f.Pkg.Delete(rels)
f.Relationships.Delete(rels)
f.Sheet.Delete(sheetXML)
delete(f.xmlAttr, sheetXML)
f.xmlAttr.Delete(sheetXML)
f.SheetCount--
}
index, err := f.GetSheetIndex(activeSheetName)
Expand Down Expand Up @@ -714,8 +714,8 @@ func (f *File) copySheet(from, to int) error {
f.Pkg.Store(toRels, rels.([]byte))
}
fromSheetXMLPath, _ := f.getSheetXMLPath(fromSheet)
fromSheetAttr := f.xmlAttr[fromSheetXMLPath]
f.xmlAttr[sheetXMLPath] = fromSheetAttr
fromSheetAttr, _ := f.xmlAttr.Load(fromSheetXMLPath)
f.xmlAttr.Store(sheetXMLPath, fromSheetAttr)
return err
}

Expand Down
7 changes: 4 additions & 3 deletions sheet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"path/filepath"
"strconv"
"strings"
"sync"
"testing"

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

// Test add pane on empty sheet views worksheet
f = NewFile()
f.checked = nil
f.checked = sync.Map{}
f.Sheet.Delete("xl/worksheets/sheet1.xml")
f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(`<worksheet xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main"><sheetData/></worksheet>`))
assert.NoError(t, f.SetPanes("Sheet1",
Expand Down Expand Up @@ -173,7 +174,7 @@ func TestSearchSheet(t *testing.T) {
f = NewFile()
f.Sheet.Delete("xl/worksheets/sheet1.xml")
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>`))
f.checked = nil
f.checked = sync.Map{}
result, err = f.SearchSheet("Sheet1", "A")
assert.EqualError(t, err, "strconv.Atoi: parsing \"A\": invalid syntax")
assert.Equal(t, []string(nil), result)
Expand Down Expand Up @@ -462,7 +463,7 @@ func TestWorksheetWriter(t *testing.T) {
f.Sheet.Delete("xl/worksheets/sheet1.xml")
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>`
f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(fmt.Sprintf(worksheet, 1)))
f.checked = nil
f.checked = sync.Map{}
assert.NoError(t, f.SetCellValue("Sheet1", "A1", 2))
f.workSheetWriter()
value, ok := f.Pkg.Load("xl/worksheets/sheet1.xml")
Expand Down
2 changes: 1 addition & 1 deletion stream.go
Original file line number Diff line number Diff line change
Expand Up @@ -678,7 +678,7 @@ func (sw *StreamWriter) Flush() error {

sheetPath := sw.file.sheetMap[sw.Sheet]
sw.file.Sheet.Delete(sheetPath)
delete(sw.file.checked, sheetPath)
sw.file.checked.Delete(sheetPath)
sw.file.Pkg.Delete(sheetPath)

return nil
Expand Down
2 changes: 1 addition & 1 deletion styles.go
Original file line number Diff line number Diff line change
Expand Up @@ -2161,7 +2161,7 @@ func (f *File) SetCellStyle(sheet, hCell, vCell string, styleID int) error {
//
// The 'Criteria' parameter is used to set the criteria by which the cell data
// will be evaluated. It has no default value. The most common criteria as
// applied to {"type":"cell"} are:
// applied to {Type: "cell"} are:
//
// between |
// not between |
Expand Down
Loading

0 comments on commit d9a0da7

Please sign in to comment.