Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

confgen: control field name presence at field-level and sheet-level #121

Merged
merged 2 commits into from
Aug 6, 2024
Merged

confgen: control field name presence at field-level and sheet-level #121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
94e6957
confgen: control field name presence at field-level and sheet-level
wenchy Aug 5, 2024
f938857
test: add more unittest cases for field prop: optional
wenchy Aug 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 11 additions & 13 deletions internal/confgen/document_parser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,19 +57,17 @@ func (sp *documentParser) parseMessage(msg protoreflect.Message, node *book.Node
} else {
fieldNode = node.FindChild(field.opts.Name)
if fieldNode == nil {
// if field.opts.Optional {
// // field not found and is optional, no need to process, just return.
// return nil
// }
// kvs := node.DebugNameKV()
// kvs = append(kvs,
// xerrors.KeyPBFieldType, xproto.GetFieldTypeName(fd),
// xerrors.KeyPBFieldName, fd.FullName(),
// xerrors.KeyPBFieldOpts, field.opts,
// )
// return xerrors.WrapKV(xerrors.E2014(field.opts.Name), kvs...)
// TODO: return err when required
return nil
if sp.parser.IsFieldOptional(field) {
// field not found and is optional, just return nil.
return nil
}
kvs := node.DebugNameKV()
kvs = append(kvs,
xerrors.KeyPBFieldType, xproto.GetFieldTypeName(fd),
xerrors.KeyPBFieldName, fd.FullName(),
xerrors.KeyPBFieldOpts, field.opts,
)
return xerrors.WrapKV(xerrors.E2014(field.opts.Name), kvs...)
}
}
fieldPresent, err := sp.parseField(field, msg, fieldNode)
Expand Down
139 changes: 139 additions & 0 deletions internal/confgen/document_parser_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
package confgen

import (
"testing"

"github.com/stretchr/testify/require"
"github.com/tableauio/tableau/format"
"github.com/tableauio/tableau/internal/importer/book"
"github.com/tableauio/tableau/proto/tableaupb/unittestpb"
"github.com/tableauio/tableau/xerrors"
)

var docTestParser *sheetParser

func init() {
docTestParser = NewExtendedSheetParser("protoconf", "Asia/Shanghai", book.MetasheetOptions(),
&SheetParserExtInfo{
InputDir: "",
SubdirRewrites: map[string]string{},
BookFormat: format.YAML,
})
}

func TestDocParser_parseFieldNotFound(t *testing.T) {
type args struct {
sheet *book.Sheet
}
tests := []struct {
name string
parser *sheetParser
args args
wantErr bool
errcode string
}{
{
name: "no duplicate key",
parser: docTestParser,
args: args{
sheet: &book.Sheet{
Name: "YamlScalarConf",
Document: &book.Node{
Kind: book.DocumentNode,
Name: "YamlScalarConf",
Children: []*book.Node{
{
Kind: book.MapNode,
Name: "",
Children: []*book.Node{
{
Name: "ID",
Value: "1",
},
{
Name: "Num",
Value: "10",
},
{
Name: "Value",
Value: "20",
},
{
Name: "Weight",
Value: "30",
},
{
Name: "Percentage",
Value: "0.5",
},
{
Name: "Ratio",
Value: "1.5",
},
{
Name: "Name",
Value: "Apple",
},
{
Name: "Blob",
Value: "VGFibGVhdQ==",
},
{
Name: "OK",
Value: "true",
},
},
},
},
},
},
},
wantErr: false,
},
{
name: "field-not-found",
parser: docTestParser,
args: args{
sheet: &book.Sheet{
Name: "YamlScalarConf",
Document: &book.Node{
Kind: book.DocumentNode,
Name: "YamlScalarConf",
Children: []*book.Node{
{
Kind: book.MapNode,
Name: "",
Children: []*book.Node{
{
Name: "ID",
Value: "1",
},
{
Name: "Num",
Value: "10",
},
},
},
},
},
},
},
wantErr: true,
errcode: "E2014",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := tt.parser.Parse(&unittestpb.YamlScalarConf{}, tt.args.sheet)
if (err != nil) != tt.wantErr {
t.Errorf("sheetParser.Parse() error = %v, wantErr %v", err, tt.wantErr)
}
if err != nil {
if tt.errcode != "" {
desc := xerrors.NewDesc(err)
require.Equal(t, tt.errcode, desc.ErrCode())
}
}
})
}
}
41 changes: 24 additions & 17 deletions internal/confgen/parser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,13 @@
return sp.extInfo.BookFormat
}

// IsFieldOptional returns whether this field is optional (field name existence).
// - table formats (Excel/CSV): field's column can be absent.
// - document formats (XML/YAML): field's name can be absent.
func (sp *sheetParser) IsFieldOptional(field *Field) bool {
return sp.opts.GetOptional() || field.opts.GetProp().GetOptional()
}

func (sp *sheetParser) Parse(protomsg proto.Message, sheet *book.Sheet) error {
if sheet.Document != nil {
docParser := &documentParser{parser: sp}
Expand Down Expand Up @@ -408,7 +415,7 @@
case tableaupb.Layout_LAYOUT_VERTICAL:
if valueFd.Kind() == protoreflect.MessageKind {
keyColName := prefix + field.opts.Name + field.opts.Key
cell, err := rc.Cell(keyColName, field.opts.Optional)
cell, err := rc.Cell(keyColName, sp.IsFieldOptional(field))
if err != nil {
return false, xerrors.WithMessageKV(err, rc.CellDebugKV(keyColName)...)
}
Expand Down Expand Up @@ -456,7 +463,7 @@
value := types.DefaultMapValueOptName // default value name
// key cell
keyColName := prefix + field.opts.Name + key
cell, err := rc.Cell(keyColName, field.opts.Optional)
cell, err := rc.Cell(keyColName, sp.IsFieldOptional(field))

Check warning on line 466 in internal/confgen/parser.go

View check run for this annotation

Codecov / codecov/patch

internal/confgen/parser.go#L466 

Added line #L466 was not covered by tests
if err != nil {
return false, xerrors.WithMessageKV(err, rc.CellDebugKV(keyColName)...)
}
Expand All @@ -468,7 +475,7 @@
newMapKey := fieldValue.MapKey()
// value cell
valueColName := prefix + field.opts.Name + value
cell, err = rc.Cell(valueColName, field.opts.Optional)
cell, err = rc.Cell(valueColName, sp.IsFieldOptional(field))

Check warning on line 478 in internal/confgen/parser.go

View check run for this annotation

Codecov / codecov/patch

internal/confgen/parser.go#L478 

Added line #L478 was not covered by tests
if err != nil {
return false, xerrors.WithMessageKV(err, rc.CellDebugKV(valueColName)...)
}
Expand Down Expand Up @@ -517,7 +524,7 @@
// log.Debug("prefix size: ", size)
for i := 1; i <= size; i++ {
keyColName := prefix + field.opts.Name + strconv.Itoa(i) + field.opts.Key
cell, err := rc.Cell(keyColName, field.opts.Optional)
cell, err := rc.Cell(keyColName, sp.IsFieldOptional(field))
if err != nil {
return false, xerrors.WithMessageKV(err, rc.CellDebugKV(keyColName)...)
}
Expand Down Expand Up @@ -574,7 +581,7 @@

case tableaupb.Layout_LAYOUT_INCELL:
colName := prefix + field.opts.Name
cell, err := rc.Cell(colName, field.opts.Optional)
cell, err := rc.Cell(colName, sp.IsFieldOptional(field))
if err != nil {
return false, xerrors.WithMessageKV(err, rc.CellDebugKV(colName)...)
}
Expand Down Expand Up @@ -849,7 +856,7 @@
if fd == nil {
return false, xerrors.ErrorKV(fmt.Sprintf("key field not found in proto definition: %s", keyProtoName), rc.CellDebugKV(keyColName)...)
}
cell, err := rc.Cell(keyColName, field.opts.Optional)
cell, err := rc.Cell(keyColName, sp.IsFieldOptional(field))
if err != nil {
return false, xerrors.WithMessageKV(err, rc.CellDebugKV(keyColName)...)
}
Expand Down Expand Up @@ -881,7 +888,7 @@
if field.opts.Span == tableaupb.Span_SPAN_INNER_CELL {
// incell-struct list
colName := prefix + field.opts.Name
cell, err := rc.Cell(colName, field.opts.Optional)
cell, err := rc.Cell(colName, sp.IsFieldOptional(field))
if err != nil {
return false, xerrors.WithMessageKV(err, rc.CellDebugKV(colName)...)
}
Expand Down Expand Up @@ -929,7 +936,7 @@
if field.fd.Kind() == protoreflect.MessageKind {
if field.opts.Span == tableaupb.Span_SPAN_INNER_CELL {
// horizontal incell-struct list
cell, err := rc.Cell(colName, field.opts.Optional)
cell, err := rc.Cell(colName, sp.IsFieldOptional(field))
if err != nil {
return false, xerrors.WithMessageKV(err, rc.CellDebugKV(colName)...)
}
Expand All @@ -950,7 +957,7 @@
subMsgName := string(field.fd.Message().FullName())
if types.IsWellKnownMessage(subMsgName) {
// built-in message type: google.protobuf.Timestamp, google.protobuf.Duration
cell, err := rc.Cell(colName, field.opts.Optional)
cell, err := rc.Cell(colName, sp.IsFieldOptional(field))
if err != nil {
kvs := rc.CellDebugKV(colName)
return false, xerrors.WithMessageKV(err, kvs...)
Expand Down Expand Up @@ -982,7 +989,7 @@
list.Append(newListValue)
} else {
// scalar list
cell, err := rc.Cell(colName, field.opts.Optional)
cell, err := rc.Cell(colName, sp.IsFieldOptional(field))
if err != nil {
return false, xerrors.WithMessageKV(err, rc.CellDebugKV(colName)...)
}
Expand Down Expand Up @@ -1014,7 +1021,7 @@
case tableaupb.Layout_LAYOUT_INCELL:
// incell list
colName := prefix + field.opts.Name
cell, err := rc.Cell(colName, field.opts.Optional)
cell, err := rc.Cell(colName, sp.IsFieldOptional(field))
if err != nil {
return false, xerrors.WithMessageKV(err, rc.CellDebugKV(colName)...)
}
Expand Down Expand Up @@ -1113,7 +1120,7 @@
colName := prefix + field.opts.Name
if field.opts.Span == tableaupb.Span_SPAN_INNER_CELL {
// incell struct
cell, err := rc.Cell(colName, field.opts.Optional)
cell, err := rc.Cell(colName, sp.IsFieldOptional(field))
if err != nil {
return false, xerrors.WithMessageKV(err, rc.CellDebugKV(colName)...)
}
Expand All @@ -1129,7 +1136,7 @@
subMsgName := string(field.fd.Message().FullName())
if types.IsWellKnownMessage(subMsgName) {
// built-in message type: google.protobuf.Timestamp, google.protobuf.Duration
cell, err := rc.Cell(colName, field.opts.Optional)
cell, err := rc.Cell(colName, sp.IsFieldOptional(field))
if err != nil {
return false, xerrors.WithMessageKV(err, rc.CellDebugKV(colName)...)
}
Expand Down Expand Up @@ -1206,7 +1213,7 @@
if field.opts.Span == tableaupb.Span_SPAN_INNER_CELL {
colName := prefix + field.opts.Name
// incell union
cell, err := rc.Cell(colName, field.opts.Optional)
cell, err := rc.Cell(colName, sp.IsFieldOptional(field))
if err != nil {
return false, xerrors.WithMessageKV(err, rc.CellDebugKV(colName)...)
}
Expand All @@ -1226,7 +1233,7 @@

// parse union type
typeColName := prefix + field.opts.Name + unionDesc.TypeName()
cell, err := rc.Cell(typeColName, field.opts.Optional)
cell, err := rc.Cell(typeColName, sp.IsFieldOptional(field))
if err != nil {
return false, xerrors.WithMessageKV(err, rc.CellDebugKV(typeColName)...)
}
Expand Down Expand Up @@ -1260,7 +1267,7 @@
subField := parseFieldDescriptor(fd, sp.opts.Sep, sp.opts.Subsep)
defer subField.release()
// incell scalar
cell, err := rc.Cell(colName, subField.opts.Optional)
cell, err := rc.Cell(colName, sp.IsFieldOptional(subField))
if err != nil {
return xerrors.WithMessageKV(err, rc.CellDebugKV(colName)...)
}
Expand Down Expand Up @@ -1370,7 +1377,7 @@
}
var newValue protoreflect.Value
colName := prefix + field.opts.Name
cell, err := rc.Cell(colName, field.opts.Optional)
cell, err := rc.Cell(colName, sp.IsFieldOptional(field))
if err != nil {
return false, xerrors.WithMessageKV(err, rc.CellDebugKV(colName)...)
}
Expand Down
3 changes: 0 additions & 3 deletions internal/confgen/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ func parseFieldDescriptor(fd protoreflect.FieldDescriptor, sheetSep, sheetSubsep
layout := tableaupb.Layout_LAYOUT_DEFAULT
sep := ""
subsep := ""
optional := false
var prop *tableaupb.FieldProp

// opts := fd.Options().(*descriptorpb.FieldOptions)
Expand All @@ -65,7 +64,6 @@ func parseFieldDescriptor(fd protoreflect.FieldDescriptor, sheetSep, sheetSubsep
layout = fieldOpts.Layout
sep = strings.TrimSpace(fieldOpts.Sep)
subsep = strings.TrimSpace(fieldOpts.Subsep)
optional = fieldOpts.Optional
prop = fieldOpts.Prop
} else {
// default processing
Expand Down Expand Up @@ -100,7 +98,6 @@ func parseFieldDescriptor(fd protoreflect.FieldDescriptor, sheetSep, sheetSubsep
pooledOpts.Layout = layout
pooledOpts.Sep = sep
pooledOpts.Subsep = subsep
pooledOpts.Optional = optional
pooledOpts.Prop = prop

return &Field{
Expand Down
3 changes: 2 additions & 1 deletion internal/importer/xml.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ func NewXMLImporter(filename string, sheets []string, parser book.SheetParser, m
Book: book.NewBook(bookName, filename, nil),
}, nil
}
log.Debugf("newBook:%+v", newBook)

// log.Debugf("book: %+v", newBook)

return &XMLImporter{
Book: newBook,
Expand Down
Loading
Loading