Fix client validators for constructor union bodies

Author: CaliLuke

http/codegen/client_cli_test.go

@@ -3,6 +3,7 @@ package codegen
 import (
 	"bytes"
 	"goa.design/goa/v3/codegen/testutil"
+	"regexp"
 	"strings"
 	"testing"
 
@@ -94,3 +95,32 @@ func TestConstructorUnionClientCLIFiles(t *testing.T) {
 		})
 	}
 }
+
+func TestConstructorUnionClientCLIPayloadValidatorsExistInClientTypes(t *testing.T) {
+	root := RunHTTPDSL(t, testdata.ConstructorUnionClientValidatorReferenceHTTPDSL)
+	services := CreateHTTPServices(root)
+
+	cliFiles := ClientCLIFiles("", services)
+	require.GreaterOrEqual(t, len(cliFiles), 2, "expected parser and payload builder files")
+	var builder bytes.Buffer
+	for _, s := range cliFiles[1].SectionTemplates {
+		require.NoError(t, s.Write(&builder))
+	}
+	builderCode := codegen.FormatTestCode(t, builder.String())
+
+	typeFiles := ClientTypeFiles("", services)
+	require.NotEmpty(t, typeFiles, "expected client type files")
+	var types bytes.Buffer
+	for _, s := range typeFiles[0].SectionTemplates {
+		require.NoError(t, s.Write(&types))
+	}
+	typesCode := codegen.FormatTestCode(t, types.String())
+
+	re := regexp.MustCompile(`Validate([A-Za-z0-9]+RequestBody)\(`)
+	matches := re.FindAllStringSubmatch(builderCode, -1)
+	require.NotEmpty(t, matches, "expected constructor-union builder to reference request-body validators")
+	for _, match := range matches {
+		name := match[1]
+		require.Contains(t, typesCode, "func Validate"+name+"(", "missing client validator for %s", name)
+	}
+}

http/codegen/service_data.go

@@ -2116,9 +2116,11 @@ func (sds *ServicesData) buildRequestBodyType(body, att *expr.AttributeExpr, e *
 		def = goTypeDef(sd.Scope, ut.Attribute(), svr, !svr)
 		desc = fmt.Sprintf("%s is the type of the %q service %q endpoint HTTP request body.",
 			varname, svc.Name, e.Name())
-		if svr {
-			// generate validation code for unmarshaled type (server-side).
-			validateDef = codegen.ValidationCode(ut.Attribute(), ut, httpctx, true, expr.IsAlias(ut), false, "body")
+		// Generate validation code for unmarshaled request bodies on the server,
+		// and for client request bodies only when constructor unions require the
+		// corresponding validator helper during CLI payload validation.
+		if svr || containsUnionType(body.Type) {
+			validateDef = codegen.ValidationCode(body, ut, httpctx, true, expr.IsAlias(ut), false, "body")
 			if validateDef != "" {
 				validateRef = fmt.Sprintf("err = Validate%s(&body)", varname)
 			}
@@ -2834,6 +2836,9 @@ func (sds *ServicesData) attributeTypeData(ut expr.UserType, req, ptr, server bo
 		// requests server-side and CLI.
 		// Alias types are validated inline in the parent type
 		validate = codegen.ValidationCode(ut.Attribute(), ut, hctx, true, expr.IsAlias(ut), false, "body")
+		if validate == "" && req && !server && needsClientRequestBodyValidatorStub(ut) {
+			validate = "// no validations"
+		}
 	}
 	if validate != "" {
 		validateRef = fmt.Sprintf("err = Validate%s(v)", name)
@@ -2850,6 +2855,44 @@ func (sds *ServicesData) attributeTypeData(ut expr.UserType, req, ptr, server bo
 	}
 }
 
+func needsClientRequestBodyValidatorStub(ut expr.UserType) bool {
+	if ut == nil || ut.Attribute() == nil || ut.Attribute().Meta == nil {
+		return false
+	}
+	_, ok := ut.Attribute().Meta.Last("oneof:type:tag")
+	return ok
+}
+
+func containsUnionType(dt expr.DataType) bool {
+	return containsUnionTypeRecursive(dt, make(map[string]struct{}))
+}
+
+func containsUnionTypeRecursive(dt expr.DataType, seen map[string]struct{}) bool {
+	switch actual := dt.(type) {
+	case nil:
+		return false
+	case *expr.Union:
+		return true
+	case expr.UserType:
+		if _, ok := seen[actual.ID()]; ok {
+			return false
+		}
+		seen[actual.ID()] = struct{}{}
+		return containsUnionTypeRecursive(actual.Attribute().Type, seen)
+	case *expr.Object:
+		for _, nat := range *actual {
+			if containsUnionTypeRecursive(nat.Attribute.Type, seen) {
+				return true
+			}
+		}
+	case *expr.Array:
+		return containsUnionTypeRecursive(actual.ElemType.Type, seen)
+	case *expr.Map:
+		return containsUnionTypeRecursive(actual.KeyType.Type, seen) || containsUnionTypeRecursive(actual.ElemType.Type, seen)
+	}
+	return false
+}
+
 // httpContext returns a context for attributes of types used to marshal and
 // unmarshal HTTP requests and responses.
 //

http/codegen/testdata/constructor_union_dsls.go

@@ -472,6 +472,43 @@ var NestedTopLevelConstructorUnionCustomKeysHTTPDSL = func() {
 	})
 }
 
+var ConstructorUnionClientValidatorReferenceHTTPDSL = func() {
+	var All = Type("ClientValidatorReferenceAll", func() {
+		Meta("name:original", "All")
+		Meta("oneof:type:tag", "all")
+	})
+	var Single = Type("ClientValidatorReferenceSingle", func() {
+		Meta("name:original", "Single")
+		Meta("oneof:type:tag", "single")
+		Attribute("task_id", String)
+		Required("task_id")
+	})
+	var Batch = Type("ClientValidatorReferenceBatch", func() {
+		Meta("name:original", "Batch")
+		Meta("oneof:type:tag", "batch")
+		Attribute("task_ids", ArrayOf(String), func() {
+			MinLength(1)
+		})
+		Required("task_ids")
+	})
+	var PayloadType = Type("ClientValidatorReferencePayload", func() {
+		Attribute("value", OneOf(All, Single, Batch), func() {
+			Meta("oneof:typename", "ClientValidatorReferenceMode")
+			Meta("oneof:type:field", "mode")
+			Meta("oneof:value:field", "value")
+		})
+		Required("value")
+	})
+	Service("ClientValidatorReference", func() {
+		Method("Show", func() {
+			Payload(PayloadType)
+			HTTP(func() {
+				POST("/")
+			})
+		})
+	})
+}
+
 var RecursiveConstructorUnionHTTPDSL = func() {
 	var Leaf = Type("Leaf", func() {
 		Attribute("value", String)