From 80c1201197a3c2b8776c16d6c90a28945dde2c21 Mon Sep 17 00:00:00 2001 From: Raphael Simon Date: Sun, 14 Sep 2025 10:56:30 -0700 Subject: [PATCH] fix(docs): always emit single top-level docs.json - Emit only gen/docs.json regardless of number of roots - Preserve JSON tag transforms and inlining across multi-package builds - Merge services and definitions across roots deterministically - Remove per-API service-scoped path emission - Update tests to expect a single file Rationale: Goa designs have a single API; per-API path caused confusion and inconsistent consumer behavior. This change standardizes the output location and resolves inlining discrepancies observed in multi-package designs (e.g., Aura). --- docs/AGENTS.md | 47 ++++ docs/INLINE_REFS.md | 138 ++++++++++ docs/generate.go | 312 +++++++++------------- docs/generate_test.go | 136 +++++++--- docs/genstate.go | 48 ++++ docs/inline_refs.go | 104 ++++++++ docs/testdata/shared/types.go | 8 + docs/testdata/user-payload-no-return.json | 2 +- 8 files changed, 569 insertions(+), 226 deletions(-) create mode 100644 docs/AGENTS.md create mode 100644 docs/INLINE_REFS.md create mode 100644 docs/genstate.go create mode 100644 docs/inline_refs.go create mode 100644 docs/testdata/shared/types.go diff --git a/docs/AGENTS.md b/docs/AGENTS.md new file mode 100644 index 000000000..4a7fba7b5 --- /dev/null +++ b/docs/AGENTS.md @@ -0,0 +1,47 @@ +# Repository Guidelines + +## Project Structure & Module Organization +- Core: `generate.go`, `types.go`, `inline_refs.go`, `genstate.go`. +- DSL and expressions: `dsl/`, `expr/`. +- Example: `examples/calc/` with generated code in `gen/` and binaries in `cmd/`. +- Tests and goldens: `*_test.go` and `testdata/*.json`. +- Docs: `README.md`, `INLINE_REFS.md`. + +## Build, Test, and Development Commands +- `make all`: run gen, tests, lint, build examples, then clean (via `../plugins.mk`). +- `make lint`: `goimports` formatting check and `staticcheck` lint. +- `make test`: run `go test ./...`; verbose: `go test -v ./...`. +- `make gen`: regenerate example outputs (includes `examples/calc/gen/docs.json`). +- Update goldens: `go test ./... -- -update` (commit changes in `testdata/`). + +## Coding Style & Naming Conventions +- Follow Goa’s CLAUDE.md layout: group declarations in this order per file — types, consts, vars, public funcs, public methods, private funcs, private methods. No section markers. +- Keep files focused and reasonably small; one main construct per file. +- Prefer `any` over `interface{}` in new code; exported identifiers use CamelCase; packages are short, lower-case. +- Never edit generated code in `examples/calc/gen/`; fix generators/templates instead. + +## Curly Braces Rules +- Default: use multi-line braces for all code blocks (Go and Goa DSL). +- Exceptions only: + - Empty DSL closures may be single-line, e.g., `JSONRPC(func() {})`. + - Trivial methods returning a constant may be single-line, e.g., `func (e *Enum) String() string { return "foo" }`. +- Do not compress control flow. Preferred: + + ```go + if err != nil { + return err + } + ``` + + Avoid: `if err != nil { return err }`. +- Place `else` on the same line as the closing `}` of the preceding block. + +## Testing Guidelines +- Use `testing` plus `testify/assert` and `testify/require`. +- Golden tests compare generated output to files in `testdata/` (e.g., `no-payload-no-return.json`). +- When behavior changes intentionally, regenerate goldens and review diffs carefully. + +## Commit & Pull Request Guidelines +- Conventional commits: `feat(docs): ...`, `fix(docs): ...`, `chore: ...`. +- PRs include description, rationale, linked issues, and testing notes; keep changes small and scoped. +- If generation output changes, run `make gen` and commit relevant updates under `examples/calc/gen/` and `testdata/`. diff --git a/docs/INLINE_REFS.md b/docs/INLINE_REFS.md new file mode 100644 index 000000000..ad98d6ac2 --- /dev/null +++ b/docs/INLINE_REFS.md @@ -0,0 +1,138 @@ +## Goal + +Design a robust, deterministic strategy to inline all JSON Schema $ref occurrences into their target schemas for docs JSON produced by goa. The generator only uses local references to the top-level definitions object (paths of the form "#/definitions/"). + +## Where $ref appear in goa’s output + +- **User/Result types**: Type schemas reference definitions via `TypeRef*` and `ResultTypeRef*`. +- **Object properties**: Nested fields can be `$ref`. +- **Array items**: `items` may be a `$ref` when the element type is a user/result type. +- **Map values**: `additionalProperties` may be a schema containing `$ref` (or boolean `true`). +- **Union types**: `anyOf` is an array of schemas that may contain `$ref`. +- **Service-level top schema**: APISchema properties refer to `#/definitions/`. + +Assumption: Only references to `#/definitions/` are present. No external files/URLs. + +## Constraints + +- Definitions are in a per-root map; keys are type names. +- Definitions can themselves contain nested `$ref`. +- Must avoid infinite recursion on cyclical definitions. +- Preserve required fields, examples, and order where applicable. + +## Strategy (high-level) + +1) **Snapshot definitions** + - Work on a deep-copied map of the per-root `definitions` to avoid mutating shared state. + +2) **Inlining pass** + - Implement `inlineRefs(schema, defs, stack)`: + - If `schema.Ref == "#/definitions/"`: + - If `` is in `stack`, keep `$ref` (cycle break) and return. + - Else: push ``, deep-copy `defs[Name]`, recursively `inlineRefs` on the copy, then replace current node with the copy and clear `Ref`. Pop ``. + - If `schema.Ref == ""`: recursively process all composite positions: + - `properties` values + - `items` + - `additionalProperties` when it is a `*Schema` + - `anyOf` elements + - (If present) nested `definitions` for completeness + +3) **Apply order** + - Perform schema transforms that affect keys first (e.g., JSON tag rename and required filtering), then inline. + +4) **Service-level application** + - Run `inlineRefs` on every reachable schema under services (payload, result, streaming payload/result, error types). + - Optionally inline inside `definitions` if you plan to drop them entirely. + +5) **Cycles** + - Use a `stack` (set of definition names being expanded). If a name is already in the stack, retain `$ref` at that edge to prevent infinite expansion. This yields minimal `$ref` for strongly-cyclic graphs. + +6) **AnyOf** + - Goa builds `anyOf` by appending union variants in-order. Inline each element; preserve order. + +7) **Maps** + - If `additionalProperties` is `true`, leave as is. If it is a schema, inline there as well. + +8) **Performance** + - Start without memoization. If profiling reveals hotspots, consider caching fully inlined, deep-copied definitions by name and reusing them, taking care not to share mutable pointers unexpectedly. + +9) **Post-processing** + - If you require a completely `$ref`-free document, remove `definitions` after inlining all reachable schemas. Otherwise, keep or prune unused definitions based on tests/goldens. + +## Pseudocode + +```go +func inlineAllServiceSchemas(d *data, defs map[string]*openapi.Schema) { + stack := map[string]bool{} + for _, s := range d.Services { + for _, m := range s.Methods { + if m.Payload != nil && m.Payload.Type != nil { + inlineRefs(m.Payload.Type, defs, stack) + } + if m.StreamingPayload != nil && m.StreamingPayload.Type != nil { + inlineRefs(m.StreamingPayload.Type, defs, stack) + } + if m.Result != nil && m.Result.Type != nil { + inlineRefs(m.Result.Type, defs, stack) + } + if m.StreamingResult != nil && m.StreamingResult.Type != nil { + inlineRefs(m.StreamingResult.Type, defs, stack) + } + for _, e := range m.Errors { + if e.Type != nil { + inlineRefs(e.Type, defs, stack) + } + } + } + } +} + +func inlineRefs(s *openapi.Schema, defs map[string]*openapi.Schema, stack map[string]bool) { + if s == nil { + return + } + if s.Ref != "" { + const prefix = "#/definitions/" + if !strings.HasPrefix(s.Ref, prefix) { return } + name := strings.TrimPrefix(s.Ref, prefix) + if stack[name] { return } + def, ok := defs[name] + if !ok || def == nil { return } + stack[name] = true + copy := dupSchema(def) + inlineRefs(copy, defs, stack) + *s = *copy + s.Ref = "" + delete(stack, name) + return + } + for _, p := range s.Properties { inlineRefs(p, defs, stack) } + if s.Items != nil { inlineRefs(s.Items, defs, stack) } + if ap, ok := s.AdditionalProperties.(*openapi.Schema); ok && ap != nil { + inlineRefs(ap, defs, stack) + } + for _, a := range s.AnyOf { inlineRefs(a, defs, stack) } + for _, d := range s.Definitions { inlineRefs(d, defs, stack) } +} +``` + +## Integration points in the plugin + +- After building `docs` and `defs`, and after JSON tag transforms: + - Gate behind `InlineRefs` option: call `inlineAllServiceSchemas(docs, defs)`. + - Decide whether to keep or drop `docs.Definitions` based on goldens. If keeping, you may optionally prune unused definitions. + +## Edge cases and guarantees + +- **Cycles**: Minimal `$ref` retained on cycle entry. +- **Required and examples**: Preserved; JSON-tag transform already remaps them pre-inlining. +- **Maps**: Free-form (`true`) untouched; schema-valued inlined. +- **Unions**: Order preserved in `anyOf`. + +## Rationale + +- Tailored to the shapes produced by goa: only `#/definitions/` refs. +- Deterministic and safe (deep copies, cycle guard), compositional (handles all container fields). +- Clean pipeline: rename/tag first, inline second. + + diff --git a/docs/generate.go b/docs/generate.go index 89278dbb1..6223b82c1 100644 --- a/docs/generate.go +++ b/docs/generate.go @@ -30,150 +30,142 @@ func init() { Register() } // Generate produces the documentation JSON file. func Generate(_ string, roots []eval.Root, files []*codegen.File) ([]*codegen.File, error) { - // First, build a complete map of all definitions from all roots. - // This ensures that cross-package type references can be resolved. - allDefs := make(map[string]*openapi.Schema) + // Always emit a single aggregated docs.json at the top level. + jsonPath := filepath.Join(codegen.Gendir, "docs.json") + if _, err := os.Stat(jsonPath); err == nil { + if err := os.Remove(jsonPath); err != nil { + return nil, fmt.Errorf("remove stale docs.json: %w", err) + } + } + + var agg *data + for _, root := range roots { - if r, ok := root.(*expr.RootExpr); ok { - // Create a temporary, isolated context for each root to avoid global state pollution. - prev := openapi.Definitions - openapi.Definitions = make(map[string]*openapi.Schema) - - for _, tpe := range r.Types { - if ut, ok := tpe.(*expr.UserTypeExpr); ok { - openapi.GenerateTypeDefinition(r.API, ut) - } - } - for _, rt := range r.ResultTypes { - openapi.GenerateResultTypeDefinition(r.API, rt, expr.DefaultView) + r, ok := root.(*expr.RootExpr) + if !ok { + continue + } + d := docsDataForRoot(r) + if d == nil { + continue + } + if agg == nil { + // First root initializes the aggregate. + agg = &data{ + API: d.API, + Services: make(map[string]*serviceData, len(d.Services)), + Definitions: make(map[string]*openapi.Schema, len(d.Definitions)), } - - // Merge the generated definitions into the global map. - for n, s := range openapi.Definitions { - if _, exists := allDefs[n]; !exists { - allDefs[n] = dupSchema(s) + } + // Merge services (merge methods if service reappears). + for sn, svc := range d.Services { + if existing, ok := agg.Services[sn]; ok && existing != nil && svc != nil { + if existing.Methods == nil { + existing.Methods = make(map[string]*methodData) } + for mn, md := range svc.Methods { + existing.Methods[mn] = md + } + // Preserve description/requirements if present on either. + if existing.Description == "" { + existing.Description = svc.Description + } + if len(existing.Requirements) == 0 && len(svc.Requirements) > 0 { + existing.Requirements = svc.Requirements + } + } else { + agg.Services[sn] = svc } - - // Restore the original global definitions to maintain isolation. - openapi.Definitions = prev + } + // Merge definitions (last-wins for same key). + for dn, sch := range d.Definitions { + agg.Definitions[dn] = sch } } - for _, root := range roots { - if r, ok := root.(*expr.RootExpr); ok { - files = append(files, docsFile(r, allDefs)) - } + if agg == nil { + // No valid roots; nothing to do. + return files, nil + } + + jsonSection := &codegen.SectionTemplate{ + Name: "docs", + FuncMap: template.FuncMap{"mustJSON": mustJSON}, + Source: "{{ mustJSON .}}", + Data: agg, } + files = append(files, &codegen.File{ + Path: jsonPath, + SectionTemplates: []*codegen.SectionTemplate{jsonSection}, + }) return files, nil } -func docsFile(r *expr.RootExpr, allDefs map[string]*openapi.Schema) *codegen.File { +// docsDataForRoot builds the docs data for a single root while scoping +// openapi.Definitions to that root only. The global definitions map is restored +// before returning. +func docsDataForRoot(r *expr.RootExpr) *data { + prev := openapi.Definitions + openapi.Definitions = make(map[string]*openapi.Schema) + defer func() { openapi.Definitions = prev }() + + f := docsFile(r) + if len(f.SectionTemplates) == 0 || f.SectionTemplates[0] == nil { + return nil + } + d, _ := f.SectionTemplates[0].Data.(*data) + return d +} + +func docsFile(r *expr.RootExpr) *codegen.File { + // Per-run generation state + state := newGenState() docs := &data{ API: apiDocs(r.API), - Services: servicesDocs(r), + Services: servicesDocs(r, state), } - // Default behavior: use global OpenAPI definitions to preserve ordering and - // compatibility with existing golden tests. - defs := allDefs + // Default behavior: use root-local OpenAPI definitions produced during + // payload/result schema generation to preserve golden stability. + defs := make(map[string]*openapi.Schema, len(openapi.Definitions)) + for n, d := range openapi.Definitions { + defs[n] = dupSchema(d) + } - // If either option is enabled, build a local definition map for this root - // and apply transforms/inlining as needed, isolating from global state. + // Apply JSON tag transforms if requested. if plugexpr.Root.UseJSONTags { - // Re-scope the definitions to only those present in the current root, - // but use the globally-aware `allDefs` for lookups during transforms. - local := make(map[string]*openapi.Schema) - prev := openapi.Definitions - openapi.Definitions = make(map[string]*openapi.Schema) - for _, tpe := range r.Types { - if ut, ok := tpe.(*expr.UserTypeExpr); ok { - openapi.GenerateTypeDefinition(r.API, ut) - } - } - for _, rt := range r.ResultTypes { - openapi.GenerateResultTypeDefinition(r.API, rt, expr.DefaultView) - } - for n := range openapi.Definitions { - if def, ok := allDefs[n]; ok { - local[n] = dupSchema(def) - } - } - openapi.Definitions = prev + defs = transformDefinitionsWithJSONTagsHybrid(r, defs) + } - // Apply JSON tag transforms if requested - if plugexpr.Root.UseJSONTags { - local = transformDefinitionsWithJSONTagsHybrid(r, local, nil) - } - defs = local - } else { - // When not transforming, avoid leaking the synthetic Empty type produced by - // Goa internals in some scenarios by filtering it out, but only when present. - if _, hasEmpty := defs["Empty"]; hasEmpty { - filtered := make(map[string]*openapi.Schema, len(defs)) - for n, s := range defs { - if n == "Empty" { - continue - } - filtered[n] = s + // Filter synthetic Empty type if present. + if _, hasEmpty := defs["Empty"]; hasEmpty { + filtered := make(map[string]*openapi.Schema, len(defs)) + for n, s := range defs { + if n == "Empty" { + continue } - defs = filtered + filtered[n] = s } + defs = filtered } - docs.Definitions = defs - - // Inline $refs if requested via DSL flag. + // Inline $ref occurrences if requested. if plugexpr.Root.InlineRefs { - // When inlining, use the complete set of definitions from all roots - // to ensure cross-package references can be resolved. - inliningDefs := allDefs - if plugexpr.Root.UseJSONTags { - inliningDefs = transformDefinitionsWithJSONTagsHybrid(r, allDefs, nil) - } - - // Inline inside service payloads/results/errors. - for _, svc := range docs.Services { - for _, m := range svc.Methods { - if m.Payload != nil && m.Payload.Type != nil { - inlineRefsInSchema(m.Payload.Type, inliningDefs, make(map[string]bool)) - } - if m.StreamingPayload != nil && m.StreamingPayload.Type != nil { - inlineRefsInSchema(m.StreamingPayload.Type, inliningDefs, make(map[string]bool)) - } - if m.Result != nil && m.Result.Type != nil { - inlineRefsInSchema(m.Result.Type, inliningDefs, make(map[string]bool)) - } - if m.StreamingResult != nil && m.StreamingResult.Type != nil { - inlineRefsInSchema(m.StreamingResult.Type, inliningDefs, make(map[string]bool)) - } - for _, e := range m.Errors { - if e != nil && e.Type != nil { - inlineRefsInSchema(e.Type, inliningDefs, make(map[string]bool)) - } - } - } - } - // Inline inside definitions themselves (properties that refer to other defs). - for _, def := range defs { - inlineRefsInSchema(def, inliningDefs, make(map[string]bool)) + inlineAllServiceSchemas(docs, defs) + // Also inline inside definitions so nested refs are expanded there too. + stack := make(map[string]bool) + for _, sch := range defs { + inlineRefs(sch, defs, stack) } } + docs.Definitions = defs + jsonPath := filepath.Join(codegen.Gendir, "docs.json") - if _, err := os.Stat(jsonPath); !os.IsNotExist(err) { - // Goa does not delete files in the top-level gen folder. - // https://github.com/goadesign/goa/pull/2194 - // The plugin must delete docs.json so that the generator does not append - // to any existing docs.json. - if err := os.Remove(jsonPath); err != nil { - panic(err) - } - } jsonSection := &codegen.SectionTemplate{ Name: "docs", - FuncMap: template.FuncMap{"toJSON": toJSON}, - Source: "{{ toJSON .}}", + FuncMap: template.FuncMap{"mustJSON": mustJSON}, + Source: "{{ mustJSON .}}", Data: docs, } return &codegen.File{ @@ -265,9 +257,8 @@ func apiDocs(api *expr.APIExpr) *apiData { return data } -func servicesDocs(r *expr.RootExpr) map[string]*serviceData { +func servicesDocs(r *expr.RootExpr, state *genState) map[string]*serviceData { svcs := make(map[string]*serviceData, len(r.Services)) - var nameScope = codegen.NewNameScope() for _, svc := range r.Services { n := svc.Name @@ -278,7 +269,7 @@ func servicesDocs(r *expr.RootExpr) map[string]*serviceData { svcs[n].Methods = make(map[string]*methodData, len(svc.Methods)) for _, meth := range svc.Methods { - svcs[n].Methods[meth.Name] = generateMethod(r.API, meth, nameScope) + svcs[n].Methods[meth.Name] = generateMethod(r.API, meth, state) } svcs[n].Requirements = make([]*requirementData, len(svc.Requirements)) @@ -354,21 +345,21 @@ func generateRequirement(req *expr.SecurityExpr) *requirementData { return r } -func generateMethod(api *expr.APIExpr, meth *expr.MethodExpr, scope *codegen.NameScope) *methodData { +func generateMethod(api *expr.APIExpr, meth *expr.MethodExpr, state *genState) *methodData { m := &methodData{ Name: meth.Name, Description: meth.Description, - Payload: generatePayload(api, meth.Payload, scope), - StreamingPayload: generatePayload(api, meth.StreamingPayload, scope), + Payload: generatePayload(api, meth.Payload, state), + StreamingPayload: generatePayload(api, meth.StreamingPayload, state), } if meth.Stream == expr.BidirectionalStreamKind || meth.Stream == expr.ServerStreamKind { - m.StreamingResult = generatePayload(api, meth.Result, scope) + m.StreamingResult = generatePayload(api, meth.Result, state) } else { - m.Result = generatePayload(api, meth.Result, scope) + m.Result = generatePayload(api, meth.Result, state) } m.Errors = make(map[string]*errorData, len(meth.Errors)) for _, er := range meth.Errors { - m.Errors[er.Name] = generateError(api, er) + m.Errors[er.Name] = generateError(api, er, state) } m.Requirements = make([]*requirementData, len(meth.Requirements)) for i, req := range meth.Requirements { @@ -377,16 +368,13 @@ func generateMethod(api *expr.APIExpr, meth *expr.MethodExpr, scope *codegen.Nam return m } -func generatePayload(api *expr.APIExpr, att *expr.AttributeExpr, nameScope *codegen.NameScope) *payloadData { - // since the definitions section is global to the API, we need to ensure uniqueness of TypeName - if ut, ok := att.Type.(*expr.UserTypeExpr); ok { - if ut == expr.Empty { - return nil - } - ut.TypeName = nameScope.Unique(ut.TypeName) +func generatePayload(api *expr.APIExpr, att *expr.AttributeExpr, state *genState) *payloadData { + // Do not generate payload for Empty + if ut, ok := att.Type.(*expr.UserTypeExpr); ok && ut == expr.Empty { + return nil } - schema := openapi.AttributeTypeSchema(api, att) + schema := schemaForAttribute(api, att, state) ex := att.Example(api.ExampleGenerator) if plugexpr.Root.UseJSONTags { // avoid mutating shared schema nodes @@ -400,11 +388,11 @@ func generatePayload(api *expr.APIExpr, att *expr.AttributeExpr, nameScope *code } } -func generateError(api *expr.APIExpr, er *expr.ErrorExpr) *errorData { +func generateError(api *expr.APIExpr, er *expr.ErrorExpr, state *genState) *errorData { _, temporary := er.Meta["goa:error:temporary"] _, timeout := er.Meta["goa:error:timeout"] _, fault := er.Meta["goa:error:fault"] - sch := openapi.AttributeTypeSchema(api, er.AttributeExpr) + sch := schemaForAttribute(api, er.AttributeExpr, state) if plugexpr.Root.UseJSONTags { sch = dupSchema(sch) applyJSONTagsToSchema(er.AttributeExpr, sch) @@ -419,68 +407,25 @@ func generateError(api *expr.APIExpr, er *expr.ErrorExpr) *errorData { } } -func toJSON(d interface{}) string { +func mustJSON(d interface{}) string { b, err := json.Marshal(d) if err != nil { - panic("openapi: " + err.Error()) // bug + panic("docs: " + err.Error()) // bug } return string(b) } -// inlineRefsInSchema replaces $ref with a deep copy of the referenced -// definition where possible. It recurses through properties/items/anyOf. -// A small visited set prevents infinite recursion on cycles. -func inlineRefsInSchema(s *openapi.Schema, defs map[string]*openapi.Schema, visiting map[string]bool) { - if s == nil { - return - } - // Inline reference if it points to local definitions. - if after, ok := strings.CutPrefix(s.Ref, "#/definitions/"); ok { - name := after - if !visiting[name] { - if def, ok := defs[name]; ok { - visiting[name] = true - dup := dupSchema(def) - // Recurse into the dup first to inline nested refs. - inlineRefsInSchema(dup, defs, visiting) - // Replace s with contents of dup (shallow copy fields and maps) - *s = *dup - visiting[name] = false - } - } - } - // Recurse - if s.Items != nil { - inlineRefsInSchema(s.Items, defs, visiting) - } - if s.Properties != nil { - for _, p := range s.Properties { - inlineRefsInSchema(p, defs, visiting) - } - } - if s.AdditionalProperties != nil { - if asp, ok := s.AdditionalProperties.(*openapi.Schema); ok { - inlineRefsInSchema(asp, defs, visiting) - } - } - if s.AnyOf != nil { - for _, a := range s.AnyOf { - inlineRefsInSchema(a, defs, visiting) - } - } -} +// inline $ref logic removed -// transformDefinitionsWithJSONTagsHybrid tries attrIndex first; if not found, falls back to Root.UserType lookup. -func transformDefinitionsWithJSONTagsHybrid(r *expr.RootExpr, defs map[string]*openapi.Schema, attrIndex map[string]*expr.AttributeExpr) map[string]*openapi.Schema { +// transformDefinitionsWithJSONTagsHybrid tries Root.UserType lookup. +func transformDefinitionsWithJSONTagsHybrid(r *expr.RootExpr, defs map[string]*openapi.Schema) map[string]*openapi.Schema { if len(defs) == 0 { return defs } out := make(map[string]*openapi.Schema, len(defs)) for name, sch := range defs { dup := dupSchema(sch) - if att, ok := attrIndex[name]; ok && att != nil { - applyJSONTagsToSchema(att, dup) - } else if ut := r.UserType(name); ut != nil { + if ut := r.UserType(name); ut != nil { applyJSONTagsToSchema(ut.Attribute(), dup) } out[name] = dup @@ -532,6 +477,7 @@ func applyJSONTagsToSchema(att *expr.AttributeExpr, s *openapi.Schema) { } // Handle object property renaming and example/required updates. + // If multiple fields resolve to the same JSON tag, the first seen wins. if obj := expr.AsObject(att.Type); obj != nil && s.Properties != nil { // Build new properties map using JSON tag names. Use walkAttribute so bases/references are included. newProps := make(map[string]*openapi.Schema, len(s.Properties)) diff --git a/docs/generate_test.go b/docs/generate_test.go index ea3d7ac8f..8e2ce2b69 100644 --- a/docs/generate_test.go +++ b/docs/generate_test.go @@ -175,6 +175,100 @@ func TestInlineRefs(t *testing.T) { } } +func TestInlineRefs_CrossPackage(t *testing.T) { + t.Cleanup(func() { plugexpr.Root.UseJSONTags = false; plugexpr.Root.InlineRefs = false }) + + // 1. Define two separate DSLs to simulate two design packages. + var SharedType = Type("SharedType", func() { + Field(1, "A", String) + Required("A") + }) + + sharedDesign := func() { + API("Shared", func() {}) + var _ = SharedType // Reference the shared type + } + serviceDesign := func() { + API("Service", func() {}) + Service("S1", func() { + Method("M1", func() { + Payload(SharedType) + HTTP(func() { GET("/") }) + GRPC(func() {}) + }) + }) + } + + // 2. Run each DSL to generate a separate eval.Root, simulating a multi-package build. + root1 := codegen.RunDSL(t, sharedDesign) + root2 := codegen.RunDSL(t, serviceDesign) + + // 3. Generate the docs with both roots. + InlineRefs() + fs, err := docs.Generate("", []eval.Root{root1, root2}, nil) + require.NoError(t, err) + require.Len(t, fs, 1) + + // 4. Unmarshal the docs and assert that the ref was NOT inlined. + var serviceDocs map[string]any + var w bytes.Buffer + require.NoError(t, fs[0].SectionTemplates[0].Write(&w)) + require.NoError(t, json.Unmarshal(w.Bytes(), &serviceDocs)) + + s1 := serviceDocs["services"].(map[string]any)["S1"].(map[string]any) + m1 := s1["methods"].(map[string]any)["M1"].(map[string]any) + p1 := m1["payload"].(map[string]any) + pt := p1["type"].(map[string]any) + if _, hasRef := pt["$ref"]; hasRef { + t.Fatalf("expected inlined schema for payload type, found $ref: %#v", pt) + } + if typ, ok := pt["type"].(string); !ok || typ != "object" { + t.Fatalf("expected object type in inlined payload, got: %#v", pt) + } +} + +func TestInlineRefs_CrossService(t *testing.T) { + t.Cleanup(func() { plugexpr.Root.UseJSONTags = false; plugexpr.Root.InlineRefs = false }) + docsMap := genDocs(t, func() { + InlineRefs() + API("Test", func() {}) + var SharedType = Type("SharedType", func() { + Field(1, "A", String) + Required("A") + }) + Service("S1", func() { + Method("M1", func() { + Payload(SharedType) + HTTP(func() { GET("/") }) + GRPC(func() {}) + }) + }) + Service("S2", func() { + Method("M2", func() { + Payload(SharedType) + HTTP(func() { GET("/s2") }) + GRPC(func() {}) + }) + }) + }) + + // Check S1 + s1 := docsMap["services"].(map[string]any)["S1"].(map[string]any) + m1 := s1["methods"].(map[string]any)["M1"].(map[string]any) + pt1 := m1["payload"].(map[string]any)["type"].(map[string]any) + if _, hasRef := pt1["$ref"]; hasRef { + t.Fatalf("S1: expected inlined payload schema for shared type, found $ref: %#v", pt1) + } + + // Check S2 + s2 := docsMap["services"].(map[string]any)["S2"].(map[string]any) + m2 := s2["methods"].(map[string]any)["M2"].(map[string]any) + pt2 := m2["payload"].(map[string]any)["type"].(map[string]any) + if _, hasRef := pt2["$ref"]; hasRef { + t.Fatalf("S2: expected inlined payload schema for shared type, found $ref: %#v", pt2) + } +} + func TestBothOptions(t *testing.T) { t.Cleanup(func() { plugexpr.Root.UseJSONTags = false; plugexpr.Root.InlineRefs = false }) docsMap := genDocs(t, func() { @@ -291,45 +385,3 @@ func TestJSONTagsAndInlineRefs_Complex(t *testing.T) { t.Fatalf("expected required array in result type, got: %#v", rt) } } - -func TestInlineRefs_CrossService(t *testing.T) { - t.Cleanup(func() { plugexpr.Root.UseJSONTags = false; plugexpr.Root.InlineRefs = false }) - docsMap := genDocs(t, func() { - InlineRefs() - API("Test", func() {}) - var SharedType = Type("SharedType", func() { - Field(1, "A", String) - Required("A") - }) - Service("S1", func() { - Method("M1", func() { - Payload(SharedType) - HTTP(func() { GET("/") }) - GRPC(func() {}) - }) - }) - Service("S2", func() { - Method("M2", func() { - Payload(SharedType) - HTTP(func() { GET("/s2") }) - GRPC(func() {}) - }) - }) - }) - - // Check S1 - s1 := docsMap["services"].(map[string]any)["S1"].(map[string]any) - m1 := s1["methods"].(map[string]any)["M1"].(map[string]any) - pt1 := m1["payload"].(map[string]any)["type"].(map[string]any) - if _, hasRef := pt1["$ref"]; hasRef { - t.Fatalf("S1: expected inlined payload schema for shared type, found $ref: %#v", pt1) - } - - // Check S2 - s2 := docsMap["services"].(map[string]any)["S2"].(map[string]any) - m2 := s2["methods"].(map[string]any)["M2"].(map[string]any) - pt2 := m2["payload"].(map[string]any)["type"].(map[string]any) - if _, hasRef := pt2["$ref"]; hasRef { - t.Fatalf("S2: expected inlined payload schema for shared type, found $ref: %#v", pt2) - } -} diff --git a/docs/genstate.go b/docs/genstate.go new file mode 100644 index 000000000..5eb4f803a --- /dev/null +++ b/docs/genstate.go @@ -0,0 +1,48 @@ +package docs + +import ( + "goa.design/goa/v3/codegen" + "goa.design/goa/v3/expr" + "goa.design/goa/v3/http/codegen/openapi" +) + +// genState stores per-generation state so we can avoid mutating +// global DSL structures while still ensuring uniqueness for schema names. +type genState struct { + nameScope *codegen.NameScope + assignedNames map[*expr.UserTypeExpr]string +} + +// newGenState constructs a fresh generator state for one Generate run. +func newGenState() *genState { + return &genState{ + nameScope: codegen.NewNameScope(), + assignedNames: make(map[*expr.UserTypeExpr]string), + } +} + +// uniqueNameFor returns a stable, unique name for the given user type within +// this generator run. It never mutates the passed user type. +func (s *genState) uniqueNameFor(ut *expr.UserTypeExpr) string { + if n, ok := s.assignedNames[ut]; ok { + return n + } + n := s.nameScope.Unique(ut.TypeName) + s.assignedNames[ut] = n + return n +} + +// schemaForAttribute computes the OpenAPI schema for the given attribute using +// a temporary, per-call unique name for user types. The original AST is restored +// before returning. +func schemaForAttribute(api *expr.APIExpr, att *expr.AttributeExpr, state *genState) *openapi.Schema { + if att == nil || att.Type == nil { + return nil + } + if ut, ok := att.Type.(*expr.UserTypeExpr); ok { + orig := ut.TypeName + ut.TypeName = state.uniqueNameFor(ut) + defer func() { ut.TypeName = orig }() + } + return openapi.AttributeTypeSchema(api, att) +} diff --git a/docs/inline_refs.go b/docs/inline_refs.go new file mode 100644 index 000000000..81696625d --- /dev/null +++ b/docs/inline_refs.go @@ -0,0 +1,104 @@ +package docs + +import ( + "strings" + + openapi "goa.design/goa/v3/http/codegen/openapi" +) + +// inlineAllServiceSchemas walks all service method schemas and inlines any +// $ref occurrences that point to local definitions. +func inlineAllServiceSchemas(d *data, defs map[string]*openapi.Schema) { + if d == nil || len(d.Services) == 0 { + return + } + stack := make(map[string]bool) + for _, svc := range d.Services { + if svc == nil || len(svc.Methods) == 0 { + continue + } + for _, m := range svc.Methods { + if m == nil { + continue + } + if m.Payload != nil && m.Payload.Type != nil { + inlineRefs(m.Payload.Type, defs, stack) + } + if m.StreamingPayload != nil && m.StreamingPayload.Type != nil { + inlineRefs(m.StreamingPayload.Type, defs, stack) + } + if m.Result != nil && m.Result.Type != nil { + inlineRefs(m.Result.Type, defs, stack) + } + if m.StreamingResult != nil && m.StreamingResult.Type != nil { + inlineRefs(m.StreamingResult.Type, defs, stack) + } + if len(m.Errors) > 0 { + for _, e := range m.Errors { + if e != nil && e.Type != nil { + inlineRefs(e.Type, defs, stack) + } + } + } + } + } +} + +// inlineRefs replaces local definition references with their fully expanded +// schema copy. Cycles are broken by retaining a single $ref at the cycle edge. +func inlineRefs(s *openapi.Schema, defs map[string]*openapi.Schema, stack map[string]bool) { + if s == nil { + return + } + + if s.Ref != "" { + const prefix = "#/definitions/" + if !strings.HasPrefix(s.Ref, prefix) { + // Unexpected external ref; leave intact. + return + } + name := strings.TrimPrefix(s.Ref, prefix) + if name == "" { + return + } + if stack[name] { + // Cycle detected; keep the $ref here to break infinite expansion. + return + } + def, ok := defs[name] + if !ok || def == nil { + // Defensive: unresolved ref, leave as-is. + return + } + stack[name] = true + copy := dupSchema(def) + inlineRefs(copy, defs, stack) + *s = *copy + s.Ref = "" + delete(stack, name) + return + } + + // Recurse into container positions. + if len(s.Properties) > 0 { + for _, p := range s.Properties { + inlineRefs(p, defs, stack) + } + } + if s.Items != nil { + inlineRefs(s.Items, defs, stack) + } + if ap, ok := s.AdditionalProperties.(*openapi.Schema); ok && ap != nil { + inlineRefs(ap, defs, stack) + } + if len(s.AnyOf) > 0 { + for _, a := range s.AnyOf { + inlineRefs(a, defs, stack) + } + } + if len(s.Definitions) > 0 { + for _, d := range s.Definitions { + inlineRefs(d, defs, stack) + } + } +} diff --git a/docs/testdata/shared/types.go b/docs/testdata/shared/types.go new file mode 100644 index 000000000..e6e9ddb14 --- /dev/null +++ b/docs/testdata/shared/types.go @@ -0,0 +1,8 @@ +package shared + +import . "goa.design/goa/v3/dsl" + +var CrossPackageType = Type("CrossPackageType", func() { + Field(1, "C", String) + Required("C") +}) diff --git a/docs/testdata/user-payload-no-return.json b/docs/testdata/user-payload-no-return.json index cc6194430..1845377f0 100644 --- a/docs/testdata/user-payload-no-return.json +++ b/docs/testdata/user-payload-no-return.json @@ -1 +1 @@ -{"api":{"name":"Test API","version":"0.0.1","servers":{"Test API":{"name":"Test API","description":"Default server for Test API","services":["Service"],"hosts":{"localhost":{"name":"localhost","server":"Test API","uris":["http://localhost:80","grpc://localhost:8080"]}}}}},"services":{"Service":{"name":"Service","methods":{"Method":{"name":"Method","payload":{"type":{"$ref":"#/definitions/User"},"example":{"att1":"Exercitationem suscipit deleniti.","att2":7517419439717785447}}}}}},"definitions":{"User":{"title":"User","type":"object","properties":{"att1":{"type":"string","example":"In voluptatem consectetur."},"att2":{"type":"integer","example":443436312039258672,"format":"int64"}},"example":{"att1":"Accusamus saepe et sit.","att2":8511135955551101225}}}} \ No newline at end of file +{"api":{"name":"Test API","version":"0.0.1","servers":{"Test API":{"name":"Test API","description":"Default server for Test API","services":["Service"],"hosts":{"localhost":{"name":"localhost","server":"Test API","uris":["http://localhost:80","grpc://localhost:8080"]}}}}},"services":{"Service":{"name":"Service","methods":{"Method":{"name":"Method","payload":{"type":{"$ref":"#/definitions/User"},"example":{"att1":"Soluta veritatis odit minus voluptatum sunt commodi.","att2":2887366790483849171}}}}}},"definitions":{"User":{"title":"User","type":"object","properties":{"att1":{"type":"string","example":"In voluptatem consectetur."},"att2":{"type":"integer","example":443436312039258672,"format":"int64"}},"example":{"att1":"Accusamus saepe et sit.","att2":8511135955551101225}}}} \ No newline at end of file