From db5513f24d614bb6fb7713d7b12d2881c75919aa Mon Sep 17 00:00:00 2001 From: Kybxd <627940450@qq.com> Date: Mon, 13 Jul 2026 20:32:18 +0800 Subject: [PATCH 1/3] feat(load): protovalidate messages on the load path Export confgen.NewValidator/Validate and reuse them so the load (checker) path validates fully-assembled messages via protovalidate's global validator, matching confgen's storeMessage coverage. --- internal/confgen/confgen.go | 9 ++------- internal/confgen/util.go | 17 ++++++++++++++--- internal/confgen/util_test.go | 2 +- load/load.go | 13 ++++++++++--- 4 files changed, 27 insertions(+), 14 deletions(-) diff --git a/internal/confgen/confgen.go b/internal/confgen/confgen.go index ea7190fb..e4957b85 100644 --- a/internal/confgen/confgen.go +++ b/internal/confgen/confgen.go @@ -23,7 +23,6 @@ import ( "google.golang.org/protobuf/reflect/protoreflect" "google.golang.org/protobuf/reflect/protoregistry" "google.golang.org/protobuf/types/descriptorpb" - "google.golang.org/protobuf/types/dynamicpb" ) // Error collection limits at each level. @@ -98,9 +97,7 @@ func (gen *Generator) GenAll() error { return err } // Create a validator with extension type resolver for custom predefined rules. - gen.validator, err = protovalidate.New( - protovalidate.WithExtensionTypeResolver(dynamicpb.NewTypes(prFiles)), - ) + gen.validator, err = NewValidator(prFiles) if err != nil { return err } @@ -126,9 +123,7 @@ func (gen *Generator) GenWorkbook(bookSpecifiers ...string) error { return err } // Create a validator with extension type resolver for custom predefined rules. - gen.validator, err = protovalidate.New( - protovalidate.WithExtensionTypeResolver(dynamicpb.NewTypes(prFiles)), - ) + gen.validator, err = NewValidator(prFiles) if err != nil { return err } diff --git a/internal/confgen/util.go b/internal/confgen/util.go index 8bb3308d..1afa7fde 100644 --- a/internal/confgen/util.go +++ b/internal/confgen/util.go @@ -26,6 +26,7 @@ import ( "google.golang.org/protobuf/reflect/protoreflect" "google.golang.org/protobuf/reflect/protoregistry" "google.golang.org/protobuf/types/descriptorpb" + "google.golang.org/protobuf/types/dynamicpb" ) var fieldOptionsPool *sync.Pool @@ -314,9 +315,19 @@ func parseOutputFormats(msg proto.Message, opt *options.ConfOutputOption) []form return format.OutputFormats } -// validate validates a proto message using the provided validator. Each violation +// NewValidator creates a protovalidate validator whose extension type resolver +// is built from prFiles, so custom predefined rules can be recognized. It is +// shared by both the confgen (generate) and load (checker) paths to keep +// validation behavior consistent. +func NewValidator(prFiles *protoregistry.Files) (protovalidate.Validator, error) { + return protovalidate.New( + protovalidate.WithExtensionTypeResolver(dynamicpb.NewTypes(prFiles)), + ) +} + +// Validate validates a proto message using the provided validator. Each violation // is wrapped as an E2027 error and all violations are joined together. -func validate(msg proto.Message, validator protovalidate.Validator) error { +func Validate(msg proto.Message, validator protovalidate.Validator) error { err := validator.Validate(msg) if err == nil { return nil @@ -342,7 +353,7 @@ func validate(msg proto.Message, validator protovalidate.Validator) error { // storeMessage stores a message to one or multiple file formats. func storeMessage(msg proto.Message, name, locationName, outputDir string, opt *options.ConfOutputOption, validator protovalidate.Validator) error { - if err := validate(msg, validator); err != nil { + if err := Validate(msg, validator); err != nil { return err } outputDir = filepath.Join(outputDir, opt.Subdir) diff --git a/internal/confgen/util_test.go b/internal/confgen/util_test.go index 287a8f0e..fadcd6d1 100644 --- a/internal/confgen/util_test.go +++ b/internal/confgen/util_test.go @@ -412,7 +412,7 @@ func Test_validate(t *testing.T) { if err != nil { t.Fatalf("failed to create validator: %v", err) } - err = validate(tt.args.msg, validator) + err = Validate(tt.args.msg, validator) if (err != nil) != tt.wantErr { t.Errorf("validate() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/load/load.go b/load/load.go index 8bd840f8..e669b3d4 100644 --- a/load/load.go +++ b/load/load.go @@ -8,6 +8,7 @@ import ( "context" "path/filepath" + "buf.build/go/protovalidate" "github.com/tableauio/tableau/format" "github.com/tableauio/tableau/internal/confgen" "github.com/tableauio/tableau/internal/importer" @@ -202,12 +203,18 @@ func loadOrigin(msg proto.Message, dir string, opts *MessagerOptions) error { case sheetInfo.HasScatter() && sheetInfo.HasMerger(): return xerrors.Newf("option Scatter and Merger cannot be both set at one sheet: %s#%s", bookName, sheetName) case sheetInfo.HasScatter(): - return loadOriginScatter(msg, sheetInfo, mainImpInfo, dir, subdirRewrites, collector) + err = loadOriginScatter(msg, sheetInfo, mainImpInfo, dir, subdirRewrites, collector) case sheetInfo.HasMerger(): - return loadOriginMerger(msg, sheetInfo, mainImpInfo, dir, subdirRewrites, collector) + err = loadOriginMerger(msg, sheetInfo, mainImpInfo, dir, subdirRewrites, collector) default: - return loadOriginMain(msg, sheetInfo, mainImpInfo, collector) + err = loadOriginMain(msg, sheetInfo, mainImpInfo, collector) + } + if err != nil { + return err } + // protovalidate the fully-assembled message, giving the load path the same + // coverage as confgen's storeMessage step. + return confgen.Validate(msg, protovalidate.GlobalValidator) } // loadOriginScatter handles the scatter branch of loadOrigin. From da8401418f3b075d00719d7c7cd007cacc4312e5 Mon Sep 17 00:00:00 2001 From: Kybxd <627940450@qq.com> Date: Mon, 13 Jul 2026 21:07:42 +0800 Subject: [PATCH 2/3] test(load): cover protovalidate error on the load path Add a ValidateConf CSV whose Name exceeds max_len:10 and a test that loads it, asserting loadOrigin surfaces the violation as E2027. Guards against regressions where the confgen.Validate call could be dropped. --- load/load_test.go | 11 +++++++++++ testdata/unittest/Unittest#ValidateConf.csv | 4 ++++ 2 files changed, 15 insertions(+) create mode 100644 testdata/unittest/Unittest#ValidateConf.csv diff --git a/load/load_test.go b/load/load_test.go index 6a6e4561..74b7b022 100644 --- a/load/load_test.go +++ b/load/load_test.go @@ -299,6 +299,17 @@ func TestLoadJSON_E0002(t *testing.T) { t.Logf("error: %s", xerrors.NewDesc(err).String()) } +// TestLoadOriginProtovalidate guards the protovalidate step in loadOrigin: +// ValidateConf constrains Name to max_len:10, and the input row supplies a +// longer value, so a fully-assembled message must surface an E2027 violation. +// This catches regressions if the confgen.Validate call is ever dropped. +func TestLoadOriginProtovalidate(t *testing.T) { + err := LoadMessagerInDir(&unittestpb.ValidateConf{}, "../testdata/", format.CSV, &MessagerOptions{}) + require.Error(t, err) + require.ErrorIs(t, err, xerrors.ErrE2027) + t.Logf("error: %s", xerrors.NewDesc(err).String()) +} + func TestLoadWithPatch(t *testing.T) { type args struct { msg proto.Message diff --git a/testdata/unittest/Unittest#ValidateConf.csv b/testdata/unittest/Unittest#ValidateConf.csv new file mode 100644 index 00000000..f6868ec0 --- /dev/null +++ b/testdata/unittest/Unittest#ValidateConf.csv @@ -0,0 +1,4 @@ +ID,Name,Tag,Prop +uint32,string,"[]string","map" +Conf ID,Conf name,Tags,Props +0,this name is too long,"a,b","x:1" From e8f49e69a99c1d798cba79621368a2369c86d4e7 Mon Sep 17 00:00:00 2001 From: Kybxd <627940450@qq.com> Date: Tue, 14 Jul 2026 15:16:13 +0800 Subject: [PATCH 3/3] feat(load): apply protovalidate to output formats too Move the protovalidate step out of loadOrigin and converge it at the LoadMessagerInDir exit. protovalidate operates on the final in-memory protobuf message, so both input (excel/csv/xml/yaml) and output (json/binpb/txtpb) formats now get the same coverage as confgen's storeMessage step. The check runs after patch merging to avoid false positives on intermediate state. Extend TestLoadOriginProtovalidate into TestLoadProtovalidate with input-format-csv and output-format-json subcases, and add the testdata/unittest/conf/ValidateConf.json fixture. --- load/load.go | 22 ++++++++++++--------- load/load_test.go | 25 +++++++++++++++++------- testdata/unittest/conf/ValidateConf.json | 4 ++++ 3 files changed, 35 insertions(+), 16 deletions(-) create mode 100644 testdata/unittest/conf/ValidateConf.json diff --git a/load/load.go b/load/load.go index e669b3d4..e9fa22c1 100644 --- a/load/load.go +++ b/load/load.go @@ -25,6 +25,16 @@ import ( // LoadMessagerInDir loads message's content in the given dir, based on format and messager options. func LoadMessagerInDir(msg proto.Message, dir string, fmt format.Format, opts *MessagerOptions) error { + if err := loadMessagerInDir(msg, dir, fmt, opts); err != nil { + return err + } + // protovalidate the fully-assembled message. protovalidate operates on the + // final in-memory protobuf, so both input (excel/csv/xml/yaml) and output + // (json/binpb/txtpb) formats get the same coverage as confgen's storeMessage. + return confgen.Validate(msg, protovalidate.GlobalValidator) +} + +func loadMessagerInDir(msg proto.Message, dir string, fmt format.Format, opts *MessagerOptions) error { if format.IsInputFormat(fmt) { return loadOrigin(msg, dir, opts) } @@ -203,18 +213,12 @@ func loadOrigin(msg proto.Message, dir string, opts *MessagerOptions) error { case sheetInfo.HasScatter() && sheetInfo.HasMerger(): return xerrors.Newf("option Scatter and Merger cannot be both set at one sheet: %s#%s", bookName, sheetName) case sheetInfo.HasScatter(): - err = loadOriginScatter(msg, sheetInfo, mainImpInfo, dir, subdirRewrites, collector) + return loadOriginScatter(msg, sheetInfo, mainImpInfo, dir, subdirRewrites, collector) case sheetInfo.HasMerger(): - err = loadOriginMerger(msg, sheetInfo, mainImpInfo, dir, subdirRewrites, collector) + return loadOriginMerger(msg, sheetInfo, mainImpInfo, dir, subdirRewrites, collector) default: - err = loadOriginMain(msg, sheetInfo, mainImpInfo, collector) - } - if err != nil { - return err + return loadOriginMain(msg, sheetInfo, mainImpInfo, collector) } - // protovalidate the fully-assembled message, giving the load path the same - // coverage as confgen's storeMessage step. - return confgen.Validate(msg, protovalidate.GlobalValidator) } // loadOriginScatter handles the scatter branch of loadOrigin. diff --git a/load/load_test.go b/load/load_test.go index 74b7b022..4fc65119 100644 --- a/load/load_test.go +++ b/load/load_test.go @@ -299,15 +299,26 @@ func TestLoadJSON_E0002(t *testing.T) { t.Logf("error: %s", xerrors.NewDesc(err).String()) } -// TestLoadOriginProtovalidate guards the protovalidate step in loadOrigin: -// ValidateConf constrains Name to max_len:10, and the input row supplies a +// TestLoadProtovalidate guards the protovalidate step in LoadMessagerInDir: +// protovalidate operates on the final in-memory message, so it must run for +// both input (excel/csv/xml/yaml) and output (json/binpb/txtpb) formats. +// ValidateConf constrains Name to max_len:10, and each fixture supplies a // longer value, so a fully-assembled message must surface an E2027 violation. // This catches regressions if the confgen.Validate call is ever dropped. -func TestLoadOriginProtovalidate(t *testing.T) { - err := LoadMessagerInDir(&unittestpb.ValidateConf{}, "../testdata/", format.CSV, &MessagerOptions{}) - require.Error(t, err) - require.ErrorIs(t, err, xerrors.ErrE2027) - t.Logf("error: %s", xerrors.NewDesc(err).String()) +func TestLoadProtovalidate(t *testing.T) { + t.Run("input-format-csv", func(t *testing.T) { + err := LoadMessagerInDir(&unittestpb.ValidateConf{}, "../testdata/", format.CSV, &MessagerOptions{}) + require.Error(t, err) + require.ErrorIs(t, err, xerrors.ErrE2027) + t.Logf("error: %s", xerrors.NewDesc(err).String()) + }) + + t.Run("output-format-json", func(t *testing.T) { + err := LoadMessagerInDir(&unittestpb.ValidateConf{}, "../testdata/unittest/conf/", format.JSON, &MessagerOptions{}) + require.Error(t, err) + require.ErrorIs(t, err, xerrors.ErrE2027) + t.Logf("error: %s", xerrors.NewDesc(err).String()) + }) } func TestLoadWithPatch(t *testing.T) { diff --git a/testdata/unittest/conf/ValidateConf.json b/testdata/unittest/conf/ValidateConf.json new file mode 100644 index 00000000..0dee462d --- /dev/null +++ b/testdata/unittest/conf/ValidateConf.json @@ -0,0 +1,4 @@ +{ + "id": 0, + "name": "this name is too long" +}