From 0b184928df869fc8954590021cc709c556a75ea1 Mon Sep 17 00:00:00 2001 From: Michael McQuade Date: Fri, 21 Aug 2026 17:11:55 -0500 Subject: [PATCH] fix(analyzer): inline bodies of the same shape took a type name from another operation Synthesized types were keyed only on shape, so two operations that each wrote the same body inline landed on one type, named for whichever was converted first. Uploading a workflow icon meant constructing an UploadStandaloneAiChatAttachmentBody. The same collapse hit inline response bodies, where one operation returned a type named for another endpoint. A body an operation writes inline now scopes what it synthesizes, so each operation gets a type named for itself, nested inline objects included. Bodies the spec names stay shared: a $ref to components/requestBodies or components/responses, a titled schema, an external-file $ref. An event stream resolves one schema twice, as the response body and as the event payload, so the scope is the body rather than each name hint and the two stay on one declared type. Regenerating testdata is byte identical, since those specs name their bodies as components. Closes #125 --- internal/analyzer/analyzer.go | 4 + internal/analyzer/operations.go | 26 ++- internal/analyzer/schemas.go | 31 ++- .../analyzer/schemas_operation_body_test.go | 204 ++++++++++++++++++ internal/analyzer/webhooks.go | 2 + .../e2e_operation_body_naming_test.go | 97 +++++++++ 6 files changed, 357 insertions(+), 7 deletions(-) create mode 100644 internal/analyzer/schemas_operation_body_test.go create mode 100644 internal/generator/e2e_operation_body_naming_test.go 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) + } +} +`) +}