Skip to content

Commit

Permalink
Merge pull request #74 from cybozu/render-distinct-error-values
Browse files Browse the repository at this point in the history 
Render Distinct Errors for Validation Failures
  • Loading branch information
@ymmt2005
ymmt2005 authored Jul 29, 2024
2 parents 3f22069 + aa6c755 commit 14a4016
Show file tree
Hide file tree
Showing 6 changed files with 318 additions and 197 deletions.
84 changes: 55 additions & 29 deletions cmd/protoc-gen-go-cybozu-validate/field.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,15 @@ func (r *Renderer) renderItemsRule(f *protogen.Field, ext *validate.FieldRules)
}

if rule.MaxItems != nil {
ei := r.addError(f, "MaxItems", fmt.Sprintf("too many items in %s of %s", fd.Name(), fd.Parent().Name()))
r.FL(`if len(x.%s) > %#v {`, f.GoName, *rule.MaxItems)
r.FL(`el = append(el, %s("too many items in %s of %s"))`, identErrorf, fd.Name(), fd.Parent().Name())
r.FL(`el = append(el, Err%s)`, ei.name)
r.PL(`}`)
}
if rule.MinItems != nil {
ei := r.addError(f, "MinItems", fmt.Sprintf("too few items in %s of %s", fd.Name(), fd.Parent().Name()))
r.FL(`if len(x.%s) < %#v {`, f.GoName, *rule.MinItems)
r.FL(`el = append(el, %s("too few items in %s of %s"))`, identErrorf, fd.Name(), fd.Parent().Name())
r.FL(`el = append(el, Err%s)`, ei.name)
r.PL(`}`)
}
return nil
Expand Down Expand Up @@ -147,8 +149,9 @@ func (r *Renderer) renderFloat(f *protogen.Field) error {
default:
r.FL(`if v := x.%s; true {`, f.GoName)
}
ei := r.addError(f, "FloatCmp", fmt.Sprintf("invalid value for %s of %s", fd.Name(), fd.Parent().FullName()))
r.FL(`if %s {`, cond)
r.FL(`el = append(el, %s("invalid value for %s of %s: %%v", v))`, identErrorf, fd.Name(), fd.Parent().FullName())
r.FL(`el = append(el, %s("%%w: %%v", Err%s, v))`, identErrorf, ei.name)
r.PL(`}`)
r.PL(`}`)
}
Expand Down Expand Up @@ -195,8 +198,9 @@ func (r *Renderer) renderDouble(f *protogen.Field) error {
default:
r.FL(`if v := x.%s; true {`, f.GoName)
}
ei := r.addError(f, "DoubleCmp", fmt.Sprintf("invalid value for %s of %s", fd.Name(), fd.Parent().FullName()))
r.FL(`if %s {`, cond)
r.FL(`el = append(el, %s("invalid value for %s of %s: %%v", v))`, identErrorf, fd.Name(), fd.Parent().FullName())
r.FL(`el = append(el, %s("%%w: %%v", Err%s, v))`, identErrorf, ei.name)
r.PL(`}`)
r.PL(`}`)
}
Expand Down Expand Up @@ -243,8 +247,9 @@ func (r *Renderer) renderInt32(f *protogen.Field) error {
default:
r.FL(`if v := x.%s; true {`, f.GoName)
}
ei := r.addError(f, "Int32Cmp", fmt.Sprintf("invalid value for %s of %s", fd.Name(), fd.Parent().FullName()))
r.FL(`if %s {`, cond)
r.FL(`el = append(el, %s("invalid value for %s of %s: %%v", v))`, identErrorf, fd.Name(), fd.Parent().FullName())
r.FL(`el = append(el, %s("%%w: %%v", Err%s, v))`, identErrorf, ei.name)
r.PL(`}`)
r.PL(`}`)
}
Expand Down Expand Up @@ -291,8 +296,9 @@ func (r *Renderer) renderInt64(f *protogen.Field) error {
default:
r.FL(`if v := x.%s; true {`, f.GoName)
}
ei := r.addError(f, "Int64Cmp", fmt.Sprintf("invalid value for %s of %s", fd.Name(), fd.Parent().FullName()))
r.FL(`if %s {`, cond)
r.FL(`el = append(el, %s("invalid value for %s of %s: %%v", v))`, identErrorf, fd.Name(), fd.Parent().FullName())
r.FL(`el = append(el, %s("%%w: %%v", Err%s, v))`, identErrorf, ei.name)
r.PL(`}`)
r.PL(`}`)
}
Expand Down Expand Up @@ -339,8 +345,9 @@ func (r *Renderer) renderUint32(f *protogen.Field) error {
default:
r.FL(`if v := x.%s; true {`, f.GoName)
}
ei := r.addError(f, "Uint32Cmp", fmt.Sprintf("invalid value for %s of %s", fd.Name(), fd.Parent().FullName()))
r.FL(`if %s {`, cond)
r.FL(`el = append(el, %s("invalid value for %s of %s: %%v", v))`, identErrorf, fd.Name(), fd.Parent().FullName())
r.FL(`el = append(el, %s("%%w: %%v", Err%s, v))`, identErrorf, ei.name)
r.PL(`}`)
r.PL(`}`)
}
Expand Down Expand Up @@ -387,8 +394,9 @@ func (r *Renderer) renderUint64(f *protogen.Field) error {
default:
r.FL(`if v := x.%s; true {`, f.GoName)
}
ei := r.addError(f, "Uint64Cmp", fmt.Sprintf("invalid value for %s of %s", fd.Name(), fd.Parent().FullName()))
r.FL(`if %s {`, cond)
r.FL(`el = append(el, %s("invalid value for %s of %s: %%v", v))`, identErrorf, fd.Name(), fd.Parent().FullName())
r.FL(`el = append(el, %s("%%w: %%v", Err%s, v))`, identErrorf, ei.name)
r.PL(`}`)
r.PL(`}`)
}
Expand Down Expand Up @@ -435,8 +443,9 @@ func (r *Renderer) renderSint32(f *protogen.Field) error {
default:
r.FL(`if v := x.%s; true {`, f.GoName)
}
ei := r.addError(f, "Sint32Cmp", fmt.Sprintf("invalid value for %s of %s", fd.Name(), fd.Parent().FullName()))
r.FL(`if %s {`, cond)
r.FL(`el = append(el, %s("invalid value for %s of %s: %%v", v))`, identErrorf, fd.Name(), fd.Parent().FullName())
r.FL(`el = append(el, %s("%%w: %%v", Err%s, v))`, identErrorf, ei.name)
r.PL(`}`)
r.PL(`}`)
}
Expand Down Expand Up @@ -483,8 +492,9 @@ func (r *Renderer) renderSint64(f *protogen.Field) error {
default:
r.FL(`if v := x.%s; true {`, f.GoName)
}
ei := r.addError(f, "Sint64Cmp", fmt.Sprintf("invalid value for %s of %s", fd.Name(), fd.Parent().FullName()))
r.FL(`if %s {`, cond)
r.FL(`el = append(el, %s("invalid value for %s of %s: %%v", v))`, identErrorf, fd.Name(), fd.Parent().FullName())
r.FL(`el = append(el, %s("%%w: %%v", Err%s, v))`, identErrorf, ei.name)
r.PL(`}`)
r.PL(`}`)
}
Expand Down Expand Up @@ -531,8 +541,9 @@ func (r *Renderer) renderFixed32(f *protogen.Field) error {
default:
r.FL(`if v := x.%s; true {`, f.GoName)
}
ei := r.addError(f, "Fixed32Cmp", fmt.Sprintf("invalid value for %s of %s", fd.Name(), fd.Parent().FullName()))
r.FL(`if %s {`, cond)
r.FL(`el = append(el, %s("invalid value for %s of %s: %%v", v))`, identErrorf, fd.Name(), fd.Parent().FullName())
r.FL(`el = append(el, %s("%%w: %%v", Err%s, v))`, identErrorf, ei.name)
r.PL(`}`)
r.PL(`}`)
}
Expand Down Expand Up @@ -579,8 +590,9 @@ func (r *Renderer) renderFixed64(f *protogen.Field) error {
default:
r.FL(`if v := x.%s; true {`, f.GoName)
}
ei := r.addError(f, "Fixed64Cmp", fmt.Sprintf("invalid value for %s of %s", fd.Name(), fd.Parent().FullName()))
r.FL(`if %s {`, cond)
r.FL(`el = append(el, %s("invalid value for %s of %s: %%v", v))`, identErrorf, fd.Name(), fd.Parent().FullName())
r.FL(`el = append(el, %s("%%w: %%v", Err%s, v))`, identErrorf, ei.name)
r.PL(`}`)
r.PL(`}`)
}
Expand Down Expand Up @@ -627,8 +639,9 @@ func (r *Renderer) renderSfixed32(f *protogen.Field) error {
default:
r.FL(`if v := x.%s; true {`, f.GoName)
}
ei := r.addError(f, "Sfixed32Cmp", fmt.Sprintf("invalid value for %s of %s", fd.Name(), fd.Parent().FullName()))
r.FL(`if %s {`, cond)
r.FL(`el = append(el, %s("invalid value for %s of %s: %%v", v))`, identErrorf, fd.Name(), fd.Parent().FullName())
r.FL(`el = append(el, %s("%%w: %%v", Err%s, v))`, identErrorf, ei.name)
r.PL(`}`)
r.PL(`}`)
}
Expand Down Expand Up @@ -675,8 +688,9 @@ func (r *Renderer) renderSfixed64(f *protogen.Field) error {
default:
r.FL(`if v := x.%s; true {`, f.GoName)
}
ei := r.addError(f, "Sfixed64Cmp", fmt.Sprintf("invalid value for %s of %s", fd.Name(), fd.Parent().FullName()))
r.FL(`if %s {`, cond)
r.FL(`el = append(el, %s("invalid value for %s of %s: %%v", v))`, identErrorf, fd.Name(), fd.Parent().FullName())
r.FL(`el = append(el, %s("%%w: %%v", Err%s, v))`, identErrorf, ei.name)
r.PL(`}`)
r.PL(`}`)
}
Expand Down Expand Up @@ -736,20 +750,23 @@ func (r *Renderer) renderString(f *protogen.Field) error {
case validate.StringRules_NFKD:
r.FL(`v = %s.String(v)`, identNormNFKD)
case validate.StringRules_PRECIS_USERNAME_CASE_MAPPED:
ei := r.addError(f, "PRECISUsernameCaseMapped", fmt.Sprintf("invalid value for %s of %s", fd.Name(), fd.Parent().FullName()))
r.FL(`if v2, err := %s.String(v); err != nil {`, identPRECISUsernameCaseMapped)
r.FL(`el = append(el, %s("invalid value for %s of %s: %%v, %%w", v, err))`, identErrorf, fd.Name(), fd.Parent().FullName())
r.FL(`el = append(el, %s("%%w: %%v, %%w", Err%s, v, err))`, identErrorf, ei.name)
r.PL(`} else {`)
r.PL(`v = v2`)
r.PL(`}`)
case validate.StringRules_PRECIS_USERNAME_CASE_PRESERVED:
ei := r.addError(f, "PRECISUsernameCasePreserved", fmt.Sprintf("invalid value for %s of %s", fd.Name(), fd.Parent().FullName()))
r.FL(`if v2, err := %s.String(v); err != nil {`, identPRECISUsernameCasePreserved)
r.FL(`el = append(el, %s("invalid value for %s of %s: %%v, %%w", v, err))`, identErrorf, fd.Name(), fd.Parent().FullName())
r.FL(`el = append(el, %s("%%w: %%v, %%w", Err%s, v, err))`, identErrorf, ei.name)
r.PL(`} else {`)
r.PL(`v = v2`)
r.PL(`}`)
case validate.StringRules_PRECIS_OPAQUE_STRING:
ei := r.addError(f, "PRECISOpaqueString", fmt.Sprintf("invalid value for %s of %s", fd.Name(), fd.Parent().FullName()))
r.FL(`if v2, err := %s.String(v); err != nil {`, identPRECISOpaqueString)
r.FL(`el = append(el, %s("invalid value for %s of %s: %%v, %%w", v, err))`, identErrorf, fd.Name(), fd.Parent().FullName())
r.FL(`el = append(el, %s("%%w: %%v, %%w", Err%s, v, err))`, identErrorf, ei.name)
r.PL(`} else {`)
r.PL(`v = v2`)
r.PL(`}`)
Expand All @@ -767,8 +784,9 @@ func (r *Renderer) renderString(f *protogen.Field) error {
if rule.MinLength != nil {
conds = append(conds, fmt.Sprintf("vlen < %#v", *rule.MinLength))
}
ei := r.addError(f, "StringLen", fmt.Sprintf("invalid value for %s of %s", fd.Name(), fd.Parent().FullName()))
r.FL(`if vlen := %s(v); %s {`, identRuneCountInString, strings.Join(conds, " || "))
r.FL(`el = append(el, %s("invalid value for %s of %s: %%v", v))`, identErrorf, fd.Name(), fd.Parent().FullName())
r.FL(`el = append(el, %s("%%w: %%v", Err%s, v))`, identErrorf, ei.name)
r.PL(`}`)
}
if rule.Regex != nil {
Expand All @@ -777,38 +795,42 @@ func (r *Renderer) renderString(f *protogen.Field) error {
return fieldError(fmt.Sprintf("invalid regular expression %q: %v", *rule.Regex, err), fd)
}
r.addRegexp(f, *rule.Regex)
ei := r.addError(f, "StringRegexp", fmt.Sprintf("invalid value for %s of %s", fd.Name(), fd.Parent().FullName()))
r.FL(`if !regex_%s.MatchString(v) {`, f.GoIdent.GoName)
r.FL(`el = append(el, %s("invalid value for %s of %s: %%v", v))`, identErrorf, fd.Name(), fd.Parent().FullName())
r.FL(`el = append(el, %s("%%w: %%v", Err%s, v))`, identErrorf, ei.name)
r.PL(`}`)
}
if rule.Predefined != nil {
switch val := rule.Predefined.(type) {
case *validate.StringRules_Email:
if val.Email {
ei := r.addError(f, "Email", fmt.Sprintf("invalid value for %s of %s", fd.Name(), fd.Parent().FullName()))
r.FL(`if a, err := %s(v); err != nil {`, identMailParseAddress)
r.FL(`el = append(el, %s("invalid value for %s of %s: %%v, %%w", v, err))`, identErrorf, fd.Name(), fd.Parent().FullName())
r.FL(`el = append(el, %s("%%w: %%v, %%w", Err%s, v, err))`, identErrorf, ei.name)
r.PL(`} else if a.Name != "" {`)
r.FL(`el = append(el, %s("invalid value for %s of %s: %%v", v))`, identErrorf, fd.Name(), fd.Parent().FullName())
r.FL(`el = append(el, %s("%%w: %%v", Err%s, v))`, identErrorf, ei.name)
r.PL(`} else {`)
r.PL(`v = a.Address`)
r.PL(`}`)
}
case *validate.StringRules_Uri:
if val.Uri {
ei := r.addError(f, "URI", fmt.Sprintf("invalid value for %s of %s", fd.Name(), fd.Parent().FullName()))
r.FL(`if u, err := %s(v); err != nil {`, identURLParse)
r.FL(`el = append(el, %s("invalid value for %s of %s: %%v, %%w", v, err))`, identErrorf, fd.Name(), fd.Parent().FullName())
r.FL(`el = append(el, %s("%%w: %%v, %%w", Err%s, v, err))`, identErrorf, ei.name)
r.PL(`} else if !u.IsAbs() {`)
r.FL(`el = append(el, %s("invalid value for %s of %s: %%v", v))`, identErrorf, fd.Name(), fd.Parent().FullName())
r.FL(`el = append(el, %s("%%w: %%v", Err%s, v))`, identErrorf, ei.name)
r.PL(`} else {`)
r.PL(`v = u.String()`)
r.PL(`}`)
}
case *validate.StringRules_E164:
if val.E164 {
ei := r.addError(f, "E164", fmt.Sprintf("invalid value for %s of %s", fd.Name(), fd.Parent().FullName()))
r.FL(`if !%s.MatchString(v) {`, identE164Pattern)
r.FL(`el = append(el, %s("invalid value for %s of %s: %%v", v))`, identErrorf, fd.Name(), fd.Parent().FullName())
r.FL(`el = append(el, %s("%%w: %%v", Err%s, v))`, identErrorf, ei.name)
r.FL(`} else if len(v)-%s(v, "-") > 16 {`, identStringsCount)
r.FL(`el = append(el, %s("invalid value for %s of %s: %%v", v))`, identErrorf, fd.Name(), fd.Parent().FullName())
r.FL(`el = append(el, %s("%%w: %%v", Err%s, v))`, identErrorf, ei.name)
r.PL(`}`)
}
default:
Expand Down Expand Up @@ -856,8 +878,9 @@ func (r *Renderer) renderBytes(f *protogen.Field) error {
default:
r.FL(`if v := x.%s; true {`, f.GoName)
}
ei := r.addError(f, "BytesLen", fmt.Sprintf("invalid value for %s of %s", fd.Name(), fd.Parent().FullName()))
r.FL(`if vlen := len(v); %s {`, cond)
r.FL(`el = append(el, %s("invalid value for %s of %s: %%v", v))`, identErrorf, fd.Name(), fd.Parent().FullName())
r.FL(`el = append(el, %s("%%w: %%v", Err%s, v))`, identErrorf, ei.name)
r.PL(`}`)
r.PL(`}`)
}
Expand Down Expand Up @@ -891,13 +914,15 @@ func (r *Renderer) renderEnum(f *protogen.Field) error {
r.FL(`if v := x.%s; true {`, f.GoName)
}
if rule.Required {
ei := r.addError(f, "EnumRequired", fmt.Sprintf("field %s of %s must not be zero-value", fd.Name(), fd.Parent().FullName()))
r.FL(`if v == %s(0) {`, f.Enum.GoIdent)
r.FL(`el = append(el, %s("field %s of %s must not be zero-value"))`, identErrorsNew, fd.Name(), fd.Parent().FullName())
r.FL(`el = append(el, Err%s)`, ei.name)
r.PL(`}`)
}
if rule.DefinedOnly {
ei := r.addError(f, "EnumDefinedOnly", fmt.Sprintf("invalid value for %s of %s", fd.Name(), fd.Parent().FullName()))
r.FL(`if _, ok := %s_name[int32(v)]; !ok {`, f.Enum.GoIdent)
r.FL(`el = append(el, %s("invalid value for %s of %s: %%v", v))`, identErrorf, fd.Name(), fd.Parent().FullName())
r.FL(`el = append(el, %s("%%w: %%v", Err%s, v))`, identErrorf, ei.name)
r.PL(`}`)
}
r.PL(`}`)
Expand Down Expand Up @@ -929,8 +954,9 @@ func (r *Renderer) renderMessageField(f *protogen.Field) error {
}

if required {
ei := r.addError(f, "MessageRequiredField", fmt.Sprintf("required field %s of %s is missing", fd.Name(), fd.Parent().FullName()))
r.PL(`if v == nil {`)
r.FL(`el = append(el, %s("required field %s of %s is missing"))`, identErrorsNew, fd.Name(), fd.Parent().FullName())
r.FL(`el = append(el, Err%s)`, ei.name)
r.PL(`}`)
}

Expand Down
27 changes: 26 additions & 1 deletion cmd/protoc-gen-go-cybozu-validate/renderer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,16 @@ type reInfo struct {
reStr string
}

type errorInfo struct {
name string
msg string
}

type Renderer struct {
me string
gen *protogen.GeneratedFile
regexps []reInfo
errors []errorInfo
}

func NewRenderer(me string, gen *protogen.GeneratedFile) *Renderer {
Expand All @@ -30,6 +36,12 @@ func (r *Renderer) addRegexp(f *protogen.Field, reStr string) {
r.regexps = append(r.regexps, reInfo{field: f, reStr: reStr})
}

func (r *Renderer) addError(f *protogen.Field, errNamePrefix string, msg string) errorInfo {
ei := errorInfo{name: errNamePrefix + "_" + f.GoIdent.GoName, msg: msg}
r.errors = append(r.errors, ei)
return ei
}

// P appends the arguments to the internal buffer without a newline at the end.
// The arguments are converted to strings as follows:
//
Expand Down Expand Up @@ -111,6 +123,15 @@ func (r *Renderer) Execute(pkg protogen.GoPackageName, msgs []*protogen.Message)
r.PL(`)`)
}

if len(r.errors) > 0 {
r.L()
r.PL(`var (`)
for _, ei := range r.errors {
r.FL(`Err%s = %s(%q)`, ei.name, identErrorsNew, ei.msg)
}
r.PL(`)`)
}

return nil
}

Expand Down Expand Up @@ -168,8 +189,12 @@ func (r *Renderer) renderOneof(o *protogen.Oneof) error {
od := o.Desc
required := proto.GetExtension(o.Desc.Options(), validate.E_Required).(bool)
if required {
ei := errorInfo{
name: "OneOfRequired_" + o.GoIdent.GoName,
msg: fmt.Sprintf("one of the fields is required in %s of %s", od.Name(), od.Parent().FullName())}
r.errors = append(r.errors, ei)
r.FL(`if x.%s == nil {`, o.GoName)
r.FL(`el = append(el, %s("one of the fields is required in %s of %s"))`, identErrorf, od.Name(), od.Parent().FullName())
r.FL(`el = append(el, Err%s)`, ei.name)
r.PL(`}`)
}

Expand Down
9 changes: 8 additions & 1 deletion cybozu/validate/design.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,14 @@ are doing.

For Go, we will add `Validate() error` method to generated structs.
The error returned is not a gRPC error to avoid dependency on a specific
gRPC implementation.
gRPC implementation. Distinct errors are also generated for each type of
validation/normalization and can be used in conjunction with `errors.Is`
by callers to take specific actions based on certain validation failures.

These distinct errors are generally in the format of
`Err<Failed validation>_<Message Name>_<Field NAME>`. For example, failed
PRECIS username case mapped normalization in message `Strings` field `s4`
is `ErrPRECISUsernameCaseMapped_Strings_S4`.

[PGV]: https://github.com/bufbuild/protoc-gen-validate
[go-proto-validators]: https://github.com/mwitkow/go-proto-validators
Expand Down
4 changes: 3 additions & 1 deletion examples/validation_custom.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@ import (
func (x *Maps) ValidateCustom() error {
total := len(x.Map1) + len(x.Map2) + len(x.Map3)
if total > 10 {
return fmt.Errorf("too many items: %d", total)
return fmt.Errorf("%w: %d", ErrValidateCustom, total)
}
return nil
}

// A Go idiom to statically check if a type implements an interface, here `validate.CustomValidator`.
var _ validate.CustomValidator = &Maps{}

var ErrValidateCustom = fmt.Errorf("too many items")
Loading

0 comments on commit 14a4016

Please sign in to comment.