Skip to content

Commit ea846a7

Browse files
fzyukiofzyukio
authored
Specify full schema for the value of MapOf, regardless of the key type (#3732)
1 parent f740ef7 commit ea846a7

File tree

3 files changed

+333
-10
lines changed

3 files changed

+333
-10
lines changed

‎http/codegen/openapi/v3/testdata/dsls/types.go

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,3 +255,128 @@ func ForcedResultTypeDSL(svcName, metName string) func() {
255255
})
256256
}
257257
}
258+
259+
func MapIntKeyBodyDSL(svcName, metName string) func() {
260+
return func() {
261+
_ = Service(svcName, func() {
262+
Method(metName, func() {
263+
Payload(func() {
264+
Attribute("intmap", MapOf(Int, ArrayOf(String)))
265+
})
266+
HTTP(func() {
267+
POST("/")
268+
})
269+
})
270+
})
271+
}
272+
}
273+
274+
func MapIntKeyObjectBodyDSL(svcName, metName string) func() {
275+
return func() {
276+
MapData := Type("MapData", func() {
277+
Attribute("id", String)
278+
Attribute("value", String)
279+
})
280+
281+
_ = Service(svcName, func() {
282+
Method(metName, func() {
283+
Payload(func() {
284+
Attribute("intmap", MapOf(Int, ArrayOf(MapData)))
285+
})
286+
HTTP(func() {
287+
POST("/")
288+
})
289+
})
290+
})
291+
}
292+
}
293+
294+
func MapIntKeyStringBodyDSL(svcName, metName string) func() {
295+
return func() {
296+
_ = Service(svcName, func() {
297+
Method(metName, func() {
298+
Payload(func() {
299+
Attribute("intmap", MapOf(Int, String))
300+
})
301+
HTTP(func() {
302+
POST("/")
303+
})
304+
})
305+
})
306+
}
307+
}
308+
309+
func MapIntKeyObjectDirectBodyDSL(svcName, metName string) func() {
310+
return func() {
311+
MapData := Type("MapData", func() {
312+
Attribute("id", String)
313+
Attribute("value", String)
314+
})
315+
316+
_ = Service(svcName, func() {
317+
Method(metName, func() {
318+
Payload(func() {
319+
Attribute("intmap", MapOf(Int, MapData))
320+
})
321+
HTTP(func() {
322+
POST("/")
323+
})
324+
})
325+
})
326+
}
327+
}
328+
329+
func MapStringKeyIntBodyDSL(svcName, metName string) func() {
330+
return func() {
331+
_ = Service(svcName, func() {
332+
Method(metName, func() {
333+
Payload(func() {
334+
Attribute("stringmap", MapOf(String, Int))
335+
})
336+
HTTP(func() {
337+
POST("/")
338+
})
339+
})
340+
})
341+
}
342+
}
343+
344+
func MapStringKeyObjectDirectBodyDSL(svcName, metName string) func() {
345+
return func() {
346+
MapData := Type("MapData", func() {
347+
Attribute("id", String)
348+
Attribute("value", String)
349+
})
350+
351+
_ = Service(svcName, func() {
352+
Method(metName, func() {
353+
Payload(func() {
354+
Attribute("stringmap", MapOf(String, MapData))
355+
})
356+
HTTP(func() {
357+
POST("/")
358+
})
359+
})
360+
})
361+
}
362+
}
363+
364+
func MapStringKeyArrayObjectBodyDSL(svcName, metName string) func() {
365+
return func() {
366+
MapData := Type("MapData", func() {
367+
Attribute("id", String)
368+
Attribute("value", String)
369+
})
370+
371+
_ = Service(svcName, func() {
372+
Method(metName, func() {
373+
Payload(func() {
374+
Attribute("stringmap", MapOf(String, ArrayOf(MapData)))
375+
})
376+
HTTP(func() {
377+
POST("/")
378+
})
379+
})
380+
})
381+
}
382+
}

‎http/codegen/openapi/v3/types.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -203,13 +203,12 @@ func (sf *schemafier) schemafy(attr *expr.AttributeExpr, noref ...bool) *openapi
203203
}
204204
case *expr.Map:
205205
s.Type = openapi.Object
206-
// OpenAPI lets you define dictionaries where the keys are strings.
207-
// See https://swagger.io/docs/specification/data-models/dictionaries/.
208-
if t.KeyType.Type == expr.String && t.ElemType.Type != expr.Any {
209-
// Use free-form objects when elements are of type "Any"
210-
s.AdditionalProperties = sf.schemafy(t.ElemType)
211-
} else if t.KeyType.Type != expr.Any {
206+
if t.ElemType.Type == expr.Any {
207+
// Use free-form objects when elements are of type "Any", otherwise, use full schema
208+
// See https://swagger.io/docs/specification/data-models/dictionaries/.
212209
s.AdditionalProperties = true
210+
} else {
211+
s.AdditionalProperties = sf.schemafy(t.ElemType)
213212
}
214213
case *expr.Union:
215214
for _, val := range t.Values {

‎http/codegen/openapi/v3/types_test.go

Lines changed: 203 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,18 @@ import (
1414

1515
// describes a type for comparison in tests.
1616
type typ struct {
17-
Type string
18-
Format string
19-
Props []attr
20-
SkipProps bool
17+
Type string
18+
Format string
19+
Props []attr
20+
SkipProps bool
21+
AdditionalProperties *additionalPropsType // nil means no additionalProperties check
22+
}
23+
24+
// additionalPropsType describes additionalProperties for testing
25+
type additionalPropsType struct {
26+
Type string // "string", "array", "object", "" (for reference)
27+
Items *additionalPropsType // for array items
28+
Ref string // for references like "#/components/schemas/MapData"
2129
}
2230

2331
type attr struct {
@@ -253,6 +261,197 @@ func matchesSchemaWithPrefix(t *testing.T, ctx string, s *openapi.Schema, types
253261
}
254262
matchesSchemaWithPrefix(t, ctx, v, types, p, n+": ")
255263
}
264+
265+
// Check additionalProperties
266+
if tt.AdditionalProperties != nil {
267+
validateAdditionalProperties(t, ctx, s.AdditionalProperties, types, tt.AdditionalProperties, prefix)
268+
}
269+
}
270+
}
271+
272+
func TestMapTypes(t *testing.T) {
273+
svcName := "test-service"
274+
275+
testCases := []struct {
276+
Name string
277+
DSL func()
278+
Expected typ
279+
}{
280+
{
281+
Name: "map_int_array_string",
282+
DSL: dsls.MapIntKeyBodyDSL(svcName, "map_int_array_string"),
283+
Expected: typ{
284+
Type: "object",
285+
Props: []attr{{Name: "intmap", Val: typ{
286+
Type: "object",
287+
AdditionalProperties: &additionalPropsType{
288+
Type: "array",
289+
Items: &additionalPropsType{Type: "string"},
290+
},
291+
}}},
292+
},
293+
},
294+
{
295+
Name: "map_int_array_object",
296+
DSL: dsls.MapIntKeyObjectBodyDSL(svcName, "map_int_array_object"),
297+
Expected: typ{
298+
Type: "object",
299+
Props: []attr{{Name: "intmap", Val: typ{
300+
Type: "object",
301+
AdditionalProperties: &additionalPropsType{
302+
Type: "array",
303+
Items: &additionalPropsType{Ref: "#/components/schemas/MapData"},
304+
},
305+
}}},
306+
},
307+
},
308+
{
309+
Name: "map_int_string",
310+
DSL: dsls.MapIntKeyStringBodyDSL(svcName, "map_int_string"),
311+
Expected: typ{
312+
Type: "object",
313+
Props: []attr{{Name: "intmap", Val: typ{
314+
Type: "object",
315+
AdditionalProperties: &additionalPropsType{Type: "string"},
316+
}}},
317+
},
318+
},
319+
{
320+
Name: "map_int_object_direct",
321+
DSL: dsls.MapIntKeyObjectDirectBodyDSL(svcName, "map_int_object_direct"),
322+
Expected: typ{
323+
Type: "object",
324+
Props: []attr{{Name: "intmap", Val: typ{
325+
Type: "object",
326+
AdditionalProperties: &additionalPropsType{Ref: "#/components/schemas/MapData"},
327+
}}},
328+
},
329+
},
330+
{
331+
Name: "map_string_int",
332+
DSL: dsls.MapStringKeyIntBodyDSL(svcName, "map_string_int"),
333+
Expected: typ{
334+
Type: "object",
335+
Props: []attr{{Name: "stringmap", Val: typ{
336+
Type: "object",
337+
AdditionalProperties: &additionalPropsType{Type: "integer"},
338+
}}},
339+
},
340+
},
341+
{
342+
Name: "map_string_object_direct",
343+
DSL: dsls.MapStringKeyObjectDirectBodyDSL(svcName, "map_string_object_direct"),
344+
Expected: typ{
345+
Type: "object",
346+
Props: []attr{{Name: "stringmap", Val: typ{
347+
Type: "object",
348+
AdditionalProperties: &additionalPropsType{Ref: "#/components/schemas/MapData"},
349+
}}},
350+
},
351+
},
352+
{
353+
Name: "map_string_array_object",
354+
DSL: dsls.MapStringKeyArrayObjectBodyDSL(svcName, "map_string_array_object"),
355+
Expected: typ{
356+
Type: "object",
357+
Props: []attr{{Name: "stringmap", Val: typ{
358+
Type: "object",
359+
AdditionalProperties: &additionalPropsType{
360+
Type: "array",
361+
Items: &additionalPropsType{Ref: "#/components/schemas/MapData"},
362+
},
363+
}}},
364+
},
365+
},
366+
}
367+
368+
for _, tc := range testCases {
369+
t.Run(tc.Name, func(t *testing.T) {
370+
// Build the OpenAPI spec
371+
root := codegen.RunDSL(t, tc.DSL)
372+
bodies, types := buildBodyTypes(root.API, root.Types, root.ResultTypes)
373+
374+
// Find the service and method
375+
svcBodies, ok := bodies[svcName]
376+
if !ok {
377+
t.Fatalf("Could not find service %s in bodies", svcName)
378+
}
379+
380+
methodBody, ok := svcBodies[tc.Name]
381+
if !ok {
382+
t.Fatalf("Could not find method %s in service bodies", tc.Name)
383+
}
384+
385+
// Get the request body schema
386+
requestBodyRef := methodBody.RequestBody.Ref
387+
if requestBodyRef == "" {
388+
t.Fatal("Expected request body reference")
389+
}
390+
391+
requestBodyTypeName := nameFromRef(requestBodyRef)
392+
requestBodySchema, ok := types[requestBodyTypeName]
393+
if !ok {
394+
t.Fatalf("Could not find request body type %s", requestBodyTypeName)
395+
}
396+
397+
// Validate the schema
398+
matchesSchema(t, tc.Name, requestBodySchema, types, tc.Expected)
399+
})
400+
}
401+
}
402+
403+
func validateAdditionalProperties(t *testing.T, ctx string, addProps interface{}, types map[string]*openapi.Schema, expected *additionalPropsType, prefix string) {
404+
if addProps == nil {
405+
t.Errorf("%s: %sexpected additionalProperties to be set", ctx, prefix)
406+
return
407+
}
408+
409+
// Check if additionalProperties is a schema
410+
schema, ok := addProps.(*openapi.Schema)
411+
if !ok {
412+
t.Errorf("%s: %sexpected additionalProperties to be schema, got %T", ctx, prefix, addProps)
413+
return
414+
}
415+
416+
validateAdditionalPropsSchema(t, ctx, schema, types, expected, prefix+"additionalProperties: ")
417+
}
418+
419+
func validateAdditionalPropsSchema(t *testing.T, ctx string, schema *openapi.Schema, types map[string]*openapi.Schema, expected *additionalPropsType, prefix string) {
420+
// Handle reference case
421+
if expected.Ref != "" {
422+
if schema.Ref == "" {
423+
t.Errorf("%s: %sexpected reference to %s, but got inline schema", ctx, prefix, expected.Ref)
424+
return
425+
}
426+
if schema.Ref != expected.Ref {
427+
t.Errorf("%s: %sexpected reference %s, got %s", ctx, prefix, expected.Ref, schema.Ref)
428+
}
429+
return
430+
}
431+
432+
// Resolve reference if present
433+
if schema.Ref != "" {
434+
typeName := nameFromRef(schema.Ref)
435+
resolvedSchema, ok := types[typeName]
436+
if !ok {
437+
t.Errorf("%s: %scould not resolve reference %s", ctx, prefix, schema.Ref)
438+
return
439+
}
440+
schema = resolvedSchema
441+
}
442+
443+
// Check type
444+
if string(schema.Type) != expected.Type {
445+
t.Errorf("%s: %sexpected type %s, got %s", ctx, prefix, expected.Type, schema.Type)
446+
}
447+
448+
// Check array items if expected
449+
if expected.Items != nil {
450+
if schema.Items == nil {
451+
t.Errorf("%s: %sexpected array items to be set", ctx, prefix)
452+
} else {
453+
validateAdditionalPropsSchema(t, ctx, schema.Items, types, expected.Items, prefix+"items: ")
454+
}
256455
}
257456
}
258457

0 commit comments

Comments
 (0)