Repository: goadesign/goa
Pull request #3474 (2024-02-01 15:29:13)

Validate Array and Map for struct:pkg:path Meta
https://github.com/goadesign/goa/issues/3361


Patch:
diff --git a/expr/attribute.go b/expr/attribute.go
index 6e8ec3c9..1490306a 100644
--- a/expr/attribute.go
+++ b/expr/attribute.go
@@ -225,17 +225,7 @@ func (a *AttributeExpr) Validate(ctx string, parent eval.Expression) *eval.Valid
 			}
 		}
 		for _, nat := range *o {
-			if ut, ok := nat.Attribute.Type.(UserType); pkgPath != "" && ok {
-				// This check ensures we error if a sub-type has a different custom package type set
-				// or if two user types have different custom packages but share a sub-type (field that's a user type)
-				if ut.Attribute().Meta != nil &&
-					ut.Attribute().Meta["struct:pkg:path"] != nil &&
-					ut.Attribute().Meta["struct:pkg:path"][0] != pkgPath {
-					verr.Add(a, "type \"%s\" has conflicting packages %s and %s", ut.Name(), ut.Attribute().Meta["struct:pkg:path"][0], pkgPath)
-				}
-
-				ut.Attribute().AddMeta("struct:pkg:path", pkgPath)
-			}
+			verr.Merge(a.validatePkgPath(pkgPath, nat.Attribute.Type))
 			ctx = fmt.Sprintf("field %s", nat.Name)
 			verr.Merge(nat.Attribute.Validate(ctx, parent))
 		}
@@ -275,6 +265,31 @@ func (a *AttributeExpr) Validate(ctx string, parent eval.Expression) *eval.Valid
 	return verr
 }
 
+func (a *AttributeExpr) validatePkgPath(pkgPath string, t DataType) *eval.ValidationErrors {
+	verr := new(eval.ValidationErrors)
+	if ar := AsArray(t); ar != nil {
+		verr.Merge(a.validatePkgPath(pkgPath, ar.ElemType.Type))
+	}
+	if mp := AsMap(t); mp != nil {
+		verr.Merge(a.validatePkgPath(pkgPath, mp.KeyType.Type))
+		verr.Merge(a.validatePkgPath(pkgPath, mp.ElemType.Type))
+	}
+	if ut, ok := t.(UserType); pkgPath != "" && ok {
+		// This check ensures we error if a sub-type has a different custom package type set
+		// or if two user types have different custom packages but share a sub-type (field that's a user type)
+		if ut.Attribute().Meta != nil &&
+			ut.Attribute().Meta["struct:pkg:path"] != nil &&
+			ut.Attribute().Meta["struct:pkg:path"][0] != pkgPath {
+			verr.Add(a, "type \"%s\" has conflicting packages %s and %s", ut.Name(), ut.Attribute().Meta["struct:pkg:path"][0], pkgPath)
+		}
+		ut.Attribute().AddMeta("struct:pkg:path", pkgPath)
+	}
+	if len(verr.Errors) > 0 {
+		return verr
+	}
+	return nil
+}
+
 // Finalize merges base and reference type attributes and finalizes the Type
 // attribute.
 func (a *AttributeExpr) Finalize() {


Test patch:
diff --git a/expr/attribute_test.go b/expr/attribute_test.go
index 809942e9..5ed3da4b 100644
--- a/expr/attribute_test.go
+++ b/expr/attribute_test.go
@@ -403,6 +403,114 @@ func TestAttributeExprValidate(t *testing.T) {
 			},
 			expected: &eval.ValidationErrors{Errors: []error{errConflictingTypes}},
 		},
+		"conflicting custom packages between sub-type in array and parent": {
+			typ: &UserTypeExpr{
+				TypeName: "FirstType",
+				AttributeExpr: &AttributeExpr{
+					Meta: MetaExpr{"struct:pkg:path": []string{"types"}},
+					Type: &Object{
+						&NamedAttributeExpr{
+							Name: "thing",
+							Attribute: &AttributeExpr{
+								Type: &Array{
+									ElemType: &AttributeExpr{
+										Type: &UserTypeExpr{
+											AttributeExpr: &AttributeExpr{
+												Meta: MetaExpr{"struct:pkg:path": []string{"types2"}},
+												Type: &Object{
+													&NamedAttributeExpr{
+														Name: "Description",
+														Attribute: &AttributeExpr{
+															Type: String,
+														},
+													},
+												},
+											},
+											TypeName: "SecondType",
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			expected: &eval.ValidationErrors{Errors: []error{errConflictingTypes}},
+		},
+		"conflicting custom packages between sub-type in map key and parent": {
+			typ: &UserTypeExpr{
+				TypeName: "FirstType",
+				AttributeExpr: &AttributeExpr{
+					Meta: MetaExpr{"struct:pkg:path": []string{"types"}},
+					Type: &Object{
+						&NamedAttributeExpr{
+							Name: "thing",
+							Attribute: &AttributeExpr{
+								Type: &Map{
+									KeyType: &AttributeExpr{
+										Type: &UserTypeExpr{
+											AttributeExpr: &AttributeExpr{
+												Meta: MetaExpr{"struct:pkg:path": []string{"types2"}},
+												Type: &Object{
+													&NamedAttributeExpr{
+														Name: "Description",
+														Attribute: &AttributeExpr{
+															Type: String,
+														},
+													},
+												},
+											},
+											TypeName: "SecondType",
+										},
+									},
+									ElemType: &AttributeExpr{
+										Type: String,
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			expected: &eval.ValidationErrors{Errors: []error{errConflictingTypes}},
+		},
+		"conflicting custom packages between sub-type in map element and parent": {
+			typ: &UserTypeExpr{
+				TypeName: "FirstType",
+				AttributeExpr: &AttributeExpr{
+					Meta: MetaExpr{"struct:pkg:path": []string{"types"}},
+					Type: &Object{
+						&NamedAttributeExpr{
+							Name: "thing",
+							Attribute: &AttributeExpr{
+								Type: &Map{
+									KeyType: &AttributeExpr{
+										Type: String,
+									},
+									ElemType: &AttributeExpr{
+										Type: &UserTypeExpr{
+											AttributeExpr: &AttributeExpr{
+												Meta: MetaExpr{"struct:pkg:path": []string{"types2"}},
+												Type: &Object{
+													&NamedAttributeExpr{
+														Name: "Description",
+														Attribute: &AttributeExpr{
+															Type: String,
+														},
+													},
+												},
+											},
+											TypeName: "SecondType",
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			expected: &eval.ValidationErrors{Errors: []error{errConflictingTypes}},
+		},
 	}
 
 	for k, tc := range cases {
@@ -414,7 +522,9 @@ func TestAttributeExprValidate(t *testing.T) {
 		if actual := attribute.Validate(ctx, nil); tc.expected != actual {
 			if len(tc.expected.Errors) != len(actual.Errors) {
 				t.Errorf("%s: expected the number of error values to match %d got %d ", k, len(tc.expected.Errors), len(actual.Errors))
-				t.Errorf("%#v", actual.Errors[0])
+				if len(actual.Errors) > 0 {
+					t.Errorf("%#v", actual.Errors[0])
+				}
 			} else {
 				for i, err := range actual.Errors {
 					if err.Error() != tc.expected.Errors[i].Error() {