From 62736c787ba439d1c2c937b7dfe244d500c6ded3 Mon Sep 17 00:00:00 2001 From: Raphael Simon Date: Thu, 22 Oct 2026 22:14:38 -0700 Subject: [PATCH] Validate selected union branches before checking fields --- .../generate_named_union_conversion_test.go | 65 ++++- ...union_value_validation_integration_test.go | 267 ++++++++++++++++++ ...ation_named_definition_integration_test.go | 6 + codegen/service/testdata/views_code.go | 2 +- codegen/templates/validation/union_sum.go.tpl | 14 +- ...ion_union-with-format-validation.go.golden | 15 +- .../validation_union-with-view.go.golden | 87 +++--- .../golden/validation_union.go.golden | 87 +++--- codegen/validation.go | 33 ++- codegen/validation_plan.go | 89 ++---- codegen/validation_union_plan.go | 84 ++++++ codegen/validation_union_value_test.go | 66 +++++ .../golden/http_union_branch_order.go.golden | 34 ++- ...er_decode_decode-body-union-user.go.golden | 4 + .../server_decode_decode-body-union.go.golden | 4 + ...s_optional_union_request_decoder.go.golden | 10 + 16 files changed, 685 insertions(+), 182 deletions(-) create mode 100644 codegen/generator/union_value_validation_integration_test.go create mode 100644 codegen/validation_union_plan.go create mode 100644 codegen/validation_union_value_test.go diff --git a/codegen/generator/generate_named_union_conversion_test.go b/codegen/generator/generate_named_union_conversion_test.go index ad93b8b532..f3b39a7e88 100644 --- a/codegen/generator/generate_named_union_conversion_test.go +++ b/codegen/generator/generate_named_union_conversion_test.go @@ -17,6 +17,15 @@ import ( "goa.design/goa/v3/expr" ) +type ( + // namedUnionValidationAttributor keeps the fixture's private validator owner + // while the service resolver supplies the generated union method names. + namedUnionValidationAttributor struct { + codegen.Attributor + validator *codegen.NameDeclaration + } +) + func TestGenerateNamedUnionConversions(t *testing.T) { root := codegen.RunDSL(t, namedUnionConversionDSL) base := &expr.AttributeExpr{Type: root.UserType("Base")} @@ -48,7 +57,9 @@ func TestGenerateNamedUnionConversions(t *testing.T) { var imports *codegen.GeneratedImportPlan var validation *codegen.ValidationPlan var holderValidation, presenceValidation, pointerValidation *codegen.ValidationPlan - var presenceLayout, pointerLayout *codegen.GoTypePlan + var presenceLayout, pointerLayout, plainLayout *codegen.GoTypePlan + var plainValidation *codegen.ValidationPlan + var plainValidator *codegen.NameDeclaration functions := make([]map[string]string, 0, len(operations)) var optionalDefinition string registry := newDefaultRegistry() @@ -133,21 +144,41 @@ func TestGenerateNamedUnionConversions(t *testing.T) { Owner: "generated.local/gen/values/types", Bind: bind, RetainNamedValue: true, Policy: codegen.GoLayoutPolicy{UseDefault: true, SumType: true}, } + plain := &expr.AttributeExpr{Type: root.UserType("PlainDerived")} + plainValidator = codegen.NewExactName(codegen.NameFunction, "ValidatePlainDerived") + if err := pkg.DeclareName(plainValidator); err != nil { + return err + } + plainLayout, err = codegen.PlanGoType(plain, options) + if err != nil { + return err + } + plainValidation, err = codegen.NewValidationPlan(plain, plainLayout, codegen.ValidationPlanOptions{Required: true}) + if err != nil { + return err + } + presenceOptions := codegen.ValidationPlanOptions{Required: true, Bind: func(request codegen.ValidatorBindingRequest) (*codegen.NameDeclaration, error) { + if request.Attribute.Type != plain.Type { + return nil, fmt.Errorf("unexpected presence validator for %s", request.Attribute.Type.Name()) + } + return plainValidator, nil + }} presenceLayout, err = codegen.PlanGoType(presence, options) if err != nil { return err } - presenceValidation, err = codegen.NewValidationPlan(presence, presenceLayout, codegen.ValidationPlanOptions{Required: true}) + presenceValidation, err = codegen.NewValidationPlan(presence, presenceLayout, presenceOptions) if err != nil { return err } - require.Empty(t, presenceValidation.ValidatorDeclarations()) + require.Equal(t, []*codegen.NameDeclaration{plainValidator}, presenceValidation.ValidatorDeclarations()) wantPresenceImports := []codegen.GoTypeImport{ {Name: "goa", Path: codegen.GoaImport("").Path}, {Path: "generated.local/gen/left/types"}, + {Name: "checks", Path: pkg.ImportPath()}, } require.Equal(t, wantPresenceImports, presenceValidation.ImportPreferences(), - "the required check alone needs the union method owner's import") + "union checks retain the method owner and nested validator imports") presenceDefinition := presence.Type.(expr.UserType).Attribute() for _, collection := range []expr.DataType{ &expr.Array{ElemType: presenceDefinition}, @@ -158,7 +189,7 @@ func TestGenerateNamedUnionConversions(t *testing.T) { if err != nil { return err } - validation, err := codegen.NewValidationPlan(attribute, layout, codegen.ValidationPlanOptions{Required: true}) + validation, err := codegen.NewValidationPlan(attribute, layout, presenceOptions) if err != nil { return err } @@ -170,7 +201,7 @@ func TestGenerateNamedUnionConversions(t *testing.T) { if err != nil { return err } - _, err = codegen.NewValidationPlan(presenceDefinition, incomplete, codegen.ValidationPlanOptions{Required: true}) + _, err = codegen.NewValidationPlan(presenceDefinition, incomplete, presenceOptions) require.ErrorContains(t, err, `required field "choice": named union layout does not retain its union definition`) options.RetainNamedValue = true options.Policy.Pointer, options.Policy.UnionPointer = true, true @@ -178,13 +209,13 @@ func TestGenerateNamedUnionConversions(t *testing.T) { if err != nil { return err } - pointerValidation, err = codegen.NewValidationPlan(presenceDefinition, pointerLayout, codegen.ValidationPlanOptions{Required: true}) + pointerValidation, err = codegen.NewValidationPlan(presenceDefinition, pointerLayout, presenceOptions) if err != nil { return err } - require.Equal(t, wantPresenceImports[:1], pointerValidation.ImportPreferences(), - "a nil presence check does not call union methods") - for _, validation := range []*codegen.ValidationPlan{validation, holderValidation, presenceValidation, pointerValidation} { + require.Equal(t, []codegen.GoTypeImport{wantPresenceImports[0], wantPresenceImports[2]}, pointerValidation.ImportPreferences(), + "pointer fields call the nested validator without checking the union method owner directly") + for _, validation := range []*codegen.ValidationPlan{validation, holderValidation, presenceValidation, pointerValidation, plainValidation} { for _, spec := range validation.ImportPreferences() { if spec.Path == codegen.GoaImport("").Path { err = imports.Require(codegen.NewImport(spec.Name, spec.Path)) @@ -274,6 +305,7 @@ func TestGenerateNamedUnionConversions(t *testing.T) { plan *codegen.ValidationPlan layout *codegen.GoTypePlan }{ + {"ValidatePlainDerived", plainValidation, plainLayout}, {"ValidateHolder", holderValidation, operations[5].sourceType}, {"ValidatePresence", presenceValidation, presenceLayout}, {"ValidatePresencePointer", pointerValidation, pointerLayout}, @@ -297,7 +329,7 @@ func TestGenerateNamedUnionConversions(t *testing.T) { }) } legacy := codegen.NewAttributeContext(false, false, true, "", codegen.NewNameScope()) - legacy.Scope = plan.Service(root).Services().ServiceAttributor("catalog", pkg.ImportPath()).Enter(presence) + legacy.Scope = &namedUnionValidationAttributor{Attributor: plan.Service(root).Services().ServiceAttributor("catalog", pkg.ImportPath()).Enter(presence), validator: plainValidator} validators = append(validators, map[string]string{ "Name": "ValidatePresenceLegacy", "Parameter": presenceLayout.Link(pkg.ImportPath(), pkg.ImportName).Ref(), @@ -406,6 +438,11 @@ func {{.Name}}(input {{.Parameter}}) {{.Result}} { runGeneratedTests(t, directory) } +// ValidatorCall sends the fixture's named field to its retained private validator. +func (a *namedUnionValidationAttributor) ValidatorCall(_ *expr.AttributeExpr, _, target, _ string) string { + return fmt.Sprintf("%s(%s)", a.validator.Name(), target) +} + func namedUnionConversionDSL() { d.Service("catalog", func() {}) base := d.Type("Base", &expr.Union{TypeName: "Choice"}, func() { @@ -517,10 +554,10 @@ func TestRequiredNamedUnionPresence(t *testing.T) { } if err := ValidatePresencePointer(&PresencePointer{}); err == nil { t.Fatal("accepted nil required pointer") } selected := {{.PlainDerived}}({{.plainText}}("")) - for _, pointer := range []*{{.PlainDerived}}{&selected, new({{.PlainDerived}})} { + for index, pointer := range []*{{.PlainDerived}}{&selected, new({{.PlainDerived}})} { before := *pointer - if err := ValidatePresencePointer(&PresencePointer{Choice: pointer}); err != nil { - t.Fatalf("pointer presence check rejected a nonnil field: %v", err) + if err := ValidatePresencePointer(&PresencePointer{Choice: pointer}); (err != nil) != (index == 1) { + t.Fatalf("pointer union invalid = %t, want %t: %v", err != nil, index == 1, err) } if !reflect.DeepEqual(*pointer, before) { t.Fatal("pointer presence validation changed its caller") } } diff --git a/codegen/generator/union_value_validation_integration_test.go b/codegen/generator/union_value_validation_integration_test.go new file mode 100644 index 0000000000..a9c0d08c37 --- /dev/null +++ b/codegen/generator/union_value_validation_integration_test.go @@ -0,0 +1,267 @@ +// These tests generate ordinary union declarations and execute planned and +// legacy validators against selected branches, optional fields, and aliases. +package generator + +import ( + "fmt" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + "goa.design/goa/v3/codegen" + d "goa.design/goa/v3/dsl" + "goa.design/goa/v3/eval" + "goa.design/goa/v3/expr" +) + +func TestGenerateUnionValueValidation(t *testing.T) { + root := codegen.RunDSL(t, unionValueValidationDSL) + type operation struct { + name string + attribute *expr.AttributeExpr + declaration *codegen.NameDeclaration + layout *codegen.GoTypePlan + validation *codegen.ValidationPlan + } + operations := []*operation{ + {name: "CheckChoicePlanned", attribute: &expr.AttributeExpr{Type: root.UserType("Choice")}}, + {name: "CheckDerivedPlanned", attribute: &expr.AttributeExpr{Type: root.UserType("Derived")}}, + {name: "CheckHolderPlanned", attribute: &expr.AttributeExpr{Type: root.UserType("Holder")}}, + } + var pkg *codegen.GeneratedPackage + var imports *codegen.GeneratedImportPlan + registry := newDefaultRegistry() + registry.registerPlugin("union-value-validation", "gen", pluginNormal, func() Plugin { + return Plugin{ + Plan: func(plan *Plan) error { + pkg = plan.Generation().Package("generated.local/gen/catalog") + imports = codegen.NewGeneratedImportPlan(pkg) + for _, op := range operations { + op.declaration = codegen.NewExactName(codegen.NameFunction, op.name) + if err := pkg.DeclareName(op.declaration); err != nil { + return err + } + } + for _, name := range []string{"CheckChoiceLegacy", "CheckDerivedLegacy"} { + if err := pkg.DeclareName(codegen.NewExactName(codegen.NameFunction, name)); err != nil { + return err + } + } + bind := func(request codegen.GoTypeBindingRequest) (codegen.GoTypeBinding, error) { + binding := codegen.GoTypeBinding{Owner: pkg.ImportPath()} + var err error + if request.Kind == codegen.GoUnion { + binding.Union, err = pkg.Union(request.Attribute) + } else { + binding.Type, err = pkg.Type(request.Attribute.Type.(expr.UserType)) + } + return binding, err + } + for _, op := range operations { + var err error + op.layout, err = codegen.PlanGoType(op.attribute, codegen.GoTypePlanOptions{ + Owner: pkg.ImportPath(), + Bind: bind, + RetainNamedValue: true, + Policy: codegen.GoLayoutPolicy{UseDefault: true, SumType: true}, + }) + if err != nil { + return err + } + op.validation, err = codegen.NewValidationPlan(op.attribute, op.layout, codegen.ValidationPlanOptions{ + Required: true, + Bind: func(request codegen.ValidatorBindingRequest) (*codegen.NameDeclaration, error) { + for _, nested := range operations { + if nested.attribute.Type == request.Attribute.Type { + return nested.declaration, nil + } + } + return nil, fmt.Errorf("unexpected nested validator for %s", request.Attribute.Type.Name()) + }, + }) + if err != nil { + return err + } + for _, spec := range op.validation.ImportPreferences() { + if spec.Path == pkg.ImportPath() { + continue + } + if err := imports.Require(codegen.NewImport(spec.Name, spec.Path)); err != nil { + return err + } + } + } + return nil + }, + Generate: func(_ *Plan, files []*codegen.File) ([]*codegen.File, error) { + if err := imports.Link(); err != nil { + return nil, err + } + functions := make([]map[string]string, 0, len(operations)+1) + for _, op := range operations { + layout := op.layout.Link(pkg.ImportPath(), pkg.ImportName) + validation, err := op.validation.Link(layout) + if err != nil { + return nil, err + } + functions = append(functions, map[string]string{ + "Name": op.name, + "Parameter": layout.Ref(), + "Body": validation.Render("value", "body"), + }) + } + for index, name := range []string{"CheckChoiceLegacy", "CheckDerivedLegacy"} { + op := operations[index] + layout := op.layout.Link(pkg.ImportPath(), pkg.ImportName) + context, err := codegen.NewAttributeContext(false, false, true, "", codegen.NewNameScope()).WithGoTypeLayout(layout) + if err != nil { + return nil, err + } + functions = append(functions, map[string]string{ + "Name": name, + "Parameter": layout.Ref(), + "Body": codegen.ValidationCode(op.attribute, nil, context, true, false, false, "value"), + }) + } + return append(files, &codegen.File{ + Path: filepath.Join(pkg.OutputDirectory(), "value_validation.go"), + SectionTemplates: []*codegen.SectionTemplate{ + codegen.Header("Union value validation", "catalog", imports.Imports()), + {Name: "validators", Source: "{{range .}}func {{.Name}}(value {{.Parameter}}) (err error) {\n{{.Body}}\nreturn err\n}\n{{end}}", Data: functions}, + }, + }), nil + }, + } + }) + run, err := newGenerationRun("gen", registry) + require.NoError(t, err) + result, err := run.execute("generated.local/gen", []eval.Root{root}) + require.NoError(t, err) + data := make(map[string]string) + for _, name := range []string{"Choice", "Derived", "Holder", "Empty"} { + declaration, err := pkg.Type(root.UserType(name)) + require.NoError(t, err) + data[name] = declaration.Name() + } + union, err := pkg.Union(root.UserType("Choice").Attribute()) + require.NoError(t, err) + data["Union"] = union.Name() + for _, name := range []string{"empty", "words", "counts", "text"} { + branch, err := pkg.UnionBranch(root.UserType("Choice").Attribute(), name) + require.NoError(t, err) + data[name] = branch.Constructor() + } + directory := t.TempDir() + writeGeneratedModule(t, directory, "generated.local") + for _, file := range result.files { + _, err := file.Render(directory) + require.NoError(t, err) + } + testFile := &codegen.File{ + Path: filepath.Join(pkg.OutputDirectory(), "value_validation_test.go"), + SectionTemplates: []*codegen.SectionTemplate{ + codegen.Header("Union value validation tests", "catalog", []*codegen.ImportSpec{codegen.SimpleImport("testing"), codegen.SimpleImport("reflect")}), + {Name: "tests", Source: unionValueValidationTests, Data: data}, + }, + } + _, err = testFile.Render(directory) + require.NoError(t, err) + runGeneratedTests(t, directory) +} + +func unionValueValidationDSL() { + empty := d.Type("Empty", func() { + }) + choice := d.Type("Choice", &expr.Union{TypeName: "ChoiceValue"}, func() { + d.Attribute("empty", empty) + d.Attribute("words", d.ArrayOf(d.String)) + d.Attribute("counts", d.MapOf(d.String, d.Int)) + d.Attribute("text", d.String, func() { + d.MinLength(1) + }) + }) + derived := d.Type("Derived", choice) + holder := d.Type("Holder", func() { + d.Attribute("required", choice) + d.Attribute("optional", choice) + d.Attribute("aliased", derived) + d.Attribute("choices", d.ArrayOf(choice)) + d.Required("required", "choices") + }) + d.Service("catalog", func() { + d.Method("inspect", func() { + d.Payload(holder) + }) + }) +} + +const unionValueValidationTests = ` +func TestSelectedUnionValidation(t *testing.T) { + for _, test := range []struct { + name string + value {{.Choice}} + invalid bool + }{ + {"missing kind", {{.Choice}}{}, true}, + {"unknown kind", {{.Choice}}({{.Union}}{kind: "other"}), true}, + {"nil object", {{.Choice}}({{.empty}}(nil)), true}, + {"empty object", {{.Choice}}({{.empty}}(&{{.Empty}}{})), false}, + {"empty array", {{.Choice}}({{.words}}([]string{})), false}, + {"empty map", {{.Choice}}({{.counts}}(map[string]int{})), false}, + {"text", {{.Choice}}({{.text}}("hello")), false}, + {"constrained text", {{.Choice}}({{.text}}("")), true}, + } { + t.Run(test.name, func(t *testing.T) { + before := test.value + for _, validate := range []func(*{{.Choice}}) error{CheckChoicePlanned, CheckChoiceLegacy} { + if err := validate(&test.value); (err != nil) != test.invalid { + t.Errorf("invalid=%t want=%t: %v", err != nil, test.invalid, err) + } + } + derived := {{.Derived}}(test.value) + for _, validate := range []func(*{{.Derived}}) error{CheckDerivedPlanned, CheckDerivedLegacy} { + if err := validate(&derived); (err != nil) != test.invalid { + t.Errorf("derived invalid=%t want=%t: %v", err != nil, test.invalid, err) + } + } + if !reflect.DeepEqual(before, test.value) { + t.Error("validation changed caller value") + } + }) + } +} + +func TestUnionCollectionNilSemantics(t *testing.T) { + for _, selected := range []{{.Union}}{ {{.words}}(nil), {{.counts}}(nil) } { + value := {{.Choice}}(selected) + wantInvalid := selected.Validate() != nil + for _, validate := range []func(*{{.Choice}}) error{CheckChoicePlanned, CheckChoiceLegacy} { + if err := validate(&value); (err != nil) != wantInvalid { + t.Errorf("collection invalid=%t want=%t: %v", err != nil, wantInvalid, err) + } + } + } +} + +func TestOptionalUnionAndCollections(t *testing.T) { + selected := {{.Choice}}({{.text}}("hello")) + value := &{{.Holder}}{Required: selected, Choices: []{{.Choice}}{}} + if err := CheckHolderPlanned(value); err != nil { + t.Errorf("optional absent unions or empty collection rejected: %v", err) + } + value.Optional = {{.Choice}}({{.empty}}(nil)) + if err := CheckHolderPlanned(value); err == nil { + t.Error("accepted present optional nil branch") + } + value.Optional = selected + value.Aliased = {{.Derived}}({{.empty}}(nil)) + if err := CheckHolderPlanned(value); err == nil { + t.Error("accepted present aliased nil branch") + } + value.Aliased = {{.Derived}}(selected) + value.Choices = []{{.Choice}}{ {{.Choice}}({{.empty}}(nil)) } + if err := CheckHolderPlanned(value); err == nil { + t.Error("accepted collection element nil branch") + } +} +` diff --git a/codegen/generator/validation_named_definition_integration_test.go b/codegen/generator/validation_named_definition_integration_test.go index f2e697cf78..163fe625e3 100644 --- a/codegen/generator/validation_named_definition_integration_test.go +++ b/codegen/generator/validation_named_definition_integration_test.go @@ -92,6 +92,9 @@ func TestGenerateNamedDefinitionValidation(t *testing.T) { "Choice": root.UserType("Base").Attribute(), "Selection": expr.AsObject(root.UserType("Holder").Attribute().Type).Attribute("Selection"), } { + union, err := left.Union(attribute) + require.NoError(t, err) + data[key+"Union"] = fixture.pkg.ImportName(left.ImportPath()) + "." + union.Name() for _, branchName := range []string{"text", "number"} { branch, err := left.UnionBranch(attribute, branchName) require.NoError(t, err) @@ -406,6 +409,9 @@ func TestDirectUnionValueAndOptionalPointer(t *testing.T) { if err := ValidateOptional(&Optional{}); err != nil { t.Errorf("absent optional union: %v", err) } + if err := ValidateOptional(&Optional{Selection: new({{.SelectionUnion}})}); err == nil { + t.Error("present optional union accepted a missing branch") + } for _, invalid := range []bool{false, true} { value := {{.Selectiontext}}("valid") if invalid { diff --git a/codegen/service/testdata/views_code.go b/codegen/service/testdata/views_code.go index ba2d6e8e0d..643f98cccd 100644 --- a/codegen/service/testdata/views_code.go +++ b/codegen/service/testdata/views_code.go @@ -850,4 +850,4 @@ func ValidateRT(result *RT) (err error) { } ` -const ResultWithOneOfInResultTypeCode = "// OneOfResource is the viewed result type that is projected based on a view.\ntype OneOfResource struct {\n\t// Type to project\n\tProjected *OneOfResourceView\n\t// View to render\n\tView string\n}\n\n// OneOfResourceView is a type that runs validations on a projected type.\ntype OneOfResourceView struct {\n\t// Data (type depends on flag)\n\tData *OneOfValueView\n}\n\n// OneOfValueView is a type that runs validations on a projected type.\ntype OneOfValueView struct {\n\tFlag Flag\n}\n\n// FlagAstringView is a type that runs validations on a projected type.\ntype FlagAstringView string\n\n// FlagAintView is a type that runs validations on a projected type.\ntype FlagAintView int64\n\n// Flag holds exactly one of its branch values.\ntype Flag struct {\n\tkind FlagKind\n\tastring FlagAstringView\n\taint FlagAintView\n}\n\n// FlagKind records which Flag branch is selected.\ntype FlagKind string\n\nconst (\n\t// FlagKindAstring identifies the astring branch.\n\tFlagKindAstring FlagKind = \"astring\"\n\t// FlagKindAint identifies the aint branch.\n\tFlagKindAint FlagKind = \"aint\"\n)\n\n// Kind returns the selected branch.\nfunc (u Flag) Kind() FlagKind {\n\treturn u.kind\n}\n\n// NewFlagAstring constructs Flag with the astring branch set.\nfunc NewFlagAstring(v FlagAstringView) Flag {\n\treturn Flag{\n\t\tkind: FlagKindAstring,\n\t\tastring: v,\n\t}\n}\n\n// AsAstring returns the value when the astring branch is selected.\nfunc (u Flag) AsAstring() (_ FlagAstringView, ok bool) {\n\tif u.kind != FlagKindAstring {\n\t\treturn\n\t}\n\treturn u.astring, true\n}\n\n// SetAstring selects the astring branch and stores v.\nfunc (u *Flag) SetAstring(v FlagAstringView) {\n\t*u = Flag{\n\t\tkind: FlagKindAstring,\n\t\tastring: v,\n\t}\n}\n\n// NewFlagAint constructs Flag with the aint branch set.\nfunc NewFlagAint(v FlagAintView) Flag {\n\treturn Flag{\n\t\tkind: FlagKindAint,\n\t\taint: v,\n\t}\n}\n\n// AsAint returns the value when the aint branch is selected.\nfunc (u Flag) AsAint() (_ FlagAintView, ok bool) {\n\tif u.kind != FlagKindAint {\n\t\treturn\n\t}\n\treturn u.aint, true\n}\n\n// SetAint selects the aint branch and stores v.\nfunc (u *Flag) SetAint(v FlagAintView) {\n\t*u = Flag{\n\t\tkind: FlagKindAint,\n\t\taint: v,\n\t}\n}\n\n// Validate ensures exactly one valid branch is selected.\nfunc (u Flag) Validate() error {\n\t_, err := u.Value()\n\treturn err\n}\n\n// Value returns the selected branch value, or the same selection error as Validate.\n// Go templates can read this method directly; an invalid selection stops execution.\nfunc (u Flag) Value() (any, error) {\n\tswitch u.kind {\n\tcase \"\":\n\t\treturn nil, goa.InvalidEnumValueError(\"type\", \"\", []any{\n\t\t\tstring(FlagKindAstring),\n\t\t\tstring(FlagKindAint),\n\t\t})\n\tcase FlagKindAstring:\n\t\treturn u.astring, nil\n\tcase FlagKindAint:\n\t\treturn u.aint, nil\n\tdefault:\n\t\treturn nil, goa.InvalidEnumValueError(\"type\", u.kind, []any{\n\t\t\tstring(FlagKindAstring),\n\t\t\tstring(FlagKindAint),\n\t\t})\n\t}\n}\n\n// MarshalJSON marshals the union into the canonical {type,value} JSON shape.\nfunc (u Flag) MarshalJSON() ([]byte, error) {\n\tvalue, err := u.Value()\n\tif err != nil {\n\t\treturn nil, err\n\t}\n\treturn json.Marshal(struct {\n\t\tType string `json:\"type\"`\n\t\tValue any `json:\"value\"`\n\t}{\n\t\tType: string(u.kind),\n\t\tValue: value,\n\t})\n}\n\n// UnmarshalJSON unmarshals the union from the canonical {type,value} JSON shape.\nfunc (u *Flag) UnmarshalJSON(data []byte) error {\n\tvar raw struct {\n\t\tType string `json:\"type\"`\n\t\tValue json.RawMessage `json:\"value\"`\n\t}\n\tif err := json.Unmarshal(data, &raw); err != nil {\n\t\treturn err\n\t}\n\tif len(raw.Value) == 0 {\n\t\treturn goa.MissingFieldError(\"value\", \"Flag\")\n\t}\n\tif bytes.Equal(bytes.TrimSpace(raw.Value), []byte(\"null\")) {\n\t\treturn goa.InvalidFieldTypeError(\"value\", nil, \"non-null JSON value\")\n\t}\n\tswitch raw.Type {\n\tcase string(FlagKindAstring):\n\t\tvar v FlagAstringView\n\t\tif err := json.Unmarshal(raw.Value, &v); err != nil {\n\t\t\treturn err\n\t\t}\n\t\tu.SetAstring(v)\n\tcase string(FlagKindAint):\n\t\tvar v FlagAintView\n\t\tif err := json.Unmarshal(raw.Value, &v); err != nil {\n\t\t\treturn err\n\t\t}\n\t\tu.SetAint(v)\n\tdefault:\n\t\tif raw.Type == \"\" {\n\t\t\treturn goa.MissingFieldError(\"type\", \"Flag\")\n\t\t}\n\t\treturn goa.InvalidEnumValueError(\"type\", raw.Type, []any{\n\t\t\tstring(FlagKindAstring),\n\t\t\tstring(FlagKindAint),\n\t\t})\n\t}\n\treturn nil\n}\n\nvar (\n\t// OneOfResourceMap is a map indexing the attribute names of OneOfResource by\n\t// view name.\n\tOneOfResourceMap = map[string][]string{\n\t\t\"default\": {\n\t\t\t\"data\",\n\t\t},\n\t}\n)\n\n// ValidateOneOfResource runs the validations defined on the viewed result type\n// OneOfResource.\nfunc ValidateOneOfResource(result *OneOfResource) (err error) {\n\tswitch result.View {\n\tcase \"default\", \"\":\n\t\terr = ValidateOneOfResourceView(result.Projected)\n\tdefault:\n\t\terr = goa.InvalidEnumValueError(\"view\", result.View, []any{\"default\"})\n\t}\n\treturn\n}\n\n// ValidateOneOfResourceView runs the validations defined on OneOfResourceView\n// using the \"default\" view.\nfunc ValidateOneOfResourceView(result *OneOfResourceView) (err error) {\n\tif result.Data == nil {\n\t\terr = goa.MergeErrors(err, goa.MissingFieldError(\"data\", \"result\"))\n\t}\n\treturn\n}\n" +const ResultWithOneOfInResultTypeCode = "// OneOfResource is the viewed result type that is projected based on a view.\ntype OneOfResource struct {\n\t// Type to project\n\tProjected *OneOfResourceView\n\t// View to render\n\tView string\n}\n\n// OneOfResourceView is a type that runs validations on a projected type.\ntype OneOfResourceView struct {\n\t// Data (type depends on flag)\n\tData *OneOfValueView\n}\n\n// OneOfValueView is a type that runs validations on a projected type.\ntype OneOfValueView struct {\n\tFlag Flag\n}\n\n// FlagAstringView is a type that runs validations on a projected type.\ntype FlagAstringView string\n\n// FlagAintView is a type that runs validations on a projected type.\ntype FlagAintView int64\n\n// Flag holds exactly one of its branch values.\ntype Flag struct {\n\tkind FlagKind\n\tastring FlagAstringView\n\taint FlagAintView\n}\n\n// FlagKind records which Flag branch is selected.\ntype FlagKind string\n\nconst (\n\t// FlagKindAstring identifies the astring branch.\n\tFlagKindAstring FlagKind = \"astring\"\n\t// FlagKindAint identifies the aint branch.\n\tFlagKindAint FlagKind = \"aint\"\n)\n\n// Kind returns the selected branch.\nfunc (u Flag) Kind() FlagKind {\n\treturn u.kind\n}\n\n// NewFlagAstring constructs Flag with the astring branch set.\nfunc NewFlagAstring(v FlagAstringView) Flag {\n\treturn Flag{\n\t\tkind: FlagKindAstring,\n\t\tastring: v,\n\t}\n}\n\n// AsAstring returns the value when the astring branch is selected.\nfunc (u Flag) AsAstring() (_ FlagAstringView, ok bool) {\n\tif u.kind != FlagKindAstring {\n\t\treturn\n\t}\n\treturn u.astring, true\n}\n\n// SetAstring selects the astring branch and stores v.\nfunc (u *Flag) SetAstring(v FlagAstringView) {\n\t*u = Flag{\n\t\tkind: FlagKindAstring,\n\t\tastring: v,\n\t}\n}\n\n// NewFlagAint constructs Flag with the aint branch set.\nfunc NewFlagAint(v FlagAintView) Flag {\n\treturn Flag{\n\t\tkind: FlagKindAint,\n\t\taint: v,\n\t}\n}\n\n// AsAint returns the value when the aint branch is selected.\nfunc (u Flag) AsAint() (_ FlagAintView, ok bool) {\n\tif u.kind != FlagKindAint {\n\t\treturn\n\t}\n\treturn u.aint, true\n}\n\n// SetAint selects the aint branch and stores v.\nfunc (u *Flag) SetAint(v FlagAintView) {\n\t*u = Flag{\n\t\tkind: FlagKindAint,\n\t\taint: v,\n\t}\n}\n\n// Validate ensures exactly one valid branch is selected.\nfunc (u Flag) Validate() error {\n\t_, err := u.Value()\n\treturn err\n}\n\n// Value returns the selected branch value, or the same selection error as Validate.\n// Go templates can read this method directly; an invalid selection stops execution.\nfunc (u Flag) Value() (any, error) {\n\tswitch u.kind {\n\tcase \"\":\n\t\treturn nil, goa.InvalidEnumValueError(\"type\", \"\", []any{\n\t\t\tstring(FlagKindAstring),\n\t\t\tstring(FlagKindAint),\n\t\t})\n\tcase FlagKindAstring:\n\t\treturn u.astring, nil\n\tcase FlagKindAint:\n\t\treturn u.aint, nil\n\tdefault:\n\t\treturn nil, goa.InvalidEnumValueError(\"type\", u.kind, []any{\n\t\t\tstring(FlagKindAstring),\n\t\t\tstring(FlagKindAint),\n\t\t})\n\t}\n}\n\n// MarshalJSON marshals the union into the canonical {type,value} JSON shape.\nfunc (u Flag) MarshalJSON() ([]byte, error) {\n\tvalue, err := u.Value()\n\tif err != nil {\n\t\treturn nil, err\n\t}\n\treturn json.Marshal(struct {\n\t\tType string `json:\"type\"`\n\t\tValue any `json:\"value\"`\n\t}{\n\t\tType: string(u.kind),\n\t\tValue: value,\n\t})\n}\n\n// UnmarshalJSON unmarshals the union from the canonical {type,value} JSON shape.\nfunc (u *Flag) UnmarshalJSON(data []byte) error {\n\tvar raw struct {\n\t\tType string `json:\"type\"`\n\t\tValue json.RawMessage `json:\"value\"`\n\t}\n\tif err := json.Unmarshal(data, &raw); err != nil {\n\t\treturn err\n\t}\n\tif len(raw.Value) == 0 {\n\t\treturn goa.MissingFieldError(\"value\", \"Flag\")\n\t}\n\tif bytes.Equal(bytes.TrimSpace(raw.Value), []byte(\"null\")) {\n\t\treturn goa.InvalidFieldTypeError(\"value\", nil, \"non-null JSON value\")\n\t}\n\tswitch raw.Type {\n\tcase string(FlagKindAstring):\n\t\tvar v FlagAstringView\n\t\tif err := json.Unmarshal(raw.Value, &v); err != nil {\n\t\t\treturn err\n\t\t}\n\t\tu.SetAstring(v)\n\tcase string(FlagKindAint):\n\t\tvar v FlagAintView\n\t\tif err := json.Unmarshal(raw.Value, &v); err != nil {\n\t\t\treturn err\n\t\t}\n\t\tu.SetAint(v)\n\tdefault:\n\t\tif raw.Type == \"\" {\n\t\t\treturn goa.MissingFieldError(\"type\", \"Flag\")\n\t\t}\n\t\treturn goa.InvalidEnumValueError(\"type\", raw.Type, []any{\n\t\t\tstring(FlagKindAstring),\n\t\t\tstring(FlagKindAint),\n\t\t})\n\t}\n\treturn nil\n}\n\nvar (\n\t// OneOfResourceMap is a map indexing the attribute names of OneOfResource by\n\t// view name.\n\tOneOfResourceMap = map[string][]string{\n\t\t\"default\": {\n\t\t\t\"data\",\n\t\t},\n\t}\n)\n\n// ValidateOneOfResource runs the validations defined on the viewed result type\n// OneOfResource.\nfunc ValidateOneOfResource(result *OneOfResource) (err error) {\n\tswitch result.View {\n\tcase \"default\", \"\":\n\t\terr = ValidateOneOfResourceView(result.Projected)\n\tdefault:\n\t\terr = goa.InvalidEnumValueError(\"view\", result.View, []any{\"default\"})\n\t}\n\treturn\n}\n\n// ValidateOneOfResourceView runs the validations defined on OneOfResourceView\n// using the \"default\" view.\nfunc ValidateOneOfResourceView(result *OneOfResourceView) (err error) {\n\tif result.Data == nil {\n\t\terr = goa.MergeErrors(err, goa.MissingFieldError(\"data\", \"result\"))\n\t}\n\tif result.Data != nil {\n\t\tif err2 := ValidateOneOfValueView(result.Data); err2 != nil {\n\t\t\terr = goa.MergeErrors(err, err2)\n\t\t}\n\t}\n\treturn\n}\n\n// ValidateOneOfValueView runs the validations defined on OneOfValueView.\nfunc ValidateOneOfValueView(result *OneOfValueView) (err error) {\n\n\tif result.Flag.Kind() != \"\" {\n\t\tif err2 := result.Flag.Validate(); err2 != nil {\n\t\t\terr = goa.MergeErrors(err, err2)\n\t\t}\n\t}\n\n\treturn\n}\n" diff --git a/codegen/templates/validation/union_sum.go.tpl b/codegen/templates/validation/union_sum.go.tpl index e3cff9d022..ead6d29660 100644 --- a/codegen/templates/validation/union_sum.go.tpl +++ b/codegen/templates/validation/union_sum.go.tpl @@ -1,7 +1,17 @@ +{{- if .optional }} +if {{ .target }}.Kind() != "" { +{{- end }} +if err2 := {{ .target }}.Validate(); err2 != nil { + err = {{ .goa }}.MergeErrors(err, err2) +}{{ if .cases }} else { switch string({{ .target }}.Kind()) { {{- range .cases }} case {{ printf "%q" .typeTag }}: - actual, _ := {{ $.target }}.As{{ .fieldName }}() - {{ .validation }} + actual, _ := {{ $.target }}.As{{ .fieldName }}() + {{ .validation }} {{- end }} } +}{{ end }} +{{- if .optional }} +} +{{- end }} diff --git a/codegen/testdata/golden/validation_union-with-format-validation.go.golden b/codegen/testdata/golden/validation_union-with-format-validation.go.golden index 7404c1869d..ae6b007b85 100644 --- a/codegen/testdata/golden/validation_union-with-format-validation.go.golden +++ b/codegen/testdata/golden/validation_union-with-format-validation.go.golden @@ -1,7 +1,14 @@ func Validate() (err error) { - switch string(target.Response.Kind()) { - case "timestamp": - actual, _ := target.Response.AsTimestamp() - err = goa.MergeErrors(err, goa.ValidateFormat("target.response.value", string(actual), goa.FormatDateTime)) + + if target.Response.Kind() != "" { + if err2 := target.Response.Validate(); err2 != nil { + err = goa.MergeErrors(err, err2) + } else { + switch string(target.Response.Kind()) { + case "timestamp": + actual, _ := target.Response.AsTimestamp() + err = goa.MergeErrors(err, goa.ValidateFormat("target.response.value", string(actual), goa.FormatDateTime)) + } + } } } diff --git a/codegen/testdata/golden/validation_union-with-view.go.golden b/codegen/testdata/golden/validation_union-with-view.go.golden index fa9ee7c056..9afd8b89f0 100644 --- a/codegen/testdata/golden/validation_union-with-view.go.golden +++ b/codegen/testdata/golden/validation_union-with-view.go.golden @@ -2,50 +2,61 @@ func Validate() (err error) { if target.RequiredUnion.Kind() == "" { err = goa.MergeErrors(err, goa.MissingFieldError("required_union", "target")) } - switch string(target.RequiredUnion.Kind()) { - case "int": - actual, _ := target.RequiredUnion.AsInt() - if actual != nil { - if err2 := ValidateInteger(actual); err2 != nil { - err = goa.MergeErrors(err, err2) + + if err2 := target.RequiredUnion.Validate(); err2 != nil { + err = goa.MergeErrors(err, err2) + } else { + switch string(target.RequiredUnion.Kind()) { + case "int": + actual, _ := target.RequiredUnion.AsInt() + if actual != nil { + if err2 := ValidateInteger(actual); err2 != nil { + err = goa.MergeErrors(err, err2) + } } - } - case "float": - actual, _ := target.RequiredUnion.AsFloat() - if actual != nil { - if err2 := ValidateFloat(actual); err2 != nil { - err = goa.MergeErrors(err, err2) + case "float": + actual, _ := target.RequiredUnion.AsFloat() + if actual != nil { + if err2 := ValidateFloat(actual); err2 != nil { + err = goa.MergeErrors(err, err2) + } } - } - case "string": - actual, _ := target.RequiredUnion.AsString() - if actual != nil { - if err2 := ValidateString(actual); err2 != nil { - err = goa.MergeErrors(err, err2) + case "string": + actual, _ := target.RequiredUnion.AsString() + if actual != nil { + if err2 := ValidateString(actual); err2 != nil { + err = goa.MergeErrors(err, err2) + } } } } - switch string(target.Union.Kind()) { - case "int": - actual, _ := target.Union.AsInt() - if actual != nil { - if err2 := ValidateInteger(actual); err2 != nil { - err = goa.MergeErrors(err, err2) - } - } - case "float": - actual, _ := target.Union.AsFloat() - if actual != nil { - if err2 := ValidateFloat(actual); err2 != nil { - err = goa.MergeErrors(err, err2) - } - } - case "string": - actual, _ := target.Union.AsString() - if actual != nil { - if err2 := ValidateString(actual); err2 != nil { - err = goa.MergeErrors(err, err2) + if target.Union.Kind() != "" { + if err2 := target.Union.Validate(); err2 != nil { + err = goa.MergeErrors(err, err2) + } else { + switch string(target.Union.Kind()) { + case "int": + actual, _ := target.Union.AsInt() + if actual != nil { + if err2 := ValidateInteger(actual); err2 != nil { + err = goa.MergeErrors(err, err2) + } + } + case "float": + actual, _ := target.Union.AsFloat() + if actual != nil { + if err2 := ValidateFloat(actual); err2 != nil { + err = goa.MergeErrors(err, err2) + } + } + case "string": + actual, _ := target.Union.AsString() + if actual != nil { + if err2 := ValidateString(actual); err2 != nil { + err = goa.MergeErrors(err, err2) + } + } } } } diff --git a/codegen/testdata/golden/validation_union.go.golden b/codegen/testdata/golden/validation_union.go.golden index fa9ee7c056..9afd8b89f0 100644 --- a/codegen/testdata/golden/validation_union.go.golden +++ b/codegen/testdata/golden/validation_union.go.golden @@ -2,50 +2,61 @@ func Validate() (err error) { if target.RequiredUnion.Kind() == "" { err = goa.MergeErrors(err, goa.MissingFieldError("required_union", "target")) } - switch string(target.RequiredUnion.Kind()) { - case "int": - actual, _ := target.RequiredUnion.AsInt() - if actual != nil { - if err2 := ValidateInteger(actual); err2 != nil { - err = goa.MergeErrors(err, err2) + + if err2 := target.RequiredUnion.Validate(); err2 != nil { + err = goa.MergeErrors(err, err2) + } else { + switch string(target.RequiredUnion.Kind()) { + case "int": + actual, _ := target.RequiredUnion.AsInt() + if actual != nil { + if err2 := ValidateInteger(actual); err2 != nil { + err = goa.MergeErrors(err, err2) + } } - } - case "float": - actual, _ := target.RequiredUnion.AsFloat() - if actual != nil { - if err2 := ValidateFloat(actual); err2 != nil { - err = goa.MergeErrors(err, err2) + case "float": + actual, _ := target.RequiredUnion.AsFloat() + if actual != nil { + if err2 := ValidateFloat(actual); err2 != nil { + err = goa.MergeErrors(err, err2) + } } - } - case "string": - actual, _ := target.RequiredUnion.AsString() - if actual != nil { - if err2 := ValidateString(actual); err2 != nil { - err = goa.MergeErrors(err, err2) + case "string": + actual, _ := target.RequiredUnion.AsString() + if actual != nil { + if err2 := ValidateString(actual); err2 != nil { + err = goa.MergeErrors(err, err2) + } } } } - switch string(target.Union.Kind()) { - case "int": - actual, _ := target.Union.AsInt() - if actual != nil { - if err2 := ValidateInteger(actual); err2 != nil { - err = goa.MergeErrors(err, err2) - } - } - case "float": - actual, _ := target.Union.AsFloat() - if actual != nil { - if err2 := ValidateFloat(actual); err2 != nil { - err = goa.MergeErrors(err, err2) - } - } - case "string": - actual, _ := target.Union.AsString() - if actual != nil { - if err2 := ValidateString(actual); err2 != nil { - err = goa.MergeErrors(err, err2) + if target.Union.Kind() != "" { + if err2 := target.Union.Validate(); err2 != nil { + err = goa.MergeErrors(err, err2) + } else { + switch string(target.Union.Kind()) { + case "int": + actual, _ := target.Union.AsInt() + if actual != nil { + if err2 := ValidateInteger(actual); err2 != nil { + err = goa.MergeErrors(err, err2) + } + } + case "float": + actual, _ := target.Union.AsFloat() + if actual != nil { + if err2 := ValidateFloat(actual); err2 != nil { + err = goa.MergeErrors(err, err2) + } + } + case "string": + actual, _ := target.Union.AsString() + if actual != nil { + if err2 := ValidateString(actual); err2 != nil { + err = goa.MergeErrors(err, err2) + } + } } } } diff --git a/codegen/validation.go b/codegen/validation.go index 0f92ad8961..9d325e3ecb 100644 --- a/codegen/validation.go +++ b/codegen/validation.go @@ -139,13 +139,15 @@ func ValidationCodeWithPathParameter(att *expr.AttributeExpr, put expr.UserType, } func recurseValidationCode(att *expr.AttributeExpr, put expr.UserType, attCtx *AttributeContext, req, alias, view bool, target string, context validationPath, seen map[expr.UserType]*bytes.Buffer) *bytes.Buffer { - return renderValidationCode(att, put, attCtx, req, alias, view, target, context, seen, true) + // Goa uses pointer references for root unions. Nested fields pass their + // stored pointer shape instead of deriving it from primitive-field rules. + return renderValidationCode(att, put, attCtx, req, alias, view, target, context, seen, expr.IsUnion(att.Type), true) } // renderValidationCode writes one validation tree. localGuards reports whether // local rule templates must check a pointer before reading it. Nested fields // disable those checks when validateAttribute wraps the whole field once. -func renderValidationCode(att *expr.AttributeExpr, put expr.UserType, attCtx *AttributeContext, req, alias, view bool, target string, context validationPath, seen map[expr.UserType]*bytes.Buffer, localGuards bool) *bytes.Buffer { +func renderValidationCode(att *expr.AttributeExpr, put expr.UserType, attCtx *AttributeContext, req, alias, view bool, target string, context validationPath, seen map[expr.UserType]*bytes.Buffer, unionPointer, localGuards bool) *bytes.Buffer { if seen == nil { seen = make(map[expr.UserType]*bytes.Buffer) } @@ -273,16 +275,15 @@ func renderValidationCode(att *expr.AttributeExpr, put expr.UserType, attCtx *At "validation": val, }) } - if len(cases) > 0 { - newline() - data := map[string]any{ - "target": target, - "cases": cases, - "goa": "goa", - } - if err := unionSumValT.Execute(buf, data); err != nil { - panic(err) // bug - } + newline() + data := map[string]any{ + "target": unionMethodReceiver(att, attCtx, target, unionPointer), + "cases": cases, + "optional": !req && !unionPointer, + "goa": "goa", + } + if err := unionSumValT.Execute(buf, data); err != nil { + panic(err) // bug } break } @@ -360,7 +361,7 @@ func validateAttribute(ctx *AttributeContext, att *expr.AttributeExpr, put expr. if expr.IsUnion(att.Type) && ctx.Scope.IsSumType() { guard = unionPointer } - code := renderValidationCode(att, put, ctx, req, false, view, target, context, seen, !guard).String() + code := renderValidationCode(att, put, ctx, req, false, view, target, context, seen, unionPointer, !guard).String() if code == "" { return "" } @@ -382,7 +383,7 @@ func validateAttribute(ctx *AttributeContext, att *expr.AttributeExpr, put expr. // the original attribute with alias=true ensures validations operate // on the correct value type without dropping field defaults. guard := validationAttributeNeedsNilGuard(att, ctx, req) - code := renderValidationCode(att, put, ctx, req, true, view, target, context, seen, !guard).String() + code := renderValidationCode(att, put, ctx, req, true, view, target, context, seen, unionPointer, !guard).String() if code == "" { return "" } @@ -406,6 +407,10 @@ func validateAttribute(ctx *AttributeContext, att *expr.AttributeExpr, put expr. panic(err) // bug } if unionValue { + if !req { + receiver := unionMethodReceiver(att, ctx, target, false) + return fmt.Sprintf("if %s.Kind() != \"\" {\n\t%s\n}", receiver, buf.String()) + } return buf.String() } if resolver, ok := ctx.Scope.(nilUserTypeValidationAttributor); ok && resolver.ValidationAcceptsNil(att) { diff --git a/codegen/validation_plan.go b/codegen/validation_plan.go index 53adf7ed8d..4fce517753 100644 --- a/codegen/validation_plan.go +++ b/codegen/validation_plan.go @@ -122,25 +122,6 @@ type ( value *validationPlanNode } - // validationUnionPlan stores generated sum-type branch operations. - validationUnionPlan struct { - cases []validationUnionCasePlan - receiver *GoTypePlan - } - - // validationUnionCasePlan stores one sum-type accessor and branch program. - validationUnionCasePlan struct { - typeTag string - fieldName string - node *validationPlanNode - } - - // validatorCallPlan stores one exact nested validator declaration. - validatorCallPlan struct { - declaration *NameDeclaration - address bool - } - // validationPlanner performs all expression reads while validation checks and // function calls are copied into a plan. validationPlanner struct { @@ -240,6 +221,9 @@ func (p *ValidationPlan) ImportPreferences() []GoTypeImport { // its branches need validation. Include those receivers at every depth. var addMethodOwners func(*validationPlanNode) addMethodOwners = func(node *validationPlanNode) { + if node.call != nil { + addReceiver(node.call.optionalUnion) + } for _, required := range node.rules.required { addReceiver(required.receiver) } @@ -365,13 +349,26 @@ func (p *validationPlanner) plan(attribute *expr.AttributeExpr, layout *GoTypePl return nil, fmt.Errorf("plan validation for %s: validator declaration must be a function, got %s", path, declaration.Kind()) } p.declarations = append(p.declarations, declaration) + // Optional union fields stored by value use an empty kind for absence. + // Keep the union method owner so absent fields skip their nested validator. + var optionalUnion *GoTypePlan + if expr.IsUnion(attribute.Type) && !required && !valuePointer { + optionalUnion = layout + for optionalUnion.kind == GoNamed && optionalUnion.value != nil { + optionalUnion = optionalUnion.value + } + if optionalUnion.kind != GoUnion { + return nil, fmt.Errorf("plan validation for %s: optional named union layout does not retain its union definition", path) + } + } return &validationPlanNode{ occurrence: attribute, layout: layout, guard: valuePointer || !layout.ReferenceIsPointer() && layout.ReferenceCanBeNil(), call: &validatorCallPlan{ - declaration: declaration, - address: layout.ReferenceIsPointer() && !valuePointer, + declaration: declaration, + address: layout.ReferenceIsPointer() && !valuePointer, + optionalUnion: optionalUnion, }, }, nil } @@ -457,43 +454,10 @@ func (p *validationPlanner) plan(attribute *expr.AttributeExpr, layout *GoTypePl node.mapValue = &validationMapPlan{key: key, value: value} } case expr.IsUnion(attribute.Type): - union := expr.AsUnion(attribute.Type) - branches := structure.Branches() - if len(branches) != len(union.Values) { - return nil, fmt.Errorf("plan validation for %s: union layout has %d branches, expected %d", path, len(branches), len(union.Values)) - } - var cases []validationUnionCasePlan - for index, branch := range union.Values { - branchPolicy := policy - branchPolicy.Pointer = branchPolicy.Pointer && expr.IsObject(branch.Attribute.Type) - child, err := p.plan( - branch.Attribute, - branches[index].withPolicy(branchPolicy), - true, - expr.IsAlias(branch.Attribute.Type), - true, - branches[index].ReferenceIsPointer(), - fmt.Sprintf("union branch %q", branch.Name), - ) - if err != nil { - return nil, err - } - if child.empty() { - continue - } - cases = append(cases, validationUnionCasePlan{ - typeTag: branch.Name, - fieldName: Goify(branch.Name, true), - node: child, - }) - } - if len(cases) > 0 { - node.union = &validationUnionPlan{cases: cases} - if structure != layout { - // Defined types do not inherit the underlying union's methods. - // Retain that declaration for the receiver conversion. - node.union.receiver = structure - } + var err error + node.union, err = p.planUnion(attribute, layout, structure, required, valuePointer, path) + if err != nil { + return nil, err } } // Required-field rules index the structural fields checked above. Keep @@ -674,6 +638,9 @@ func attributeNeedsValidation(attribute *expr.AttributeExpr, policy GoLayoutPoli return attributeNeedsValidation(mapping.KeyType, keyPolicy, seen) || attributeNeedsValidation(mapping.ElemType, valuePolicy, seen) case expr.IsUnion(attribute.Type): + if policy.SumType { + return true + } for _, branch := range expr.AsUnion(attribute.Type).Values { branchPolicy := policy branchPolicy.Pointer = policy.Pointer && expr.IsObject(branch.Attribute.Type) @@ -760,6 +727,10 @@ func (p LinkedValidationPlan) renderNode(node *validationPlanNode, target, conte }); err != nil { panic(err) } + if node.call.optionalUnion != nil { + receiver := p.layout.Enter(node.call.optionalUnion).Name() + return fmt.Sprintf("if (%s)(%s).Kind() != \"\" {\n\t%s\n}", receiver, target, buffer.String()) + } if node.guard { return fmt.Sprintf("if %s != nil {\n\t%s\n}", target, buffer.String()) } @@ -827,7 +798,7 @@ func (p LinkedValidationPlan) renderNode(node *validationPlanNode, target, conte unionTarget = fmt.Sprintf("(%s)(%s)", receiver, target) } var buffer bytes.Buffer - if err := unionSumValT.Execute(&buffer, map[string]any{"target": unionTarget, "cases": cases}); err != nil { + if err := unionSumValT.Execute(&buffer, map[string]any{"target": unionTarget, "cases": cases, "optional": node.union.optional, "goa": p.goaPackage()}); err != nil { panic(err) } sections = append(sections, buffer.String()) diff --git a/codegen/validation_union_plan.go b/codegen/validation_union_plan.go new file mode 100644 index 0000000000..3afddabf90 --- /dev/null +++ b/codegen/validation_union_plan.go @@ -0,0 +1,84 @@ +// Union validation keeps the chosen branch check and its field checks together. +// The renderer uses these retained layouts after generated names become fixed. +package codegen + +import ( + "fmt" + + "goa.design/goa/v3/expr" +) + +type ( + // validationUnionPlan stores generated sum-type branch operations. + validationUnionPlan struct { + cases []validationUnionCasePlan + receiver *GoTypePlan + optional bool + } + + // validationUnionCasePlan stores one sum-type accessor and branch program. + validationUnionCasePlan struct { + typeTag string + fieldName string + node *validationPlanNode + } + + // validatorCallPlan stores one exact nested validator declaration. + validatorCallPlan struct { + declaration *NameDeclaration + address bool + optionalUnion *GoTypePlan + } +) + +// planUnion records selection validation even when no branch has field rules. +// Optional value fields retain their empty-kind absence; present pointers and +// required values use the generated union's own validity check. +func (p *validationPlanner) planUnion( + attribute *expr.AttributeExpr, + layout, structure *GoTypePlan, + required, valuePointer bool, + path string, +) (*validationUnionPlan, error) { + policy := layout.Policy() + union := expr.AsUnion(attribute.Type) + branches := structure.Branches() + if len(branches) != len(union.Values) { + return nil, fmt.Errorf("plan validation for %s: union layout has %d branches, expected %d", path, len(branches), len(union.Values)) + } + var cases []validationUnionCasePlan + for index, branch := range union.Values { + branchPolicy := policy + branchPolicy.Pointer = branchPolicy.Pointer && expr.IsObject(branch.Attribute.Type) + child, err := p.plan( + branch.Attribute, + branches[index].withPolicy(branchPolicy), + true, + expr.IsAlias(branch.Attribute.Type), + true, + branches[index].ReferenceIsPointer(), + fmt.Sprintf("union branch %q", branch.Name), + ) + if err != nil { + return nil, err + } + if child.empty() { + continue + } + cases = append(cases, validationUnionCasePlan{ + typeTag: branch.Name, + fieldName: Goify(branch.Name, true), + node: child, + }) + } + planned := &validationUnionPlan{ + cases: cases, + optional: !required && !valuePointer, + } + if structure != layout { + // Defined types do not inherit the underlying union's methods. + // Retain that declaration for the receiver conversion. + planned.receiver = structure + } + return planned, nil +} diff --git a/codegen/validation_union_value_test.go b/codegen/validation_union_value_test.go new file mode 100644 index 0000000000..9a9a91ad35 --- /dev/null +++ b/codegen/validation_union_value_test.go @@ -0,0 +1,66 @@ +// These tests check union selection before branch field validation. Optional +// value unions may be absent; pointer unions use nil alone for absence. +package codegen + +import ( + "testing" + + "github.com/stretchr/testify/require" + "goa.design/goa/v3/expr" +) + +func TestValidationChecksUnconstrainedUnionSelection(t *testing.T) { + choice := &expr.AttributeExpr{Type: &expr.Union{ + TypeName: "Selection", + Values: []*expr.NamedAttributeExpr{ + {Name: "text", Attribute: &expr.AttributeExpr{Type: expr.String}}, + {Name: "empty", Attribute: &expr.AttributeExpr{Type: &expr.Object{}}}, + }, + }} + for _, test := range []struct { + name string + required, pointer bool + }{ + {name: "required value", required: true}, + {name: "optional value"}, + {name: "required pointer", required: true, pointer: true}, + {name: "optional pointer", pointer: true}, + } { + t.Run(test.name, func(t *testing.T) { + attribute := &expr.AttributeExpr{Type: &expr.Object{{Name: "choice", Attribute: choice}}} + if test.required { + attribute.Validation = &expr.ValidationExpr{Required: []string{"choice"}} + } + policy := GoLayoutPolicy{UseDefault: true, SumType: true, UnionPointer: test.pointer, Pointer: test.pointer} + require.True(t, NeedsValidation(attribute, policy)) + const owner = "generated.local/gen/selection" + generation, err := NewGeneration("generated.local/gen", nil) + require.NoError(t, err) + declaration := declareGoTypeTestUnion(t, generation, owner, choice.Type.(*expr.Union)) + layout, err := PlanGoType(attribute, GoTypePlanOptions{ + Owner: owner, Policy: policy, + Bind: goTypeTestBinder(map[expr.DataType]GoTypeBinding{choice.Type: {Owner: owner, Union: declaration}}), + }) + require.NoError(t, err) + plan, err := NewValidationPlan(attribute, layout, ValidationPlanOptions{Required: test.required}) + require.NoError(t, err) + require.NoError(t, generation.Freeze()) + linked, err := plan.Link(layout.Link(layout.Owner(), validationPlanTestQualifier)) + require.NoError(t, err) + context := NewAttributeContext(test.pointer, false, true, "", NewNameScope()) + context.UnionPointer = test.pointer + for _, source := range []string{ + linked.Render("target", "body"), + ValidationCode(attribute, nil, context, test.required, false, false, "target"), + } { + require.Contains(t, source, "target.Choice.Validate()") + if !test.required && !test.pointer { + require.Contains(t, source, `if target.Choice.Kind() != "" {`) + } else { + require.NotContains(t, source, `if target.Choice.Kind() != "" {`) + } + } + }) + } + require.False(t, NeedsValidation(choice, GoLayoutPolicy{UseDefault: true})) +} diff --git a/http/codegen/testdata/golden/http_union_branch_order.go.golden b/http/codegen/testdata/golden/http_union_branch_order.go.golden index 147c4fd610..a660891942 100644 --- a/http/codegen/testdata/golden/http_union_branch_order.go.golden +++ b/http/codegen/testdata/golden/http_union_branch_order.go.golden @@ -309,12 +309,17 @@ func ValidateEditResponseBody(body *EditResponseBody) (err error) { err = goa.MergeErrors(err, goa.MissingFieldError("relationship", "body")) } if body.Relationship != nil { - switch string(body.Relationship.Kind()) { - case "editable": - actual, _ := body.Relationship.AsEditable() - if actual != nil { - if err2 := validateEditableResponseBody(actual, "body.relationship.value"); err2 != nil { - err = goa.MergeErrors(err, err2) + + if err2 := body.Relationship.Validate(); err2 != nil { + err = goa.MergeErrors(err, err2) + } else { + switch string(body.Relationship.Kind()) { + case "editable": + actual, _ := body.Relationship.AsEditable() + if actual != nil { + if err2 := validateEditableResponseBody(actual, "body.relationship.value"); err2 != nil { + err = goa.MergeErrors(err, err2) + } } } } @@ -330,12 +335,17 @@ func validateEditResponseBody(body *EditResponseBody, path string) (err error) { err = goa.MergeErrors(err, goa.MissingFieldError("relationship", path)) } if body.Relationship != nil { - switch string(body.Relationship.Kind()) { - case "editable": - actual, _ := body.Relationship.AsEditable() - if actual != nil { - if err2 := validateEditableResponseBody(actual, path+".relationship.value"); err2 != nil { - err = goa.MergeErrors(err, err2) + + if err2 := body.Relationship.Validate(); err2 != nil { + err = goa.MergeErrors(err, err2) + } else { + switch string(body.Relationship.Kind()) { + case "editable": + actual, _ := body.Relationship.AsEditable() + if actual != nil { + if err2 := validateEditableResponseBody(actual, path+".relationship.value"); err2 != nil { + err = goa.MergeErrors(err, err2) + } } } } diff --git a/http/codegen/testdata/golden/server_decode_decode-body-union-user.go.golden b/http/codegen/testdata/golden/server_decode_decode-body-union-user.go.golden index c7ec4b091c..a535564d4e 100644 --- a/http/codegen/testdata/golden/server_decode_decode-body-union-user.go.golden +++ b/http/codegen/testdata/golden/server_decode_decode-body-union-user.go.golden @@ -18,6 +18,10 @@ func DecodeMethodBodyUnionUserRequest(mux goahttp.Muxer, decoder func(*http.Requ } return payload, goa.DecodePayloadError(err.Error()) } + err = ValidateMethodBodyUnionUserRequestBody(&body) + if err != nil { + return payload, err + } payload = NewMethodBodyUnionUserUnionUser(&body) return payload, nil diff --git a/http/codegen/testdata/golden/server_decode_decode-body-union.go.golden b/http/codegen/testdata/golden/server_decode_decode-body-union.go.golden index d59ea889ec..7a978aa814 100644 --- a/http/codegen/testdata/golden/server_decode_decode-body-union.go.golden +++ b/http/codegen/testdata/golden/server_decode_decode-body-union.go.golden @@ -18,6 +18,10 @@ func DecodeMethodBodyUnionRequest(mux goahttp.Muxer, decoder func(*http.Request) } return payload, goa.DecodePayloadError(err.Error()) } + err = ValidateMethodBodyUnionRequestBody(&body) + if err != nil { + return payload, err + } payload = NewMethodBodyUnionUnion(&body) return payload, nil diff --git a/jsonrpc/codegen/testdata/golden/params_optional_union_request_decoder.go.golden b/jsonrpc/codegen/testdata/golden/params_optional_union_request_decoder.go.golden index d30e57b65b..d0c1d6bb40 100644 --- a/jsonrpc/codegen/testdata/golden/params_optional_union_request_decoder.go.golden +++ b/jsonrpc/codegen/testdata/golden/params_optional_union_request_decoder.go.golden @@ -20,6 +20,16 @@ func DecodeOptionalUnionRequest(mux goahttp.Muxer, decoder func(*http.Request) g return payload, goa.DecodePayloadError(err.Error()) } } + if body != nil { + + if err2 := body.Validate(); err2 != nil { + err = goa.MergeErrors(err, err2) + } + + if err != nil { + return payload, err + } + } payload = NewOptionalUnionPayload(body) return payload, nil