diff --git a/internal/analyzer/analyzer.go b/internal/analyzer/analyzer.go index 371118d..0585fd3 100644 --- a/internal/analyzer/analyzer.go +++ b/internal/analyzer/analyzer.go @@ -26,6 +26,10 @@ type Analyzer struct { // inlineMultipartBodies holds the naming hints of multipart bodies written // inline, which have no schema name to record instead. inlineMultipartBodies map[string]bool + // bodyScope names the body being converted while it is one an operation + // writes inline. Such a body has no name in the spec beyond the operation's, + // so what it synthesizes is kept out of the scope every other body draws on. + bodyScope string // goNameBySchema maps every component schema to its Go type name, filled in // before any conversion so a reference to a schema that has not been converted // yet still resolves to the name it will end up with. diff --git a/internal/analyzer/operations.go b/internal/analyzer/operations.go index 807a9e2..89f4f80 100644 --- a/internal/analyzer/operations.go +++ b/internal/analyzer/operations.go @@ -483,14 +483,31 @@ func (a *Analyzer) convertRequestBody(rb *v3high.RequestBody, nameHint string) ( return nil, nil } + end := a.enterBodyScope(nameHint, rb.GoLow().IsReference()) + goType := a.resolveMediaTypeSchema(mediaType, nameHint) + end() + return &ir.RequestBodyDef{ Required: rb.Required != nil && *rb.Required, Description: rb.Description, ContentType: contentType, - TypeName: bodyGoType(contentType, a.resolveMediaTypeSchema(mediaType, nameHint), mediaTypeSchema(mediaType)), + TypeName: bodyGoType(contentType, goType, mediaTypeSchema(mediaType)), }, nil } +// enterBodyScope scopes the conversion of a body an operation writes inline, +// returning the func that ends that scope. A body the spec declares as a +// component is named by the spec, so it is converted in the shared scope and the +// operations referencing it land on one type. +func (a *Analyzer) enterBodyScope(nameHint string, component bool) func() { + prev := a.bodyScope + a.bodyScope = nameHint + if component { + a.bodyScope = "" + } + return func() { a.bodyScope = prev } +} + // formEncodedContentType reports whether a body is sent as form data, whose // encoders walk the value property by property. func formEncodedContentType(contentType string) bool { @@ -627,6 +644,7 @@ func (a *Analyzer) convertSingleResponse(code string, resp *v3high.Response, nam } if resp.Content != nil { + end := a.enterBodyScope(nameHint, resp.GoLow().IsReference()) for contentType, mediaType := range resp.Content.FromOldest() { if strings.Contains(contentType, "json") { rd.ContentType = contentType @@ -645,6 +663,7 @@ func (a *Analyzer) convertSingleResponse(code string, resp *v3high.Response, nam break } } + end() } return rd @@ -666,7 +685,12 @@ func (a *Analyzer) eventStreamType(resp *v3high.Response, nameHint string) strin if !ok || mediaType == nil { return "" } + // The event payload is scoped to the response, not to its own hint: the same + // schema is also resolved as the response body, and one schema declares one + // type. + end := a.enterBodyScope(nameHint, resp.GoLow().IsReference()) goType := a.resolveMediaTypeSchema(mediaType, nameHint+"Event") + end() if goType == "" { // Events with no schema are still events; their data arrives as text. return "string" diff --git a/internal/analyzer/schemas.go b/internal/analyzer/schemas.go index 20c0f14..a5f06e8 100644 --- a/internal/analyzer/schemas.go +++ b/internal/analyzer/schemas.go @@ -722,8 +722,9 @@ func (a *Analyzer) resolveGoType(schema *highbase.Schema, nameHint string) strin // synthesizeInlineUnion creates a named union TypeDef for an inline // oneOf/anyOf schema so its $ref variants keep their generated types instead // of degrading to any. Identical unions (same variants and discriminator) are -// synthesized once and reuse the first occurrence's name; the resulting types -// are emitted after the component schemas. Returns false when the schema is +// synthesized once and reuse the first occurrence's name, unless they sit in +// bodies different operations write inline; the resulting types are emitted +// after the component schemas. Returns false when the schema is // not a union, has no $ref variants worth naming, or no nameHint is available. func (a *Analyzer) synthesizeInlineUnion(schema *highbase.Schema, nameHint string) (string, bool) { variants := schema.OneOf @@ -773,6 +774,7 @@ func (a *Analyzer) synthesizeInlineUnion(schema *highbase.Schema, nameHint strin if schema.Discriminator != nil { key += "|" + schema.Discriminator.PropertyName } + key = a.synthesisKey(key, schema.Title != "") if existing, ok := a.synthesizedByKey[key]; ok { return existing.Name, true } @@ -789,7 +791,8 @@ func (a *Analyzer) synthesizeInlineUnion(schema *highbase.Schema, nameHint strin // synthesizeInlineObject declares a named struct for an object written inline, so // the properties it lists stay typed instead of collapsing into any. Two -// declarations of one shape share a type. +// declarations of one shape share a type, unless they sit in bodies different +// operations write inline. func (a *Analyzer) synthesizeInlineObject(schema *highbase.Schema, nameHint string) (string, bool) { // Multipart is a property of where the schema is used, so it is looked up // under the hint the request body passed, before a title renames it. @@ -797,6 +800,7 @@ func (a *Analyzer) synthesizeInlineObject(schema *highbase.Schema, nameHint stri // A titled schema names itself, which keeps the generated name stable when // the property that reaches it first is renamed. + selfNamed := true switch { case schema.Title != "": nameHint = schema.Title @@ -805,12 +809,14 @@ func (a *Analyzer) synthesizeInlineObject(schema *highbase.Schema, nameHint stri // so it is named for itself rather than for whichever property in this // document happened to reach it first. nameHint = externalRefName(schema) + default: + selfNamed = false } if nameHint == "" { return "", false } - key := a.inlineObjectKey(schema, nameHint) + key := a.synthesisKey(a.inlineObjectKey(schema, nameHint), selfNamed) if existing, ok := a.synthesizedByKey[key]; ok { return existing.Name, true } @@ -833,14 +839,15 @@ func (a *Analyzer) synthesizeInlineAllOf(schema *highbase.Schema, nameHint strin // properties are file parts, not base64 text. multipartBody := a.inlineMultipartBodies[nameHint] - if schema.Title != "" { + selfNamed := schema.Title != "" + if selfNamed { nameHint = schema.Title } if nameHint == "" { return "", false } - key := a.inlineAllOfKey(schema, nameHint) + key := a.synthesisKey(a.inlineAllOfKey(schema, nameHint), selfNamed) if existing, ok := a.synthesizedByKey[key]; ok { return existing.Name, true } @@ -855,6 +862,18 @@ func (a *Analyzer) synthesizeInlineAllOf(schema *highbase.Schema, nameHint strin return goName, true } +// synthesisKey identifies a synthesized type. A schema inside a body an +// operation writes inline is keyed by that body as well as by its shape, so two +// operations that happen to declare the same body get a type each, named for the +// operation declaring it, rather than sharing one named for whichever was +// converted first. +func (a *Analyzer) synthesisKey(key string, selfNamed bool) string { + if a.bodyScope == "" || selfNamed { + return key + } + return "at:" + a.bodyScope + "|" + key +} + // inlineAllOfKey identifies a composition by what it composes, so the same one // written twice lands on one type. func (a *Analyzer) inlineAllOfKey(schema *highbase.Schema, nameHint string) string { diff --git a/internal/analyzer/schemas_operation_body_test.go b/internal/analyzer/schemas_operation_body_test.go new file mode 100644 index 0000000..7655cd9 --- /dev/null +++ b/internal/analyzer/schemas_operation_body_test.go @@ -0,0 +1,204 @@ +package analyzer + +import "testing" + +const operationBodySpec = `openapi: 3.1.0 +info: { title: t, version: "1" } +paths: + /avatar: + post: + operationId: uploadUserAvatar + requestBody: + content: + multipart/form-data: + schema: + type: object + properties: + file: { type: string, format: binary } + required: [file] + responses: + "200": + description: ok + content: + application/json: + schema: + type: object + properties: + url: { type: string } + /icon: + post: + operationId: uploadWorkflowIcon + requestBody: + content: + multipart/form-data: + schema: + type: object + properties: + file: { type: string, format: binary } + required: [file] + responses: + "200": + description: ok + content: + application/json: + schema: + type: object + properties: + url: { type: string } +` + +func TestOperationBody_InlineBodiesOfOneShapeStayPerOperation(t *testing.T) { + pkg, typeMap := analyzeSpec(t, operationBodySpec) + + want := map[string]struct{ body, response string }{ + "UploadUserAvatar": {"UploadUserAvatarBody", "UploadUserAvatarResponse"}, + "UploadWorkflowIcon": {"UploadWorkflowIconBody", "UploadWorkflowIconResponse"}, + } + for _, op := range pkg.Operations { + w, ok := want[op.Name] + if !ok { + t.Fatalf("unexpected operation %s", op.Name) + } + if op.RequestBody == nil || op.RequestBody.TypeName != w.body { + t.Errorf("%s body = %v, want %s", op.Name, op.RequestBody, w.body) + } + if got := op.Responses[0].TypeName; got != w.response { + t.Errorf("%s response = %q, want %q", op.Name, got, w.response) + } + if typeMap[w.body] == nil { + t.Errorf("%s not generated", w.body) + } + if typeMap[w.response] == nil { + t.Errorf("%s not generated", w.response) + } + } +} + +const sharedComponentBodySpec = `openapi: 3.1.0 +info: { title: t, version: "1" } +paths: + /a: + post: + operationId: createA + requestBody: + $ref: '#/components/requestBodies/Upload' + responses: + "400": { $ref: '#/components/responses/Failure' } + /b: + post: + operationId: createB + requestBody: + $ref: '#/components/requestBodies/Upload' + responses: + "400": { $ref: '#/components/responses/Failure' } +components: + requestBodies: + Upload: + content: + application/json: + schema: + type: object + properties: + file: { type: string } + responses: + Failure: + description: nope + content: + application/json: + schema: + type: object + properties: + message: { type: string } +` + +func TestOperationBody_ComponentBodyStaysShared(t *testing.T) { + pkg, _ := analyzeSpec(t, sharedComponentBodySpec) + + var bodies, responses []string + for _, op := range pkg.Operations { + bodies = append(bodies, op.RequestBody.TypeName) + responses = append(responses, op.Responses[0].TypeName) + } + if len(bodies) != 2 || bodies[0] != bodies[1] { + t.Errorf("component request body types = %v, want both operations on one type", bodies) + } + if len(responses) != 2 || responses[0] != responses[1] { + t.Errorf("component response types = %v, want both operations on one type", responses) + } +} + +const titledBodySpec = `openapi: 3.1.0 +info: { title: t, version: "1" } +paths: + /a: + post: + operationId: createA + requestBody: + content: + application/json: + schema: + type: object + title: Upload + properties: + file: { type: string } + responses: + "204": { description: ok } + /b: + post: + operationId: createB + requestBody: + content: + application/json: + schema: + type: object + title: Upload + properties: + file: { type: string } + responses: + "204": { description: ok } +` + +func TestOperationBody_TitledBodyStaysShared(t *testing.T) { + pkg, _ := analyzeSpec(t, titledBodySpec) + + for _, op := range pkg.Operations { + if got := op.RequestBody.TypeName; got != "Upload" { + t.Errorf("%s body = %q, want Upload", op.Name, got) + } + } +} + +const inlineEventStreamSpec = `openapi: 3.1.0 +info: { title: t, version: "1" } +paths: + /events: + get: + operationId: streamEvents + responses: + "200": + description: ok + content: + text/event-stream: + schema: + type: object + properties: + id: { type: string } +` + +// An event payload is resolved twice, once as the response body and once as the +// event, so scoping it to the response keeps the two on one declared type. +func TestOperationBody_EventPayloadDeclaresOneType(t *testing.T) { + pkg, _ := analyzeSpec(t, inlineEventStreamSpec) + + op := pkg.Operations[0] + if op.EventType != op.Responses[0].TypeName { + t.Errorf("event type %q, response type %q, want one type", op.EventType, op.Responses[0].TypeName) + } + var synthesized []string + for _, td := range pkg.Types { + synthesized = append(synthesized, td.Name) + } + if len(synthesized) != 1 { + t.Errorf("generated types = %v, want one", synthesized) + } +} diff --git a/internal/analyzer/webhooks.go b/internal/analyzer/webhooks.go index 1c5763d..594fecc 100644 --- a/internal/analyzer/webhooks.go +++ b/internal/analyzer/webhooks.go @@ -100,7 +100,9 @@ func (a *Analyzer) inboundPayloadType(op *v3high.Operation, nameHint string) (st if !strings.Contains(contentType, "json") || mediaType == nil { return "", false } + end := a.enterBodyScope(nameHint+"Payload", op.RequestBody.GoLow().IsReference()) goType := a.resolveMediaTypeSchema(mediaType, nameHint+"Payload") + end() if goType == "" || goType == "any" { return "", false } diff --git a/internal/generator/e2e_operation_body_naming_test.go b/internal/generator/e2e_operation_body_naming_test.go new file mode 100644 index 0000000..aaec9d6 --- /dev/null +++ b/internal/generator/e2e_operation_body_naming_test.go @@ -0,0 +1,97 @@ +package generator + +import "testing" + +const repeatedInlineBodySpec = `openapi: 3.1.0 +info: { title: uploads, version: "1" } +paths: + /avatar: + post: + operationId: uploadUserAvatar + requestBody: + required: true + content: + multipart/form-data: + schema: + type: object + properties: + file: { type: string, format: binary } + required: [file] + responses: + "200": + description: ok + content: + application/json: + schema: + type: object + properties: + url: { type: string } + /icon: + post: + operationId: uploadWorkflowIcon + requestBody: + required: true + content: + multipart/form-data: + schema: + type: object + properties: + file: { type: string, format: binary } + required: [file] + responses: + "200": + description: ok + content: + application/json: + schema: + type: object + properties: + url: { type: string } +` + +// TestE2E_RepeatedInlineBodyNaming covers two operations whose inline bodies +// happen to share a shape. They were collapsed onto one type, so uploading a +// workflow icon meant constructing a type named for the avatar endpoint. +func TestE2E_RepeatedInlineBodyNaming(t *testing.T) { + files, _ := generateFromSpec(t, repeatedInlineBodySpec, "uploadsapi") + + runGeneratedWireTest(t, files, "repeatedinlinebody", `package uploadsapi + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +func TestEachOperationHasItsOwnBodyAndResponse(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`+"`"+`{"url":"`+"`"+` + r.URL.Path + `+"`"+`"}`+"`"+`)) + })) + defer srv.Close() + client := NewClient(srv.URL) + + avatar, err := client.UploadUserAvatar(t.Context(), UploadUserAvatarBody{ + File: FormFile{Filename: "a.png", Content: []byte("a")}, + }) + if err != nil { + t.Fatalf("UploadUserAvatar: %v", err) + } + var _ *UploadUserAvatarResponse = avatar + if avatar.URL == nil || *avatar.URL != "/avatar" { + t.Errorf("avatar url = %v", avatar.URL) + } + + icon, err := client.UploadWorkflowIcon(t.Context(), UploadWorkflowIconBody{ + File: FormFile{Filename: "b.png", Content: []byte("b")}, + }) + if err != nil { + t.Fatalf("UploadWorkflowIcon: %v", err) + } + var _ *UploadWorkflowIconResponse = icon + if icon.URL == nil || *icon.URL != "/icon" { + t.Errorf("icon url = %v", icon.URL) + } +} +`) +}