Skip to content

Commit

Permalink
Add "openapi:typename" meta to user types (#3572)
Browse files Browse the repository at this point in the history
This update forces OpenAPI generators to use specified type names rather than merging structurally identical types automatically.

Note: Goa-generated types (e.g., from anonymous payload or result definitions) will still be merged if structurally identical. This approach helps prevent the unnecessary proliferation of types in OpenAPI specifications.
  • Loading branch information
raphael authored Aug 14, 2024
1 parent d9f4dc7 commit df1919c
Show file tree
Hide file tree
Showing 41 changed files with 226 additions and 280 deletions.
1 change: 1 addition & 0 deletions dsl/result_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ func ResultType(identifier string, args ...any) *expr.ResultTypeExpr {
}
// Add the type to the generated types root for later evaluation.
rt := expr.NewResultTypeExpr(typeName, identifier, fn)
rt.Meta = expr.MetaExpr{"openapi:typename": []string{typeName}}
expr.Root.ResultTypes = append(expr.Root.ResultTypes, rt)

return rt
Expand Down
8 changes: 6 additions & 2 deletions dsl/user_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,12 @@ func Type(name string, args ...any) expr.UserType {
}

t := &expr.UserTypeExpr{
TypeName: name,
AttributeExpr: &expr.AttributeExpr{Type: base, DSLFunc: fn},
TypeName: name,
AttributeExpr: &expr.AttributeExpr{
Type: base,
DSLFunc: fn,
Meta: expr.MetaExpr{"openapi:typename": []string{name}},
},
}
expr.Root.Types = append(expr.Root.Types, t)
return t
Expand Down
95 changes: 29 additions & 66 deletions http/codegen/openapi/v2/files_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

"github.com/getkin/kin-openapi/openapi2"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

httpgen "goa.design/goa/v3/http/codegen"
"goa.design/goa/v3/http/codegen/openapi"
Expand Down Expand Up @@ -99,15 +100,15 @@ func TestSections(t *testing.T) {
t.Fatalf("failed to read golden file: %s", err)
}
if !bytes.Equal(buf.Bytes(), want) {
var left, right string
var got, expected string
if filepath.Ext(o.Path) == ".json" {
left = prettifyJSON(t, buf.Bytes())
right = prettifyJSON(t, want)
got = prettifyJSON(t, buf.Bytes())
expected = prettifyJSON(t, want)
} else {
left = buf.String()
right = string(want)
got = buf.String()
expected = string(want)
}
assert.Equal(t, left, right)
assert.Equal(t, expected, got)
}
})
}
Expand Down Expand Up @@ -145,51 +146,32 @@ func TestValidations(t *testing.T) {
openapi.Definitions = make(map[string]*openapi.Schema)
root := httpgen.RunHTTPDSL(t, c.DSL)
oFiles, err := openapiv2.Files(root)
if err != nil {
t.Fatalf("OpenAPI failed with %s", err)
}
if len(oFiles) == 0 {
t.Fatalf("No swagger files")
}
require.NoError(t, err, "OpenAPI failed")
require.NotEmpty(t, oFiles, "No swagger files")
for i, o := range oFiles {
tname := fmt.Sprintf("file%d", i)
s := o.SectionTemplates
t.Run(tname, func(t *testing.T) {
if len(s) != 1 {
t.Fatalf("expected 1 section, got %d", len(s))
}
if s[0].Source == "" {
t.Fatalf("empty section template")
}
if s[0].Data == nil {
t.Fatalf("nil data")
}
require.Len(t, s, 1, "expected 1 section, got %d", len(s))
require.NotEmpty(t, s[0].Source, "empty section template")
require.NotNil(t, s[0].Data, "nil data")
var buf bytes.Buffer
tmpl := template.Must(template.New("openapi").Funcs(s[0].FuncMap).Parse(s[0].Source))
if err := tmpl.Execute(&buf, s[0].Data); err != nil {
t.Fatalf("failed to render template: %s", err)
}
require.NoError(t, tmpl.Execute(&buf, s[0].Data), "failed to render template")
if filepath.Ext(o.Path) == ".json" {
if err := validateSwagger(buf.Bytes()); err != nil {
t.Fatalf("invalid swagger: %s", err)
}
require.NoError(t, validateSwagger(buf.Bytes()), "invalid swagger")
}

golden := filepath.Join(goldenPath, fmt.Sprintf("%s_%s.golden", c.Name, tname))
if *update {
if err := os.WriteFile(golden, buf.Bytes(), 0644); err != nil {
t.Fatalf("failed to update golden file: %s", err)
}
require.NoError(t, os.WriteFile(golden, buf.Bytes(), 0644), "failed to update golden file")
return
}

want, err := os.ReadFile(golden)
require.NoError(t, err, "failed to read golden file")
want = bytes.ReplaceAll(want, []byte{'\r', '\n'}, []byte{'\n'})
if err != nil {
t.Fatalf("failed to read golden file: %s", err)
}
if !bytes.Equal(buf.Bytes(), want) {
t.Errorf("result do not match the golden file:\n--BEGIN--\n%s\n--END--\n", buf.Bytes())
}
assert.Equal(t, string(want), buf.String())
})
}
})
Expand All @@ -212,51 +194,32 @@ func TestExtensions(t *testing.T) {
openapi.Definitions = make(map[string]*openapi.Schema)
root := httpgen.RunHTTPDSL(t, c.DSL)
oFiles, err := openapiv2.Files(root)
if err != nil {
t.Fatalf("OpenAPI failed with %s", err)
}
if len(oFiles) == 0 {
t.Fatalf("No swagger files")
}
require.NoError(t, err, "OpenAPI failed")
require.NotEmpty(t, oFiles, "No swagger files")
for i, o := range oFiles {
tname := fmt.Sprintf("file%d", i)
s := o.SectionTemplates
t.Run(tname, func(t *testing.T) {
if len(s) != 1 {
t.Fatalf("expected 1 section, got %d", len(s))
}
if s[0].Source == "" {
t.Fatalf("empty section template")
}
if s[0].Data == nil {
t.Fatalf("nil data")
}
require.Len(t, s, 1, "expected 1 section, got %d", len(s))
require.NotEmpty(t, s[0].Source, "empty section template")
require.NotNil(t, s[0].Data, "nil data")
var buf bytes.Buffer
tmpl := template.Must(template.New("openapi").Funcs(s[0].FuncMap).Parse(s[0].Source))
if err := tmpl.Execute(&buf, s[0].Data); err != nil {
t.Fatalf("failed to render template: %s", err)
}
require.NoError(t, tmpl.Execute(&buf, s[0].Data), "failed to render template")
if filepath.Ext(o.Path) == ".json" {
if err := validateSwagger(buf.Bytes()); err != nil {
t.Fatalf("invalid swagger: %s", err)
}
require.NoError(t, validateSwagger(buf.Bytes()), "invalid swagger")
}

golden := filepath.Join(goldenPath, fmt.Sprintf("%s_%s.golden", c.Name, tname))
if *update {
if err := os.WriteFile(golden, buf.Bytes(), 0644); err != nil {
t.Fatalf("failed to update golden file: %s", err)
}
require.NoError(t, os.WriteFile(golden, buf.Bytes(), 0644), "failed to update golden file")
return
}

want, err := os.ReadFile(golden)
want = bytes.ReplaceAll(want, []byte{'\r', '\n'}, []byte{'\n'})
if err != nil {
t.Fatalf("failed to read golden file: %s", err)
}
if !bytes.Equal(buf.Bytes(), want) {
t.Errorf("result do not match the golden file:\n--BEGIN--\n%s\n--END--\n", buf.Bytes())
}
require.NoError(t, err, "failed to read golden file")
assert.Equal(t, string(want), buf.String())
})
}
})
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"swagger":"2.0","info":{"title":"","version":"0.0.1","x-test-api":"API"},"host":"goa.design","consumes":["application/json","application/xml","application/gob"],"produces":["application/json","application/xml","application/gob"],"paths":{"/":{"post":{"operationId":"testService#testEndpoint","parameters":[{"in":"body","name":"TestEndpointRequestBody","required":true,"schema":{"$ref":"#/definitions/TestServiceTestEndpointRequestBody"}}],"responses":{"200":{"description":"OK response.","schema":{"$ref":"#/definitions/TestServiceTestEndpointResponseBody"}}},"schemes":["https"],"summary":"testEndpoint testService","tags":["testService"],"x-test-operation":"Operation"},"x-test-foo":"bar"}},"definitions":{"TestServiceTestEndpointRequestBody":{"title":"TestServiceTestEndpointRequestBody","type":"object","properties":{"string":{"example":"","type":"string","x-test-schema":"Payload"}},"example":{"string":""}},"TestServiceTestEndpointResponseBody":{"title":"TestServiceTestEndpointResponseBody","type":"object","properties":{"string":{"example":"","type":"string","x-test-schema":"Result"}},"example":{"string":""}}},"tags":[{"description":"Description of Backend","externalDocs":{"description":"See more docs here","url":"http://example.com"},"name":"Backend","x-data":{"foo":"bar"}}]}
{"swagger":"2.0","info":{"title":"","version":"0.0.1","x-test-api":"API"},"host":"goa.design","consumes":["application/json","application/xml","application/gob"],"produces":["application/json","application/xml","application/gob"],"paths":{"/":{"post":{"operationId":"testService#testEndpoint","parameters":[{"in":"body","name":"TestEndpointRequestBody","required":true,"schema":{"$ref":"#/definitions/Payload"}}],"responses":{"200":{"description":"OK response.","schema":{"$ref":"#/definitions/Result"}}},"schemes":["https"],"summary":"testEndpoint testService","tags":["testService"],"x-test-operation":"Operation"},"x-test-foo":"bar"}},"definitions":{"Payload":{"title":"Payload","type":"object","properties":{"string":{"example":"","type":"string","x-test-schema":"Payload"}},"example":{"string":""}},"Result":{"title":"Result","type":"object","properties":{"string":{"example":"","type":"string","x-test-schema":"Result"}},"example":{"string":""}}},"tags":[{"description":"Description of Backend","externalDocs":{"description":"See more docs here","url":"http://example.com"},"name":"Backend","x-data":{"foo":"bar"}}]}
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ paths:
name: TestEndpointRequestBody
required: true
schema:
$ref: '#/definitions/TestServiceTestEndpointRequestBody'
$ref: '#/definitions/Payload'
responses:
"200":
description: OK response.
schema:
$ref: '#/definitions/TestServiceTestEndpointResponseBody'
$ref: '#/definitions/Result'
schemes:
- https
summary: testEndpoint testService
Expand All @@ -35,8 +35,8 @@ paths:
x-test-operation: Operation
x-test-foo: bar
definitions:
TestServiceTestEndpointRequestBody:
title: TestServiceTestEndpointRequestBody
Payload:
title: Payload
type: object
properties:
string:
Expand All @@ -45,8 +45,8 @@ definitions:
x-test-schema: Payload
example:
string: ""
TestServiceTestEndpointResponseBody:
title: TestServiceTestEndpointResponseBody
Result:
title: Result
type: object
properties:
string:
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"swagger":"2.0","info":{"title":"","version":"0.0.1"},"host":"goa.design","consumes":["application/json","application/xml","application/gob"],"produces":["application/json","application/xml","application/gob"],"paths":{"/":{"get":{"tags":["testService"],"summary":"testEndpoint testService","operationId":"testService#testEndpoint","parameters":[{"name":"TestEndpointRequestBody","in":"body","required":true,"schema":{"$ref":"#/definitions/TestServiceTestEndpointRequestBody"}}],"responses":{"200":{"description":"OK response.","schema":{"$ref":"#/definitions/TestServiceTestEndpointResponseBody"}}},"schemes":["https"]},"post":{"tags":["anotherTestService"],"summary":"testEndpoint anotherTestService","operationId":"anotherTestService#testEndpoint","parameters":[{"name":"TestEndpointRequestBody","in":"body","required":true,"schema":{"$ref":"#/definitions/AnotherTestServiceTestEndpointRequestBody"}}],"responses":{"200":{"description":"OK response.","schema":{"$ref":"#/definitions/AnotherTestServiceTestEndpointResponseBody"}}},"schemes":["https"]}}},"definitions":{"AnotherTestServiceTestEndpointRequestBody":{"title":"AnotherTestServiceTestEndpointRequestBody","type":"object","properties":{"string":{"type":"string","example":""}},"example":{"string":""}},"AnotherTestServiceTestEndpointResponseBody":{"title":"AnotherTestServiceTestEndpointResponseBody","type":"object","properties":{"string":{"type":"string","example":""}},"example":{"string":""}},"TestServiceTestEndpointRequestBody":{"title":"TestServiceTestEndpointRequestBody","type":"object","properties":{"string":{"type":"string","example":""}},"example":{"string":""}},"TestServiceTestEndpointResponseBody":{"title":"TestServiceTestEndpointResponseBody","type":"object","properties":{"string":{"type":"string","example":""}},"example":{"string":""}}}}
{"swagger":"2.0","info":{"title":"","version":"0.0.1"},"host":"goa.design","consumes":["application/json","application/xml","application/gob"],"produces":["application/json","application/xml","application/gob"],"paths":{"/":{"get":{"tags":["testService"],"summary":"testEndpoint testService","operationId":"testService#testEndpoint","parameters":[{"name":"TestEndpointRequestBody","in":"body","required":true,"schema":{"$ref":"#/definitions/Payload"}}],"responses":{"200":{"description":"OK response.","schema":{"$ref":"#/definitions/Result"}}},"schemes":["https"]},"post":{"tags":["anotherTestService"],"summary":"testEndpoint anotherTestService","operationId":"anotherTestService#testEndpoint","parameters":[{"name":"TestEndpointRequestBody","in":"body","required":true,"schema":{"$ref":"#/definitions/Payload"}}],"responses":{"200":{"description":"OK response.","schema":{"$ref":"#/definitions/Result"}}},"schemes":["https"]}}},"definitions":{"Payload":{"title":"Payload","type":"object","properties":{"string":{"type":"string","example":""}},"example":{"string":""}},"Result":{"title":"Result","type":"object","properties":{"string":{"type":"string","example":""}},"example":{"string":""}}}}
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ paths:
in: body
required: true
schema:
$ref: '#/definitions/TestServiceTestEndpointRequestBody'
$ref: '#/definitions/Payload'
responses:
"200":
description: OK response.
schema:
$ref: '#/definitions/TestServiceTestEndpointResponseBody'
$ref: '#/definitions/Result'
schemes:
- https
post:
Expand All @@ -41,44 +41,26 @@ paths:
in: body
required: true
schema:
$ref: '#/definitions/AnotherTestServiceTestEndpointRequestBody'
$ref: '#/definitions/Payload'
responses:
"200":
description: OK response.
schema:
$ref: '#/definitions/AnotherTestServiceTestEndpointResponseBody'
$ref: '#/definitions/Result'
schemes:
- https
definitions:
AnotherTestServiceTestEndpointRequestBody:
title: AnotherTestServiceTestEndpointRequestBody
Payload:
title: Payload
type: object
properties:
string:
type: string
example: ""
example:
string: ""
AnotherTestServiceTestEndpointResponseBody:
title: AnotherTestServiceTestEndpointResponseBody
type: object
properties:
string:
type: string
example: ""
example:
string: ""
TestServiceTestEndpointRequestBody:
title: TestServiceTestEndpointRequestBody
type: object
properties:
string:
type: string
example: ""
example:
string: ""
TestServiceTestEndpointResponseBody:
title: TestServiceTestEndpointResponseBody
Result:
title: Result
type: object
properties:
string:
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"swagger":"2.0","info":{"title":"","version":"0.0.1"},"host":"localhost:80","consumes":["application/json","application/xml","application/gob"],"produces":["application/json","application/xml","application/gob"],"paths":{"/":{"get":{"tags":["testService"],"summary":"testEndpointDefault testService","operationId":"testService#testEndpointDefault","produces":["application/custom+json"],"responses":{"200":{"description":"OK response.","schema":{"$ref":"#/definitions/TestServiceTestEndpointDefaultResponseBody"}}},"schemes":["http"]}},"/tiny":{"get":{"tags":["testService"],"summary":"testEndpointTiny testService","operationId":"testService#testEndpointTiny","responses":{"200":{"description":"OK response.","schema":{"$ref":"#/definitions/TestServiceTestEndpointTinyResponseBodyTiny"}}},"schemes":["http"]}}},"definitions":{"TestServiceTestEndpointDefaultResponseBody":{"title":"Mediatype identifier: application/json; view=default","type":"object","properties":{"int":{"type":"integer","example":1,"format":"int64"},"string":{"type":"string","example":""}},"description":"TestEndpointDefaultResponseBody result type (default view)","example":{"int":1,"string":""}},"TestServiceTestEndpointTinyResponseBodyTiny":{"title":"Mediatype identifier: application/json; view=tiny","type":"object","properties":{"string":{"type":"string","example":""}},"description":"TestEndpointTinyResponseBody result type (tiny view) (default view)","example":{"string":""}}}}
{"swagger":"2.0","info":{"title":"","version":"0.0.1"},"host":"localhost:80","consumes":["application/json","application/xml","application/gob"],"produces":["application/json","application/xml","application/gob"],"paths":{"/":{"get":{"tags":["testService"],"summary":"testEndpointDefault testService","operationId":"testService#testEndpointDefault","produces":["application/custom+json"],"responses":{"200":{"description":"OK response.","schema":{"$ref":"#/definitions/JSON"}}},"schemes":["http"]}},"/tiny":{"get":{"tags":["testService"],"summary":"testEndpointTiny testService","operationId":"testService#testEndpointTiny","responses":{"200":{"description":"OK response.","schema":{"$ref":"#/definitions/TestServiceTestEndpointTinyResponseBodyTiny"}}},"schemes":["http"]}}},"definitions":{"JSON":{"title":"Mediatype identifier: application/json; view=default","type":"object","properties":{"int":{"type":"integer","example":1,"format":"int64"},"string":{"type":"string","example":""}},"description":"TestEndpointDefaultResponseBody result type (default view)","example":{"int":1,"string":""}},"TestServiceTestEndpointTinyResponseBodyTiny":{"title":"Mediatype identifier: application/json; view=tiny","type":"object","properties":{"string":{"type":"string","example":""}},"description":"TestEndpointTinyResponseBody result type (tiny view) (default view)","example":{"string":""}}}}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ paths:
"200":
description: OK response.
schema:
$ref: '#/definitions/TestServiceTestEndpointDefaultResponseBody'
$ref: '#/definitions/JSON'
schemes:
- http
/tiny:
Expand All @@ -41,7 +41,7 @@ paths:
schemes:
- http
definitions:
TestServiceTestEndpointDefaultResponseBody:
JSON:
title: 'Mediatype identifier: application/json; view=default'
type: object
properties:
Expand Down
Loading

0 comments on commit df1919c

Please sign in to comment.