diff --git a/http/codegen/openapi/v3/testdata/dsls/types.go b/http/codegen/openapi/v3/testdata/dsls/types.go index d3ad784974..ceb150f9e3 100644 --- a/http/codegen/openapi/v3/testdata/dsls/types.go +++ b/http/codegen/openapi/v3/testdata/dsls/types.go @@ -255,3 +255,128 @@ func ForcedResultTypeDSL(svcName, metName string) func() { }) } } + +func MapIntKeyBodyDSL(svcName, metName string) func() { + return func() { + _ = Service(svcName, func() { + Method(metName, func() { + Payload(func() { + Attribute("intmap", MapOf(Int, ArrayOf(String))) + }) + HTTP(func() { + POST("/") + }) + }) + }) + } +} + +func MapIntKeyObjectBodyDSL(svcName, metName string) func() { + return func() { + MapData := Type("MapData", func() { + Attribute("id", String) + Attribute("value", String) + }) + + _ = Service(svcName, func() { + Method(metName, func() { + Payload(func() { + Attribute("intmap", MapOf(Int, ArrayOf(MapData))) + }) + HTTP(func() { + POST("/") + }) + }) + }) + } +} + +func MapIntKeyStringBodyDSL(svcName, metName string) func() { + return func() { + _ = Service(svcName, func() { + Method(metName, func() { + Payload(func() { + Attribute("intmap", MapOf(Int, String)) + }) + HTTP(func() { + POST("/") + }) + }) + }) + } +} + +func MapIntKeyObjectDirectBodyDSL(svcName, metName string) func() { + return func() { + MapData := Type("MapData", func() { + Attribute("id", String) + Attribute("value", String) + }) + + _ = Service(svcName, func() { + Method(metName, func() { + Payload(func() { + Attribute("intmap", MapOf(Int, MapData)) + }) + HTTP(func() { + POST("/") + }) + }) + }) + } +} + +func MapStringKeyIntBodyDSL(svcName, metName string) func() { + return func() { + _ = Service(svcName, func() { + Method(metName, func() { + Payload(func() { + Attribute("stringmap", MapOf(String, Int)) + }) + HTTP(func() { + POST("/") + }) + }) + }) + } +} + +func MapStringKeyObjectDirectBodyDSL(svcName, metName string) func() { + return func() { + MapData := Type("MapData", func() { + Attribute("id", String) + Attribute("value", String) + }) + + _ = Service(svcName, func() { + Method(metName, func() { + Payload(func() { + Attribute("stringmap", MapOf(String, MapData)) + }) + HTTP(func() { + POST("/") + }) + }) + }) + } +} + +func MapStringKeyArrayObjectBodyDSL(svcName, metName string) func() { + return func() { + MapData := Type("MapData", func() { + Attribute("id", String) + Attribute("value", String) + }) + + _ = Service(svcName, func() { + Method(metName, func() { + Payload(func() { + Attribute("stringmap", MapOf(String, ArrayOf(MapData))) + }) + HTTP(func() { + POST("/") + }) + }) + }) + } +} diff --git a/http/codegen/openapi/v3/types.go b/http/codegen/openapi/v3/types.go index cd758d98e4..7d4ff1a312 100644 --- a/http/codegen/openapi/v3/types.go +++ b/http/codegen/openapi/v3/types.go @@ -203,13 +203,12 @@ func (sf *schemafier) schemafy(attr *expr.AttributeExpr, noref ...bool) *openapi } case *expr.Map: s.Type = openapi.Object - // OpenAPI lets you define dictionaries where the keys are strings. - // See https://swagger.io/docs/specification/data-models/dictionaries/. - if t.KeyType.Type == expr.String && t.ElemType.Type != expr.Any { - // Use free-form objects when elements are of type "Any" - s.AdditionalProperties = sf.schemafy(t.ElemType) - } else if t.KeyType.Type != expr.Any { + if t.ElemType.Type == expr.Any { + // Use free-form objects when elements are of type "Any", otherwise, use full schema + // See https://swagger.io/docs/specification/data-models/dictionaries/. s.AdditionalProperties = true + } else { + s.AdditionalProperties = sf.schemafy(t.ElemType) } case *expr.Union: for _, val := range t.Values { diff --git a/http/codegen/openapi/v3/types_test.go b/http/codegen/openapi/v3/types_test.go index e329bc59f9..756b62b182 100644 --- a/http/codegen/openapi/v3/types_test.go +++ b/http/codegen/openapi/v3/types_test.go @@ -14,10 +14,18 @@ import ( // describes a type for comparison in tests. type typ struct { - Type string - Format string - Props []attr - SkipProps bool + Type string + Format string + Props []attr + SkipProps bool + AdditionalProperties *additionalPropsType // nil means no additionalProperties check +} + +// additionalPropsType describes additionalProperties for testing +type additionalPropsType struct { + Type string // "string", "array", "object", "" (for reference) + Items *additionalPropsType // for array items + Ref string // for references like "#/components/schemas/MapData" } type attr struct { @@ -253,6 +261,197 @@ func matchesSchemaWithPrefix(t *testing.T, ctx string, s *openapi.Schema, types } matchesSchemaWithPrefix(t, ctx, v, types, p, n+": ") } + + // Check additionalProperties + if tt.AdditionalProperties != nil { + validateAdditionalProperties(t, ctx, s.AdditionalProperties, types, tt.AdditionalProperties, prefix) + } + } +} + +func TestMapTypes(t *testing.T) { + svcName := "test-service" + + testCases := []struct { + Name string + DSL func() + Expected typ + }{ + { + Name: "map_int_array_string", + DSL: dsls.MapIntKeyBodyDSL(svcName, "map_int_array_string"), + Expected: typ{ + Type: "object", + Props: []attr{{Name: "intmap", Val: typ{ + Type: "object", + AdditionalProperties: &additionalPropsType{ + Type: "array", + Items: &additionalPropsType{Type: "string"}, + }, + }}}, + }, + }, + { + Name: "map_int_array_object", + DSL: dsls.MapIntKeyObjectBodyDSL(svcName, "map_int_array_object"), + Expected: typ{ + Type: "object", + Props: []attr{{Name: "intmap", Val: typ{ + Type: "object", + AdditionalProperties: &additionalPropsType{ + Type: "array", + Items: &additionalPropsType{Ref: "#/components/schemas/MapData"}, + }, + }}}, + }, + }, + { + Name: "map_int_string", + DSL: dsls.MapIntKeyStringBodyDSL(svcName, "map_int_string"), + Expected: typ{ + Type: "object", + Props: []attr{{Name: "intmap", Val: typ{ + Type: "object", + AdditionalProperties: &additionalPropsType{Type: "string"}, + }}}, + }, + }, + { + Name: "map_int_object_direct", + DSL: dsls.MapIntKeyObjectDirectBodyDSL(svcName, "map_int_object_direct"), + Expected: typ{ + Type: "object", + Props: []attr{{Name: "intmap", Val: typ{ + Type: "object", + AdditionalProperties: &additionalPropsType{Ref: "#/components/schemas/MapData"}, + }}}, + }, + }, + { + Name: "map_string_int", + DSL: dsls.MapStringKeyIntBodyDSL(svcName, "map_string_int"), + Expected: typ{ + Type: "object", + Props: []attr{{Name: "stringmap", Val: typ{ + Type: "object", + AdditionalProperties: &additionalPropsType{Type: "integer"}, + }}}, + }, + }, + { + Name: "map_string_object_direct", + DSL: dsls.MapStringKeyObjectDirectBodyDSL(svcName, "map_string_object_direct"), + Expected: typ{ + Type: "object", + Props: []attr{{Name: "stringmap", Val: typ{ + Type: "object", + AdditionalProperties: &additionalPropsType{Ref: "#/components/schemas/MapData"}, + }}}, + }, + }, + { + Name: "map_string_array_object", + DSL: dsls.MapStringKeyArrayObjectBodyDSL(svcName, "map_string_array_object"), + Expected: typ{ + Type: "object", + Props: []attr{{Name: "stringmap", Val: typ{ + Type: "object", + AdditionalProperties: &additionalPropsType{ + Type: "array", + Items: &additionalPropsType{Ref: "#/components/schemas/MapData"}, + }, + }}}, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.Name, func(t *testing.T) { + // Build the OpenAPI spec + root := codegen.RunDSL(t, tc.DSL) + bodies, types := buildBodyTypes(root.API, root.Types, root.ResultTypes) + + // Find the service and method + svcBodies, ok := bodies[svcName] + if !ok { + t.Fatalf("Could not find service %s in bodies", svcName) + } + + methodBody, ok := svcBodies[tc.Name] + if !ok { + t.Fatalf("Could not find method %s in service bodies", tc.Name) + } + + // Get the request body schema + requestBodyRef := methodBody.RequestBody.Ref + if requestBodyRef == "" { + t.Fatal("Expected request body reference") + } + + requestBodyTypeName := nameFromRef(requestBodyRef) + requestBodySchema, ok := types[requestBodyTypeName] + if !ok { + t.Fatalf("Could not find request body type %s", requestBodyTypeName) + } + + // Validate the schema + matchesSchema(t, tc.Name, requestBodySchema, types, tc.Expected) + }) + } +} + +func validateAdditionalProperties(t *testing.T, ctx string, addProps interface{}, types map[string]*openapi.Schema, expected *additionalPropsType, prefix string) { + if addProps == nil { + t.Errorf("%s: %sexpected additionalProperties to be set", ctx, prefix) + return + } + + // Check if additionalProperties is a schema + schema, ok := addProps.(*openapi.Schema) + if !ok { + t.Errorf("%s: %sexpected additionalProperties to be schema, got %T", ctx, prefix, addProps) + return + } + + validateAdditionalPropsSchema(t, ctx, schema, types, expected, prefix+"additionalProperties: ") +} + +func validateAdditionalPropsSchema(t *testing.T, ctx string, schema *openapi.Schema, types map[string]*openapi.Schema, expected *additionalPropsType, prefix string) { + // Handle reference case + if expected.Ref != "" { + if schema.Ref == "" { + t.Errorf("%s: %sexpected reference to %s, but got inline schema", ctx, prefix, expected.Ref) + return + } + if schema.Ref != expected.Ref { + t.Errorf("%s: %sexpected reference %s, got %s", ctx, prefix, expected.Ref, schema.Ref) + } + return + } + + // Resolve reference if present + if schema.Ref != "" { + typeName := nameFromRef(schema.Ref) + resolvedSchema, ok := types[typeName] + if !ok { + t.Errorf("%s: %scould not resolve reference %s", ctx, prefix, schema.Ref) + return + } + schema = resolvedSchema + } + + // Check type + if string(schema.Type) != expected.Type { + t.Errorf("%s: %sexpected type %s, got %s", ctx, prefix, expected.Type, schema.Type) + } + + // Check array items if expected + if expected.Items != nil { + if schema.Items == nil { + t.Errorf("%s: %sexpected array items to be set", ctx, prefix) + } else { + validateAdditionalPropsSchema(t, ctx, schema.Items, types, expected.Items, prefix+"items: ") + } } }