Repository: goadesign/goa
Pull request #3642 (2025-02-03 03:21:15)

OpenAPIv3: Generate different schemas for types with different validation rules
See https://github.com/goadesign/goa/pull/3640


Patch:
diff --git a/go.mod b/go.mod
index f2a5d754..d6129efd 100644
--- a/go.mod
+++ b/go.mod
@@ -8,6 +8,7 @@ require (
 	github.com/dimfeld/httppath v0.0.0-20170720192232-ee938bf73598
 	github.com/getkin/kin-openapi v0.128.0
 	github.com/go-chi/chi/v5 v5.2.0
+	github.com/gohugoio/hashstructure v0.3.0
 	github.com/google/uuid v1.6.0
 	github.com/gorilla/websocket v1.5.3
 	github.com/manveru/faker v0.0.0-20171103152722-9fbc68a78c4d
diff --git a/go.sum b/go.sum
index 2c8593e4..062af0f7 100644
--- a/go.sum
+++ b/go.sum
@@ -16,6 +16,8 @@ github.com/go-openapi/swag v0.23.0 h1:vsEVJDUo2hPJ2tu0/Xc+4noaxyEffXNIs3cOULZ+Gr
 github.com/go-openapi/swag v0.23.0/go.mod h1:esZ8ITTYEsH1V2trKHjAN8Ai7xHb8RV+YSZ577vPjgQ=
 github.com/go-test/deep v1.0.8 h1:TDsG77qcSprGbC6vTN8OuXp5g+J+b5Pcguhf7Zt61VM=
 github.com/go-test/deep v1.0.8/go.mod h1:5C2ZWiW0ErCdrYzpqxLbTX7MG14M9iiw8DgHncVwcsE=
+github.com/gohugoio/hashstructure v0.3.0 h1:orHavfqnBv0ffQmobOp41Y9HKEMcjrR/8EFAzpngmGs=
+github.com/gohugoio/hashstructure v0.3.0/go.mod h1:8ohPTAfQLTs2WdzB6k9etmQYclDUeNsIHGPAFejbsEA=
 github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek=
 github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps=
 github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
diff --git a/http/codegen/openapi/v3/types.go b/http/codegen/openapi/v3/types.go
index b96deabc..95425e58 100644
--- a/http/codegen/openapi/v3/types.go
+++ b/http/codegen/openapi/v3/types.go
@@ -8,6 +8,8 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/gohugoio/hashstructure"
+
 	"goa.design/goa/v3/codegen"
 	"goa.design/goa/v3/expr"
 	"goa.design/goa/v3/http/codegen/openapi"
@@ -376,7 +378,7 @@ func toString(val any) string {
 // identifiers. Structurally identical means same primitive types, arrays with
 // structurally equivalent element types, maps with structurally equivalent key
 // and value types or object with identical attribute names and structurally
-// equivalent types and identical set of required attributes.
+// equivalent types and identical set of validation rules.
 func (*schemafier) hashAttribute(att *expr.AttributeExpr, h hash.Hash64) uint64 {
 	return *hashAttribute(att, h, make(map[string]*uint64))
 }
@@ -393,6 +395,7 @@ func hashAttribute(att *expr.AttributeExpr, h hash.Hash64, seen map[string]*uint
 	}
 	seen[t.Hash()] = res
 
+	hv := hashValidation(att.Validation, h)
 	switch t.Kind() {
 	case expr.ObjectKind:
 		o := expr.AsObject(t)
@@ -404,25 +407,26 @@ func hashAttribute(att *expr.AttributeExpr, h hash.Hash64, seen map[string]*uint
 			vh := hashAttribute(m.Attribute, h, seen)
 			*res = *res ^ orderedHash(kh, *vh, h)
 		}
-		// Objects with a different set of required attributes should produce
-		// different hashes.
-		if att.Validation != nil {
-			for _, req := range att.Validation.Required {
-				rh := hashString(req, h)
-				*res = *res ^ rh
-			}
+		if hv != 0 {
+			*res = orderedHash(*res, hv, h)
 		}
 
 	case expr.ArrayKind:
 		kh := hashString("[]", h)
 		vh := hashAttribute(expr.AsArray(t).ElemType, h, seen)
 		*res = orderedHash(kh, *vh, h)
+		if hv != 0 {
+			*res = orderedHash(*res, hv, h)
+		}
 
 	case expr.MapKind:
 		m := expr.AsMap(t)
 		kh := hashAttribute(m.KeyType, h, seen)
 		vh := hashAttribute(m.ElemType, h, seen)
 		*res = orderedHash(*kh, *vh, h)
+		if hv != 0 {
+			*res = orderedHash(*res, hv, h)
+		}
 
 	case expr.UserTypeKind:
 		*res = *hashAttribute(t.(expr.UserType).Attribute(), h, seen)
@@ -438,8 +442,31 @@ func hashAttribute(att *expr.AttributeExpr, h hash.Hash64, seen map[string]*uint
 
 	default: // Primitives or Any
 		*res = hashString(t.Name(), h)
+		if hv != 0 {
+			*res = orderedHash(*res, hv, h)
+		}
+	}
+
+	return res
+}
+
+func hashValidation(val *expr.ValidationExpr, h hash.Hash64) uint64 {
+	// Note: we can't use hashstructure for attributes because it doesn't
+	// handle recursive structures.
+	if val == nil {
+		return 0
 	}
 
+	res, err := hashstructure.Hash(val, &hashstructure.HashOptions{
+		Hasher:          h,
+		ZeroNil:         false,
+		IgnoreZeroValue: true,
+		SlicesAsSets:    true,
+	})
+	if err != nil {
+		// should really never happen (OOM maybe)
+		return 0
+	}
 	return res
 }
 


Test patch:
diff --git a/http/codegen/openapi/v3/files_test.go b/http/codegen/openapi/v3/files_test.go
index f90e9a77..7efa6519 100644
--- a/http/codegen/openapi/v3/files_test.go
+++ b/http/codegen/openapi/v3/files_test.go
@@ -117,7 +117,7 @@ func TestFiles(t *testing.T) {
 							left = buf.String()
 							right = string(want)
 						}
-						assert.Equal(t, left, right)
+						assert.Equal(t, right, left)
 					}
 				})
 			}
diff --git a/http/codegen/openapi/v3/testdata/dsls/types.go b/http/codegen/openapi/v3/testdata/dsls/types.go
index 8e13029b..d3ad7849 100644
--- a/http/codegen/openapi/v3/testdata/dsls/types.go
+++ b/http/codegen/openapi/v3/testdata/dsls/types.go
@@ -15,6 +15,49 @@ func StringBodyDSL(svcName, metName string) func() {
 	}
 }
 
+func StringEnumBodyDSL() func() {
+	return func() {
+		var T1 = Type("T1", func() {
+			Attribute("my_attr", String, func() {
+				Enum("a", "b", "c")
+			})
+		})
+
+		var T2 = Type("T2", func() {
+			Attribute("my_attr", String, func() {
+				Enum("d", "e")
+			})
+		})
+
+		var _ = Service("svc_enum_1", func() {
+			Method("method_enum", func() {
+				Payload(func() {
+					Reference(T1)
+					Attribute("my_attr")
+					Required("my_attr")
+				})
+				HTTP(func() {
+					POST("/")
+				})
+			})
+		})
+
+		var _ = Service("svc_enum_2", func() {
+			Method("method_enum", func() {
+				Payload(func() {
+					Reference(T2)
+					Attribute("my_attr")
+					Required("my_attr")
+				})
+
+				HTTP(func() {
+					POST("/other")
+				})
+			})
+		})
+	}
+}
+
 func AliasStringBodyDSL(svcName, metName string) func() {
 	return func() {
 		var UUID = Type("UUID", String, func() {
diff --git a/http/codegen/openapi/v3/types_test.go b/http/codegen/openapi/v3/types_test.go
index 3d641256..aa3b0136 100644
--- a/http/codegen/openapi/v3/types_test.go
+++ b/http/codegen/openapi/v3/types_test.go
@@ -1,6 +1,7 @@
 package openapiv3
 
 import (
+	"encoding/json"
 	"hash/fnv"
 	"strings"
 	"testing"
@@ -255,69 +256,180 @@ func matchesSchemaWithPrefix(t *testing.T, ctx string, s *openapi.Schema, types
 	}
 }
 
+func TestTypesOnlyDifferByEnum(t *testing.T) {
+	api := codegen.RunDSL(t, dsls.StringEnumBodyDSL()).API
+
+	bodies, types := buildBodyTypes(api)
+
+	svc1, ok := bodies["svc_enum_1"]
+	if !ok {
+		t.Errorf("bodies does not contain details for service %q", "svc_enum_1")
+		return
+	}
+	svc2, ok := bodies["svc_enum_2"]
+	if !ok {
+		t.Errorf("bodies does not contain details for service %q", "svc_enum_2")
+		return
+	}
+
+	svc1MethodRB := svc1["method_enum"].RequestBody.Ref
+	svc2MethodRB := svc2["method_enum"].RequestBody.Ref
+
+	if svc1MethodRB == svc2MethodRB {
+		t.Errorf("expected different refs, got %q", svc1MethodRB)
+
+		name := nameFromRef(svc1MethodRB)
+		derefed := types[name]
+		jsoned, _ := json.Marshal(derefed)
+		t.Errorf("shared referenced type (%s) was: %v", name, string(jsoned))
+		return
+	}
+}
+
 func TestHashAttribute(t *testing.T) {
-	var (
-		h1 = uint64(12943244719806607708)
-		h2 = uint64(7733915756259492975)
-		h3 = uint64(7729867354446285276)
-		h4 = uint64(12938215553621425391)
-		h5 = uint64(590638987843676710)
-		h6 = uint64(2958992150570065940)
-		h7 = uint64(17427721879237743911)
+	type (
+		testAttr struct {
+			name string
+			att  *expr.AttributeExpr
+		}
+
+		hashBehavior int
+
+		testGroup struct {
+			name     string
+			attrs    []testAttr
+			behavior hashBehavior
+		}
+	)
 
+	const (
+		uniqueHashes hashBehavior = iota
+		identicalHashes
+	)
+
+	var (
 		metaNotGenerate = expr.MetaExpr{"openapi:generate": []string{"false"}}
 		metaEmpty       = expr.MetaExpr{}
 	)
-	cases := []struct {
-		Name string
-		att  *expr.AttributeExpr
-		h    uint64
-	}{
-		{"bool", &expr.AttributeExpr{Type: expr.Boolean}, 1200285950329868815},
-		{"int", &expr.AttributeExpr{Type: expr.Int}, 15618947606512183472},
-		{"int32", &expr.AttributeExpr{Type: expr.Int32}, 9710406772214674507},
-		{"int64", &expr.AttributeExpr{Type: expr.Int64}, 9710410070749559206},
-		{"uint", &expr.AttributeExpr{Type: expr.UInt}, 9334303408231097877},
-		{"uint32", &expr.AttributeExpr{Type: expr.UInt32}, 14693036559411812390},
-		{"uint64", &expr.AttributeExpr{Type: expr.UInt64}, 14693033260876927695},
-		{"float32", &expr.AttributeExpr{Type: expr.Float32}, 3496747786213902106},
-		{"float64", &expr.AttributeExpr{Type: expr.Float64}, 3496753283772043155},
-		{"string", &expr.AttributeExpr{Type: expr.String}, 11035750783128163470},
-		{"bytes", &expr.AttributeExpr{Type: expr.Bytes}, 9376284137219620846},
-		{"any", &expr.AttributeExpr{Type: expr.Any}, 15626582615256966821},
-		{"array-bool", &expr.AttributeExpr{Type: &expr.Array{ElemType: &expr.AttributeExpr{Type: expr.Boolean}}}, 11710318443436489022},
-		{"array-int", &expr.AttributeExpr{Type: &expr.Array{ElemType: &expr.AttributeExpr{Type: expr.Int}}}, 16304700464423429033},
-		{"map-str-int", &expr.AttributeExpr{Type: &expr.Map{KeyType: &expr.AttributeExpr{Type: expr.String}, ElemType: &expr.AttributeExpr{Type: expr.Int}}}, 957614225485715479},
-		{"map-str-str", &expr.AttributeExpr{Type: &expr.Map{KeyType: &expr.AttributeExpr{Type: expr.String}, ElemType: &expr.AttributeExpr{Type: expr.String}}}, 10408036596908747853},
-		{"map-int-str", &expr.AttributeExpr{Type: &expr.Map{KeyType: &expr.AttributeExpr{Type: expr.Int}, ElemType: &expr.AttributeExpr{Type: expr.String}}}, 16377853221392883275},
-		{"map-int-int", &expr.AttributeExpr{Type: &expr.Map{KeyType: &expr.AttributeExpr{Type: expr.Int}, ElemType: &expr.AttributeExpr{Type: expr.Int}}}, 3290208366554661977},
-		{"obj-str-req", newObj("foo", expr.String, true), h6},
-		{"obj-str-noreq", newObj("foo", expr.String, false), h7},
-		{"obj-int-req", newObj("foo", expr.Int, true), 8915021286725901502},
-		{"obj-int-noreq", newObj("foo", expr.Int, false), 11777831908257753485},
-		{"obj-other", newObj("bar", expr.Int, false), 12868551315046025641},
-		{"obj-str-str-noreq", newObj2("foo", "bar", expr.String, expr.String), h1},
-		{"obj-str-str-req1", newObj2("foo", "bar", expr.String, expr.String, "foo"), h2},
-		{"obj-str-str-req2", newObj2("foo", "bar", expr.String, expr.String, "bar"), h3},
-		{"obj-str-str-req3", newObj2("foo", "bar", expr.String, expr.String, "foo", "bar"), h4},
-		{"obj-str-str-notgen-req", newObj2Meta("foo", "bar", expr.String, expr.String, metaEmpty, metaNotGenerate, "foo"), h6},
-		{"obj-str-str-notgen-noreq", newObj2Meta("foo", "bar", expr.String, expr.String, metaEmpty, metaNotGenerate), h7},
-		{"obj-int-str-noreq", newObj2("foo", "bar", expr.Int, expr.String), 16228531529443692022},
-		{"obj1-str-str-noreq", newObj2("bar", "foo", expr.String, expr.String), h1},
-		{"obj1-str-str-req1", newObj2("bar", "foo", expr.String, expr.String, "foo"), h2},
-		{"obj1-str-str-req2", newObj2("bar", "foo", expr.String, expr.String, "bar"), h3},
-		{"obj1-str-str-req3", newObj2("bar", "foo", expr.String, expr.String, "bar", "foo"), h4},
-		{"result", newRT("id", newObj("foo", expr.String, true)), h5},
-		{"result-diff", newRT("id2", newObj("foo", expr.String, true)), 15618941009442414240},
-		{"result-same", newRT("id", newObj("foo", expr.Int, true)), h5},
+
+	cases := []testGroup{
+		{
+			name:     "Primitive types",
+			behavior: uniqueHashes,
+			attrs: []testAttr{
+				{name: "bool", att: &expr.AttributeExpr{Type: expr.Boolean}},
+				{name: "int", att: &expr.AttributeExpr{Type: expr.Int}},
+				{name: "int32", att: &expr.AttributeExpr{Type: expr.Int32}},
+				{name: "int64", att: &expr.AttributeExpr{Type: expr.Int64}},
+				{name: "uint", att: &expr.AttributeExpr{Type: expr.UInt}},
+				{name: "uint32", att: &expr.AttributeExpr{Type: expr.UInt32}},
+				{name: "uint64", att: &expr.AttributeExpr{Type: expr.UInt64}},
+				{name: "float32", att: &expr.AttributeExpr{Type: expr.Float32}},
+				{name: "float64", att: &expr.AttributeExpr{Type: expr.Float64}},
+				{name: "string", att: &expr.AttributeExpr{Type: expr.String}},
+				{name: "bytes", att: &expr.AttributeExpr{Type: expr.Bytes}},
+				{name: "any", att: &expr.AttributeExpr{Type: expr.Any}},
+			},
+		}, {
+			name:     "Collection types",
+			behavior: uniqueHashes,
+			attrs: []testAttr{
+				{name: "array-bool", att: &expr.AttributeExpr{Type: &expr.Array{ElemType: &expr.AttributeExpr{Type: expr.Boolean}}}},
+				{name: "array-int", att: &expr.AttributeExpr{Type: &expr.Array{ElemType: &expr.AttributeExpr{Type: expr.Int}}}},
+				{name: "map-str-int", att: &expr.AttributeExpr{Type: &expr.Map{KeyType: &expr.AttributeExpr{Type: expr.String}, ElemType: &expr.AttributeExpr{Type: expr.Int}}}},
+				{name: "map-str-str", att: &expr.AttributeExpr{Type: &expr.Map{KeyType: &expr.AttributeExpr{Type: expr.String}, ElemType: &expr.AttributeExpr{Type: expr.String}}}},
+			},
+		}, {
+			name:     "Objects with validation rules",
+			behavior: uniqueHashes,
+			attrs: []testAttr{
+				{name: "no-validation", att: newObj("foo", expr.String, false)},
+				{name: "required-validation", att: newObj("foo", expr.String, true)},
+				{name: "pattern-validation", att: &expr.AttributeExpr{
+					Type: expr.String,
+					Validation: &expr.ValidationExpr{
+						Pattern: "^[a-z]+$",
+					},
+				}},
+				{name: "enum-validation", att: &expr.AttributeExpr{
+					Type: expr.String,
+					Validation: &expr.ValidationExpr{
+						Values: []any{"foo", "bar"},
+					},
+				}},
+			},
+		}, {
+			name:     "Result types with different views",
+			behavior: uniqueHashes,
+			attrs: []testAttr{
+				{name: "no-view", att: newRT("id", newObj("foo", expr.String, true))},
+				{name: "default-view", att: newRTWithView("id", newObj("foo", expr.String, true), "default")},
+				{name: "tiny-view", att: newRTWithView("id", newObj("foo", expr.String, true), "tiny")},
+			},
+		}, {
+			name:     "Objects with openapi:generate:false metadata",
+			behavior: identicalHashes,
+			attrs: []testAttr{
+				{name: "obj-with-skipped-field", att: newObj2Meta("foo", "bar", expr.String, expr.String, metaEmpty, metaNotGenerate)},
+				{name: "obj-without-skipped-field", att: newObj("foo", expr.String, false)},
+			},
+		}, {
+			name:     "Complex map types",
+			behavior: uniqueHashes,
+			attrs: []testAttr{
+				{name: "map-int-array", att: &expr.AttributeExpr{Type: &expr.Map{
+					KeyType:  &expr.AttributeExpr{Type: expr.Int},
+					ElemType: &expr.AttributeExpr{Type: &expr.Array{ElemType: &expr.AttributeExpr{Type: expr.String}}},
+				}}},
+				{name: "map-array-int", att: &expr.AttributeExpr{Type: &expr.Map{
+					KeyType:  &expr.AttributeExpr{Type: &expr.Array{ElemType: &expr.AttributeExpr{Type: expr.String}}},
+					ElemType: &expr.AttributeExpr{Type: expr.Int},
+				}}},
+			},
+		}, {
+			name:     "Nested user types",
+			behavior: uniqueHashes,
+			attrs: []testAttr{
+				{name: "single-nest", att: newUserType("foo", newObj("bar", expr.String, false))},
+				{name: "double-nest", att: newUserType("foo", newUserType("bar", newObj("baz", expr.String, false)))},
+			},
+		}, {
+			name:     "Recursive types",
+			behavior: identicalHashes,
+			attrs: []testAttr{
+				{name: "recursive-1", att: newRecursiveType("foo")},
+				{name: "recursive-2", att: newRecursiveType("foo")},
+			},
+		},
 	}
+
 	h := fnv.New64()
 	sf := newSchemafier(expr.NewRandom("test"))
-	for _, c := range cases {
-		t.Run(c.Name, func(t *testing.T) {
-			got := sf.hashAttribute(c.att, h)
-			if got != c.h {
-				t.Errorf("got %v, expected %v", got, c.h)
+
+	for _, group := range cases {
+		t.Run(group.name, func(t *testing.T) {
+			seen := make(map[uint64][]string)
+
+			// Collect all hashes in this group
+			for _, attr := range group.attrs {
+				hash := sf.hashAttribute(attr.att, h)
+				seen[hash] = append(seen[hash], attr.name)
+			}
+
+			switch group.behavior {
+			case uniqueHashes:
+				// Verify all hashes are different
+				for hash, names := range seen {
+					if len(names) > 1 {
+						t.Errorf("expected unique hashes but got collision between %v (hash: %d)",
+							names, hash)
+					}
+				}
+			case identicalHashes:
+				// Verify all hashes are the same
+				if len(seen) > 1 {
+					t.Errorf("expected identical hashes but got different ones: %v", seen)
+				}
 			}
 		})
 	}
@@ -334,18 +446,6 @@ func newObj(n string, t expr.DataType, req bool) *expr.AttributeExpr {
 	return attr
 }
 
-func newObj2(n, o string, t, u expr.DataType, reqs ...string) *expr.AttributeExpr {
-	attr := &expr.AttributeExpr{
-		Type: &expr.Object{
-			{Name: n, Attribute: &expr.AttributeExpr{Type: t}},
-			{Name: o, Attribute: &expr.AttributeExpr{Type: u}},
-		},
-		Validation: &expr.ValidationExpr{},
-	}
-	attr.Validation.Required = append(attr.Validation.Required, reqs...)
-	return attr
-}
-
 func newObj2Meta(n, o string, t, u expr.DataType, l, m expr.MetaExpr, reqs ...string) *expr.AttributeExpr {
 	attr := &expr.AttributeExpr{
 		Type: &expr.Object{
@@ -369,6 +469,45 @@ func newRT(id string, att *expr.AttributeExpr) *expr.AttributeExpr {
 	}
 }
 
+// Helper function for result types with views
+func newRTWithView(id string, att *expr.AttributeExpr, view string) *expr.AttributeExpr {
+	rt := newRT(id, att)
+	rt.Type.(*expr.ResultTypeExpr).AttributeExpr.Meta = expr.MetaExpr{
+		expr.ViewMetaKey: []string{view},
+	}
+	return rt
+}
+
+// Helper function for user types
+func newUserType(name string, att *expr.AttributeExpr) *expr.AttributeExpr {
+	return &expr.AttributeExpr{
+		Type: &expr.UserTypeExpr{
+			AttributeExpr: att,
+			TypeName:      name,
+		},
+	}
+}
+
+// Helper function for recursive types
+func newRecursiveType(name string) *expr.AttributeExpr {
+	// Create a user type that references itself
+	ut := &expr.UserTypeExpr{
+		TypeName: name,
+	}
+	att := &expr.AttributeExpr{
+		Type: &expr.Object{
+			&expr.NamedAttributeExpr{
+				Name: "self",
+				Attribute: &expr.AttributeExpr{
+					Type: ut,
+				},
+			},
+		},
+	}
+	ut.AttributeExpr = att
+	return &expr.AttributeExpr{Type: ut}
+}
+
 // nameFromRef does the reverse of toRef: it returns the type name from its
 // JSON Schema reference.
 func nameFromRef(ref string) string {