Skip to content

Commit

Permalink
Expand error messages and fix shared strings parse failure (#39)
Browse files Browse the repository at this point in the history
  • Loading branch information
dglsparsons authored Aug 12, 2021
1 parent 8e6e706 commit 4b61f02
Show file tree
Hide file tree
Showing 9 changed files with 68 additions and 61 deletions.
3 changes: 2 additions & 1 deletion date.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package xlsxreader

import (
"fmt"
"math"
"strconv"
"time"
Expand All @@ -23,7 +24,7 @@ var excelEpoch = time.Date(1899, 12, 30, 0, 0, 0, 0, time.UTC)
func convertExcelDateToDateString(value string) (string, error) {
floatValue, err := strconv.ParseFloat(value, 64)
if err != nil {
return "", err
return "", fmt.Errorf("unable to parse date float value: %w", err)
}

numberOfDays := math.Trunc(floatValue)
Expand Down
20 changes: 10 additions & 10 deletions file.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,14 @@ func getFileForName(files []*zip.File, name string) (*zip.File, error) {
func readFile(file *zip.File) ([]byte, error) {
rc, err := file.Open()
if err != nil {
return []byte{}, err
return []byte{}, fmt.Errorf("unable to open file: %w", err)
}
defer rc.Close()

buff := bytes.NewBuffer(nil)
_, err = io.Copy(buff, rc)
if err != nil {
return []byte{}, err
return []byte{}, fmt.Errorf("unable to copy bytes: %w", err)
}
return buff.Bytes(), nil
}
Expand All @@ -66,13 +66,13 @@ func (xl *XlsxFileCloser) Close() error {
func OpenFile(filename string) (*XlsxFileCloser, error) {
zipFile, err := zip.OpenReader(filename)
if err != nil {
return nil, err
return nil, fmt.Errorf("unable to open file reader: %w", err)
}

x := XlsxFile{}
if err := x.init(&zipFile.Reader); err != nil {
zipFile.Close()
return nil, err
return nil, fmt.Errorf("unable to initialise file: %w", err)
}

return &XlsxFileCloser{
Expand Down Expand Up @@ -105,13 +105,13 @@ func OpenReaderZip(rc *zip.ReadCloser) (*XlsxFileCloser, error) {
func NewReader(xlsxBytes []byte) (*XlsxFile, error) {
r, err := zip.NewReader(bytes.NewReader(xlsxBytes), int64(len(xlsxBytes)))
if err != nil {
return nil, err
return nil, fmt.Errorf("unable to create new reader: %w", err)
}

x := XlsxFile{}
err = x.init(r)
if err != nil {
return nil, err
return nil, fmt.Errorf("unable to initialise file: %w", err)
}

return &x, nil
Expand All @@ -124,7 +124,7 @@ func NewReaderZip(r *zip.Reader) (*XlsxFile, error) {
x := XlsxFile{}

if err := x.init(r); err != nil {
return nil, err
return nil, fmt.Errorf("unable to initialise file: %w", err)
}

return &x, nil
Expand All @@ -133,17 +133,17 @@ func NewReaderZip(r *zip.Reader) (*XlsxFile, error) {
func (x *XlsxFile) init(zipReader *zip.Reader) error {
sharedStrings, err := getSharedStrings(zipReader.File)
if err != nil {
return err
return fmt.Errorf("unable to get shared strings: %w", err)
}

sheets, sheetFiles, err := getWorksheets(zipReader.File)
if err != nil {
return err
return fmt.Errorf("unable to get worksheets: %w", err)
}

dateStyles, err := getDateFormatStyles(zipReader.File)
if err != nil {
return err
return fmt.Errorf("unable to get date styles: %w", err)
}

x.sharedStrings = sharedStrings
Expand Down
4 changes: 2 additions & 2 deletions file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ func TestGettingFileByNameFailure(t *testing.T) {
_, err := getFileForName(zipFiles, "OOPS")

require.EqualError(t, err, "file not found: OOPS")

}

func TestOpeningMissingFile(t *testing.T) {
_, err := OpenFile("this_doesnt_exist.zip")
require.EqualError(t, err, "open this_doesnt_exist.zip: no such file or directory")
require.Error(t, err)
require.Contains(t, err.Error(), "open this_doesnt_exist.zip: no such file or directory")
}

func TestHandlingSpuriousWorkbookLinks(t *testing.T) {
Expand Down
18 changes: 8 additions & 10 deletions rows.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ func (rr *rawRow) unmarshalXML(d *xml.Decoder, start xml.StartElement) error {
var err error

if rr.Index, err = strconv.Atoi(attr.Value); err != nil {
return err
return fmt.Errorf("unable to parse row index: %w", err)
}
}

for {
tok, err := d.Token()
if err != nil {
return err
return fmt.Errorf("error retrieving xml token: %w", err)
}

var se xml.StartElement
Expand All @@ -51,7 +51,7 @@ func (rr *rawRow) unmarshalXML(d *xml.Decoder, start xml.StartElement) error {

var rc rawCell
if err = rc.unmarshalXML(d, se); err != nil {
return err
return fmt.Errorf("unable to unmarshal cell: %w", err)
}

rr.RawCells = append(rr.RawCells, rc)
Expand Down Expand Up @@ -87,7 +87,7 @@ func (rc *rawCell) unmarshalXML(d *xml.Decoder, start xml.StartElement) error {
for {
tok, err := d.Token()
if err != nil {
return err
return fmt.Errorf("error retrieving xml token: %w", err)
}

var se xml.StartElement
Expand Down Expand Up @@ -120,7 +120,7 @@ func (rc *rawCell) unmarshalXML(d *xml.Decoder, start xml.StartElement) error {
}

if err != nil {
return err
return fmt.Errorf("unable to parse cell data: %w", err)
}
}
}
Expand All @@ -129,7 +129,7 @@ func (rc *rawCell) unmarshalInlineString(d *xml.Decoder, start xml.StartElement)
for {
tok, err := d.Token()
if err != nil {
return err
return fmt.Errorf("error retrieving xml token: %w", err)
}

var se xml.StartElement
Expand All @@ -152,7 +152,7 @@ func (rc *rawCell) unmarshalInlineString(d *xml.Decoder, start xml.StartElement)

v, err := getCharData(d)
if err != nil {
return err
return fmt.Errorf("unable to parse string: %w", err)
}

rc.InlineString = &v
Expand Down Expand Up @@ -246,8 +246,7 @@ func (x *XlsxFile) getCellType(r rawCell) CellType {
return TypeDateTime
case "n", "":
return TypeNumerical
case "s",
"inlineStr":
case "s", "inlineStr":
return TypeString
default:
return TypeString
Expand Down Expand Up @@ -284,7 +283,6 @@ func (x *XlsxFile) readSheetRows(sheet string, ch chan<- Row) {
}

switch startElement := token.(type) {

case xml.StartElement:
if startElement.Name.Local == "row" {
row := x.parseRow(decoder, &startElement)
Expand Down
19 changes: 11 additions & 8 deletions rows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,15 @@ var testFile = XlsxFile{
dateStyles: map[int]bool{1: true, 3: true},
}

var inlineStr = "The meaning of life"
var dateValue = "43489.25"
var invalidValue = "wat"
var sharedString = "2"
var offsetTooHighSharedString = "32"
var dateString = "2005-06-04"
var boolString = "1"
var (
inlineStr = "The meaning of life"
dateValue = "43489.25"
invalidValue = "wat"
sharedString = "2"
offsetTooHighSharedString = "32"
dateString = "2005-06-04"
boolString = "1"
)

var cellValueTests = []struct {
Name string
Expand Down Expand Up @@ -100,7 +102,8 @@ func TestGettingValueFromRawCell(t *testing.T) {
val, err := testFile.getCellValue(test.Cell)

if test.Error != "" {
require.EqualError(t, err, test.Error)
require.Error(t, err)
require.Contains(t, err.Error(), test.Error)
} else {
require.NoError(t, err)
require.Equal(t, test.Expected, val)
Expand Down
28 changes: 14 additions & 14 deletions shared_strings.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"archive/zip"
"encoding/xml"
"errors"
"fmt"
"io"
"strconv"
"strings"
Expand All @@ -19,19 +20,19 @@ func (sv *sharedStringsValue) unmarshalXML(d *xml.Decoder, start xml.StartElemen
for {
tok, err := d.Token()
if err != nil {
return err
return fmt.Errorf("error retrieving xml token: %w", err)
}

var se xml.StartElement

switch el := tok.(type) {
case xml.StartElement:
se = el
case xml.EndElement:
if el == start.End() {
return nil
}
continue
case xml.StartElement:
se = el
default:
continue
}
Expand All @@ -46,7 +47,7 @@ func (sv *sharedStringsValue) unmarshalXML(d *xml.Decoder, start xml.StartElemen
}

if err != nil {
return err
return fmt.Errorf("unable to parse string: %w", err)
}
}
}
Expand All @@ -55,7 +56,7 @@ func (sv *sharedStringsValue) decodeRichText(d *xml.Decoder, start xml.StartElem
for {
tok, err := d.Token()
if err != nil {
return err
return fmt.Errorf("unable to get shared strings value token: %w", err)
}

var se xml.StartElement
Expand All @@ -79,7 +80,7 @@ func (sv *sharedStringsValue) decodeRichText(d *xml.Decoder, start xml.StartElem
var s string

if s, err = getCharData(d); err != nil {
return err
return fmt.Errorf("unable to parse string: %w", err)
}

sv.RichText = append(sv.RichText, s)
Expand Down Expand Up @@ -137,12 +138,12 @@ func getSharedStrings(files []*zip.File) ([]string, error) {
return []string{}, nil
}
if err != nil {
return nil, err
return nil, fmt.Errorf("unable to get shared strings file: %w", err)
}

f, err := ssFile.Open()
if err != nil {
return nil, err
return nil, fmt.Errorf("unable to open shared strings file: %w", err)
}

defer f.Close()
Expand All @@ -155,12 +156,11 @@ func getSharedStrings(files []*zip.File) ([]string, error) {
dec := xml.NewDecoder(f)
for {
token, err := dec.Token()
switch err {
case nil:
case io.EOF:
if err == io.EOF {
return sharedStrings, nil
default:
return nil, err
}
if err != nil {
return nil, fmt.Errorf("error decoding token: %w", err)
}

startElement, ok := token.(xml.StartElement)
Expand All @@ -175,7 +175,7 @@ func getSharedStrings(files []*zip.File) ([]string, error) {

value.Reset()
if err := value.unmarshalXML(dec, startElement); err != nil {
return nil, err
return nil, fmt.Errorf("error unmarshaling shared strings value %+v: %w", startElement, err)
}

sharedStrings = append(sharedStrings, value.String())
Expand Down
16 changes: 8 additions & 8 deletions sheets.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,32 +49,32 @@ func getFileNameFromRelationships(rels []relationship, s sheet) (string, error)
func getWorksheets(files []*zip.File) ([]string, *map[string]*zip.File, error) {
wbFile, err := getFileForName(files, "xl/workbook.xml")
if err != nil {
return nil, nil, err
return nil, nil, fmt.Errorf("unable to get workbook file: %w", err)
}
data, err := readFile(wbFile)
if err != nil {
return nil, nil, err
return nil, nil, fmt.Errorf("unable to read workbook file: %w", err)
}

var wb workbook
err = xml.Unmarshal(data, &wb)
if err != nil {
return nil, nil, err
return nil, nil, fmt.Errorf("unable to parse workbook file: %w", err)
}

relsFile, err := getFileForName(files, "xl/_rels/workbook.xml.rels")
if err != nil {
return nil, nil, err
return nil, nil, fmt.Errorf("unable to get relationships file: %w", err)
}
relsData, err := readFile(relsFile)
if err != nil {
return nil, nil, err
return nil, nil, fmt.Errorf("unable to read relationships file: %w", err)
}

rels := relationships{}
err = xml.Unmarshal(relsData, &rels)
if err != nil {
return nil, nil, err
return nil, nil, fmt.Errorf("unable to parse relationships file: %w", err)
}

wsFileMap := map[string]*zip.File{}
Expand All @@ -83,11 +83,11 @@ func getWorksheets(files []*zip.File) ([]string, *map[string]*zip.File, error) {
for i, sheet := range wb.Sheets {
sheetFilename, err := getFileNameFromRelationships(rels.Relationships, sheet)
if err != nil {
return nil, nil, err
return nil, nil, fmt.Errorf("unable to get file name from relationships: %w", err)
}
sheetFile, err := getFileForName(files, sheetFilename)
if err != nil {
return nil, nil, err
return nil, nil, fmt.Errorf("unable to get file for sheet name %s: %w", sheetFilename, err)
}

wsFileMap[sheet.Name] = sheetFile
Expand Down
Loading

0 comments on commit 4b61f02

Please sign in to comment.