Repository: goadesign/goa
Pull request #3732 (2025-06-26 03:55:27)

Specify full schema for the value of MapOf, regardless of the key type
### Issue:
When using `MapOf` in Goa designs, depending on the type of they key, we can have different results:
- If the key is String, then the value is properly typed (via `additionalProperties` + `$ref` annotation)
- Otherwise, the value is always free-form dict (`additionalProperties: true`)

### Cause:
Goa generates OpenAPI 3 specs according to https://swagger.io/docs/specification/v3_0/data-models/dictionaries/, which states that 

> OpenAPI lets you define dictionaries where the keys are strings

However, the condition (keys are strings) are implicit and cannot be changed any way (RFC 8259). So no matter what KeyType is in `MapOf(KeyType, ...)`, the results are the same, and any reader of the generated specs will interpret the map as having String key.

So by using `additionalProperties: true` we lose information that would otherwise be useful for readers of the specs (for example, oapi-codegen wouldn't be able to properly generate the correct type).

### Solution:
- Remove the restrictive condition check for KeyType == String

### Example:

#### Design
```golang

// Design
package design

import . "goa.design/goa/v3/dsl"

var _ = API("map-demo", func() {
	Title("Map Types Demonstration API")
	Description("Simple API demonstrating various map types")
	Version("1.0")
})

// Simple data object for demonstration
var DataObject = Type("DataObject", func() {
	Attribute("id", String, "Object ID")
	Attribute("value", String, "Object value")
	Required("id", "value")
})

var _ = Service("maps", func() {
	Description("Service demonstrating map types")

	Method("int-to-object", func() {
		Description("Returns map from integer to object")
		Result(MapOf(Int, DataObject))
		HTTP(func() {
			GET("/int-to-object")
			Response(StatusOK)
		})
	})

	Method("string-to-object", func() {
		Description("Returns map from string to object")
		Result(MapOf(String, DataObject))
		HTTP(func() {
			GET("/string-to-object")
			Response(StatusOK)
		})
	})

	Method("int-to-array", func() {
		Description("Returns map from integer to array of objects")
		Result(MapOf(Int, ArrayOf(DataObject)))
		HTTP(func() {
			GET("/int-to-array")
			Response(StatusOK)
		})
	})

	Method("string-to-array", func() {
		Description("Returns map from string to array of objects")
		Result(MapOf(String, ArrayOf(DataObject)))
		HTTP(func() {
			GET("/string-to-array")
			Response(StatusOK)
		})
	})
})
```

#### Current behaviour (as of v3.21.1)

```yaml
openapi: 3.0.3
info:
    title: Map Types Demonstration API
    description: Simple API demonstrating various map types
    version: "1.0"
servers:
    - url: http://localhost:80
      description: Default server for map-demo
paths:
    /int-to-array:
        get:
            tags:
                - maps
            summary: int-to-array maps
            description: Returns map from integer to array of objects
            operationId: maps#int-to-array
            responses:
                "200":
                    description: OK response.
                    content:
                        application/json:
                            schema:
                                type: object
                                additionalProperties: true <-- (1)
    /int-to-object:
        get:
            tags:
                - maps
            summary: int-to-object maps
            description: Returns map from integer to object
            operationId: maps#int-to-object
            responses:
                "200":
                    description: OK response.
                    content:
                        application/json:
                            schema:
                                type: object
                                additionalProperties: true  <-- (2)
    /string-to-array:
        get:
            tags:
                - maps
            summary: string-to-array maps
            description: Returns map from string to array of objects
            operationId: maps#string-to-array
            responses:
                "200":
                    description: OK response.
                    content:
                        application/json:
                            schema:
                                type: object
                                additionalProperties:
                                    type: array
                                    items:
                                        $ref: '#/components/schemas/DataObject' <-- (3)
    /string-to-object:
        get:
            tags:
                - maps
            summary: string-to-object maps
            description: Returns map from string to object
            operationId: maps#string-to-object
            responses:
                "200":
                    description: OK response.
                    content:
                        application/json:
                            schema:
                                type: object
                                additionalProperties:
                                    $ref: '#/components/schemas/DataObject' <-- (4)
components:
    schemas:
        DataObject:
            type: object
            properties:
                id:
                    type: string
                    description: Object ID
                value:
                    type: string
                    description: Object value
            example:
                id: Voluptatem aut accusantium.
                value: Quo illo.
            required:
                - id
                - value
tags:
    - name: maps
      description: Service demonstrating map types
```

#### New behaviour:

```yaml
openapi: 3.0.3
info:
    title: Map Types Demonstration API
    description: Simple API demonstrating various map types
    version: "1.0"
servers:
    - url: http://localhost:80
      description: Default server for map-demo
paths:
    /int-to-array:
        get:
            tags:
                - maps
            summary: int-to-array maps
            description: Returns map from integer to array of objects
            operationId: maps#int-to-array
            responses:
                "200":
                    description: OK response.
                    content:
                        application/json:
                            schema:
                                type: object
                                additionalProperties:
                                    type: array
                                    items:
                                        $ref: '#/components/schemas/DataObject' <-- (1)
    /int-to-object:
        get:
            tags:
                - maps
            summary: int-to-object maps
            description: Returns map from integer to object
            operationId: maps#int-to-object
            responses:
                "200":
                    description: OK response.
                    content:
                        application/json:
                            schema:
                                type: object
                                additionalProperties:
                                    $ref: '#/components/schemas/DataObject' <-- (2)
    /string-to-array:
        get:
            tags:
                - maps
            summary: string-to-array maps
            description: Returns map from string to array of objects
            operationId: maps#string-to-array
            responses:
                "200":
                    description: OK response.
                    content:
                        application/json:
                            schema:
                                type: object
                                additionalProperties:
                                    type: array
                                    items:
                                        $ref: '#/components/schemas/DataObject' <-- (3)
    /string-to-object:
        get:
            tags:
                - maps
            summary: string-to-object maps
            description: Returns map from string to object
            operationId: maps#string-to-object
            responses:
                "200":
                    description: OK response.
                    content:
                        application/json:
                            schema:
                                type: object
                                additionalProperties:
                                    $ref: '#/components/schemas/DataObject' <-- (4)
components:
    schemas:
        DataObject:
            type: object
            properties:
                id:
                    type: string
                    description: Object ID
                value:
                    type: string
                    description: Object value
            example:
                id: Voluptatem aut accusantium.
                value: Quo illo.
            required:
                - id
                - value
tags:
    - name: maps
      description: Service demonstrating map types

```

#### Compare:
(1): current = free-form, new = fully typed
(2): current = free-form, new = fully typed
(3): current = fully-typed, new = fully typed (same behaviour, because keys are strings)
(4): current = fully-typed, new = fully typed (same behaviour, because keys are strings)


raphael: This makes sense, thank you for the PR! There seems to be an issue with the tests (https://github.com/goadesign/goa/actions/runs/15892630093/job/44863034090?pr=3732), I'll be happy to merge once this is fixed. Running `make` locally can help finding issues faster. 
raphael: Great, thank you!

Patch:
diff --git a/http/codegen/openapi/v3/types.go b/http/codegen/openapi/v3/types.go
index cd758d98..7d4ff1a3 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 {


Test patch:
diff --git a/http/codegen/openapi/v3/testdata/dsls/types.go b/http/codegen/openapi/v3/testdata/dsls/types.go
index d3ad7849..ceb150f9 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_test.go b/http/codegen/openapi/v3/types_test.go
index e329bc59..756b62b1 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: ")
+		}
 	}
 }