Skip to content

Commit

Permalink
Use attribute names in error messages (#3297)
Browse files Browse the repository at this point in the history 
Instead of generated variable names.
  • Loading branch information
@raphael
raphael committed Jun 11, 2023
1 parent 8537a1f commit 27c249f
Show file tree
Hide file tree
Showing 13 changed files with 194 additions and 161 deletions.
9 changes: 9 additions & 0 deletions codegen/validation.go
View file
Expand Up @@ -44,6 +44,15 @@ func init() {
userValT = template.Must(template.New("user").Funcs(fm).Parse(userValTmpl))
}

// AttributeValidationCode produces Go code that runs the validations defined
// in the given attribute against the value held by the variable named target.
//
// See ValidationCode for a description of the arguments.
func AttributeValidationCode(att *expr.AttributeExpr, put expr.UserType, attCtx *AttributeContext, req, alias bool, target, attName string) string {
seen := make(map[string]*bytes.Buffer)
return recurseValidationCode(att, put, attCtx, req, alias, target, attName, seen).String()
}

// ValidationCode produces Go code that runs the validations defined in the
// given attribute and its children recursively against the value held by the
// variable named target.
Expand Down
8 changes: 4 additions & 4 deletions grpc/codegen/service_data.go
View file
Expand Up @@ -766,15 +766,15 @@ func addValidation(att *expr.AttributeExpr, attName string, sd *ServiceData, req
// req if true indicates that the validations are generated for validating
// request messages.
func collectValidations(att *expr.AttributeExpr, attName string, ctx *codegen.AttributeContext, req bool, sd *ServiceData) {
attName = codegen.Goify(attName, false)
gattName := codegen.Goify(attName, false)
switch dt := att.Type.(type) {
case expr.UserType:
if expr.IsPrimitive(dt) {
// Alias type - validation is generatd inline in parent type validation code.
return
}
vtx := protoBufTypeContext(sd.PkgName, sd.Scope, false)
def := codegen.ValidationCode(att, dt, vtx, true, false, attName)
def := codegen.AttributeValidationCode(att, dt, vtx, true, false, gattName, attName)
name := protoBufMessageName(att, sd.Scope)
kind := validateClient
if req {
Expand All @@ -793,7 +793,7 @@ func collectValidations(att *expr.AttributeExpr, attName string, ctx *codegen.At
sd.validations = append(sd.validations, &ValidationData{
Name: "Validate" + name,
Def: def,
ArgName: attName,
ArgName: gattName,
SrcName: name,
SrcRef: protoBufGoFullTypeRef(att, sd.PkgName, sd.Scope),
Kind: kind,
Expand Down Expand Up @@ -1299,7 +1299,7 @@ func extractMetadata(a *expr.MappedAttributeExpr, service *expr.AttributeExpr, s
mp.KeyType.Type.Kind() == expr.StringKind &&
mp.ElemType.Type.Kind() == expr.ArrayKind &&
expr.AsArray(mp.ElemType.Type).ElemType.Type.Kind() == expr.StringKind,
Validate: codegen.ValidationCode(c, nil, ctx, required, false, varn),
Validate: codegen.AttributeValidationCode(c, nil, ctx, required, false, varn, name),
DefaultValue: c.DefaultValue,
Example: c.Example(expr.Root.API.ExampleGenerator),
})
Expand Down
8 changes: 4 additions & 4 deletions grpc/codegen/testdata/client_cli_code.go
View file
Expand Up @@ -31,10 +31,10 @@ func BuildMethodAPayload(payloadWithValidationMethodAMetadataInt string, payload
return nil, fmt.Errorf("invalid value for metadataInt, must be INT")
}
if *metadataInt < 0 {
err = goa.MergeErrors(err, goa.InvalidRangeError("metadataInt", *metadataInt, 0, true))
err = goa.MergeErrors(err, goa.InvalidRangeError("MetadataInt", *metadataInt, 0, true))
}
if *metadataInt > 100 {
err = goa.MergeErrors(err, goa.InvalidRangeError("metadataInt", *metadataInt, 100, false))
err = goa.MergeErrors(err, goa.InvalidRangeError("MetadataInt", *metadataInt, 100, false))
}
if err != nil {
return nil, err
Expand All @@ -46,10 +46,10 @@ func BuildMethodAPayload(payloadWithValidationMethodAMetadataInt string, payload
if payloadWithValidationMethodAMetadataString != "" {
metadataString = &payloadWithValidationMethodAMetadataString
if utf8.RuneCountInString(*metadataString) < 5 {
err = goa.MergeErrors(err, goa.InvalidLengthError("metadataString", *metadataString, utf8.RuneCountInString(*metadataString), 5, true))
err = goa.MergeErrors(err, goa.InvalidLengthError("MetadataString", *metadataString, utf8.RuneCountInString(*metadataString), 5, true))
}
if utf8.RuneCountInString(*metadataString) > 10 {
err = goa.MergeErrors(err, goa.InvalidLengthError("metadataString", *metadataString, utf8.RuneCountInString(*metadataString), 10, false))
err = goa.MergeErrors(err, goa.InvalidLengthError("MetadataString", *metadataString, utf8.RuneCountInString(*metadataString), 10, false))
}
if err != nil {
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion grpc/codegen/testdata/request_decoder_code.go
View file
Expand Up @@ -210,7 +210,7 @@ func DecodeMethodMessageWithValidateRequest(ctx context.Context, v any, md metad
}
if inMetadata != nil {
if *inMetadata > 100 {
err = goa.MergeErrors(err, goa.InvalidRangeError("inMetadata", *inMetadata, 100, false))
err = goa.MergeErrors(err, goa.InvalidRangeError("InMetadata", *inMetadata, 100, false))
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions grpc/codegen/testdata/response_decoder_code.go
View file
Expand Up @@ -137,7 +137,7 @@ func DecodeMethodMessageWithValidateResponse(ctx context.Context, v any, hdr, tr
}
if inHeader != nil {
if *inHeader < 1 {
err = goa.MergeErrors(err, goa.InvalidRangeError("inHeader", *inHeader, 1, true))
err = goa.MergeErrors(err, goa.InvalidRangeError("InHeader", *inHeader, 1, true))
}
}
Expand All @@ -152,7 +152,7 @@ func DecodeMethodMessageWithValidateResponse(ctx context.Context, v any, hdr, tr
}
if inTrailer != nil {
if !(*inTrailer == true) {
err = goa.MergeErrors(err, goa.InvalidEnumValueError("inTrailer", *inTrailer, []any{true}))
err = goa.MergeErrors(err, goa.InvalidEnumValueError("InTrailer", *inTrailer, []any{true}))
}
}
}
Expand Down
36 changes: 18 additions & 18 deletions http/codegen/client.go
View file
Expand Up @@ -423,31 +423,31 @@ func {{ .RequestEncoder }}(encoder func(*http.Request) goahttp.Encoder) func(*ht
{
{{- end }}
head := {{ if .FieldPointer }}*{{ end }}p.{{ .FieldName }}
{{- if (and (eq .Name "Authorization") (isBearer $.HeaderSchemes)) }}
{{- if (and (eq .HTTPName "Authorization") (isBearer $.HeaderSchemes)) }}
if !strings.Contains(head, " ") {
req.Header.Set({{ printf "%q" .Name }}, "Bearer "+head)
req.Header.Set({{ printf "%q" .HTTPName }}, "Bearer "+head)
} else {
{{- end }}
{{- if eq .Type.Name "array" }}
for _, val := range head {
{{- if eq .Type.ElemType.Type.Name "string" }}
req.Header.Add({{ printf "%q" .Name }}, val)
req.Header.Add({{ printf "%q" .HTTPName }}, val)
{{- else if (and (isAlias .Type.ElemType.Type) (eq (underlyingType .Type.ElemType.Type).Name "string")) }}
req.Header.Set({{ printf "%q" .Name }}, string(val))
req.Header.Set({{ printf "%q" .HTTPName }}, string(val))
{{- else }}
{{ template "type_conversion" (typeConversionData .Type.ElemType.Type (aliasedType .FieldType).ElemType.Type "valStr" "val") }}
req.Header.Add({{ printf "%q" .Name }}, valStr)
req.Header.Add({{ printf "%q" .HTTPName }}, valStr)
{{- end }}
}
{{- else if (and (isAlias .FieldType) (eq (underlyingType .FieldType).Name "string")) }}
req.Header.Set({{ printf "%q" .Name }}, string(head))
req.Header.Set({{ printf "%q" .HTTPName }}, string(head))
{{- else if eq .Type.Name "string" }}
req.Header.Set({{ printf "%q" .Name }}, head)
req.Header.Set({{ printf "%q" .HTTPName }}, head)
{{- else }}
{{ template "type_conversion" (typeConversionData .Type .FieldType "headStr" "head") }}
req.Header.Set({{ printf "%q" .Name }}, headStr)
req.Header.Set({{ printf "%q" .HTTPName }}, headStr)
{{- end }}
{{- if (and (eq .Name "Authorization") (isBearer $.HeaderSchemes)) }}
{{- if (and (eq .HTTPName "Authorization") (isBearer $.HeaderSchemes)) }}
}
{{- end }}
}
Expand All @@ -465,7 +465,7 @@ func {{ .RequestEncoder }}(encoder func(*http.Request) goahttp.Encoder) func(*ht
{{ template "type_conversion" (typeConversionData .Type .FieldType "vraw" "v") }}
{{- end }}
req.AddCookie(&http.Cookie{
Name: {{ printf "%q" .Name }},
Name: {{ printf "%q" .HTTPName }},
Value: v,
{{- if .MaxAge }}
MaxAge: {{ .MaxAge }},
Expand Down Expand Up @@ -505,20 +505,20 @@ func {{ .RequestEncoder }}(encoder func(*http.Request) goahttp.Encoder) func(*ht
}
{{- else if .StringSlice }}
for _, value := range p{{ if .FieldName }}.{{ .FieldName }}{{ end }} {
values.Add("{{ .Name }}", value)
values.Add("{{ .HTTPName }}", value)
}
{{- else if .Slice }}
for _, value := range p{{ if .FieldName }}.{{ .FieldName }}{{ end }} {
{{ template "type_conversion" (typeConversionData .Type.ElemType.Type (aliasedType .FieldType).ElemType.Type "valueStr" "value") }}
values.Add("{{ .Name }}", valueStr)
values.Add("{{ .HTTPName }}", valueStr)
}
{{- else if .Map }}
{{- template "map_conversion" (mapConversionData .Type .FieldType .Name "p" .FieldName true) }}
{{- template "map_conversion" (mapConversionData .Type .FieldType .HTTPName "p" .FieldName true) }}
{{- else if .FieldName }}
{{- if .FieldPointer }}
if p.{{ .FieldName }} != nil {
{{- end }}
values.Add("{{ .Name }}",
values.Add("{{ .HTTPName }}",
{{- if or (eq .Type.Name "bytes") (and (isAlias .FieldType) (eq (underlyingType .FieldType).Name "string")) }} string(
{{- else if not (eq .Type.Name "string") }} fmt.Sprintf("%v",
{{- end }}
Expand All @@ -530,12 +530,12 @@ func {{ .RequestEncoder }}(encoder func(*http.Request) goahttp.Encoder) func(*ht
{{- end }}
{{- else }}
{{- if eq .Type.Name "string" }}
values.Add("{{ .Name }}", p)
values.Add("{{ .HTTPName }}", p)
{{- else if (and (isAlias .Type) (eq (underlyingType .Type).Name "string")) }}
values.Add("{{ .Name }}", string(p))
values.Add("{{ .HTTPName }}", string(p))
{{- else }}
{{ template "type_conversion" (typeConversionData .Type .FieldType "pStr" "p") }}
values.Add("{{ .Name }}", pStr)
values.Add("{{ .HTTPName }}", pStr)
{{- end }}
{{- end }}
{{- end }}
Expand Down Expand Up @@ -877,7 +877,7 @@ const singleResponseT = ` {{- if .ClientBody }}
for _, c := range cookies {
switch c.Name {
{{- range .Cookies }}
case {{ printf "%q" .Name }}:
case {{ printf "%q" .HTTPName }}:
{{ .VarName }}Raw = c.Value
{{- end }}
}
Expand Down
24 changes: 12 additions & 12 deletions http/codegen/multipart_test.go
View file
Expand Up @@ -77,12 +77,12 @@ func TestServerMultipartNewFunc(t *testing.T) {
DSL func()
Code string
}{
{"multipart-body-primitive", testdata.PayloadMultipartPrimitiveDSL, testdata.MultipartPrimitiveDecoderFuncCode},
{"multipart-body-user-type", testdata.PayloadMultipartUserTypeDSL, testdata.MultipartUserTypeDecoderFuncCode},
{"multipart-body-array-type", testdata.PayloadMultipartArrayTypeDSL, testdata.MultipartArrayTypeDecoderFuncCode},
{"multipart-body-map-type", testdata.PayloadMultipartMapTypeDSL, testdata.MultipartMapTypeDecoderFuncCode},
{"multipart-with-param", testdata.PayloadMultipartWithParamDSL, testdata.MultipartWithParamDecoderFuncCode},
{"multipart-with-params-and-headers", testdata.PayloadMultipartWithParamsAndHeadersDSL, testdata.MultipartWithParamsAndHeadersDecoderFuncCode},
{"server-multipart-body-primitive", testdata.PayloadMultipartPrimitiveDSL, testdata.MultipartPrimitiveDecoderFuncCode},
{"server-multipart-body-user-type", testdata.PayloadMultipartUserTypeDSL, testdata.MultipartUserTypeDecoderFuncCode},
{"server-multipart-body-array-type", testdata.PayloadMultipartArrayTypeDSL, testdata.MultipartArrayTypeDecoderFuncCode},
{"server-multipart-body-map-type", testdata.PayloadMultipartMapTypeDSL, testdata.MultipartMapTypeDecoderFuncCode},
{"server-multipart-with-param", testdata.PayloadMultipartWithParamDSL, testdata.MultipartWithParamDecoderFuncCode},
{"server-multipart-with-params-and-headers", testdata.PayloadMultipartWithParamsAndHeadersDSL, testdata.MultipartWithParamsAndHeadersDecoderFuncCode},
}
for _, c := range cases {
t.Run(c.Name, func(t *testing.T) {
Expand Down Expand Up @@ -110,12 +110,12 @@ func TestClientMultipartNewFunc(t *testing.T) {
DSL func()
Code string
}{
{"multipart-body-primitive", testdata.PayloadMultipartPrimitiveDSL, testdata.MultipartPrimitiveEncoderFuncCode},
{"multipart-body-user-type", testdata.PayloadMultipartUserTypeDSL, testdata.MultipartUserTypeEncoderFuncCode},
{"multipart-body-array-type", testdata.PayloadMultipartArrayTypeDSL, testdata.MultipartArrayTypeEncoderFuncCode},
{"multipart-body-map-type", testdata.PayloadMultipartMapTypeDSL, testdata.MultipartMapTypeEncoderFuncCode},
{"multipart-with-param", testdata.PayloadMultipartWithParamDSL, testdata.MultipartWithParamEncoderFuncCode},
{"multipart-with-params-and-headers", testdata.PayloadMultipartWithParamsAndHeadersDSL, testdata.MultipartWithParamsAndHeadersEncoderFuncCode},
{"client-multipart-body-primitive", testdata.PayloadMultipartPrimitiveDSL, testdata.MultipartPrimitiveEncoderFuncCode},
{"client-multipart-body-user-type", testdata.PayloadMultipartUserTypeDSL, testdata.MultipartUserTypeEncoderFuncCode},
{"client-multipart-body-array-type", testdata.PayloadMultipartArrayTypeDSL, testdata.MultipartArrayTypeEncoderFuncCode},
{"client-multipart-body-map-type", testdata.PayloadMultipartMapTypeDSL, testdata.MultipartMapTypeEncoderFuncCode},
{"client-multipart-with-param", testdata.PayloadMultipartWithParamDSL, testdata.MultipartWithParamEncoderFuncCode},
{"client-multipart-with-params-and-headers", testdata.PayloadMultipartWithParamsAndHeadersDSL, testdata.MultipartWithParamsAndHeadersEncoderFuncCode},
}
for _, c := range cases {
t.Run(c.Name, func(t *testing.T) {
Expand Down

0 comments on commit 27c249f

Please sign in to comment.