Skip to content

Commit 746e4c1

Browse files
committed
Fix example indexing, clean up rendering, add coverage.
1 parent 5f74b03 commit 746e4c1

File tree

11 files changed

+362
-50
lines changed

11 files changed

+362
-50
lines changed

bundler/bundler.go

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,28 @@ import (
2828
// ErrInvalidModel is returned when the model is not usable.
2929
var ErrInvalidModel = errors.New("invalid model")
3030

31+
type invalidModelBuildError struct {
32+
cause error
33+
}
34+
35+
func (e *invalidModelBuildError) Error() string {
36+
if e == nil || e.cause == nil {
37+
return ErrInvalidModel.Error()
38+
}
39+
return e.cause.Error()
40+
}
41+
42+
func (e *invalidModelBuildError) Unwrap() error {
43+
if e == nil {
44+
return nil
45+
}
46+
return e.cause
47+
}
48+
49+
func (e *invalidModelBuildError) Is(target error) bool {
50+
return target == ErrInvalidModel
51+
}
52+
3153
// buildV3ModelFromBytes is a helper that parses bytes and builds a v3 model.
3254
// Returns the model and any build errors. The model may be non-nil even when err is non-nil
3355
// (e.g., circular reference warnings), allowing bundling to proceed with warnings.
@@ -39,7 +61,7 @@ func buildV3ModelFromBytes(bytes []byte, configuration *datamodel.DocumentConfig
3961

4062
v3Doc, buildErr := doc.BuildV3Model()
4163
if v3Doc == nil {
42-
return nil, errors.Join(ErrInvalidModel, buildErr)
64+
return nil, &invalidModelBuildError{cause: buildErr}
4365
}
4466
// Return both model and error - caller decides how to handle warnings/errors
4567
return &v3Doc.Model, buildErr
@@ -76,7 +98,7 @@ func BundleBytesComposed(bytes []byte, configuration *datamodel.DocumentConfigur
7698

7799
v3Doc, err := doc.BuildV3Model()
78100
if err != nil {
79-
return nil, errors.Join(ErrInvalidModel, err)
101+
return nil, &invalidModelBuildError{cause: err}
80102
}
81103

82104
bundledBytes, e := compose(&v3Doc.Model, compositionConfig)
@@ -93,7 +115,7 @@ func BundleBytesComposedWithOrigins(bytes []byte, configuration *datamodel.Docum
93115

94116
v3Doc, err := doc.BuildV3Model()
95117
if err != nil {
96-
return nil, errors.Join(ErrInvalidModel, err)
118+
return nil, &invalidModelBuildError{cause: err}
97119
}
98120

99121
result, e := composeWithOrigins(&v3Doc.Model, compositionConfig)

bundler/bundler_test.go

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -416,12 +416,10 @@ components:
416416
_, e := BundleBytes(digi, config)
417417
require.Error(t, e)
418418
unwrap := utils.UnwrapErrors(e)
419-
require.Len(t, unwrap, 2)
419+
require.Len(t, unwrap, 1)
420420
assert.ErrorIs(t, unwrap[0], ErrInvalidModel)
421-
unwrapNext := utils.UnwrapErrors(unwrap[1])
422-
require.Len(t, unwrapNext, 2)
423-
assert.Equal(t, "component `bork` does not exist in the specification", unwrapNext[0].Error())
424-
assert.Equal(t, "cannot resolve reference `bork`, it's missing: $.bork [5:7]", unwrapNext[1].Error())
421+
assert.Equal(t, "component `bork` does not exist in the specification\ncannot resolve reference `bork`, it's missing: $.bork [5:7]", unwrap[0].Error())
422+
assert.NotContains(t, unwrap[0].Error(), "invalid model")
425423

426424
logEntries := strings.Split(byteBuf.String(), "\n")
427425
if len(logEntries) == 1 && logEntries[0] == "" {
@@ -1799,6 +1797,20 @@ paths: {}`)
17991797
assert.Contains(t, err.Error(), "different version")
18001798
}
18011799

1800+
func TestBundleBytesComposedWithOrigins_InvalidModel(t *testing.T) {
1801+
swagger2Spec := []byte(`swagger: "2.0"
1802+
info:
1803+
title: Test API
1804+
version: 1.0.0
1805+
paths: {}`)
1806+
1807+
_, err := BundleBytesComposedWithOrigins(swagger2Spec, nil, nil)
1808+
require.Error(t, err)
1809+
assert.ErrorIs(t, err, ErrInvalidModel)
1810+
assert.Contains(t, err.Error(), "different version")
1811+
assert.NotContains(t, err.Error(), "invalid model")
1812+
}
1813+
18021814
// TestBundleBytesWithConfig_BackwardCompatibility tests that existing behavior is preserved
18031815
// when ResolveDiscriminatorExternalRefs is not enabled.
18041816
func TestBundleBytesWithConfig_BackwardCompatibility(t *testing.T) {

index/extract_refs.go

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,43 @@ func isArrayOfSchemaContainingNode(v string) bool {
3838
// keyword (sample data, not schema). A segment named "example" or "examples" that is preceded
3939
// by "properties" or "patternProperties" is a schema property name, not an OpenAPI keyword.
4040
func underOpenAPIExamplePath(seenPath []string) bool {
41-
for i, p := range seenPath {
42-
if p == "example" || p == "examples" {
43-
if i == 0 || (seenPath[i-1] != "properties" && seenPath[i-1] != "patternProperties") {
44-
return true
41+
for i := range seenPath {
42+
if isOpenAPIExampleKeywordSegment(seenPath, i) {
43+
return true
44+
}
45+
}
46+
return false
47+
}
48+
49+
func isOpenAPIExampleKeywordSegment(seenPath []string, idx int) bool {
50+
if idx < 0 || idx >= len(seenPath) {
51+
return false
52+
}
53+
switch seenPath[idx] {
54+
case "example", "examples":
55+
return idx == 0 || (seenPath[idx-1] != "properties" && seenPath[idx-1] != "patternProperties")
56+
default:
57+
return false
58+
}
59+
}
60+
61+
// underOpenAPIExamplePayloadPath reports whether seenPath points to raw example payload content,
62+
// not the Example Object itself. This lets the walker keep traversing legitimate Example Objects
63+
// for $ref values while still skipping sample data under example/value/dataValue.
64+
func underOpenAPIExamplePayloadPath(seenPath []string) bool {
65+
for i := range seenPath {
66+
if !isOpenAPIExampleKeywordSegment(seenPath, i) {
67+
continue
68+
}
69+
switch seenPath[i] {
70+
case "example":
71+
return true
72+
case "examples":
73+
if len(seenPath) > i+2 {
74+
switch seenPath[i+2] {
75+
case "value", "dataValue":
76+
return true
77+
}
4578
}
4679
}
4780
}

index/extract_refs_ref.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ func (index *SpecIndex) extractReferenceAt(
2727
if len(node.Content) <= keyIndex+1 {
2828
return nil
2929
}
30+
if underOpenAPIExamplePayloadPath(seenPath) {
31+
return nil
32+
}
3033

3134
isExtensionPath := false
3235
for _, spi := range seenPath {

index/extract_refs_test.go

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -737,6 +737,31 @@ func TestUnderOpenAPIExamplePath(t *testing.T) {
737737
}
738738
}
739739

740+
func TestUnderOpenAPIExamplePayloadPath(t *testing.T) {
741+
tests := []struct {
742+
name string
743+
path []string
744+
want bool
745+
}{
746+
{"empty", nil, false},
747+
{"example_payload", []string{"paths", "get", "responses", "200", "content", "application/json", "schema", "example"}, true},
748+
{"nested_under_example_payload", []string{"components", "schemas", "Foo", "example", "nested"}, true},
749+
{"examples_collection", []string{"components", "examples"}, false},
750+
{"example_object_entry", []string{"components", "examples", "ReusableExample"}, false},
751+
{"examples_value_payload", []string{"content", "application/json", "examples", "sample", "value"}, true},
752+
{"examples_value_nested_payload", []string{"content", "application/json", "examples", "sample", "value", "nested"}, true},
753+
{"examples_data_value_payload", []string{"components", "examples", "sample", "dataValue"}, true},
754+
{"property_named_example", []string{"components", "schemas", "Foo", "properties", "example"}, false},
755+
{"property_named_examples_value", []string{"components", "schemas", "Foo", "properties", "examples", "value"}, false},
756+
{"real_example_after_property_example", []string{"components", "schemas", "Foo", "properties", "example", "example"}, true},
757+
}
758+
for _, tt := range tests {
759+
t.Run(tt.name, func(t *testing.T) {
760+
assert.Equal(t, tt.want, underOpenAPIExamplePayloadPath(tt.path))
761+
})
762+
}
763+
}
764+
740765
func TestExtractRefs_InlineSchemaHelpers(t *testing.T) {
741766
idx := NewTestSpecIndex().Load().(*SpecIndex)
742767
idx.specAbsolutePath = "test.yaml"
@@ -961,4 +986,87 @@ func TestExtractRefs_WalkHelpers(t *testing.T) {
961986
assert.False(t, shouldSkipMapSchemaCollection([]string{"properties", "example"}))
962987
assert.False(t, shouldSkipMapSchemaCollection([]string{"patternProperties", "example"}))
963988
assert.True(t, shouldSkipMapSchemaCollection([]string{"x-test"}))
989+
990+
var noAppendNode yaml.Node
991+
_ = yaml.Unmarshal([]byte("summary: hello\nvalue: world"), &noAppendNode)
992+
state.seenPath = []string{"components", "examples", "sample"}
993+
state.lastAppended = false
994+
idx.unwindExtractRefsPath(noAppendNode.Content[0], &state, 1)
995+
assert.Equal(t, []string{"components", "examples", "sample"}, state.seenPath)
996+
997+
var appendNode yaml.Node
998+
_ = yaml.Unmarshal([]byte("value: hello\nnext: world"), &appendNode)
999+
state.lastAppended = true
1000+
idx.unwindExtractRefsPath(appendNode.Content[0], &state, 1)
1001+
assert.Equal(t, []string{"components", "examples"}, state.seenPath)
1002+
assert.False(t, state.lastAppended)
1003+
}
1004+
1005+
func TestExtractReferenceAt_IgnoresRefsInsideExamplePayloads(t *testing.T) {
1006+
idx := NewTestSpecIndex().Load().(*SpecIndex)
1007+
idx.specAbsolutePath = "test.yaml"
1008+
1009+
var refNode yaml.Node
1010+
_ = yaml.Unmarshal([]byte(`$ref: '#/components/schemas/Pet'`), &refNode)
1011+
1012+
ref := idx.extractReferenceAt(refNode.Content[0], nil, 0, []string{"components", "examples", "sample", "value"}, nil, false, "")
1013+
assert.Nil(t, ref)
1014+
assert.Empty(t, idx.GetAllReferences())
1015+
assert.Empty(t, idx.GetAllSequencedReferences())
1016+
}
1017+
1018+
func TestSpecIndex_ExtractRefs_ExampleObjectRefsIndexedButPayloadRefsIgnored(t *testing.T) {
1019+
spec := `openapi: 3.2.0
1020+
info:
1021+
title: Example refs
1022+
version: 1.0.0
1023+
paths:
1024+
/widgets:
1025+
get:
1026+
responses:
1027+
"200":
1028+
description: ok
1029+
content:
1030+
application/json:
1031+
examples:
1032+
responseRef:
1033+
$ref: '#/components/examples/ReusableExample'
1034+
inlinePayload:
1035+
summary: payload example
1036+
value:
1037+
nested:
1038+
$ref: '#/components/schemas/ShouldNotIndex'
1039+
components:
1040+
examples:
1041+
ReusableExample:
1042+
$ref: '#/components/examples/LeafExample'
1043+
LeafExample:
1044+
summary: reusable
1045+
value:
1046+
ok: true
1047+
DataValueExample:
1048+
dataValue:
1049+
nested:
1050+
$ref: '#/components/schemas/ShouldNotIndexData'
1051+
schemas:
1052+
ShouldNotIndex:
1053+
type: object
1054+
ShouldNotIndexData:
1055+
type: object
1056+
`
1057+
1058+
var rootNode yaml.Node
1059+
_ = yaml.Unmarshal([]byte(spec), &rootNode)
1060+
1061+
idx := NewSpecIndexWithConfig(&rootNode, CreateOpenAPIIndexConfig())
1062+
1063+
rawRefs := make(map[string]bool)
1064+
for _, ref := range idx.GetAllReferences() {
1065+
rawRefs[ref.RawRef] = true
1066+
}
1067+
1068+
assert.True(t, rawRefs["#/components/examples/ReusableExample"])
1069+
assert.True(t, rawRefs["#/components/examples/LeafExample"])
1070+
assert.False(t, rawRefs["#/components/schemas/ShouldNotIndex"])
1071+
assert.False(t, rawRefs["#/components/schemas/ShouldNotIndexData"])
9641072
}

index/extract_refs_walk.go

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ type extractRefsState struct {
1616
scope *SchemaIdScope
1717
parentBaseURI string
1818
seenPath []string
19+
lastAppended bool
1920
level int
2021
poly bool
2122
polyName string
@@ -67,10 +68,6 @@ func (index *SpecIndex) walkExtractRefs(node, parent *yaml.Node, state *extractR
6768

6869
var found []*Reference
6970
for i, n := range node.Content {
70-
if utils.IsNodeMap(n) || utils.IsNodeArray(n) {
71-
found = append(found, index.walkChildExtractRefs(n, node, state)...)
72-
}
73-
7471
// In YAML mapping nodes, Content alternates key-value: even indices (0, 2, 4...)
7572
// are keys, odd indices (1, 3, 5...) are values.
7673
if i%2 == 0 {
@@ -79,13 +76,20 @@ func (index *SpecIndex) walkExtractRefs(node, parent *yaml.Node, state *extractR
7976
}
8077
}
8178

79+
if utils.IsNodeMap(n) || utils.IsNodeArray(n) {
80+
found = append(found, index.walkChildExtractRefs(n, node, state)...)
81+
}
82+
8283
index.unwindExtractRefsPath(node, state, i)
8384
}
8485

8586
return found
8687
}
8788

8889
func (index *SpecIndex) walkChildExtractRefs(node, parent *yaml.Node, state *extractRefsState) []*Reference {
90+
if underOpenAPIExamplePayloadPath(state.seenPath) {
91+
return nil
92+
}
8993
state.level++
9094
if isPoly, _ := index.checkPolymorphicNode(state.prev); isPoly {
9195
state.poly = true
@@ -103,6 +107,7 @@ func (index *SpecIndex) handleExtractRefsKey(
103107
found *[]*Reference,
104108
) bool {
105109
keyNode := node.Content[keyIndex]
110+
state.lastAppended = false
106111
if keyNode == nil {
107112
return false
108113
}
@@ -134,6 +139,7 @@ func (index *SpecIndex) handleExtractRefsKey(
134139

135140
if keyNode.Value != "$ref" && keyNode.Value != "$id" && keyNode.Value != "" {
136141
action := index.extractNodeMetadata(node, parent, state.seenPath, keyIndex)
142+
state.lastAppended = action.appendSegment
137143
if action.appendSegment {
138144
state.seenPath = append(state.seenPath, strings.ReplaceAll(keyNode.Value, "/", "~1"))
139145
state.prev = keyNode.Value
@@ -161,7 +167,9 @@ func (index *SpecIndex) unwindExtractRefsPath(node *yaml.Node, state *extractRef
161167
return
162168
}
163169
next := node.Content[currentIndex+1]
164-
if currentIndex%2 != 0 && next != nil && !utils.IsNodeArray(next) && !utils.IsNodeMap(next) && len(state.seenPath) > 0 {
170+
if currentIndex%2 != 0 && state.lastAppended &&
171+
next != nil && !utils.IsNodeArray(next) && !utils.IsNodeMap(next) && len(state.seenPath) > 0 {
165172
state.seenPath = state.seenPath[:len(state.seenPath)-1]
173+
state.lastAppended = false
166174
}
167175
}

index/schema_id_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1532,6 +1532,56 @@ components:
15321532
assert.True(t, found, "Should find invalid $id error")
15331533
}
15341534

1535+
// Test that $id values embedded inside OpenAPI example payloads are ignored.
1536+
func TestSchemaId_IgnoresIdsInsideExamplePayloads(t *testing.T) {
1537+
spec := `openapi: "3.1.0"
1538+
info:
1539+
title: Test API
1540+
version: 1.0.0
1541+
components:
1542+
schemas:
1543+
Widget:
1544+
type: object
1545+
properties:
1546+
outputSchema:
1547+
type: object
1548+
example:
1549+
definitions: {}
1550+
properties:
1551+
id:
1552+
$id: '#widget/example/id'
1553+
type: string
1554+
nested:
1555+
$id: '#widget/example/nested'
1556+
type: string
1557+
examples:
1558+
sample:
1559+
value:
1560+
child:
1561+
$id: '#widget/examples/child'
1562+
type: integer
1563+
`
1564+
1565+
var rootNode yaml.Node
1566+
err := yaml.Unmarshal([]byte(spec), &rootNode)
1567+
assert.NoError(t, err)
1568+
1569+
config := CreateClosedAPIIndexConfig()
1570+
config.SpecAbsolutePath = "https://example.com/openapi.yaml"
1571+
index := NewSpecIndexWithConfig(&rootNode, config)
1572+
assert.NotNil(t, index)
1573+
1574+
allIds := index.GetAllSchemaIds()
1575+
assert.Len(t, allIds, 0)
1576+
1577+
errors := index.GetReferenceIndexErrors()
1578+
for _, e := range errors {
1579+
if e != nil {
1580+
assert.NotContains(t, e.Error(), "invalid $id")
1581+
}
1582+
}
1583+
}
1584+
15351585
// Test fragment navigation with DocumentNode wrapper
15361586
func TestNavigateToFragment_DocumentNode(t *testing.T) {
15371587
yamlContent := `type: object

0 commit comments

Comments
 (0)