Skip to content

Commit 5585390

Browse files
committed
fix(docs): ensure cross-package refs are inlined
The docs plugin was failing to inline type references for types defined in other design packages. This was because it processed each design root in isolation, resulting in an incomplete map of definitions when resolving refs. This commit refactors the generator to first build a global map of all type definitions from all design roots. This complete map is then used for the inlining process, ensuring that cross-package and cross-service references are correctly resolved. A new test case, , has been added to verify this behavior and prevent regressions.
1 parent 7a69dc1 commit 5585390

2 files changed

Lines changed: 160 additions & 13 deletions

File tree

docs/generate.go

Lines changed: 53 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,28 +30,59 @@ func init() { Register() }
3030

3131
// Generate produces the documentation JSON file.
3232
func Generate(_ string, roots []eval.Root, files []*codegen.File) ([]*codegen.File, error) {
33+
// First, build a complete map of all definitions from all roots.
34+
// This ensures that cross-package type references can be resolved.
35+
allDefs := make(map[string]*openapi.Schema)
3336
for _, root := range roots {
3437
if r, ok := root.(*expr.RootExpr); ok {
35-
files = append(files, docsFile(r))
38+
// Create a temporary, isolated context for each root to avoid global state pollution.
39+
prev := openapi.Definitions
40+
openapi.Definitions = make(map[string]*openapi.Schema)
41+
42+
for _, tpe := range r.Types {
43+
if ut, ok := tpe.(*expr.UserTypeExpr); ok {
44+
openapi.GenerateTypeDefinition(r.API, ut)
45+
}
46+
}
47+
for _, rt := range r.ResultTypes {
48+
openapi.GenerateResultTypeDefinition(r.API, rt, expr.DefaultView)
49+
}
50+
51+
// Merge the generated definitions into the global map.
52+
for n, s := range openapi.Definitions {
53+
if _, exists := allDefs[n]; !exists {
54+
allDefs[n] = dupSchema(s)
55+
}
56+
}
57+
58+
// Restore the original global definitions to maintain isolation.
59+
openapi.Definitions = prev
60+
}
61+
}
62+
63+
for _, root := range roots {
64+
if r, ok := root.(*expr.RootExpr); ok {
65+
files = append(files, docsFile(r, allDefs))
3666
}
3767
}
3868
return files, nil
3969
}
4070

41-
func docsFile(r *expr.RootExpr) *codegen.File {
42-
71+
func docsFile(r *expr.RootExpr, allDefs map[string]*openapi.Schema) *codegen.File {
4372
docs := &data{
4473
API: apiDocs(r.API),
4574
Services: servicesDocs(r),
4675
}
4776

4877
// Default behavior: use global OpenAPI definitions to preserve ordering and
4978
// compatibility with existing golden tests.
50-
defs := openapi.Definitions
79+
defs := allDefs
5180

5281
// If either option is enabled, build a local definition map for this root
5382
// and apply transforms/inlining as needed, isolating from global state.
54-
if plugexpr.Root.UseJSONTags || plugexpr.Root.InlineRefs {
83+
if plugexpr.Root.UseJSONTags {
84+
// Re-scope the definitions to only those present in the current root,
85+
// but use the globally-aware `allDefs` for lookups during transforms.
5586
local := make(map[string]*openapi.Schema)
5687
prev := openapi.Definitions
5788
openapi.Definitions = make(map[string]*openapi.Schema)
@@ -63,8 +94,10 @@ func docsFile(r *expr.RootExpr) *codegen.File {
6394
for _, rt := range r.ResultTypes {
6495
openapi.GenerateResultTypeDefinition(r.API, rt, expr.DefaultView)
6596
}
66-
for n, s := range openapi.Definitions {
67-
local[n] = dupSchema(s)
97+
for n := range openapi.Definitions {
98+
if def, ok := allDefs[n]; ok {
99+
local[n] = dupSchema(def)
100+
}
68101
}
69102
openapi.Definitions = prev
70103

@@ -92,31 +125,38 @@ func docsFile(r *expr.RootExpr) *codegen.File {
92125

93126
// Inline $refs if requested via DSL flag.
94127
if plugexpr.Root.InlineRefs {
128+
// When inlining, use the complete set of definitions from all roots
129+
// to ensure cross-package references can be resolved.
130+
inliningDefs := allDefs
131+
if plugexpr.Root.UseJSONTags {
132+
inliningDefs = transformDefinitionsWithJSONTagsHybrid(r, allDefs, nil)
133+
}
134+
95135
// Inline inside service payloads/results/errors.
96136
for _, svc := range docs.Services {
97137
for _, m := range svc.Methods {
98138
if m.Payload != nil && m.Payload.Type != nil {
99-
inlineRefsInSchema(m.Payload.Type, defs, make(map[string]bool))
139+
inlineRefsInSchema(m.Payload.Type, inliningDefs, make(map[string]bool))
100140
}
101141
if m.StreamingPayload != nil && m.StreamingPayload.Type != nil {
102-
inlineRefsInSchema(m.StreamingPayload.Type, defs, make(map[string]bool))
142+
inlineRefsInSchema(m.StreamingPayload.Type, inliningDefs, make(map[string]bool))
103143
}
104144
if m.Result != nil && m.Result.Type != nil {
105-
inlineRefsInSchema(m.Result.Type, defs, make(map[string]bool))
145+
inlineRefsInSchema(m.Result.Type, inliningDefs, make(map[string]bool))
106146
}
107147
if m.StreamingResult != nil && m.StreamingResult.Type != nil {
108-
inlineRefsInSchema(m.StreamingResult.Type, defs, make(map[string]bool))
148+
inlineRefsInSchema(m.StreamingResult.Type, inliningDefs, make(map[string]bool))
109149
}
110150
for _, e := range m.Errors {
111151
if e != nil && e.Type != nil {
112-
inlineRefsInSchema(e.Type, defs, make(map[string]bool))
152+
inlineRefsInSchema(e.Type, inliningDefs, make(map[string]bool))
113153
}
114154
}
115155
}
116156
}
117157
// Inline inside definitions themselves (properties that refer to other defs).
118158
for _, def := range defs {
119-
inlineRefsInSchema(def, defs, make(map[string]bool))
159+
inlineRefsInSchema(def, inliningDefs, make(map[string]bool))
120160
}
121161
}
122162

docs/generate_test.go

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,3 +291,110 @@ func TestJSONTagsAndInlineRefs_Complex(t *testing.T) {
291291
t.Fatalf("expected required array in result type, got: %#v", rt)
292292
}
293293
}
294+
295+
func TestInlineRefs_MethodRootPayloadResult(t *testing.T) {
296+
t.Cleanup(func() { plugexpr.Root.UseJSONTags = false; plugexpr.Root.InlineRefs = false })
297+
298+
docsMap := genDocs(t, func() {
299+
InlineRefs()
300+
API("Test", func() {})
301+
var PayloadType = Type("PayloadType", func() {
302+
Field(1, "Foo", String)
303+
Required("Foo")
304+
})
305+
var ResultType = Type("ResultType", func() {
306+
Field(1, "Bar", String)
307+
Required("Bar")
308+
})
309+
Service("S", func() {
310+
Method("M1", func() {
311+
Payload(PayloadType)
312+
Result(ResultType)
313+
HTTP(func() { GET("/") })
314+
GRPC(func() {})
315+
})
316+
})
317+
})
318+
319+
svc := docsMap["services"].(map[string]any)["S"].(map[string]any)
320+
methods := svc["methods"].(map[string]any)
321+
m1 := methods["M1"].(map[string]any)
322+
pt := m1["payload"].(map[string]any)["type"].(map[string]any)
323+
if _, hasRef := pt["$ref"]; hasRef {
324+
t.Fatalf("expected inlined payload schema at method root, found $ref: %#v", pt)
325+
}
326+
rt := m1["result"].(map[string]any)["type"].(map[string]any)
327+
if _, hasRef := rt["$ref"]; hasRef {
328+
t.Fatalf("expected inlined result schema at method root, found $ref: %#v", rt)
329+
}
330+
}
331+
332+
func TestInlineRefs_CrossPackage(t *testing.T) {
333+
t.Cleanup(func() { plugexpr.Root.UseJSONTags = false; plugexpr.Root.InlineRefs = false })
334+
docsMap := genDocs(t, func() {
335+
InlineRefs()
336+
API("Test", func() {})
337+
// Service "S1" uses a type from a different package.
338+
Service("S1", func() {
339+
Method("M1", func() {
340+
Payload(testdata.CrossPackageType)
341+
HTTP(func() { GET("/") })
342+
GRPC(func() {})
343+
})
344+
})
345+
})
346+
347+
svc := docsMap["services"].(map[string]any)["S1"].(map[string]any)
348+
methods := svc["methods"].(map[string]any)
349+
m1 := methods["M1"].(map[string]any)
350+
pt := m1["payload"].(map[string]any)["type"].(map[string]any)
351+
if _, hasRef := pt["$ref"]; hasRef {
352+
t.Fatalf("expected inlined payload schema for cross-package type, found $ref: %#v", pt)
353+
}
354+
assert.Equal(t, "object", pt["type"])
355+
props := pt["properties"].(map[string]any)
356+
_, hasA := props["A"]
357+
assert.True(t, hasA)
358+
}
359+
360+
func TestInlineRefs_CrossService(t *testing.T) {
361+
t.Cleanup(func() { plugexpr.Root.UseJSONTags = false; plugexpr.Root.InlineRefs = false })
362+
docsMap := genDocs(t, func() {
363+
InlineRefs()
364+
API("Test", func() {})
365+
var SharedType = Type("SharedType", func() {
366+
Field(1, "A", String)
367+
Required("A")
368+
})
369+
Service("S1", func() {
370+
Method("M1", func() {
371+
Payload(SharedType)
372+
HTTP(func() { GET("/") })
373+
GRPC(func() {})
374+
})
375+
})
376+
Service("S2", func() {
377+
Method("M2", func() {
378+
Payload(SharedType)
379+
HTTP(func() { GET("/s2") })
380+
GRPC(func() {})
381+
})
382+
})
383+
})
384+
385+
// Check S1
386+
s1 := docsMap["services"].(map[string]any)["S1"].(map[string]any)
387+
m1 := s1["methods"].(map[string]any)["M1"].(map[string]any)
388+
pt1 := m1["payload"].(map[string]any)["type"].(map[string]any)
389+
if _, hasRef := pt1["$ref"]; hasRef {
390+
t.Fatalf("S1: expected inlined payload schema for shared type, found $ref: %#v", pt1)
391+
}
392+
393+
// Check S2
394+
s2 := docsMap["services"].(map[string]any)["S2"].(map[string]any)
395+
m2 := s2["methods"].(map[string]any)["M2"].(map[string]any)
396+
pt2 := m2["payload"].(map[string]any)["type"].(map[string]any)
397+
if _, hasRef := pt2["$ref"]; hasRef {
398+
t.Fatalf("S2: expected inlined payload schema for shared type, found $ref: %#v", pt2)
399+
}
400+
}

0 commit comments

Comments
 (0)