Repository: goadesign/goa
Pull request #3103 (2022-07-26 04:21:07)

Fix marshaling of arrays with extended alias types


Patch:
diff --git a/http/codegen/service_data.go b/http/codegen/service_data.go
index 9b7f469a..7b8397be 100644
--- a/http/codegen/service_data.go
+++ b/http/codegen/service_data.go
@@ -655,8 +655,7 @@ func (d ServicesData) analyze(hs *expr.HTTPServiceExpr) *ServiceData {
 					name := fmt.Sprintf("%s%sPath%s", ep.VarName, svc.StructName, suffix)
 					for j, arg := range params {
 						patt := pathParamsObj.Attribute(arg)
-						att := expr.DupAtt(patt)
-						makeHTTPType(att)
+						att := makeHTTPType(patt)
 						pointer := a.Params.IsPrimitivePointer(arg, true)
 						name := rd.Scope.Name(codegen.Goify(arg, false))
 						var vcode string
@@ -919,10 +918,15 @@ func (d ServicesData) analyze(hs *expr.HTTPServiceExpr) *ServiceData {
 // * removes aliased user type by replacing them with the underlying type.
 // * changes unions into structs with Type and Value fields.
 //
-func makeHTTPType(att *expr.AttributeExpr, seen ...map[string]struct{}) {
+func makeHTTPType(att *expr.AttributeExpr) *expr.AttributeExpr {
 	if att == nil {
-		return
+		return nil
 	}
+	att = expr.DupAtt(att)
+	return makeHTTPTypeRecursive(att, make(map[string]struct{}))
+}
+
+func makeHTTPTypeRecursive(att *expr.AttributeExpr, seen map[string]struct{}) *expr.AttributeExpr {
 	switch dt := att.Type.(type) {
 	case expr.UserType:
 		if _, ok := dt.(*expr.ResultTypeExpr); !ok && !expr.IsObject(dt) {
@@ -938,27 +942,22 @@ func makeHTTPType(att *expr.AttributeExpr, seen ...map[string]struct{}) {
 			}
 			att.DefaultValue = dt.Attribute().DefaultValue
 		}
-		var s map[string]struct{}
-		if len(seen) > 0 {
-			s = seen[0]
-		} else {
-			s = make(map[string]struct{})
-			seen = append(seen, s)
+		if _, ok := seen[dt.ID()]; ok {
+			return att
 		}
-		if _, ok := s[dt.ID()]; ok {
-			return
-		}
-		s[dt.ID()] = struct{}{}
-		makeHTTPType(dt.Attribute(), seen...)
+		seen[dt.ID()] = struct{}{}
+		dt.SetAttribute(makeHTTPTypeRecursive(dt.Attribute(), seen))
 	case *expr.Array:
-		makeHTTPType(dt.ElemType, seen...)
+		dt.ElemType = makeHTTPTypeRecursive(dt.ElemType, seen)
 	case *expr.Map:
-		makeHTTPType(dt.KeyType, seen...)
-		makeHTTPType(dt.ElemType, seen...)
+		dt.KeyType = makeHTTPTypeRecursive(dt.KeyType, seen)
+		dt.ElemType = makeHTTPTypeRecursive(dt.ElemType, seen)
 	case *expr.Object:
-		for _, nat := range *dt {
-			makeHTTPType(nat.Attribute, seen...)
+		obj := make(expr.Object, len(*dt))
+		for i, nat := range *dt {
+			obj[i] = &expr.NamedAttributeExpr{Name: nat.Name, Attribute: makeHTTPTypeRecursive(nat.Attribute, seen)}
 		}
+		att.Type = &obj
 	case *expr.Union:
 		values := expr.AsUnion(dt).Values
 		names := make([]interface{}, len(values))
@@ -995,13 +994,14 @@ func makeHTTPType(att *expr.AttributeExpr, seen ...map[string]struct{}) {
 		att.Type = &obj
 		att.Validation = &expr.ValidationExpr{Required: []string{"Type", "Value"}}
 	}
+	return att
 }
 
 // buildPayloadData returns the data structure used to describe the endpoint
 // payload including the HTTP request details. It also returns the user types
 // used by the request body type recursively if any.
 func buildPayloadData(e *expr.HTTPEndpointExpr, sd *ServiceData) *PayloadData {
-	makeHTTPType(e.Body)
+	e.Body = makeHTTPType(e.Body)
 	var (
 		payload    = e.MethodExpr.Payload
 		svc        = sd.Service
@@ -1499,7 +1499,8 @@ func buildResponses(e *expr.HTTPEndpointExpr, result *expr.AttributeExpr, viewed
 		}
 		notag := -1
 		for i, resp := range e.Responses {
-			makeHTTPType(resp.Body)
+			resp.Body = expr.DupAtt(resp.Body)
+			resp.Body = makeHTTPType(resp.Body)
 			if resp.Tag[0] == "" {
 				if notag > -1 {
 					continue // we don't want more than one response with no tag
@@ -1761,7 +1762,7 @@ func buildErrorsData(e *expr.HTTPEndpointExpr, sd *ServiceData) []*ErrorGroupDat
 
 	data := make(map[string][]*ErrorData)
 	for _, v := range e.HTTPErrors {
-		makeHTTPType(v.Response.Body)
+		v.Response.Body = makeHTTPType(v.Response.Body)
 		var (
 			init *InitData
 			body = v.Response.Body.Type
@@ -2307,7 +2308,7 @@ func extractPathParams(a *expr.MappedAttributeExpr, service *expr.AttributeExpr,
 			stringSlice = arr.ElemType.Type.Kind() == expr.StringKind
 		}
 
-		makeHTTPType(c)
+		c = makeHTTPType(c)
 		var (
 			varn = scope.Name(codegen.Goify(name, false))
 			arr  = expr.AsArray(c.Type)
@@ -2363,7 +2364,7 @@ func extractQueryParams(a *expr.MappedAttributeExpr, service *expr.AttributeExpr
 			stringSlice = arr.ElemType.Type.Kind() == expr.StringKind
 		}
 
-		makeHTTPType(c)
+		c = makeHTTPType(c)
 		var (
 			varn    = scope.Name(codegen.Goify(name, false))
 			arr     = expr.AsArray(c.Type)
@@ -2429,14 +2430,12 @@ func extractHeaders(a *expr.MappedAttributeExpr, svcAtt *expr.AttributeExpr, svc
 		var hattr *expr.AttributeExpr
 		var stringSlice bool
 		{
-			hattr = expr.DupAtt(attr)
-
 			// The StringSlice field of ParamData must be false for aliased primitive types
-			if arr := expr.AsArray(hattr.Type); arr != nil {
+			if arr := expr.AsArray(attr.Type); arr != nil {
 				stringSlice = arr.ElemType.Type.Kind() == expr.StringKind
 			}
 
-			makeHTTPType(hattr)
+			hattr = makeHTTPType(attr)
 		}
 		var (
 			varn    = scope.Name(codegen.Goify(name, false))
@@ -2495,8 +2494,7 @@ func extractCookies(a *expr.MappedAttributeExpr, svcAtt *expr.AttributeExpr, svc
 			if hattr = svcAtt.Find(name); hattr == nil {
 				hattr = svcAtt
 			}
-			hattr = expr.DupAtt(hattr)
-			makeHTTPType(hattr)
+			hattr = makeHTTPType(hattr)
 		}
 		var (
 			varn    = scope.Name(codegen.Goify(name, false))


Test patch:
diff --git a/http/codegen/server_encode_test.go b/http/codegen/server_encode_test.go
index 09e88978..3aaeb263 100644
--- a/http/codegen/server_encode_test.go
+++ b/http/codegen/server_encode_test.go
@@ -83,7 +83,7 @@ func TestEncode(t *testing.T) {
 		{"empty-server-response-with-tags", testdata.EmptyServerResponseWithTagsDSL, testdata.EmptyServerResponseWithTagsEncodeCode},
 
 		{"result-with-custom-pkg-type", testdata.ResultWithCustomPkgTypeDSL, testdata.ResultWithCustomPkgTypeEncodeCode},
-		{"result-with-embedded-custom-pkg-type", testdata.ResultWithEmbeddedCustomPkgTypeDSL, testdata.ResultWithEmbeddedCustomPkgTypeEncodeCode},
+		{"result-with-embedded-custom-pkg-type", testdata.EmbeddedCustomPkgTypeDSL, testdata.ResultWithEmbeddedCustomPkgTypeEncodeCode},
 	}
 	for _, c := range cases {
 		t.Run(c.Name, func(t *testing.T) {
@@ -110,7 +110,8 @@ func TestEncodeMarshallingAndUnmarshalling(t *testing.T) {
 		DSL  func()
 		Code []string
 	}{
-		{"result-with-embedded-custom-pkg-type", testdata.ResultWithEmbeddedCustomPkgTypeDSL, []string{testdata.ResultWithEmbeddedCustomPkgTypeUnmarshalCode, testdata.ResultWithEmbeddedCustomPkgTypeMarshalCode}},
+		{"result-with-embedded-custom-pkg-type", testdata.EmbeddedCustomPkgTypeDSL, []string{testdata.EmbeddedCustomPkgTypeUnmarshalCode, testdata.EmbeddedCustomPkgTypeMarshalCode}},
+		{"result-with-array-alias-extended", testdata.ArrayAliasExtendedDSL, []string{testdata.ArrayAliasExtendedUnmarshalCode, testdata.ArrayAliasExtendedMarshalCode}},
 	}
 	const totalSectionsOffset = 3
 	for _, c := range cases {
diff --git a/http/codegen/testdata/result_dsls.go b/http/codegen/testdata/result_dsls.go
index 05568cf4..2ee44c3d 100644
--- a/http/codegen/testdata/result_dsls.go
+++ b/http/codegen/testdata/result_dsls.go
@@ -779,7 +779,7 @@ var ResultWithCustomPkgTypeDSL = func() {
 	})
 }
 
-var ResultWithEmbeddedCustomPkgTypeDSL = func() {
+var EmbeddedCustomPkgTypeDSL = func() {
 	var Foo = Type("Foo", func() {
 		Meta("struct:pkg:path", "foo")
 		Attribute("bar", String)
@@ -801,6 +801,29 @@ var ResultWithEmbeddedCustomPkgTypeDSL = func() {
 	})
 }
 
+var ArrayAliasExtendedDSL = func() {
+	var Foo = Type("Foo", String)
+
+	var Extension = Type("Extension", func() {
+		Attribute("Foo", Foo)
+	})
+
+	var ResultType = Type("ResultType", func() {
+		Extend(Extension)
+	})
+
+	var _ = Service("FooService", func() {
+		Method("FooMethod", func() {
+			Payload(ArrayOf(ResultType))
+			Result(ArrayOf(ResultType))
+			HTTP(func() {
+				GET("/")
+			})
+		})
+	})
+
+}
+
 var EmptyErrorResponseBodyDSL = func() {
 	Service("ServiceEmptyErrorResponseBody", func() {
 		Method("MethodEmptyErrorResponseBody", func() {
diff --git a/http/codegen/testdata/result_encode_types.go b/http/codegen/testdata/result_encode_types.go
index eedb6255..036961ca 100644
--- a/http/codegen/testdata/result_encode_types.go
+++ b/http/codegen/testdata/result_encode_types.go
@@ -1,6 +1,6 @@
 package testdata
 
-var ResultWithEmbeddedCustomPkgTypeUnmarshalCode = `// unmarshalFooRequestBodyToFooFoo builds a value of type *foo.Foo from a value
+var EmbeddedCustomPkgTypeUnmarshalCode = `// unmarshalFooRequestBodyToFooFoo builds a value of type *foo.Foo from a value
 // of type *FooRequestBody.
 func unmarshalFooRequestBodyToFooFoo(v *FooRequestBody) *foo.Foo {
 	if v == nil {
@@ -14,7 +14,7 @@ func unmarshalFooRequestBodyToFooFoo(v *FooRequestBody) *foo.Foo {
 }
 `
 
-var ResultWithEmbeddedCustomPkgTypeMarshalCode = `// marshalFooFooToFooResponseBody builds a value of type *FooResponseBody from
+var EmbeddedCustomPkgTypeMarshalCode = `// marshalFooFooToFooResponseBody builds a value of type *FooResponseBody from
 // a value of type *foo.Foo.
 func marshalFooFooToFooResponseBody(v *foo.Foo) *FooResponseBody {
 	if v == nil {
@@ -27,3 +27,29 @@ func marshalFooFooToFooResponseBody(v *foo.Foo) *FooResponseBody {
 	return res
 }
 `
+
+var ArrayAliasExtendedMarshalCode = `// marshalFooserviceResultTypeToResultTypeResponse builds a value of type
+// *ResultTypeResponse from a value of type *fooservice.ResultType.
+func marshalFooserviceResultTypeToResultTypeResponse(v *fooservice.ResultType) *ResultTypeResponse {
+	res := &ResultTypeResponse{}
+	if v.Foo != nil {
+		foo := string(*v.Foo)
+		res.Foo = &foo
+	}
+
+	return res
+}
+`
+
+var ArrayAliasExtendedUnmarshalCode = `// unmarshalResultTypeRequestBodyToFooserviceResultType builds a value of type
+// *fooservice.ResultType from a value of type *ResultTypeRequestBody.
+func unmarshalResultTypeRequestBodyToFooserviceResultType(v *ResultTypeRequestBody) *fooservice.ResultType {
+	res := &fooservice.ResultType{}
+	if v.Foo != nil {
+		foo := fooservice.Foo(*v.Foo)
+		res.Foo = &foo
+	}
+
+	return res
+}
+`